Allow to change display_name for databricks_service_principal (#3523)

* Allow to change `display_name` for `databricks_service_principal`

We now allow to change display name of service principals, so we can remove `force_new`
for it.

Fixes #3519

* Address review comments and fix few tests

Author: alexott

docs/resources/service_principal.md

@@ -98,7 +98,7 @@ resource "databricks_service_principal" "sp" {
 The following arguments are available:
 
 - `application_id` This is the Azure Application ID of the given Azure service principal and will be their form of access and identity. For Databricks-managed service principals this value is auto-generated.
-- `display_name` - (Required) This is an alias for the service principal and can be the full name of the service principal.
+- `display_name` - (Required for Databricks-managed service principals) This is an alias for the service principal and can be the full name of the service principal.
 - `external_id` - (Optional) ID of the service principal in an external identity provider.
 - `allow_cluster_create` - (Optional) Allow the service principal to have [cluster](cluster.md) create privileges. Defaults to false. More fine grained permissions could be assigned with [databricks_permissions](permissions.md#Cluster-usage) and `cluster_id` argument. Everyone without `allow_cluster_create` argument set, but with [permission to use](permissions.md#Cluster-Policy-usage) Cluster Policy would be able to create clusters, but within the boundaries of that specific policy.
 - `allow_instance_pool_create` - (Optional) Allow the service principal to have [instance pool](instance_pool.md) create privileges. Defaults to false. More fine grained permissions could be assigned with [databricks_permissions](permissions.md#Instance-Pool-usage) and [instance_pool_id](permissions.md#instance_pool_id) argument.

internal/acceptance/service_principal_test.go

@@ -3,6 +3,7 @@ package acceptance
 import (
 	"context"
 	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/databricks/databricks-sdk-go"
@@ -91,28 +92,40 @@ func TestAccServicePrinicpalHomeDeleteNotDeleted(t *testing.T) {
 
 func TestMwsAccServicePrincipalResourceOnAzure(t *testing.T) {
 	GetEnvOrSkipTest(t, "ARM_CLIENT_ID")
+	azureSpnRenamed := strings.ReplaceAll(azureSpn, `"SPN `, `"SPN Renamed `)
 	accountLevel(t, step{
 		Template: azureSpn,
+	}, step{
+		Template: azureSpnRenamed,
 	})
 }
 
 func TestAccServicePrincipalResourceOnAzure(t *testing.T) {
 	GetEnvOrSkipTest(t, "ARM_CLIENT_ID")
+	azureSpnRenamed := strings.ReplaceAll(azureSpn, `"SPN `, `"SPN Renamed `)
 	workspaceLevel(t, step{
 		Template: azureSpn,
+	}, step{
+		Template: azureSpnRenamed,
 	})
 }
 
 func TestMwsAccServicePrincipalResourceOnAws(t *testing.T) {
 	GetEnvOrSkipTest(t, "TEST_ROOT_BUCKET")
+	awsSpnRenamed := strings.ReplaceAll(awsSpn, `"SPN `, `"SPN Renamed `)
 	accountLevel(t, step{
 		Template: awsSpn,
+	}, step{
+		Template: awsSpnRenamed,
 	})
 }
 
 func TestAccServicePrincipalResourceOnAws(t *testing.T) {
 	GetEnvOrSkipTest(t, "TEST_EC2_INSTANCE_PROFILE")
+	awsSpnRenamed := strings.ReplaceAll(awsSpn, `"SPN `, `"SPN Renamed `)
 	workspaceLevel(t, step{
 		Template: awsSpn,
+	}, step{
+		Template: awsSpnRenamed,
 	})
 }

scim/resource_service_principal.go

@@ -99,7 +99,7 @@ func (a ServicePrincipalsAPI) Delete(servicePrincipalID string) error {
 func ResourceServicePrincipal() common.Resource {
 	type entity struct {
 		ApplicationID string `json:"application_id,omitempty" tf:"computed,force_new"`
-		DisplayName   string `json:"display_name,omitempty" tf:"computed,force_new"`
+		DisplayName   string `json:"display_name,omitempty" tf:"computed"`
 		Active        bool   `json:"active,omitempty"`
 		ExternalID    string `json:"external_id,omitempty" tf:"suppress_diff"`
 	}
@@ -138,6 +138,8 @@ func ResourceServicePrincipal() common.Resource {
 				Optional: true,
 				Computed: true,
 			}
+			m["application_id"].AtLeastOneOf = []string{"application_id", "display_name"}
+			m["display_name"].AtLeastOneOf = []string{"application_id", "display_name"}
 			return m
 		})
 	spFromData := func(d *schema.ResourceData) User {

scim/resource_service_principal_test.go

@@ -141,6 +141,16 @@ func TestResourceServicePrincipalCreate(t *testing.T) {
 	}.ApplyNoError(t)
 }
 
+func TestResourceServicePrincipalCreate_ErrorNoDisplayNameOrAppId(t *testing.T) {
+	qa.ResourceFixture{
+		Resource: ResourceServicePrincipal(),
+		Create:   true,
+		HCL: `
+		allow_cluster_create = true
+		`,
+	}.ExpectError(t, "invalid config supplied. [application_id] Missing required argument. [display_name] Missing required argument")
+}
+
 func TestResourceServicePrincipalCreate_Error(t *testing.T) {
 	qa.ResourceFixture{
 		Fixtures: qa.HTTPFailures,
@@ -541,7 +551,7 @@ func TestResourceServicePrincipalDelete_NonExistingDir(t *testing.T) {
 }
 
 func TestCreateForceOverridesManuallyAddedServicePrincipalErrorNotMatched(t *testing.T) {
-	d := ResourceUser().ToResource().TestResourceData()
+	d := ResourceServicePrincipal().ToResource().TestResourceData()
 	d.Set("force", true)
 	rerr := createForceOverridesManuallyAddedServicePrincipal(
 		fmt.Errorf("nonsense"), d,
@@ -561,7 +571,7 @@ func TestCreateForceOverwriteCannotListServicePrincipals(t *testing.T) {
 			},
 		},
 	}, func(ctx context.Context, client *common.DatabricksClient) {
-		d := ResourceUser().ToResource().TestResourceData()
+		d := ResourceServicePrincipal().ToResource().TestResourceData()
 		d.Set("force", true)
 		err := createForceOverridesManuallyAddedServicePrincipal(
 			fmt.Errorf("Service principal with application ID %s already exists.", appID),
@@ -624,7 +634,7 @@ func TestCreateForceOverwriteFindsAndSetsServicePrincipalID(t *testing.T) {
 			},
 		},
 	}, func(ctx context.Context, client *common.DatabricksClient) {
-		d := ResourceUser().ToResource().TestResourceData()
+		d := ResourceServicePrincipal().ToResource().TestResourceData()
 		d.Set("force", true)
 		d.Set("application_id", appID)
 		err := createForceOverridesManuallyAddedServicePrincipal(