Merge pull request #1492 from hashicorp/bugfixes/consistency

Bugfixes for consistency issues

internal/services/administrativeunits/administrative_unit_resource.go

@@ -8,15 +8,19 @@ import (
 	"errors"
 	"fmt"
 	"log"
+	"net/http"
 	"strings"
 	"time"
 
 	"github.com/hashicorp/go-azure-helpers/lang/pointer"
 	"github.com/hashicorp/go-azure-helpers/lang/response"
+	administrativeunitBeta "github.com/hashicorp/go-azure-sdk/microsoft-graph/administrativeunits/beta/administrativeunit"
+	"github.com/hashicorp/go-azure-sdk/microsoft-graph/common-types/beta"
 	"github.com/hashicorp/go-azure-sdk/microsoft-graph/common-types/stable"
 	"github.com/hashicorp/go-azure-sdk/microsoft-graph/directory/stable/administrativeunit"
 	"github.com/hashicorp/go-azure-sdk/microsoft-graph/directory/stable/administrativeunitmember"
 	"github.com/hashicorp/go-azure-sdk/sdk/nullable"
+	"github.com/hashicorp/go-azure-sdk/sdk/odata"
 	"github.com/hashicorp/go-uuid"
 	"github.com/hashicorp/terraform-provider-azuread/internal/clients"
 	"github.com/hashicorp/terraform-provider-azuread/internal/helpers/consistency"
@@ -188,23 +192,31 @@ func administrativeUnitResourceCreate(ctx context.Context, d *pluginsdk.Resource
 	id := stable.NewDirectoryAdministrativeUnitID(*administrativeUnit.Id)
 	updateResp, err := client.UpdateAdministrativeUnit(ctx, id, stable.AdministrativeUnit{
 		DisplayName: nullable.Value(tempDisplayName),
-	}, administrativeunit.DefaultUpdateAdministrativeUnitOperationOptions())
+	}, administrativeunit.UpdateAdministrativeUnitOperationOptions{
+		RetryFunc: func(resp *http.Response, o *odata.OData) (bool, error) {
+			return response.WasNotFound(resp), nil
+		},
+	})
 	if err != nil {
 		if response.WasNotFound(updateResp.HttpResponse) {
-			return tf.ErrorDiagF(err, "Timed out whilst waiting for new administrative unit to be replicated in Azure AD")
+			return tf.ErrorDiagF(err, "Timed out whilst waiting for new %s to be replicated in Azure AD", id)
 		}
-		return tf.ErrorDiagF(err, "Failed to patch administrative unit after creating")
+		return tf.ErrorDiagF(err, "Failed to patch %s after creating", id)
 	}
 
 	// Set correct original display name
 	updateResp, err = client.UpdateAdministrativeUnit(ctx, id, stable.AdministrativeUnit{
 		DisplayName: nullable.Value(displayName),
-	}, administrativeunit.DefaultUpdateAdministrativeUnitOperationOptions())
+	}, administrativeunit.UpdateAdministrativeUnitOperationOptions{
+		RetryFunc: func(resp *http.Response, o *odata.OData) (bool, error) {
+			return response.WasNotFound(resp), nil
+		},
+	})
 	if err != nil {
 		if response.WasNotFound(updateResp.HttpResponse) {
-			return tf.ErrorDiagF(err, "Timed out whilst waiting for new administrative unit to be replicated in Azure AD")
+			return tf.ErrorDiagF(err, "Timed out whilst waiting for new %s to be replicated in Azure AD", id)
 		}
-		return tf.ErrorDiagF(err, "Failed to patch administrative unit after creating")
+		return tf.ErrorDiagF(err, "Failed to patch %s after creating", id)
 	}
 
 	// Add members after the administrative unit is created
@@ -217,7 +229,7 @@ func administrativeUnitResourceCreate(ctx context.Context, d *pluginsdk.Resource
 			}
 
 			if _, err = memberClient.AddAdministrativeUnitMemberRef(ctx, id, addMemberProperties, administrativeunitmember.DefaultAddAdministrativeUnitMemberRefOperationOptions()); err != nil {
-				return tf.ErrorDiagF(err, "Could not add member %q to administrative unit with object ID: %q", memberId, d.Id())
+				return tf.ErrorDiagF(err, "Could not add member %q to %s", memberId, id)
 			}
 		}
 	}
