Merge pull request #432 from cyberark/ONYX-15541

Secrets Rotation: only write secret files if changed

pkg/log/messages/info_messages.go

@@ -29,3 +29,4 @@ const CSPFK014I string = "CSPFK014I Authenticator setting %s provided by %s"
 const CSPFK015I string = "CSPFK015I DAP/Conjur Secrets pushed to shared volume successfully"
 const CSPFK016I string = "CSPFK016I There are no secrets to be retrieved from Conjur"
 const CSPFK017I string = "CSPFK017I Creating default file name for secret group '%s'"
+const CSPFK018I string = "CSPFK018I No change in secret file, no secret files written"

pkg/secrets/pushtofile/push_to_writer.go

@@ -1,11 +1,16 @@
 package pushtofile
 
 import (
+	"bytes"
+	"crypto/sha256"
 	"fmt"
 	"io"
 	"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
@@ -30,6 +35,13 @@ type openWriteCloserFunc func(
 	permissions os.FileMode,
 ) (io.WriteCloser, error)
 
+type checksum []byte
+
+// prevFileChecksums maps a secret group name to a sha256 checksum of the
+// corresponding secret file content. This is used to detect changes in
+// secret file content.
+var prevFileChecksums = map[string]checksum{}
+
 // openFileAsWriteCloser opens a file to write-to with some permissions.
 func openFileAsWriteCloser(path string, permissions os.FileMode) (io.WriteCloser, error) {
 	dir := filepath.Dir(path)
@@ -65,7 +77,7 @@ func pushToWriter(
 		secretsMap[s.Alias] = s
 	}
 
-	t, err := template.New(groupName).Funcs(template.FuncMap{
+	tpl, err := template.New(groupName).Funcs(template.FuncMap{
 		// secret is a custom utility function for streamlined access to secret values.
 		// It panics for secrets aliases not specified on the group.
 		"secret": func(alias string) string {
@@ -85,8 +97,62 @@ func pushToWriter(
 		return err
 	}
 
-	return t.Execute(writer, templateData{
+	// Render the secret file content
+	tplData := templateData{
 		SecretsArray: groupSecrets,
 		SecretsMap:   secretsMap,
-	})
+	}
+	fileContent, err := renderFile(tpl, tplData)
+	if err != nil {
+		return err
+	}
+
+	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, _ := fileChecksum(fileContent)
+
+	// If file contents have changed, write the file and update checksum
+	if contentHasChanged(groupName, checksum) {
+		if _, err := writer.Write(fileContent.Bytes()); err != nil {
+			return err
+		}
+		prevFileChecksums[groupName] = checksum
+	} else {
+		log.Info(messages.CSPFK018I)
+	}
+	return nil
+}
+
+// fileChecksum calculates a checksum on the file content
+func fileChecksum(buf *bytes.Buffer) (checksum, error) {
+	hash := sha256.New()
+	bufCopy := bytes.NewBuffer(buf.Bytes())
+	if _, err := io.Copy(hash, bufCopy); err != nil {
+		return nil, err
+	}
+	checksum := hash.Sum(nil)
+	return checksum, nil
+}
+
+func contentHasChanged(groupName string, fileChecksum checksum) bool {
+	if prevChecksum, exists := prevFileChecksums[groupName]; exists {
+		if bytes.Equal(fileChecksum, prevChecksum) {
+			return false
+		}
+	}
+	return true
 }

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

@@ -2,14 +2,15 @@ package pushtofile
 
 import (
 	"fmt"
-	"github.com/cyberark/conjur-authn-k8s-client/pkg/log"
-	"github.com/cyberark/secrets-provider-for-k8s/pkg/log/messages"
 	"io/ioutil"
 	"os"
 	"path"
 	"path/filepath"
 	"sort"
 	"strings"
+
+	"github.com/cyberark/conjur-authn-k8s-client/pkg/log"
+	"github.com/cyberark/secrets-provider-for-k8s/pkg/log/messages"
 )
 
 const secretGroupPrefix = "conjur.org/conjur-secrets."
@@ -70,9 +71,8 @@ func (sg *SecretGroup) pushToFileWithDeps(
 	secrets []*Secret,
 ) (err error) {
 	// Make sure all the secret specs are accounted for
-	err = validateSecretsAgainstSpecs(secrets, sg.SecretSpecs)
-	if err != nil {
-		return
+	if err := validateSecretsAgainstSpecs(secrets, sg.SecretSpecs); err != nil {
+		return err
 	}
 
 	// Determine file template from
@@ -85,13 +85,13 @@ func (sg *SecretGroup) pushToFileWithDeps(
 		sg.SecretSpecs,
 	)
 	if err != nil {
-		return
+		return err
 	}
 
 	//// Open and push to file
 	wc, err := depOpenWriteCloser(sg.FilePath, sg.FilePermissions)
 	if err != nil {
-		return
+		return err
 	}
 	defer func() {
 		_ = wc.Close()
@@ -103,16 +103,16 @@ func (sg *SecretGroup) pushToFileWithDeps(
 			err = maskError
 		}
 	}()
-	pushToWriterErr := depPushToWriter(
+	err = depPushToWriter(
 		wc,
 		sg.Name,
 		fileTemplate,
 		secrets,
 	)
-	if pushToWriterErr != nil {
+	if err != nil {
 		err = maskError
 	}
-	return
+	return err
 }
 
 func (sg *SecretGroup) absoluteFilePath(secretsBasePath string) (string, error) {