mgmt, retry on 404 in service principal create code, for eventual consistency (Azure#30791)

Author: weidongxu-microsoft

sdk/resourcemanager/azure-resourcemanager-appservice/src/test/java/com/azure/resourcemanager/appservice/WebAppsMsiTests.java

@@ -83,7 +83,7 @@ public void canCRUDWebAppWithMsi() throws Exception {
 
             ResourceManagerUtils.sleep(Duration.ofMinutes(1));
 
-            Response<String> response = curl("http://" + webappName1 + "." + "azurewebsites.net/appservicemsi/");
+            Response<String> response = curl("https://" + webappName1 + "." + "azurewebsites.net/appservicemsi/");
             Assertions.assertEquals(200, response.getStatusCode());
             String body = response.getValue();
             Assertions.assertNotNull(body);
@@ -173,9 +173,9 @@ public void canCRUDWebAppWithUserAssignedMsi() throws Exception {
                 "appservicemsi.war",
                 WebAppsMsiTests.class.getResourceAsStream("/appservicemsi.war"));
 
-            ResourceManagerUtils.sleep(Duration.ofSeconds(30));
+            ResourceManagerUtils.sleep(Duration.ofMinutes(1));
 
-            Response<String> response = curl("http://" + webappName1 + "." + "azurewebsites.net/appservicemsi/");
+            Response<String> response = curl("https://" + webappName1 + "." + "azurewebsites.net/appservicemsi/");
             Assertions.assertEquals(200, response.getStatusCode());
             String body = response.getValue();
             Assertions.assertNotNull(body);

sdk/resourcemanager/azure-resourcemanager-appservice/src/test/resources/session-records/WebAppsMsiTests.canCRUDWebAppWithMsi.json

@@ -2,13 +2,10 @@
 
 ## 2.19.0-beta.1 (Unreleased)
 
-### Features Added
-
-### Breaking Changes
-
 ### Bugs Fixed
 
-### Other Changes
+- Supported delayed retry on 404 for eventual consistency, after creating AAD service principal.
+- Improved the delayed retry on 400 for service principal, when creating role assignment. Now the retry will continue for only about a minute.
 
 ## 2.18.0 (2022-08-26)
 

sdk/resourcemanager/azure-resourcemanager-appservice/src/test/resources/session-records/WebAppsMsiTests.canCRUDWebAppWithUserAssignedMsi.json

@@ -43,6 +43,8 @@
     <javaModulesSurefireArgLine>
       --add-opens com.azure.resourcemanager.authorization/com.azure.resourcemanager.authorization=ALL-UNNAMED
       --add-opens com.azure.resourcemanager.resources/com.azure.resourcemanager.resources=ALL-UNNAMED
+
+      --add-opens com.azure.core/com.azure.core.implementation.util=ALL-UNNAMED
     </javaModulesSurefireArgLine>
   </properties>
 

sdk/resourcemanager/azure-resourcemanager-authorization/CHANGELOG.md

@@ -3,7 +3,6 @@
 
 package com.azure.resourcemanager.authorization.implementation;
 
-import com.azure.core.exception.HttpResponseException;
 import com.azure.resourcemanager.authorization.AuthorizationManager;
 import com.azure.resourcemanager.authorization.fluent.models.ApplicationsAddPasswordRequestBodyInner;
 import com.azure.resourcemanager.authorization.fluent.models.MicrosoftGraphApplicationInner;
@@ -14,15 +13,12 @@
 import com.azure.resourcemanager.authorization.models.CertificateCredential;
 import com.azure.resourcemanager.authorization.models.PasswordCredential;
 import com.azure.resourcemanager.resources.fluentcore.model.implementation.CreatableUpdatableImpl;
-import com.azure.resourcemanager.resources.fluentcore.utils.ResourceManagerUtils;
 import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
 import reactor.util.retry.Retry;
-import reactor.util.retry.RetryBackoffSpec;
 
 import java.net.MalformedURLException;
 import java.net.URL;
-import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
@@ -65,7 +61,7 @@ public boolean isInCreateMode() {
 
     @Override
     public Mono<ActiveDirectoryApplication> createResourceAsync() {
-        Retry retry = backoffRetryFor404();
+        Retry retry = RetryUtils.backoffRetryFor404ResourceNotFound();
 
         return manager
             .serviceClient()
@@ -82,15 +78,6 @@ public Mono<ActiveDirectoryApplication> updateResourceAsync() {
             .then(submitCredentialAsync(null).doOnComplete(this::postRequest).then(refreshAsync()));
     }
 
-    static RetryBackoffSpec backoffRetryFor404() {
-        return Retry
-            // 10 + 20 + 40 = 70 seconds
-            .backoff(3, ResourceManagerUtils.InternalRuntimeContext.getDelayDuration(Duration.ofSeconds(10)))
-            .filter(e -> (e instanceof HttpResponseException)
-                && (((HttpResponseException) e).getResponse().getStatusCode() == 404))
-            // do not convert to RetryExhaustedException
-            .onRetryExhaustedThrow((spec, signal) -> signal.failure());
-    }
 
     private void refreshCredentials(MicrosoftGraphApplicationInner inner) {
         cachedCertificateCredentials.clear();
@@ -118,7 +105,6 @@ private Flux<?> submitCredentialAsync(Retry retry) {
                     manager().serviceClient().getApplications()
                         .addPasswordAsync(id(), new ApplicationsAddPasswordRequestBodyInner()
                             .withPasswordCredential(passwordCredential.innerModel()));
-
                 if (retry != null) {
                     monoAddPassword = monoAddPassword.retryWhen(retry);
                 }

sdk/resourcemanager/azure-resourcemanager-authorization/pom.xml

@@ -0,0 +1,49 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+package com.azure.resourcemanager.authorization.implementation;
+
+import com.azure.core.management.exception.ManagementException;
+import com.azure.resourcemanager.resources.fluentcore.utils.ResourceManagerUtils;
+import reactor.util.retry.Retry;
+import reactor.util.retry.RetryBackoffSpec;
+
+import java.time.Duration;
+
+class RetryUtils {
+
+    // for MSGraph API
+    static RetryBackoffSpec backoffRetryFor404ResourceNotFound() {
+        return backoffRetry(404, "Request_ResourceNotFound");
+    }
+
+    // for MSGraph API, when appId of the service principal does not reference a valid application object
+    static RetryBackoffSpec backoffRetryFor400BadRequest() {
+        return backoffRetry(400, "Request_BadRequest");
+    }
+
+    // for Microsoft.Authorization API
+    static RetryBackoffSpec backoffRetryFor400PrincipalNotFound() {
+        return backoffRetry(400, "PrincipalNotFound");
+    }
+
+    private static RetryBackoffSpec backoffRetry(int statusCode, String errorCode) {
+        return Retry
+            // 10 + 20 + 40 = 70 seconds
+            .backoff(3, ResourceManagerUtils.InternalRuntimeContext.getDelayDuration(Duration.ofSeconds(10)))
+            .filter(throwable -> {
+                boolean resourceNotFoundException = false;
+                if (throwable instanceof ManagementException) {
+                    ManagementException exception = (ManagementException) throwable;
+                    if (exception.getResponse().getStatusCode() == statusCode
+                        && exception.getValue() != null
+                        && errorCode.equalsIgnoreCase(exception.getValue().getCode())) {
+                        resourceNotFoundException = true;
+                    }
+                }
+                return resourceNotFoundException;
+            })
+            // do not convert to RetryExhaustedException
+            .onRetryExhaustedThrow((spec, signal) -> signal.failure());
+    }
+}

sdk/resourcemanager/azure-resourcemanager-authorization/src/main/java/com/azure/resourcemanager/authorization/implementation/ActiveDirectoryApplicationImpl.java

@@ -3,7 +3,6 @@
 
 package com.azure.resourcemanager.authorization.implementation;
 
-import com.azure.core.management.exception.ManagementException;
 import com.azure.core.util.logging.ClientLogger;
 import com.azure.resourcemanager.authorization.AuthorizationManager;
 import com.azure.resourcemanager.authorization.models.ActiveDirectoryGroup;
@@ -17,14 +16,7 @@
 import com.azure.resourcemanager.resources.models.ResourceGroup;
 import com.azure.resourcemanager.resources.fluentcore.arm.models.Resource;
 import com.azure.resourcemanager.resources.fluentcore.model.implementation.CreatableImpl;
-import com.azure.resourcemanager.resources.fluentcore.utils.ResourceManagerUtils;
-import reactor.core.Exceptions;
-import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
-import reactor.util.retry.Retry;
-
-import java.time.Duration;
-import java.util.Locale;
 
 /** Implementation for ServicePrincipal and its parent interfaces. */
 class RoleAssignmentImpl extends CreatableImpl<RoleAssignment, RoleAssignmentInner, RoleAssignmentImpl>
@@ -93,33 +85,8 @@ public Mono<RoleAssignment> createResourceAsync() {
                         .roleServiceClient()
                         .getRoleAssignments()
                         .createAsync(scope(), name(), roleAssignmentPropertiesInner)
-                        .retryWhen(Retry.withThrowable(
-                            throwableFlux ->
-                                throwableFlux
-                                    .zipWith(
-                                        Flux.range(1, 30),
-                                        (throwable, integer) -> {
-                                            if (throwable instanceof ManagementException) {
-                                                ManagementException managementException =
-                                                    (ManagementException) throwable;
-                                                String exceptionMessage =
-                                                    managementException.getMessage().toLowerCase(Locale.ROOT);
-                                                if (exceptionMessage.contains("principalnotfound")
-                                                    || exceptionMessage.contains("does not exist in the directory")) {
-                                                    /*
-                                                     * ref:
-                                                     * https://github.com/Azure/azure-cli/blob/dev/src/command_modules/azure-cli-role/azure/cli/command_modules/role/custom.py#L1048-L1065
-                                                     */
-                                                    return integer;
-                                                } else {
-                                                    throw logger.logExceptionAsError(Exceptions.propagate(throwable));
-                                                }
-                                            } else {
-                                                throw logger.logExceptionAsError(Exceptions.propagate(throwable));
-                                            }
-                                        })
-                                    .flatMap(i -> Mono.delay(ResourceManagerUtils.InternalRuntimeContext
-                                        .getDelayDuration(Duration.ofSeconds(i)))))))
+                        // if the service principal is newly created (also apply to the case that MSI is new), wait for eventual consistency from AAD
+                        .retryWhen(RetryUtils.backoffRetryFor400PrincipalNotFound()))
             .map(innerToFluentMap(this));
     }
 

sdk/resourcemanager/azure-resourcemanager-authorization/src/main/java/com/azure/resourcemanager/authorization/implementation/RetryUtils.java

@@ -4,6 +4,7 @@
 package com.azure.resourcemanager.authorization.implementation;
 
 import com.azure.resourcemanager.authorization.AuthorizationManager;
+import com.azure.resourcemanager.authorization.fluent.models.MicrosoftGraphPasswordCredentialInner;
 import com.azure.resourcemanager.authorization.fluent.models.MicrosoftGraphServicePrincipalInner;
 import com.azure.resourcemanager.authorization.fluent.models.ServicePrincipalsAddPasswordRequestBodyInner;
 import com.azure.resourcemanager.authorization.models.ActiveDirectoryApplication;
@@ -17,6 +18,7 @@
 import com.azure.resourcemanager.resources.models.ResourceGroup;
 import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
+import reactor.util.retry.Retry;
 
 import java.util.ArrayList;
 import java.util.Collections;
@@ -93,6 +95,8 @@ protected Mono<MicrosoftGraphServicePrincipalInner> getInnerAsync() {
 
     @Override
     public Mono<ServicePrincipal> createResourceAsync() {
+        Retry retry = isInCreateMode() ? RetryUtils.backoffRetryFor404ResourceNotFound() : null;
+
         Mono<ServicePrincipal> sp;
         if (isInCreateMode()) {
             innerModel().withAccountEnabled(true);
@@ -102,6 +106,11 @@ public Mono<ServicePrincipal> createResourceAsync() {
             }
             sp = manager.serviceClient().getServicePrincipalsServicePrincipals()
                 .createServicePrincipalAsync(innerModel()).map(innerToFluentMap(this));
+
+            if (applicationCreatable != null) {
+                // retry on 400, if app is created with "withNewApplication"
+                sp = sp.retryWhen(RetryUtils.backoffRetryFor400BadRequest());
+            }
         } else {
             sp = manager().serviceClient().getServicePrincipalsServicePrincipals()
                 .updateServicePrincipalAsync(id(), new MicrosoftGraphServicePrincipalInner()
@@ -112,7 +121,10 @@ public Mono<ServicePrincipal> createResourceAsync() {
         return sp
             .flatMap(
                 servicePrincipal ->
-                    submitCredentialsAsync(servicePrincipal).mergeWith(submitRolesAsync(servicePrincipal)).last())
+                    submitCredentialsAsync(servicePrincipal, retry)
+                        // retry for Microsoft.Authorization is done in RoleAssignmentImpl
+                        .mergeWith(submitRolesAsync(servicePrincipal))
+                        .last())
             .map(
                 servicePrincipal -> {
                     for (PasswordCredentialImpl<?> passwordCredential : passwordCredentialsToCreate) {
@@ -128,7 +140,7 @@ public Mono<ServicePrincipal> createResourceAsync() {
                 });
     }
 
-    private Mono<ServicePrincipal> submitCredentialsAsync(final ServicePrincipal sp) {
+    private Mono<ServicePrincipal> submitCredentialsAsync(final ServicePrincipal servicePrincipal, Retry retry) {
         return Flux.defer(() ->
 //                    Flux.fromIterable(certificateCredentialsToCreate)
 //                        .flatMap(certificateCredential ->
@@ -142,20 +154,31 @@ private Mono<ServicePrincipal> submitCredentialsAsync(final ServicePrincipal sp)
 //                                new ServicePrincipalsRemoveKeyRequestBody()
 //                                    .withKeyId(UUID.fromString(id)))),
                     Flux.fromIterable(passwordCredentialsToCreate)
-                        .flatMap(passwordCredential ->
-                            manager().serviceClient().getServicePrincipals()
-                                .addPasswordAsync(id(),
-                                    new ServicePrincipalsAddPasswordRequestBodyInner()
-                                        .withPasswordCredential(passwordCredential.innerModel()))
-                                .doOnNext(passwordCredential::setInner)
-                        )
+                        .flatMap(passwordCredential -> {
+                            Mono<MicrosoftGraphPasswordCredentialInner> monoAddPassword =
+                                manager().serviceClient().getServicePrincipals()
+                                    .addPasswordAsync(id(),
+                                        new ServicePrincipalsAddPasswordRequestBodyInner()
+                                            .withPasswordCredential(passwordCredential.innerModel()));
+                            if (retry != null) {
+                                monoAddPassword = monoAddPassword.retryWhen(retry);
+                            }
+                            monoAddPassword = monoAddPassword.doOnNext(passwordCredential::setInner);
+                            return monoAddPassword;
+                        })
 //                    Flux.fromIterable(passwordCredentialsToDelete)
 //                        .flatMap(id -> manager().serviceClient().getServicePrincipals()
 //                            .removePasswordAsync(id(),
 //                                new ServicePrincipalsRemovePasswordRequestBody()
 //                                    .withKeyId(UUID.fromString(id))))
             )
-            .then(refreshAsync());
+            .then(Mono.defer(() -> {
+                Mono<ServicePrincipal> monoRefresh = refreshAsync();
+                if (retry != null) {
+                    monoRefresh = monoRefresh.retryWhen(retry);
+                }
+                return monoRefresh;
+            }));
     }
 
     private Mono<ServicePrincipal> submitRolesAsync(final ServicePrincipal servicePrincipal) {
@@ -166,15 +189,13 @@ private Mono<ServicePrincipal> submitRolesAsync(final ServicePrincipal servicePr
             create =
                 Flux
                     .fromIterable(rolesToCreate.entrySet())
-                    .flatMap(
-                        roleEntry ->
-                            manager()
-                                .roleAssignments()
-                                .define(this.manager().internalContext().randomUuid())
-                                .forServicePrincipal(servicePrincipal)
-                                .withBuiltInRole(roleEntry.getValue())
-                                .withScope(roleEntry.getKey())
-                                .createAsync())
+                    .flatMap(roleEntry -> manager()
+                        .roleAssignments()
+                        .define(this.manager().internalContext().randomUuid())
+                        .forServicePrincipal(servicePrincipal)
+                        .withBuiltInRole(roleEntry.getValue())
+                        .withScope(roleEntry.getKey())
+                        .createAsync())
                     .doOnNext(
                         indexable ->
                             cachedRoleAssignments.put(indexable.id(), indexable))