@@ -266,13 +278,13 @@ func administrativeUnitResourceUpdate(ctx context.Context, d *pluginsdk.Resource
 	}
 
 	if _, err := client.UpdateAdministrativeUnit(ctx, id, administrativeUnit, administrativeunit.DefaultUpdateAdministrativeUnitOperationOptions()); err != nil {
-		return tf.ErrorDiagF(err, "Updating administrative unit with ID: %q", d.Id())
+		return tf.ErrorDiagF(err, "Updating %s", id)
 	}
 
 	if d.HasChange("members") {
 		membersResp, err := memberClient.ListAdministrativeUnitMembers(ctx, id, administrativeunitmember.DefaultListAdministrativeUnitMembersOperationOptions())
 		if err != nil {
-			return tf.ErrorDiagF(err, "Could not retrieve members for administrative unit with object ID: %q", d.Id())
+			return tf.ErrorDiagF(err, "Could not retrieve members for %s", id)
 		}
 
 		existingMembers := make([]string, 0)
@@ -285,7 +297,7 @@ func administrativeUnitResourceUpdate(ctx context.Context, d *pluginsdk.Resource
 
 		for _, memberForRemoval := range membersForRemoval {
 			if _, err = memberClient.RemoveAdministrativeUnitMemberRef(ctx, stable.NewDirectoryAdministrativeUnitIdMemberID(administrativeUnitId, memberForRemoval), administrativeunitmember.DefaultRemoveAdministrativeUnitMemberRefOperationOptions()); err != nil {
-				return tf.ErrorDiagF(err, "Could not remove members from administrative unit with object ID: %q", d.Id())
+				return tf.ErrorDiagF(err, "Could not remove members from %s", id)
 			}
 		}
 
@@ -297,7 +309,7 @@ func administrativeUnitResourceUpdate(ctx context.Context, d *pluginsdk.Resource
 			}
 
 			if _, err = memberClient.AddAdministrativeUnitMemberRef(ctx, id, addMemberProperties, administrativeunitmember.DefaultAddAdministrativeUnitMemberRefOperationOptions()); err != nil {
-				return tf.ErrorDiagF(err, "Could not add member %q to administrative unit with object ID: %q", memberId, d.Id())
+				return tf.ErrorDiagF(err, "Could not add member %q to %s", memberId, id)
 			}
 		}
 	}
