manifest: change computed_field logic to mark all as unknown instead of only on changes

.changelog/2432.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+kubernetes_manifest: Fix unexpected annotation by setting values in `computed_fields` to be Unknown always
+```

manifest/provider/apply.go

@@ -24,6 +24,43 @@ var defaultCreateTimeout = "10m"
 var defaultUpdateTimeout = "10m"
 var defaultDeleteTimeout = "10m"
 
+// findBackfillValue tries to optimistically find an attribute value pointed to by an attribute path (ap) inside
+// a complex type container value (m) by making semantically equivalent adaptations to the attribute path steps.
+// It considers Object and Map types as semantically equivalent AFA indexing attributes goes.
+func findBackfillValue(m interface{}, ap *tftypes.AttributePath) (interface{}, *tftypes.AttributePath, error) {
+	v, restPath, err := tftypes.WalkAttributePath(m, ap)
+	if err != nil {
+		if len(restPath.Steps()) > 0 {
+			// Attribute might not be found, because the attribute path step type  doesn't matcht
+			// the container type being indexed (core parses HCL to only Object and Tupple, but not Map and List).
+			// In that case, the attribute paths constructed for values in "object"
+			// will need adjusting for type differences between "manifest"
+			fs := restPath.Steps()[0]
+			switch e := fs.(type) {
+			case tftypes.ElementKeyString:
+				// if expecting a Map, try indexing with AttributeName instead
+				tp := tftypes.NewAttributePath().WithAttributeName(string(e))
+				tv, rp, err := tftypes.WalkAttributePath(v, tp)
+				if err != nil {
+					return v, rp, err
+				}
+				return findBackfillValue(tv, tftypes.NewAttributePathWithSteps(restPath.Steps()[1:]))
+
+			case tftypes.AttributeName:
+				// if expecting an Object, try indexing with ElementKeyString instead
+				tp := tftypes.NewAttributePath().WithElementKeyString(string(e))
+				tv, rp, err := tftypes.WalkAttributePath(v, tp)
+				if err != nil {
+					return v, rp, err
+				}
+				return findBackfillValue(tv, tftypes.NewAttributePathWithSteps(restPath.Steps()[1:]))
+			}
+		}
+		return nil, nil, err
+	}
+	return v, restPath, err
+}
+
 // ApplyResourceChange function
 func (s *RawProviderServer) ApplyResourceChange(ctx context.Context, req *tfprotov5.ApplyResourceChangeRequest) (*tfprotov5.ApplyResourceChangeResponse, error) {
 	resp := &tfprotov5.ApplyResourceChangeResponse{}
@@ -192,10 +229,8 @@ func (s *RawProviderServer) ApplyResourceChange(ctx context.Context, req *tfprot
 			if !isComputed {
 				return v, nil
 			}
-			if v.IsKnown() {
-				return v, nil
-			}
-			ppMan, restPath, err := tftypes.WalkAttributePath(plannedStateVal["manifest"], ap)
+
+			ppMan, restPath, err := findBackfillValue(plannedStateVal["manifest"], ap)
 			if err != nil {
 				if len(restPath.Steps()) > 0 {
 					// attribute not in manifest

manifest/provider/plan.go

@@ -422,6 +422,9 @@ func (s *RawProviderServer) PlanResourceChange(ctx context.Context, req *tfproto
 		}
 		updatedObj, err := tftypes.Transform(completePropMan, func(ap *tftypes.AttributePath, v tftypes.Value) (tftypes.Value, error) {
 			_, isComputed := computedFields[ap.String()]
+			if isComputed {
+				return tftypes.NewValue(v.Type(), tftypes.UnknownValue), nil
+			}
 			if v.IsKnown() { // this is a value from current configuration - include it in the plan
 				hasChanged := false
 				wasCfg, restPath, err := tftypes.WalkAttributePath(priorMan, ap)
@@ -437,15 +440,6 @@ func (s *RawProviderServer) PlanResourceChange(ctx context.Context, req *tfproto
 						resp.RequiresReplace = append(resp.RequiresReplace, tftypes.NewAttributePathWithSteps(apm))
 					}
 				}
-				if isComputed {
-					if hasChanged {
-						return tftypes.NewValue(v.Type(), tftypes.UnknownValue), nil
-					}
-					nowVal, restPath, err := tftypes.WalkAttributePath(proposedVal["object"], ap)
-					if err == nil && len(restPath.Steps()) == 0 {
-						return nowVal.(tftypes.Value), nil
-					}
-				}
 				return v, nil
 			}
 			// check if value was present in the previous configuration

manifest/test/acceptance/computed_attr_test.go

@@ -44,7 +44,7 @@ func TestKubernetesManifest_ComputedFields(t *testing.T) {
 		step1.Close()
 		k8shelper.AssertNamespacedResourceDoesNotExist(t, "v1", "secrets", namespace, name)
 		k8shelper.AssertNamespacedResourceDoesNotExist(t, "v1", "services", namespace, name)
-		k8shelper.AssertNamespacedResourceDoesNotExist(t, "v1", "deployments", namespace, name)
+		k8shelper.AssertNamespacedResourceDoesNotExist(t, "apps/v1", "deployments", namespace, name)
 		k8shelper.AssertResourceDoesNotExist(t, "admissionregistration.k8s.io", "mutatingwebhookconfigurations", name)
 	}()
 
@@ -54,7 +54,6 @@ func TestKubernetesManifest_ComputedFields(t *testing.T) {
 	step1.Apply(ctx)
 	k8shelper.AssertNamespacedResourceExists(t, "v1", "secrets", namespace, name)
 	k8shelper.AssertNamespacedResourceExists(t, "v1", "services", namespace, name)
-	k8shelper.AssertNamespacedResourceExists(t, "apps/v1", "deployments", namespace, name)
 	k8shelper.AssertResourceExists(t, "admissionregistration.k8s.io/v1", "mutatingwebhookconfigurations", name)
 
 	// wait for API to finish installing the webhook

manifest/test/acceptance/computed_deployment_test.go

@@ -0,0 +1,63 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+//go:build acceptance
+// +build acceptance
+
+package acceptance
+
+import (
+	"context"
+	"strings"
+	"testing"
+
+	"github.com/hashicorp/go-hclog"
+	"github.com/hashicorp/terraform-provider-kubernetes/manifest/provider"
+	"github.com/hashicorp/terraform-provider-kubernetes/manifest/test/helper/kubernetes"
+	tfstatehelper "github.com/hashicorp/terraform-provider-kubernetes/manifest/test/helper/state"
+)
+
+func TestKubernetesManifest_ComputedDeploymentFields(t *testing.T) {
+	ctx := context.Background()
+
+	reattachInfo, err := provider.ServeTest(ctx, hclog.Default(), t)
+	if err != nil {
+		t.Errorf("Failed to create provider instance: %q", err)
+	}
+
+	name := strings.ToLower(randName())
+	namespace := strings.ToLower(randName())
+
+	k8shelper.CreateNamespace(t, namespace)
+	defer k8shelper.DeleteResource(t, namespace, kubernetes.NewGroupVersionResource("v1", "namespaces"))
+
+	tfvars := TFVARS{
+		"name":      name,
+		"namespace": namespace,
+	}
+
+	tf := tfhelper.RequireNewWorkingDir(ctx, t)
+	tf.SetReattachInfo(ctx, reattachInfo)
+	defer func() {
+		tf.Destroy(ctx)
+		tf.Close()
+		k8shelper.AssertNamespacedResourceDoesNotExist(t, "apps/v1", "deployments", namespace, name)
+	}()
+
+	tfconfig := loadTerraformConfig(t, "ComputedFields/computed_deployment.tf", tfvars)
+	tf.SetConfig(ctx, string(tfconfig))
+	tf.Init(ctx)
+	tf.Apply(ctx)
+
+	s, err := tf.State(ctx)
+	if err != nil {
+		t.Fatalf("Failed to retrieve terraform state: %q", err)
+	}
+	tfstate := tfstatehelper.NewHelper(s)
+	tfstate.AssertAttributeValues(t, tfstatehelper.AttributeValues{
+		"kubernetes_manifest.deployment_resource_diff.object.metadata.name":                                           name,
+		"kubernetes_manifest.deployment_resource_diff.object.metadata.namespace":                                      namespace,
+		"kubernetes_manifest.deployment_resource_diff.object.spec.template.spec.containers.0.resources.limits.cpu":    "250m",
+		"kubernetes_manifest.deployment_resource_diff.object.spec.template.spec.containers.0.resources.limits.memory": "512Mi",
+	})
+}

manifest/test/acceptance/testdata/ComputedFields/computed_deployment.tf

@@ -0,0 +1,52 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+resource "kubernetes_manifest" "deployment_resource_diff" {
+computed_fields = ["spec.template.spec.containers[0].resources.limits[\"cpu\"]"]
+    manifest = {
+        apiVersion = "apps/v1"
+        kind       = "Deployment"
+
+        metadata = {
+            name = var.name
+            namespace = var.namespace
+        }
+
+        spec = {
+    replicas = 3
+
+    selector = {
+      matchLabels = {
+        test = "MyExampleApp"
+      }
+    }
+
+    template = {
+      metadata= {
+        labels = {
+          test = "MyExampleApp"
+        }
+      }
+
+
+      spec = {
+        containers = [{
+          image = "nginx:1.21.6"
+          name  = "example"
+
+          resources = {
+            limits = {
+              cpu    = "0.25"
+              memory = "512Mi"
+            }
+            requests = {
+              cpu    = "250m"
+              memory = "50Mi"
+            }
+          }
+        }]
+      }
+    }
+  }
+  }
+}