Fix normalization of base64 encoded secrets.data values to strip whit…

…espace (#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

@@ -5,6 +5,7 @@
 - Fix Helm Chart resource lookup key handling for objects in default namespace (https://github.com/pulumi/pulumi-kubernetes/pull/2655)
 - Update Kubernetes schemas and libraries to v1.29.0 (https://github.com/pulumi/pulumi-kubernetes/pull/2690)
 - 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)

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"
 

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) {

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"),

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

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;

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"
+    }
+}

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"
+    ]
+}
+