refactor(core): remove skin and providerVersion (#8263)

* refactor(core): remove skins

* refactor(core): remove provider version

Co-authored-by: Chris Thielen <christopherthielen@users.noreply.github.com>

app/scripts/modules/core/src/account/AccountService.ts

@@ -7,6 +7,7 @@ import { Application } from 'core/application/application.model';
 import { API } from 'core/api/ApiService';
 import { CloudProviderRegistry } from '../cloudProvider/CloudProviderRegistry';
 import { SETTINGS } from 'core/config/settings';
+import { ILoadBalancer, IServerGroup } from 'core/domain';
 
 export interface IRegion {
   account?: string;
@@ -23,8 +24,6 @@ export interface IAccount {
   name: string;
   requiredGroupMembership: string[];
   type: string;
-  providerVersion?: string;
-  skin?: string;
 }
 
 export interface IArtifactAccount {
@@ -110,11 +109,8 @@ export class AccountService {
     return this.listAllAccounts().then(accounts => accounts.find(a => a.name === account));
   }
 
-  public static getAllAccountDetailsForProvider(
-    provider: string,
-    providerVersion: string = null,
-  ): IPromise<IAccountDetails[]> {
-    return this.listAccounts(provider, providerVersion).catch((error: any) => {
+  public static getAllAccountDetailsForProvider(provider: string): IPromise<IAccountDetails[]> {
+    return this.listAccounts(provider).catch((error: any) => {
       $log.warn(`Failed to load accounts for provider "${provider}"; exception:`, error);
       return [];
     });
@@ -172,19 +168,14 @@ export class AccountService {
     });
   }
 
-  public static listAllAccounts(provider: string = null, providerVersion: string = null): IPromise<IAccountDetails[]> {
+  public static listAllAccounts(provider: string = null): IPromise<IAccountDetails[]> {
     return $q
       .when(this.accounts$.toPromise())
-      .then((accounts: IAccountDetails[]) => accounts.filter(account => !provider || account.type === provider))
-      .then((accounts: IAccountDetails[]) =>
-        accounts.filter(account => !providerVersion || account.providerVersion === providerVersion),
-      );
+      .then((accounts: IAccountDetails[]) => accounts.filter(account => !provider || account.type === provider));
   }
 
-  public static listAccounts(provider: string = null, providerVersion: string = null): IPromise<IAccountDetails[]> {
-    return this.listAllAccounts(provider, providerVersion).then(accounts =>
-      accounts.filter(account => account.authorized !== false),
-    );
+  public static listAccounts(provider: string = null): IPromise<IAccountDetails[]> {
+    return this.listAllAccounts(provider).then(accounts => accounts.filter(account => account.authorized !== false));
   }
 
   public static applicationAccounts(application: Application = null): IPromise<IAccountDetails[]> {
@@ -214,6 +205,27 @@ export class AccountService {
   public static listProviders(application: Application = null): IPromise<string[]> {
     return $q.when(this.listProviders$(application).toPromise());
   }
+
+  public static getAccountForInstance(cloudProvider: string, instanceId: string, app: Application): IPromise<string> {
+    return app.ready().then(() => {
+      const serverGroups = app.getDataSource('serverGroups').data as IServerGroup[];
+      const loadBalancers = app.getDataSource('loadBalancers').data as ILoadBalancer[];
+      const loadBalancerServerGroups = loadBalancers.map(lb => lb.serverGroups).reduce((acc, sg) => acc.concat(sg), []);
+
+      const hasInstance = (obj: IServerGroup | ILoadBalancer) => {
+        return (
+          obj.cloudProvider === cloudProvider && (obj.instances || []).some(instance => instance.id === instanceId)
+        );
+      };
+
+      const all: Array<IServerGroup | ILoadBalancer> = []
+        .concat(serverGroups)
+        .concat(loadBalancers)
+        .concat(loadBalancerServerGroups);
+      const found = all.find(hasInstance);
+      return found && found.account;
+    });
+  }
 }
 
 AccountService.initialize();

app/scripts/modules/core/src/cloudProvider/CloudProviderRegistry.spec.ts

@@ -100,29 +100,4 @@ describe('CloudProviderRegistry: API', function() {
       expect(CloudProviderRegistry.hasValue('boo', 'bar.baz')).toBe(false);
     });
   });
-
-  describe('skinned provider configs', () => {
-    it('returns a value from a skinned provider config when skin is specified', () => {
-      CloudProviderRegistry.registerProvider('kubernetes', { name: 'kubernetes', skin: 'v1' });
-      CloudProviderRegistry.registerProvider('kubernetes', { name: 'kubernetes', skin: 'v2' });
-
-      expect(CloudProviderRegistry.getValue('kubernetes', 'skin', 'v1')).toBe('v1');
-      expect(CloudProviderRegistry.getValue('kubernetes', 'skin', 'v2')).toBe('v2');
-    });
-
-    it('returns a value from the default skin if skin is not specified', () => {
-      CloudProviderRegistry.registerProvider('kubernetes', { name: 'kubernetes', skin: 'v1' });
-      CloudProviderRegistry.registerProvider('kubernetes', { name: 'kubernetes', skin: 'v2', defaultSkin: true });
-
-      expect(CloudProviderRegistry.getValue('kubernetes', 'skin')).toBe('v2');
-    });
-
-    // This behavior is implicitly tested in other tests, but demonstrates that the provider
-    // configs do not need to add a `defaultSkin` flag if there is only one config for that provider.
-    it('behaves reasonably if a provider does not define a default config version', () => {
-      CloudProviderRegistry.registerProvider('gce', { name: 'gce', key: 'value' });
-
-      expect(CloudProviderRegistry.getValue('gce', 'key')).toBe('value');
-    });
-  });
 });

app/scripts/modules/core/src/cloudProvider/CloudProviderRegistry.ts

@@ -10,9 +10,6 @@ export interface ICloudProviderLogo {
 export interface ICloudProviderConfig {
   name: string;
   logo?: ICloudProviderLogo;
-  providerVersion?: string;
-  skin?: string;
-  defaultSkin?: boolean;
   [attribute: string]: any;
 }
 
@@ -24,33 +21,22 @@ class Providers {
     // The tests depend on this behavior, but maybe something else does as well.
     this.providers = without(
       this.providers,
-      this.providers.find(p => p.cloudProvider === cloudProvider && p.config.skin === config.skin),
+      this.providers.find(p => p.cloudProvider === cloudProvider),
     ).concat([{ cloudProvider, config }]);
   }
 
-  public get(cloudProvider: string, skin?: string): ICloudProviderConfig {
-    if (skin) {
-      const provider = this.providers.find(p => p.cloudProvider === cloudProvider && p.config.skin === skin);
-      return provider ? provider.config : this.getDefaultSkin(cloudProvider);
-    } else {
-      return this.getDefaultSkin(cloudProvider);
-    }
+  public get(cloudProvider: string): ICloudProviderConfig {
+    const provider = this.providers.find(p => p.cloudProvider === cloudProvider);
+    return provider ? provider.config : null;
   }
 
-  public has(cloudProvider: string, skin?: string): boolean {
-    return !!this.get(cloudProvider, skin);
+  public has(cloudProvider: string): boolean {
+    return !!this.get(cloudProvider);
   }
 
   public keys(): string[] {
     return uniq(this.providers.map(p => p.cloudProvider));
   }
-
-  private getDefaultSkin(cloudProvider: string): ICloudProviderConfig {
-    const provider = this.providers.some(p => p.cloudProvider === cloudProvider && p.config.defaultSkin)
-      ? this.providers.find(p => p.cloudProvider === cloudProvider && p.config.defaultSkin)
-      : this.providers.find(p => p.cloudProvider === cloudProvider);
-    return provider ? provider.config : null;
-  }
 }
 
 export class CloudProviderRegistry {
@@ -65,22 +51,20 @@ export class CloudProviderRegistry {
     }
   }
 
-  public static getProvider(cloudProvider: string, skin?: string): ICloudProviderConfig {
-    return this.providers.has(cloudProvider, skin) ? cloneDeep(this.providers.get(cloudProvider, skin)) : null;
+  public static getProvider(cloudProvider: string): ICloudProviderConfig {
+    return this.providers.has(cloudProvider) ? cloneDeep(this.providers.get(cloudProvider)) : null;
   }
 
   public static listRegisteredProviders(): string[] {
     return Array.from(this.providers.keys());
   }
 
-  public static overrideValue(cloudProvider: string, key: string, overrideValue: any, skin?: string) {
-    if (!this.providers.has(cloudProvider, skin)) {
-      console.warn(
-        `Cannot override "${key}" for provider "${cloudProvider}${skin ? `:${skin}` : ''}" (provider not registered)`,
-      );
+  public static overrideValue(cloudProvider: string, key: string, overrideValue: any) {
+    if (!this.providers.has(cloudProvider)) {
+      console.warn(`Cannot override "${key}" for provider "${cloudProvider}" (provider not registered)`);
       return;
     }
-    const config = this.providers.get(cloudProvider, skin);
+    const config = this.providers.get(cloudProvider);
     const parentKeys = key.split('.');
     const lastKey = parentKeys.pop();
     let current = config;
@@ -95,15 +79,15 @@ export class CloudProviderRegistry {
     current[lastKey] = overrideValue;
   }
 
-  public static hasValue(cloudProvider: string, key: string, skin?: string) {
-    return this.providers.has(cloudProvider, skin) && this.getValue(cloudProvider, key, skin) !== null;
+  public static hasValue(cloudProvider: string, key: string) {
+    return this.providers.has(cloudProvider) && this.getValue(cloudProvider, key) !== null;
   }
 
-  public static getValue(cloudProvider: string, key: string, skin?: string): any {
-    if (!key || !this.providers.has(cloudProvider, skin)) {
+  public static getValue(cloudProvider: string, key: string): any {
+    if (!key || !this.providers.has(cloudProvider)) {
       return null;
     }
-    const config = this.getProvider(cloudProvider, skin);
+    const config = this.getProvider(cloudProvider);
     const keyParts = key.split('.');
     let current = config;
     let notFound = false;

app/scripts/modules/core/src/cloudProvider/cloudProvider.module.ts

@@ -1,7 +1,6 @@
 import { module } from 'angular';
 
 import { CLOUD_PROVIDER_LOGO } from './cloudProviderLogo.component';
-import { SKIN_SELECTOR_CTRL } from './skinSelection/skinSelector.component';
 import { CORE_CLOUDPROVIDER_CLOUDPROVIDERLABEL_DIRECTIVE } from './cloudProviderLabel.directive';
 import { CORE_CLOUDPROVIDER_PROVIDERSELECTION_PROVIDERSELECTOR_DIRECTIVE } from './providerSelection/providerSelector.directive';
 
@@ -10,5 +9,4 @@ module(CLOUD_PROVIDER_MODULE, [
   CLOUD_PROVIDER_LOGO,
   CORE_CLOUDPROVIDER_CLOUDPROVIDERLABEL_DIRECTIVE,
   CORE_CLOUDPROVIDER_PROVIDERSELECTION_PROVIDERSELECTOR_DIRECTIVE,
-  SKIN_SELECTOR_CTRL,
 ]);

app/scripts/modules/core/src/cloudProvider/index.ts

@@ -3,4 +3,3 @@ export * from './providerSelection/ProviderSelectionService';
 export * from './CloudProviderLabel';
 export * from './CloudProviderLogo';
 export * from './providerService.delegate';
-export * from './skin.service';

app/scripts/modules/core/src/cloudProvider/providerSelection/ProviderSelectionService.spec.ts

@@ -148,7 +148,6 @@ describe('ProviderSelectionService: API', () => {
     let provider = '';
     hasValue = true;
     const k8s = fakeAccount('kubernetes');
-    k8s.skin = 'v2';
     accounts = [k8s];
     CloudProviderRegistry.registerProvider('kubernetes', config);
     SETTINGS.defaultProvider = 'defaultProvider';
@@ -165,7 +164,6 @@ describe('ProviderSelectionService: API', () => {
     let provider = '';
     hasValue = true;
     const k8s = fakeAccount('kubernetes');
-    k8s.skin = 'v2';
     accounts = [k8s, fakeAccount('titus')];
     CloudProviderRegistry.registerProvider('titus', config);
     CloudProviderRegistry.registerProvider('kubernetes', config);

app/scripts/modules/core/src/cloudProvider/providerSelection/ProviderSelectionService.ts

@@ -24,7 +24,7 @@ export class ProviderSelectionService {
 
       if (filterFn) {
         reducedAccounts = reducedAccounts.filter((acc: IAccountDetails) => {
-          return filterFn(application, acc, CloudProviderRegistry.getProvider(acc.cloudProvider, acc.skin));
+          return filterFn(application, acc, CloudProviderRegistry.getProvider(acc.cloudProvider));
         });
       }
 

app/scripts/modules/core/src/cloudProvider/providerService.delegate.ts

@@ -9,13 +9,13 @@ export class ProviderServiceDelegate {
   public static $inject = ['$injector', '$q'];
   constructor(private $injector: IInjectorService, private $q: IQService) {}
 
-  public hasDelegate(provider: string, serviceKey: string, skin?: string): boolean {
-    const service: string = CloudProviderRegistry.getValue(provider, serviceKey, skin);
+  public hasDelegate(provider: string, serviceKey: string): boolean {
+    const service: string = CloudProviderRegistry.getValue(provider, serviceKey);
     return isFunction(service) || (isString(service) && this.$injector.has(service));
   }
 
-  public getDelegate<T>(provider: string, serviceKey: string, skin?: string): T {
-    const service = CloudProviderRegistry.getValue(provider, serviceKey, skin);
+  public getDelegate<T>(provider: string, serviceKey: string): T {
+    const service = CloudProviderRegistry.getValue(provider, serviceKey);
     if (isString(service) && this.$injector.has(service)) {
       // service is a string, it's an AngularJS service
       return this.$injector.get<T>(service, 'providerDelegate');