UT for check for changes to P2F content

pkg/secrets/pushtofile/push_to_writer.go

@@ -5,10 +5,12 @@ import (
 	"crypto/sha256"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"text/template"
+
+	"github.com/cyberark/conjur-authn-k8s-client/pkg/log"
+	"github.com/cyberark/secrets-provider-for-k8s/pkg/log/messages"
 )
 
 // templateData describes the form in which data is presented to push-to-file templates
@@ -95,15 +97,6 @@ func pushToWriter(
 		return err
 	}
 
-	// ***TEMP DEBUG STUFF***//
-	if false {
-		tpl.Execute(writer, templateData{
-			SecretsArray: groupSecrets,
-			SecretsMap:   secretsMap,
-		})
-		return nil
-	}
-
 	// Render the secret file content
 	tplData := templateData{
 		SecretsArray: groupSecrets,
@@ -114,30 +107,34 @@ func pushToWriter(
 		return err
 	}
 
-	if writer == ioutil.Discard {
+	return writeContent(writer, fileContent, groupName)
+}
+
+func renderFile(tpl *template.Template, tplData templateData) (*bytes.Buffer, error) {
+	buf := &bytes.Buffer{}
+	err := tpl.Execute(buf, tplData)
+	return buf, err
+}
+
+func writeContent(writer io.Writer, fileContent *bytes.Buffer, groupName string) error {
+	if writer == io.Discard {
 		_, err := writer.Write(fileContent.Bytes())
 		return err
 	}
 
 	// Calculate a sha256 checksum on the content
-	checksum, err := fileChecksum(fileContent)
+	checksum, _ := fileChecksum(fileContent)
 
 	// If file contents have changed, write the file and update checksum
 	if contentHasChanged(groupName, checksum) {
-		fmt.Printf("Writing secret file content: \n%s\n", fileContent.String())
 		if _, err := writer.Write(fileContent.Bytes()); err != nil {
 			return err
 		}
 		prevFileChecksums[groupName] = checksum
+	} else {
+		log.Info(messages.CSPFK018I)
 	}
-
-	return err
-}
-
-func renderFile(tpl *template.Template, tplData templateData) (*bytes.Buffer, error) {
-	buf := &bytes.Buffer{}
-	err := tpl.Execute(buf, tplData)
-	return buf, err
+	return nil
 }
 
 // fileChecksum calculates a checksum on the file content

pkg/secrets/pushtofile/push_to_writer_test.go

@@ -151,3 +151,58 @@ func Test_pushToWriter(t *testing.T) {
 		tc.Run(t)
 	}
 }
+
+func Test_pushToWriter_contentChanges(t *testing.T) {
+	t.Run("content changes", func(t *testing.T) {
+		// Call pushToWriter with a simple template and secret.
+		secrets := []*Secret{{Alias: "alias", Value: "secret value"}}
+		groupName := "group path"
+		template := `{{secret "alias"}}`
+
+		buf := new(bytes.Buffer)
+		err := pushToWriter(
+			buf,
+			groupName,
+			template,
+			secrets,
+		)
+		assert.NoError(t, err)
+		assert.Equal(t, "secret value", buf.String())
+
+		// Now clear the buffer and call pushToWriter again. Since the secret is the same,
+		// it should not update the buffer.
+		buf.Reset()
+		err = pushToWriter(
+			buf,
+			groupName,
+			template,
+			secrets,
+		)
+
+		assert.NoError(t, err)
+		assert.Zero(t, buf.Len())
+
+		// Now change the secret and call pushToWriter again. This time, the buffer should
+		// be updated because the secret has changed.
+		err = pushToWriter(
+			buf,
+			groupName,
+			template,
+			[]*Secret{{Alias: "alias", Value: "secret changed"}},
+		)
+		assert.NoError(t, err)
+		assert.Equal(t, "secret changed", buf.String())
+
+		// Repeat the test but this time change the template instead of the secret. The buffer should still
+		// be updated because the rendered output should be different.
+		buf.Reset()
+		err = pushToWriter(
+			buf,
+			groupName,
+			`- {{secret "alias"}}`,
+			[]*Secret{{Alias: "alias", Value: "secret changed"}},
+		)
+		assert.NoError(t, err)
+		assert.Equal(t, "- secret changed", buf.String())
+	})
+}

