Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: GenerateDAG: prevent using duplicate name labels in Dockerfiles #523

Merged
merged 1 commit into from
Apr 30, 2024
Merged

fix: GenerateDAG: prevent using duplicate name labels in Dockerfiles #523

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions pkg/dib/generate_dag.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,11 @@ func GenerateDAG(buildPath, registryPrefix, customHashListPath string, buildArgs
}
img.IgnorePatterns = ignorePatterns

if n, ok := cache[img.Name]; ok {
return fmt.Errorf("duplicate image name %q found while reading file %q: previous file was %q",
img.Name, filePath, path.Join(n.Image.Dockerfile.ContextPath, n.Image.Dockerfile.Filename))
}

allParents[img.Name] = dckfile.From
cache[img.Name] = dag.NewNode(img)
}
Expand Down
64 changes: 34 additions & 30 deletions pkg/dib/generate_dag_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package dib_test

import (
"fmt"
"os"
"os/exec"
"path"
Expand All @@ -12,13 +13,16 @@ import (
"github.com/stretchr/testify/require"
)

const fixtureDir = "../../test/fixtures/docker"
const (
buildPath1 = "../../test/fixtures/docker"
buildPath2 = "../../test/fixtures/docker-duplicates"
registryPrefix = "eu.gcr.io/my-test-repository"
)

//nolint:paralleltest
func TestGenerateDAG(t *testing.T) {
t.Run("basic tests", func(t *testing.T) {
graph, err := dib.GenerateDAG(fixtureDir,
"eu.gcr.io/my-test-repository", "", map[string]string{})
graph, err := dib.GenerateDAG(buildPath1, registryPrefix, "", nil)
require.NoError(t, err)

nodes := flattenNodes(graph)
Expand All @@ -27,7 +31,7 @@ func TestGenerateDAG(t *testing.T) {
multistageNode := nodes["multistage"]

rootImage := rootNode.Image
assert.Equal(t, "eu.gcr.io/my-test-repository/bullseye", rootImage.Name)
assert.Equal(t, path.Join(registryPrefix, "bullseye"), rootImage.Name)
assert.Equal(t, "bullseye", rootImage.ShortName)
assert.Empty(t, rootNode.Parents())
assert.Len(t, rootNode.Children(), 3)
Expand All @@ -37,10 +41,9 @@ func TestGenerateDAG(t *testing.T) {
})

t.Run("modifying the root node should change all hashes", func(t *testing.T) {
tmpDir := copyFixtures(t)
buildPath := copyFixtures(t, buildPath1)

graph0, err := dib.GenerateDAG(tmpDir,
"eu.gcr.io/my-test-repository", "", map[string]string{})
graph0, err := dib.GenerateDAG(buildPath, registryPrefix, "", nil)
require.NoError(t, err)

nodes0 := flattenNodes(graph0)
Expand All @@ -50,13 +53,12 @@ func TestGenerateDAG(t *testing.T) {

// When I add a new file in bullseye/ (root node)
require.NoError(t, os.WriteFile(
path.Join(tmpDir, "bullseye/newfile"),
path.Join(buildPath, "bullseye/newfile"),
[]byte("any content"),
os.ModePerm))

// Then ONLY the hash of the child node bullseye/multistage should have changed
graph1, err := dib.GenerateDAG(tmpDir,
"eu.gcr.io/my-test-repository", "", map[string]string{})
graph1, err := dib.GenerateDAG(buildPath, registryPrefix, "", nil)
require.NoError(t, err)

nodes1 := flattenNodes(graph1)
Expand All @@ -70,10 +72,9 @@ func TestGenerateDAG(t *testing.T) {
})

t.Run("modifying a child node should change only its hash", func(t *testing.T) {
tmpDir := copyFixtures(t)
buildPath := copyFixtures(t, buildPath1)

graph0, err := dib.GenerateDAG(tmpDir,
"eu.gcr.io/my-test-repository", "", map[string]string{})
graph0, err := dib.GenerateDAG(buildPath, registryPrefix, "", nil)
require.NoError(t, err)

nodes0 := flattenNodes(graph0)
Expand All @@ -83,13 +84,12 @@ func TestGenerateDAG(t *testing.T) {

// When I add a new file in bullseye/multistage/ (child node)
require.NoError(t, os.WriteFile(
path.Join(tmpDir, "bullseye/multistage/newfile"),
path.Join(buildPath, "bullseye/multistage/newfile"),
[]byte("file contents"),
os.ModePerm))

// Then ONLY the hash of the child node bullseye/multistage should have changed
graph1, err := dib.GenerateDAG(tmpDir,
"eu.gcr.io/my-test-repository", "", map[string]string{})
graph1, err := dib.GenerateDAG(buildPath, registryPrefix, "", nil)
require.NoError(t, err)

nodes1 := flattenNodes(graph1)
Expand All @@ -103,14 +103,11 @@ func TestGenerateDAG(t *testing.T) {
})

t.Run("using custom hash list should change only hashes of nodes with custom label", func(t *testing.T) {
graph0, err := dib.GenerateDAG(fixtureDir,
"eu.gcr.io/my-test-repository", "", map[string]string{})
graph0, err := dib.GenerateDAG(buildPath1, registryPrefix, "", nil)
require.NoError(t, err)

graph1, err := dib.GenerateDAG(fixtureDir,
"eu.gcr.io/my-test-repository",
"../../test/fixtures/dib/valid_wordlist.txt",
map[string]string{})
graph1, err := dib.GenerateDAG(buildPath1, registryPrefix,
"../../test/fixtures/dib/valid_wordlist.txt", nil)
require.NoError(t, err)

nodes0 := flattenNodes(graph0)
Expand All @@ -126,13 +123,10 @@ func TestGenerateDAG(t *testing.T) {
})

t.Run("using arg used in root node should change all hashes", func(t *testing.T) {
graph0, err := dib.GenerateDAG(fixtureDir,
"eu.gcr.io/my-test-repository", "",
map[string]string{})
graph0, err := dib.GenerateDAG(buildPath1, registryPrefix, "", nil)
require.NoError(t, err)

graph1, err := dib.GenerateDAG(fixtureDir,
"eu.gcr.io/my-test-repository", "",
graph1, err := dib.GenerateDAG(buildPath1, registryPrefix, "",
map[string]string{
"HELLO": "world",
})
Expand All @@ -145,14 +139,24 @@ func TestGenerateDAG(t *testing.T) {

assert.NotEqual(t, rootNode1.Image.Hash, rootNode0.Image.Hash)
})

t.Run("duplicates", func(t *testing.T) {
graph, err := dib.GenerateDAG(buildPath2, registryPrefix, "", nil)
require.Error(t, err)
require.Nil(t, graph)
require.EqualError(t, err,
fmt.Sprintf(
"duplicate image name \"%s/duplicate\" found while reading file \"%s/bullseye/duplicate2/Dockerfile\": previous file was \"%s/bullseye/duplicate1/Dockerfile\"", //nolint:lll
registryPrefix, buildPath2, buildPath2))
})
}

// copyFixtures copies the directory fixtureDir into a temporary one to be free to edit files.
func copyFixtures(t *testing.T) string {
// copyFixtures copies the buildPath directory into a temporary one to be free to edit files.
func copyFixtures(t *testing.T, buildPath string) string {
t.Helper()
cwd, err := os.Getwd()
require.NoError(t, err)
src := path.Join(cwd, fixtureDir)
src := path.Join(cwd, buildPath)
dest := t.TempDir()
cmd := exec.Command("cp", "-r", src, dest)
require.NoError(t, cmd.Run())
Expand Down
8 changes: 8 additions & 0 deletions test/fixtures/docker-duplicates/bullseye/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
FROM debian:bullseye

LABEL name="bullseye"
LABEL version="v1"

ARG HELLO="there"

RUN echo "Hello $HELLO"
5 changes: 5 additions & 0 deletions test/fixtures/docker-duplicates/bullseye/duplicate1/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
FROM eu.gcr.io/my-test-repository/bullseye:v1

LABEL name="duplicate"
LABEL version="v1"
LABEL dib.use-custom-hash-list="true"
5 changes: 5 additions & 0 deletions test/fixtures/docker-duplicates/bullseye/duplicate2/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
FROM eu.gcr.io/my-test-repository/bullseye:v1

LABEL name="duplicate"
LABEL version="v2"
LABEL dib.use-custom-hash-list="true"