fix: GenerateDAG: new tests

pkg/dib/generate_dag.go

@@ -28,6 +28,15 @@ const (
 // GenerateDAG discovers and parses all Dockerfiles at a given path,
 // and generates the DAG representing the relationships between images.
 func GenerateDAG(buildPath, registryPrefix, customHashListPath string, buildArgs map[string]string) (*dag.DAG, error) {
+	graph, err := buildGraph(buildPath, registryPrefix)
+	if err != nil {
+		return nil, err
+	}
+
+	return computeHashes(graph, customHashListPath, buildArgs)
+}
+
+func buildGraph(buildPath, registryPrefix string) (*dag.DAG, error) {
 	var allFiles []string
 	cache := make(map[string]*dag.Node)
 	allParents := make(map[string][]dockerfile.ImageRef)
@@ -121,19 +130,6 @@ func GenerateDAG(buildPath, registryPrefix, customHashListPath string, buildArgs
 		}
 	}
 
-	if err := generateHashes(graph, allFiles, customHashListPath, buildArgs); err != nil {
-		return nil, err
-	}
-
-	return graph, nil
-}
-
-func generateHashes(graph *dag.DAG, allFiles []string, customHashListPath string, buildArgs map[string]string) error {
-	customHumanizedHashList, err := LoadCustomHashList(customHashListPath)
-	if err != nil {
-		return fmt.Errorf("could not load custom humanized hash list: %w", err)
-	}
-
 	fileBelongsTo := map[string]*dag.Node{}
 	for _, file := range allFiles {
 		fileBelongsTo[file] = nil
@@ -173,6 +169,15 @@ func generateHashes(graph *dag.DAG, allFiles []string, customHashListPath string
 		}
 	})
 
