Skip to content

Commit

Permalink
Return an empty secrets list for task definitions (#7096)
Browse files Browse the repository at this point in the history
The first time I ran #7009 on
a real ECS deployment, I observed new task definitions getting
registered when they shouldn't have.

It turns out the actual ECS task definition defaults to an empty list of
secrets for each container definition that does not explicitly provide
them whereas our stub was doing a direct pass-through of the container
definitions.

This fixes our stub to bring its defaults more in line with the actual
ECS API and fixes our comparison to ensure we're checking for secrets
equality correctly.

I didn't explicitly pass in an empty list with our
RegisterTaskDefinitions call because we omit a lot of other kwargs and
let ECS handle their defaults; it felt wrong to pass in an empty list
every time.
  • Loading branch information
jmsanders committed Mar 17, 2022
1 parent aa1dc4a commit 3f6458c
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ def _task_definition(self, family, metadata, image):
if (
container_definition.get("image") == image
and container_definition.get("name") == self.container_name
and container_definition.get("secrets") == secrets_dict.get("secrets")
and container_definition.get("secrets") == secrets_dict.get("secrets", [])
):
return task_definition

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,4 +148,4 @@ def test_empty_secrets(
task_definition = task_definition["taskDefinition"]

# No secrets
assert "secrets" not in task_definition["containerDefinitions"][0]
assert not task_definition["containerDefinitions"][0].get("secrets")
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,13 @@ def register_task_definition(self, **kwargs):
memory = kwargs.get("memory")
cpu = kwargs.get("cpu")

# Container definitions default to empty secret lists
container_definitions = kwargs.get("containerDefinitions", [])
for container_definition in container_definitions:
if not container_definition.get("secrets"):
container_definition["secrets"] = []
kwargs["containerDefinitions"] = container_definitions

if self._valid_cpu_and_memory(cpu=cpu, memory=memory):
task_definition = {
"family": family,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,15 @@ def test_register_task_definition(ecs):
)
assert response["taskDefinition"]["networkMode"] == "bridge"

# Secrets default to an empty list
response = ecs.register_task_definition(
family="secrets",
containerDefinitions=[{"image": "hello_world:latest"}],
memory="512",
cpu="256",
)
assert response["taskDefinition"]["containerDefinitions"][0]["secrets"] == []


def test_run_task(ecs, ec2, subnet):
with pytest.raises(ParamValidationError):
Expand Down

0 comments on commit 3f6458c

Please sign in to comment.