fix(pipelines): save pipelines task shouldn’t reuse pipeline id (#2951)

When save pipelines task is used multiple times in a stage, it now reuses the pipeline ID. This is problematic because if new pipelines are created they will reuse a previous pipeline ID and overwrite it rather than creating a new pipeline. I think the real problem here is the use of “pipeline.id” in the stage context for multiple purposes (both of which make sense independently).

1) SaveServiceAccountTask generates a new pipeline.id if one doesn’t exist for use with regenerateCronTriggerIds. It effectively changes the contract from a new pipeline not having an id, to it being generated beforehand.

2) Later in this class, after a pipeline is updated or created, its id is stored in “pipeline.id” to aid with refresh logic.

But the two of these together keeps the context “pipeline.id” from the last saved pipeline, and reuses it in the case that the pipeline.id is missing (when it should be created).
It may be that these two usages should have different names in the stage context. In that case this loop wouldn’t happen. Since SavePipelinesFromArtifactStage is the only stage where multiple pipelines are saved, and it does not currently use managed service accounts, it seems easier to just avoid the new regenerateCronTriggers logic when isSavingMultiplePipelines is true. Changing the model can have unintended consequences.

orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/tasks/SavePipelineTask.java

@@ -82,8 +82,12 @@ public TaskResult execute(Stage stage) {
     if (serviceAccount != null) {
       updateServiceAccount(pipeline, serviceAccount);
     }
-
-    if (stage.getContext().get("pipeline.id") != null && pipeline.get("id") == null) {
+    final Boolean isSavingMultiplePipelines =
+        (Boolean)
+            Optional.ofNullable(stage.getContext().get("isSavingMultiplePipelines")).orElse(false);
+    if (stage.getContext().get("pipeline.id") != null
+        && pipeline.get("id") == null
+        && !isSavingMultiplePipelines) {
       pipeline.put("id", stage.getContext().get("pipeline.id"));
 
       // We need to tell front50 to regenerate cron trigger id's
@@ -117,10 +121,6 @@ public TaskResult execute(Stage stage) {
     if (response.getStatus() == HttpStatus.OK.value()) {
       status = ExecutionStatus.SUCCEEDED;
     } else {
-      final Boolean isSavingMultiplePipelines =
-          (Boolean)
-              Optional.ofNullable(stage.getContext().get("isSavingMultiplePipelines"))
-                  .orElse(false);
       if (isSavingMultiplePipelines) {
         status = ExecutionStatus.FAILED_CONTINUE;
       } else {