From 2afb05b5838ad59ebd082c8d8f9930abb4f3ec6b Mon Sep 17 00:00:00 2001 From: andremarianiello Date: Tue, 14 Nov 2023 13:12:13 -0500 Subject: [PATCH] Support multiple mounts on RUN layers (#423) --- internal/dockerfile2dot/build.go | 48 +++++------ internal/dockerfile2dot/build_test.go | 8 +- internal/dockerfile2dot/convert.go | 64 +++++++------- internal/dockerfile2dot/convert_test.go | 110 +++++++++++++++++------- internal/dockerfile2dot/load_test.go | 12 +-- internal/dockerfile2dot/structs.go | 4 +- 6 files changed, 145 insertions(+), 101 deletions(-) diff --git a/internal/dockerfile2dot/build.go b/internal/dockerfile2dot/build.go index 009832d..97fd18d 100644 --- a/internal/dockerfile2dot/build.go +++ b/internal/dockerfile2dot/build.go @@ -154,36 +154,34 @@ func addEdgesForStage( simplifiedDockerfile SimplifiedDockerfile, layers bool, edgestyle string, ) { for layerIndex, layer := range stage.Layers { - if layer.WaitFor.Name == "" { - continue - } - - edgeAttrs := map[string]string{} - if layer.WaitFor.Type == waitForType(waitForCopy) { - edgeAttrs["arrowhead"] = "empty" - if edgestyle == "default" { - edgeAttrs["style"] = "dashed" + for _, waitFor := range layer.WaitFors { + edgeAttrs := map[string]string{} + if waitFor.Type == waitForType(waitForCopy) { + edgeAttrs["arrowhead"] = "empty" + if edgestyle == "default" { + edgeAttrs["style"] = "dashed" + } + } else if waitFor.Type == waitForType(waitForMount) { + edgeAttrs["arrowhead"] = "ediamond" + if edgestyle == "default" { + edgeAttrs["style"] = "dotted" + } } - } else if layer.WaitFor.Type == waitForType(waitForMount) { - edgeAttrs["arrowhead"] = "ediamond" - if edgestyle == "default" { - edgeAttrs["style"] = "dotted" + + sourceNodeID, additionalEdgeAttrs := getWaitForNodeID( + simplifiedDockerfile, waitFor.Name, layers, + ) + for k, v := range additionalEdgeAttrs { + edgeAttrs[k] = v } - } - sourceNodeID, additionalEdgeAttrs := getWaitForNodeID( - simplifiedDockerfile, layer.WaitFor.Name, layers, - ) - for k, v := range additionalEdgeAttrs { - edgeAttrs[k] = v - } + targetNodeID := fmt.Sprintf("stage_%d", stageIndex) + if layers { + targetNodeID = targetNodeID + fmt.Sprintf("_layer_%d", layerIndex) + } - targetNodeID := fmt.Sprintf("stage_%d", stageIndex) - if layers { - targetNodeID = targetNodeID + fmt.Sprintf("_layer_%d", layerIndex) + _ = graph.AddEdge(sourceNodeID, targetNodeID, true, edgeAttrs) } - - _ = graph.AddEdge(sourceNodeID, targetNodeID, true, edgeAttrs) } } diff --git a/internal/dockerfile2dot/build_test.go b/internal/dockerfile2dot/build_test.go index a7b5877..60dec0d 100644 --- a/internal/dockerfile2dot/build_test.go +++ b/internal/dockerfile2dot/build_test.go @@ -39,10 +39,10 @@ func TestBuildDotFile(t *testing.T) { Layers: []Layer{ { Label: "FROM...", - WaitFor: WaitFor{ + WaitFors: []WaitFor{{ Name: "build", Type: waitForType(waitForFrom), - }, + }}, }, }, }, @@ -74,10 +74,10 @@ func TestBuildDotFile(t *testing.T) { Layers: []Layer{ { Label: "FROM...", - WaitFor: WaitFor{ + WaitFors: []WaitFor{{ Name: "build", Type: waitForType(waitForFrom), - }, + }}, }, }, }, diff --git a/internal/dockerfile2dot/convert.go b/internal/dockerfile2dot/convert.go index 0c861dc..a482fa5 100644 --- a/internal/dockerfile2dot/convert.go +++ b/internal/dockerfile2dot/convert.go @@ -63,10 +63,10 @@ func dockerfileToSimplifiedDockerfile( layer := newLayer(child, argReplacements, maxLabelLength) // Set the waitFor ID - layer.WaitFor = WaitFor{ + layer.WaitFors = []WaitFor{{ Name: replaceArgVars(child.Next.Value, argReplacements), Type: waitForType(waitForFrom), - } + }} simplifiedDockerfile.Stages[stageIndex].Layers = append( simplifiedDockerfile.Stages[stageIndex].Layers, @@ -83,10 +83,10 @@ func dockerfileToSimplifiedDockerfile( regex := regexp.MustCompile("--from=(.+)") result := regex.FindSubmatch([]byte(flag)) if len(result) > 1 { - layer.WaitFor = WaitFor{ + layer.WaitFors = []WaitFor{{ Name: string(result[1]), Type: waitForType(waitForCopy), - } + }} } } @@ -103,11 +103,13 @@ func dockerfileToSimplifiedDockerfile( // If there is a "--mount=(.*)from=..." option, set the waitFor ID for _, flag := range child.Flags { regex := regexp.MustCompile("--mount=.*from=(.+?)(?:,| |$)") - result := regex.FindSubmatch([]byte(flag)) - if len(result) > 1 { - layer.WaitFor = WaitFor{ - Name: string(result[1]), - Type: waitForType(waitForMount), + matches := regex.FindAllSubmatch([]byte(flag), -1) + for _, match := range matches { + if len(match) > 1 { + layer.WaitFors = append(layer.WaitFors, WaitFor{ + Name: string(match[1]), + Type: waitForType(waitForMount), + }) } } } @@ -155,33 +157,31 @@ func addExternalImages( ) { for _, stage := range simplifiedDockerfile.Stages { for _, layer := range stage.Layers { - // Check if the layer waits for anything - if layer.WaitFor.Name == "" { - continue - } + for _, waitFor := range layer.WaitFors { - // Check if the layer waits for a stage - if _, ok := stages[layer.WaitFor.Name]; ok { - continue - } + // Check if the layer waits for a stage + if _, ok := stages[waitFor.Name]; ok { + continue + } - // Check if we already added the external image - externalImageAlreadyAdded := false - for _, externalImage := range simplifiedDockerfile.ExternalImages { - if externalImage.Name == layer.WaitFor.Name { - externalImageAlreadyAdded = true - break + // Check if we already added the external image + externalImageAlreadyAdded := false + for _, externalImage := range simplifiedDockerfile.ExternalImages { + if externalImage.Name == waitFor.Name { + externalImageAlreadyAdded = true + break + } + } + if externalImageAlreadyAdded { + continue } - } - if externalImageAlreadyAdded { - continue - } - // Add the external image - simplifiedDockerfile.ExternalImages = append( - simplifiedDockerfile.ExternalImages, - ExternalImage{Name: layer.WaitFor.Name}, - ) + // Add the external image + simplifiedDockerfile.ExternalImages = append( + simplifiedDockerfile.ExternalImages, + ExternalImage{Name: waitFor.Name}, + ) + } } } } diff --git a/internal/dockerfile2dot/convert_test.go b/internal/dockerfile2dot/convert_test.go index cae32cc..cd45b11 100644 --- a/internal/dockerfile2dot/convert_test.go +++ b/internal/dockerfile2dot/convert_test.go @@ -1,6 +1,7 @@ package dockerfile2dot import ( + "fmt" "testing" "github.com/google/go-cmp/cmp" @@ -30,8 +31,9 @@ func Test_dockerfileToSimplifiedDockerfile(t *testing.T) { { Layers: []Layer{ { - Label: "FROM scratch", - WaitFor: WaitFor{Name: "scratch", Type: waitForType(waitForFrom)}}, + Label: "FROM scratch", + WaitFors: []WaitFor{{Name: "scratch", Type: waitForType(waitForFrom)}}, + }, }, }, }, @@ -60,24 +62,62 @@ RUN --mount=type=cache,from=buildcache,source=/go/pkg/mod/cache/,target=/go/pkg/ Name: "base", Layers: []Layer{ { - Label: "FROM ubuntu as base", - WaitFor: WaitFor{Name: "ubuntu", Type: waitForType(waitForFrom)}, + Label: "FROM ubuntu as base", + WaitFors: []WaitFor{{Name: "ubuntu", Type: waitForType(waitForFrom)}}, }, }, }, { Layers: []Layer{ { - Label: "FROM scratch", - WaitFor: WaitFor{Name: "scratch", Type: waitForType(waitForFrom)}, + Label: "FROM scratch", + WaitFors: []WaitFor{{Name: "scratch", Type: waitForType(waitForFrom)}}, + }, + { + Label: "COPY --from=base . .", + WaitFors: []WaitFor{{Name: "base", Type: waitForType(waitForCopy)}}, + }, + { + Label: "RUN --mount=type=...", + WaitFors: []WaitFor{{Name: "buildcache", Type: waitForType(waitForMount)}}, }, + }, + }, + }, + }, + }, + { + name: "Wait for multiple mounts", + args: args{ + content: []byte(` +# syntax=docker/dockerfile:1 +FROM ubuntu as base +RUN \ + --mount=type=cache,from=buildcache,source=/go/pkg/mod/cache/,target=/go/pkg/mod/cache/ \ + --mount=from=artifacts,source=/artifacts/embeddata,target=/artifacts/embeddata go build +`), + maxLabelLength: 20, + }, + want: SimplifiedDockerfile{ + ExternalImages: []ExternalImage{ + {Name: "ubuntu"}, + {Name: "buildcache"}, + {Name: "artifacts"}, + }, + Stages: []Stage{ + { + Name: "base", + Layers: []Layer{ { - Label: "COPY --from=base . .", - WaitFor: WaitFor{Name: "base", Type: waitForType(waitForCopy)}, + Label: "FROM ubuntu as base", + WaitFors: []WaitFor{{Name: "ubuntu", Type: waitForType(waitForFrom)}}, }, { - Label: "RUN --mount=type=...", - WaitFor: WaitFor{Name: "buildcache", Type: waitForType(waitForMount)}, + Label: "RUN --mount=typ...", + WaitFors: []WaitFor{ + {Name: "buildcache", Type: waitForType(waitForMount)}, + {Name: "artifacts", Type: waitForType(waitForMount)}, + }, }, }, }, @@ -103,12 +143,12 @@ RUN --mount=from=build,source=/build/,target=/build/ go build { Layers: []Layer{ { - Label: "FROM scratch", - WaitFor: WaitFor{Name: "scratch", Type: waitForType(waitForFrom)}, + Label: "FROM scratch", + WaitFors: []WaitFor{{Name: "scratch", Type: waitForType(waitForFrom)}}, }, { - Label: "RUN --mount=from=...", - WaitFor: WaitFor{Name: "build", Type: waitForType(waitForMount)}, + Label: "RUN --mount=from=...", + WaitFors: []WaitFor{{Name: "build", Type: waitForType(waitForMount)}}, }, }, }, @@ -147,8 +187,8 @@ RUN --mount=type=cache,source=/go/pkg/mod/cache/,target=/go/pkg/mod/cache/,from= Name: "base", Layers: []Layer{ { - Label: "FROM ubuntu:22.04...", - WaitFor: WaitFor{Name: "ubuntu:22.04", Type: waitForType(waitForFrom)}, + Label: "FROM ubuntu:22.04...", + WaitFors: []WaitFor{{Name: "ubuntu:22.04", Type: waitForType(waitForFrom)}}, }, { Label: "USER app", @@ -160,26 +200,26 @@ RUN --mount=type=cache,source=/go/pkg/mod/cache/,target=/go/pkg/mod/cache/,from= Layers: []Layer{ { Label: "FROM php:8.0-fpm-...", - WaitFor: WaitFor{ + WaitFors: []WaitFor{{ Name: "php:8.0-fpm-alpine3.15", Type: waitForType(waitForFrom), - }, + }}, }, }, }, { Layers: []Layer{ { - Label: "FROM scratch", - WaitFor: WaitFor{Name: "scratch", Type: waitForType(waitForFrom)}, + Label: "FROM scratch", + WaitFors: []WaitFor{{Name: "scratch", Type: waitForType(waitForFrom)}}, }, { - Label: "COPY --from=base . .", - WaitFor: WaitFor{Name: "base", Type: waitForType(waitForCopy)}, + Label: "COPY --from=base . .", + WaitFors: []WaitFor{{Name: "base", Type: waitForType(waitForCopy)}}, }, { - Label: "RUN --mount=type=...", - WaitFor: WaitFor{Name: "buildcache", Type: waitForType(waitForMount)}, + Label: "RUN --mount=type=...", + WaitFors: []WaitFor{{Name: "buildcache", Type: waitForType(waitForMount)}}, }, }, }, @@ -220,10 +260,10 @@ COPY --from=download-get-pip get-pip.py ./ Layers: []Layer{ { Label: "FROM scratch AS d...", - WaitFor: WaitFor{ + WaitFors: []WaitFor{{ Name: "scratch", Type: waitForType(waitForFrom), - }, + }}, }, {Label: "ADD https://deb.n..."}, }, @@ -233,10 +273,11 @@ COPY --from=download-get-pip get-pip.py ./ Layers: []Layer{ { Label: "FROM scratch AS d...", - WaitFor: WaitFor{ + WaitFors: []WaitFor{{ Name: "scratch", Type: waitForType(waitForFrom), }}, + }, {Label: "ADD https://boots..."}, }, }, @@ -245,24 +286,24 @@ COPY --from=download-get-pip get-pip.py ./ Layers: []Layer{ { Label: "FROM alpine AS final", - WaitFor: WaitFor{ + WaitFors: []WaitFor{{ Name: "alpine", Type: waitForType(waitForFrom), - }, + }}, }, { Label: "COPY --from=downl...", - WaitFor: WaitFor{ + WaitFors: []WaitFor{{ Name: "download-node-setup", Type: waitForType(waitForCopy), - }, + }}, }, { Label: "COPY --from=downl...", - WaitFor: WaitFor{ + WaitFors: []WaitFor{{ Name: "download-get-pip", Type: waitForType(waitForCopy), - }, + }}, }, }, }, @@ -276,6 +317,9 @@ COPY --from=download-get-pip get-pip.py ./ tt.args.content, tt.args.maxLabelLength, ) + if tt.name == "Wait for multiple mounts" { + fmt.Printf("%q", got.Stages[0].Layers[1]) + } if err != nil { t.Errorf("dockerfileToSimplifiedDockerfile() error = %v", err) return diff --git a/internal/dockerfile2dot/load_test.go b/internal/dockerfile2dot/load_test.go index 65280f2..b1996c3 100644 --- a/internal/dockerfile2dot/load_test.go +++ b/internal/dockerfile2dot/load_test.go @@ -15,7 +15,7 @@ func TestLoadAndParseDockerfile(t *testing.T) { } dockerfileFS := afero.NewMemMapFs() - _ = afero.WriteFile(dockerfileFS, "Dockerfile", []byte(`FROM scratch`), 0644) + _ = afero.WriteFile(dockerfileFS, "Dockerfile", []byte(`FROM scratch`), 0o644) tests := []struct { name string @@ -44,8 +44,9 @@ func TestLoadAndParseDockerfile(t *testing.T) { { Layers: []Layer{ { - Label: "FROM scratch", - WaitFor: WaitFor{Name: "scratch", Type: waitForType(waitForFrom)}}, + Label: "FROM scratch", + WaitFors: []WaitFor{{Name: "scratch", Type: waitForType(waitForFrom)}}, + }, }, }, }, @@ -64,8 +65,9 @@ func TestLoadAndParseDockerfile(t *testing.T) { { Layers: []Layer{ { - Label: "FROM scratch", - WaitFor: WaitFor{Name: "scratch", Type: waitForType(waitForFrom)}}, + Label: "FROM scratch", + WaitFors: []WaitFor{{Name: "scratch", Type: waitForType(waitForFrom)}}, + }, }, }, }, diff --git a/internal/dockerfile2dot/structs.go b/internal/dockerfile2dot/structs.go index 3346fb0..7335bcf 100644 --- a/internal/dockerfile2dot/structs.go +++ b/internal/dockerfile2dot/structs.go @@ -22,8 +22,8 @@ type Stage struct { // Layer stores the changes compared to the image it’s based on within a // multi-stage Dockerfile. type Layer struct { - Label string // the command and truncated args - WaitFor WaitFor // stage or external image for which this layer needs to wait + Label string // the command and truncated args + WaitFors []WaitFor // stages or external images for which this layer needs to wait } // ExternalImage holds the name of an external image.