diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 2cd2b01..52d9c9a 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/cmd/root_test.go b/internal/cmd/root_test.go index ff36ef8..ca1ab81 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 a708eff..d4c1468 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")) + } + + if graphErr != nil { + return "", graphErr } // 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 + } + } + + 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, simplifiedDockerfile, stageIndex, stage, attrs); err != nil { + return err } } else { // Add the build stages. @@ -115,45 +143,117 @@ func BuildDotFile( attrs["fillcolor"] = "grey90" } - _ = graph.AddNode("G", fmt.Sprintf("stage_%d", stageIndex), attrs) + set(graph.AddNode("G", fmt.Sprintf("stage_%d", stageIndex), attrs)) + } + + if graphErr != nil { + return graphErr } - // 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, + 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{ + "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 graphErr +} + +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 +269,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 +282,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 + } + } + + set(graph.AddSubGraph("G", "cluster_legend", nil)) - _ = graph.AddNode("cluster_legend", "key", + set(graph.AddNode("cluster_legend", "key", map[string]string{ "shape": "plaintext", "fontname": "monospace", @@ -198,8 +311,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 +323,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,48 +366,56 @@ 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 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 - 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 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 - 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 resolve node ID for %q (expected stage index, stage name, or external image ID)", + nameOrID, + ) } diff --git a/internal/dockerfile2dot/build_test.go b/internal/dockerfile2dot/build_test.go index 31ad3db..0291303 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 @@ -141,7 +203,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 +212,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 4484588..974ddaa 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 for %q: %w", filename, absErr) } return SimplifiedDockerfile{}, errors.New("could not find a Dockerfile at " + absFilePath) } - panic(err) + return SimplifiedDockerfile{}, err } return dockerfileToSimplifiedDockerfile(content, maxLabelLength, scratchMode) }