Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
@@ -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
2 changes: 2 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
```
Expand Down
33 changes: 21 additions & 12 deletions internal/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -67,6 +68,7 @@ It creates a visual graph representation of the build process.`,
inputFS,
filenameFlag,
int(maxLabelLengthFlag),
separateScratchFlag,
)
if err != nil {
return
Expand Down Expand Up @@ -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",
Expand Down
23 changes: 23 additions & 0 deletions internal/cmd/root_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
`
Expand Down Expand Up @@ -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 ];

}
`,
},
}
Expand Down
6 changes: 3 additions & 3 deletions internal/dockerfile2dot/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
59 changes: 53 additions & 6 deletions internal/dockerfile2dot/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,16 @@ func TestBuildDotFile(t *testing.T) {
},
},
ExternalImages: []ExternalImage{
{Name: "build"},
{Name: "release"},
{ID: "build", Name: "build"},
{ID: "release", Name: "release"},
},
Stages: []Stage{
{
Layers: []Layer{
{
Label: "FROM...",
WaitFors: []WaitFor{{
Name: "build",
ID: "build",
Type: waitForType(waitForFrom),
}},
},
Expand All @@ -66,16 +66,16 @@ func TestBuildDotFile(t *testing.T) {
},
},
ExternalImages: []ExternalImage{
{Name: "build"},
{Name: "release"},
{ID: "build", Name: "build"},
{ID: "release", Name: "release"},
},
Stages: []Stage{
{
Layers: []Layer{
{
Label: "FROM...",
WaitFors: []WaitFor{{
Name: "build",
ID: "build",
Type: waitForType(waitForFrom),
}},
},
Expand All @@ -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) {
Expand Down
52 changes: 38 additions & 14 deletions internal/dockerfile2dot/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package dockerfile2dot

import (
"bytes"
"fmt"
"regexp"
"strings"

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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),
}}

Expand All @@ -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),
}}
}
Expand All @@ -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),
})
}
Expand Down Expand Up @@ -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,
) {
for _, stage := range simplifiedDockerfile.Stages {
for _, layer := range stage.Layers {
for _, waitFor := range layer.WaitFors {
// Counter to generate unique IDs for separate scratch instances
scratchCounter := 0

// Check if the layer waits for a stage
if _, ok := stages[waitFor.Name]; ok {
// 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 {

// 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
simplifiedDockerfile.Stages[stageIndex].Layers[layerIndex].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
}
Expand All @@ -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
},
)
}
}
Expand Down
Loading
Loading