feat(api): Allow retrieval of security groups by ID (#4540)

* feat(api): Allow retrieval of security groups by ID * fix(pr): Fix type for API parameter * fix(pr): Fix formatting * fix(pr): Implement method for AliCloud
spinnaker · Apr 23, 2020 · 8fa6ef2 · 8fa6ef2
1 parent 472da4b
commit 8fa6ef2
Show file tree

Hide file tree

Showing 13 changed files with 109 additions and 8 deletions.
diff --git a/...m/netflix/spinnaker/clouddriver/alicloud/provider/view/AliCloudSecurityGroupProvider.java b/...m/netflix/spinnaker/clouddriver/alicloud/provider/view/AliCloudSecurityGroupProvider.java
@@ -52,6 +52,16 @@ public Collection<AliCloudSecurityGroup> getAllByAccount(boolean includeRules, S
   @Override
   public AliCloudSecurityGroup get(String account, String region, String name, String vpcId) {
     String key = Keys.getSecurityGroupKey(name, "*", region, account, vpcId);
+    return getByKey(key);
+  }
+
+  @Override
+  public AliCloudSecurityGroup getById(String account, String region, String id, String vpcId) {
+    String key = Keys.getSecurityGroupKey("*", id, region, account, vpcId);
+    return getByKey(key);
+  }
+
+  private AliCloudSecurityGroup getByKey(String key) {
     Collection<String> allSecurityGroupKeys =
         cacheView.filterIdentifiers(Namespace.SECURITY_GROUPS.ns, key);
     Collection<CacheData> allData =

diff --git a/...vy/com/netflix/spinnaker/clouddriver/aws/provider/view/AmazonSecurityGroupProvider.groovy b/...vy/com/netflix/spinnaker/clouddriver/aws/provider/view/AmazonSecurityGroupProvider.groovy
@@ -140,6 +140,7 @@ class AmazonSecurityGroupProvider implements SecurityGroupProvider<AmazonSecurit
     }
   }
 
+  @Override
   AmazonSecurityGroup getById(String account, String region, String securityGroupId, String vpcId) {
     getAllMatchingKeyPattern(Keys.getSecurityGroupKey('*', securityGroupId, region, account, vpcId), true)[0]
   }

diff --git a/...om/netflix/spinnaker/clouddriver/aws/provider/view/AmazonSecurityGroupProviderSpec.groovy b/...om/netflix/spinnaker/clouddriver/aws/provider/view/AmazonSecurityGroupProviderSpec.groovy
@@ -162,6 +162,23 @@ class AmazonSecurityGroupProviderSpec extends Specification {
 
   }
 
+  void "getById returns match based on account, region, and id"() {
+
+    when:
+    def result = provider.getById(account, region, id, null)
+
+    then:
+    result != null
+    result.accountName == account
+    result.region == region
+    result.id == id
+
+    where:
+    account = 'prod'
+    region = 'us-east-1'
+    id = 'a'
+  }
+
   void "should add both ipRangeRules and securityGroup rules"() {
     given:
     String groupId = 'id-a'

diff --git a/...pinnaker/clouddriver/azure/resources/securitygroup/view/AzureSecurityGroupProvider.groovy b/...pinnaker/clouddriver/azure/resources/securitygroup/view/AzureSecurityGroupProvider.groovy
@@ -73,6 +73,11 @@ class AzureSecurityGroupProvider implements SecurityGroupProvider<AzureSecurityG
     getAllMatchingKeyPattern(Keys.getSecurityGroupKey(azureCloudProvider, name, '*', region, account), true)[0]
   }
 
+  @Override
+  AzureSecurityGroup getById(String account, String region, String id, String vnet) {
+    getAllMatchingKeyPattern(Keys.getSecurityGroupKey(azureCloudProvider, '*', id, region, account), true)[0]
+  }
+
   Set<AzureSecurityGroup> getAllMatchingKeyPattern(String pattern, boolean includeRules) {
     loadResults(includeRules, cacheView.filterIdentifiers(Keys.Namespace.SECURITY_GROUPS.ns, pattern))
   }

diff --git a/.../src/main/groovy/com/netflix/spinnaker/clouddriver/model/NoopSecurityGroupProvider.groovy b/.../src/main/groovy/com/netflix/spinnaker/clouddriver/model/NoopSecurityGroupProvider.groovy
@@ -49,4 +49,9 @@ class NoopSecurityGroupProvider implements SecurityGroupProvider {
   SecurityGroup get(String account, String region, String name, String vpcId) {
     null
   }
+
+  @Override
+  SecurityGroup getById(String account, String region, String id, String vpcId) {
+    null
+  }
 }
diff --git a/...core/src/main/groovy/com/netflix/spinnaker/clouddriver/model/SecurityGroupProvider.groovy b/...core/src/main/groovy/com/netflix/spinnaker/clouddriver/model/SecurityGroupProvider.groovy
@@ -32,4 +32,5 @@ interface SecurityGroupProvider<T extends SecurityGroup> {
 
   T get(String account, String region, String name, String vpcId)
 
+  T getById(String account, String region, String id, String vpcId)
 }
diff --git a/...in/java/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsSecurityGroupProvider.java b/...in/java/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsSecurityGroupProvider.java
@@ -87,6 +87,13 @@ public EcsSecurityGroup get(String account, String region, String name, String v
         amazonSecurityGroupProvider.get(awsAccount, region, name, vpcId));
   }
 