@@ -313,11 +325,11 @@ func administrativeUnitResourceRead(ctx context.Context, d *pluginsdk.ResourceDa
 	resp, err := client.GetAdministrativeUnit(ctx, id, administrativeunit.DefaultGetAdministrativeUnitOperationOptions())
 	if err != nil {
 		if response.WasNotFound(resp.HttpResponse) {
-			log.Printf("[DEBUG] Administrative Unit with ID %q was not found - removing from state", d.Id())
+			log.Printf("[DEBUG] %s was not found - removing from state", id)
 			d.SetId("")
 			return nil
 		}
-		return tf.ErrorDiagF(err, "Retrieving administrative unit with object ID: %q", d.Id())
+		return tf.ErrorDiagF(err, "Retrieving %s", id)
 	}
 
 	administrativeUnit := resp.Model
@@ -330,7 +342,7 @@ func administrativeUnitResourceRead(ctx context.Context, d *pluginsdk.ResourceDa
 
 	membersResp, err := memberClient.ListAdministrativeUnitMembers(ctx, id, administrativeunitmember.DefaultListAdministrativeUnitMembersOperationOptions())
 	if err != nil {
-		return tf.ErrorDiagF(err, "Could not retrieve members for administrative unit with object ID: %q", d.Id())
+		return tf.ErrorDiagF(err, "Could not retrieve members for %s", id)
 	}
 
 	members := make([]string, 0)
@@ -350,23 +362,24 @@ func administrativeUnitResourceRead(ctx context.Context, d *pluginsdk.ResourceDa
 
 func administrativeUnitResourceDelete(ctx context.Context, d *pluginsdk.ResourceData, meta interface{}) pluginsdk.Diagnostics {
 	client := meta.(*clients.Client).AdministrativeUnits.AdministrativeUnitClient
-	id := stable.NewDirectoryAdministrativeUnitID(d.Id())
+	clientBeta := meta.(*clients.Client).AdministrativeUnits.AdministrativeUnitClientBeta
+	id := beta.NewAdministrativeUnitID(d.Id())
 
-	if _, err := client.DeleteAdministrativeUnit(ctx, id, administrativeunit.DefaultDeleteAdministrativeUnitOperationOptions()); err != nil {
-		return tf.ErrorDiagF(err, "Deleting administrative unit with object ID: %q", id.AdministrativeUnitId)
+	if _, err := clientBeta.DeleteAdministrativeUnit(ctx, id, administrativeunitBeta.DefaultDeleteAdministrativeUnitOperationOptions()); err != nil {
+		return tf.ErrorDiagF(err, "Deleting %s", id)
 	}
 
 	// Wait for administrative unit object to be deleted
 	if err := consistency.WaitForDeletion(ctx, func(ctx context.Context) (*bool, error) {
-		if resp, err := client.GetAdministrativeUnit(ctx, id, administrativeunit.DefaultGetAdministrativeUnitOperationOptions()); err != nil {
+		if resp, err := client.GetAdministrativeUnit(ctx, stable.NewDirectoryAdministrativeUnitID(id.AdministrativeUnitId), administrativeunit.DefaultGetAdministrativeUnitOperationOptions()); err != nil {
 			if response.WasNotFound(resp.HttpResponse) {
 				return pointer.To(false), nil
 			}
 			return nil, err
 		}
 		return pointer.To(true), nil
 	}); err != nil {
-		return tf.ErrorDiagF(err, "Waiting for deletion of administrative unit with object ID %q", id.AdministrativeUnitId)
+		return tf.ErrorDiagF(err, "Waiting for deletion of %s", id)
 	}
 
 	return nil

internal/services/administrativeunits/client/client.go

@@ -4,6 +4,7 @@
 package client
 
 import (
+	administrativeunitBeta "github.com/hashicorp/go-azure-sdk/microsoft-graph/administrativeunits/beta/administrativeunit"
 	"github.com/hashicorp/go-azure-sdk/microsoft-graph/directory/stable/administrativeunit"
 	"github.com/hashicorp/go-azure-sdk/microsoft-graph/directory/stable/administrativeunitmember"
 	"github.com/hashicorp/go-azure-sdk/microsoft-graph/directory/stable/administrativeunitscopedrolemember"
@@ -12,6 +13,7 @@ import (
 
 type Client struct {
 	AdministrativeUnitClient                 *administrativeunit.AdministrativeUnitClient
+	AdministrativeUnitClientBeta             *administrativeunitBeta.AdministrativeUnitClient
 	AdministrativeUnitMemberClient           *administrativeunitmember.AdministrativeUnitMemberClient
 	AdministrativeUnitScopedRoleMemberClient *administrativeunitscopedrolemember.AdministrativeUnitScopedRoleMemberClient
 }
@@ -23,6 +25,13 @@ func NewClient(o *common.ClientOptions) (*Client, error) {
 	}
 	o.Configure(administrativeUnitClient.Client)
 
+	// Beta API needed to delete administrative units - the stable API is broken and returns 404 with `{"Message":"The OData path is invalid."}`
+	administrativeUnitClientBeta, err := administrativeunitBeta.NewAdministrativeUnitClientWithBaseURI(o.Environment.MicrosoftGraph)
+	if err != nil {
+		return nil, err
+	}
+	o.Configure(administrativeUnitClientBeta.Client)
+
 	memberClient, err := administrativeunitmember.NewAdministrativeUnitMemberClientWithBaseURI(o.Environment.MicrosoftGraph)
 	if err != nil {
 		return nil, err
@@ -37,6 +46,7 @@ func NewClient(o *common.ClientOptions) (*Client, error) {
 
 	return &Client{
 		AdministrativeUnitClient:                 administrativeUnitClient,
+		AdministrativeUnitClientBeta:             administrativeUnitClientBeta,
 		AdministrativeUnitMemberClient:           memberClient,
 		AdministrativeUnitScopedRoleMemberClient: scopedRoleMemberClient,
 	}, nil

internal/services/applications/application_data_source.go

@@ -600,8 +600,8 @@ func applicationDataSourceRead(ctx context.Context, d *pluginsdk.ResourceData, m
 	d.SetId(id.ID())
 
 	tf.Set(d, "api", flattenApplicationApi(app.Api, true))
-	tf.Set(d, "app_roles", flattenApplicationAppRoles(app.AppRoles))
-	tf.Set(d, "app_role_ids", flattenApplicationAppRoleIDs(app.AppRoles))
+	tf.Set(d, "app_roles", applications.FlattenAppRoles(app.AppRoles))
+	tf.Set(d, "app_role_ids", applications.FlattenAppRoleIDs(app.AppRoles))
 	tf.Set(d, "application_id", app.AppId.GetOrZero())
 	tf.Set(d, "client_id", app.AppId.GetOrZero())
 	tf.Set(d, "device_only_auth_enabled", app.IsDeviceOnlyAuthSupported.GetOrZero())
@@ -624,7 +624,7 @@ func applicationDataSourceRead(ctx context.Context, d *pluginsdk.ResourceData, m
 	tf.Set(d, "web", flattenApplicationWeb(app.Web))
 
 	if app.Api != nil {
-		tf.Set(d, "oauth2_permission_scope_ids", flattenApplicationOAuth2PermissionScopeIDs(app.Api.OAuth2PermissionScopes))
+		tf.Set(d, "oauth2_permission_scope_ids", applications.FlattenOAuth2PermissionScopeIDs(app.Api.OAuth2PermissionScopes))
 	}
 
 	if app.Info != nil {

internal/services/applications/application_resource.go

@@ -1633,8 +1633,8 @@ func applicationResourceRead(ctx context.Context, d *pluginsdk.ResourceData, met
 	}
 
 	tf.Set(d, "api", flattenApplicationApi(app.Api, false))
-	tf.Set(d, "app_role", flattenApplicationAppRoles(app.AppRoles))
-	tf.Set(d, "app_role_ids", flattenApplicationAppRoleIDs(app.AppRoles))
+	tf.Set(d, "app_role", applications.FlattenAppRoles(app.AppRoles))
+	tf.Set(d, "app_role_ids", applications.FlattenAppRoleIDs(app.AppRoles))
 	tf.Set(d, "application_id", app.AppId.GetOrZero())
 	tf.Set(d, "client_id", app.AppId.GetOrZero())
 	tf.Set(d, "description", app.Description.GetOrZero())
@@ -1659,7 +1659,7 @@ func applicationResourceRead(ctx context.Context, d *pluginsdk.ResourceData, met
 	tf.Set(d, "web", flattenApplicationWeb(app.Web))
 
 	if app.Api != nil {
-		tf.Set(d, "oauth2_permission_scope_ids", flattenApplicationOAuth2PermissionScopeIDs(app.Api.OAuth2PermissionScopes))
+		tf.Set(d, "oauth2_permission_scope_ids", applications.FlattenOAuth2PermissionScopeIDs(app.Api.OAuth2PermissionScopes))
 	}
 
 	if app.Info != nil {

internal/services/applications/application_resource_test.go

@@ -292,6 +292,8 @@ func TestAccApplication_oauth2PermissionScopes(t *testing.T) {
 		data.UUID(),
 	}
 
+	// Note: ImportSteps missing here due to inconsistencies with SDKv2 handling of sets
+
 	data.ResourceTest(t, r, []acceptance.TestStep{
 		{
 			Config: r.basic(data),
@@ -301,7 +303,6 @@ func TestAccApplication_oauth2PermissionScopes(t *testing.T) {
 				check.That(data.ResourceName).Key("oauth2_permission_scope_ids.%").HasValue("0"),
 			),
 		},
-		data.ImportStep(),
 		{
 			Config: r.oauth2PermissionScopes(data, scopeIDs),
 			Check: acceptance.ComposeTestCheckFunc(
@@ -310,7 +311,6 @@ func TestAccApplication_oauth2PermissionScopes(t *testing.T) {
 				check.That(data.ResourceName).Key("oauth2_permission_scope_ids.%").HasValue("2"),
 			),
 		},
-		data.ImportStep(),
 		{
 			Config: r.oauth2PermissionScopesUpdate(data, scopeIDs),
 			Check: acceptance.ComposeTestCheckFunc(
@@ -319,7 +319,6 @@ func TestAccApplication_oauth2PermissionScopes(t *testing.T) {
 				check.That(data.ResourceName).Key("oauth2_permission_scope_ids.%").HasValue("3"),
 			),
 		},
-		data.ImportStep(),
 		{
 			Config: r.oauth2PermissionScopes(data, scopeIDs),
 			Check: acceptance.ComposeTestCheckFunc(
@@ -328,7 +327,6 @@ func TestAccApplication_oauth2PermissionScopes(t *testing.T) {
 				check.That(data.ResourceName).Key("oauth2_permission_scope_ids.%").HasValue("2"),
 			),
 		},
-		data.ImportStep(),
 		{
 			Config: r.basic(data),
 			Check: acceptance.ComposeTestCheckFunc(
@@ -337,7 +335,6 @@ func TestAccApplication_oauth2PermissionScopes(t *testing.T) {
 				check.That(data.ResourceName).Key("oauth2_permission_scope_ids.%").HasValue("0"),
 			),
 		},
-		data.ImportStep(),
 	})
 }
 

internal/services/applications/applications.go

@@ -684,19 +684,11 @@ func flattenApplicationApi(in *stable.ApiApplication, dataSource bool) []map[str
 	return []map[string]interface{}{{
 		"known_client_applications":      tf.FlattenStringSlicePtr(in.KnownClientApplications),
 		"mapped_claims_enabled":          mappedClaims,
-		scopesKey:                        flattenApplicationOAuth2PermissionScopes(in.OAuth2PermissionScopes),
+		scopesKey:                        applications.FlattenOAuth2PermissionScopes(in.OAuth2PermissionScopes),
 		"requested_access_token_version": accessTokenVersion,
 	}}
 }
 
-func flattenApplicationAppRoleIDs(in *[]stable.AppRole) map[string]string {
-	return applications.FlattenAppRoleIDs(in)
-}
-
-func flattenApplicationAppRoles(in *[]stable.AppRole) []map[string]interface{} {
-	return applications.FlattenAppRoles(in)
-}
-
 func flattenApplicationGroupMembershipClaims(in nullable.Type[string]) []interface{} {
 	if in.IsNull() {
 		return []interface{}{}
@@ -721,14 +713,6 @@ func flattenApplicationImplicitGrant(in *stable.ImplicitGrantSettings) []map[str
 	}}
 }
 
-func flattenApplicationOAuth2PermissionScopeIDs(in *[]stable.PermissionScope) map[string]string {
-	return applications.FlattenOAuth2PermissionScopeIDs(in)
-}
-
-func flattenApplicationOAuth2PermissionScopes(in *[]stable.PermissionScope) []map[string]interface{} {
-	return applications.FlattenOAuth2PermissionScopes(in)
-}
-
 func flattenApplicationOptionalClaims(in *stable.OptionalClaims) []map[string]interface{} {
 	if in == nil {
 		return []map[string]interface{}{}

internal/services/directoryroles/directory_role_eligibility_schedule_request_resource.go

@@ -32,7 +32,7 @@ func directoryRoleEligibilityScheduleRequestResource() *pluginsdk.Resource {
 		DeleteContext: directoryRoleEligibilityScheduleRequestResourceDelete,
 
 		Timeouts: &pluginsdk.ResourceTimeout{
-			Create: pluginsdk.DefaultTimeout(5 * time.Minute),
+			Create: pluginsdk.DefaultTimeout(10 * time.Minute),
 			Read:   pluginsdk.DefaultTimeout(5 * time.Minute),
 			Delete: pluginsdk.DefaultTimeout(5 * time.Minute),
 		},

internal/services/directoryroles/directory_role_eligibility_schedule_request_resource_test.go

@@ -34,20 +34,6 @@ func TestAccRoleEligibilityScheduleRequest_builtin(t *testing.T) {
 	})
 }
 
-func TestAccRoleEligibilityScheduleRequest_custom(t *testing.T) {
-	data := acceptance.BuildTestData(t, "azuread_directory_role_eligibility_schedule_request", "test")
-	r := RoleEligibilityScheduleRequestResource{}
-
-	data.ResourceTestIgnoreDangling(t, r, []acceptance.TestStep{
-		{
-			Config: r.custom(data),
-			Check: acceptance.ComposeTestCheckFunc(
-				check.That(data.ResourceName).ExistsInAzure(r),
-			),
-		},
-	})
-}
-
 func (r RoleEligibilityScheduleRequestResource) Exists(ctx context.Context, clients *clients.Client, state *terraform.InstanceState) (*bool, error) {
 	client := clients.DirectoryRoles.DirectoryRoleEligibilityScheduleRequestClient
 	id := stable.NewRoleManagementDirectoryRoleEligibilityScheduleRequestID(state.ID)
@@ -89,36 +75,3 @@ resource "azuread_directory_role_eligibility_schedule_request" "test" {
 }
 `, data.RandomInteger, data.RandomPassword)
 }
-
-func (r RoleEligibilityScheduleRequestResource) custom(data acceptance.TestData) string {
-	return fmt.Sprintf(`
-provider "azuread" {}
-
-data "azuread_domains" "test" {
-  only_initial = true
-}
-
-resource "azuread_user" "test" {
-  user_principal_name = "acctestManager.%[1]d@${data.azuread_domains.test.domains.0.domain_name}"
-  display_name        = "acctestManager-%[1]d"
-  password            = "%[2]s"
-}
-
-resource "azuread_custom_directory_role" "test" {
-  display_name = "acctestCustomRole-%[1]d"
-  enabled      = true
-  version      = "1.0"
-
-  permissions {
-    allowed_resource_actions = ["microsoft.directory/applications/standard/read"]
-  }
-}
-
-resource "azuread_directory_role_eligibility_schedule_request" "test" {
-  role_definition_id = azuread_custom_directory_role.test.object_id
-  principal_id       = azuread_user.test.object_id
-  directory_scope_id = "/"
-  justification      = "abc"
-}
-`, data.RandomInteger, data.RandomPassword)
-}