diff --git a/.changelog/2432.txt b/.changelog/2432.txt new file mode 100644 index 0000000000..1dfc066175 --- /dev/null +++ b/.changelog/2432.txt @@ -0,0 +1,3 @@ +```release-note:bug +kubernetes_manifest: Fix unexpected annotation by setting values in `computed_fields` to be Unknown always +``` diff --git a/manifest/provider/apply.go b/manifest/provider/apply.go index 22451dc5ed..009c2fddda 100644 --- a/manifest/provider/apply.go +++ b/manifest/provider/apply.go @@ -24,6 +24,43 @@ var defaultCreateTimeout = "10m" var defaultUpdateTimeout = "10m" var defaultDeleteTimeout = "10m" +// findBackfillValue tries to optimistically find an attribute value pointed to by an attribute path (ap) inside +// a complex type container value (m) by making semantically equivalent adaptations to the attribute path steps. +// It considers Object and Map types as semantically equivalent AFA indexing attributes goes. +func findBackfillValue(m interface{}, ap *tftypes.AttributePath) (interface{}, *tftypes.AttributePath, error) { + v, restPath, err := tftypes.WalkAttributePath(m, ap) + if err != nil { + if len(restPath.Steps()) > 0 { + // Attribute might not be found, because the attribute path step type doesn't matcht + // the container type being indexed (core parses HCL to only Object and Tupple, but not Map and List). + // In that case, the attribute paths constructed for values in "object" + // will need adjusting for type differences between "manifest" + fs := restPath.Steps()[0] + switch e := fs.(type) { + case tftypes.ElementKeyString: + // if expecting a Map, try indexing with AttributeName instead + tp := tftypes.NewAttributePath().WithAttributeName(string(e)) + tv, rp, err := tftypes.WalkAttributePath(v, tp) + if err != nil { + return v, rp, err + } + return findBackfillValue(tv, tftypes.NewAttributePathWithSteps(restPath.Steps()[1:])) + + case tftypes.AttributeName: + // if expecting an Object, try indexing with ElementKeyString instead + tp := tftypes.NewAttributePath().WithElementKeyString(string(e)) + tv, rp, err := tftypes.WalkAttributePath(v, tp) + if err != nil { + return v, rp, err + } + return findBackfillValue(tv, tftypes.NewAttributePathWithSteps(restPath.Steps()[1:])) + } + } + return nil, nil, err + } + return v, restPath, err +} + // ApplyResourceChange function func (s *RawProviderServer) ApplyResourceChange(ctx context.Context, req *tfprotov5.ApplyResourceChangeRequest) (*tfprotov5.ApplyResourceChangeResponse, error) { resp := &tfprotov5.ApplyResourceChangeResponse{} @@ -192,10 +229,8 @@ func (s *RawProviderServer) ApplyResourceChange(ctx context.Context, req *tfprot if !isComputed { return v, nil } - if v.IsKnown() { - return v, nil - } - ppMan, restPath, err := tftypes.WalkAttributePath(plannedStateVal["manifest"], ap) + + ppMan, restPath, err := findBackfillValue(plannedStateVal["manifest"], ap) if err != nil { if len(restPath.Steps()) > 0 { // attribute not in manifest diff --git a/manifest/provider/plan.go b/manifest/provider/plan.go index 8bd0a25cf9..1c54743c9a 100644 --- a/manifest/provider/plan.go +++ b/manifest/provider/plan.go @@ -422,6 +422,9 @@ func (s *RawProviderServer) PlanResourceChange(ctx context.Context, req *tfproto } updatedObj, err := tftypes.Transform(completePropMan, func(ap *tftypes.AttributePath, v tftypes.Value) (tftypes.Value, error) { _, isComputed := computedFields[ap.String()] + if isComputed { + return tftypes.NewValue(v.Type(), tftypes.UnknownValue), nil + } if v.IsKnown() { // this is a value from current configuration - include it in the plan hasChanged := false wasCfg, restPath, err := tftypes.WalkAttributePath(priorMan, ap) @@ -437,15 +440,6 @@ func (s *RawProviderServer) PlanResourceChange(ctx context.Context, req *tfproto resp.RequiresReplace = append(resp.RequiresReplace, tftypes.NewAttributePathWithSteps(apm)) } } - if isComputed { - if hasChanged { - return tftypes.NewValue(v.Type(), tftypes.UnknownValue), nil - } - nowVal, restPath, err := tftypes.WalkAttributePath(proposedVal["object"], ap) - if err == nil && len(restPath.Steps()) == 0 { - return nowVal.(tftypes.Value), nil - } - } return v, nil } // check if value was present in the previous configuration diff --git a/manifest/test/acceptance/computed_attr_test.go b/manifest/test/acceptance/computed_attr_test.go index 2484a451d8..cac616599d 100644 --- a/manifest/test/acceptance/computed_attr_test.go +++ b/manifest/test/acceptance/computed_attr_test.go @@ -44,7 +44,7 @@ func TestKubernetesManifest_ComputedFields(t *testing.T) { step1.Close() k8shelper.AssertNamespacedResourceDoesNotExist(t, "v1", "secrets", namespace, name) k8shelper.AssertNamespacedResourceDoesNotExist(t, "v1", "services", namespace, name) - k8shelper.AssertNamespacedResourceDoesNotExist(t, "v1", "deployments", namespace, name) + k8shelper.AssertNamespacedResourceDoesNotExist(t, "apps/v1", "deployments", namespace, name) k8shelper.AssertResourceDoesNotExist(t, "admissionregistration.k8s.io", "mutatingwebhookconfigurations", name) }() @@ -54,7 +54,6 @@ func TestKubernetesManifest_ComputedFields(t *testing.T) { step1.Apply(ctx) k8shelper.AssertNamespacedResourceExists(t, "v1", "secrets", namespace, name) k8shelper.AssertNamespacedResourceExists(t, "v1", "services", namespace, name) - k8shelper.AssertNamespacedResourceExists(t, "apps/v1", "deployments", namespace, name) k8shelper.AssertResourceExists(t, "admissionregistration.k8s.io/v1", "mutatingwebhookconfigurations", name) // wait for API to finish installing the webhook diff --git a/manifest/test/acceptance/computed_deployment_test.go b/manifest/test/acceptance/computed_deployment_test.go new file mode 100644 index 0000000000..3cfdba7082 --- /dev/null +++ b/manifest/test/acceptance/computed_deployment_test.go @@ -0,0 +1,63 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +//go:build acceptance +// +build acceptance + +package acceptance + +import ( + "context" + "strings" + "testing" + + "github.com/hashicorp/go-hclog" + "github.com/hashicorp/terraform-provider-kubernetes/manifest/provider" + "github.com/hashicorp/terraform-provider-kubernetes/manifest/test/helper/kubernetes" + tfstatehelper "github.com/hashicorp/terraform-provider-kubernetes/manifest/test/helper/state" +) + +func TestKubernetesManifest_ComputedDeploymentFields(t *testing.T) { + ctx := context.Background() + + reattachInfo, err := provider.ServeTest(ctx, hclog.Default(), t) + if err != nil { + t.Errorf("Failed to create provider instance: %q", err) + } + + name := strings.ToLower(randName()) + namespace := strings.ToLower(randName()) + + k8shelper.CreateNamespace(t, namespace) + defer k8shelper.DeleteResource(t, namespace, kubernetes.NewGroupVersionResource("v1", "namespaces")) + + tfvars := TFVARS{ + "name": name, + "namespace": namespace, + } + + tf := tfhelper.RequireNewWorkingDir(ctx, t) + tf.SetReattachInfo(ctx, reattachInfo) + defer func() { + tf.Destroy(ctx) + tf.Close() + k8shelper.AssertNamespacedResourceDoesNotExist(t, "apps/v1", "deployments", namespace, name) + }() + + tfconfig := loadTerraformConfig(t, "ComputedFields/computed_deployment.tf", tfvars) + tf.SetConfig(ctx, string(tfconfig)) + tf.Init(ctx) + tf.Apply(ctx) + + s, err := tf.State(ctx) + if err != nil { + t.Fatalf("Failed to retrieve terraform state: %q", err) + } + tfstate := tfstatehelper.NewHelper(s) + tfstate.AssertAttributeValues(t, tfstatehelper.AttributeValues{ + "kubernetes_manifest.deployment_resource_diff.object.metadata.name": name, + "kubernetes_manifest.deployment_resource_diff.object.metadata.namespace": namespace, + "kubernetes_manifest.deployment_resource_diff.object.spec.template.spec.containers.0.resources.limits.cpu": "250m", + "kubernetes_manifest.deployment_resource_diff.object.spec.template.spec.containers.0.resources.limits.memory": "512Mi", + }) +} diff --git a/manifest/test/acceptance/testdata/ComputedFields/computed_deployment.tf b/manifest/test/acceptance/testdata/ComputedFields/computed_deployment.tf new file mode 100644 index 0000000000..6dadf2a185 --- /dev/null +++ b/manifest/test/acceptance/testdata/ComputedFields/computed_deployment.tf @@ -0,0 +1,52 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +resource "kubernetes_manifest" "deployment_resource_diff" { +computed_fields = ["spec.template.spec.containers[0].resources.limits[\"cpu\"]"] + manifest = { + apiVersion = "apps/v1" + kind = "Deployment" + + metadata = { + name = var.name + namespace = var.namespace + } + + spec = { + replicas = 3 + + selector = { + matchLabels = { + test = "MyExampleApp" + } + } + + template = { + metadata= { + labels = { + test = "MyExampleApp" + } + } + + + spec = { + containers = [{ + image = "nginx:1.21.6" + name = "example" + + resources = { + limits = { + cpu = "0.25" + memory = "512Mi" + } + requests = { + cpu = "250m" + memory = "50Mi" + } + } + }] + } + } + } + } +} \ No newline at end of file