From 011dead70d4ae4c62dbdaf2317f54cee1c5a1321 Mon Sep 17 00:00:00 2001 From: Michael Dwan Date: Tue, 15 Apr 2025 10:30:33 -0600 Subject: [PATCH 01/12] wip support for build+runtime environment in cog.yaml --- pkg/config/config.go | 2 + pkg/config/data/config_schema_v1.0.json | 36 ++++++++++++++ pkg/dockerfile/standard_generator.go | 47 +++++++++++++++++++ .../fixtures/env-project/cog.yaml | 5 ++ .../fixtures/env-project/predict.py | 7 +++ 5 files changed, 97 insertions(+) create mode 100644 test-integration/test_integration/fixtures/env-project/cog.yaml create mode 100644 test-integration/test_integration/fixtures/env-project/predict.py diff --git a/pkg/config/config.go b/pkg/config/config.go index e2e1342a82..ce3c039404 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -56,6 +56,7 @@ type Build struct { CuDNN string `json:"cudnn,omitempty" yaml:"cudnn"` Fast bool `json:"fast,omitempty" yaml:"fast"` PythonOverrides string `json:"python_overrides,omitempty" yaml:"python_overrides"` + Environment []string `json:"environment,omitempty" yaml:"environment"` pythonRequirementsContent []string } @@ -75,6 +76,7 @@ type Config struct { Predict string `json:"predict,omitempty" yaml:"predict"` Train string `json:"train,omitempty" yaml:"train"` Concurrency *Concurrency `json:"concurrency,omitempty" yaml:"concurrency"` + Environment []string `json:"environment,omitempty" yaml:"environment"` } func DefaultConfig() *Config { diff --git a/pkg/config/data/config_schema_v1.0.json b/pkg/config/data/config_schema_v1.0.json index a9a8b4e68c..dd29c54e3b 100644 --- a/pkg/config/data/config_schema_v1.0.json +++ b/pkg/config/data/config_schema_v1.0.json @@ -151,6 +151,24 @@ "type": "boolean", "description": "A flag to enable the experimental fast-push feature from a config level." }, + "environment": { + "$id": "#/properties/build/properties/environment", + "type": [ + "array", + "null" + ], + "description": "A list of environment variables to make available at build time, in the format `NAME=value`", + "additionalItems": true, + "items": { + "$id": "#/properties/build/properties/environment/items", + "anyOf": [ + { + "$id": "#/properties/build/properties/environment/items/anyOf/0", + "type": "string" + } + ] + } + }, "python_overrides": { "$id": "#/properties/build/properties/python_overrides", "type": "string", @@ -207,6 +225,24 @@ "type": "object", "additionalItems": true } + }, + "environment": { + "$id": "#/properties/environment", + "type": [ + "array", + "null" + ], + "description": "A list of environment variables to make available to the image at runtime, in the format `NAME=value`", + "additionalItems": true, + "items": { + "$id": "#/properties/environment/items", + "anyOf": [ + { + "$id": "#/properties/environment/items/anyOf/0", + "type": "string" + } + ] + } } }, "additionalProperties": false diff --git a/pkg/dockerfile/standard_generator.go b/pkg/dockerfile/standard_generator.go index b07a035896..440dd7e0fd 100644 --- a/pkg/dockerfile/standard_generator.go +++ b/pkg/dockerfile/standard_generator.go @@ -142,6 +142,10 @@ func (g *StandardGenerator) GenerateInitialSteps() (string, error) { if err != nil { return "", err } + envs, err := g.envVars() + if err != nil { + return "", err + } runCommands, err := g.runCommands() if err != nil { return "", err @@ -159,6 +163,7 @@ func (g *StandardGenerator) GenerateInitialSteps() (string, error) { steps := []string{ "#syntax=docker/dockerfile:1.4", "FROM " + baseImage, + envs, aptInstalls, installCog, pipInstalls, @@ -176,6 +181,7 @@ func (g *StandardGenerator) GenerateInitialSteps() (string, error) { "FROM " + baseImage, g.preamble(), g.installTini(), + envs, aptInstalls, installPython, pipInstalls, @@ -512,6 +518,22 @@ This is the offending line: %s`, command) return strings.Join(lines, "\n"), nil } +func (g *StandardGenerator) envVars() (string, error) { + if len(g.Config.Environment) == 0 { + return "", nil + } + + var steps []string + for _, input := range g.Config.Environment { + name, val, err := parseNameValueString(input, true) + if err != nil { + return "", err + } + steps = append(steps, fmt.Sprintf("ENV %s=%q", name, val)) + } + return joinStringsWithoutLineSpace(steps), nil +} + // writeTemp writes a temporary file that can be used as part of the build process // It returns the lines to add to Dockerfile to make it available and the filename it ends up as inside the container func (g *StandardGenerator) writeTemp(filename string, contents []byte) ([]string, string, error) { @@ -614,3 +636,28 @@ func stripPatchVersion(versionString string) (string, bool, error) { return strippedVersion, changed, nil } + +func parseNameValueString(input string, valFromHostEnv bool) (string, string, error) { + if input == "" { + return "", "", fmt.Errorf("empty input string") + } + + parts := strings.SplitN(input, "=", 2) + name := parts[0] + + if name == "" { + return "", "", fmt.Errorf("empty name in input") + } + + if len(parts) == 2 { + // NAME=VAL format + return name, parts[1], nil + } + + // NAME format + if valFromHostEnv { + return name, os.Getenv(name), nil + } + + return name, "", nil +} diff --git a/test-integration/test_integration/fixtures/env-project/cog.yaml b/test-integration/test_integration/fixtures/env-project/cog.yaml new file mode 100644 index 0000000000..272ecd6dc8 --- /dev/null +++ b/test-integration/test_integration/fixtures/env-project/cog.yaml @@ -0,0 +1,5 @@ +build: + python_version: "3.8" +predict: "predict.py:Predictor" +environment: + - NAME=wat diff --git a/test-integration/test_integration/fixtures/env-project/predict.py b/test-integration/test_integration/fixtures/env-project/predict.py new file mode 100644 index 0000000000..6029e472c4 --- /dev/null +++ b/test-integration/test_integration/fixtures/env-project/predict.py @@ -0,0 +1,7 @@ +from cog import BasePredictor +import os + + +class Predictor(BasePredictor): + def predict(self, name: str) -> str: + return f"ENV[{name}]={os.getenv(name)}" From 96be629d3fa900c908ec3abd81dad0b8618a4a7f Mon Sep 17 00:00:00 2001 From: Michael Dwan Date: Tue, 22 Apr 2025 21:27:51 -0600 Subject: [PATCH 02/12] support fast boot, cleaning --- pkg/config/config.go | 12 ++ pkg/config/config_test.go | 139 ++++++++++++++++++ pkg/config/data/config_schema_v1.0.json | 32 +--- pkg/dockerfile/env.go | 47 ++++++ pkg/dockerfile/fast_generator.go | 8 + pkg/dockerfile/standard_generator.go | 39 +---- .../fixtures/env-project/cog.yaml | 6 +- 7 files changed, 215 insertions(+), 68 deletions(-) create mode 100644 pkg/dockerfile/env.go diff --git a/pkg/config/config.go b/pkg/config/config.go index ce3c039404..60795485f0 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -579,3 +579,15 @@ func sliceContains(slice []string, s string) bool { } return false } + +func (c *Config) ParseEnvironment() (map[string]string, error) { + env := map[string]string{} + for _, input := range c.Environment { + parts := strings.SplitN(input, "=", 2) + if len(parts) != 2 { + return nil, fmt.Errorf("environment variable %q is not in the KEY=VALUE format", input) + } + env[parts[0]] = parts[1] + } + return env, nil +} diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 75f4c46a71..a6700eec71 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -2,11 +2,14 @@ package config import ( "encoding/json" + "fmt" "os" "path" + "strings" "testing" "github.com/hashicorp/go-version" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gopkg.in/yaml.v2" ) @@ -719,3 +722,139 @@ build: _, err := FromYAML([]byte(yamlString)) require.NoError(t, err) } + +func TestEnvironmentConfig(t *testing.T) { + t.Run("PatternValidation", func(t *testing.T) { + testCases := map[string]bool{ + "NAME=VALUE": true, + "NAME=VALUE1,VALUE2": true, + "random_case=yo": true, + "JUST_A_NAME": false, + "NO_VAL=": false, + } + + for input, isValid := range testCases { + yamlString := fmt.Sprintf("environment:\n- %s", input) + _, err := FromYAML([]byte(yamlString)) + fmt.Printf("err: %+v\n", err) + if isValid { + require.NoError(t, err, "Expected no error for input: %s", input) + } else { + require.Error(t, err, "Expected error for input: %s", input) + } + } + }) + + t.Run("Parsing", func(t *testing.T) { + + }) +} + +func TestEnvironmentPattern(t *testing.T) { + testCases := map[string]bool{ + "NAME=VALUE": true, + "NAME=VALUE1,VALUE2": true, + "random_case=yo": true, + "JUST_A_NAME": false, + "NO_VAL=": false, + } + + for input, isValid := range testCases { + t.Run(input, func(t *testing.T) { + yamlString := fmt.Sprintf("environment:\n- %s", input) + _, err := FromYAML([]byte(yamlString)) + fmt.Printf("err: %+v\n", err) + if isValid { + require.NoError(t, err, "Expected no error for input: %s", input) + } else { + require.Error(t, err, "Expected error for input: %s", input) + } + }) + } +} + +func TestEnvironmentParsing(t *testing.T) { + var testCases = []struct { + input string + parsed map[string]string + }{ + { + input: "NAME=VALUE", + parsed: map[string]string{"NAME": "VALUE"}, + }, + { + input: "NAME=VALUE1,VALUE2", + parsed: map[string]string{"NAME": "VALUE1,VALUE2"}, + }, + { + input: "random_case=yo", + parsed: map[string]string{"random_case": "yo"}, + }, + { + input: "MULTIPLE=EQUAL=SIGNS", + parsed: map[string]string{"MULTIPLE": "EQUAL=SIGNS"}, + }, + { + input: "EMPTY_VALUE=", + parsed: map[string]string{"EMPTY_VALUE": ""}, + }, + { + input: "WITH_SPACES=VALUE WITH SPACES", + parsed: map[string]string{"WITH_SPACES": "VALUE WITH SPACES"}, + }, + { + input: "WITH_QUOTES=\"VALUE WITH QUOTES\"", + parsed: map[string]string{"WITH_QUOTES": "\"VALUE WITH QUOTES\""}, + }, + { + input: "WITH_SPACES=VALUE WITH SPACES", + parsed: map[string]string{"WITH_SPACES": "VALUE WITH SPACES"}, + }, + { + input: "WITH_QUOTES=\"VALUE WITH QUOTES\"", + parsed: map[string]string{"WITH_QUOTES": "\"VALUE WITH QUOTES\""}, + }, + { + input: "EMPTY_VALUE=", + parsed: map[string]string{"EMPTY_VALUE": ""}, + }, + } + + for _, tc := range testCases { + t.Run(tc.input, func(t *testing.T) { + config := &Config{ + Environment: []string{tc.input}, + } + parsed, err := config.ParseEnvironment() + require.NoError(t, err) + assert.Equal(t, tc.parsed, parsed) + }) + } +} + +func TestEnvironmentParsingInvalid(t *testing.T) { + var testCases = []struct { + input string + }{ + { + input: "NO_EQUALS", + }, + } + + for _, tc := range testCases { + t.Run(tc.input, func(t *testing.T) { + config := &Config{ + Environment: []string{tc.input}, + } + _, err := config.ParseEnvironment() + assert.ErrorContains(t, err, "is not in the KEY=VALUE format") + }) + } +} + +// cleanTestYAML prepares an inline test YAML string for parsing +func cleanTestYAML(val string) []byte { + val = strings.ReplaceAll(val, "\t", " ") + + return []byte(val) +} diff --git a/pkg/config/data/config_schema_v1.0.json b/pkg/config/data/config_schema_v1.0.json index dd29c54e3b..96126c2524 100644 --- a/pkg/config/data/config_schema_v1.0.json +++ b/pkg/config/data/config_schema_v1.0.json @@ -151,24 +151,6 @@ "type": "boolean", "description": "A flag to enable the experimental fast-push feature from a config level." }, - "environment": { - "$id": "#/properties/build/properties/environment", - "type": [ - "array", - "null" - ], - "description": "A list of environment variables to make available at build time, in the format `NAME=value`", - "additionalItems": true, - "items": { - "$id": "#/properties/build/properties/environment/items", - "anyOf": [ - { - "$id": "#/properties/build/properties/environment/items/anyOf/0", - "type": "string" - } - ] - } - }, "python_overrides": { "$id": "#/properties/build/properties/python_overrides", "type": "string", @@ -227,21 +209,17 @@ } }, "environment": { - "$id": "#/properties/environment", + "$id": "#/properties/properties/environment", "type": [ "array", "null" ], - "description": "A list of environment variables to make available to the image at runtime, in the format `NAME=value`", + "description": "A list of environment variables to make available at build time, in the format `NAME=value`", "additionalItems": true, "items": { - "$id": "#/properties/environment/items", - "anyOf": [ - { - "$id": "#/properties/environment/items/anyOf/0", - "type": "string" - } - ] + "$id": "#/properties/properties/environment/items", + "type": "string", + "pattern": "^[A-Za-z_][A-Za-z0-9_]*=[^\\s]+$" } } }, diff --git a/pkg/dockerfile/env.go b/pkg/dockerfile/env.go new file mode 100644 index 0000000000..dbb08ff560 --- /dev/null +++ b/pkg/dockerfile/env.go @@ -0,0 +1,47 @@ +package dockerfile + +import ( + "fmt" + "maps" + "slices" + "strings" + + "github.com/replicate/cog/pkg/config" + "github.com/replicate/cog/pkg/util/console" +) + +func envLineFromConfig(c *config.Config) (string, error) { + vars, err := c.ParseEnvironment() + if err != nil { + return "", fmt.Errorf("failed to expand environment variables: %w", err) + } + if len(vars) == 0 { + return "", nil + } + + sortedNames := slices.Sorted(maps.Keys(vars)) + + var r8PrefixNames, cogPrefixNames []string + for _, name := range sortedNames { + if strings.HasPrefix(name, "R8_") { + r8PrefixNames = append(r8PrefixNames, name) + } else if strings.HasPrefix(name, "COG_") { + cogPrefixNames = append(cogPrefixNames, name) + } + } + + if len(r8PrefixNames) > 0 { + console.Warnf("Environment variables starting with R8_ may have unintended behavior. (%v)", strings.Join(r8PrefixNames, " ")) + } + if len(cogPrefixNames) > 0 { + console.Warnf("Environment variables starting with COG_ may have unintended behavior. (%v)", strings.Join(cogPrefixNames, " ")) + } + + out := "ENV" + for _, name := range sortedNames { + out = out + " " + name + "=" + vars[name] + } + out += "\n" + + return out, nil +} diff --git a/pkg/dockerfile/fast_generator.go b/pkg/dockerfile/fast_generator.go index 1b378bc5ad..bf5a0faa82 100644 --- a/pkg/dockerfile/fast_generator.go +++ b/pkg/dockerfile/fast_generator.go @@ -424,6 +424,14 @@ func (g *FastGenerator) installSrc(lines []string, weights []weights.Weight) ([] } func (g *FastGenerator) entrypoint(lines []string) ([]string, error) { + line, err := envLineFromConfig(g.Config) + if err != nil { + return nil, err + } + if line != "" { + lines = append(lines, line) + } + return append(lines, []string{ "WORKDIR /src", "ENV VERBOSE=0", diff --git a/pkg/dockerfile/standard_generator.go b/pkg/dockerfile/standard_generator.go index 440dd7e0fd..ce78c0ee87 100644 --- a/pkg/dockerfile/standard_generator.go +++ b/pkg/dockerfile/standard_generator.go @@ -519,19 +519,7 @@ This is the offending line: %s`, command) } func (g *StandardGenerator) envVars() (string, error) { - if len(g.Config.Environment) == 0 { - return "", nil - } - - var steps []string - for _, input := range g.Config.Environment { - name, val, err := parseNameValueString(input, true) - if err != nil { - return "", err - } - steps = append(steps, fmt.Sprintf("ENV %s=%q", name, val)) - } - return joinStringsWithoutLineSpace(steps), nil + return envLineFromConfig(g.Config) } // writeTemp writes a temporary file that can be used as part of the build process @@ -636,28 +624,3 @@ func stripPatchVersion(versionString string) (string, bool, error) { return strippedVersion, changed, nil } - -func parseNameValueString(input string, valFromHostEnv bool) (string, string, error) { - if input == "" { - return "", "", fmt.Errorf("empty input string") - } - - parts := strings.SplitN(input, "=", 2) - name := parts[0] - - if name == "" { - return "", "", fmt.Errorf("empty name in input") - } - - if len(parts) == 2 { - // NAME=VAL format - return name, parts[1], nil - } - - // NAME format - if valFromHostEnv { - return name, os.Getenv(name), nil - } - - return name, "", nil -} diff --git a/test-integration/test_integration/fixtures/env-project/cog.yaml b/test-integration/test_integration/fixtures/env-project/cog.yaml index 272ecd6dc8..a51b9c0604 100644 --- a/test-integration/test_integration/fixtures/env-project/cog.yaml +++ b/test-integration/test_integration/fixtures/env-project/cog.yaml @@ -1,5 +1,5 @@ -build: - python_version: "3.8" predict: "predict.py:Predictor" environment: - - NAME=wat + - NAME=yo + - R8_SOMETHING=abc + - A_B_C=123 From 4cf9a8aaf8c8188e33992e8e545bdfeee9991146 Mon Sep 17 00:00:00 2001 From: Michael Dwan Date: Wed, 23 Apr 2025 08:29:44 -0600 Subject: [PATCH 03/12] remove unused code --- pkg/config/config_test.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index a6700eec71..404bd20710 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -5,7 +5,6 @@ import ( "fmt" "os" "path" - "strings" "testing" "github.com/hashicorp/go-version" @@ -851,10 +850,3 @@ func TestEnvironmentParsingInvalid(t *testing.T) { }) } } - -// cleanTestYAML prepares an inline test YAML string for parsing -func cleanTestYAML(val string) []byte { - val = strings.ReplaceAll(val, "\t", " ") - - return []byte(val) -} From ccf23df2769e19e75c84d01a21b5558190c5a40d Mon Sep 17 00:00:00 2001 From: Michael Dwan Date: Wed, 23 Apr 2025 09:34:58 -0600 Subject: [PATCH 04/12] add env var integration test --- .../test_integration/test_predict.py | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test-integration/test_integration/test_predict.py b/test-integration/test_integration/test_predict.py index 7965641467..49b6d98814 100644 --- a/test-integration/test_integration/test_predict.py +++ b/test-integration/test_integration/test_predict.py @@ -542,3 +542,23 @@ def test_predict_granite_project(docker_image): ) assert result.returncode == 0 assert result.stdout == "2.11.3\n" + + +def test_predict_env_vars(docker_image): + project_dir = Path(__file__).parent / "fixtures/env-project" + build_process = subprocess.run( + ["cog", "build", "-t", docker_image], + cwd=project_dir, + capture_output=True, + ) + assert build_process.returncode == 0 + result = subprocess.run( + ["cog", "predict", "--debug", docker_image, "-i", "name=TEST_VAR"], + cwd=project_dir, + check=True, + capture_output=True, + text=True, + timeout=DEFAULT_TIMEOUT, + ) + assert result.returncode == 0 + assert result.stdout == "ENV[TEST_VAR]=test_value\n" From 8f13e21a55ae2e7f3b44fca9e7bd66e8658a7850 Mon Sep 17 00:00:00 2001 From: Michael Dwan Date: Wed, 23 Apr 2025 09:35:34 -0600 Subject: [PATCH 05/12] fix config for integration test fixture --- .../test_integration/fixtures/env-project/cog.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test-integration/test_integration/fixtures/env-project/cog.yaml b/test-integration/test_integration/fixtures/env-project/cog.yaml index a51b9c0604..df6b70a38e 100644 --- a/test-integration/test_integration/fixtures/env-project/cog.yaml +++ b/test-integration/test_integration/fixtures/env-project/cog.yaml @@ -1,5 +1,5 @@ predict: "predict.py:Predictor" environment: - - NAME=yo - - R8_SOMETHING=abc + - TEST_VAR=test_value + - R8_PREFIX_VAR=hello - A_B_C=123 From ae288968b3bed5102996913592b24086d61dcdd2 Mon Sep 17 00:00:00 2001 From: Michael Dwan Date: Wed, 23 Apr 2025 10:50:11 -0600 Subject: [PATCH 06/12] cog_binary fixture for configuring binary used in integration tests --- test-integration/test_integration/conftest.py | 35 +++++++++++++++++-- .../test_integration/test_predict.py | 6 ++-- tox.ini | 2 ++ 3 files changed, 37 insertions(+), 6 deletions(-) diff --git a/test-integration/test_integration/conftest.py b/test-integration/test_integration/conftest.py index 7a6f4f87da..9ee583b6fe 100644 --- a/test-integration/test_integration/conftest.py +++ b/test-integration/test_integration/conftest.py @@ -1,21 +1,50 @@ import os +import shutil +from pathlib import Path import pytest +from _pytest.config import Config +from _pytest.main import Session from .util import random_string, remove_docker_image -def pytest_sessionstart(session): +def pytest_sessionstart(session: Session) -> None: os.environ["COG_NO_UPDATE_CHECK"] = "1" @pytest.fixture -def docker_image_name(): +def cog_binary(pytestconfig: Config) -> Path: + """Get the path to the cog binary used in integration tests.""" + if os.environ.get("COG_BINARY"): + cog_path = Path(os.environ["COG_BINARY"]) + if not cog_path.is_absolute(): + # Only make relative to rootdir if it's a relative path + rootdir = Path(pytestconfig.rootdir) + cog_path = rootdir / cog_path + return cog_path.resolve() + + # Check if cog exists in project root. + # this is where integration tests dump the test build + project_cog = Path(pytestconfig.rootdir) / "cog" + if project_cog.exists(): + return project_cog + + # Fall back to cog in PATH + cog_path = shutil.which("cog") + if cog_path: + return Path(cog_path) + + raise FileNotFoundError("Could not find cog binary") + + +@pytest.fixture +def docker_image_name() -> str: return "cog-test-" + random_string(10) @pytest.fixture -def docker_image(docker_image_name): +def docker_image(docker_image_name: str) -> str: yield docker_image_name # We expect the image to exist by this point and will fail if it doesn't. # If you just need a name, use docker_image_name. diff --git a/test-integration/test_integration/test_predict.py b/test-integration/test_integration/test_predict.py index 49b6d98814..7967da8bf9 100644 --- a/test-integration/test_integration/test_predict.py +++ b/test-integration/test_integration/test_predict.py @@ -544,16 +544,16 @@ def test_predict_granite_project(docker_image): assert result.stdout == "2.11.3\n" -def test_predict_env_vars(docker_image): +def test_predict_env_vars(docker_image, cog_binary): project_dir = Path(__file__).parent / "fixtures/env-project" build_process = subprocess.run( - ["cog", "build", "-t", docker_image], + [cog_binary, "build", "-t", docker_image], cwd=project_dir, capture_output=True, ) assert build_process.returncode == 0 result = subprocess.run( - ["cog", "predict", "--debug", docker_image, "-i", "name=TEST_VAR"], + [cog_binary, "predict", "--debug", docker_image, "-i", "name=TEST_VAR"], cwd=project_dir, check=True, capture_output=True, diff --git a/tox.ini b/tox.ini index 63f22d94de..e9060dae47 100644 --- a/tox.ini +++ b/tox.ini @@ -71,4 +71,6 @@ deps = pytest-rerunfailures pytest-timeout pytest-xdist +pass_env = + COG_BINARY commands = pytest {posargs:-n auto -vv --reruns 3} From e59d0563577d82b61c88e3d16d35c793085e7ba2 Mon Sep 17 00:00:00 2001 From: Michael Dwan Date: Fri, 25 Apr 2025 15:07:58 -0600 Subject: [PATCH 07/12] remove unused Environment field on Build --- pkg/config/config.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 60795485f0..d7fc5e49d5 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -56,7 +56,6 @@ type Build struct { CuDNN string `json:"cudnn,omitempty" yaml:"cudnn"` Fast bool `json:"fast,omitempty" yaml:"fast"` PythonOverrides string `json:"python_overrides,omitempty" yaml:"python_overrides"` - Environment []string `json:"environment,omitempty" yaml:"environment"` pythonRequirementsContent []string } From 79733061597dceac8cbdf6157b411b376ffed13d Mon Sep 17 00:00:00 2001 From: Michael Dwan Date: Tue, 6 May 2025 14:14:48 -0600 Subject: [PATCH 08/12] updated denylist, more validation, more tests --- go.mod | 4 +- go.sum | 3 + pkg/config/config.go | 26 ++-- pkg/config/config_test.go | 131 ---------------- pkg/config/env_variables.go | 76 +++++++++ pkg/config/env_variables_test.go | 145 ++++++++++++++++++ pkg/dockerfile/env.go | 28 +--- .../fixtures/env-project/cog.yaml | 3 +- 8 files changed, 246 insertions(+), 170 deletions(-) create mode 100644 pkg/config/env_variables.go create mode 100644 pkg/config/env_variables_test.go diff --git a/go.mod b/go.mod index fb47e2d3f1..2b30a7b980 100644 --- a/go.mod +++ b/go.mod @@ -34,7 +34,7 @@ require ( golang.org/x/term v0.29.0 golang.org/x/tools v0.30.0 gopkg.in/yaml.v2 v2.4.0 - gotest.tools/gotestsum v1.12.1 + gotest.tools/gotestsum v1.12.2 sigs.k8s.io/yaml v1.4.0 ) @@ -125,7 +125,7 @@ require ( github.com/golangci/plugin-module-register v0.1.1 // indirect github.com/golangci/revgrep v0.8.0 // indirect github.com/golangci/unconvert v0.0.0-20240309020433-c5143eacb3ed // indirect - github.com/google/go-cmp v0.6.0 // indirect + github.com/google/go-cmp v0.7.0 // indirect github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 // indirect github.com/gordonklaus/ineffassign v0.1.0 // indirect github.com/gostaticanalysis/analysisutil v0.7.1 // indirect diff --git a/go.sum b/go.sum index c43fbb8cab..5a236bfdcd 100644 --- a/go.sum +++ b/go.sum @@ -221,6 +221,7 @@ github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeN github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= +github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= github.com/google/go-containerregistry v0.20.2 h1:B1wPJ1SN/S7pB+ZAimcciVD+r+yV/l/DSArMxlbwseo= github.com/google/go-containerregistry v0.20.2/go.mod h1:z38EKdKh4h7IP2gSfUUqEvalZBqs6AoLeWfUy34nQC8= github.com/google/pprof v0.0.0-20241210010833-40e02aabc2ad h1:a6HEuzUHeKH6hwfN/ZoQgRgVIWFJljSWa/zetS2WTvg= @@ -687,6 +688,8 @@ gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gotest.tools/gotestsum v1.12.1 h1:dvcxFBTFR1QsQmrCQa4k/vDXow9altdYz4CjdW+XeBE= gotest.tools/gotestsum v1.12.1/go.mod h1:mwDmLbx9DIvr09dnAoGgQPLaSXszNpXpWo2bsQge5BE= +gotest.tools/gotestsum v1.12.2 h1:eli4tu9Q2D/ogDsEGSr8XfQfl7mT0JsGOG6DFtUiZ/Q= +gotest.tools/gotestsum v1.12.2/go.mod h1:kjRtCglPZVsSU0hFHX3M5VWBM6Y63emHuB14ER1/sow= gotest.tools/v3 v3.5.1 h1:EENdUnS3pdur5nybKYIh2Vfgc8IUNBjxDPSjtiJcOzU= gotest.tools/v3 v3.5.1/go.mod h1:isy3WKz7GK6uNw/sbHzfKBLvlvXwUyV06n6brMxxopU= honnef.co/go/tools v0.6.0 h1:TAODvD3knlq75WCp2nyGJtT4LeRV/o7NN9nYPeVJXf8= diff --git a/pkg/config/config.go b/pkg/config/config.go index d7fc5e49d5..f1d93ca554 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -76,6 +76,8 @@ type Config struct { Train string `json:"train,omitempty" yaml:"train"` Concurrency *Concurrency `json:"concurrency,omitempty" yaml:"concurrency"` Environment []string `json:"environment,omitempty" yaml:"environment"` + + parsedEnvironment map[string]string } func DefaultConfig() *Config { @@ -320,6 +322,11 @@ func (c *Config) ValidateAndComplete(projectDir string) error { } } + // parse and validate environment variables + if err := c.loadEnvironment(); err != nil { + errs = append(errs, err) + } + if len(errs) > 0 { return errors.Join(errs...) } @@ -579,14 +586,15 @@ func sliceContains(slice []string, s string) bool { return false } -func (c *Config) ParseEnvironment() (map[string]string, error) { - env := map[string]string{} - for _, input := range c.Environment { - parts := strings.SplitN(input, "=", 2) - if len(parts) != 2 { - return nil, fmt.Errorf("environment variable %q is not in the KEY=VALUE format", input) - } - env[parts[0]] = parts[1] +func (c *Config) ParsedEnvironment() map[string]string { + return c.parsedEnvironment +} + +func (c *Config) loadEnvironment() error { + env, err := parseAndValidateEnvironment(c.Environment) + if err != nil { + return err } - return env, nil + c.parsedEnvironment = env + return nil } diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 404bd20710..75f4c46a71 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -2,13 +2,11 @@ package config import ( "encoding/json" - "fmt" "os" "path" "testing" "github.com/hashicorp/go-version" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gopkg.in/yaml.v2" ) @@ -721,132 +719,3 @@ build: _, err := FromYAML([]byte(yamlString)) require.NoError(t, err) } - -func TestEnvironmentConfig(t *testing.T) { - t.Run("PatternValidation", func(t *testing.T) { - testCases := map[string]bool{ - "NAME=VALUE": true, - "NAME=VALUE1,VALUE2": true, - "random_case=yo": true, - "JUST_A_NAME": false, - "NO_VAL=": false, - } - - for input, isValid := range testCases { - yamlString := fmt.Sprintf("environment:\n- %s", input) - _, err := FromYAML([]byte(yamlString)) - fmt.Printf("err: %+v\n", err) - if isValid { - require.NoError(t, err, "Expected no error for input: %s", input) - } else { - require.Error(t, err, "Expected error for input: %s", input) - } - } - }) - - t.Run("Parsing", func(t *testing.T) { - - }) -} - -func TestEnvironmentPattern(t *testing.T) { - testCases := map[string]bool{ - "NAME=VALUE": true, - "NAME=VALUE1,VALUE2": true, - "random_case=yo": true, - "JUST_A_NAME": false, - "NO_VAL=": false, - } - - for input, isValid := range testCases { - t.Run(input, func(t *testing.T) { - yamlString := fmt.Sprintf("environment:\n- %s", input) - _, err := FromYAML([]byte(yamlString)) - fmt.Printf("err: %+v\n", err) - if isValid { - require.NoError(t, err, "Expected no error for input: %s", input) - } else { - require.Error(t, err, "Expected error for input: %s", input) - } - }) - } -} - -func TestEnvironmentParsing(t *testing.T) { - var testCases = []struct { - input string - parsed map[string]string - }{ - { - input: "NAME=VALUE", - parsed: map[string]string{"NAME": "VALUE"}, - }, - { - input: "NAME=VALUE1,VALUE2", - parsed: map[string]string{"NAME": "VALUE1,VALUE2"}, - }, - { - input: "random_case=yo", - parsed: map[string]string{"random_case": "yo"}, - }, - { - input: "MULTIPLE=EQUAL=SIGNS", - parsed: map[string]string{"MULTIPLE": "EQUAL=SIGNS"}, - }, - { - input: "EMPTY_VALUE=", - parsed: map[string]string{"EMPTY_VALUE": ""}, - }, - { - input: "WITH_SPACES=VALUE WITH SPACES", - parsed: map[string]string{"WITH_SPACES": "VALUE WITH SPACES"}, - }, - { - input: "WITH_QUOTES=\"VALUE WITH QUOTES\"", - parsed: map[string]string{"WITH_QUOTES": "\"VALUE WITH QUOTES\""}, - }, - { - input: "WITH_SPACES=VALUE WITH SPACES", - parsed: map[string]string{"WITH_SPACES": "VALUE WITH SPACES"}, - }, - { - input: "WITH_QUOTES=\"VALUE WITH QUOTES\"", - parsed: map[string]string{"WITH_QUOTES": "\"VALUE WITH QUOTES\""}, - }, - { - input: "EMPTY_VALUE=", - parsed: map[string]string{"EMPTY_VALUE": ""}, - }, - } - - for _, tc := range testCases { - t.Run(tc.input, func(t *testing.T) { - config := &Config{ - Environment: []string{tc.input}, - } - parsed, err := config.ParseEnvironment() - require.NoError(t, err) - assert.Equal(t, tc.parsed, parsed) - }) - } -} - -func TestEnvironmentParsingInvalid(t *testing.T) { - var testCases = []struct { - input string - }{ - { - input: "NO_EQUALS", - }, - } - - for _, tc := range testCases { - t.Run(tc.input, func(t *testing.T) { - config := &Config{ - Environment: []string{tc.input}, - } - _, err := config.ParseEnvironment() - assert.ErrorContains(t, err, "is not in the KEY=VALUE format") - }) - } -} diff --git a/pkg/config/env_variables.go b/pkg/config/env_variables.go new file mode 100644 index 0000000000..3cbcf44d60 --- /dev/null +++ b/pkg/config/env_variables.go @@ -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 +} diff --git a/pkg/config/env_variables_test.go b/pkg/config/env_variables_test.go new file mode 100644 index 0000000000..2d403f5707 --- /dev/null +++ b/pkg/config/env_variables_test.go @@ -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: "DelimetedValue", + 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") + }) +} diff --git a/pkg/dockerfile/env.go b/pkg/dockerfile/env.go index dbb08ff560..bb00a07ef2 100644 --- a/pkg/dockerfile/env.go +++ b/pkg/dockerfile/env.go @@ -1,44 +1,20 @@ package dockerfile import ( - "fmt" "maps" "slices" - "strings" "github.com/replicate/cog/pkg/config" - "github.com/replicate/cog/pkg/util/console" ) func envLineFromConfig(c *config.Config) (string, error) { - vars, err := c.ParseEnvironment() - if err != nil { - return "", fmt.Errorf("failed to expand environment variables: %w", err) - } + vars := c.ParsedEnvironment() if len(vars) == 0 { return "", nil } - sortedNames := slices.Sorted(maps.Keys(vars)) - - var r8PrefixNames, cogPrefixNames []string - for _, name := range sortedNames { - if strings.HasPrefix(name, "R8_") { - r8PrefixNames = append(r8PrefixNames, name) - } else if strings.HasPrefix(name, "COG_") { - cogPrefixNames = append(cogPrefixNames, name) - } - } - - if len(r8PrefixNames) > 0 { - console.Warnf("Environment variables starting with R8_ may have unintended behavior. (%v)", strings.Join(r8PrefixNames, " ")) - } - if len(cogPrefixNames) > 0 { - console.Warnf("Environment variables starting with COG_ may have unintended behavior. (%v)", strings.Join(cogPrefixNames, " ")) - } - out := "ENV" - for _, name := range sortedNames { + for _, name := range slices.Sorted(maps.Keys(vars)) { out = out + " " + name + "=" + vars[name] } out += "\n" diff --git a/test-integration/test_integration/fixtures/env-project/cog.yaml b/test-integration/test_integration/fixtures/env-project/cog.yaml index df6b70a38e..4827f35ccb 100644 --- a/test-integration/test_integration/fixtures/env-project/cog.yaml +++ b/test-integration/test_integration/fixtures/env-project/cog.yaml @@ -1,5 +1,4 @@ predict: "predict.py:Predictor" environment: + - NAME=michael - TEST_VAR=test_value - - R8_PREFIX_VAR=hello - - A_B_C=123 From e64e420b2664b6b2a81601e9e5931da5be4d99fb Mon Sep 17 00:00:00 2001 From: Michael Dwan Date: Tue, 6 May 2025 15:07:07 -0600 Subject: [PATCH 09/12] tidy mod files after merge oddness --- go.mod | 5 ++--- go.sum | 3 --- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index 0c4c38f13d..5901a89987 100644 --- a/go.mod +++ b/go.mod @@ -28,13 +28,11 @@ require ( github.com/vincent-petithory/dataurl v1.0.0 github.com/xeipuuv/gojsonschema v1.2.0 github.com/xeonx/timeago v1.0.0-rc5 - golang.org/x/crypto v0.37.0 golang.org/x/exp v0.0.0-20250408133849-7e4ce0ab07d0 golang.org/x/sync v0.13.0 golang.org/x/sys v0.32.0 golang.org/x/term v0.31.0 gopkg.in/yaml.v2 v2.4.0 - gotest.tools/gotestsum v1.12.2 sigs.k8s.io/yaml v1.4.0 ) @@ -272,6 +270,7 @@ require ( go.uber.org/automaxprocs v1.6.0 // indirect go.uber.org/multierr v1.11.0 // indirect go.uber.org/zap v1.27.0 // indirect + golang.org/x/crypto v0.37.0 // indirect golang.org/x/exp/typeparams v0.0.0-20250210185358-939b2ce775ac // indirect golang.org/x/mod v0.24.0 // indirect golang.org/x/net v0.39.0 // indirect @@ -281,7 +280,7 @@ require ( google.golang.org/grpc v1.71.0 // indirect google.golang.org/protobuf v1.36.6 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect - gotest.tools/gotestsum v1.12.1 // indirect + gotest.tools/gotestsum v1.12.2 // indirect honnef.co/go/tools v0.6.1 // indirect mvdan.cc/gofumpt v0.7.0 // indirect mvdan.cc/unparam v0.0.0-20240528143540-8a5130ca722f // indirect diff --git a/go.sum b/go.sum index 9f3bdefcf6..1270b5536a 100644 --- a/go.sum +++ b/go.sum @@ -245,7 +245,6 @@ github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeN github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= -github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= github.com/google/go-containerregistry v0.20.3 h1:oNx7IdTI936V8CQRveCjaxOiegWwvM7kqkbXTpyiovI= github.com/google/go-containerregistry v0.20.3/go.mod h1:w00pIgBRDVUDFM6bq+Qx8lwNWK+cxgCuX1vd3PIBDNI= github.com/google/pprof v0.0.0-20241210010833-40e02aabc2ad h1:a6HEuzUHeKH6hwfN/ZoQgRgVIWFJljSWa/zetS2WTvg= @@ -765,8 +764,6 @@ gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= -gotest.tools/gotestsum v1.12.1 h1:dvcxFBTFR1QsQmrCQa4k/vDXow9altdYz4CjdW+XeBE= -gotest.tools/gotestsum v1.12.1/go.mod h1:mwDmLbx9DIvr09dnAoGgQPLaSXszNpXpWo2bsQge5BE= gotest.tools/gotestsum v1.12.2 h1:eli4tu9Q2D/ogDsEGSr8XfQfl7mT0JsGOG6DFtUiZ/Q= gotest.tools/gotestsum v1.12.2/go.mod h1:kjRtCglPZVsSU0hFHX3M5VWBM6Y63emHuB14ER1/sow= gotest.tools/v3 v3.5.2 h1:7koQfIKdy+I8UTetycgUqXWSDwpgv193Ka+qRsmBY8Q= From da662f4a9387017da664c2565df94ed8a7942a65 Mon Sep 17 00:00:00 2001 From: Michael Dwan Date: Tue, 6 May 2025 15:08:33 -0600 Subject: [PATCH 10/12] omit empty Environment config when writing yaml --- pkg/config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 140a696dc8..6ccd271419 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -75,7 +75,7 @@ type Config struct { Predict string `json:"predict,omitempty" yaml:"predict"` Train string `json:"train,omitempty" yaml:"train,omitempty"` Concurrency *Concurrency `json:"concurrency,omitempty" yaml:"concurrency,omitempty"` - Environment []string `json:"environment,omitempty" yaml:"environment"` + Environment []string `json:"environment,omitempty" yaml:"environment,omitempty"` parsedEnvironment map[string]string } From 2fec62014bddf1a8c8a350cd647ea2c4bdb3f5e6 Mon Sep 17 00:00:00 2001 From: Michael Dwan Date: Tue, 6 May 2025 15:19:55 -0600 Subject: [PATCH 11/12] typo Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Michael Dwan --- pkg/config/env_variables_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/config/env_variables_test.go b/pkg/config/env_variables_test.go index 2d403f5707..8450891072 100644 --- a/pkg/config/env_variables_test.go +++ b/pkg/config/env_variables_test.go @@ -31,7 +31,7 @@ func TestEnvironmentConfig(t *testing.T) { Expected: map[string]string{"NAME": `"VALUE WITH QUOTES"`}, }, { - Name: "DelimetedValue", + Name: "DelimitedValue", Input: []string{"NAME=VALUE1,VALUE2"}, Expected: map[string]string{"NAME": "VALUE1,VALUE2"}, }, From 2d75642611d57e6bdd9f93ee3ab9cb54699fc6ae Mon Sep 17 00:00:00 2001 From: Michael Dwan Date: Wed, 7 May 2025 10:36:43 -0600 Subject: [PATCH 12/12] correct environment entry description --- pkg/config/data/config_schema_v1.0.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/config/data/config_schema_v1.0.json b/pkg/config/data/config_schema_v1.0.json index 96126c2524..4fbec34f6f 100644 --- a/pkg/config/data/config_schema_v1.0.json +++ b/pkg/config/data/config_schema_v1.0.json @@ -214,7 +214,7 @@ "array", "null" ], - "description": "A list of environment variables to make available at build time, in the format `NAME=value`", + "description": "A list of environment variables to make available during builds and at runtime, in the format `NAME=value`", "additionalItems": true, "items": { "$id": "#/properties/properties/environment/items",