From b21427b90c0a46c1af93f7a0c79e293420553a44 Mon Sep 17 00:00:00 2001 From: Ramon Quitales Date: Wed, 13 Dec 2023 15:15:57 -0800 Subject: [PATCH] Fix normalization of base64 encoded secrets.data values to strip whitespace (#2715) ### Proposed changes This pull request introduces a normalization mechanism for `secret.data` base64 encoded values, specifically stripping whitespace to prevent unintended spurious diffs. **Changes Made:** - Created comprehensive integration and unit tests to verify and highlight the broken behavior associated with `secret.data` values containing whitespaces. - Updated the provider to incorporate the normalization process, ensuring that all related tests pass seamlessly. ### Related issues (optional) Fixes: #2681 --- CHANGELOG.md | 1 + provider/pkg/clients/unstructured.go | 40 ++++++++++++++++++- provider/pkg/clients/unstructured_test.go | 26 ++++++++++++ tests/sdk/nodejs/nodejs_test.go | 36 +++++++++++++++++ tests/sdk/nodejs/secrets-new-line/Pulumi.yaml | 3 ++ tests/sdk/nodejs/secrets-new-line/index.ts | 30 ++++++++++++++ .../sdk/nodejs/secrets-new-line/package.json | 12 ++++++ .../sdk/nodejs/secrets-new-line/tsconfig.json | 22 ++++++++++ 8 files changed, 168 insertions(+), 2 deletions(-) create mode 100644 tests/sdk/nodejs/secrets-new-line/Pulumi.yaml create mode 100644 tests/sdk/nodejs/secrets-new-line/index.ts create mode 100644 tests/sdk/nodejs/secrets-new-line/package.json create mode 100644 tests/sdk/nodejs/secrets-new-line/tsconfig.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c40cc85f7..d1be350a10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ - Fix: Refine URN lookup by using its core type for more accurate resource identification (https://github.com/pulumi/pulumi-kubernetes/issues/2719) - Fix Helm Chart resource lookup key handling for objects in default namespace (https://github.com/pulumi/pulumi-kubernetes/pull/2655) - Fix panic when using `PULUMI_KUBERNETES_MANAGED_BY_LABEL` env var with SSA created objects (https://github.com/pulumi/pulumi-kubernetes/pull/2711) +- Fix normalization of base64 encoded secrets.data values to strip whitespace (https://github.com/pulumi/pulumi-kubernetes/issues/2715) ## 4.5.5 (November 28, 2023) - Fix: Make the invoke calls for Helm charts and YAML config resilient to the value being None or an empty dict (https://github.com/pulumi/pulumi-kubernetes/pull/2665) diff --git a/provider/pkg/clients/unstructured.go b/provider/pkg/clients/unstructured.go index 1c23e0d698..2aa33574a4 100644 --- a/provider/pkg/clients/unstructured.go +++ b/provider/pkg/clients/unstructured.go @@ -115,10 +115,15 @@ func normalizeSecret(uns *unstructured.Unstructured) *unstructured.Unstructured contract.Assertf(IsSecret(uns), "normalizeSecret called on a non-Secret resource: %s:%s", uns.GetAPIVersion(), uns.GetKind()) stringData, found, err := unstructured.NestedStringMap(uns.Object, "stringData") - if err != nil || !found { - return uns + if err != nil || !found || len(stringData) == 0 { + // Normalize the .data field if .stringData is not present or empty. + return normalizeSecretData(uns) } + return normalizeSecretStringData(stringData, uns) +} + +func normalizeSecretStringData(stringData map[string]string, uns *unstructured.Unstructured) *unstructured.Unstructured { data, found, err := unstructured.NestedMap(uns.Object, "data") if err != nil || !found { data = map[string]any{} @@ -138,6 +143,37 @@ func normalizeSecret(uns *unstructured.Unstructured) *unstructured.Unstructured return uns } +// normalizeSecretData normalizes the .data field of a Secret resource by trimming whitespace from string values. +// This is necessary because the apiserver will trim whitespace from the .data field values, but the provider does not. +func normalizeSecretData(uns *unstructured.Unstructured) *unstructured.Unstructured { + data, found, err := unstructured.NestedMap(uns.Object, "data") + if err != nil || !found || len(data) == 0 { + return uns + } + + for k, v := range data { + if s, ok := v.(string); ok { + // Trim whitespace from the string value, for consistency with the apiserver which + // does the decoding and re-encoding to validate the value provided is valid base64. + // See: https://github.com/kubernetes/kubernetes/blob/41890534532931742770a7dc98f78bcdc59b1a6f/staging/src/k8s.io/apimachinery/pkg/runtime/codec.go#L212-L260 + base64Decoded, err := base64.StdEncoding.DecodeString(s) + if err != nil { + // TODO: propagate error upwards to parent Normalize function to fail early. It is safe to + // ignore this error for now, since the apiserver will reject the resource if the value cannot + // be decoded. + continue + } + base64ReEncoded := base64.StdEncoding.EncodeToString(base64Decoded) + + data[k] = base64ReEncoded + } + } + + contract.IgnoreError(unstructured.SetNestedMap(uns.Object, data, "data")) + + return uns +} + func PodFromUnstructured(uns *unstructured.Unstructured) (*corev1.Pod, error) { const expectedAPIVersion = "v1" diff --git a/provider/pkg/clients/unstructured_test.go b/provider/pkg/clients/unstructured_test.go index fc3e6081e1..3e113ef7cb 100644 --- a/provider/pkg/clients/unstructured_test.go +++ b/provider/pkg/clients/unstructured_test.go @@ -284,6 +284,31 @@ var ( }, }, } + + secretNewLineUnstructured = &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "v1", + "kind": "Secret", + "metadata": map[string]any{ + "name": "foo"}, + "data": map[string]any{ + "foo": "dGhpcyBpcyBhIHRlc3Qgc3RyaW5n\n", + }, + }, + } + + secretNewLineNormalizedUnstructured = &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "v1", + "kind": "Secret", + "metadata": map[string]any{ + "name": "foo", + }, + "data": map[string]any{ + "foo": "dGhpcyBpcyBhIHRlc3Qgc3RyaW5n", + }, + }, + } ) func TestFromUnstructured(t *testing.T) { @@ -329,6 +354,7 @@ func TestNormalize(t *testing.T) { {"CRD with status", args{uns: crdStatusUnstructured}, crdUnstructured, false}, {"Secret with stringData input", args{uns: secretUnstructured}, secretNormalizedUnstructured, false}, {"Secret with data input", args{uns: secretNormalizedUnstructured}, secretNormalizedUnstructured, false}, + {"Secret with data containing trailing new line", args{uns: secretNewLineUnstructured}, secretNewLineNormalizedUnstructured, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/tests/sdk/nodejs/nodejs_test.go b/tests/sdk/nodejs/nodejs_test.go index 7d52d510af..30187b98a1 100644 --- a/tests/sdk/nodejs/nodejs_test.go +++ b/tests/sdk/nodejs/nodejs_test.go @@ -1133,6 +1133,42 @@ func TestSecrets(t *testing.T) { integration.ProgramTest(t, &test) } +// TestSecretDataNewLine tests that secrets with new lines in the base64 encoding are handled correctly. +// See: https://github.com/pulumi/pulumi-kubernetes/issues/2681 +func TestSecretDataNewLine(t *testing.T) { + test := baseOptions.With(integration.ProgramTestOptions{ + Dir: "secrets-new-line", + ExpectRefreshChanges: false, + SkipRefresh: false, + OrderedConfig: []integration.ConfigValue{ + { + Key: "pulumi:disable-default-providers[0]", + Value: "kubernetes", + Path: true, + }, + }, + ExtraRuntimeValidation: func(t *testing.T, stackInfo integration.RuntimeValidationStackInfo) { + assert.NotNil(t, stackInfo.Deployment) + + data, ok := stackInfo.Outputs["data"] + assert.Truef(t, ok, "missing expected output \"data\"") + + stringData, ok := stackInfo.Outputs["stringData"] + assert.Falsef(t, ok, "unexpected non-empty output: \"stringData\"") + + assert.NotEmptyf(t, data, "data field is empty") + assert.Emptyf(t, stringData, "stringData field is not empty") + + }, + EditDirs: []integration.EditDir{{ + Dir: "secrets-new-line", + ExpectNoChanges: true, // Re-running the same program should not cause any changes. + Additive: true, + }}, + }) + integration.ProgramTest(t, &test) +} + func TestServerSideApply(t *testing.T) { test := baseOptions.With(integration.ProgramTestOptions{ Dir: filepath.Join("server-side-apply", "step1"), diff --git a/tests/sdk/nodejs/secrets-new-line/Pulumi.yaml b/tests/sdk/nodejs/secrets-new-line/Pulumi.yaml new file mode 100644 index 0000000000..2c5ae3255d --- /dev/null +++ b/tests/sdk/nodejs/secrets-new-line/Pulumi.yaml @@ -0,0 +1,3 @@ +name: secrets-new-line +description: Tests secrets support with newline at end of data +runtime: nodejs diff --git a/tests/sdk/nodejs/secrets-new-line/index.ts b/tests/sdk/nodejs/secrets-new-line/index.ts new file mode 100644 index 0000000000..970df4a878 --- /dev/null +++ b/tests/sdk/nodejs/secrets-new-line/index.ts @@ -0,0 +1,30 @@ +// Copyright 2016-2022, Pulumi Corporation. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import * as pulumi from "@pulumi/pulumi"; +import * as k8s from "@pulumi/kubernetes"; + +const config = new pulumi.Config(); + +const provider = new k8s.Provider("k8s"); + +const newlineSecret = new k8s.core.v1.Secret("newline", { + data: { + password: "dGhpcyBpcyBhIHRlc3Qgc3RyaW5n\n\n\n", // "decoded base64 value: 'this is a test string'" + } +}, {provider}); + + +export const stringData = newlineSecret.stringData; +export const data = newlineSecret.data; diff --git a/tests/sdk/nodejs/secrets-new-line/package.json b/tests/sdk/nodejs/secrets-new-line/package.json new file mode 100644 index 0000000000..2c5e06cbde --- /dev/null +++ b/tests/sdk/nodejs/secrets-new-line/package.json @@ -0,0 +1,12 @@ +{ + "name": "secrets", + "version": "0.1.0", + "dependencies": { + "@pulumi/pulumi": "latest" + }, + "devDependencies": { + }, + "peerDependencies": { + "@pulumi/kubernetes": "latest" + } +} diff --git a/tests/sdk/nodejs/secrets-new-line/tsconfig.json b/tests/sdk/nodejs/secrets-new-line/tsconfig.json new file mode 100644 index 0000000000..5dacccbd42 --- /dev/null +++ b/tests/sdk/nodejs/secrets-new-line/tsconfig.json @@ -0,0 +1,22 @@ +{ + "compilerOptions": { + "outDir": "bin", + "target": "es6", + "module": "commonjs", + "moduleResolution": "node", + "declaration": true, + "sourceMap": true, + "stripInternal": true, + "experimentalDecorators": true, + "pretty": true, + "noFallthroughCasesInSwitch": true, + "noImplicitAny": true, + "noImplicitReturns": true, + "forceConsistentCasingInFileNames": true, + "strictNullChecks": true + }, + "files": [ + "index.ts" + ] +} +