From c2bfb6ae301212def0112633afa15714a4f72abd Mon Sep 17 00:00:00 2001 From: Tianon Gravi Date: Thu, 6 Mar 2025 15:33:04 -0800 Subject: [PATCH 1/2] WIP: allow *parsing* cross-compile (allows for more efficient builds when we already know we're already emulation-building) --- cmd/bashbrew/docker.go | 2 +- pkg/dockerfile/parse.go | 128 +++++++++++++++++++++++------------ pkg/dockerfile/parse_test.go | 69 +++++++++++++------ 3 files changed, 137 insertions(+), 62 deletions(-) diff --git a/cmd/bashbrew/docker.go b/cmd/bashbrew/docker.go index 596d06c4..d7d6cc0a 100644 --- a/cmd/bashbrew/docker.go +++ b/cmd/bashbrew/docker.go @@ -23,7 +23,7 @@ func (r Repo) ArchLastStageFrom(arch string, entry *manifest.Manifest2822Entry) if err != nil { return "", err } - return dockerfileMeta.StageFroms[len(dockerfileMeta.StageFroms)-1], nil + return dockerfileMeta.Stages[len(dockerfileMeta.Stages)-1].From, nil } func (r Repo) DockerFroms(entry *manifest.Manifest2822Entry) ([]string, error) { diff --git a/pkg/dockerfile/parse.go b/pkg/dockerfile/parse.go index cba566c8..e5662fb7 100644 --- a/pkg/dockerfile/parse.go +++ b/pkg/dockerfile/parse.go @@ -2,16 +2,24 @@ package dockerfile import ( "bufio" + "fmt" "io" "strconv" "strings" "unicode" ) +type Stage struct { + From string // image name (or parent stage's image name, if "FROM stage-name") + FromStage string // original stage name, if "FROM stage-name" + Name string // empty for unnamed stages + Platform string // empty string, $BUILDPLATFORM, or $TARGETPLATFORM + // TODO somehow, we need to expose the platform of each stage to meta-scripts so it can know that, for example, the build stage base image is only needed for the *host* platform, not the target platform +} + type Metadata struct { - StageFroms []string // every image "FROM" instruction value (or the parent stage's FROM value in the case of a named stage) - StageNames []string // the name of any named stage (in order) - StageNameFroms map[string]string // map of stage names to FROM values (or the parent stage's FROM value in the case of a named stage), useful for resolving stage names to FROM values + Stages []Stage + NamedStages map[string]int // map of stage names to index in Stages slice Froms []string // every "FROM" or "COPY --from=xxx" value (minus named and/or numbered stages in the case of "--from=") } @@ -23,7 +31,7 @@ func Parse(dockerfile string) (Metadata, error) { func ParseReader(dockerfile io.Reader) (Metadata, error) { meta := Metadata{ // panic: assignment to entry in nil map - StageNameFroms: map[string]string{}, + NamedStages: map[string]int{}, // (nil slices work fine) } @@ -77,76 +85,100 @@ func ParseReader(dockerfile io.Reader) (Metadata, error) { instruction := strings.ToUpper(fields[0]) - // TODO balk at ARG / $ in from values + args := fields[1:] switch instruction { case "FROM": - from := fields[1] - - if stageFrom, ok := meta.StageNameFroms[from]; ok { - // if this is a valid stage name, we should resolve it back to the original FROM value of that previous stage (we don't care about inter-stage dependencies for the purposes of either tag dependency calculation or tag building -- just how many there are and what external things they require) - from = stageFrom + var stage Stage + if platform, ok := strings.CutPrefix(args[0], "--platform="); ok { + stage.Platform = platform + args = args[1:] + switch stage.Platform { + case "$BUILDPLATFORM", "$TARGETPLATFORM": + // explicitly allowed for more efficient cross-compiling (see also condition outside the meta loop to ensure the final stage is either without platform or explicitly --platform=$TARGETPLATFORM) + default: + return meta, fmt.Errorf("FROM has unsupported --platform=%q -- any --platform must be generic or unspecified for correct dependency calculation", stage.Platform) + } } - // make sure to add ":latest" if it's implied - from = latestizeRepoTag(from) + stage.From = args[0] + args = args[1:] - meta.StageFroms = append(meta.StageFroms, from) - meta.Froms = append(meta.Froms, from) + if strings.ContainsRune(stage.From, '$') { + return meta, fmt.Errorf("FROM %q contains invalid/disallowed character '$' -- explicit FROM values are required for dependency calculation", stage.From) + } + + if i, ok := meta.NamedStages[stage.From]; ok { + // if this is a valid stage name, we should resolve it back to the original FROM value of that previous stage (we don't care about inter-stage dependencies for the purposes of either tag dependency calculation or tag building -- just how many there are and what external things they require) + parent := meta.Stages[i] + if stage.Platform == "" { + stage.Platform = parent.Platform + } else if stage.Platform != parent.Platform { + return meta, fmt.Errorf("FROM %q has --platform=%q but stage %q has --platform=%q", stage.From, stage.Platform, stage.From, parent.Platform) + } + stage.FromStage = stage.From + stage.From = parent.From + } else { + // make sure to add ":latest" if it's implied + stage.From = latestizeRepoTag(stage.From) + } - if len(fields) == 4 && strings.ToUpper(fields[2]) == "AS" { - stageName := fields[3] - meta.StageNames = append(meta.StageNames, stageName) - meta.StageNameFroms[stageName] = from + i := len(meta.Stages) + if len(args) == 2 && strings.ToUpper(args[0]) == "AS" { + stage.Name = args[1] + meta.NamedStages[stage.Name] = i } + meta.Stages = append(meta.Stages, stage) + + meta.Froms = append(meta.Froms, stage.From) case "COPY": - for _, arg := range fields[1:] { + for _, arg := range args { if !strings.HasPrefix(arg, "--") { // doesn't appear to be a "flag"; time to bail! break } - if !strings.HasPrefix(arg, "--from=") { + from, ok := strings.CutPrefix(arg, "--from=") + if !ok { // ignore any flags we're not interested in continue } - from := arg[len("--from="):] - if stageFrom, ok := meta.StageNameFroms[from]; ok { + if i, ok := meta.NamedStages[from]; ok { // see note above regarding stage names in FROM - from = stageFrom - } else if stageNumber, err := strconv.Atoi(from); err == nil && stageNumber < len(meta.StageFroms) { + from = meta.Stages[i].From + } else if stageNumber, err := strconv.Atoi(from); err == nil && stageNumber < len(meta.Stages) { // must be a stage number, we should resolve it too - from = meta.StageFroms[stageNumber] + from = meta.Stages[stageNumber].From + } else { + // make sure to add ":latest" if it's implied + from = latestizeRepoTag(from) } - // make sure to add ":latest" if it's implied - from = latestizeRepoTag(from) - meta.Froms = append(meta.Froms, from) } case "RUN": // TODO combine this and the above COPY-parsing code somehow sanely - for _, arg := range fields[1:] { + for _, arg := range args { if !strings.HasPrefix(arg, "--") { // doesn't appear to be a "flag"; time to bail! break } - if !strings.HasPrefix(arg, "--mount=") { + csv, ok := strings.CutPrefix(arg, "--mount=") + if !ok { // ignore any flags we're not interested in continue } - csv := arg[len("--mount="):] // TODO more correct CSV parsing fields := strings.Split(csv, ",") var mountType, from string for _, field := range fields { - if strings.HasPrefix(field, "type=") { - mountType = field[len("type="):] + if val, ok := strings.CutPrefix(field, "type="); ok { + mountType = val continue } - if strings.HasPrefix(field, "from=") { - from = field[len("from="):] + if val, ok := strings.CutPrefix(field, "from="); ok { + from = val continue } } @@ -155,21 +187,33 @@ func ParseReader(dockerfile io.Reader) (Metadata, error) { continue } - if stageFrom, ok := meta.StageNameFroms[from]; ok { + if i, ok := meta.NamedStages[from]; ok { // see note above regarding stage names in FROM - from = stageFrom - } else if stageNumber, err := strconv.Atoi(from); err == nil && stageNumber < len(meta.StageFroms) { + from = meta.Stages[i].From + } else if stageNumber, err := strconv.Atoi(from); err == nil && stageNumber < len(meta.Stages) { // must be a stage number, we should resolve it too - from = meta.StageFroms[stageNumber] + from = meta.Stages[stageNumber].From + } else { + // make sure to add ":latest" if it's implied + from = latestizeRepoTag(from) } - // make sure to add ":latest" if it's implied - from = latestizeRepoTag(from) - meta.Froms = append(meta.Froms, from) } } } + + // TODO maybe we *shouldn't* support parsing a fully empty Dockerfile? 🤔 (we actively use an "empty" Dockerfile in the tests to test edge cases of continuation though that are otherwise hard to test, so it's probably ~fine) + if len(meta.Stages) > 0 { + finalStage := meta.Stages[len(meta.Stages)-1] + switch finalStage.Platform { + case "", "$TARGETPLATFORM": + // yay, all is well + default: + return meta, fmt.Errorf("final stage/FROM (%q) has --platform=%q but must be unspecified or $TARGETPLATFORM", finalStage.From, finalStage.Platform) + } + } + return meta, scanner.Err() } diff --git a/pkg/dockerfile/parse_test.go b/pkg/dockerfile/parse_test.go index e4cd31c9..6cb6b88a 100644 --- a/pkg/dockerfile/parse_test.go +++ b/pkg/dockerfile/parse_test.go @@ -63,16 +63,17 @@ func TestParse(t *testing.T) { COPY --from=bar / / COPY --from=foo2 / / COPY --chown=1234:5678 /foo /bar + COPY --from=hello-world /hello /usr/local/bin/ `, metadata: dockerfile.Metadata{ - StageFroms: []string{"bash:latest", "busybox:uclibc", "bash:5", "bash:latest", "scratch"}, - StageNames: []string{"foo", "bar", "foo2"}, - StageNameFroms: map[string]string{ - "foo": "bash:latest", - "bar": "bash:5", - "foo2": "bash:latest", + Stages: []dockerfile.Stage{ + {From: "bash:latest", Name: "foo"}, + {From: "busybox:uclibc"}, + {From: "bash:5", Name: "bar"}, + {From: "bash:latest", FromStage: "foo", Name: "foo2"}, + {From: "scratch"}, }, - Froms: []string{"bash:latest", "busybox:uclibc", "bash:5", "bash:latest", "scratch", "bash:latest", "bash:5", "bash:latest"}, + Froms: []string{"bash:latest", "busybox:uclibc", "bash:5", "bash:latest", "scratch", "bash:latest", "bash:5", "bash:latest", "hello-world:latest"}, }, }, { @@ -127,8 +128,13 @@ func TestParse(t *testing.T) { RUN --mount=type=bind,from=2 cat /foo `, metadata: dockerfile.Metadata{ - StageFroms: []string{"bash:latest", "scratch", "scratch", "bash:latest"}, - Froms: []string{"bash:latest", "scratch", "bash:latest", "scratch", "scratch", "bash:latest", "scratch"}, + Stages: []dockerfile.Stage{ + {From: "bash:latest"}, + {From: "scratch"}, + {From: "scratch"}, + {From: "bash:latest"}, + }, + Froms: []string{"bash:latest", "scratch", "bash:latest", "scratch", "scratch", "bash:latest", "scratch"}, }, }, { @@ -138,8 +144,8 @@ func TestParse(t *testing.T) { RUN --mount=type=bind,from=busybox:uclibc,target=/tmp ["/tmp/bin/sh","-euxc","echo foo > /foo"] `, metadata: dockerfile.Metadata{ - StageFroms: []string{"scratch"}, - Froms: []string{"scratch", "busybox:uclibc"}, + Stages: []dockerfile.Stage{{From: "scratch"}}, + Froms: []string{"scratch", "busybox:uclibc"}, }, }, { @@ -152,10 +158,27 @@ func TestParse(t *testing.T) { RUN --mount=type=bind,from=bb,target=/tmp ["/tmp/bin/sh","-euxc","echo foo > /foo"] `, metadata: dockerfile.Metadata{ - StageFroms: []string{"busybox:uclibc", "scratch"}, - StageNames: []string{"bb"}, - StageNameFroms: map[string]string{"bb": "busybox:uclibc"}, - Froms: []string{"busybox:uclibc", "scratch", "busybox:uclibc"}, + Stages: []dockerfile.Stage{ + {From: "busybox:uclibc", Name: "bb"}, + {From: "scratch"}, + }, + Froms: []string{"busybox:uclibc", "scratch", "busybox:uclibc"}, + }, + }, + { + name: "FROM --platform", + dockerfile: ` + FROM --platform=$BUILDPLATFORM golang AS build + RUN do some stuff + FROM --platform=$TARGETPLATFORM debian + COPY --from=build /some/binary /some/other/place + `, + metadata: dockerfile.Metadata{ + Stages: []dockerfile.Stage{ + {From: "golang:latest", Name: "build", Platform: "$BUILDPLATFORM"}, + {From: "debian:latest", Platform: "$TARGETPLATFORM"}, + }, + Froms: []string{"golang:latest", "debian:latest", "golang:latest"}, }, }, } { @@ -164,11 +187,19 @@ func TestParse(t *testing.T) { if td.name == "" { td.name = td.dockerfile } - if len(td.metadata.Froms) > 0 && len(td.metadata.StageFroms) == 0 { - td.metadata.StageFroms = td.metadata.Froms + if len(td.metadata.Froms) > 0 && len(td.metadata.Stages) == 0 { + td.metadata.Stages = make([]dockerfile.Stage, len(td.metadata.Froms)) + for i, from := range td.metadata.Froms { + td.metadata.Stages[i].From = from + } } - if td.metadata.StageNameFroms == nil { - td.metadata.StageNameFroms = map[string]string{} + if td.metadata.NamedStages == nil { + td.metadata.NamedStages = map[string]int{} + for i, stage := range td.metadata.Stages { + if stage.Name != "" { + td.metadata.NamedStages[stage.Name] = i + } + } } t.Run(td.name, func(t *testing.T) { parsed, err := dockerfile.Parse(td.dockerfile) From 68cd0037eb0569a97d5bce32286aceee6e158de2 Mon Sep 17 00:00:00 2001 From: Tianon Gravi Date: Thu, 20 Aug 2026 16:25:38 -0700 Subject: [PATCH 2/2] Implement cross-platform parsing This should provide enough detail that we can resolve platforms correctly downstream in meta-scripts. Co-authored-by: Tianon Gravi Assisted-By: "claude my eyes right out" --- cmd/bashbrew/docker.go | 18 ++++- pkg/dockerfile/parse.go | 151 +++++++++++++++++++++-------------- pkg/dockerfile/parse_test.go | 103 ++++++++++++++---------- 3 files changed, 169 insertions(+), 103 deletions(-) diff --git a/cmd/bashbrew/docker.go b/cmd/bashbrew/docker.go index d7d6cc0a..b71c66e7 100644 --- a/cmd/bashbrew/docker.go +++ b/cmd/bashbrew/docker.go @@ -19,11 +19,16 @@ import ( // this returns the "FROM" value for the last stage (which essentially determines the "base" for the final published image) func (r Repo) ArchLastStageFrom(arch string, entry *manifest.Manifest2822Entry) (string, error) { - dockerfileMeta, err := r.archDockerfileMetadata(arch, entry) + parents, err := r.ArchDockerfileParents(arch, entry) if err != nil { return "", err } - return dockerfileMeta.Stages[len(dockerfileMeta.Stages)-1].From, nil + for i := len(parents) - 1; i >= 0; i-- { + if parents[i].Kind == "FROM" { + return parents[i].From, nil + } + } + return "", fmt.Errorf("no FROM found for arch %q from entry %q", arch, entry.String()) } func (r Repo) DockerFroms(entry *manifest.Manifest2822Entry) ([]string, error) { @@ -38,6 +43,15 @@ func (r Repo) ArchDockerFroms(arch string, entry *manifest.Manifest2822Entry) ([ return dockerfileMeta.Froms, nil } +// exposes every parent reference (both "FROM" and external "COPY --from="/"RUN --mount=...,from=") tagged with "Kind" and (for "FROM") "Platform", so callers can decide for themselves which are build-pinned vs target-pinned, instead of bashbrew choosing one slice to hand back and leaving the complement as a set-subtraction exercise +func (r Repo) ArchDockerfileParents(arch string, entry *manifest.Manifest2822Entry) ([]dockerfile.Parent, error) { + dockerfileMeta, err := r.archDockerfileMetadata(arch, entry) + if err != nil { + return nil, err + } + return dockerfileMeta.Parents, nil +} + func (r Repo) dockerfileMetadata(entry *manifest.Manifest2822Entry) (dockerfile.Metadata, error) { return r.archDockerfileMetadata(arch, entry) } diff --git a/pkg/dockerfile/parse.go b/pkg/dockerfile/parse.go index e5662fb7..1249daaa 100644 --- a/pkg/dockerfile/parse.go +++ b/pkg/dockerfile/parse.go @@ -9,17 +9,34 @@ import ( "unicode" ) -type Stage struct { - From string // image name (or parent stage's image name, if "FROM stage-name") - FromStage string // original stage name, if "FROM stage-name" - Name string // empty for unnamed stages - Platform string // empty string, $BUILDPLATFORM, or $TARGETPLATFORM - // TODO somehow, we need to expose the platform of each stage to meta-scripts so it can know that, for example, the build stage base image is only needed for the *host* platform, not the target platform +// internal-only bookkeeping used while scanning to resolve "FROM stage-name", +// "COPY --from=stage-name-or-number", and "RUN --mount=...,from=..." back to +// real image references; never exposed outside this package +// TODO stage name/reference (e.g. "build") is available here if a future consumer needs it +type namedStage struct { + from string + platform string +} + +// Parent represents a single parent reference in a Dockerfile: every "FROM" +// (Kind == "FROM") and every "COPY --from="/"RUN --mount=...,from=" that +// references an external image rather than a local stage (Kind == "COPY" or +// "RUN"). A "--from=" referencing a local stage does not get its own Parent, +// since the underlying image is already represented by that stage's own +// Kind == "FROM" entry -- and, per BuildKit's own behavior, an external +// "--from=" reference always resolves against the target platform (there is +// no way to make it inherit a stage's platform, so Platform is always "" +// for Kind != "FROM"). +type Parent struct { + From string + // "" (also covers an explicit "$TARGETPLATFORM", normalized away here -- + // they're semantically identical) or "$BUILDPLATFORM"; always "" for Kind != "FROM" + Platform string `json:",omitempty"` + Kind string // "FROM", "COPY", "RUN" } type Metadata struct { - Stages []Stage - NamedStages map[string]int // map of stage names to index in Stages slice + Parents []Parent Froms []string // every "FROM" or "COPY --from=xxx" value (minus named and/or numbered stages in the case of "--from=") } @@ -29,10 +46,26 @@ func Parse(dockerfile string) (Metadata, error) { } func ParseReader(dockerfile io.Reader) (Metadata, error) { - meta := Metadata{ - // panic: assignment to entry in nil map - NamedStages: map[string]int{}, - // (nil slices work fine) + var meta Metadata + + // parsing-time-only bookkeeping (see "namedStage" above) + var stages []namedStage + namedStages := map[string]int{} + + // isStage tells callers to skip adding a Parent for this --from= -- a stage reference's + // platform (if any, e.g. via "FROM --platform=$BUILDPLATFORM foo AS bar") is already + // captured on that stage's own Kind == "FROM" Parent, so recording it again here would + // either duplicate or (worse) misrepresent it as target-pinned + resolveFrom := func(from string) (resolved string, isStage bool) { + if i, ok := namedStages[from]; ok { + // see note above regarding stage names in FROM + return stages[i].from, true + } else if stageNumber, err := strconv.Atoi(from); err == nil && stageNumber < len(stages) { + // must be a stage number, we should resolve it too + return stages[stageNumber].from, true + } + // make sure to add ":latest" if it's implied + return latestizeRepoTag(from), false } scanner := bufio.NewScanner(dockerfile) @@ -89,48 +122,58 @@ func ParseReader(dockerfile io.Reader) (Metadata, error) { switch instruction { case "FROM": - var stage Stage + var stage namedStage + var stageName string + explicitPlatform := "" // exactly as written: "", "$BUILDPLATFORM", or "$TARGETPLATFORM" -- kept distinct from "" (unspecified) until after stage-name resolution below, since normalizing "$TARGETPLATFORM" away too early would make an *explicit* "--platform=$TARGETPLATFORM" indistinguishable from "wrote nothing at all", silently inheriting a referenced stage's "$BUILDPLATFORM" instead of erroring on the mismatch + if platform, ok := strings.CutPrefix(args[0], "--platform="); ok { - stage.Platform = platform + explicitPlatform = platform args = args[1:] - switch stage.Platform { + switch explicitPlatform { case "$BUILDPLATFORM", "$TARGETPLATFORM": // explicitly allowed for more efficient cross-compiling (see also condition outside the meta loop to ensure the final stage is either without platform or explicitly --platform=$TARGETPLATFORM) default: - return meta, fmt.Errorf("FROM has unsupported --platform=%q -- any --platform must be generic or unspecified for correct dependency calculation", stage.Platform) + return meta, fmt.Errorf("FROM has unsupported --platform=%q -- any --platform must be generic or unspecified for correct dependency calculation", explicitPlatform) } } + // normalized for comparison/storage -- "" and "$TARGETPLATFORM" are semantically identical; "explicitPlatform" (unnormalized) is kept for the "was anything written at all" check below and for a more useful error message + normalizedPlatform := explicitPlatform + if normalizedPlatform == "$TARGETPLATFORM" { + normalizedPlatform = "" + } + stage.platform = normalizedPlatform - stage.From = args[0] + from := args[0] args = args[1:] - if strings.ContainsRune(stage.From, '$') { - return meta, fmt.Errorf("FROM %q contains invalid/disallowed character '$' -- explicit FROM values are required for dependency calculation", stage.From) + if strings.ContainsRune(from, '$') { + return meta, fmt.Errorf("FROM %q contains invalid/disallowed character '$' -- explicit FROM values are required for dependency calculation", from) } - if i, ok := meta.NamedStages[stage.From]; ok { + if i, ok := namedStages[from]; ok { // if this is a valid stage name, we should resolve it back to the original FROM value of that previous stage (we don't care about inter-stage dependencies for the purposes of either tag dependency calculation or tag building -- just how many there are and what external things they require) - parent := meta.Stages[i] - if stage.Platform == "" { - stage.Platform = parent.Platform - } else if stage.Platform != parent.Platform { - return meta, fmt.Errorf("FROM %q has --platform=%q but stage %q has --platform=%q", stage.From, stage.Platform, stage.From, parent.Platform) + parent := stages[i] + if explicitPlatform == "" { + // bare "FROM stage-name" makes no platform assertion of its own -- just continue whatever the referenced stage resolved to + stage.platform = parent.platform + } else if normalizedPlatform != parent.platform { + return meta, fmt.Errorf("FROM %q has --platform=%q but stage %q has --platform=%q", from, explicitPlatform, from, parent.platform) } - stage.FromStage = stage.From - stage.From = parent.From + stage.from = parent.from } else { // make sure to add ":latest" if it's implied - stage.From = latestizeRepoTag(stage.From) + stage.from = latestizeRepoTag(from) } - i := len(meta.Stages) + i := len(stages) if len(args) == 2 && strings.ToUpper(args[0]) == "AS" { - stage.Name = args[1] - meta.NamedStages[stage.Name] = i + stageName = args[1] + namedStages[stageName] = i } - meta.Stages = append(meta.Stages, stage) + stages = append(stages, stage) - meta.Froms = append(meta.Froms, stage.From) + meta.Froms = append(meta.Froms, stage.from) + meta.Parents = append(meta.Parents, Parent{From: stage.from, Platform: stage.platform, Kind: "FROM"}) case "COPY": for _, arg := range args { @@ -144,18 +187,11 @@ func ParseReader(dockerfile io.Reader) (Metadata, error) { continue } - if i, ok := meta.NamedStages[from]; ok { - // see note above regarding stage names in FROM - from = meta.Stages[i].From - } else if stageNumber, err := strconv.Atoi(from); err == nil && stageNumber < len(meta.Stages) { - // must be a stage number, we should resolve it too - from = meta.Stages[stageNumber].From - } else { - // make sure to add ":latest" if it's implied - from = latestizeRepoTag(from) + resolved, isStage := resolveFrom(from) + meta.Froms = append(meta.Froms, resolved) + if !isStage { + meta.Parents = append(meta.Parents, Parent{From: resolved, Kind: "COPY"}) } - - meta.Froms = append(meta.Froms, from) } case "RUN": // TODO combine this and the above COPY-parsing code somehow sanely @@ -187,30 +223,23 @@ func ParseReader(dockerfile io.Reader) (Metadata, error) { continue } - if i, ok := meta.NamedStages[from]; ok { - // see note above regarding stage names in FROM - from = meta.Stages[i].From - } else if stageNumber, err := strconv.Atoi(from); err == nil && stageNumber < len(meta.Stages) { - // must be a stage number, we should resolve it too - from = meta.Stages[stageNumber].From - } else { - // make sure to add ":latest" if it's implied - from = latestizeRepoTag(from) + resolved, isStage := resolveFrom(from) + meta.Froms = append(meta.Froms, resolved) + if !isStage { + meta.Parents = append(meta.Parents, Parent{From: resolved, Kind: "RUN"}) } - - meta.Froms = append(meta.Froms, from) } } } // TODO maybe we *shouldn't* support parsing a fully empty Dockerfile? 🤔 (we actively use an "empty" Dockerfile in the tests to test edge cases of continuation though that are otherwise hard to test, so it's probably ~fine) - if len(meta.Stages) > 0 { - finalStage := meta.Stages[len(meta.Stages)-1] - switch finalStage.Platform { - case "", "$TARGETPLATFORM": - // yay, all is well + if len(stages) > 0 { + finalStage := stages[len(stages)-1] + switch finalStage.platform { + case "": + // yay, all is well (note: "$TARGETPLATFORM" is normalized to "" above, so this covers both) default: - return meta, fmt.Errorf("final stage/FROM (%q) has --platform=%q but must be unspecified or $TARGETPLATFORM", finalStage.From, finalStage.Platform) + return meta, fmt.Errorf("final stage/FROM (%q) has --platform=%q but must be unspecified or $TARGETPLATFORM", finalStage.from, finalStage.platform) } } diff --git a/pkg/dockerfile/parse_test.go b/pkg/dockerfile/parse_test.go index 6cb6b88a..03a5201f 100644 --- a/pkg/dockerfile/parse_test.go +++ b/pkg/dockerfile/parse_test.go @@ -12,23 +12,27 @@ func TestParse(t *testing.T) { name string dockerfile string metadata dockerfile.Metadata + wantErr bool }{ { dockerfile: `FROM scratch`, metadata: dockerfile.Metadata{ - Froms: []string{"scratch"}, + Froms: []string{"scratch"}, + Parents: []dockerfile.Parent{{From: "scratch", Kind: "FROM"}}, }, }, { dockerfile: `from bash`, metadata: dockerfile.Metadata{ - Froms: []string{"bash:latest"}, + Froms: []string{"bash:latest"}, + Parents: []dockerfile.Parent{{From: "bash:latest", Kind: "FROM"}}, }, }, { dockerfile: `fRoM bash:5`, metadata: dockerfile.Metadata{ - Froms: []string{"bash:5"}, + Froms: []string{"bash:5"}, + Parents: []dockerfile.Parent{{From: "bash:5", Kind: "FROM"}}, }, }, { @@ -48,6 +52,10 @@ func TestParse(t *testing.T) { `, metadata: dockerfile.Metadata{ Froms: []string{"scratch", "bash:latest"}, + Parents: []dockerfile.Parent{ + {From: "scratch", Kind: "FROM"}, + {From: "bash:latest", Kind: "FROM"}, + }, }, }, { @@ -66,14 +74,18 @@ func TestParse(t *testing.T) { COPY --from=hello-world /hello /usr/local/bin/ `, metadata: dockerfile.Metadata{ - Stages: []dockerfile.Stage{ - {From: "bash:latest", Name: "foo"}, - {From: "busybox:uclibc"}, - {From: "bash:5", Name: "bar"}, - {From: "bash:latest", FromStage: "foo", Name: "foo2"}, - {From: "scratch"}, - }, Froms: []string{"bash:latest", "busybox:uclibc", "bash:5", "bash:latest", "scratch", "bash:latest", "bash:5", "bash:latest", "hello-world:latest"}, + Parents: []dockerfile.Parent{ + // one per FROM (including "foo2", resolved to "foo"'s underlying image) + {From: "bash:latest", Kind: "FROM"}, + {From: "busybox:uclibc", Kind: "FROM"}, + {From: "bash:5", Kind: "FROM"}, + {From: "bash:latest", Kind: "FROM"}, + {From: "scratch", Kind: "FROM"}, + // COPY --from=foo/bar/foo2 all reference local stages, already represented above -- no Parent + // COPY --chown=... has no --from= at all + {From: "hello-world:latest", Kind: "COPY"}, + }, }, }, { @@ -112,6 +124,10 @@ func TestParse(t *testing.T) { `, metadata: dockerfile.Metadata{ Froms: []string{"scratch", "scratch"}, + Parents: []dockerfile.Parent{ + {From: "scratch", Kind: "FROM"}, + {From: "scratch", Kind: "FROM"}, + }, }, }, { @@ -128,13 +144,14 @@ func TestParse(t *testing.T) { RUN --mount=type=bind,from=2 cat /foo `, metadata: dockerfile.Metadata{ - Stages: []dockerfile.Stage{ - {From: "bash:latest"}, - {From: "scratch"}, - {From: "scratch"}, - {From: "bash:latest"}, - }, Froms: []string{"bash:latest", "scratch", "bash:latest", "scratch", "scratch", "bash:latest", "scratch"}, + Parents: []dockerfile.Parent{ + // COPY/RUN --from= all reference local stages, already represented here -- no separate Parent + {From: "bash:latest", Kind: "FROM"}, + {From: "scratch", Kind: "FROM"}, + {From: "scratch", Kind: "FROM"}, + {From: "bash:latest", Kind: "FROM"}, + }, }, }, { @@ -144,8 +161,11 @@ func TestParse(t *testing.T) { RUN --mount=type=bind,from=busybox:uclibc,target=/tmp ["/tmp/bin/sh","-euxc","echo foo > /foo"] `, metadata: dockerfile.Metadata{ - Stages: []dockerfile.Stage{{From: "scratch"}}, - Froms: []string{"scratch", "busybox:uclibc"}, + Froms: []string{"scratch", "busybox:uclibc"}, + Parents: []dockerfile.Parent{ + {From: "scratch", Kind: "FROM"}, + {From: "busybox:uclibc", Kind: "RUN"}, + }, }, }, { @@ -158,11 +178,12 @@ func TestParse(t *testing.T) { RUN --mount=type=bind,from=bb,target=/tmp ["/tmp/bin/sh","-euxc","echo foo > /foo"] `, metadata: dockerfile.Metadata{ - Stages: []dockerfile.Stage{ - {From: "busybox:uclibc", Name: "bb"}, - {From: "scratch"}, - }, Froms: []string{"busybox:uclibc", "scratch", "busybox:uclibc"}, + Parents: []dockerfile.Parent{ + // RUN --mount=...,from=bb references a local stage, already represented here -- no separate Parent + {From: "busybox:uclibc", Kind: "FROM"}, + {From: "scratch", Kind: "FROM"}, + }, }, }, { @@ -174,35 +195,37 @@ func TestParse(t *testing.T) { COPY --from=build /some/binary /some/other/place `, metadata: dockerfile.Metadata{ - Stages: []dockerfile.Stage{ - {From: "golang:latest", Name: "build", Platform: "$BUILDPLATFORM"}, - {From: "debian:latest", Platform: "$TARGETPLATFORM"}, - }, Froms: []string{"golang:latest", "debian:latest", "golang:latest"}, + Parents: []dockerfile.Parent{ + {From: "golang:latest", Platform: "$BUILDPLATFORM", Kind: "FROM"}, + // explicit "$TARGETPLATFORM" is normalized to "" -- semantically identical, no reason to make callers treat two values as equivalent + {From: "debian:latest", Kind: "FROM"}, + // COPY --from=build references a local stage, already represented above -- no separate Parent + }, }, }, + { + name: "FROM --platform mismatch with referenced stage", + dockerfile: ` + FROM --platform=$BUILDPLATFORM golang AS build + FROM --platform=$TARGETPLATFORM build + `, + wantErr: true, + }, } { td := td // some light normalization if td.name == "" { td.name = td.dockerfile } - if len(td.metadata.Froms) > 0 && len(td.metadata.Stages) == 0 { - td.metadata.Stages = make([]dockerfile.Stage, len(td.metadata.Froms)) - for i, from := range td.metadata.Froms { - td.metadata.Stages[i].From = from - } - } - if td.metadata.NamedStages == nil { - td.metadata.NamedStages = map[string]int{} - for i, stage := range td.metadata.Stages { - if stage.Name != "" { - td.metadata.NamedStages[stage.Name] = i - } - } - } t.Run(td.name, func(t *testing.T) { parsed, err := dockerfile.Parse(td.dockerfile) + if td.wantErr { + if err == nil { + t.Fatalf("expected an error, got:\n%#v", parsed) + } + return + } if err != nil { t.Fatal(err) }