manifest: remove forced replacement with `x-kubernetes-preserve-unkno…

…wn-fields` (#2437) * Do not force replacement on type changes caused by CRD attributes with `x-kubernetes-preserve-unknown-fields` * Add warning for when type changes
hashicorp · Sep 20, 2024 · afd774d · afd774d
1 parent 52bccce
commit afd774d
Show file tree

Hide file tree

Showing 9 changed files with 104 additions and 27 deletions.
diff --git a/.changelog/2437.txt b/.changelog/2437.txt
@@ -0,0 +1,3 @@
+```release-note
+`kubernetes_manifest`: add TypeCheck for `x-kubernetes-preserve-unknown-fields` to prevent unnecessary replacement
+```
diff --git a/manifest/openapi/schema.go b/manifest/openapi/schema.go
@@ -114,7 +114,7 @@ func getTypeFromSchema(elem *openapi3.Schema, stackdepth uint64, typeCache *sync
 				return tftypes.String, nil
 			}
 		}
-		return tftypes.DynamicPseudoType, nil
+		return tftypes.DynamicPseudoType, nil // this is where DynamicType is set for when an attribute is tagged as 'x-kubernetes-preserve-unknown-fields'
 
 	case "array":
 		switch {

diff --git a/manifest/provider/plan.go b/manifest/provider/plan.go
@@ -459,8 +459,11 @@ func (s *RawProviderServer) PlanResourceChange(ctx context.Context, req *tfproto
 				if hasChanged {
 					h, ok := hints[morph.ValueToTypePath(ap).String()]
 					if ok && h == manifest.PreserveUnknownFieldsLabel {
-						apm := append(tftypes.NewAttributePath().WithAttributeName("manifest").Steps(), ap.Steps()...)
-						resp.RequiresReplace = append(resp.RequiresReplace, tftypes.NewAttributePathWithSteps(apm))
+						resp.Diagnostics = append(resp.Diagnostics, &tfprotov5.Diagnostic{
+							Severity: tfprotov5.DiagnosticSeverityWarning,
+							Summary:  fmt.Sprintf("The attribute path %v value's type is an x-kubernetes-preserve-unknown-field", morph.ValueToTypePath(ap).String()),
+							Detail:   "Changes to the type may cause some unexpected behavior.",
+						})
 					}
 				}
 				if isComputed {

diff --git a/manifest/test/acceptance/customresource_x_preserve_unknown_fields_test.go b/manifest/test/acceptance/customresource_x_preserve_unknown_fields_test.go
@@ -114,4 +114,39 @@ func TestKubernetesManifest_CustomResource_x_preserve_unknown_fields(t *testing.
 			"baz": interface{}("42"),
 		},
 	})
+
+	tfconfig = loadTerraformConfig(t, "x-kubernetes-preserve-unknown-fields/test-cr-3.tf", tfvars)
+	step1.SetConfig(ctx, string(tfconfig))
+	step1.Apply(ctx)
+
+	s3, err := step1.State(ctx)
+	if err != nil {
+		t.Fatalf("Failed to retrieve terraform state: %q", err)
+	}
+	tfstate3 := tfstatehelper.NewHelper(s3)
+	tfstate3.AssertAttributeValues(t, tfstatehelper.AttributeValues{
+		"kubernetes_manifest.test.object.metadata.name":      name,
+		"kubernetes_manifest.test.object.metadata.namespace": namespace,
+		"kubernetes_manifest.test.object.spec.count":         json.Number("100"),
+		"kubernetes_manifest.test.object.spec.resources": map[string]interface{}{
+			"foo": interface{}([]interface{}{"bar"}),
+			"baz": interface{}("42"),
+		},
+	})
+
+	tfconfig = loadTerraformConfig(t, "x-kubernetes-preserve-unknown-fields/test-cr-4.tf", tfvars)
+	step1.SetConfig(ctx, string(tfconfig))
+	step1.Apply(ctx)
+
+	s4, err := step1.State(ctx)
+	if err != nil {
+		t.Fatalf("Failed to retrieve terraform state: %q", err)
+	}
+	tfstate4 := tfstatehelper.NewHelper(s4)
+	tfstate4.AssertAttributeValues(t, tfstatehelper.AttributeValues{
+		"kubernetes_manifest.test.object.metadata.name":      name,
+		"kubernetes_manifest.test.object.metadata.namespace": namespace,
+		"kubernetes_manifest.test.object.spec.count":         json.Number("100"),
+		"kubernetes_manifest.test.object.spec.resources":     false,
+	})
 }
