From 82dd42832851c587f09b1810f41aea85030fa833 Mon Sep 17 00:00:00 2001 From: Patrick Hoefler Date: Thu, 12 Mar 2026 21:01:06 +0100 Subject: [PATCH 1/6] fix: replace panics with proper error returns in dockerfile2dot - LoadAndParseDockerfile: return errors instead of panicking on filepath.Abs failure and non-ErrNotExist filesystem errors - getWaitForNodeID: return (string, map[string]string, error) instead of panicking when a node ID cannot be resolved - BuildDotFile: change signature to (string, error) and surface all gographviz errors via a first-error collector; extract helper functions to keep cyclomatic complexity within limits - addEdgesForStage, addLegend: return error and propagate Co-Authored-By: Claude Sonnet 4.6 --- internal/cmd/root.go | 5 +- internal/dockerfile2dot/build.go | 302 +++++++++++++++++--------- internal/dockerfile2dot/build_test.go | 8 +- internal/dockerfile2dot/load.go | 9 +- 4 files changed, 215 insertions(+), 109 deletions(-) diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 2cd2b01f..52d9c9a5 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -86,7 +86,7 @@ It creates a visual graph representation of the build process.`, } defer os.Remove(dotFile.Name()) - dotFileContent := dockerfile2dot.BuildDotFile( + dotFileContent, err := dockerfile2dot.BuildDotFile( dockerfile, f.concentrate, f.edgestyle.String(), @@ -96,6 +96,9 @@ It creates a visual graph representation of the build process.`, fmt.Sprintf("%.2f", f.nodesep), fmt.Sprintf("%.2f", f.ranksep), ) + if err != nil { + return + } _, err = dotFile.Write([]byte(dotFileContent)) if err != nil { diff --git a/internal/dockerfile2dot/build.go b/internal/dockerfile2dot/build.go index a708eff4..164a6767 100644 --- a/internal/dockerfile2dot/build.go +++ b/internal/dockerfile2dot/build.go @@ -19,25 +19,68 @@ func BuildDotFile( maxLabelLength int, nodesep string, ranksep string, -) string { +) (string, error) { // Create a new graph graph := gographviz.NewEscape() - _ = graph.SetName("G") - _ = graph.SetDir(true) - _ = graph.AddAttr("G", "compound", "true") // allow edges between clusters - _ = graph.AddAttr("G", "nodesep", nodesep) - _ = graph.AddAttr("G", "rankdir", "LR") - _ = graph.AddAttr("G", "ranksep", ranksep) + + var graphErr error + set := func(err error) { + if graphErr == nil { + graphErr = err + } + } + + set(graph.SetName("G")) + set(graph.SetDir(true)) + set(graph.AddAttr("G", "compound", "true")) // allow edges between clusters + set(graph.AddAttr("G", "nodesep", nodesep)) + set(graph.AddAttr("G", "rankdir", "LR")) + set(graph.AddAttr("G", "ranksep", ranksep)) if concentrate { - _ = graph.AddAttr("G", "concentrate", "true") + set(graph.AddAttr("G", "concentrate", "true")) } // Add the legend if requested if legend { - addLegend(graph, edgestyle) + if err := addLegend(graph, edgestyle); err != nil { + return "", err + } + } + + if err := addExternalImagesToGraph(graph, simplifiedDockerfile, maxLabelLength); err != nil { + return "", err + } + + if err := addStages(graph, simplifiedDockerfile, maxLabelLength, layers, edgestyle); err != nil { + return "", err + } + + // Add the ARGS that appear before the first stage, if layers are requested + if layers { + if err := addBeforeFirstStage(graph, simplifiedDockerfile); err != nil { + return "", err + } + } + + if graphErr != nil { + return "", graphErr + } + + return graph.String(), nil +} + +func addExternalImagesToGraph( + graph *gographviz.Escape, + simplifiedDockerfile SimplifiedDockerfile, + maxLabelLength int, +) error { + var graphErr error + set := func(err error) { + if graphErr == nil { + graphErr = err + } } - // Add the external images for externalImageIndex, externalImage := range simplifiedDockerfile.ExternalImages { label := externalImage.Name if len(label) > maxLabelLength { @@ -48,7 +91,7 @@ func BuildDotFile( label = truncate.Truncate(label, maxLabelLength, "...", truncatePosition) } - _ = graph.AddNode( + set(graph.AddNode( "G", fmt.Sprintf("external_image_%d", externalImageIndex), map[string]string{ @@ -59,7 +102,24 @@ func BuildDotFile( "color": "grey20", "fontcolor": "grey20", }, - ) + )) + } + + return graphErr +} + +func addStages( + graph *gographviz.Escape, + simplifiedDockerfile SimplifiedDockerfile, + maxLabelLength int, + layers bool, + edgestyle string, +) error { + var graphErr error + set := func(err error) { + if graphErr == nil { + graphErr = err + } } for stageIndex, stage := range simplifiedDockerfile.Stages { @@ -72,40 +132,8 @@ func BuildDotFile( // Add layers if requested if layers { - cluster := fmt.Sprintf("cluster_stage_%d", stageIndex) - - clusterAttrs := map[string]string{ - "label": getStageLabel(stageIndex, stage, 0), - "margin": "16", - } - - if stageIndex == len(simplifiedDockerfile.Stages)-1 { - clusterAttrs["style"] = "filled" - clusterAttrs["fillcolor"] = "grey90" - } - - _ = graph.AddSubGraph("G", cluster, clusterAttrs) - - for layerIndex, layer := range stage.Layers { - attrs["label"] = "\"" + layer.Label + "\"" - attrs["penwidth"] = "0.5" - attrs["style"] = "\"filled,rounded\"" - attrs["fillcolor"] = "white" - _ = graph.AddNode( - cluster, - fmt.Sprintf("stage_%d_layer_%d", stageIndex, layerIndex), - attrs, - ) - - // Add edges between layers to guarantee the correct order - if layerIndex > 0 { - _ = graph.AddEdge( - fmt.Sprintf("stage_%d_layer_%d", stageIndex, layerIndex-1), - fmt.Sprintf("stage_%d_layer_%d", stageIndex, layerIndex), - true, - nil, - ) - } + if err := addStageWithLayers(graph, set, simplifiedDockerfile, stageIndex, stage, attrs); err != nil { + return err } } else { // Add the build stages. @@ -115,45 +143,107 @@ func BuildDotFile( attrs["fillcolor"] = "grey90" } - _ = graph.AddNode("G", fmt.Sprintf("stage_%d", stageIndex), attrs) + set(graph.AddNode("G", fmt.Sprintf("stage_%d", stageIndex), attrs)) } - // Add the egdes for this build stage - addEdgesForStage( + // Add the edges for this build stage + if err := addEdgesForStage( stageIndex, stage, graph, simplifiedDockerfile, layers, edgestyle, - ) + ); err != nil { + return err + } } - // Add the ARGS that appear before the first stage, if layers are requested - if layers { - if len(simplifiedDockerfile.BeforeFirstStage) > 0 { - _ = graph.AddSubGraph( - "G", - "cluster_before_first_stage", - map[string]string{"label": "Before First Stage"}, - ) - for argIndex, arg := range simplifiedDockerfile.BeforeFirstStage { - _ = graph.AddNode( - "cluster_before_first_stage", - fmt.Sprintf("before_first_stage_%d", argIndex), - map[string]string{ - "label": arg.Label, - "shape": "box", - "style": "rounded", - "width": "2", - }, - ) - } + return graphErr +} + +func addStageWithLayers( + graph *gographviz.Escape, + set func(error), + simplifiedDockerfile SimplifiedDockerfile, + stageIndex int, + stage Stage, + attrs map[string]string, +) error { + cluster := fmt.Sprintf("cluster_stage_%d", stageIndex) + + clusterAttrs := map[string]string{ + "label": getStageLabel(stageIndex, stage, 0), + "margin": "16", + } + + if stageIndex == len(simplifiedDockerfile.Stages)-1 { + clusterAttrs["style"] = "filled" + clusterAttrs["fillcolor"] = "grey90" + } + + set(graph.AddSubGraph("G", cluster, clusterAttrs)) + + for layerIndex, layer := range stage.Layers { + attrs["label"] = "\"" + layer.Label + "\"" + attrs["penwidth"] = "0.5" + attrs["style"] = "\"filled,rounded\"" + attrs["fillcolor"] = "white" + set(graph.AddNode( + cluster, + fmt.Sprintf("stage_%d_layer_%d", stageIndex, layerIndex), + attrs, + )) + + // Add edges between layers to guarantee the correct order + if layerIndex > 0 { + set(graph.AddEdge( + fmt.Sprintf("stage_%d_layer_%d", stageIndex, layerIndex-1), + fmt.Sprintf("stage_%d_layer_%d", stageIndex, layerIndex), + true, + nil, + )) + } + } + + return nil +} + +func addBeforeFirstStage( + graph *gographviz.Escape, + simplifiedDockerfile SimplifiedDockerfile, +) error { + if len(simplifiedDockerfile.BeforeFirstStage) == 0 { + return nil + } + + var graphErr error + set := func(err error) { + if graphErr == nil { + graphErr = err } } - return graph.String() + set(graph.AddSubGraph( + "G", + "cluster_before_first_stage", + map[string]string{"label": "Before First Stage"}, + )) + for argIndex, arg := range simplifiedDockerfile.BeforeFirstStage { + set(graph.AddNode( + "cluster_before_first_stage", + fmt.Sprintf("before_first_stage_%d", argIndex), + map[string]string{ + "label": arg.Label, + "shape": "box", + "style": "rounded", + "width": "2", + }, + )) + } + + return graphErr } func addEdgesForStage( stageIndex int, stage Stage, graph *gographviz.Escape, simplifiedDockerfile SimplifiedDockerfile, layers bool, edgestyle string, -) { +) error { for layerIndex, layer := range stage.Layers { for _, waitFor := range layer.WaitFors { edgeAttrs := map[string]string{} @@ -169,9 +259,12 @@ func addEdgesForStage( } } - sourceNodeID, additionalEdgeAttrs := getWaitForNodeID( + sourceNodeID, additionalEdgeAttrs, err := getWaitForNodeID( simplifiedDockerfile, waitFor.ID, layers, ) + if err != nil { + return err + } maps.Copy(edgeAttrs, additionalEdgeAttrs) targetNodeID := fmt.Sprintf("stage_%d", stageIndex) @@ -179,15 +272,25 @@ func addEdgesForStage( targetNodeID = targetNodeID + fmt.Sprintf("_layer_%d", layerIndex) } - _ = graph.AddEdge(sourceNodeID, targetNodeID, true, edgeAttrs) + if err := graph.AddEdge(sourceNodeID, targetNodeID, true, edgeAttrs); err != nil { + return err + } } } + return nil } -func addLegend(graph *gographviz.Escape, edgestyle string) { - _ = graph.AddSubGraph("G", "cluster_legend", nil) +func addLegend(graph *gographviz.Escape, edgestyle string) error { + var graphErr error + set := func(err error) { + if graphErr == nil { + graphErr = err + } + } - _ = graph.AddNode("cluster_legend", "key", + set(graph.AddSubGraph("G", "cluster_legend", nil)) + + set(graph.AddNode("cluster_legend", "key", map[string]string{ "shape": "plaintext", "fontname": "monospace", @@ -198,8 +301,8 @@ func addLegend(graph *gographviz.Escape, edgestyle string) { RUN --mount=(.*)from=...  >`, }, - ) - _ = graph.AddNode("cluster_legend", "key2", + )) + set(graph.AddNode("cluster_legend", "key2", map[string]string{ "shape": "plaintext", "fontname": "monospace", @@ -210,27 +313,29 @@ func addLegend(graph *gographviz.Escape, edgestyle string) {   >`, }, - ) + )) - _ = graph.AddPortEdge("key", "i0:e", "key2", "i0:w", true, nil) + set(graph.AddPortEdge("key", "i0:e", "key2", "i0:w", true, nil)) copyEdgeAttrs := map[string]string{"arrowhead": "empty"} if edgestyle == "default" { copyEdgeAttrs["style"] = "dashed" } - _ = graph.AddPortEdge( + set(graph.AddPortEdge( "key", "i1:e", "key2", "i1:w", true, copyEdgeAttrs, - ) + )) mountEdgeAttrs := map[string]string{"arrowhead": "ediamond"} if edgestyle == "default" { mountEdgeAttrs["style"] = "dotted" } - _ = graph.AddPortEdge( + set(graph.AddPortEdge( "key", "i2:e", "key2", "i2:w", true, mountEdgeAttrs, - ) + )) + + return graphErr } func getStageLabel(stageIndex int, stage Stage, maxLabelLength int) string { @@ -251,22 +356,19 @@ func getStageLabel(stageIndex int, stage Stage, maxLabelLength int) string { // name or the external image name. func getWaitForNodeID( simplifiedDockerfile SimplifiedDockerfile, nameOrID string, layers bool, -) (nodeID string, attrs map[string]string) { - attrs = map[string]string{} +) (string, map[string]string, error) { + attrs := map[string]string{} // If it can be converted to an integer, it's a stage ID if stageIndex, convertErr := strconv.Atoi(nameOrID); convertErr == nil { if layers { // Return the last layer of the stage - nodeID = fmt.Sprintf( + return fmt.Sprintf( "stage_%d_layer_%d", stageIndex, len(simplifiedDockerfile.Stages[stageIndex].Layers)-1, - ) - attrs["ltail"] = fmt.Sprintf("cluster_stage_%d", stageIndex) - } else { - nodeID = fmt.Sprintf("stage_%d", stageIndex) + ), map[string]string{"ltail": fmt.Sprintf("cluster_stage_%d", stageIndex)}, nil } - return + return fmt.Sprintf("stage_%d", stageIndex), attrs, nil } // Check if it's a stage name @@ -274,25 +376,21 @@ func getWaitForNodeID( if nameOrID == stage.Name { if layers { // Return the last layer of the stage - nodeID = fmt.Sprintf( + return fmt.Sprintf( "stage_%d_layer_%d", stageIndex, len(simplifiedDockerfile.Stages[stageIndex].Layers)-1, - ) - attrs["ltail"] = fmt.Sprintf("cluster_stage_%d", stageIndex) - } else { - nodeID = fmt.Sprintf("stage_%d", stageIndex) + ), map[string]string{"ltail": fmt.Sprintf("cluster_stage_%d", stageIndex)}, nil } - return + return fmt.Sprintf("stage_%d", stageIndex), attrs, nil } } // Check if it's an external image ID for externalImageIndex, externalImage := range simplifiedDockerfile.ExternalImages { if nameOrID == externalImage.ID { - nodeID = fmt.Sprintf("external_image_%d", externalImageIndex) - return + return fmt.Sprintf("external_image_%d", externalImageIndex), attrs, nil } } - panic("Could not find node ID for " + nameOrID) + return "", nil, fmt.Errorf("could not find node ID for %s", nameOrID) } diff --git a/internal/dockerfile2dot/build_test.go b/internal/dockerfile2dot/build_test.go index 31ad3db5..80da3c2a 100644 --- a/internal/dockerfile2dot/build_test.go +++ b/internal/dockerfile2dot/build_test.go @@ -141,7 +141,7 @@ func TestBuildDotFile(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := BuildDotFile( + got, err := BuildDotFile( tt.args.simplifiedDockerfile, tt.args.concentrate, tt.args.edgestyle, @@ -150,7 +150,11 @@ func TestBuildDotFile(t *testing.T) { tt.args.maxLabelLength, tt.args.nodesep, tt.args.ranksep, - ); !strings.Contains(got, tt.wantContains) { + ) + if err != nil { + t.Fatalf("BuildDotFile() error = %v", err) + } + if !strings.Contains(got, tt.wantContains) { t.Errorf( "BuildDotFile() = %v, did not contain %v", got, tt.wantContains, ) diff --git a/internal/dockerfile2dot/load.go b/internal/dockerfile2dot/load.go index 44845887..e7dde907 100644 --- a/internal/dockerfile2dot/load.go +++ b/internal/dockerfile2dot/load.go @@ -4,6 +4,7 @@ package dockerfile2dot import ( "errors" + "fmt" "io/fs" "path/filepath" @@ -21,13 +22,13 @@ func LoadAndParseDockerfile( content, err := afero.ReadFile(inputFS, filename) if err != nil { if errors.Is(err, fs.ErrNotExist) { - absFilePath, err := filepath.Abs(filename) - if err != nil { - panic(err) + absFilePath, absErr := filepath.Abs(filename) + if absErr != nil { + return SimplifiedDockerfile{}, fmt.Errorf("could not get absolute path: %w", absErr) } return SimplifiedDockerfile{}, errors.New("could not find a Dockerfile at " + absFilePath) } - panic(err) + return SimplifiedDockerfile{}, err } return dockerfileToSimplifiedDockerfile(content, maxLabelLength, scratchMode) } From e598cb95230675849b23c080e01fcc3c41f0aceb Mon Sep 17 00:00:00 2001 From: Patrick Hoefler Date: Thu, 12 Mar 2026 21:09:33 +0100 Subject: [PATCH 2/6] fix: add bounds checks in getWaitForNodeID and error-path tests - Validate numeric stage index is in range before indexing Stages slice - Validate stage has at least one layer before using Layers[-1] when layers mode is enabled; same check for stage-name resolution path - Add TestBuildDotFileErrors covering: unresolvable WaitFor ID, out-of-range numeric stage reference (flat and layers mode) Co-Authored-By: Claude Sonnet 4.6 --- internal/dockerfile2dot/build.go | 12 ++++++ internal/dockerfile2dot/build_test.go | 62 +++++++++++++++++++++++++++ 2 files changed, 74 insertions(+) diff --git a/internal/dockerfile2dot/build.go b/internal/dockerfile2dot/build.go index 164a6767..78309ef9 100644 --- a/internal/dockerfile2dot/build.go +++ b/internal/dockerfile2dot/build.go @@ -361,7 +361,16 @@ func getWaitForNodeID( // If it can be converted to an integer, it's a stage ID if stageIndex, convertErr := strconv.Atoi(nameOrID); convertErr == nil { + if stageIndex < 0 || stageIndex >= len(simplifiedDockerfile.Stages) { + return "", nil, fmt.Errorf( + "stage index %d out of range (have %d stages)", + 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", @@ -375,6 +384,9 @@ func getWaitForNodeID( 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", diff --git a/internal/dockerfile2dot/build_test.go b/internal/dockerfile2dot/build_test.go index 80da3c2a..02913037 100644 --- a/internal/dockerfile2dot/build_test.go +++ b/internal/dockerfile2dot/build_test.go @@ -5,6 +5,68 @@ import ( "testing" ) +func TestBuildDotFileErrors(t *testing.T) { + tests := []struct { + name string + simplifiedDockerfile SimplifiedDockerfile + layers bool + }{ + { + name: "unresolvable WaitFor ID returns error", + simplifiedDockerfile: SimplifiedDockerfile{ + Stages: []Stage{{ + Layers: []Layer{{ + Label: "FROM scratch", + WaitFors: []WaitFor{{ + ID: "nonexistent", + Type: waitForType(waitForFrom), + }}, + }}, + }}, + }, + }, + { + name: "out-of-range numeric stage reference returns error", + simplifiedDockerfile: SimplifiedDockerfile{ + Stages: []Stage{{ + Layers: []Layer{{ + Label: "FROM ...", + WaitFors: []WaitFor{{ + ID: "99", + Type: waitForType(waitForFrom), + }}, + }}, + }}, + }, + }, + { + name: "out-of-range numeric stage reference with layers returns error", + simplifiedDockerfile: SimplifiedDockerfile{ + Stages: []Stage{{ + Layers: []Layer{{ + Label: "FROM ...", + WaitFors: []WaitFor{{ + ID: "99", + Type: waitForType(waitForFrom), + }}, + }}, + }}, + }, + layers: true, + }, + } + 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", + ) + if err == nil { + t.Error("BuildDotFile() expected an error, got nil") + } + }) + } +} + func TestBuildDotFile(t *testing.T) { type args struct { simplifiedDockerfile SimplifiedDockerfile From 438110d620cc76028a8ed29264feded53301082a Mon Sep 17 00:00:00 2001 From: Patrick Hoefler Date: Thu, 12 Mar 2026 21:18:38 +0100 Subject: [PATCH 3/6] fix: quote DOT labels for before-first-stage nodes consistently - arg.Label values come from Dockerfile ARG instructions and can contain spaces, '=', and other non-identifier characters; wrap them in explicit DOT quotes ("...") the same way stage layer labels are handled elsewhere in the file - Quote the "Before First Stage" cluster subgraph label explicitly for consistency; gographviz.Escape would auto-quote it anyway but being explicit matches the pattern used for other multi-word labels - Cluster stage labels (getStageLabel) are left to gographviz.Escape since it correctly quotes names with hyphens while leaving plain identifiers unquoted, matching the existing golden test output Co-Authored-By: Claude Sonnet 4.6 --- internal/dockerfile2dot/build.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/dockerfile2dot/build.go b/internal/dockerfile2dot/build.go index 78309ef9..30f6eada 100644 --- a/internal/dockerfile2dot/build.go +++ b/internal/dockerfile2dot/build.go @@ -222,14 +222,14 @@ func addBeforeFirstStage( set(graph.AddSubGraph( "G", "cluster_before_first_stage", - map[string]string{"label": "Before First Stage"}, + map[string]string{"label": "\"Before First Stage\""}, )) for argIndex, arg := range simplifiedDockerfile.BeforeFirstStage { set(graph.AddNode( "cluster_before_first_stage", fmt.Sprintf("before_first_stage_%d", argIndex), map[string]string{ - "label": arg.Label, + "label": "\"" + arg.Label + "\"", "shape": "box", "style": "rounded", "width": "2", From 138ae35a55c4fbd2a2d668c8c6f9c871930a8d8e Mon Sep 17 00:00:00 2001 From: Patrick Hoefler Date: Thu, 12 Mar 2026 21:25:10 +0100 Subject: [PATCH 4/6] fix: make addStageWithLayers own its error collection and improve error message - Remove the set func(error) parameter from addStageWithLayers; give it its own var/set pair and return graphErr instead of always nil, making the error propagation explicit and the if err != nil check meaningful - Use %q and add context to the unresolvable node ID error message so callers can distinguish whitespace/special-char identifiers and know what was expected Co-Authored-By: Claude Sonnet 4.6 --- internal/dockerfile2dot/build.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/internal/dockerfile2dot/build.go b/internal/dockerfile2dot/build.go index 30f6eada..9407e61f 100644 --- a/internal/dockerfile2dot/build.go +++ b/internal/dockerfile2dot/build.go @@ -132,7 +132,7 @@ func addStages( // Add layers if requested if layers { - if err := addStageWithLayers(graph, set, simplifiedDockerfile, stageIndex, stage, attrs); err != nil { + if err := addStageWithLayers(graph, simplifiedDockerfile, stageIndex, stage, attrs); err != nil { return err } } else { @@ -159,12 +159,18 @@ func addStages( func addStageWithLayers( graph *gographviz.Escape, - set func(error), simplifiedDockerfile SimplifiedDockerfile, stageIndex int, stage Stage, attrs map[string]string, ) error { + var graphErr error + set := func(err error) { + if graphErr == nil { + graphErr = err + } + } + cluster := fmt.Sprintf("cluster_stage_%d", stageIndex) clusterAttrs := map[string]string{ @@ -201,7 +207,7 @@ func addStageWithLayers( } } - return nil + return graphErr } func addBeforeFirstStage( @@ -404,5 +410,8 @@ func getWaitForNodeID( } } - return "", nil, fmt.Errorf("could not find node ID for %s", nameOrID) + return "", nil, fmt.Errorf( + "could not resolve node ID for %q (expected stage index, stage name, or external image ID)", + nameOrID, + ) } From 31049edffe9dd35e241a85ada2b67f3123b37c05 Mon Sep 17 00:00:00 2001 From: Patrick Hoefler Date: Thu, 12 Mar 2026 21:30:55 +0100 Subject: [PATCH 5/6] fix: surface earliest error consistently across BuildDotFile helpers - Check graphErr before each helper call in BuildDotFile so that a SetName/AddAttr failure is not masked by a later helper's error - Check graphErr before addEdgesForStage in addStages so that an earlier node/subgraph error is returned first - Include the input filename in the filepath.Abs error message so users can see which path failed to resolve Co-Authored-By: Claude Sonnet 4.6 --- internal/dockerfile2dot/build.go | 12 ++++++++---- internal/dockerfile2dot/load.go | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/internal/dockerfile2dot/build.go b/internal/dockerfile2dot/build.go index 9407e61f..c7a1e241 100644 --- a/internal/dockerfile2dot/build.go +++ b/internal/dockerfile2dot/build.go @@ -40,6 +40,10 @@ func BuildDotFile( set(graph.AddAttr("G", "concentrate", "true")) } + if graphErr != nil { + return "", graphErr + } + // Add the legend if requested if legend { if err := addLegend(graph, edgestyle); err != nil { @@ -62,10 +66,6 @@ func BuildDotFile( } } - if graphErr != nil { - return "", graphErr - } - return graph.String(), nil } @@ -146,6 +146,10 @@ func addStages( set(graph.AddNode("G", fmt.Sprintf("stage_%d", stageIndex), attrs)) } + if graphErr != nil { + return graphErr + } + // Add the edges for this build stage if err := addEdgesForStage( stageIndex, stage, graph, simplifiedDockerfile, layers, edgestyle, diff --git a/internal/dockerfile2dot/load.go b/internal/dockerfile2dot/load.go index e7dde907..974ddaa0 100644 --- a/internal/dockerfile2dot/load.go +++ b/internal/dockerfile2dot/load.go @@ -24,7 +24,7 @@ func LoadAndParseDockerfile( if errors.Is(err, fs.ErrNotExist) { absFilePath, absErr := filepath.Abs(filename) if absErr != nil { - return SimplifiedDockerfile{}, fmt.Errorf("could not get absolute path: %w", absErr) + return SimplifiedDockerfile{}, fmt.Errorf("could not get absolute path for %q: %w", filename, absErr) } return SimplifiedDockerfile{}, errors.New("could not find a Dockerfile at " + absFilePath) } From df2b1a73f68fc8aedf856fee45e045eae2c41f11 Mon Sep 17 00:00:00 2001 From: Patrick Hoefler Date: Thu, 12 Mar 2026 21:37:59 +0100 Subject: [PATCH 6/6] fix: quote cluster stage labels explicitly in addStageWithLayers Stage names can contain hyphens, dots, or other non-identifier characters that require quoting in DOT format. Apply the same "\"...\"" pattern used for node labels throughout the file so the quoting is explicit and not reliant on gographviz auto-escaping. Update the raw DOT expectations in the layers flag test cases to reflect the new quoted form (label="ubuntu", label="release"). Canon test expectations are unchanged because graphviz strips redundant quotes from simple identifiers when normalising to canonical format. Co-Authored-By: Claude Sonnet 4.6 --- internal/cmd/root_test.go | 4 ++-- internal/dockerfile2dot/build.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/cmd/root_test.go b/internal/cmd/root_test.go index ff36ef89..ca1ab812 100644 --- a/internal/cmd/root_test.go +++ b/internal/cmd/root_test.go @@ -174,7 +174,7 @@ It creates a visual graph representation of the build process. stage_0_layer_1->stage_2_layer_1[ arrowhead=empty, ltail=cluster_stage_0, style=dashed ]; stage_1_layer_1->stage_2_layer_2[ arrowhead=empty, ltail=cluster_stage_1, style=dashed ]; subgraph cluster_stage_0 { - label=ubuntu; + label="ubuntu"; margin=16; stage_0_layer_0 [ fillcolor=white, label="FROM ubuntu:lates...", penwidth=0.5, shape=box, style="filled,rounded", width=2 ]; stage_0_layer_1 [ fillcolor=white, label="RUN apt-get updat...", penwidth=0.5, shape=box, style="filled,rounded", width=2 ]; @@ -191,7 +191,7 @@ It creates a visual graph representation of the build process. ; subgraph cluster_stage_2 { fillcolor=grey90; - label=release; + label="release"; margin=16; style=filled; stage_2_layer_0 [ fillcolor=white, label="FROM scratch AS r...", penwidth=0.5, shape=box, style="filled,rounded", width=2 ]; diff --git a/internal/dockerfile2dot/build.go b/internal/dockerfile2dot/build.go index c7a1e241..d4c14681 100644 --- a/internal/dockerfile2dot/build.go +++ b/internal/dockerfile2dot/build.go @@ -178,7 +178,7 @@ func addStageWithLayers( cluster := fmt.Sprintf("cluster_stage_%d", stageIndex) clusterAttrs := map[string]string{ - "label": getStageLabel(stageIndex, stage, 0), + "label": "\"" + getStageLabel(stageIndex, stage, 0) + "\"", "margin": "16", }