manifest: remove forced replacement with x-kubernetes-preserve-unknown-fields

[BBBmau]

Description

This check will prevent x-kubernetes-preserve-unknown-fields to cause a replacement whenever the value is changed. The idea is that it should ONLY cause a replacement if the type itself is changed and not the value.

Fixes #2371
Fixes #2375
Fixes #1928
Fixes #2410

Acceptance tests

• Have you added an acceptance test for the functionality being added?
• Have you run the acceptance tests on this branch?

Output from acceptance testing:

┌─(~/Dev/terraform-provider-kubernetes/manifest)───────────────────────────────────────────────(mau@mau-JKDT676NCP:s077)─┐
└─(14:48:09 on fix-unknownFieldsLogic-manifest ✹ ✭)──> make testacc TESTARGS="-run TestKubernetesManifest_CustomResource_x_preserve_unknown_fields"
go test -count=1 -tags acceptance "./test/acceptance" -v -run TestKubernetesManifest_CustomResource_x_preserve_unknown_fields -timeout 120m
2024/03/12 14:48:28 Testing against Kubernetes API version: v1.27.3
=== RUN   TestKubernetesManifest_CustomResource_x_preserve_unknown_fields
--- PASS: TestKubernetesManifest_CustomResource_x_preserve_unknown_fields (14.23s)
PASS
ok      github.com/hashicorp/terraform-provider-kubernetes/manifest/test/acceptance     14.832s```

Release Note

Release note for CHANGELOG:

`kubernetes_manifest`: add TypeCheck for `x-kubernetes-preserve-unknown-fields` to prevent unnecessary replacement

References

Community Note

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
• If you are interested in working on this issue or have submitted a pull request, please leave a comment

[BBBmau]

@alexsomesan

I'll need some feedback since when I run it through the debug. the plan produces a plannedchange of just the value and not the entire resource. However if we were to run it without debug mode it still produces the same replacement action, which goes against the logic that's been applied in this PR.

After resolving that we'll look into adding a test for this. 💯

[BBBmau]

Adding a note here that when adding a value into the tuple it will cause a recreation despite the type being the same:

          valuesObject = {
            "mau": ["test"] -> ["test", "test2"]
            }

Plan: 1 to add, 0 to change, 1 to destroy.
╷
│ Warning: The attribute path AttributeName("spec").AttributeName("sources").ElementKeyInt(-1).AttributeName("helm").AttributeName("valuesObject") value's type has changed
│ 
│   with kubernetes_manifest.raw,
│   on main.tf line 3, in resource "kubernetes_manifest" "raw":
│    3: resource "kubernetes_manifest" "raw" {
│ 
│ Changes to the type will cause a forced replacement.

An extra check is needed before we can get this merged.

[BBBmau]

Adding a note here that when adding a value into the tuple it will cause a recreation despite the type being the same:

          valuesObject = {
            "mau": ["test"] -> ["test", "test2"]
            }

Plan: 1 to add, 0 to change, 1 to destroy.
╷
│ Warning: The attribute path AttributeName("spec").AttributeName("sources").ElementKeyInt(-1).AttributeName("helm").AttributeName("valuesObject") value's type has changed
│ 
│   with kubernetes_manifest.raw,
│   on main.tf line 3, in resource "kubernetes_manifest" "raw":
│    3: resource "kubernetes_manifest" "raw" {
│ 
│ Changes to the type will cause a forced replacement.

An extra check is needed before we can get this merged.

Recent commit addresses this check by using Diff method from tftypes.Value which returns an Error if a change in type occurs on values.

_, diff := wasCfg.(tftypes.Value).Diff(nowCfg.(tftypes.Value)) // passing values of two different types will result in an error.

Prior we would check by using the Equal method:
typeChanged := !(wasCfg.(tftypes.Value).Type().Equal(nowCfg.(tftypes.Value).Type())) this didn't work when adding to the tuple despite the type still staying the same. Could be an issue with the Equal method type. From the docs: Equal performs deep type equality checks, including attribute/element types and whether attributes are optional or not.

[BBBmau]

In the past, type changes was something that terraform panic to. This was early in the development of terraform core and appears it is not an issue anymore. Because of this we can remove the forced replacement for x-kubernetes-preserve-unknown-fields but will add a warning when users are intending to change types.

[hongshaoyang]

This PR fixes #1829

[TessaIO]

Any update on this PR?

[CORVU5]

Any update on when we can expect this to be released?

[FarhadF]

@BBBmau @alexsomesan whats blocking this PR?

[alexsomesan]

Looking good.
I added an extra test case, just to be on the safe side.