feat(ecs): resolve containerDefinitions[].secrets from SSM / Secrets Manager#1687
feat(ecs): resolve containerDefinitions[].secrets from SSM / Secrets Manager#1687hampsterx wants to merge 1 commit into
Conversation
RegisterTaskDefinition now parses and stores containerDefinitions[].secrets (name + valueFrom) and round-trips them on register/describe as references, matching AWS (the resolved values are never returned). At RunTask launch, EcsContainerManager resolves each secret to an environment variable through the in-process SsmService / SecretsManagerService (the same local resolution CodeBuildRunner already uses, no extra HTTP hops): - SSM parameter by name or full ARN -> Parameter.value - Secrets Manager full ARN -> SecretString - a full ARN's own region is used, so cross-region references resolve correctly - env precedence: task-def environment < resolved secrets < RunTask container override environment (override still wins on a name collision) When a referenced secret/parameter cannot be resolved the launch fails the way AWS does: no container is started and the task goes STOPPED with a ResourceInitializationError stoppedReason, rather than silently starting the container without the variable. Deferred (noted in floci-io#1624): the Secrets Manager valueFrom selector suffix (:json-key:version-stage:version-id) is not yet parsed; such a valueFrom is resolved as the plain secret. Adds handler round-trip tests, container-manager resolution / override / cross-region unit tests, a stopped-task failure test, and Java SDK compatibility coverage.
|
| Filename | Overview |
|---|---|
| src/main/java/io/github/hectorvent/floci/services/ecs/container/EcsContainerManager.java | Adds secret resolution at task launch; the fail-fast pre-resolution loop and cross-region ARN handling are correct, but null SecretString from a binary Secrets Manager secret is silently formatted as the literal string "null" rather than raising ResourceInitializationError. |
| src/main/java/io/github/hectorvent/floci/services/ecs/EcsJsonHandler.java | Adds secrets[] serialisation/deserialisation; parse and emit are symmetric and never leak resolved values, matching AWS behaviour. |
| src/main/java/io/github/hectorvent/floci/services/ecs/model/Secret.java | New record type for secret references; minimal and correct. |
| src/main/java/io/github/hectorvent/floci/services/ecs/model/ContainerDefinition.java | Adds List secrets field with getter/setter; no issues. |
| src/test/java/io/github/hectorvent/floci/services/ecs/container/EcsContainerManagerSecretsTest.java | Good unit test coverage for SSM name, SM ARN, cross-region ARN, override precedence, and unresolved secret; does not cover binary (null SecretString) case. |
| src/test/java/io/github/hectorvent/floci/services/ecs/EcsServiceContainerSecretsTest.java | Integration-style test verifying STOPPED task with ResourceInitializationError when containerManager.startTask() throws AwsException. |
| src/test/java/io/github/hectorvent/floci/services/ecs/EcsIntegrationTest.java | Adds register/describe round-trip test for secrets; order renumbering is consistent with the insertion of the new @order(16) test. |
| compatibility-tests/sdk-test-java/src/test/java/com/floci/test/EcsTests.java | Adds SDK-level register+describe round-trip for secrets; straightforward and correct. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Client
participant EcsJsonHandler
participant EcsService
participant EcsContainerManager
participant SsmService
participant SecretsManagerService
participant Docker
Client->>EcsJsonHandler: RunTask (task def with secrets[])
EcsJsonHandler->>EcsService: runTask(...)
EcsService->>EcsContainerManager: startTask(task, taskDef, overrides, region)
Note over EcsContainerManager: Pre-resolution pass (fail-fast)
loop For each ContainerDefinition
EcsContainerManager->>EcsContainerManager: buildEnvVars(def, override, region)
loop For each secret
alt valueFrom starts with arn:aws:secretsmanager:
EcsContainerManager->>SecretsManagerService: getSecretValue(arn, region)
SecretsManagerService-->>EcsContainerManager: SecretVersion.getSecretString()
else SSM name or ARN
EcsContainerManager->>SsmService: getParameter(name, region)
SsmService-->>EcsContainerManager: Parameter.getValue()
end
end
end
alt Any secret resolution fails
EcsContainerManager-->>EcsService: throws AwsException (ResourceInitializationError)
EcsService-->>Client: task STOPPED with stoppedReason
else All secrets resolved
loop For each ContainerDefinition
EcsContainerManager->>Docker: createAndStart(containerSpec with env vars)
Docker-->>EcsContainerManager: ContainerInfo
end
EcsContainerManager-->>EcsService: EcsTaskHandle
EcsService-->>Client: tasks[] RUNNING
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Client
participant EcsJsonHandler
participant EcsService
participant EcsContainerManager
participant SsmService
participant SecretsManagerService
participant Docker
Client->>EcsJsonHandler: RunTask (task def with secrets[])
EcsJsonHandler->>EcsService: runTask(...)
EcsService->>EcsContainerManager: startTask(task, taskDef, overrides, region)
Note over EcsContainerManager: Pre-resolution pass (fail-fast)
loop For each ContainerDefinition
EcsContainerManager->>EcsContainerManager: buildEnvVars(def, override, region)
loop For each secret
alt valueFrom starts with arn:aws:secretsmanager:
EcsContainerManager->>SecretsManagerService: getSecretValue(arn, region)
SecretsManagerService-->>EcsContainerManager: SecretVersion.getSecretString()
else SSM name or ARN
EcsContainerManager->>SsmService: getParameter(name, region)
SsmService-->>EcsContainerManager: Parameter.getValue()
end
end
end
alt Any secret resolution fails
EcsContainerManager-->>EcsService: throws AwsException (ResourceInitializationError)
EcsService-->>Client: task STOPPED with stoppedReason
else All secrets resolved
loop For each ContainerDefinition
EcsContainerManager->>Docker: createAndStart(containerSpec with env vars)
Docker-->>EcsContainerManager: ContainerInfo
end
EcsContainerManager-->>EcsService: EcsTaskHandle
EcsService-->>Client: tasks[] RUNNING
end
Reviews (1): Last reviewed commit: "feat(ecs): resolve containerDefinitions[..." | Re-trigger Greptile
| private String resolveSecretValue(String valueFrom, String region) { | ||
| // A full ARN carries its own region; use it so cross-region references resolve | ||
| // against the right store instead of the task's region. Bare SSM names fall back | ||
| // to the task region. | ||
| String secretRegion = arnRegion(valueFrom, region); | ||
| try { | ||
| if (valueFrom != null && valueFrom.startsWith("arn:aws:secretsmanager:")) { | ||
| // The ECS `:json-key:version-stage:version-id` selector suffix on a | ||
| // Secrets Manager valueFrom is not yet supported: the value is resolved | ||
| // as the plain secret (ARN -> SecretString). See floci-io/floci#1624. | ||
| return secretsManagerService.getSecretValue(valueFrom, null, null, secretRegion).getSecretString(); | ||
| } | ||
| String parameterName = ssmParameterName(valueFrom); | ||
| return ssmService.getParameter(parameterName, secretRegion).getValue(); | ||
| } catch (AwsException e) { | ||
| throw new AwsException(e.getErrorCode(), | ||
| "ResourceInitializationError: unable to pull secrets or registry auth: " | ||
| + valueFrom + ": " + e.getMessage(), | ||
| e.getHttpStatus()); | ||
| } | ||
| } |
There was a problem hiding this comment.
Binary secret silently injects literal
"null" as env var
secretsManagerService.getSecretValue(...) can return a SecretVersion whose getSecretString() is null when the secret was stored as binary-only. In that case resolveSecretValue returns null, which propagates to buildEnvVars where it is formatted as entry.getKey() + "=" + entry.getValue() — producing "DB_PASSWORD=null" (the literal four-character string) in the container environment. The design contract states a bad secret must halt launch with ResourceInitializationError, but this path starts the container silently with a wrong value.
Additionally, the catch (AwsException e) block won't intercept a NullPointerException thrown if getSecretValue() itself returns null — that unchecked exception would bypass the ResourceInitializationError wrapping and propagate raw out of startTask.
Closes #1624.
What
Support
containerDefinitions[].secretsend to end: parse and round-trip theSecretreferences on register/describe, and inject the resolved values as environment variables when a task launches. Previouslysecretswas dropped on register and never resolved, so containers came up without the env vars their task definition declares viasecrets.How
EcsJsonHandlerparsessecrets[](name+valueFrom) into the container definition and emits them back onRegisterTaskDefinition/DescribeTaskDefinitionas references. Resolved values are never returned, matching AWS.EcsContainerManagerresolves each secret through the in-processSsmService/SecretsManagerService, the same local resolutionCodeBuildRunneralready uses (no extra HTTP hops):Parameter.valueSecretStringenvironment< resolvedsecrets< RunTask container-overrideenvironment. Overrides still win on a name collision.Failure behavior
Per the design question in #1624, this matches AWS rather than CodeBuild's lenient skip: if a referenced secret/parameter can't be resolved, no container is started and the task goes
STOPPEDwith aResourceInitializationErrorstopped reason (naming the offendingvalueFrom), instead of silently starting without the variable.Deferred
The Secrets Manager
valueFromselector suffix (:json-key:version-stage:version-id) is not yet parsed; such avalueFromis resolved as the plain secret. Called out as deferrable in #1624; happy to follow up.Test plan
EcsIntegrationTest(register/describe round-trip ofsecrets)EcsContainerManagerSecretsTest(SSM name + SM ARN resolution, override-wins, SSM-ARN name parsing, cross-region ARN region, unresolved → propagates with no container created)EcsServiceContainerSecretsTest(unresolved secret → task STOPPED withResourceInitializationErrorreason)EcsTestsJava SDK compatibility: register + describe round-trips theSecret./mvnw test -Dtest='Ecs*')