From 5bee676ddd2a973dd72c702649e6efc2becd0cf5 Mon Sep 17 00:00:00 2001 From: francesco-racciatti Date: Mon, 12 Feb 2024 00:06:47 +0100 Subject: [PATCH 1/2] fix: use entrypoint/command from template when possible Set the final entrypoint as defined in kilt-definition and use the overridden entrypoint/command from the template when possible. When the template overrides the entrypoint, the command won't be taken from the image. Signed-off-by: francesco-racciatti --- .../cloudformation/cfnpatcher/cfn_test.go | 23 ++++++++++ .../fixtures/patching/hints_no_overrides.json | 18 ++++++++ .../patching/hints_no_overrides.patched.json | 46 +++++++++++++++++++ .../patching/hints_override_command.json | 19 ++++++++ .../hints_override_command.patched.json | 44 ++++++++++++++++++ .../patching/hints_override_entrypoint.json | 19 ++++++++ .../hints_override_entrypoint.patched.json | 43 +++++++++++++++++ .../hints_override_entrypoint_command.json | 20 ++++++++ ...s_override_entrypoint_command.patched.json | 44 ++++++++++++++++++ .../cloudformation/cfnpatcher/template.go | 13 ++++-- 10 files changed, 286 insertions(+), 3 deletions(-) create mode 100644 runtimes/cloudformation/cfnpatcher/fixtures/patching/hints_no_overrides.json create mode 100644 runtimes/cloudformation/cfnpatcher/fixtures/patching/hints_no_overrides.patched.json create mode 100644 runtimes/cloudformation/cfnpatcher/fixtures/patching/hints_override_command.json create mode 100644 runtimes/cloudformation/cfnpatcher/fixtures/patching/hints_override_command.patched.json create mode 100644 runtimes/cloudformation/cfnpatcher/fixtures/patching/hints_override_entrypoint.json create mode 100644 runtimes/cloudformation/cfnpatcher/fixtures/patching/hints_override_entrypoint.patched.json create mode 100644 runtimes/cloudformation/cfnpatcher/fixtures/patching/hints_override_entrypoint_command.json create mode 100644 runtimes/cloudformation/cfnpatcher/fixtures/patching/hints_override_entrypoint_command.patched.json diff --git a/runtimes/cloudformation/cfnpatcher/cfn_test.go b/runtimes/cloudformation/cfnpatcher/cfn_test.go index a051a3d..e6e00cd 100644 --- a/runtimes/cloudformation/cfnpatcher/cfn_test.go +++ b/runtimes/cloudformation/cfnpatcher/cfn_test.go @@ -47,6 +47,13 @@ var defaultTests = [...]string{ "patching/volumes_from", } +var enableHints = [...]string{ + "patching/hints_no_overrides", + "patching/hints_override_command", + "patching/hints_override_entrypoint", + "patching/hints_override_entrypoint_command", +} + var parameterizedEnvarsTests = [...]string{ "patching/parameterize_env_add", "patching/parameterize_env_merge", @@ -205,6 +212,22 @@ func TestPatching(t *testing.T) { } } +func TestPatchingWithRepoHints(t *testing.T) { + l := log.Output(zerolog.ConsoleWriter{Out: os.Stderr}).With().Caller().Logger() + + for _, testName := range enableHints { + t.Run(testName, func(t *testing.T) { + runTest(t, testName, l.WithContext(context.Background()), + Configuration{ + Kilt: defaultConfig, + OptIn: false, + RecipeConfig: "{}", + UseRepositoryHints: true, + }) + }) + } +} + func TestPatchingSidecarEnv(t *testing.T) { l := log.Output(zerolog.ConsoleWriter{Out: os.Stderr}).With().Caller().Logger() diff --git a/runtimes/cloudformation/cfnpatcher/fixtures/patching/hints_no_overrides.json b/runtimes/cloudformation/cfnpatcher/fixtures/patching/hints_no_overrides.json new file mode 100644 index 0000000..f9b93e7 --- /dev/null +++ b/runtimes/cloudformation/cfnpatcher/fixtures/patching/hints_no_overrides.json @@ -0,0 +1,18 @@ +{ + "Resources": { + "taskdef": { + "Type": "AWS::ECS::TaskDefinition", + "Properties": { + "RequiresCompatibilities": [ + "FARGATE" + ], + "ContainerDefinitions": [ + { + "Name": "app", + "Image": "nginx" + } + ] + } + } + } +} \ No newline at end of file diff --git a/runtimes/cloudformation/cfnpatcher/fixtures/patching/hints_no_overrides.patched.json b/runtimes/cloudformation/cfnpatcher/fixtures/patching/hints_no_overrides.patched.json new file mode 100644 index 0000000..6bc95ab --- /dev/null +++ b/runtimes/cloudformation/cfnpatcher/fixtures/patching/hints_no_overrides.patched.json @@ -0,0 +1,46 @@ +{ + "Resources": { + "taskdef": { + "Properties": { + "ContainerDefinitions": [ + { + "Command": [ + "/docker-entrypoint.sh", + "nginx", + "-g", + "daemon off;" + ], + "EntryPoint": [ + "/kilt/run", + "--" + ], + "Image": "nginx", + "Name": "app", + "LinuxParameters": { + "Capabilities": { + "Add": ["SYS_PTRACE"] + } + }, + "VolumesFrom": [ + { + "ReadOnly": true, + "SourceContainer": "KiltImage" + } + ] + }, + { + "EntryPoint": [ + "/kilt/wait" + ], + "Image": "KILT:latest", + "Name": "KiltImage" + } + ], + "RequiresCompatibilities": [ + "FARGATE" + ] + }, + "Type": "AWS::ECS::TaskDefinition" + } + } +} \ No newline at end of file diff --git a/runtimes/cloudformation/cfnpatcher/fixtures/patching/hints_override_command.json b/runtimes/cloudformation/cfnpatcher/fixtures/patching/hints_override_command.json new file mode 100644 index 0000000..fcb8a7f --- /dev/null +++ b/runtimes/cloudformation/cfnpatcher/fixtures/patching/hints_override_command.json @@ -0,0 +1,19 @@ +{ + "Resources": { + "taskdef": { + "Type": "AWS::ECS::TaskDefinition", + "Properties": { + "RequiresCompatibilities": [ + "FARGATE" + ], + "ContainerDefinitions": [ + { + "Name": "app", + "Image": "nginx", + "Command": ["my-command"] + } + ] + } + } + } +} \ No newline at end of file diff --git a/runtimes/cloudformation/cfnpatcher/fixtures/patching/hints_override_command.patched.json b/runtimes/cloudformation/cfnpatcher/fixtures/patching/hints_override_command.patched.json new file mode 100644 index 0000000..549728c --- /dev/null +++ b/runtimes/cloudformation/cfnpatcher/fixtures/patching/hints_override_command.patched.json @@ -0,0 +1,44 @@ +{ + "Resources": { + "taskdef": { + "Properties": { + "ContainerDefinitions": [ + { + "Command": [ + "/docker-entrypoint.sh", + "my-command" + ], + "EntryPoint": [ + "/kilt/run", + "--" + ], + "Image": "nginx", + "Name": "app", + "LinuxParameters": { + "Capabilities": { + "Add": ["SYS_PTRACE"] + } + }, + "VolumesFrom": [ + { + "ReadOnly": true, + "SourceContainer": "KiltImage" + } + ] + }, + { + "EntryPoint": [ + "/kilt/wait" + ], + "Image": "KILT:latest", + "Name": "KiltImage" + } + ], + "RequiresCompatibilities": [ + "FARGATE" + ] + }, + "Type": "AWS::ECS::TaskDefinition" + } + } +} \ No newline at end of file diff --git a/runtimes/cloudformation/cfnpatcher/fixtures/patching/hints_override_entrypoint.json b/runtimes/cloudformation/cfnpatcher/fixtures/patching/hints_override_entrypoint.json new file mode 100644 index 0000000..30b52ed --- /dev/null +++ b/runtimes/cloudformation/cfnpatcher/fixtures/patching/hints_override_entrypoint.json @@ -0,0 +1,19 @@ +{ + "Resources": { + "taskdef": { + "Type": "AWS::ECS::TaskDefinition", + "Properties": { + "RequiresCompatibilities": [ + "FARGATE" + ], + "ContainerDefinitions": [ + { + "Name": "app", + "Image": "nginx", + "EntryPoint": ["my-entrypoint"] + } + ] + } + } + } +} \ No newline at end of file diff --git a/runtimes/cloudformation/cfnpatcher/fixtures/patching/hints_override_entrypoint.patched.json b/runtimes/cloudformation/cfnpatcher/fixtures/patching/hints_override_entrypoint.patched.json new file mode 100644 index 0000000..2edd906 --- /dev/null +++ b/runtimes/cloudformation/cfnpatcher/fixtures/patching/hints_override_entrypoint.patched.json @@ -0,0 +1,43 @@ +{ + "Resources": { + "taskdef": { + "Properties": { + "ContainerDefinitions": [ + { + "Command": [ + "my-entrypoint" + ], + "EntryPoint": [ + "/kilt/run", + "--" + ], + "Image": "nginx", + "Name": "app", + "LinuxParameters": { + "Capabilities": { + "Add": ["SYS_PTRACE"] + } + }, + "VolumesFrom": [ + { + "ReadOnly": true, + "SourceContainer": "KiltImage" + } + ] + }, + { + "EntryPoint": [ + "/kilt/wait" + ], + "Image": "KILT:latest", + "Name": "KiltImage" + } + ], + "RequiresCompatibilities": [ + "FARGATE" + ] + }, + "Type": "AWS::ECS::TaskDefinition" + } + } +} \ No newline at end of file diff --git a/runtimes/cloudformation/cfnpatcher/fixtures/patching/hints_override_entrypoint_command.json b/runtimes/cloudformation/cfnpatcher/fixtures/patching/hints_override_entrypoint_command.json new file mode 100644 index 0000000..0114124 --- /dev/null +++ b/runtimes/cloudformation/cfnpatcher/fixtures/patching/hints_override_entrypoint_command.json @@ -0,0 +1,20 @@ +{ + "Resources": { + "taskdef": { + "Type": "AWS::ECS::TaskDefinition", + "Properties": { + "RequiresCompatibilities": [ + "FARGATE" + ], + "ContainerDefinitions": [ + { + "Name": "app", + "Image": "nginx", + "Command": ["my-command"], + "EntryPoint": ["my-entrypoint"] + } + ] + } + } + } +} \ No newline at end of file diff --git a/runtimes/cloudformation/cfnpatcher/fixtures/patching/hints_override_entrypoint_command.patched.json b/runtimes/cloudformation/cfnpatcher/fixtures/patching/hints_override_entrypoint_command.patched.json new file mode 100644 index 0000000..ca532e5 --- /dev/null +++ b/runtimes/cloudformation/cfnpatcher/fixtures/patching/hints_override_entrypoint_command.patched.json @@ -0,0 +1,44 @@ +{ + "Resources": { + "taskdef": { + "Properties": { + "ContainerDefinitions": [ + { + "Command": [ + "my-entrypoint", + "my-command" + ], + "EntryPoint": [ + "/kilt/run", + "--" + ], + "Image": "nginx", + "Name": "app", + "LinuxParameters": { + "Capabilities": { + "Add": ["SYS_PTRACE"] + } + }, + "VolumesFrom": [ + { + "ReadOnly": true, + "SourceContainer": "KiltImage" + } + ] + }, + { + "EntryPoint": [ + "/kilt/wait" + ], + "Image": "KILT:latest", + "Name": "KiltImage" + } + ], + "RequiresCompatibilities": [ + "FARGATE" + ] + }, + "Type": "AWS::ECS::TaskDefinition" + } + } +} \ No newline at end of file diff --git a/runtimes/cloudformation/cfnpatcher/template.go b/runtimes/cloudformation/cfnpatcher/template.go index 44b7e2f..5cebabb 100644 --- a/runtimes/cloudformation/cfnpatcher/template.go +++ b/runtimes/cloudformation/cfnpatcher/template.go @@ -11,7 +11,10 @@ import ( func fillContainerInfo(ctx context.Context, container *gabs.Container, parameters *gabs.Container, configuration *Configuration) { l := log.Ctx(ctx) - if container.Exists("Command") && container.Exists("EntryPoint") { + hasOverriddenEntrypoint := container.Exists("EntryPoint") + hasOverriddenCommand := container.Exists("Command") + + if hasOverriddenEntrypoint && hasOverriddenCommand { return } @@ -44,10 +47,14 @@ func fillContainerInfo(ctx context.Context, container *gabs.Container, parameter if err != nil { l.Warn().Str("image", image).Err(err).Msg("could not retrieve metadata from repository") } else { - if repoInfo.Entrypoint != nil { + // Use the image's entrypoint if the task definition does not override it + if repoInfo.Entrypoint != nil && !hasOverriddenEntrypoint { + l.Info().Str("image", container.S("Image").String()).Msgf("using default entrypoint %s", repoInfo.Entrypoint) container.Set(repoInfo.Entrypoint, "EntryPoint") } - if repoInfo.Command != nil { + // Use the image's command if the task definition overrides neither the entrypoint nor the command + if repoInfo.Command != nil && !hasOverriddenCommand && !hasOverriddenEntrypoint { + l.Info().Str("image", container.S("Image").String()).Msgf("using default command %s", repoInfo.Command) container.Set(repoInfo.Command, "Command") } } From 8fe6906f8ca2e396f243f68b0cc829ce297991b6 Mon Sep 17 00:00:00 2001 From: francesco-racciatti Date: Fri, 16 Feb 2024 16:32:14 +0100 Subject: [PATCH 2/2] build: add test target for runtime/cloudformation Signed-off-by: francesco-racciatti --- .github/workflows/presubmit.yml | 3 +-- runtimes/cloudformation/Makefile | 5 ++++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/.github/workflows/presubmit.yml b/.github/workflows/presubmit.yml index 995aeb5..e9648f4 100644 --- a/.github/workflows/presubmit.yml +++ b/.github/workflows/presubmit.yml @@ -43,8 +43,7 @@ jobs: - name: Test run: | - cd runtimes/cloudformation - go test -v ./... + make -C runtimes/cloudformation test build-cfn-runtime: name: Build CloudFormation runtime diff --git a/runtimes/cloudformation/Makefile b/runtimes/cloudformation/Makefile index 80eaf12..a090f6e 100644 --- a/runtimes/cloudformation/Makefile +++ b/runtimes/cloudformation/Makefile @@ -11,4 +11,7 @@ clean: rm agent-kilt.zip || true rm cmd/handler/handler || true -.PHONY: clean +test: + go test -v ./... + +.PHONY: clean test