diff --git a/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/cf/clients/CustomServiceKeysClient.java b/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/cf/clients/CustomServiceKeysClient.java index 6a51e401e6..733843fd25 100644 --- a/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/cf/clients/CustomServiceKeysClient.java +++ b/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/cf/clients/CustomServiceKeysClient.java @@ -3,10 +3,8 @@ import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.Objects; -import java.util.UUID; +import java.util.function.Function; import java.util.stream.Collectors; -import java.util.stream.Stream; import org.cloudfoundry.client.v3.serviceinstances.ServiceInstanceType; import org.cloudfoundry.multiapps.controller.client.facade.CloudCredentials; @@ -32,8 +30,7 @@ public CustomServiceKeysClient(ApplicationConfiguration configuration, WebClient } public List getServiceKeysByMetadataAndGuids(String spaceGuid, String mtaId, String mtaNamespace, - List services, - List existingServiceGuids) { + List services) { String labelSelector = MtaMetadataCriteriaBuilder.builder() .label(MtaMetadataLabels.SPACE_GUID) .hasValue(spaceGuid) @@ -46,40 +43,31 @@ public List getServiceKeysByMetadataAndGuids(String space .build() .get(); - List managedGuids = extractManagedServiceGuids(services); - - List allServiceGuids = Stream.concat(managedGuids.stream(), existingServiceGuids.stream()) - .filter(Objects::nonNull) - .toList(); - - if (allServiceGuids.isEmpty()) { - return List.of(); - } return new CustomControllerClientErrorHandler().handleErrorsOrReturnResult( - () -> getServiceKeysByMetadataInternal(labelSelector, allServiceGuids)); + () -> getServiceKeysByMetadataInternal(labelSelector, services)); } - private List extractManagedServiceGuids(List services) { - return getManagedServices(services).stream() - .map(DeployedMtaService::getGuid) - .map(UUID::toString) - .toList(); - } - - private List getServiceKeysByMetadataInternal(String labelSelector, List guids) { - - String uriSuffix = INCLUDE_SERVICE_INSTANCE_RESOURCES_PARAM - + "&service_instance_guids=" + String.join(",", guids); - - return getListOfResources(new ServiceKeysResponseMapper(), - SERVICE_KEYS_BY_METADATA_SELECTOR_URI + uriSuffix, + private List getServiceKeysByMetadataInternal(String labelSelector, List services) { + String uriSuffix = INCLUDE_SERVICE_INSTANCE_RESOURCES_PARAM; + List managedServices = getManagedServices(services); + if (managedServices != null) { + uriSuffix += "&service_instance_guids=" + managedServices.stream() + .map(service -> service.getGuid() + .toString()) + .collect(Collectors.joining(",")); + } + return getListOfResources(new ServiceKeysResponseMapper(managedServices), SERVICE_KEYS_BY_METADATA_SELECTOR_URI + uriSuffix, labelSelector); } private List getManagedServices(List services) { - return services.stream() - .filter(this::serviceIsNotUserProvided) - .toList(); + if (services == null) { + return null; + } + List managedServices = services.stream() + .filter(this::serviceIsNotUserProvided) + .collect(Collectors.toList()); + return managedServices.isEmpty() ? null : managedServices; } private boolean serviceIsNotUserProvided(DeployedMtaService service) { @@ -88,12 +76,23 @@ private boolean serviceIsNotUserProvided(DeployedMtaService service) { } protected class ServiceKeysResponseMapper extends ResourcesResponseMapper { - public ServiceKeysResponseMapper() { + + List mtaServices; + + public ServiceKeysResponseMapper(List mtaServices) { + this.mtaServices = mtaServices; } @Override public List getMappedResources() { - Map serviceMapping = getIncludedServiceInstancesMapping(); + Map serviceMapping; + if (mtaServices != null) { + serviceMapping = mtaServices.stream() + .collect(Collectors.toMap(service -> service.getGuid() + .toString(), Function.identity())); + } else { + serviceMapping = getIncludedServiceInstancesMapping(); + } return getQueriedResources().stream() .map(resource -> resourceMapper.mapServiceKeyResource(resource, serviceMapping)) .collect(Collectors.toList()); diff --git a/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/util/NameUtil.java b/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/util/NameUtil.java index 2441ef044d..675f4603e9 100644 --- a/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/util/NameUtil.java +++ b/multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/util/NameUtil.java @@ -73,8 +73,7 @@ public static String computeNamespacedNameWithLength(String name, String namespa private static String getNameWithNamespaceSuffix(String name, String namespace, int maxLength) { String namespaceSuffix = getNamespaceSuffix(namespace); - String shortenedName = getNameWithProperLength(name, - calculateNameLengthWithoutNamespaceAndBlueGreenSuffix(namespaceSuffix, maxLength)); + String shortenedName = getNameWithProperLength(name, calculateNameLengthWithoutNamespaceAndBlueGreenSuffix(namespaceSuffix, maxLength)); return correctNameSuffix(shortenedName, name, namespaceSuffix); } @@ -121,7 +120,6 @@ public static String computeUserNamespaceWithSystemNamespace(String systemNamesp } return systemNamespace; } - private static String getShortenedName(String name, int maxLength) { String nameHashCode = getHashCodeAsHexString(name); if (maxLength < nameHashCode.length()) { @@ -159,14 +157,6 @@ public static String getServiceName(Resource resource) { .get(SupportedParameters.SERVICE_NAME); } - public static String getServiceInstanceNameOrDefault(Resource resource) { - String serviceInstanceName = getServiceName(resource); - if (serviceInstanceName == null || serviceInstanceName.isBlank()) { - return resource.getName(); - } - return serviceInstanceName; - } - public static class NameRequirements { public static final String XS_APP_NAME_PATTERN = "(?!sap_system)[a-zA-Z0-9\\._\\-\\\\/]{1,240}"; diff --git a/multiapps-controller-core/src/test/java/org/cloudfoundry/multiapps/controller/core/util/NameUtilTest.java b/multiapps-controller-core/src/test/java/org/cloudfoundry/multiapps/controller/core/util/NameUtilTest.java index 264f2c56a8..db4c1f3731 100644 --- a/multiapps-controller-core/src/test/java/org/cloudfoundry/multiapps/controller/core/util/NameUtilTest.java +++ b/multiapps-controller-core/src/test/java/org/cloudfoundry/multiapps/controller/core/util/NameUtilTest.java @@ -1,20 +1,16 @@ package org.cloudfoundry.multiapps.controller.core.util; -import java.util.HashMap; -import java.util.Map; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + import java.util.stream.Stream; -import org.cloudfoundry.multiapps.controller.core.model.SupportedParameters; import org.cloudfoundry.multiapps.controller.core.util.NameUtil.NameRequirements; -import org.cloudfoundry.multiapps.mta.model.Resource; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; - class NameUtilTest { static Stream testGetNameWithProperLength() { @@ -98,43 +94,4 @@ void testComputeNamespacedNameWithLength(String name, String namespace, boolean NameUtil.computeNamespacedNameWithLength(name, namespace, applyNamespace, applyNamespaceAsSuffix, maxLength)); } - @Test - void testGetServiceInstanceNameOrDefault_UsesServiceNameParameter() { - Resource resource = createResource("resource-name", "service-name-from-param"); - - String serviceInstanceName = NameUtil.getServiceInstanceNameOrDefault(resource); - - assertEquals("service-name-from-param", serviceInstanceName); - } - - @Test - void testGetServiceInstanceNameOrDefault_FallsBackToResourceNameWhenServiceNameMissing() { - Resource resource = createResource("resource-name-1", null); - - String serviceInstanceName = NameUtil.getServiceInstanceNameOrDefault(resource); - - assertEquals("resource-name-1", serviceInstanceName); - } - - @Test - void testGetServiceInstanceNameOrDefault_FallsBackToResourceNameWhenServiceNameBlank() { - Resource resource = createResource("resource-name-2", " "); - - String serviceInstanceName = NameUtil.getServiceInstanceNameOrDefault(resource); - - assertEquals("resource-name-2", serviceInstanceName); - } - - private Resource createResource(String resourceName, String serviceInstanceName) { - Resource resource = Resource.createV3(); - resource.setName(resourceName); - - Map params = new HashMap<>(); - if (serviceInstanceName != null) { - params.put(SupportedParameters.SERVICE_NAME, serviceInstanceName); - } - resource.setParameters(params); - - return resource; - } } diff --git a/multiapps-controller-process/src/main/java/org/cloudfoundry/multiapps/controller/process/Messages.java b/multiapps-controller-process/src/main/java/org/cloudfoundry/multiapps/controller/process/Messages.java index de999943a7..b1fcedc0b7 100644 --- a/multiapps-controller-process/src/main/java/org/cloudfoundry/multiapps/controller/process/Messages.java +++ b/multiapps-controller-process/src/main/java/org/cloudfoundry/multiapps/controller/process/Messages.java @@ -789,8 +789,6 @@ public class Messages { public static final String GETTING_FEATURES_FOR_APPLICATION_0 = "Getting features for application \"{0}\""; public static final String TOTAL_SIZE_OF_ALL_RESOLVED_CONTENT_0 = "Total size for all resolved content {0}"; - public static final String IGNORING_NOT_FOUND_OPTIONAL_SERVICE = "Service {0} not found but is optional"; - public static final String IGNORING_NOT_FOUND_INACTIVE_SERVICE = "Service {0} not found but is inactive"; // Not log messages public static final String SERVICE_TYPE = "{0}/{1}"; diff --git a/multiapps-controller-process/src/main/java/org/cloudfoundry/multiapps/controller/process/steps/BuildCloudUndeployModelStep.java b/multiapps-controller-process/src/main/java/org/cloudfoundry/multiapps/controller/process/steps/BuildCloudUndeployModelStep.java index 50641076be..75c29b4619 100644 --- a/multiapps-controller-process/src/main/java/org/cloudfoundry/multiapps/controller/process/steps/BuildCloudUndeployModelStep.java +++ b/multiapps-controller-process/src/main/java/org/cloudfoundry/multiapps/controller/process/steps/BuildCloudUndeployModelStep.java @@ -65,17 +65,9 @@ protected StepPhase executeStep(ProcessContext context) { getStepLogger().debug(Messages.BUILDING_CLOUD_UNDEPLOY_MODEL); DeployedMta deployedMta = context.getVariable(Variables.DEPLOYED_MTA); - List serviceKeysToDelete = computeServiceKeysToDelete(context); - getStepLogger().debug(Messages.SERVICE_KEYS_FOR_DELETION, serviceKeysToDelete); - if (deployedMta == null) { setComponentsToUndeploy(context, Collections.emptyList(), Collections.emptyList(), Collections.emptyList(), Collections.emptyList(), Collections.emptyList()); - - if (!serviceKeysToDelete.isEmpty()) { - context.setVariable(Variables.SERVICE_KEYS_TO_DELETE, - getServiceKeysToDelete(context, serviceKeysToDelete)); - } return StepPhase.DONE; } @@ -106,6 +98,9 @@ protected StepPhase executeStep(ProcessContext context) { servicesForApplications, serviceNames); getStepLogger().debug(Messages.SERVICES_TO_DELETE, servicesToDelete); + List serviceKeysToDelete = computeServiceKeysToDelete(context); + getStepLogger().debug(Messages.SERVICE_KEYS_FOR_DELETION, serviceKeysToDelete); + List appsToUndeploy = computeAppsToUndeploy(deployedAppsToUndeploy, context.getControllerClient()); DeployedMta backupMta = context.getVariable(Variables.BACKUP_MTA); diff --git a/multiapps-controller-process/src/main/java/org/cloudfoundry/multiapps/controller/process/steps/DetectDeployedMtaStep.java b/multiapps-controller-process/src/main/java/org/cloudfoundry/multiapps/controller/process/steps/DetectDeployedMtaStep.java index 204b1359c8..eb25617a37 100644 --- a/multiapps-controller-process/src/main/java/org/cloudfoundry/multiapps/controller/process/steps/DetectDeployedMtaStep.java +++ b/multiapps-controller-process/src/main/java/org/cloudfoundry/multiapps/controller/process/steps/DetectDeployedMtaStep.java @@ -9,27 +9,19 @@ import org.apache.commons.lang3.StringUtils; import org.cloudfoundry.multiapps.controller.client.facade.CloudControllerClient; import org.cloudfoundry.multiapps.controller.client.facade.CloudCredentials; -import org.cloudfoundry.multiapps.controller.client.facade.CloudOperationException; -import org.cloudfoundry.multiapps.controller.client.facade.domain.CloudServiceInstance; import org.cloudfoundry.multiapps.controller.core.cf.clients.CustomServiceKeysClient; import org.cloudfoundry.multiapps.controller.core.cf.clients.WebClientFactory; import org.cloudfoundry.multiapps.controller.core.cf.detect.DeployedMtaDetector; import org.cloudfoundry.multiapps.controller.core.cf.metadata.MtaMetadata; -import org.cloudfoundry.multiapps.controller.core.cf.v2.ResourceType; import org.cloudfoundry.multiapps.controller.core.model.DeployedMta; import org.cloudfoundry.multiapps.controller.core.model.DeployedMtaService; import org.cloudfoundry.multiapps.controller.core.model.DeployedMtaServiceKey; import org.cloudfoundry.multiapps.controller.core.security.serialization.SecureSerialization; import org.cloudfoundry.multiapps.controller.core.security.token.TokenService; -import org.cloudfoundry.multiapps.controller.core.util.CloudModelBuilderUtil; import org.cloudfoundry.multiapps.controller.core.util.NameUtil; import org.cloudfoundry.multiapps.controller.process.Constants; import org.cloudfoundry.multiapps.controller.process.Messages; import org.cloudfoundry.multiapps.controller.process.variables.Variables; -import org.cloudfoundry.multiapps.mta.model.DeploymentDescriptor; -import org.cloudfoundry.multiapps.mta.model.Resource; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.context.annotation.Scope; @@ -46,8 +38,6 @@ public class DetectDeployedMtaStep extends SyncFlowableStep { @Inject private WebClientFactory webClientFactory; - private static final Logger LOGGER = LoggerFactory.getLogger(DetectDeployedMtaStep.class); - @Override protected StepPhase executeStep(ProcessContext context) { getStepLogger().debug(Messages.DETECTING_DEPLOYED_MTA); @@ -104,69 +94,14 @@ private void detectBackupMta(String mtaId, String mtaNamespace, CloudControllerC private List detectDeployedServiceKeys(String mtaId, String mtaNamespace, DeployedMta deployedMta, ProcessContext context) { - List deployedManagedMtaServices = Optional.ofNullable(deployedMta) - .map(DeployedMta::getServices) - .orElse(List.of()); + List deployedMtaServices = deployedMta == null ? null : deployedMta.getServices(); String spaceGuid = context.getVariable(Variables.SPACE_GUID); String userGuid = context.getVariable(Variables.USER_GUID); var token = tokenService.getToken(userGuid); var creds = new CloudCredentials(token, true); CustomServiceKeysClient serviceKeysClient = getCustomServiceKeysClient(creds, context.getVariable(Variables.CORRELATION_ID)); - - List existingInstanceGuids = getExistingServiceGuids(context); - - return serviceKeysClient.getServiceKeysByMetadataAndGuids( - spaceGuid, mtaId, mtaNamespace, deployedManagedMtaServices, existingInstanceGuids - ); - } - - private List getExistingServiceGuids(ProcessContext context) { - CloudControllerClient client = context.getControllerClient(); - List resources = getExistingServiceResourcesFromDescriptor(context); - - return resources.stream() - .map(resource -> resolveServiceGuid(client, resource)) - .flatMap(Optional::stream) - .toList(); - } - - private Optional resolveServiceGuid(CloudControllerClient client, Resource resource) { - String serviceInstanceName = NameUtil.getServiceInstanceNameOrDefault(resource); - - try { - CloudServiceInstance instance = client.getServiceInstance(serviceInstanceName); - return Optional.of(instance.getGuid() - .toString()); - } catch (CloudOperationException e) { - if (resource.isOptional()) { - logIgnoredService(Messages.IGNORING_NOT_FOUND_OPTIONAL_SERVICE, resource.getName(), e); - return Optional.empty(); - } - if (!resource.isActive()) { - logIgnoredService(Messages.IGNORING_NOT_FOUND_INACTIVE_SERVICE, resource.getName(), e); - return Optional.empty(); - } - throw e; - } - } - - private void logIgnoredService(String message, String serviceName, Exception e) { - String formattedMessage = MessageFormat.format(message, serviceName); - getStepLogger().debug(formattedMessage); - LOGGER.error(formattedMessage, e); - } - - private List getExistingServiceResourcesFromDescriptor(ProcessContext context) { - DeploymentDescriptor descriptor = context.getVariable(Variables.DEPLOYMENT_DESCRIPTOR); - - if (descriptor == null) { - return List.of(); - } - return descriptor.getResources() - .stream() - .filter(resource -> CloudModelBuilderUtil.getResourceType(resource) == ResourceType.EXISTING_SERVICE) - .toList(); + return serviceKeysClient.getServiceKeysByMetadataAndGuids(spaceGuid, mtaId, mtaNamespace, deployedMtaServices); } private void logNoMtaDeployedDetected(String mtaId, String mtaNamespace) { diff --git a/multiapps-controller-process/src/test/java/org/cloudfoundry/multiapps/controller/process/steps/DetectDeployedMtaStepTest.java b/multiapps-controller-process/src/test/java/org/cloudfoundry/multiapps/controller/process/steps/DetectDeployedMtaStepTest.java index bbb856972c..39faae73aa 100644 --- a/multiapps-controller-process/src/test/java/org/cloudfoundry/multiapps/controller/process/steps/DetectDeployedMtaStepTest.java +++ b/multiapps-controller-process/src/test/java/org/cloudfoundry/multiapps/controller/process/steps/DetectDeployedMtaStepTest.java @@ -6,7 +6,7 @@ import org.cloudfoundry.client.v3.Metadata; import org.cloudfoundry.multiapps.common.test.TestUtil; -import org.cloudfoundry.multiapps.common.test.Tester; +import org.cloudfoundry.multiapps.common.test.Tester.Expectation; import org.cloudfoundry.multiapps.common.util.JsonUtil; import org.cloudfoundry.multiapps.controller.client.facade.CloudControllerClient; import org.cloudfoundry.multiapps.controller.client.facade.CloudCredentials; @@ -31,12 +31,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyList; -import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.ArgumentMatchers.argThat; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.when; class DetectDeployedMtaStepTest extends SyncFlowableStepTest { @@ -68,30 +62,19 @@ void testExecuteWithDeployedMta() { .getKeys(); List deployedComponents = List.of(deployedMta); - when(deployedMtaDetector.detectDeployedMtas(any(CloudControllerClient.class))) - .thenReturn(deployedComponents); - when(deployedMtaDetector.detectDeployedMtaByNameAndNamespace(eq(MTA_ID), eq(null), - any(CloudControllerClient.class))) - .thenReturn(Optional.of(deployedMta)); - - when(customClientMock.getServiceKeysByMetadataAndGuids( - eq(SPACE_GUID), eq(MTA_ID), isNull(), - eq(deployedMta.getServices()), - argThat(List::isEmpty))) - .thenReturn(deployedKeys); - - when(customClientMock.getServiceKeysByMetadataAndGuids( - eq(SPACE_GUID), eq(MTA_ID), isNull(), - anyList(), - argThat(list -> list != null && !list.isEmpty()))) - .thenReturn(Collections.emptyList()); + when(deployedMtaDetector.detectDeployedMtas(Mockito.any(CloudControllerClient.class))).thenReturn(deployedComponents); + when(deployedMtaDetector.detectDeployedMtaByNameAndNamespace(Mockito.eq(MTA_ID), Mockito.eq(null), + Mockito.any( + CloudControllerClient.class))).thenReturn( + Optional.of(deployedMta)); + when(customClientMock.getServiceKeysByMetadataAndGuids(Mockito.eq(SPACE_GUID), Mockito.eq(MTA_ID), Mockito.isNull(), + Mockito.eq(deployedMta.getServices()))).thenReturn(deployedKeys); step.execute(execution); assertStepFinishedSuccessfully(); - tester.test(() -> context.getVariable(Variables.DEPLOYED_MTA), - new Tester.Expectation(Tester.Expectation.Type.JSON, DEPLOYED_MTA_LOCATION)); + tester.test(() -> context.getVariable(Variables.DEPLOYED_MTA), new Expectation(Expectation.Type.JSON, DEPLOYED_MTA_LOCATION)); assertEquals(deployedKeys, context.getVariable(Variables.DEPLOYED_MTA_SERVICE_KEYS)); } @@ -99,16 +82,8 @@ void testExecuteWithDeployedMta() { void testExecuteWithoutDeployedMta() { when(deployedMtaDetector.detectDeployedMtas(client)).thenReturn(Collections.emptyList()); when(deployedMtaDetector.detectDeployedMtaByNameAndNamespace(MTA_ID, null, client)).thenReturn(Optional.empty()); - when(customClientMock.getServiceKeysByMetadataAndGuids( - eq(SPACE_GUID), eq(MTA_ID), isNull(), - eq(Collections.emptyList()), - eq(Collections.emptyList()))) - .thenReturn(Collections.emptyList()); - - when(customClientMock.getServiceKeysByMetadataAndGuids( - anyString(), anyString(), isNull(), - anyList(), anyList())) - .thenReturn(Collections.emptyList()); + when(customClientMock.getServiceKeysByMetadataAndGuids(SPACE_GUID, MTA_ID, null, + Collections.emptyList())).thenReturn(Collections.emptyList()); step.execute(execution); @@ -155,10 +130,10 @@ void testExecuteWithBackupdMta() { .build()) .build(); - when(deployedMtaDetector.detectDeployedMtaByNameAndNamespace(eq(MTA_ID), eq(null), + when(deployedMtaDetector.detectDeployedMtaByNameAndNamespace(Mockito.eq(MTA_ID), Mockito.eq(null), Mockito.any())).thenReturn(Optional.of(deployedMta)); - when(deployedMtaDetector.detectDeployedMtaByNameAndNamespace(eq(MTA_ID), - eq(NameUtil.computeUserNamespaceWithSystemNamespace( + when(deployedMtaDetector.detectDeployedMtaByNameAndNamespace(Mockito.eq(MTA_ID), + Mockito.eq(NameUtil.computeUserNamespaceWithSystemNamespace( Constants.MTA_BACKUP_NAMESPACE, null)), Mockito.any())).thenReturn(Optional.of(backupMta));