Merge pull request #1406 from hashicorp/bugfix/application-instantiat…

…e-404-api-bug-workaround

`azuread_application`: work around very buggy API when instantiating from template

go.mod

@@ -63,3 +63,5 @@ require (
 )
 
 go 1.21.3
+
+replace github.com/manicminer/hamilton => github.com/MarkDordoy/hamilton v0.17.1-0.20240611151114-899c6ce169f6

go.sum

@@ -1,5 +1,7 @@
 dario.cat/mergo v1.0.0 h1:AGCNq9Evsj31mOgNPcLyXc+4PNABt905YmuqPYYpBWk=
 dario.cat/mergo v1.0.0/go.mod h1:uNxQE+84aUszobStD9th8a29P2fMDhsBdgRYvZOxGmk=
+github.com/MarkDordoy/hamilton v0.17.1-0.20240611151114-899c6ce169f6 h1:yRxIRrSebI7v7BspqjteIZKrnNQDWhRA68NVeU8cTmI=
+github.com/MarkDordoy/hamilton v0.17.1-0.20240611151114-899c6ce169f6/go.mod h1:u80g9rPtJpCG7EC0iayttt8UfeAp6jknClixgZGE950=
 github.com/Microsoft/go-winio v0.6.1 h1:9/kr64B9VUZrLm5YYwbGtUJnMgqWVOdUAXu6Migciow=
 github.com/Microsoft/go-winio v0.6.1/go.mod h1:LRdKpFKfdobln8UmuiYcKPot9D2v6svN5+sAH+4kjUM=
 github.com/ProtonMail/go-crypto v0.0.0-20230828082145-3c4c8a2d2371 h1:kkhsdkhsCvIsutKu5zLMgWtgh9YxGCNAw8Ad8hjwfYg=
@@ -111,8 +113,6 @@ github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE=
 github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc=
 github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw=
-github.com/manicminer/hamilton v0.70.0 h1:XMgVcwVtUGq2aBXqVAynaUDZdPXCXC8/rd7D5Zsg3UM=
-github.com/manicminer/hamilton v0.70.0/go.mod h1:u80g9rPtJpCG7EC0iayttt8UfeAp6jknClixgZGE950=
 github.com/mattn/go-colorable v0.1.9/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc=
 github.com/mattn/go-colorable v0.1.12/go.mod h1:u5H1YNBxpqRaxsYJYSkiCWKzEfiAb1Gb520KVy5xxl4=
 github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA=

internal/services/applications/application_redirect_uris_resource.go

