From d8db276011acc4f72b2f22ac24f61aa88a8d8f5b Mon Sep 17 00:00:00 2001
From: Sean Gillespie <sean.gillespie@temporal.io>
Date: Fri, 19 Jan 2024 12:45:39 -0800
Subject: [PATCH] map empty certificate filter lists to null lists
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Terraform considers the null list and the empty list to be two separate values, in the same way that Go does. To ensure consistency, this commit explicitly transforms missing certificate filter lists into null lists in both directions (TF -> API and API -> TF).

Fixes this issue:

```
│ Error: Provider produced inconsistent result after apply
│
│ When applying changes to temporalcloud_namespace.terraform, provider "provider[\"registry.terraform.io/hashicorp/temporalcloud\"]"
│ produced an unexpected new value: .certificate_filters: was null, but now
│ cty.ListValEmpty(cty.Object(map[string]cty.Type{"common_name":cty.String, "organization":cty.String,
│ "organizational_unit":cty.String, "subject_alternative_name":cty.String})).
│
│ This is a bug in the provider, which should be reported in the provider's own issue tracker.
```
---
 internal/provider/namespace_resource.go      | 38 ++++++++-----
 internal/provider/namespace_resource_test.go | 57 ++++++++++++++++++--
 2 files changed, 78 insertions(+), 17 deletions(-)

diff --git a/internal/provider/namespace_resource.go b/internal/provider/namespace_resource.go
index e94156f..7ef849d 100644
--- a/internal/provider/namespace_resource.go
+++ b/internal/provider/namespace_resource.go
@@ -351,27 +351,33 @@ func updateModelFromSpec(ctx context.Context, diags diag.Diagnostics, state *nam
 		return
 	}
 
