Set unique placeholder values for each substitution (#801)

Fix an issue where substitution values (that are from secrets or can't be found) were not unique which may result in resource names that are set with the same value. This may mask bugs if users do not set resource values to the same thing when using secrets. Fixes #754
allenporter · Nov 2, 2024 · 8d46755 · 8d46755
1 parent 0544971
commit 8d46755
Show file tree

Hide file tree

Showing 9 changed files with 278 additions and 14 deletions.
diff --git a/flux_local/manifest.py b/flux_local/manifest.py
@@ -47,8 +47,7 @@
 SECRET_KIND = "Secret"
 CONFIG_MAP_KIND = "ConfigMap"
 DEFAULT_NAMESPACE = "flux-system"
-VALUE_PLACEHOLDER = "..PLACEHOLDER.."
-VALUE_B64_PLACEHOLDER = base64.b64encode(VALUE_PLACEHOLDER.encode())
+VALUE_PLACEHOLDER_TEMPLATE = "..PLACEHOLDER_{name}.."
 HELM_REPOSITORY = "HelmRepository"
 GIT_REPOSITORY = "GitRepository"
 OCI_REPOSITORY = "OCIRepository"
@@ -430,10 +429,12 @@ def parse_doc(cls, doc: dict[str, Any]) -> "Secret":
         # placeholder values anyway.
         if data := doc.get("data"):
             for key, value in data.items():
-                data[key] = VALUE_B64_PLACEHOLDER
+                data[key] = base64.b64encode(
+                    VALUE_PLACEHOLDER_TEMPLATE.format(name=key).encode()
+                )
         if string_data := doc.get("stringData"):
             for key, value in string_data.items():
-                string_data[key] = VALUE_PLACEHOLDER
+                string_data[key] = VALUE_PLACEHOLDER_TEMPLATE.format(name=key)
         return Secret(
             name=name, namespace=namespace, data=data, string_data=string_data
         )

diff --git a/flux_local/values.py b/flux_local/values.py
@@ -14,7 +14,7 @@
     CONFIG_MAP_KIND,
     ConfigMap,
     Secret,
-    VALUE_PLACEHOLDER,
+    VALUE_PLACEHOLDER_TEMPLATE,
     ValuesReference,
 )
 from .exceptions import HelmException, InputException, InvalidValuesReference
@@ -163,7 +163,7 @@ def _lookup_value_reference(
             # When a target path is specified, the value is expected to be
             # a simple value type. Create a synthetic placeholder value, otherwise
             # there is nothing to replace.
-            return VALUE_PLACEHOLDER
+            return VALUE_PLACEHOLDER_TEMPLATE.format(name=ref.name)
         return None
 
     elif (found_value := found_data.get(ref.values_key)) is None:

diff --git a/tests/__snapshots__/test_values.ambr b/tests/__snapshots__/test_values.ambr
@@ -22,7 +22,7 @@
       'tag': '7.0.6',
     }),
     'tls': dict({
-      'crt': '..PLACEHOLDER..',
+      'crt': '..PLACEHOLDER_podinfo-tls-values..',
     }),
   })
 # ---
