fix(pipelines): Fix pipeline triggers for some templated pipelines #2963
Conversation
| @@ -87,7 +87,7 @@ class DependentPipelineStarter implements ApplicationContextAware { | |||
| strategy : suppliedParameters.strategy == true | |||
| ] | |||
|
|
|||
| if (pipelineConfig.parameterConfig || !suppliedParameters.empty) { | |||
| if (pipelineConfig.parameterConfig || !suppliedParameters.isEmpty()) { | |||
There was a problem hiding this comment.
This was just a bug, suppliedParameters.empty tried to look up the key empty in the map instead of calling the isEmpty() method.
jervi
changed the title
|
Work in progress, do not merge yet. |
Pipeline triggers and pipeline stages were broken for templated pipelines that use a dynamic source that references an artifact. Resolved by resolving the artifacts a bit earlier (before the preprocessors).
jervi
force-pushed
the
fix_pipeline_triggers_with_dynamic_source
branch
from
1bb02fb
to
424d08a
Compare
jervi
changed the title
|
Updated by adding a test and some minor adjustments. Should be ready for review now. |
| } | ||
|
|
||
| if (pipelineConfig.errors != null) { | ||
| throw new ValidationException("Pipeline template is invalid", pipelineConfig.errors as List<Map<String, Object>>) |
There was a problem hiding this comment.
Template parsing errors were silently being swallowed, so I added this to surface them to Deck in a proper way. Without this, the code will fail here with the error Cannot get property 'id' on null object.
Sorry, removed - I thought the author can always edit labels. |
| if (parentPipelineStageId != null) { | ||
| pipelineConfig.receivedArtifacts = artifactResolver?.getArtifacts(parentPipeline.stageById(parentPipelineStageId)) | ||
| } else { | ||
| pipelineConfig.receivedArtifacts = artifactResolver?.getAllArtifacts(parentPipeline) | ||
| } | ||
|
|
||
| pipelineConfig.trigger = trigger | ||
| pipelineConfig.expectedArtifacts = expectedArtifacts | ||
|
|
||
| def artifactError = null | ||
| try { | ||
| artifactResolver?.resolveArtifacts(pipelineConfig) |
There was a problem hiding this comment.
will this resolve correctly before the pipeline template is expanded?
There was a problem hiding this comment.
The artifacts should be found in the trigger, which is not a part of the pipeline template, so yes, I believe so.
There was a problem hiding this comment.
Sorry about closing and opening and resolving and unresolving. I’m on mobile and apparently pressing all the buttons 🙄
In our use case the pipeline definition is the artifact, so we have to resolve the artifacts before we can render the pipeline.
|
@spinnaker/reviewers Can we get this merged? |
|
@jervi : sorry about that, merging now |
…pinnaker#2963) Pipeline triggers and pipeline stages were broken for templated pipelines that use a dynamic source that references an artifact. Resolved by resolving the artifacts a bit earlier (before the preprocessors). (cherry picked from commit 2e07fcc)
Pipeline triggers and pipeline stages were broken for templated pipelines that use a dynamic source that references an artifact. Resolved by resolving the artifacts a bit earlier (before the preprocessors).