From afd774dc67df8ab3d621d58d975988f35ff8a6c7 Mon Sep 17 00:00:00 2001 From: Mauricio Alvarez Leon <65101411+BBBmau@users.noreply.github.com> Date: Fri, 20 Sep 2024 04:28:34 -0700 Subject: [PATCH] manifest: remove forced replacement with `x-kubernetes-preserve-unknown-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 --- .changelog/2437.txt | 3 ++ manifest/openapi/schema.go | 2 +- manifest/provider/plan.go | 7 ++-- ...resource_x_preserve_unknown_fields_test.go | 35 +++++++++++++++++++ .../crd/test.tf | 21 ++++++----- .../test-cr-1.tf | 12 +++---- .../test-cr-2.tf | 14 ++++---- .../test-cr-3.tf | 20 +++++++++++ .../test-cr-4.tf | 17 +++++++++ 9 files changed, 104 insertions(+), 27 deletions(-) create mode 100644 .changelog/2437.txt create mode 100644 manifest/test/acceptance/testdata/x-kubernetes-preserve-unknown-fields/test-cr-3.tf create mode 100644 manifest/test/acceptance/testdata/x-kubernetes-preserve-unknown-fields/test-cr-4.tf diff --git a/.changelog/2437.txt b/.changelog/2437.txt new file mode 100644 index 0000000000..af56b38073 --- /dev/null +++ b/.changelog/2437.txt @@ -0,0 +1,3 @@ +```release-note +`kubernetes_manifest`: add TypeCheck for `x-kubernetes-preserve-unknown-fields` to prevent unnecessary replacement +``` \ No newline at end of file diff --git a/manifest/openapi/schema.go b/manifest/openapi/schema.go index 1ea0691ab0..3633b96762 100644 --- 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 index b025e36298..052963c5e3 100644 --- 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 index d7efe85130..2e69377250 100644 --- 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 index fd5f2266ae..04d6e81da3 100644 --- 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 index 9fc886b193..92f977bd9e 100644 --- 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 index bc34ea35fd..2f3938f22d 100644 --- 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 new file mode 100644 index 0000000000..3952db83d9 --- /dev/null +++ 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 new file mode 100644 index 0000000000..6952822113 --- /dev/null +++ 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 + } + } +}