WOR-1777 Handle remaining instances of missing azure resources (#1776)

* add LZS exceptions to common error handling when deleting azure resources * allow management exceptions from VmHelper to be handled by DeleteAzureControlledResourceStep
DataBiosphere · Jul 18, 2024 · e292640 · e292640
1 parent 52d69f1
commit e292640
Show file tree

Hide file tree

Showing 4 changed files with 120 additions and 3 deletions.
diff --git a/.../workspace/service/resource/controlled/cloud/azure/DeleteAzureControlledResourceStep.java b/.../workspace/service/resource/controlled/cloud/azure/DeleteAzureControlledResourceStep.java
@@ -1,8 +1,10 @@
 package bio.terra.workspace.service.resource.controlled.cloud.azure;
 
+import bio.terra.lz.futureservice.client.ApiException;
 import bio.terra.stairway.FlightContext;
 import bio.terra.stairway.StepResult;
 import bio.terra.stairway.StepStatus;
+import bio.terra.workspace.amalgam.landingzone.azure.LandingZoneServiceApiException;
 import bio.terra.workspace.common.exception.AzureManagementExceptionUtils;
 import bio.terra.workspace.service.resource.controlled.flight.delete.DeleteControlledResourceStep;
 import com.azure.core.management.exception.ManagementException;
@@ -70,6 +72,18 @@ protected StepResult handleResourceDeleteException(Exception e, FlightContext co
       // retry any other non-4xx errors
       return new StepResult(AzureManagementExceptionUtils.maybeRetryStatus(ex), ex);
     }
+
+    // When retrieving shared resources from the landing zone,
+    // LZS can encounter the same management exceptions as above
+    // To detect this, we need to examine the message for those error codes
+    if (e instanceof LandingZoneServiceApiException && e.getCause() instanceof ApiException) {
+      var msg = e.getCause().getMessage();
+      if (msg != null
+          && missingResourceManagementCodes.stream().anyMatch(code -> msg.contains(code))) {
+        return StepResult.getStepResultSuccess();
+      }
+    }
+
     return new StepResult(StepStatus.STEP_RESULT_FAILURE_RETRY, e);
   }
 }
diff --git a/...kspace/service/resource/controlled/cloud/azure/vm/RemoveManagedIdentitiesAzureVmStep.java b/...kspace/service/resource/controlled/cloud/azure/vm/RemoveManagedIdentitiesAzureVmStep.java
@@ -7,6 +7,7 @@
 import bio.terra.workspace.service.resource.controlled.cloud.azure.DeleteAzureControlledResourceStep;
 import bio.terra.workspace.service.workspace.flight.WorkspaceFlightMapKeys.ControlledResourceKeys;
 import bio.terra.workspace.service.workspace.model.AzureCloudContext;
+import com.azure.core.management.exception.ManagementException;
 import com.azure.resourcemanager.compute.ComputeManager;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -33,8 +34,17 @@ public StepResult deleteResource(FlightContext context) {
             .get(ControlledResourceKeys.AZURE_CLOUD_CONTEXT, AzureCloudContext.class);
 
     ComputeManager computeManager = crlService.getComputeManager(azureCloudContext, azureConfig);
-    return AzureVmHelper.removeAllUserAssignedManagedIdentitiesFromVm(
-        azureCloudContext, computeManager, resource.getVmName());
+    var helperResult =
+        AzureVmHelper.removeAllUserAssignedManagedIdentitiesFromVm(
+            azureCloudContext, computeManager, resource.getVmName());
+    if (helperResult.isSuccess()) {
+      return helperResult;
+    } else if (helperResult.getException().isPresent()
+        && helperResult.getException().get() instanceof ManagementException) {
+      throw (ManagementException) helperResult.getException().get();
+    } else {
+      return helperResult;
+    }
   }
 
   @Override

diff --git a/...ace/service/resource/controlled/cloud/azure/database/DeleteAzureDatabaseStepUnitTest.java b/...ace/service/resource/controlled/cloud/azure/database/DeleteAzureDatabaseStepUnitTest.java
@@ -5,11 +5,13 @@
 import static org.mockito.Mockito.*;
 
 import bio.terra.common.iam.BearerToken;
+import bio.terra.lz.futureservice.client.ApiException;
 import bio.terra.stairway.FlightContext;
 import bio.terra.stairway.FlightMap;
 import bio.terra.stairway.StepResult;
 import bio.terra.stairway.StepStatus;
 import bio.terra.workspace.amalgam.landingzone.azure.LandingZoneApiDispatch;
+import bio.terra.workspace.amalgam.landingzone.azure.LandingZoneServiceApiException;
 import bio.terra.workspace.app.configuration.external.AzureConfiguration;
 import bio.terra.workspace.common.exception.AzureManagementExceptionUtils;
 import bio.terra.workspace.generated.model.ApiAzureLandingZoneDeployedResource;
@@ -31,6 +33,7 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.Mock;
+import org.mockito.Mockito;
 import org.mockito.junit.jupiter.MockitoExtension;
 
 @Tag("unit")
