terraform-provider-kubernetes
terraform-provider-kubernetes copied to clipboard
hashicorp
manifest: remove forced replacement with `x-kubernetes-preserve-unknown-fields`
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 https://github.com/hashicorp/terraform-provider-kubernetes/issues/2371 Fixes https://github.com/hashicorp/terraform-provider-kubernetes/issues/2375 Fixes https://github.com/hashicorp/terraform-provider-kubernetes/issues/1928 Fixes https://github.com/hashicorp/terraform-provider-kubernetes/issues/2410
Acceptance tests
- [x] Have you added an acceptance test for the functionality being added?
- [x] 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
@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. 💯
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.
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.
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.