diff --git a/manifest/test/acceptance/testdata/x-kubernetes-preserve-unknown-fields/crd/test.tf b/manifest/test/acceptance/testdata/x-kubernetes-preserve-unknown-fields/crd/test.tf
@@ -4,14 +4,14 @@
 resource "kubernetes_manifest" "customresourcedefinition_cephrbdmirrors_ceph_rook_io" {
   manifest = {
     apiVersion = "apiextensions.k8s.io/v1"
-    kind = "CustomResourceDefinition"
+    kind       = "CustomResourceDefinition"
     metadata = {
       name = "${var.plural}.${var.group}"
     }
     spec = {
       group = var.group
       names = {
-        kind = var.kind
+        kind   = var.kind
         plural = var.plural
       }
       scope = "Namespaced"
@@ -24,14 +24,14 @@ resource "kubernetes_manifest" "customresourcedefinition_cephrbdmirrors_ceph_roo
                 spec = {
                   properties = {
                     annotations = {
-                      nullable = true
-                      type = "object"
+                      nullable                               = true
+                      type                                   = "object"
                       "x-kubernetes-preserve-unknown-fields" = true
                     }
                     count = {
                       maximum = 100
                       minimum = 1
-                      type = "integer"
+                      type    = "integer"
                     }
                     peers = {
                       properties = {
@@ -45,30 +45,29 @@ resource "kubernetes_manifest" "customresourcedefinition_cephrbdmirrors_ceph_roo
                       type = "object"
                     }
                     placement = {
-                      nullable = true
-                      type = "object"
+                      nullable                               = true
+                      type                                   = "object"
                       "x-kubernetes-preserve-unknown-fields" = true
                     }
                     priorityClassName = {
                       type = "string"
                     }
                     resources = {
-                      nullable = true
-                      type = "object"
+                      nullable                               = true
                       "x-kubernetes-preserve-unknown-fields" = true
                     }
                   }
                   type = "object"
                 }
                 status = {
-                  type = "object"
+                  type                                   = "object"
                   "x-kubernetes-preserve-unknown-fields" = true
                 }
               }
               type = "object"
             }
           }
-          served = true
+          served  = true
           storage = true
           subresources = {
             status = {}

diff --git a/manifest/test/acceptance/testdata/x-kubernetes-preserve-unknown-fields/test-cr-1.tf b/manifest/test/acceptance/testdata/x-kubernetes-preserve-unknown-fields/test-cr-1.tf
@@ -4,16 +4,16 @@
 resource "kubernetes_manifest" "test" {
   manifest = {
     apiVersion = var.group_version
-    kind = var.kind
+    kind       = var.kind
     metadata = {
-      name = var.name
+      name      = var.name
       namespace = var.namespace
     }
     spec = {
-        count = 100
-        resources = {
-            foo = "bar"
-        }
+      count = 100
+      resources = {
+        foo = "bar"
+      }
     }
   }
 }
diff --git a/manifest/test/acceptance/testdata/x-kubernetes-preserve-unknown-fields/test-cr-2.tf b/manifest/test/acceptance/testdata/x-kubernetes-preserve-unknown-fields/test-cr-2.tf
@@ -4,17 +4,17 @@
 resource "kubernetes_manifest" "test" {
   manifest = {
     apiVersion = var.group_version
-    kind = var.kind
+    kind       = var.kind
     metadata = {
-      name = var.name
+      name      = var.name
       namespace = var.namespace
     }
     spec = {
-        count = 100
-        resources = {
-            foo = "bar"
-            baz = "42"
-        }
+      count = 100
+      resources = {
+        foo = "bar"
+        baz = "42"
+      }
     }
   }
 }
diff --git a/manifest/test/acceptance/testdata/x-kubernetes-preserve-unknown-fields/test-cr-3.tf b/manifest/test/acceptance/testdata/x-kubernetes-preserve-unknown-fields/test-cr-3.tf
@@ -0,0 +1,20 @@
+# Copyright (c) HashiCorp, Inc.
+# SPDX-License-Identifier: MPL-2.0
+
+resource "kubernetes_manifest" "test" {
+  manifest = {
+    apiVersion = var.group_version
+    kind       = var.kind
+    metadata = {
+      name      = var.name
+      namespace = var.namespace
+    }
+    spec = {
+      count = 100
+      resources = {
+        foo = ["bar"]
+        baz = "42"
+      }
+    }
+  }
+}
diff --git a/manifest/test/acceptance/testdata/x-kubernetes-preserve-unknown-fields/test-cr-4.tf b/manifest/test/acceptance/testdata/x-kubernetes-preserve-unknown-fields/test-cr-4.tf
@@ -0,0 +1,17 @@
+# Copyright (c) HashiCorp, Inc.
+# SPDX-License-Identifier: MPL-2.0
+
+resource "kubernetes_manifest" "test" {
+  manifest = {
+    apiVersion = var.group_version
+    kind       = var.kind
+    metadata = {
+      name      = var.name
+      namespace = var.namespace
+    }
+    spec = {
+      count     = 100
+      resources = false
+    }
+  }
+}