From 06ab289d61138b953929495e53c3b435244cfd2c Mon Sep 17 00:00:00 2001 From: Ben Hale Date: Tue, 12 Aug 2025 14:26:34 -0700 Subject: [PATCH] Per-request Organization Configuration Previously whenever a request was made to deploy a service, only the target (space) could be changed on a per-request basis. This change adds support for configuring the organization on a per-request basis as well. This change adds a new DeploymentProperty called 'organization' as a peer to 'target'. CloudFoundryOperationsUtils is updated to only have two ways to configure the CloudFoundryOperations, both of which encapsulation of setting both the org and space. The ability to change only the space is removed. The rest of the change is updating and simplifying the rest of the code to remove the branches were the space was optionally set in favor of always using the OperationsUtils passing in the properties where the optionality is centralized. Signed-off-by: Ben Hale --- .../cloudfoundry/CloudFoundryAppDeployer.java | 118 ++++++------------ .../CloudFoundryOperationsUtils.java | 35 +++--- .../CloudFoundryAppDeployerTests.java | 2 +- ...ndryAppDeployerUpdateApplicationTests.java | 2 +- .../CloudFoundryOperationsUtilsTests.java | 33 +++-- .../deployer/DeploymentProperties.java | 5 + 6 files changed, 82 insertions(+), 113 deletions(-) diff --git a/spring-cloud-app-broker-deployer-cloudfoundry/src/main/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryAppDeployer.java b/spring-cloud-app-broker-deployer-cloudfoundry/src/main/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryAppDeployer.java index 2ce0dc293..68f22b024 100644 --- a/spring-cloud-app-broker-deployer-cloudfoundry/src/main/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryAppDeployer.java +++ b/spring-cloud-app-broker-deployer-cloudfoundry/src/main/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryAppDeployer.java @@ -408,7 +408,13 @@ private Mono getSpaceId(Map properties) { else { space = this.targetProperties.getDefaultSpace(); } - return this.operations.spaces().get(GetSpaceRequest.builder().name(space).build()).map(SpaceDetail::getId); + + return this.operationsUtils + .getOperations(properties) + .flatMap(cfOperations -> cfOperations.spaces() + .get(GetSpaceRequest.builder().name(space).build()) + ) + .map(SpaceDetail::getId); } private String getDomainId(String domain, List domains) { @@ -669,16 +675,11 @@ private Mono pushApplication(DeployApplicationRequest request, Map requestPushApplication; - if (deploymentProperties.containsKey(DeploymentProperties.TARGET_PROPERTY_KEY)) { - String space = deploymentProperties.get(DeploymentProperties.TARGET_PROPERTY_KEY); - requestPushApplication = pushManifestInSpace(applicationManifestRequest, space); - } - else { - requestPushApplication = pushManifest(applicationManifestRequest); - } - - return requestPushApplication + return this.operationsUtils + .getOperations(deploymentProperties) + .flatMap(cfOperations -> cfOperations.applications() + .pushManifest(applicationManifestRequest) + ) .doOnSuccess((v) -> LOG.info("Success pushing app manifest. appName={}", request.getName())) .doOnError((e) -> LOG.error(String.format("Error pushing app manifest. appName=%s, " + ERROR_LOG_TEMPLATE, request.getName(), e.getMessage()), e)); @@ -731,15 +732,6 @@ private ApplicationManifest buildAppManifest(DeployApplicationRequest request, return manifest.build(); } - private Mono pushManifest(PushApplicationManifestRequest request) { - return this.operations.applications().pushManifest(request); - } - - private Mono pushManifestInSpace(PushApplicationManifestRequest request, String spaceName) { - return createSpace(spaceName).then(this.operationsUtils.getOperationsForSpace(spaceName)) - .flatMap((cfOperations) -> cfOperations.applications().pushManifest(request)); - } - private Mono createSpace(String spaceName) { return getSpaceId(spaceName).switchIfEmpty(Mono.justOrEmpty(this.targetProperties.getDefaultOrg()) .flatMap((orgName) -> getOrganizationId(orgName).flatMap((orgId) -> this.client.spaces() @@ -803,42 +795,18 @@ public Mono undeploy(UndeployApplicationRequest req String appName = request.getName(); Map deploymentProperties = request.getProperties(); - Mono requestDeleteApplication; - if (deploymentProperties.containsKey(DeploymentProperties.TARGET_PROPERTY_KEY)) { - String space = deploymentProperties.get(DeploymentProperties.TARGET_PROPERTY_KEY); - requestDeleteApplication = deleteApplicationInSpace(appName, space); - } - else { - requestDeleteApplication = deleteApplication(appName); - } - - return requestDeleteApplication.timeout(Duration.ofSeconds(this.defaultDeploymentProperties.getApiTimeout())) - .doOnSuccess((v) -> LOG.info("Success undeploying application. appName={}", appName)) - .doOnError(logError(String.format("Error undeploying application. appName=%s", appName))) - .then(Mono.just(UndeployApplicationResponse.builder().name(appName).build())); - } - - private Mono deleteApplication(String name) { - return this.operations.applications() - .delete(DeleteApplicationRequest.builder() - .deleteRoutes(this.defaultDeploymentProperties.isDeleteRoutes()) - .name(name) - .build()); - } - - private Mono deleteApplicationInSpace(String name, String spaceName) { - return getSpaceId(spaceName) - .doOnError((e) -> LOG.error(String.format("Unable to get space name. spaceName=%s, " + ERROR_LOG_TEMPLATE, - spaceName, e.getMessage()), e)) - .then(this.operationsUtils.getOperationsForSpace(spaceName)) - .flatMap((cfOperations) -> cfOperations.applications() + return this.operationsUtils + .getOperations(deploymentProperties) + .flatMap(cfOperations -> cfOperations.applications() .delete(DeleteApplicationRequest.builder() .deleteRoutes(this.defaultDeploymentProperties.isDeleteRoutes()) - .name(name) + .name(appName) .build()) - .doOnError((e) -> LOG.error(String - .format("Error deleting application. appName=%s, " + ERROR_LOG_TEMPLATE, name, e.getMessage()), e))) - .onErrorResume((e) -> Mono.empty()); + ) + .timeout(Duration.ofSeconds(this.defaultDeploymentProperties.getApiTimeout())) + .doOnSuccess((v) -> LOG.info("Success undeploying application. appName={}", appName)) + .doOnError(logError(String.format("Error undeploying application. appName=%s", appName))) + .then(Mono.just(UndeployApplicationResponse.builder().name(appName).build())); } @Override @@ -1120,11 +1088,13 @@ public Mono getServiceInstance(GetServiceInstanceReq } private Mono getServiceInstance(String name, SpaceEntity spaceEntity) { - return getOrganization(spaceEntity.getOrganizationId()).flatMap((organizationEntity) -> this.operationsUtils - .getOperationsForOrgAndSpace(organizationEntity.getName(), spaceEntity.getName()) + return getOrganization(spaceEntity.getOrganizationId()) + .flatMap((organizationEntity) -> this.operationsUtils + .getOperationsForOrgAndSpace(organizationEntity.getName(), spaceEntity.getName()) + ) .flatMap((cfOperations) -> cfOperations.services() - .getInstance( - org.cloudfoundry.operations.services.GetServiceInstanceRequest.builder().name(name).build()))); + .getInstance(org.cloudfoundry.operations.services.GetServiceInstanceRequest.builder().name(name).build()) + ); } private Mono getServiceInstance(String serviceInstanceId) { @@ -1161,18 +1131,12 @@ public Mono createServiceInstance(CreateServiceIn Mono createServiceInstanceResponseMono = Mono .just(CreateServiceInstanceResponse.builder().name(request.getServiceInstanceName()).build()); - if (request.getProperties().containsKey(DeploymentProperties.TARGET_PROPERTY_KEY)) { - return createSpace(request.getProperties().get(DeploymentProperties.TARGET_PROPERTY_KEY)) - .then(this.operationsUtils.getOperations(request.getProperties()) - .flatMap((cfOperations) -> cfOperations.services() - .createInstance(createServiceInstanceRequest) - .then(createServiceInstanceResponseMono))); - } - else { - return this.operations.services() + return this.operationsUtils + .getOperations(request.getProperties()) + .flatMap(cfOperations -> cfOperations.services() .createInstance(createServiceInstanceRequest) - .then(createServiceInstanceResponseMono); - } + ) + .then(createServiceInstanceResponseMono); } @Override @@ -1187,18 +1151,12 @@ public Mono deleteServiceInstance(DeleteServiceIn String serviceInstanceName = request.getServiceInstanceName(); Map deploymentProperties = request.getProperties(); - Mono requestDeleteServiceInstance; - if (deploymentProperties.containsKey(DeploymentProperties.TARGET_PROPERTY_KEY)) { - requestDeleteServiceInstance = this.operationsUtils.getOperations(deploymentProperties) - .flatMap((cfOperations) -> unbindServiceInstance(serviceInstanceName, cfOperations) - .then(deleteServiceInstance(serviceInstanceName, cfOperations, deploymentProperties))); - } - else { - requestDeleteServiceInstance = unbindServiceInstance(serviceInstanceName, this.operations) - .then(deleteServiceInstance(serviceInstanceName, this.operations, deploymentProperties)); - } - - return requestDeleteServiceInstance + return this.operationsUtils + .getOperations(request.getProperties()) + .flatMap(cfOperations -> + unbindServiceInstance(serviceInstanceName, cfOperations) + .then(deleteServiceInstance(serviceInstanceName, cfOperations, deploymentProperties)) + ) .doOnSuccess( (v) -> LOG.info("Success deleting service instance. serviceInstanceName={}", serviceInstanceName)) .doOnError(logError( diff --git a/spring-cloud-app-broker-deployer-cloudfoundry/src/main/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryOperationsUtils.java b/spring-cloud-app-broker-deployer-cloudfoundry/src/main/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryOperationsUtils.java index d47d98fab..64589431b 100644 --- a/spring-cloud-app-broker-deployer-cloudfoundry/src/main/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryOperationsUtils.java +++ b/spring-cloud-app-broker-deployer-cloudfoundry/src/main/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryOperationsUtils.java @@ -34,29 +34,28 @@ public CloudFoundryOperationsUtils(CloudFoundryOperations operations) { } protected Mono getOperations(Map properties) { - return Mono.defer(() -> { - if (!CollectionUtils.isEmpty(properties) - && properties.containsKey(DeploymentProperties.TARGET_PROPERTY_KEY)) { - return getOperationsForSpace(properties.get(DeploymentProperties.TARGET_PROPERTY_KEY)); + return Mono.fromSupplier(() -> { + DefaultCloudFoundryOperations.Builder builder = DefaultCloudFoundryOperations.builder() + .from((DefaultCloudFoundryOperations) this.operations); + + if (!CollectionUtils.isEmpty(properties) && properties.containsKey(DeploymentProperties.ORGANIZATION_PROPERTY_KEY)) { + builder.organization(properties.get(DeploymentProperties.ORGANIZATION_PROPERTY_KEY)); } - return Mono.just(this.operations); - }); - } - protected Mono getOperationsForSpace(String space) { - return Mono.just(this.operations) - .cast(DefaultCloudFoundryOperations.class) - .map((cfOperations) -> DefaultCloudFoundryOperations.builder().from(cfOperations).space(space).build()); + if (!CollectionUtils.isEmpty(properties) && properties.containsKey(DeploymentProperties.TARGET_PROPERTY_KEY)) { + builder.space(properties.get(DeploymentProperties.TARGET_PROPERTY_KEY)); + } + + return builder.build(); + }); } protected Mono getOperationsForOrgAndSpace(String organization, String space) { - return Mono.just(this.operations) - .cast(DefaultCloudFoundryOperations.class) - .map((cfOperations) -> DefaultCloudFoundryOperations.builder() - .from(cfOperations) - .organization(organization) - .space(space) - .build()); + return Mono.fromSupplier(() -> DefaultCloudFoundryOperations.builder() + .from((DefaultCloudFoundryOperations) this.operations) + .organization(organization) + .space(space) + .build()); } } diff --git a/spring-cloud-app-broker-deployer-cloudfoundry/src/test/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryAppDeployerTests.java b/spring-cloud-app-broker-deployer-cloudfoundry/src/test/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryAppDeployerTests.java index 409788e7b..4b56cb019 100644 --- a/spring-cloud-app-broker-deployer-cloudfoundry/src/test/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryAppDeployerTests.java +++ b/spring-cloud-app-broker-deployer-cloudfoundry/src/test/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryAppDeployerTests.java @@ -163,7 +163,7 @@ void setUp() { given(this.cloudFoundryClient.organizations()).willReturn(this.clientOrganizations); given(this.cloudFoundryClient.applicationsV2()).willReturn(this.clientApplications); given(this.operationsUtils.getOperations(anyMap())).willReturn(Mono.just(this.cloudFoundryOperations)); - given(this.operationsUtils.getOperationsForSpace(anyString())) + given(this.operationsUtils.getOperationsForOrgAndSpace(anyString(), anyString())) .willReturn(Mono.just(this.cloudFoundryOperations)); given(this.operationsUtils.getOperationsForOrgAndSpace(anyString(), anyString())) .willReturn(Mono.just(this.cloudFoundryOperations)); diff --git a/spring-cloud-app-broker-deployer-cloudfoundry/src/test/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryAppDeployerUpdateApplicationTests.java b/spring-cloud-app-broker-deployer-cloudfoundry/src/test/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryAppDeployerUpdateApplicationTests.java index 034a8d673..b71ecbc0f 100644 --- a/spring-cloud-app-broker-deployer-cloudfoundry/src/test/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryAppDeployerUpdateApplicationTests.java +++ b/spring-cloud-app-broker-deployer-cloudfoundry/src/test/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryAppDeployerUpdateApplicationTests.java @@ -173,7 +173,7 @@ void setUp() { given(this.cloudFoundryClient.deploymentsV3()).willReturn(this.deploymentsV3); given(this.cloudFoundryClient.routes()).willReturn(this.routes); given(this.operationsUtils.getOperations(anyMap())).willReturn(Mono.just(this.cloudFoundryOperations)); - given(this.operationsUtils.getOperationsForSpace(anyString())) + given(this.operationsUtils.getOperationsForOrgAndSpace(anyString(), anyString())) .willReturn(Mono.just(this.cloudFoundryOperations)); this.appDeployer = new CloudFoundryAppDeployer(deploymentProperties, this.cloudFoundryOperations, diff --git a/spring-cloud-app-broker-deployer-cloudfoundry/src/test/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryOperationsUtilsTests.java b/spring-cloud-app-broker-deployer-cloudfoundry/src/test/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryOperationsUtilsTests.java index b04c66e50..7c27ca70b 100644 --- a/spring-cloud-app-broker-deployer-cloudfoundry/src/test/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryOperationsUtilsTests.java +++ b/spring-cloud-app-broker-deployer-cloudfoundry/src/test/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryOperationsUtilsTests.java @@ -17,6 +17,8 @@ package org.springframework.cloud.appbroker.deployer.cloudfoundry; import java.util.Collections; +import java.util.HashMap; +import java.util.Map; import org.cloudfoundry.operations.CloudFoundryOperations; import org.cloudfoundry.operations.DefaultCloudFoundryOperations; @@ -43,29 +45,34 @@ void setUp() { @Test void getOperationsWithEmptyProperties() { - StepVerifier.create(this.operationsUtils.getOperations(Collections.emptyMap())) - .expectNext(this.operations) + StepVerifier + .create(this.operationsUtils.getOperations(Collections.emptyMap())) + .assertNext((ops) -> { + String organization = (String) ReflectionTestUtils.getField(ops, "organization"); + assertThat(organization).isNull(); + + String space = (String) ReflectionTestUtils.getField(ops, "space"); + assertThat(space).isNull(); + }) .verifyComplete(); } @Test void getOperationsWithProperties() { + Map properties = new HashMap<>(); + properties.put(DeploymentProperties.ORGANIZATION_PROPERTY_KEY, "foo-organization-1"); + properties.put(DeploymentProperties.TARGET_PROPERTY_KEY, "foo-space-1"); + StepVerifier - .create(this.operationsUtils - .getOperations(Collections.singletonMap(DeploymentProperties.TARGET_PROPERTY_KEY, "foo-space1"))) + .create(this.operationsUtils.getOperations(properties)) .assertNext((ops) -> { + String organization = (String) ReflectionTestUtils.getField(ops, "organization"); + assertThat(organization).isEqualTo("foo-organization-1"); + String space = (String) ReflectionTestUtils.getField(ops, "space"); - assertThat(space).isEqualTo("foo-space1"); + assertThat(space).isEqualTo("foo-space-1"); }) .verifyComplete(); } - @Test - void getOperationsForSpace() { - StepVerifier.create(this.operationsUtils.getOperationsForSpace("foo-space2")).assertNext((ops) -> { - String space = (String) ReflectionTestUtils.getField(ops, "space"); - assertThat(space).isEqualTo("foo-space2"); - }).verifyComplete(); - } - } diff --git a/spring-cloud-app-broker-deployer/src/main/java/org/springframework/cloud/appbroker/deployer/deployer/DeploymentProperties.java b/spring-cloud-app-broker-deployer/src/main/java/org/springframework/cloud/appbroker/deployer/deployer/DeploymentProperties.java index 561993f70..62e10788d 100644 --- a/spring-cloud-app-broker-deployer/src/main/java/org/springframework/cloud/appbroker/deployer/deployer/DeploymentProperties.java +++ b/spring-cloud-app-broker-deployer/src/main/java/org/springframework/cloud/appbroker/deployer/deployer/DeploymentProperties.java @@ -65,6 +65,11 @@ public class DeploymentProperties { */ public static final String TARGET_PROPERTY_KEY = "target"; + /** + * The deployment property for the organization where the app will be deployed. + */ + public static final String ORGANIZATION_PROPERTY_KEY = "organization"; + /** * The deployment property indicating whether the application should be automatically * started after deployment. Defaults to true.