From 9e8ef5f5f5df9abc0d2f84c0266a489b9f966f3d Mon Sep 17 00:00:00 2001 From: Cameron Fieber <cfieber@netflix.com> Date: Mon, 11 May 2015 23:19:24 -0700 Subject: [PATCH] supports front50 service config to filter apps by account - adds oort cluster accounts to account application names - if includedAccounts is provided as a comma separated list in service configuration, only apps matching an includedAccount are returned in the application list --- .../gate/config/ServiceConfiguration.groovy | 2 +- .../gate/services/ApplicationService.groovy | 92 ++++++++++------ .../gate/ApplicationServiceSpec.groovy | 101 +++++++++++++++++- 3 files changed, 158 insertions(+), 37 deletions(-) diff --git a/gate-web/src/main/groovy/com/netflix/spinnaker/gate/config/ServiceConfiguration.groovy b/gate-web/src/main/groovy/com/netflix/spinnaker/gate/config/ServiceConfiguration.groovy index 9bbd85c018..6e9ea362d6 100644 --- a/gate-web/src/main/groovy/com/netflix/spinnaker/gate/config/ServiceConfiguration.groovy +++ b/gate-web/src/main/groovy/com/netflix/spinnaker/gate/config/ServiceConfiguration.groovy @@ -27,7 +27,7 @@ import org.springframework.stereotype.Component @ConfigurationProperties class ServiceConfiguration { List<String> discoveryHosts - Map<String, Service> services + Map<String, Service> services = [:] @Autowired ApplicationContext ctx 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 index aca360405f..0b4b9c2a3e 100644 --- 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 @@ -16,11 +16,14 @@ package com.netflix.spinnaker.gate.services import com.google.common.base.Preconditions +import com.netflix.spinnaker.gate.config.Service +import com.netflix.spinnaker.gate.config.ServiceConfiguration import com.netflix.spinnaker.gate.services.commands.HystrixFactory import com.netflix.spinnaker.gate.services.internal.Front50Service import com.netflix.spinnaker.gate.services.internal.MayoService import com.netflix.spinnaker.gate.services.internal.OortService import com.netflix.spinnaker.gate.services.internal.OrcaService +import groovy.transform.CompileDynamic import groovy.transform.CompileStatic import groovy.util.logging.Slf4j import org.springframework.beans.factory.annotation.Autowired @@ -38,6 +41,9 @@ import java.util.concurrent.Future class ApplicationService { private static final String GROUP = "applications" + @Autowired + ServiceConfiguration serviceConfiguration + @Autowired OortService oortService @@ -63,7 +69,7 @@ class ApplicationService { List<Future<List<Map>>> futures = executorService.invokeAll(applicationListRetrievers) List<List<Map>> all = futures.collect { it.get() } // spread operator doesn't work here; no clue why. List<Map> flat = (List<Map>) all?.flatten()?.toList() - mergeApps(flat).collect { it.attributes } + mergeApps(flat, serviceConfiguration.getService('front50')).collect { it.attributes } } execute() } @@ -74,7 +80,7 @@ class ApplicationService { def futures = executorService.invokeAll(applicationRetrievers) List<Map> applications = (List<Map>) futures.collect { it.get() } - def mergedApps = mergeApps(applications) + def mergedApps = mergeApps(applications, serviceConfiguration.getService('front50')) return mergedApps ? mergedApps[0] : null } execute() } @@ -156,50 +162,70 @@ class ApplicationService { } as Collection<Callable<Map>>) } - private static List<Map> mergeApps(List<Map<String, Object>> applications) { - Map<String, Map<String, Object>> merged = [:] - for (Map<String, Object> app in applications) { - if (!app) continue - String key = (app.name as String)?.toLowerCase() - if (key && !merged.containsKey(key)) { - merged[key] = [name: key, attributes:[:], clusters: [:]] as Map<String, Object> + @CompileDynamic + private static List<Map> mergeApps(List<Map<String, Object>> applications, Service applicationServiceConfig) { + + try { + Closure<String> mergeAccounts = { String s1, String s2 -> + [s1, s2].collect { String s -> + s?.split(',')?.toList()?.findResults { it.trim() ?: null } ?: [] + }.flatten().toSet().sort().join(',') } - Map mergedApp = (Map)merged[key] - if (app.containsKey("clusters")) { - // Oort - (mergedApp.clusters as Map).putAll(app.clusters as Map) - - (app["attributes"] as Map).entrySet().each { - if (it.value && !(mergedApp.attributes as Map)[it.key]) { - // don't overwrite existing attributes with metadata from oort - (mergedApp.attributes as Map)[it.key] = it.value - } + Map<String, Map<String, Object>> merged = [:] + for (Map<String, Object> app in applications) { + if (!app) continue + String key = (app.name as String)?.toLowerCase() + if (key && !merged.containsKey(key)) { + merged[key] = [name: key, attributes: [:], clusters: [:]] as Map<String, Object> } - } else { - if (app.containsKey("attributes")) { - // previously merged - ((app.attributes as Map).entrySet()).each { - if (it.value) { + Map mergedApp = (Map) merged[key] + if (app.containsKey("clusters") || app.containsKey("clusterNames")) { + // Oort + 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 = mergeAccounts(accounts, mergedApp.attributes.accounts) + + (app["attributes"] as Map).entrySet().each { + if (it.value && !(mergedApp.attributes as Map)[it.key]) { + // don't overwrite existing attributes with metadata from oort (mergedApp.attributes as Map)[it.key] = it.value } } } else { - // Front50 - app.entrySet().each { - if (it.value) { + Map attributes = app.attributes ?: app + attributes.entrySet().each { + if (it.key == 'accounts') { + mergedApp.attributes.accounts = mergeAccounts(mergedApp.attributes.accounts, it.value) + } else if (it.value) { (mergedApp.attributes as Map)[it.key] = it.value } } } + + // ensure that names are consistently lower-cased. + mergedApp.name = key.toLowerCase() + mergedApp.attributes['name'] = mergedApp.name } - // ensure that names are consistently lower-cased. - mergedApp.name = key.toLowerCase() - mergedApp.attributes['name'] = mergedApp.name + Set<String> applicationFilter = applicationServiceConfig.config?.includedAccounts?.split(',')?.toList()?.findResults { it.trim() ?: null } ?: null + return merged.values().toList().findAll { Map<String, Object> account -> + if (applicationFilter == null) { + return true + } + String[] accounts = account?.attributes?.accounts?.split(',') + if (accounts == null) { + return true + } + return accounts.any { applicationFilter.contains(it) } + } + } catch (Throwable t) { + t.printStackTrace() + throw t } - - // application doesn't exist if no attributes were found - return merged.values().toList() } static class ApplicationListRetriever implements Callable<List<Map>> { 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 index 6a48f1025b..468f59d4d8 100644 --- a/gate-web/src/test/groovy/com/netflix/spinnaker/gate/ApplicationServiceSpec.groovy +++ b/gate-web/src/test/groovy/com/netflix/spinnaker/gate/ApplicationServiceSpec.groovy @@ -16,6 +16,8 @@ package com.netflix.spinnaker.gate import com.netflix.hystrix.strategy.concurrency.HystrixRequestContext +import com.netflix.spinnaker.gate.config.Service +import com.netflix.spinnaker.gate.config.ServiceConfiguration import com.netflix.spinnaker.gate.services.ApplicationService import com.netflix.spinnaker.gate.services.CredentialsService import com.netflix.spinnaker.gate.services.internal.Front50Service @@ -34,14 +36,16 @@ class ApplicationServiceSpec extends Specification { def front50 = Mock(Front50Service) def credentialsService = Mock(CredentialsService) def oort = Mock(OortService) + def config = new ServiceConfiguration(services: [front50: new Service()]) + service.serviceConfiguration = config service.front50Service = front50 service.oortService = oort service.credentialsService = credentialsService service.executorService = Executors.newFixedThreadPool(1) and: - def oortApp = [name: name, attributes: [oortName: name, name: "bad"], clusters: [prod: [cluster]]] + def oortApp = [name: name, attributes: [oortName: name, name: "bad"], clusters: [(account): [cluster]]] def front50App = [name: name, email: email, owner: owner] when: @@ -64,6 +68,89 @@ class ApplicationServiceSpec extends Specification { providerType = "aws" } + void "should include accounts from front50 and from oort clusters"() { + setup: + HystrixRequestContext.initializeContext() + + def service = new ApplicationService() + def front50 = Mock(Front50Service) + def credentialsService = Mock(CredentialsService) + def oort = Mock(OortService) + def config = new ServiceConfiguration(services: [front50: new Service()]) + + service.serviceConfiguration = config + service.front50Service = front50 + service.oortService = oort + service.credentialsService = credentialsService + service.executorService = Executors.newFixedThreadPool(1) + + and: + def oortApp = [name: name, attributes: [oortName: name, name: "bad"], clusters: [(oortAccount): [cluster]]] + def front50App = [name: name, email: email, owner: owner] + + when: + def app = service.get(name) + + then: + 1 * front50.credentials >> { [] } + 1 * credentialsService.getAccounts() >> [[name: front50Account, type: providerType]] + 1 * oort.getApplication(name) >> oortApp + 1 * front50.getMetaData(front50Account, name) >> front50App + + app == [name: name, attributes: (oortApp.attributes + front50App + [accounts: [oortAccount, front50Account].toSet().sort().join(',')]), clusters: oortApp.clusters] + + where: + name = "foo" + email = "bar@baz.bz" + owner = "danw" + cluster = "cluster1" + oortAccount = "test" + front50Account = "prod" + providerType = "aws" + + } + + void "should return null when application account does not match includedAccounts"() { + setup: + HystrixRequestContext.initializeContext() + + def service = new ApplicationService() + def front50 = Mock(Front50Service) + def credentialsService = Mock(CredentialsService) + def oort = Mock(OortService) + def config = new ServiceConfiguration(services: [front50: new Service(config: [includedAccounts: includedAccount])]) + + service.serviceConfiguration = config + service.front50Service = front50 + service.oortService = oort + service.credentialsService = credentialsService + service.executorService = Executors.newFixedThreadPool(1) + + when: + def app = service.get(name) + + then: + 1 * front50.credentials >> { [] } + 1 * credentialsService.getAccounts() >> [[name: account, type: providerType]] + 1 * oort.getApplication(name) >> null + 1 * front50.getMetaData(account, name) >> [name: name, foo: 'bar'] + + (app == null) == expectedNull + + where: + account | includedAccount | expectedNull + "test" | "test" | false + "prod" | "test" | true + "prod" | "prod,test" | false + "prod" | "test,dev" | true + "test" | null | false + "test" | "" | false + + name = "foo" + providerType = "aws" + } + + void "should return null when no application attributes are available"() { setup: HystrixRequestContext.initializeContext() @@ -72,7 +159,9 @@ class ApplicationServiceSpec extends Specification { def front50 = Mock(Front50Service) def credentialsService = Mock(CredentialsService) def oort = Mock(OortService) + def config = new ServiceConfiguration(services: [front50: new Service()]) + service.serviceConfiguration = config service.front50Service = front50 service.oortService = oort service.credentialsService = credentialsService @@ -99,7 +188,8 @@ class ApplicationServiceSpec extends Specification { setup: def credentialsService = Mock(CredentialsService) def front50Service = Mock(Front50Service) - def service = new ApplicationService(credentialsService: credentialsService, front50Service: front50Service) + def config = new ServiceConfiguration(services: [front50: new Service()]) + def service = new ApplicationService(credentialsService: credentialsService, front50Service: front50Service, serviceConfiguration: config) when: def applicationListRetrievers = service.buildApplicationListRetrievers() @@ -118,7 +208,8 @@ class ApplicationServiceSpec extends Specification { setup: def credentialsService = Mock(CredentialsService) def front50Service = Mock(Front50Service) - def service = new ApplicationService(credentialsService: credentialsService, front50Service: front50Service) + def config = new ServiceConfiguration(services: [front50: new Service()]) + def service = new ApplicationService(credentialsService: credentialsService, front50Service: front50Service, serviceConfiguration: config) when: def applicationListRetrievers = service.buildApplicationListRetrievers() @@ -141,7 +232,9 @@ class ApplicationServiceSpec extends Specification { def front50 = Mock(Front50Service) def credentialsService = Mock(CredentialsService) def oort = Mock(OortService) + def config = new ServiceConfiguration(services: [front50: new Service()]) + service.serviceConfiguration = config service.front50Service = front50 service.oortService = oort service.credentialsService = credentialsService @@ -178,7 +271,9 @@ class ApplicationServiceSpec extends Specification { def front50 = Mock(Front50Service) def credentialsService = Mock(CredentialsService) def oort = Mock(OortService) + def config = new ServiceConfiguration(services: [front50: new Service()]) + service.serviceConfiguration = config service.front50Service = front50 service.oortService = oort service.credentialsService = credentialsService