-	certificateFilterObjects := make([]types.Object, len(ns.GetSpec().GetMtlsAuth().GetCertificateFilters()))
-	for i, certFilter := range ns.GetSpec().GetMtlsAuth().GetCertificateFilters() {
-		model := namespaceCertificateFilterModel{
-			CommonName:             stringOrNull(certFilter.GetCommonName()),
-			Organization:           stringOrNull(certFilter.GetOrganization()),
-			OrganizationalUnit:     stringOrNull(certFilter.GetOrganizationalUnit()),
-			SubjectAlternativeName: stringOrNull(certFilter.GetSubjectAlternativeName()),
+	certificateFilter := types.ListNull(types.ObjectType{AttrTypes: namespaceCertificateFilterAttrs})
+	if len(ns.GetSpec().GetMtlsAuth().GetCertificateFilters()) > 0 {
+		certificateFilterObjects := make([]types.Object, len(ns.GetSpec().GetMtlsAuth().GetCertificateFilters()))
+		for i, certFilter := range ns.GetSpec().GetMtlsAuth().GetCertificateFilters() {
+			model := namespaceCertificateFilterModel{
+				CommonName:             stringOrNull(certFilter.GetCommonName()),
+				Organization:           stringOrNull(certFilter.GetOrganization()),
+				OrganizationalUnit:     stringOrNull(certFilter.GetOrganizationalUnit()),
+				SubjectAlternativeName: stringOrNull(certFilter.GetSubjectAlternativeName()),
+			}
+			obj, diag := types.ObjectValueFrom(ctx, namespaceCertificateFilterAttrs, model)
+			diags.Append(diag...)
+			if diags.HasError() {
+				return
+			}
+			certificateFilterObjects[i] = obj
 		}
-		obj, diag := types.ObjectValueFrom(ctx, namespaceCertificateFilterAttrs, model)
+
+		filters, diag := types.ListValueFrom(ctx, types.ObjectType{AttrTypes: namespaceCertificateFilterAttrs}, certificateFilterObjects)
 		diags.Append(diag...)
 		if diags.HasError() {
 			return
 		}
-		certificateFilterObjects[i] = obj
-	}
 
-	certificateFilter, diag := types.ListValueFrom(ctx, types.ObjectType{AttrTypes: namespaceCertificateFilterAttrs}, certificateFilterObjects)
-	diags.Append(diag...)
-	if diags.HasError() {
-		return
+		certificateFilter = filters
 	}
+
 	state.Regions = planRegions
 	state.CertificateFilters = certificateFilter
 	state.AcceptedClientCA = types.StringValue(ns.GetSpec().GetMtlsAuth().GetAcceptedClientCa())
@@ -386,6 +392,10 @@ func getCertFiltersFromModel(ctx context.Context, diags diag.Diagnostics, model
 		return nil
 	}
 
+	if len(elements) == 0 {
+		return nil
+	}
+
 	certificateFilters := make([]*namespacev1.CertificateFilterSpec, len(elements))
 	for i, filter := range elements {
 		var model namespaceCertificateFilterModel
diff --git a/internal/provider/namespace_resource_test.go b/internal/provider/namespace_resource_test.go
index 85f9e39..a453795 100644
--- a/internal/provider/namespace_resource_test.go
+++ b/internal/provider/namespace_resource_test.go
@@ -8,13 +8,42 @@ import (
 )
 
 func TestAccBasicNamespace(t *testing.T) {
+	config := func(name string, retention int) string {
+		return fmt.Sprintf(`
+provider "temporalcloud" {
+
+}
+
+resource "temporalcloud_namespace" "terraform" {
+  name               = "%s"
+  regions            = ["aws-us-east-1"]
+  accepted_client_ca = base64encode(<<PEM
+-----BEGIN CERTIFICATE-----
+MIIByTCCAVCgAwIBAgIRAWHkC+6JUf3s9Tq43mdp2zgwCgYIKoZIzj0EAwMwEzER
+MA8GA1UEChMIdGVtcG9yYWwwHhcNMjMwODEwMDAwOTQ1WhcNMjQwODA5MDAxMDQ1
+WjATMREwDwYDVQQKEwh0ZW1wb3JhbDB2MBAGByqGSM49AgEGBSuBBAAiA2IABCzQ
+7DwwGSQKM6Zrx3Qtw7IubfxiJ3RSXCqmcGhEbFVeocwAdEgMYlwSlUiWtDZVR2dM
+XM9UZLWK4aGGnDNS5Mhcz6ibSBS7Owf4tRZZA9SpFCjNw2HraaiUVV+EUgxoe6No
+MGYwDgYDVR0PAQH/BAQDAgGGMA8GA1UdEwEB/wQFMAMBAf8wHQYDVR0OBBYEFG4N
+8lIXqQKxwVs/ixVzdF6XGZm+MCQGA1UdEQQdMBuCGWNsaWVudC5yb290LnRlbXBv
+cmFsLlB1VHMwCgYIKoZIzj0EAwMDZwAwZAIwRLfm9S7rKGd30KdQvUMcOcDJlmDw
+6/oM6UOJFxLeGcpYbgxQ/bFize+Yx9Q9kNeMAjA7GiFsaipaKtWHy5MCOCas3ZP6
++ttLaXNXss3Z5Wk5vhDQnyE8JR3rPeQ2cHXLiA0=
+-----END CERTIFICATE-----
+PEM
+)
+
+  retention_days     = %d
+}`, name, retention)
+	}
+
 	resource.Test(t, resource.TestCase{
 		PreCheck:                 func() { testAccPreCheck(t) },
 		ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
 		Steps: []resource.TestStep{
 			{
 				// New namespace with retention of 7
-				Config: testAccBasicNamespaceConfig("terraform-test", 7),
+				Config: config("tf-basic-namespace", 7),
 			},
 			/* Does not work yet: CLD-1971
 			{
@@ -25,10 +54,12 @@ func TestAccBasicNamespace(t *testing.T) {
 			// Delete testing automatically occurs in TestCase
 		},
 	})
+
 }
 
-func testAccBasicNamespaceConfig(name string, retention int) string {
-	return fmt.Sprintf(`
+func TestAccBasicNamespaceWithCertFilters(t *testing.T) {
+	config := func(name string, retention int) string {
+		return fmt.Sprintf(`
 provider "temporalcloud" {
 
 }
@@ -61,4 +92,24 @@ PEM
 
 }
 	`, name, retention)
+
+	}
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:                 func() { testAccPreCheck(t) },
+		ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
+		Steps: []resource.TestStep{
+			{
+				// New namespace with retention of 7
+				Config: config("terraform-test", 7),
+			},
+			/* Does not work yet: CLD-1971
+			{
+				// Update retention to 14
+				Config: testAccBasicNamespaceConfig("terraform-test", 14),
+			},
+			*/
+			// Delete testing automatically occurs in TestCase
+		},
+	})
 }