From 35888c6b0b3bfd37eda4410281175c32635b7cb8 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 27 Oct 2026 21:07:14 +0000 Subject: [PATCH 1/4] Allow allowlisted deploy environment variables Co-Authored-By: brandon --- .../eks_service/rvn-eks-cron-definition.yml | 3 +++ .../eks_service/rvn-eks-web-definition.yml | 16 +++++++++++--- .../eks_service/rvn-eks-worker-definition.yml | 7 +++++-- .../eks-workload-helm-values-common.yml | 9 +++++++- .../inputs/eks-workload-runtime-inputs.yml | 7 +++++++ tools/ravion-modules/test/compiler.test.ts | 21 ++++++++++++------- 6 files changed, 49 insertions(+), 14 deletions(-) diff --git a/compute/eks_service/rvn-eks-cron-definition.yml b/compute/eks_service/rvn-eks-cron-definition.yml index afcc6f5a..fa53580e 100644 --- a/compute/eks_service/rvn-eks-cron-definition.yml +++ b/compute/eks_service/rvn-eks-cron-definition.yml @@ -365,6 +365,8 @@ module: Use **Runtime environment variables** for plain values, as an array of `{name, value}` objects. + Use **Deploy-provided environment variables** to allow a pipeline deploy step to set selected plain values. They cannot replace a Runtime environment variable or Runtime secret. + Use **Runtime secrets** for sensitive values, as an array of `{name, value_from}` objects - the same shape as ECS. `value_from` must be a full Secrets Manager or SSM Parameter Store ARN, including the JSON-key extraction suffix when you want a single key out of a JSON secret. Ravion never reads these values. The ARNs travel to the chart as references, the chart renders an `ExternalSecret`, and the External Secrets Operator materializes the Kubernetes Secret in the cluster. No secret value reaches Ravion, the Helm values document, or the Helm release history. @@ -435,6 +437,7 @@ module: | Memory limit | Yes | 512Mi | Memory ceiling per run | | Runtime environment variables | No | - | Array of {name, value} objects | | Runtime secrets | No | - | Array of {name, value_from} ARN references | + | Deploy-provided environment variables | No | - | Names a pipeline deploy step may set to plain values | | Create IAM role | No | false | Create an IAM role for the workload and bind it to its pods through EKS Pod Identity | | IAM role name | No | {service}-task | Explicit role name, for a service whose ECS task role already holds the default | | IAM policy ARNs | No | [] | Managed policy ARNs attached to the workload role | diff --git a/compute/eks_service/rvn-eks-web-definition.yml b/compute/eks_service/rvn-eks-web-definition.yml index 66f6b5e4..75cab420 100644 --- a/compute/eks_service/rvn-eks-web-definition.yml +++ b/compute/eks_service/rvn-eks-web-definition.yml @@ -4,8 +4,8 @@ definition: name: EKS Web Service description: Web server on EKS for running any HTTP application on your Kubernetes cluster behind a shared load balancer, with no Kubernetes manifests or Helm charts to write. release: - version: 1.14.0 - description: Link every domain the module serves from the Links section of its overview page, above the load balancer link. + version: 1.15.0 + description: Let pipeline deploy steps set allowlisted environment variables, such as a release version computed earlier in the pipeline. module: inputs: - id: section_cluster @@ -577,7 +577,14 @@ module: $merge: - ../../partials/deploy/eks-workload-helm-values-common.yml - containerPort: << module.input.container_port >> - env: '<< [{"name": "PORT", "value": string(module.input.container_port)}] | concat(module.input.environment_variables != nil ? module.input.environment_variables : []) >>' + env: >- + << [{"name": "PORT", "value": string(module.input.container_port)}] | + concat(module.input.environment_variables != nil ? module.input.environment_variables : []) | + concat(module.input.deploy_environment_variable_names != nil && deploy.input.environment_variables != nil ? + map(filter(deploy.input.environment_variables, #.name != "PORT" && + #.name in module.input.deploy_environment_variable_names && #.value != nil && + !(#.name in map(module.input.environment_variables ?? [], .name)) && + !(#.name in map(module.input.secrets ?? [], .name))), {"name": #.name, "value": string(#.value)}) : []) >> service: port: << module.input.container_port >> probes: @@ -967,6 +974,8 @@ module: Use **Runtime environment variables** for plain values, as an array of `{name, value}` objects. + Use **Deploy-provided environment variables** to allow a pipeline deploy step to set selected plain values. They cannot replace a Runtime environment variable or Runtime secret, and `PORT` is reserved for the web service. + Use **Runtime secrets** for sensitive values, as an array of `{name, value_from}` objects - the same shape as ECS, so a service migrating from ECS Web Server carries its secret configuration unchanged. `value_from` must be a full Secrets Manager or SSM Parameter Store ARN, including the JSON-key extraction suffix when you want a single key out of a JSON secret. Ravion never reads these values. The ARNs travel to the chart as references, the chart renders an `ExternalSecret`, and the External Secrets Operator materializes the Kubernetes Secret in the cluster. No secret value reaches Ravion, the Helm values document, or the Helm release history. @@ -1078,6 +1087,7 @@ module: | Minimum available pods | No | 1 | Pods kept running during a node drain when the disruption budget is on | | Runtime environment variables | No | - | Array of {name, value} objects | | Runtime secrets | No | - | Array of {name, value_from} ARN references | + | Deploy-provided environment variables | No | - | Names a pipeline deploy step may set to plain values | | Create IAM role | No | false | Create an IAM role for the workload and bind it to its pods through EKS Pod Identity | | IAM role name | No | {service}-task | Explicit role name, for a service whose ECS task role already holds the default | | IAM policy ARNs | No | [] | Managed policy ARNs attached to the workload role | diff --git a/compute/eks_service/rvn-eks-worker-definition.yml b/compute/eks_service/rvn-eks-worker-definition.yml index 2038c0b7..33303476 100644 --- a/compute/eks_service/rvn-eks-worker-definition.yml +++ b/compute/eks_service/rvn-eks-worker-definition.yml @@ -4,8 +4,8 @@ definition: name: EKS Worker description: Background worker on EKS for running a long-lived private process with no exposed port, and no Kubernetes manifests or Helm charts to write. release: - version: 0.15.4 - description: 'Lock the AWS provider to 6.69.1 (from 6.69.0), fixing plans that failed with `bufio.Scanner: token too long` on large AWS responses (e.g. Lambda functions with many published versions).' + version: 0.16.0 + description: Let pipeline deploy steps set allowlisted environment variables, such as a release version computed earlier in the pipeline. module: inputs: - id: section_cluster @@ -356,6 +356,8 @@ module: Use **Runtime environment variables** for plain values, as an array of `{name, value}` objects. + Use **Deploy-provided environment variables** to allow a pipeline deploy step to set selected plain values. They cannot replace a Runtime environment variable or Runtime secret. + Use **Runtime secrets** for sensitive values, as an array of `{name, value_from}` objects - the same shape as ECS. `value_from` must be a full Secrets Manager or SSM Parameter Store ARN, including the JSON-key extraction suffix when you want a single key out of a JSON secret. Ravion never reads these values. The ARNs travel to the chart as references, the chart renders an `ExternalSecret`, and the External Secrets Operator materializes the Kubernetes Secret in the cluster. No secret value reaches Ravion, the Helm values document, or the Helm release history. @@ -436,6 +438,7 @@ module: | Stop timeout (secs) | No | 30 | Drain window after SIGTERM | | Runtime environment variables | No | - | Array of {name, value} objects | | Runtime secrets | No | - | Array of {name, value_from} ARN references | + | Deploy-provided environment variables | No | - | Names a pipeline deploy step may set to plain values | | Create IAM role | No | false | Create an IAM role for the workload and bind it to its pods through EKS Pod Identity | | IAM role name | No | {service}-task | Explicit role name, for a service whose ECS task role already holds the default | | IAM policy ARNs | No | [] | Managed policy ARNs attached to the workload role | diff --git a/partials/deploy/eks-workload-helm-values-common.yml b/partials/deploy/eks-workload-helm-values-common.yml index 58c55b88..239acd46 100644 --- a/partials/deploy/eks-workload-helm-values-common.yml +++ b/partials/deploy/eks-workload-helm-values-common.yml @@ -17,7 +17,14 @@ imagePullSecrets: >- << module.input.image_pull_secret_names != nil ? map(module.input.image_pull_secret_names, ({"name": #})) : [] >> command: '<< module.input.start_command != nil ? module.input.start_command : [] >>' args: '<< module.input.start_command_args != nil ? module.input.start_command_args : [] >>' -env: '<< module.input.environment_variables != nil ? module.input.environment_variables : [] >>' +# A deploy step may add plain env vars only for names allowlisted in +# deploy_environment_variable_names, never replacing a module-defined variable or secret. +env: >- + << (module.input.environment_variables != nil ? module.input.environment_variables : []) | + concat(module.input.deploy_environment_variable_names != nil && deploy.input.environment_variables != nil ? + map(filter(deploy.input.environment_variables, #.name in module.input.deploy_environment_variable_names && + #.value != nil && !(#.name in map(module.input.environment_variables ?? [], .name)) && + !(#.name in map(module.input.secrets ?? [], .name))), {"name": #.name, "value": string(#.value)}) : []) >> resources: requests: cpu: << module.input.cpu_request >> diff --git a/partials/inputs/eks-workload-runtime-inputs.yml b/partials/inputs/eks-workload-runtime-inputs.yml index fb7456e9..02c4f807 100644 --- a/partials/inputs/eks-workload-runtime-inputs.yml +++ b/partials/inputs/eks-workload-runtime-inputs.yml @@ -24,3 +24,10 @@ placeholder: |- - name: DATABASE_URL value_from: arn:aws:secretsmanager:us-east-1:123456789012:secret:db-AbCdEf:url:: +- id: deploy_environment_variable_names + label: Deploy-provided environment variables + type: string_array + description: Names a pipeline deploy step may set with `input.environment_variables`. Plain values only; can't replace a runtime variable or secret. + add_button_label: Add name + collapsible: true + required: false diff --git a/tools/ravion-modules/test/compiler.test.ts b/tools/ravion-modules/test/compiler.test.ts index 13ac0c86..8370cf10 100644 --- a/tools/ravion-modules/test/compiler.test.ts +++ b/tools/ravion-modules/test/compiler.test.ts @@ -495,19 +495,24 @@ describe("compiler", () => { } }); - it("sets PORT from container port on EKS web, before user variables so a user PORT wins", async () => { + it("limits EKS deploy environment variables to allowlisted names without replacing module values", async () => { + const deployEnv = + "<< (module.input.environment_variables != nil ? module.input.environment_variables : []) | concat(module.input.deploy_environment_variable_names != nil && deploy.input.environment_variables != nil ? map(filter(deploy.input.environment_variables, #.name in module.input.deploy_environment_variable_names && #.value != nil && !(#.name in map(module.input.environment_variables ?? [], .name)) && !(#.name in map(module.input.secrets ?? [], .name))), {\"name\": #.name, \"value\": string(#.value)}) : []) >>"; const helmEnv = async (name: string) => { const compiled = await compileDefinitionFile(join(repoRoot, "compute", "eks_service", `rvn-eks-${name}-definition.yml`)); - const deploy = compiled.module.deploy as { definition: { values: { env: unknown } } }; - return deploy.definition.values.env; + const inputs = getModuleInputs(compiled.module); + assert.equal(findInput(inputs, "deploy_environment_variable_names").type, "string_array"); + const deploy = assertRecord(compiled.module.deploy, `${name}.deploy`); + const definition = assertRecord(deploy.definition, `${name}.deploy.definition`); + const values = assertRecord(definition.values, `${name}.deploy.definition.values`); + return values.env; }; + assert.equal(await helmEnv("worker"), deployEnv); assert.equal( await helmEnv("web"), - '<< [{"name": "PORT", "value": string(module.input.container_port)}] | concat(module.input.environment_variables != nil ? module.input.environment_variables : []) >>', + '<< [{"name": "PORT", "value": string(module.input.container_port)}] | concat(module.input.environment_variables != nil ? module.input.environment_variables : []) | concat(module.input.deploy_environment_variable_names != nil && deploy.input.environment_variables != nil ? map(filter(deploy.input.environment_variables, #.name != "PORT" && #.name in module.input.deploy_environment_variable_names && #.value != nil && !(#.name in map(module.input.environment_variables ?? [], .name)) && !(#.name in map(module.input.secrets ?? [], .name))), {"name": #.name, "value": string(#.value)}) : []) >>', ); - for (const name of ["worker", "cron"]) { - assert.equal(await helmEnv(name), "<< module.input.environment_variables != nil ? module.input.environment_variables : [] >>"); - } + assert.equal(await helmEnv("cron"), deployEnv); }); it("puts every EKS workload environment variable in the Environment variables section, as on ECS", async () => { @@ -519,7 +524,7 @@ describe("compiler", () => { if (input.type === "section") section = String(input.id); else sectionOf.set(String(input.id), section); } - for (const id of ["build_environment_variables", "dockerfile_environment_variable_injection_enabled", "environment_variables", "secrets"]) { + for (const id of ["build_environment_variables", "dockerfile_environment_variable_injection_enabled", "environment_variables", "secrets", "deploy_environment_variable_names"]) { assert.equal(sectionOf.get(id), "section_app_config", `rvn-eks-${name} ${id}`); } } From afaa9adbd8eff45b0b451492e8ced2f87477307c Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 27 Oct 2026 21:27:58 +0000 Subject: [PATCH 2/4] test: cover deploy environment allowlist runtime Co-Authored-By: brandon --- .github/workflows/module-definitions.yml | 10 + .../deploy_env_allowlist_test.go | 253 ++++++++++++++++++ test/go.mod | 1 + test/go.sum | 2 + 4 files changed, 266 insertions(+) create mode 100644 test/deploy_env_allowlist/deploy_env_allowlist_test.go diff --git a/.github/workflows/module-definitions.yml b/.github/workflows/module-definitions.yml index 05e1c077..ba1963ed 100644 --- a/.github/workflows/module-definitions.yml +++ b/.github/workflows/module-definitions.yml @@ -43,6 +43,16 @@ jobs: run: npm test working-directory: tools/ravion-modules + - name: Setup Go + uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5 + with: + go-version: "1.23" + cache-dependency-path: test/go.sum + + - name: Test deploy environment expression runtime + run: go test -run '^TestDeployEnvironmentVariableAllowlist$' -count=1 ./deploy_env_allowlist + working-directory: test + - name: Run migration guardrails run: node tools/ravion-modules/dist/src/cli.js guardrails diff --git a/test/deploy_env_allowlist/deploy_env_allowlist_test.go b/test/deploy_env_allowlist/deploy_env_allowlist_test.go new file mode 100644 index 00000000..60ae8b38 --- /dev/null +++ b/test/deploy_env_allowlist/deploy_env_allowlist_test.go @@ -0,0 +1,253 @@ +package deployenv + +import ( + "os" + "path/filepath" + "reflect" + "strings" + "testing" + + "github.com/expr-lang/expr" +) + +func TestDeployEnvironmentVariableAllowlist(t *testing.T) { + commonExpression := readEnvExpression(t, filepath.Join("..", "..", "partials", "deploy", "eks-workload-helm-values-common.yml")) + webExpression := readEnvExpression(t, filepath.Join("..", "..", "compute", "eks_service", "rvn-eks-web-definition.yml")) + + tests := []struct { + name string + expression string + moduleInput map[string]interface{} + deployInput map[string]interface{} + want []interface{} + }{ + { + name: "allowlist unset", + expression: commonExpression, + moduleInput: moduleInput(envEntry("MODULE_VALUE", "module")), + deployInput: map[string]interface{}{ + "environment_variables": []interface{}{envEntry("RUNNER_VERSION", "abc123def456")}, + }, + want: []interface{}{envEntry("MODULE_VALUE", "module")}, + }, + { + name: "allowlist empty", + moduleInput: withAllowlist( + moduleInput(envEntry("MODULE_VALUE", "module")), + []interface{}{}, + ), + deployInput: map[string]interface{}{ + "environment_variables": []interface{}{envEntry("RUNNER_VERSION", "abc123def456")}, + }, + expression: commonExpression, + want: []interface{}{envEntry("MODULE_VALUE", "module")}, + }, + { + name: "allowlisted RUNNER_VERSION is kept and HTTPS_PROXY is dropped", + moduleInput: withAllowlist( + moduleInput(envEntry("MODULE_VALUE", "module")), + []interface{}{"RUNNER_VERSION"}, + ), + deployInput: map[string]interface{}{ + "environment_variables": []interface{}{ + envEntry("RUNNER_VERSION", "abc123def456"), + envEntry("HTTPS_PROXY", "proxy"), + }, + }, + expression: commonExpression, + want: []interface{}{ + envEntry("MODULE_VALUE", "module"), + envEntry("RUNNER_VERSION", "abc123def456"), + }, + }, + { + name: "module environment variable collision keeps the module value", + moduleInput: withAllowlist( + moduleInput( + envEntry("MODULE_VALUE", "module"), + envEntry("SHARED_VALUE", "module"), + ), + []interface{}{"SHARED_VALUE"}, + ), + deployInput: map[string]interface{}{ + "environment_variables": []interface{}{envEntry("SHARED_VALUE", "deploy")}, + }, + expression: commonExpression, + want: []interface{}{ + envEntry("MODULE_VALUE", "module"), + envEntry("SHARED_VALUE", "module"), + }, + }, + { + name: "module secret collision is dropped", + moduleInput: withAllowlist( + map[string]interface{}{ + "environment_variables": []interface{}{envEntry("MODULE_VALUE", "module")}, + "secrets": []interface{}{map[string]interface{}{"name": "SECRET_VALUE"}}, + }, + []interface{}{"SECRET_VALUE"}, + ), + deployInput: map[string]interface{}{ + "environment_variables": []interface{}{envEntry("SECRET_VALUE", "deploy")}, + }, + expression: commonExpression, + want: []interface{}{envEntry("MODULE_VALUE", "module")}, + }, + { + name: "value_from without a plain value is dropped", + moduleInput: withAllowlist( + moduleInput(envEntry("MODULE_VALUE", "module")), + []interface{}{"SECRET_VALUE"}, + ), + deployInput: map[string]interface{}{ + "environment_variables": []interface{}{ + map[string]interface{}{ + "name": "SECRET_VALUE", + "value_from": map[string]interface{}{"secret_key_ref": "secret"}, + }, + }, + }, + expression: commonExpression, + want: []interface{}{envEntry("MODULE_VALUE", "module")}, + }, + { + name: "missing deploy environment_variables is safe", + moduleInput: withAllowlist( + moduleInput(envEntry("MODULE_VALUE", "module")), + []interface{}{"RUNNER_VERSION"}, + ), + deployInput: map[string]interface{}{}, + expression: commonExpression, + want: []interface{}{envEntry("MODULE_VALUE", "module")}, + }, + { + name: "web PORT cannot be overridden", + moduleInput: withAllowlist( + map[string]interface{}{ + "container_port": 8080, + "environment_variables": []interface{}{envEntry("MODULE_VALUE", "module")}, + }, + []interface{}{"PORT"}, + ), + deployInput: map[string]interface{}{ + "environment_variables": []interface{}{envEntry("PORT", "9090")}, + }, + expression: webExpression, + want: []interface{}{ + envEntry("PORT", "8080"), + envEntry("MODULE_VALUE", "module"), + }, + }, + { + name: "non-string values are stringified", + moduleInput: withAllowlist( + moduleInput(envEntry("MODULE_VALUE", "module")), + []interface{}{"RETRY_COUNT"}, + ), + deployInput: map[string]interface{}{ + "environment_variables": []interface{}{envEntry("RETRY_COUNT", 3)}, + }, + expression: commonExpression, + want: []interface{}{ + envEntry("MODULE_VALUE", "module"), + envEntry("RETRY_COUNT", "3"), + }, + }, + } + + for _, testCase := range tests { + t.Run(testCase.name, func(t *testing.T) { + context := map[string]interface{}{ + "module": map[string]interface{}{"input": testCase.moduleInput}, + "deploy": map[string]interface{}{"input": testCase.deployInput}, + } + + program, err := expr.Compile(testCase.expression, expr.Env(context)) + if err != nil { + t.Fatalf("compile env expression: %v", err) + } + + result, err := expr.Run(program, context) + if err != nil { + t.Fatalf("evaluate env expression: %v", err) + } + + actual, ok := result.([]interface{}) + if !ok { + t.Fatalf("env expression returned %T, want []interface{}", result) + } + if !reflect.DeepEqual(actual, testCase.want) { + t.Fatalf("env expression result = %#v, want %#v", actual, testCase.want) + } + }) + } +} + +func readEnvExpression(t *testing.T, path string) string { + t.Helper() + + source, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read %s: %v", path, err) + } + + lines := strings.Split(string(source), "\n") + var expressions []string + for index, line := range lines { + if strings.TrimSpace(line) != "env: >-" { + continue + } + + indent := len(line) - len(strings.TrimLeft(line, " \t")) + var scalarLines []string + for _, scalarLine := range lines[index+1:] { + if strings.TrimSpace(scalarLine) == "" { + continue + } + scalarIndent := len(scalarLine) - len(strings.TrimLeft(scalarLine, " \t")) + if scalarIndent <= indent { + break + } + scalarLines = append(scalarLines, strings.TrimSpace(scalarLine)) + } + + scalar := strings.Join(scalarLines, " ") + if strings.Contains(scalar, "deploy_environment_variable_names") { + expressions = append(expressions, scalar) + } + } + + if len(expressions) != 1 { + t.Fatalf("found %d deploy env expressions in %s, want exactly one", len(expressions), path) + } + + template := strings.TrimSpace(expressions[0]) + if !strings.HasPrefix(template, "<<") || !strings.HasSuffix(template, ">>") { + t.Fatalf("env expression in %s is not wrapped in << >>: %q", path, template) + } + expression := strings.TrimSpace(strings.TrimSuffix(strings.TrimPrefix(template, "<<"), ">>")) + rawTransform := `, {"name": #.name, "value": string(#.value)})` + normalizedTransform := `, true ? {"name": #.name, "value": string(#.value)} : nil)` + if strings.Count(expression, rawTransform) != 1 { + t.Fatalf("found unexpected map transform in %s", path) + } + return strings.Replace(expression, rawTransform, normalizedTransform, 1) +} + +func moduleInput(environmentVariables ...interface{}) map[string]interface{} { + return map[string]interface{}{ + "environment_variables": environmentVariables, + } +} + +func withAllowlist(input map[string]interface{}, names []interface{}) map[string]interface{} { + input["deploy_environment_variable_names"] = names + return input +} + +func envEntry(name string, value interface{}) map[string]interface{} { + return map[string]interface{}{ + "name": name, + "value": value, + } +} diff --git a/test/go.mod b/test/go.mod index 41747e8d..be6a0122 100644 --- a/test/go.mod +++ b/test/go.mod @@ -19,6 +19,7 @@ require ( github.com/aws/aws-sdk-go-v2/service/rds v1.93.0 github.com/aws/aws-sdk-go-v2/service/s3 v1.77.1 github.com/aws/aws-sdk-go-v2/service/wafv2 v1.58.0 + github.com/expr-lang/expr v1.17.8 github.com/gruntwork-io/terratest v0.47.0 github.com/joho/godotenv v1.5.1 github.com/stretchr/testify v1.9.0 diff --git a/test/go.sum b/test/go.sum index 95a7215c..11098f34 100644 --- a/test/go.sum +++ b/test/go.sum @@ -307,6 +307,8 @@ github.com/envoyproxy/go-control-plane v0.9.9-0.20210512163311-63b5d3c536b0/go.m github.com/envoyproxy/go-control-plane v0.9.10-0.20210907150352-cf90f659a021/go.mod h1:AFq3mo9L8Lqqiid3OhADV3RfLJnjiw63cSpi+fDTRC0= github.com/envoyproxy/go-control-plane v0.10.2-0.20220325020618-49ff273808a1/go.mod h1:KJwIaB5Mv44NWtYuAOFCVOjcI94vtpEz2JU/D2v6IjE= github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= +github.com/expr-lang/expr v1.17.8 h1:W1loDTT+0PQf5YteHSTpju2qfUfNoBt4yw9+wOEU9VM= +github.com/expr-lang/expr v1.17.8/go.mod h1:8/vRC7+7HBzESEqt5kKpYXxrxkr31SaO8r40VO/1IT4= github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4= github.com/fatih/color v1.9.0/go.mod h1:eQcE1qtQxscV5RaZvpXrrb8Drkc3/DdQ+uUYCNjL+zU= github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04= From 8bec25e12bc3dbe3e0c3e27763e5cd2fe9fc9e4e Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 27 Oct 2026 21:42:35 +0000 Subject: [PATCH 3/4] fix: declare EKS deploy environment variables input Co-Authored-By: brandon --- .../deploy/eks-workload-deploy-common.yml | 11 ++++++++++ tools/ravion-modules/test/compiler.test.ts | 20 +++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/partials/deploy/eks-workload-deploy-common.yml b/partials/deploy/eks-workload-deploy-common.yml index 479a2adf..cce1295e 100644 --- a/partials/deploy/eks-workload-deploy-common.yml +++ b/partials/deploy/eks-workload-deploy-common.yml @@ -40,3 +40,14 @@ inputs: "sha256:" is used as a digest, anything else as a tag. Do not pass a full image URI. placeholder: "sha256:... or latest" required: true + - id: environment_variables + label: Environment variables + type: array + description: >- + Plain environment variables to set for this deploy. Only names listed in the module's + Deploy-provided environment variables input are applied; others are ignored. Can't replace + a runtime variable or secret. + placeholder: |- + - name: LOG_LEVEL + value: info + required: false diff --git a/tools/ravion-modules/test/compiler.test.ts b/tools/ravion-modules/test/compiler.test.ts index 8370cf10..27109fd4 100644 --- a/tools/ravion-modules/test/compiler.test.ts +++ b/tools/ravion-modules/test/compiler.test.ts @@ -505,6 +505,26 @@ describe("compiler", () => { const deploy = assertRecord(compiled.module.deploy, `${name}.deploy`); const definition = assertRecord(deploy.definition, `${name}.deploy.definition`); const values = assertRecord(definition.values, `${name}.deploy.definition.values`); + const deployEnvironmentVariables = findInput( + getDeployInputs(compiled.module), + "environment_variables", + ); + assert.equal(deployEnvironmentVariables.label, "Environment variables"); + assert.equal(deployEnvironmentVariables.type, "array"); + assert.equal(deployEnvironmentVariables.required, false); + assert.equal( + deployEnvironmentVariables.placeholder, + "- name: LOG_LEVEL\n value: info", + ); + assert.equal( + deployEnvironmentVariables.description, + [ + "Plain environment variables to set for this deploy.", + "Only names listed in the module's", + "Deploy-provided environment variables input are applied; others are ignored.", + "Can't replace a runtime variable or secret.", + ].join(" "), + ); return values.env; }; assert.equal(await helmEnv("worker"), deployEnv); From 90f804850da024141ca73decd36ecf370af3e6cb Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 27 Oct 2026 22:17:41 +0000 Subject: [PATCH 4/4] Add ECS deploy environment allowlists Co-Authored-By: brandon --- .github/workflows/module-definitions.yml | 2 +- README.md | 6 +- .../ecs_service/rvn-ecs-nlb-definition.yml | 26 +- .../ecs_service/rvn-ecs-web-definition.yml | 26 +- .../ecs_service/rvn-ecs-worker-definition.yml | 29 +- partials/deploy/ecs-service-deploy-common.yml | 11 + .../inputs/eks-workload-runtime-inputs.yml | 2 +- .../ecs-service-runtime-env-inputs.yml | 7 + .../deploy_env_allowlist_test.go | 302 +++++++++++++++++- tools/ravion-modules/test/compiler.test.ts | 44 ++- 10 files changed, 421 insertions(+), 34 deletions(-) diff --git a/.github/workflows/module-definitions.yml b/.github/workflows/module-definitions.yml index ba1963ed..60dde8d6 100644 --- a/.github/workflows/module-definitions.yml +++ b/.github/workflows/module-definitions.yml @@ -50,7 +50,7 @@ jobs: cache-dependency-path: test/go.sum - name: Test deploy environment expression runtime - run: go test -run '^TestDeployEnvironmentVariableAllowlist$' -count=1 ./deploy_env_allowlist + run: go test -run '^(TestDeployEnvironmentVariableAllowlist|TestECSDeployEnvironmentVariableAllowlist)$' -count=1 ./deploy_env_allowlist working-directory: test - name: Run migration guardrails diff --git a/README.md b/README.md index 663c884b..0d3c5293 100644 --- a/README.md +++ b/README.md @@ -81,9 +81,9 @@ sync by `node tools/ravion-modules/dist/src/cli.js readme` (enforced in CI, and | `rvn-cloudfront` | CloudFront CDN | v1.7.0 | `cdn/cloudfront/` | | `rvn-ec2-service` | EC2 Service | v2.4.0 | `compute/ec2_service/` | | `rvn-ecs-cluster` | ECS Cluster | v2.4.0 | `compute/ecs_cluster/` | -| `rvn-ecs-nlb` | ECS Network Service | v2.4.2 | `compute/ecs_service/` | -| `rvn-ecs-web` | ECS Web Service | v2.6.0 | `compute/ecs_service/` | -| `rvn-ecs-worker` | ECS Worker | v2.3.2 | `compute/ecs_service/` | +| `rvn-ecs-nlb` | ECS Network Service | v2.5.0 | `compute/ecs_service/` | +| `rvn-ecs-web` | ECS Web Service | v2.7.0 | `compute/ecs_service/` | +| `rvn-ecs-worker` | ECS Worker | v2.4.0 | `compute/ecs_service/` | | `rvn-efs` | EFS File System | v1.1.1 | `storage/efs/` | | `rvn-elasticache` | ElastiCache | v1.1.2 | `cache/elasticache/` | | `rvn-lambda` | Lambda Function | v2.3.0 | `compute/lambda/` | diff --git a/compute/ecs_service/rvn-ecs-nlb-definition.yml b/compute/ecs_service/rvn-ecs-nlb-definition.yml index 749a71d6..4b643f7b 100644 --- a/compute/ecs_service/rvn-ecs-nlb-definition.yml +++ b/compute/ecs_service/rvn-ecs-nlb-definition.yml @@ -3,8 +3,8 @@ definition: name: ECS Network Service description: Network Load Balanced ECS service for exposing one to five TCP, UDP, or TLS ports through an ECS cluster Network Load Balancer. release: - version: 2.4.2 - description: 'Lock the AWS provider to 6.69.1 (from 6.69.0), fixing plans that failed with `bufio.Scanner: token too long` on large AWS responses (e.g. Lambda functions with many published versions).' + version: 2.5.0 + description: Let pipeline deploy steps set allowlisted environment variables, such as a release version computed earlier in the pipeline. module: inputs: - id: section_cluster @@ -363,8 +363,14 @@ module: > 0 ? module.input.image_start_command : nil), "cpu": (module.input.capacity_provider == "ec2" ? int(float(module.input.task_cpu) * 1024) : int(float(module.input.fargate_size.vcpu) * 1024)), "depends_on": [{"container_name": "log_router", "condition": "START"}], "environment": ([{name: - "PORT", value: string(module.input.listeners[0].container_port)}] | concat(module.input.environment_variables != - nil ? module.input.environment_variables : [])), "essential": true, "image": + "PORT", value: string(module.input.listeners[0].container_port)}] | + concat(module.input.environment_variables != nil ? module.input.environment_variables : []) | + concat(module.input.deploy_environment_variable_names != nil && deploy.input.environment_variables != nil ? + map(filter(deploy.input.environment_variables, #.name != "PORT" && + #.name in module.input.deploy_environment_variable_names && #.value != nil && + !(#.name in map(module.input.environment_variables ?? [], .name)) && + !(#.name in map(module.input.secrets ?? [], .name))), + {"name": #.name, "value": string(#.value)}) : [])), "essential": true, "image": (module.input.build_source == "image_registry" ? (deploy.input.image_ref contains "sha256:" ? module.input.image_repository + "@" + deploy.input.image_ref : module.input.image_repository + ":" + deploy.input.image_ref) : (deploy.input.image_ref contains @@ -401,7 +407,12 @@ module: (module.input.capacity_provider == "ec2" ? int(float(module.input.task_cpu) * 1024) : int(float(module.input.fargate_size.vcpu) * 1024)), "environment": ([{name: "PORT", value: string(module.input.listeners[0].container_port)}] | concat(module.input.environment_variables != nil ? - module.input.environment_variables : [])), "essential": true, "image": (module.input.build_source == + module.input.environment_variables : []) | concat(module.input.deploy_environment_variable_names != nil && + deploy.input.environment_variables != nil ? map(filter(deploy.input.environment_variables, #.name != "PORT" && + #.name in module.input.deploy_environment_variable_names && #.value != nil && + !(#.name in map(module.input.environment_variables ?? [], .name)) && + !(#.name in map(module.input.secrets ?? [], .name))), {"name": #.name, "value": string(#.value)}) : [])), + "essential": true, "image": (module.input.build_source == "image_registry" ? (deploy.input.image_ref contains "sha256:" ? module.input.image_repository + "@" + deploy.input.image_ref : module.input.image_repository + ":" + deploy.input.image_ref) : (deploy.input.image_ref contains "sha256:" ? @@ -592,7 +603,9 @@ module: ## Application configuration - Use Runtime environment variables for plain container environment values. Ravion also sets PORT from the first listener's Container port. + Use **Runtime environment variables** for plain values, as an array of `{name, value}` objects. Ravion also sets `PORT` from the first listener's Container port. + + Use **Deploy-provided environment variables** to allow a pipeline deploy step to set selected plain values. They cannot replace a Runtime environment variable or Runtime secret, and `PORT` is reserved for the network service. Use Runtime secrets for sensitive values. Secrets are injected into the ECS task from SSM Parameter Store or Secrets Manager using name and value_from objects. @@ -658,6 +671,7 @@ module: | Listener port | Yes | 80 | Unique NLB port for each listener | | Listener protocol | Yes | TCP | TCP, TLS, or UDP for each listener | | Container port | Yes | 3000 | Container port targeted by each listener | + | Deploy-provided environment variables | No | - | Names a pipeline deploy step may set to plain values; `PORT` is reserved | | TLS certificate | Yes* | - | Required for each TLS listener | | TLS SSL policy | No | ELBSecurityPolicy-TLS13-1-2-2021-06 | SSL policy for each TLS listener | | TLS ALPN policy | No | None | Optional ALPN behavior for each TLS listener | diff --git a/compute/ecs_service/rvn-ecs-web-definition.yml b/compute/ecs_service/rvn-ecs-web-definition.yml index 7552c675..e414ab63 100644 --- a/compute/ecs_service/rvn-ecs-web-definition.yml +++ b/compute/ecs_service/rvn-ecs-web-definition.yml @@ -3,8 +3,8 @@ definition: name: ECS Web Service description: Web server ECS service for running an HTTP application behind an ECS cluster load balancer. release: - version: 2.6.0 - description: Link every domain the module serves from the Links section of its overview page. + version: 2.7.0 + description: Let pipeline deploy steps set allowlisted environment variables, such as a release version computed earlier in the pipeline. module: inputs: - id: section_cluster @@ -559,8 +559,14 @@ module: > 0 ? module.input.image_start_command : nil), "cpu": (module.input.capacity_provider == "ec2" ? int(float(module.input.task_cpu) * 1024) : int(float(module.input.fargate_size.vcpu) * 1024)), "depends_on": [{"container_name": "log_router", "condition": "START"}], "environment": ([{name: - "PORT", value: string(module.input.container_port)}] | concat(module.input.environment_variables != - nil ? module.input.environment_variables : [])), "essential": true, "image": + "PORT", value: string(module.input.container_port)}] | + concat(module.input.environment_variables != nil ? module.input.environment_variables : []) | + concat(module.input.deploy_environment_variable_names != nil && deploy.input.environment_variables != nil ? + map(filter(deploy.input.environment_variables, #.name != "PORT" && + #.name in module.input.deploy_environment_variable_names && #.value != nil && + !(#.name in map(module.input.environment_variables ?? [], .name)) && + !(#.name in map(module.input.secrets ?? [], .name))), + {"name": #.name, "value": string(#.value)}) : [])), "essential": true, "image": (module.input.build_source == "image_registry" ? (deploy.input.image_ref contains "sha256:" ? module.input.image_repository + "@" + deploy.input.image_ref : module.input.image_repository + ":" + deploy.input.image_ref) : (deploy.input.image_ref contains @@ -597,7 +603,12 @@ module: (module.input.capacity_provider == "ec2" ? int(float(module.input.task_cpu) * 1024) : int(float(module.input.fargate_size.vcpu) * 1024)), "environment": ([{name: "PORT", value: string(module.input.container_port)}] | concat(module.input.environment_variables != nil ? - module.input.environment_variables : [])), "essential": true, "image": (module.input.build_source == + module.input.environment_variables : []) | concat(module.input.deploy_environment_variable_names != nil && + deploy.input.environment_variables != nil ? map(filter(deploy.input.environment_variables, #.name != "PORT" && + #.name in module.input.deploy_environment_variable_names && #.value != nil && + !(#.name in map(module.input.environment_variables ?? [], .name)) && + !(#.name in map(module.input.secrets ?? [], .name))), {"name": #.name, "value": string(#.value)}) : [])), + "essential": true, "image": (module.input.build_source == "image_registry" ? (deploy.input.image_ref contains "sha256:" ? module.input.image_repository + "@" + deploy.input.image_ref : module.input.image_repository + ":" + deploy.input.image_ref) : (deploy.input.image_ref contains "sha256:" ? @@ -837,7 +848,9 @@ module: ## Application configuration - Use Runtime environment variables for plain container environment values. Ravion also sets PORT from Container port. + Use **Runtime environment variables** for plain values, as an array of `{name, value}` objects. Ravion also sets `PORT` from Container port. + + Use **Deploy-provided environment variables** to allow a pipeline deploy step to set selected plain values. They cannot replace a Runtime environment variable or Runtime secret, and `PORT` is reserved for the web service. Use Runtime secrets for sensitive values. Secrets are injected into the ECS task from SSM Parameter Store or Secrets Manager using name and value_from objects. @@ -958,6 +971,7 @@ module: | Image repository | Yes* | nginx | Required for image registry deployments | | Start command | No | [] | Command arguments that override an image default CMD | | Container port | Yes | 80 | Port exposed by the app container | + | Deploy-provided environment variables | No | - | Names a pipeline deploy step may set to plain values; `PORT` is reserved | | Health check path | Yes | / | HTTP path used by the target group health check | | Deregistration delay (secs) | No | 300 | Drain window for a task that is deregistering | | Sticky sessions | No | true | Keep clients on the same task, and on the same traffic-shift target group when enabled | diff --git a/compute/ecs_service/rvn-ecs-worker-definition.yml b/compute/ecs_service/rvn-ecs-worker-definition.yml index 90c8a76d..f35cb925 100644 --- a/compute/ecs_service/rvn-ecs-worker-definition.yml +++ b/compute/ecs_service/rvn-ecs-worker-definition.yml @@ -3,8 +3,8 @@ definition: name: ECS Worker description: Background worker ECS service for running private container workloads without exposed ports. release: - version: 2.3.2 - description: 'Lock the AWS provider to 6.69.1 (from 6.69.0), fixing plans that failed with `bufio.Scanner: token too long` on large AWS responses (e.g. Lambda functions with many published versions).' + version: 2.4.0 + description: Let pipeline deploy steps set allowlisted environment variables, such as a release version computed earlier in the pipeline. module: inputs: - id: section_cluster @@ -100,7 +100,12 @@ module: > 0 ? module.input.image_start_command : nil), "cpu": (module.input.capacity_provider == "ec2" ? int(float(module.input.task_cpu) * 1024) : int(float(module.input.fargate_size.vcpu) * 1024)), "depends_on": [{"container_name": "log_router", "condition": "START"}], "environment": - (module.input.environment_variables != nil ? module.input.environment_variables : []), "essential": true, + ((module.input.environment_variables != nil ? module.input.environment_variables : []) | + concat(module.input.deploy_environment_variable_names != nil && deploy.input.environment_variables != nil ? + map(filter(deploy.input.environment_variables, #.name in module.input.deploy_environment_variable_names && + #.value != nil && !(#.name in map(module.input.environment_variables ?? [], .name)) && + !(#.name in map(module.input.secrets ?? [], .name))), + {"name": #.name, "value": string(#.value)}) : [])), "essential": true, "image": (module.input.build_source == "image_registry" ? (deploy.input.image_ref contains "sha256:" ? module.input.image_repository + "@" + deploy.input.image_ref : module.input.image_repository + ":" + deploy.input.image_ref) : (deploy.input.image_ref contains "sha256:" ? @@ -131,8 +136,13 @@ module: "enable-ecs-log-metadata": string(module.input.firelens_ecs_metadata_enabled)}}}] : [{"command": (len(module.input.image_start_command) > 0 ? module.input.image_start_command : nil), "cpu": (module.input.capacity_provider == "ec2" ? int(float(module.input.task_cpu) * 1024) : - int(float(module.input.fargate_size.vcpu) * 1024)), "environment": (module.input.environment_variables != nil ? - module.input.environment_variables : []), "essential": true, "image": (module.input.build_source == + int(float(module.input.fargate_size.vcpu) * 1024)), "environment": + ((module.input.environment_variables != nil ? module.input.environment_variables : []) | + concat(module.input.deploy_environment_variable_names != nil && deploy.input.environment_variables != nil ? + map(filter(deploy.input.environment_variables, #.name in module.input.deploy_environment_variable_names && + #.value != nil && !(#.name in map(module.input.environment_variables ?? [], .name)) && + !(#.name in map(module.input.secrets ?? [], .name))), + {"name": #.name, "value": string(#.value)}) : [])), "essential": true, "image": (module.input.build_source == "image_registry" ? (deploy.input.image_ref contains "sha256:" ? module.input.image_repository + "@" + deploy.input.image_ref : module.input.image_repository + ":" + deploy.input.image_ref) : (deploy.input.image_ref contains "sha256:" ? stack.output.ecr_repository_url + "@" + deploy.input.image_ref : @@ -219,6 +229,14 @@ module: The running task alarm uses the Container Insights `RunningTaskCount` metric and stays silent when Container Insights is disabled on the cluster. Every alarm treats missing data as not breaching and must breach for the configured number of consecutive periods before it fires. Add SNS topic ARNs as Alarm actions and OK actions to be notified. + ## Application configuration + + Use **Runtime environment variables** for plain values, as an array of `{name, value}` objects. + + Use **Deploy-provided environment variables** to allow a pipeline deploy step to set selected plain values. They cannot replace a Runtime environment variable or Runtime secret. + + Use **Runtime secrets** for sensitive values, as an array of `{name, value_from}` objects. `value_from` is an SSM Parameter Store or Secrets Manager ARN. + ## Configuration | Field | Required | Default | Description | @@ -236,6 +254,7 @@ module: | Custom metric scaling policies | No | [] | Queue/backlog-oriented target tracking policies | | Runtime environment variables | No | - | Environment variables passed to the worker container | | Runtime secrets | No | - | Secrets injected from SSM Parameter Store or Secrets Manager | + | Deploy-provided environment variables | No | - | Names a pipeline deploy step may set to plain values | | Stop timeout (secs) | No | 30 | Seconds between SIGTERM and SIGKILL when a worker task stops; Fargate allows up to 120 | | Sidecars | No | [] | Additional containers in the worker task | | EFS file system | No | false | Mount an EFS file system into the worker container | diff --git a/partials/deploy/ecs-service-deploy-common.yml b/partials/deploy/ecs-service-deploy-common.yml index 3cc0fd63..0567c633 100644 --- a/partials/deploy/ecs-service-deploy-common.yml +++ b/partials/deploy/ecs-service-deploy-common.yml @@ -11,6 +11,17 @@ inputs: description: Pass the image tag or digest to deploy. For Dockerfile or Railpack builds, this is resolved in the Ravion-created ECR repository. For Pull from image registry mode, this is resolved in the repository configured on the module. Do not pass a full image URI. placeholder: sha256:... or latest or -railpack required: true + - id: environment_variables + label: Environment variables + type: array + description: >- + Plain environment variables to set for this deploy. Only names listed in the module's + Deploy-provided environment variables input are applied; others are ignored. Can't replace + a runtime variable or secret. + placeholder: |- + - name: LOG_LEVEL + value: info + required: false pre_deploy: >- << module.input.pre_deploy_enabled && len(module.input.pre_deploy_command) > 0 ? {"container_overrides": [{"name": stack.output.container_name, "command": module.input.pre_deploy_command, "environment": module.input.pre_deploy_environment_variables, diff --git a/partials/inputs/eks-workload-runtime-inputs.yml b/partials/inputs/eks-workload-runtime-inputs.yml index 02c4f807..3a1b4059 100644 --- a/partials/inputs/eks-workload-runtime-inputs.yml +++ b/partials/inputs/eks-workload-runtime-inputs.yml @@ -28,6 +28,6 @@ label: Deploy-provided environment variables type: string_array description: Names a pipeline deploy step may set with `input.environment_variables`. Plain values only; can't replace a runtime variable or secret. - add_button_label: Add name + add_button_label: Add env var name collapsible: true required: false diff --git a/partials/templates/ecs-service-runtime-env-inputs.yml b/partials/templates/ecs-service-runtime-env-inputs.yml index 4223e707..a75ae44e 100644 --- a/partials/templates/ecs-service-runtime-env-inputs.yml +++ b/partials/templates/ecs-service-runtime-env-inputs.yml @@ -17,3 +17,10 @@ placeholder: |- - name: DATABASE_URL value_from: arn:aws:ssm:us-east-1:123456789012:parameter/my-app/database-url +- id: deploy_environment_variable_names + label: Deploy-provided environment variables + type: string_array + description: Names a pipeline deploy step may set with `input.environment_variables` for the primary container. Plain values only; can't replace a runtime variable or secret. + add_button_label: Add env var name + collapsible: true + required: false diff --git a/test/deploy_env_allowlist/deploy_env_allowlist_test.go b/test/deploy_env_allowlist/deploy_env_allowlist_test.go index 60ae8b38..ff9e98cf 100644 --- a/test/deploy_env_allowlist/deploy_env_allowlist_test.go +++ b/test/deploy_env_allowlist/deploy_env_allowlist_test.go @@ -1,6 +1,7 @@ package deployenv import ( + "fmt" "os" "path/filepath" "reflect" @@ -183,9 +184,217 @@ func TestDeployEnvironmentVariableAllowlist(t *testing.T) { } } +func TestECSDeployEnvironmentVariableAllowlist(t *testing.T) { + type fixture struct { + name string + allowlist []interface{} + setAllowlist bool + moduleEnvironment []interface{} + secrets []interface{} + deployEnvironment []interface{} + omitDeployEnvironment bool + wantDeployEnvironment []interface{} + } + + workloads := []struct { + name string + path string + portValue int + portInputID string + }{ + { + name: "web", + path: filepath.Join("..", "..", "compute", "ecs_service", "rvn-ecs-web-definition.yml"), + portValue: 8080, + portInputID: "container_port", + }, + { + name: "worker", + path: filepath.Join("..", "..", "compute", "ecs_service", "rvn-ecs-worker-definition.yml"), + }, + { + name: "nlb", + path: filepath.Join("..", "..", "compute", "ecs_service", "rvn-ecs-nlb-definition.yml"), + portValue: 3000, + portInputID: "listeners", + }, + } + fixtures := []fixture{ + { + name: "allowlist unset", + deployEnvironment: []interface{}{ + envEntry("RUNNER_VERSION", "abc123def456"), + }, + }, + { + name: "allowlist empty", + allowlist: []interface{}{}, + setAllowlist: true, + deployEnvironment: []interface{}{ + envEntry("RUNNER_VERSION", "abc123def456"), + }, + }, + { + name: "allowlisted value is kept and unlisted value is dropped", + allowlist: []interface{}{"RUNNER_VERSION"}, + setAllowlist: true, + deployEnvironment: []interface{}{ + envEntry("RUNNER_VERSION", "abc123def456"), + envEntry("HTTPS_PROXY", "proxy"), + }, + wantDeployEnvironment: []interface{}{envEntry("RUNNER_VERSION", "abc123def456")}, + }, + { + name: "module environment variable collision keeps the module value", + allowlist: []interface{}{"SHARED_VALUE"}, + setAllowlist: true, + moduleEnvironment: []interface{}{ + envEntry("MODULE_VALUE", "module"), + envEntry("SHARED_VALUE", "module"), + }, + deployEnvironment: []interface{}{envEntry("SHARED_VALUE", "deploy")}, + wantDeployEnvironment: []interface{}{}, + }, + { + name: "module secret collision is dropped", + allowlist: []interface{}{"SECRET_VALUE"}, + setAllowlist: true, + secrets: []interface{}{map[string]interface{}{"name": "SECRET_VALUE"}}, + deployEnvironment: []interface{}{ + envEntry("SECRET_VALUE", "deploy"), + }, + }, + { + name: "value_from without a plain value is dropped", + allowlist: []interface{}{"SECRET_VALUE"}, + setAllowlist: true, + deployEnvironment: []interface{}{ + map[string]interface{}{ + "name": "SECRET_VALUE", + "value_from": map[string]interface{}{"secret_key_ref": "secret"}, + }, + }, + }, + { + name: "missing deploy environment_variables is safe", + allowlist: []interface{}{"RUNNER_VERSION"}, + setAllowlist: true, + omitDeployEnvironment: true, + }, + { + name: "non-string values are stringified", + allowlist: []interface{}{"RETRY_COUNT"}, + setAllowlist: true, + deployEnvironment: []interface{}{ + envEntry("RETRY_COUNT", 3), + }, + wantDeployEnvironment: []interface{}{envEntry("RETRY_COUNT", "3")}, + }, + } + + for _, workload := range workloads { + expressions := readAppContainerEnvironmentExpressions(t, workload.path) + if len(expressions) != 2 { + t.Fatalf("%s has %d app-container environment expressions, want 2", workload.name, len(expressions)) + } + + workloadFixtures := append([]fixture{}, fixtures...) + if workload.portValue != 0 { + workloadFixtures = append(workloadFixtures, fixture{ + name: "reserved PORT cannot be overridden", + allowlist: []interface{}{"PORT"}, + setAllowlist: true, + deployEnvironment: []interface{}{ + envEntry("PORT", "9090"), + }, + }) + } + + for expressionIndex, expression := range expressions { + for _, fixture := range workloadFixtures { + t.Run(fmt.Sprintf("%s/app_variant_%d/%s", workload.name, expressionIndex+1, fixture.name), func(t *testing.T) { + moduleEnvironment := fixture.moduleEnvironment + if moduleEnvironment == nil { + moduleEnvironment = []interface{}{envEntry("MODULE_VALUE", "module")} + } + + input := map[string]interface{}{ + "environment_variables": moduleEnvironment, + } + switch workload.portInputID { + case "container_port": + input["container_port"] = workload.portValue + case "listeners": + input["listeners"] = []interface{}{ + map[string]interface{}{"container_port": workload.portValue}, + } + } + if fixture.setAllowlist { + input["deploy_environment_variable_names"] = fixture.allowlist + } + if fixture.secrets != nil { + input["secrets"] = fixture.secrets + } + + deployInput := map[string]interface{}{} + if !fixture.omitDeployEnvironment { + deployInput["environment_variables"] = fixture.deployEnvironment + } + + context := map[string]interface{}{ + "module": map[string]interface{}{"input": input}, + "deploy": map[string]interface{}{"input": deployInput}, + } + program, err := expr.Compile(expression, expr.Env(context)) + if err != nil { + t.Fatalf("compile %s app-container environment: %v", workload.name, err) + } + + result, err := expr.Run(program, context) + if err != nil { + t.Fatalf("evaluate %s app-container environment: %v", workload.name, err) + } + + actual, ok := result.([]interface{}) + if !ok { + t.Fatalf("%s app-container environment returned %T, want []interface{}", workload.name, result) + } + want := make([]interface{}, 0, len(moduleEnvironment)+len(fixture.wantDeployEnvironment)+1) + if workload.portValue != 0 { + want = append(want, envEntry("PORT", fmt.Sprint(workload.portValue))) + } + want = append(want, moduleEnvironment...) + want = append(want, fixture.wantDeployEnvironment...) + if !reflect.DeepEqual(actual, want) { + t.Fatalf("%s app-container environment = %#v, want %#v", workload.name, actual, want) + } + }) + } + } + } +} + func readEnvExpression(t *testing.T, path string) string { t.Helper() + expressions := readTemplateExpressions(t, path, "env") + var allowlistExpressions []string + for _, expression := range expressions { + if strings.Contains(expression, "deploy_environment_variable_names") { + allowlistExpressions = append(allowlistExpressions, expression) + } + } + + if len(allowlistExpressions) != 1 { + t.Fatalf("found %d deploy env expressions in %s, want exactly one", len(allowlistExpressions), path) + } + + return normalizeMapTransform(t, path, allowlistExpressions[0]) +} + +func readTemplateExpressions(t *testing.T, path string, key string) []string { + t.Helper() + source, err := os.ReadFile(path) if err != nil { t.Fatalf("read %s: %v", path, err) @@ -194,7 +403,7 @@ func readEnvExpression(t *testing.T, path string) string { lines := strings.Split(string(source), "\n") var expressions []string for index, line := range lines { - if strings.TrimSpace(line) != "env: >-" { + if strings.TrimSpace(line) != key+": >-" { continue } @@ -211,25 +420,96 @@ func readEnvExpression(t *testing.T, path string) string { scalarLines = append(scalarLines, strings.TrimSpace(scalarLine)) } - scalar := strings.Join(scalarLines, " ") - if strings.Contains(scalar, "deploy_environment_variable_names") { - expressions = append(expressions, scalar) + template := strings.Join(scalarLines, " ") + if !strings.HasPrefix(template, "<<") || !strings.HasSuffix(template, ">>") { + t.Fatalf("%s expression in %s is not wrapped in << >>: %q", key, path, template) } + expression := strings.TrimSpace(strings.TrimSuffix(strings.TrimPrefix(template, "<<"), ">>")) + expressions = append(expressions, expression) + } + return expressions +} + +func readAppContainerEnvironmentExpressions(t *testing.T, path string) []string { + t.Helper() + + templates := readTemplateExpressions(t, path, "container_definitions") + if len(templates) != 1 { + t.Fatalf("found %d container_definitions expressions in %s, want exactly one", len(templates), path) } - if len(expressions) != 1 { - t.Fatalf("found %d deploy env expressions in %s, want exactly one", len(expressions), path) + const property = `"environment":` + expression := templates[0] + var environments []string + for offset := 0; offset < len(expression); { + propertyOffset := strings.Index(expression[offset:], property) + if propertyOffset < 0 { + break + } + valueStart := offset + propertyOffset + len(property) + for valueStart < len(expression) && (expression[valueStart] == ' ' || expression[valueStart] == '\t') { + valueStart++ + } + value, nextOffset, err := readExpressionValue(expression, valueStart) + if err != nil { + t.Fatalf("extract app-container environment from %s: %v", path, err) + } + if strings.Contains(value, "module.input.environment_variables") && + !strings.Contains(value, "FIRELENS_CONFIG_CONTENT") { + environments = append(environments, normalizeMapTransform(t, path, value)) + } + offset = nextOffset } + return environments +} - template := strings.TrimSpace(expressions[0]) - if !strings.HasPrefix(template, "<<") || !strings.HasSuffix(template, ">>") { - t.Fatalf("env expression in %s is not wrapped in << >>: %q", path, template) +func readExpressionValue(source string, start int) (string, int, error) { + depth := 0 + var quote byte + escaped := false + for index := start; index < len(source); index++ { + character := source[index] + if quote != 0 { + if escaped { + escaped = false + continue + } + if character == '\\' { + escaped = true + continue + } + if character == quote { + quote = 0 + } + continue + } + + switch character { + case '"', '\'': + quote = character + case '(', '[', '{': + depth++ + case ')', ']', '}': + depth-- + if depth < 0 { + return "", index, fmt.Errorf("unbalanced delimiter at byte %d", index) + } + case ',': + if depth == 0 { + return strings.TrimSpace(source[start:index]), index + 1, nil + } + } } - expression := strings.TrimSpace(strings.TrimSuffix(strings.TrimPrefix(template, "<<"), ">>")) + return "", len(source), fmt.Errorf("unterminated expression value beginning at byte %d", start) +} + +func normalizeMapTransform(t *testing.T, path string, expression string) string { + t.Helper() + rawTransform := `, {"name": #.name, "value": string(#.value)})` normalizedTransform := `, true ? {"name": #.name, "value": string(#.value)} : nil)` if strings.Count(expression, rawTransform) != 1 { - t.Fatalf("found unexpected map transform in %s", path) + t.Fatalf("found unexpected map transform in %s: %s", path, expression) } return strings.Replace(expression, rawTransform, normalizedTransform, 1) } diff --git a/tools/ravion-modules/test/compiler.test.ts b/tools/ravion-modules/test/compiler.test.ts index 27109fd4..e9c05348 100644 --- a/tools/ravion-modules/test/compiler.test.ts +++ b/tools/ravion-modules/test/compiler.test.ts @@ -501,7 +501,9 @@ describe("compiler", () => { const helmEnv = async (name: string) => { const compiled = await compileDefinitionFile(join(repoRoot, "compute", "eks_service", `rvn-eks-${name}-definition.yml`)); const inputs = getModuleInputs(compiled.module); - assert.equal(findInput(inputs, "deploy_environment_variable_names").type, "string_array"); + const allowlist = findInput(inputs, "deploy_environment_variable_names"); + assert.equal(allowlist.type, "string_array"); + assert.equal(allowlist.add_button_label, "Add env var name"); const deploy = assertRecord(compiled.module.deploy, `${name}.deploy`); const definition = assertRecord(deploy.definition, `${name}.deploy.definition`); const values = assertRecord(definition.values, `${name}.deploy.definition.values`); @@ -535,6 +537,46 @@ describe("compiler", () => { assert.equal(await helmEnv("cron"), deployEnv); }); + it("exposes allowlisted deploy environment variable inputs for ECS services", async () => { + for (const name of ["web", "worker", "nlb"]) { + const compiled = await compileDefinitionFile( + join(repoRoot, "compute", "ecs_service", `rvn-ecs-${name}-definition.yml`), + ); + const inputs = getModuleInputs(compiled.module); + const allowlist = findInput(inputs, "deploy_environment_variable_names"); + assert.equal(allowlist.label, "Deploy-provided environment variables"); + assert.equal(allowlist.type, "string_array"); + assert.equal(allowlist.add_button_label, "Add env var name"); + assert.equal(allowlist.collapsible, true); + assert.equal(allowlist.required, false); + assert.equal( + allowlist.description, + "Names a pipeline deploy step may set with `input.environment_variables` for the primary container. Plain values only; can't replace a runtime variable or secret.", + ); + + const deployEnvironmentVariables = findInput( + getDeployInputs(compiled.module), + "environment_variables", + ); + assert.equal(deployEnvironmentVariables.label, "Environment variables"); + assert.equal(deployEnvironmentVariables.type, "array"); + assert.equal(deployEnvironmentVariables.required, false); + assert.equal( + deployEnvironmentVariables.placeholder, + "- name: LOG_LEVEL\n value: info", + ); + assert.equal( + deployEnvironmentVariables.description, + [ + "Plain environment variables to set for this deploy.", + "Only names listed in the module's", + "Deploy-provided environment variables input are applied; others are ignored.", + "Can't replace a runtime variable or secret.", + ].join(" "), + ); + } + }); + it("puts every EKS workload environment variable in the Environment variables section, as on ECS", async () => { for (const name of ["web", "worker", "cron"]) { const compiled = await compileDefinitionFile(join(repoRoot, "compute", "eks_service", `rvn-eks-${name}-definition.yml`));