terraform-provider-kubernetes
terraform-provider-kubernetes copied to clipboard
manifest: change `computed_field` logic to mark all as unknown instead of only on changes
Description
The idea is to treat all values in computed_field as an UnknownValue, before the values would only be treated as an UnknownValue when a change was present. This will insure that values in annotations/labels are covered especially in cases where the user did not set the label/annotation themselves.
Fixes #1591
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)─┐
└─(13:48:26 on computed_fields_allUnknown ✹ ✭)──> make testacc TESTARGS="-run TestKubernetesManifest_ComputedDeploymentFields"
go test -count=1 -tags acceptance "./test/acceptance" -v -run TestKubernetesManifest_ComputedDeploymentFields -timeout 120m
2024/03/12 13:49:29 Testing against Kubernetes API version: v1.27.3
=== RUN TestKubernetesManifest_ComputedDeploymentFields
--- PASS: TestKubernetesManifest_ComputedDeploymentFields (2.87s)
PASS
ok github.com/hashicorp/terraform-provider-kubernetes/manifest/test/acceptance 3.534s
Release Note
Release note for CHANGELOG:
kubernetes_manifest: Fix unexpected annotation by setting values in `computed_fields` to be Unknown always
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
latest commit solves the issue from referenced issue, however we still run into this error if the user chooses to set computed_fields and not include annotations/labels in the list of paths. However this does still follow the intended design from the docs:
IMPORTANT: By default, metadata.labels and metadata.annotations are already included in the list. You don't have to set them explicitly in the computed_fields list. To turn off these defaults, set the value of computed_fields to an empty list or a concrete list of other fields. For example computed_fields = [].
If the solution to this looks good then i think the only thing left is to add a test for this case to check that annotations/labels are seen as UnknownValues always
I've attempted to run the test on my machine but running into issues, could be because im not using kind and instead using minikube:
└─(17:43:54 on computed_fields_allUnknown ✹)──> make testacc TESTARGS="-run TestKubernetesManifest_ComputedFields" 2 ↵ ──(Fri,Mar01)─┘
go test -count=1 -tags acceptance "./test/acceptance" -v -run TestKubernetesManifest_ComputedFields -timeout 120m
2024/03/01 17:44:13 Testing against Kubernetes API version: v1.29.0-eks-c417bb3
=== RUN TestKubernetesManifest_ComputedFields
2024-03-01T17:45:04.939-0800 [ERROR] [ApplyResourceChange][Apply]: API error="Internal error occurred: failed calling webhook \"tf-acc-test-wbezqriecd.hashicorp.com\": failed to call webhook: Post \"https://tf-acc-test-wbezqriecd.tf-acc-test-cnbarqjfjb.svc:443/mutate?timeout=10s\": no endpoints available for service \"tf-acc-test-wbezqriecd\"" API response=<nil>
computed_attr_test.go:78: Error when trying to get resource tf-acc-test-cnbarqjfjb/tf-acc-test-wbezqriecd: configmaps "tf-acc-test-wbezqriecd" not found
--- FAIL: TestKubernetesManifest_ComputedFields (109.39s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x8 pc=0x103e41168]
goroutine 70 [running]:
testing.tRunner.func1.2({0x104420dc0, 0x1056a9360})
/opt/homebrew/Cellar/go/1.22.0/libexec/src/testing/testing.go:1631 +0x1c4
testing.tRunner.func1()
/opt/homebrew/Cellar/go/1.22.0/libexec/src/testing/testing.go:1634 +0x33c
panic({0x104420dc0?, 0x1056a9360?})
/opt/homebrew/Cellar/go/1.22.0/libexec/src/runtime/panic.go:770 +0x124
github.com/hashicorp/terraform-provider-kubernetes/manifest/test/helper/state.getAttributesValuesFromResource(0x103ef09c3?, {0x140012d1e78, 0x18})
/Users/mau/Dev/terraform-provider-kubernetes/manifest/test/helper/state/state_helper.go:37 +0x28
github.com/hashicorp/terraform-provider-kubernetes/manifest/test/helper/state.(*Helper).GetAttributeValue(0x1400089bac8, 0x140004309c0, {0x103ef09c3, 0x3f})
/Users/mau/Dev/terraform-provider-kubernetes/manifest/test/helper/state/state_helper.go:113 +0x60
github.com/hashicorp/terraform-provider-kubernetes/manifest/test/helper/state.(*Helper).AssertAttributeValues(0x140018cdac8, 0x140004309c0, 0x1400089ba98)
/Users/mau/Dev/terraform-provider-kubernetes/manifest/test/helper/state/state_helper.go:147 +0x98
github.com/hashicorp/terraform-provider-kubernetes/manifest/test/acceptance.TestKubernetesManifest_ComputedFields(0x140004309c0)
/Users/mau/Dev/terraform-provider-kubernetes/manifest/test/acceptance/computed_attr_test.go:85 +0xb34
testing.tRunner(0x140004309c0, 0x104701c68)
/opt/homebrew/Cellar/go/1.22.0/libexec/src/testing/testing.go:1689 +0xec
created by testing.(*T).Run in goroutine 1
/opt/homebrew/Cellar/go/1.22.0/libexec/src/testing/testing.go:1742 +0x318
FAIL github.com/hashicorp/terraform-provider-kubernetes/manifest/test/acceptance 112.438s
FAIL
make: *** [testacc] Error 1
┌─(~/Dev/terraform-provider-kubernetes/manifest)───────────────────────────────────────────────────────────────────────────────────────
┌─(~/Dev/terraform-provider-kubernetes/manifest)──────────────────────────────────────────────────────────────────────────────────────────────────────────────────(mau@mau-JKDT676NCP:s023)─┐
└─(17:46:52 on computed_fields_allUnknown ✹)──> git commit -m "cleanup plan code and add computedFields test when setting computedFields"
[computed_fields_allUnknown a86a18182] cleanup plan code and add computedFields test when setting computedFields
4 files changed, 62 insertions(+), 9 deletions(-)
Latest commit addresses the case where API changes the config value set by the user.
How it appears in config:
...
resources = {
limits = {
cpu = "0.25"
memory = "512Mi"
}
How it appears in YAML
...
Limits:
cpu: 250m
memory: 512Mi
~~I played around more with this and found this config from the original issue to cause a change despite no change actually happening. The original panic doesn't happen however.~~ This is actually the designed behavior now, this can be ignored! TFCONFIG:
resource "kubernetes_manifest" "daemonset_prometheus_daemonset" {
manifest = {
apiVersion = "apps/v1"
kind = "DaemonSet"
metadata = {
name = "prometheus-daemonset"
namespace = "default"
}
spec = {
selector = {
matchLabels = {
name = "prometheus-exporter"
tier = "monitoring"
}
}
template = {
metadata = {
labels = {
name = "prometheus-exporter"
tier = "monitoring"
}
}
spec = {
containers = [
{
image = "prom/node-exporter"
name = "prometheus"
ports = [
{
containerPort = 80
},
]
},
]
}
}
}
}
}
# kubernetes_manifest.daemonset_prometheus_daemonset will be updated in-place
~ resource "kubernetes_manifest" "daemonset_prometheus_daemonset" {
~ object = {
~ metadata = {
~ annotations = {
- "deprecated.daemonset.template.generation" = "1"
} -> (known after apply)
+ labels = (known after apply)
name = "prometheus-daemonset"
# (12 unchanged attributes hidden)
}
# (3 unchanged attributes hidden)
}
# (1 unchanged attribute hidden)
}
Plan: 0 to add, 1 to change, 0 to destroy.
Changes to the findBackfillValue could resolve this.