Improve how we guard against erasing unwanted enabled clients

auth0 · May 9, 2023 · 1697650 · 1697650
1 parent 4ecc7d4
commit 1697650
Show file tree

Hide file tree

Showing 3 changed files with 172 additions and 224 deletions.
diff --git a/internal/auth0/connection/resource_clients.go b/internal/auth0/connection/resource_clients.go
@@ -2,9 +2,11 @@ package connection
 
 import (
 	"context"
+	"fmt"
 	"net/http"
 
 	"github.com/auth0/go-auth0/management"
+	"github.com/google/go-cmp/cmp"
 	"github.com/hashicorp/go-multierror"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
@@ -72,24 +74,23 @@ func createConnectionClients(ctx context.Context, data *schema.ResourceData, met
 		return diag.FromErr(err)
 	}
 
-	data.SetId(connection.GetID())
+	// This is never nil because the enabled clients is a required parameter.
+	enabledClients := value.Strings(data.GetRawConfig().GetAttr("enabled_clients"))
 
-	if len(connection.GetEnabledClients()) != 0 {
+	if diagnostics := guardAgainstErasingUnwantedEnabledClients(
+		connection.GetID(),
+		*enabledClients,
+		connection.GetEnabledClients(),
+	); diagnostics.HasError() {
 		data.SetId("")
-
-		return diag.Diagnostics{
-			diag.Diagnostic{
-				Severity: diag.Error,
-				Summary:  "Connection with non empty enabled clients",
-				Detail: "The connection already has enabled clients attached to it. " +
-					"Import the resource instead to get an accurate diff that can be reviewed.",
-			},
-		}
+		return diagnostics
 	}
 
+	data.SetId(connection.GetID())
+
 	if err := api.Connection.Update(
 		connectionID,
-		&management.Connection{EnabledClients: value.Strings(data.GetRawConfig().GetAttr("enabled_clients"))},
+		&management.Connection{EnabledClients: enabledClients},
 	); err != nil {
 		return diag.FromErr(err)
 	}
@@ -167,3 +168,28 @@ func deleteConnectionClients(ctx context.Context, data *schema.ResourceData, met
 
 	return nil
 }
+
+func guardAgainstErasingUnwantedEnabledClients(
+	connectionID string,
+	configEnabledClients []string,
+	connectionEnabledClients []string,
+) diag.Diagnostics {
+	if len(connectionEnabledClients) == 0 {
+		return nil
+	}
+
+	if cmp.Equal(configEnabledClients, connectionEnabledClients) {
+		return nil
+	}
+
+	return diag.Diagnostics{
+		diag.Diagnostic{
+			Severity: diag.Error,
+			Summary:  "Connection with non empty enabled clients",
+			Detail: cmp.Diff(configEnabledClients, connectionEnabledClients) +
+				fmt.Sprintf("\nThe connection already has enabled clients attached to it. "+
+					"Import the resource instead in order to proceed with the changes. "+
+					"Run: 'terraform import auth0_connection_clients.<given-name> %s'.", connectionID),
+		},
+	}
+}
diff --git a/internal/auth0/connection/resource_clients_test.go b/internal/auth0/connection/resource_clients_test.go
@@ -96,15 +96,6 @@ resource "auth0_connection_clients" "my_conn_client_assoc" {
 }
 `
 
-const testAccConnectionClientsWithNoEnabledClients = givenASingleConnection + `
-resource "auth0_connection_clients" "my_conn_client_assoc" {
-	depends_on = [ auth0_connection.my_conn ]
-
-	connection_id = auth0_connection.my_conn.id
-	enabled_clients = []
-}
-`
-
 func TestAccConnectionClients(t *testing.T) {
 	acctest.Test(t, resource.TestCase{
 		Steps: []resource.TestStep{
@@ -133,11 +124,14 @@ func TestAccConnectionClients(t *testing.T) {
 				),
 			},
 			{
-				Config: acctest.ParseTestName(testAccConnectionClientsWithNoEnabledClients, t.Name()),
+				Config: acctest.ParseTestName(givenASingleConnection, t.Name()),
+			},
+			{
+				RefreshState: true,
 				Check: resource.ComposeTestCheckFunc(
-					resource.TestCheckResourceAttr("auth0_connection_clients.my_conn_client_assoc", "strategy", "auth0"),
-					resource.TestCheckResourceAttr("auth0_connection_clients.my_conn_client_assoc", "name", fmt.Sprintf("Acceptance-Test-Connection-%s", t.Name())),
-					resource.TestCheckResourceAttr("auth0_connection_clients.my_conn_client_assoc", "enabled_clients.#", "0"),
+					resource.TestCheckResourceAttr("auth0_connection.my_conn", "strategy", "auth0"),
+					resource.TestCheckResourceAttr("auth0_connection.my_conn", "name", fmt.Sprintf("Acceptance-Test-Connection-%s", t.Name())),
+					resource.TestCheckResourceAttr("auth0_connection.my_conn", "enabled_clients.#", "0"),
 				),
 			},
 		},