From 1f1b48ba31aed986bad5a9abd9ae8e76034e201f Mon Sep 17 00:00:00 2001 From: Patrick Hoefler Date: Sat, 12 Jul 2025 14:54:55 +0200 Subject: [PATCH 1/3] feat: add --separate-scratch flag to create distinct nodes for each scratch image --- .github/copilot-instructions.md | 18 ++ Makefile | 2 + README.md | 1 + internal/cmd/root.go | 33 ++-- internal/cmd/root_test.go | 23 +++ internal/dockerfile2dot/build.go | 6 +- internal/dockerfile2dot/build_test.go | 59 ++++++- internal/dockerfile2dot/convert.go | 48 ++++-- internal/dockerfile2dot/convert_test.go | 214 ++++++++++++++++++------ internal/dockerfile2dot/load.go | 3 +- internal/dockerfile2dot/load_test.go | 57 ++----- internal/dockerfile2dot/structs.go | 26 +-- 12 files changed, 352 insertions(+), 138 deletions(-) create mode 100644 .github/copilot-instructions.md diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 00000000..6aa41727 --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,18 @@ +# GitHub Copilot Custom Instructions + +## Testing Guidelines + +- **No temporary test files**: Never create standalone test files like "test-*.dockerfile", "quick-test.*", etc. +- **Use existing test suites**: Add proper test cases to existing test files in the appropriate `*_test.go` files +- **Follow test patterns**: Use the established testing patterns in the codebase (table-driven tests, proper assertions) +- **Clean testing**: If testing is needed, extend the existing test suite rather than creating throwaway files +- **Focus on correct behavior**: Tests should verify expected functionality, not document bugs or regressions +- **Positive test naming**: Name tests to describe what they verify (e.g., `Test_separateScratchConnections`) rather than what's broken +- **Clear test comments**: Comments should explain what behavior is being tested, not reference historical issues + +## Code Quality + +- Follow the existing code style and patterns +- Use proper error handling +- Write comprehensive tests for new functionality +- Maintain backward compatibility unless explicitly breaking changes are needed diff --git a/Makefile b/Makefile index dfc2d40e..65e6de40 100644 --- a/Makefile +++ b/Makefile @@ -20,6 +20,8 @@ build-docker-image-ubuntu: build-linux build-linux: clean CGO_ENABLED=0 GOOS=linux go build $(FLAGS) +check: lint test + clean: go clean diff --git a/README.md b/README.md index da06fb46..373dd56e 100644 --- a/README.md +++ b/README.md @@ -154,6 +154,7 @@ Flags: -n, --nodesep float minimum space between two adjacent nodes in the same rank (default 1) -o, --output output file format, one of: canon, dot, pdf, png, raw, svg (default pdf) -r, --ranksep float minimum separation between ranks (default 0.5) + --separate-scratch create separate nodes for each scratch image instead of collapsing them -u, --unflatten uint stagger length of leaf edges between [1,u] (default 0) --version display the version of dockerfilegraph ``` diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 6352ed33..5b706379 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -15,18 +15,19 @@ import ( ) var ( - concentrateFlag bool - dpiFlag uint - edgestyleFlag enum - filenameFlag string - layersFlag bool - legendFlag bool - maxLabelLengthFlag uint - nodesepFlag float64 - outputFlag enum - ranksepFlag float64 - unflattenFlag uint - versionFlag bool + concentrateFlag bool + dpiFlag uint + edgestyleFlag enum + filenameFlag string + layersFlag bool + legendFlag bool + maxLabelLengthFlag uint + nodesepFlag float64 + outputFlag enum + ranksepFlag float64 + separateScratchFlag bool + unflattenFlag uint + versionFlag bool ) // dfgWriter is a writer that prints to stdout. When testing, we @@ -67,6 +68,7 @@ It creates a visual graph representation of the build process.`, inputFS, filenameFlag, int(maxLabelLengthFlag), + separateScratchFlag, ) if err != nil { return @@ -227,6 +229,13 @@ It creates a visual graph representation of the build process.`, "minimum separation between ranks", ) + rootCmd.Flags().BoolVar( + &separateScratchFlag, + "separate-scratch", + false, + "create separate nodes for each scratch image instead of collapsing them", + ) + rootCmd.Flags().UintVarP( &unflattenFlag, "unflatten", diff --git a/internal/cmd/root_test.go b/internal/cmd/root_test.go index a09c8a22..b14b581c 100644 --- a/internal/cmd/root_test.go +++ b/internal/cmd/root_test.go @@ -38,6 +38,7 @@ Flags: -n, --nodesep float minimum space between two adjacent nodes in the same rank (default 1) -o, --output output file format, one of: canon, dot, pdf, png, raw, svg (default pdf) -r, --ranksep float minimum separation between ranks (default 0.5) + --separate-scratch create separate nodes for each scratch image instead of collapsing them -u, --unflatten uint stagger length of leaf edges between [1,u] (default 0) --version display the version of dockerfilegraph ` @@ -488,6 +489,28 @@ It creates a visual graph representation of the build process. width=2]; external_image_3 -> stage_2; } +`, + }, + { + name: "separate scratch flag", + cliArgs: []string{"--separate-scratch", "-o", "raw"}, + dockerfileContent: "FROM scratch AS app1\nCOPY app1.txt /app1.txt\n\n" + + "FROM scratch AS app2\nCOPY app2.txt /app2.txt\n", + wantOut: "Successfully created Dockerfile.raw\n", + wantOutFile: "Dockerfile.raw", + wantOutFileContent: `digraph G { + compound=true; + nodesep=1.00; + rankdir=LR; + ranksep=0.50; + external_image_0->stage_0; + external_image_1->stage_1; + external_image_0 [ color=grey20, fontcolor=grey20, label="scratch", shape=box, style="dashed,rounded", width=2 ]; + external_image_1 [ color=grey20, fontcolor=grey20, label="scratch", shape=box, style="dashed,rounded", width=2 ]; + stage_0 [ label="app1", shape=box, style=rounded, width=2 ]; + stage_1 [ fillcolor=grey90, label="app2", shape=box, style="filled,rounded", width=2 ]; + +} `, }, } diff --git a/internal/dockerfile2dot/build.go b/internal/dockerfile2dot/build.go index 97fd18de..55ceae8f 100644 --- a/internal/dockerfile2dot/build.go +++ b/internal/dockerfile2dot/build.go @@ -169,7 +169,7 @@ func addEdgesForStage( } sourceNodeID, additionalEdgeAttrs := getWaitForNodeID( - simplifiedDockerfile, waitFor.Name, layers, + simplifiedDockerfile, waitFor.ID, layers, ) for k, v := range additionalEdgeAttrs { edgeAttrs[k] = v @@ -287,9 +287,9 @@ func getWaitForNodeID( } } - // Check if it's an external image name + // Check if it's an external image ID for externalImageIndex, externalImage := range simplifiedDockerfile.ExternalImages { - if nameOrID == externalImage.Name { + if nameOrID == externalImage.ID { nodeID = fmt.Sprintf("external_image_%d", externalImageIndex) return } diff --git a/internal/dockerfile2dot/build_test.go b/internal/dockerfile2dot/build_test.go index 60dec0da..31ad3db5 100644 --- a/internal/dockerfile2dot/build_test.go +++ b/internal/dockerfile2dot/build_test.go @@ -31,8 +31,8 @@ func TestBuildDotFile(t *testing.T) { }, }, ExternalImages: []ExternalImage{ - {Name: "build"}, - {Name: "release"}, + {ID: "build", Name: "build"}, + {ID: "release", Name: "release"}, }, Stages: []Stage{ { @@ -40,7 +40,7 @@ func TestBuildDotFile(t *testing.T) { { Label: "FROM...", WaitFors: []WaitFor{{ - Name: "build", + ID: "build", Type: waitForType(waitForFrom), }}, }, @@ -66,8 +66,8 @@ func TestBuildDotFile(t *testing.T) { }, }, ExternalImages: []ExternalImage{ - {Name: "build"}, - {Name: "release"}, + {ID: "build", Name: "build"}, + {ID: "release", Name: "release"}, }, Stages: []Stage{ { @@ -75,7 +75,7 @@ func TestBuildDotFile(t *testing.T) { { Label: "FROM...", WaitFors: []WaitFor{{ - Name: "build", + ID: "build", Type: waitForType(waitForFrom), }}, }, @@ -91,6 +91,53 @@ func TestBuildDotFile(t *testing.T) { }, wantContains: "release", }, + { + name: "separate scratch images show correct labels", + args: args{ + simplifiedDockerfile: SimplifiedDockerfile{ + ExternalImages: []ExternalImage{ + {ID: "scratch-0", Name: "scratch"}, + {ID: "scratch-1", Name: "scratch"}, + }, + Stages: []Stage{ + { + Name: "app1", + Layers: []Layer{ + { + Label: "FROM scratch AS app1", + WaitFors: []WaitFor{{ + ID: "scratch-0", + Type: waitForType(waitForFrom), + }}, + }, + }, + }, + { + Name: "app2", + Layers: []Layer{ + { + Label: "FROM scratch AS app2", + WaitFors: []WaitFor{{ + ID: "scratch-1", + Type: waitForType(waitForFrom), + }}, + }, + }, + }, + }, + }, + concentrate: false, + edgestyle: "default", + layers: false, + legend: false, + maxLabelLength: 20, + nodesep: "0.5", + ranksep: "0.5", + }, + wantContains: `external_image_0 [ color=grey20, fontcolor=grey20, label="scratch", shape=box, ` + + `style="dashed,rounded", width=2 ]; + external_image_1 [ color=grey20, fontcolor=grey20, label="scratch"`, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/internal/dockerfile2dot/convert.go b/internal/dockerfile2dot/convert.go index b5c640a1..cd6167c7 100644 --- a/internal/dockerfile2dot/convert.go +++ b/internal/dockerfile2dot/convert.go @@ -2,6 +2,7 @@ package dockerfile2dot import ( "bytes" + "fmt" "regexp" "strings" @@ -56,6 +57,7 @@ func newLayer( func dockerfileToSimplifiedDockerfile( content []byte, maxLabelLength int, + separateScratch bool, ) (simplifiedDockerfile SimplifiedDockerfile, err error) { result, err := parser.Parse(bytes.NewReader(content)) if err != nil { @@ -91,7 +93,7 @@ func dockerfileToSimplifiedDockerfile( // Set the waitFor ID layer.WaitFors = []WaitFor{{ - Name: replaceArgVars(node.Next.Value, argReplacements), + ID: replaceArgVars(node.Next.Value, argReplacements), Type: waitForType(waitForFrom), }} @@ -110,7 +112,7 @@ func dockerfileToSimplifiedDockerfile( result := fromFlagRegex.FindSubmatch([]byte(flag)) if len(result) > 1 { layer.WaitFors = []WaitFor{{ - Name: string(result[1]), + ID: string(result[1]), Type: waitForType(waitForCopy), }} } @@ -132,7 +134,7 @@ func dockerfileToSimplifiedDockerfile( for _, match := range matches { if len(match) > 1 { layer.WaitFors = append(layer.WaitFors, WaitFor{ - Name: string(match[1]), + ID: string(match[1]), Type: waitForType(waitForMount), }) } @@ -174,27 +176,46 @@ func dockerfileToSimplifiedDockerfile( } } - addExternalImages(&simplifiedDockerfile, stages) + addExternalImages(&simplifiedDockerfile, stages, separateScratch) return } +// addExternalImages processes all layers and identifies external images. func addExternalImages( - simplifiedDockerfile *SimplifiedDockerfile, stages map[string]struct{}, + simplifiedDockerfile *SimplifiedDockerfile, + stages map[string]struct{}, + separateScratch bool, ) { + // Counter to generate unique IDs for separate scratch instances + scratchCounter := 0 + + // Iterate through all stages and layers to find external image dependencies for _, stage := range simplifiedDockerfile.Stages { for _, layer := range stage.Layers { - for _, waitFor := range layer.WaitFors { + for waitForIndex, waitFor := range layer.WaitFors { - // Check if the layer waits for a stage - if _, ok := stages[waitFor.Name]; ok { + // Skip if this references an internal stage (not an external image) + if _, ok := stages[waitFor.ID]; ok { continue } - // Check if we already added the external image + // Start with the original image reference as both ID and name + imageID := waitFor.ID + originalName := waitFor.ID + + // Handle scratch image separation: generate unique IDs while preserving display name + if originalName == "scratch" && separateScratch { + imageID = fmt.Sprintf("scratch-%d", scratchCounter) + scratchCounter++ + // Update the layer's waitFor reference to use the unique ID for graph connections + layer.WaitFors[waitForIndex].ID = imageID + } + + // Avoid duplicate external image entries (based on unique ID) externalImageAlreadyAdded := false for _, externalImage := range simplifiedDockerfile.ExternalImages { - if externalImage.Name == waitFor.Name { + if externalImage.ID == imageID { externalImageAlreadyAdded = true break } @@ -203,10 +224,13 @@ func addExternalImages( continue } - // Add the external image + // Add the external image with proper ID/Name separation simplifiedDockerfile.ExternalImages = append( simplifiedDockerfile.ExternalImages, - ExternalImage{Name: waitFor.Name}, + ExternalImage{ + ID: imageID, // Unique identifier for graph connections + Name: originalName, // Display name for graph labels + }, ) } } diff --git a/internal/dockerfile2dot/convert_test.go b/internal/dockerfile2dot/convert_test.go index d5dc1372..117b20dc 100644 --- a/internal/dockerfile2dot/convert_test.go +++ b/internal/dockerfile2dot/convert_test.go @@ -9,8 +9,9 @@ import ( func Test_dockerfileToSimplifiedDockerfile(t *testing.T) { type args struct { - content []byte - maxLabelLength int + content []byte + maxLabelLength int + separateScratch bool } tests := []struct { name string @@ -20,19 +21,20 @@ func Test_dockerfileToSimplifiedDockerfile(t *testing.T) { { name: "Most minimal Dockerfile", args: args{ - content: []byte("FROM scratch"), - maxLabelLength: 20, + content: []byte("FROM scratch"), + maxLabelLength: 20, + separateScratch: false, }, want: SimplifiedDockerfile{ ExternalImages: []ExternalImage{ - {Name: "scratch"}, + {ID: "scratch", Name: "scratch"}, }, Stages: []Stage{ { Layers: []Layer{ { Label: "FROM scratch", - WaitFors: []WaitFor{{Name: "scratch", Type: waitForType(waitForFrom)}}, + WaitFors: []WaitFor{{ID: "scratch", Type: waitForType(waitForFrom)}}, }, }, }, @@ -49,13 +51,14 @@ FROM scratch COPY --from=base . . RUN --mount=type=cache,from=buildcache,source=/go/pkg/mod/cache/,target=/go/pkg/mod/cache/ go build `), - maxLabelLength: 20, + maxLabelLength: 20, + separateScratch: false, }, want: SimplifiedDockerfile{ ExternalImages: []ExternalImage{ - {Name: "ubuntu"}, - {Name: "scratch"}, - {Name: "buildcache"}, + {ID: "ubuntu", Name: "ubuntu"}, + {ID: "scratch", Name: "scratch"}, + {ID: "buildcache", Name: "buildcache"}, }, Stages: []Stage{ { @@ -63,7 +66,7 @@ RUN --mount=type=cache,from=buildcache,source=/go/pkg/mod/cache/,target=/go/pkg/ Layers: []Layer{ { Label: "FROM ubuntu as base", - WaitFors: []WaitFor{{Name: "ubuntu", Type: waitForType(waitForFrom)}}, + WaitFors: []WaitFor{{ID: "ubuntu", Type: waitForType(waitForFrom)}}, }, }, }, @@ -71,15 +74,15 @@ RUN --mount=type=cache,from=buildcache,source=/go/pkg/mod/cache/,target=/go/pkg/ Layers: []Layer{ { Label: "FROM scratch", - WaitFors: []WaitFor{{Name: "scratch", Type: waitForType(waitForFrom)}}, + WaitFors: []WaitFor{{ID: "scratch", Type: waitForType(waitForFrom)}}, }, { Label: "COPY --from=base . .", - WaitFors: []WaitFor{{Name: "base", Type: waitForType(waitForCopy)}}, + WaitFors: []WaitFor{{ID: "base", Type: waitForType(waitForCopy)}}, }, { Label: "RUN --mount=type=...", - WaitFors: []WaitFor{{Name: "buildcache", Type: waitForType(waitForMount)}}, + WaitFors: []WaitFor{{ID: "buildcache", Type: waitForType(waitForMount)}}, }, }, }, @@ -96,13 +99,14 @@ 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, + maxLabelLength: 20, + separateScratch: false, }, want: SimplifiedDockerfile{ ExternalImages: []ExternalImage{ - {Name: "ubuntu"}, - {Name: "buildcache"}, - {Name: "artifacts"}, + {ID: "ubuntu", Name: "ubuntu"}, + {ID: "buildcache", Name: "buildcache"}, + {ID: "artifacts", Name: "artifacts"}, }, Stages: []Stage{ { @@ -110,13 +114,13 @@ RUN \ Layers: []Layer{ { Label: "FROM ubuntu as base", - WaitFors: []WaitFor{{Name: "ubuntu", Type: waitForType(waitForFrom)}}, + WaitFors: []WaitFor{{ID: "ubuntu", Type: waitForType(waitForFrom)}}, }, { Label: "RUN --mount=type=...", WaitFors: []WaitFor{ - {Name: "buildcache", Type: waitForType(waitForMount)}, - {Name: "artifacts", Type: waitForType(waitForMount)}, + {ID: "buildcache", Type: waitForType(waitForMount)}, + {ID: "artifacts", Type: waitForType(waitForMount)}, }, }, }, @@ -136,19 +140,19 @@ RUN --mount=from=build,source=/build/,target=/build/ go build }, want: SimplifiedDockerfile{ ExternalImages: []ExternalImage{ - {Name: "scratch"}, - {Name: "build"}, + {ID: "scratch", Name: "scratch"}, + {ID: "build", Name: "build"}, }, Stages: []Stage{ { Layers: []Layer{ { Label: "FROM scratch", - WaitFors: []WaitFor{{Name: "scratch", Type: waitForType(waitForFrom)}}, + WaitFors: []WaitFor{{ID: "scratch", Type: waitForType(waitForFrom)}}, }, { Label: "RUN --mount=from=...", - WaitFors: []WaitFor{{Name: "build", Type: waitForType(waitForMount)}}, + WaitFors: []WaitFor{{ID: "build", Type: waitForType(waitForMount)}}, }, }, }, @@ -177,10 +181,10 @@ RUN --mount=type=cache,source=/go/pkg/mod/cache/,target=/go/pkg/mod/cache/,from= }, want: SimplifiedDockerfile{ ExternalImages: []ExternalImage{ - {Name: "ubuntu:22.04"}, - {Name: "php:8.0-fpm-alpine3.15"}, - {Name: "scratch"}, - {Name: "buildcache"}, + {ID: "ubuntu:22.04", Name: "ubuntu:22.04"}, + {ID: "php:8.0-fpm-alpine3.15", Name: "php:8.0-fpm-alpine3.15"}, + {ID: "scratch", Name: "scratch"}, + {ID: "buildcache", Name: "buildcache"}, }, Stages: []Stage{ { @@ -188,7 +192,7 @@ RUN --mount=type=cache,source=/go/pkg/mod/cache/,target=/go/pkg/mod/cache/,from= Layers: []Layer{ { Label: "FROM ubuntu:22.04...", - WaitFors: []WaitFor{{Name: "ubuntu:22.04", Type: waitForType(waitForFrom)}}, + WaitFors: []WaitFor{{ID: "ubuntu:22.04", Type: waitForType(waitForFrom)}}, }, { Label: "USER app", @@ -201,7 +205,7 @@ RUN --mount=type=cache,source=/go/pkg/mod/cache/,target=/go/pkg/mod/cache/,from= { Label: "FROM php:8.0-fpm-...", WaitFors: []WaitFor{{ - Name: "php:8.0-fpm-alpine3.15", + ID: "php:8.0-fpm-alpine3.15", Type: waitForType(waitForFrom), }}, }, @@ -211,15 +215,15 @@ RUN --mount=type=cache,source=/go/pkg/mod/cache/,target=/go/pkg/mod/cache/,from= Layers: []Layer{ { Label: "FROM scratch", - WaitFors: []WaitFor{{Name: "scratch", Type: waitForType(waitForFrom)}}, + WaitFors: []WaitFor{{ID: "scratch", Type: waitForType(waitForFrom)}}, }, { Label: "COPY --from=base . .", - WaitFors: []WaitFor{{Name: "base", Type: waitForType(waitForCopy)}}, + WaitFors: []WaitFor{{ID: "base", Type: waitForType(waitForCopy)}}, }, { Label: "RUN --mount=type=...", - WaitFors: []WaitFor{{Name: "buildcache", Type: waitForType(waitForMount)}}, + WaitFors: []WaitFor{{ID: "buildcache", Type: waitForType(waitForMount)}}, }, }, }, @@ -251,8 +255,8 @@ COPY --from=download-get-pip get-pip.py ./ }, want: SimplifiedDockerfile{ ExternalImages: []ExternalImage{ - {Name: "scratch"}, - {Name: "alpine"}, + {ID: "scratch", Name: "scratch"}, + {ID: "alpine", Name: "alpine"}, }, Stages: []Stage{ { @@ -261,7 +265,7 @@ COPY --from=download-get-pip get-pip.py ./ { Label: "FROM scratch AS d...", WaitFors: []WaitFor{{ - Name: "scratch", + ID: "scratch", Type: waitForType(waitForFrom), }}, }, @@ -274,7 +278,7 @@ COPY --from=download-get-pip get-pip.py ./ { Label: "FROM scratch AS d...", WaitFors: []WaitFor{{ - Name: "scratch", + ID: "scratch", Type: waitForType(waitForFrom), }}, }, @@ -287,21 +291,21 @@ COPY --from=download-get-pip get-pip.py ./ { Label: "FROM alpine AS final", WaitFors: []WaitFor{{ - Name: "alpine", + ID: "alpine", Type: waitForType(waitForFrom), }}, }, { Label: "COPY --from=downl...", WaitFors: []WaitFor{{ - Name: "download-node-setup", + ID: "download-node-setup", Type: waitForType(waitForCopy), }}, }, { Label: "COPY --from=downl...", WaitFors: []WaitFor{{ - Name: "download-get-pip", + ID: "download-get-pip", Type: waitForType(waitForCopy), }}, }, @@ -328,8 +332,8 @@ RUN echo "Stage 2" }, want: SimplifiedDockerfile{ ExternalImages: []ExternalImage{ - {Name: "hello-world-1:latest"}, - {Name: "hello-world-2:latest"}, + {ID: "hello-world-1:latest", Name: "hello-world-1:latest"}, + {ID: "hello-world-2:latest", Name: "hello-world-2:latest"}, }, Stages: []Stage{ { @@ -338,7 +342,7 @@ RUN echo "Stage 2" { Label: "FROM hello-world-...", WaitFors: []WaitFor{{ - Name: "hello-world-1:latest", + ID: "hello-world-1:latest", Type: waitForType(waitForFrom), }}, }, @@ -351,7 +355,7 @@ RUN echo "Stage 2" { Label: "FROM hello-world-...", WaitFors: []WaitFor{{ - Name: "hello-world-2:latest", + ID: "hello-world-2:latest", Type: waitForType(waitForFrom), }}, }, @@ -382,20 +386,20 @@ FROM $IMAGE2 }, want: SimplifiedDockerfile{ ExternalImages: []ExternalImage{ - {Name: ""}, - {Name: "scratch"}, + {ID: "", Name: ""}, + {ID: "scratch", Name: "scratch"}, }, Stages: []Stage{ { Layers: []Layer{{ Label: "FROM", - WaitFors: []WaitFor{{Name: "", Type: waitForType(waitForFrom)}}, + WaitFors: []WaitFor{{ID: "", Type: waitForType(waitForFrom)}}, }}, }, { Layers: []Layer{{ Label: "FROM scratch", - WaitFors: []WaitFor{{Name: "scratch", Type: waitForType(waitForFrom)}}, + WaitFors: []WaitFor{{ID: "scratch", Type: waitForType(waitForFrom)}}, }}, }, }, @@ -405,12 +409,124 @@ FROM $IMAGE2 }, }, }, + { + name: "Separate scratch images", + args: args{ + content: []byte(` +FROM scratch AS app1 +COPY app1.txt /app1.txt + +FROM scratch AS app2 +COPY app2.txt /app2.txt +`), + maxLabelLength: 20, + separateScratch: true, + }, + want: SimplifiedDockerfile{ + ExternalImages: []ExternalImage{ + {ID: "scratch-0", Name: "scratch"}, + {ID: "scratch-1", Name: "scratch"}, + }, + Stages: []Stage{ + { + Name: "app1", + Layers: []Layer{ + { + Label: "FROM scratch AS app1", + WaitFors: []WaitFor{{ID: "scratch-0", Type: waitForType(waitForFrom)}}, + }, + {Label: "COPY app1.txt /ap..."}, + }, + }, + { + Name: "app2", + Layers: []Layer{ + { + Label: "FROM scratch AS app2", + WaitFors: []WaitFor{{ID: "scratch-1", Type: waitForType(waitForFrom)}}, + }, + {Label: "COPY app2.txt /ap..."}, + }, + }, + }, + }, + }, + { + name: "Single scratch image with separate flag", + args: args{ + content: []byte(`FROM scratch AS app +COPY app.txt /app.txt`), + maxLabelLength: 20, + separateScratch: true, + }, + want: SimplifiedDockerfile{ + ExternalImages: []ExternalImage{ + {ID: "scratch-0", Name: "scratch"}, + }, + Stages: []Stage{ + { + Name: "app", + Layers: []Layer{ + { + Label: "FROM scratch AS app", + WaitFors: []WaitFor{{ID: "scratch-0", Type: waitForType(waitForFrom)}}, + }, + {Label: "COPY app.txt /app..."}, + }, + }, + }, + }, + }, + { + name: "No scratch images with separate flag enabled", + args: args{ + content: []byte(`FROM ubuntu AS base +COPY app.txt /app.txt + +FROM alpine AS final +COPY --from=base /app.txt /final.txt`), + maxLabelLength: 20, + separateScratch: true, + }, + want: SimplifiedDockerfile{ + ExternalImages: []ExternalImage{ + {ID: "ubuntu", Name: "ubuntu"}, + {ID: "alpine", Name: "alpine"}, + }, + Stages: []Stage{ + { + Name: "base", + Layers: []Layer{ + { + Label: "FROM ubuntu AS base", + WaitFors: []WaitFor{{ID: "ubuntu", Type: waitForType(waitForFrom)}}, + }, + {Label: "COPY app.txt /app..."}, + }, + }, + { + Name: "final", + Layers: []Layer{ + { + Label: "FROM alpine AS final", + WaitFors: []WaitFor{{ID: "alpine", Type: waitForType(waitForFrom)}}, + }, + { + Label: "COPY --from=base ...", + WaitFors: []WaitFor{{ID: "base", Type: waitForType(waitForCopy)}}, + }, + }, + }, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got, err := dockerfileToSimplifiedDockerfile( tt.args.content, tt.args.maxLabelLength, + tt.args.separateScratch, ) if tt.name == "Wait for multiple mounts" { fmt.Printf("%q", got.Stages[0].Layers[1]) diff --git a/internal/dockerfile2dot/load.go b/internal/dockerfile2dot/load.go index e9b68c7d..215c1061 100644 --- a/internal/dockerfile2dot/load.go +++ b/internal/dockerfile2dot/load.go @@ -16,6 +16,7 @@ func LoadAndParseDockerfile( inputFS afero.Fs, filename string, maxLabelLength int, + separateScratch bool, ) (SimplifiedDockerfile, error) { content, err := afero.ReadFile(inputFS, filename) if err != nil { @@ -28,5 +29,5 @@ func LoadAndParseDockerfile( } panic(err) } - return dockerfileToSimplifiedDockerfile(content, maxLabelLength) + return dockerfileToSimplifiedDockerfile(content, maxLabelLength, separateScratch) } diff --git a/internal/dockerfile2dot/load_test.go b/internal/dockerfile2dot/load_test.go index b1996c3a..ec6d1183 100644 --- a/internal/dockerfile2dot/load_test.go +++ b/internal/dockerfile2dot/load_test.go @@ -3,15 +3,16 @@ package dockerfile2dot import ( "testing" - "github.com/google/go-cmp/cmp" "github.com/spf13/afero" ) +// TestLoadAndParseDockerfile tests the file loading functionality of LoadAndParseDockerfile. +// This test focuses on file I/O concerns (file not found, path resolution) rather than +// Dockerfile parsing logic, which is tested in convert_test.go. func TestLoadAndParseDockerfile(t *testing.T) { type args struct { - inputFS afero.Fs - filename string - maxLabelLength int + inputFS afero.Fs + filename string } dockerfileFS := afero.NewMemMapFs() @@ -20,7 +21,6 @@ func TestLoadAndParseDockerfile(t *testing.T) { tests := []struct { name string args args - want SimplifiedDockerfile wantErr bool }{ { @@ -34,60 +34,31 @@ func TestLoadAndParseDockerfile(t *testing.T) { { name: "should work in the current working directory", args: args{ - inputFS: dockerfileFS, - filename: "Dockerfile", - maxLabelLength: 20, - }, - want: SimplifiedDockerfile{ - ExternalImages: []ExternalImage{{Name: "scratch"}}, - Stages: []Stage{ - { - Layers: []Layer{ - { - Label: "FROM scratch", - WaitFors: []WaitFor{{Name: "scratch", Type: waitForType(waitForFrom)}}, - }, - }, - }, - }, + inputFS: dockerfileFS, + filename: "Dockerfile", }, + wantErr: false, }, { name: "should work in any directory", args: args{ - inputFS: dockerfileFS, - filename: "subdir/../Dockerfile", - maxLabelLength: 20, - }, - want: SimplifiedDockerfile{ - ExternalImages: []ExternalImage{{Name: "scratch"}}, - Stages: []Stage{ - { - Layers: []Layer{ - { - Label: "FROM scratch", - WaitFors: []WaitFor{{Name: "scratch", Type: waitForType(waitForFrom)}}, - }, - }, - }, - }, + inputFS: dockerfileFS, + filename: "subdir/../Dockerfile", }, + wantErr: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := LoadAndParseDockerfile( + _, err := LoadAndParseDockerfile( tt.args.inputFS, tt.args.filename, - tt.args.maxLabelLength, + 20, // Default maxLabelLength + false, // Default separateScratch ) if (err != nil) != tt.wantErr { t.Errorf("LoadAndParseDockerfile() error = %v, wantErr %v", err, tt.wantErr) - return - } - if diff := cmp.Diff(tt.want, got); diff != "" { - t.Errorf("LoadAndParseDockerfile() mismatch (-want +got):\n%s", diff) } }) } diff --git a/internal/dockerfile2dot/structs.go b/internal/dockerfile2dot/structs.go index 7335bcf7..1df76415 100644 --- a/internal/dockerfile2dot/structs.go +++ b/internal/dockerfile2dot/structs.go @@ -15,33 +15,35 @@ type SimplifiedDockerfile struct { // Stage represents a single build stage within the multi-stage Dockerfile or // an external image. type Stage struct { - Name string // the part after the AS in the FROM line - Layers []Layer // the layers of the stage + Name string // The part after the AS in the FROM line + Layers []Layer // The layers of the stage } -// Layer stores the changes compared to the image it’s based on within a +// 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 - WaitFors []WaitFor // stages or external images 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. type ExternalImage struct { - Name string + ID string // Unique identifier for this external image instance + Name string // The original name of the external image } +// waitForType represents the type of dependency between stages or images. type waitForType int const ( - waitForCopy waitForType = iota - waitForFrom - waitForMount + waitForCopy waitForType = iota // COPY dependency from another stage + waitForFrom // FROM dependency on another stage or image + waitForMount // MOUNT dependency for build cache ) // WaitFor holds the name of the stage or external image for which the builder -// has to wait, and the type, i.e. the reason why it has to wait for it +// has to wait, and the type, i.e. the reason why it has to wait for it. type WaitFor struct { - Name string // the name of the stage or external image for which the builder has to wait - Type waitForType // the reason why it has to wait + ID string // The unique identifier of the stage or external image for which the builder has to wait + Type waitForType // The reason why it has to wait } From 27ee95986282b8d15d51947a7111a3527bce3987 Mon Sep 17 00:00:00 2001 From: Patrick Hoefler Date: Sat, 12 Jul 2025 21:47:15 +0200 Subject: [PATCH 2/3] refactor: update image ID --- internal/dockerfile2dot/convert.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/dockerfile2dot/convert.go b/internal/dockerfile2dot/convert.go index cd6167c7..5c3b2013 100644 --- a/internal/dockerfile2dot/convert.go +++ b/internal/dockerfile2dot/convert.go @@ -191,8 +191,8 @@ func addExternalImages( scratchCounter := 0 // Iterate through all stages and layers to find external image dependencies - for _, stage := range simplifiedDockerfile.Stages { - for _, layer := range stage.Layers { + for stageIndex, stage := range simplifiedDockerfile.Stages { + for layerIndex, layer := range stage.Layers { for waitForIndex, waitFor := range layer.WaitFors { // Skip if this references an internal stage (not an external image) @@ -209,7 +209,7 @@ func addExternalImages( imageID = fmt.Sprintf("scratch-%d", scratchCounter) scratchCounter++ // Update the layer's waitFor reference to use the unique ID for graph connections - layer.WaitFors[waitForIndex].ID = imageID + simplifiedDockerfile.Stages[stageIndex].Layers[layerIndex].WaitFors[waitForIndex].ID = imageID } // Avoid duplicate external image entries (based on unique ID) From e311e4cc19e6510d6f14b75c696c38db725659f9 Mon Sep 17 00:00:00 2001 From: Patrick Hoefler Date: Sat, 12 Jul 2025 21:49:30 +0200 Subject: [PATCH 3/3] refactor: remove debug print statement from convert_test.go --- internal/dockerfile2dot/convert_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/internal/dockerfile2dot/convert_test.go b/internal/dockerfile2dot/convert_test.go index 117b20dc..0beb7ea0 100644 --- a/internal/dockerfile2dot/convert_test.go +++ b/internal/dockerfile2dot/convert_test.go @@ -1,7 +1,6 @@ package dockerfile2dot import ( - "fmt" "testing" "github.com/google/go-cmp/cmp" @@ -528,9 +527,6 @@ COPY --from=base /app.txt /final.txt`), tt.args.maxLabelLength, tt.args.separateScratch, ) - 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