fix(core): Merge account list string for applications that span provi…

…ders (#883)
spinnaker · Aug 28, 2019 · 159b0b5 · 159b0b5
1 parent 0977577
commit 159b0b5
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 3 deletions.
diff --git a/gate-web/src/main/groovy/com/netflix/spinnaker/gate/services/ApplicationService.groovy b/gate-web/src/main/groovy/com/netflix/spinnaker/gate/services/ApplicationService.groovy
@@ -178,10 +178,15 @@ class ApplicationService {
         if (app.clusters) {
           (mergedApp.clusters as Map).putAll(app.clusters as Map)
         }
-        String accounts = (app.clusters as Map)?.keySet()?.join(',') ?:
-          (app.clusterNames as Map)?.keySet()?.join(',')
 
-        mergedApp.attributes.accounts = accounts
+        Set<String> accounts = (app.clusters as Map)?.keySet() ?: (app.clusterNames as Map)?.keySet()
+        if (accounts) {
+          if (mergedApp.attributes.cloudDriverAccounts) {
+            accounts = new HashSet<String>(accounts)
+            accounts.addAll(mergedApp.attributes.cloudDriverAccounts.split(',').toList())
+          }
+          mergedApp.attributes.cloudDriverAccounts = accounts.join(',')
+        }
 
         (app["attributes"] as Map).entrySet().each {
           if (it.value && !(mergedApp.attributes as Map)[it.key]) {
@@ -203,6 +208,14 @@ class ApplicationService {
       mergedApp.attributes['name'] = mergedApp.name
     }
 
+    // Overwrite any Front50 account names
+    merged.each { key, mergedApp ->
+      if (mergedApp.attributes.cloudDriverAccounts) {
+        mergedApp.attributes.accounts = mergedApp.attributes.cloudDriverAccounts
+        (mergedApp.attributes as Map).remove('cloudDriverAccounts')
+      }
+    }
+
     Set<String> applicationFilter = applicationServiceConfig.config?.includedAccounts?.split(',')?.toList()?.findResults {
       it.trim() ?: null
     } ?: null

diff --git a/gate-web/src/test/groovy/com/netflix/spinnaker/gate/ApplicationServiceSpec.groovy b/gate-web/src/test/groovy/com/netflix/spinnaker/gate/ApplicationServiceSpec.groovy
@@ -207,6 +207,44 @@ class ApplicationServiceSpec extends Specification {
     email = "foo@bar.bz"
   }
 
+  void "should properly merge accounts for retrieved apps with clusterNames"() {
+    setup:
+    HystrixRequestContext.initializeContext()
+
+    def service = new ApplicationService()
+    def front50 = Mock(Front50Service)
+    def clouddriver = Mock(ClouddriverService)
+    def config = new ServiceConfiguration(services: [front50: new Service()])
+
+    service.serviceConfiguration = config
+    service.front50Service = front50
+    service.clouddriverService = clouddriver
+    service.executorService = Executors.newFixedThreadPool(1)
+
+    and:
+    def clouddriverApp1 = [name: name.toUpperCase(), attributes: [name: name], clusterNames: [prod: ["cluster-prod"]]]
+    def clouddriverApp2 = [name: name.toUpperCase(), attributes: [name: name], clusterNames: [dev: ["cluster-dev"]]]
+    def front50App = [name: name.toLowerCase(), email: email, accounts: "test"]
+
+    when:
+    service.refreshApplicationsCache()
+    def apps = service.getAllApplications()
+
+    then:
+    1 * clouddriver.getAllApplicationsUnrestricted(true) >> [clouddriverApp1, clouddriverApp2]
+    1 * front50.getAllApplicationsUnrestricted() >> [front50App]
+
+    1 == apps.size()
+    apps[0].email == email
+    apps[0].name == name
+    apps[0].clusters == null
+    apps[0].accounts == "prod,dev"
+
+    where:
+    name = "foo"
+    email = "foo@bar.bz"
+  }
+
   @Unroll
   void "should merge accounts"() {
     expect: