From 783a801f6aaaadd6f66ee6fc29781b889752a40e Mon Sep 17 00:00:00 2001
From: Aaron Friel <mayreply@aaronfriel.com>
Date: Wed, 15 Mar 2023 17:03:39 -0700
Subject: [PATCH] image: add tests, ensure path hashes are os-agnostic

---
 provider/provider.go                          | 22 +++++------
 provider/provider_test.go                     | 39 ++++++++++++++++++-
 .../filemode-matters/step1/Dockerfile         |  5 +++
 .../testdata/filemode-matters/step1/foo.sh    |  3 ++
 .../filemode-matters/step2-chmod-x/Dockerfile |  5 +++
 .../filemode-matters/step2-chmod-x/foo.sh     |  3 ++
 .../basedir-with-ignored-files/.dockerignore  |  3 ++
 .../basedir-with-ignored-files/Dockerfile     |  5 +++
 .../basedir-with-ignored-files/bar/app.js     |  1 +
 .../basedir-with-ignored-files/bar/ignored.js |  0
 .../basedir-with-ignored-files/foo.txt        |  1 +
 .../basedir-with-ignored-files/ignored.txt    |  0
 provider/testdata/ignores/basedir/Dockerfile  |  5 +++
 provider/testdata/ignores/basedir/bar/app.js  |  1 +
 provider/testdata/ignores/basedir/foo.txt     |  1 +
 .../renaming-matters/step1/Dockerfile         |  5 +++
 .../testdata/renaming-matters/step1/foo.a.txt |  1 +
 .../renaming-matters/step2/Dockerfile         |  5 +++
 .../testdata/renaming-matters/step2/foo.b.txt |  1 +
 provider/testdata/symlinks/Dockerfile         |  5 +++
 provider/testdata/symlinks/linkedFromDeep.txt |  1 +
 provider/testdata/symlinks/linkedToDeep.txt   |  1 +
 provider/testdata/symlinks/proxy-for-sub      |  1 +
 .../symlinks/sub/dir/linkedFromTop.txt        |  1 +
 .../testdata/symlinks/sub/dir/linkedToTop.txt |  1 +
 25 files changed, 104 insertions(+), 12 deletions(-)
 create mode 100644 provider/testdata/filemode-matters/step1/Dockerfile
 create mode 100644 provider/testdata/filemode-matters/step1/foo.sh
 create mode 100644 provider/testdata/filemode-matters/step2-chmod-x/Dockerfile
 create mode 100755 provider/testdata/filemode-matters/step2-chmod-x/foo.sh
 create mode 100644 provider/testdata/ignores/basedir-with-ignored-files/.dockerignore
 create mode 100644 provider/testdata/ignores/basedir-with-ignored-files/Dockerfile
 create mode 100644 provider/testdata/ignores/basedir-with-ignored-files/bar/app.js
 create mode 100644 provider/testdata/ignores/basedir-with-ignored-files/bar/ignored.js
 create mode 100644 provider/testdata/ignores/basedir-with-ignored-files/foo.txt
 create mode 100644 provider/testdata/ignores/basedir-with-ignored-files/ignored.txt
 create mode 100644 provider/testdata/ignores/basedir/Dockerfile
 create mode 100644 provider/testdata/ignores/basedir/bar/app.js
 create mode 100644 provider/testdata/ignores/basedir/foo.txt
 create mode 100644 provider/testdata/renaming-matters/step1/Dockerfile
 create mode 100644 provider/testdata/renaming-matters/step1/foo.a.txt
 create mode 100644 provider/testdata/renaming-matters/step2/Dockerfile
 create mode 100644 provider/testdata/renaming-matters/step2/foo.b.txt
 create mode 100644 provider/testdata/symlinks/Dockerfile
 create mode 100644 provider/testdata/symlinks/linkedFromDeep.txt
 create mode 120000 provider/testdata/symlinks/linkedToDeep.txt
 create mode 120000 provider/testdata/symlinks/proxy-for-sub
 create mode 100644 provider/testdata/symlinks/sub/dir/linkedFromTop.txt
 create mode 120000 provider/testdata/symlinks/sub/dir/linkedToTop.txt

