fix(cloudfoundry): delete binding by default binding name (#5420)

Co-authored-by: Jorge <jorgebee65@hotmail.com>

clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/client/model/v2/CreateServiceBinding.java

@@ -27,5 +27,6 @@
 public class CreateServiceBinding {
   private final String serviceInstanceGuid;
   private final String appGuid;
+  private final String name;
   private Map<String, Object> parameters;
 }

clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/deploy/converters/DeleteCloudFoundryServiceBindingAtomicOperationConverter.java

@@ -20,30 +20,21 @@
 import com.netflix.spinnaker.clouddriver.cloudfoundry.client.CloudFoundryApiException;
 import com.netflix.spinnaker.clouddriver.cloudfoundry.deploy.description.DeleteCloudFoundryServiceBindingDescription;
 import com.netflix.spinnaker.clouddriver.cloudfoundry.deploy.ops.DeleteCloudFoundryServiceBindingAtomicOperation;
-import com.netflix.spinnaker.clouddriver.helpers.OperationPoller;
 import com.netflix.spinnaker.clouddriver.orchestration.AtomicOperation;
 import com.netflix.spinnaker.clouddriver.orchestration.AtomicOperations;
 import java.util.Map;
 import org.jetbrains.annotations.Nullable;
-import org.springframework.beans.factory.annotation.Qualifier;
 import org.springframework.stereotype.Component;
 
 @CloudFoundryOperation(AtomicOperations.DELETE_SERVICE_BINDINGS)
 @Component
 public class DeleteCloudFoundryServiceBindingAtomicOperationConverter
     extends AbstractCloudFoundryServerGroupAtomicOperationConverter {
-  private final OperationPoller operationPoller;
-
-  public DeleteCloudFoundryServiceBindingAtomicOperationConverter(
-      @Qualifier("cloudFoundryOperationPoller") OperationPoller operationPoller) {
-    this.operationPoller = operationPoller;
-  }
 
   @Nullable
   @Override
   public AtomicOperation convertOperation(Map input) {
-    return new DeleteCloudFoundryServiceBindingAtomicOperation(
-        operationPoller, convertDescription(input));
+    return new DeleteCloudFoundryServiceBindingAtomicOperation(convertDescription(input));
   }
 
   @Override

clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/deploy/ops/CreateCloudFoundryServiceBindingAtomicOperation.java

@@ -83,7 +83,10 @@ public Void operate(List<Void> priorOutputs) {
                     removeBindings(serviceGuid, description.getServerGroupId());
                   }
                   return new CreateServiceBinding(
-                      serviceGuid, description.getServerGroupId(), s.getParameters());
+                      serviceGuid,
+                      description.getServerGroupId(),
+                      s.getServiceInstanceName(),
+                      s.getParameters());
                 })
             .collect(Collectors.toList());
 

clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/deploy/ops/DeleteCloudFoundryServiceBindingAtomicOperation.java

@@ -16,26 +16,20 @@
 
 package com.netflix.spinnaker.clouddriver.cloudfoundry.deploy.ops;
 
-import static com.netflix.spinnaker.clouddriver.cloudfoundry.deploy.ops.CloudFoundryOperationUtils.describeProcessState;
-
-import com.netflix.spinnaker.clouddriver.cloudfoundry.client.CloudFoundryApiException;
-import com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.v3.ProcessStats;
+import com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.v2.Resource;
+import com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.v2.ServiceBinding;
 import com.netflix.spinnaker.clouddriver.cloudfoundry.deploy.description.DeleteCloudFoundryServiceBindingDescription;
 import com.netflix.spinnaker.clouddriver.data.task.Task;
 import com.netflix.spinnaker.clouddriver.data.task.TaskRepository;
