Workaround API consistency bugs for various resources

go.mod

@@ -8,7 +8,6 @@ require (
 	github.com/aws/aws-sdk-go v1.38.43 // indirect
 	github.com/fatih/color v1.11.0 // indirect
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
-	github.com/google/uuid v1.1.2
 	github.com/hashicorp/errwrap v1.1.0 // indirect
 	github.com/hashicorp/go-cty v1.4.1-0.20200414143053-d3edf31b6320
 	github.com/hashicorp/go-hclog v0.16.1 // indirect

go.sum

@@ -187,7 +187,6 @@ github.com/google/pprof v0.0.0-20210122040257-d980be63207e/go.mod h1:kpwsk12EmLe
 github.com/google/pprof v0.0.0-20210226084205-cbba55b83ad5/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE=
 github.com/google/pprof v0.0.0-20210506205249-923b5ab0fc1a/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE=
 github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI=
-github.com/google/uuid v1.1.2 h1:EVhdT+1Kseyi1/pUmXKaFxYsDNy9RQYkMWRH68J/W7Y=
 github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/googleapis/gax-go/v2 v2.0.4/go.mod h1:0Wqv26UfaUD9n4G6kQubkQ+KchISgw+vpHVxEJEs9eg=
 github.com/googleapis/gax-go/v2 v2.0.5 h1:sjZBwGj9Jlw33ImPtvFviGYvseOtDM7hkSKB7+Tv3SM=

internal/acceptance/data.go

@@ -6,7 +6,7 @@ import (
 	"strconv"
 	"testing"
 
-	"github.com/google/uuid"
+	"github.com/hashicorp/go-uuid"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest"
 
 	"github.com/hashicorp/terraform-provider-azuread/internal/tf"
@@ -39,7 +39,11 @@ type TestData struct {
 }
 
 func (t *TestData) UUID() string {
-	return uuid.New().String()
+	uuid, err := uuid.GenerateUUID()
+	if err != nil {
+		panic(err)
+	}
+	return uuid
 }
 
 // BuildTestData generates some test data for the given resource
@@ -49,14 +53,15 @@ func BuildTestData(t *testing.T, resourceType string, resourceLabel string) Test
 	testData := TestData{
 		RandomInteger:  tf.AccRandTimeInt(),
 		RandomString:   acctest.RandString(5),
-		RandomID:       uuid.New().String(),
 		RandomPassword: fmt.Sprintf("%s%s", "p@$$Wd", acctest.RandString(6)),
 		ResourceName:   fmt.Sprintf("%s.%s", resourceType, resourceLabel),
 
 		ResourceType:  resourceType,
 		resourceLabel: resourceLabel,
 	}
 
+	testData.RandomID = testData.UUID()
+
 	return testData
 }
 

internal/common/client_options.go

@@ -45,7 +45,7 @@ func (o ClientOptions) ConfigureClient(c *msgraph.Client) {
 	*c.ResponseMiddlewares = append(*c.ResponseMiddlewares, o.responseLogger)
 
 	// Default retry limit, can be overridden from within a resource
-	c.RetryableClient.RetryMax = 8
+	c.RetryableClient.RetryMax = 9
 }
 
 func (o ClientOptions) requestLogger(req *http.Request) (*http.Request, error) {

internal/helpers/consistency.go

@@ -0,0 +1,43 @@
+package helpers
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"time"
+
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
+)
+
+type existsFunc func(ctx context.Context) (*bool, error)
+
+func WaitForDeletion(ctx context.Context, f existsFunc) error {
+	deadline, ok := ctx.Deadline()
+	if !ok {
+		return errors.New("context has no deadline")
+	}
+
+	timeout := time.Until(deadline)
+	_, err := (&resource.StateChangeConf{
+		Pending:                   []string{"Waiting"},
+		Target:                    []string{"Deleted"},
+		Timeout:                   timeout,
+		MinTimeout:                5 * time.Second,
+		ContinuousTargetOccurence: 5,
+		Refresh: func() (interface{}, string, error) {
+			exists, err := f(ctx)
+			if err != nil {
+				return nil, "Error", fmt.Errorf("retrieving resource: %+v", err)
+			}
+			if exists == nil {
+				return nil, "Error", fmt.Errorf("retrieving resource: exists was nil")
+			}
+			if *exists {
+				return "stub", "Waiting", nil
+			}
+			return "stub", "Deleted", nil
+		},
+	}).WaitForStateContext(ctx)
+
+	return err
+}

internal/helpers/credentials.go

@@ -28,6 +28,30 @@ func (e CredentialError) Error() string {
 	return e.str
 }
 
+func GetKeyCredential(keyCredentials *[]msgraph.KeyCredential, id string) (credential *msgraph.KeyCredential) {
+	if keyCredentials != nil {
+		for _, cred := range *keyCredentials {
+			if cred.KeyId != nil && strings.EqualFold(*cred.KeyId, id) {
+				credential = &cred
+				break
+			}
+		}
+	}
+	return
+}
+
+func GetPasswordCredential(passwordCredentials *[]msgraph.PasswordCredential, id string) (credential *msgraph.PasswordCredential) {
+	if passwordCredentials != nil {
+		for _, cred := range *passwordCredentials {
+			if cred.KeyId != nil && strings.EqualFold(*cred.KeyId, id) {
+				credential = &cred
+				break
+			}
+		}
+	}
+	return
+}
+
 func KeyCredentialForResource(d *schema.ResourceData) (*msgraph.KeyCredential, error) {
 	keyType := d.Get("type").(string)
 	value := d.Get("value").(string)

internal/services/applications/application_certificate_resource.go

@@ -20,6 +20,7 @@ import (
 	"github.com/hashicorp/terraform-provider-azuread/internal/helpers"
 	"github.com/hashicorp/terraform-provider-azuread/internal/services/applications/parse"
 	"github.com/hashicorp/terraform-provider-azuread/internal/tf"
+	"github.com/hashicorp/terraform-provider-azuread/internal/utils"
 	"github.com/hashicorp/terraform-provider-azuread/internal/validate"
 )
 
@@ -228,16 +229,7 @@ func applicationCertificateResourceRead(ctx context.Context, d *schema.ResourceD
 		return tf.ErrorDiagPathF(err, "application_object_id", "Retrieving Application with object ID %q", id.ObjectId)
 	}
 
-	var credential *msgraph.KeyCredential
-	if app.KeyCredentials != nil {
-		for _, cred := range *app.KeyCredentials {
-			if cred.KeyId != nil && strings.EqualFold(*cred.KeyId, id.KeyId) {
-				credential = &cred
-				break
-			}
-		}
-	}
-
+	credential := helpers.GetKeyCredential(app.KeyCredentials, id.KeyId)
 	if credential == nil {
 		log.Printf("[DEBUG] Certificate credential %q (ID %q) was not found - removing from state!", id.KeyId, id.ObjectId)
 		d.SetId("")
@@ -301,5 +293,24 @@ func applicationCertificateResourceDelete(ctx context.Context, d *schema.Resourc
 		return tf.ErrorDiagF(err, "Removing certificate credential %q from application with object ID %q", id.KeyId, id.ObjectId)
 	}
 
+	// Wait for application certificate to be deleted
+	if err := helpers.WaitForDeletion(ctx, func(ctx context.Context) (*bool, error) {
+		client.BaseClient.DisableRetries = true
+
+		app, _, err := client.Get(ctx, id.ObjectId, odata.Query{})
+		if err != nil {
+			return nil, err
+		}
+
+		credential := helpers.GetKeyCredential(app.KeyCredentials, id.KeyId)
+		if credential == nil {
+			return utils.Bool(false), nil
+		}
+
+		return utils.Bool(true), nil
+	}); err != nil {
+		return tf.ErrorDiagF(err, "Waiting for deletion of certificate credential %q from application with object ID %q", id.KeyId, id.ObjectId)
+	}
+
 	return nil
 }

internal/services/applications/application_password_resource.go

@@ -13,14 +13,14 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
-	"github.com/manicminer/hamilton/msgraph"
 	"github.com/manicminer/hamilton/odata"
 
 	"github.com/hashicorp/terraform-provider-azuread/internal/clients"
 	"github.com/hashicorp/terraform-provider-azuread/internal/helpers"
 	"github.com/hashicorp/terraform-provider-azuread/internal/services/applications/migrations"
 	"github.com/hashicorp/terraform-provider-azuread/internal/services/applications/parse"
 	"github.com/hashicorp/terraform-provider-azuread/internal/tf"
+	"github.com/hashicorp/terraform-provider-azuread/internal/utils"
 	"github.com/hashicorp/terraform-provider-azuread/internal/validate"
 )
 
@@ -219,16 +219,7 @@ func applicationPasswordResourceRead(ctx context.Context, d *schema.ResourceData
 		return tf.ErrorDiagPathF(err, "application_object_id", "Retrieving application with object ID %q", id.ObjectId)
 	}
 
-	var credential *msgraph.PasswordCredential
-	if app.PasswordCredentials != nil {
-		for _, cred := range *app.PasswordCredentials {
-			if cred.KeyId != nil && strings.EqualFold(*cred.KeyId, id.KeyId) {
-				credential = &cred
-				break
-			}
-		}
-	}
-
+	credential := helpers.GetPasswordCredential(app.PasswordCredentials, id.KeyId)
 	if credential == nil {
 		log.Printf("[DEBUG] Password credential %q (ID %q) was not found - removing from state!", id.KeyId, id.ObjectId)
 		d.SetId("")
@@ -279,5 +270,24 @@ func applicationPasswordResourceDelete(ctx context.Context, d *schema.ResourceDa
 		return tf.ErrorDiagF(err, "Removing password credential %q from application with object ID %q", id.KeyId, id.ObjectId)
 	}
 
+	// Wait for application password to be deleted
+	if err := helpers.WaitForDeletion(ctx, func(ctx context.Context) (*bool, error) {
+		client.BaseClient.DisableRetries = true
+
+		app, _, err := client.Get(ctx, id.ObjectId, odata.Query{})
+		if err != nil {
+			return nil, err
+		}
+
+		credential := helpers.GetPasswordCredential(app.PasswordCredentials, id.KeyId)
+		if credential == nil {
+			return utils.Bool(false), nil
+		}
+
+		return utils.Bool(true), nil
+	}); err != nil {
+		return tf.ErrorDiagF(err, "Waiting for deletion of password credential %q from application with object ID %q", id.KeyId, id.ObjectId)
+	}
+
 	return nil
 }

internal/services/applications/application_resource.go

@@ -940,11 +940,18 @@ func applicationResourceCreate(ctx context.Context, d *schema.ResourceData, meta
 		return applicationResourceUpdate(ctx, d, meta)
 	}
 
+	// Set a temporary display name as we'll attempt to patch the application with the correct name after creating it
+	uuid, err := uuid.GenerateUUID()
+	if err != nil {
+		return tf.ErrorDiagF(err, "Failed to generate a UUID")
+	}
+	tempDisplayName := fmt.Sprintf("TERRAFORM_UPDATE_%s", uuid)
+
 	// Create a new application
 	properties := msgraph.Application{
 		Api:                   expandApplicationApi(d.Get("api").([]interface{})),
 		AppRoles:              expandApplicationAppRoles(d.Get("app_role").(*schema.Set).List()),
-		DisplayName:           utils.String(displayName),
+		DisplayName:           utils.String(tempDisplayName),
 		GroupMembershipClaims: expandApplicationGroupMembershipClaims(d.Get("group_membership_claims").(*schema.Set).List()),
 		IdentifierUris:        tf.ExpandStringSlicePtr(d.Get("identifier_uris").(*schema.Set).List()),
 		Info: &msgraph.InformationalUrl{
@@ -1032,14 +1039,19 @@ func applicationResourceCreate(ctx context.Context, d *schema.ResourceData, meta
 
 	d.SetId(*app.ID)
 
-	// Wait until the application is updatable (the SDK handles retries for us)
-	_, err = client.Update(ctx, msgraph.Application{
+	// Attempt to patch the newly created group with the correct name, which will tell us whether it exists yet
+	// The SDK handles retries for us here in the event of 404, 429 or 5xx, then returns after giving up
+	status, err := client.Update(ctx, msgraph.Application{
 		DirectoryObject: msgraph.DirectoryObject{
 			ID: app.ID,
 		},
+		DisplayName: utils.String(displayName),
 	})
 	if err != nil {
-		return tf.ErrorDiagF(err, "Timed out whilst waiting for new application to be replicated in Azure AD")
+		if status == http.StatusNotFound {
+			return tf.ErrorDiagF(err, "Timed out whilst waiting for new application to be replicated in Azure AD")
+		}
+		return tf.ErrorDiagF(err, "Failed to patch application after creating")
 	}
 
 	if len(ownersExtra) > 0 {
@@ -1276,19 +1288,34 @@ func applicationResourceRead(ctx context.Context, d *schema.ResourceData, meta i
 
 func applicationResourceDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 	client := meta.(*clients.Client).Applications.ApplicationsClient
+	appId := d.Id()
 
-	_, status, err := client.Get(ctx, d.Id(), odata.Query{})
+	_, status, err := client.Get(ctx, appId, odata.Query{})
 	if err != nil {
 		if status == http.StatusNotFound {
-			return tf.ErrorDiagPathF(fmt.Errorf("Application was not found"), "id", "Retrieving Application with object ID %q", d.Id())
+			return tf.ErrorDiagPathF(fmt.Errorf("Application was not found"), "id", "Retrieving Application with object ID %q", appId)
 		}
 
-		return tf.ErrorDiagPathF(err, "id", "Retrieving application with object ID %q", d.Id())
+		return tf.ErrorDiagPathF(err, "id", "Retrieving application with object ID %q", appId)
 	}
 
-	status, err = client.Delete(ctx, d.Id())
+	status, err = client.Delete(ctx, appId)
 	if err != nil {
-		return tf.ErrorDiagPathF(err, "id", "Deleting application with object ID %q, got status %d", d.Id(), status)
+		return tf.ErrorDiagPathF(err, "id", "Deleting application with object ID %q, got status %d", appId, status)
+	}
+
+	// Wait for application object to be deleted
+	if err := helpers.WaitForDeletion(ctx, func(ctx context.Context) (*bool, error) {
+		client.BaseClient.DisableRetries = true
+		if _, status, err := client.Get(ctx, appId, odata.Query{}); err != nil {
+			if status == http.StatusNotFound {
+				return utils.Bool(false), nil
+			}
+			return nil, err
+		}
+		return utils.Bool(true), nil
+	}); err != nil {
+		return tf.ErrorDiagF(err, "Waiting for deletion of application with object ID %q", appId)
 	}
 
 	return nil