Skip to content

Fix service keys not renewed when existing service is used #1724

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Yavor16 merged 7 commits into from
Oct 30, 2025
Merged

Fix service keys not renewed when existing service is used #1724

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5dfb5b2
Fix service keys not renewed when existing service is used
stiv03 Oct 15, 2025
db592ac
fix when no deployedMta
stiv03 Oct 21, 2025
33c58bc
refactoring
stiv03 Oct 22, 2025
d4be72f
fix comments
stiv03 Oct 24, 2025
a09e9d4
refactoring
stiv03 Oct 24, 2025
d4808ce
fix comments
stiv03 Oct 27, 2025
330db68
log the error
stiv03 Oct 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 34 additions & 33 deletions ...n/java/org/cloudfoundry/multiapps/controller/core/cf/clients/CustomServiceKeysClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.function.Function;
import java.util.Objects;
import java.util.UUID;
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;
Expand All @@ -30,7 +32,8 @@ public CustomServiceKeysClient(ApplicationConfiguration configuration, WebClient
}

public List<DeployedMtaServiceKey> getServiceKeysByMetadataAndGuids(String spaceGuid, String mtaId, String mtaNamespace,
List<DeployedMtaService> services) {
List<DeployedMtaService> services,
List<String> existingServiceGuids) {
String labelSelector = MtaMetadataCriteriaBuilder.builder()
.label(MtaMetadataLabels.SPACE_GUID)
.hasValue(spaceGuid)
Expand All @@ -43,31 +46,40 @@ public List<DeployedMtaServiceKey> getServiceKeysByMetadataAndGuids(String space
.build()
.get();

List<String> managedGuids = extractManagedServiceGuids(services);

List<String> allServiceGuids = Stream.concat(managedGuids.stream(), existingServiceGuids.stream())
.filter(Objects::nonNull)
.toList();

if (allServiceGuids.isEmpty()) {
return List.of();
}
return new CustomControllerClientErrorHandler().handleErrorsOrReturnResult(
() -> getServiceKeysByMetadataInternal(labelSelector, services));
() -> getServiceKeysByMetadataInternal(labelSelector, allServiceGuids));
}

private List<DeployedMtaServiceKey> getServiceKeysByMetadataInternal(String labelSelector, List<DeployedMtaService> services) {
String uriSuffix = INCLUDE_SERVICE_INSTANCE_RESOURCES_PARAM;
List<DeployedMtaService> 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,
private List<String> extractManagedServiceGuids(List<DeployedMtaService> services) {
return getManagedServices(services).stream()
.map(DeployedMtaService::getGuid)
.map(UUID::toString)
.toList();
}

private List<DeployedMtaServiceKey> getServiceKeysByMetadataInternal(String labelSelector, List<String> 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,
labelSelector);
}

private List<DeployedMtaService> getManagedServices(List<DeployedMtaService> services) {
if (services == null) {
return null;
}
List<DeployedMtaService> managedServices = services.stream()
.filter(this::serviceIsNotUserProvided)
.collect(Collectors.toList());
return managedServices.isEmpty() ? null : managedServices;
return services.stream()
.filter(this::serviceIsNotUserProvided)
.toList();
}