-import com.netflix.spinnaker.clouddriver.helpers.OperationPoller;
 import com.netflix.spinnaker.clouddriver.orchestration.AtomicOperation;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
 import java.util.stream.Collectors;
 import lombok.RequiredArgsConstructor;
 
 @RequiredArgsConstructor
 public class DeleteCloudFoundryServiceBindingAtomicOperation implements AtomicOperation<Void> {
 
   private static final String PHASE = "DELETE_SERVICE_BINDINGS";
-  private final OperationPoller operationPoller;
   private final DeleteCloudFoundryServiceBindingDescription description;
 
   private static Task getTask() {
@@ -45,107 +39,48 @@ private static Task getTask() {
   @Override
   public Void operate(List<Void> priorOutputs) {
 
-    List<String> serviceInstanceNames =
+    List<String> unbindingServiceInstanceNames =
         description.getServiceUnbindingRequests().stream()
             .map(s -> s.getServiceInstanceName())
             .collect(Collectors.toList());
 
     getTask()
         .updateStatus(
             PHASE,
-            "Deleting Cloud Foundry service bindings between application '"
+            "Unbinding Cloud Foundry application '"
                 + description.getServerGroupName()
-                + "' and services: "
-                + serviceInstanceNames);
-
-    Map<String, String> serviceInstanceGuids = new HashMap<>();
-
-    description
-        .getClient()
-        .getApplications()
-        .getServiceBindingsByApp(description.getServerGroupId())
-        .stream()
-        .forEach(
-            s ->
-                serviceInstanceGuids.put(
-                    s.getEntity().getName(), s.getEntity().getServiceInstanceGuid()));
+                + "' from services: "
+                + unbindingServiceInstanceNames);
 
-    if (serviceInstanceNames.size() != description.getServiceUnbindingRequests().size()) {
-      throw new CloudFoundryApiException(
-          "Number of service instances found does not match the number of service unbinding requests.");
-    }
+    List<Resource<ServiceBinding>> bindings =
+        description
+            .getClient()
+            .getApplications()
+            .getServiceBindingsByApp(description.getServerGroupId());
 
-    List<String> unbindings =
-        description.getServiceUnbindingRequests().stream()
-            .map(
-                s -> {
-                  String serviceGuid = serviceInstanceGuids.get(s.getServiceInstanceName());
-                  if (serviceGuid == null || serviceGuid.isEmpty()) {
-                    throw new CloudFoundryApiException(
-                        "Unable to find service with the name: '"
-                            + s.getServiceInstanceName()
-                            + "'");
-                  }
-                  return serviceGuid;
-                })
-            .collect(Collectors.toList());
-
-    unbindings.forEach(u -> removeBindings(u, description.getServerGroupId()));
-
-    // Restart by default
-    getTask().updateStatus(PHASE, "Restaging application '" + description.getServerGroupName());
-    description.getClient().getApplications().restageApplication(description.getServerGroupId());
-
-    ProcessStats.State state =
-        operationPoller.waitForOperation(
-            () ->
-                description
-                    .getClient()
-                    .getApplications()
-                    .getAppState(description.getServerGroupId()),
-            inProgressState ->
-                inProgressState == ProcessStats.State.RUNNING
-                    || inProgressState == ProcessStats.State.CRASHED,
-            null,
-            getTask(),
-            description.getServerGroupName(),
-            PHASE);
-
-    if (state != ProcessStats.State.RUNNING) {
-      getTask()
-          .updateStatus(
-              PHASE,
-              "Failed to delete Cloud Foundry service bindings between application '"
-                  + description.getServerGroupName()
-                  + "' and services: "
-                  + serviceInstanceNames);
-      throw new CloudFoundryApiException(
-          "Failed to start '"
-              + description.getServerGroupName()
-              + "' which instead "
-              + describeProcessState(state));
-    }
+    removeBindings(bindings, unbindingServiceInstanceNames);
 
     getTask()
         .updateStatus(
             PHASE,
-            "Deleted Cloud Foundry service from application '"
+            "Successfully unbound Cloud Foundry application '"
                 + description.getServerGroupName()
-                + "' and services: "
-                + serviceInstanceNames);
+                + "' from services: "
+                + unbindingServiceInstanceNames);
 
     return null;
   }
 
-  private void removeBindings(String serviceGuid, String appGuid) {
-    description.getClient().getApplications().getServiceBindingsByApp(appGuid).stream()
-        .filter(s -> serviceGuid.equalsIgnoreCase(s.getEntity().getServiceInstanceGuid()))
-        .findAny()
-        .ifPresent(
-            s ->
-                description
-                    .getClient()
-                    .getServiceInstances()
-                    .deleteServiceBinding(s.getMetadata().getGuid()));
+  private void removeBindings(
+      List<Resource<ServiceBinding>> bindings, List<String> unbindingServiceInstanceNames) {
+    bindings.stream()
+        .filter(b -> unbindingServiceInstanceNames.contains(b.getEntity().getName()))
+        .forEach(
+            b -> {
+              description
+                  .getClient()
+                  .getServiceInstances()
+                  .deleteServiceBinding(b.getMetadata().getGuid());
+            });
   }
 }

clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/deploy/ops/DeployCloudFoundryServerGroupAtomicOperation.java

@@ -170,8 +170,8 @@ private void createServiceBindings(
     List<CreateServiceBinding> bindings =
         serviceNames.stream()
             .map(
-                s -> {
-                  String serviceGuid = serviceInstanceGuids.get(s);
+                name -> {
+                  String serviceGuid = serviceInstanceGuids.get(name);
                   if (serviceGuid == null || serviceGuid.isEmpty()) {
                     getTask()
                         .updateStatus(
@@ -183,13 +183,13 @@ private void createServiceBindings(
 
                     throw new CloudFoundryApiException(
                         "Unable to find service with the name: '"
-                            + s
+                            + name
                             + "' in "
                             + serverGroup.getSpace());
                   }
 
                   return new CreateServiceBinding(
-                      serviceGuid, serverGroup.getId(), Collections.emptyMap());
+                      serviceGuid, serverGroup.getId(), name, Collections.emptyMap());
                 })
             .collect(Collectors.toList());
 

clouddriver-cloudfoundry/src/test/java/com/netflix/spinnaker/clouddriver/cloudfoundry/client/ServiceInstancesTest.java

@@ -86,7 +86,7 @@ void shouldCreateServiceBindingWhenServiceExists() {
     Page<ServiceInstance> serviceMappingPageOne = Page.singleton(null, "service-instance-guid");
     CreateServiceBinding binding =
         new CreateServiceBinding(
-            "service-instance-guid", cloudFoundryServerGroup.getId(), emptyMap());
+            "service-instance-guid", cloudFoundryServerGroup.getId(), "service-name", emptyMap());
     serviceMappingPageOne.setTotalResults(0);
     serviceMappingPageOne.setTotalPages(0);
     when(serviceInstanceService.all(eq(null), any()))
@@ -121,7 +121,7 @@ void shouldCreateServiceBindingWhenUserProvidedServiceExists() {
     Page<ServiceInstance> serviceMappingPageOne = createEmptyOsbServiceInstancePage();
     CreateServiceBinding binding =
         new CreateServiceBinding(
-            "service-instance-guid", cloudFoundryServerGroup.getId(), emptyMap());
+            "service-instance-guid", cloudFoundryServerGroup.getId(), "service-name", emptyMap());
     when(serviceInstanceService.all(eq(null), any()))
         .thenAnswer(invocation -> Calls.response(Response.success(serviceMappingPageOne)));
     when(serviceInstanceService.all(eq(1), any()))
@@ -157,7 +157,7 @@ void shouldSucceedServiceBindingWhenServiceBindingExists() {
     Page<ServiceInstance> serviceMappingPageOne = Page.singleton(null, "service-instance-guid");
     CreateServiceBinding binding =
         new CreateServiceBinding(
-            "service-instance-guid", cloudFoundryServerGroup.getId(), emptyMap());
+            "service-instance-guid", cloudFoundryServerGroup.getId(), "service-name", emptyMap());
     serviceMappingPageOne.setTotalResults(0);
     serviceMappingPageOne.setTotalPages(0);
     when(serviceInstanceService.all(eq(null), any()))

clouddriver-cloudfoundry/src/test/java/com/netflix/spinnaker/clouddriver/cloudfoundry/deploy/ops/DeleteCloudFoundryServiceBindingAtomicOperationTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2021 Netflix, Inc.
+ * Copyright 2021 Armory, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -23,30 +23,27 @@
 
 import com.netflix.spinnaker.clouddriver.cloudfoundry.client.CloudFoundryClient;
 import com.netflix.spinnaker.clouddriver.cloudfoundry.client.MockCloudFoundryClient;
+import com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.v2.AbstractServiceInstance;
 import com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.v2.Resource;
 import com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.v2.ServiceBinding;
-import com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.v3.ProcessStats;
+import com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.v2.ServiceInstance;
 import com.netflix.spinnaker.clouddriver.cloudfoundry.deploy.description.DeleteCloudFoundryServiceBindingDescription;
 import com.netflix.spinnaker.clouddriver.cloudfoundry.model.CloudFoundryOrganization;
 import com.netflix.spinnaker.clouddriver.cloudfoundry.model.CloudFoundrySpace;
 import com.netflix.spinnaker.clouddriver.data.task.Task;
-import com.netflix.spinnaker.clouddriver.helpers.OperationPoller;
 import java.util.Collections;
 import java.util.List;
-import java.util.function.Supplier;
 import org.junit.jupiter.api.Test;
 
 public class DeleteCloudFoundryServiceBindingAtomicOperationTest
     extends AbstractCloudFoundryAtomicOperationTest {
 
-  OperationPoller poller = mock(OperationPoller.class);
-  CloudFoundryClient client = new MockCloudFoundryClient();
-
   private final CloudFoundrySpace cloudFoundrySpace =
       CloudFoundrySpace.builder()
           .name("space")
           .organization(CloudFoundryOrganization.builder().name("org").build())
           .build();
+  CloudFoundryClient client = new MockCloudFoundryClient();
 
   @Test
   public void shouldDeleteServiceBinding() {
@@ -63,36 +60,47 @@ public void shouldDeleteServiceBinding() {
     desc.setServiceUnbindingRequests(Collections.singletonList(unbinding));
 
     DeleteCloudFoundryServiceBindingAtomicOperation operation =
-        new DeleteCloudFoundryServiceBindingAtomicOperation(poller, desc);
+        new DeleteCloudFoundryServiceBindingAtomicOperation(desc);
+    ServiceBinding appServiceInstance = new ServiceBinding();
+    appServiceInstance.setName("service1");
+    appServiceInstance.setAppGuid("app1");
+    appServiceInstance.setServiceInstanceGuid("service-guid-123");
+
+    Resource<ServiceBinding> appResource = new Resource<>();
+    Resource.Metadata appMetadata = new Resource.Metadata();
+    appMetadata.setGuid("service-guid-123");
+    appResource.setEntity(appServiceInstance);
+    appResource.setMetadata(appMetadata);
 
-    ServiceBinding serviceInstance = new ServiceBinding();
+    List<Resource<ServiceBinding>> appInstances = List.of(appResource);
+    when(desc.getClient().getApplications().getServiceBindingsByApp(any()))
+        .thenReturn(appInstances);
+
+    ServiceInstance serviceInstance = new ServiceInstance();
     serviceInstance.setName("service1");
-    serviceInstance.setAppGuid("app1");
-    serviceInstance.setServiceInstanceGuid("service-guid-123");
 
-    Resource<ServiceBinding> resource = new Resource<>();
+    Resource<ServiceInstance> resource = new Resource<>();
     Resource.Metadata metadata = new Resource.Metadata();
-    metadata.setGuid("123abc");
+    metadata.setGuid("service-guid-123");
     resource.setEntity(serviceInstance);
     resource.setMetadata(metadata);
 
-    List<Resource<ServiceBinding>> instances = List.of(resource);
-    when(desc.getClient().getApplications().getServiceBindingsByApp(any())).thenReturn(instances);
-    when(poller.waitForOperation(any(Supplier.class), any(), any(), any(), any(), any()))
-        .thenReturn(ProcessStats.State.RUNNING);
+    List<Resource<? extends AbstractServiceInstance>> serviceInstances = List.of(resource);
+
+    when(desc.getClient().getServiceInstances().findAllServicesBySpaceAndNames(any(), any()))
+        .thenReturn(serviceInstances);
 
     Task task = runOperation(operation);
 
     verify(client.getServiceInstances()).deleteServiceBinding(any());
     assertThat(task.getHistory())
         .has(
-            status(
-                "Deleting Cloud Foundry service bindings between application 'app1' and services: [service1]"),
+            status("Unbinding Cloud Foundry application 'app1' from services: [service1]"),
             atIndex(1));
     assertThat(task.getHistory())
         .has(
             status(
-                "Deleted Cloud Foundry service from application 'app1' and services: [service1]"),
-            atIndex(3));
+                "Successfully unbound Cloud Foundry application 'app1' from services: [service1]"),
+            atIndex(2));
   }
 }