Merge pull request #570 from seh/tolerate-absent-post-build-subst-ref…

…erences

Tolerate absence of resources in post-build substitution

api/v1beta2/kustomization_types.go

@@ -17,10 +17,10 @@ limitations under the License.
 package v1beta2
 
 import (
-	apimeta "k8s.io/apimachinery/pkg/api/meta"
 	"time"
 
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	apimeta "k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 
@@ -207,6 +207,13 @@ type SubstituteReference struct {
 	// +kubebuilder:validation:MaxLength=253
 	// +required
 	Name string `json:"name"`
+
+	// Optional indicates whether the referenced resource must exist, or whether to
+	// tolerate its absence. If true and the referenced resource is absent, proceed
+	// as if the resource was present but empty, without any variables defined.
+	// +kubebuilder:default:=false
+	// +optional
+	Optional bool `json:"optional,omitempty"`
 }
 
 // KustomizationStatus defines the observed state of a kustomization.

config/crd/bases/kustomize.toolkit.fluxcd.io_kustomizations.yaml

@@ -894,6 +894,14 @@ spec:
                           maxLength: 253
                           minLength: 1
                           type: string
+                        optional:
+                          default: false
+                          description: Optional indicates whether the referenced resource
+                            must exist, or whether to tolerate its absence. If true
+                            and the referenced resource is absent, proceed as if the
+                            resource was present but empty, without any variables
+                            defined.
+                          type: boolean
                       required:
                       - kind
                       - name

controllers/kustomization_varsub.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/drone/envsubst"
 	corev1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/kustomize/api/resource"
@@ -48,26 +49,32 @@ func substituteVariables(
 		case "ConfigMap":
 			resource := &corev1.ConfigMap{}
 			if err := kubeClient.Get(ctx, namespacedName, resource); err != nil {
+				if reference.Optional && apierrors.IsNotFound(err) {
+					continue
+				}
 				return nil, fmt.Errorf("substitute from 'ConfigMap/%s' error: %w", reference.Name, err)
 			}
 			for k, v := range resource.Data {
-				vars[k] = strings.Replace(v, "\n", "", -1)
+				vars[k] = strings.ReplaceAll(v, "\n", "")
 			}
 		case "Secret":
 			resource := &corev1.Secret{}
 			if err := kubeClient.Get(ctx, namespacedName, resource); err != nil {
+				if reference.Optional && apierrors.IsNotFound(err) {
+					continue
+				}
 				return nil, fmt.Errorf("substitute from 'Secret/%s' error: %w", reference.Name, err)
 			}
 			for k, v := range resource.Data {
-				vars[k] = strings.Replace(string(v), "\n", "", -1)
+				vars[k] = strings.ReplaceAll(string(v), "\n", "")
 			}
 		}
 	}
 
 	// load in-line vars (overrides the ones from resources)
 	if kustomization.Spec.PostBuild.Substitute != nil {
 		for k, v := range kustomization.Spec.PostBuild.Substitute {
-			vars[k] = strings.Replace(v, "\n", "", -1)
+			vars[k] = strings.ReplaceAll(v, "\n", "")
 		}
 	}
 

controllers/kustomization_varsub_test.go

@@ -193,3 +193,159 @@ stringData:
 		g.Expect(resultSA.Labels[fmt.Sprintf("%s/namespace", kustomizev1.GroupVersion.Group)]).To(Equal(client.ObjectKeyFromObject(resultK).Namespace))
 	})
 }