+	return graph, nil
+}
+
+func computeHashes(graph *dag.DAG, customHashListPath string, buildArgs map[string]string) (*dag.DAG, error) {
+	customHumanizedHashList, err := LoadCustomHashList(customHashListPath)
+	if err != nil {
+		return nil, fmt.Errorf("could not load custom humanized hash list: %w", err)
+	}
+
 	for {
 		needRepass := false
 		err := graph.WalkErr(func(node *dag.Node) error {
@@ -221,10 +226,10 @@ func generateHashes(graph *dag.DAG, allFiles []string, customHashListPath string
 			return nil
 		})
 		if err != nil {
-			return err
+			return nil, err
 		}
 		if !needRepass {
-			return nil
+			return graph, nil
 		}
 	}
 }

pkg/dib/generate_dag_internal_test.go

@@ -0,0 +1,59 @@
+package dib
+
+import (
+	"testing"
+
+	"github.com/davecgh/go-spew/spew"
+	"github.com/radiofrance/dib/pkg/dag"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+const (
+	buildPath      = "../../test/fixtures/dag"
+	registryPrefix = "eu.gcr.io/my-test-repository"
+)
+
+func Test_buildGraph(t *testing.T) {
+	t.Parallel()
+
+	graph, err := buildGraph(buildPath, registryPrefix)
+	require.NoError(t, err)
+	graph.WalkInDepth(func(node *dag.Node) {
+		switch node.Image.ShortName {
+		case "root1":
+			require.Len(t, node.Files, 9, spew.Sdump(node.Files))
+			assert.Contains(t, node.Files, buildPath+"/root1/Dockerfile")
+			assert.Contains(t, node.Files, buildPath+"/root1/custom-hash-list/Dockerfile")
+			assert.Contains(t, node.Files, buildPath+"/root1/dockerignore/.dockerignore")
+			assert.Contains(t, node.Files, buildPath+"/root1/dockerignore/Dockerfile")
+			assert.Contains(t, node.Files, buildPath+"/root1/dockerignore/ignored.txt")
+			assert.Contains(t, node.Files, buildPath+"/root1/multistage/Dockerfile")
+			assert.Contains(t, node.Files, buildPath+"/root1/skipbuild/Dockerfile")
+			assert.Contains(t, node.Files, buildPath+"/root1/with-a-file/Dockerfile")
+			assert.Contains(t, node.Files, buildPath+"/root1/with-a-file/included.txt")
+		case "custom-hash-list":
+			require.Len(t, node.Files, 1, spew.Sdump(node.Files))
+			assert.Contains(t, node.Files, buildPath+"/root1/custom-hash-list/Dockerfile")
+		case "dockerignore":
+			require.Len(t, node.Files, 1, spew.Sdump(node.Files))
+			assert.Contains(t, node.Files, buildPath+"/root1/dockerignore/Dockerfile")
+		case "multistage":
+			require.Len(t, node.Files, 1, spew.Sdump(node.Files))
+			assert.Contains(t, node.Files, buildPath+"/root1/multistage/Dockerfile")
+		case "with-a-file":
+			require.Len(t, node.Files, 2, spew.Sdump(node.Files))
+			assert.Contains(t, node.Files, buildPath+"/root1/with-a-file/Dockerfile")
+			assert.Contains(t, node.Files, buildPath+"/root1/with-a-file/included.txt")
+		case "root2":
+			require.Len(t, node.Files, 2, spew.Sdump(node.Files))
+			assert.Contains(t, node.Files, buildPath+"/root2/Dockerfile")
+			assert.Contains(t, node.Files, buildPath+"/root2/root3/Dockerfile")
+		case "root3":
+			require.Len(t, node.Files, 1, spew.Sdump(node.Files))
+			assert.Contains(t, node.Files, buildPath+"/root2/root3/Dockerfile")
+		default:
+			t.Errorf("unexpected image: %s", node.Image.ShortName)
+		}
+	})
+}

pkg/dib/generate_dag_test.go

@@ -1,3 +1,4 @@
+//nolint:paralleltest
 package dib_test
 
 import (
@@ -14,97 +15,106 @@ import (
 )
 
 const (
-	buildPath1     = "../../test/fixtures/docker"
-	buildPath2     = "../../test/fixtures/docker-duplicates"
+	buildPath1     = "../../test/fixtures/dag"
+	buildPath2     = "../../test/fixtures/dag-with-duplicates"
 	registryPrefix = "eu.gcr.io/my-test-repository"
 )
 
-//nolint:paralleltest,dupl
 func TestGenerateDAG(t *testing.T) {
+	baseDir := path.Join(buildPath1, "root1")
+	root1Files := []string{
+		path.Join(baseDir, "Dockerfile"),
+		path.Join(baseDir, "custom-hash-list", "Dockerfile"),
+		path.Join(baseDir, "dockerignore", ".dockerignore"),
+		path.Join(baseDir, "dockerignore", "Dockerfile"),
+		path.Join(baseDir, "dockerignore", "ignored.txt"),
+		path.Join(baseDir, "multistage", "Dockerfile"),
+		path.Join(baseDir, "skipbuild", "Dockerfile"),
+		path.Join(baseDir, "with-a-file", "Dockerfile"),
+		path.Join(baseDir, "with-a-file", "included.txt"),
+	}
+	root1Hash, err := dib.HashFiles(baseDir, root1Files, nil, nil)
+	require.NoError(t, err)
+
 	t.Run("basic tests", func(t *testing.T) {
-		graph, err := dib.GenerateDAG(buildPath1, registryPrefix, "", nil)
+		buildPath := copyFixtures(t, buildPath1)
+
+		graph, err := dib.GenerateDAG(buildPath, registryPrefix, "", nil)
 		require.NoError(t, err)
 
 		nodes := flattenNodes(graph)
-		rootNode := nodes["bullseye"]
-		subNode := nodes["sub-image"]
+		rootNode := nodes["root1"]
 		multistageNode := nodes["multistage"]
 
 		rootImage := rootNode.Image
-		assert.Equal(t, path.Join(registryPrefix, "bullseye"), rootImage.Name)
-		assert.Equal(t, "bullseye", rootImage.ShortName)
+		assert.Equal(t, path.Join(registryPrefix, "root1"), rootImage.Name)
+		assert.Equal(t, "root1", rootImage.ShortName)
 		assert.Empty(t, rootNode.Parents())
-		assert.Len(t, rootNode.Children(), 3)
-		assert.Len(t, subNode.Parents(), 1)
+		assert.Len(t, rootNode.Children(), 4)
 		assert.Len(t, multistageNode.Parents(), 1)
-		assert.Equal(t, []string{"latest"}, multistageNode.Image.ExtraTags)
 	})
 
-	t.Run("modifying the root node should change all hashes", func(t *testing.T) {
+	t.Run("modifying root1 node", func(t *testing.T) {
 		buildPath := copyFixtures(t, buildPath1)
 
 		graph0, err := dib.GenerateDAG(buildPath, registryPrefix, "", nil)
 		require.NoError(t, err)
 
 		nodes0 := flattenNodes(graph0)
-		rootNode0 := nodes0["bullseye"]
-		subNode0 := nodes0["sub-image"]
-		multistageNode0 := nodes0["multistage"]
+		rootNode0 := nodes0["root1"]
+		childNode0 := nodes0["with-a-file"]
 
-		// When I add a new file in bullseye/ (root node)
-		//nolint:gosec
-		require.NoError(t, os.WriteFile(
-			path.Join(buildPath, "bullseye/newfile"),
+		// When I add a new file in the root1 folder
+		require.NoError(t, os.WriteFile( //nolint:gosec
+			path.Join(buildPath, "root1/newfile"),
 			[]byte("any content"),
 			os.ModePerm))
 
-		// Then ONLY the hash of the child node bullseye/multistage should have changed
+		// Then all the hashes of root1 and its children should have changed
 		graph1, err := dib.GenerateDAG(buildPath, registryPrefix, "", nil)
 		require.NoError(t, err)
 
 		nodes1 := flattenNodes(graph1)
-		rootNode1 := nodes1["bullseye"]
-		subNode1 := nodes1["sub-image"]
-		multistageNode1 := nodes1["multistage"]
+		rootNode1 := nodes1["root1"]
+		childNode1 := nodes1["with-a-file"]
 
 		assert.NotEqual(t, rootNode0.Image.Hash, rootNode1.Image.Hash)
-		assert.NotEqual(t, subNode0.Image.Hash, subNode1.Image.Hash)
-		assert.NotEqual(t, multistageNode0.Image.Hash, multistageNode1.Image.Hash)
+		assert.NotEqual(t, childNode0.Image.Hash, childNode1.Image.Hash)
 	})
 
-	t.Run("modifying a child node should change only its hash", func(t *testing.T) {
+	t.Run("modifying a leaf node", func(t *testing.T) {
 		buildPath := copyFixtures(t, buildPath1)
 
 		graph0, err := dib.GenerateDAG(buildPath, registryPrefix, "", nil)
 		require.NoError(t, err)
 
 		nodes0 := flattenNodes(graph0)
-		rootNode0 := nodes0["bullseye"]
-		subNode0 := nodes0["sub-image"]
+		rootNode0 := nodes0["root1"]
+		subNode0 := nodes0["with-a-file"]
 		multistageNode0 := nodes0["multistage"]
 
-		// When I add a new file in bullseye/multistage/ (child node)
-		//nolint:gosec
-		require.NoError(t, os.WriteFile(
-			path.Join(buildPath, "bullseye/multistage/newfile"),
+		// When I add a new file in the root1/multistage folder (leaf node)
+		require.NoError(t, os.WriteFile( //nolint:gosec
+			path.Join(buildPath, "root1/multistage/newfile"),
 			[]byte("file contents"),
 			os.ModePerm))
 
-		// Then ONLY the hash of the child node bullseye/multistage should have changed
+		// Then all the hashes of root1 and its children should have changed,
+		// because the context of root1 has changed (one more file)
 		graph1, err := dib.GenerateDAG(buildPath, registryPrefix, "", nil)
 		require.NoError(t, err)
 
 		nodes1 := flattenNodes(graph1)
-		rootNode1 := nodes1["bullseye"]
-		subNode1 := nodes1["sub-image"]
+		rootNode1 := nodes1["root1"]
+		subNode1 := nodes1["with-a-file"]
 		multistageNode1 := nodes1["multistage"]
 
-		assert.Equal(t, rootNode0.Image.Hash, rootNode1.Image.Hash)
-		assert.Equal(t, subNode0.Image.Hash, subNode1.Image.Hash)
+		assert.NotEqual(t, rootNode0.Image.Hash, rootNode1.Image.Hash)
+		assert.NotEqual(t, subNode0.Image.Hash, subNode1.Image.Hash)
 		assert.NotEqual(t, multistageNode0.Image.Hash, multistageNode1.Image.Hash)
 	})
 
-	t.Run("using custom hash list should change only hashes of nodes with custom label", func(t *testing.T) {
+	t.Run("using custom hash list", func(t *testing.T) {
 		graph0, err := dib.GenerateDAG(buildPath1, registryPrefix, "", nil)
 		require.NoError(t, err)
 
@@ -113,18 +123,34 @@ func TestGenerateDAG(t *testing.T) {
 		require.NoError(t, err)
 
 		nodes0 := flattenNodes(graph0)
-		rootNode0 := nodes0["bullseye"]
-		subNode0 := nodes0["sub-image"]
+		rootNode0 := nodes0["root1"]
+		customNode0 := nodes0["custom-hash-list"]
 		nodes1 := flattenNodes(graph1)
-		rootNode1 := nodes1["bullseye"]
-		subNode1 := nodes1["sub-image"]
+		rootNode1 := nodes1["root1"]
+		customNode1 := nodes1["custom-hash-list"]
+
+		// Recompute hash of custom-hash-list, which is the only node that has the label 'dib.use-custom-hash-list'
+		baseDir = path.Join(buildPath1, "root1", "custom-hash-list")
+		subImageFiles := []string{
+			path.Join(baseDir, "Dockerfile"),
+		}
+
+		oldHash, err := dib.HashFiles(baseDir, subImageFiles, []string{root1Hash}, nil)
+		require.NoError(t, err)
+
+		customHashListPath := "../../test/fixtures/dib/valid_wordlist.txt"
+		customList, err := dib.LoadCustomHashList(customHashListPath)
+		require.NoError(t, err)
+		newHash, err := dib.HashFiles(baseDir, subImageFiles, []string{root1Hash}, customList)
+		require.NoError(t, err)
 
 		assert.Equal(t, rootNode1.Image.Hash, rootNode0.Image.Hash)
-		assert.Equal(t, "violet-minnesota-alabama-alpha", subNode0.Image.Hash)
-		assert.Equal(t, "golduck-dialga-abra-aegislash", subNode1.Image.Hash)
+		assert.Equal(t, oldHash, customNode0.Image.Hash)
+		assert.Equal(t, newHash, customNode1.Image.Hash)
+		assert.NotEqual(t, customNode0.Image.Hash, customNode1.Image.Hash)
 	})
 
-	t.Run("using arg used in root node should change all hashes", func(t *testing.T) {
+	t.Run("changing build args", func(t *testing.T) {
 		graph0, err := dib.GenerateDAG(buildPath1, registryPrefix, "", nil)
 		require.NoError(t, err)
 
@@ -135,11 +161,14 @@ func TestGenerateDAG(t *testing.T) {
 		require.NoError(t, err)
 
 		nodes0 := flattenNodes(graph0)
-		rootNode0 := nodes0["bullseye"]
+		rootNode0 := nodes0["root1"]
+		multistageNode0 := nodes0["multistage"]
 		nodes1 := flattenNodes(graph1)
-		rootNode1 := nodes1["bullseye"]
+		rootNode1 := nodes1["root1"]
+		multistageNode1 := nodes1["multistage"]
 
-		assert.NotEqual(t, rootNode1.Image.Hash, rootNode0.Image.Hash)
+		assert.NotEqual(t, rootNode0.Image.Hash, rootNode1.Image.Hash)
+		assert.NotEqual(t, multistageNode0.Image.Hash, multistageNode1.Image.Hash)
 	})
 
 	t.Run("duplicates", func(t *testing.T) {
@@ -148,7 +177,7 @@ func TestGenerateDAG(t *testing.T) {
 		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
+				"duplicate image name \"%s/duplicate\" found while reading file \"%s/root/duplicate2/Dockerfile\": previous file was \"%s/root/duplicate1/Dockerfile\"", //nolint:lll
 				registryPrefix, buildPath2, buildPath2))
 	})
 }
@@ -162,7 +191,7 @@ func copyFixtures(t *testing.T, buildPath string) string {
 	dest := t.TempDir()
 	cmd := exec.Command("cp", "-r", src, dest)
 	require.NoError(t, cmd.Run())
-	return dest + "/docker"
+	return dest + "/dag"
 }
 
 func flattenNodes(graph *dag.DAG) map[string]*dag.Node {

pkg/graphviz/graphviz_test.go

@@ -18,7 +18,7 @@ func Test_GenerateDotviz(t *testing.T) {
 	require.NoError(t, err)
 
 	graph, err := dib.GenerateDAG(
-		path.Join(cwd, "../../test/fixtures/docker"),
+		path.Join(cwd, "../../test/fixtures/dag"),
 		"eu.gcr.io/my-test-repository", "",
 		map[string]string{})
 	require.NoError(t, err)
@@ -34,19 +34,17 @@ func Test_GenerateDotviz(t *testing.T) {
 
 	content, err := os.ReadFile(dotFile)
 	require.NoError(t, err)
-	assert.Len(t, content, 647)
-	assert.Contains(t, string(content),
-		"\"eu.gcr.io/my-test-repository/bullseye\" [fillcolor=white style=filled];")
-	assert.Contains(t, string(content),
-		"\"eu.gcr.io/my-test-repository/bullseye\" -> \"eu.gcr.io/my-test-repository/kaniko\";")
-	assert.Contains(t, string(content),
-		"\"eu.gcr.io/my-test-repository/bullseye\" -> \"eu.gcr.io/my-test-repository/multistage\";")
-	assert.Contains(t, string(content),
-		"\"eu.gcr.io/my-test-repository/bullseye\" -> \"eu.gcr.io/my-test-repository/sub-image\";")
-	assert.Contains(t, string(content),
-		"\"eu.gcr.io/my-test-repository/kaniko\" [fillcolor=white style=filled];")
-	assert.Contains(t, string(content),
-		"\"eu.gcr.io/my-test-repository/multistage\" [fillcolor=white style=filled];")
-	assert.Contains(t, string(content),
-		"\"eu.gcr.io/my-test-repository/sub-image\" [fillcolor=white style=filled];")
+	f := string(content)
+	assert.Len(t, f, 958)
+	assert.Contains(t, f, `"eu.gcr.io/my-test-repository/root1" [fillcolor=white style=filled];`)
+	assert.Contains(t, f, `"eu.gcr.io/my-test-repository/custom-hash-list" [fillcolor=white style=filled];`)
+	assert.Contains(t, f, `"eu.gcr.io/my-test-repository/dockerignore" [fillcolor=white style=filled];`)
+	assert.Contains(t, f, `"eu.gcr.io/my-test-repository/multistage" [fillcolor=white style=filled];`)
+	assert.Contains(t, f, `"eu.gcr.io/my-test-repository/with-a-file" [fillcolor=white style=filled];`)
+	assert.Contains(t, f, `"eu.gcr.io/my-test-repository/root2" [fillcolor=white style=filled];`)
+	assert.Contains(t, f, `"eu.gcr.io/my-test-repository/root3" [fillcolor=white style=filled];`)
+	assert.Contains(t, f, `"eu.gcr.io/my-test-repository/root1" -> "eu.gcr.io/my-test-repository/custom-hash-list";`)
+	assert.Contains(t, f, `"eu.gcr.io/my-test-repository/root1" -> "eu.gcr.io/my-test-repository/dockerignore";`)
+	assert.Contains(t, f, `"eu.gcr.io/my-test-repository/root1" -> "eu.gcr.io/my-test-repository/multistage";`)
+	assert.Contains(t, f, `"eu.gcr.io/my-test-repository/root1" -> "eu.gcr.io/my-test-repository/with-a-file";`)
 }

test/fixtures/dag-with-duplicates/root/Dockerfile

@@ -0,0 +1,3 @@
+FROM debian:bullseye
+
+LABEL name="root"