From 1dab52166b7edcf2921576e5eba273b567dc1bb6 Mon Sep 17 00:00:00 2001 From: Patrick Hoefler Date: Fri, 13 Mar 2026 13:54:42 +0100 Subject: [PATCH 1/5] feat: add --target flag for target-specific visualization Adds a --target flag that limits the graph to stages transitively needed to build the specified targets, eliding everything else. Also includes three refactorings that fit naturally with this change: - Introduce ParseOptions and BuildOptions structs to replace the growing positional parameter lists on LoadAndParseDockerfile and BuildDotFile - Move ScratchModeFromString into the dockerfile2dot package next to the ScratchMode type it converts to - Extract a shared findStageIndex helper to eliminate duplicate stage lookup logic between convert.go and build.go Co-Authored-By: Claude Sonnet 4.6 --- README.md | 2 + internal/cmd/root.go | 48 ++-- internal/cmd/root_test.go | 46 ++++ internal/dockerfile2dot/build.go | 70 ++--- internal/dockerfile2dot/build_test.go | 19 +- internal/dockerfile2dot/convert.go | 34 ++- internal/dockerfile2dot/convert_test.go | 59 +++- internal/dockerfile2dot/filter.go | 133 +++++++++ internal/dockerfile2dot/filter_test.go | 351 ++++++++++++++++++++++++ internal/dockerfile2dot/load.go | 16 +- internal/dockerfile2dot/load_test.go | 4 +- internal/dockerfile2dot/structs.go | 52 ++++ 12 files changed, 732 insertions(+), 102 deletions(-) create mode 100644 internal/dockerfile2dot/filter.go create mode 100644 internal/dockerfile2dot/filter_test.go diff --git a/README.md b/README.md index 7f06c73..3d11664 100644 --- a/README.md +++ b/README.md @@ -108,6 +108,7 @@ dockerfilegraph - `--legend` - Add a legend explaining the notation - `--layers` - Show all Docker layers - `--separate ubuntu,alpine` - Display selected external images as separate nodes per usage, reducing edge clutter +- `--target release,app` - Only show stages required to build the given target(s), eliding everything else **All Available Options:** @@ -133,6 +134,7 @@ Flags: -r, --ranksep float minimum separation between ranks (default 0.5) --scratch how to handle scratch images, one of: collapsed, hidden, separated (default collapsed) --separate strings external images to display as separate nodes per usage (e.g. --separate ubuntu,alpine) + --target strings only show stages required to build the given target(s) (e.g. --target release,app) -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 0a90821..b17389b 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -28,6 +28,7 @@ type cliFlags struct { ranksep float64 scratch enum separate []string + target []string unflatten uint version bool } @@ -67,16 +68,16 @@ It creates a visual graph representation of the build process.`, return } - // Determine scratch mode from flag - scratchMode := scratchModeFromString(f.scratch.String()) - // Load and parse the Dockerfile. dockerfile, err := dockerfile2dot.LoadAndParseDockerfile( inputFS, f.filename, - int(f.maxLabelLength), - scratchMode, - f.separate, + dockerfile2dot.ParseOptions{ + MaxLabelLength: int(f.maxLabelLength), + ScratchMode: dockerfile2dot.ScratchModeFromString(f.scratch.String()), + SeparateImages: f.separate, + Targets: f.target, + }, ) if err != nil { return @@ -90,13 +91,15 @@ It creates a visual graph representation of the build process.`, dotFileContent, err := dockerfile2dot.BuildDotFile( dockerfile, - f.concentrate, - f.edgestyle.String(), - f.layers, - f.legend, - int(f.maxLabelLength), - f.nodesep, - f.ranksep, + dockerfile2dot.BuildOptions{ + Concentrate: f.concentrate, + EdgeStyle: f.edgestyle.String(), + Layers: f.layers, + Legend: f.legend, + MaxLabelLength: int(f.maxLabelLength), + NodeSep: f.nodesep, + RankSep: f.ranksep, + }, ) if err != nil { return @@ -257,6 +260,13 @@ It creates a visual graph representation of the build process.`, "external images to display as separate nodes per usage (e.g. --separate ubuntu,alpine)", ) + rootCmd.Flags().StringSliceVar( + &f.target, + "target", + nil, + "only show stages required to build the given target(s) (e.g. --target release,app)", + ) + rootCmd.Flags().UintVarP( &f.unflatten, "unflatten", @@ -338,15 +348,3 @@ func checkFlags(maxLabelLength uint) error { } return nil } - -// scratchModeFromString converts a validated enum string to a ScratchMode constant. -func scratchModeFromString(s string) dockerfile2dot.ScratchMode { - switch s { - case "separated": - return dockerfile2dot.ScratchSeparated - case "hidden": - return dockerfile2dot.ScratchHidden - default: - return dockerfile2dot.ScratchCollapsed - } -} diff --git a/internal/cmd/root_test.go b/internal/cmd/root_test.go index ba4c88c..3d63d9e 100644 --- a/internal/cmd/root_test.go +++ b/internal/cmd/root_test.go @@ -40,6 +40,7 @@ Flags: -r, --ranksep float minimum separation between ranks (default 0.5) --scratch how to handle scratch images, one of: collapsed, hidden, separated (default collapsed) --separate strings external images to display as separate nodes per usage (e.g. --separate ubuntu,alpine) + --target strings only show stages required to build the given target(s) (e.g. --target release,app) -u, --unflatten uint stagger length of leaf edges between [1,u] (default 0) --version display the version of dockerfilegraph ` @@ -587,6 +588,51 @@ It creates a visual graph representation of the build process. } `, }, + { + name: "target flag single target", + cliArgs: []string{"--target", "ubuntu", "-o", "raw"}, + 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_0 [ color=grey20, fontcolor=grey20, label="ubuntu:latest", shape=box, style="dashed,rounded", width=2 ]; + stage_0 [ fillcolor=grey90, label="ubuntu", shape=box, style="filled,rounded", width=2 ]; + +} +`, + }, + { + name: "target flag two targets", + cliArgs: []string{"--target", "ubuntu,build-tool-dependencies", "-o", "raw"}, + 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_2->stage_1[ arrowhead=ediamond, style=dotted ]; + external_image_0 [ color=grey20, fontcolor=grey20, label="ubuntu:latest", shape=box, style="dashed,rounded", width=2 ]; + external_image_1 [ color=grey20, fontcolor=grey20, label="golang:1.19", shape=box, style="dashed,rounded", width=2 ]; + external_image_2 [ color=grey20, fontcolor=grey20, label="buildcache", shape=box, style="dashed,rounded", width=2 ]; + stage_0 [ label="ubuntu", shape=box, style=rounded, width=2 ]; + stage_1 [ fillcolor=grey90, label="build-tool-depend...", shape=box, style="filled,rounded", width=2 ]; + +} +`, + }, + { + name: "target flag invalid target", + cliArgs: []string{"--target", "nonexistent", "-o", "raw"}, + wantErr: true, + wantOutRegex: `Error: target "nonexistent" not found in Dockerfile`, + }, { name: "separate flag multiple images", cliArgs: []string{"--separate", "ubuntu:latest,alpine", "-o", "raw"}, diff --git a/internal/dockerfile2dot/build.go b/internal/dockerfile2dot/build.go index 6a0a499..c568ac6 100644 --- a/internal/dockerfile2dot/build.go +++ b/internal/dockerfile2dot/build.go @@ -12,13 +12,7 @@ import ( // BuildDotFile builds a GraphViz .dot file from a simplified Dockerfile func BuildDotFile( simplifiedDockerfile SimplifiedDockerfile, - concentrate bool, - edgestyle string, - layers bool, - legend bool, - maxLabelLength int, - nodesep float64, - ranksep float64, + opts BuildOptions, ) (string, error) { // Create a new graph graph := gographviz.NewEscape() @@ -33,10 +27,10 @@ func BuildDotFile( set(graph.SetName("G")) set(graph.SetDir(true)) set(graph.AddAttr("G", "compound", "true")) // allow edges between clusters - set(graph.AddAttr("G", "nodesep", fmt.Sprintf("%.2f", nodesep))) + set(graph.AddAttr("G", "nodesep", fmt.Sprintf("%.2f", opts.NodeSep))) set(graph.AddAttr("G", "rankdir", "LR")) - set(graph.AddAttr("G", "ranksep", fmt.Sprintf("%.2f", ranksep))) - if concentrate { + set(graph.AddAttr("G", "ranksep", fmt.Sprintf("%.2f", opts.RankSep))) + if opts.Concentrate { set(graph.AddAttr("G", "concentrate", "true")) } @@ -45,22 +39,22 @@ func BuildDotFile( } // Add the legend if requested - if legend { - if err := addLegend(graph, edgestyle); err != nil { + if opts.Legend { + if err := addLegend(graph, opts.EdgeStyle); err != nil { return "", err } } - if err := addExternalImagesToGraph(graph, simplifiedDockerfile, maxLabelLength); err != nil { + if err := addExternalImagesToGraph(graph, simplifiedDockerfile, opts.MaxLabelLength); err != nil { return "", err } - if err := addStages(graph, simplifiedDockerfile, maxLabelLength, layers, edgestyle); err != nil { + if err := addStages(graph, simplifiedDockerfile, opts.MaxLabelLength, opts.Layers, opts.EdgeStyle); err != nil { return "", err } // Add the ARGS that appear before the first stage, if layers are requested - if layers { + if opts.Layers { if err := addBeforeFirstStage(graph, simplifiedDockerfile); err != nil { return "", err } @@ -369,7 +363,7 @@ func getWaitForNodeID( ) (string, map[string]string, error) { attrs := map[string]string{} - // If it can be converted to an integer, it's a stage ID + // If it can be converted to an integer, it's a numeric stage reference if stageIndex, convertErr := strconv.Atoi(nameOrID); convertErr == nil { if stageIndex < 0 || stageIndex >= len(simplifiedDockerfile.Stages) { return "", nil, fmt.Errorf( @@ -377,34 +371,12 @@ func getWaitForNodeID( stageIndex, len(simplifiedDockerfile.Stages), ) } - if layers { - if len(simplifiedDockerfile.Stages[stageIndex].Layers) == 0 { - return "", nil, fmt.Errorf("stage %d has no layers", stageIndex) - } - // Return the last layer of the stage - return fmt.Sprintf( - "stage_%d_layer_%d", - stageIndex, len(simplifiedDockerfile.Stages[stageIndex].Layers)-1, - ), map[string]string{"ltail": fmt.Sprintf("cluster_stage_%d", stageIndex)}, nil - } - return fmt.Sprintf("stage_%d", stageIndex), attrs, nil + return stageNodeID(simplifiedDockerfile, stageIndex, nameOrID, layers) } // Check if it's a stage name - for stageIndex, stage := range simplifiedDockerfile.Stages { - if nameOrID == stage.Name { - if layers { - if len(simplifiedDockerfile.Stages[stageIndex].Layers) == 0 { - return "", nil, fmt.Errorf("stage %q has no layers", nameOrID) - } - // Return the last layer of the stage - return fmt.Sprintf( - "stage_%d_layer_%d", - stageIndex, len(simplifiedDockerfile.Stages[stageIndex].Layers)-1, - ), map[string]string{"ltail": fmt.Sprintf("cluster_stage_%d", stageIndex)}, nil - } - return fmt.Sprintf("stage_%d", stageIndex), attrs, nil - } + if stageIndex, found := findStageIndex(simplifiedDockerfile.Stages, nameOrID); found { + return stageNodeID(simplifiedDockerfile, stageIndex, nameOrID, layers) } // Check if it's an external image ID @@ -419,3 +391,19 @@ func getWaitForNodeID( nameOrID, ) } + +// stageNodeID returns the graph node ID for a stage, handling the layers case. +func stageNodeID( + sdf SimplifiedDockerfile, stageIndex int, nameOrID string, layers bool, +) (string, map[string]string, error) { + if layers { + if len(sdf.Stages[stageIndex].Layers) == 0 { + return "", nil, fmt.Errorf("stage %q has no layers", nameOrID) + } + lastLayer := len(sdf.Stages[stageIndex].Layers) - 1 + return fmt.Sprintf("stage_%d_layer_%d", stageIndex, lastLayer), + map[string]string{"ltail": fmt.Sprintf("cluster_stage_%d", stageIndex)}, + nil + } + return fmt.Sprintf("stage_%d", stageIndex), map[string]string{}, nil +} diff --git a/internal/dockerfile2dot/build_test.go b/internal/dockerfile2dot/build_test.go index 0356640..de1d8be 100644 --- a/internal/dockerfile2dot/build_test.go +++ b/internal/dockerfile2dot/build_test.go @@ -58,7 +58,8 @@ func TestBuildDotFileErrors(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { _, err := BuildDotFile( - tt.simplifiedDockerfile, false, "default", tt.layers, false, 20, 0.5, 0.5, + tt.simplifiedDockerfile, + BuildOptions{EdgeStyle: "default", MaxLabelLength: 20, NodeSep: 0.5, RankSep: 0.5, Layers: tt.layers}, ) if err == nil { t.Error("BuildDotFile() expected an error, got nil") @@ -205,13 +206,15 @@ func TestBuildDotFile(t *testing.T) { t.Run(tt.name, func(t *testing.T) { got, err := BuildDotFile( tt.args.simplifiedDockerfile, - tt.args.concentrate, - tt.args.edgestyle, - tt.args.layers, - tt.args.legend, - tt.args.maxLabelLength, - tt.args.nodesep, - tt.args.ranksep, + BuildOptions{ + Concentrate: tt.args.concentrate, + EdgeStyle: tt.args.edgestyle, + Layers: tt.args.layers, + Legend: tt.args.legend, + MaxLabelLength: tt.args.maxLabelLength, + NodeSep: tt.args.nodesep, + RankSep: tt.args.ranksep, + }, ) if err != nil { t.Fatalf("BuildDotFile() error = %v", err) diff --git a/internal/dockerfile2dot/convert.go b/internal/dockerfile2dot/convert.go index dfb397b..baa0c63 100644 --- a/internal/dockerfile2dot/convert.go +++ b/internal/dockerfile2dot/convert.go @@ -57,9 +57,7 @@ func newLayer( func dockerfileToSimplifiedDockerfile( content []byte, - maxLabelLength int, - scratchMode ScratchMode, - separateImages []string, + opts ParseOptions, ) (simplifiedDockerfile SimplifiedDockerfile, err error) { result, err := parser.Parse(bytes.NewReader(content)) if err != nil { @@ -78,7 +76,7 @@ func dockerfileToSimplifiedDockerfile( case instructionFrom: // Create a new stage stageIndex++ - stage, layer := processFromInstruction(node, argReplacements, maxLabelLength, scratchMode, stages) + stage, layer := processFromInstruction(node, argReplacements, opts.MaxLabelLength, opts.ScratchMode, stages) simplifiedDockerfile.Stages = append(simplifiedDockerfile.Stages, stage) // Add a new layer @@ -88,14 +86,14 @@ func dockerfileToSimplifiedDockerfile( ) case instructionCopy: - layer := processCopyInstruction(node, argReplacements, maxLabelLength, scratchMode) + layer := processCopyInstruction(node, argReplacements, opts.MaxLabelLength, opts.ScratchMode) simplifiedDockerfile.Stages[stageIndex].Layers = append( simplifiedDockerfile.Stages[stageIndex].Layers, layer, ) case instructionRun: - layer := processRunInstruction(node, argReplacements, maxLabelLength, scratchMode) + layer := processRunInstruction(node, argReplacements, opts.MaxLabelLength, opts.ScratchMode) simplifiedDockerfile.Stages[stageIndex].Layers = append( simplifiedDockerfile.Stages[stageIndex].Layers, layer, @@ -103,7 +101,7 @@ func dockerfileToSimplifiedDockerfile( default: if stageIndex == -1 { - layer := processBeforeFirstStage(node, &argReplacements, maxLabelLength) + layer := processBeforeFirstStage(node, &argReplacements, opts.MaxLabelLength) simplifiedDockerfile.BeforeFirstStage = append( simplifiedDockerfile.BeforeFirstStage, layer, @@ -111,7 +109,7 @@ func dockerfileToSimplifiedDockerfile( break } - layer := newLayer(node, argReplacements, maxLabelLength) + layer := newLayer(node, argReplacements, opts.MaxLabelLength) simplifiedDockerfile.Stages[stageIndex].Layers = append( simplifiedDockerfile.Stages[stageIndex].Layers, layer, @@ -119,7 +117,7 @@ func dockerfileToSimplifiedDockerfile( } } - addExternalImages(&simplifiedDockerfile, stages, scratchMode, separateImages) + addExternalImages(&simplifiedDockerfile, stages, opts.ScratchMode, opts.SeparateImages) return } @@ -256,14 +254,12 @@ func addExternalImages( // Track already-seen image IDs to avoid duplicates seen := make(map[string]struct{}) - numStages := len(simplifiedDockerfile.Stages) - // Iterate through all stages and layers to find external image dependencies for stageIndex, stage := range simplifiedDockerfile.Stages { for layerIndex, layer := range stage.Layers { for waitForIndex, waitFor := range layer.WaitFors { imageID, skip := resolveExternalImageID( - waitFor.ID, stages, numStages, scratchMode, separateSet, separateCounters, + waitFor.ID, simplifiedDockerfile.Stages, stages, scratchMode, separateSet, separateCounters, ) if skip { continue @@ -290,20 +286,22 @@ func addExternalImages( // (internal stage reference or scratch in hidden mode). func resolveExternalImageID( rawID string, - stages map[string]struct{}, - numStages int, + allStages []Stage, + stageNameSet map[string]struct{}, scratchMode ScratchMode, separateSet map[string]struct{}, separateCounters map[string]int, ) (imageID string, skip bool) { // Skip internal stage references by name - if _, ok := stages[rawID]; ok { + if _, ok := stageNameSet[rawID]; ok { return "", true } - // Skip internal stage references by numeric index - if idx, err := strconv.Atoi(rawID); err == nil && idx >= 0 && idx < numStages { - return "", true + // Skip internal stage references by numeric index (in-range only) + if _, err := strconv.Atoi(rawID); err == nil { + if _, found := findStageIndex(allStages, rawID); found { + return "", true + } } // Handle scratch modes diff --git a/internal/dockerfile2dot/convert_test.go b/internal/dockerfile2dot/convert_test.go index c10e6f0..9905444 100644 --- a/internal/dockerfile2dot/convert_test.go +++ b/internal/dockerfile2dot/convert_test.go @@ -6,6 +6,57 @@ import ( "github.com/google/go-cmp/cmp" ) +func TestScratchModeFromString(t *testing.T) { + tests := []struct { + input string + want ScratchMode + }{ + {"collapsed", ScratchCollapsed}, + {"separated", ScratchSeparated}, + {"hidden", ScratchHidden}, + {"", ScratchCollapsed}, + {"invalid", ScratchCollapsed}, + } + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + if got := ScratchModeFromString(tt.input); got != tt.want { + t.Errorf("ScratchModeFromString(%q) = %v, want %v", tt.input, got, tt.want) + } + }) + } +} + +func Test_findStageIndex(t *testing.T) { + stages := []Stage{ + {Name: "base"}, + {Name: ""}, + {Name: "final"}, + } + tests := []struct { + nameOrID string + wantIndex int + wantFound bool + }{ + {"base", 0, true}, + {"final", 2, true}, + {"0", 0, true}, + {"2", 2, true}, + {"1", 1, true}, + {"3", 3, false}, // out of range + {"-1", -1, false}, // negative + {"unknown", -1, false}, + } + for _, tt := range tests { + t.Run(tt.nameOrID, func(t *testing.T) { + gotIndex, gotFound := findStageIndex(stages, tt.nameOrID) + if gotFound != tt.wantFound || gotIndex != tt.wantIndex { + t.Errorf("findStageIndex(%q) = (%d, %v), want (%d, %v)", + tt.nameOrID, gotIndex, gotFound, tt.wantIndex, tt.wantFound) + } + }) + } +} + func Test_dockerfileToSimplifiedDockerfile(t *testing.T) { type args struct { content []byte @@ -851,9 +902,11 @@ RUN echo other`), t.Run(tt.name, func(t *testing.T) { got, err := dockerfileToSimplifiedDockerfile( tt.args.content, - tt.args.maxLabelLength, - tt.args.scratchMode, - tt.args.separateImages, + ParseOptions{ + MaxLabelLength: tt.args.maxLabelLength, + ScratchMode: tt.args.scratchMode, + SeparateImages: tt.args.separateImages, + }, ) if err != nil { t.Errorf("dockerfileToSimplifiedDockerfile() error = %v", err) diff --git a/internal/dockerfile2dot/filter.go b/internal/dockerfile2dot/filter.go new file mode 100644 index 0000000..6440047 --- /dev/null +++ b/internal/dockerfile2dot/filter.go @@ -0,0 +1,133 @@ +package dockerfile2dot + +import ( + "fmt" + "strconv" +) + +// filterToTargets returns a new SimplifiedDockerfile containing only the +// stages that are transitively needed to build any of the named targets. +// External images referenced by the retained stages are also retained. +// Returns an error if any target name does not correspond to a stage. +func filterToTargets(sdf SimplifiedDockerfile, targets []string) (SimplifiedDockerfile, error) { + targetIndices, err := resolveTargetIndices(sdf.Stages, targets) + if err != nil { + return SimplifiedDockerfile{}, err + } + + reachable := collectReachableIndices(sdf.Stages, targetIndices) + + // Build sorted list of retained indices (preserving original order). + retained := make([]int, 0, len(reachable)) + for i := range sdf.Stages { + if _, ok := reachable[i]; ok { + retained = append(retained, i) + } + } + + // Build old-index → new-index mapping for remapping numeric WaitFor IDs. + oldToNew := make(map[int]int, len(retained)) + for newIdx, oldIdx := range retained { + oldToNew[oldIdx] = newIdx + } + + filteredStages := buildFilteredStages(sdf.Stages, retained, oldToNew) + filteredExternal := filterExternalImages(sdf.ExternalImages, filteredStages) + + return SimplifiedDockerfile{ + BeforeFirstStage: sdf.BeforeFirstStage, + Stages: filteredStages, + ExternalImages: filteredExternal, + }, nil +} + +// resolveTargetIndices validates target names and returns their stage indices. +func resolveTargetIndices(stages []Stage, targets []string) ([]int, error) { + indices := make([]int, 0, len(targets)) + for _, target := range targets { + idx, found := findStageIndex(stages, target) + if !found { + return nil, fmt.Errorf("target %q not found in Dockerfile", target) + } + indices = append(indices, idx) + } + return indices, nil +} + +// collectReachableIndices performs a BFS from target indices, following WaitFor +// dependencies backwards through the stage graph. +func collectReachableIndices(stages []Stage, targetIndices []int) map[int]struct{} { + reachable := make(map[int]struct{}) + queue := make([]int, len(targetIndices)) + copy(queue, targetIndices) + + for len(queue) > 0 { + idx := queue[0] + queue = queue[1:] + + if _, seen := reachable[idx]; seen { + continue + } + reachable[idx] = struct{}{} + + for _, layer := range stages[idx].Layers { + for _, waitFor := range layer.WaitFors { + if depIdx, found := findStageIndex(stages, waitFor.ID); found { + queue = append(queue, depIdx) + } + } + } + } + return reachable +} + +// buildFilteredStages copies the retained stages, remapping any numeric WaitFor IDs. +func buildFilteredStages(stages []Stage, retained []int, oldToNew map[int]int) []Stage { + filtered := make([]Stage, 0, len(retained)) + for _, oldIdx := range retained { + filtered = append(filtered, remapStage(stages[oldIdx], oldToNew)) + } + return filtered +} + +// remapStage returns a copy of stage with numeric WaitFor IDs updated to new indices. +func remapStage(stage Stage, oldToNew map[int]int) Stage { + newLayers := make([]Layer, len(stage.Layers)) + for li, layer := range stage.Layers { + newLayers[li] = remapLayer(layer, oldToNew) + } + return Stage{Name: stage.Name, Layers: newLayers} +} + +// remapLayer returns a copy of layer with numeric WaitFor IDs updated to new indices. +func remapLayer(layer Layer, oldToNew map[int]int) Layer { + newWaitFors := make([]WaitFor, len(layer.WaitFors)) + for wi, wf := range layer.WaitFors { + if origIdx, err := strconv.Atoi(wf.ID); err == nil { + if newIdx, ok := oldToNew[origIdx]; ok { + wf.ID = strconv.Itoa(newIdx) + } + } + newWaitFors[wi] = wf + } + return Layer{Label: layer.Label, WaitFors: newWaitFors} +} + +// filterExternalImages retains only external images referenced by the filtered stages. +func filterExternalImages(allImages []ExternalImage, filteredStages []Stage) []ExternalImage { + referencedIDs := make(map[string]struct{}) + for _, stage := range filteredStages { + for _, layer := range stage.Layers { + for _, wf := range layer.WaitFors { + referencedIDs[wf.ID] = struct{}{} + } + } + } + filtered := make([]ExternalImage, 0, len(allImages)) + for _, img := range allImages { + if _, ok := referencedIDs[img.ID]; ok { + filtered = append(filtered, img) + } + } + return filtered +} diff --git a/internal/dockerfile2dot/filter_test.go b/internal/dockerfile2dot/filter_test.go new file mode 100644 index 0000000..011d42c --- /dev/null +++ b/internal/dockerfile2dot/filter_test.go @@ -0,0 +1,351 @@ +package dockerfile2dot + +import ( + "testing" + + "github.com/google/go-cmp/cmp" +) + +// stageFrom is a helper that builds a Stage with a single FROM layer. +func stageFrom(name, image string, wfType waitForType) Stage { + return Stage{ + Name: name, + Layers: []Layer{{ + Label: "FROM " + image, + WaitFors: []WaitFor{{ID: image, Type: wfType}}, + }}, + } +} + +func Test_filterToTargets(t *testing.T) { + tests := []struct { + name string + sdf SimplifiedDockerfile + targets []string + want SimplifiedDockerfile + wantErr bool + }{ + { + name: "single target retains only that stage and its ancestors", + sdf: SimplifiedDockerfile{ + Stages: []Stage{ + stageFrom("base", "ubuntu", waitForFrom), + stageFrom("mid", "base", waitForFrom), + stageFrom("final", "mid", waitForFrom), + stageFrom("unrelated", "alpine", waitForFrom), + }, + ExternalImages: []ExternalImage{ + {ID: "ubuntu", Name: "ubuntu"}, + {ID: "alpine", Name: "alpine"}, + }, + }, + targets: []string{"final"}, + want: SimplifiedDockerfile{ + Stages: []Stage{ + stageFrom("base", "ubuntu", waitForFrom), + stageFrom("mid", "base", waitForFrom), + stageFrom("final", "mid", waitForFrom), + }, + ExternalImages: []ExternalImage{ + {ID: "ubuntu", Name: "ubuntu"}, + }, + }, + }, + { + name: "single target with no deps retains only that stage", + sdf: SimplifiedDockerfile{ + Stages: []Stage{ + stageFrom("standalone", "alpine", waitForFrom), + stageFrom("other", "ubuntu", waitForFrom), + }, + ExternalImages: []ExternalImage{ + {ID: "alpine", Name: "alpine"}, + {ID: "ubuntu", Name: "ubuntu"}, + }, + }, + targets: []string{"standalone"}, + want: SimplifiedDockerfile{ + Stages: []Stage{ + stageFrom("standalone", "alpine", waitForFrom), + }, + ExternalImages: []ExternalImage{ + {ID: "alpine", Name: "alpine"}, + }, + }, + }, + { + name: "two targets union their ancestors", + sdf: SimplifiedDockerfile{ + Stages: []Stage{ + stageFrom("base", "ubuntu", waitForFrom), + stageFrom("app1", "base", waitForFrom), + stageFrom("app2", "alpine", waitForFrom), + stageFrom("unused", "scratch", waitForFrom), + }, + ExternalImages: []ExternalImage{ + {ID: "ubuntu", Name: "ubuntu"}, + {ID: "alpine", Name: "alpine"}, + {ID: "scratch", Name: "scratch"}, + }, + }, + targets: []string{"app1", "app2"}, + want: SimplifiedDockerfile{ + Stages: []Stage{ + stageFrom("base", "ubuntu", waitForFrom), + stageFrom("app1", "base", waitForFrom), + stageFrom("app2", "alpine", waitForFrom), + }, + ExternalImages: []ExternalImage{ + {ID: "ubuntu", Name: "ubuntu"}, + {ID: "alpine", Name: "alpine"}, + }, + }, + }, + { + name: "two targets with shared ancestor", + sdf: SimplifiedDockerfile{ + Stages: []Stage{ + stageFrom("shared", "ubuntu", waitForFrom), + stageFrom("app1", "shared", waitForFrom), + stageFrom("app2", "shared", waitForFrom), + }, + ExternalImages: []ExternalImage{ + {ID: "ubuntu", Name: "ubuntu"}, + }, + }, + targets: []string{"app1", "app2"}, + want: SimplifiedDockerfile{ + Stages: []Stage{ + stageFrom("shared", "ubuntu", waitForFrom), + stageFrom("app1", "shared", waitForFrom), + stageFrom("app2", "shared", waitForFrom), + }, + ExternalImages: []ExternalImage{ + {ID: "ubuntu", Name: "ubuntu"}, + }, + }, + }, + { + name: "COPY and RUN mount dependencies are followed", + sdf: SimplifiedDockerfile{ + Stages: []Stage{ + stageFrom("builder", "ubuntu", waitForFrom), + stageFrom("cache", "alpine", waitForFrom), + { + Name: "final", + Layers: []Layer{ + {Label: "FROM scratch", WaitFors: []WaitFor{{ID: "scratch", Type: waitForFrom}}}, + {Label: "COPY --from=builder", WaitFors: []WaitFor{{ID: "builder", Type: waitForCopy}}}, + {Label: "RUN --mount=from=cache", WaitFors: []WaitFor{{ID: "cache", Type: waitForMount}}}, + }, + }, + stageFrom("unused", "debian", waitForFrom), + }, + ExternalImages: []ExternalImage{ + {ID: "ubuntu", Name: "ubuntu"}, + {ID: "alpine", Name: "alpine"}, + {ID: "scratch", Name: "scratch"}, + {ID: "debian", Name: "debian"}, + }, + }, + targets: []string{"final"}, + want: SimplifiedDockerfile{ + Stages: []Stage{ + stageFrom("builder", "ubuntu", waitForFrom), + stageFrom("cache", "alpine", waitForFrom), + { + Name: "final", + Layers: []Layer{ + {Label: "FROM scratch", WaitFors: []WaitFor{{ID: "scratch", Type: waitForFrom}}}, + {Label: "COPY --from=builder", WaitFors: []WaitFor{{ID: "builder", Type: waitForCopy}}}, + {Label: "RUN --mount=from=cache", WaitFors: []WaitFor{{ID: "cache", Type: waitForMount}}}, + }, + }, + }, + ExternalImages: []ExternalImage{ + {ID: "ubuntu", Name: "ubuntu"}, + {ID: "alpine", Name: "alpine"}, + {ID: "scratch", Name: "scratch"}, + }, + }, + }, + { + // Stages: 0=base, 1=unused, 2=final (COPY --from=0) + // After filtering to "final": stages become 0=base, 1=final + // WaitFor "0" stays "0" (base is still at index 0 in filtered result) + name: "numeric WaitFor IDs are remapped after stage elision (no shift)", + sdf: SimplifiedDockerfile{ + Stages: []Stage{ + stageFrom("base", "ubuntu", waitForFrom), + stageFrom("unused", "alpine", waitForFrom), + { + Name: "final", + Layers: []Layer{ + {Label: "FROM scratch", WaitFors: []WaitFor{{ID: "scratch", Type: waitForFrom}}}, + {Label: "COPY --from=0", WaitFors: []WaitFor{{ID: "0", Type: waitForCopy}}}, + }, + }, + }, + ExternalImages: []ExternalImage{ + {ID: "ubuntu", Name: "ubuntu"}, + {ID: "alpine", Name: "alpine"}, + {ID: "scratch", Name: "scratch"}, + }, + }, + targets: []string{"final"}, + want: SimplifiedDockerfile{ + Stages: []Stage{ + stageFrom("base", "ubuntu", waitForFrom), + { + Name: "final", + Layers: []Layer{ + {Label: "FROM scratch", WaitFors: []WaitFor{{ID: "scratch", Type: waitForFrom}}}, + {Label: "COPY --from=0", WaitFors: []WaitFor{{ID: "0", Type: waitForCopy}}}, + }, + }, + }, + ExternalImages: []ExternalImage{ + {ID: "ubuntu", Name: "ubuntu"}, + {ID: "scratch", Name: "scratch"}, + }, + }, + }, + { + // Stages: 0=elided, 1=base, 2=final (COPY --from=1) + // After filtering to "final": stages become 0=base, 1=final + // WaitFor "1" in final becomes "0" + name: "numeric WaitFor IDs are remapped when earlier stage is elided", + sdf: SimplifiedDockerfile{ + Stages: []Stage{ + stageFrom("elided", "debian", waitForFrom), + stageFrom("base", "ubuntu", waitForFrom), + { + Name: "final", + Layers: []Layer{ + {Label: "FROM scratch", WaitFors: []WaitFor{{ID: "scratch", Type: waitForFrom}}}, + {Label: "COPY --from=1", WaitFors: []WaitFor{{ID: "1", Type: waitForCopy}}}, + }, + }, + }, + ExternalImages: []ExternalImage{ + {ID: "debian", Name: "debian"}, + {ID: "ubuntu", Name: "ubuntu"}, + {ID: "scratch", Name: "scratch"}, + }, + }, + targets: []string{"final"}, + want: SimplifiedDockerfile{ + Stages: []Stage{ + stageFrom("base", "ubuntu", waitForFrom), + { + Name: "final", + Layers: []Layer{ + {Label: "FROM scratch", WaitFors: []WaitFor{{ID: "scratch", Type: waitForFrom}}}, + {Label: "COPY --from=1", WaitFors: []WaitFor{{ID: "0", Type: waitForCopy}}}, + }, + }, + }, + ExternalImages: []ExternalImage{ + {ID: "ubuntu", Name: "ubuntu"}, + {ID: "scratch", Name: "scratch"}, + }, + }, + }, + { + name: "BeforeFirstStage is always preserved", + sdf: SimplifiedDockerfile{ + BeforeFirstStage: []Layer{ + {Label: "ARG VERSION=1.0"}, + }, + Stages: []Stage{ + stageFrom("app", "alpine", waitForFrom), + stageFrom("other", "ubuntu", waitForFrom), + }, + ExternalImages: []ExternalImage{ + {ID: "alpine", Name: "alpine"}, + {ID: "ubuntu", Name: "ubuntu"}, + }, + }, + targets: []string{"app"}, + want: SimplifiedDockerfile{ + BeforeFirstStage: []Layer{ + {Label: "ARG VERSION=1.0"}, + }, + Stages: []Stage{ + stageFrom("app", "alpine", waitForFrom), + }, + ExternalImages: []ExternalImage{ + {ID: "alpine", Name: "alpine"}, + }, + }, + }, + { + name: "invalid target returns error", + sdf: SimplifiedDockerfile{ + Stages: []Stage{ + {Name: "app", Layers: []Layer{{Label: "FROM alpine"}}}, + }, + }, + targets: []string{"nonexistent"}, + wantErr: true, + }, + { + name: "target that is the first and only stage", + sdf: SimplifiedDockerfile{ + Stages: []Stage{ + stageFrom("only", "scratch", waitForFrom), + }, + ExternalImages: []ExternalImage{ + {ID: "scratch", Name: "scratch"}, + }, + }, + targets: []string{"only"}, + want: SimplifiedDockerfile{ + Stages: []Stage{ + stageFrom("only", "scratch", waitForFrom), + }, + ExternalImages: []ExternalImage{ + {ID: "scratch", Name: "scratch"}, + }, + }, + }, + { + name: "external images not referenced by kept stages are elided", + sdf: SimplifiedDockerfile{ + Stages: []Stage{ + stageFrom("a", "img-a", waitForFrom), + stageFrom("b", "img-b", waitForFrom), + }, + ExternalImages: []ExternalImage{ + {ID: "img-a", Name: "img-a"}, + {ID: "img-b", Name: "img-b"}, + }, + }, + targets: []string{"a"}, + want: SimplifiedDockerfile{ + Stages: []Stage{ + stageFrom("a", "img-a", waitForFrom), + }, + ExternalImages: []ExternalImage{ + {ID: "img-a", Name: "img-a"}, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := filterToTargets(tt.sdf, tt.targets) + if (err != nil) != tt.wantErr { + t.Errorf("filterToTargets() error = %v, wantErr %v", err, tt.wantErr) + return + } + if tt.wantErr { + return + } + if diff := cmp.Diff(tt.want, got); diff != "" { + t.Errorf("filterToTargets() mismatch (-want +got):\n%s", diff) + } + }) + } +} diff --git a/internal/dockerfile2dot/load.go b/internal/dockerfile2dot/load.go index 6cd2c6d..6850ed2 100644 --- a/internal/dockerfile2dot/load.go +++ b/internal/dockerfile2dot/load.go @@ -16,9 +16,7 @@ import ( func LoadAndParseDockerfile( inputFS afero.Fs, filename string, - maxLabelLength int, - scratchMode ScratchMode, - separateImages []string, + opts ParseOptions, ) (SimplifiedDockerfile, error) { content, err := afero.ReadFile(inputFS, filename) if err != nil { @@ -31,5 +29,15 @@ func LoadAndParseDockerfile( } return SimplifiedDockerfile{}, err } - return dockerfileToSimplifiedDockerfile(content, maxLabelLength, scratchMode, separateImages) + sdf, err := dockerfileToSimplifiedDockerfile(content, opts) + if err != nil { + return SimplifiedDockerfile{}, err + } + if len(opts.Targets) > 0 { + sdf, err = filterToTargets(sdf, opts.Targets) + if err != nil { + return SimplifiedDockerfile{}, err + } + } + return sdf, nil } diff --git a/internal/dockerfile2dot/load_test.go b/internal/dockerfile2dot/load_test.go index 2955e71..5a9da2d 100644 --- a/internal/dockerfile2dot/load_test.go +++ b/internal/dockerfile2dot/load_test.go @@ -54,9 +54,7 @@ func TestLoadAndParseDockerfile(t *testing.T) { _, err := LoadAndParseDockerfile( tt.args.inputFS, tt.args.filename, - 20, // Default maxLabelLength - ScratchCollapsed, // Default scratchMode - nil, // No separate images + ParseOptions{MaxLabelLength: 20}, ) if (err != nil) != tt.wantErr { t.Errorf("LoadAndParseDockerfile() error = %v, wantErr %v", err, tt.wantErr) diff --git a/internal/dockerfile2dot/structs.go b/internal/dockerfile2dot/structs.go index e08f3ae..3b0c799 100644 --- a/internal/dockerfile2dot/structs.go +++ b/internal/dockerfile2dot/structs.go @@ -1,5 +1,7 @@ package dockerfile2dot +import "strconv" + // SimplifiedDockerfile contains the parts of the Dockerfile // that are relevant for generating the multi-stage build graph. type SimplifiedDockerfile struct { @@ -57,3 +59,53 @@ type WaitFor struct { 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 } + +// findStageIndex returns the index of the stage identified by nameOrID (a stage +// name or a decimal numeric index string) and true if found. Returns -1 and +// false if not found or if a numeric index is out of range. +func findStageIndex(stages []Stage, nameOrID string) (int, bool) { + if idx, err := strconv.Atoi(nameOrID); err == nil { + if idx >= 0 && idx < len(stages) { + return idx, true + } + return idx, false + } + for i, s := range stages { + if s.Name == nameOrID { + return i, true + } + } + return -1, false +} + +// ScratchModeFromString converts a validated string to a ScratchMode constant. +// The empty string and any unrecognized value returns ScratchCollapsed. +func ScratchModeFromString(s string) ScratchMode { + switch s { + case "separated": + return ScratchSeparated + case "hidden": + return ScratchHidden + default: + return ScratchCollapsed + } +} + +// ParseOptions controls how a Dockerfile is parsed into a SimplifiedDockerfile. +type ParseOptions struct { + MaxLabelLength int + ScratchMode ScratchMode + SeparateImages []string + Targets []string +} + +// BuildOptions controls how a SimplifiedDockerfile is rendered into a DOT file. +type BuildOptions struct { + Concentrate bool + EdgeStyle string + Layers bool + Legend bool + MaxLabelLength int + NodeSep float64 + RankSep float64 +} From 13b4a528451bbade3351f6d39a86a8a98e77fa91 Mon Sep 17 00:00:00 2001 From: Patrick Hoefler Date: Fri, 13 Mar 2026 14:16:44 +0100 Subject: [PATCH 2/5] fix: normalize whitespace in --target and exclude internal stage refs from external image filter Two correctness fixes from code review: - Trim whitespace from --target values so "--target a, b" works (consistent with how --separate normalizes its inputs) - In filterExternalImages, skip WaitFor IDs that resolve to an internal stage so a kept stage named "alpine" doesn't incorrectly retain an unrelated external image with the same ID from an elided stage Co-Authored-By: Claude Sonnet 4.6 --- internal/dockerfile2dot/filter.go | 16 +++++-- internal/dockerfile2dot/filter_test.go | 63 ++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 3 deletions(-) diff --git a/internal/dockerfile2dot/filter.go b/internal/dockerfile2dot/filter.go index 6440047..411924b 100644 --- a/internal/dockerfile2dot/filter.go +++ b/internal/dockerfile2dot/filter.go @@ -3,6 +3,7 @@ package dockerfile2dot import ( "fmt" "strconv" + "strings" ) // filterToTargets returns a new SimplifiedDockerfile containing only the @@ -42,12 +43,17 @@ func filterToTargets(sdf SimplifiedDockerfile, targets []string) (SimplifiedDock } // resolveTargetIndices validates target names and returns their stage indices. +// Whitespace is trimmed from each target; empty entries are skipped. func resolveTargetIndices(stages []Stage, targets []string) ([]int, error) { indices := make([]int, 0, len(targets)) for _, target := range targets { - idx, found := findStageIndex(stages, target) + trimmed := strings.TrimSpace(target) + if trimmed == "" { + continue + } + idx, found := findStageIndex(stages, trimmed) if !found { - return nil, fmt.Errorf("target %q not found in Dockerfile", target) + return nil, fmt.Errorf("target %q not found in Dockerfile", trimmed) } indices = append(indices, idx) } @@ -114,12 +120,16 @@ func remapLayer(layer Layer, oldToNew map[int]int) Layer { } // filterExternalImages retains only external images referenced by the filtered stages. +// WaitFor IDs that resolve to internal stages are excluded. func filterExternalImages(allImages []ExternalImage, filteredStages []Stage) []ExternalImage { referencedIDs := make(map[string]struct{}) for _, stage := range filteredStages { for _, layer := range stage.Layers { for _, wf := range layer.WaitFors { - referencedIDs[wf.ID] = struct{}{} + // Only count as an external image reference if it doesn't resolve to an internal stage. + if _, found := findStageIndex(filteredStages, wf.ID); !found { + referencedIDs[wf.ID] = struct{}{} + } } } } diff --git a/internal/dockerfile2dot/filter_test.go b/internal/dockerfile2dot/filter_test.go index 011d42c..fcc9999 100644 --- a/internal/dockerfile2dot/filter_test.go +++ b/internal/dockerfile2dot/filter_test.go @@ -331,6 +331,69 @@ func Test_filterToTargets(t *testing.T) { }, }, }, + { + // Stage "alpine" depends on internal stage "base" (same name as a common image). + // The external image "alpine" from an elided stage must not be incorrectly retained + // because "base" happens to match a stage name. + name: "internal stage WaitFor IDs are not treated as external image references", + sdf: SimplifiedDockerfile{ + Stages: []Stage{ + stageFrom("base", "ubuntu", waitForFrom), + { + Name: "app", + Layers: []Layer{{ + Label: "FROM base", + WaitFors: []WaitFor{{ID: "base", Type: waitForFrom}}, + }}, + }, + stageFrom("other", "alpine", waitForFrom), + }, + ExternalImages: []ExternalImage{ + {ID: "ubuntu", Name: "ubuntu"}, + {ID: "alpine", Name: "alpine"}, + }, + }, + targets: []string{"app"}, + want: SimplifiedDockerfile{ + Stages: []Stage{ + stageFrom("base", "ubuntu", waitForFrom), + { + Name: "app", + Layers: []Layer{{ + Label: "FROM base", + WaitFors: []WaitFor{{ID: "base", Type: waitForFrom}}, + }}, + }, + }, + ExternalImages: []ExternalImage{ + {ID: "ubuntu", Name: "ubuntu"}, + }, + }, + }, + { + name: "target with leading/trailing whitespace is normalized", + sdf: SimplifiedDockerfile{ + Stages: []Stage{ + stageFrom("app", "alpine", waitForFrom), + stageFrom("other", "ubuntu", waitForFrom), + }, + ExternalImages: []ExternalImage{ + {ID: "alpine", Name: "alpine"}, + {ID: "ubuntu", Name: "ubuntu"}, + }, + }, + targets: []string{" app ", " other "}, + want: SimplifiedDockerfile{ + Stages: []Stage{ + stageFrom("app", "alpine", waitForFrom), + stageFrom("other", "ubuntu", waitForFrom), + }, + ExternalImages: []ExternalImage{ + {ID: "alpine", Name: "alpine"}, + {ID: "ubuntu", Name: "ubuntu"}, + }, + }, + }, } for _, tt := range tests { From 39dab0d25dc1e017d81d00add4d8fd569a99c8a0 Mon Sep 17 00:00:00 2001 From: Patrick Hoefler Date: Fri, 13 Mar 2026 14:19:35 +0100 Subject: [PATCH 3/5] docs: align findStageIndex comment with actual return behavior The comment claimed -1 is returned for out-of-range numeric indices, but the implementation returns the parsed index value. Update the comment to accurately describe both the numeric out-of-range and name-not-found cases. Co-Authored-By: Claude Sonnet 4.6 --- internal/dockerfile2dot/structs.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/dockerfile2dot/structs.go b/internal/dockerfile2dot/structs.go index 3b0c799..94b6b68 100644 --- a/internal/dockerfile2dot/structs.go +++ b/internal/dockerfile2dot/structs.go @@ -61,8 +61,9 @@ type WaitFor struct { } // findStageIndex returns the index of the stage identified by nameOrID (a stage -// name or a decimal numeric index string) and true if found. Returns -1 and -// false if not found or if a numeric index is out of range. +// name or a decimal numeric index string) and true if found. For a numeric +// index that parses but is out of range, it returns that index and false. For +// a non-numeric name that is not found, it returns -1 and false. func findStageIndex(stages []Stage, nameOrID string) (int, bool) { if idx, err := strconv.Atoi(nameOrID); err == nil { if idx >= 0 && idx < len(stages) { From 985139d51ad55624713cc5799285c458caa99933 Mon Sep 17 00:00:00 2001 From: Patrick Hoefler Date: Fri, 13 Mar 2026 14:26:41 +0100 Subject: [PATCH 4/5] fix: error on all-empty targets and fix doc comment grammar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - resolveTargetIndices now returns an error when all target entries are empty or whitespace-only, preventing a silent empty graph - Fix grammar in ScratchModeFromString doc comment ("returns" → "return") Co-Authored-By: Claude Sonnet 4.6 --- internal/dockerfile2dot/filter.go | 3 +++ internal/dockerfile2dot/filter_test.go | 10 ++++++++++ internal/dockerfile2dot/structs.go | 2 +- 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/internal/dockerfile2dot/filter.go b/internal/dockerfile2dot/filter.go index 411924b..72bc901 100644 --- a/internal/dockerfile2dot/filter.go +++ b/internal/dockerfile2dot/filter.go @@ -57,6 +57,9 @@ func resolveTargetIndices(stages []Stage, targets []string) ([]int, error) { } indices = append(indices, idx) } + if len(indices) == 0 { + return nil, fmt.Errorf("no valid targets specified") + } return indices, nil } diff --git a/internal/dockerfile2dot/filter_test.go b/internal/dockerfile2dot/filter_test.go index fcc9999..d4a537c 100644 --- a/internal/dockerfile2dot/filter_test.go +++ b/internal/dockerfile2dot/filter_test.go @@ -289,6 +289,16 @@ func Test_filterToTargets(t *testing.T) { targets: []string{"nonexistent"}, wantErr: true, }, + { + name: "all-whitespace targets return error", + sdf: SimplifiedDockerfile{ + Stages: []Stage{ + {Name: "app", Layers: []Layer{{Label: "FROM alpine"}}}, + }, + }, + targets: []string{" ", " "}, + wantErr: true, + }, { name: "target that is the first and only stage", sdf: SimplifiedDockerfile{ diff --git a/internal/dockerfile2dot/structs.go b/internal/dockerfile2dot/structs.go index 94b6b68..696ee6c 100644 --- a/internal/dockerfile2dot/structs.go +++ b/internal/dockerfile2dot/structs.go @@ -80,7 +80,7 @@ func findStageIndex(stages []Stage, nameOrID string) (int, bool) { } // ScratchModeFromString converts a validated string to a ScratchMode constant. -// The empty string and any unrecognized value returns ScratchCollapsed. +// The empty string and any unrecognized value return ScratchCollapsed. func ScratchModeFromString(s string) ScratchMode { switch s { case "separated": From 6757784666bf28a6af2c8cf44f3929fd8f45a2f6 Mon Sep 17 00:00:00 2001 From: Patrick Hoefler Date: Fri, 13 Mar 2026 14:39:45 +0100 Subject: [PATCH 5/5] docs: document order-aware resolution limitation in findStageIndex Add a NOTE comment explaining that findStageIndex searches all stages regardless of definition order, which differs from Docker's behavior of only allowing references to previously-defined stages. This is a pre-existing limitation affecting multiple call sites across the codebase, not introduced by the --target feature. Co-Authored-By: Claude Opus 4.6 --- internal/dockerfile2dot/structs.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/dockerfile2dot/structs.go b/internal/dockerfile2dot/structs.go index 696ee6c..aff8dd3 100644 --- a/internal/dockerfile2dot/structs.go +++ b/internal/dockerfile2dot/structs.go @@ -64,6 +64,12 @@ type WaitFor struct { // name or a decimal numeric index string) and true if found. For a numeric // index that parses but is out of range, it returns that index and false. For // a non-numeric name that is not found, it returns -1 and false. +// +// NOTE: This function searches all stages regardless of definition order. +// Docker only allows referencing previously-defined stages, so a stage name +// that shadows an external image name could theoretically be misresolved. +// In practice this is unlikely because stage aliases rarely collide with +// base image names. A future improvement could accept a position limit. func findStageIndex(stages []Stage, nameOrID string) (int, bool) { if idx, err := strconv.Atoi(nameOrID); err == nil { if idx >= 0 && idx < len(stages) {