diff --git a/provider/provider.go b/provider/provider.go
index 64345d77..9fac6d79 100644
--- a/provider/provider.go
+++ b/provider/provider.go
@@ -9,6 +9,7 @@ import (
 	"io"
 	"io/fs"
 	"os"
+	"path"
 	"path/filepath"
 	"runtime"
 	"strings"
@@ -432,8 +433,7 @@ type contextHashAccumulator struct {
 	input             bytes.Buffer // This will hold the file info and content bytes to pass to a hash object
 }
 
-func (accumulator *contextHashAccumulator) hashPath(path string, fileMode fs.FileMode) error {
-
+func (accumulator *contextHashAccumulator) hashPath(file string, fileMode fs.FileMode) error {
 	hash := sha256.New()
 
 	if fileMode.Type() == fs.ModeSymlink {
@@ -442,31 +442,31 @@ func (accumulator *contextHashAccumulator) hashPath(path string, fileMode fs.Fil
 		// a) ignore changes at the symlink target
 		// b) detect if the symlink _itself_ changes
 		// c) avoid a panic on io.Copy if the symlink target is a directory
-		symLinkPath, err := filepath.EvalSymlinks(filepath.Join(accumulator.dockerContextPath, path))
+		symLinkPath, err := filepath.EvalSymlinks(filepath.Join(accumulator.dockerContextPath, file))
 		if err != nil {
-			return fmt.Errorf("could not evaluate symlink at %s: %w", path, err)
+			return fmt.Errorf("could not evaluate symlink at %s: %w", file, err)
 		}
-		symLinkReader := strings.NewReader(symLinkPath)
-		_, err = io.Copy(hash, symLinkReader)
+		// Hashed content is the clean, os-agnostic file path:
+		_, err = io.Copy(hash, strings.NewReader(path.Clean(symLinkPath)))
 		if err != nil {
-			return fmt.Errorf("could not copy symlink path %s to hash: %w", path, err)
+			return fmt.Errorf("could not copy symlink path %s to hash: %w", file, err)
 		}
 	} else {
 		// For regular files, we can hash their content.
 		// TODO: consider only hashing file metadata to improve performance
-		f, err := os.Open(filepath.Join(accumulator.dockerContextPath, path))
+		f, err := os.Open(filepath.Join(accumulator.dockerContextPath, file))
 		if err != nil {
-			return fmt.Errorf("could not open file %s: %w", path, err)
+			return fmt.Errorf("could not open file %s: %w", file, err)
 		}
 		defer f.Close()
 		_, err = io.Copy(hash, f)
 		if err != nil {
-			return fmt.Errorf("could not copy file %s to hash: %w", path, err)
+			return fmt.Errorf("could not copy file %s to hash: %w", file, err)
 		}
 	}
 
 	// Capture all information in the accumulator buffer and add a separator
-	accumulator.input.Write([]byte(path))
+	accumulator.input.Write([]byte(filepath.Clean(file))) // use os-agnostic filepath
 	accumulator.input.Write([]byte(fileMode.String()))
 	accumulator.input.Write(hash.Sum(nil))
 	accumulator.input.WriteByte(0)
diff --git a/provider/provider_test.go b/provider/provider_test.go
index 79904378..dcf7324b 100644
--- a/provider/provider_test.go
+++ b/provider/provider_test.go
@@ -1,10 +1,12 @@
 package provider
 
 import (
+	"testing"
+
 	"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
 	rpc "github.com/pulumi/pulumi/sdk/v3/proto/go"
 	"github.com/stretchr/testify/assert"
-	"testing"
+	"github.com/stretchr/testify/require"
 )
 
 func TestDiffUpdates(t *testing.T) {
@@ -106,3 +108,38 @@ func TestDiffUpdates(t *testing.T) {
 	})
 
 }
+
+func TestHashIgnoresFile(t *testing.T) {
+	baseResult, err := hashContext("./testdata/ignores/basedir", "./Dockerfile")
+	require.NoError(t, err)
+
+	result, err := hashContext("./testdata/ignores/basedir-with-ignored-files", "./Dockerfile")
+	require.NoError(t, err)
+
+	assert.Equal(t, result, baseResult)
+}
+
+func TestHashRenamingMatters(t *testing.T) {
+	baseResult, err := hashContext("./testdata/renaming-matters/step1", "./Dockerfile")
+	require.NoError(t, err)
+
+	result, err := hashContext("./testdata/renaming-matters/step2", "./Dockerfile")
+	require.NoError(t, err)
+
+	assert.NotEqual(t, result, baseResult)
+}
+
+func TestHashFilemodeMatters(t *testing.T) {
+	baseResult, err := hashContext("./testdata/filemode-matters/step1", "./Dockerfile")
+	require.NoError(t, err)
+
+	result, err := hashContext("./testdata/filemode-matters/step2-chmod-x", "./Dockerfile")
+	require.NoError(t, err)
+
+	assert.NotEqual(t, result, baseResult)
+}
+
+func TestHashDeepSymlinks(t *testing.T) {
+	_, err := hashContext("./testdata/symlinks", "./Dockerfile")
+	assert.NoError(t, err)
+}
diff --git a/provider/testdata/filemode-matters/step1/Dockerfile b/provider/testdata/filemode-matters/step1/Dockerfile
new file mode 100644
index 00000000..a79d7c27
--- /dev/null
+++ b/provider/testdata/filemode-matters/step1/Dockerfile
@@ -0,0 +1,5 @@
+FROM scratch
+
+WORKDIR /app
+
+COPY ./bar .
diff --git a/provider/testdata/filemode-matters/step1/foo.sh b/provider/testdata/filemode-matters/step1/foo.sh
new file mode 100644
index 00000000..86f1c628
--- /dev/null
+++ b/provider/testdata/filemode-matters/step1/foo.sh
@@ -0,0 +1,3 @@
+#!/usr/bin/env bash
+
+echo "Hello, World!"
diff --git a/provider/testdata/filemode-matters/step2-chmod-x/Dockerfile b/provider/testdata/filemode-matters/step2-chmod-x/Dockerfile
new file mode 100644
index 00000000..a79d7c27
--- /dev/null
+++ b/provider/testdata/filemode-matters/step2-chmod-x/Dockerfile
@@ -0,0 +1,5 @@
+FROM scratch
+
+WORKDIR /app
+
+COPY ./bar .
diff --git a/provider/testdata/filemode-matters/step2-chmod-x/foo.sh b/provider/testdata/filemode-matters/step2-chmod-x/foo.sh
new file mode 100755
index 00000000..86f1c628
--- /dev/null
+++ b/provider/testdata/filemode-matters/step2-chmod-x/foo.sh
@@ -0,0 +1,3 @@
+#!/usr/bin/env bash
+
+echo "Hello, World!"
diff --git a/provider/testdata/ignores/basedir-with-ignored-files/.dockerignore b/provider/testdata/ignores/basedir-with-ignored-files/.dockerignore
new file mode 100644
index 00000000..eb97a727
--- /dev/null
+++ b/provider/testdata/ignores/basedir-with-ignored-files/.dockerignore
@@ -0,0 +1,3 @@
+.dockerignore
+ignored.txt
+bar/ignored.js
diff --git a/provider/testdata/ignores/basedir-with-ignored-files/Dockerfile b/provider/testdata/ignores/basedir-with-ignored-files/Dockerfile
new file mode 100644
index 00000000..a79d7c27
--- /dev/null
+++ b/provider/testdata/ignores/basedir-with-ignored-files/Dockerfile
@@ -0,0 +1,5 @@
+FROM scratch
+
+WORKDIR /app
+
+COPY ./bar .
diff --git a/provider/testdata/ignores/basedir-with-ignored-files/bar/app.js b/provider/testdata/ignores/basedir-with-ignored-files/bar/app.js
new file mode 100644
index 00000000..22541085
--- /dev/null
+++ b/provider/testdata/ignores/basedir-with-ignored-files/bar/app.js
@@ -0,0 +1 @@
+console.log("Pulumi 💜");
diff --git a/provider/testdata/ignores/basedir-with-ignored-files/bar/ignored.js b/provider/testdata/ignores/basedir-with-ignored-files/bar/ignored.js
new file mode 100644
index 00000000..e69de29b
diff --git a/provider/testdata/ignores/basedir-with-ignored-files/foo.txt b/provider/testdata/ignores/basedir-with-ignored-files/foo.txt
new file mode 100644
index 00000000..5716ca59
--- /dev/null
+++ b/provider/testdata/ignores/basedir-with-ignored-files/foo.txt
@@ -0,0 +1 @@
+bar
diff --git a/provider/testdata/ignores/basedir-with-ignored-files/ignored.txt b/provider/testdata/ignores/basedir-with-ignored-files/ignored.txt
new file mode 100644
index 00000000..e69de29b
diff --git a/provider/testdata/ignores/basedir/Dockerfile b/provider/testdata/ignores/basedir/Dockerfile
new file mode 100644
index 00000000..a79d7c27
--- /dev/null
+++ b/provider/testdata/ignores/basedir/Dockerfile
@@ -0,0 +1,5 @@
+FROM scratch
+
+WORKDIR /app
+
+COPY ./bar .
diff --git a/provider/testdata/ignores/basedir/bar/app.js b/provider/testdata/ignores/basedir/bar/app.js
new file mode 100644
index 00000000..22541085
--- /dev/null
+++ b/provider/testdata/ignores/basedir/bar/app.js
@@ -0,0 +1 @@
+console.log("Pulumi 💜");
diff --git a/provider/testdata/ignores/basedir/foo.txt b/provider/testdata/ignores/basedir/foo.txt
new file mode 100644
index 00000000..5716ca59
--- /dev/null
+++ b/provider/testdata/ignores/basedir/foo.txt
@@ -0,0 +1 @@
+bar
diff --git a/provider/testdata/renaming-matters/step1/Dockerfile b/provider/testdata/renaming-matters/step1/Dockerfile
new file mode 100644
index 00000000..a79d7c27
--- /dev/null
+++ b/provider/testdata/renaming-matters/step1/Dockerfile
@@ -0,0 +1,5 @@
+FROM scratch
+
+WORKDIR /app
+
+COPY ./bar .
diff --git a/provider/testdata/renaming-matters/step1/foo.a.txt b/provider/testdata/renaming-matters/step1/foo.a.txt
new file mode 100644
index 00000000..345e6aef
--- /dev/null
+++ b/provider/testdata/renaming-matters/step1/foo.a.txt
@@ -0,0 +1 @@
+Test
diff --git a/provider/testdata/renaming-matters/step2/Dockerfile b/provider/testdata/renaming-matters/step2/Dockerfile
new file mode 100644
index 00000000..a79d7c27
--- /dev/null
+++ b/provider/testdata/renaming-matters/step2/Dockerfile
@@ -0,0 +1,5 @@
+FROM scratch
+
+WORKDIR /app
+
+COPY ./bar .
diff --git a/provider/testdata/renaming-matters/step2/foo.b.txt b/provider/testdata/renaming-matters/step2/foo.b.txt
new file mode 100644
index 00000000..345e6aef
--- /dev/null
+++ b/provider/testdata/renaming-matters/step2/foo.b.txt
@@ -0,0 +1 @@
+Test
diff --git a/provider/testdata/symlinks/Dockerfile b/provider/testdata/symlinks/Dockerfile
new file mode 100644
index 00000000..a79d7c27
--- /dev/null
+++ b/provider/testdata/symlinks/Dockerfile
@@ -0,0 +1,5 @@
+FROM scratch
+
+WORKDIR /app
+
+COPY ./bar .
diff --git a/provider/testdata/symlinks/linkedFromDeep.txt b/provider/testdata/symlinks/linkedFromDeep.txt
new file mode 100644
index 00000000..552085f0
--- /dev/null
+++ b/provider/testdata/symlinks/linkedFromDeep.txt
@@ -0,0 +1 @@
+Hello,
diff --git a/provider/testdata/symlinks/linkedToDeep.txt b/provider/testdata/symlinks/linkedToDeep.txt
new file mode 120000
index 00000000..da27877a
--- /dev/null
+++ b/provider/testdata/symlinks/linkedToDeep.txt
@@ -0,0 +1 @@
+sub/dir/linkedFromTop.txt
\ No newline at end of file
diff --git a/provider/testdata/symlinks/proxy-for-sub b/provider/testdata/symlinks/proxy-for-sub
new file mode 120000
index 00000000..3de0f365
--- /dev/null
+++ b/provider/testdata/symlinks/proxy-for-sub
@@ -0,0 +1 @@
+sub
\ No newline at end of file
diff --git a/provider/testdata/symlinks/sub/dir/linkedFromTop.txt b/provider/testdata/symlinks/sub/dir/linkedFromTop.txt
new file mode 100644
index 00000000..496c8755
--- /dev/null
+++ b/provider/testdata/symlinks/sub/dir/linkedFromTop.txt
@@ -0,0 +1 @@
+World!
diff --git a/provider/testdata/symlinks/sub/dir/linkedToTop.txt b/provider/testdata/symlinks/sub/dir/linkedToTop.txt
new file mode 120000
index 00000000..2cf8adf8
--- /dev/null
+++ b/provider/testdata/symlinks/sub/dir/linkedToTop.txt
@@ -0,0 +1 @@
+../../linkedFromDeep.txt
\ No newline at end of file