Gracefully handle ignoreChanges without user needing to refresh a stack

provider/pkg/await/await.go

@@ -445,6 +445,10 @@ func updateResource(c *UpdateConfig, liveOldObj *unstructured.Unstructured, clie
 
 // csaUpdate handles the logic for updating a resource using client-side apply.
 func csaUpdate(c *UpdateConfig, liveOldObj *unstructured.Unstructured, client dynamic.ResourceInterface) (*unstructured.Unstructured, error) {
+	// Handle ignoreChanges for CSA to use the last known value applied to the cluster, rather than what's in state which may be outdated.
+	// We ignore errors here as it occurs when there is an issue traversing the field path. If this occurs, then use the last value in state
+	// optimistically rather than failing the update.
+	_ = handleCSAIgnoreFields(c, liveOldObj)
 	// Create merge patch (prefer strategic merge patch, fall back to JSON merge patch).
 	patch, patchType, _, err := openapi.PatchForResourceUpdate(c.Resources, c.Previous, c.Inputs, liveOldObj)
 	if err != nil {
@@ -498,13 +502,43 @@ func ssaUpdate(c *UpdateConfig, liveOldObj *unstructured.Unstructured, client dy
 	return currentOutputs, nil
 }
 
+// handleCSAIgnoreFields handles updating the inputs to use the last known value applied to the cluster. If the value is not present,
+// then we use what is declared in state as per the specs of Pulumi's ignoreChanges.
+func handleCSAIgnoreFields(c *UpdateConfig, liveOldObj *unstructured.Unstructured) error {
+	for _, ignorePath := range c.IgnoreChanges {
+		ipParsed, err := resource.ParsePropertyPath(ignorePath)
+		if err != nil {
+			// NB: This shouldn't really happen since we already validated the ignoreChanges paths in the parent Diff function.
+			return fmt.Errorf("unable to parse ignoreField path %q: %w", ignorePath, err)
+		}
+
+		pathComponents := strings.Split(ipParsed.String(), ".")
+
+		lastLiveVal, found, err := unstructured.NestedFieldCopy(liveOldObj.Object, pathComponents...)
+		if found && err == nil {
+			// We only care if the field is found, as not found indicates that the field does not exist in the live state so we don't have to worry about changing the inputs to match
+			// the live state.
+			err := unstructured.SetNestedField(c.Inputs.Object, lastLiveVal, pathComponents...)
+			if err != nil {
+				return fmt.Errorf("unable to set field %q with last used value %q: %w", ignorePath, lastLiveVal, err)
+			}
+		}
+		if err != nil {
+			// A type error occurred when attempting to get the nested field from the live object.
+			return fmt.Errorf("unable to parse field to ignore %q from live object: %w", ignorePath, err)
+		}
+	}
+
+	return nil
+}
+
 // handleSSAIgnoreFields handles updating the inputs to either drop fields that are present on the cluster and not managed
 // by the current field manager, or to set the value of the field to the last known value applied to the cluster.
 func handleSSAIgnoreFields(c *UpdateConfig, liveOldObj *unstructured.Unstructured) error {
 	managedFields := liveOldObj.GetManagedFields()
 	// Keep track of fields that are managed by the current field manager, and fields that are managed by other field managers.
-	managedFieldsSet := fieldpath.NewSet()
-	currManagerFieldsSet := fieldpath.NewSet()
+	theirFields, ourFields := new(fieldpath.Set), new(fieldpath.Set)
+	fieldpath.MakePathOrDie()
 
 	for _, f := range managedFields {
 		s, err := fluxssa.FieldsToSet(*f.FieldsV1)
@@ -514,9 +548,9 @@ func handleSSAIgnoreFields(c *UpdateConfig, liveOldObj *unstructured.Unstructure
 
 		switch f.Manager {
 		case c.FieldManager:
-			currManagerFieldsSet = currManagerFieldsSet.Union(&s)
+			ourFields = ourFields.Union(&s)
 		default:
-			managedFieldsSet = managedFieldsSet.Union(&s)
+			theirFields = theirFields.Union(&s)
 		}
 	}
 
@@ -527,6 +561,7 @@ func handleSSAIgnoreFields(c *UpdateConfig, liveOldObj *unstructured.Unstructure
 			return fmt.Errorf("unable to parse ignoreField path %q: %w", ignorePath, err)
 		}
 
+		// TODO: Enhance support for ignoreField path to support nested arrays.
 		pathComponents := strings.Split(ipParsed.String(), ".")
 		pe, err := fieldpath.MakePath(makeInterfaceSlice(pathComponents)...)
 		if err != nil {
@@ -535,7 +570,7 @@ func handleSSAIgnoreFields(c *UpdateConfig, liveOldObj *unstructured.Unstructure
 
 		// Drop the field from the inputs if it is present on the cluster and managed by another manager, and is not shared with current manager. This ensures
 		// that we don't get any conflict errors, or mistakenly setting the current field manager as a shared manager of that field.
-		if managedFieldsSet.Has(pe) && !currManagerFieldsSet.Has(pe) {
+		if theirFields.Has(pe) && !ourFields.Has(pe) {
 			unstructured.RemoveNestedField(c.Inputs.Object, pathComponents...)
 			continue
 		}
@@ -551,18 +586,19 @@ func handleSSAIgnoreFields(c *UpdateConfig, liveOldObj *unstructured.Unstructure
 		//
 		// NOTE: If the field has been reverted to its default value, ignoreChanges will still not update this field to what is supplied
 		// by the user in their Pulumi program.
-		lastVal, found, err := unstructured.NestedFieldCopy(liveOldObj.Object, pathComponents...)
+		lastLiveVal, found, err := unstructured.NestedFieldCopy(liveOldObj.Object, pathComponents...)
 		if found && err == nil {
 			// We only care if the field is found, as not found indicates that the field does not exist in the live state so we don't have to worry about changing the inputs to match
-			// the live state. If this occurs, then Pulumi will set the field back to the declared value. Or should we also ensure that the field is never touch again by Pulumi?
-			err := unstructured.SetNestedField(c.Inputs.Object, lastVal, pathComponents...)
+			// the live state. If this occurs, then Pulumi will set the field back to the declared value as ignoreChanges will use the declared value if one is not found in state as per
+			// the intent of ignoreChanges.
+			err := unstructured.SetNestedField(c.Inputs.Object, lastLiveVal, pathComponents...)
 			if err != nil {
-				return fmt.Errorf("unable to set field %q with last used value %q: %w", ignorePath, lastVal, err)
+				return fmt.Errorf("unable to set field %q with last used value %q: %w", ignorePath, lastLiveVal, err)
 			}
 		}
 		if err != nil {
 			// A type error occurred when attempting to get the nested field from the live object.
-			return fmt.Errorf("unable to get field %q from live object: %w", ignorePath, err)
+			return fmt.Errorf("unable to parse field to ignore %q from live object: %w", ignorePath, err)
 		}
 	}
 

tests/sdk/nodejs/ignore-changes-csa/step1/index.ts

@@ -0,0 +1,50 @@
+// Copyright 2016-2023, 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 k8s from "@pulumi/kubernetes";
+
+// Create provider with SSA enabled.
+const provider = new k8s.Provider("k8s", {enableServerSideApply: false});
+
+const ns = new k8s.core.v1.Namespace("test-ignore-changes", undefined, { provider });
+
+const deployment = new k8s.apps.v1.Deployment(
+  "test-ignore-changes",
+  {
+    metadata: {
+      namespace: ns.metadata.name,
+    },
+    spec: {
+      selector: { matchLabels: { app: "test-ignore-changes" } },
+      replicas: 2,
+      template: {
+        metadata: {
+          labels: { app: "test-ignore-changes" },
+        },
+        spec: {
+          containers: [
+            {
+              name: "nginx",
+              image: "nginx:1.25.2",
+            },
+          ],
+        },
+      },
+    },
+  },
+  { provider: provider, ignoreChanges: ["spec.replicas"] }
+);
+
+export const deploymentName = deployment.metadata.name;
+export const deploymentNamespace = deployment.metadata.namespace;

tests/sdk/nodejs/ignore-changes-csa/step2/index.ts

@@ -0,0 +1,50 @@
+// Copyright 2016-2023, 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 k8s from "@pulumi/kubernetes";
+
+// Create provider with SSA enabled.
+const provider = new k8s.Provider("k8s", {enableServerSideApply: false});
+
+const ns = new k8s.core.v1.Namespace("test-ignore-changes", undefined, { provider });
+
+const deployment = new k8s.apps.v1.Deployment(
+  "test-ignore-changes",
+  {
+    metadata: {
+      namespace: ns.metadata.name,
+    },
+    spec: {
+      selector: { matchLabels: { app: "test-ignore-changes" } },
+      replicas: 2,
+      template: {
+        metadata: {
+          labels: { app: "test-ignore-changes" },
+        },
+        spec: {
+          containers: [
+            {
+              name: "nginx",
+              image: "nginx:1.25.1",
+            },
+          ],
+        },
+      },
+    },
+  },
+  { provider: provider, ignoreChanges: ["spec.replicas"] }
+);
+
+export const deploymentName = deployment.metadata.name;
+export const deploymentNamespace = deployment.metadata.namespace;

tests/sdk/nodejs/ignore-changes-csa/step3/index.ts

@@ -0,0 +1,50 @@
+// Copyright 2016-2023, 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 k8s from "@pulumi/kubernetes";
+
+// Create provider with SSA enabled.
+const provider = new k8s.Provider("k8s", {enableServerSideApply: false});
+
+const ns = new k8s.core.v1.Namespace("test-ignore-changes", undefined, { provider });
+
+const deployment = new k8s.apps.v1.Deployment(
+  "test-ignore-changes",
+  {
+    metadata: {
+      namespace: ns.metadata.name,
+    },
+    spec: {
+      selector: { matchLabels: { app: "test-ignore-changes" } },
+      replicas: 2,
+      template: {
+        metadata: {
+          labels: { app: "test-ignore-changes" },
+        },
+        spec: {
+          containers: [
+            {
+              name: "nginx",
+              image: "nginx:1.25",
+            },
+          ],
+        },
+      },
+    },
+  },
+  { provider: provider, ignoreChanges: ["spec.replicas"] }
+);
+
+export const deploymentName = deployment.metadata.name;
+export const deploymentNamespace = deployment.metadata.namespace;

tests/sdk/nodejs/ignore-changes-ssa/deployment-patch-2.yaml

@@ -0,0 +1,3 @@
+# file used for basis of kubectl patch ...
+spec:
+  replicas: 4

tests/sdk/nodejs/ignore-changes-ssa/deployment-patch.yaml

@@ -0,0 +1,3 @@
+# file used for basis of kubectl patch ...
+spec:
+  replicas: 3

tests/sdk/nodejs/ignore-changes-ssa/step1/Pulumi.yaml

@@ -0,0 +1,3 @@
+name: skip-update-unreachable-tests
+description: Tests skipUpdateUnreachable flag
+runtime: nodejs

tests/sdk/nodejs/ignore-changes-ssa/step1/package.json

@@ -0,0 +1,11 @@
+{
+    "name": "skip-update-unreachable",
+    "version": "0.1.0",
+    "dependencies": {
+        "@pulumi/pulumi": "latest",
+        "@pulumi/random": "latest"
+    },
+    "peerDependencies": {
+        "@pulumi/kubernetes": "latest"
+    }
+}

tests/sdk/nodejs/ignore-changes-ssa/step1/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"
+    ]
+}
+

tests/sdk/nodejs/nodejs_test.go

@@ -1803,14 +1803,31 @@ func simluateClusterKubeconfig(t *testing.T, numOfClusters int) ([]string, error
 	return kubeconfigs, err
 }
 
-// TestIgnoreChanges tests that we can successfully ignore changes to a resource without SSA conflicts.
-// SkipRefresh *must* be true to properly test that conflict is handled when the state is not refreshed.
+// TestIgnoreChanges tests that we can successfully ignore changes to a resource without SSA conflicts,
+// and that we use the right field value when ignoring changes obtained from the live cluster.
+// SkipRefresh *must* be true to properly test that conflict is handled when the state is not refreshed
+// and drift has occurred.
 // https://github.com/pulumi/pulumi-kubernetes/issues/2542
 func TestIgnoreChanges(t *testing.T) {
+	testCases := []struct{ name, folderName string }{
+		{name: "Server Side Apply Mode", folderName: "ignore-changes-ssa"},
+		{name: "Client Side Apply Mode", folderName: "ignore-changes-csa"},
+	}
+
+	for _, tc := range testCases {
+		// NB: the Pulumi Program test runs in parallel, so we need to shadow the tc var.
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			ignoreChageTest(t, tc.folderName)
+		})
+	}
+}
+
+func ignoreChageTest(t *testing.T, testFolderName string) {
 	var depName, depNS string
 
 	test := baseOptions.With(integration.ProgramTestOptions{
-		Dir:                  filepath.Join("ignore-changes", "step1"),
+		Dir:                  filepath.Join(testFolderName, "step1"),
 		ExpectRefreshChanges: true,
 		// SkipRefresh MUST be true as the bug is not reproducible when the state is refreshed.
 		SkipRefresh: true,
@@ -1844,7 +1861,7 @@ func TestIgnoreChanges(t *testing.T) {
 			assert.Equal(t, "'2'", string(depReplicas))
 
 			// Patch deployment replicas to 3 using patch file in preparation for ignore changes to be tested in step2.
-			_, err = tests.Kubectl("patch --field-manager replica/manager deployment -n", depNS, depName, "--patch-file", filepath.Join("ignore-changes", "deployment-patch.yaml"))
+			_, err = tests.Kubectl("patch --field-manager replica/manager deployment -n", depNS, depName, "--patch-file", filepath.Join(testFolderName, "deployment-patch.yaml"))
 			assert.NoError(t, err)
 			depReplicas, err = tests.Kubectl("get deployment -o=jsonpath='{.spec.replicas}' -n", depNS, depName)
 			assert.NoError(t, err)
@@ -1853,7 +1870,7 @@ func TestIgnoreChanges(t *testing.T) {
 		EditDirs: []integration.EditDir{
 			{
 				// Repeat step1 again where no changes are made to the deployment since we ignore changes to spec.replicas.
-				Dir:      filepath.Join("ignore-changes", "step1"),
+				Dir:      filepath.Join(testFolderName, "step1"),
 				Additive: true,
 				ExtraRuntimeValidation: func(t *testing.T, stackInfo integration.RuntimeValidationStackInfo) {
 					// Validate replicas was not updated back to 1.
@@ -1863,7 +1880,7 @@ func TestIgnoreChanges(t *testing.T) {
 				},
 			},
 			{
-				Dir:      filepath.Join("ignore-changes", "step2"),
+				Dir:      filepath.Join(testFolderName, "step2"),
 				Additive: true,
 				ExtraRuntimeValidation: func(t *testing.T, stackInfo integration.RuntimeValidationStackInfo) {
 					// Validate image was updated, but spec.replicas was not.
@@ -1878,15 +1895,15 @@ func TestIgnoreChanges(t *testing.T) {
 					// Now use kubectl patch to update spec.replicas to 4 and see if we can correctly ignore changes to spec.replicas again when the field manager is
 					// "kubectl-patch" since we have logic to override certain field managers with manager name prefixes. This is due to fluxssa.PatchReplaceFieldsManagers
 					// doing a prefix match on the field manager name instead of an exact match on the given field manager name.
-					_, err = tests.Kubectl("patch deployment -n", depNS, depName, "--patch-file", filepath.Join("ignore-changes", "deployment-patch-2.yaml"))
+					_, err = tests.Kubectl("patch deployment -n", depNS, depName, "--patch-file", filepath.Join(testFolderName, "deployment-patch-2.yaml"))
 					assert.NoError(t, err)
 					depReplicas, err = tests.Kubectl("get deployment -o=jsonpath='{.spec.replicas}' -n", depNS, depName)
 					assert.NoError(t, err)
 					assert.Equal(t, "'4'", string(depReplicas))
 				},
 			},
 			{
-				Dir:      filepath.Join("ignore-changes", "step3"),
+				Dir:      filepath.Join(testFolderName, "step3"),
 				Additive: true,
 				ExtraRuntimeValidation: func(t *testing.T, stackInfo integration.RuntimeValidationStackInfo) {
 					// Validate image was updated, but spec.replicas was not.