pkg/secrets/pushtofile/secret_group.go

@@ -69,7 +69,7 @@ func (sg *SecretGroup) pushToFileWithDeps(
 	depOpenWriteCloser openWriteCloserFunc,
 	depPushToWriter pushToWriterFunc,
 	secrets []*Secret,
-) error {
+) (err error) {
 	// Make sure all the secret specs are accounted for
 	if err := validateSecretsAgainstSpecs(secrets, sg.SecretSpecs); err != nil {
 		return err

pkg/secrets/pushtofile/secret_group_test.go

@@ -549,80 +549,80 @@ func TestNewSecretGroups(t *testing.T) {
 	})
 
 	// Test file permissions settings
-    type assertFunc func([]*SecretGroup, []error)
-    assertExpectedFileMode := func(expectedFileMode os.FileMode) assertFunc {
-        return func(groups []*SecretGroup, errs []error) {
-            assert.Len(t, errs, 0)
-            assert.Len(t, groups, 1)
-            assert.Equal(t, groups[0].FilePermissions, expectedFileMode)
-        }
-    }
-    assertExpectedErr := func(expectedErrStr string) assertFunc {
-        return func(_ []*SecretGroup, errs []error) {
-            assert.NotEmpty(t, errs)
-            assert.Contains(t, errs[0].Error(), expectedErrStr)
-        }
-    }
-    filePermsTestCases := []struct {
-        description string
-        permStr     string // Set to "" to skip file permissions annotation
-        assertFunc  assertFunc
-    }{
-        // Happy path test cases
-        {
-            description: "secret file permissions defaulted if not configure",
-            permStr:     "",
-            assertFunc:  assertExpectedFileMode(defaultFilePermissions),
-        }, {
-            description: "File perms '-rw-rw-r--'",
-            permStr:     "-rw-rw-r--",
-            assertFunc:  assertExpectedFileMode(os.FileMode(0664)),
-        }, {
-            description: "File perms 'rw-rw-r--' (no leading dash)",
-            permStr:     "rw-rw-r--",
-            assertFunc:  assertExpectedFileMode(os.FileMode(0664)),
-        }, {
-            description: "File perms '-rw-r--r--'",
-            permStr:     "-rw-r--r--",
-            assertFunc:  assertExpectedFileMode(os.FileMode(0644)),
-        }, {
-            description: "File perms '-rwxrwxrwx'",
-            permStr:     "-rwxrwxrwx",
-            assertFunc:  assertExpectedFileMode(os.FileMode(0777)),
-        },
-        // Unhappy path test cases
-        {
-            description: "File permission string with leading 'd'",
-            permStr:     "drw-rw-r--",
-            assertFunc:  assertExpectedErr("Invalid permissions format"),
-        }, {
-            description: "File perms '----------' (0000)",
-            permStr:     "----------",
-            assertFunc:  assertExpectedErr("owner permissions must atleast have read and write"),
-        }, {
-            description: "File permission string with invalid character",
-            permStr:     "-rw-r--U--",
-            assertFunc:  assertExpectedErr("Invalid permissions format"),
-        }, {
-            description: "File permission string with less than 9 characters",
-            permStr:     "-rw-rw-",
-            assertFunc:  assertExpectedErr("Invalid permissions format"),
-        },
-    }
-    for _, tc := range filePermsTestCases {
-        t.Run(tc.description, func(t *testing.T) {
-            // Run the test case
-            annotations := map[string]string{
-                "conjur.org/conjur-secrets.first": `- path/to/secret/first1`,
-            }
-            if tc.permStr != "" {
-                annotations["conjur.org/secret-file-permissions.first"] = tc.permStr
-            }
-            groups, errs := NewSecretGroups("", "", annotations)
-            // Verify results
-            tc.assertFunc(groups, errs)
-        })
-    }
+	type assertFunc func([]*SecretGroup, []error)
+	assertExpectedFileMode := func(expectedFileMode os.FileMode) assertFunc {
+		return func(groups []*SecretGroup, errs []error) {
+			assert.Len(t, errs, 0)
+			assert.Len(t, groups, 1)
+			assert.Equal(t, groups[0].FilePermissions, expectedFileMode)
+		}
+	}
+	assertExpectedErr := func(expectedErrStr string) assertFunc {
+		return func(_ []*SecretGroup, errs []error) {
+			assert.NotEmpty(t, errs)
+			assert.Contains(t, errs[0].Error(), expectedErrStr)
+		}
+	}
+	filePermsTestCases := []struct {
+		description string
+		permStr     string // Set to "" to skip file permissions annotation
+		assertFunc  assertFunc
+	}{
+		// Happy path test cases
+		{
+			description: "secret file permissions defaulted if not configure",
+			permStr:     "",
+			assertFunc:  assertExpectedFileMode(defaultFilePermissions),
+		}, {
+			description: "File perms '-rw-rw-r--'",
+			permStr:     "-rw-rw-r--",
+			assertFunc:  assertExpectedFileMode(os.FileMode(0664)),
+		}, {
+			description: "File perms 'rw-rw-r--' (no leading dash)",
+			permStr:     "rw-rw-r--",
+			assertFunc:  assertExpectedFileMode(os.FileMode(0664)),
+		}, {
+			description: "File perms '-rw-r--r--'",
+			permStr:     "-rw-r--r--",
+			assertFunc:  assertExpectedFileMode(os.FileMode(0644)),
+		}, {
+			description: "File perms '-rwxrwxrwx'",
+			permStr:     "-rwxrwxrwx",
+			assertFunc:  assertExpectedFileMode(os.FileMode(0777)),
+		},
+		// Unhappy path test cases
+		{
+			description: "File permission string with leading 'd'",
+			permStr:     "drw-rw-r--",
+			assertFunc:  assertExpectedErr("Invalid permissions format"),
+		}, {
+			description: "File perms '----------' (0000)",
+			permStr:     "----------",
+			assertFunc:  assertExpectedErr("owner permissions must atleast have read and write"),
+		}, {
+			description: "File permission string with invalid character",
+			permStr:     "-rw-r--U--",
+			assertFunc:  assertExpectedErr("Invalid permissions format"),
+		}, {
+			description: "File permission string with less than 9 characters",
+			permStr:     "-rw-rw-",
+			assertFunc:  assertExpectedErr("Invalid permissions format"),
+		},
+	}
+	for _, tc := range filePermsTestCases {
+		t.Run(tc.description, func(t *testing.T) {
+			// Run the test case
+			annotations := map[string]string{
+				"conjur.org/conjur-secrets.first": `- path/to/secret/first1`,
+			}
+			if tc.permStr != "" {
+				annotations["conjur.org/secret-file-permissions.first"] = tc.permStr
+			}
+			groups, errs := NewSecretGroups("", "", annotations)
+			// Verify results
+			tc.assertFunc(groups, errs)
+		})
+	}
 }
 
 var pushToFileWithDepsTestCases = []pushToFileWithDepsTestCase{
@@ -823,8 +823,8 @@ func TestSecretGroup_PushToFile(t *testing.T) {
 	defer os.Remove(dir)
 
 	for _, tc := range []struct {
-		description string
-		path string
+		description     string
+		path            string
 		filePermissions os.FileMode
 	}{
 		{"file with existing parent folder, perms 0660", "./file", 0660},
@@ -834,6 +834,9 @@ func TestSecretGroup_PushToFile(t *testing.T) {
 		t.Run(tc.description, func(t *testing.T) {
 			absoluteFilePath := path.Join(dir, tc.path)
 
+			// Reset the P2F cache so the files will be written even if the values haven't changed
+			prevFileChecksums = map[string]checksum{}
+
 			// Create a group, and push to file
 			group := SecretGroup{
 				Name:            "groupname",