@@ -32,8 +32,8 @@
       'mode': 'true',
     }),
     'oauth': dict({
-      'clientId': '..PLACEHOLDER..',
-      'clientSecret': '..PLACEHOLDER..',
+      'clientId': '..PLACEHOLDER_tailscale-operator..',
+      'clientSecret': '..PLACEHOLDER_tailscale-operator..',
     }),
     'operatorConfig': dict({
       'defaultTags': list([

diff --git a/tests/test_values.py b/tests/test_values.py
@@ -172,7 +172,7 @@ def test_values_references_with_missing_secret() -> None:
     assert updated_hr.values == {
         "test": "test",
         "target": {
-            "path": "..PLACEHOLDER..",
+            "path": "..PLACEHOLDER_test-values-secret..",
         },
     }
 
@@ -437,8 +437,8 @@ def test_values_references_secret() -> None:
     assert updated_hr.values == {
         "test": "test",
         "target": {
-            "path1": "..PLACEHOLDER..",
-            "path2": "..PLACEHOLDER..",
+            "path1": "..PLACEHOLDER_some-key1..",
+            "path2": "..PLACEHOLDER_some-key2..",
         },
     }
 

diff --git a/tests/testdata/cluster/apps/prod/certificates.yaml b/tests/testdata/cluster/apps/prod/certificates.yaml
@@ -27,3 +27,31 @@ spec:
     - "${SECRET_DOMAIN2}"
     - "*.${SECRET_DOMAIN2}"
     - ${cluster_label}
+---
+apiVersion: cert-manager.io/v1
+kind: Certificate
+metadata:
+  name: "${SECRET_DOMAIN3/./-}-staging"
+spec:
+  secretName: "${SECRET_DOMAIN3/./-}-staging-tls"
+  issuerRef:
+    name: letsencrypt-staging
+    kind: ClusterIssuer
+  commonName: "${SECRET_DOMAIN3}"
+  dnsNames:
+    - "${SECRET_DOMAIN3}"
+    - "*.${SECRET_DOMAIN3}"
+---
+apiVersion: cert-manager.io/v1
+kind: Certificate
+metadata:
+  name: "${SECRET_DOMAIN4/./-}-staging"
+spec:
+  secretName: "${SECRET_DOMAIN4/./-}-staging-tls"
+  issuerRef:
+    name: letsencrypt-staging
+    kind: ClusterIssuer
+  commonName: "${SECRET_DOMAIN4}"
+  dnsNames:
+    - "${SECRET_DOMAIN4}"
+    - "*.${SECRET_DOMAIN4}"
diff --git a/tests/testdata/cluster/clusters/prod/apps.yaml b/tests/testdata/cluster/clusters/prod/apps.yaml
@@ -19,6 +19,8 @@ spec:
     substituteFrom:
       - kind: ConfigMap
         name: cluster-config
+      - kind: Secret
+        name: cluster-secrets
   prune: true
   wait: true
   timeout: 5m0s
diff --git a/tests/testdata/cluster/clusters/prod/flux-system/cluster-secrets.yaml b/tests/testdata/cluster/clusters/prod/flux-system/cluster-secrets.yaml
@@ -0,0 +1,9 @@
+---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: cluster-secrets
+  namespace: flux-system
+stringData:
+  SECRET_DOMAIN3: SECRET
+  SECRET_DOMAIN4: SECRET
diff --git a/tests/tool/__snapshots__/test_build.ambr b/tests/tool/__snapshots__/test_build.ambr
@@ -1149,8 +1149,8 @@
       config.kubernetes.io/index: '2'
       internal.config.kubernetes.io/index: '2'
   stringData:
-    client_id: ..PLACEHOLDER..
-    client_secret: ..PLACEHOLDER..
+    client_id: ..PLACEHOLDER_tailscale-operator..
+    client_secret: ..PLACEHOLDER_tailscale-operator..
   ---
   # Source: tailscale-operator/templates/apiserverproxy-rbac.yaml
   # Copyright (c) Tailscale Inc & AUTHORS
@@ -2943,6 +2943,46 @@
       kind: ClusterIssuer
       name: letsencrypt-staging
     secretName: other-com-staging-tls
+  ---
+  apiVersion: cert-manager.io/v1
+  kind: Certificate
+  metadata:
+    labels:
+      kustomize.toolkit.fluxcd.io/name: apps
+      kustomize.toolkit.fluxcd.io/namespace: flux-system
+    name: -.PLACEHOLDER_SECRET_DOMAIN3..-staging
+    annotations:
+      config.kubernetes.io/index: '5'
+      internal.config.kubernetes.io/index: '5'
+  spec:
+    commonName: ..PLACEHOLDER_SECRET_DOMAIN3..
+    dnsNames:
+    - ..PLACEHOLDER_SECRET_DOMAIN3..
+    - '*...PLACEHOLDER_SECRET_DOMAIN3..'
+    issuerRef:
+      kind: ClusterIssuer
+      name: letsencrypt-staging
+    secretName: -.PLACEHOLDER_SECRET_DOMAIN3..-staging-tls
+  ---
+  apiVersion: cert-manager.io/v1
+  kind: Certificate
+  metadata:
+    labels:
+      kustomize.toolkit.fluxcd.io/name: apps
+      kustomize.toolkit.fluxcd.io/namespace: flux-system
+    name: -.PLACEHOLDER_SECRET_DOMAIN4..-staging
+    annotations:
+      config.kubernetes.io/index: '6'
+      internal.config.kubernetes.io/index: '6'
+  spec:
+    commonName: ..PLACEHOLDER_SECRET_DOMAIN4..
+    dnsNames:
+    - ..PLACEHOLDER_SECRET_DOMAIN4..
+    - '*...PLACEHOLDER_SECRET_DOMAIN4..'
+    issuerRef:
+      kind: ClusterIssuer
+      name: letsencrypt-staging
+    secretName: -.PLACEHOLDER_SECRET_DOMAIN4..-staging-tls
 
   ---
   apiVersion: kustomize.toolkit.fluxcd.io/v1
@@ -2967,6 +3007,8 @@
       substituteFrom:
       - kind: ConfigMap
         name: cluster-config
+      - kind: Secret
+        name: cluster-secrets
     prune: true
     sourceRef:
       kind: GitRepository
@@ -4686,6 +4728,46 @@
       kind: ClusterIssuer
       name: letsencrypt-staging
     secretName: other-com-staging-tls
+  ---
+  apiVersion: cert-manager.io/v1
+  kind: Certificate
+  metadata:
+    labels:
+      kustomize.toolkit.fluxcd.io/name: apps
+      kustomize.toolkit.fluxcd.io/namespace: flux-system
+    name: -.PLACEHOLDER_SECRET_DOMAIN3..-staging
+    annotations:
+      config.kubernetes.io/index: '5'
+      internal.config.kubernetes.io/index: '5'
+  spec:
+    commonName: ..PLACEHOLDER_SECRET_DOMAIN3..
+    dnsNames:
+    - ..PLACEHOLDER_SECRET_DOMAIN3..
+    - '*...PLACEHOLDER_SECRET_DOMAIN3..'
+    issuerRef:
+      kind: ClusterIssuer
+      name: letsencrypt-staging
+    secretName: -.PLACEHOLDER_SECRET_DOMAIN3..-staging-tls
+  ---
+  apiVersion: cert-manager.io/v1
+  kind: Certificate
+  metadata:
+    labels:
+      kustomize.toolkit.fluxcd.io/name: apps
+      kustomize.toolkit.fluxcd.io/namespace: flux-system
+    name: -.PLACEHOLDER_SECRET_DOMAIN4..-staging
+    annotations:
+      config.kubernetes.io/index: '6'
+      internal.config.kubernetes.io/index: '6'
+  spec:
+    commonName: ..PLACEHOLDER_SECRET_DOMAIN4..
+    dnsNames:
+    - ..PLACEHOLDER_SECRET_DOMAIN4..
+    - '*...PLACEHOLDER_SECRET_DOMAIN4..'
+    issuerRef:
+      kind: ClusterIssuer
+      name: letsencrypt-staging
+    secretName: -.PLACEHOLDER_SECRET_DOMAIN4..-staging-tls
 
   ---
   apiVersion: kustomize.toolkit.fluxcd.io/v1
@@ -4710,6 +4792,8 @@
       substituteFrom:
       - kind: ConfigMap
         name: cluster-config
+      - kind: Secret
+        name: cluster-secrets
     prune: true
     sourceRef:
       kind: GitRepository
@@ -6529,6 +6613,46 @@
       kind: ClusterIssuer
       name: letsencrypt-staging
     secretName: other-com-staging-tls
+  ---
+  apiVersion: cert-manager.io/v1
+  kind: Certificate
+  metadata:
+    labels:
+      kustomize.toolkit.fluxcd.io/name: apps
+      kustomize.toolkit.fluxcd.io/namespace: flux-system
+    name: -.PLACEHOLDER_SECRET_DOMAIN3..-staging
+    annotations:
+      config.kubernetes.io/index: '5'
+      internal.config.kubernetes.io/index: '5'
+  spec:
+    commonName: ..PLACEHOLDER_SECRET_DOMAIN3..
+    dnsNames:
+    - ..PLACEHOLDER_SECRET_DOMAIN3..
+    - '*...PLACEHOLDER_SECRET_DOMAIN3..'
+    issuerRef:
+      kind: ClusterIssuer
+      name: letsencrypt-staging
+    secretName: -.PLACEHOLDER_SECRET_DOMAIN3..-staging-tls
+  ---
+  apiVersion: cert-manager.io/v1
+  kind: Certificate
+  metadata:
+    labels:
+      kustomize.toolkit.fluxcd.io/name: apps
+      kustomize.toolkit.fluxcd.io/namespace: flux-system
+    name: -.PLACEHOLDER_SECRET_DOMAIN4..-staging
+    annotations:
+      config.kubernetes.io/index: '6'
+      internal.config.kubernetes.io/index: '6'
+  spec:
+    commonName: ..PLACEHOLDER_SECRET_DOMAIN4..
+    dnsNames:
+    - ..PLACEHOLDER_SECRET_DOMAIN4..
+    - '*...PLACEHOLDER_SECRET_DOMAIN4..'
+    issuerRef:
+      kind: ClusterIssuer
+      name: letsencrypt-staging
+    secretName: -.PLACEHOLDER_SECRET_DOMAIN4..-staging-tls
 
 
   '''
@@ -6642,6 +6766,46 @@
       kind: ClusterIssuer
       name: letsencrypt-staging
     secretName: other-com-staging-tls
+  ---
+  apiVersion: cert-manager.io/v1
+  kind: Certificate
+  metadata:
+    labels:
+      kustomize.toolkit.fluxcd.io/name: apps
+      kustomize.toolkit.fluxcd.io/namespace: flux-system
+    name: -.PLACEHOLDER_SECRET_DOMAIN3..-staging
+    annotations:
+      config.kubernetes.io/index: '5'
+      internal.config.kubernetes.io/index: '5'
+  spec:
+    commonName: ..PLACEHOLDER_SECRET_DOMAIN3..
+    dnsNames:
+    - ..PLACEHOLDER_SECRET_DOMAIN3..
+    - '*...PLACEHOLDER_SECRET_DOMAIN3..'
+    issuerRef:
+      kind: ClusterIssuer
+      name: letsencrypt-staging
+    secretName: -.PLACEHOLDER_SECRET_DOMAIN3..-staging-tls
+  ---
+  apiVersion: cert-manager.io/v1
+  kind: Certificate
+  metadata:
+    labels:
+      kustomize.toolkit.fluxcd.io/name: apps
+      kustomize.toolkit.fluxcd.io/namespace: flux-system
+    name: -.PLACEHOLDER_SECRET_DOMAIN4..-staging
+    annotations:
+      config.kubernetes.io/index: '6'
+      internal.config.kubernetes.io/index: '6'
+  spec:
+    commonName: ..PLACEHOLDER_SECRET_DOMAIN4..
+    dnsNames:
+    - ..PLACEHOLDER_SECRET_DOMAIN4..
+    - '*...PLACEHOLDER_SECRET_DOMAIN4..'
+    issuerRef:
+      kind: ClusterIssuer
+      name: letsencrypt-staging
+    secretName: -.PLACEHOLDER_SECRET_DOMAIN4..-staging-tls
 
   ---
   apiVersion: kustomize.toolkit.fluxcd.io/v1
@@ -6666,6 +6830,8 @@
       substituteFrom:
       - kind: ConfigMap
         name: cluster-config
+      - kind: Secret
+        name: cluster-secrets
     prune: true
     sourceRef:
       kind: GitRepository