From 8d46755254e1ca58b9639ad897e74c45e64592ee Mon Sep 17 00:00:00 2001 From: Allen Porter Date: Sat, 2 Nov 2024 08:53:38 -0700 Subject: [PATCH] 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 --- flux_local/manifest.py | 9 +- flux_local/values.py | 4 +- tests/__snapshots__/test_values.ambr | 6 +- tests/test_values.py | 6 +- .../cluster/apps/prod/certificates.yaml | 28 +++ .../testdata/cluster/clusters/prod/apps.yaml | 2 + .../prod/flux-system/cluster-secrets.yaml | 9 + tests/tool/__snapshots__/test_build.ambr | 170 +++++++++++++++++- tests/tool/__snapshots__/test_diff_ks.ambr | 58 ++++++ 9 files changed, 278 insertions(+), 14 deletions(-) create mode 100644 tests/testdata/cluster/clusters/prod/flux-system/cluster-secrets.yaml diff --git a/flux_local/manifest.py b/flux_local/manifest.py index 88e50915..99915bee 100644 --- 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 index f110f28c..a8ee86a3 100644 --- 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 index 9133e805..913073e7 100644 --- 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 index b5f797af..fb56bc63 100644 --- 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 index 3d670d14..3177bc06 100644 --- 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 index c08d1fba..10790ee4 100644 --- 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 new file mode 100644 index 00000000..b25dac9f --- /dev/null +++ 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 index e2112cc7..4d71a5c9 100644 --- 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 diff --git a/tests/tool/__snapshots__/test_diff_ks.ambr b/tests/tool/__snapshots__/test_diff_ks.ambr index 3baf67e2..bcc7c6b1 100644 --- a/tests/tool/__snapshots__/test_diff_ks.ambr +++ b/tests/tool/__snapshots__/test_diff_ks.ambr @@ -165,6 +165,64 @@ - name: letsencrypt-staging - secretName: other-com-staging-tls - + - kustomization_path: tests/testdata/cluster/apps/prod + kind: Certificate + namespace: flux-system + name: -.PLACEHOLDER_SECRET_DOMAIN3..-staging + diff: |- + --- tests/testdata/cluster/apps/prod Kustomization: flux-system/apps Certificate: flux-system/-.PLACEHOLDER_SECRET_DOMAIN3..-staging + + +++ tests/testdata/cluster/apps/prod Kustomization: flux-system/apps Certificate: flux-system/-.PLACEHOLDER_SECRET_DOMAIN3..-staging + + @@ -1,18 +0,0 @@ + + ---- + -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 + -spec: + - commonName: ..PLACEHOLDER_SECRET_DOMAIN3.. + - dnsNames: + - - ..PLACEHOLDER_SECRET_DOMAIN3.. + - - '*...PLACEHOLDER_SECRET_DOMAIN3..' + - issuerRef: + - kind: ClusterIssuer + - name: letsencrypt-staging + - secretName: -.PLACEHOLDER_SECRET_DOMAIN3..-staging-tls + - + - kustomization_path: tests/testdata/cluster/apps/prod + kind: Certificate + namespace: flux-system + name: -.PLACEHOLDER_SECRET_DOMAIN4..-staging + diff: |- + --- tests/testdata/cluster/apps/prod Kustomization: flux-system/apps Certificate: flux-system/-.PLACEHOLDER_SECRET_DOMAIN4..-staging + + +++ tests/testdata/cluster/apps/prod Kustomization: flux-system/apps Certificate: flux-system/-.PLACEHOLDER_SECRET_DOMAIN4..-staging + + @@ -1,18 +0,0 @@ + + ---- + -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 + -spec: + - commonName: ..PLACEHOLDER_SECRET_DOMAIN4.. + - dnsNames: + - - ..PLACEHOLDER_SECRET_DOMAIN4.. + - - '*...PLACEHOLDER_SECRET_DOMAIN4..' + - issuerRef: + - kind: ClusterIssuer + - name: letsencrypt-staging + - secretName: -.PLACEHOLDER_SECRET_DOMAIN4..-staging-tls + - '''