From 20224f44b9b5193793ebe5594e4c7e570143cd4e Mon Sep 17 00:00:00 2001 From: Gilbert MINH Date: Tue, 28 Nov 2023 18:49:48 +0100 Subject: [PATCH 1/4] feat: Add support for build arguments to build command --- cmd/build.go | 26 ++++++++++-- cmd/list.go | 2 +- internal/logger/logger.go | 2 +- pkg/dag/image_test.go | 3 +- pkg/dib/build.go | 14 +++++-- pkg/dib/build_test.go | 2 +- pkg/dib/generate_dag.go | 30 ++++++++++++-- pkg/dib/generate_dag_test.go | 40 ++++++++++++++----- pkg/docker/builder.go | 18 ++++----- pkg/dockerfile/dockerfile.go | 33 +++++++++------ pkg/dockerfile/dockerfile_test.go | 33 ++++++++++++--- pkg/exec/shell.go | 9 +++-- pkg/graphviz/graphviz_test.go | 4 +- pkg/kaniko/builder.go | 4 +- test/fixtures/docker/bullseye/Dockerfile | 4 ++ .../fixtures/dockerfile/simple-arg.dockerfile | 2 + 16 files changed, 170 insertions(+), 56 deletions(-) create mode 100644 test/fixtures/dockerfile/simple-arg.dockerfile diff --git a/cmd/build.go b/cmd/build.go index 3c497b63..7042eb98 100644 --- a/cmd/build.go +++ b/cmd/build.go @@ -2,6 +2,7 @@ package main import ( "fmt" + "os" "path" "strings" @@ -46,7 +47,22 @@ Otherwise, dib will create a new tag based on the previous tag.`, opts := dib.BuildOpts{} hydrateOptsFromViper(&opts) - if err := doBuild(opts); err != nil { + buildArgs := map[string]string{} + for _, arg := range opts.BuildArg { + key, val, hasVal := strings.Cut(arg, "=") + if hasVal { + buildArgs[key] = val + } else { + // check if the env is set in the local environment and use that value if it is + if val, present := os.LookupEnv(key); present { + buildArgs[key] = val + } else { + delete(buildArgs, key) + } + } + } + + if err := doBuild(opts, buildArgs); err != nil { logger.Fatalf("Build failed: %v", err) } @@ -80,9 +96,11 @@ func init() { fmt.Sprintf("Build Backend used to run image builds. Supported backends: %v", supportedBackends)) buildCmd.Flags().Int("rate-limit", 1, "Concurrent number of builds that can run simultaneously") + buildCmd.Flags().StringArray("build-arg", []string{}, + "`argument=value` to supply to the builder") } -func doBuild(opts dib.BuildOpts) error { +func doBuild(opts dib.BuildOpts, buildArgs map[string]string) error { if opts.Backend == types.BackendKaniko && opts.LocalOnly { logger.Warnf("Using Backend \"kaniko\" with the --local-only flag is partially supported.") } @@ -98,7 +116,7 @@ func doBuild(opts dib.BuildOpts) error { logger.Infof("Building images in directory \"%s\"", buildPath) logger.Debugf("Generate DAG") - graph, err := dib.GenerateDAG(buildPath, opts.RegistryURL, opts.HashListFilePath) + graph, err := dib.GenerateDAG(buildPath, opts.RegistryURL, opts.HashListFilePath, buildArgs) if err != nil { return fmt.Errorf("cannot generate DAG: %w", err) } @@ -135,7 +153,7 @@ func doBuild(opts dib.BuildOpts) error { return fmt.Errorf("invalid backend \"%s\": not supported", opts.Backend) } - res := dibBuilder.RebuildGraph(builder, ratelimit.NewChannelRateLimiter(opts.RateLimit)) + res := dibBuilder.RebuildGraph(builder, ratelimit.NewChannelRateLimiter(opts.RateLimit), buildArgs) res.Print() if err := report.Generate(res, dibBuilder.Graph); err != nil { diff --git a/cmd/list.go b/cmd/list.go index f7ae35c4..68111b0b 100644 --- a/cmd/list.go +++ b/cmd/list.go @@ -46,7 +46,7 @@ func doList(opts dib.ListOpts) error { } buildPath := path.Join(workingDir, opts.BuildPath) - graph, err := dib.GenerateDAG(buildPath, opts.RegistryURL, opts.HashListFilePath) + graph, err := dib.GenerateDAG(buildPath, opts.RegistryURL, opts.HashListFilePath, map[string]string{}) if err != nil { return fmt.Errorf("cannot generate DAG: %w", err) } diff --git a/internal/logger/logger.go b/internal/logger/logger.go index 74cbdee6..8fea57b5 100644 --- a/internal/logger/logger.go +++ b/internal/logger/logger.go @@ -56,7 +56,6 @@ func SetLevel(level *string) { switch *level { case "debug": - Infof("debug mode enabled") log.Level = LogLevelDebug case "info": log.Level = LogLevelInfo @@ -71,6 +70,7 @@ func SetLevel(level *string) { } logger.Store(log) + Infof("Log level set to %s", log.Level) } // LogLevelStyle returns the style of the prefix for each log level. diff --git a/pkg/dag/image_test.go b/pkg/dag/image_test.go index 71c1e207..a839fea3 100644 --- a/pkg/dag/image_test.go +++ b/pkg/dag/image_test.go @@ -76,6 +76,7 @@ func Test_Print(t *testing.T) { " tag: \"3.17\"\n" + " digest: 9ed4aefc74f6792b5a804d1d146fe4b4a2299147b0f50eaf2b08435d7b38c27e\n" + " labels:\n" + - " dib.extra-tags: \"3.17\"\n" + " dib.extra-tags: \"3.17\"\n" + + " args: {}\n" assert.Equal(t, expected, image.Print()) } diff --git a/pkg/dib/build.go b/pkg/dib/build.go index 33664bc0..161426c7 100644 --- a/pkg/dib/build.go +++ b/pkg/dib/build.go @@ -41,12 +41,14 @@ type BuildOpts struct { Trivy trivy.Config `mapstructure:"trivy"` Kaniko kaniko.Config `mapstructure:"kaniko"` RateLimit int `mapstructure:"rate_limit"` + BuildArg []string `mapstructure:"build_arg"` } // RebuildGraph iterates over the graph to rebuild all the images that are marked to be rebuilt. func (p *Builder) RebuildGraph( builder types.ImageBuilder, rateLimiter ratelimit.RateLimiter, + buildArgs map[string]string, ) *report.Report { buildOpts, err := yaml.Marshal(&p.BuildOpts) if err != nil { @@ -63,6 +65,7 @@ func (p *Builder) RebuildGraph( res.GetBuildReportDir(), res.GetJunitReportDir(), res.GetTrivyReportDir(), + buildArgs, ) for buildReport := range buildReportsChan { @@ -77,6 +80,7 @@ func (p *Builder) rebuildGraph( builder types.ImageBuilder, rateLimiter ratelimit.RateLimiter, buildReportDir, junitReportDir, trivyReportDir string, + buildArgs map[string]string, ) { p.Graph. Filter( @@ -100,7 +104,7 @@ func (p *Builder) rebuildGraph( if img.NeedsRebuild { meta := LoadCommonMetadata(&exec.ShellExecutor{}) if err := buildNode(node, builder, rateLimiter, meta, - p.PlaceholderTag, p.LocalOnly, buildReportDir, + p.PlaceholderTag, p.LocalOnly, buildReportDir, buildArgs, ); err != nil { img.RebuildFailed = true buildReportsChan <- buildReport.WithError(err) @@ -139,6 +143,7 @@ func buildNode( placeholderTag string, localOnly bool, buildReportDir string, + buildArgs map[string]string, ) error { rateLimiter.Acquire() defer rateLimiter.Release() @@ -151,11 +156,13 @@ func buildNode( for _, parent := range node.Parents() { tagsToReplace[parent.Image.DockerRef(placeholderTag)] = parent.Image.CurrentRef() } - if err := dockerfile.ReplaceTags(*img.Dockerfile, tagsToReplace); err != nil { + if err := dockerfile.ReplaceInFile( + path.Join(img.Dockerfile.ContextPath, img.Dockerfile.Filename), tagsToReplace); err != nil { return fmt.Errorf("failed to replace tag in dockerfile %s: %w", img.Dockerfile.ContextPath, err) } defer func() { - if err := dockerfile.ResetTags(*img.Dockerfile, tagsToReplace); err != nil { + if err := dockerfile.ResetFile( + path.Join(img.Dockerfile.ContextPath, img.Dockerfile.Filename), tagsToReplace); err != nil { logger.Warnf("failed to reset tag in dockerfile %s: %v", img.Dockerfile.ContextPath, err) } }() @@ -178,6 +185,7 @@ func buildNode( Labels: meta.WithImage(img).ToLabels(), Push: !localOnly, LogOutput: fileOutput, + BuildArgs: buildArgs, } logger.Infof("Building \"%s\" in context \"%s\"", img.CurrentRef(), img.Dockerfile.ContextPath) diff --git a/pkg/dib/build_test.go b/pkg/dib/build_test.go index 69a2a232..ac9047d7 100644 --- a/pkg/dib/build_test.go +++ b/pkg/dib/build_test.go @@ -205,7 +205,7 @@ func TestRebuildGraph(t *testing.T) { }, } - res := dibBuilder.RebuildGraph(builder, mock.RateLimiter{}) + res := dibBuilder.RebuildGraph(builder, mock.RateLimiter{}, map[string]string{}) assert.Len(t, res.BuildReports, len(test.expBuildReports)) for i, buildReport := range res.BuildReports { diff --git a/pkg/dib/generate_dag.go b/pkg/dib/generate_dag.go index 78baa903..d801ca88 100644 --- a/pkg/dib/generate_dag.go +++ b/pkg/dib/generate_dag.go @@ -27,7 +27,7 @@ const ( // GenerateDAG discovers and parses all Dockerfiles at a given path, // and generates the DAG representing the relationships between images. -func GenerateDAG(buildPath string, registryPrefix string, customHashListPath string) (*dag.DAG, error) { +func GenerateDAG(buildPath, registryPrefix, customHashListPath string, buildArgs map[string]string) (*dag.DAG, error) { var allFiles []string cache := make(map[string]*dag.Node) allParents := make(map[string][]dockerfile.ImageRef) @@ -38,6 +38,7 @@ func GenerateDAG(buildPath string, registryPrefix string, customHashListPath str if !info.IsDir() { allFiles = append(allFiles, filePath) } + if dockerfile.IsDockerfile(filePath) { dckfile, err := dockerfile.ParseDockerfile(filePath) if err != nil { @@ -115,14 +116,14 @@ func GenerateDAG(buildPath string, registryPrefix string, customHashListPath str } } - if err := generateHashes(graph, allFiles, customHashListPath); err != nil { + if err := generateHashes(graph, allFiles, customHashListPath, buildArgs); err != nil { return nil, err } return graph, nil } -func generateHashes(graph *dag.DAG, allFiles []string, customHashListPath string) error { +func generateHashes(graph *dag.DAG, allFiles []string, customHashListPath string, buildArgs map[string]string) error { customHumanizedHashList, err := loadCustomHumanizedHashList(customHashListPath) if err != nil { return err @@ -189,6 +190,29 @@ func generateHashes(graph *dag.DAG, allFiles []string, customHashListPath string humanizedKeywords = customHumanizedHashList } + filename := path.Join(node.Image.Dockerfile.ContextPath, node.Image.Dockerfile.Filename) + + argInstructionsToReplace := make(map[string]string) + for key, newArg := range buildArgs { + prevArgInstruction, ok := node.Image.Dockerfile.Args[key] + if ok { + argInstructionsToReplace[prevArgInstruction] = fmt.Sprintf("ARG %s=%s", key, newArg) + logger.Debugf("Overriding ARG instruction %q in %q [%q -> %q]", + key, filename, prevArgInstruction, fmt.Sprintf("ARG %s=%s", key, newArg)) + } + } + + if err := dockerfile.ReplaceInFile( + filename, argInstructionsToReplace); err != nil { + return fmt.Errorf("failed to replace ARG instructions in file %s: %w", filename, err) + } + defer func() { + if err := dockerfile.ResetFile( + filename, argInstructionsToReplace); err != nil { + logger.Warnf("failed to reset ARG instructions in file %q: %v", filename, err) + } + }() + hash, err := hashFiles(node.Image.Dockerfile.ContextPath, nodeFiles[node], parentHashes, humanizedKeywords) if err != nil { return fmt.Errorf("could not hash files for node %s: %w", node.Image.Name, err) diff --git a/pkg/dib/generate_dag_test.go b/pkg/dib/generate_dag_test.go index 173e432f..47a6dee2 100644 --- a/pkg/dib/generate_dag_test.go +++ b/pkg/dib/generate_dag_test.go @@ -18,7 +18,7 @@ const fixtureDir = "../../test/fixtures/docker" func TestGenerateDAG(t *testing.T) { t.Run("basic tests", func(t *testing.T) { graph, err := dib.GenerateDAG(fixtureDir, - "eu.gcr.io/my-test-repository", "") + "eu.gcr.io/my-test-repository", "", map[string]string{}) require.NoError(t, err) nodes := flattenNodes(graph) @@ -40,7 +40,7 @@ func TestGenerateDAG(t *testing.T) { tmpDir := copyFixtures(t) graph0, err := dib.GenerateDAG(tmpDir, - "eu.gcr.io/my-test-repository", "") + "eu.gcr.io/my-test-repository", "", map[string]string{}) require.NoError(t, err) nodes0 := flattenNodes(graph0) @@ -56,7 +56,7 @@ func TestGenerateDAG(t *testing.T) { // Then ONLY the hash of the child node bullseye/multistage should have changed graph1, err := dib.GenerateDAG(tmpDir, - "eu.gcr.io/my-test-repository", "") + "eu.gcr.io/my-test-repository", "", map[string]string{}) require.NoError(t, err) nodes1 := flattenNodes(graph1) @@ -73,7 +73,7 @@ func TestGenerateDAG(t *testing.T) { tmpDir := copyFixtures(t) graph0, err := dib.GenerateDAG(tmpDir, - "eu.gcr.io/my-test-repository", "") + "eu.gcr.io/my-test-repository", "", map[string]string{}) require.NoError(t, err) nodes0 := flattenNodes(graph0) @@ -89,7 +89,7 @@ func TestGenerateDAG(t *testing.T) { // Then ONLY the hash of the child node bullseye/multistage should have changed graph1, err := dib.GenerateDAG(tmpDir, - "eu.gcr.io/my-test-repository", "") + "eu.gcr.io/my-test-repository", "", map[string]string{}) require.NoError(t, err) nodes1 := flattenNodes(graph1) @@ -104,12 +104,13 @@ func TestGenerateDAG(t *testing.T) { t.Run("using custom hash list should change only hashes of nodes with custom label", func(t *testing.T) { graph0, err := dib.GenerateDAG(fixtureDir, - "eu.gcr.io/my-test-repository", "") + "eu.gcr.io/my-test-repository", "", map[string]string{}) require.NoError(t, err) graph1, err := dib.GenerateDAG(fixtureDir, "eu.gcr.io/my-test-repository", - "../../test/fixtures/dib/valid_wordlist.txt") + "../../test/fixtures/dib/valid_wordlist.txt", + map[string]string{}) require.NoError(t, err) nodes0 := flattenNodes(graph0) @@ -120,8 +121,29 @@ func TestGenerateDAG(t *testing.T) { subNode1 := nodes1["sub-image"] assert.Equal(t, rootNode1.Image.Hash, rootNode0.Image.Hash) - assert.Equal(t, "berlin-undress-hydrogen-april", subNode0.Image.Hash) - assert.Equal(t, "archeops-glaceon-chinchou-aipom", subNode1.Image.Hash) + assert.Equal(t, "violet-minnesota-alabama-alpha", subNode0.Image.Hash) + assert.Equal(t, "golduck-dialga-abra-aegislash", subNode1.Image.Hash) + }) + + t.Run("using arg used in root node should change all hashes", func(t *testing.T) { + graph0, err := dib.GenerateDAG(fixtureDir, + "eu.gcr.io/my-test-repository", "", + map[string]string{}) + require.NoError(t, err) + + graph1, err := dib.GenerateDAG(fixtureDir, + "eu.gcr.io/my-test-repository", "", + map[string]string{ + "HELLO": "world", + }) + require.NoError(t, err) + + nodes0 := flattenNodes(graph0) + rootNode0 := nodes0["bullseye"] + nodes1 := flattenNodes(graph1) + rootNode1 := nodes1["bullseye"] + + assert.NotEqual(t, rootNode1.Image.Hash, rootNode0.Image.Hash) }) } diff --git a/pkg/docker/builder.go b/pkg/docker/builder.go index cf341f13..b4873677 100644 --- a/pkg/docker/builder.go +++ b/pkg/docker/builder.go @@ -15,7 +15,7 @@ type ImageBuilderTagger struct { dryRun bool } -// NewImageBuilderTagger creates a new instance of an ImageBuilder. +// NewImageBuilderTagger creates a new instance of an ImageBuilderTagger. func NewImageBuilderTagger(executor exec.Executor, dryRun bool) *ImageBuilderTagger { return &ImageBuilderTagger{executor, dryRun} } @@ -37,7 +37,7 @@ func (b ImageBuilderTagger) Build(opts types.ImageBuilderOpts) error { } for _, tag := range opts.Tags { - dockerArgs = append(dockerArgs, "--tag="+tag) + dockerArgs = append(dockerArgs, fmt.Sprintf("--tag=%s", tag)) } dockerArgs = append(dockerArgs, opts.Context) @@ -53,15 +53,15 @@ func (b ImageBuilderTagger) Build(opts types.ImageBuilderOpts) error { return nil } - err := b.exec.ExecuteWithWriter(opts.LogOutput, "docker", dockerArgs...) - if err != nil { + if err := b.exec.ExecuteWithWriter( + opts.LogOutput, "docker", dockerArgs...); err != nil { return err } - if opts.Push && len(opts.Tags) > 0 { + if opts.Push { for _, tag := range opts.Tags { - err = b.exec.ExecuteWithWriter(opts.LogOutput, "docker", "push", tag) - if err != nil { + if err := b.exec.ExecuteWithWriter( + opts.LogOutput, "docker", "push", tag); err != nil { return err } } @@ -70,12 +70,12 @@ func (b ImageBuilderTagger) Build(opts types.ImageBuilderOpts) error { return nil } -// Tag runs a `docker tag`command to retag the source tag with the destination tag. +// Tag runs a docker tag command to re-tag the source tag with the destination tag. func (b ImageBuilderTagger) Tag(src, dest string) error { if b.dryRun { logger.Infof("[DRY-RUN] docker tag %s %s", src, dest) return nil } - logger.Debugf("Running `docker tag %s %s`", src, dest) + return b.exec.ExecuteStdout("docker", "tag", src, dest) } diff --git a/pkg/dockerfile/dockerfile.go b/pkg/dockerfile/dockerfile.go index 8e4d7bfa..0de15169 100644 --- a/pkg/dockerfile/dockerfile.go +++ b/pkg/dockerfile/dockerfile.go @@ -16,6 +16,7 @@ const dockerfileName = "Dockerfile" var ( rxFrom = regexp.MustCompile(`^FROM (?P(?P[^:@\s]+):?(?P[^\s@]+)?@?(?Psha256:.*)?)(?: as .*)?$`) //nolint:lll rxLabel = regexp.MustCompile(`^LABEL (\S+)="(\S+)"$`) + rxArg = regexp.MustCompile(`^ARG\s+([a-zA-Z_]\w*)(\s*=\s*[^#\n]*)?$`) ) // Dockerfile holds the information from a Dockerfile. @@ -24,6 +25,7 @@ type Dockerfile struct { Filename string From []ImageRef Labels map[string]string + Args map[string]string } // ImageRef holds the information about an image reference present in FROM statements. @@ -41,6 +43,10 @@ func (d *Dockerfile) addLabel(name string, value string) { d.Labels[name] = value } +func (d *Dockerfile) addArg(name string, value string) { + d.Args[name] = value +} + // IsDockerfile checks whether a file is a Dockerfile. func IsDockerfile(filename string) bool { return strings.HasSuffix(filename, dockerfileName) @@ -59,6 +65,7 @@ func ParseDockerfile(filename string) (*Dockerfile, error) { dckFile.ContextPath = path.Dir(filename) dckFile.Filename = path.Base(filename) dckFile.Labels = map[string]string{} + dckFile.Args = map[string]string{} scanner := bufio.NewScanner(file) for scanner.Scan() { @@ -81,24 +88,27 @@ func ParseDockerfile(filename string) (*Dockerfile, error) { case rxLabel.MatchString(txt): result := rxLabel.FindStringSubmatch(txt) dckFile.addLabel(result[1], result[2]) + case rxArg.MatchString(txt): + result := rxArg.FindStringSubmatch(txt) + dckFile.addArg(result[1], txt) } } if err := scanner.Err(); err != nil { return nil, err } - logger.Debugf("Successfully parsed dockerfile. From=%v, Labels=%v", dckFile.From, dckFile.Labels) + logger.Debugf("Successfully parsed dockerfile. From=%v, Labels=%v, Args=%v", + dckFile.From, dckFile.Labels, dckFile.Args) return &dckFile, nil } -// ReplaceTags replaces all matching tags by a replacement tag. -// The diff map keys are source image refs, and their values are the replacement refs. -// Many references to images may be replaced in the Dockerfile, -// those from the FROM statements, and also --from arguments. -func ReplaceTags(d Dockerfile, diff map[string]string) error { +// ReplaceInFile replaces all matching references by a replacement. +// The diff map keys are source references, and the values are replacements. +// Many references to images may be replaced, those from the FROM statements, and also --from arguments. +func ReplaceInFile(path string, diff map[string]string) error { for ref, newRef := range diff { - err := replace(path.Join(d.ContextPath, d.Filename), ref, newRef) + err := replace(path, ref, newRef) if err != nil { return fmt.Errorf("cannot replace \"%s\" with \"%s\": %w", ref, newRef, err) } @@ -106,13 +116,10 @@ func ReplaceTags(d Dockerfile, diff map[string]string) error { return nil } -// ResetTags resets the tags that were replaced by ReplaceTags by doing the opposite process. -// The diff map is the same map that was passed previously to ReplaceTags. -// Again, many references to images may be replaced in the Dockerfile, -// those from the FROM statements, and also --from arguments. -func ResetTags(d Dockerfile, diff map[string]string) error { +// ResetFile resets the strings that were replaced by ReplaceInFile by doing the opposite process. +func ResetFile(path string, diff map[string]string) error { for initialRef, newRef := range diff { - err := replace(path.Join(d.ContextPath, d.Filename), newRef, initialRef) + err := replace(path, newRef, initialRef) if err != nil { return fmt.Errorf("cannot reset tag \"%s\" to \"%s\": %w", newRef, initialRef, err) } diff --git a/pkg/dockerfile/dockerfile_test.go b/pkg/dockerfile/dockerfile_test.go index 94175849..de11036f 100644 --- a/pkg/dockerfile/dockerfile_test.go +++ b/pkg/dockerfile/dockerfile_test.go @@ -10,13 +10,14 @@ import ( "github.com/stretchr/testify/require" ) -func Test(t *testing.T) { +func TestParseDockerfile(t *testing.T) { t.Parallel() tests := map[string]struct { filename string expectedFrom []dockerfile.ImageRef expectedLabels map[string]string + expectedArgs map[string]string }{ "simple dockerfile": { filename: "simple.dockerfile", @@ -30,6 +31,7 @@ func Test(t *testing.T) { expectedLabels: map[string]string{ "name": "example", }, + expectedArgs: map[string]string{}, }, "simple dockerfile with digest": { filename: "simple-digest.dockerfile", @@ -43,6 +45,7 @@ func Test(t *testing.T) { expectedLabels: map[string]string{ "name": "example", }, + expectedArgs: map[string]string{}, }, "simple dockerfile with tag": { filename: "simple-tag.dockerfile", @@ -56,6 +59,21 @@ func Test(t *testing.T) { expectedLabels: map[string]string{ "name": "example", }, + expectedArgs: map[string]string{}, + }, + "simple dockerfile with arg": { + filename: "simple-arg.dockerfile", + expectedFrom: []dockerfile.ImageRef{ + { + Name: "registry.com/example", + Tag: "latest", + Digest: "", + }, + }, + expectedLabels: map[string]string{}, + expectedArgs: map[string]string{ + "HELLO": `ARG HELLO="there"`, + }, }, "simple dockerfile with tag and digest": { filename: "simple-tag-digest.dockerfile", @@ -69,6 +87,7 @@ func Test(t *testing.T) { expectedLabels: map[string]string{ "name": "example", }, + expectedArgs: map[string]string{}, }, "multistage dockerfile": { filename: "multistage.dockerfile", @@ -87,6 +106,7 @@ func Test(t *testing.T) { expectedLabels: map[string]string{ "name": "example", }, + expectedArgs: map[string]string{}, }, "multistage dockerfile with digest": { filename: "multistage-digest.dockerfile", @@ -105,6 +125,7 @@ func Test(t *testing.T) { expectedLabels: map[string]string{ "name": "example", }, + expectedArgs: map[string]string{}, }, "multistage dockerfile with tag": { filename: "multistage-tag.dockerfile", @@ -123,6 +144,7 @@ func Test(t *testing.T) { expectedLabels: map[string]string{ "name": "example", }, + expectedArgs: map[string]string{}, }, "multistage dockerfile with tag and digest": { filename: "multistage-tag-digest.dockerfile", @@ -141,6 +163,7 @@ func Test(t *testing.T) { expectedLabels: map[string]string{ "name": "example", }, + expectedArgs: map[string]string{}, }, } for name, test := range tests { @@ -149,15 +172,15 @@ func Test(t *testing.T) { t.Parallel() cwd, err := os.Getwd() - if err != nil { - t.Fatal("Failed to get current working directory.") - } - fullpath := path.Join(cwd, "../../test/fixtures/dockerfile", test.filename) + require.NoError(t, err) + fullpath := path.Join(cwd, "../../test/fixtures/dockerfile", test.filename) result, err := dockerfile.ParseDockerfile(fullpath) require.NoError(t, err) + assert.Equal(t, test.expectedFrom, result.From) assert.Equal(t, test.expectedLabels, result.Labels) + assert.Equal(t, test.expectedArgs, result.Args) }) } } diff --git a/pkg/exec/shell.go b/pkg/exec/shell.go index 872163ce..676f3388 100644 --- a/pkg/exec/shell.go +++ b/pkg/exec/shell.go @@ -38,8 +38,9 @@ func (e ShellExecutor) Execute(name string, args ...string) (string, error) { cmd.Stdout = &stdout cmd.Dir = e.Dir + logger.Debugf("Exec cmd: %s", cmd) if err := cmd.Run(); err != nil { - return stderr.String(), fmt.Errorf("failed to execute command `%s`: %w", name, err) + return stderr.String(), fmt.Errorf("failed to execute command: %s: %w", cmd, err) } return stdout.String(), nil @@ -53,9 +54,9 @@ func (e ShellExecutor) ExecuteWithWriters(stdout, stderr io.Writer, name string, cmd.Stdout = stdout cmd.Dir = e.Dir - logger.Debugf("cmd: %s", cmd.String()) + logger.Debugf("Exec cmd: %s", cmd) if err := cmd.Run(); err != nil { - return fmt.Errorf("failed to execute command `%s`: %w", name, err) + return fmt.Errorf("failed to execute command: %s: %w", cmd, err) } return nil @@ -63,10 +64,12 @@ func (e ShellExecutor) ExecuteWithWriters(stdout, stderr io.Writer, name string, // ExecuteWithWriter executes a command and forwards both stdout and stderr to a single io.Writer. func (e ShellExecutor) ExecuteWithWriter(writer io.Writer, name string, args ...string) error { + logger.Debugf("Exec cmd: %s %v", name, args) return e.ExecuteWithWriters(writer, writer, name, args...) } // ExecuteStdout executes a shell command and prints to the standard output. func (e ShellExecutor) ExecuteStdout(name string, args ...string) error { + logger.Debugf("Exec cmd: %s %v", name, args) return e.ExecuteWithWriters(os.Stdout, os.Stderr, name, args...) } diff --git a/pkg/graphviz/graphviz_test.go b/pkg/graphviz/graphviz_test.go index 483e5bad..28037dda 100644 --- a/pkg/graphviz/graphviz_test.go +++ b/pkg/graphviz/graphviz_test.go @@ -18,7 +18,9 @@ func Test_GenerateDotviz(t *testing.T) { require.NoError(t, err) graph, err := dib.GenerateDAG( - path.Join(cwd, "../../test/fixtures/docker"), "eu.gcr.io/my-test-repository", "") + path.Join(cwd, "../../test/fixtures/docker"), + "eu.gcr.io/my-test-repository", "", + map[string]string{}) require.NoError(t, err) dir, err := os.MkdirTemp("/tmp", "dib-test") diff --git a/pkg/kaniko/builder.go b/pkg/kaniko/builder.go index 208f0915..c1080a7b 100644 --- a/pkg/kaniko/builder.go +++ b/pkg/kaniko/builder.go @@ -18,7 +18,7 @@ import ( type ContextProvider interface { // PrepareContext allows to do some operations on the build context before the executor runs, // like moving it to a remote location in order to be accessible by remote executors. - // It must returns an URL compatible with Kaniko's `--context` flag. + // It must return a URL compatible with Kaniko's `--context` flag. PrepareContext(opts types.ImageBuilderOpts) (string, error) } @@ -83,7 +83,7 @@ func (b Builder) Build(opts types.ImageBuilderOpts) error { } for _, tag := range opts.Tags { - kanikoArgs = append(kanikoArgs, "--destination="+tag) + kanikoArgs = append(kanikoArgs, fmt.Sprintf("--destination=%s", tag)) } for k, v := range opts.BuildArgs { diff --git a/test/fixtures/docker/bullseye/Dockerfile b/test/fixtures/docker/bullseye/Dockerfile index 6d820b00..03dfbc9a 100644 --- a/test/fixtures/docker/bullseye/Dockerfile +++ b/test/fixtures/docker/bullseye/Dockerfile @@ -2,3 +2,7 @@ FROM debian:bullseye LABEL name="bullseye" LABEL version="v1" + +ARG HELLO="there" + +RUN echo "Hello $HELLO" diff --git a/test/fixtures/dockerfile/simple-arg.dockerfile b/test/fixtures/dockerfile/simple-arg.dockerfile new file mode 100644 index 00000000..faa51470 --- /dev/null +++ b/test/fixtures/dockerfile/simple-arg.dockerfile @@ -0,0 +1,2 @@ +FROM registry.com/example:latest +ARG HELLO="there" From 8e5a957e3bcec1bd6817a22dc88be4625ae0a929 Mon Sep 17 00:00:00 2001 From: Antoine Gelloz Date: Mon, 19 Feb 2024 15:44:50 +0100 Subject: [PATCH 2/4] review by the team --- cmd/build.go | 4 ++-- cmd/list.go | 21 ++++++++++++++++++++- docs/configuration.md | 2 +- docs/roadmap.md | 2 +- pkg/dib/list.go | 3 ++- pkg/dockerfile/dockerfile_test.go | 24 ++++++++++++++++++++++++ 6 files changed, 50 insertions(+), 6 deletions(-) diff --git a/cmd/build.go b/cmd/build.go index 7042eb98..2f0757de 100644 --- a/cmd/build.go +++ b/cmd/build.go @@ -51,11 +51,11 @@ Otherwise, dib will create a new tag based on the previous tag.`, for _, arg := range opts.BuildArg { key, val, hasVal := strings.Cut(arg, "=") if hasVal { - buildArgs[key] = val + buildArgs[key] = os.ExpandEnv(val) } else { // check if the env is set in the local environment and use that value if it is if val, present := os.LookupEnv(key); present { - buildArgs[key] = val + buildArgs[key] = os.ExpandEnv(val) } else { delete(buildArgs, key) } diff --git a/cmd/list.go b/cmd/list.go index 68111b0b..bf733c3a 100644 --- a/cmd/list.go +++ b/cmd/list.go @@ -2,7 +2,9 @@ package main import ( "fmt" + "os" "path" + "strings" "github.com/radiofrance/dib/internal/logger" "github.com/radiofrance/dib/pkg/dib" @@ -32,6 +34,8 @@ func init() { listCmd.Flags().StringP("output", "o", "", ""+ "Output format (console|go-template-file)\n"+ "You can provide a custom format using go-template: like this: \"-o go-template-file=...\".") + listCmd.Flags().StringArray("build-arg", []string{}, + "`argument=value` to supply to the builder") } func doList(opts dib.ListOpts) error { @@ -45,8 +49,23 @@ func doList(opts dib.ListOpts) error { return err } + buildArgs := map[string]string{} + for _, arg := range opts.BuildArg { + key, val, hasVal := strings.Cut(arg, "=") + if hasVal { + buildArgs[key] = os.ExpandEnv(val) + } else { + // check if the env is set in the local environment and use that value if it is + if val, present := os.LookupEnv(key); present { + buildArgs[key] = os.ExpandEnv(val) + } else { + delete(buildArgs, key) + } + } + } + buildPath := path.Join(workingDir, opts.BuildPath) - graph, err := dib.GenerateDAG(buildPath, opts.RegistryURL, opts.HashListFilePath, map[string]string{}) + graph, err := dib.GenerateDAG(buildPath, opts.RegistryURL, opts.HashListFilePath, buildArgs) if err != nil { return fmt.Errorf("cannot generate DAG: %w", err) } diff --git a/docs/configuration.md b/docs/configuration.md index 6bc6b467..913fedcf 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -10,7 +10,7 @@ default values in the configuration file, and then override with environment var Example: ```shell -dib build --registry-url=gcr.io/project +dib build --registry-url=gcr.io/project --build-arg=foo=bar ``` ### Environment variables diff --git a/docs/roadmap.md b/docs/roadmap.md index 5345b185..00ae3907 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -6,7 +6,7 @@ Roadmap DIB is still a work in progress, but we plan to release a stable version (v1.0.0) after we have added the following features: -- **Per-image configuration:** Some images may require additional build args, or have their own tagging scheme. Being +- **Per-image configuration:** Some images may require their own tagging scheme. Being able to configure those settings for each image is necessary. diff --git a/pkg/dib/list.go b/pkg/dib/list.go index 03e2a4e0..a5441f55 100644 --- a/pkg/dib/list.go +++ b/pkg/dib/list.go @@ -117,5 +117,6 @@ type ListOpts struct { HashListFilePath string `mapstructure:"hash_list_file_path"` // List specific options - Output string `mapstructure:"output"` + Output string `mapstructure:"output"` + BuildArg []string `mapstructure:"build_arg"` } diff --git a/pkg/dockerfile/dockerfile_test.go b/pkg/dockerfile/dockerfile_test.go index de11036f..6431d953 100644 --- a/pkg/dockerfile/dockerfile_test.go +++ b/pkg/dockerfile/dockerfile_test.go @@ -184,3 +184,27 @@ func TestParseDockerfile(t *testing.T) { }) } } + +func TestReplaceAndReset(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + filename := path.Join(tmpDir, "replace.dockerfile") + oldContent := `FROM registry.com/example\nLABEL name="example"` + require.NoError(t, os.WriteFile(filename, + []byte(oldContent), 0o600)) + diff := map[string]string{ + "registry.com": "registries.io", + "example": "other", + } + newContent := `FROM registries.io/other\nLABEL name="other"` + require.NoError(t, dockerfile.ReplaceInFile(filename, diff)) + content, err := os.ReadFile(filename) + require.NoError(t, err) + assert.Equal(t, newContent, string(content)) + + require.NoError(t, dockerfile.ResetFile(filename, diff)) + content, err = os.ReadFile(filename) + require.NoError(t, err) + assert.Equal(t, oldContent, string(content)) +} From 2dfa83479b3e99012b225624141b2bd6fb587ae8 Mon Sep 17 00:00:00 2001 From: Antoine Gelloz Date: Tue, 20 Feb 2024 11:58:41 +0100 Subject: [PATCH 3/4] review by team --- docs/configuration.md | 2 +- docs/examples/config/reference.yaml | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 913fedcf..6bc6b467 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -10,7 +10,7 @@ default values in the configuration file, and then override with environment var Example: ```shell -dib build --registry-url=gcr.io/project --build-arg=foo=bar +dib build --registry-url=gcr.io/project ``` ### Environment variables diff --git a/docs/examples/config/reference.yaml b/docs/examples/config/reference.yaml index 86e32e36..e0c7446c 100644 --- a/docs/examples/config/reference.yaml +++ b/docs/examples/config/reference.yaml @@ -1,5 +1,5 @@ --- -# Log level: "trace", "debug", "info", "warning", "error", "fatal", "panic". Defaults to "info". +# Log level: "debug", "info", "warning", "error", "fatal". Defaults to "info". log_level: info # URL of the registry where the images should be stored. @@ -18,6 +18,11 @@ placeholder_tag: latest # when using the Kubernetes executor as build pods are scheduled across multiple nodes. rate_limit: 1 +# Use build arguments to set build-time variables. The format is a list of strings. +build_arg: + - FOO1="bar1" + - FOO2="bar2" + # Path to the directory where the reports are generated. The directory will be created if it doesn't exist. reports_dir: reports From 5852832e338f11f114a98fe8f6098384ac353294 Mon Sep 17 00:00:00 2001 From: Antoine Gelloz Date: Tue, 20 Feb 2024 14:36:03 +0100 Subject: [PATCH 4/4] review by team --- docs/examples/config/reference.yaml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/examples/config/reference.yaml b/docs/examples/config/reference.yaml index e0c7446c..6835b82b 100644 --- a/docs/examples/config/reference.yaml +++ b/docs/examples/config/reference.yaml @@ -18,10 +18,11 @@ placeholder_tag: latest # when using the Kubernetes executor as build pods are scheduled across multiple nodes. rate_limit: 1 -# Use build arguments to set build-time variables. The format is a list of strings. +# Use build arguments to set build-time variables. The format is a list of strings. Env vars are expanded. build_arg: - FOO1="bar1" - - FOO2="bar2" + - FOO2=$BAR + - FOO3=${BAR} # Path to the directory where the reports are generated. The directory will be created if it doesn't exist. reports_dir: reports