From aedb26d375d5d29cb57a13e291abed284c623857 Mon Sep 17 00:00:00 2001 From: BBBmau Date: Wed, 21 Feb 2024 14:46:04 -0800 Subject: [PATCH 01/12] fix crash when unexpeceted annotation appears --- manifest/provider/apply.go | 2 +- manifest/provider/plan.go | 13 ++++--------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/manifest/provider/apply.go b/manifest/provider/apply.go index 22451dc5ed..0460a79c33 100644 --- a/manifest/provider/apply.go +++ b/manifest/provider/apply.go @@ -193,7 +193,7 @@ func (s *RawProviderServer) ApplyResourceChange(ctx context.Context, req *tfprot return v, nil } if v.IsKnown() { - return v, nil + return tftypes.NewValue(v.Type(), tftypes.UnknownValue), nil } ppMan, restPath, err := tftypes.WalkAttributePath(plannedStateVal["manifest"], ap) if err != nil { diff --git a/manifest/provider/plan.go b/manifest/provider/plan.go index 8bd0a25cf9..5005b4df84 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) @@ -430,6 +433,7 @@ func (s *RawProviderServer) PlanResourceChange(ctx context.Context, req *tfproto } nowCfg, restPath, err := tftypes.WalkAttributePath(ppMan, ap) hasChanged = err == nil && len(restPath.Steps()) == 0 && wasCfg.(tftypes.Value).IsKnown() && !wasCfg.(tftypes.Value).Equal(nowCfg.(tftypes.Value)) + if hasChanged { h, ok := hints[morph.ValueToTypePath(ap).String()] if ok && h == manifest.PreserveUnknownFieldsLabel { @@ -437,15 +441,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 From a86a18182bc93b16c4176fbeef5ef5d9c6fa108b Mon Sep 17 00:00:00 2001 From: BBBmau Date: Fri, 1 Mar 2024 17:47:26 -0800 Subject: [PATCH 02/12] cleanup plan code and add computedFields test when setting computedFields --- manifest/provider/apply.go | 4 +- manifest/provider/plan.go | 1 - .../test/acceptance/computed_attr_test.go | 16 ++++-- .../testdata/ComputedFields/computed.tf | 50 +++++++++++++++++++ 4 files changed, 62 insertions(+), 9 deletions(-) diff --git a/manifest/provider/apply.go b/manifest/provider/apply.go index 0460a79c33..f2fb866bce 100644 --- a/manifest/provider/apply.go +++ b/manifest/provider/apply.go @@ -192,9 +192,7 @@ func (s *RawProviderServer) ApplyResourceChange(ctx context.Context, req *tfprot if !isComputed { return v, nil } - if v.IsKnown() { - return tftypes.NewValue(v.Type(), tftypes.UnknownValue), nil - } + ppMan, restPath, err := tftypes.WalkAttributePath(plannedStateVal["manifest"], ap) if err != nil { if len(restPath.Steps()) > 0 { diff --git a/manifest/provider/plan.go b/manifest/provider/plan.go index 5005b4df84..1c54743c9a 100644 --- a/manifest/provider/plan.go +++ b/manifest/provider/plan.go @@ -433,7 +433,6 @@ func (s *RawProviderServer) PlanResourceChange(ctx context.Context, req *tfproto } nowCfg, restPath, err := tftypes.WalkAttributePath(ppMan, ap) hasChanged = err == nil && len(restPath.Steps()) == 0 && wasCfg.(tftypes.Value).IsKnown() && !wasCfg.(tftypes.Value).Equal(nowCfg.(tftypes.Value)) - if hasChanged { h, ok := hints[morph.ValueToTypePath(ap).String()] if ok && h == manifest.PreserveUnknownFieldsLabel { diff --git a/manifest/test/acceptance/computed_attr_test.go b/manifest/test/acceptance/computed_attr_test.go index 2484a451d8..cfa7404c7b 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) }() @@ -66,6 +66,7 @@ func TestKubernetesManifest_ComputedFields(t *testing.T) { defer func() { step2.Destroy(ctx) step2.Close() + k8shelper.AssertNamespacedResourceDoesNotExist(t, "apps/v1", "deployments", namespace, name) k8shelper.AssertNamespacedResourceDoesNotExist(t, "v1", "configmaps", namespace, name) }() @@ -73,6 +74,7 @@ func TestKubernetesManifest_ComputedFields(t *testing.T) { step2.SetConfig(ctx, string(tfconfig)) step2.Init(ctx) step2.Apply(ctx) + k8shelper.AssertNamespacedResourceExists(t, "apps/v1", "deployments", namespace, name) k8shelper.AssertNamespacedResourceExists(t, "v1", "configmaps", namespace, name) s2, err := step2.State(ctx) @@ -81,9 +83,13 @@ func TestKubernetesManifest_ComputedFields(t *testing.T) { } tfstate := tfstatehelper.NewHelper(s2) tfstate.AssertAttributeValues(t, tfstatehelper.AttributeValues{ - "kubernetes_manifest.test.object.metadata.name": name, - "kubernetes_manifest.test.object.metadata.namespace": namespace, - "kubernetes_manifest.test.object.metadata.annotations.tf-k8s-acc": "true", - "kubernetes_manifest.test.object.metadata.annotations.mutated": "true", + "kubernetes_manifest.test.object.metadata.name": name, + "kubernetes_manifest.test.object.metadata.namespace": namespace, + "kubernetes_manifest.test.object.metadata.annotations.tf-k8s-acc": "true", + "kubernetes_manifest.test.object.metadata.annotations.mutated": "true", + "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.tf b/manifest/test/acceptance/testdata/ComputedFields/computed.tf index 21b1138666..4a79074736 100644 --- a/manifest/test/acceptance/testdata/ComputedFields/computed.tf +++ b/manifest/test/acceptance/testdata/ComputedFields/computed.tf @@ -14,3 +14,53 @@ resource "kubernetes_manifest" "test" { } } } + +resource "kubernetes_manifest" "deployment_resource_diff" { +computed_fields = ["spec.template.spec.containers[0].resources.limits"] + 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 From 5fd30f4be0fb171870deb55c3f9d35cca8ae31fb Mon Sep 17 00:00:00 2001 From: BBBmau Date: Fri, 1 Mar 2024 17:54:31 -0800 Subject: [PATCH 03/12] add changelog-entry --- .changelog/2432.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/2432.txt 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 +``` From f6f3f176312b5adfb22efd41cf1ffce5c8da0f1c Mon Sep 17 00:00:00 2001 From: BBBmau Date: Sat, 2 Mar 2024 10:21:40 -0800 Subject: [PATCH 04/12] fix manifest tests --- manifest/test/acceptance/computed_attr_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/manifest/test/acceptance/computed_attr_test.go b/manifest/test/acceptance/computed_attr_test.go index cfa7404c7b..d6a9126710 100644 --- a/manifest/test/acceptance/computed_attr_test.go +++ b/manifest/test/acceptance/computed_attr_test.go @@ -14,6 +14,7 @@ import ( "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" ) @@ -70,6 +71,8 @@ func TestKubernetesManifest_ComputedFields(t *testing.T) { k8shelper.AssertNamespacedResourceDoesNotExist(t, "v1", "configmaps", namespace, name) }() + k8shelper.CreateNamespace(t, namespace) + defer k8shelper.DeleteResource(t, namespace, kubernetes.NewGroupVersionResource("v1", "namespaces")) tfconfig = loadTerraformConfig(t, "ComputedFields/computed.tf", tfvars) step2.SetConfig(ctx, string(tfconfig)) step2.Init(ctx) From d6dd7dade1160e51ba0bb86a0ffe6069afb5a830 Mon Sep 17 00:00:00 2001 From: BBBmau Date: Sat, 2 Mar 2024 10:29:52 -0800 Subject: [PATCH 05/12] remove createNamespace --- manifest/test/acceptance/computed_attr_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/manifest/test/acceptance/computed_attr_test.go b/manifest/test/acceptance/computed_attr_test.go index d6a9126710..cfa7404c7b 100644 --- a/manifest/test/acceptance/computed_attr_test.go +++ b/manifest/test/acceptance/computed_attr_test.go @@ -14,7 +14,6 @@ import ( "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" ) @@ -71,8 +70,6 @@ func TestKubernetesManifest_ComputedFields(t *testing.T) { k8shelper.AssertNamespacedResourceDoesNotExist(t, "v1", "configmaps", namespace, name) }() - k8shelper.CreateNamespace(t, namespace) - defer k8shelper.DeleteResource(t, namespace, kubernetes.NewGroupVersionResource("v1", "namespaces")) tfconfig = loadTerraformConfig(t, "ComputedFields/computed.tf", tfvars) step2.SetConfig(ctx, string(tfconfig)) step2.Init(ctx) From dd0d50d795bc7d5bbb7c6f99ded44bd975614d84 Mon Sep 17 00:00:00 2001 From: BBBmau Date: Tue, 12 Mar 2024 13:49:58 -0700 Subject: [PATCH 06/12] add TestKubernetesManifest_ComputedDeploymentFields --- .../test/acceptance/computed_attr_test.go | 15 ++--- .../acceptance/computed_deployment_test.go | 63 +++++++++++++++++++ .../testdata/ComputedFields/computed.tf | 50 --------------- .../ComputedFields/computed_deployment.tf | 49 +++++++++++++++ 4 files changed, 116 insertions(+), 61 deletions(-) create mode 100644 manifest/test/acceptance/computed_deployment_test.go create mode 100644 manifest/test/acceptance/testdata/ComputedFields/computed_deployment.tf diff --git a/manifest/test/acceptance/computed_attr_test.go b/manifest/test/acceptance/computed_attr_test.go index cfa7404c7b..cac616599d 100644 --- a/manifest/test/acceptance/computed_attr_test.go +++ b/manifest/test/acceptance/computed_attr_test.go @@ -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 @@ -66,7 +65,6 @@ func TestKubernetesManifest_ComputedFields(t *testing.T) { defer func() { step2.Destroy(ctx) step2.Close() - k8shelper.AssertNamespacedResourceDoesNotExist(t, "apps/v1", "deployments", namespace, name) k8shelper.AssertNamespacedResourceDoesNotExist(t, "v1", "configmaps", namespace, name) }() @@ -74,7 +72,6 @@ func TestKubernetesManifest_ComputedFields(t *testing.T) { step2.SetConfig(ctx, string(tfconfig)) step2.Init(ctx) step2.Apply(ctx) - k8shelper.AssertNamespacedResourceExists(t, "apps/v1", "deployments", namespace, name) k8shelper.AssertNamespacedResourceExists(t, "v1", "configmaps", namespace, name) s2, err := step2.State(ctx) @@ -83,13 +80,9 @@ func TestKubernetesManifest_ComputedFields(t *testing.T) { } tfstate := tfstatehelper.NewHelper(s2) tfstate.AssertAttributeValues(t, tfstatehelper.AttributeValues{ - "kubernetes_manifest.test.object.metadata.name": name, - "kubernetes_manifest.test.object.metadata.namespace": namespace, - "kubernetes_manifest.test.object.metadata.annotations.tf-k8s-acc": "true", - "kubernetes_manifest.test.object.metadata.annotations.mutated": "true", - "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", + "kubernetes_manifest.test.object.metadata.name": name, + "kubernetes_manifest.test.object.metadata.namespace": namespace, + "kubernetes_manifest.test.object.metadata.annotations.tf-k8s-acc": "true", + "kubernetes_manifest.test.object.metadata.annotations.mutated": "true", }) } 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.tf b/manifest/test/acceptance/testdata/ComputedFields/computed.tf index 4a79074736..21b1138666 100644 --- a/manifest/test/acceptance/testdata/ComputedFields/computed.tf +++ b/manifest/test/acceptance/testdata/ComputedFields/computed.tf @@ -14,53 +14,3 @@ resource "kubernetes_manifest" "test" { } } } - -resource "kubernetes_manifest" "deployment_resource_diff" { -computed_fields = ["spec.template.spec.containers[0].resources.limits"] - 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 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..2be7485b0e --- /dev/null +++ b/manifest/test/acceptance/testdata/ComputedFields/computed_deployment.tf @@ -0,0 +1,49 @@ +resource "kubernetes_manifest" "deployment_resource_diff" { +computed_fields = ["spec.template.spec.containers[0].resources.limits"] + 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 From d756ffbd06e159ef13caa69a1dccfc8f5df5ad5f Mon Sep 17 00:00:00 2001 From: BBBmau Date: Tue, 12 Mar 2024 13:52:39 -0700 Subject: [PATCH 07/12] add copyright header --- .../acceptance/testdata/ComputedFields/computed_deployment.tf | 3 +++ 1 file changed, 3 insertions(+) diff --git a/manifest/test/acceptance/testdata/ComputedFields/computed_deployment.tf b/manifest/test/acceptance/testdata/ComputedFields/computed_deployment.tf index 2be7485b0e..4301293380 100644 --- a/manifest/test/acceptance/testdata/ComputedFields/computed_deployment.tf +++ b/manifest/test/acceptance/testdata/ComputedFields/computed_deployment.tf @@ -1,3 +1,6 @@ +// 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"] manifest = { From 3368fd33a60e16de0f1cc2ecf6fdca74f1e5709d Mon Sep 17 00:00:00 2001 From: Mauricio Alvarez Leon <65101411+BBBmau@users.noreply.github.com> Date: Wed, 3 Apr 2024 13:11:58 -0700 Subject: [PATCH 08/12] Update manifest/provider/apply.go Co-authored-by: Alex Somesan --- manifest/provider/apply.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/manifest/provider/apply.go b/manifest/provider/apply.go index f2fb866bce..e4c7b24cdf 100644 --- a/manifest/provider/apply.go +++ b/manifest/provider/apply.go @@ -193,7 +193,7 @@ func (s *RawProviderServer) ApplyResourceChange(ctx context.Context, req *tfprot 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 From bd56585119a4dfb76402b5e81f9267ca7f99753e Mon Sep 17 00:00:00 2001 From: BBBmau Date: Wed, 3 Apr 2024 13:14:14 -0700 Subject: [PATCH 09/12] add findBackfillvalue --- manifest/provider/apply.go | 44 ++++++++++++++++++++++++++++++++------ 1 file changed, 37 insertions(+), 7 deletions(-) diff --git a/manifest/provider/apply.go b/manifest/provider/apply.go index e4c7b24cdf..98f6d99176 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{} @@ -194,13 +231,6 @@ func (s *RawProviderServer) ApplyResourceChange(ctx context.Context, req *tfprot } ppMan, restPath, err := findBackfillValue(plannedStateVal["manifest"], ap) - if err != nil { - if len(restPath.Steps()) > 0 { - // attribute not in manifest - return v, nil - } - return v, ap.NewError(err) - } nv, d := morph.ValueToType(ppMan.(tftypes.Value), v.Type(), tftypes.NewAttributePath()) if len(d) > 0 { resp.Diagnostics = append(resp.Diagnostics, &tfprotov5.Diagnostic{ From 6cdde2c85242febf045328080573575b13a91387 Mon Sep 17 00:00:00 2001 From: BBBmau Date: Wed, 3 Apr 2024 13:16:03 -0700 Subject: [PATCH 10/12] add comment for findBackfillValue --- manifest/provider/apply.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/manifest/provider/apply.go b/manifest/provider/apply.go index 98f6d99176..4ae39a441f 100644 --- a/manifest/provider/apply.go +++ b/manifest/provider/apply.go @@ -230,7 +230,8 @@ func (s *RawProviderServer) ApplyResourceChange(ctx context.Context, req *tfprot return v, nil } - ppMan, restPath, err := findBackfillValue(plannedStateVal["manifest"], ap) + // the use of restpath and err are used in recursive calls of findBackfillValue + ppMan, _, _ := findBackfillValue(plannedStateVal["manifest"], ap) nv, d := morph.ValueToType(ppMan.(tftypes.Value), v.Type(), tftypes.NewAttributePath()) if len(d) > 0 { resp.Diagnostics = append(resp.Diagnostics, &tfprotov5.Diagnostic{ From cca62498e36adcc7127d97984210151ce9020e9e Mon Sep 17 00:00:00 2001 From: Mauricio Alvarez Leon <65101411+BBBmau@users.noreply.github.com> Date: Wed, 3 Apr 2024 13:44:13 -0700 Subject: [PATCH 11/12] Update manifest/test/acceptance/testdata/ComputedFields/computed_deployment.tf Co-authored-by: Alex Somesan --- .../acceptance/testdata/ComputedFields/computed_deployment.tf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/manifest/test/acceptance/testdata/ComputedFields/computed_deployment.tf b/manifest/test/acceptance/testdata/ComputedFields/computed_deployment.tf index 4301293380..6dadf2a185 100644 --- a/manifest/test/acceptance/testdata/ComputedFields/computed_deployment.tf +++ b/manifest/test/acceptance/testdata/ComputedFields/computed_deployment.tf @@ -2,7 +2,7 @@ // SPDX-License-Identifier: MPL-2.0 resource "kubernetes_manifest" "deployment_resource_diff" { -computed_fields = ["spec.template.spec.containers[0].resources.limits"] +computed_fields = ["spec.template.spec.containers[0].resources.limits[\"cpu\"]"] manifest = { apiVersion = "apps/v1" kind = "Deployment" From e3b7aaf75128933232a19281082b8b4d8e1bd72f Mon Sep 17 00:00:00 2001 From: BBBmau Date: Wed, 3 Apr 2024 13:18:27 -0700 Subject: [PATCH 12/12] err check --- manifest/provider/apply.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/manifest/provider/apply.go b/manifest/provider/apply.go index 4ae39a441f..009c2fddda 100644 --- a/manifest/provider/apply.go +++ b/manifest/provider/apply.go @@ -230,8 +230,14 @@ func (s *RawProviderServer) ApplyResourceChange(ctx context.Context, req *tfprot return v, nil } - // the use of restpath and err are used in recursive calls of findBackfillValue - ppMan, _, _ := findBackfillValue(plannedStateVal["manifest"], ap) + ppMan, restPath, err := findBackfillValue(plannedStateVal["manifest"], ap) + if err != nil { + if len(restPath.Steps()) > 0 { + // attribute not in manifest + return v, nil + } + return v, ap.NewError(err) + } nv, d := morph.ValueToType(ppMan.(tftypes.Value), v.Type(), tftypes.NewAttributePath()) if len(d) > 0 { resp.Diagnostics = append(resp.Diagnostics, &tfprotov5.Diagnostic{