Skip to content

Revert existing services service key changes #1745

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
IvanBorislavovDimitrov merged 2 commits into from
Dec 3, 2025
Merged

Revert existing services service key changes #1745

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ee34c94
Revert " Fix existing service instance not found by resource name (#1…
IvanBorislavovDimitrov Dec 3, 2025
7384b9e
Revert "Fix service keys not renewed when existing service is used (#…
IvanBorislavovDimitrov Dec 3, 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: 33 additions & 34 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,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;
Expand All @@ -32,8 +30,7 @@ public CustomServiceKeysClient(ApplicationConfiguration configuration, WebClient
}

public List<DeployedMtaServiceKey> getServiceKeysByMetadataAndGuids(String spaceGuid, String mtaId, String mtaNamespace,
List<DeployedMtaService> services,
List<String> existingServiceGuids) {
List<DeployedMtaService> services) {
String labelSelector = MtaMetadataCriteriaBuilder.builder()
.label(MtaMetadataLabels.SPACE_GUID)
.hasValue(spaceGuid)
Expand All @@ -46,40 +43,31 @@ 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, allServiceGuids));
() -> getServiceKeysByMetadataInternal(labelSelector, services));
}

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,
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,
labelSelector);
}

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

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

protected class ServiceKeysResponseMapper extends ResourcesResponseMapper<DeployedMtaServiceKey> {
public ServiceKeysResponseMapper() {

List<DeployedMtaService> mtaServices;

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

@Override
public List<DeployedMtaServiceKey> getMappedResources() {
Map<String, CloudServiceInstance> serviceMapping = getIncludedServiceInstancesMapping();
Map<String, CloudServiceInstance> 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());
Expand Down
12 changes: 1 addition & 11 deletions ...ntroller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/util/NameUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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()) {
Expand Down Expand Up @@ -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}";
Expand Down
49 changes: 3 additions & 46 deletions ...ller-core/src/test/java/org/cloudfoundry/multiapps/controller/core/util/NameUtilTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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<Arguments> testGetNameWithProperLength() {
Expand Down Expand Up @@ -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<String, Object> params = new HashMap<>();
if (serviceInstanceName != null) {
params.put(SupportedParameters.SERVICE_NAME, serviceInstanceName);
}
resource.setParameters(params);

return resource;
}
}
2 changes: 0 additions & 2 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,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}";
Expand Down
11 changes: 3 additions & 8 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,17 +65,9 @@ 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 @@ -106,6 +98,9 @@ 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
69 changes: 2 additions & 67 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,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;
Expand All @@ -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);
Expand Down Expand Up @@ -104,69 +94,14 @@ private void detectBackupMta(String mtaId, String mtaNamespace, CloudControllerC

private List<DeployedMtaServiceKey> detectDeployedServiceKeys(String mtaId, String mtaNamespace, DeployedMta deployedMta,
ProcessContext context) {
List<DeployedMtaService> deployedManagedMtaServices = Optional.ofNullable(deployedMta)
.map(DeployedMta::getServices)
.orElse(List.of());
List<DeployedMtaService> 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<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) {
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<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();
return serviceKeysClient.getServiceKeysByMetadataAndGuids(spaceGuid, mtaId, mtaNamespace, deployedMtaServices);
}

private void logNoMtaDeployedDetected(String mtaId, String mtaNamespace) {
Expand Down
Loading
Loading