From 6c0ddfc000328283b83ecd563031b6cd386bcc1d Mon Sep 17 00:00:00 2001 From: Scott Frederick Date: Mon, 8 Oct 2018 11:38:52 -0500 Subject: [PATCH 1/4] Remove hard-coded defaults from CF deployment properties to allow platform defaults when no values are specified by the broker configuration. --- .../deployer/BackingApplication.java | 2 +- .../cloud/appbroker/deployer/TargetSpec.java | 4 +- .../cloudfoundry/CloudFoundryAppDeployer.java | 47 ++--- .../CloudFoundryDeploymentProperties.java | 20 +- .../CloudFoundryAppDeployerTest.java | 185 ++++++++++++++++-- .../deployer/DeployApplicationRequest.java | 3 - .../deployer/UndeployApplicationRequest.java | 2 +- .../deployer/util/ByteSizeUtils.java | 12 +- 8 files changed, 211 insertions(+), 64 deletions(-) diff --git a/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/deployer/BackingApplication.java b/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/deployer/BackingApplication.java index ccdf18a..807f43e 100644 --- a/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/deployer/BackingApplication.java +++ b/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/deployer/BackingApplication.java @@ -60,7 +60,7 @@ public class BackingApplication { private BackingApplication() { } - private BackingApplication(String name, String path, + BackingApplication(String name, String path, Map properties, Map environment, List services, diff --git a/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/deployer/TargetSpec.java b/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/deployer/TargetSpec.java index 5d2e2b8..5080b73 100644 --- a/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/deployer/TargetSpec.java +++ b/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/deployer/TargetSpec.java @@ -23,7 +23,7 @@ public class TargetSpec { private TargetSpec() { } - private TargetSpec(String name) { + TargetSpec(String name) { this.name = name; } @@ -44,7 +44,7 @@ public class TargetSpec { private String name; - private TargetSpecBuilder() { + TargetSpecBuilder() { } public TargetSpecBuilder name(String name) { 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 0e83cd0..6f7e1bd 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 @@ -151,12 +151,12 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware .path(getApplication(appResource)) .environmentVariables(getEnvironmentVariables(request.getEnvironment())) .services(request.getServices()) + .instances(instances(deploymentProperties)) + .memory(memory(deploymentProperties)) .disk(diskQuota(deploymentProperties)) .healthCheckType(healthCheck(deploymentProperties)) .healthCheckHttpEndpoint(healthCheckEndpoint(deploymentProperties)) .timeout(healthCheckTimeout(deploymentProperties)) - .instances(instances(deploymentProperties)) - .memory(memory(deploymentProperties)) .noRoute(toggleNoRoute(deploymentProperties)); Optional.ofNullable(host(deploymentProperties)).ifPresent(manifest::host); @@ -252,7 +252,7 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware return new DefaultApplications( ((DefaultCloudFoundryOperations) this.operations).getCloudFoundryClientPublisher(), ((DefaultCloudFoundryOperations) this.operations).getDopplerClientPublisher(), - (this.operations).spaces().get(GetSpaceRequest.builder().name(space).build()).map(SpaceDetail::getId)); + this.operations.spaces().get(GetSpaceRequest.builder().name(space).build()).map(SpaceDetail::getId)); } private DefaultSpaces createSpaceOperations() { @@ -321,8 +321,8 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware private ApplicationHealthCheck healthCheck(Map properties) { return Optional.ofNullable(properties.get(CloudFoundryDeploymentProperties.HEALTHCHECK_PROPERTY_KEY)) - .map(this::toApplicationHealthCheck) - .orElse(this.defaultDeploymentProperties.getHealthCheck()); + .map(this::toApplicationHealthCheck) + .orElse(this.defaultDeploymentProperties.getHealthCheck()); } private ApplicationHealthCheck toApplicationHealthCheck(String raw) { @@ -336,19 +336,19 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware private String healthCheckEndpoint(Map properties) { return Optional.ofNullable(properties.get(CloudFoundryDeploymentProperties.HEALTHCHECK_HTTP_ENDPOINT_PROPERTY_KEY)) - .orElse(this.defaultDeploymentProperties.getHealthCheckHttpEndpoint()); + .orElse(this.defaultDeploymentProperties.getHealthCheckHttpEndpoint()); } private Integer healthCheckTimeout(Map properties) { - String timeoutString = properties - .getOrDefault(CloudFoundryDeploymentProperties.HEALTHCHECK_TIMEOUT_PROPERTY_KEY, this.defaultDeploymentProperties.getHealthCheckTimeout()); - return Integer.parseInt(timeoutString); + return Optional.ofNullable(properties.get(CloudFoundryDeploymentProperties.HEALTHCHECK_TIMEOUT_PROPERTY_KEY)) + .map(Integer::parseInt) + .orElse(this.defaultDeploymentProperties.getHealthCheckTimeout()); } - private int instances(Map properties) { + private Integer instances(Map properties) { return Optional.ofNullable(properties.get(DeploymentProperties.COUNT_PROPERTY_KEY)) - .map(Integer::parseInt) - .orElse(this.defaultDeploymentProperties.getInstances()); + .map(Integer::parseInt) + .orElse(this.defaultDeploymentProperties.getInstances()); } private String host(Map properties) { @@ -382,30 +382,31 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware .orElse(null); } - private int memory(Map properties) { - String withUnit = properties - .getOrDefault(DeploymentProperties.MEMORY_PROPERTY_KEY, this.defaultDeploymentProperties.getMemory()); - return (int) ByteSizeUtils.parseToMebibytes(withUnit); + private Integer memory(Map properties) { + return Optional.ofNullable(properties.get(DeploymentProperties.MEMORY_PROPERTY_KEY)) + .map(ByteSizeUtils::parseToMebibytes) + .orElse(ByteSizeUtils.parseToMebibytes(defaultDeploymentProperties.getMemory())); } - private int diskQuota(Map properties) { - String withUnit = properties - .getOrDefault(DeploymentProperties.DISK_PROPERTY_KEY, this.defaultDeploymentProperties.getDisk()); - return (int) ByteSizeUtils.parseToMebibytes(withUnit); + private Integer diskQuota(Map properties) { + return Optional.ofNullable(properties.get(DeploymentProperties.DISK_PROPERTY_KEY)) + .map(ByteSizeUtils::parseToMebibytes) + .orElse(ByteSizeUtils.parseToMebibytes(defaultDeploymentProperties.getDisk())); } private String buildpack(Map properties) { return Optional.ofNullable(properties.get(CloudFoundryDeploymentProperties.BUILDPACK_PROPERTY_KEY)) - .orElse(this.defaultDeploymentProperties.getBuildpack()); + .orElse(this.defaultDeploymentProperties.getBuildpack()); } private String javaOpts(Map properties) { return Optional.ofNullable(properties.get(CloudFoundryDeploymentProperties.JAVA_OPTS_PROPERTY_KEY)) - .orElse(this.defaultDeploymentProperties.getJavaOpts()); + .orElse(this.defaultDeploymentProperties.getJavaOpts()); } private Predicate isNotFoundError() { - return t -> t instanceof AbstractCloudFoundryException && ((AbstractCloudFoundryException) t).getStatusCode() == HttpStatus.NOT_FOUND.value(); + return t -> t instanceof AbstractCloudFoundryException && + ((AbstractCloudFoundryException) t).getStatusCode() == HttpStatus.NOT_FOUND.value(); } /** diff --git a/spring-cloud-app-broker-deployer-cloudfoundry/src/main/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryDeploymentProperties.java b/spring-cloud-app-broker-deployer-cloudfoundry/src/main/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryDeploymentProperties.java index 538ac2c..770ee92 100644 --- a/spring-cloud-app-broker-deployer-cloudfoundry/src/main/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryDeploymentProperties.java +++ b/spring-cloud-app-broker-deployer-cloudfoundry/src/main/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryDeploymentProperties.java @@ -76,37 +76,37 @@ public class CloudFoundryDeploymentProperties { /** * The buildpack to use for deploying the application. */ - private String buildpack = "https://github.com/cloudfoundry/java-buildpack.git#v4.7.1"; + private String buildpack; /** * The amount of memory to allocate, if not overridden per-app. Default unit is mebibytes, 'M' and 'G" suffixes supported. */ - private String memory = "1024m"; + private String memory; /** * The amount of disk space to allocate, if not overridden per-app. Default unit is mebibytes, 'M' and 'G" suffixes supported. */ - private String disk = "1024m"; + private String disk; /** * The type of health check to perform on deployed application, if not overridden per-app. Defaults to PORT */ - private ApplicationHealthCheck healthCheck = ApplicationHealthCheck.PORT; + private ApplicationHealthCheck healthCheck; /** * The path that the http health check will use, defaults to @{code /health} */ - private String healthCheckHttpEndpoint = "/health"; + private String healthCheckHttpEndpoint; /** * The timeout value for health checks in seconds. Defaults to 120 seconds. */ - private String healthCheckTimeout = "120"; + private Integer healthCheckTimeout; /** * The number of instances to run. */ - private int instances = 1; + private Integer instances; /** * Flag to enable prefixing the app name with a random prefix. @@ -179,7 +179,7 @@ public class CloudFoundryDeploymentProperties { this.disk = disk; } - public int getInstances() { + public Integer getInstances() { return instances; } @@ -235,11 +235,11 @@ public class CloudFoundryDeploymentProperties { this.healthCheckHttpEndpoint = healthCheckHttpEndpoint; } - public String getHealthCheckTimeout() { + public Integer getHealthCheckTimeout() { return healthCheckTimeout; } - public void setHealthCheckTimeout(String healthCheckTimeout) { + public void setHealthCheckTimeout(Integer healthCheckTimeout) { this.healthCheckTimeout = healthCheckTimeout; } 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 607245e..4801e34 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 @@ -16,9 +16,11 @@ package org.springframework.cloud.appbroker.deployer.cloudfoundry; -import java.util.Collections; +import java.io.File; +import java.util.ArrayList; import org.cloudfoundry.operations.CloudFoundryOperations; +import org.cloudfoundry.operations.applications.ApplicationHealthCheck; import org.cloudfoundry.operations.applications.ApplicationManifest; import org.cloudfoundry.operations.applications.Applications; import org.cloudfoundry.operations.applications.PushApplicationManifestRequest; @@ -28,6 +30,7 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.ArgumentMatcher; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.cloud.appbroker.deployer.DeploymentProperties; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; @@ -45,8 +48,6 @@ import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) class CloudFoundryAppDeployerTest { - private static final int APP_MEMORY = 1024; - private static final String APP_MEMORY_STRING = "1024M"; private static final String APP_NAME = "test-app"; private static final String APP_PATH = "test.jar"; @@ -60,43 +61,185 @@ class CloudFoundryAppDeployerTest { @Mock private ResourceLoader resourceLoader; + + private CloudFoundryDeploymentProperties deploymentProperties; @BeforeEach void setUp() { - CloudFoundryDeploymentProperties deploymentProperties = new CloudFoundryDeploymentProperties(); - deploymentProperties.setDisk(APP_MEMORY_STRING); + deploymentProperties = new CloudFoundryDeploymentProperties(); - when(applications.pushManifest(any())).thenReturn(Mono.empty()); - when(cloudFoundryOperations.applications()).thenReturn(applications); - when(resourceLoader.getResource(APP_PATH)).thenReturn(new FileSystemResource(APP_PATH)); + when(applications.pushManifest(any())) + .thenReturn(Mono.empty()); + when(cloudFoundryOperations.applications()) + .thenReturn(applications); + when(resourceLoader.getResource(APP_PATH)) + .thenReturn(new FileSystemResource(APP_PATH)); appDeployer = new CloudFoundryAppDeployer( deploymentProperties, cloudFoundryOperations, resourceLoader); } @Test - void shouldDeployAppWithNameAndPath() { - StepVerifier.create(appDeployer.deploy(DeployApplicationRequest.builder() + void deployAppWithPlatformDefaults() { + DeployApplicationRequest request = DeployApplicationRequest.builder() .name(APP_NAME) - .properties(Collections.emptyMap()) .path(APP_PATH) - .build())) + .build(); + + StepVerifier.create(appDeployer.deploy(request)) .assertNext(response -> assertThat(response.getName()).isEqualTo(APP_NAME)) .verifyComplete(); - verify(applications).pushManifest(argThat(matchesExpectedManifest())); + ApplicationManifest expectedManifest = baseManifest() + .name(APP_NAME) + .path(new File(APP_PATH).toPath()) + .build(); + + verify(applications).pushManifest(argThat(matchesManifest(expectedManifest))); } - private ArgumentMatcher matchesExpectedManifest() { - return request -> { - if (request.getManifests().size() == 1) { - ApplicationManifest manifest = request.getManifests().get(0); - return APP_NAME.equals(manifest.getName()) - && APP_PATH.equals(manifest.getPath().toString()) - && manifest.getDisk() == APP_MEMORY; + @Test + void deployAppWithPropertiesInRequest() { + DeployApplicationRequest request = DeployApplicationRequest.builder() + .name(APP_NAME) + .path(APP_PATH) + .property(DeploymentProperties.COUNT_PROPERTY_KEY, "3") + .property(DeploymentProperties.MEMORY_PROPERTY_KEY, "2G") + .property(DeploymentProperties.DISK_PROPERTY_KEY, "3G") + .property(CloudFoundryDeploymentProperties.HEALTHCHECK_PROPERTY_KEY, "http") + .property(CloudFoundryDeploymentProperties.HEALTHCHECK_HTTP_ENDPOINT_PROPERTY_KEY, "/healthcheck") + .property(CloudFoundryDeploymentProperties.BUILDPACK_PROPERTY_KEY, "buildpack") + .property(CloudFoundryDeploymentProperties.DOMAIN_PROPERTY, "domain") + .property(CloudFoundryDeploymentProperties.HOST_PROPERTY, "host") + .property(CloudFoundryDeploymentProperties.NO_ROUTE_PROPERTY, "true") + .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()) + .instances(3) + .memory(2048) + .disk(3072) + .healthCheckType(ApplicationHealthCheck.HTTP) + .healthCheckHttpEndpoint("/healthcheck") + .buildpack("buildpack") + .domain("domain") + .host("host") + .noRoute(true) + .build(); + + verify(applications).pushManifest(argThat(matchesManifest(expectedManifest))); + } + + @Test + void deployAppWithDefaultProperties() { + deploymentProperties.setInstances(3); + deploymentProperties.setMemory("2G"); + deploymentProperties.setDisk("3G"); + deploymentProperties.setBuildpack("buildpack"); + deploymentProperties.setHealthCheck(ApplicationHealthCheck.HTTP); + deploymentProperties.setHealthCheckHttpEndpoint("/healthcheck"); + deploymentProperties.setDomain("domain"); + deploymentProperties.setHost("host"); + + DeployApplicationRequest request = DeployApplicationRequest.builder() + .name(APP_NAME) + .path(APP_PATH) + .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()) + .instances(3) + .memory(2048) + .disk(3072) + .healthCheckType(ApplicationHealthCheck.HTTP) + .healthCheckHttpEndpoint("/healthcheck") + .buildpack("buildpack") + .domain("domain") + .host("host") + .build(); + + verify(applications).pushManifest(argThat(matchesManifest(expectedManifest))); + } + + @Test + void deployAppWithRequestOverridingDefaultProperties() { + deploymentProperties.setInstances(3); + deploymentProperties.setMemory("2G"); + deploymentProperties.setDisk("3G"); + deploymentProperties.setBuildpack("buildpack1"); + deploymentProperties.setHealthCheck(ApplicationHealthCheck.HTTP); + deploymentProperties.setHealthCheckHttpEndpoint("/healthcheck1"); + deploymentProperties.setDomain("domain1"); + deploymentProperties.setHost("host1"); + + DeployApplicationRequest request = DeployApplicationRequest.builder() + .name(APP_NAME) + .path(APP_PATH) + .property(DeploymentProperties.COUNT_PROPERTY_KEY, "5") + .property(DeploymentProperties.MEMORY_PROPERTY_KEY, "4G") + .property(DeploymentProperties.DISK_PROPERTY_KEY, "5G") + .property(CloudFoundryDeploymentProperties.HEALTHCHECK_PROPERTY_KEY, "port") + .property(CloudFoundryDeploymentProperties.HEALTHCHECK_HTTP_ENDPOINT_PROPERTY_KEY, "/healthcheck2") + .property(CloudFoundryDeploymentProperties.BUILDPACK_PROPERTY_KEY, "buildpack2") + .property(CloudFoundryDeploymentProperties.DOMAIN_PROPERTY, "domain2") + .property(CloudFoundryDeploymentProperties.HOST_PROPERTY, "host2") + .property(CloudFoundryDeploymentProperties.NO_ROUTE_PROPERTY, "true") + .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()) + .instances(5) + .memory(4096) + .disk(5120) + .healthCheckType(ApplicationHealthCheck.PORT) + .healthCheckHttpEndpoint("/healthcheck2") + .buildpack("buildpack2") + .domain("domain2") + .host("host2") + .noRoute(true) + .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<>()); + } + + private ArgumentMatcher matchesManifest(ApplicationManifest expectedManifest) { + return new ArgumentMatcher() { + @Override + public boolean matches(PushApplicationManifestRequest request) { + if (request.getManifests().size() == 1) { + return request.getManifests().get(0).equals(expectedManifest); + } + + return false; } - return false; + @Override + public String toString() { + return expectedManifest.toString(); + } }; } diff --git a/spring-cloud-app-broker-deployer/src/main/java/org/springframework/cloud/appbroker/deployer/DeployApplicationRequest.java b/spring-cloud-app-broker-deployer/src/main/java/org/springframework/cloud/appbroker/deployer/DeployApplicationRequest.java index 57059a3..503c65d 100644 --- a/spring-cloud-app-broker-deployer/src/main/java/org/springframework/cloud/appbroker/deployer/DeployApplicationRequest.java +++ b/spring-cloud-app-broker-deployer/src/main/java/org/springframework/cloud/appbroker/deployer/DeployApplicationRequest.java @@ -123,9 +123,6 @@ public class DeployApplicationRequest { } public DeployApplicationRequestBuilder services(List services) { - if (services == null) { - return this; - } this.services.addAll(services); return this; } diff --git a/spring-cloud-app-broker-deployer/src/main/java/org/springframework/cloud/appbroker/deployer/UndeployApplicationRequest.java b/spring-cloud-app-broker-deployer/src/main/java/org/springframework/cloud/appbroker/deployer/UndeployApplicationRequest.java index 3275b6f..770ef39 100644 --- a/spring-cloud-app-broker-deployer/src/main/java/org/springframework/cloud/appbroker/deployer/UndeployApplicationRequest.java +++ b/spring-cloud-app-broker-deployer/src/main/java/org/springframework/cloud/appbroker/deployer/UndeployApplicationRequest.java @@ -23,7 +23,7 @@ public class UndeployApplicationRequest { private final String name; private final Map properties; - private UndeployApplicationRequest(String name, Map properties) { + UndeployApplicationRequest(String name, Map properties) { this.name = name; this.properties = properties; } diff --git a/spring-cloud-app-broker-deployer/src/main/java/org/springframework/cloud/appbroker/deployer/util/ByteSizeUtils.java b/spring-cloud-app-broker-deployer/src/main/java/org/springframework/cloud/appbroker/deployer/util/ByteSizeUtils.java index 4612dc1..952bfd5 100644 --- a/spring-cloud-app-broker-deployer/src/main/java/org/springframework/cloud/appbroker/deployer/util/ByteSizeUtils.java +++ b/spring-cloud-app-broker-deployer/src/main/java/org/springframework/cloud/appbroker/deployer/util/ByteSizeUtils.java @@ -16,6 +16,8 @@ package org.springframework.cloud.appbroker.deployer.util; +import org.springframework.util.StringUtils; + import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -26,7 +28,7 @@ import java.util.regex.Pattern; */ public final class ByteSizeUtils { - private static final Pattern SIZE_PATTERN = Pattern.compile("(?\\d+)(?(m|g)?)", Pattern.CASE_INSENSITIVE); + private static final Pattern SIZE_PATTERN = Pattern.compile("(?\\d+)(?([mg])?)", Pattern.CASE_INSENSITIVE); private ByteSizeUtils() { } @@ -35,13 +37,17 @@ public final class ByteSizeUtils { * Return the number of mebibytes (1024*1024) denoted by the given text, where an optional case-insensitive unit of * 'm' or 'g' can be used to mean mebi- or gebi- bytes, respectively. Lack of unit assumes mebibytes. */ - public static long parseToMebibytes(String text) { + public static Integer parseToMebibytes(String text) { + if (!StringUtils.hasText(text)) { + return null; + } + Matcher matcher = SIZE_PATTERN.matcher(text); if (!matcher.matches()) { throw new IllegalArgumentException(String.format("Could not parse '%s' as a byte size." + " Expected a number with optional 'm' or 'g' suffix", text)); } - long size = Long.parseLong(matcher.group("amount")); + int size = Integer.parseInt(matcher.group("amount")); if (matcher.group("unit").equalsIgnoreCase("g")) { size *= 1024L; } From 43950e446ccae8a6fb2577bc1475141c52ccb893 Mon Sep 17 00:00:00 2001 From: Scott Frederick Date: Mon, 8 Oct 2018 14:12:28 -0500 Subject: [PATCH 2/4] Allow CF deployment properties defaults to be provided via configuration. --- .../AppBrokerAutoConfiguration.java | 2 +- .../AppDeployerAutoConfiguration.java | 45 ------- ...udFoundryAppDeployerAutoConfiguration.java | 126 ++++++++++++++++++ .../CloudFoundryClientAutoConfiguration.java | 98 -------------- .../main/resources/META-INF/spring.factories | 3 +- .../AppBrokerAutoConfigurationTest.java | 5 +- ...ndryAppDeployerAutoConfigurationTest.java} | 37 +++-- .../targets/SpacePerServiceInstance.java | 6 +- .../cloudfoundry/CloudFoundryAppDeployer.java | 23 ++-- .../CloudFoundryDeploymentProperties.java | 107 +-------------- .../CloudFoundryTargetProperties.java | 9 +- .../CloudFoundryAppDeployerTest.java | 12 +- .../deployer/DeploymentProperties.java | 87 +++++++----- ...teInstanceWithPropertiesComponentTest.java | 8 +- 14 files changed, 245 insertions(+), 323 deletions(-) delete mode 100644 spring-cloud-app-broker-autoconfigure/src/main/java/org/springframework/cloud/appbroker/autoconfigure/AppDeployerAutoConfiguration.java create mode 100644 spring-cloud-app-broker-autoconfigure/src/main/java/org/springframework/cloud/appbroker/autoconfigure/CloudFoundryAppDeployerAutoConfiguration.java delete mode 100644 spring-cloud-app-broker-autoconfigure/src/main/java/org/springframework/cloud/appbroker/autoconfigure/CloudFoundryClientAutoConfiguration.java rename spring-cloud-app-broker-autoconfigure/src/test/java/org/springframework/cloud/appbroker/autoconfigure/{CloudFoundryClientAutoConfigurationTest.java => CloudFoundryAppDeployerAutoConfigurationTest.java} (61%) rename spring-cloud-app-broker-autoconfigure/src/main/java/org/springframework/cloud/appbroker/autoconfigure/CloudFoundryProperties.java => spring-cloud-app-broker-deployer-cloudfoundry/src/main/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryTargetProperties.java (87%) diff --git a/spring-cloud-app-broker-autoconfigure/src/main/java/org/springframework/cloud/appbroker/autoconfigure/AppBrokerAutoConfiguration.java b/spring-cloud-app-broker-autoconfigure/src/main/java/org/springframework/cloud/appbroker/autoconfigure/AppBrokerAutoConfiguration.java index 3a893b1..a5bbb95 100644 --- a/spring-cloud-app-broker-autoconfigure/src/main/java/org/springframework/cloud/appbroker/autoconfigure/AppBrokerAutoConfiguration.java +++ b/spring-cloud-app-broker-autoconfigure/src/main/java/org/springframework/cloud/appbroker/autoconfigure/AppBrokerAutoConfiguration.java @@ -51,7 +51,7 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @Configuration -@AutoConfigureAfter(AppDeployerAutoConfiguration.class) +@AutoConfigureAfter(CloudFoundryAppDeployerAutoConfiguration.class) @ConditionalOnBean(AppDeployer.class) public class AppBrokerAutoConfiguration { diff --git a/spring-cloud-app-broker-autoconfigure/src/main/java/org/springframework/cloud/appbroker/autoconfigure/AppDeployerAutoConfiguration.java b/spring-cloud-app-broker-autoconfigure/src/main/java/org/springframework/cloud/appbroker/autoconfigure/AppDeployerAutoConfiguration.java deleted file mode 100644 index c5a5c51..0000000 --- a/spring-cloud-app-broker-autoconfigure/src/main/java/org/springframework/cloud/appbroker/autoconfigure/AppDeployerAutoConfiguration.java +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Copyright 2016-2018 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.cloud.appbroker.autoconfigure; - -import org.cloudfoundry.operations.CloudFoundryOperations; - -import org.springframework.boot.autoconfigure.AutoConfigureAfter; -import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; -import org.springframework.cloud.appbroker.deployer.AppDeployer; -import org.springframework.cloud.appbroker.deployer.cloudfoundry.CloudFoundryAppDeployer; -import org.springframework.cloud.appbroker.deployer.cloudfoundry.CloudFoundryDeploymentProperties; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; -import org.springframework.core.io.ResourceLoader; - -@Configuration -@AutoConfigureAfter(CloudFoundryClientAutoConfiguration.class) -public class AppDeployerAutoConfiguration { - - @Bean - @ConditionalOnBean(CloudFoundryOperations.class) - public AppDeployer cloudFoundryAppDeployer(CloudFoundryOperations cloudFoundryOperations, - CloudFoundryProperties cloudFoundryProperties, - ResourceLoader resourceLoader) { - CloudFoundryDeploymentProperties cloudFoundryDeploymentProperties = new CloudFoundryDeploymentProperties(); - - cloudFoundryDeploymentProperties.setDefaultOrg(cloudFoundryProperties.getDefaultOrg()); - cloudFoundryDeploymentProperties.setUsername(cloudFoundryProperties.getUsername()); - return new CloudFoundryAppDeployer(cloudFoundryDeploymentProperties, cloudFoundryOperations, resourceLoader); - } -} diff --git a/spring-cloud-app-broker-autoconfigure/src/main/java/org/springframework/cloud/appbroker/autoconfigure/CloudFoundryAppDeployerAutoConfiguration.java b/spring-cloud-app-broker-autoconfigure/src/main/java/org/springframework/cloud/appbroker/autoconfigure/CloudFoundryAppDeployerAutoConfiguration.java new file mode 100644 index 0000000..61c4071 --- /dev/null +++ b/spring-cloud-app-broker-autoconfigure/src/main/java/org/springframework/cloud/appbroker/autoconfigure/CloudFoundryAppDeployerAutoConfiguration.java @@ -0,0 +1,126 @@ +/* + * Copyright 2016-2018 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.appbroker.autoconfigure; + +import org.cloudfoundry.client.CloudFoundryClient; +import org.cloudfoundry.doppler.DopplerClient; +import org.cloudfoundry.operations.CloudFoundryOperations; + +import org.cloudfoundry.operations.DefaultCloudFoundryOperations; +import org.cloudfoundry.reactor.ConnectionContext; +import org.cloudfoundry.reactor.DefaultConnectionContext; +import org.cloudfoundry.reactor.TokenProvider; +import org.cloudfoundry.reactor.client.ReactorCloudFoundryClient; +import org.cloudfoundry.reactor.doppler.ReactorDopplerClient; +import org.cloudfoundry.reactor.tokenprovider.PasswordGrantTokenProvider; +import org.cloudfoundry.reactor.uaa.ReactorUaaClient; +import org.cloudfoundry.uaa.UaaClient; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.cloud.appbroker.deployer.AppDeployer; +import org.springframework.cloud.appbroker.deployer.cloudfoundry.CloudFoundryAppDeployer; +import org.springframework.cloud.appbroker.deployer.cloudfoundry.CloudFoundryDeploymentProperties; +import org.springframework.cloud.appbroker.deployer.cloudfoundry.CloudFoundryTargetProperties; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.core.io.ResourceLoader; + +import java.util.Optional; + +@Configuration +@ConditionalOnProperty(CloudFoundryAppDeployerAutoConfiguration.PROPERTY_PREFIX + ".api-host") +@EnableConfigurationProperties +public class CloudFoundryAppDeployerAutoConfiguration { + static final String PROPERTY_PREFIX = "spring.cloud.appbroker.deployer.cloudfoundry"; + + @Bean + @ConfigurationProperties(PROPERTY_PREFIX + ".properties") + public CloudFoundryDeploymentProperties cloudFoundryDeploymentProperties() { + return new CloudFoundryDeploymentProperties(); + } + + @Bean + @ConfigurationProperties(PROPERTY_PREFIX) + CloudFoundryTargetProperties cloudFoundryTargetProperties() { + return new CloudFoundryTargetProperties(); + } + + @Bean + public AppDeployer cloudFoundryAppDeployer(CloudFoundryOperations cloudFoundryOperations, + CloudFoundryTargetProperties targetProperties, + CloudFoundryDeploymentProperties deploymentProperties, + ResourceLoader resourceLoader) { + return new CloudFoundryAppDeployer(targetProperties, deploymentProperties, cloudFoundryOperations, resourceLoader); + } + + @Bean + ReactorCloudFoundryClient cloudFoundryClient(ConnectionContext connectionContext, TokenProvider tokenProvider) { + return ReactorCloudFoundryClient.builder() + .connectionContext(connectionContext) + .tokenProvider(tokenProvider) + .build(); + } + + @Bean + CloudFoundryOperations cloudFoundryOperations(CloudFoundryTargetProperties properties, CloudFoundryClient client, + DopplerClient dopplerClient, UaaClient uaaClient) { + return DefaultCloudFoundryOperations.builder() + .cloudFoundryClient(client) + .dopplerClient(dopplerClient) + .uaaClient(uaaClient) + .organization(properties.getDefaultOrg()) + .space(properties.getDefaultSpace()) + .build(); + } + + @Bean + DefaultConnectionContext connectionContext(CloudFoundryTargetProperties properties) { + return DefaultConnectionContext.builder() + .apiHost(properties.getApiHost()) + .port(Optional.ofNullable(properties.getApiPort())) + .skipSslValidation(properties.isSkipSslValidation()) + .secure(properties.isSecure()) + .build(); + } + + @Bean + ReactorDopplerClient dopplerClient(ConnectionContext connectionContext, TokenProvider tokenProvider) { + return ReactorDopplerClient.builder() + .connectionContext(connectionContext) + .tokenProvider(tokenProvider) + .build(); + } + + @Bean + @ConditionalOnProperty({CloudFoundryAppDeployerAutoConfiguration.PROPERTY_PREFIX + ".username", + CloudFoundryAppDeployerAutoConfiguration.PROPERTY_PREFIX + ".password"}) + PasswordGrantTokenProvider tokenProvider(CloudFoundryTargetProperties properties) { + return PasswordGrantTokenProvider.builder() + .password(properties.getPassword()) + .username(properties.getUsername()) + .build(); + } + + @Bean + ReactorUaaClient uaaClient(ConnectionContext connectionContext, TokenProvider tokenProvider) { + return ReactorUaaClient.builder() + .connectionContext(connectionContext) + .tokenProvider(tokenProvider) + .build(); + } +} diff --git a/spring-cloud-app-broker-autoconfigure/src/main/java/org/springframework/cloud/appbroker/autoconfigure/CloudFoundryClientAutoConfiguration.java b/spring-cloud-app-broker-autoconfigure/src/main/java/org/springframework/cloud/appbroker/autoconfigure/CloudFoundryClientAutoConfiguration.java deleted file mode 100644 index e2aef3a..0000000 --- a/spring-cloud-app-broker-autoconfigure/src/main/java/org/springframework/cloud/appbroker/autoconfigure/CloudFoundryClientAutoConfiguration.java +++ /dev/null @@ -1,98 +0,0 @@ -/* - * Copyright 2016-2018 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.cloud.appbroker.autoconfigure; - -import java.util.Optional; -import org.cloudfoundry.client.CloudFoundryClient; -import org.cloudfoundry.doppler.DopplerClient; -import org.cloudfoundry.operations.CloudFoundryOperations; -import org.cloudfoundry.operations.DefaultCloudFoundryOperations; -import org.cloudfoundry.reactor.ConnectionContext; -import org.cloudfoundry.reactor.DefaultConnectionContext; -import org.cloudfoundry.reactor.TokenProvider; -import org.cloudfoundry.reactor.client.ReactorCloudFoundryClient; -import org.cloudfoundry.reactor.doppler.ReactorDopplerClient; -import org.cloudfoundry.reactor.tokenprovider.PasswordGrantTokenProvider; -import org.cloudfoundry.reactor.uaa.ReactorUaaClient; -import org.cloudfoundry.uaa.UaaClient; -import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; -import org.springframework.boot.context.properties.EnableConfigurationProperties; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; - -@Configuration -@ConditionalOnProperty(CloudFoundryProperties.PROPERTY_PREFIX + ".api-host") -@EnableConfigurationProperties(CloudFoundryProperties.class) -public class CloudFoundryClientAutoConfiguration { - - @Bean - ReactorCloudFoundryClient cloudFoundryClient(ConnectionContext connectionContext, TokenProvider tokenProvider) { - return ReactorCloudFoundryClient.builder() - .connectionContext(connectionContext) - .tokenProvider(tokenProvider) - .build(); - } - - @Bean - CloudFoundryOperations cloudFoundryOperations(CloudFoundryProperties properties, CloudFoundryClient client, - DopplerClient dopplerClient, UaaClient uaaClient) { - return DefaultCloudFoundryOperations.builder() - .cloudFoundryClient(client) - .dopplerClient(dopplerClient) - .uaaClient(uaaClient) - .organization(properties.getDefaultOrg()) - .space(properties.getDefaultSpace()) - .build(); - } - - @Bean - DefaultConnectionContext connectionContext(CloudFoundryProperties properties) { - return DefaultConnectionContext.builder() - .apiHost(properties.getApiHost()) - .port(Optional.ofNullable(properties.getApiPort())) - .skipSslValidation(properties.isSkipSslValidation()) - .secure(properties.isSecure()) - .build(); - } - - @Bean - ReactorDopplerClient dopplerClient(ConnectionContext connectionContext, TokenProvider tokenProvider) { - return ReactorDopplerClient.builder() - .connectionContext(connectionContext) - .tokenProvider(tokenProvider) - .build(); - } - - @Bean - @ConditionalOnProperty({CloudFoundryProperties.PROPERTY_PREFIX + ".username", - CloudFoundryProperties.PROPERTY_PREFIX + ".password"}) - PasswordGrantTokenProvider tokenProvider(CloudFoundryProperties properties) { - return PasswordGrantTokenProvider.builder() - .password(properties.getPassword()) - .username(properties.getUsername()) - .build(); - } - - @Bean - ReactorUaaClient uaaClient(ConnectionContext connectionContext, TokenProvider tokenProvider) { - return ReactorUaaClient.builder() - .connectionContext(connectionContext) - .tokenProvider(tokenProvider) - .build(); - } - -} diff --git a/spring-cloud-app-broker-autoconfigure/src/main/resources/META-INF/spring.factories b/spring-cloud-app-broker-autoconfigure/src/main/resources/META-INF/spring.factories index cef5be4..4fe3741 100644 --- a/spring-cloud-app-broker-autoconfigure/src/main/resources/META-INF/spring.factories +++ b/spring-cloud-app-broker-autoconfigure/src/main/resources/META-INF/spring.factories @@ -1,4 +1,3 @@ org.springframework.boot.autoconfigure.EnableAutoConfiguration=\ -org.springframework.cloud.appbroker.autoconfigure.CloudFoundryClientAutoConfiguration,\ -org.springframework.cloud.appbroker.autoconfigure.AppDeployerAutoConfiguration,\ +org.springframework.cloud.appbroker.autoconfigure.CloudFoundryAppDeployerAutoConfiguration,\ org.springframework.cloud.appbroker.autoconfigure.AppBrokerAutoConfiguration \ No newline at end of file diff --git a/spring-cloud-app-broker-autoconfigure/src/test/java/org/springframework/cloud/appbroker/autoconfigure/AppBrokerAutoConfigurationTest.java b/spring-cloud-app-broker-autoconfigure/src/test/java/org/springframework/cloud/appbroker/autoconfigure/AppBrokerAutoConfigurationTest.java index 5814e6b..aab9fa3 100644 --- a/spring-cloud-app-broker-autoconfigure/src/test/java/org/springframework/cloud/appbroker/autoconfigure/AppBrokerAutoConfigurationTest.java +++ b/spring-cloud-app-broker-autoconfigure/src/test/java/org/springframework/cloud/appbroker/autoconfigure/AppBrokerAutoConfigurationTest.java @@ -40,10 +40,9 @@ class AppBrokerAutoConfigurationTest { .withConfiguration(AutoConfigurations.of(AppBrokerAutoConfiguration.class)); @Test - void servicesAreCreatedWithAppDeployerConfigured() { + void servicesAreCreatedWithCloudFoundryConfigured() { this.contextRunner - .withConfiguration(AutoConfigurations.of(AppDeployerAutoConfiguration.class, - CloudFoundryClientAutoConfiguration.class)) + .withConfiguration(AutoConfigurations.of(CloudFoundryAppDeployerAutoConfiguration.class)) .withPropertyValues( "spring.cloud.appbroker.services[0].service-name=service1", "spring.cloud.appbroker.services[0].plan-name=service1-plan1", diff --git a/spring-cloud-app-broker-autoconfigure/src/test/java/org/springframework/cloud/appbroker/autoconfigure/CloudFoundryClientAutoConfigurationTest.java b/spring-cloud-app-broker-autoconfigure/src/test/java/org/springframework/cloud/appbroker/autoconfigure/CloudFoundryAppDeployerAutoConfigurationTest.java similarity index 61% rename from spring-cloud-app-broker-autoconfigure/src/test/java/org/springframework/cloud/appbroker/autoconfigure/CloudFoundryClientAutoConfigurationTest.java rename to spring-cloud-app-broker-autoconfigure/src/test/java/org/springframework/cloud/appbroker/autoconfigure/CloudFoundryAppDeployerAutoConfigurationTest.java index 61af4ba..20b70dd 100644 --- a/spring-cloud-app-broker-autoconfigure/src/test/java/org/springframework/cloud/appbroker/autoconfigure/CloudFoundryClientAutoConfigurationTest.java +++ b/spring-cloud-app-broker-autoconfigure/src/test/java/org/springframework/cloud/appbroker/autoconfigure/CloudFoundryAppDeployerAutoConfigurationTest.java @@ -28,13 +28,16 @@ import org.junit.jupiter.api.Test; import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.cloud.appbroker.deployer.AppDeployer; +import org.springframework.cloud.appbroker.deployer.cloudfoundry.CloudFoundryDeploymentProperties; +import org.springframework.cloud.appbroker.deployer.cloudfoundry.CloudFoundryTargetProperties; import static org.assertj.core.api.Assertions.assertThat; -class CloudFoundryClientAutoConfigurationTest { +class CloudFoundryAppDeployerAutoConfigurationTest { private final ApplicationContextRunner contextRunner = new ApplicationContextRunner() - .withConfiguration(AutoConfigurations.of(CloudFoundryClientAutoConfiguration.class)); + .withConfiguration(AutoConfigurations.of(CloudFoundryAppDeployerAutoConfiguration.class)); @Test void clientIsCreatedWithPasswordGrantConfiguration() { @@ -45,15 +48,28 @@ class CloudFoundryClientAutoConfigurationTest { "spring.cloud.appbroker.deployer.cloudfoundry.default-org=example-org", "spring.cloud.appbroker.deployer.cloudfoundry.default-space=example-space", "spring.cloud.appbroker.deployer.cloudfoundry.username=user", - "spring.cloud.appbroker.deployer.cloudfoundry.password=secret" + "spring.cloud.appbroker.deployer.cloudfoundry.password=secret", + "spring.cloud.appbroker.deployer.cloudfoundry.properties.memory=2G", + "spring.cloud.appbroker.deployer.cloudfoundry.properties.count=3", + "spring.cloud.appbroker.deployer.cloudfoundry.properties.buildpack=example-buildpack", + "spring.cloud.appbroker.deployer.cloudfoundry.properties.domain=example.com" ) .run((context) -> { - assertThat(context).hasSingleBean(CloudFoundryProperties.class); - CloudFoundryProperties cloudFoundryProperties = context.getBean(CloudFoundryProperties.class); - assertThat(cloudFoundryProperties.getApiHost()).isEqualTo("api.example.com"); - assertThat(cloudFoundryProperties.getApiPort()).isEqualTo(443); - assertThat(cloudFoundryProperties.getDefaultOrg()).isEqualTo("example-org"); - assertThat(cloudFoundryProperties.getDefaultSpace()).isEqualTo("example-space"); + assertThat(context).hasSingleBean(CloudFoundryTargetProperties.class); + CloudFoundryTargetProperties targetProperties = context.getBean(CloudFoundryTargetProperties.class); + assertThat(targetProperties.getApiHost()).isEqualTo("api.example.com"); + assertThat(targetProperties.getApiPort()).isEqualTo(443); + assertThat(targetProperties.getDefaultOrg()).isEqualTo("example-org"); + assertThat(targetProperties.getDefaultSpace()).isEqualTo("example-space"); + + assertThat(context).hasSingleBean(CloudFoundryDeploymentProperties.class); + CloudFoundryDeploymentProperties deploymentProperties = context.getBean(CloudFoundryDeploymentProperties.class); + assertThat(deploymentProperties.getMemory()).isEqualTo("2G"); + assertThat(deploymentProperties.getCount()).isEqualTo(3); + assertThat(deploymentProperties.getBuildpack()).isEqualTo("example-buildpack"); + assertThat(deploymentProperties.getDomain()).isEqualTo("example.com"); + + assertThat(context).hasSingleBean(AppDeployer.class); assertThat(context).hasSingleBean(ReactorCloudFoundryClient.class); assertThat(context).hasSingleBean(ReactorDopplerClient.class); @@ -68,7 +84,8 @@ class CloudFoundryClientAutoConfigurationTest { void clientIsNotCreatedWithoutConfiguration() { this.contextRunner .run((context) -> { - assertThat(context).doesNotHaveBean(CloudFoundryProperties.class); + assertThat(context).doesNotHaveBean(CloudFoundryTargetProperties.class); + assertThat(context).doesNotHaveBean(CloudFoundryDeploymentProperties.class); assertThat(context).doesNotHaveBean(ReactorCloudFoundryClient.class); assertThat(context).doesNotHaveBean(ReactorDopplerClient.class); assertThat(context).doesNotHaveBean(ReactorUaaClient.class); diff --git a/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/extensions/targets/SpacePerServiceInstance.java b/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/extensions/targets/SpacePerServiceInstance.java index db11087..6e1a85d 100644 --- a/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/extensions/targets/SpacePerServiceInstance.java +++ b/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/extensions/targets/SpacePerServiceInstance.java @@ -33,8 +33,10 @@ public class SpacePerServiceInstance extends TargetFactory apply(BackingApplication backingApplication, String serviceInstanceId) { - backingApplication.getProperties().put(DeploymentProperties.HOST_KEY, backingApplication.getName() + "-" + serviceInstanceId); - backingApplication.getProperties().put(DeploymentProperties.TARGET_KEY, serviceInstanceId); + backingApplication.addProperty(DeploymentProperties.HOST_PROPERTY_KEY, + backingApplication.getName() + "-" + serviceInstanceId); + backingApplication.addProperty(DeploymentProperties.TARGET_PROPERTY_KEY, + serviceInstanceId); return Mono.just(backingApplication); } 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 6f7e1bd..f72643e 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 @@ -74,15 +74,18 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); + private final CloudFoundryTargetProperties targetProperties; private final CloudFoundryDeploymentProperties defaultDeploymentProperties; private final CloudFoundryOperations operations; private ResourceLoader resourceLoader; - public CloudFoundryAppDeployer(CloudFoundryDeploymentProperties deploymentProperties, + public CloudFoundryAppDeployer(CloudFoundryTargetProperties targetProperties, + CloudFoundryDeploymentProperties deploymentProperties, CloudFoundryOperations operations, ResourceLoader resourceLoader) { + this.targetProperties = targetProperties; this.defaultDeploymentProperties = deploymentProperties; this.operations = operations; this.resourceLoader = resourceLoader; @@ -133,8 +136,8 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware .build(); Mono requestPushApplication = requestPushApplication(applicationManifestRequest); - if (deploymentProperties.containsKey(DeploymentProperties.TARGET_KEY)) { - String space = deploymentProperties.get(DeploymentProperties.TARGET_KEY); + if (deploymentProperties.containsKey(DeploymentProperties.TARGET_PROPERTY_KEY)) { + String space = deploymentProperties.get(DeploymentProperties.TARGET_PROPERTY_KEY); requestPushApplication = requestPushApplicationInSpace(applicationManifestRequest, space); } @@ -202,7 +205,7 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware private Mono getOrganizationIdPublisher() { OrganizationInfoRequest organizationInfoRequest = - OrganizationInfoRequest.builder().name(this.defaultDeploymentProperties.getDefaultOrg()).build(); + OrganizationInfoRequest.builder().name(this.targetProperties.getDefaultOrg()).build(); return this.operations.organizations().get(organizationInfoRequest).map(OrganizationDetail::getId); } @@ -214,8 +217,8 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware Map deploymentProperties = request.getProperties(); Mono requestDeleteApplication; - if (deploymentProperties.containsKey(DeploymentProperties.TARGET_KEY)) { - String space = deploymentProperties.get(DeploymentProperties.TARGET_KEY); + if (deploymentProperties.containsKey(DeploymentProperties.TARGET_PROPERTY_KEY)) { + String space = deploymentProperties.get(DeploymentProperties.TARGET_PROPERTY_KEY); requestDeleteApplication = requestDeleteApplicationInSpace(appName, space) .then(createSpaceOperations().delete(DeleteSpaceRequest.builder().name(space).build())); } else { @@ -259,7 +262,7 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware return new DefaultSpaces( ((DefaultCloudFoundryOperations) this.operations).getCloudFoundryClientPublisher() , getOrganizationIdPublisher(), - Mono.just(this.defaultDeploymentProperties.getUsername())); + Mono.just(this.targetProperties.getUsername())); } private Map getEnvironmentVariables(Map environment) { @@ -309,7 +312,7 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware } private boolean useSpringApplicationJson(Map environment) { - return Optional.ofNullable(environment.get(CloudFoundryDeploymentProperties.USE_SPRING_APPLICATION_JSON_KEY)) + return Optional.ofNullable(environment.get(DeploymentProperties.USE_SPRING_APPLICATION_JSON_KEY)) .map(Boolean::valueOf) .orElse(this.defaultDeploymentProperties.isUseSpringApplicationJson()); } @@ -348,11 +351,11 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware private Integer instances(Map properties) { return Optional.ofNullable(properties.get(DeploymentProperties.COUNT_PROPERTY_KEY)) .map(Integer::parseInt) - .orElse(this.defaultDeploymentProperties.getInstances()); + .orElse(this.defaultDeploymentProperties.getCount()); } private String host(Map properties) { - return Optional.ofNullable(properties.get(CloudFoundryDeploymentProperties.HOST_PROPERTY)) + return Optional.ofNullable(properties.get(DeploymentProperties.HOST_PROPERTY_KEY)) .orElse(this.defaultDeploymentProperties.getHost()); } diff --git a/spring-cloud-app-broker-deployer-cloudfoundry/src/main/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryDeploymentProperties.java b/spring-cloud-app-broker-deployer-cloudfoundry/src/main/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryDeploymentProperties.java index 770ee92..da3d5fb 100644 --- a/spring-cloud-app-broker-deployer-cloudfoundry/src/main/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryDeploymentProperties.java +++ b/spring-cloud-app-broker-deployer-cloudfoundry/src/main/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryDeploymentProperties.java @@ -17,7 +17,7 @@ package org.springframework.cloud.appbroker.deployer.cloudfoundry; import org.cloudfoundry.operations.applications.ApplicationHealthCheck; -import org.springframework.beans.factory.annotation.Value; +import org.springframework.cloud.appbroker.deployer.DeploymentProperties; import java.time.Duration; import java.util.HashSet; @@ -32,7 +32,7 @@ import java.util.Set; * @author Ilayaperumal Gopinathan */ @SuppressWarnings({"unused", "PMD.TooManyFields"}) -public class CloudFoundryDeploymentProperties { +public class CloudFoundryDeploymentProperties extends DeploymentProperties { static final String HEALTHCHECK_PROPERTY_KEY = "health-check"; static final String HEALTHCHECK_HTTP_ENDPOINT_PROPERTY_KEY = "health-check-http-endpoint"; @@ -47,21 +47,12 @@ public class CloudFoundryDeploymentProperties { static final String NO_ROUTE_PROPERTY = "no-route"; - static final String HOST_PROPERTY = "host"; - static final String DOMAIN_PROPERTY = "domain"; static final String BUILDPACK_PROPERTY_KEY = "buildpack"; static final String JAVA_OPTS_PROPERTY_KEY = "javaOpts"; - static final String USE_SPRING_APPLICATION_JSON_KEY = "use-spring-application-json"; - - /** - * The host name to use as part of the route. Defaults to hostname derived by Cloud Foundry. - */ - private String host; - /** * The domain to use when mapping routes for applications. */ @@ -78,16 +69,6 @@ public class CloudFoundryDeploymentProperties { */ private String buildpack; - /** - * The amount of memory to allocate, if not overridden per-app. Default unit is mebibytes, 'M' and 'G" suffixes supported. - */ - private String memory; - - /** - * The amount of disk space to allocate, if not overridden per-app. Default unit is mebibytes, 'M' and 'G" suffixes supported. - */ - private String disk; - /** * The type of health check to perform on deployed application, if not overridden per-app. Defaults to PORT */ @@ -103,11 +84,6 @@ public class CloudFoundryDeploymentProperties { */ private Integer healthCheckTimeout; - /** - * The number of instances to run. - */ - private Integer instances; - /** * Flag to enable prefixing the app name with a random prefix. */ @@ -123,11 +99,6 @@ public class CloudFoundryDeploymentProperties { */ private long statusTimeout = 5_000L; - /** - * Flag to indicate whether application properties are fed into SPRING_APPLICATION_JSON or ENVIRONMENT VARIABLES. - */ - private boolean useSpringApplicationJson = true; - /** * If set, override the timeout allocated for staging the app by the client. */ @@ -138,12 +109,6 @@ public class CloudFoundryDeploymentProperties { */ private Duration startupTimeout = Duration.ofMinutes(5L); - /** - * String to use as prefix for name of deployed app. Defaults to spring.application.name. - */ - @Value("${spring.application.name:}") - private String appNamePrefix; - /** * Whether to also delete routes when un-deploying an application. */ @@ -151,10 +116,6 @@ public class CloudFoundryDeploymentProperties { private String javaOpts; - private String defaultOrg; - - private String username; - public String getBuildpack() { return buildpack; } @@ -163,30 +124,6 @@ public class CloudFoundryDeploymentProperties { this.buildpack = buildpack; } - public String getMemory() { - return memory; - } - - public void setMemory(String memory) { - this.memory = memory; - } - - public String getDisk() { - return disk; - } - - public void setDisk(String disk) { - this.disk = disk; - } - - public Integer getInstances() { - return instances; - } - - public void setInstances(int instances) { - this.instances = instances; - } - public boolean isEnableRandomAppNamePrefix() { return enableRandomAppNamePrefix; } @@ -195,14 +132,6 @@ public class CloudFoundryDeploymentProperties { this.enableRandomAppNamePrefix = enableRandomAppNamePrefix; } - public String getAppNamePrefix() { - return appNamePrefix; - } - - public void setAppNamePrefix(String appNamePrefix) { - this.appNamePrefix = appNamePrefix; - } - public long getApiTimeout() { return apiTimeout; } @@ -211,14 +140,6 @@ public class CloudFoundryDeploymentProperties { this.apiTimeout = apiTimeout; } - public boolean isUseSpringApplicationJson() { - return useSpringApplicationJson; - } - - public void setUseSpringApplicationJson(boolean useSpringApplicationJson) { - this.useSpringApplicationJson = useSpringApplicationJson; - } - public ApplicationHealthCheck getHealthCheck() { return healthCheck; } @@ -251,14 +172,6 @@ public class CloudFoundryDeploymentProperties { this.domain = domain; } - public String getHost() { - return host; - } - - public void setHost(String host) { - this.host = host; - } - public Set getRoutes() { return routes; } @@ -306,20 +219,4 @@ public class CloudFoundryDeploymentProperties { public void setJavaOpts(String javaOpts) { this.javaOpts = javaOpts; } - - public String getDefaultOrg() { - return defaultOrg; - } - - public void setDefaultOrg(String defaultOrg) { - this.defaultOrg = defaultOrg; - } - - public String getUsername() { - return username; - } - - public void setUsername(String username) { - this.username = username; - } } diff --git a/spring-cloud-app-broker-autoconfigure/src/main/java/org/springframework/cloud/appbroker/autoconfigure/CloudFoundryProperties.java b/spring-cloud-app-broker-deployer-cloudfoundry/src/main/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryTargetProperties.java similarity index 87% rename from spring-cloud-app-broker-autoconfigure/src/main/java/org/springframework/cloud/appbroker/autoconfigure/CloudFoundryProperties.java rename to spring-cloud-app-broker-deployer-cloudfoundry/src/main/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryTargetProperties.java index 1ba5a56..39b9c26 100644 --- a/spring-cloud-app-broker-autoconfigure/src/main/java/org/springframework/cloud/appbroker/autoconfigure/CloudFoundryProperties.java +++ b/spring-cloud-app-broker-deployer-cloudfoundry/src/main/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryTargetProperties.java @@ -14,18 +14,13 @@ * limitations under the License. */ -package org.springframework.cloud.appbroker.autoconfigure; +package org.springframework.cloud.appbroker.deployer.cloudfoundry; import java.net.URI; import org.cloudfoundry.reactor.ProxyConfiguration; -import org.springframework.boot.context.properties.ConfigurationProperties; - -@ConfigurationProperties(CloudFoundryProperties.PROPERTY_PREFIX) -public class CloudFoundryProperties { - - static final String PROPERTY_PREFIX = "spring.cloud.appbroker.deployer.cloudfoundry"; +public class CloudFoundryTargetProperties { private String apiHost; private Integer apiPort; 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 4801e34..be08cfe 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 @@ -61,7 +61,7 @@ class CloudFoundryAppDeployerTest { @Mock private ResourceLoader resourceLoader; - + private CloudFoundryDeploymentProperties deploymentProperties; @BeforeEach @@ -76,7 +76,7 @@ class CloudFoundryAppDeployerTest { .thenReturn(new FileSystemResource(APP_PATH)); appDeployer = new CloudFoundryAppDeployer( - deploymentProperties, cloudFoundryOperations, resourceLoader); + new CloudFoundryTargetProperties(), deploymentProperties, cloudFoundryOperations, resourceLoader); } @Test @@ -110,7 +110,7 @@ class CloudFoundryAppDeployerTest { .property(CloudFoundryDeploymentProperties.HEALTHCHECK_HTTP_ENDPOINT_PROPERTY_KEY, "/healthcheck") .property(CloudFoundryDeploymentProperties.BUILDPACK_PROPERTY_KEY, "buildpack") .property(CloudFoundryDeploymentProperties.DOMAIN_PROPERTY, "domain") - .property(CloudFoundryDeploymentProperties.HOST_PROPERTY, "host") + .property(DeploymentProperties.HOST_PROPERTY_KEY, "host") .property(CloudFoundryDeploymentProperties.NO_ROUTE_PROPERTY, "true") .build(); @@ -137,7 +137,7 @@ class CloudFoundryAppDeployerTest { @Test void deployAppWithDefaultProperties() { - deploymentProperties.setInstances(3); + deploymentProperties.setCount(3); deploymentProperties.setMemory("2G"); deploymentProperties.setDisk("3G"); deploymentProperties.setBuildpack("buildpack"); @@ -173,7 +173,7 @@ class CloudFoundryAppDeployerTest { @Test void deployAppWithRequestOverridingDefaultProperties() { - deploymentProperties.setInstances(3); + deploymentProperties.setCount(3); deploymentProperties.setMemory("2G"); deploymentProperties.setDisk("3G"); deploymentProperties.setBuildpack("buildpack1"); @@ -192,7 +192,7 @@ class CloudFoundryAppDeployerTest { .property(CloudFoundryDeploymentProperties.HEALTHCHECK_HTTP_ENDPOINT_PROPERTY_KEY, "/healthcheck2") .property(CloudFoundryDeploymentProperties.BUILDPACK_PROPERTY_KEY, "buildpack2") .property(CloudFoundryDeploymentProperties.DOMAIN_PROPERTY, "domain2") - .property(CloudFoundryDeploymentProperties.HOST_PROPERTY, "host2") + .property(DeploymentProperties.HOST_PROPERTY_KEY, "host2") .property(CloudFoundryDeploymentProperties.NO_ROUTE_PROPERTY, "true") .build(); 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 61e7525..d157867 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 @@ -16,8 +16,6 @@ package org.springframework.cloud.appbroker.deployer; -import org.springframework.cloud.appbroker.deployer.util.ByteSizeUtils; - public class DeploymentProperties { /** * The deployment property for the count (number of app instances). @@ -31,23 +29,6 @@ public class DeploymentProperties { */ public static final String GROUP_PROPERTY_KEY = "group"; - /** - * The deployment property that indicates if each app instance should have an index value - * within a sequence from 0 to N-1, where N is the value of the {@value #COUNT_PROPERTY_KEY} - * property. If not provided, a deployer should assume app instance indexing is not necessary. - */ - public static final String INDEXED_PROPERTY_KEY = "indexed"; - - /** - * The property to be set at each instance level to specify the sequence number - * amongst 0 to N-1, where N is the value of the {@value #COUNT_PROPERTY_KEY} property. - * Specified as CAPITAL_WITH_UNDERSCORES as this is typically passed as an environment - * variable, but when targeting a Spring app, other variations may apply. - * - * @see #INDEXED_PROPERTY_KEY - */ - public static final String INSTANCE_INDEX_PROPERTY_KEY = "INSTANCE_INDEX"; - /** * The deployment property for the memory setting for the container that will run the app. * The memory is specified in Mebibytes, @@ -57,8 +38,6 @@ public class DeploymentProperties { * 1 MiB = 2^20 bytes = 1024*1024 bytes vs. the decimal based 1MB = 10^6 bytes = 1000*1000 bytes, *

* Implementations are expected to translate this value to the target platform as faithfully as possible. - * - * @see ByteSizeUtils */ public static final String MEMORY_PROPERTY_KEY = "memory"; @@ -71,27 +50,69 @@ public class DeploymentProperties { * 1 MiB = 2^20 bytes = 1024*1024 bytes vs. the decimal based 1MB = 10^6 bytes = 1000*1000 bytes, *

* Implementations are expected to translate this value to the target platform as faithfully as possible. - * - * @see ByteSizeUtils */ public static final String DISK_PROPERTY_KEY = "disk"; - /** - * The deployment property for the cpu setting for the container that will run the app. - * The cpu is specified as whole multiples or decimal fractions of virtual cores. Some platforms will not - * support setting cpu and will ignore this setting. Other platforms may require whole numbers and might - * round up. Exactly how this property affects the deployments will vary between implementations. - */ - public static final String CPU_PROPERTY_KEY = "cpu"; - /** * The deployment property for the host that will be used in the app. */ - public static final String HOST_KEY = "host"; + public static final String HOST_PROPERTY_KEY = "host"; /** * The deployment property for the location where the app will be deployed. * The location will vary between implementations. */ - public static final String TARGET_KEY = "target"; + public static final String TARGET_PROPERTY_KEY = "target"; + + public static final String USE_SPRING_APPLICATION_JSON_KEY = "use-spring-application-json"; + + private String host; + + private String memory; + + private String disk; + + private Integer count; + + private boolean useSpringApplicationJson = true; + + public String getMemory() { + return memory; + } + + public void setMemory(String memory) { + this.memory = memory; + } + + public String getDisk() { + return disk; + } + + public void setDisk(String disk) { + this.disk = disk; + } + + public Integer getCount() { + return count; + } + + public void setCount(int count) { + this.count = count; + } + + public String getHost() { + return host; + } + + public void setHost(String host) { + this.host = host; + } + + public boolean isUseSpringApplicationJson() { + return useSpringApplicationJson; + } + + public void setUseSpringApplicationJson(boolean useSpringApplicationJson) { + this.useSpringApplicationJson = useSpringApplicationJson; + } } diff --git a/spring-cloud-app-broker-sample/src/test/java/org.springframework.cloud.appbroker/sample/CreateInstanceWithPropertiesComponentTest.java b/spring-cloud-app-broker-sample/src/test/java/org.springframework.cloud.appbroker/sample/CreateInstanceWithPropertiesComponentTest.java index 52447e7..32ba2f8 100644 --- a/spring-cloud-app-broker-sample/src/test/java/org.springframework.cloud.appbroker/sample/CreateInstanceWithPropertiesComponentTest.java +++ b/spring-cloud-app-broker-sample/src/test/java/org.springframework.cloud.appbroker/sample/CreateInstanceWithPropertiesComponentTest.java @@ -32,6 +32,10 @@ import static org.hamcrest.Matchers.is; import static org.springframework.cloud.appbroker.sample.CreateInstanceWithPropertiesComponentTest.APP_NAME; @TestPropertySource(properties = { + "spring.cloud.appbroker.deployer.cloudfoundry.properties.memory=1G", + "spring.cloud.appbroker.deployer.cloudfoundry.properties.count=1", + "spring.cloud.appbroker.deployer.cloudfoundry.properties.health-check=http", + "spring.cloud.appbroker.deployer.cloudfoundry.properties.health-check-http-endpoint=/myhealth", "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", @@ -56,7 +60,9 @@ class CreateInstanceWithPropertiesComponentTest extends WiremockComponentTest { cloudControllerFixture.stubPushApp(APP_NAME, matchingJsonPath("$.[?(@.memory == '2048')]"), matchingJsonPath("$.[?(@.instances == '2')]"), - matchingJsonPath("$.[?(@.health_check_timeout == '180')]")); + matchingJsonPath("$.[?(@.health_check_timeout == '180')]"), + matchingJsonPath("$.[?(@.health_check_type == 'http')]"), + matchingJsonPath("$.[?(@.health_check_http_endpoint == '/myhealth')]")); // when a service instance is created given(brokerFixture.serviceInstanceRequest()) From 0ae92a598abd86469335d3a1a993a6efe634eee9 Mon Sep 17 00:00:00 2001 From: Scott Frederick Date: Mon, 8 Oct 2018 15:14:39 -0500 Subject: [PATCH 3/4] Support setting env vars without SPRING_APPLICATION_JSON. --- .../cloudfoundry/CloudFoundryAppDeployer.java | 47 ++++++++-------- .../CloudFoundryAppDeployerTest.java | 54 ++++++++++++++++++- .../deployer/DeploymentProperties.java | 6 --- ...eInstanceWithEnvironmentComponentTest.java | 22 ++++++-- 4 files changed, 92 insertions(+), 37 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 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() From 9dba3970c6d4ba293bd2b6c6e6666b164aecede0 Mon Sep 17 00:00:00 2001 From: Scott Frederick Date: Tue, 9 Oct 2018 10:19:37 -0500 Subject: [PATCH 4/4] Default backing app memory to 1014M in acceptance tests. --- .../fixtures/cf/CloudFoundryService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spring-cloud-app-broker-acceptance-tests/src/test/java/org.springframework.cloud.appbroker.acceptance/fixtures/cf/CloudFoundryService.java b/spring-cloud-app-broker-acceptance-tests/src/test/java/org.springframework.cloud.appbroker.acceptance/fixtures/cf/CloudFoundryService.java index 1a2bce0..ebd8091 100644 --- a/spring-cloud-app-broker-acceptance-tests/src/test/java/org.springframework.cloud.appbroker.acceptance/fixtures/cf/CloudFoundryService.java +++ b/spring-cloud-app-broker-acceptance-tests/src/test/java/org.springframework.cloud.appbroker.acceptance/fixtures/cf/CloudFoundryService.java @@ -281,7 +281,7 @@ public class CloudFoundryService { deployerVariables.put("spring.cloud.appbroker.deployer.cloudfoundry.default-org", cloudFoundryProperties.getDefaultOrg()); deployerVariables.put("spring.cloud.appbroker.deployer.cloudfoundry.default-space", cloudFoundryProperties.getDefaultSpace()); deployerVariables.put("spring.cloud.appbroker.deployer.cloudfoundry.skip-ssl-validation", String.valueOf(cloudFoundryProperties.isSkipSslValidation())); - deployerVariables.put("spring.cloud.appbroker.apps[0].cloudfoundry.skip-ssl-validation", String.valueOf(cloudFoundryProperties.isSkipSslValidation())); + deployerVariables.put("spring.cloud.appbroker.deployer.cloudfoundry.properties.memory", "1024M"); return deployerVariables; }