+  @Override
+  public EcsSecurityGroup getById(String account, String region, String id, String vpcId) {
+    String awsAccount = ecsAccountMapper.fromEcsAccountNameToAwsAccountName(account);
+    return amazonPrimitiveConverter.convertToEcsSecurityGroup(
+        amazonSecurityGroupProvider.getById(awsAccount, region, id, vpcId));
+  }
+
   @Override
   public String getCloudProvider() {
     return cloudProvider;

diff --git a/...com/netflix/spinnaker/clouddriver/google/provider/view/GoogleSecurityGroupProvider.groovy b/...com/netflix/spinnaker/clouddriver/google/provider/view/GoogleSecurityGroupProvider.groovy
@@ -85,6 +85,11 @@ class GoogleSecurityGroupProvider implements SecurityGroupProvider<GoogleSecurit
     getAllMatchingKeyPattern(Keys.getSecurityGroupKey(name, '*', region, account), true)[0]
   }
 
+  @Override
+  GoogleSecurityGroup getById(String account, String region, String id, String vpcId) {
+    getAllMatchingKeyPattern(Keys.getSecurityGroupKey('*', id, region, account), true)[0]
+  }
+
   Set<GoogleSecurityGroup> getAllMatchingKeyPattern(String pattern, boolean includeRules) {
     loadResults(includeRules, cacheView.filterIdentifiers(SECURITY_GROUPS.ns, pattern))
   }

diff --git a/...lix/spinnaker/clouddriver/huaweicloud/provider/view/HuaweiCloudSecurityGroupProvider.java b/...lix/spinnaker/clouddriver/huaweicloud/provider/view/HuaweiCloudSecurityGroupProvider.java
@@ -170,6 +170,27 @@ public HuaweiCloudSecurityGroup get(String account, String region, String name,
         .orElse(null);
   }
 
+  @Override
+  public HuaweiCloudSecurityGroup getById(String account, String region, String id, String vpcId) {
+    Set<HuaweiCloudSecurityGroup> result =
+        loadResults(
+            Keys.getSecurityGroupKey("*", id, account, region),
+            this.cacheView,
+            this.objectMapper,
+            true);
+
+    return result.stream()
+        .filter(
+            it -> {
+              boolean e1 = HuaweiCloudUtils.isEmptyStr(it.getVpcId());
+              boolean e2 = HuaweiCloudUtils.isEmptyStr(vpcId);
+
+              return (e1 == e2) && (e1 || vpcId.equals(it.getVpcId()));
+            })
+        .findFirst()
+        .orElse(null);
+  }
+
   private static Set<HuaweiCloudSecurityGroup> loadResults(
       String pattern, Cache cacheView, ObjectMapper objectMapper, boolean includeRules) {
 

diff --git a/...pinnaker/clouddriver/kubernetes/v1/provider/view/KubernetesV1SecurityGroupProvider.groovy b/...pinnaker/clouddriver/kubernetes/v1/provider/view/KubernetesV1SecurityGroupProvider.groovy
@@ -70,6 +70,13 @@ class KubernetesV1SecurityGroupProvider implements SecurityGroupProvider<Kuberne
     lookup(account, namespace, name, true).getAt(0)
   }
 
