From 0f2ebb15ac0bbb6cdf1b4780d2ea1723e2b37a82 Mon Sep 17 00:00:00 2001 From: Michel Roca Date: Mon, 10 Jun 2024 17:22:05 +0200 Subject: [PATCH] feat: add docker build custom options --- internal/pkg/cli/deploy/workload.go | 3 +- .../pkg/docker/dockerengine/dockerengine.go | 22 ++++---- .../docker/dockerengine/dockerengine_test.go | 21 ++++---- internal/pkg/manifest/rd_web_svc_test.go | 2 + internal/pkg/manifest/svc_test.go | 2 + internal/pkg/manifest/workload.go | 34 ++++++++----- internal/pkg/manifest/workload_test.go | 51 ++++++++++++++++--- site/content/docs/include/image-config.en.md | 4 +- site/content/docs/include/image-config.ja.md | 4 +- .../docs/manifest/rd-web-service.en.md | 4 +- .../docs/manifest/rd-web-service.ja.md | 4 +- 11 files changed, 92 insertions(+), 59 deletions(-) diff --git a/internal/pkg/cli/deploy/workload.go b/internal/pkg/cli/deploy/workload.go index 8ca10485727..fcd5de33e0e 100644 --- a/internal/pkg/cli/deploy/workload.go +++ b/internal/pkg/cli/deploy/workload.go @@ -590,8 +590,7 @@ func buildArgsPerContainer(name, workspacePath string, img ContainerImageIdentif Dockerfile: aws.StringValue(buildArgs.Dockerfile), Context: aws.StringValue(buildArgs.Context), Args: buildArgs.Args, - CacheFrom: buildArgs.CacheFrom, - Target: aws.StringValue(buildArgs.Target), + Options: buildArgs.Options, Platform: mf.ContainerPlatform(), Tags: tags, Labels: labels, diff --git a/internal/pkg/docker/dockerengine/dockerengine.go b/internal/pkg/docker/dockerengine/dockerengine.go index 0ad7aa6fd6d..f62617ec0a2 100644 --- a/internal/pkg/docker/dockerengine/dockerengine.go +++ b/internal/pkg/docker/dockerengine/dockerengine.go @@ -60,6 +60,12 @@ const ( containerStatusExited = "exited" ) +// Deprecated predefined options to pass to `docker build`. The `options: [...]` config should be used instead. +const ( + BuildOptionCacheFrom = "--cache-from" + BuildOptionTarget = "--target" +) + // DockerCmdClient represents the docker client to interact with the server via external commands. type DockerCmdClient struct { runner Cmd @@ -85,8 +91,7 @@ type BuildArguments struct { Dockerfile string // Optional. One of Dockerfile or DockerfileContent is required. Dockerfile to pass to `docker build` via --file flag. DockerfileContent string // Optional. One of Dockerfile or DockerfileContent is required. Dockerfile content to pass to `docker build` via stdin. Context string // Optional. Build context directory to pass to `docker build`. - Target string // Optional. The target build stage to pass to `docker build`. - CacheFrom []string // Optional. Images to consider as cache sources to pass to `docker build` + Options []string // Optional. Additional build options and flags to pass to `docker build`. See https://docs.docker.com/reference/cli/docker/image/build/#options Platform string // Optional. OS/Arch to pass to `docker build`. Args map[string]string // Optional. Build args to pass via `--build-arg` flags. Equivalent to ARG directives in dockerfile. Labels map[string]string // Required. Set metadata for an image. @@ -130,21 +135,14 @@ func (in *BuildArguments) GenerateDockerBuildArgs(c DockerCmdClient) ([]string, args := []string{"build"} + // Add all custom options first. + args = append(args, in.Options...) + // Add additional image tags to the docker build call. for _, tag := range in.Tags { args = append(args, "-t", imageName(in.URI, tag)) } - // Add cache from options. - for _, imageFrom := range in.CacheFrom { - args = append(args, "--cache-from", imageFrom) - } - - // Add target option. - if in.Target != "" { - args = append(args, "--target", in.Target) - } - // Add platform option. if in.Platform != "" { args = append(args, "--platform", in.Platform) diff --git a/internal/pkg/docker/dockerengine/dockerengine_test.go b/internal/pkg/docker/dockerengine/dockerengine_test.go index 10d6c342379..c3f99a1de2c 100644 --- a/internal/pkg/docker/dockerengine/dockerengine_test.go +++ b/internal/pkg/docker/dockerengine/dockerengine_test.go @@ -43,8 +43,7 @@ func TestDockerCommand_Build(t *testing.T) { context string tags []string args map[string]string - target string - cacheFrom []string + options []string envVars map[string]string labels map[string]string setupMocks func(controller *gomock.Controller) @@ -177,18 +176,17 @@ func TestDockerCommand_Build(t *testing.T) { "-f", "mockPath/to/mockDockerfile"}, gomock.Any(), gomock.Any()).Return(nil) }, }, - "runs with cache_from and target fields": { - path: mockPath, - tags: []string{"latest"}, - target: "foobar", - cacheFrom: []string{"foo/bar:latest", "foo/bar/baz:1.2.3"}, + "success with options field": { + path: mockPath, + tags: []string{"latest"}, + options: []string{"--foo", "bar", "--baz"}, setupMocks: func(c *gomock.Controller) { mockCmd = NewMockCmd(c) mockCmd.EXPECT().RunWithContext(ctx, "docker", []string{"build", + "--foo", + "bar", + "--baz", "-t", fmt.Sprintf("%s:%s", mockURI, "latest"), - "--cache-from", "foo/bar:latest", - "--cache-from", "foo/bar/baz:1.2.3", - "--target", "foobar", filepath.FromSlash("mockPath/to"), "-f", "mockPath/to/mockDockerfile"}, gomock.Any(), gomock.Any()).Return(nil) }, @@ -224,8 +222,7 @@ func TestDockerCommand_Build(t *testing.T) { DockerfileContent: tc.dockerfileContent, URI: mockURI, Args: tc.args, - Target: tc.target, - CacheFrom: tc.cacheFrom, + Options: tc.options, Tags: tc.tags, Labels: tc.labels, } diff --git a/internal/pkg/manifest/rd_web_svc_test.go b/internal/pkg/manifest/rd_web_svc_test.go index 024c70c93b6..6dcda59047c 100644 --- a/internal/pkg/manifest/rd_web_svc_test.go +++ b/internal/pkg/manifest/rd_web_svc_test.go @@ -141,6 +141,7 @@ func TestRequestDrivenWebService_UnmarshalYaml(t *testing.T) { " build:\n" + " dockerfile: ./Dockerfile\n" + " context: context/dir\n" + + " options: [\"--pull\", \"--cache-from\", \"foo/bar:cache\"]\n" + " target: build-stage\n" + " cache_from:\n" + " - image:tag\n" + @@ -158,6 +159,7 @@ func TestRequestDrivenWebService_UnmarshalYaml(t *testing.T) { BuildArgs: DockerBuildArgs{ Context: aws.String("context/dir"), Dockerfile: aws.String("./Dockerfile"), + Options: []string{"--pull", "--cache-from", "foo/bar:cache"}, Target: aws.String("build-stage"), CacheFrom: []string{"image:tag"}, Args: map[string]string{"a": "1", "b": "2"}, diff --git a/internal/pkg/manifest/svc_test.go b/internal/pkg/manifest/svc_test.go index db896d33e6b..92afc4528ed 100644 --- a/internal/pkg/manifest/svc_test.go +++ b/internal/pkg/manifest/svc_test.go @@ -67,6 +67,7 @@ sidecars: build: dockerfile: "web/Dockerfile" context: "pathto/Dockerfile" + options: ["--pull", "--cache-from", "foo/bar:cache"] target: "build-stage" cache_from: - foo/bar:latest @@ -173,6 +174,7 @@ environments: BuildArgs: DockerBuildArgs{ Dockerfile: aws.String("web/Dockerfile"), Context: aws.String("pathto/Dockerfile"), + Options: []string{"--pull", "--cache-from", "foo/bar:cache"}, Target: aws.String("build-stage"), CacheFrom: []string{"foo/bar:latest"}, Args: map[string]string{ diff --git a/internal/pkg/manifest/workload.go b/internal/pkg/manifest/workload.go index 28d7d713305..0a51d2b0c2d 100644 --- a/internal/pkg/manifest/workload.go +++ b/internal/pkg/manifest/workload.go @@ -176,8 +176,7 @@ func (i *ImageLocationOrBuild) BuildConfig(rootDirectory string) *DockerBuildArg Dockerfile: aws.String(filepath.Join(rootDirectory, i.dockerfilePath())), Context: aws.String(filepath.Join(rootDirectory, i.contextPath())), Args: i.args(), - Target: i.target(), - CacheFrom: i.cacheFrom(), + Options: i.options(), } } @@ -227,15 +226,23 @@ func (i *ImageLocationOrBuild) args() map[string]string { return i.Build.BuildArgs.Args } -// target returns the build target stage if it exists, otherwise nil. -func (i *ImageLocationOrBuild) target() *string { - return i.Build.BuildArgs.Target -} +// options returns the additional options from build section, if some are manually defined. Otherwise, it returns an empty array. +func (i *ImageLocationOrBuild) options() []string { + options := i.Build.BuildArgs.Options + + // Add the deprecated target value + if i.Build.BuildArgs.Target != nil { + options = append(options, dockerengine.BuildOptionTarget, *i.Build.BuildArgs.Target) + } + + // Add the deprecated cacheFrom values + if i.Build.BuildArgs.CacheFrom != nil { + for _, cacheFrom := range i.Build.BuildArgs.CacheFrom { + options = append(options, dockerengine.BuildOptionCacheFrom, cacheFrom) + } + } -// cacheFrom returns the cache from build section, if it exists. -// Otherwise it returns nil. -func (i *ImageLocationOrBuild) cacheFrom() []string { - return i.Build.BuildArgs.CacheFrom + return options } // ImageOverride holds fields that override Dockerfile image defaults. @@ -396,12 +403,13 @@ type DockerBuildArgs struct { Context *string `yaml:"context,omitempty"` Dockerfile *string `yaml:"dockerfile,omitempty"` Args map[string]string `yaml:"args,omitempty"` - Target *string `yaml:"target,omitempty"` - CacheFrom []string `yaml:"cache_from,omitempty"` + Options []string `yaml:"options,omitempty"` + Target *string `yaml:"target,omitempty"` // Deprecated. Use options with [--target, VALUE] instead. + CacheFrom []string `yaml:"cache_from,omitempty"` // Deprecated. Use options with [--cache-from, VALUE] instead. } func (b *DockerBuildArgs) isEmpty() bool { - if b.Context == nil && b.Dockerfile == nil && b.Args == nil && b.Target == nil && b.CacheFrom == nil { + if b.Context == nil && b.Dockerfile == nil && b.Args == nil && b.Options == nil && b.Target == nil && b.CacheFrom == nil { return true } return false diff --git a/internal/pkg/manifest/workload_test.go b/internal/pkg/manifest/workload_test.go index a041b6550fa..bbfc97e73e7 100644 --- a/internal/pkg/manifest/workload_test.go +++ b/internal/pkg/manifest/workload_test.go @@ -270,7 +270,32 @@ func TestBuildArgs_UnmarshalYAML(t *testing.T) { BuildString: nil, }, }, - "Dockerfile with cache from and target build opts": { + "Dockerfile with build opts": { + inContent: []byte(`build: + options: + - --pull + - --target + - "foobar" + - --cache-from + - foo/bar:latest + - --cache-from + - foo/bar/baz:1.2.3`), + wantedStruct: BuildArgsOrString{ + BuildArgs: DockerBuildArgs{ + Options: []string{ + "--pull", + "--target", + "foobar", + "--cache-from", + "foo/bar:latest", + "--cache-from", + "foo/bar/baz:1.2.3", + }, + }, + BuildString: nil, + }, + }, + "Dockerfile with deprecated cache from and target build opts": { inContent: []byte(`build: cache_from: - foo/bar:latest @@ -313,6 +338,7 @@ func TestBuildArgs_UnmarshalYAML(t *testing.T) { require.Equal(t, tc.wantedStruct.BuildArgs.Context, b.Build.BuildArgs.Context) require.Equal(t, tc.wantedStruct.BuildArgs.Dockerfile, b.Build.BuildArgs.Dockerfile) require.Equal(t, tc.wantedStruct.BuildArgs.Args, b.Build.BuildArgs.Args) + require.Equal(t, tc.wantedStruct.BuildArgs.Options, b.Build.BuildArgs.Options) require.Equal(t, tc.wantedStruct.BuildArgs.Target, b.Build.BuildArgs.Target) require.Equal(t, tc.wantedStruct.BuildArgs.CacheFrom, b.Build.BuildArgs.CacheFrom) } @@ -817,10 +843,23 @@ func TestBuildConfig(t *testing.T) { }, }, }, - "including build options": { + "custom build options": { inBuild: BuildArgsOrString{ BuildArgs: DockerBuildArgs{ - Target: aws.String("foobar"), + Options: []string{"--foo"}, + }, + }, + wantedBuild: DockerBuildArgs{ + Dockerfile: aws.String(filepath.Join(mockWsRoot, "Dockerfile")), + Context: aws.String(mockWsRoot), + Options: []string{"--foo"}, + }, + }, + "deprecated build options": { + inBuild: BuildArgsOrString{ + BuildArgs: DockerBuildArgs{ + Options: []string{"--foo"}, + Target: aws.String("foobar"), CacheFrom: []string{ "foo/bar:latest", "foo/bar/baz:1.2.3", @@ -830,11 +869,7 @@ func TestBuildConfig(t *testing.T) { wantedBuild: DockerBuildArgs{ Dockerfile: aws.String(filepath.Join(mockWsRoot, "Dockerfile")), Context: aws.String(mockWsRoot), - Target: aws.String("foobar"), - CacheFrom: []string{ - "foo/bar:latest", - "foo/bar/baz:1.2.3", - }, + Options: []string{"--foo", "--target", "foobar", "--cache-from", "foo/bar:latest", "--cache-from", "foo/bar/baz:1.2.3"}, }, }, } diff --git a/site/content/docs/include/image-config.en.md b/site/content/docs/include/image-config.en.md index 6fd17b4d4a4..a0be19df210 100644 --- a/site/content/docs/include/image-config.en.md +++ b/site/content/docs/include/image-config.en.md @@ -14,9 +14,7 @@ image: build: dockerfile: path/to/dockerfile context: context/dir - target: build-stage - cache_from: - - image:tag + option: ["--target", "build-stage", "--cache-from", "image:tag"] args: key: value ``` diff --git a/site/content/docs/include/image-config.ja.md b/site/content/docs/include/image-config.ja.md index 2ba45790a17..92d3079e410 100644 --- a/site/content/docs/include/image-config.ja.md +++ b/site/content/docs/include/image-config.ja.md @@ -14,9 +14,7 @@ image: build: dockerfile: path/to/dockerfile context: context/dir - target: build-stage - cache_from: - - image:tag + option: ["--target", "build-stage", "--cache-from", "image:tag"] args: key: value ``` diff --git a/site/content/docs/manifest/rd-web-service.en.md b/site/content/docs/manifest/rd-web-service.en.md index 5c1609c7eff..0673fd97707 100644 --- a/site/content/docs/manifest/rd-web-service.en.md +++ b/site/content/docs/manifest/rd-web-service.en.md @@ -155,9 +155,7 @@ image: build: dockerfile: path/to/dockerfile context: context/dir - target: build-stage - cache_from: - - image:tag + option: ["--target", "build-stage", "--cache-from", "image:tag"] args: key: value ``` diff --git a/site/content/docs/manifest/rd-web-service.ja.md b/site/content/docs/manifest/rd-web-service.ja.md index f11fe36c294..b3c0a09328c 100644 --- a/site/content/docs/manifest/rd-web-service.ja.md +++ b/site/content/docs/manifest/rd-web-service.ja.md @@ -157,9 +157,7 @@ image: build: dockerfile: path/to/dockerfile context: context/dir - target: build-stage - cache_from: - - image:tag + option: ["--target", "build-stage", "--cache-from", "image:tag"] args: key: value ```