@@ -199,7 +199,7 @@ func (r ApplicationRedirectUrisResource) Update() sdk.ResourceFunc {
 			applicationId := parse.NewApplicationID(id.ApplicationId)
 
 			var model ApplicationRedirectUrisModel
-			if err := metadata.Decode(&model); err != nil {
+			if err = metadata.Decode(&model); err != nil {
 				return fmt.Errorf("decoding: %+v", err)
 			}
 

internal/services/applications/application_registration_resource.go

@@ -353,6 +353,9 @@ func (r ApplicationRegistrationResource) Update() sdk.ResourceFunc {
 				return fmt.Errorf("decoding: %+v", err)
 			}
 
+			tf.LockByName(applicationResourceName, id.ApplicationId)
+			defer tf.UnlockByName(applicationResourceName, id.ApplicationId)
+
 			properties := msgraph.Application{
 				DirectoryObject: msgraph.DirectoryObject{
 					Id: &id.ApplicationId,

internal/services/applications/application_resource.go

@@ -954,15 +954,97 @@ func applicationResourceCreate(ctx context.Context, d *pluginsdk.ResourceData, m
 	}
 
 	if templateId != "" {
+		// Validate the template exists
+		if _, status, err := appTemplatesClient.Get(ctx, templateId, odata.Query{}); err != nil {
+			if status == http.StatusNotFound {
+				return tf.ErrorDiagPathF(err, "template_id", "Could not find application template with ID %q", templateId)
+			}
+			return tf.ErrorDiagF(err, "Could not retrieve application template with ID %q", templateId)
+		}
+
+		// Generate a temporary display name to assert uniqueness when handling buggy 404 when instantiating
+		uuid, err := uuid.GenerateUUID()
+		if err != nil {
+			return tf.ErrorDiagF(err, "Failed to generate a UUID")
+		}
+		tempDisplayName := fmt.Sprintf("TERRAFORM_INSTANTIATE_%s", uuid)
+
 		// Instantiate application from template gallery and return via the update function
 		properties := msgraph.ApplicationTemplate{
 			ID:          pointer.To(templateId),
-			DisplayName: pointer.To(displayName),
+			DisplayName: pointer.To(tempDisplayName),
 		}
 
-		result, _, err := appTemplatesClient.Instantiate(ctx, properties)
+		// When the /instantiate operation returns 404, it has probably created the application anyway. There is no way to tell this
+		// other than polling for the application object which is created out-of-band, so we create it with a quasi-unique temporary
+		// displayName and then poll for it.
+		result, status, err := appTemplatesClient.Instantiate(ctx, properties)
 		if err != nil {
-			return tf.ErrorDiagF(err, "Could not instantiate application from template")
+			if status != http.StatusNotFound {
+				return tf.ErrorDiagF(err, "Could not instantiate application from template")
+			}
+
+			deadline, ok := ctx.Deadline()
+			if !ok {
+				return tf.ErrorDiagF(errors.New("context has no deadline"), "internal-error: context has no deadline")
+			}
+
+			// Since the API response can't be trusted, we'll have to take on responsibility for ensuring
+			// the application object and service principal objects were created as expected.
+			pollingResult, err := (&pluginsdk.StateChangeConf{ //nolint:staticcheck
+				Pending:    []string{"Waiting"},
+				Target:     []string{"Found"},
+				Timeout:    time.Until(deadline),
+				MinTimeout: 5 * time.Second,
+				Refresh: func() (interface{}, string, error) {
+					// List applications with matching applicationTemplateId and displayName (using the temporary display name we generated above)
+					filter := fmt.Sprintf("applicationTemplateId eq '%s' and displayName eq '%s'", odata.EscapeSingleQuote(templateId), odata.EscapeSingleQuote(tempDisplayName))
+					applicationsResult, _, err := client.List(ctx, odata.Query{Filter: filter})
+					if err != nil {
+						return nil, "Error", err
+					}
+					if applicationsResult == nil {
+						return nil, "Waiting", nil
+					}
+					for _, application := range *applicationsResult {
+						if id := application.ID(); id != nil && application.AppId != nil && application.ApplicationTemplateId != nil && *application.ApplicationTemplateId == templateId && application.DisplayName != nil && *application.DisplayName == tempDisplayName {
+							// We should ensure the service principal was also created
+							servicePrincipalsClient := meta.(*clients.Client).Applications.ServicePrincipalsClient
+
+							// List service principals for the created application
+							servicePrincipalsFilter := fmt.Sprintf("appId eq '%s'", odata.EscapeSingleQuote(*application.AppId))
+							servicePrincipalsResult, _, err := servicePrincipalsClient.List(ctx, odata.Query{Filter: servicePrincipalsFilter})
+							if err != nil {
+								return nil, "Error", err
+							}
+							if servicePrincipalsResult == nil {
+								return nil, "Waiting", nil
+							}
+							for _, servicePrincipal := range *servicePrincipalsResult {
+								// Validate the appId and applicationTemplateId match the application
+								if servicePrincipalId := servicePrincipal.ID(); servicePrincipalId != nil && servicePrincipal.AppId != nil && *servicePrincipal.AppId == *application.AppId && servicePrincipal.ApplicationTemplateId != nil && *servicePrincipal.ApplicationTemplateId == templateId {
+									return msgraph.ApplicationTemplate{
+										Application:      &application,
+										ServicePrincipal: &servicePrincipal,
+									}, "Found", nil
+								}
+							}
+						}
+					}
+					return nil, "Waiting", nil
+				},
+			}).WaitForStateContext(ctx)
+
+			if err != nil {
+				return tf.ErrorDiagF(err, "Could not instantiate application from template")
+			}
+			if pollingResult == nil {
+				return tf.ErrorDiagF(errors.New("attempted to poll for application and service principal but they were not found"), "Could not instantiate application from template")
+			}
+
+			if template, ok := pollingResult.(msgraph.ApplicationTemplate); ok {
+				result = &template
+			}
 		}
 
 		if result.Application == nil {
@@ -976,7 +1058,8 @@ func applicationResourceCreate(ctx context.Context, d *pluginsdk.ResourceData, m
 		id := parse.NewApplicationID(*result.Application.ID())
 		d.SetId(id.ID())
 
-		// The application was created out of band, so we'll update it just as if it was imported
+		// The application was created out of band, so we'll update it just as if it was imported. This will also
+		// set the correct displayName for the application.
 		return applicationResourceUpdate(ctx, d, meta)
 	}
 
@@ -1106,7 +1189,7 @@ func applicationResourceCreate(ctx context.Context, d *pluginsdk.ResourceData, m
 	// See https://github.com/hashicorp/terraform-provider-azuread/issues/914
 	if acceptMappedClaims != nil {
 		api.AcceptMappedClaims = acceptMappedClaims
-		if _, err := client.Update(ctx, msgraph.Application{
+		if _, err = client.Update(ctx, msgraph.Application{
 			DirectoryObject: msgraph.DirectoryObject{
 				Id: app.Id,
 			},
@@ -1119,7 +1202,7 @@ func applicationResourceCreate(ctx context.Context, d *pluginsdk.ResourceData, m
 	if len(ownersExtra) > 0 {
 		// Add any remaining owners after the application is created
 		app.Owners = &ownersExtra
-		if _, err := client.AddOwners(ctx, app); err != nil {
+		if _, err = client.AddOwners(ctx, app); err != nil {
 			return tf.ErrorDiagF(err, "Could not add owners to application with object ID: %q", id.ApplicationId)
 		}
 	}
@@ -1133,7 +1216,7 @@ func applicationResourceCreate(ctx context.Context, d *pluginsdk.ResourceData, m
 
 	// Upload the application image
 	if imageContentType != "" && len(imageData) > 0 {
-		_, err := client.UploadLogo(ctx, id.ApplicationId, imageContentType, imageData)
+		_, err = client.UploadLogo(ctx, id.ApplicationId, imageContentType, imageData)
 		if err != nil {
 			return tf.ErrorDiagF(err, "Could not upload logo image for application with object ID: %q", id.ApplicationId)
 		}
@@ -1151,6 +1234,9 @@ func applicationResourceUpdate(ctx context.Context, d *pluginsdk.ResourceData, m
 		return tf.ErrorDiagPathF(err, "id", "Parsing ID")
 	}
 
+	tf.LockByName(applicationResourceName, id.ApplicationId)
+	defer tf.UnlockByName(applicationResourceName, id.ApplicationId)
+
 	displayName := d.Get("display_name").(string)
 
 	// Perform this check at apply time to catch any duplicate names created during the same apply

internal/services/applications/client/client.go

@@ -13,6 +13,7 @@ type Client struct {
 	ApplicationsClientBeta     *msgraph.ApplicationsClient
 	ApplicationTemplatesClient *msgraph.ApplicationTemplatesClient
 	DirectoryObjectsClient     *msgraph.DirectoryObjectsClient
+	ServicePrincipalsClient    *msgraph.ServicePrincipalsClient
 }
 
 func NewClient(o *common.ClientOptions) *Client {
@@ -31,10 +32,14 @@ func NewClient(o *common.ClientOptions) *Client {
 	directoryObjectsClient := msgraph.NewDirectoryObjectsClient()
 	o.ConfigureClient(&directoryObjectsClient.BaseClient)
 
+	servicePrincipalsClient := msgraph.NewServicePrincipalsClient()
+	o.ConfigureClient(&servicePrincipalsClient.BaseClient)
+
 	return &Client{
 		ApplicationsClient:         applicationsClient,
 		ApplicationsClientBeta:     applicationsClientBeta,
 		ApplicationTemplatesClient: applicationTemplatesClient,
 		DirectoryObjectsClient:     directoryObjectsClient,
+		ServicePrincipalsClient:    servicePrincipalsClient,
 	}
 }

vendor/modules.txt

@@ -209,7 +209,7 @@ github.com/hashicorp/terraform-svchost
 # github.com/hashicorp/yamux v0.1.1
 ## explicit; go 1.15
 github.com/hashicorp/yamux
-# github.com/manicminer/hamilton v0.70.0
+# github.com/manicminer/hamilton v0.70.0 => github.com/MarkDordoy/hamilton v0.17.1-0.20240611151114-899c6ce169f6
 ## explicit; go 1.21
 github.com/manicminer/hamilton/errors
 github.com/manicminer/hamilton/internal/utils
@@ -428,3 +428,4 @@ google.golang.org/protobuf/types/known/timestamppb
 ## explicit; go 1.19
 software.sslmate.com/src/go-pkcs12
 software.sslmate.com/src/go-pkcs12/internal/rc2
+# github.com/manicminer/hamilton => github.com/MarkDordoy/hamilton v0.17.1-0.20240611151114-899c6ce169f6