+
+func TestKustomizationReconciler_VarsubOptional(t *testing.T) {
+	ctx := context.Background()
+
+	g := NewWithT(t)
+	id := "vars-" + randStringRunes(5)
+	revision := "v1.0.0/" + randStringRunes(7)
+
+	err := createNamespace(id)
+	g.Expect(err).NotTo(HaveOccurred(), "failed to create test namespace")
+
+	err = createKubeConfigSecret(id)
+	g.Expect(err).NotTo(HaveOccurred(), "failed to create kubeconfig secret")
+
+	manifests := func(name string) []testserver.File {
+		return []testserver.File{
+			{
+				Name: "service-account.yaml",
+				Body: fmt.Sprintf(`
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: %[1]s
+  namespace: %[1]s
+  labels:
+    color: "${color:=blue}"
+    shape: "${shape:=square}"
+`, name),
+			},
+		}
+	}
+
+	artifact, err := testServer.ArtifactFromFiles(manifests(id))
+	g.Expect(err).NotTo(HaveOccurred())
+
+	repositoryName := types.NamespacedName{
+		Name:      randStringRunes(5),
+		Namespace: id,
+	}
+
+	err = applyGitRepository(repositoryName, artifact, revision)
+	g.Expect(err).NotTo(HaveOccurred())
+
+	configName := types.NamespacedName{
+		Name:      randStringRunes(5),
+		Namespace: id,
+	}
+	configMap := &corev1.ConfigMap{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      configName.Name,
+			Namespace: configName.Namespace,
+		},
+		Data: map[string]string{"color": "\nred\n"},
+	}
+	g.Expect(k8sClient.Create(ctx, configMap)).Should(Succeed())
+
+	secretName := types.NamespacedName{
+		Name:      randStringRunes(5),
+		Namespace: id,
+	}
+	secret := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      secretName.Name,
+			Namespace: secretName.Namespace,
+		},
+		StringData: map[string]string{"shape": "\ntriangle\n"},
+	}
+	g.Expect(k8sClient.Create(ctx, secret)).Should(Succeed())
+
+	inputK := &kustomizev1.Kustomization{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      id,
+			Namespace: id,
+		},
+		Spec: kustomizev1.KustomizationSpec{
+			KubeConfig: &kustomizev1.KubeConfig{
+				SecretRef: meta.LocalObjectReference{
+					Name: "kubeconfig",
+				},
+			},
+			Interval: metav1.Duration{Duration: reconciliationInterval},
+			Path:     "./",
+			Prune:    true,
+			SourceRef: kustomizev1.CrossNamespaceSourceReference{
+				Kind: sourcev1.GitRepositoryKind,
+				Name: repositoryName.Name,
+			},
+			PostBuild: &kustomizev1.PostBuild{
+				Substitute: map[string]string{"var_substitution_enabled": "true"},
+				SubstituteFrom: []kustomizev1.SubstituteReference{
+					{
+						Kind:     "ConfigMap",
+						Name:     configName.Name,
+						Optional: true,
+					},
+					{
+						Kind:     "Secret",
+						Name:     secretName.Name,
+						Optional: true,
+					},
+				},
+			},
+			HealthChecks: []meta.NamespacedObjectKindReference{
+				{
+					APIVersion: "v1",
+					Kind:       "ServiceAccount",
+					Name:       id,
+					Namespace:  id,
+				},
+			},
+		},
+	}
+	g.Expect(k8sClient.Create(ctx, inputK)).Should(Succeed())
+
+	resultSA := &corev1.ServiceAccount{}
+
+	ensureReconciles := func(nameSuffix string) {
+		t.Run("reconciles successfully"+nameSuffix, func(t *testing.T) {
+			g.Eventually(func() bool {
+				resultK := &kustomizev1.Kustomization{}
+				_ = k8sClient.Get(ctx, client.ObjectKeyFromObject(inputK), resultK)
+				for _, c := range resultK.Status.Conditions {
+					if c.Reason == meta.ReconciliationSucceededReason {
+						return true
+					}
+				}
+				return false
+			}, timeout, interval).Should(BeTrue())
+
+			g.Expect(k8sClient.Get(ctx, types.NamespacedName{Name: id, Namespace: id}, resultSA)).Should(Succeed())
+		})
+	}
+
+	ensureReconciles(" with optional ConfigMap")
+	t.Run("replaces vars from optional ConfigMap", func(t *testing.T) {
+		g.Expect(resultSA.Labels["color"]).To(Equal("red"))
+		g.Expect(resultSA.Labels["shape"]).To(Equal("triangle"))
+	})
+
+	for _, o := range []client.Object{
+		configMap,
+		secret,
+	} {
+		g.Expect(k8sClient.Delete(ctx, o)).Should(Succeed())
+	}
+
+	// Force a second detectable reconciliation of the Kustomization.
+	g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(inputK), inputK)).Should(Succeed())
+	inputK.Status.Conditions = nil
+	g.Expect(k8sClient.Status().Update(ctx, inputK)).Should(Succeed())
+	ensureReconciles(" without optional ConfigMap")
+	t.Run("replaces vars tolerating absent ConfigMap", func(t *testing.T) {
+		g.Expect(resultSA.Labels["color"]).To(Equal("blue"))
+		g.Expect(resultSA.Labels["shape"]).To(Equal("square"))
+	})
+}

