-
Notifications
You must be signed in to change notification settings - Fork 642
Environment variables in cog.yaml #2274
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
011dead
wip support for build+runtime environment in cog.yaml
michaeldwan 96be629
support fast boot, cleaning
michaeldwan 4cf9a8a
remove unused code
michaeldwan ccf23df
add env var integration test
michaeldwan 8f13e21
fix config for integration test fixture
michaeldwan ae28896
cog_binary fixture for configuring binary used in integration tests
michaeldwan e59d056
remove unused Environment field on Build
michaeldwan fff173f
Merge branch 'main' into md/env-vars
michaeldwan 7973306
updated denylist, more validation, more tests
michaeldwan f11522b
Merge branch 'md/env-vars' of github.com:replicate/cog into md/env-vars
michaeldwan c40c78d
Merge branch 'main' into md/env-vars
michaeldwan e64e420
tidy mod files after merge oddness
michaeldwan da662f4
omit empty Environment config when writing yaml
michaeldwan 2fec620
typo
michaeldwan 2d75642
correct environment entry description
michaeldwan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| package config | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "strings" | ||
| ) | ||
|
|
||
| // EnvironmentVariableDenyList is a list of environment variable patterns that are | ||
| // used internally during build or runtime and thus not allowed to be set by the user. | ||
| // There are ways around this restriction, but it's likely to cause unexpected behavior | ||
| // and hard to debug issues. So on Cog's predict-build-push happy path, we don't allow | ||
| // these to be set. | ||
| // This list may change at any time. For more context, see: | ||
| // https://github.com/replicate/cog/pull/2274/#issuecomment-2831823185 | ||
| var EnvironmentVariableDenyList = []string{ | ||
| // paths | ||
| "PATH", | ||
| "LD_LIBRARY_PATH", | ||
| "PYTHONPATH", | ||
| "VIRTUAL_ENV", | ||
| "PYTHONUNBUFFERED", | ||
| // Replicate | ||
| "R8_*", | ||
| "REPLICATE_*", | ||
| // Nvidia | ||
| "LIBRARY_PATH", | ||
| "CUDA_*", | ||
| "NVIDIA_*", | ||
| "NV_*", | ||
| // pget | ||
| "PGET_*", | ||
| "HF_ENDPOINT", | ||
| "HF_HUB_ENABLE_HF_TRANSFER", | ||
| // k8s | ||
| "KUBERNETES_*", | ||
| } | ||
|
|
||
| // validateEnvName checks if the given environment variable name is allowed. | ||
| // Returns an error if the name matches any of the restricted patterns. | ||
| func validateEnvName(name string) error { | ||
| for _, pattern := range EnvironmentVariableDenyList { | ||
| // Check for exact match | ||
| if pattern == name { | ||
| return fmt.Errorf("environment variable %q is not allowed", name) | ||
| } | ||
|
|
||
| // Check for wildcard pattern | ||
| if strings.HasSuffix(pattern, "*") { | ||
| if strings.HasPrefix(name, pattern[:len(pattern)-1]) { | ||
| return fmt.Errorf("environment variable %q is not allowed", name) | ||
| } | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // parseAndValidateEnvironment converts a slice of strings in the format of KEY=VALUE | ||
| // to a map[string]string. An error is returned if the format is incorrect or if either | ||
| // the variable name or value are invalid. | ||
| func parseAndValidateEnvironment(input []string) (map[string]string, error) { | ||
| env := map[string]string{} | ||
| for _, input := range input { | ||
| parts := strings.SplitN(input, "=", 2) | ||
| if len(parts) != 2 || parts[0] == "" { | ||
| return nil, fmt.Errorf("environment variable %q is not in the KEY=VALUE format", input) | ||
| } | ||
| if err := validateEnvName(parts[0]); err != nil { | ||
| return nil, err | ||
| } | ||
| if _, ok := env[parts[0]]; ok { | ||
| return nil, fmt.Errorf("environment variable %q is already defined", parts[0]) | ||
| } | ||
| env[parts[0]] = parts[1] | ||
| } | ||
| return env, nil | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,145 @@ | ||
| package config | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestEnvironmentConfig(t *testing.T) { | ||
| t.Run("ParsingValidInput", func(t *testing.T) { | ||
| cases := []struct { | ||
| Name string | ||
| Input []string | ||
| Expected map[string]string | ||
| }{ | ||
| { | ||
| Name: "ValidInput", | ||
| Input: []string{"NAME=VALUE"}, | ||
| Expected: map[string]string{"NAME": "VALUE"}, | ||
| }, | ||
| { | ||
| Name: "ValidInputWithSpaces", | ||
| Input: []string{"NAME=VALUE WITH SPACES"}, | ||
| Expected: map[string]string{"NAME": "VALUE WITH SPACES"}, | ||
| }, | ||
| { | ||
| Name: "ValidInputWithQuotes", | ||
| Input: []string{"NAME=\"VALUE WITH QUOTES\""}, | ||
| Expected: map[string]string{"NAME": `"VALUE WITH QUOTES"`}, | ||
| }, | ||
| { | ||
| Name: "DelimitedValue", | ||
| Input: []string{"NAME=VALUE1,VALUE2"}, | ||
| Expected: map[string]string{"NAME": "VALUE1,VALUE2"}, | ||
| }, | ||
| { | ||
| Name: "EmptyValue", | ||
| Input: []string{"NAME="}, | ||
| Expected: map[string]string{"NAME": ""}, | ||
| }, | ||
| { | ||
| Name: "EmptyValueWithSpaces", | ||
| Input: []string{"NAME= "}, | ||
| Expected: map[string]string{"NAME": " "}, | ||
| }, | ||
| { | ||
| Name: "LowerCaseName", | ||
| Input: []string{"name=VALUE"}, | ||
| Expected: map[string]string{"name": "VALUE"}, | ||
| }, | ||
| { | ||
| Name: "MixedCaseName", | ||
| Input: []string{"MiXeD_Case=VALUE"}, | ||
| Expected: map[string]string{"MiXeD_Case": "VALUE"}, | ||
| }, | ||
| { | ||
| Name: "EqualSignInValue", | ||
| Input: []string{"NAME=VALUE=EQUAL"}, | ||
| Expected: map[string]string{"NAME": "VALUE=EQUAL"}, | ||
| }, | ||
| { | ||
| Name: "EqualSignInValueWithSpaces", | ||
| Input: []string{"NAME=VALUE=EQUAL WITH SPACES"}, | ||
| Expected: map[string]string{"NAME": "VALUE=EQUAL WITH SPACES"}, | ||
| }, | ||
| { | ||
| Name: "MultiLineValue", | ||
| Input: []string{"NAME=VALUE1\nVALUE2"}, | ||
| Expected: map[string]string{"NAME": "VALUE1\nVALUE2"}, | ||
| }, | ||
| { | ||
| Name: "MultiplePairs", | ||
| Input: []string{"NAME1=VALUE1", "NAME2=VALUE2"}, | ||
| Expected: map[string]string{"NAME1": "VALUE1", "NAME2": "VALUE2"}, | ||
| }, | ||
| } | ||
|
|
||
| for _, c := range cases { | ||
| t.Run(c.Name, func(t *testing.T) { | ||
| parsed, err := parseAndValidateEnvironment(c.Input) | ||
| require.NoError(t, err) | ||
| require.Equal(t, c.Expected, parsed) | ||
| }) | ||
| } | ||
| }) | ||
|
|
||
| t.Run("ParsingInvalidInput", func(t *testing.T) { | ||
| cases := []struct { | ||
| Name string | ||
| Input []string | ||
| ExpectedErrorMessage string | ||
| }{ | ||
| { | ||
| Name: "NameWithoutValue", | ||
| Input: []string{"NAME"}, | ||
| ExpectedErrorMessage: `environment variable "NAME" is not in the KEY=VALUE format`, | ||
| }, | ||
| { | ||
| Name: "EmptyName", | ||
| Input: []string{"=VALUE"}, | ||
| ExpectedErrorMessage: `environment variable "=VALUE" is not in the KEY=VALUE format`, | ||
| }, | ||
| } | ||
|
|
||
| for _, c := range cases { | ||
| t.Run(c.Name, func(t *testing.T) { | ||
| _, err := parseAndValidateEnvironment(c.Input) | ||
| require.Error(t, err) | ||
| require.ErrorContains(t, err, c.ExpectedErrorMessage) | ||
| }) | ||
| } | ||
| }) | ||
|
|
||
| t.Run("EnforceDenyList", func(t *testing.T) { | ||
| for _, pattern := range EnvironmentVariableDenyList { | ||
| // test that exact matches are rejected | ||
| t.Run(fmt.Sprintf("Rejects %q", pattern), func(t *testing.T) { | ||
| input := fmt.Sprintf("%s=VALUE", pattern) | ||
| _, err := parseAndValidateEnvironment([]string{input}) | ||
| require.Error(t, err) | ||
| require.ErrorContains(t, err, fmt.Sprintf("environment variable %q is not allowed", pattern)) | ||
| }) | ||
|
|
||
| // test that prefix matches are rejected | ||
| if strings.HasSuffix(pattern, "*") { | ||
| t.Run(fmt.Sprintf("Rejects %q prefix", pattern), func(t *testing.T) { | ||
| name := strings.TrimSuffix(pattern, "*") + "SUFFIX" | ||
| input := fmt.Sprintf("%s=VALUE", name) | ||
| _, err := parseAndValidateEnvironment([]string{input}) | ||
| require.Error(t, err) | ||
| require.ErrorContains(t, err, fmt.Sprintf("environment variable %q is not allowed", name)) | ||
| }) | ||
| } | ||
| } | ||
| }) | ||
|
|
||
| t.Run("DuplicateNamesAreRejected", func(t *testing.T) { | ||
| input := []string{"NAME=VALUE", "NAME=VALUE2"} | ||
| _, err := parseAndValidateEnvironment(input) | ||
| require.Error(t, err) | ||
| require.ErrorContains(t, err, "environment variable \"NAME\" is already defined") | ||
| }) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| package dockerfile | ||
|
|
||
| import ( | ||
| "maps" | ||
| "slices" | ||
|
|
||
| "github.com/replicate/cog/pkg/config" | ||
| ) | ||
|
|
||
| func envLineFromConfig(c *config.Config) (string, error) { | ||
| vars := c.ParsedEnvironment() | ||
| if len(vars) == 0 { | ||
| return "", nil | ||
| } | ||
|
|
||
| out := "ENV" | ||
| for _, name := range slices.Sorted(maps.Keys(vars)) { | ||
| out = out + " " + name + "=" + vars[name] | ||
| } | ||
| out += "\n" | ||
|
|
||
| return out, nil | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Consider using a strings.Builder to construct the ENV line more efficiently for better readability and potential performance improvements.