@@ -53,7 +56,7 @@ public class DeleteAzureDatabaseStepUnitTest {
   @BeforeEach
   void setup() {
     when(context.getWorkingMap()).thenReturn(workingMap);
-    when(postgresManager.databases()).thenReturn(databases);
+    Mockito.lenient().when(postgresManager.databases()).thenReturn(databases);
     when(workingMap.get(
             WorkspaceFlightMapKeys.ControlledResourceKeys.AZURE_CLOUD_CONTEXT,
             AzureCloudContext.class))
@@ -142,6 +145,37 @@ void noResourceFoundReturnsSuccess() throws Exception {
     assertThat(step.doStep(context), equalTo(StepResult.getStepResultSuccess()));
   }
 
+  @Test
+  void lzsManagementExceptionReturnsSuccess() throws Exception {
+    var landingZoneId = UUID.randomUUID();
+    var workspaceId = UUID.randomUUID();
+    var workspace = mock(Workspace.class);
+    when(workspaceService.getWorkspace(workspaceId)).thenReturn(workspace);
+    var token = new BearerToken("test-token");
+    when(samService.getWsmServiceAccountToken()).thenReturn(token.getToken());
+    when(landingZoneApiDispatch.getLandingZoneId(token, workspace)).thenReturn(landingZoneId);
+    // Message and code of actual exception returned from LZS
+    var apiException =
+        new ApiException(
+            500,
+            "Status code 403, \"{\"error\":{\"code\":\"AuthorizationFailed\",\"message\":\"The client 'a305fdb2-74c8-4637-bb57-9499afee87ad' with object id 'a305fdb2-74c8-4637-bb57-9499afee87ad' does not have authorization to perform action 'Microsoft.Resources/subscriptions/resourcegroups/read' over scope '/subscriptions/df547342-9cfd-44ef-a6dd-df0ede32f1e3/resourcegroups/terraDemo1' or the scope is invalid. If access was recently granted, please refresh your credentials.\"}}\"");
+    doThrow(new LandingZoneServiceApiException(apiException))
+        .when(landingZoneApiDispatch)
+        .getSharedDatabase(token, landingZoneId);
+    var step =
+        new DeleteAzureDatabaseStep(
+            azureConfig,
+            crlService,
+            resource,
+            landingZoneApiDispatch,
+            samService,
+            workspaceService,
+            workspaceId);
+
+    assertThat(step.doStep(context), equalTo(StepResult.getStepResultSuccess()));
+    verify(landingZoneApiDispatch).getSharedDatabase(token, landingZoneId);
+  }
+
   @Test
   void unknownExceptionRetries() throws Exception {
     var resourceGroupId = "test-resource-group-id";

diff --git a/...ervice/resource/controlled/cloud/azure/vm/RemoveManagedIdentitiesAzureVmStepUnitTest.java b/...ervice/resource/controlled/cloud/azure/vm/RemoveManagedIdentitiesAzureVmStepUnitTest.java
@@ -0,0 +1,59 @@
+package bio.terra.workspace.service.resource.controlled.cloud.azure.vm;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.mockito.Mockito.*;
+
+import bio.terra.stairway.FlightContext;
+import bio.terra.stairway.FlightMap;
+import bio.terra.stairway.StepResult;
+import bio.terra.workspace.service.crl.CrlService;
+import bio.terra.workspace.service.workspace.flight.WorkspaceFlightMapKeys;
+import bio.terra.workspace.service.workspace.model.AzureCloudContext;
+import com.azure.core.http.HttpResponse;
+import com.azure.core.management.exception.ManagementError;
+import com.azure.core.management.exception.ManagementException;
+import com.azure.resourcemanager.compute.ComputeManager;
+import com.azure.resourcemanager.compute.models.VirtualMachines;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+@Tag("unit")
+@ExtendWith(MockitoExtension.class)
+public class RemoveManagedIdentitiesAzureVmStepUnitTest {
+
+  @Test
+  public void removeManagedIdentitiesAzureFromVm_returnsSuccessOnMissingMRG()
+      throws InterruptedException {
+    var flightContext = mock(FlightContext.class);
+    var workingMap = mock(FlightMap.class);
+    when(flightContext.getWorkingMap()).thenReturn(workingMap);
+    var cloudContext = mock(AzureCloudContext.class);
+    when(cloudContext.getAzureResourceGroupId()).thenReturn("resource-group-id");
+    when(workingMap.get(
+            WorkspaceFlightMapKeys.ControlledResourceKeys.AZURE_CLOUD_CONTEXT,
+            AzureCloudContext.class))
+        .thenReturn(cloudContext);
+
+    var error = new ManagementError("SubscriptionNotFound", "some message about the subscription");
+    var virtualMachines = mock(VirtualMachines.class);
+    doThrow(new ManagementException("some message", mock(HttpResponse.class), error))
+        .when(virtualMachines)
+        .getByResourceGroup(any(), any());
+    var computeManager = mock(ComputeManager.class);
+    when(computeManager.virtualMachines()).thenReturn(virtualMachines);
+
+    var crlService = mock(CrlService.class);
+    when(crlService.getComputeManager(any(), any())).thenReturn(computeManager);
+
+    var resource = mock(ControlledAzureVmResource.class);
+    when(resource.getVmName()).thenReturn("vm-name");
+    var step = new RemoveManagedIdentitiesAzureVmStep(mock(), crlService, resource);
+    step.doStep(flightContext);
+    StepResult stepResult = step.doStep(flightContext);
+
+    assertThat(stepResult, equalTo(StepResult.getStepResultSuccess()));
+  }
+}