+  @Override
+  KubernetesV1SecurityGroup getById(String account, String namespace, String id, String vpcId) {
+    lookup(account, namespace, "*", true).find { KubernetesV1SecurityGroup sg ->
+      sg.id == id
+    }
+  }
+
   Set<KubernetesV1SecurityGroup> lookup(String account, String namespace, String name, boolean includeRule) {
     def keys = cacheView.filterIdentifiers(Keys.Namespace.SECURITY_GROUPS.ns, Keys.getSecurityGroupKey(account, namespace, name))
     cacheView.getAll(Keys.Namespace.SECURITY_GROUPS.ns, keys).collect {

diff --git a/...er/clouddriver/kubernetes/v2/caching/view/provider/KubernetesV2SecurityGroupProvider.java b/...er/clouddriver/kubernetes/v2/caching/view/provider/KubernetesV2SecurityGroupProvider.java
@@ -144,4 +144,9 @@ public KubernetesV2SecurityGroup get(
         .findFirst()
         .orElse(null);
   }
+
+  @Override
+  public KubernetesV2SecurityGroup getById(String account, String region, String id, String vpcId) {
+    throw new UnsupportedOperationException("Not currently implemented.");
+  }
 }
diff --git a/...com/netflix/spinnaker/clouddriver/oracle/provider/view/OracleSecurityGroupProvider.groovy b/...com/netflix/spinnaker/clouddriver/oracle/provider/view/OracleSecurityGroupProvider.groovy
@@ -67,6 +67,12 @@ class OracleSecurityGroupProvider implements SecurityGroupProvider<OracleSecurit
     getAllMatchingKeyPattern(Keys.getSecurityGroupKey(name, '*', region, account), true)[0]
   }
 
+  @Override
+  OracleSecurityGroup getById(String account, String region, String id, String vpcId) {
+    // We ignore vpcId here.
+    getAllMatchingKeyPattern(Keys.getSecurityGroupKey('*', id, region, account), true)[0]
+  }
+
   Set<OracleSecurityGroup> getAllMatchingKeyPattern(String pattern, boolean includeRules) {
     def identifiers = cacheView.filterIdentifiers(Keys.Namespace.SECURITY_GROUPS.ns, pattern)
     return loadResults(includeRules, identifiers)

diff --git a/.../main/groovy/com/netflix/spinnaker/clouddriver/controllers/SecurityGroupController.groovy b/.../main/groovy/com/netflix/spinnaker/clouddriver/controllers/SecurityGroupController.groovy
@@ -154,18 +154,29 @@ class SecurityGroupController {
   }
 
   @PreAuthorize("hasPermission(#account, 'ACCOUNT', 'READ')")
-  @RequestMapping(method = RequestMethod.GET, value = "/{account}/{cloudProvider}/{region}/{securityGroupName:.+}")
-  SecurityGroup get(@PathVariable String account,
-                    @PathVariable String cloudProvider,
-                    @PathVariable String region,
-                    @PathVariable String securityGroupName,
-                    @RequestParam(value = "vpcId", required = false) String vpcId) {
+  @RequestMapping(method = RequestMethod.GET, value = "/{account}/{cloudProvider}/{region}/{securityGroupNameOrId:.+}")
+  SecurityGroup get(
+    @PathVariable String account,
+    @PathVariable String cloudProvider,
+    @PathVariable String region,
+    @PathVariable String securityGroupNameOrId,
+    @RequestParam(value = "vpcId", required = false) String vpcId,
+    @RequestParam(value = "getById", required = false, defaultValue = "false") boolean getById
+  ) {
     def securityGroup = securityGroupProviders.findResults { secGrpProv ->
-      secGrpProv.cloudProvider == cloudProvider ? secGrpProv.get(account, region, securityGroupName, vpcId) : null
+      if (secGrpProv.cloudProvider == cloudProvider) {
+        if (getById) {
+          secGrpProv.getById(account, region, securityGroupNameOrId, vpcId)
+        } else {
+          secGrpProv.get(account, region, securityGroupNameOrId, vpcId)
+        }
+      } else {
+        null
+      }
     }
 
     if (securityGroup.size() != 1) {
-      throw new NotFoundException("Security group '${securityGroupName}' does not exist")
+      throw new NotFoundException("Security group '${securityGroupNameOrId}' does not exist")
     }
 
     return securityGroup.first()