From 59ba88e285feebc3c445ad71ae77b15448b771ef Mon Sep 17 00:00:00 2001
From: Maggie Neterval <mneterval@google.com>
Date: Wed, 15 Apr 2020 18:40:25 -0400
Subject: [PATCH] fix(artifacts): dedupe custom artifact types (#8179)

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
---
 .../artifact/expectedArtifact.service.spec.ts   | 17 +++++------------
 .../src/pipeline/config/PipelineRegistry.ts     |  8 +-------
 .../artifacts/custom/custom.artifact.ts         |  2 +-
 3 files changed, 7 insertions(+), 20 deletions(-)

diff --git a/app/scripts/modules/core/src/artifact/expectedArtifact.service.spec.ts b/app/scripts/modules/core/src/artifact/expectedArtifact.service.spec.ts
index 8947b359def..004bb3023f6 100644
--- a/app/scripts/modules/core/src/artifact/expectedArtifact.service.spec.ts
+++ b/app/scripts/modules/core/src/artifact/expectedArtifact.service.spec.ts
@@ -40,16 +40,9 @@ describe('ExpectedArtifactService', () => {
         isDefault: true,
       },
     ].map(k => ({ ...baseKindConfig, ...k }));
-    const customKindConfig = {
-      ...baseKindConfig,
-      key: 'custom',
-      customKind: true,
-      isMatch: true,
-      isDefault: true,
-    };
+
     beforeAll(() => {
       kindConfigs.forEach(kindConfig => Registry.pipeline.registerArtifactKind(kindConfig));
-      Registry.pipeline.registerCustomArtifactKind(customKindConfig);
     });
 
     it('infers kind from type', () => {
@@ -77,7 +70,7 @@ describe('ExpectedArtifactService', () => {
         type: 'bar-type',
       };
       const kindConfig = ExpectedArtifactService.getKindConfig(artifact, false);
-      expect(kindConfig).toEqual(customKindConfig);
+      expect(kindConfig).toEqual(Registry.pipeline.getCustomArtifactKind());
     });
 
     it('returns the custom kind when customKind is true, regardless of type, when isDefault is true', () => {
@@ -87,7 +80,7 @@ describe('ExpectedArtifactService', () => {
         type: 'bar-type',
       };
       const kindConfig = ExpectedArtifactService.getKindConfig(artifact, true);
-      expect(kindConfig).toEqual(customKindConfig);
+      expect(kindConfig).toEqual(Registry.pipeline.getCustomArtifactKind());
     });
 
     it('returns the default kind if neither kind nor type are stored on artifact', () => {
@@ -95,7 +88,7 @@ describe('ExpectedArtifactService', () => {
         id: 'artifact-id',
       };
       const kindConfig = ExpectedArtifactService.getKindConfig(artifact, false);
-      expect(kindConfig).toEqual(customKindConfig);
+      expect(kindConfig).toEqual(Registry.pipeline.getCustomArtifactKind());
     });
 
     it('returns the default kind if neither kind nor type are stored on artifact when isDefault is true', () => {
@@ -103,7 +96,7 @@ describe('ExpectedArtifactService', () => {
         id: 'artifact-id',
       };
       const kindConfig = ExpectedArtifactService.getKindConfig(artifact, true);
-      expect(kindConfig).toEqual(customKindConfig);
+      expect(kindConfig).toEqual(Registry.pipeline.getCustomArtifactKind());
     });
   });
 });
diff --git a/app/scripts/modules/core/src/pipeline/config/PipelineRegistry.ts b/app/scripts/modules/core/src/pipeline/config/PipelineRegistry.ts
index f0a091af268..08c3a436f9e 100644
--- a/app/scripts/modules/core/src/pipeline/config/PipelineRegistry.ts
+++ b/app/scripts/modules/core/src/pipeline/config/PipelineRegistry.ts
@@ -30,7 +30,6 @@ export class PipelineRegistry {
   private transformers: ITransformer[] = [];
   private notificationTypes: INotificationTypeConfig[] = [];
   private artifactKinds: IArtifactKindConfig[] = artifactKindConfigs;
-  private customArtifactKind: IArtifactKindConfig;
 
   constructor() {
     this.getStageConfig = memoize(this.getStageConfig.bind(this), (stage: IStage) =>
@@ -169,11 +168,6 @@ export class PipelineRegistry {
     defaults(originalArtifactKind, artifactKindConfig);
   }
 
-  public registerCustomArtifactKind(artifactKindConfig: IArtifactKindConfig): void {
-    this.customArtifactKind = artifactKindConfig;
-    this.registerArtifactKind(artifactKindConfig);
-  }
-
   public getExecutionTransformers(): ITransformer[] {
     return this.transformers;
   }
@@ -199,7 +193,7 @@ export class PipelineRegistry {
   }
 
   public getCustomArtifactKind(): IArtifactKindConfig {
-    return cloneDeep(this.customArtifactKind);
+    return cloneDeep(this.artifactKinds.find(k => k.key === 'custom'));
   }
 
   private getCloudProvidersForStage(
diff --git a/app/scripts/modules/core/src/pipeline/config/triggers/artifacts/custom/custom.artifact.ts b/app/scripts/modules/core/src/pipeline/config/triggers/artifacts/custom/custom.artifact.ts
index 7158d2bc26e..b20f2c71d46 100644
--- a/app/scripts/modules/core/src/pipeline/config/triggers/artifacts/custom/custom.artifact.ts
+++ b/app/scripts/modules/core/src/pipeline/config/triggers/artifacts/custom/custom.artifact.ts
@@ -16,7 +16,7 @@ function controllerFn(artifact: IArtifact) {
 export const CUSTOM_ARTIFACT = 'spinnaker.core.pipeline.trigger.custom.artifact';
 module(CUSTOM_ARTIFACT, [])
   .config(() => {
-    Registry.pipeline.registerCustomArtifactKind({
+    Registry.pipeline.mergeArtifactKind({
       label: 'Custom',
       description: 'A custom-defined artifact. (Deprecated)',
       key: 'custom',