CHANGELOG.md

@@ -2,6 +2,25 @@
 
 All notable changes to this project are documented in this file.
 
+## 0.24.4
+
+**Release date:** 2022-04-28
+
+This prerelease ensures we recover from Kustomize build panics to guarantee
+continuity of operations when running into invalid object data.
+
+In addition, handling of file formats while decrypting Secret generator entries
+with SOPS has been improved to ensure encrypted files in format A can be
+decrypted to target format B.
+
+Fixes:
+- Use Secret generator keys for SOPS format hint
+  [#636](https://github.com/fluxcd/kustomize-controller/pull/636)
+
+Improvements:
+- generator: recover from kustomize build panics
+  [#637](https://github.com/fluxcd/kustomize-controller/pull/637)
+
 ## 0.24.3
 
 **Release date:** 2022-04-22

config/manager/kustomization.yaml

@@ -5,4 +5,4 @@ resources:
 images:
   - name: fluxcd/kustomize-controller
     newName: fluxcd/kustomize-controller
-    newTag: v0.24.3
+    newTag: v0.24.4

controllers/kustomization_decryptor.go

@@ -369,21 +369,16 @@ func (d *KustomizeDecryptor) DecryptEnvSources(path string) error {
 
 // decryptKustomizationEnvSources returns a visitKustomization implementation
 // which attempts to decrypt any EnvSources entry it finds in the Kustomization
-// file it is called with.
-// After a successful decrypt, the absolute path of the file is added to the
+// file with which it is called.
+// After decrypting successfully, it adds the absolute path of the file to the
 // given map.
 func (d *KustomizeDecryptor) decryptKustomizationEnvSources(visited map[string]struct{}) visitKustomization {
 	return func(root, path string, kus *kustypes.Kustomization) error {
-		visitRef := func(ref string, format formats.Format) error {
-			refParts := strings.Split(ref, "=")
-			if len(refParts) > 1 {
-				ref = refParts[1]
+		visitRef := func(sourcePath string, format formats.Format) error {
+			if !filepath.IsAbs(sourcePath) {
+				sourcePath = filepath.Join(path, sourcePath)
 			}
-			if !filepath.IsAbs(ref) {
-				ref = filepath.Join(path, ref)
-			}
-
-			absRef, _, err := securePaths(root, ref)
+			absRef, _, err := securePaths(root, sourcePath)
 			if err != nil {
 				return err
 			}
@@ -403,7 +398,15 @@ func (d *KustomizeDecryptor) decryptKustomizationEnvSources(visited map[string]s
 
 		for _, gen := range kus.SecretGenerator {
 			for _, fileSrc := range gen.FileSources {
-				if err := visitRef(fileSrc, formats.FormatForPath(fileSrc)); err != nil {
+				parts := strings.SplitN(fileSrc, "=", 2)
+				key := parts[0]
+				var filePath string
+				if len(parts) > 1 {
+					filePath = parts[1]
+				} else {
+					filePath = key
+				}
+				if err := visitRef(filePath, formats.FormatForPath(key)); err != nil {
 					return err
 				}
 			}

controllers/kustomization_decryptor_test.go

@@ -695,7 +695,7 @@ func TestKustomizeDecryptor_DecryptResource(t *testing.T) {
 		},
 	}
 
-	t.Run("SOPS encrypted resource", func(t *testing.T) {
+	t.Run("SOPS-encrypted Secret resource", func(t *testing.T) {
 		g := NewWithT(t)
 
 		kus := kustomization.DeepCopy()
@@ -736,7 +736,7 @@ func TestKustomizeDecryptor_DecryptResource(t *testing.T) {
 		g.Expect(got.MarshalJSON()).To(Equal(secretData))
 	})
 
-	t.Run("SOPS encrypted binary Secret data field", func(t *testing.T) {
+	t.Run("SOPS-encrypted binary-format Secret data field", func(t *testing.T) {
 		g := NewWithT(t)
 
 		kus := kustomization.DeepCopy()
@@ -771,7 +771,7 @@ func TestKustomizeDecryptor_DecryptResource(t *testing.T) {
 		g.Expect(got.GetDataMap()).To(HaveKeyWithValue("file.ini", base64.StdEncoding.EncodeToString(plainData)))
 	})
 
-	t.Run("SOPS encrypted YAML Secret data field", func(t *testing.T) {
+	t.Run("SOPS-encrypted YAML-format Secret data field", func(t *testing.T) {
 		g := NewWithT(t)
 
 		kus := kustomization.DeepCopy()
@@ -849,12 +849,14 @@ func TestKustomizeDecryptor_DecryptResource(t *testing.T) {
 
 func TestKustomizeDecryptor_decryptKustomizationEnvSources(t *testing.T) {
 	type file struct {
-		name       string
-		symlink    string
-		data       []byte
-		encrypt    bool
-		expectData bool
+		name           string
+		symlink        string
+		data           []byte
+		originalFormat *formats.Format
+		encrypt        bool
+		expectData     bool
 	}
+	binaryFormat := formats.Binary
 	tests := []struct {
 		name            string
 		wordirSuffix    string
@@ -869,6 +871,9 @@ func TestKustomizeDecryptor_decryptKustomizationEnvSources(t *testing.T) {
 			path: "subdir",
 			files: []file{
 				{name: "subdir/app.env", data: []byte("var1=value1\n"), encrypt: true, expectData: true},
+				// NB: Despite the file extension representing the SOPS-encrypted JSON output
+				// format, the original data is plain text, or "binary."
+				{name: "subdir/combination.json", data: []byte("The safe combination is ..."), originalFormat: &binaryFormat, encrypt: true, expectData: true},
 				{name: "subdir/file.txt", data: []byte("file"), encrypt: true, expectData: true},
 				{name: "secret.env", data: []byte("var2=value2\n"), encrypt: true, expectData: true},
 			},
@@ -877,13 +882,13 @@ func TestKustomizeDecryptor_decryptKustomizationEnvSources(t *testing.T) {
 					GeneratorArgs: kustypes.GeneratorArgs{
 						Name: "envSecret",
 						KvPairSources: kustypes.KvPairSources{
-							FileSources: []string{"file.txt"},
-							EnvSources:  []string{"app.env", "key=../secret.env"},
+							FileSources: []string{"file.txt", "combo=combination.json"},
+							EnvSources:  []string{"app.env", "../secret.env"},
 						},
 					},
 				},
 			},
-			expectVisited: []string{"subdir/app.env", "subdir/file.txt", "secret.env"},
+			expectVisited: []string{"subdir/app.env", "subdir/combination.json", "subdir/file.txt", "secret.env"},
 		},
 		{
 			name:  "decryption error",
@@ -987,7 +992,12 @@ func TestKustomizeDecryptor_decryptKustomizationEnvSources(t *testing.T) {
 				}
 				data := f.data
 				if f.encrypt {
-					format := formats.FormatForPath(f.name)
+					var format formats.Format
+					if f.originalFormat != nil {
+						format = *f.originalFormat
+					} else {
+						format = formats.FormatForPath(f.name)
+					}
 					data, err = d.sopsEncryptWithFormat(sops.Metadata{
 						KeyGroups: []sops.KeyGroup{
 							{&sopsage.MasterKey{Recipient: id.Recipient().String()}},
@@ -1159,7 +1169,7 @@ func TestKustomizeDecryptor_decryptSopsFile(t *testing.T) {
 
 				b, err := os.ReadFile(filepath.Join(tmpDir, f.name))
 				g.Expect(err).ToNot(HaveOccurred())
-				g.Expect(bytes.Compare(f.data, b) == 0).To(Equal(f.expectData))
+				g.Expect(bytes.Equal(f.data, b)).To(Equal(f.expectData))
 			}
 		})
 	}

controllers/kustomization_generator.go

@@ -247,7 +247,7 @@ var kustomizeBuildMutex sync.Mutex
 //  - load files from outside the kustomization dir path
 //    (but not outside root)
 //  - disable plugins except for the builtin ones
-func secureBuildKustomization(root, dirPath string) (resmap.ResMap, error) {
+func secureBuildKustomization(root, dirPath string) (_ resmap.ResMap, err error) {
 	// Create secure FS for root
 	fs, err := securefs.MakeFsOnDiskSecureBuild(root)
 	if err != nil {
@@ -259,6 +259,15 @@ func secureBuildKustomization(root, dirPath string) (resmap.ResMap, error) {
 	kustomizeBuildMutex.Lock()
 	defer kustomizeBuildMutex.Unlock()
 
+	// Kustomize tends to panic in unpredicted ways due to (accidental)
+	// invalid object data; recover when this happens to ensure continuity of
+	// operations
+	defer func() {
+		if r := recover(); r != nil {
+			err = fmt.Errorf("recovered from kustomize build panic: %v", r)
+		}
+	}()
+
 	buildOptions := &krusty.Options{
 		LoadRestrictions: kustypes.LoadRestrictionsNone,
 		PluginConfig:     kustypes.DisabledPluginConfig(),

controllers/kustomization_generator_test.go

@@ -30,3 +30,16 @@ func Test_secureBuildKustomization(t *testing.T) {
 		g.Expect(err).ToNot(HaveOccurred())
 	})
 }
+
+func Test_secureBuildKustomization_panic(t *testing.T) {
+	t.Run("build panic", func(t *testing.T) {
+		g := NewWithT(t)
+
+		_, err := secureBuildKustomization("testdata/panic", "testdata/panic")
+		g.Expect(err).To(HaveOccurred())
+		g.Expect(err.Error()).To(ContainSubstring("recovered from kustomize build panic"))
+		// Run again to ensure the lock is released
+		_, err = secureBuildKustomization("testdata/panic", "testdata/panic")
+		g.Expect(err).To(HaveOccurred())
+	})
+}

controllers/suite_test.go

@@ -43,7 +43,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/client-go/kubernetes/scheme"
-	kuberecorder "k8s.io/client-go/tools/record"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
@@ -68,7 +67,6 @@ var (
 	k8sClient    client.Client
 	testEnv      *testenv.Environment
 	testServer   *testserver.ArtifactServer
-	testEventsH  kuberecorder.EventRecorder
 	testMetricsH controller.Metrics
 	ctx          = ctrl.SetupSignalHandler()
 	kubeConfig   []byte

controllers/testdata/panic/kustomization.yaml

@@ -0,0 +1,7 @@
+---
+apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+namespace: test
+namePrefix: test
+resources:
+  - secret.age.yaml

controllers/testdata/panic/secret.age.yaml

@@ -0,0 +1,27 @@
+apiVersion: ENC[AES256_GCM,data:RwzrBF8wy16SpfbQoeADeKyz,iv:DuJce2Ebx1Y49DaLCOJ74OOkgiv21roxhz/sZqKCSSs=,tag:Gg9XHapZI5q+rvtgeY6nrg==,type:str]
+kind: ENC[AES256_GCM,data:RwzrBF8wy16SpfbQoeADeKyz,iv:DuJce2Ebx1Y49DaLCOJ74OOkgiv21roxhz/sZqKCSSs=,tag:Gg9XHapZI5q+rvtgeY6nrg==,type:str]
+metadata:
+    name: ENC[AES256_GCM,data:RwzrBF8wy16SpfbQoeADeKyz,iv:DuJce2Ebx1Y49DaLCOJ74OOkgiv21roxhz/sZqKCSSs=,tag:Gg9XHapZI5q+rvtgeY6nrg==,type:str]
+    namespace: ENC[AES256_GCM,data:RwzrBF8wy16SpfbQoeADeKyz,iv:DuJce2Ebx1Y49DaLCOJ74OOkgiv21roxhz/sZqKCSSs=,tag:Gg9XHapZI5q+rvtgeY6nrg==,type:str]
+stringData:
+    secret: ENC[AES256_GCM,data:RwzrBF8wy16SpfbQoeADeKyz,iv:DuJce2Ebx1Y49DaLCOJ74OOkgiv21roxhz/sZqKCSSs=,tag:Gg9XHapZI5q+rvtgeY6nrg==,type:str]
+sops:
+    kms: []
+    gcp_kms: []
+    azure_kv: []
+    hc_vault: []
+    age:
+        - recipient: age1l44xcng8dqj32nlv6d930qvvrny05hglzcv9qpc7kxjc6902ma4qufys29
+          enc: |
+            -----BEGIN AGE ENCRYPTED FILE-----
+            YWdlLWVuY3J5cHRpb24ub3JnL3YxCi0+IFgyNTUxOSBNeGduOFZjRWw2WTFQdWdu
+            OS83OEZaN1E1aU1zSThhMlNEZzd0aEYvdURFCnE3bmJ5c3J2cDNEbXhselFPVC9v
+            NFhMRjZjOHZOdEpoYjdiS0ZPd2pvN1kKLS0tIDZUVEFoblpDNWhnaWxYRTBjaktk
+            bHRXV0o1K2ZDNm5Mem5SdzNBMTNuNFUKylE2cRLqydjj6e4+4Giwn4y8mIPej+CM
+            Bab3UWiK1da2rFNTOEnoHl6QDAVxNrWdrrIa5k22SzApT88VtJ4xuQ==
+            -----END AGE ENCRYPTED FILE-----
+    lastmodified: "2021-04-06T09:07:05Z"
+    mac: ENC[AES256_GCM,data:oaM8qFtEP8dOCd/Tr5yb08uetsnDtZO8o1rCayN53ncQ1HUAdhRBrFdmbYx1YTh1mwQVVN6sGYqFZU1LBMVv5pTqvpwd41biJZEg8NznXQWx0GA2Z6HOrblGhFZKrqky3P5xN+6j63zkJizXWgBMKzRvBnsVKxjZGr/lk1vVVv4=,iv:p4y9Fo3SArkEMuoK2d9sQYgNdc0iw/StFhg/5LnhcXM=,tag:61JGbnEw35tv6WnGj46JOw==,type:str]
+    pgp: []
+    encrypted_regex: ^(data|stringData)$
+    version: 3.7.0

go.mod

@@ -12,7 +12,7 @@ require (
 	github.com/cyphar/filepath-securejoin v0.2.3
 	github.com/dimchansky/utfbom v1.1.1
 	github.com/drone/envsubst v1.0.3
-	github.com/fluxcd/kustomize-controller/api v0.24.3
+	github.com/fluxcd/kustomize-controller/api v0.24.4
 	github.com/fluxcd/pkg/apis/acl v0.0.3
 	github.com/fluxcd/pkg/apis/kustomize v0.3.3
 	github.com/fluxcd/pkg/apis/meta v0.12.2