private boolean serviceIsNotUserProvided(DeployedMtaService service) {
Expand All @@ -76,23 +88,12 @@ private boolean serviceIsNotUserProvided(DeployedMtaService service) {
}

protected class ServiceKeysResponseMapper extends ResourcesResponseMapper<DeployedMtaServiceKey> {

List<DeployedMtaService> mtaServices;

public ServiceKeysResponseMapper(List<DeployedMtaService> mtaServices) {
this.mtaServices = mtaServices;
public ServiceKeysResponseMapper() {
}

@Override
public List<DeployedMtaServiceKey> getMappedResources() {
Map<String, CloudServiceInstance> serviceMapping;
if (mtaServices != null) {
serviceMapping = mtaServices.stream()
.collect(Collectors.toMap(service -> service.getGuid()
.toString(), Function.identity()));
} else {
serviceMapping = getIncludedServiceInstancesMapping();
}
Map<String, CloudServiceInstance> serviceMapping = getIncludedServiceInstancesMapping();
return getQueriedResources().stream()
.map(resource -> resourceMapper.mapServiceKeyResource(resource, serviceMapping))
.collect(Collectors.toList());
Expand Down
2 changes: 2 additions & 0 deletions ...troller-process/src/main/java/org/cloudfoundry/multiapps/controller/process/Messages.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -789,6 +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_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}";
Expand Down
11 changes: 8 additions & 3 deletions ...java/org/cloudfoundry/multiapps/controller/process/steps/BuildCloudUndeployModelStep.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,17 @@ protected StepPhase executeStep(ProcessContext context) {
getStepLogger().debug(Messages.BUILDING_CLOUD_UNDEPLOY_MODEL);
DeployedMta deployedMta = context.getVariable(Variables.DEPLOYED_MTA);

List<DeployedMtaServiceKey> 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;
}

Expand Down Expand Up @@ -98,9 +106,6 @@ protected StepPhase executeStep(ProcessContext context) {
servicesForApplications, serviceNames);
getStepLogger().debug(Messages.SERVICES_TO_DELETE, servicesToDelete);

List<DeployedMtaServiceKey> serviceKeysToDelete = computeServiceKeysToDelete(context);
getStepLogger().debug(Messages.SERVICE_KEYS_FOR_DELETION, serviceKeysToDelete);

List<CloudApplication> appsToUndeploy = computeAppsToUndeploy(deployedAppsToUndeploy, context.getControllerClient());

DeployedMta backupMta = context.getVariable(Variables.BACKUP_MTA);
Expand Down
67 changes: 65 additions & 2 deletions .../main/java/org/cloudfoundry/multiapps/controller/process/steps/DetectDeployedMtaStep.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,27 @@
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;
Expand All @@ -38,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);
Expand Down Expand Up @@ -94,15 +104,68 @@ private void detectBackupMta(String mtaId, String mtaNamespace, CloudControllerC

private List<DeployedMtaServiceKey> detectDeployedServiceKeys(String mtaId, String mtaNamespace, DeployedMta deployedMta,
ProcessContext context) {
List<DeployedMtaService> deployedMtaServices = deployedMta == null ? null : deployedMta.getServices();
List<DeployedMtaService> 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);
var token = tokenService.getToken(user, userGuid);
var creds = new CloudCredentials(token, true);

CustomServiceKeysClient serviceKeysClient = getCustomServiceKeysClient(creds, context.getVariable(Variables.CORRELATION_ID));
return serviceKeysClient.getServiceKeysByMetadataAndGuids(spaceGuid, mtaId, mtaNamespace, deployedMtaServices);

List<String> existingInstanceGuids = getExistingServiceGuids(context);

return serviceKeysClient.getServiceKeysByMetadataAndGuids(
spaceGuid, mtaId, mtaNamespace, deployedManagedMtaServices, existingInstanceGuids
);
}

private List<String> getExistingServiceGuids(ProcessContext context) {
CloudControllerClient client = context.getControllerClient();
List<Resource> resources = getExistingServiceResourcesFromDescriptor(context);

return resources.stream()
.map(resource -> resolveServiceGuid(client, resource))
.flatMap(Optional::stream)
.toList();
}

private Optional<String> resolveServiceGuid(CloudControllerClient client, Resource resource) {
try {
CloudServiceInstance instance = client.getServiceInstance(resource.getName());
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<Resource> 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();
}

private void logNoMtaDeployedDetected(String mtaId, String mtaNamespace) {
Expand Down
53 changes: 39 additions & 14 deletions ...t/java/org/cloudfoundry/multiapps/controller/process/steps/DetectDeployedMtaStepTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<DetectDeployedMtaStep> {
Expand Down Expand Up @@ -62,28 +68,47 @@ void testExecuteWithDeployedMta() {
.getKeys();
List<DeployedMta> 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));
}

@Test
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);

Expand Down Expand Up @@ -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));
Expand Down
Loading