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 86b5f92..bf8b285 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -21,6 +21,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.time.Duration; import java.time.Instant; +import java.util.Arrays; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -207,7 +208,14 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware .doOnSuccess(response -> LOG.info("Success getting application {}", name)) .doOnError(e -> LOG.warn(String.format("Error getting application %s: %s", name, e.getMessage()))) .map(ApplicationDetail::getId) - .flatMap(applicationId -> associateHostName(applicationId, request.getProperties())) + .flatMap(applicationId -> { + if (request.getProperties().containsKey("routes")) { + return associateRoutes(applicationId, request.getProperties()); + } + else { + return associateHostName(applicationId, request.getProperties()); + } + }) .flatMap(applicationId -> updateEnvironment(request, applicationId)) .flatMap(applicationId -> Mono.zip(Mono.just(applicationId), upgradeApplication(request, applicationId))) @@ -242,7 +250,8 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware return Mono.just(applicationId); } - return operationsUtils.getOperations(properties).map(cfOperations -> cfOperations.domains().list()) + return operationsUtils.getOperations(properties) + .map(cfOperations -> cfOperations.domains().list()) .flatMap(Flux::collectList) .map(allDomains -> Stream.concat(Stream.of(domain), domains.stream()) .map(d -> getDomainId(d, allDomains)) @@ -257,6 +266,30 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware .then(Mono.just(applicationId)); } + private Mono associateRoutes(String applicationId, Map properties) { + List routes = Arrays.stream(properties.get("routes").split(",")) + .map(uri -> uri.split("\\.", 2)) + .collect(Collectors.toList()); + + return Mono.just(routes) + .zipWith(operationsUtils.getOperations(properties) + .map(cfOperations -> cfOperations.domains().list()) + .flatMap(domains -> domains.collectMap(Domain::getName))) + .zipWith(getSpaceId(properties)) + .flatMapMany(routesDomainsAndSpace -> { + List routesComponents = routesDomainsAndSpace.getT1().getT1(); + Map domainsByName = routesDomainsAndSpace.getT1().getT2(); + String spaceId = routesDomainsAndSpace.getT2(); + + return Flux.fromStream(routesComponents.stream() + .map(route -> new String[] {route[0], domainsByName.get(route[1]).getId()})) + .flatMap( + hostAndDomainId -> associateHostForDomain(applicationId, hostAndDomainId[0], hostAndDomainId[1], + spaceId)); + }) + .then(Mono.just(applicationId)); + } + private Mono associateHostForDomain(String applicationId, String host, String domainId, String spaceId) { return client.routes() .create(org.cloudfoundry.client.v2.routes.CreateRouteRequest.builder() @@ -541,31 +574,29 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware .timeout(healthCheckTimeout(deploymentProperties)) .noRoute(toggleNoRoute(deploymentProperties)); - Optional.ofNullable(host(deploymentProperties)).ifPresent(manifest::host); - Optional.ofNullable(domain(deploymentProperties)).ifPresent(manifest::domain); Optional.ofNullable(routePath(deploymentProperties)).ifPresent(manifest::routePath); - if (route(deploymentProperties) != null) { - manifest.route(Route.builder().route(route(deploymentProperties)).build()); + if (routes(deploymentProperties).isEmpty()) { + Optional.ofNullable(host(deploymentProperties)).ifPresent(manifest::host); + Optional.ofNullable(domain(deploymentProperties)).ifPresent(manifest::domain); + if (!domains(deploymentProperties).isEmpty()) { + domains(deploymentProperties).forEach(manifest::domain); + } } - - if (!routes(deploymentProperties).isEmpty()) { + else { Set routes = routes(deploymentProperties).stream() .map(r -> Route.builder().route(r).build()) .collect(Collectors.toSet()); manifest.routes(routes); } - if (!domains(deploymentProperties).isEmpty()) { - domains(deploymentProperties).forEach(manifest::domain); - } - if (getDockerImage(appResource) == null) { manifest.buildpack(buildpack(deploymentProperties)); } else { manifest.docker(Docker.builder().image(getDockerImage(appResource)).build()); } + return manifest.build(); } @@ -820,10 +851,6 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware return routePath; } - private String route(Map properties) { - return properties.get(CloudFoundryDeploymentProperties.ROUTE_PROPERTY); - } - private Set routes(Map properties) { Set routes = new HashSet<>(); routes.addAll(this.defaultDeploymentProperties.getRoutes()); 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 1adeee4..74f00dd 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 @@ -55,11 +55,6 @@ public class CloudFoundryDeploymentProperties extends DeploymentProperties { */ protected static final String ROUTE_PATH_PROPERTY = "route-path"; - /** - * Key for storing the route deployment property - */ - protected static final String ROUTE_PROPERTY = "route"; - /** * Key for storing the routes deployment property */ 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 f2d8bd3..36736a0 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 @@ -41,6 +41,7 @@ import org.cloudfoundry.operations.applications.ApplicationHealthCheck; import org.cloudfoundry.operations.applications.ApplicationManifest; import org.cloudfoundry.operations.applications.Applications; import org.cloudfoundry.operations.applications.PushApplicationManifestRequest; +import org.cloudfoundry.operations.applications.Route; import org.cloudfoundry.operations.organizations.OrganizationDetail; import org.cloudfoundry.operations.organizations.OrganizationInfoRequest; import org.cloudfoundry.operations.organizations.OrganizationQuota; @@ -85,7 +86,7 @@ import static org.mockito.BDDMockito.given; import static org.mockito.BDDMockito.then; import static org.springframework.cloud.appbroker.deployer.DeploymentProperties.TARGET_PROPERTY_KEY; -@SuppressWarnings("UnassignedFluxMonoInstance") +@SuppressWarnings({"UnassignedFluxMonoInstance", "PMD.ExcessiveClassLength"}) @ExtendWith(MockitoExtension.class) @MockitoSettings(strictness = Strictness.LENIENT) class CloudFoundryAppDeployerTest { @@ -352,6 +353,52 @@ class CloudFoundryAppDeployerTest { then(operationsApplications).should().pushManifest(argThat(matchesManifest(expectedManifest))); } + @Test + void deployAppWithRoutesAndMutuallyExclusiveProperties() { + deploymentProperties.setCount(3); + deploymentProperties.setMemory("2G"); + deploymentProperties.setDisk("3G"); + deploymentProperties.setBuildpack("buildpack1"); + deploymentProperties.setHealthCheck(ApplicationHealthCheck.HTTP); + deploymentProperties.setHealthCheckHttpEndpoint("/healthcheck1"); + deploymentProperties.setHost("host1"); + deploymentProperties.setDomain("domain1"); + deploymentProperties.setDomains(singleton("domain2")); + + DeployApplicationRequest request = DeployApplicationRequest.builder() + .name(APP_NAME) + .path(APP_PATH) + .serviceInstanceId(SERVICE_INSTANCE_ID) + .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, "domain1") + .property(CloudFoundryDeploymentProperties.DOMAINS_PROPERTY, "domain2") + .property(CloudFoundryDeploymentProperties.ROUTES_PROPERTY, "route.domain3") + .build(); + + StepVerifier.create(appDeployer.deploy(request)) + .assertNext(response -> assertThat(response.getName()).isEqualTo(APP_NAME)) + .verifyComplete(); + + ApplicationManifest expectedManifest = baseManifestWithSpringAppJson() + .name(APP_NAME) + .path(new File(APP_PATH).toPath()) + .instances(5) + .memory(4096) + .disk(5120) + .healthCheckType(ApplicationHealthCheck.PORT) + .healthCheckHttpEndpoint("/healthcheck2") + .buildpack("buildpack2") + .routes(Route.builder().route("route.domain3").build()) + .build(); + + then(operationsApplications).should().pushManifest(argThat(matchesManifest(expectedManifest))); + } + @Test void deployAppWithEnvironmentUsingSpringAppJson() { deploymentProperties.setUseSpringApplicationJson(true); diff --git a/spring-cloud-app-broker-deployer-cloudfoundry/src/test/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryAppDeployerUpdateApplicationTest.java b/spring-cloud-app-broker-deployer-cloudfoundry/src/test/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryAppDeployerUpdateApplicationTest.java index e1fcb56..841735e 100644 --- a/spring-cloud-app-broker-deployer-cloudfoundry/src/test/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryAppDeployerUpdateApplicationTest.java +++ b/spring-cloud-app-broker-deployer-cloudfoundry/src/test/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryAppDeployerUpdateApplicationTest.java @@ -17,8 +17,11 @@ package org.springframework.cloud.appbroker.deployer.cloudfoundry; import java.util.HashMap; +import java.util.List; import java.util.Map; +import java.util.stream.IntStream; +import org.assertj.core.api.SoftAssertions; import org.cloudfoundry.client.CloudFoundryClient; import org.cloudfoundry.client.v2.Metadata; import org.cloudfoundry.client.v2.applications.ApplicationsV2; @@ -83,6 +86,7 @@ import static org.mockito.ArgumentMatchers.anyMap; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.BDDMockito.given; import static org.mockito.BDDMockito.then; +import static org.mockito.Mockito.times; import static org.springframework.cloud.appbroker.deployer.DeploymentProperties.TARGET_PROPERTY_KEY; @ExtendWith(MockitoExtension.class) @@ -359,26 +363,7 @@ class CloudFoundryAppDeployerUpdateApplicationTest { @Test void updateAppWithHostAndDomain() { - given(applicationsV2.update(any())) - .willReturn(Mono.just(UpdateApplicationResponse.builder() - .build())); - - given(domains.list()).willReturn(getDomains()); - - given(spaces.get(any())).willReturn( - Mono.just( - SpaceDetail.builder() - .id("space-id") - .name("space-name") - .organization("org-name") - .build())); - given(routes.create(any())) - .willReturn(Mono.just(CreateRouteResponse.builder() - .metadata(Metadata.builder() - .id("route-id") - .build()).build())); - given(applicationsV2.associateRoute(any())) - .willReturn(Mono.empty()); + mockDomainsAndRoutesUpdate(); Map properties = new HashMap<>(); properties.put("host", "my.host"); @@ -402,23 +387,78 @@ class CloudFoundryAppDeployerUpdateApplicationTest { .build()); then(applicationsV2).should().associateRoute(AssociateApplicationRouteRequest.builder() .applicationId(APP_ID) - .routeId("route-id") + .routeId("myDomainComId-route") .build()); } + @Test + void updateAppWithHostDomainAndDomains() { + mockDomainsAndRoutesUpdate(); + given(domains.list()).willReturn(Flux.fromStream(IntStream.range(1, 4) + .mapToObj(i -> Domain.builder().name("domain" + i).id("domain" + i + "-id").status(Status.OWNED).build()))); + + Map properties = new HashMap<>(); + properties.put("host", "foo"); + properties.put("domain", "domain1"); + properties.put("domains", "domain2,domain3"); + UpdateApplicationRequest request = UpdateApplicationRequest.builder() + .name(APP_NAME) + .properties(properties) + .build(); + + StepVerifier.create( + appDeployer.update(request)) + .assertNext(response -> assertThat(response.getName()).isEqualTo(APP_NAME)) + .verifyComplete(); + + ArgumentCaptor createRouteRequestArgumentCaptor = ArgumentCaptor.forClass(CreateRouteRequest.class); + then(routes).should(times(3)).create(createRouteRequestArgumentCaptor.capture()); + + List routeRequests = createRouteRequestArgumentCaptor.getAllValues(); + SoftAssertions softAssertions = new SoftAssertions(); + softAssertions.assertThat(routeRequests.get(0).getHost()).isEqualTo("foo"); + softAssertions.assertThat(routeRequests.get(0).getDomainId()).isEqualTo("domain1-id"); + softAssertions.assertThat(routeRequests.get(1).getHost()).isEqualTo("foo"); + softAssertions.assertThat(routeRequests.get(1).getDomainId()).isEqualTo("domain2-id"); + softAssertions.assertThat(routeRequests.get(2).getHost()).isEqualTo("foo"); + softAssertions.assertThat(routeRequests.get(2).getDomainId()).isEqualTo("domain3-id"); + softAssertions.assertAll(); + } + + @Test + void updateAppWithRoutes() { + mockDomainsAndRoutesUpdate(); + + given(domains.list()).willReturn(Flux.fromStream(IntStream.range(1, 4) + .mapToObj(i -> Domain.builder().name("domain" + i).id("domain" + i + "-id").status(Status.OWNED).build()))); + + Map properties = singletonMap("routes", "foo.domain1,bar.domain2"); + + UpdateApplicationRequest request = UpdateApplicationRequest.builder() + .name(APP_NAME) + .properties(properties) + .build(); + + StepVerifier.create( + appDeployer.update(request)) + .assertNext(response -> assertThat(response.getName()).isEqualTo(APP_NAME)) + .verifyComplete(); + + ArgumentCaptor createRouteRequestArgumentCaptor = ArgumentCaptor.forClass(CreateRouteRequest.class); + then(routes).should(times(2)).create(createRouteRequestArgumentCaptor.capture()); + + List routeRequests = createRouteRequestArgumentCaptor.getAllValues(); + SoftAssertions softAssertions = new SoftAssertions(); + softAssertions.assertThat(routeRequests.get(0).getHost()).isEqualTo("foo"); + softAssertions.assertThat(routeRequests.get(0).getDomainId()).isEqualTo("domain1-id"); + softAssertions.assertThat(routeRequests.get(1).getHost()).isEqualTo("bar"); + softAssertions.assertThat(routeRequests.get(1).getDomainId()).isEqualTo("domain2-id"); + softAssertions.assertAll(); + } + @Test void updateAppWithHostAndMergedDomains() { - given(applicationsV2.update(any())) - .willReturn(Mono.just(UpdateApplicationResponse.builder() - .build())); - - given(spaces.get(any())).willReturn(Mono.just(SpaceDetail.builder() - .id("space-id") - .name("space-name") - .organization("org-name") - .build())); - - mockDomainsAndRoutes(); + mockDomainsAndRoutesUpdate(); Map properties = new HashMap<>(); properties.put("host", "my.host"); @@ -442,17 +482,7 @@ class CloudFoundryAppDeployerUpdateApplicationTest { @Test void updateAppWithHostAndDomains() { - given(applicationsV2.update(any())) - .willReturn(Mono.just(UpdateApplicationResponse.builder() - .build())); - - given(spaces.get(any())).willReturn(Mono.just(SpaceDetail.builder() - .id("space-id") - .name("space-name") - .organization("org-name") - .build())); - - mockDomainsAndRoutes(); + mockDomainsAndRoutesUpdate(); Map properties = new HashMap<>(); properties.put("host", "my.host"); @@ -474,28 +504,7 @@ class CloudFoundryAppDeployerUpdateApplicationTest { @Test void updateAppWithHostAndNoDomain() { - given(applicationsV2.update(any())) - .willReturn(Mono.just(UpdateApplicationResponse.builder() - .build())); - - given(domains.list()) - .willReturn(getDomains()); - - given(spaces.get(any())) - .willReturn(Mono.just(SpaceDetail.builder() - .id("space-id") - .name("space-name") - .organization("org-name") - .build())); - given(routes.create(any())) - .willReturn(Mono.just(CreateRouteResponse.builder() - .metadata(Metadata - .builder() - .id("route-id") - .build()) - .build())); - given(applicationsV2.associateRoute(any())) - .willReturn(Mono.empty()); + mockDomainsAndRoutesUpdate(); Map properties = new HashMap<>(); properties.put("host", "my.host"); @@ -517,7 +526,7 @@ class CloudFoundryAppDeployerUpdateApplicationTest { .build()); then(applicationsV2).should().associateRoute(AssociateApplicationRouteRequest.builder() .applicationId(APP_ID) - .routeId("route-id") + .routeId("myDomainDefaultId-route") .build()); } @@ -671,7 +680,17 @@ class CloudFoundryAppDeployerUpdateApplicationTest { return domainId + "-route"; } - private void mockDomainsAndRoutes() { + private void mockDomainsAndRoutesUpdate() { + given(applicationsV2.update(any())) + .willReturn(Mono.just(UpdateApplicationResponse.builder() + .build())); + given(spaces.get(any())).willReturn( + Mono.just( + SpaceDetail.builder() + .id("space-id") + .name("space-name") + .organization("org-name") + .build())); given(domains.list()).willReturn(getDomains()); given(routes.create(any())).willAnswer(invocation -> { CreateRouteRequest createRouteRequest = invocation.getArgument(0);