From afc8d310be32a1b71f24a7b7f5ad385c220203ed Mon Sep 17 00:00:00 2001 From: Lars Wander Date: Fri, 13 Oct 2017 19:24:40 -0400 Subject: [PATCH] feat(artifacts): Simplify expected artifacts --- .../modules/core/src/domain/IArtifact.ts | 11 ++ app/scripts/modules/core/src/domain/IBuild.ts | 4 +- .../core/src/domain/IExpectedArtifact.ts | 24 +--- app/scripts/modules/core/src/domain/index.ts | 2 + .../modules/core/src/help/help.contents.ts | 22 +-- .../src/pipeline/config/pipelineConfig.less | 2 +- .../triggers/artifacts/artifact.component.ts | 55 +------- .../config/triggers/artifacts/artifact.html | 128 ++++++------------ .../artifacts/expectedArtifact.component.ts | 36 +++++ .../triggers/artifacts/expectedArtifact.html | 43 ++++++ .../config/triggers/trigger.directive.js | 15 +- .../config/triggers/trigger.module.js | 2 + .../config/triggers/triggers.directive.js | 2 +- .../pipeline/config/triggers/triggers.html | 2 +- .../validation/pipelineConfig.validator.ts | 8 +- 15 files changed, 172 insertions(+), 184 deletions(-) create mode 100644 app/scripts/modules/core/src/domain/IArtifact.ts create mode 100644 app/scripts/modules/core/src/pipeline/config/triggers/artifacts/expectedArtifact.component.ts create mode 100644 app/scripts/modules/core/src/pipeline/config/triggers/artifacts/expectedArtifact.html diff --git a/app/scripts/modules/core/src/domain/IArtifact.ts b/app/scripts/modules/core/src/domain/IArtifact.ts new file mode 100644 index 00000000000..006f9c04c6e --- /dev/null +++ b/app/scripts/modules/core/src/domain/IArtifact.ts @@ -0,0 +1,11 @@ +export interface IArtifact { + type: string; + name: string; + version: string; + location: string; + reference: string; + metadata: any; + artifactAccount: string; + provenance: string; + uuid: string; +} diff --git a/app/scripts/modules/core/src/domain/IBuild.ts b/app/scripts/modules/core/src/domain/IBuild.ts index a15583e0d97..b2b827a4295 100644 --- a/app/scripts/modules/core/src/domain/IBuild.ts +++ b/app/scripts/modules/core/src/domain/IBuild.ts @@ -1,4 +1,4 @@ -export interface IArtifact { +export interface IBuildArtifact { displayPath: string; fileName: string; relativePath: string; @@ -12,5 +12,5 @@ export interface IBuild { result: string; timestamp: Date; url: string; - artifacts: IArtifact[]; + artifacts: IBuildArtifact[]; } diff --git a/app/scripts/modules/core/src/domain/IExpectedArtifact.ts b/app/scripts/modules/core/src/domain/IExpectedArtifact.ts index 80def0c5ed6..e1e5b10bb11 100644 --- a/app/scripts/modules/core/src/domain/IExpectedArtifact.ts +++ b/app/scripts/modules/core/src/domain/IExpectedArtifact.ts @@ -1,22 +1,8 @@ -export interface IExpectedArtifact { - fields: IArtifactField[]; -} - -export interface IArtifactField { - fieldName: string; - fieldType: FieldType; - value?: string; - missingPolicy?: MissingArtifactPolicy; - expression?: string; -} +import { IArtifact } from 'core/domain/IArtifact'; -export enum FieldType { - MustMatch = 'MUST_MATCH', - FindIfMissing = 'FIND_IF_MISSING' +export interface IExpectedArtifact { + matchArtifact: IArtifact; + usePriorArtifact: boolean; + defaultArtifact: IArtifact; } -export enum MissingArtifactPolicy { - FailPipeline = 'FAIL_PIPELINE', - Ignore = 'IGNORE', - PriorPipeline = 'PRIOR_PIPELINE' -} diff --git a/app/scripts/modules/core/src/domain/index.ts b/app/scripts/modules/core/src/domain/index.ts index e13120d61f8..08d2da79b47 100644 --- a/app/scripts/modules/core/src/domain/index.ts +++ b/app/scripts/modules/core/src/domain/index.ts @@ -1,5 +1,7 @@ export * from '../search/searchResult/model/IApplicationSearchResult'; +export * from './IArtifact' + export * from './IBuild'; export * from './IBuildDiffInfo'; export * from './IBuildInfo'; diff --git a/app/scripts/modules/core/src/help/help.contents.ts b/app/scripts/modules/core/src/help/help.contents.ts index 300e8d08cd0..ed499ae73ec 100644 --- a/app/scripts/modules/core/src/help/help.contents.ts +++ b/app/scripts/modules/core/src/help/help.contents.ts @@ -85,17 +85,17 @@ module(HELP_CONTENTS, []) the configuration based on the newest server group in the cluster.

If you want to start from scratch, select "None".

You can always edit the cluster configuration after you've created it.

`, - 'pipeline.config.expectedArtifact.fieldName': ` -

The Artifact field name.

`, - 'pipeline.config.expectedArtifact.fieldType': ` -

The Artifact field type. This must be one of 'MUST_MATCH' or 'FIND_IF_MISSING'.

-

MUST_MATCH means the incoming Artifact's field value must match exactly with the expected value.

-

FIND_IF_MISSING means the field is optional and a 'Missing Policy' will fire if the value is not present.

`, - 'pipeline.config.expectedArtifact.missingPolicy': ` -

The policy if the field type is 'FIND_IF_MISSING' and the field value is not present.

-

FAIL_PIPELINE fails the pipeline if the field value is missing.

-

IGNORE ignores the fact that the value is missing and runs the pipeline anyway.

-

PRIOR_PIPELINE looks up the field's value from a prior pipeline execution.

`, + 'pipeline.config.expectedArtifact.custom.matchArtifact': ` +

This specifies which fields in your incoming artifact to match against. Every field that you supply will be used to match against all incoming artifacts. If all fields specified match, the incoming artifact is bound to your pipeline context.

+

For example, if you want to match against any GCS object, only supply type=gcs/object. If you also want to restrict the matches by other fields, include those as well.

`, + 'pipeline.config.expectedArtifact.custom.ifMissing': ` +

If no artifact was supplied by your trigger to match against this expected artifact, you have a few options: +

+

`, 'loadBalancer.advancedSettings.healthTimeout': '

Configures the timeout, in seconds, for reaching the healthCheck target. Must be less than the interval.

Default: 5

', 'loadBalancer.advancedSettings.healthInterval': '

Configures the interval, in seconds, between ELB health checks. Must be greater than the timeout.

Default: 10

', 'loadBalancer.advancedSettings.healthyThreshold': '

Configures the number of healthy observations before reinstituting an instance into the ELB’s traffic rotation.

Default: 10

', diff --git a/app/scripts/modules/core/src/pipeline/config/pipelineConfig.less b/app/scripts/modules/core/src/pipeline/config/pipelineConfig.less index 4b078b060f6..60593fba6db 100644 --- a/app/scripts/modules/core/src/pipeline/config/pipelineConfig.less +++ b/app/scripts/modules/core/src/pipeline/config/pipelineConfig.less @@ -225,7 +225,7 @@ triggers { margin-top: 25px; margin-bottom: 10px; } - trigger, artifact { + trigger, expected-artifact { display: block; padding: 10px 20px; background-color: var(--color-white); diff --git a/app/scripts/modules/core/src/pipeline/config/triggers/artifacts/artifact.component.ts b/app/scripts/modules/core/src/pipeline/config/triggers/artifacts/artifact.component.ts index 685a17f5433..337c9af2f2e 100644 --- a/app/scripts/modules/core/src/pipeline/config/triggers/artifacts/artifact.component.ts +++ b/app/scripts/modules/core/src/pipeline/config/triggers/artifacts/artifact.component.ts @@ -1,64 +1,17 @@ import { IComponentController, IComponentOptions, module } from 'angular'; import { PIPELINE_CONFIG_PROVIDER } from 'core/pipeline/config/pipelineConfigProvider'; -import { FieldType, IArtifactField, IExpectedArtifact, MissingArtifactPolicy } from 'core/domain/IExpectedArtifact'; -import { IPipeline } from 'core/domain/IPipeline'; +import { IArtifact } from 'core/domain/IArtifact'; class ArtifactController implements IComponentController { - public artifact: IExpectedArtifact; - public pipeline: IPipeline; - public missingPolicies: string[] = Object.keys(MissingArtifactPolicy).map(key => MissingArtifactPolicy[key as any]); - public fieldNames: string[] = ['type', 'name', 'version', 'location', 'reference', 'artifactAccount', 'provenance', 'uuid']; - public fieldTypes: string[] = Object.keys(FieldType).map(key => FieldType[key as any]); - - public removeArtifact(): void { - const artifactIndex = this.pipeline.expectedArtifacts.indexOf(this.artifact); - this.pipeline.expectedArtifacts.splice(artifactIndex, 1); - - this.pipeline.triggers - .forEach(t => t.expectedArtifacts = t.expectedArtifacts.filter(a => !this.equals(a, this.artifact))); - } - - public addField(): void { - const newField: IArtifactField = { - fieldName: '', - fieldType: FieldType.MustMatch, - value: '', - missingPolicy: MissingArtifactPolicy.FailPipeline - }; - - if (!this.artifact.fields) { - this.artifact.fields = []; - } - this.artifact.fields.push(newField); - } - - public removeField(fieldIndex: number): void { - this.artifact.fields.splice(fieldIndex, 1); - } - - private equals(first: IExpectedArtifact, other: IExpectedArtifact): boolean { - const fieldIsEqual = (firstField: IArtifactField, otherField: IArtifactField): boolean => { - return firstField.fieldName === otherField.fieldName - && firstField.fieldType === otherField.fieldType - && firstField.value === otherField.value - && firstField.missingPolicy === otherField.missingPolicy - && firstField.expression === otherField.expression; - }; - if (first.fields.length !== other.fields.length) { - return false; - } - - return first.fields.every(firstField => { - return other.fields.some(otherField => fieldIsEqual(firstField, otherField)); - }); - } + public artifact: IArtifact; } class ArtifactComponent implements IComponentOptions { - public bindings = { artifact: '<', pipeline: '<' }; + public bindings = { artifact: '=' }; public templateUrl = require('./artifact.html'); public controller = ArtifactController; + public controllerAs = 'ctrl'; } export const ARTIFACT = 'spinnaker.core.pipeline.trigger.artifact'; diff --git a/app/scripts/modules/core/src/pipeline/config/triggers/artifacts/artifact.html b/app/scripts/modules/core/src/pipeline/config/triggers/artifacts/artifact.html index 838348efd91..d058393b6c2 100644 --- a/app/scripts/modules/core/src/pipeline/config/triggers/artifacts/artifact.html +++ b/app/scripts/modules/core/src/pipeline/config/triggers/artifacts/artifact.html @@ -1,87 +1,47 @@ -
-
-
-
-
-
-
- -
- -
- -
- - -
-
-
-
-
- -
- -
-
- -
- -
-
-
-
-
-
-
-
-

- You don't have any fields declared for this artifact. -

-
-
-
- -
-
-
- -
+
+
+ +
+ +
+ +
+
+
+ +
+ +
+ +
+ +
+
+
+ +
+ +
diff --git a/app/scripts/modules/core/src/pipeline/config/triggers/artifacts/expectedArtifact.component.ts b/app/scripts/modules/core/src/pipeline/config/triggers/artifacts/expectedArtifact.component.ts new file mode 100644 index 00000000000..90c7d2c7fdf --- /dev/null +++ b/app/scripts/modules/core/src/pipeline/config/triggers/artifacts/expectedArtifact.component.ts @@ -0,0 +1,36 @@ +import { equals, IComponentController, IComponentOptions, module } from 'angular'; + +import { PIPELINE_CONFIG_PROVIDER } from 'core/pipeline/config/pipelineConfigProvider'; +import { IExpectedArtifact } from 'core/domain/IExpectedArtifact'; +import { IPipeline } from 'core/domain/IPipeline'; + +class ExpectedArtifactController implements IComponentController { + public expectedArtifact: IExpectedArtifact; + public pipeline: IPipeline; + + private static expectedArtifactEquals(first: IExpectedArtifact, other: IExpectedArtifact): boolean { + // Interesting to point out that if two artifact's match artifacts equal, they match the same artifacts & are effectively equal + return equals(first.matchArtifact, other.matchArtifact); + } + + public removeExpectedArtifact(): void { + this.pipeline.expectedArtifacts = this.pipeline.expectedArtifacts + .filter(a => !ExpectedArtifactController.expectedArtifactEquals(a, this.expectedArtifact)); + + this.pipeline.triggers + .forEach(t => t.expectedArtifacts = t.expectedArtifacts + .filter(a => !ExpectedArtifactController.expectedArtifactEquals(a, this.expectedArtifact))); + } +} + +class ExpectedArtifactComponent implements IComponentOptions { + public bindings = { expectedArtifact: '=', pipeline: '=' }; + public templateUrl = require('./expectedArtifact.html'); + public controller = ExpectedArtifactController; + public controllerAs = 'ctrl'; +} + +export const EXPECTED_ARTIFACT = 'spinnaker.core.pipeline.trigger.expected.artifact'; +module(EXPECTED_ARTIFACT, [ + PIPELINE_CONFIG_PROVIDER, +]).component('expectedArtifact', new ExpectedArtifactComponent()); diff --git a/app/scripts/modules/core/src/pipeline/config/triggers/artifacts/expectedArtifact.html b/app/scripts/modules/core/src/pipeline/config/triggers/artifacts/expectedArtifact.html new file mode 100644 index 00000000000..a1704020210 --- /dev/null +++ b/app/scripts/modules/core/src/pipeline/config/triggers/artifacts/expectedArtifact.html @@ -0,0 +1,43 @@ +
+
+
+
+
+ Match against + +
+
+ +
+
+ + If missing + +
+ + +
+
+ + +
+
+
+
+ Default artifact +
+
+
+ +
+
+
+
+
diff --git a/app/scripts/modules/core/src/pipeline/config/triggers/trigger.directive.js b/app/scripts/modules/core/src/pipeline/config/triggers/trigger.directive.js index dbc8d05cae1..6d105a9c5c4 100644 --- a/app/scripts/modules/core/src/pipeline/config/triggers/trigger.directive.js +++ b/app/scripts/modules/core/src/pipeline/config/triggers/trigger.directive.js @@ -1,8 +1,9 @@ 'use strict'; +import { copy } from 'angular'; + import { SETTINGS } from 'core/config/settings'; import { PIPELINE_CONFIG_PROVIDER } from 'core/pipeline/config/pipelineConfigProvider'; -import { FieldType } from 'core/domain'; const angular = require('angular'); @@ -36,14 +37,12 @@ module.exports = angular.module('spinnaker.core.pipeline.config.trigger.triggerD $scope.pipeline.triggers.splice(triggerIndex, 1); }; - this.artifactFilter = function(artifact) { - // Ensure suggested artifacts have a name and type - return artifact.type && artifact.name; - }; - this.summarizeExpectedArtifact = function(expected) { - const fieldSummaries = expected.fields.filter(field => field.fieldType === FieldType.MustMatch).map(field => field.fieldName + ':' + field.value); - return fieldSummaries.join(', '); + const artifact = copy(expected.matchArtifact); + return Object.keys(artifact) + .filter((k) => artifact[k]) + .map((k) => (`${k}: ${artifact[k]}`)) + .join(', '); }; this.loadTrigger = () => { diff --git a/app/scripts/modules/core/src/pipeline/config/triggers/trigger.module.js b/app/scripts/modules/core/src/pipeline/config/triggers/trigger.module.js index a26d40935e1..2da6b1e2993 100644 --- a/app/scripts/modules/core/src/pipeline/config/triggers/trigger.module.js +++ b/app/scripts/modules/core/src/pipeline/config/triggers/trigger.module.js @@ -6,6 +6,7 @@ import { RUN_AS_USER_SELECTOR_COMPONENT } from './runAsUserSelector.component'; import { TRAVIS_TRIGGER } from './travis/travisTrigger.module'; import { GIT_TRIGGER } from './git/git.trigger'; import { PUBSUB_TRIGGER } from './pubsub/pubsub.trigger'; +import { EXPECTED_ARTIFACT } from './artifacts/expectedArtifact.component'; import { ARTIFACT } from './artifacts/artifact.component'; module.exports = angular.module('spinnaker.core.pipeline.config.trigger', [ @@ -17,6 +18,7 @@ module.exports = angular.module('spinnaker.core.pipeline.config.trigger', [ require('./pipeline/pipelineTrigger.module.js').name, PUBSUB_TRIGGER, ARTIFACT, + EXPECTED_ARTIFACT, require('./trigger.directive.js').name, require('./triggers.directive.js').name, RUN_AS_USER_SELECTOR_COMPONENT, diff --git a/app/scripts/modules/core/src/pipeline/config/triggers/triggers.directive.js b/app/scripts/modules/core/src/pipeline/config/triggers/triggers.directive.js index 07b237399a3..6cf4e848350 100644 --- a/app/scripts/modules/core/src/pipeline/config/triggers/triggers.directive.js +++ b/app/scripts/modules/core/src/pipeline/config/triggers/triggers.directive.js @@ -34,7 +34,7 @@ module.exports = angular.module('spinnaker.core.pipeline.config.trigger.triggers }; this.addArtifact = () => { - const newArtifact = {fields: []}; + const newArtifact = {matchArtifact: {}, usePriorExecution: false, useDefaultArtifact: false, defaultArtifact: {}}; if (!$scope.pipeline.expectedArtifacts) { $scope.pipeline.expectedArtifacts = []; diff --git a/app/scripts/modules/core/src/pipeline/config/triggers/triggers.html b/app/scripts/modules/core/src/pipeline/config/triggers/triggers.html index fa9424d1d42..4b188b4ae2f 100644 --- a/app/scripts/modules/core/src/pipeline/config/triggers/triggers.html +++ b/app/scripts/modules/core/src/pipeline/config/triggers/triggers.html @@ -44,7 +44,7 @@

- +

diff --git a/app/scripts/modules/core/src/pipeline/config/validation/pipelineConfig.validator.ts b/app/scripts/modules/core/src/pipeline/config/validation/pipelineConfig.validator.ts index 2e3561ed698..1178af6fd71 100644 --- a/app/scripts/modules/core/src/pipeline/config/validation/pipelineConfig.validator.ts +++ b/app/scripts/modules/core/src/pipeline/config/validation/pipelineConfig.validator.ts @@ -10,7 +10,6 @@ import { ITriggerTypeConfig } from 'core/domain'; import { PIPELINE_CONFIG_PROVIDER, PipelineConfigProvider } from 'core/pipeline/config/pipelineConfigProvider'; -import { FieldType } from '@spinnaker/core'; export interface IStageValidationResults { stage: IStage; @@ -149,11 +148,8 @@ export class PipelineConfigValidator implements IServiceProvider { if (pipeline.strategy && !(pipeline.stages.some(stage => stage.type === 'deploy'))) { messages.push('To be able to create new server groups, a custom strategy should contain a Deploy stage.'); } - if ((pipeline.expectedArtifacts || []).some(a => a.fields.some(f => !f.fieldName || !f.fieldType || !f.value))) { - messages.push('Name, Type, Value are required attributes for artifact fields.'); - } - if ((pipeline.expectedArtifacts || []).some(a => a.fields.some(f => f.fieldType === FieldType.FindIfMissing && !f.missingPolicy))) { - messages.push('If Field Type is set to "FIND_IF_MISSING", a Missing Policy must be set.'); + if ((pipeline.expectedArtifacts || []).some(a => !a.matchArtifact || a.matchArtifact === {})) { + messages.push('Every expected artifact must specify an artifact to match against.'); } return messages; }