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 f72643e..8fb65d2 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 @@ -19,7 +19,6 @@ package org.springframework.cloud.appbroker.deployer.cloudfoundry; import java.io.IOException; import java.nio.file.Path; import java.time.Duration; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -152,7 +151,7 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware ApplicationManifest.Builder manifest = ApplicationManifest.builder() .name(request.getName()) .path(getApplication(appResource)) - .environmentVariables(getEnvironmentVariables(request.getEnvironment())) + .environmentVariables(getEnvironmentVariables(deploymentProperties, request.getEnvironment())) .services(request.getServices()) .instances(instances(deploymentProperties)) .memory(memory(deploymentProperties)) @@ -265,17 +264,13 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware Mono.just(this.targetProperties.getUsername())); } - private Map getEnvironmentVariables(Map environment) { - Map envVariables = new HashMap<>(getApplicationEnvironment(environment)); + private Map getEnvironmentVariables(Map properties, + Map environment) { + Map envVariables = getApplicationEnvironment(properties, environment); - String javaOpts = javaOpts(environment); + String javaOpts = javaOpts(properties); if (StringUtils.hasText(javaOpts)) { - envVariables.put("JAVA_OPTS", javaOpts(environment)); - } - - String group = environment.get(DeploymentProperties.GROUP_PROPERTY_KEY); - if (StringUtils.hasText(group)) { - envVariables.put("SPRING_CLOUD_APPLICATION_GROUP", group); + envVariables.put("JAVA_OPTS", javaOpts); } envVariables.put("SPRING_CLOUD_APPLICATION_GUID", "${vcap.application.name}:${vcap.application.instance_index}"); @@ -284,35 +279,37 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware return envVariables; } - private Map getApplicationEnvironment(Map environment) { + private Map getApplicationEnvironment(Map properties, + Map environment) { Map applicationEnvironment = getSanitizedApplicationEnvironment(environment); - if (!useSpringApplicationJson(environment)) { - return applicationEnvironment; + if (!applicationEnvironment.isEmpty() && useSpringApplicationJson(properties)) { + try { + String jsonEnvironment = OBJECT_MAPPER.writeValueAsString(applicationEnvironment); + applicationEnvironment = new HashMap<>(1); + applicationEnvironment.put("SPRING_APPLICATION_JSON", jsonEnvironment); + } catch (JsonProcessingException e) { + throw new IllegalArgumentException("Error writing environment to SPRING_APPLICATION_JSON", e); + } } - try { - return Collections.singletonMap("SPRING_APPLICATION_JSON", - OBJECT_MAPPER.writeValueAsString(applicationEnvironment)); - } catch (JsonProcessingException e) { - throw new IllegalArgumentException("Error writing environment to SPRING_APPLICATION_JSON", e); - } + return applicationEnvironment; } private Map getSanitizedApplicationEnvironment(Map environment) { - Map applicationProperties = new HashMap<>(environment); + Map applicationEnvironment = new HashMap<>(environment); // Remove server.port as CF assigns a port for us, and we don't want to override that - Optional.ofNullable(applicationProperties.remove("server.port")) + Optional.ofNullable(applicationEnvironment.remove("server.port")) .ifPresent(port -> logger.warn("Ignoring 'server.port={}', " + "as Cloud Foundry will assign a local dynamic port. " + "Route to the app will use port 80.", port)); - return applicationProperties; + return applicationEnvironment; } - private boolean useSpringApplicationJson(Map environment) { - return Optional.ofNullable(environment.get(DeploymentProperties.USE_SPRING_APPLICATION_JSON_KEY)) + private boolean useSpringApplicationJson(Map properties) { + return Optional.ofNullable(properties.get(DeploymentProperties.USE_SPRING_APPLICATION_JSON_KEY)) .map(Boolean::valueOf) .orElse(this.defaultDeploymentProperties.isUseSpringApplicationJson()); } diff --git a/spring-cloud-app-broker-deployer-cloudfoundry/src/test/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryAppDeployerTest.java b/spring-cloud-app-broker-deployer-cloudfoundry/src/test/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryAppDeployerTest.java index be08cfe..b814098 100644 --- a/spring-cloud-app-broker-deployer-cloudfoundry/src/test/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryAppDeployerTest.java +++ b/spring-cloud-app-broker-deployer-cloudfoundry/src/test/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryAppDeployerTest.java @@ -217,11 +217,63 @@ class CloudFoundryAppDeployerTest { verify(applications).pushManifest(argThat(matchesManifest(expectedManifest))); } + @Test + void deployAppWithEnvironmentUsingSpringAppJson() { + deploymentProperties.setUseSpringApplicationJson(true); + + DeployApplicationRequest request = DeployApplicationRequest.builder() + .name(APP_NAME) + .path(APP_PATH) + .property(CloudFoundryDeploymentProperties.JAVA_OPTS_PROPERTY_KEY, "-Xms512m -Xmx1024m") + .environment("ENV_VAR_1", "value1") + .environment("ENV_VAR_2", "value2") + .build(); + + StepVerifier.create(appDeployer.deploy(request)) + .assertNext(response -> assertThat(response.getName()).isEqualTo(APP_NAME)) + .verifyComplete(); + + ApplicationManifest expectedManifest = baseManifest() + .name(APP_NAME) + .path(new File(APP_PATH).toPath()) + .environmentVariable("JAVA_OPTS", "-Xms512m -Xmx1024m") + .environmentVariable("SPRING_APPLICATION_JSON", "{\"ENV_VAR_2\":\"value2\",\"ENV_VAR_1\":\"value1\"}") + .build(); + + verify(applications).pushManifest(argThat(matchesManifest(expectedManifest))); + } + + @Test + void deployAppWithEnvironmentNotUsingSpringAppJson() { + deploymentProperties.setUseSpringApplicationJson(false); + + DeployApplicationRequest request = DeployApplicationRequest.builder() + .name(APP_NAME) + .path(APP_PATH) + .property(CloudFoundryDeploymentProperties.JAVA_OPTS_PROPERTY_KEY, "-Xms512m -Xmx1024m") + .environment("ENV_VAR_1", "value1") + .environment("ENV_VAR_2", "value2") + .build(); + + StepVerifier.create(appDeployer.deploy(request)) + .assertNext(response -> assertThat(response.getName()).isEqualTo(APP_NAME)) + .verifyComplete(); + + ApplicationManifest expectedManifest = baseManifest() + .name(APP_NAME) + .path(new File(APP_PATH).toPath()) + .environmentVariable("JAVA_OPTS", "-Xms512m -Xmx1024m") + .environmentVariable("ENV_VAR_1", "value1") + .environmentVariable("ENV_VAR_2", "value2") + .build(); + + verify(applications).pushManifest(argThat(matchesManifest(expectedManifest))); + } + private ApplicationManifest.Builder baseManifest() { return ApplicationManifest.builder() .environmentVariable("SPRING_APPLICATION_INDEX", "${vcap.application.instance_index}") .environmentVariable("SPRING_CLOUD_APPLICATION_GUID", "${vcap.application.name}:${vcap.application.instance_index}") - .environmentVariable("SPRING_APPLICATION_JSON", "{}") .services(new ArrayList<>()); } diff --git a/spring-cloud-app-broker-deployer/src/main/java/org/springframework/cloud/appbroker/deployer/DeploymentProperties.java b/spring-cloud-app-broker-deployer/src/main/java/org/springframework/cloud/appbroker/deployer/DeploymentProperties.java index d157867..9c79a10 100644 --- a/spring-cloud-app-broker-deployer/src/main/java/org/springframework/cloud/appbroker/deployer/DeploymentProperties.java +++ b/spring-cloud-app-broker-deployer/src/main/java/org/springframework/cloud/appbroker/deployer/DeploymentProperties.java @@ -23,12 +23,6 @@ public class DeploymentProperties { */ public static final String COUNT_PROPERTY_KEY = "count"; - /** - * The deployment property for the group to which an app belongs. - * If not provided, a deployer should assume no group. - */ - public static final String GROUP_PROPERTY_KEY = "group"; - /** * The deployment property for the memory setting for the container that will run the app. * The memory is specified in Mebibytes, diff --git a/spring-cloud-app-broker-sample/src/test/java/org.springframework.cloud.appbroker/sample/CreateInstanceWithEnvironmentComponentTest.java b/spring-cloud-app-broker-sample/src/test/java/org.springframework.cloud.appbroker/sample/CreateInstanceWithEnvironmentComponentTest.java index 892b965..0b1d362 100644 --- a/spring-cloud-app-broker-sample/src/test/java/org.springframework.cloud.appbroker/sample/CreateInstanceWithEnvironmentComponentTest.java +++ b/spring-cloud-app-broker-sample/src/test/java/org.springframework.cloud.appbroker/sample/CreateInstanceWithEnvironmentComponentTest.java @@ -29,19 +29,26 @@ import static io.restassured.RestAssured.given; import static org.assertj.core.api.Assertions.assertThat; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; -import static org.springframework.cloud.appbroker.sample.CreateInstanceWithEnvironmentComponentTest.APP_NAME; +import static org.springframework.cloud.appbroker.sample.CreateInstanceWithEnvironmentComponentTest.APP_NAME_1; +import static org.springframework.cloud.appbroker.sample.CreateInstanceWithEnvironmentComponentTest.APP_NAME_2; @TestPropertySource(properties = { "spring.cloud.appbroker.services[0].service-name=example", "spring.cloud.appbroker.services[0].plan-name=standard", "spring.cloud.appbroker.services[0].apps[0].path=classpath:demo.jar", - "spring.cloud.appbroker.services[0].apps[0].name=" + APP_NAME, + "spring.cloud.appbroker.services[0].apps[0].name=" + APP_NAME_1, "spring.cloud.appbroker.services[0].apps[0].environment.ENV_VAR_1=value1", "spring.cloud.appbroker.services[0].apps[0].environment.ENV_VAR_2=true", + "spring.cloud.appbroker.services[0].apps[1].path=classpath:demo.jar", + "spring.cloud.appbroker.services[0].apps[1].name=" + APP_NAME_2, + "spring.cloud.appbroker.services[0].apps[1].properties.use-spring-application-json=false", + "spring.cloud.appbroker.services[0].apps[1].environment.ENV_VAR_3=value3", + "spring.cloud.appbroker.services[0].apps[1].environment.ENV_VAR_4=true", }) class CreateInstanceWithEnvironmentComponentTest extends WiremockComponentTest { - static final String APP_NAME = "app-with-env"; + static final String APP_NAME_1 = "app-with-env1"; + static final String APP_NAME_2 = "app-with-env2"; @Autowired private OpenServiceBrokerApiFixture brokerFixture; @@ -51,11 +58,16 @@ class CreateInstanceWithEnvironmentComponentTest extends WiremockComponentTest { @Test void pushAppWithEnvironmentVariables() { - cloudControllerFixture.stubAppDoesNotExist(APP_NAME); - cloudControllerFixture.stubPushApp(APP_NAME, + cloudControllerFixture.stubAppDoesNotExist(APP_NAME_1); + cloudControllerFixture.stubPushApp(APP_NAME_1, matchingJsonPath("$.environment_json[?(@.SPRING_APPLICATION_JSON =~ /.*ENV_VAR_1.*:.*value1.*/)]"), matchingJsonPath("$.environment_json[?(@.SPRING_APPLICATION_JSON =~ /.*ENV_VAR_2.*:.*true.*/)]")); + cloudControllerFixture.stubAppDoesNotExist(APP_NAME_2); + cloudControllerFixture.stubPushApp(APP_NAME_2, + matchingJsonPath("$.environment_json[?(@.ENV_VAR_3 == 'value3')]"), + matchingJsonPath("$.environment_json[?(@.ENV_VAR_4 == 'true')]")); + // when a service instance is created given(brokerFixture.serviceInstanceRequest()) .when()