fix(cf): service binding name validation (#5628)

* fix(cf): sanitise service binding names * fix(cf): sanitise service binding names Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
spinnaker · Mar 8, 2022 · 272fb8f · 272fb8f
1 parent a272525
commit 272fb8f
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 15 deletions.
diff --git a/...n/java/com/netflix/spinnaker/clouddriver/cloudfoundry/client/CloudFoundryClientUtils.java b/...n/java/com/netflix/spinnaker/clouddriver/cloudfoundry/client/CloudFoundryClientUtils.java
@@ -34,6 +34,8 @@
 import retrofit2.Response;
 
 public final class CloudFoundryClientUtils {
+  private static final String VALID_BINDING_NAME_PATTERN = "[^A-Za-z0-9-]+";
+
   private static final ObjectMapper mapper =
       new ObjectMapper()
           .setPropertyNamingStrategy(PropertyNamingStrategy.SNAKE_CASE)
@@ -112,4 +114,8 @@ static <R> List<Resource<R>> collectPageResources(
   public static ObjectMapper getMapper() {
     return mapper;
   }
+
+  public static String convertToValidServiceBindingName(final String name) {
+    return name.replaceAll(VALID_BINDING_NAME_PATTERN, "-");
+  }
 }
diff --git a/.../com/netflix/spinnaker/clouddriver/cloudfoundry/client/model/v2/CreateServiceBinding.java b/.../com/netflix/spinnaker/clouddriver/cloudfoundry/client/model/v2/CreateServiceBinding.java
@@ -16,17 +16,33 @@
 
 package com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.v2;
 
+import static com.netflix.spinnaker.clouddriver.cloudfoundry.client.CloudFoundryClientUtils.convertToValidServiceBindingName;
+
 import java.util.Map;
-import lombok.AllArgsConstructor;
 import lombok.Getter;
-import lombok.RequiredArgsConstructor;
 
-@RequiredArgsConstructor
-@AllArgsConstructor
 @Getter
 public class CreateServiceBinding {
   private final String serviceInstanceGuid;
   private final String appGuid;
   private final String name;
   private Map<String, Object> parameters;
+
+  public CreateServiceBinding(
+      final String serviceInstanceGuid,
+      final String appGuid,
+      final String name,
+      final Map<String, Object> parameters) {
+    this.serviceInstanceGuid = serviceInstanceGuid;
+    this.appGuid = appGuid;
+    this.name = convertToValidServiceBindingName(name);
+    this.parameters = parameters;
+  }
+
+  public CreateServiceBinding(
+      final String serviceInstanceGuid, final String appGuid, final String name) {
+    this.serviceInstanceGuid = serviceInstanceGuid;
+    this.appGuid = appGuid;
+    this.name = convertToValidServiceBindingName(name);
+  }
 }
diff --git a/.../clouddriver/cloudfoundry/deploy/ops/DeleteCloudFoundryServiceBindingAtomicOperation.java b/.../clouddriver/cloudfoundry/deploy/ops/DeleteCloudFoundryServiceBindingAtomicOperation.java
@@ -16,6 +16,7 @@
 
 package com.netflix.spinnaker.clouddriver.cloudfoundry.deploy.ops;
 
+import com.netflix.spinnaker.clouddriver.cloudfoundry.client.CloudFoundryClientUtils;
 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;
@@ -38,10 +39,16 @@ private static Task getTask() {
 
   @Override
   public Void operate(List<Void> priorOutputs) {
-
     List<String> unbindingServiceInstanceNames =
         description.getServiceUnbindingRequests().stream()
-            .map(s -> s.getServiceInstanceName())
+            .map(
+                DeleteCloudFoundryServiceBindingDescription.ServiceUnbindingRequest
+                    ::getServiceInstanceName)
+            .collect(Collectors.toList());
+
+    List<String> unbindingServiceBindingNames =
+        unbindingServiceInstanceNames.stream()
+            .map(CloudFoundryClientUtils::convertToValidServiceBindingName)
             .collect(Collectors.toList());
 
     getTask()
@@ -58,7 +65,7 @@ public Void operate(List<Void> priorOutputs) {
             .getApplications()
             .getServiceBindingsByApp(description.getServerGroupId());
 
-    removeBindings(bindings, unbindingServiceInstanceNames);
+    removeBindings(bindings, unbindingServiceBindingNames);
 
     getTask()
         .updateStatus(
@@ -72,15 +79,14 @@ public Void operate(List<Void> priorOutputs) {
   }
 
   private void removeBindings(
-      List<Resource<ServiceBinding>> bindings, List<String> unbindingServiceInstanceNames) {
+      List<Resource<ServiceBinding>> bindings, List<String> unbindingServiceBindingNames) {
     bindings.stream()
-        .filter(b -> unbindingServiceInstanceNames.contains(b.getEntity().getName()))
+        .filter(b -> unbindingServiceBindingNames.contains(b.getEntity().getName()))
         .forEach(
-            b -> {
-              description
-                  .getClient()
-                  .getServiceInstances()
-                  .deleteServiceBinding(b.getMetadata().getGuid());
-            });
+            b ->
+                description
+                    .getClient()
+                    .getServiceInstances()
+                    .deleteServiceBinding(b.getMetadata().getGuid()));
   }
 }
diff --git a/...ava/com/netflix/spinnaker/clouddriver/cloudfoundry/client/HttpCloudFoundryClientTest.java b/...ava/com/netflix/spinnaker/clouddriver/cloudfoundry/client/HttpCloudFoundryClientTest.java
@@ -25,9 +25,11 @@
 
 import com.github.tomakehurst.wiremock.WireMockServer;
 import com.github.tomakehurst.wiremock.matching.UrlPattern;
+import com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.v2.CreateServiceBinding;
 import com.netflix.spinnaker.clouddriver.cloudfoundry.config.CloudFoundryConfigurationProperties;
 import com.netflix.spinnaker.clouddriver.cloudfoundry.model.CloudFoundryOrganization;
 import java.util.Optional;
+import java.util.UUID;
 import java.util.concurrent.ForkJoinPool;
 import okhttp3.OkHttpClient;
 import org.jetbrains.annotations.NotNull;
@@ -194,6 +196,17 @@ void createRetryInterceptorShouldReturnOnSecondAttempt(
         .containsExactly("orgId", "orgName");
   }
 
+  @Test
+  void shouldReplaceInvalidNameCharacters() {
+    String invalidBindingName = "test-service-binding~123#test";
+    String sanitisedBindingName = "test-service-binding-123-test";
+
+    CreateServiceBinding binding =
+        new CreateServiceBinding(
+            UUID.randomUUID().toString(), UUID.randomUUID().toString(), invalidBindingName);
+    assertThat(binding.getName()).isEqualTo(sanitisedBindingName);
+  }
+
   private void stubServer(
       WireMockServer server, int status, String currentState, String nextState) {
     stubServer(server, status, currentState, nextState, "");