Update cache key to contain account id

spinnaker · Feb 10, 2021 · 074fa3e · 074fa3e
1 parent 3e4fa4c
commit 074fa3e
Show file tree

Hide file tree

Showing 8 changed files with 69 additions and 40 deletions.
diff --git a/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/cache/Keys.java b/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/cache/Keys.java
@@ -86,9 +86,9 @@ public static Map<String, String> parse(String key) {
     Map<String, String> result = new HashMap<>();
     result.put("provider", parts[0]);
     result.put("type", parts[1]);
+    result.put("account", parts[2]);
 
     if (parts[1].equals(HEALTH.getNs())) {
-      result.put("account", parts[2]);
       result.put("region", parts[3]);
       result.put("taskId", parts[4]);
       return result;
@@ -99,59 +99,48 @@ public static Map<String, String> parse(String key) {
 
     switch (namespace) {
       case APPLICATIONS:
-        result.put("application", parts[2]);
+        result.put("application", parts[3]);
         break;
       case SERVICES:
-        result.put("account", parts[2]);
         result.put("region", parts[3]);
         result.put("serviceName", parts[4]);
         break;
       case ECS_CLUSTERS:
-        result.put("account", parts[2]);
         result.put("region", parts[3]);
         result.put("clusterName", parts[4]);
         break;
       case TASKS:
-        result.put("account", parts[2]);
         result.put("region", parts[3]);
         result.put("taskId", parts[4]);
         break;
       case CONTAINER_INSTANCES:
-        result.put("account", parts[2]);
         result.put("region", parts[3]);
         result.put("containerInstanceArn", parts[4]);
         break;
       case TASK_DEFINITIONS:
-        result.put("account", parts[2]);
         result.put("region", parts[3]);
         result.put("taskDefinitionArn", parts[4]);
         break;
       case ALARMS:
-        result.put("account", parts[2]);
         result.put("region", parts[3]);
         result.put("alarmArn", parts[4]);
         break;
       case IAM_ROLE:
-        result.put("account", parts[2]);
         result.put("roleName", parts[3]);
         break;
       case SECRETS:
-        result.put("account", parts[2]);
         result.put("region", parts[3]);
         result.put("secretName", parts[4]);
         break;
       case SERVICE_DISCOVERY_REGISTRIES:
-        result.put("account", parts[2]);
         result.put("region", parts[3]);
         result.put("serviceId", parts[4]);
         break;
       case SCALABLE_TARGETS:
-        result.put("account", parts[2]);
         result.put("region", parts[3]);
         result.put("resource", parts[4]);
         break;
       case TARGET_HEALTHS:
-        result.put("account", parts[2]);
         result.put("region", parts[3]);
         result.put("targetGroupArn", parts[4]);
         break;
@@ -175,8 +164,14 @@ public static String getClusterKey(String account, String region, String cluster
     return buildKey(Namespace.ECS_CLUSTERS.ns, account, region, clusterName);
   }
 
-  public static String getApplicationKey(String name) {
-    return ID + SEPARATOR + Namespace.APPLICATIONS + SEPARATOR + name.toLowerCase();
+  public static String getApplicationKey(String account, String name) {
+    return ID
+        + SEPARATOR
+        + Namespace.APPLICATIONS
+        + SEPARATOR
+        + account
+        + SEPARATOR
+        + name.toLowerCase();
   }
 
   public static String getTaskKey(String account, String region, String taskId) {

diff --git a/...in/java/com/netflix/spinnaker/clouddriver/ecs/provider/agent/ApplicationCachingAgent.java b/...in/java/com/netflix/spinnaker/clouddriver/ecs/provider/agent/ApplicationCachingAgent.java
@@ -121,7 +121,7 @@ protected Map<String, Collection<CacheData>> generateFreshData(
 
     for (Application application : applications) {
       Map<String, Object> attributes = convertApplicationToAttributes(application);
-      String applicationKey = Keys.getApplicationKey(application.getName());
+      String applicationKey = Keys.getApplicationKey(accountName, application.getName());
 
       applicationData.add(
           new DefaultCacheData(applicationKey, attributes, application.getRelationships()));

diff --git a/...-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/view/EcsApplicationProvider.java b/...-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/view/EcsApplicationProvider.java
@@ -51,11 +51,18 @@ public EcsApplicationProvider(Cache cacheView) {
 
   @Override
   public Application getApplication(String name) {
-    return translate(
-        cacheView.get(
+    Collection<CacheData> applicationsData =
+        cacheView.getAll(
             APPLICATIONS.ns,
-            Keys.getApplicationKey(name),
-            RelationshipCacheFilter.include(SERVICES.ns)));
+            cacheView.filterIdentifiers(APPLICATIONS.ns, ID + ";*"),
+            RelationshipCacheFilter.include(SERVICES.ns));
+    log.info(
+        "getApplication found {} Spinnaker applications in the cache with name {}.",
+        applicationsData.size(),
+        name);
+    Set<Application> applications = applicationsData.stream().map(this::translate).collect(toSet());
+    log.debug("Aggregating all applications for app {}", name);
+    return aggregateApplicationsForAllAccounts(applications, name);
   }
 
   @Override
@@ -74,10 +81,7 @@ public Set<Application> getApplications(boolean expand) {
   private Application translate(CacheData cacheData) {
     log.debug("Translating CacheData to EcsApplication");
     if (cacheData == null) {
-      HashMap<String, String> attributes = new HashMap<>();
-      HashMap<String, Set<String>> clusterNames = new HashMap<>();
-      EcsApplication application = new EcsApplication("test", attributes, clusterNames);
-      return application;
+      return null;
     }
 
     String appName = (String) cacheData.getAttributes().get("name");
@@ -122,4 +126,34 @@ private Set<String> getServiceRelationships(CacheData cacheData) {
         ? Collections.emptySet()
         : new HashSet<>(serviceRelationships);
   }
+
+  private Application aggregateApplicationsForAllAccounts(
+      Set<Application> applications, String appName) {
+    HashMap<String, String> attributes = new HashMap<>();
+    attributes.put("name", appName);
+
+    HashMap<String, Set<String>> clusterNames = new HashMap<>();
+
+    EcsApplication aggregatedApp = new EcsApplication(appName, attributes, clusterNames);
+    if (aggregatedApp == null) {
+      return null;
+    }
+
+    applications.forEach(
+        application -> {
+          Map<String, Set<String>> appClusters = application.getClusterNames();
+          appClusters
+              .keySet()
+              .forEach(
+                  account -> {
+                    if (aggregatedApp.getClusterNames().get(account) != null) {
+                      application.getClusterNames().get(account).addAll(appClusters.get(account));
+                    } else {
+                      application.getClusterNames().put(account, appClusters.get(account));
+                    }
+                  });
+        });
+
+    return aggregatedApp;
+  }
 }
diff --git a/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/cache/KeysSpec.groovy b/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/cache/KeysSpec.groovy
@@ -52,23 +52,23 @@ class KeysSpec extends Specification {
   }
 
   def 'should parse a given application key properly'() {
-    given:
-    def application_1 = 'test-application-1'
-    def application_2 = 'test-application-2'
-
     expect:
-    Keys.parse(ID + SEPARATOR + APPLICATIONS.ns + SEPARATOR + application_1) == [provider: ID, type: APPLICATIONS.ns, application: application_1]
-    Keys.parse(ID + SEPARATOR + APPLICATIONS.ns + SEPARATOR + application_2) == [provider: ID, type: APPLICATIONS.ns, application: application_2]
+    Keys.parse(ID + SEPARATOR + APPLICATIONS.ns + SEPARATOR + account + SEPARATOR + application) == [provider: ID, type: APPLICATIONS.ns, account: account, application: application]
+
+    where:
+    account          | application
+    'test-account-1' | 'test-application-1'
+    'test-account-2' | 'test-application-2'
   }
 
   def 'should generate the proper application key'() {
-    given:
-    def application_1 = 'test-application-1'
-    def application_2 = 'test-application-2'
-
     expect:
-    Keys.getApplicationKey(application_1) == ID + SEPARATOR + APPLICATIONS.ns + SEPARATOR + application_1
-    Keys.getApplicationKey(application_2) == ID + SEPARATOR + APPLICATIONS.ns + SEPARATOR + application_2
+    Keys.getApplicationKey(account, application) == ID + SEPARATOR + APPLICATIONS.ns + SEPARATOR + account + SEPARATOR + application
+
+    where:
+    account          | application
+    'test-account-1' | 'test-application-1'
+    'test-account-2' | 'test-application-2'
   }
 
   def 'should parse a given iam role key properly'() {

diff --git a/.../test/groovy/com/netflix/spinnaker/clouddriver/ecs/view/EcsApplicationProviderSpec.groovy b/.../test/groovy/com/netflix/spinnaker/clouddriver/ecs/view/EcsApplicationProviderSpec.groovy
@@ -67,7 +67,7 @@ class EcsApplicationProviderSpec extends Specification {
     def relationships = [Keys.getServiceKey(accountName, region, serviceName)]
 
     cache.filterIdentifiers(_, _) >> []
-    cache.get(_, _, _) >> new DefaultCacheData(appName, attributes, [(Keys.Namespace.SERVICES.ns):relationships])
+    cache.getAll(_, _, _) >> [new DefaultCacheData(appName, attributes, [(Keys.Namespace.SERVICES.ns):relationships])]
 
     when:
     def retrievedApp = provider.getApplication(appName)

diff --git a/...src/test/java/com/netflix/spinnaker/clouddriver/ecs/cache/ApplicationCacheClientTest.java b/...src/test/java/com/netflix/spinnaker/clouddriver/ecs/cache/ApplicationCacheClientTest.java
@@ -39,7 +39,7 @@ public class ApplicationCacheClientTest extends CommonCacheClient {
   public void shouldConvert() {
     // Given
     ObjectMapper mapper = new ObjectMapper();
-    String key = Keys.getApplicationKey(APP_NAME);
+    String key = Keys.getApplicationKey(ACCOUNT, APP_NAME);
 
     Application application = new Application();
 

diff --git a/.../test/java/com/netflix/spinnaker/clouddriver/ecs/provider/agent/ApplicationCacheTest.java b/.../test/java/com/netflix/spinnaker/clouddriver/ecs/provider/agent/ApplicationCacheTest.java
@@ -46,7 +46,7 @@ public class ApplicationCacheTest extends CommonCachingAgent {
   @Test
   public void shouldRetrieveFromWrittenCache() {
     // Given
-    String key = Keys.getApplicationKey(APP_NAME);
+    String key = Keys.getApplicationKey(ACCOUNT, APP_NAME);
 
     Application application = new Application();
     application.setName(APP_NAME);

diff --git a/...ava/com/netflix/spinnaker/clouddriver/ecs/provider/agent/ApplicationCachingAgentTest.java b/...ava/com/netflix/spinnaker/clouddriver/ecs/provider/agent/ApplicationCachingAgentTest.java
@@ -86,7 +86,7 @@ public void shouldGenerateFreshData() {
     List<Application> applications = new LinkedList<>();
     Set<String> keys = new HashSet<>();
     for (String appName : applicationNames) {
-      keys.add(Keys.getApplicationKey(appName));
+      keys.add(Keys.getApplicationKey(ACCOUNT, appName));
       Application application = new Application();
       application.setName(appName);
       applications.add(application);