diff --git a/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/DeleteAzureControlledResourceStep.java b/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/DeleteAzureControlledResourceStep.java index aac21b98ef..bf2e047295 100644 --- a/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/DeleteAzureControlledResourceStep.java +++ b/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/DeleteAzureControlledResourceStep.java @@ -1,5 +1,6 @@ package bio.terra.workspace.service.resource.controlled.cloud.azure; +import bio.terra.landingzone.db.exception.LandingZoneNotFoundException; import bio.terra.lz.futureservice.client.ApiException; import bio.terra.stairway.FlightContext; import bio.terra.stairway.StepResult; @@ -7,16 +8,27 @@ 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 bio.terra.workspace.service.resource.controlled.model.ControlledResource; import com.azure.core.management.exception.ManagementException; import java.util.Arrays; import java.util.List; +import java.util.Optional; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.http.HttpStatus; /** * Common base class for deleting Azure resources. Wraps the Azure resource deletion logic with * handling for common exceptions. */ -public abstract class DeleteAzureControlledResourceStep implements DeleteControlledResourceStep { +public abstract class DeleteAzureControlledResourceStep + implements DeleteControlledResourceStep { + private final Logger logger = LoggerFactory.getLogger(getClass()); + protected final R resource; + + protected DeleteAzureControlledResourceStep(R resource) { + this.resource = resource; + } @Override public StepResult doStep(FlightContext context) throws InterruptedException { @@ -76,14 +88,32 @@ protected StepResult handleResourceDeleteException(Exception e, FlightContext co // 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))) { + if (e instanceof LandingZoneServiceApiException && e.getCause() instanceof ApiException apiEx) { + var code = + Optional.ofNullable(apiEx.getMessage()) + .flatMap( + msg -> + missingResourceManagementCodes.stream() + .filter(c -> msg.contains(c)) + .findFirst()); + if (code.isPresent()) { + logger.debug( + "Unable to delete resource {} from workspace {}, because LZS encountered management exception {}: Returning success for delete step", + resource.getResourceType().name(), + resource.getWorkspaceId(), + code.get()); return StepResult.getStepResultSuccess(); } } + if (e instanceof LandingZoneNotFoundException) { + // If the landing zone is not present, it's probably because it was removed directly + logger.debug( + "Unable to delete resource {} from workspace {}, because no landing zone was found in LZS: Returning success for delete step", + resource.getResourceType().name(), + resource.getWorkspaceId()); + return StepResult.getStepResultSuccess(); + } return new StepResult(StepStatus.STEP_RESULT_FAILURE_RETRY, e); } } diff --git a/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/batchpool/DeleteAzureBatchPoolStep.java b/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/batchpool/DeleteAzureBatchPoolStep.java index 9788202279..a47954d29a 100644 --- a/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/batchpool/DeleteAzureBatchPoolStep.java +++ b/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/batchpool/DeleteAzureBatchPoolStep.java @@ -19,23 +19,23 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class DeleteAzureBatchPoolStep extends DeleteAzureControlledResourceStep { +public class DeleteAzureBatchPoolStep + extends DeleteAzureControlledResourceStep { private static final Logger logger = LoggerFactory.getLogger(DeleteAzureBatchPoolStep.class); private final AzureConfiguration azureConfig; private final CrlService crlService; private final LandingZoneBatchAccountFinder landingZoneBatchAccountFinder; - private final ControlledAzureBatchPoolResource resource; public DeleteAzureBatchPoolStep( AzureConfiguration azureConfig, CrlService crlService, LandingZoneBatchAccountFinder landingZoneBatchAccountFinder, ControlledAzureBatchPoolResource resource) { + super(resource); this.azureConfig = azureConfig; this.crlService = crlService; this.landingZoneBatchAccountFinder = landingZoneBatchAccountFinder; - this.resource = resource; } @Override diff --git a/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/database/DeleteAzureDatabaseStep.java b/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/database/DeleteAzureDatabaseStep.java index 73d68ee55a..3acc57d062 100644 --- a/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/database/DeleteAzureDatabaseStep.java +++ b/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/database/DeleteAzureDatabaseStep.java @@ -21,11 +21,11 @@ * A step for deleting a controlled Azure Database resource. This step uses the following process to * actually delete the Azure Database. */ -public class DeleteAzureDatabaseStep extends DeleteAzureControlledResourceStep { +public class DeleteAzureDatabaseStep + extends DeleteAzureControlledResourceStep { private static final Logger logger = LoggerFactory.getLogger(DeleteAzureDatabaseStep.class); private final AzureConfiguration azureConfig; private final CrlService crlService; - private final ControlledAzureDatabaseResource resource; private final LandingZoneApiDispatch landingZoneApiDispatch; private final SamService samService; private final WorkspaceService workspaceService; @@ -39,9 +39,9 @@ public DeleteAzureDatabaseStep( SamService samService, WorkspaceService workspaceService, UUID workspaceId) { + super(resource); this.crlService = crlService; this.azureConfig = azureConfig; - this.resource = resource; this.landingZoneApiDispatch = landingZoneApiDispatch; this.samService = samService; this.workspaceService = workspaceService; diff --git a/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/disk/DeleteAzureDiskStep.java b/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/disk/DeleteAzureDiskStep.java index 1d433a4f94..7f82565789 100644 --- a/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/disk/DeleteAzureDiskStep.java +++ b/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/disk/DeleteAzureDiskStep.java @@ -27,6 +27,7 @@ public class DeleteAzureDiskStep extends DeleteAzureControlledResourceStep { public DeleteAzureDiskStep( AzureConfiguration azureConfig, CrlService crlService, ControlledAzureDiskResource resource) { + super(resource); this.crlService = crlService; this.azureConfig = azureConfig; this.resource = resource; diff --git a/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/disk/DetachAzureDiskFromVmStep.java b/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/disk/DetachAzureDiskFromVmStep.java index 1a37d4b925..fb47ec7440 100644 --- a/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/disk/DetachAzureDiskFromVmStep.java +++ b/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/disk/DetachAzureDiskFromVmStep.java @@ -7,7 +7,7 @@ import bio.terra.workspace.app.configuration.external.AzureConfiguration; import bio.terra.workspace.common.exception.AzureManagementExceptionUtils; import bio.terra.workspace.service.crl.CrlService; -import bio.terra.workspace.service.resource.controlled.flight.delete.DeleteControlledResourceStep; +import bio.terra.workspace.service.resource.controlled.cloud.azure.DeleteAzureControlledResourceStep; import bio.terra.workspace.service.workspace.model.AzureCloudContext; import com.azure.core.management.exception.ManagementException; import com.azure.resourcemanager.compute.ComputeManager; @@ -24,85 +24,91 @@ * functionality of detaching data disk from a virtual machine before disk deletion. It is not * possible to delete a disk without detaching it. */ -public class DetachAzureDiskFromVmStep implements DeleteControlledResourceStep { +public class DetachAzureDiskFromVmStep + extends DeleteAzureControlledResourceStep { private static final Logger logger = LoggerFactory.getLogger(DetachAzureDiskFromVmStep.class); private final CrlService crlService; private final AzureConfiguration azureConfig; - private final ControlledAzureDiskResource resource; public DetachAzureDiskFromVmStep( AzureConfiguration azureConfig, CrlService crlService, ControlledAzureDiskResource resource) { + super(resource); this.crlService = crlService; this.azureConfig = azureConfig; - this.resource = resource; } @Override - public StepResult doStep(FlightContext context) throws InterruptedException, RetryException { + public StepResult deleteResource(FlightContext context) + throws InterruptedException, RetryException { final AzureCloudContext azureCloudContext = DeleteAzureDiskFlightUtils.getAzureCloudContext(context); final String diskResourceId = DeleteAzureDiskFlightUtils.getAzureDiskResourceId(azureCloudContext, resource); ComputeManager computeManager = crlService.getComputeManager(azureCloudContext, azureConfig); - try { - var disk = computeManager.disks().getById(diskResourceId); - if (!disk.isAttachedToVirtualMachine()) { - logger.info( - "Disk {} in MRG {} for workspace {} is not attached to vm.", - resource.getResourceId(), - azureCloudContext.getAzureResourceGroupId(), - resource.getWorkspaceId()); - return StepResult.getStepResultSuccess(); - } - var vm = getVirtualMachine(computeManager, disk.virtualMachineId()); - if (vm.isEmpty()) { - // this is unlikely event - disk is attached, but the vm cannot be found; let's retry - logger.warn( - "Disk {} in MRG {} for workspace {} is attached to vm, but vm cannot be found. Retrying.", - resource.getResourceId(), - azureCloudContext.getAzureResourceGroupId(), - resource.getWorkspaceId()); - return new StepResult(StepStatus.STEP_RESULT_FAILURE_RETRY); - } - var attachedDisk = tryToFindAttachedDisk(vm.get(), resource.getDiskName()); - if (attachedDisk.isPresent()) { - detachDisk(vm.get(), attachedDisk.get().getKey()); - } else { - // this is unlikely event - disk is attached, but the vm doesn't - // have it in the list of attached disks; let's retry - logger.warn( - "Disk {} in MRG {} for workspace {} is attached, but not found in the list of attached disk for vm {}.", - resource.getResourceId(), - azureCloudContext.getAzureResourceGroupId(), - resource.getWorkspaceId(), - disk.virtualMachineId()); - return new StepResult(StepStatus.STEP_RESULT_FAILURE_RETRY); - } - } catch (ManagementException ex) { - if (AzureManagementExceptionUtils.isExceptionCode( - ex, AzureManagementExceptionUtils.RESOURCE_NOT_FOUND)) { - logger.info( - "Disk {} does not exist in MRG {} for workspace {}.", - resource.getResourceId(), - azureCloudContext.getAzureResourceGroupId(), - resource.getWorkspaceId()); - return StepResult.getStepResultSuccess(); - } - if (AzureManagementExceptionUtils.isExceptionCode( - ex, AzureManagementExceptionUtils.OPERATION_NOT_ALLOWED)) { - logger.error( - "Failed attempt to detach disk {} from vm in MRG {} for workspace {}.", - resource.getResourceId(), - azureCloudContext.getAzureResourceGroupId(), - resource.getWorkspaceId()); - return new StepResult(StepStatus.STEP_RESULT_FAILURE_FATAL, ex); - } - return new StepResult(StepStatus.STEP_RESULT_FAILURE_RETRY, ex); + var disk = computeManager.disks().getById(diskResourceId); + if (!disk.isAttachedToVirtualMachine()) { + logger.info( + "Disk {} in MRG {} for workspace {} is not attached to vm.", + resource.getResourceId(), + azureCloudContext.getAzureResourceGroupId(), + resource.getWorkspaceId()); + return StepResult.getStepResultSuccess(); + } + var vm = getVirtualMachine(computeManager, disk.virtualMachineId()); + if (vm.isEmpty()) { + // this is unlikely event - disk is attached, but the vm cannot be found; let's retry + logger.warn( + "Disk {} in MRG {} for workspace {} is attached to vm, but vm cannot be found. Retrying.", + resource.getResourceId(), + azureCloudContext.getAzureResourceGroupId(), + resource.getWorkspaceId()); + return new StepResult(StepStatus.STEP_RESULT_FAILURE_RETRY); + } + var attachedDisk = tryToFindAttachedDisk(vm.get(), resource.getDiskName()); + if (attachedDisk.isPresent()) { + detachDisk(vm.get(), attachedDisk.get().getKey()); + } else { + // this is unlikely event - disk is attached, but the vm doesn't + // have it in the list of attached disks; let's retry + logger.warn( + "Disk {} in MRG {} for workspace {} is attached, but not found in the list of attached disk for vm {}.", + resource.getResourceId(), + azureCloudContext.getAzureResourceGroupId(), + resource.getWorkspaceId(), + disk.virtualMachineId()); + return new StepResult(StepStatus.STEP_RESULT_FAILURE_RETRY); } return StepResult.getStepResultSuccess(); } + @Override + protected StepResult handleResourceDeleteException(Exception e, FlightContext context) { + final AzureCloudContext azureCloudContext = + DeleteAzureDiskFlightUtils.getAzureCloudContext(context); + if (e instanceof ManagementException ex + && AzureManagementExceptionUtils.isExceptionCode( + ex, AzureManagementExceptionUtils.RESOURCE_NOT_FOUND)) { + logger.info( + "Disk '{}' does not exist in MRG {} for workspace {}.", + resource.getResourceId(), + azureCloudContext.getAzureResourceGroupId(), + resource.getWorkspaceId()); + return StepResult.getStepResultSuccess(); + } else if (e instanceof ManagementException ex + && AzureManagementExceptionUtils.isExceptionCode( + ex, AzureManagementExceptionUtils.OPERATION_NOT_ALLOWED)) { + logger.error( + "Failed attempt to detach disk {} from vm in MRG {} for workspace {}.", + resource.getResourceId(), + azureCloudContext.getAzureResourceGroupId(), + resource.getWorkspaceId()); + return new StepResult(StepStatus.STEP_RESULT_FAILURE_FATAL, ex); + } else { + return super.handleResourceDeleteException(e, context); + } + } + @Override public StepResult undoStep(FlightContext context) throws InterruptedException { final AzureCloudContext azureCloudContext = diff --git a/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/disk/GetAzureDiskAttachedVmStep.java b/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/disk/GetAzureDiskAttachedVmStep.java index ec789d9459..f8170c6052 100644 --- a/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/disk/GetAzureDiskAttachedVmStep.java +++ b/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/disk/GetAzureDiskAttachedVmStep.java @@ -2,12 +2,11 @@ import bio.terra.stairway.FlightContext; import bio.terra.stairway.StepResult; -import bio.terra.stairway.StepStatus; import bio.terra.stairway.exception.RetryException; import bio.terra.workspace.app.configuration.external.AzureConfiguration; import bio.terra.workspace.common.exception.AzureManagementExceptionUtils; import bio.terra.workspace.service.crl.CrlService; -import bio.terra.workspace.service.resource.controlled.flight.delete.DeleteControlledResourceStep; +import bio.terra.workspace.service.resource.controlled.cloud.azure.DeleteAzureControlledResourceStep; import bio.terra.workspace.service.workspace.model.AzureCloudContext; import com.azure.core.management.exception.ManagementException; import com.azure.resourcemanager.compute.ComputeManager; @@ -16,54 +15,58 @@ /** * This steps works as a companion step for a disk deletion process. It gets identifier of a virtual - * machine to which data disk is attached and saves it into flight's working map. This data is - * required for 'undo' operation at DetachAzureDiskFromVmStep. It is required in case a disk has - * been detached from a virtual machine, but we need to attach it back in corresponding 'undo' - * operation. + * machine to which data disk is attached and saves it into flight's working map. It is required in + * case a disk has been detached from a virtual machine, but we need to attach it back in + * corresponding 'undo' operation. This is a separate step because the data is required for the + * 'undo' operation at DetachAzureDiskFromVmStep, and we need to guarantee its persistence - which + * we can't guarantee if it is from the 'do' portion of the same step. */ -public class GetAzureDiskAttachedVmStep implements DeleteControlledResourceStep { +public class GetAzureDiskAttachedVmStep + extends DeleteAzureControlledResourceStep { private static final Logger logger = LoggerFactory.getLogger(GetAzureDiskAttachedVmStep.class); private final CrlService crlService; private final AzureConfiguration azureConfig; - private final ControlledAzureDiskResource resource; public GetAzureDiskAttachedVmStep( AzureConfiguration azureConfig, CrlService crlService, ControlledAzureDiskResource resource) { + super(resource); this.crlService = crlService; this.azureConfig = azureConfig; - this.resource = resource; } @Override - public StepResult doStep(FlightContext context) throws InterruptedException, RetryException { + public StepResult deleteResource(FlightContext context) + throws InterruptedException, RetryException { final AzureCloudContext azureCloudContext = DeleteAzureDiskFlightUtils.getAzureCloudContext(context); final String diskResourceId = DeleteAzureDiskFlightUtils.getAzureDiskResourceId(azureCloudContext, resource); ComputeManager computeManager = crlService.getComputeManager(azureCloudContext, azureConfig); - try { - var disk = computeManager.disks().getById(diskResourceId); - if (disk.isAttachedToVirtualMachine()) { - context - .getWorkingMap() - .put(DeleteAzureDiskFlightUtils.DISK_ATTACHED_VM_ID_KEY, disk.virtualMachineId()); - } - } catch (ManagementException ex) { - if (AzureManagementExceptionUtils.isExceptionCode( - ex, AzureManagementExceptionUtils.RESOURCE_NOT_FOUND)) { - logger.info( - "Disk '{}' does not exist in workspace {}.", - resource.getResourceId(), - resource.getWorkspaceId()); - return StepResult.getStepResultSuccess(); - } - return new StepResult(StepStatus.STEP_RESULT_FAILURE_RETRY, ex); + var disk = computeManager.disks().getById(diskResourceId); + if (disk.isAttachedToVirtualMachine()) { + context + .getWorkingMap() + .put(DeleteAzureDiskFlightUtils.DISK_ATTACHED_VM_ID_KEY, disk.virtualMachineId()); } return StepResult.getStepResultSuccess(); } + @Override + protected StepResult handleResourceDeleteException(Exception e, FlightContext context) { + if (e instanceof ManagementException ex + && AzureManagementExceptionUtils.isExceptionCode( + ex, AzureManagementExceptionUtils.RESOURCE_NOT_FOUND)) { + logger.info( + "Disk '{}' does not exist in workspace {}.", + resource.getResourceId(), + resource.getWorkspaceId()); + return StepResult.getStepResultSuccess(); + } + return super.handleResourceDeleteException(e, context); + } + @Override public StepResult undoStep(FlightContext context) throws InterruptedException { // nothing to undo here diff --git a/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/kubernetesNamespace/ControlledAzureKubernetesNamespaceResource.java b/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/kubernetesNamespace/ControlledAzureKubernetesNamespaceResource.java index fd98b79fdf..dd5b678c44 100644 --- a/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/kubernetesNamespace/ControlledAzureKubernetesNamespaceResource.java +++ b/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/kubernetesNamespace/ControlledAzureKubernetesNamespaceResource.java @@ -283,10 +283,10 @@ private List getFederatedCredentialsDeleteSteps( return List.of( getGetManagedIdentityStep(flightBeanBag, MissingIdentityBehavior.ALLOW_MISSING), new DeleteFederatedCredentialStep( - getKubernetesNamespace(), flightBeanBag.getAzureConfig(), flightBeanBag.getCrlService(), - MissingIdentityBehavior.ALLOW_MISSING)); + MissingIdentityBehavior.ALLOW_MISSING, + this)); } else { return List.of(); } diff --git a/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/kubernetesNamespace/DeleteKubernetesNamespaceStep.java b/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/kubernetesNamespace/DeleteKubernetesNamespaceStep.java index 220b3f87d0..4455205dc3 100644 --- a/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/kubernetesNamespace/DeleteKubernetesNamespaceStep.java +++ b/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/kubernetesNamespace/DeleteKubernetesNamespaceStep.java @@ -18,19 +18,19 @@ import org.slf4j.LoggerFactory; import org.springframework.http.HttpStatus; -public class DeleteKubernetesNamespaceStep extends DeleteAzureControlledResourceStep { +public class DeleteKubernetesNamespaceStep + extends DeleteAzureControlledResourceStep { private static final Logger logger = LoggerFactory.getLogger(DeleteKubernetesNamespaceStep.class); private final UUID workspaceId; private final KubernetesClientProvider kubernetesClientProvider; - private final ControlledAzureKubernetesNamespaceResource resource; public DeleteKubernetesNamespaceStep( UUID workspaceId, KubernetesClientProvider kubernetesClientProvider, ControlledAzureKubernetesNamespaceResource resource) { + super(resource); this.workspaceId = workspaceId; this.kubernetesClientProvider = kubernetesClientProvider; - this.resource = resource; } @Override diff --git a/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/kubernetesNamespace/DeleteNamespaceRoleStep.java b/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/kubernetesNamespace/DeleteNamespaceRoleStep.java index da6052f393..951c471de5 100644 --- a/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/kubernetesNamespace/DeleteNamespaceRoleStep.java +++ b/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/kubernetesNamespace/DeleteNamespaceRoleStep.java @@ -10,19 +10,19 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class DeleteNamespaceRoleStep extends DeleteAzureControlledResourceStep { +public class DeleteNamespaceRoleStep + extends DeleteAzureControlledResourceStep { private static final Logger logger = LoggerFactory.getLogger(DeleteNamespaceRoleStep.class); private final UUID workspaceId; private final AzureDatabaseUtilsRunner azureDatabaseUtilsRunner; - private final ControlledAzureKubernetesNamespaceResource resource; public DeleteNamespaceRoleStep( UUID workspaceId, AzureDatabaseUtilsRunner azureDatabaseUtilsRunner, ControlledAzureKubernetesNamespaceResource resource) { + super(resource); this.workspaceId = workspaceId; this.azureDatabaseUtilsRunner = azureDatabaseUtilsRunner; - this.resource = resource; } private String getDeletePodName() { diff --git a/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/managedIdentity/DeleteAzureManagedIdentityStep.java b/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/managedIdentity/DeleteAzureManagedIdentityStep.java index 360cecb839..7dbe18299c 100644 --- a/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/managedIdentity/DeleteAzureManagedIdentityStep.java +++ b/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/managedIdentity/DeleteAzureManagedIdentityStep.java @@ -14,20 +14,20 @@ * A step for deleting a controlled Azure Managed Identity resource. This step uses the following * process to actually delete the Azure Managed Identity. */ -public class DeleteAzureManagedIdentityStep extends DeleteAzureControlledResourceStep { +public class DeleteAzureManagedIdentityStep + extends DeleteAzureControlledResourceStep { private static final Logger logger = LoggerFactory.getLogger(DeleteAzureManagedIdentityStep.class); private final AzureConfiguration azureConfig; private final CrlService crlService; - private final ControlledAzureManagedIdentityResource resource; public DeleteAzureManagedIdentityStep( AzureConfiguration azureConfig, CrlService crlService, ControlledAzureManagedIdentityResource resource) { + super(resource); this.crlService = crlService; this.azureConfig = azureConfig; - this.resource = resource; } @Override diff --git a/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/managedIdentity/DeleteFederatedCredentialStep.java b/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/managedIdentity/DeleteFederatedCredentialStep.java index fea76d7673..67aeb7e8a0 100644 --- a/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/managedIdentity/DeleteFederatedCredentialStep.java +++ b/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/managedIdentity/DeleteFederatedCredentialStep.java @@ -5,24 +5,25 @@ import bio.terra.workspace.app.configuration.external.AzureConfiguration; import bio.terra.workspace.service.crl.CrlService; import bio.terra.workspace.service.resource.controlled.cloud.azure.DeleteAzureControlledResourceStep; +import bio.terra.workspace.service.resource.controlled.cloud.azure.kubernetesNamespace.ControlledAzureKubernetesNamespaceResource; import bio.terra.workspace.service.workspace.flight.WorkspaceFlightMapKeys.ControlledResourceKeys; import bio.terra.workspace.service.workspace.model.AzureCloudContext; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class DeleteFederatedCredentialStep extends DeleteAzureControlledResourceStep { +public class DeleteFederatedCredentialStep + extends DeleteAzureControlledResourceStep { private static final Logger logger = LoggerFactory.getLogger(DeleteFederatedCredentialStep.class); - public final String k8sNamespace; private final AzureConfiguration azureConfig; private final CrlService crlService; private final MissingIdentityBehavior missingIdentityBehavior; public DeleteFederatedCredentialStep( - String k8sNamespace, AzureConfiguration azureConfig, CrlService crlService, - MissingIdentityBehavior missingIdentityBehavior) { - this.k8sNamespace = k8sNamespace; + MissingIdentityBehavior missingIdentityBehavior, + ControlledAzureKubernetesNamespaceResource resource) { + super(resource); this.azureConfig = azureConfig; this.crlService = crlService; this.missingIdentityBehavior = missingIdentityBehavior; @@ -49,7 +50,10 @@ public StepResult deleteResource(FlightContext context) { .manager() .serviceClient() .getFederatedIdentityCredentials() - .delete(azureCloudContext.getAzureResourceGroupId(), uamiName, k8sNamespace); + .delete( + azureCloudContext.getAzureResourceGroupId(), + uamiName, + resource.getKubernetesNamespace()); return StepResult.getStepResultSuccess(); } diff --git a/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/storageContainer/DeleteAzureStorageContainerStep.java b/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/storageContainer/DeleteAzureStorageContainerStep.java index 0431172697..040b516d13 100644 --- a/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/storageContainer/DeleteAzureStorageContainerStep.java +++ b/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/storageContainer/DeleteAzureStorageContainerStep.java @@ -1,7 +1,6 @@ package bio.terra.workspace.service.resource.controlled.cloud.azure.storageContainer; import bio.terra.common.iam.BearerToken; -import bio.terra.landingzone.db.exception.LandingZoneNotFoundException; import bio.terra.stairway.FlightContext; import bio.terra.stairway.StepResult; import bio.terra.workspace.amalgam.landingzone.azure.LandingZoneApiDispatch; @@ -21,12 +20,12 @@ import org.slf4j.LoggerFactory; /** A step for deleting a controlled Azure Storage Con resource. */ -public class DeleteAzureStorageContainerStep extends DeleteAzureControlledResourceStep { +public class DeleteAzureStorageContainerStep + extends DeleteAzureControlledResourceStep { private static final Logger logger = LoggerFactory.getLogger(DeleteAzureStorageContainerStep.class); private final AzureConfiguration azureConfig; private final CrlService crlService; - private final ControlledAzureStorageContainerResource resource; private final LandingZoneApiDispatch landingZoneApiDispatch; private final SamService samService; private final WorkspaceService workspaceService; @@ -38,9 +37,9 @@ public DeleteAzureStorageContainerStep( SamService samService, ControlledAzureStorageContainerResource resource, WorkspaceService workspaceService) { + super(resource); this.crlService = crlService; this.azureConfig = azureConfig; - this.resource = resource; this.landingZoneApiDispatch = landingZoneApiDispatch; this.samService = samService; this.workspaceService = workspaceService; @@ -55,38 +54,30 @@ public StepResult deleteResource(FlightContext context) { final StorageManager manager = crlService.getStorageManager(azureCloudContext, azureConfig); - try { - // Storage container was created based on landing zone shared storage account - var bearerToken = new BearerToken(samService.getWsmServiceAccountToken()); - UUID landingZoneId = - landingZoneApiDispatch.getLandingZoneId( - bearerToken, workspaceService.getWorkspace(resource.getWorkspaceId())); - Optional sharedStorageAccount = - landingZoneApiDispatch.getSharedStorageAccount(bearerToken, landingZoneId); - if (sharedStorageAccount.isEmpty()) { - // storage account is gone, so no container to delete - return StepResult.getStepResultSuccess(); - } else { - StorageAccount storageAccount = - manager.storageAccounts().getById(sharedStorageAccount.get().getResourceId()); - var storageAccountName = storageAccount.name(); - logger.info( - "Attempting to delete storage container '{}' in account '{}'", - resource.getStorageContainerName(), - storageAccountName); - manager - .blobContainers() - .delete( - azureCloudContext.getAzureResourceGroupId(), - storageAccountName, - resource.getStorageContainerName()); - return StepResult.getStepResultSuccess(); - } - } catch (LandingZoneNotFoundException lzne) { // Thrown by landingZoneApiDispatch - // If the landing zone is not present, it's probably because it was removed directly - logger.debug( - "Unable to delete storage container from workspace {}, because no landing zone was found in LZS", - resource.getWorkspaceId()); + // Storage container was created based on landing zone shared storage account + var bearerToken = new BearerToken(samService.getWsmServiceAccountToken()); + UUID landingZoneId = + landingZoneApiDispatch.getLandingZoneId( + bearerToken, workspaceService.getWorkspace(resource.getWorkspaceId())); + Optional sharedStorageAccount = + landingZoneApiDispatch.getSharedStorageAccount(bearerToken, landingZoneId); + if (sharedStorageAccount.isEmpty()) { + // storage account is gone, so no container to delete + return StepResult.getStepResultSuccess(); + } else { + StorageAccount storageAccount = + manager.storageAccounts().getById(sharedStorageAccount.get().getResourceId()); + var storageAccountName = storageAccount.name(); + logger.info( + "Attempting to delete storage container '{}' in account '{}'", + resource.getStorageContainerName(), + storageAccountName); + manager + .blobContainers() + .delete( + azureCloudContext.getAzureResourceGroupId(), + storageAccountName, + resource.getStorageContainerName()); return StepResult.getStepResultSuccess(); } } diff --git a/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/vm/DeleteAzureNetworkInterfaceStep.java b/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/vm/DeleteAzureNetworkInterfaceStep.java index ef8b33afaf..36e0f07f12 100644 --- a/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/vm/DeleteAzureNetworkInterfaceStep.java +++ b/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/vm/DeleteAzureNetworkInterfaceStep.java @@ -11,21 +11,21 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class DeleteAzureNetworkInterfaceStep extends DeleteAzureControlledResourceStep { +public class DeleteAzureNetworkInterfaceStep + extends DeleteAzureControlledResourceStep { private static final Logger logger = LoggerFactory.getLogger(CreateAzureNetworkInterfaceStep.class); private final AzureConfiguration azureConfig; private final CrlService crlService; // Network interface is not a freestanding WSM resource. It is tightly coupled to the Vm. - private final ControlledAzureVmResource resource; private final String networkInterfaceName; public DeleteAzureNetworkInterfaceStep( AzureConfiguration azureConfig, CrlService crlService, ControlledAzureVmResource resource) { + super(resource); this.crlService = crlService; this.azureConfig = azureConfig; - this.resource = resource; this.networkInterfaceName = String.format("nic-%s", resource.getVmName()); } diff --git a/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/vm/DeleteAzureVmStep.java b/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/vm/DeleteAzureVmStep.java index 11eaf4f0bd..31e733e89a 100644 --- a/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/vm/DeleteAzureVmStep.java +++ b/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/vm/DeleteAzureVmStep.java @@ -11,17 +11,17 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class DeleteAzureVmStep extends DeleteAzureControlledResourceStep { +public class DeleteAzureVmStep + extends DeleteAzureControlledResourceStep { private static final Logger logger = LoggerFactory.getLogger(DeleteAzureVmStep.class); private final AzureConfiguration azureConfig; private final CrlService crlService; - private final ControlledAzureVmResource resource; public DeleteAzureVmStep( AzureConfiguration azureConfig, CrlService crlService, ControlledAzureVmResource resource) { + super(resource); this.crlService = crlService; this.azureConfig = azureConfig; - this.resource = resource; } @Override diff --git a/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/vm/RemoveManagedIdentitiesAzureVmStep.java b/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/vm/RemoveManagedIdentitiesAzureVmStep.java index 309e3f67a5..ce2698ffe5 100644 --- a/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/vm/RemoveManagedIdentitiesAzureVmStep.java +++ b/service/src/main/java/bio/terra/workspace/service/resource/controlled/cloud/azure/vm/RemoveManagedIdentitiesAzureVmStep.java @@ -12,18 +12,18 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class RemoveManagedIdentitiesAzureVmStep extends DeleteAzureControlledResourceStep { +public class RemoveManagedIdentitiesAzureVmStep + extends DeleteAzureControlledResourceStep { private static final Logger logger = LoggerFactory.getLogger(RemoveManagedIdentitiesAzureVmStep.class); private final AzureConfiguration azureConfig; private final CrlService crlService; - private final ControlledAzureVmResource resource; public RemoveManagedIdentitiesAzureVmStep( AzureConfiguration azureConfig, CrlService crlService, ControlledAzureVmResource resource) { + super(resource); this.azureConfig = azureConfig; this.crlService = crlService; - this.resource = resource; } @Override diff --git a/service/src/test/java/bio/terra/workspace/service/resource/controlled/cloud/azure/database/DeleteAzureDatabaseStepUnitTest.java b/service/src/test/java/bio/terra/workspace/service/resource/controlled/cloud/azure/database/DeleteAzureDatabaseStepUnitTest.java index 2bdc2dd99a..3b94289792 100644 --- a/service/src/test/java/bio/terra/workspace/service/resource/controlled/cloud/azure/database/DeleteAzureDatabaseStepUnitTest.java +++ b/service/src/test/java/bio/terra/workspace/service/resource/controlled/cloud/azure/database/DeleteAzureDatabaseStepUnitTest.java @@ -17,6 +17,7 @@ import bio.terra.workspace.generated.model.ApiAzureLandingZoneDeployedResource; import bio.terra.workspace.service.crl.CrlService; import bio.terra.workspace.service.iam.SamService; +import bio.terra.workspace.service.resource.model.WsmResourceType; import bio.terra.workspace.service.workspace.WorkspaceService; import bio.terra.workspace.service.workspace.flight.WorkspaceFlightMapKeys; import bio.terra.workspace.service.workspace.model.AzureCloudContext; @@ -162,6 +163,7 @@ void lzsManagementExceptionReturnsSuccess() throws Exception { doThrow(new LandingZoneServiceApiException(apiException)) .when(landingZoneApiDispatch) .getSharedDatabase(token, landingZoneId); + when(resource.getResourceType()).thenReturn(WsmResourceType.CONTROLLED_AZURE_DATABASE); var step = new DeleteAzureDatabaseStep( azureConfig, diff --git a/service/src/test/java/bio/terra/workspace/service/resource/controlled/cloud/azure/disk/DetachAzureDiskFromVmStepTest.java b/service/src/test/java/bio/terra/workspace/service/resource/controlled/cloud/azure/disk/DetachAzureDiskFromVmStepTest.java index 235bcfce5b..bedd4df6f8 100644 --- a/service/src/test/java/bio/terra/workspace/service/resource/controlled/cloud/azure/disk/DetachAzureDiskFromVmStepTest.java +++ b/service/src/test/java/bio/terra/workspace/service/resource/controlled/cloud/azure/disk/DetachAzureDiskFromVmStepTest.java @@ -32,6 +32,8 @@ import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; @@ -126,6 +128,23 @@ void doStep_DiskNameDoesntMatchAnyOfAttachedDisks() throws InterruptedException verify(virtualMachineUpdateMock, never()).apply(); } + @ParameterizedTest + @ValueSource( + strings = {"SubscriptionNotFound", "InvalidAuthenticationTokenTenant", "AuthorizationFailed"}) + void doStep_missingMRGManagementExceptionsReturnsSuccess(String exceptionCode) + throws InterruptedException { + setupCommonMocks(); + when(computeManagerMock.disks()).thenReturn(disksMock); + ManagementException missingMRGExceptionMock = setupManagementExceptionMock(exceptionCode); + when(disksMock.getById(any())).thenThrow(missingMRGExceptionMock); + + StepResult stepResult = detachAzureDiskFromVmStep.doStep(flightContextMock); + + assertThat(stepResult, equalTo(StepResult.getStepResultSuccess())); + verify(disksMock).getById(any()); + verify(virtualMachineUpdateMock, never()).apply(); + } + @Test void undoStep_DiskWasInitiallyAttachedToVm() throws InterruptedException { setupCommonMocks(); diff --git a/service/src/test/java/bio/terra/workspace/service/resource/controlled/cloud/azure/disk/GetAzureDiskAttachedVmStepTest.java b/service/src/test/java/bio/terra/workspace/service/resource/controlled/cloud/azure/disk/GetAzureDiskAttachedVmStepTest.java index d0f201d338..7b92fa9f6c 100644 --- a/service/src/test/java/bio/terra/workspace/service/resource/controlled/cloud/azure/disk/GetAzureDiskAttachedVmStepTest.java +++ b/service/src/test/java/bio/terra/workspace/service/resource/controlled/cloud/azure/disk/GetAzureDiskAttachedVmStepTest.java @@ -25,6 +25,8 @@ import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; @@ -89,6 +91,20 @@ void doStep_DiskIsNotFound() throws InterruptedException { verify(flightWorkingMapMock, never()).put(any(), any()); } + @ParameterizedTest + @ValueSource( + strings = {"SubscriptionNotFound", "InvalidAuthenticationTokenTenant", "AuthorizationFailed"}) + void doStep_missingMRGManagementExceptionsReturnsSuccess(String exceptionCode) + throws InterruptedException { + setupCommonMocks(); + ManagementException resourceNotFoundExceptionMock = setupManagementExceptionMock(exceptionCode); + when(disksMock.getById(anyString())).thenThrow(resourceNotFoundExceptionMock); + + StepResult stepResult = getAzureDiskAttachedVmStep.doStep(flightContextMock); + assertThat(stepResult, equalTo(StepResult.getStepResultSuccess())); + verify(flightWorkingMapMock, never()).put(any(), any()); + } + @Test void undoStep_Success() throws InterruptedException { // always return success