From f83d63aca40ed9699bc79cdbcc95a7a2457f3f75 Mon Sep 17 00:00:00 2001 From: Aleksandr Rybolovlev Date: Wed, 17 Apr 2024 20:24:07 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fix=20manifest=20object=20type?= =?UTF-8?q?=20mutation=20(#2372)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Adapt Object branch of `morph.DeepUnknown` to use type of Value instead of mutating object type * Move the helper function PointerOf to the folder internal to make it available for all providers * Add a new manifest test helper function `CreatePod` * Add a new acceptance test for `kubernetes_resources` data source * Remove internal pointer package --- .changelog/2372.txt | 3 + manifest/morph/scaffold.go | 7 +- manifest/provider/datasource.go | 8 +- .../acceptance/datasource_resources_test.go | 132 ++++++++++++++++++ .../DataSourceResources/pods_data_source.tf | 8 ++ .../helper/kubernetes/kubernetes_helper.go | 26 +++- 6 files changed, 174 insertions(+), 10 deletions(-) create mode 100644 .changelog/2372.txt create mode 100644 manifest/test/acceptance/datasource_resources_test.go create mode 100644 manifest/test/acceptance/testdata/DataSourceResources/pods_data_source.tf diff --git a/.changelog/2372.txt b/.changelog/2372.txt new file mode 100644 index 0000000000..0422ed3cfe --- /dev/null +++ b/.changelog/2372.txt @@ -0,0 +1,3 @@ +```release-note:bug +`data_source/kubernetes_resources`: fix an issue where the provider exit with an error when the data source `kubernetes_resources` receives multiple Kubernetes objects containing tuples with different numbers of elements. +``` diff --git a/manifest/morph/scaffold.go b/manifest/morph/scaffold.go index 657a987a89..853a3bc4c8 100644 --- a/manifest/morph/scaffold.go +++ b/manifest/morph/scaffold.go @@ -23,6 +23,7 @@ func DeepUnknown(t tftypes.Type, v tftypes.Value, p *tftypes.AttributePath) (tft atts := t.(tftypes.Object).AttributeTypes var vals map[string]tftypes.Value ovals := make(map[string]tftypes.Value, len(atts)) + otypes := make(map[string]tftypes.Type, len(atts)) err := v.As(&vals) if err != nil { return tftypes.Value{}, p.NewError(err) @@ -34,11 +35,9 @@ func DeepUnknown(t tftypes.Type, v tftypes.Value, p *tftypes.AttributePath) (tft return tftypes.Value{}, np.NewError(err) } ovals[name] = nv - if nv.Type().Is(tftypes.Tuple{}) { - atts[name] = nv.Type() - } + otypes[name] = nv.Type() } - return tftypes.NewValue(tftypes.Object{AttributeTypes: atts}, ovals), nil + return tftypes.NewValue(tftypes.Object{AttributeTypes: otypes}, ovals), nil case t.Is(tftypes.Map{}): if v.IsNull() { return tftypes.NewValue(t, tftypes.UnknownValue), nil diff --git a/manifest/provider/datasource.go b/manifest/provider/datasource.go index 5362c1b95e..a4331018da 100644 --- a/manifest/provider/datasource.go +++ b/manifest/provider/datasource.go @@ -130,7 +130,7 @@ func (s *RawProviderServer) ReadPluralDataSource(ctx context.Context, req *tfpro if err != nil { resp.Diagnostics = append(resp.Diagnostics, &tfprotov5.Diagnostic{ Severity: tfprotov5.DiagnosticSeverityError, - Summary: "Failed to save resource state", + Summary: "Failed to save resource state", // FIX ME Detail: err.Error(), }) return resp, nil @@ -188,7 +188,7 @@ func (s *RawProviderServer) ReadPluralDataSource(ctx context.Context, req *tfpro if err != nil { resp.Diagnostics = append(resp.Diagnostics, &tfprotov5.Diagnostic{ Severity: tfprotov5.DiagnosticSeverityError, - Summary: "Failed to save resource state", + Summary: "Failed to save resource state", // FIX ME Detail: err.Error(), }) return resp, nil @@ -325,7 +325,7 @@ func (s *RawProviderServer) ReadSingularDataSource(ctx context.Context, req *tfp if err != nil { resp.Diagnostics = append(resp.Diagnostics, &tfprotov5.Diagnostic{ Severity: tfprotov5.DiagnosticSeverityError, - Summary: "Failed to save resource state", + Summary: "Failed to save resource state", // FIX ME Detail: err.Error(), }) return resp, nil @@ -378,7 +378,7 @@ func (s *RawProviderServer) ReadSingularDataSource(ctx context.Context, req *tfp if err != nil { resp.Diagnostics = append(resp.Diagnostics, &tfprotov5.Diagnostic{ Severity: tfprotov5.DiagnosticSeverityError, - Summary: "Failed to save resource state", + Summary: "Failed to save resource state", // FIX ME Detail: err.Error(), }) return resp, nil diff --git a/manifest/test/acceptance/datasource_resources_test.go b/manifest/test/acceptance/datasource_resources_test.go new file mode 100644 index 0000000000..76761d8921 --- /dev/null +++ b/manifest/test/acceptance/datasource_resources_test.go @@ -0,0 +1,132 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +//go:build acceptance +// +build acceptance + +package acceptance + +import ( + "context" + "fmt" + "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" + corev1 "k8s.io/api/core/v1" + "k8s.io/utils/ptr" +) + +func TestDataSourceKubernetesResources_DiffTuples(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 := randName() + namespace := randName() + + tf := tfhelper.RequireNewWorkingDir(ctx, t) + tf.SetReattachInfo(ctx, reattachInfo) + defer func() { + tf.Destroy(ctx) + tf.Close() + k8shelper.AssertResourceDoesNotExist(t, "v1", "namespaces", name) + }() + + // Create a Namespace to provision the rest of the resources in it. + k8shelper.CreateNamespace(t, namespace) + defer k8shelper.DeleteResource(t, namespace, kubernetes.NewGroupVersionResource("v1", "namespaces")) + + // Create Pods to use a data source. + // The only difference between the two pods is the number of VolumeMounts(tuples) that will be created. + // This is necessary to ensure that DeepUnknown doesn't mutate object type when iterates over items. + // kubernetes_manifest failed to create pods with volumes, thus create them manually. + k8shelper.AssertNamespacedResourceDoesNotExist(t, "v1", "pods", namespace, name) + + volumes := []corev1.Volume{ + { + Name: "config", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }, + } + podSpecs := []corev1.PodSpec{ + { + TerminationGracePeriodSeconds: ptr.To(int64(1)), + Containers: []corev1.Container{ + { + Name: "this", + Image: "busybox", + Command: []string{"sleep", "infinity"}, + VolumeMounts: []corev1.VolumeMount{ + { + Name: "config", + MountPath: "/config-a", + }, + { + Name: "config", + MountPath: "/config-b", + }, + }, + }, + }, + Volumes: volumes, + }, + { + TerminationGracePeriodSeconds: ptr.To(int64(1)), + Containers: []corev1.Container{ + { + Name: "this", + Image: "busybox", + Command: []string{"sleep", "infinity"}, + VolumeMounts: []corev1.VolumeMount{ + { + Name: "config", + MountPath: "/config-a", + }, + { + Name: "config", + MountPath: "/config-b", + }, + { + Name: "config", + MountPath: "/config-c", + }, + }, + }, + }, + Volumes: volumes, + }, + } + + for i, ps := range podSpecs { + k8shelper.CreatePod(t, fmt.Sprintf("%s-%d", name, i), namespace, ps) + } + + // Get pods + tfvars := TFVARS{ + "namespace": namespace, + } + tfconfig := loadTerraformConfig(t, "DataSourceResources/pods_data_source.tf", tfvars) + tf.SetConfig(ctx, tfconfig) + tf.Init(ctx) + err = tf.Apply(ctx) + if err != nil { + t.Fatal(err.Error()) + } + + st, err := tf.State(ctx) + if err != nil { + t.Fatalf("Failed to retrieve terraform state: %q", err) + } + tfstate := tfstatehelper.NewHelper(st) + + // check the data source + tfstate.AssertAttributeLen(t, "data.kubernetes_resources.pods.objects", len(podSpecs)) +} diff --git a/manifest/test/acceptance/testdata/DataSourceResources/pods_data_source.tf b/manifest/test/acceptance/testdata/DataSourceResources/pods_data_source.tf new file mode 100644 index 0000000000..c420c2f4bf --- /dev/null +++ b/manifest/test/acceptance/testdata/DataSourceResources/pods_data_source.tf @@ -0,0 +1,8 @@ +# Copyright (c) HashiCorp, Inc. +# SPDX-License-Identifier: MPL-2.0 + +data "kubernetes_resources" "pods" { + kind = "Pod" + api_version = "v1" + namespace = var.namespace +} diff --git a/manifest/test/helper/kubernetes/kubernetes_helper.go b/manifest/test/helper/kubernetes/kubernetes_helper.go index 8ddcb76871..021bd0d892 100644 --- a/manifest/test/helper/kubernetes/kubernetes_helper.go +++ b/manifest/test/helper/kubernetes/kubernetes_helper.go @@ -12,7 +12,9 @@ import ( "log" "testing" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -23,8 +25,6 @@ import ( "k8s.io/client-go/tools/clientcmd" k8sretry "k8s.io/client-go/util/retry" "k8s.io/kubectl/pkg/scheme" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // Helper is a Kubernetes dynamic client wrapped with a set of helper functions @@ -119,6 +119,28 @@ func (k *Helper) CreateConfigMap(t *testing.T, name string, namespace string, da } } +// CreatePod creates a new Pod +func (k *Helper) CreatePod(t *testing.T, name, namespace string, podSpec corev1.PodSpec) { + t.Helper() + + pod := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": map[string]interface{}{ + "name": name, + "namespace": namespace, + }, + "spec": podSpec, + }, + } + gvr := NewGroupVersionResource("v1", "pods") + _, err := k.dynClient.Resource(gvr).Namespace(namespace).Create(context.TODO(), pod, metav1.CreateOptions{}) + if err != nil { + t.Fatalf("Failed to create pod %q/%q: %v", namespace, name, err) + } +} + // DeleteResource deletes a resource referred to by the name and GVK func (k *Helper) DeleteResource(t *testing.T, name string, gvr schema.GroupVersionResource) { t.Helper()