controllers/suite_test.go

@@ -121,8 +121,11 @@ func runInContext(registerControllers func(*testenv.Environment), run func() err
 		panic(fmt.Sprintf("Failed to create k8s client: %v", err))
 	}
 
-	// Create a vault test instance
+	// Create a Vault test instance.
 	pool, resource, err := createVaultTestInstance()
+	if err != nil {
+		panic(fmt.Sprintf("Failed to create Vault instance: %v", err))
+	}
 	defer func() {
 		pool.Purge(resource)
 	}()

docs/api/kustomize.md

@@ -1126,6 +1126,20 @@ string
 referring resource.</p>
 </td>
 </tr>
+<tr>
+<td>
+<code>optional</code><br>
+<em>
+bool
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>Optional indicates whether the referenced resource must exist, or whether to
+tolerate its absence. If true and the referenced resource is absent, proceed
+as if the resource was present but empty, without any variables defined.</p>
+</td>
+</tr>
 </tbody>
 </table>
 </div>

docs/spec/v1beta2/kustomization.md

@@ -285,10 +285,10 @@ On multi-tenant clusters, platform admins can disable cross-namespace references
 
 If your repository contains plain Kubernetes manifests, the
 `kustomization.yaml` file is automatically generated for all the Kubernetes
-manifests in the `spec.path` of the Flux `Kustomization` and sub-directories.
-This expects all YAML files present under that path to be valid kubernetes
-manifests and needs non-kubernetes ones to be excluded using `.sourceignore`
-file or `spec.ignore` on `GitRepository` object.
+manifests in the directory tree specified in the `spec.path` field of the Flux `Kustomization`.
+All YAML files present under that path must be valid Kubernetes
+manifests, unless they're excluded either by way of the `.sourceignore`
+file or the `spec.ignore` field on the corresponding `GitRepository` object.
 
 Example of excluding CI workflows and SOPS config files:
 
@@ -748,6 +748,16 @@ With `spec.postBuild.substituteFrom` you can provide a list of ConfigMaps and Se
 from which the variables are loaded.
 The ConfigMap and Secret data keys are used as the var names.
 
+The `spec.postBuild.substituteFrom.optional` field indicates how the
+controller should handle a referenced ConfigMap or Secret being absent
+at renconciliation time. The controller's default behavior ― with
+`optional` unspecified or set to `false` ― has it fail reconciliation if
+the referenced object is missing. By setting the `optional` field to
+`true`, you can indicate that controller should use the referenced
+object if it's there, but also tolerate its absence, treating that
+absence as if the object had been present but empty, defining no
+variables.
+
 This offers basic templating for your manifests including support
 for [bash string replacement functions](https://github.com/drone/envsubst) e.g.:
 
@@ -790,8 +800,11 @@ spec:
     substituteFrom:
       - kind: ConfigMap
         name: cluster-vars
+        # Use this ConfigMap if it exists, but proceed if it doesn't.
+        optional: true
       - kind: Secret
         name: cluster-secret-vars
+        # Fail if this Secret does not exist.
 ```
 
 Note that for substituting variables in a secret, `spec.stringData` field must be used i.e
@@ -1040,10 +1053,10 @@ spec:
 ### HashiCorp Vault
 
 Export the `VAULT_ADDR`  and `VAULT_TOKEN` environment variables to your shell,
-then use `sops` to encrypt a kubernetes secret (see [HashiCorp Vault](https://www.vaultproject.io/docs/secrets/transit)
+then use `sops` to encrypt a Kubernetes Secret (see [HashiCorp Vault](https://www.vaultproject.io/docs/secrets/transit)
 for more details on enabling the transit backend and [sops](https://github.com/mozilla/sops#encrypting-using-hashicorp-vault)).
 
-Then use `sops` to encrypt a kubernetes secret:
+Then use `sops` to encrypt a Kubernetes Secret:
 
 ```console
 $ export VAULT_ADDR=https://vault.example.com:8200