message: feat(provider/cf): add option for user to define service creation/deletion timeout

diff:
@@ -47,7 +47,6 @@ public class ServiceInstances {
private final ServiceInstanceService api;
private final Organizations orgs;
private final Spaces spaces;
- private Duration timeout = Duration.ofSeconds(450);
private Duration pollingInterval = Duration.ofSeconds(10);
public void createServiceBindingsByName(CloudFoundryServerGroup cloudFoundryServerGroup, @Nullable List<String> serviceNames) throws CloudFoundryApiException {
@@ -133,7 +132,7 @@ public class ServiceInstances {
return services.get(0);
}
- public void destroyServiceInstance(CloudFoundrySpace space, String serviceInstanceName) throws CloudFoundryApiException {
+ public void destroyServiceInstance(CloudFoundrySpace space, String serviceInstanceName, Duration timeout) throws CloudFoundryApiException {
Resource<ServiceInstance> serviceInstance = getServiceInstance(space, serviceInstanceName);
String serviceInstanceId = serviceInstance.getMetadata().getGuid();
List<Resource<ServiceBinding>> serviceBindings = collectPageResources("service bindings",
@@ -144,11 +143,12 @@ public class ServiceInstances {
}
safelyCall(() -> api.destroyServiceInstance(serviceInstanceId));
- pollServiceInstanceStatus(serviceInstanceName, serviceInstanceId, DELETE);
+ pollServiceInstanceStatus(serviceInstanceName, serviceInstanceId, DELETE, timeout);
}
public void createServiceInstance(String newServiceInstanceName, String serviceName, String servicePlanName,
- Set<String> tags, Map<String, Object> parameters, CloudFoundrySpace space)
+ Set<String> tags, Map<String, Object> parameters, CloudFoundrySpace space,
+ Duration timeout)
throws CloudFoundryApiException, ResourceNotFoundException {
List<CloudFoundryServicePlan> cloudFoundryServicePlans = findAllServicePlansByServiceName(serviceName);
@@ -190,11 +190,11 @@ public class ServiceInstances {
}
}
- pollServiceInstanceStatus(newServiceInstanceName, guid, type);
+ pollServiceInstanceStatus(newServiceInstanceName, guid, type, timeout);
}
// package-level for testing visibility
- void pollServiceInstanceStatus(String serviceInstanceName, String guid, LastOperation.Type type) {
+ void pollServiceInstanceStatus(String serviceInstanceName, String guid, LastOperation.Type type, Duration timeout) {
RetryConfig retryConfig = RetryConfig.custom()
.waitDuration(pollingInterval)
.maxAttempts((int) (timeout.getSeconds() / pollingInterval.getSeconds()))

diff:
+/*
+ * Copyright 2018 Pivotal, Inc.
+ *
+ * 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 com.netflix.spinnaker.clouddriver.cloudfoundry.deploy.description;
+
+import com.netflix.spinnaker.clouddriver.cloudfoundry.model.CloudFoundrySpace;
+import lombok.Data;
+import lombok.EqualsAndHashCode;
+
+import java.time.Duration;
+
+@Data
+@EqualsAndHashCode(callSuper = true)
+public abstract class AbstractCloudFoundryServiceDescription extends AbstractCloudFoundryDescription {
+ private String serviceName;
+ private CloudFoundrySpace space;
+ private long timeout = 450;
+
+ public Duration getTimeout() {
+ return Duration.ofSeconds(timeout);
+ }
+}

diff:
@@ -24,7 +24,7 @@ import java.util.Set;
@Data
@EqualsAndHashCode(callSuper = true)
-public class DeployCloudFoundryServiceDescription extends DestroyCloudFoundryServiceDescription {
+public class DeployCloudFoundryServiceDescription extends AbstractCloudFoundryServiceDescription {
String service;
String servicePlan;
Set<String> tags;

diff:
package com.netflix.spinnaker.clouddriver.cloudfoundry.deploy.description;
-import com.netflix.spinnaker.clouddriver.cloudfoundry.model.CloudFoundrySpace;
import lombok.Data;
import lombok.EqualsAndHashCode;
@Data
@EqualsAndHashCode(callSuper = true)
-public class DestroyCloudFoundryServiceDescription extends AbstractCloudFoundryDescription {
- String serviceName;
- CloudFoundrySpace space;
+public class DestroyCloudFoundryServiceDescription extends AbstractCloudFoundryServiceDescription {
}

diff:
@@ -45,7 +45,8 @@ public class DeployCloudFoundryServiceAtomicOperation implements AtomicOperation
description.getServicePlan(),
description.getTags(),
description.getParameterMap(),
- description.getSpace());
+ description.getSpace(),
+ description.getTimeout());
task.updateStatus(PHASE, "Created service instance '" + description.getServiceName() + "'");
return null;
}

diff:
@@ -39,7 +39,7 @@ public class DestroyCloudFoundryServiceAtomicOperation implements AtomicOperatio
task.updateStatus(PHASE, "Initializing the removal of service instance '" + description.getServiceName() + "' from space " + description.getSpace().getName());
description.getClient()
.getServiceInstances()
- .destroyServiceInstance(description.getSpace(), description.getServiceName());
+ .destroyServiceInstance(description.getSpace(), description.getServiceName(), description.getTimeout());
task.updateStatus(PHASE, "Done removing service instance '" + description.getServiceName() + "'");
return null;
}

diff:
@@ -51,7 +51,7 @@ public class ServiceInstancesTest {
CloudFoundryConfigurationProperties configProps = new CloudFoundryConfigurationProperties();
configProps.setAsyncOperationTimeoutSecondsDefault(2);
configProps.setPollingIntervalSeconds(1);
- serviceInstances = new ServiceInstances(serviceInstanceService, orgs, spaces, Duration.ofSeconds(4), Duration.ofSeconds(1));
+ serviceInstances = new ServiceInstances(serviceInstanceService, orgs, spaces, Duration.ofSeconds(1));
Page<Service> serviceMappingPageOne = Page.singleton(new Service().setLabel("service1"), "service-guid");
when(serviceInstanceService.findService(any(), anyListOf(String.class))).thenReturn(serviceMappingPageOne);
@@ -124,7 +124,8 @@ public class ServiceInstancesTest {
"ServicePlan1",
Collections.emptySet(),
null,
- cloudFoundrySpace);
+ cloudFoundrySpace,
+ Duration.ofSeconds(4));
verify(serviceInstanceService, times(1)).createServiceInstance(any());
verify(serviceInstanceService, never()).updateServiceInstance(any(), any());
@@ -144,7 +145,8 @@ public class ServiceInstancesTest {
"ServicePlan1",
Collections.emptySet(),
null,
- cloudFoundrySpace);
+ cloudFoundrySpace,
+ Duration.ofSeconds(4));
verify(serviceInstanceService, times(1)).createServiceInstance(any());
verify(serviceInstanceService, never()).getServiceInstanceById("service-instance-guid");
@@ -164,7 +166,8 @@ public class ServiceInstancesTest {
"servicePlanName",
Collections.emptySet(),
null,
- cloudFoundrySpace);
+ cloudFoundrySpace,
+ Duration.ofSeconds(4));
}
@Test
@@ -185,7 +188,8 @@ public class ServiceInstancesTest {
"ServicePlan1",
Collections.emptySet(),
null,
- cloudFoundrySpace);
+ cloudFoundrySpace,
+ Duration.ofSeconds(4));
verify(serviceInstanceService, times(1)).createServiceInstance(any());
verify(serviceInstanceService, times(1)).updateServiceInstance(any(), any());
@@ -210,7 +214,8 @@ public class ServiceInstancesTest {
"ServicePlan1",
Collections.emptySet(),
null,
- cloudFoundrySpace);
+ cloudFoundrySpace,
+ Duration.ofSeconds(4));
verify(serviceInstanceService, times(1)).createServiceInstance(any());
verify(serviceInstanceService, times(1)).updateServiceInstance(any(), any());
@@ -241,7 +246,8 @@ public class ServiceInstancesTest {
"ServicePlan1",
Collections.emptySet(),
null,
- cloudFoundrySpace);
+ cloudFoundrySpace,
+ Duration.ofSeconds(4));
}
@Test
@@ -312,7 +318,7 @@ public class ServiceInstancesTest {
when(serviceInstanceService.destroyServiceInstance(any())).thenReturn(new Response("url", 202, "reason", Collections.emptyList(), null));
when(serviceInstanceService.getServiceInstanceById(any())).thenReturn(pollingServiceInstanceResource).thenThrow(retrofitErrorNotFound);
- serviceInstances.destroyServiceInstance(cloudFoundrySpace, "newServiceInstanceName");
+ serviceInstances.destroyServiceInstance(cloudFoundrySpace, "newServiceInstanceName", Duration.ofSeconds(4));
verify(serviceInstanceService, times(1)).findAllServiceInstancesBySpaceId(any(), any(), anyListOf(String.class));
verify(serviceInstanceService, times(1)).destroyServiceInstance(any());
@@ -337,7 +343,7 @@ public class ServiceInstancesTest {
when(serviceInstanceService.destroyServiceInstance(any())).thenThrow(destroyFailed);
try {
- serviceInstances.destroyServiceInstance(cloudFoundrySpace, "serviceInstanceName");
+ serviceInstances.destroyServiceInstance(cloudFoundrySpace, "serviceInstanceName", Duration.ofSeconds(4));
fail("Expected CloudFoundryApiException");
} catch(CloudFoundryApiException cfe) {
// expected behavior
@@ -356,7 +362,7 @@ public class ServiceInstancesTest {
.thenReturn(Page.singleton(new ServiceBinding(), "service-binding-guid"));
try {
- serviceInstances.destroyServiceInstance(cloudFoundrySpace, "serviceInstanceName");
+ serviceInstances.destroyServiceInstance(cloudFoundrySpace, "serviceInstanceName", Duration.ofSeconds(4));
fail("Expected CloudFoundryApiException");
} catch(CloudFoundryApiException cfe) {
// expected behavior
@@ -379,7 +385,7 @@ public class ServiceInstancesTest {
when(serviceInstanceService.getServiceInstanceById(any()))
.thenReturn(polledServiceInstanceResource, polledServiceInstanceResource, succeededServiceInstanceResource);
- serviceInstances.pollServiceInstanceStatus("new-service-instance-name", "service-instance-guid", DELETE);
+ serviceInstances.pollServiceInstanceStatus("new-service-instance-name", "service-instance-guid", DELETE, Duration.ofSeconds(4));
verify(serviceInstanceService, times(3)).getServiceInstanceById("service-instance-guid");
}
@@ -393,7 +399,7 @@ public class ServiceInstancesTest {
when(serviceInstanceService.getServiceInstanceById(any())).thenReturn(polledServiceInstanceResource);
try {
- serviceInstances.pollServiceInstanceStatus("service-instance-name", "service-instance-guid", DELETE);
+ serviceInstances.pollServiceInstanceStatus("service-instance-name", "service-instance-guid", DELETE, Duration.ofSeconds(4));
fail("Expected CloudFoundryApiException");
} catch (CloudFoundryApiException cfe) {
assertThat(cfe.getMessage()).contains("Service instance 'service-instance-name' DELETE did not complete");
@@ -415,7 +421,7 @@ public class ServiceInstancesTest {
when(serviceInstanceService.getServiceInstanceById(any())).thenReturn(polledServiceInstanceResource, polledServiceInstanceResource, failedServiceInstanceResource);
try {
- serviceInstances.pollServiceInstanceStatus("service-instance-name", "service-instance-guid", UPDATE);
+ serviceInstances.pollServiceInstanceStatus("service-instance-name", "service-instance-guid", UPDATE, Duration.ofSeconds(4));
fail("Expected CloudFoundryApiException");
} catch (CloudFoundryApiException cfe) {
assertThat(cfe.getMessage()).contains("Service instance 'service-instance-name' UPDATE failed");


message: fix(provider/cf): correct pagination logic when iterating over multiple pages

change_type: MODIFY

diff:
@@ -55,7 +55,7 @@ final class CloudFoundryClientUtils {
.orElseThrow(() -> new CloudFoundryApiException("Unable to retrieve " + resourceNamePluralized));
List<R> allResources = new ArrayList<>(firstPage.getResources());
- for (int page = 2; page < Math.max(2, firstPage.getPagination().getTotalPages()); page++) {
+ for (int page = 2; page <= Math.max(2, firstPage.getPagination().getTotalPages()); page++) {
final int p = page;
allResources.addAll(safelyCall(() -> fetchPage.apply(p))
.orElseThrow(() -> new CloudFoundryApiException("Unable to retrieve " + resourceNamePluralized))
@@ -70,7 +70,7 @@ final class CloudFoundryClientUtils {
.orElseThrow(() -> new CloudFoundryApiException("Unable to retrieve " + resourceNamePluralized));
List<Resource<R>> allResources = new ArrayList<>(firstPage.getResources());
- for (int page = 2; page < Math.max(2, firstPage.getTotalPages()); page++) {
+ for (int page = 2; page <= Math.max(2, firstPage.getTotalPages()); page++) {
final int p = page;
allResources.addAll(safelyCall(() -> fetchPage.apply(p))
.orElseThrow(() -> new CloudFoundryApiException("Unable to retrieve " + resourceNamePluralized))

change_type: ADD

diff:
+/*
+ * Copyright 2018 Pivotal, Inc.
+ *
+ * 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 com.netflix.spinnaker.clouddriver.cloudfoundry.client;
+
+import com.netflix.spinnaker.clouddriver.cloudfoundry.client.api.ApplicationService;
+import com.netflix.spinnaker.clouddriver.cloudfoundry.client.api.DomainService;
+import com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.v2.Domain;
+import com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.v2.Page;
+import com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.v3.Application;
+import com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.v3.Pagination;
+import org.junit.jupiter.api.Test;
+
+import java.util.Collections;
+import java.util.List;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+class CloudFoundryClientUtilsTest {
+
+ @Test
+ void collectPagesIteratesOverMultiplePages() {
+ ApplicationService applicationService = mock(ApplicationService.class);
+ Application applicationOne = new Application().setName("app-name-one");
+ List pageOneResources = Collections.singletonList(applicationOne);
+ Pagination<Application> pageOne = new Pagination<>();
+ pageOne.setPagination(new Pagination.Details().setTotalPages(2));
+ pageOne.setResources(pageOneResources);
+ Application applicationTwo = new Application().setName("app-name-two");
+ List pageTwoResources = Collections.singletonList(applicationTwo);
+ Pagination<Application> pageTwo = new Pagination<>();
+ pageTwo.setPagination(new Pagination.Details().setTotalPages(2));
+ pageTwo.setResources(pageTwoResources);
+
+ when(applicationService.all(null, null, null)).thenReturn(pageOne);
+ when(applicationService.all(2, null, null)).thenReturn(pageTwo);
+
+ List results = CloudFoundryClientUtils.collectPages("applications", page -> applicationService.all(page, null, null));
+
+ assertThat(results).containsExactly(applicationOne, applicationTwo);
+ }
+
+ @Test
+ void collectPageResourcesIteratesOverMultiplePages() {
+ DomainService domainService = mock(DomainService.class);
+ Domain domainOne = new Domain().setName("domain-name-one");
+ Page pageOne = Page.singleton(domainOne, "domain-one-guid");
+ Domain domainTwo = new Domain().setName("domain-name-two");
+ Page pageTwo = Page.singleton(domainTwo, "domain-two-guid");
+
+ when(domainService.allShared(null)).thenReturn(pageOne);
+ when(domainService.allShared(2)).thenReturn(pageTwo);
+
+ List results = CloudFoundryClientUtils.collectPageResources("shared domains", domainService::allShared);
+
+ assertThat(results).containsExactly(pageOne.getResources().get(0), pageTwo.getResources().get(0));
+ }
+}

license: Apache License 2.0

repo: spinnaker/clouddriver


diff:
@@ -50,6 +50,7 @@ import static java.util.stream.Collectors.toSet;
@Slf4j
public class Applications {
private final String account;
+ private final String appsManagerUri;
private final ApplicationService api;
private final Spaces spaces;
@@ -209,6 +210,7 @@ public class Applications {
return CloudFoundryServerGroup.builder()
.account(account)
+ .appsManagerUri(appsManagerUri)
.name(application.getName())
.id(application.getGuid())
.memory(process != null ? process.getMemoryInMb() : null)

diff:
@@ -114,7 +114,7 @@ public class HttpCloudFoundryClient implements CloudFoundryClient {
}
}
- public HttpCloudFoundryClient(String account, String apiHost, String user, String password) {
+ public HttpCloudFoundryClient(String account, String appsManagerUri, String apiHost, String user, String password) {
this.apiHost = apiHost;
this.user = user;
this.password = password;
@@ -165,7 +165,7 @@ public class HttpCloudFoundryClient implements CloudFoundryClient {
this.organizations = new Organizations(createService(OrganizationService.class));
this.spaces = new Spaces(createService(SpaceService.class), organizations);
- this.applications = new Applications(account, createService(ApplicationService.class), spaces);
+ this.applications = new Applications(account, appsManagerUri, createService(ApplicationService.class), spaces);
this.domains = new Domains(createService(DomainService.class), organizations);
this.serviceInstances = new ServiceInstances(createService(ServiceInstanceService.class), organizations, spaces);
this.routes = new Routes(account, createService(RouteService.class), applications, domains, spaces);

diff:
@@ -46,6 +46,7 @@ public class CloudFoundryConfigurationProperties {
public static class ManagedAccount {
private String name;
private String api;
+ private String appsManagerUri;
private String user;
private String password;
private String environment;

diff:
@@ -28,13 +28,13 @@ import com.netflix.spinnaker.clouddriver.model.Image;
import com.netflix.spinnaker.clouddriver.model.ServerGroup;
import com.netflix.spinnaker.clouddriver.names.NamerRegistry;
import com.netflix.spinnaker.moniker.Moniker;
+import io.vavr.collection.HashMap;
import lombok.Builder;
import lombok.EqualsAndHashCode;
import lombok.Value;
import lombok.experimental.Wither;
import javax.annotation.Nullable;
-import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -54,6 +54,9 @@ public class CloudFoundryServerGroup extends CloudFoundryModel implements Server
@JsonView(Views.Cache.class)
String account;
+ @JsonView(Views.Cache.class)
+ String appsManagerUri;
+
@JsonView(Views.Cache.class)
String name;
@@ -146,11 +149,12 @@ public class CloudFoundryServerGroup extends CloudFoundryModel implements Server
}
public Map getBuildInfo() {
- Map<String, Object> buildInfo = new HashMap<>();
- buildInfo.put("droplet", droplet);
- buildInfo.put("serviceInstances", serviceInstances);
- buildInfo.put("id", id);
- return buildInfo;
+ return HashMap.<Object, Object>of(
+ "appsManagerUri", appsManagerUri,
+ "droplet", droplet,
+ "id", id,
+ "serviceInstances", serviceInstances)
+ .toJavaMap();
}
@Deprecated

diff:
@@ -47,10 +47,10 @@ public class CloudFoundryCredentials implements AccountCredentials<CloudFoundryC
private final CloudFoundryClient credentials;
- public CloudFoundryCredentials(String name, String apiHost, String userName, String password, String environment) {
+ protected CloudFoundryCredentials(String name, String appsManagerUri, String apiHost, String userName, String password, String environment) {
this.name = name;
this.environment = environment;
- this.credentials = new HttpCloudFoundryClient(name, apiHost, userName, password);
+ this.credentials = new HttpCloudFoundryClient(name, appsManagerUri, apiHost, userName, password);
}
public CloudFoundryClient getClient() {

diff:
@@ -65,6 +65,7 @@ public class CloudFoundryCredentialsInitializer implements CredentialsInitialize
for (CloudFoundryConfigurationProperties.ManagedAccount managedAccount : accountsToAdd) {
CloudFoundryCredentials cloudFoundryAccountCredentials = new CloudFoundryCredentials(
managedAccount.getName(),
+ managedAccount.getAppsManagerUri(),
managedAccount.getApi(),
managedAccount.getUser(),
managedAccount.getPassword(),

diff:
@@ -42,11 +42,11 @@ import static org.mockito.Mockito.*;
class ApplicationsTest {
private ApplicationService applicationService = mock(ApplicationService.class);
private Spaces spaces = mock(Spaces.class);
- private Applications apps = new Applications("pws", applicationService, spaces);
+ private Applications apps = new Applications("pws", "some-apps-man-uri", applicationService, spaces);
@Test
void errorHandling() {
- CloudFoundryClient client = new HttpCloudFoundryClient("pws", "api.run.pivotal.io",
+ CloudFoundryClient client = new HttpCloudFoundryClient("pws", "some.api.uri.example.com", "api.run.pivotal.io",
"baduser", "badpassword");
assertThatThrownBy(() -> client.getApplications().all())
@@ -123,6 +123,7 @@ class ApplicationsTest {
assertThat(cloudFoundryServerGroup).isNotNull();
assertThat(cloudFoundryServerGroup.getId()).isEqualTo("some-app-guid");
assertThat(cloudFoundryServerGroup.getName()).isEqualTo("some-app-name");
+ assertThat(cloudFoundryServerGroup.getAppsManagerUri()).isEqualTo("some-apps-man-uri");
assertThat(cloudFoundryServerGroup.getServiceInstances().size()).isEqualTo(1);
assertThat(cloudFoundryServerGroup.getServiceInstances().get(0).getTags()).containsExactly("tag1", "tag2");
}

diff:
@@ -48,7 +48,7 @@ public class HttpCloudFoundryClientTest {
fail("Should not happen!");
}
- HttpCloudFoundryClient cloudFoundryClient = new HttpCloudFoundryClient("account", "host", "user", "password");
+ HttpCloudFoundryClient cloudFoundryClient = new HttpCloudFoundryClient("account", "uri", "host", "user", "password");
Response response = cloudFoundryClient.createRetryInterceptor(chain);
try {
@@ -73,7 +73,7 @@ public class HttpCloudFoundryClientTest {
fail("Should not happen!");
}
- HttpCloudFoundryClient cloudFoundryClient = new HttpCloudFoundryClient("account", "host", "user", "password");
+ HttpCloudFoundryClient cloudFoundryClient = new HttpCloudFoundryClient("account", "uri", "host", "user", "password");
Response response = cloudFoundryClient.createRetryInterceptor(chain);
try {
@@ -98,7 +98,7 @@ public class HttpCloudFoundryClientTest {
fail("Should not happen!");
}
- HttpCloudFoundryClient cloudFoundryClient = new HttpCloudFoundryClient("account", "host", "user", "password");
+ HttpCloudFoundryClient cloudFoundryClient = new HttpCloudFoundryClient("account", "uri", "host", "user", "password");
Response response = cloudFoundryClient.createRetryInterceptor(chain);
try {

diff:
@@ -62,7 +62,7 @@ class DeployCloudFoundryServerGroupAtomicOperationConverterTest {
}
private final CloudFoundryCredentials cloudFoundryCredentials = new CloudFoundryCredentials(
- "test", "", "", "", "") {
+ "test", "", "", "", "", "") {
public CloudFoundryClient getClient() {
return cloudFoundryClient;
}

diff:
@@ -23,7 +23,7 @@ import org.junit.jupiter.api.Test;
class CloudFoundryCredentialsTest {
@Test
void serialize() throws JsonProcessingException {
- CloudFoundryCredentials creds = new CloudFoundryCredentials("pws", "api.run.pivotal.io", "fakeuser", "fakepasword", "dev");
+ CloudFoundryCredentials creds = new CloudFoundryCredentials("pws", "some.uri.example.com", "api.run.pivotal.io", "fakeuser", "fakepasword", "dev");
new ObjectMapper().writeValueAsString(creds);
}
}

original_message:
feat(provider/cf): add apps manager uri to server group model

Co-Authored-By: Stu Pollock <spollock@pivotal.io>


message:
fix(provider/cf): stop NPE on empty error response
spinnaker/spinnaker#3648

diff:
@@ -21,19 +21,26 @@ import lombok.Getter;
import javax.annotation.Nullable;
import java.util.Objects;
+import java.util.Optional;
import java.util.stream.Collectors;
import static java.util.Arrays.stream;
@Getter
public class CloudFoundryApiException extends RuntimeException {
+ private static final String UNKNOWN_ERROR = "Unknown Error";
+
@Nullable
private ErrorDescription.Code errorCode;
public CloudFoundryApiException(ErrorDescription errorCause) {
- super(getMessage(errorCause.getErrors().toArray(new String[0])));
+ super(Optional.ofNullable(errorCause)
+ .map(e -> getMessage(e.getErrors().toArray(new String[0])))
+ .orElse(UNKNOWN_ERROR));
+ if (errorCause != null) {
this.errorCode = errorCause.getCode();
}
+ }
public CloudFoundryApiException(Throwable t, String... errors) {
super(getMessage(errors), t);

diff:
+/*
+ * Copyright 2018 Pivotal, Inc.
+ *
+ * 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 com.netflix.spinnaker.clouddriver.cloudfoundry.client;
+
+import com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.ErrorDescription;
+import org.junit.jupiter.api.Test;
+
+import java.util.Arrays;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+class CloudFoundryApiExceptionTest {
+ @Test
+ void constructorWithErrorDescription() {
+ CloudFoundryApiException e = new CloudFoundryApiException(
+ new ErrorDescription().setErrors(Arrays.asList(
+ new ErrorDescription.Detail().setDetail("Main Error"),
+ new ErrorDescription.Detail().setDetail("Foo"))));
+
+ assertThat(e.getMessage()).contains("Main Error and Foo");
+ }
+
+ @Test
+ void constructorHandlesNullErrorDescription() {
+ CloudFoundryApiException e = new CloudFoundryApiException((ErrorDescription) null);
+
+ assertThat(e.getMessage()).contains("Unknown Error");
+ }
+}

original_message:
fix(provider/cf): stop NPE on empty error response (#3180)

spinnaker/spinnaker#3648

Co-Authored-By: Stu Pollock <spollock@pivotal.io>


diff:
@@ -363,6 +363,6 @@ public class Applications {
public ProcessStats.State getProcessState(String appGuid) throws CloudFoundryApiException {
return safelyCall(() -> this.api.findProcessStatsById(appGuid))
.flatMap(pr -> pr.getResources().stream().findAny().map(ProcessStats::getState))
- .orElse(null);
+ .orElse(ProcessStats.State.DOWN);
}
}

diff:
@@ -28,6 +28,9 @@ import com.netflix.spinnaker.clouddriver.cloudfoundry.model.CloudFoundryServerGr
import com.netflix.spinnaker.clouddriver.cloudfoundry.model.CloudFoundrySpace;
import io.vavr.collection.HashMap;
import org.junit.jupiter.api.Test;
+import retrofit.RetrofitError;
+import retrofit.client.Response;
+import retrofit.mime.TypedInput;
import java.time.ZonedDateTime;
import java.util.Arrays;
@@ -148,4 +151,20 @@ class ApplicationsTest {
verify(applicationService).updateProcess("guid1", new UpdateProcess("command1", null));
}
+ @Test
+ void getProcessState(){
+ ProcessStats processStats = new ProcessStats().setState(ProcessStats.State.RUNNING);
+ ProcessResources processResources = new ProcessResources().setResources(Collections.singletonList(processStats));
+ when(applicationService.findProcessStatsById(anyString())).thenReturn(processResources);
+ ProcessStats.State result = apps.getProcessState("some-app-guid");
+ assertThat(result).isEqualTo(ProcessStats.State.RUNNING);
+ }
+
+ @Test
+ void getProcessStateWhenStatsNotFound(){
+ Response errorResponse = new Response("http://capi.io", 404,"Not Found", Collections.EMPTY_LIST, null);
+ when(applicationService.findProcessStatsById(anyString())).thenThrow(RetrofitError.httpError("http://capi.io", errorResponse, null, null));
+ ProcessStats.State result = apps.getProcessState("some-app-guid");
+ assertThat(result).isEqualTo(ProcessStats.State.DOWN);
+ }
}

original_message: fix(provider/cf): set the status to DOWN when no process stats returned (#3186)


message: fix(provider/cf): correct pagination logic when iterating

diff:
@@ -55,7 +55,7 @@ final class CloudFoundryClientUtils {
.orElseThrow(() -> new CloudFoundryApiException("Unable to retrieve " + resourceNamePluralized));
List<R> allResources = new ArrayList<>(firstPage.getResources());
- for (int page = 2; page <= Math.max(2, firstPage.getPagination().getTotalPages()); page++) {
+ for (int page = 2; page <= firstPage.getPagination().getTotalPages(); page++) {
final int p = page;
allResources.addAll(safelyCall(() -> fetchPage.apply(p))
.orElseThrow(() -> new CloudFoundryApiException("Unable to retrieve " + resourceNamePluralized))
@@ -70,7 +70,7 @@ final class CloudFoundryClientUtils {
.orElseThrow(() -> new CloudFoundryApiException("Unable to retrieve " + resourceNamePluralized));
List<Resource<R>> allResources = new ArrayList<>(firstPage.getResources());
- for (int page = 2; page <= Math.max(2, firstPage.getTotalPages()); page++) {
+ for (int page = 2; page <= firstPage.getTotalPages(); page++) {
final int p = page;
allResources.addAll(safelyCall(() -> fetchPage.apply(p))
.orElseThrow(() -> new CloudFoundryApiException("Unable to retrieve " + resourceNamePluralized))

diff:
@@ -33,6 +33,22 @@ import static org.mockito.Mockito.when;
class CloudFoundryClientUtilsTest {
+ @Test
+ void collectPagesIteratesOverOnePage() {
+ ApplicationService applicationService = mock(ApplicationService.class);
+ Application applicationOne = new Application().setName("app-name-one");
+ List pageOneResources = Collections.singletonList(applicationOne);
+ Pagination<Application> pageOne = new Pagination<>();
+ pageOne.setPagination(new Pagination.Details().setTotalPages(1));
+ pageOne.setResources(pageOneResources);
+
+ when(applicationService.all(null, null, null)).thenReturn(pageOne);
+
+ List results = CloudFoundryClientUtils.collectPages("applications", page -> applicationService.all(page, null, null));
+
+ assertThat(results).containsExactly(applicationOne);
+ }
+
@Test
void collectPagesIteratesOverMultiplePages() {
ApplicationService applicationService = mock(ApplicationService.class);
@@ -55,13 +71,27 @@ class CloudFoundryClientUtilsTest {
assertThat(results).containsExactly(applicationOne, applicationTwo);
}
+
+ @Test
+ void collectPageResourcesIteratesOverOnePage() {
+ DomainService domainService = mock(DomainService.class);
+ Domain domainOne = new Domain().setName("domain-name-one");
+ Page pageOne = Page.singleton(domainOne, "domain-one-guid").setTotalPages(1).setTotalResults(1);
+
+ when(domainService.allShared(null)).thenReturn(pageOne);
+
+ List results = CloudFoundryClientUtils.collectPageResources("shared domains", domainService::allShared);
+
+ assertThat(results).containsExactly(pageOne.getResources().get(0));
+ }
+
@Test
void collectPageResourcesIteratesOverMultiplePages() {
DomainService domainService = mock(DomainService.class);
Domain domainOne = new Domain().setName("domain-name-one");
- Page pageOne = Page.singleton(domainOne, "domain-one-guid");
+ Page pageOne = Page.singleton(domainOne, "domain-one-guid").setTotalPages(2).setTotalResults(2);
Domain domainTwo = new Domain().setName("domain-name-two");
- Page pageTwo = Page.singleton(domainTwo, "domain-two-guid");
+ Page pageTwo = Page.singleton(domainTwo, "domain-two-guid").setTotalPages(2).setTotalResults(2);
when(domainService.allShared(null)).thenReturn(pageOne);
when(domainService.allShared(2)).thenReturn(pageTwo);


diff:
@@ -216,9 +216,14 @@ public class Applications {
}
}
+ String serverGroupAppManagerUri = appsManagerUri;
+ if (StringUtils.isNotEmpty(appsManagerUri)){
+ serverGroupAppManagerUri = appsManagerUri + "/organizations/" + space.getOrganization().getId() + "/spaces/" + space.getId() + "/applications/" + application.getGuid();
+ }
+
return CloudFoundryServerGroup.builder()
.account(account)
- .appsManagerUri(appsManagerUri)
+ .appsManagerUri(serverGroupAppManagerUri)
.name(application.getName())
.id(application.getGuid())
.memory(process != null ? process.getMemoryInMb() : null)

diff:
@@ -126,7 +126,7 @@ class ApplicationsTest {
assertThat(cloudFoundryServerGroup).isNotNull();
assertThat(cloudFoundryServerGroup.getId()).isEqualTo("some-app-guid");
assertThat(cloudFoundryServerGroup.getName()).isEqualTo("some-app-name");
- assertThat(cloudFoundryServerGroup.getAppsManagerUri()).isEqualTo("some-apps-man-uri");
+ assertThat(cloudFoundryServerGroup.getAppsManagerUri()).isEqualTo("some-apps-man-uri/organizations/org-id/spaces/space-id/applications/some-app-guid");
assertThat(cloudFoundryServerGroup.getServiceInstances().size()).isEqualTo(1);
assertThat(cloudFoundryServerGroup.getServiceInstances().get(0).getTags()).containsExactly("tag1", "tag2");
}

original_message: feat(provider/cf): update the appsManagerUri to the specific serverGroup (#3192)


diff:
@@ -51,6 +51,7 @@ import static java.util.stream.Collectors.toSet;
public class Applications {
private final String account;
private final String appsManagerUri;
+ private final String metricsUri;
private final ApplicationService api;
private final Spaces spaces;
@@ -221,9 +222,15 @@ public class Applications {
serverGroupAppManagerUri = appsManagerUri + "/organizations/" + space.getOrganization().getId() + "/spaces/" + space.getId() + "/applications/" + application.getGuid();
}
+ String serverGroupMetricsUri = metricsUri;
+ if (StringUtils.isNotEmpty(metricsUri)){
+ serverGroupMetricsUri = metricsUri + "/apps/" + application.getGuid();
+ }
+
return CloudFoundryServerGroup.builder()
.account(account)
.appsManagerUri(serverGroupAppManagerUri)
+ .metricsUri(serverGroupMetricsUri)
.name(application.getName())
.id(application.getGuid())
.memory(process != null ? process.getMemoryInMb() : null)

diff:
@@ -114,7 +114,7 @@ public class HttpCloudFoundryClient implements CloudFoundryClient {
}
}
- public HttpCloudFoundryClient(String account, String appsManagerUri, String apiHost, String user, String password) {
+ public HttpCloudFoundryClient(String account, String appsManagerUri, String metricsUri, String apiHost, String user, String password) {
this.apiHost = apiHost;
this.user = user;
this.password = password;
@@ -165,7 +165,7 @@ public class HttpCloudFoundryClient implements CloudFoundryClient {
this.organizations = new Organizations(createService(OrganizationService.class));
this.spaces = new Spaces(createService(SpaceService.class), organizations);
- this.applications = new Applications(account, appsManagerUri, createService(ApplicationService.class), spaces);
+ this.applications = new Applications(account, appsManagerUri, metricsUri, createService(ApplicationService.class), spaces);
this.domains = new Domains(createService(DomainService.class), organizations);
this.serviceInstances = new ServiceInstances(createService(ServiceInstanceService.class), organizations, spaces);
this.routes = new Routes(account, createService(RouteService.class), applications, domains, spaces);

diff:
@@ -47,6 +47,7 @@ public class CloudFoundryConfigurationProperties {
private String name;
private String api;
private String appsManagerUri;
+ private String metricsUri;
private String user;
private String password;
private String environment;

diff:
@@ -57,6 +57,9 @@ public class CloudFoundryServerGroup extends CloudFoundryModel implements Server
@JsonView(Views.Cache.class)
String appsManagerUri;
+ @JsonView(Views.Cache.class)
+ String metricsUri;
+
@JsonView(Views.Cache.class)
String name;
@@ -159,6 +162,7 @@ public class CloudFoundryServerGroup extends CloudFoundryModel implements Server
public Map getBuildInfo() {
return HashMap.<Object, Object>of(
"appsManagerUri", appsManagerUri,
+ "metricsUri", metricsUri,
"droplet", droplet,
"id", id,
"serviceInstances", serviceInstances)

diff:
@@ -47,10 +47,10 @@ public class CloudFoundryCredentials implements AccountCredentials<CloudFoundryC
private final CloudFoundryClient credentials;
- protected CloudFoundryCredentials(String name, String appsManagerUri, String apiHost, String userName, String password, String environment) {
+ protected CloudFoundryCredentials(String name, String appsManagerUri, String metricsUri, String apiHost, String userName, String password, String environment) {
this.name = name;
this.environment = environment;
- this.credentials = new HttpCloudFoundryClient(name, appsManagerUri, apiHost, userName, password);
+ this.credentials = new HttpCloudFoundryClient(name, appsManagerUri, metricsUri, apiHost, userName, password);
}
public CloudFoundryClient getClient() {

diff:
@@ -66,6 +66,7 @@ public class CloudFoundryCredentialsInitializer implements CredentialsInitialize
CloudFoundryCredentials cloudFoundryAccountCredentials = new CloudFoundryCredentials(
managedAccount.getName(),
managedAccount.getAppsManagerUri(),
+ managedAccount.getMetricsUri(),
managedAccount.getApi(),
managedAccount.getUser(),
managedAccount.getPassword(),

diff:
@@ -45,12 +45,12 @@ import static org.mockito.Mockito.*;
class ApplicationsTest {
private ApplicationService applicationService = mock(ApplicationService.class);
private Spaces spaces = mock(Spaces.class);
- private Applications apps = new Applications("pws", "some-apps-man-uri", applicationService, spaces);
+ private Applications apps = new Applications("pws", "some-apps-man-uri", "some-metrics-uri", applicationService, spaces);
@Test
void errorHandling() {
- CloudFoundryClient client = new HttpCloudFoundryClient("pws", "some.api.uri.example.com", "api.run.pivotal.io",
- "baduser", "badpassword");
+ CloudFoundryClient client = new HttpCloudFoundryClient("pws", "some.api.uri.example.com", "some-metrics-uri",
+ "api.run.pivotal.io", "baduser", "badpassword");
assertThatThrownBy(() -> client.getApplications().all())
.isInstanceOf(CloudFoundryApiException.class);
@@ -127,6 +127,7 @@ class ApplicationsTest {
assertThat(cloudFoundryServerGroup.getId()).isEqualTo("some-app-guid");
assertThat(cloudFoundryServerGroup.getName()).isEqualTo("some-app-name");
assertThat(cloudFoundryServerGroup.getAppsManagerUri()).isEqualTo("some-apps-man-uri/organizations/org-id/spaces/space-id/applications/some-app-guid");
+ assertThat(cloudFoundryServerGroup.getMetricsUri()).isEqualTo("some-metrics-uri/apps/some-app-guid");
assertThat(cloudFoundryServerGroup.getServiceInstances().size()).isEqualTo(1);
assertThat(cloudFoundryServerGroup.getServiceInstances().get(0).getTags()).containsExactly("tag1", "tag2");
}

diff:
@@ -48,7 +48,7 @@ class HttpCloudFoundryClientTest {
fail("Should not happen!");
}
- HttpCloudFoundryClient cloudFoundryClient = new HttpCloudFoundryClient("account", "uri", "host", "user", "password");
+ HttpCloudFoundryClient cloudFoundryClient = new HttpCloudFoundryClient("account", "appsManUri", "metricsUri","host", "user", "password");
Response response = cloudFoundryClient.createRetryInterceptor(chain);
try {
@@ -73,7 +73,7 @@ class HttpCloudFoundryClientTest {
fail("Should not happen!");
}
- HttpCloudFoundryClient cloudFoundryClient = new HttpCloudFoundryClient("account", "uri", "host", "user", "password");
+ HttpCloudFoundryClient cloudFoundryClient = new HttpCloudFoundryClient("account", "appsManUri", "metricsUri", "host", "user", "password");
Response response = cloudFoundryClient.createRetryInterceptor(chain);
try {
@@ -98,7 +98,7 @@ class HttpCloudFoundryClientTest {
fail("Should not happen!");
}
- HttpCloudFoundryClient cloudFoundryClient = new HttpCloudFoundryClient("account", "uri", "host", "user", "password");
+ HttpCloudFoundryClient cloudFoundryClient = new HttpCloudFoundryClient("account", "appsManUri", "metricsUri", "host", "user", "password");
Response response = cloudFoundryClient.createRetryInterceptor(chain);
try {

diff:
@@ -61,7 +61,7 @@ class DeployCloudFoundryServerGroupAtomicOperationConverterTest {
}
private final CloudFoundryCredentials cloudFoundryCredentials = new CloudFoundryCredentials(
- "test", "", "", "", "", "") {
+ "test", "", "", "", "", "", "") {
public CloudFoundryClient getClient() {
return cloudFoundryClient;
}

diff:
@@ -64,7 +64,7 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
}
private final CloudFoundryCredentials cloudFoundryCredentials = new CloudFoundryCredentials(
- "test", "", "", "", "", "") {
+ "test", "", "", "", "", "", "") {
public CloudFoundryClient getClient() {
return cloudFoundryClient;
}

diff:
@@ -59,7 +59,7 @@ class ScaleCloudFoundryServerGroupAtomicOperationConverterTest {
}
private final CloudFoundryCredentials cloudFoundryCredentials = new CloudFoundryCredentials(
- "test", "", "", "", "", "") {
+ "test", "", "", "", "", "", "") {
public CloudFoundryClient getClient() {
return cloudFoundryClient;
}

diff:
@@ -23,7 +23,7 @@ import org.junit.jupiter.api.Test;
class CloudFoundryCredentialsTest {
@Test
void serialize() throws JsonProcessingException {
- CloudFoundryCredentials creds = new CloudFoundryCredentials("pws", "some.uri.example.com", "api.run.pivotal.io", "fakeuser", "fakepasword", "dev");
+ CloudFoundryCredentials creds = new CloudFoundryCredentials("pws", "some.uri.example.com", "some.uri.example.com", "api.run.pivotal.io", "fakeuser", "fakepasword", "dev");
new ObjectMapper().writeValueAsString(creds);
}
}

original_message: feat(provider/cf): add metrics links to SG (#3210)


message:
fix(provider/cf): cloning across CF foundations fails
spinnaker/spinnaker#3730

diff:
@@ -87,13 +87,13 @@ public class DeployCloudFoundryServerGroupAtomicOperationConverter extends Abstr
converted.setArtifact(convertToArtifact(artifactCredentials, artifactSource.get("reference").toString()));
converted.setArtifactCredentials(artifactCredentials);
} else if ("package".equals(artifactSource.get("type"))) {
- CloudFoundryCredentials accountCredentials = getCredentialsObject(artifactSource.get("account").toString());
- converted.setArtifactCredentials(new PackageArtifactCredentials(credentials.getClient()));
+ CloudFoundryCredentials artifactCredentials = getCredentialsObject(artifactSource.get("account").toString());
+ converted.setArtifactCredentials(new PackageArtifactCredentials(artifactCredentials.getClient()));
Artifact artifact = new Artifact();
artifact.setType("package");
artifact.setReference(getServerGroupId(artifactSource.get("serverGroupName").toString(),
- artifactSource.get("region").toString(), accountCredentials.getClient()));
+ artifactSource.get("region").toString(), artifactCredentials.getClient()));
converted.setArtifact(artifact);
}

diff:
@@ -19,6 +19,7 @@ package com.netflix.spinnaker.clouddriver.cloudfoundry.deploy.converters;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.netflix.spinnaker.clouddriver.artifacts.ArtifactCredentialsRepository;
import com.netflix.spinnaker.clouddriver.cloudfoundry.artifacts.ArtifactCredentialsFromString;
+import com.netflix.spinnaker.clouddriver.cloudfoundry.artifacts.PackageArtifactCredentials;
import com.netflix.spinnaker.clouddriver.cloudfoundry.client.*;
import com.netflix.spinnaker.clouddriver.cloudfoundry.deploy.description.DeployCloudFoundryServerGroupDescription;
import com.netflix.spinnaker.clouddriver.cloudfoundry.model.CloudFoundryOrganization;
@@ -34,6 +35,7 @@ import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.stubbing.Answer;
+import java.util.Arrays;
import java.util.Collections;
import java.util.Map;
import java.util.Optional;
@@ -43,7 +45,8 @@ import static org.mockito.Matchers.any;
import static org.mockito.Mockito.when;
class DeployCloudFoundryServerGroupAtomicOperationConverterTest {
- private final CloudFoundryClient cloudFoundryClient = new MockCloudFoundryClient();
+ private static CloudFoundryCredentials createCredentials(String name) {
+ CloudFoundryClient cloudFoundryClient = new MockCloudFoundryClient();
{
when(cloudFoundryClient.getOrganizations().findByName(any()))
.thenAnswer((Answer<Optional<CloudFoundryOrganization>>) invocation -> {
@@ -60,25 +63,27 @@ class DeployCloudFoundryServerGroupAtomicOperationConverterTest {
});
}
- private final CloudFoundryCredentials cloudFoundryCredentials = new CloudFoundryCredentials(
- "test", "", "", "", "", "", "") {
+ return new CloudFoundryCredentials(name, "", "", "", "", "", "")
+ {
public CloudFoundryClient getClient() {
return cloudFoundryClient;
}
};
+ }
+ private List<String> accounts = List.of("test", "sourceAccount", "destinationAccount");
private final ArtifactCredentialsRepository artifactCredentialsRepository = new ArtifactCredentialsRepository();
{
- artifactCredentialsRepository.save(new ArtifactCredentialsFromString(
- "test",
+ accounts.toStream().forEach(account -> artifactCredentialsRepository.save(new ArtifactCredentialsFromString(
+ account,
List.of("a").asJava(),
"applications: [{instances: 42}]"
- ));
+ )));
}
private final AccountCredentialsRepository accountCredentialsRepository = new MapBackedAccountCredentialsRepository();
{
- accountCredentialsRepository.update("test", cloudFoundryCredentials);
+ accounts.toStream().forEach(account -> accountCredentialsRepository.update(account, createCredentials(account)));
}
private final AccountCredentialsProvider accountCredentialsProvider =
new DefaultAccountCredentialsProvider(accountCredentialsRepository);
@@ -95,11 +100,11 @@ class DeployCloudFoundryServerGroupAtomicOperationConverterTest {
@Test
void convertDescriptionWithPackageArtifactSourceAndDownloadedManifest() {
final Map input = HashMap.of(
- "credentials", "test",
+ "credentials", "destinationAccount",
"region", "org > space",
"artifact", HashMap.of(
"type", "package",
- "account", "test",
+ "account", "sourceAccount",
"serverGroupName", "serverGroupName1",
"region", "org > space"
).toJavaMap(),
@@ -112,7 +117,10 @@ class DeployCloudFoundryServerGroupAtomicOperationConverterTest {
final DeployCloudFoundryServerGroupDescription result = converter.convertDescription(input);
- assertThat(result.getAccountName()).isEqualTo("test");
+ CloudFoundryCredentials sourceCredentials = converter.getCredentialsObject("sourceAccount");
+ assertThat(result.getAccountName()).isEqualTo("destinationAccount");
+ assertThat(result.getArtifactCredentials())
+ .isEqualToComparingFieldByFieldRecursively(new PackageArtifactCredentials(sourceCredentials.getClient()));
assertThat(result.getSpace()).isEqualToComparingFieldByFieldRecursively(
CloudFoundrySpace.builder().id("spaceID").name("space").organization(
CloudFoundryOrganization.builder().id("orgID").name("org").build()).build());

original_message:
fix(provider/cf): cloning across CF foundations fails (#3226)

spinnaker/spinnaker#3730

Co-Authored-By: Stu Pollock <spollock@pivotal.io>
Signed-off-by: Jammy Louie <jlouie@pivotal.io>


diff:
@@ -98,12 +98,12 @@ class AppengineClusterProvider implements ClusterProvider<AppengineCluster> {
@Override
Map<String, Set<AppengineCluster>> getClusterSummaries(String applicationName) {
- translateClusters(getClusterData(applicationName), false)?.groupBy { it.accountName } as Map<String, Set<AppengineCluster>>
+ translateClusters(getClusterData(applicationName), false)?.groupBy { it.accountName }.collectEntries { k, v -> [k, new HashSet<>(v)] }
}
@Override
Map<String, Set<AppengineCluster>> getClusterDetails(String applicationName) {
- translateClusters(getClusterData(applicationName), true)?.groupBy { it.accountName } as Map<String, Set<AppengineCluster>>
+ translateClusters(getClusterData(applicationName), true)?.groupBy { it.accountName }.collectEntries { k, v -> [k, new HashSet<>(v)] }
}
Set<CacheData> getClusterData(String applicationName) {

diff:
@@ -222,12 +222,14 @@ public class ProjectClustersService {
public String stack;
public String detail;
public List<ApplicationClusterModel> applications;
+ public ServerGroup.InstanceCounts instanceCounts;
public ClusterModel(String account, String stack, String detail, List<ApplicationClusterModel> applications) {
this.account = account;
this.stack = stack;
this.detail = detail;
this.applications = applications;
+ this.instanceCounts = getInstanceCounts();
}
ServerGroup.InstanceCounts getInstanceCounts() {

diff:
@@ -68,12 +68,12 @@ class GoogleClusterProvider implements ClusterProvider<GoogleCluster.View> {
@Override
Map<String, Set<GoogleCluster.View>> getClusterDetails(String applicationName) {
- getClusters(applicationName, true /* detailed */)
+ getClusters(applicationName, true /* detailed */).collectEntries { k, v -> [k, new HashSet<>(v)] }
}
@Override
Map<String, Set<GoogleCluster.View>> getClusterSummaries(String applicationName) {
- getClusters(applicationName, false /* detailed */)
+ getClusters(applicationName, false /* detailed */).collectEntries { k, v -> [k, new HashSet<>(v)] }
}
Map<String, Set<GoogleCluster.View>> getClusters(String applicationName, boolean includeInstanceDetails) {

original_message: fix(core): include instanceCounts when fetching project clusters, fix ClassCastException for gce and appengine fetch cluster methods (#3244)


diff:
@@ -264,7 +264,9 @@ public class ProjectClustersService {
);
incrementInstanceCounts(serverGroup, regionCluster.instanceCounts);
- JenkinsBuildInfo buildInfo = extractJenkinsBuildInfo(serverGroup.getImagesSummary().getSummaries());
+ ServerGroup.ImagesSummary imagesSummary = serverGroup.getImagesSummary();
+ List<? extends ServerGroup.ImageSummary> imageSummaries = imagesSummary == null ? new ArrayList() : imagesSummary.getSummaries();
+ JenkinsBuildInfo buildInfo = extractJenkinsBuildInfo(imageSummaries);
Optional<DeployedBuild> existingBuild = regionCluster.builds.stream()
.filter(b -> b.buildNumber.equals(buildInfo.number) &&
Optional.ofNullable(b.host).equals(Optional.ofNullable(buildInfo.host)) &&
@@ -278,7 +280,7 @@ public class ProjectClustersService {
buildInfo.name,
buildInfo.number,
serverGroup.getCreatedTime(),
- getServerGroupBuildInfoImages(serverGroup.getImagesSummary().getSummaries())
+ getServerGroupBuildInfoImages(imageSummaries)
))
).accept(existingBuild);
});

diff:
@@ -371,6 +371,27 @@ public class KubernetesCacheDataConverter {
return result;
}
+ static CacheData getClusterRelationships(String account, CacheData cacheData) {
+ Moniker moniker = getMoniker(cacheData);
+
+ if (moniker == null) {
+ return null;
+ }
+
+ String cluster = moniker.getCluster();
+ String application = moniker.getApp();
+
+ if (cluster == null || application == null) {
+ return null;
+ }
+
+ Map<String, Object> attributes = new HashMap<>();
+ Map<String, Collection<String>> relationships = new HashMap<>();
+ relationships.put(CLUSTERS.toString(), Collections.singletonList(Keys.cluster(account, application, cluster)));
+ CacheData appToCluster = defaultCacheData(Keys.application(application), logicalTtlSeconds, attributes, relationships);
+ return appToCluster;
+ }
+
static void logStratifiedCacheData(String agentType, Map<String, Collection<CacheData>> stratifiedCacheData) {
for (Map.Entry<String, Collection<CacheData>> entry : stratifiedCacheData.entrySet()) {
log.info(agentType + ": grouping " + entry.getKey() + " has " + entry.getValue().size() + " entries and " + relationshipCount(entry.getValue()) + " relationships");
@@ -442,7 +463,7 @@ public class KubernetesCacheDataConverter {
/*
* Worth noting the strange behavior here. If we are inverting a relationship to create a cache data for
* either a cluster or an application we need to insert attributes to ensure the cache data gets entered into
- * the cache. If we are caching anything else, we don't want competing agents to overrwrite attributes, so
+ * the cache. If we are caching anything else, we don't want competing agents to overwrite attributes, so
* we leave them blank.
*/
private static Optional<CacheData> invertSingleRelationship(String group, String key, String relationship) {

diff:
@@ -178,6 +178,11 @@ public abstract class KubernetesV2CachingAgent extends KubernetesCachingAgent<Ku
resourceData.addAll(invertedRelationships);
+ resourceData.addAll(resourceData.stream()
+ .map(rs -> KubernetesCacheDataConverter.getClusterRelationships(accountName, rs))
+ .filter(Objects::nonNull)
+ .collect(Collectors.toList()));
+
Map<String, Collection<CacheData>> entries = KubernetesCacheDataConverter.stratifyCacheDataByGroup(KubernetesCacheDataConverter.dedupCacheData(resourceData));
KubernetesCacheDataConverter.logStratifiedCacheData(getAgentType(), entries);

diff:
@@ -38,12 +38,14 @@ public class KubernetesV2Cluster implements Cluster {
String accountName;
Set<ServerGroup> serverGroups = new HashSet<>();
Set<LoadBalancer> loadBalancers = new HashSet<>();
+ String application;
public KubernetesV2Cluster(String rawKey) {
Keys.ClusterCacheKey key = (Keys.ClusterCacheKey) Keys.parseKey(rawKey).get();
this.name = key.getName();
this.accountName = key.getAccount();
- this.moniker = Moniker.builder().cluster(name).build(); // TODO(lwander) if it turns out that cluster -> app is important, enforce constraints here.
+ this.application = key.getApplication();
+ this.moniker = Moniker.builder().cluster(name).app(application).build();
}
public KubernetesV2Cluster(String rawKey, List<KubernetesV2ServerGroup> serverGroups, List<KubernetesV2LoadBalancer> loadBalancers) {

original_message: fix(kubernetes): fix fetching clusters for project (#3247)


message: fix(cf): repair scale-to-zero issue

diff:
@@ -25,7 +25,6 @@ import javax.annotation.Nullable;
@Data
@EqualsAndHashCode(callSuper = true)
public class ScaleCloudFoundryServerGroupDescription extends AbstractCloudFoundryServerGroupDescription {
- @Nullable
ServerGroup.Capacity capacity;
@Nullable

diff:
@@ -27,6 +27,7 @@ import com.netflix.spinnaker.clouddriver.orchestration.AtomicOperation;
import lombok.RequiredArgsConstructor;
import java.util.List;
+import java.util.Optional;
import static com.netflix.spinnaker.clouddriver.cloudfoundry.deploy.ops.CloudFoundryOperationUtils.describeProcessState;
@@ -56,17 +57,20 @@ public class ScaleCloudFoundryServerGroupAtomicOperation implements AtomicOperat
ProcessStats.State state = operationPoller.waitForOperation(
() -> client.getApplications().getProcessState(description.getServerGroupId()),
- inProgressState -> (inProgressState == ProcessStats.State.RUNNING || inProgressState == ProcessStats.State.CRASHED),
+ inProgressState -> (
+ inProgressState == ProcessStats.State.RUNNING
+ || inProgressState == ProcessStats.State.CRASHED
+ || inProgressState == ProcessStats.State.DOWN),
null, getTask(), description.getServerGroupName(), PHASE);
- if (state != ProcessStats.State.RUNNING) {
+ if (state == ProcessStats.State.RUNNING ||
+ (state == ProcessStats.State.DOWN && description.getCapacity().getDesired() == 0)) {
+ getTask().updateStatus(PHASE, "Resized '" + description.getServerGroupName() + "'");
+ } else {
getTask().updateStatus(PHASE, "Failed to start '" + description.getServerGroupName() + "' which instead " + describeProcessState(state));
getTask().fail();
- return null;
}
- getTask().updateStatus(PHASE, "Resized '" + description.getServerGroupName() + "'");
-
return null;
}
}

diff:
@@ -73,4 +73,19 @@ class ScaleCloudFoundryServerGroupAtomicOperationTest extends AbstractCloudFound
.has(status("Resizing 'myapp'"), atIndex(1))
.has(status("Failed to start 'myapp' which instead crashed"), atIndex(2));
}
+
+ @Test
+ void scaleDownToZero() {
+ desc.setCapacity(new ServerGroup.Capacity(1, 1, 0));
+ OperationPoller poller = mock(OperationPoller.class);
+
+ //noinspection unchecked
+ when(poller.waitForOperation(any(Supplier.class), any(), anyLong(), any(), any(), any())).thenReturn(ProcessStats.State.DOWN);
+
+ ScaleCloudFoundryServerGroupAtomicOperation op = new ScaleCloudFoundryServerGroupAtomicOperation(poller, desc);
+
+ assertThat(runOperation(op).getHistory())
+ .has(status("Resizing 'myapp'"), atIndex(1))
+ .has(status("Resized 'myapp'"), atIndex(2));
+ }
}

original_message:
fix(cf): repair scale-to-zero issue (#3257)

Co-Authored-By: Stu Pollock <spollock@pivotal.io>


diff:
@@ -48,6 +48,7 @@ class BasicGoogleDeployDescription extends BaseGoogleInstanceDescription impleme
GoogleAutoscalingPolicy autoscalingPolicy
GoogleHttpLoadBalancingPolicy loadBalancingPolicy
GoogleAutoHealingPolicy autoHealingPolicy
+ Boolean overwriteAncestorAutoHealingPolicy = false
/**
* Optional explicit specification of zones for an RMIG.
*/

diff:
@@ -257,10 +257,8 @@ class CopyLastGoogleServerGroupAtomicOperation extends GoogleAtomicOperation<Dep
GoogleAutoHealingPolicy ancestorAutoHealingPolicyDescription =
GCEUtil.buildAutoHealingPolicyDescriptionFromAutoHealingPolicy(ancestorAutoHealingPolicy)
- println(",, description autohealing policy ${description.autoHealingPolicy}")
- println(",, ancestor autohealing policy ${ancestorAutoscalingPolicy}")
- println(",, ancestor autohealing policy description ${ancestorAutoscalingPolicyDescription}")
- newDescription.autoHealingPolicy = description.autoHealingPolicy ?: ancestorAutoHealingPolicyDescription
+ newDescription.autoHealingPolicy = (description.autoHealingPolicy != null || description.overwriteAncestorAutoHealingPolicy) ?
+ description.autoHealingPolicy : ancestorAutoHealingPolicyDescription
return newDescription
}

original_message: fix(google): prevent parent server group from overwriting null clone autohealing policies (#3266)


diff:
@@ -272,28 +272,28 @@ class GCEUtil {
static def queryHealthCheck(String projectName,
String account,
String healthCheckName,
+ GoogleHealthCheck.HealthCheckKind healthCheckKind,
Compute compute,
Cache cacheView,
Task task,
String phase,
GoogleExecutorTraits executor) {
- task.updateStatus phase, "Looking up http(s) health check $healthCheckName..."
-
- def httpHealthCheckIdentifiers = cacheView.filterIdentifiers(HTTP_HEALTH_CHECKS.ns, Keys.getHttpHealthCheckKey(account, healthCheckName))
- def results = cacheView.getAll(HTTP_HEALTH_CHECKS.ns, httpHealthCheckIdentifiers, RelationshipCacheFilter.none())
+ task.updateStatus phase, "Looking up health check $healthCheckName..."
- if (results[0]?.attributes?.httpHealthCheck) {
- return results[0]?.attributes?.httpHealthCheck
- } else {
- try {
- // TODO(duftler): Update this to use the cache instead of a live call once we are caching https health checks.
- return executor.timeExecute(
- compute.httpsHealthChecks().get(projectName, healthCheckName),
- "compute.httpsHealthChecks.get",
- executor.TAG_SCOPE, executor.SCOPE_GLOBAL)
- } catch (GoogleJsonResponseException | SocketTimeoutException | SocketException _) {
- updateStatusAndThrowNotFoundException("Http(s) health check $healthCheckName not found.", task, phase)
- }
+ switch (healthCheckKind) {
+ case GoogleHealthCheck.HealthCheckKind.healthCheck:
+ return queryNestedHealthCheck(projectName, account, healthCheckName, compute, cacheView, task, phase, executor)
+ case GoogleHealthCheck.HealthCheckKind.httpHealthCheck:
+ return queryLegacyHttpHealthCheck(account, healthCheckName, cacheView, task, phase)
+ case GoogleHealthCheck.HealthCheckKind.httpsHealthCheck:
+ return queryLegacyHttpsHealthCheck(projectName, healthCheckName, compute, task, phase, executor)
+ default:
+ // Note: Cache queries for these health checks must occur in this order since queryLegacyHttpsHealthCheck() will make a live
+ // call that fails on a missing health check.
+ // todo(mneterval): return null instead of querying for each type once all health check payloads include `healthCheckKind`
+ return queryNestedHealthCheck(projectName, account, healthCheckName, compute, cacheView, task, phase, executor) ?:
+ queryLegacyHttpHealthCheck(account, healthCheckName, cacheView, task, phase) ?:
+ queryLegacyHttpsHealthCheck(projectName, healthCheckName, compute, task, phase, executor)
}
}
@@ -313,6 +313,36 @@ class GCEUtil {
return results[0]?.attributes?.healthCheck
}
+ static def queryLegacyHttpHealthCheck(String account,
+ String healthCheckName,
+ Cache cacheView,
+ Task task,
+ String phase) {
+ task.updateStatus phase, "Looking up http health check $healthCheckName..."
+
+ def httpHealthCheckIdentifiers = cacheView.filterIdentifiers(HTTP_HEALTH_CHECKS.ns, Keys.getHttpHealthCheckKey(account, healthCheckName))
+ def results = cacheView.getAll(HTTP_HEALTH_CHECKS.ns, httpHealthCheckIdentifiers, RelationshipCacheFilter.none())
+ return results[0]?.attributes?.httpHealthCheck
+ }
+
+ static def queryLegacyHttpsHealthCheck(String projectName,
+ String healthCheckName,
+ Compute compute,
+ Task task,
+ String phase,
+ GoogleExecutorTraits executor) {
+ task.updateStatus phase, "Looking up https health check $healthCheckName..."
+ try {
+ // TODO(duftler): Update this to use the cache instead of a live call once we are caching https health checks.
+ return executor.timeExecute(
+ compute.httpsHealthChecks().get(projectName, healthCheckName),
+ "compute.httpsHealthChecks.get",
+ executor.TAG_SCOPE, executor.SCOPE_GLOBAL)
+ } catch (GoogleJsonResponseException | SocketTimeoutException | SocketException _) {
+ updateStatusAndThrowNotFoundException("Https health check $healthCheckName not found.", task, phase)
+ }
+ }
+
static List<ForwardingRule> queryRegionalForwardingRules(String projectName,
String region,
List<String> forwardingRuleNames,

diff:
@@ -396,12 +396,9 @@ class BasicGoogleDeployHandler implements DeployHandler<BasicGoogleDeployDescrip
description.autoscalingPolicy = GCEUtil.buildAutoscalingPolicyDescriptionFromAutoscalingPolicy(ancestorServerGroup.autoscalingPolicy)
}
- // Note: Cache queries for these health checks must occur in this order since queryHealthCheck() will make a live
- // call that fails on a missing health check.
def autoHealingHealthCheck = null
if (description.autoHealingPolicy?.healthCheck) {
- autoHealingHealthCheck = GCEUtil.queryNestedHealthCheck(project, description.accountName, description.autoHealingPolicy.healthCheck, compute, cacheView, task, BASE_PHASE, this) ?:
- GCEUtil.queryHealthCheck(project, description.accountName, description.autoHealingPolicy.healthCheck, compute, cacheView, task, BASE_PHASE, this)
+ autoHealingHealthCheck = GCEUtil.queryHealthCheck(project, description.accountName, description.autoHealingPolicy.healthCheck, description.autoHealingPolicy.healthCheckKind, compute, cacheView, task, BASE_PHASE, this)
}
List<InstanceGroupManagerAutoHealingPolicy> autoHealingPolicy =

diff:
@@ -249,7 +249,7 @@ class UpsertGoogleAutoscalingPolicyAtomicOperation extends GoogleAtomicOperation
return newDescription
}
- ["healthCheck", "initialDelaySec"].each {
+ ["healthCheck", "initialDelaySec", "healthCheckKind"].each {
if (update[it] != null) {
newDescription[it] = update[it]
}
@@ -278,10 +278,7 @@ class UpsertGoogleAutoscalingPolicyAtomicOperation extends GoogleAtomicOperation
}
private buildAutoHealingPolicyFromAutoHealingPolicyDescription(GoogleAutoHealingPolicy autoHealingPolicyDescription, String project, Compute compute) {
- // Note: Cache queries for these health checks must occur in this order since queryHealthCheck() will make a live
- // call that fails on a missing health check.
- def autoHealingHealthCheck = GCEUtil.queryNestedHealthCheck(project, description.accountName, autoHealingPolicyDescription.healthCheck, compute, cacheView, task, BASE_PHASE, this) ?:
- GCEUtil.queryHealthCheck(project, description.accountName, autoHealingPolicyDescription.healthCheck, compute, cacheView, task, BASE_PHASE, this)
+ def autoHealingHealthCheck = GCEUtil.queryHealthCheck(project, description.accountName, autoHealingPolicyDescription.healthCheck, autoHealingPolicyDescription.healthCheckKind, compute, cacheView, task, BASE_PHASE, this)
List<InstanceGroupManagerAutoHealingPolicy> autoHealingPolicy = autoHealingPolicyDescription?.healthCheck
? [new InstanceGroupManagerAutoHealingPolicy(

diff:
@@ -25,6 +25,7 @@ import groovy.transform.ToString
@ToString(includeNames = true)
class GoogleAutoHealingPolicy {
String healthCheck
+ GoogleHealthCheck.HealthCheckKind healthCheckKind
Integer initialDelaySec
FixedOrPercent maxUnavailable

original_message: fix(google): differentiate among autohealing health check kinds (#3282)


diff:
@@ -274,7 +274,17 @@ public class ProjectClustersService {
.findFirst();
new OptionalConsumer<>(
- (DeployedBuild b) -> b.deployed = Math.max(b.deployed, serverGroup.getCreatedTime()),
+ (DeployedBuild b) -> {
+ b.deployed = Math.max(b.deployed, serverGroup.getCreatedTime());
+ List images = getServerGroupBuildInfoImages(imageSummaries);
+ if (images != null) {
+ images.forEach(image -> {
+ if (image != null && !b.images.contains(image)) {
+ b.images.add(image);
+ }
+ });
+ }
+ },
() -> regionCluster.builds.add(new DeployedBuild(
buildInfo.host,
buildInfo.name,

diff:
@@ -50,6 +50,8 @@ import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
+import static java.util.Collections.singletonList;
+
@EqualsAndHashCode(callSuper = true)
@Data
@Slf4j
@@ -205,4 +207,46 @@ public class KubernetesV2ServerGroup extends ManifestBasedModel implements Serve
.isDisabled(isDisabled())
.build();
}
+
+ @Override
+ public ImagesSummary getImagesSummary() {
+ Map<String, Object> buildInfo = getBuildInfo();
+ Set<String> images = (HashSet<String>) buildInfo.get("images");
+ return new ImagesSummary() {
+ @Override
+ public List<? extends ImageSummary> getSummaries() {
+ return singletonList(
+ new ImageSummary() {
+
+ @Override
+ public String getServerGroupName() {
+ return getManifest().getName();
+ }
+
+ @Override
+ public String getImageId() {
+ return null;
+ }
+
+ @Override
+ public String getImageName() {
+ return null;
+ }
+
+ @Override
+ public Map<String, Object> getImage() {
+ return null;
+ }
+
+ @Override
+ public Map<String, Object> getBuildInfo() {
+ return new ImmutableMap.Builder<String, Object>()
+ .put("images", new ArrayList<>(images))
+ .build();
+ }
+ }
+ );
+ }
+ };
+ }
}

original_message: fix(kubernetes): fix image resolution for project clusters view (#3286)


diff:
@@ -35,7 +35,7 @@ dependencies {
compile spinnaker.dependency('korkSwagger')
compile spinnaker.dependency('bootActuator')
compile spinnaker.dependency('bootDataRest')
- testCompile ("org.springframework.boot:spring-boot-starter-test")
+ testCompile spinnaker.dependency('bootTest')
//this brings in the jetty GzipFilter which boot will autoconfigure
runtime 'org.eclipse.jetty:jetty-servlets:9.2.11.v20150529'

original_message: chore(*): Pin spring-boot-starter-test to springBoot version (#3336)


diff:
@@ -77,6 +77,10 @@ public class DeployCloudFoundryServiceAtomicOperationConverter extends AbstractC
}
converted.setUserProvidedServiceAttributes(userProvidedAttrs);
break;
+ case "userProvidedArtifact":
+ converted.setServiceType("userProvided");
+ downloadAndProcessManifest(manifest, credentialsRepository, myMap -> converted.setUserProvidedServiceAttributes(convertUserProvidedServiceManifest(myMap)));
+ break;
}
return converted;
}
@@ -88,9 +92,14 @@ public class DeployCloudFoundryServiceAtomicOperationConverter extends AbstractC
.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES)
.convertValue(manifestMap, new TypeReference<ServiceManifest>() {
});
-
+ if (manifest.getService() == null) {
+ throw new IllegalArgumentException("Manifest is missing the service");
+ } else if (manifest.getServiceName() == null) {
+ throw new IllegalArgumentException("Manifest is missing the service name");
+ } else if(manifest.getServicePlan() == null) {
+ throw new IllegalArgumentException("Manifest is missing the service plan");
+ }
DeployCloudFoundryServiceDescription.ServiceAttributes attrs = new DeployCloudFoundryServiceDescription.ServiceAttributes();
-
attrs.setService(manifest.getService());
attrs.setServiceName(manifest.getServiceName());
attrs.setServicePlan(manifest.getServicePlan());
@@ -99,6 +108,25 @@ public class DeployCloudFoundryServiceAtomicOperationConverter extends AbstractC
return attrs;
}
+ @VisibleForTesting
+ DeployCloudFoundryServiceDescription.UserProvidedServiceAttributes convertUserProvidedServiceManifest(Map manifestMap) {
+ UserProvidedServiceManifest manifest = new ObjectMapper()
+ .setPropertyNamingStrategy(PropertyNamingStrategy.SNAKE_CASE)
+ .disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES)
+ .convertValue(manifestMap, new TypeReference<UserProvidedServiceManifest>() {
+ });
+ if (manifest.getServiceName() == null) {
+ throw new IllegalArgumentException("Manifest is missing the service name");
+ }
+ DeployCloudFoundryServiceDescription.UserProvidedServiceAttributes attrs = new DeployCloudFoundryServiceDescription.UserProvidedServiceAttributes();
+ attrs.setServiceName(manifest.getServiceName());
+ attrs.setSyslogDrainUrl(manifest.getSyslogDrainUrl());
+ attrs.setRouteServiceUrl(manifest.getRouteServiceUrl());
+ attrs.setTags(manifest.getTags());
+ attrs.setCredentialsMap(parseParameters(manifest.getCredentialsMap()));
+ return attrs;
+ }
+
private Map<String, Object> parseParameters(String parameterString) {
if (!StringUtils.isBlank(parameterString)) {
try {
@@ -114,13 +142,10 @@ public class DeployCloudFoundryServiceAtomicOperationConverter extends AbstractC
@Data
private static class ServiceManifest {
- @Nullable
private String service;
- @Nullable
private String serviceName;
- @Nullable
private String servicePlan;
@Nullable
@@ -129,4 +154,21 @@ public class DeployCloudFoundryServiceAtomicOperationConverter extends AbstractC
@Nullable
private String parameters;
}
+
+ @Data
+ private static class UserProvidedServiceManifest {
+ private String serviceName;
+
+ @Nullable
+ private String syslogDrainUrl;
+
+ @Nullable
+ private String routeServiceUrl;
+
+ @Nullable
+ private Set<String> tags;
+
+ @Nullable
+ private String credentialsMap;
+ }
}

diff:
@@ -108,6 +108,7 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
void convertManifestMapToServiceAttributes() {
final Map input = HashMap.of(
"service", "my-service",
+ "service_name", "my-service-name",
"service_plan", "my-service-plan",
"tags", List.of(
"my-tag"
@@ -118,6 +119,7 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
assertThat(converter.convertManifest(input)).isEqualToComparingFieldByFieldRecursively(
new DeployCloudFoundryServiceDescription.ServiceAttributes()
.setService("my-service")
+ .setServiceName("my-service-name")
.setServicePlan("my-service-plan")
.setTags(Collections.singleton("my-tag"))
.setParameterMap(HashMap.<String, Object>of(
@@ -126,10 +128,91 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
);
}
+ @Test
+ void convertManifestMapToServiceAttributesMissingServiceThrowsException() {
+ final Map input = HashMap.of(
+ "service_name", "my-service-name",
+ "service_plan", "my-service-plan",
+ "tags", List.of(
+ "my-tag"
+ ).asJava(),
+ "parameters", "{\"foo\": \"bar\"}"
+ ).toJavaMap();
+
+ assertThrows(IllegalArgumentException.class, () -> converter.convertManifest(input), "Manifest is missing the service");
+ }
+
+ @Test
+ void convertManifestMapToServiceAttributesMissingServiceNameThrowsException() {
+ final Map input = HashMap.of(
+ "service_name", "my-service-name",
+ "service_plan", "my-service-plan",
+ "tags", List.of(
+ "my-tag"
+ ).asJava(),
+ "parameters", "{\"foo\": \"bar\"}"
+ ).toJavaMap();
+
+ assertThrows(IllegalArgumentException.class, () -> converter.convertManifest(input), "Manifest is missing the service name");
+ }
+
+ @Test
+ void convertManifestMapToServiceAttributesMissingServicePlanThrowsException() {
+ final Map input = HashMap.of(
+ "service", "my-service",
+ "service_name", "my-service-name",
+ "tags", List.of(
+ "my-tag"
+ ).asJava(),
+ "parameters", "{\"foo\": \"bar\"}"
+ ).toJavaMap();
+
+ assertThrows(IllegalArgumentException.class, () -> converter.convertManifest(input), "Manifest is missing the service plan");
+ }
+
+ @Test
+ void convertCupsManifestMapToUserProvidedServiceAttributes() {
+ final Map input = HashMap.of(
+ "service_name", "my-service-name",
+ "syslog_drain_url", "test-syslog-drain-url",
+ "route_service_url", "test-route-service-url",
+ "tags", List.of(
+ "my-tag"
+ ).asJava(),
+ "credentials_map", "{\"foo\": \"bar\"}"
+ ).toJavaMap();
+
+ assertThat(converter.convertUserProvidedServiceManifest(input)).isEqualToComparingFieldByFieldRecursively(
+ new DeployCloudFoundryServiceDescription.UserProvidedServiceAttributes()
+ .setServiceName("my-service-name")
+ .setSyslogDrainUrl("test-syslog-drain-url")
+ .setRouteServiceUrl("test-route-service-url")
+ .setTags(Collections.singleton("my-tag"))
+ .setCredentialsMap(HashMap.<String, Object>of(
+ "foo", "bar"
+ ).toJavaMap())
+ );
+ }
+
+ @Test
+ void convertCupsManifestMapToUserProvidedServiceAttributesMissingServiceNameThrowsException() {
+ final Map input = HashMap.of(
+ "syslog_drain_url", "test-syslog-drain-url",
+ "route_service_url", "test-route-service-url",
+ "tags", List.of(
+ "my-tag"
+ ).asJava(),
+ "credentials_map", "{\"foo\": \"bar\"}"
+ ).toJavaMap();
+
+ assertThrows(IllegalArgumentException.class, () -> converter.convertUserProvidedServiceManifest(input), "Manifest is missing the service name");
+ }
+
@Test
void convertManifestMapToServiceAttributesEmptyParamString() {
final Map input = HashMap.of(
"service", "my-service",
+ "service_name", "my-service-name",
"service_plan", "my-service-plan",
"tags", List.of(
"my-tag"
@@ -140,6 +223,7 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
assertThat(converter.convertManifest(input)).isEqualToComparingFieldByFieldRecursively(
new DeployCloudFoundryServiceDescription.ServiceAttributes()
.setService("my-service")
+ .setServiceName("my-service-name")
.setServicePlan("my-service-plan")
.setTags(Collections.singleton("my-tag"))
);

change_type: DELETE

diff:
-/*
- * Copyright 2018 Pivotal, Inc.
- *
- * 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 com.netflix.spinnaker.clouddriver.cloudfoundry.security;
-
-import com.fasterxml.jackson.core.JsonProcessingException;
-import com.fasterxml.jackson.databind.ObjectMapper;
-import org.junit.jupiter.api.Test;
-
-class CloudFoundryCredentialsTest {
- @Test
- void serialize() throws JsonProcessingException {
- CloudFoundryCredentials creds = new CloudFoundryCredentials("pws", "some.uri.example.com", "some.uri.example.com", "api.run.pivotal.io", "fakeuser", "fakepasword", "dev");
- new ObjectMapper().writeValueAsString(creds);
- }
-}

original_message:
feat(cf): Add artifacts for user-defined services (#3343)

Co-Authored-By: Jammy Louie <jlouie@pivotal.io>


diff:
@@ -198,7 +198,7 @@ public class Applications {
.stream()
.flatMap(vcap -> vcap.getValue().stream()
.map(instance -> CloudFoundryServiceInstance.builder()
- .serviceName(vcap.getKey())
+ .serviceInstanceName(vcap.getKey())
.name(instance.getName())
.plan(instance.getPlan())
.tags(instance.getTags())

diff:
@@ -94,14 +94,14 @@ public class DeployCloudFoundryServiceAtomicOperationConverter extends AbstractC
});
if (manifest.getService() == null) {
throw new IllegalArgumentException("Manifest is missing the service");
- } else if (manifest.getServiceName() == null) {
- throw new IllegalArgumentException("Manifest is missing the service name");
+ } else if (manifest.getServiceInstanceName() == null) {
+ throw new IllegalArgumentException("Manifest is missing the service instance name");
} else if(manifest.getServicePlan() == null) {
throw new IllegalArgumentException("Manifest is missing the service plan");
}
DeployCloudFoundryServiceDescription.ServiceAttributes attrs = new DeployCloudFoundryServiceDescription.ServiceAttributes();
attrs.setService(manifest.getService());
- attrs.setServiceName(manifest.getServiceName());
+ attrs.setServiceInstanceName(manifest.getServiceInstanceName());
attrs.setServicePlan(manifest.getServicePlan());
attrs.setTags(manifest.getTags());
attrs.setParameterMap(parseParameters(manifest.getParameters()));
@@ -115,11 +115,11 @@ public class DeployCloudFoundryServiceAtomicOperationConverter extends AbstractC
.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES)
.convertValue(manifestMap, new TypeReference<UserProvidedServiceManifest>() {
});
- if (manifest.getServiceName() == null) {
+ if (manifest.getServiceInstanceName() == null) {
throw new IllegalArgumentException("Manifest is missing the service name");
}
DeployCloudFoundryServiceDescription.UserProvidedServiceAttributes attrs = new DeployCloudFoundryServiceDescription.UserProvidedServiceAttributes();
- attrs.setServiceName(manifest.getServiceName());
+ attrs.setServiceInstanceName(manifest.getServiceInstanceName());
attrs.setSyslogDrainUrl(manifest.getSyslogDrainUrl());
attrs.setRouteServiceUrl(manifest.getRouteServiceUrl());
attrs.setTags(manifest.getTags());
@@ -144,7 +144,7 @@ public class DeployCloudFoundryServiceAtomicOperationConverter extends AbstractC
private static class ServiceManifest {
private String service;
- private String serviceName;
+ private String serviceInstanceName;
private String servicePlan;
@@ -157,7 +157,7 @@ public class DeployCloudFoundryServiceAtomicOperationConverter extends AbstractC
@Data
private static class UserProvidedServiceManifest {
- private String serviceName;
+ private String serviceInstanceName;
@Nullable
private String syslogDrainUrl;

diff:
@@ -46,7 +46,7 @@ public class DeployCloudFoundryServiceDescription extends AbstractCloudFoundrySe
@Data
public static class ServiceAttributes {
String service;
- String serviceName;
+ String serviceInstanceName;
String servicePlan;
@Nullable
@@ -58,7 +58,7 @@ public class DeployCloudFoundryServiceDescription extends AbstractCloudFoundrySe
@Data
public static class UserProvidedServiceAttributes {
- String serviceName;
+ String serviceInstanceName;
@Nullable
Set<String> tags;

diff:
@@ -22,5 +22,5 @@ import lombok.EqualsAndHashCode;
@Data
@EqualsAndHashCode(callSuper = true)
public class DestroyCloudFoundryServiceDescription extends AbstractCloudFoundryServiceDescription {
- private String serviceName;
+ private String serviceInstanceName;
}

diff:
@@ -39,35 +39,35 @@ public class DeployCloudFoundryServiceAtomicOperation implements AtomicOperation
switch (description.getServiceType()) {
case "service":
DeployCloudFoundryServiceDescription.ServiceAttributes serviceAttributes = description.getServiceAttributes();
- task.updateStatus(PHASE, "Creating service instance '" + serviceAttributes.getServiceName() + "' from service " + serviceAttributes.getService() + " and service plan " + serviceAttributes.getServicePlan());
+ task.updateStatus(PHASE, "Creating service instance '" + serviceAttributes.getServiceInstanceName() + "' from service " + serviceAttributes.getService() + " and service plan " + serviceAttributes.getServicePlan());
description
.getClient()
.getServiceInstances()
.createServiceInstance(
- serviceAttributes.getServiceName(),
+ serviceAttributes.getServiceInstanceName(),
serviceAttributes.getService(),
serviceAttributes.getServicePlan(),
serviceAttributes.getTags(),
serviceAttributes.getParameterMap(),
description.getSpace(),
description.getTimeout());
- task.updateStatus(PHASE, "Created service instance '" + serviceAttributes.getServiceName() + "'");
+ task.updateStatus(PHASE, "Created service instance '" + serviceAttributes.getServiceInstanceName() + "'");
break;
case "userProvided":
DeployCloudFoundryServiceDescription.UserProvidedServiceAttributes userProvidedServiceAttributes = description.getUserProvidedServiceAttributes();
- task.updateStatus(PHASE, "Creating user provided service instance '" + userProvidedServiceAttributes.getServiceName() + "'");
+ task.updateStatus(PHASE, "Creating user provided service instance '" + userProvidedServiceAttributes.getServiceInstanceName() + "'");
description
.getClient()
.getServiceInstances()
.createUserProvidedServiceInstance(
- userProvidedServiceAttributes.getServiceName(),
+ userProvidedServiceAttributes.getServiceInstanceName(),
userProvidedServiceAttributes.getSyslogDrainUrl(),
userProvidedServiceAttributes.getTags(),
userProvidedServiceAttributes.getCredentialsMap(),
userProvidedServiceAttributes.getRouteServiceUrl(),
description.getSpace()
);
- task.updateStatus(PHASE, "Created user provided service instance '" + userProvidedServiceAttributes.getServiceName() + "'");
+ task.updateStatus(PHASE, "Created user provided service instance '" + userProvidedServiceAttributes.getServiceInstanceName() + "'");
break;
}
return null;

diff:
@@ -36,11 +36,11 @@ public class DestroyCloudFoundryServiceAtomicOperation implements AtomicOperatio
@Override
public Void operate(List priorOutputs) {
Task task = getTask();
- task.updateStatus(PHASE, "Initializing the removal of service instance '" + description.getServiceName() + "' from space " + description.getSpace().getName());
+ task.updateStatus(PHASE, "Initializing the removal of service instance '" + description.getServiceInstanceName() + "' from space " + description.getSpace().getName());
description.getClient()
.getServiceInstances()
- .destroyServiceInstance(description.getSpace(), description.getServiceName(), description.getTimeout());
- task.updateStatus(PHASE, "Done removing service instance '" + description.getServiceName() + "'");
+ .destroyServiceInstance(description.getSpace(), description.getServiceInstanceName(), description.getTimeout());
+ task.updateStatus(PHASE, "Done removing service instance '" + description.getServiceInstanceName() + "'");
return null;
}
}

diff:
@@ -31,7 +31,7 @@ import java.util.Set;
@JsonDeserialize(builder = CloudFoundryServiceInstance.CloudFoundryServiceInstanceBuilder.class)
public class CloudFoundryServiceInstance {
@JsonView(Views.Cache.class)
- String serviceName;
+ String serviceInstanceName;
@JsonView(Views.Cache.class)
String name;

diff:
@@ -151,7 +151,7 @@ class ServiceInstancesTest {
when(serviceInstanceService.getServiceInstanceById(any())).thenReturn(succeededServiceInstanceResource);
serviceInstances.createServiceInstance("new-service-instance-name",
- "serviceName",
+ "serviceInstanceName",
"ServicePlan1",
Collections.emptySet(),
null,
@@ -173,7 +173,7 @@ class ServiceInstancesTest {
assertThrows(CloudFoundryApiException.class, () ->
serviceInstances.createServiceInstance("newServiceInstanceName",
- "serviceName",
+ "serviceInstanceName",
"ServicePlan1",
Collections.emptySet(),
null,
@@ -196,7 +196,7 @@ class ServiceInstancesTest {
assertThrows(ResourceNotFoundException.class, () ->
serviceInstances.createServiceInstance("newServiceInstanceName",
- "serviceName",
+ "serviceInstanceName",
"servicePlanName",
Collections.emptySet(),
null,
@@ -219,7 +219,7 @@ class ServiceInstancesTest {
when(serviceInstanceService.getServiceInstanceById(any())).thenReturn(polledServiceInstanceResource);
serviceInstances.createServiceInstance("newServiceInstanceName",
- "serviceName",
+ "serviceInstanceName",
"ServicePlan1",
Collections.emptySet(),
null,
@@ -246,7 +246,7 @@ class ServiceInstancesTest {
assertThrows(CloudFoundryApiException.class, () ->
serviceInstances.createServiceInstance("newServiceInstanceName",
- "serviceName",
+ "serviceInstanceName",
"ServicePlan1",
Collections.emptySet(),
null,
@@ -282,7 +282,7 @@ class ServiceInstancesTest {
assertThrows(CloudFoundryApiException.class, () ->
serviceInstances.createServiceInstance("newServiceInstanceName",
- "serviceName",
+ "serviceInstanceName",
"ServicePlan1",
Collections.emptySet(),
null,

diff:
@@ -77,7 +77,7 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
artifactCredentialsRepository.save(new ArtifactCredentialsFromString(
"test",
List.of("a").asJava(),
- "service_name: my-service-name\n" +
+ "service_instance_name: my-service-instance-name\n" +
"service: my-service\n" +
"service_plan: my-service-plan\n" +
"tags:\n" +
@@ -108,7 +108,7 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
void convertManifestMapToServiceAttributes() {
final Map input = HashMap.of(
"service", "my-service",
- "service_name", "my-service-name",
+ "service_instance_name", "my-service-instance-name",
"service_plan", "my-service-plan",
"tags", List.of(
"my-tag"
@@ -119,7 +119,7 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
assertThat(converter.convertManifest(input)).isEqualToComparingFieldByFieldRecursively(
new DeployCloudFoundryServiceDescription.ServiceAttributes()
.setService("my-service")
- .setServiceName("my-service-name")
+ .setServiceInstanceName("my-service-instance-name")
.setServicePlan("my-service-plan")
.setTags(Collections.singleton("my-tag"))
.setParameterMap(HashMap.<String, Object>of(
@@ -131,7 +131,7 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
@Test
void convertManifestMapToServiceAttributesMissingServiceThrowsException() {
final Map input = HashMap.of(
- "service_name", "my-service-name",
+ "service_instance_name", "my-service-instance-name",
"service_plan", "my-service-plan",
"tags", List.of(
"my-tag"
@@ -145,7 +145,7 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
@Test
void convertManifestMapToServiceAttributesMissingServiceNameThrowsException() {
final Map input = HashMap.of(
- "service_name", "my-service-name",
+ "service_instance_name", "my-service-instance-name",
"service_plan", "my-service-plan",
"tags", List.of(
"my-tag"
@@ -160,7 +160,7 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
void convertManifestMapToServiceAttributesMissingServicePlanThrowsException() {
final Map input = HashMap.of(
"service", "my-service",
- "service_name", "my-service-name",
+ "service_instance_name", "my-service-instance-name",
"tags", List.of(
"my-tag"
).asJava(),
@@ -173,7 +173,7 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
@Test
void convertCupsManifestMapToUserProvidedServiceAttributes() {
final Map input = HashMap.of(
- "service_name", "my-service-name",
+ "service_instance_name", "my-service-instance-name",
"syslog_drain_url", "test-syslog-drain-url",
"route_service_url", "test-route-service-url",
"tags", List.of(
@@ -184,7 +184,7 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
assertThat(converter.convertUserProvidedServiceManifest(input)).isEqualToComparingFieldByFieldRecursively(
new DeployCloudFoundryServiceDescription.UserProvidedServiceAttributes()
- .setServiceName("my-service-name")
+ .setServiceInstanceName("my-service-instance-name")
.setSyslogDrainUrl("test-syslog-drain-url")
.setRouteServiceUrl("test-route-service-url")
.setTags(Collections.singleton("my-tag"))
@@ -212,7 +212,7 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
void convertManifestMapToServiceAttributesEmptyParamString() {
final Map input = HashMap.of(
"service", "my-service",
- "service_name", "my-service-name",
+ "service_instance_name", "my-service-instance-name",
"service_plan", "my-service-plan",
"tags", List.of(
"my-tag"
@@ -223,7 +223,7 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
assertThat(converter.convertManifest(input)).isEqualToComparingFieldByFieldRecursively(
new DeployCloudFoundryServiceDescription.ServiceAttributes()
.setService("my-service")
- .setServiceName("my-service-name")
+ .setServiceInstanceName("my-service-instance-name")
.setServicePlan("my-service-plan")
.setTags(Collections.singleton("my-tag"))
);
@@ -260,7 +260,7 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
assertThat(result.getServiceAttributes()).isEqualToComparingFieldByFieldRecursively(
new DeployCloudFoundryServiceDescription.ServiceAttributes()
.setService("my-service")
- .setServiceName("my-service-name")
+ .setServiceInstanceName("my-service-instance-name")
.setServicePlan("my-service-plan")
.setTags(Collections.singleton("tag1"))
.setParameterMap(HashMap.<String, Object>of(
@@ -276,7 +276,7 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
"region", "org > space",
"manifest", HashMap.of(
"type", "userProvided",
- "serviceName", "userProvidedServiceName",
+ "serviceInstanceName", "userProvidedServiceName",
"tags", List.of(
"my-tag"
).asJava(),
@@ -290,7 +290,7 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
assertThat(result.getServiceAttributes()).isNull();
assertThat(result.getUserProvidedServiceAttributes()).isEqualToComparingFieldByFieldRecursively(
new DeployCloudFoundryServiceDescription.UserProvidedServiceAttributes()
- .setServiceName("userProvidedServiceName")
+ .setServiceInstanceName("userProvidedServiceName")
.setSyslogDrainUrl("http://syslogDrainUrl.io")
.setRouteServiceUrl("http://routeServiceUrl.io")
.setTags(Collections.singleton("my-tag"))
@@ -307,7 +307,7 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
"region", "org > space",
"manifest", HashMap.of(
"type", "userProvided",
- "serviceName", "userProvidedServiceName",
+ "serviceInstanceName", "userProvidedServiceName",
"tags", List.of(
"my-tag"
).asJava(),
@@ -320,7 +320,7 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
assertThat(result.getServiceAttributes()).isNull();
assertThat(result.getUserProvidedServiceAttributes()).isEqualToComparingFieldByFieldRecursively(
new DeployCloudFoundryServiceDescription.UserProvidedServiceAttributes()
- .setServiceName("userProvidedServiceName")
+ .setServiceInstanceName("userProvidedServiceName")
.setSyslogDrainUrl("http://syslogDrainUrl.io")
.setRouteServiceUrl("http://routeServiceUrl.io")
.setTags(Collections.singleton("my-tag"))

diff:
@@ -30,7 +30,7 @@ class DeployCloudFoundryServiceAtomicOperationTest extends AbstractCloudFoundryA
desc.setServiceType("service");
desc.setClient(client);
desc.setServiceAttributes(new DeployCloudFoundryServiceDescription.ServiceAttributes()
- .setServiceName("some-service-name")
+ .setServiceInstanceName("some-service-name")
.setService("some-service")
.setServicePlan("some-service-plan")
);
@@ -47,7 +47,7 @@ class DeployCloudFoundryServiceAtomicOperationTest extends AbstractCloudFoundryA
desc.setServiceType("userProvided");
desc.setClient(client);
desc.setUserProvidedServiceAttributes(new DeployCloudFoundryServiceDescription.UserProvidedServiceAttributes()
- .setServiceName("some-service-name")
+ .setServiceInstanceName("some-service-name")
);
DeployCloudFoundryServiceAtomicOperation op = new DeployCloudFoundryServiceAtomicOperation(desc);

original_message: chore(cf): rename serviceName to serviceInstanceName (#3358)