From 5dfb5b28207ddffc6d6a836b4c4946714420df33 Mon Sep 17 00:00:00 2001 From: stiv03 Date: Wed, 15 Oct 2025 15:39:48 +0300 Subject: [PATCH 1/7] Fix service keys not renewed when existing service is used --- .../cf/clients/CustomServiceKeysClient.java | 53 ++++++++++++++----- .../process/steps/DetectDeployedMtaStep.java | 45 +++++++++++++++- .../steps/DetectDeployedMtaStepTest.java | 53 ++++++++++++++----- 3 files changed, 123 insertions(+), 28 deletions(-) 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 733843fd25..ff45826e89 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,8 +3,10 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Objects; 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; @@ -30,7 +32,8 @@ public CustomServiceKeysClient(ApplicationConfiguration configuration, WebClient } public List getServiceKeysByMetadataAndGuids(String spaceGuid, String mtaId, String mtaNamespace, - List services) { + List services, + List existingServiceGuids) { String labelSelector = MtaMetadataCriteriaBuilder.builder() .label(MtaMetadataLabels.SPACE_GUID) .hasValue(spaceGuid) @@ -43,20 +46,46 @@ public List getServiceKeysByMetadataAndGuids(String space .build() .get(); - return new CustomControllerClientErrorHandler().handleErrorsOrReturnResult( - () -> getServiceKeysByMetadataInternal(labelSelector, services)); + List managedGuids = extractManagedServiceGuids(services); + + List allServiceGuids = Stream.concat(managedGuids.stream(), existingServiceGuids.stream()) + .filter(Objects::nonNull) + .distinct() + .toList(); + + if (allServiceGuids.isEmpty()) { + return List.of(); + } + + return getServiceKeysByMetadataInternal(labelSelector, allServiceGuids); + } + + private List extractManagedServiceGuids(List services) { + if (services == null || services.isEmpty()) { + return List.of(); + } + List managed = getManagedServices(services); + if (managed == null || managed.isEmpty()) { + return List.of(); + } + return managed.stream() + .map(s -> s.getGuid() != null ? s.getGuid() + .toString() : null) + .filter(Objects::nonNull) + .distinct() + .toList(); } - 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(",")); + private List getServiceKeysByMetadataInternal(String labelSelector, List guids) { + + if (guids == null || guids.isEmpty()) { + return List.of(); } - return getListOfResources(new ServiceKeysResponseMapper(managedServices), SERVICE_KEYS_BY_METADATA_SELECTOR_URI + uriSuffix, + String uriSuffix = INCLUDE_SERVICE_INSTANCE_RESOURCES_PARAM + + "&service_instance_guids=" + String.join(",", guids); + + return getListOfResources(new ServiceKeysResponseMapper(null), + SERVICE_KEYS_BY_METADATA_SELECTOR_URI + uriSuffix, labelSelector); } 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 09711ac4c2..8c4d99c148 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,19 +9,24 @@ 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.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.springframework.beans.factory.annotation.Qualifier; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.context.annotation.Scope; @@ -94,7 +99,7 @@ private void detectBackupMta(String mtaId, String mtaNamespace, CloudControllerC private List detectDeployedServiceKeys(String mtaId, String mtaNamespace, DeployedMta deployedMta, ProcessContext context) { - List deployedMtaServices = deployedMta == null ? null : deployedMta.getServices(); + List deployedManagedMtaServices = deployedMta == null ? null : deployedMta.getServices(); String spaceGuid = context.getVariable(Variables.SPACE_GUID); String user = context.getVariable(Variables.USER); String userGuid = context.getVariable(Variables.USER_GUID); @@ -102,7 +107,43 @@ private List detectDeployedServiceKeys(String mtaId, Stri var creds = new CloudCredentials(token, true); CustomServiceKeysClient serviceKeysClient = getCustomServiceKeysClient(creds, context.getVariable(Variables.CORRELATION_ID)); - return serviceKeysClient.getServiceKeysByMetadataAndGuids(spaceGuid, mtaId, mtaNamespace, deployedMtaServices); + + List existingInstanceGuids = getExistingServiceGuids(context); + + return serviceKeysClient.getServiceKeysByMetadataAndGuids( + spaceGuid, mtaId, mtaNamespace, deployedManagedMtaServices, existingInstanceGuids + ); + } + + private List getExistingServiceGuids(ProcessContext context) { + CloudControllerClient client = context.getControllerClient(); + List existingNames = getExistingServiceNamesFromDescriptor(context); + + return existingNames.stream() + .map(name -> resolveServiceGuid(client, name)) + .toList(); + } + + private String resolveServiceGuid(CloudControllerClient client, String serviceName) { + CloudServiceInstance instance = client.getServiceInstance(serviceName, false); + return instance != null && instance.getMetadata() != null + ? instance.getMetadata() + .getGuid() + .toString() : null; + } + + private List getExistingServiceNamesFromDescriptor(ProcessContext context) { + DeploymentDescriptor descriptor = context.getVariable(Variables.DEPLOYMENT_DESCRIPTOR); + + if (descriptor == null || descriptor.getResources() == null) { + return List.of(); + } + return descriptor.getResources() + .stream() + .filter(resource -> CloudModelBuilderUtil.getResourceType(resource) == ResourceType.EXISTING_SERVICE) + .map(Resource::getName) + .distinct() + .toList(); } 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 39faae73aa..bbb856972c 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.Expectation; +import org.cloudfoundry.multiapps.common.test.Tester; import org.cloudfoundry.multiapps.common.util.JsonUtil; import org.cloudfoundry.multiapps.controller.client.facade.CloudControllerClient; import org.cloudfoundry.multiapps.controller.client.facade.CloudCredentials; @@ -31,6 +31,12 @@ 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 { @@ -62,19 +68,30 @@ void testExecuteWithDeployedMta() { .getKeys(); List deployedComponents = List.of(deployedMta); - 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); + 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()); step.execute(execution); assertStepFinishedSuccessfully(); - tester.test(() -> context.getVariable(Variables.DEPLOYED_MTA), new Expectation(Expectation.Type.JSON, DEPLOYED_MTA_LOCATION)); + tester.test(() -> context.getVariable(Variables.DEPLOYED_MTA), + new Tester.Expectation(Tester.Expectation.Type.JSON, DEPLOYED_MTA_LOCATION)); assertEquals(deployedKeys, context.getVariable(Variables.DEPLOYED_MTA_SERVICE_KEYS)); } @@ -82,8 +99,16 @@ void testExecuteWithDeployedMta() { void testExecuteWithoutDeployedMta() { when(deployedMtaDetector.detectDeployedMtas(client)).thenReturn(Collections.emptyList()); when(deployedMtaDetector.detectDeployedMtaByNameAndNamespace(MTA_ID, null, client)).thenReturn(Optional.empty()); - when(customClientMock.getServiceKeysByMetadataAndGuids(SPACE_GUID, MTA_ID, null, - Collections.emptyList())).thenReturn(Collections.emptyList()); + 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()); step.execute(execution); @@ -130,10 +155,10 @@ void testExecuteWithBackupdMta() { .build()) .build(); - when(deployedMtaDetector.detectDeployedMtaByNameAndNamespace(Mockito.eq(MTA_ID), Mockito.eq(null), + when(deployedMtaDetector.detectDeployedMtaByNameAndNamespace(eq(MTA_ID), eq(null), Mockito.any())).thenReturn(Optional.of(deployedMta)); - when(deployedMtaDetector.detectDeployedMtaByNameAndNamespace(Mockito.eq(MTA_ID), - Mockito.eq(NameUtil.computeUserNamespaceWithSystemNamespace( + when(deployedMtaDetector.detectDeployedMtaByNameAndNamespace(eq(MTA_ID), + eq(NameUtil.computeUserNamespaceWithSystemNamespace( Constants.MTA_BACKUP_NAMESPACE, null)), Mockito.any())).thenReturn(Optional.of(backupMta)); From db592ac086a02e59e98b3d42a807dc7b284901d4 Mon Sep 17 00:00:00 2001 From: stiv03 Date: Tue, 21 Oct 2025 15:25:05 +0300 Subject: [PATCH 2/7] fix when no deployedMta --- .../cf/clients/CustomServiceKeysClient.java | 18 +++--------------- .../steps/BuildCloudUndeployModelStep.java | 11 ++++++++--- 2 files changed, 11 insertions(+), 18 deletions(-) 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 ff45826e89..3e314fef55 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 @@ -4,7 +4,6 @@ import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -84,7 +83,7 @@ private List getServiceKeysByMetadataInternal(String labe String uriSuffix = INCLUDE_SERVICE_INSTANCE_RESOURCES_PARAM + "&service_instance_guids=" + String.join(",", guids); - return getListOfResources(new ServiceKeysResponseMapper(null), + return getListOfResources(new ServiceKeysResponseMapper(), SERVICE_KEYS_BY_METADATA_SELECTOR_URI + uriSuffix, labelSelector); } @@ -105,23 +104,12 @@ private boolean serviceIsNotUserProvided(DeployedMtaService service) { } protected class ServiceKeysResponseMapper extends ResourcesResponseMapper { - - List mtaServices; - - public ServiceKeysResponseMapper(List mtaServices) { - this.mtaServices = mtaServices; + public ServiceKeysResponseMapper() { } @Override public List getMappedResources() { - Map serviceMapping; - if (mtaServices != null) { - serviceMapping = mtaServices.stream() - .collect(Collectors.toMap(service -> service.getGuid() - .toString(), Function.identity())); - } else { - serviceMapping = getIncludedServiceInstancesMapping(); - } + Map serviceMapping = getIncludedServiceInstancesMapping(); return getQueriedResources().stream() .map(resource -> resourceMapper.mapServiceKeyResource(resource, serviceMapping)) .collect(Collectors.toList()); 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 75c29b4619..50641076be 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,9 +65,17 @@ 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; } @@ -98,9 +106,6 @@ 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); From 33c58bcc09ea2de12529e67cab0950d8143dba7a Mon Sep 17 00:00:00 2001 From: stiv03 Date: Wed, 22 Oct 2025 11:09:38 +0300 Subject: [PATCH 3/7] refactoring --- .../cf/clients/CustomServiceKeysClient.java | 37 ++++++++----------- .../process/steps/DetectDeployedMtaStep.java | 6 +-- 2 files changed, 18 insertions(+), 25 deletions(-) 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 3e314fef55..9a63734648 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 @@ -60,26 +60,22 @@ public List getServiceKeysByMetadataAndGuids(String space } private List extractManagedServiceGuids(List services) { - if (services == null || services.isEmpty()) { - return List.of(); - } - List managed = getManagedServices(services); - if (managed == null || managed.isEmpty()) { - return List.of(); + if (services != null) { + List managed = getManagedServices(services); + if (!managed.isEmpty()) { + return managed.stream() + .map(s -> s.getGuid() != null ? s.getGuid() + .toString() : null) + .filter(Objects::nonNull) + .distinct() + .toList(); + } } - return managed.stream() - .map(s -> s.getGuid() != null ? s.getGuid() - .toString() : null) - .filter(Objects::nonNull) - .distinct() - .toList(); + return List.of(); } private List getServiceKeysByMetadataInternal(String labelSelector, List guids) { - if (guids == null || guids.isEmpty()) { - return List.of(); - } String uriSuffix = INCLUDE_SERVICE_INSTANCE_RESOURCES_PARAM + "&service_instance_guids=" + String.join(",", guids); @@ -89,13 +85,12 @@ private List getServiceKeysByMetadataInternal(String labe } private List getManagedServices(List services) { - if (services == null) { - return null; + if (services != null) { + return services.stream() + .filter(this::serviceIsNotUserProvided) + .collect(Collectors.toList()); } - List managedServices = services.stream() - .filter(this::serviceIsNotUserProvided) - .collect(Collectors.toList()); - return managedServices.isEmpty() ? null : managedServices; + return List.of(); } private boolean serviceIsNotUserProvided(DeployedMtaService service) { 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 8c4d99c148..659df29496 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 @@ -126,10 +126,8 @@ private List getExistingServiceGuids(ProcessContext context) { private String resolveServiceGuid(CloudControllerClient client, String serviceName) { CloudServiceInstance instance = client.getServiceInstance(serviceName, false); - return instance != null && instance.getMetadata() != null - ? instance.getMetadata() - .getGuid() - .toString() : null; + return instance != null ? instance.getGuid() + .toString() : null; } private List getExistingServiceNamesFromDescriptor(ProcessContext context) { From d4be72fa2d9d531352f44b47ce64c07711219041 Mon Sep 17 00:00:00 2001 From: stiv03 Date: Fri, 24 Oct 2025 14:25:34 +0300 Subject: [PATCH 4/7] fix comments --- .../cf/clients/CustomServiceKeysClient.java | 31 +++++--------- .../process/steps/DetectDeployedMtaStep.java | 40 +++++++++++++------ 2 files changed, 37 insertions(+), 34 deletions(-) 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 9a63734648..7592275106 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 @@ -4,6 +4,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.UUID; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -49,29 +50,20 @@ public List getServiceKeysByMetadataAndGuids(String space List allServiceGuids = Stream.concat(managedGuids.stream(), existingServiceGuids.stream()) .filter(Objects::nonNull) - .distinct() .toList(); if (allServiceGuids.isEmpty()) { return List.of(); } - - return getServiceKeysByMetadataInternal(labelSelector, allServiceGuids); + return new CustomControllerClientErrorHandler().handleErrorsOrReturnResult( + () -> getServiceKeysByMetadataInternal(labelSelector, allServiceGuids)); } private List extractManagedServiceGuids(List services) { - if (services != null) { - List managed = getManagedServices(services); - if (!managed.isEmpty()) { - return managed.stream() - .map(s -> s.getGuid() != null ? s.getGuid() - .toString() : null) - .filter(Objects::nonNull) - .distinct() - .toList(); - } - } - return List.of(); + return getManagedServices(services).stream() + .map(DeployedMtaService::getGuid) + .map(UUID::toString) + .toList(); } private List getServiceKeysByMetadataInternal(String labelSelector, List guids) { @@ -85,12 +77,9 @@ private List getServiceKeysByMetadataInternal(String labe } private List getManagedServices(List services) { - if (services != null) { - return services.stream() - .filter(this::serviceIsNotUserProvided) - .collect(Collectors.toList()); - } - return List.of(); + return services.stream() + .filter(this::serviceIsNotUserProvided) + .collect(Collectors.toList()); } private boolean serviceIsNotUserProvided(DeployedMtaService service) { 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 659df29496..8ef1ce898b 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 @@ -2,6 +2,7 @@ import java.text.MessageFormat; import java.util.List; +import java.util.Objects; import java.util.Optional; import jakarta.inject.Inject; @@ -9,6 +10,7 @@ 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; @@ -99,7 +101,9 @@ private void detectBackupMta(String mtaId, String mtaNamespace, CloudControllerC private List detectDeployedServiceKeys(String mtaId, String mtaNamespace, DeployedMta deployedMta, ProcessContext context) { - List deployedManagedMtaServices = deployedMta == null ? null : deployedMta.getServices(); + List deployedManagedMtaServices = Optional.ofNullable(deployedMta) + .map(DeployedMta::getServices) + .orElse(List.of()); String spaceGuid = context.getVariable(Variables.SPACE_GUID); String user = context.getVariable(Variables.USER); String userGuid = context.getVariable(Variables.USER_GUID); @@ -117,30 +121,40 @@ private List detectDeployedServiceKeys(String mtaId, Stri private List getExistingServiceGuids(ProcessContext context) { CloudControllerClient client = context.getControllerClient(); - List existingNames = getExistingServiceNamesFromDescriptor(context); + List resources = getExistingServiceResourcesFromDescriptor(context); - return existingNames.stream() - .map(name -> resolveServiceGuid(client, name)) - .toList(); + return resources.stream() + .map(resource -> resolveServiceGuid(client, resource)) + .filter(Objects::nonNull) + .toList(); } - private String resolveServiceGuid(CloudControllerClient client, String serviceName) { - CloudServiceInstance instance = client.getServiceInstance(serviceName, false); - return instance != null ? instance.getGuid() - .toString() : null; + private String resolveServiceGuid(CloudControllerClient client, Resource resource) { + try { + CloudServiceInstance instance = client.getServiceInstance(resource.getName()); + return instance.getGuid() + .toString(); + } catch (CloudOperationException e) { + if (isOptionalOrInactive(resource)) { + return null; + } + throw e; + } + } + + private boolean isOptionalOrInactive(Resource resource) { + return Boolean.TRUE.equals(resource.isOptional()) || Boolean.FALSE.equals(resource.isActive()); } - private List getExistingServiceNamesFromDescriptor(ProcessContext context) { + private List getExistingServiceResourcesFromDescriptor(ProcessContext context) { DeploymentDescriptor descriptor = context.getVariable(Variables.DEPLOYMENT_DESCRIPTOR); - if (descriptor == null || descriptor.getResources() == null) { + if (descriptor == null) { return List.of(); } return descriptor.getResources() .stream() .filter(resource -> CloudModelBuilderUtil.getResourceType(resource) == ResourceType.EXISTING_SERVICE) - .map(Resource::getName) - .distinct() .toList(); } From a09e9d4a730dc7cef11466a7a2d4bdd866b64167 Mon Sep 17 00:00:00 2001 From: stiv03 Date: Fri, 24 Oct 2025 15:14:36 +0300 Subject: [PATCH 5/7] refactoring --- .../controller/core/cf/clients/CustomServiceKeysClient.java | 2 +- .../cloudfoundry/multiapps/controller/process/Messages.java | 1 + .../controller/process/steps/DetectDeployedMtaStep.java | 3 ++- 3 files changed, 4 insertions(+), 2 deletions(-) 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 7592275106..6a51e401e6 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 @@ -79,7 +79,7 @@ private List getServiceKeysByMetadataInternal(String labe private List getManagedServices(List services) { return services.stream() .filter(this::serviceIsNotUserProvided) - .collect(Collectors.toList()); + .toList(); } private boolean serviceIsNotUserProvided(DeployedMtaService service) { 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 a2f1261b98..7597d33d0d 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,6 +789,7 @@ 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_OR_INACTIVE_SERVICE = "Service {0} not found but is optional or 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/DetectDeployedMtaStep.java b/multiapps-controller-process/src/main/java/org/cloudfoundry/multiapps/controller/process/steps/DetectDeployedMtaStep.java index 8ef1ce898b..81b348242e 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 @@ -136,6 +136,7 @@ private String resolveServiceGuid(CloudControllerClient client, Resource resourc .toString(); } catch (CloudOperationException e) { if (isOptionalOrInactive(resource)) { + getStepLogger().debug(Messages.IGNORING_NOT_FOUND_OPTIONAL_OR_INACTIVE_SERVICE, resource.getName()); return null; } throw e; @@ -143,7 +144,7 @@ private String resolveServiceGuid(CloudControllerClient client, Resource resourc } private boolean isOptionalOrInactive(Resource resource) { - return Boolean.TRUE.equals(resource.isOptional()) || Boolean.FALSE.equals(resource.isActive()); + return resource.isOptional() || !resource.isActive(); } private List getExistingServiceResourcesFromDescriptor(ProcessContext context) { From d4808ce1d81986b3039eab7cbca40f41464989fa Mon Sep 17 00:00:00 2001 From: stiv03 Date: Mon, 27 Oct 2025 15:26:50 +0200 Subject: [PATCH 6/7] fix comments --- .../controller/process/Messages.java | 3 +- .../process/steps/DetectDeployedMtaStep.java | 28 ++++++++++++------- 2 files changed, 20 insertions(+), 11 deletions(-) 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 7597d33d0d..e2c264a8d0 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,7 +789,8 @@ 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_OR_INACTIVE_SERVICE = "Service {0} not found but is optional or inactive"; + 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/DetectDeployedMtaStep.java b/multiapps-controller-process/src/main/java/org/cloudfoundry/multiapps/controller/process/steps/DetectDeployedMtaStep.java index 81b348242e..42cb88288b 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 @@ -2,7 +2,6 @@ import java.text.MessageFormat; import java.util.List; -import java.util.Objects; import java.util.Optional; import jakarta.inject.Inject; @@ -29,6 +28,8 @@ 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; @@ -45,6 +46,8 @@ 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); @@ -125,26 +128,31 @@ private List getExistingServiceGuids(ProcessContext context) { return resources.stream() .map(resource -> resolveServiceGuid(client, resource)) - .filter(Objects::nonNull) + .flatMap(Optional::stream) .toList(); } - private String resolveServiceGuid(CloudControllerClient client, Resource resource) { + private Optional resolveServiceGuid(CloudControllerClient client, Resource resource) { try { CloudServiceInstance instance = client.getServiceInstance(resource.getName()); - return instance.getGuid() - .toString(); + return Optional.of(instance.getGuid() + .toString()); } catch (CloudOperationException e) { - if (isOptionalOrInactive(resource)) { - getStepLogger().debug(Messages.IGNORING_NOT_FOUND_OPTIONAL_OR_INACTIVE_SERVICE, resource.getName()); - return null; + if (resource.isOptional()) { + logIgnoredService(Messages.IGNORING_NOT_FOUND_OPTIONAL_SERVICE, resource.getName()); + return Optional.empty(); + } + if (!resource.isActive()) { + logIgnoredService(Messages.IGNORING_NOT_FOUND_INACTIVE_SERVICE, resource.getName()); + return Optional.empty(); } throw e; } } - private boolean isOptionalOrInactive(Resource resource) { - return resource.isOptional() || !resource.isActive(); + private void logIgnoredService(String message, String serviceName) { + getStepLogger().debug(message, serviceName); + LOGGER.error(message, serviceName); } private List getExistingServiceResourcesFromDescriptor(ProcessContext context) { From 330db686faea47970a424942562c0ccafa2c8155 Mon Sep 17 00:00:00 2001 From: stiv03 Date: Tue, 28 Oct 2025 10:57:45 +0200 Subject: [PATCH 7/7] log the error --- .../process/steps/DetectDeployedMtaStep.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) 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 42cb88288b..2125e00c23 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 @@ -139,20 +139,21 @@ private Optional resolveServiceGuid(CloudControllerClient client, Resour .toString()); } catch (CloudOperationException e) { if (resource.isOptional()) { - logIgnoredService(Messages.IGNORING_NOT_FOUND_OPTIONAL_SERVICE, resource.getName()); + 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()); + logIgnoredService(Messages.IGNORING_NOT_FOUND_INACTIVE_SERVICE, resource.getName(), e); return Optional.empty(); } throw e; } } - private void logIgnoredService(String message, String serviceName) { - getStepLogger().debug(message, serviceName); - LOGGER.error(message, serviceName); + 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) {