fix(bake): Fail bake stage when image names don't match across regions (

#3219) * fix(bake): Fail bake stage when image names don't match across regions
spinnaker · Oct 9, 2019 · 6080990 · 6080990
1 parent a5bd525
commit 6080990
Show file tree

Hide file tree

Showing 2 changed files with 90 additions and 1 deletion.
diff --git a/orca-bakery/src/main/groovy/com/netflix/spinnaker/orca/bakery/pipeline/BakeStage.groovy b/orca-bakery/src/main/groovy/com/netflix/spinnaker/orca/bakery/pipeline/BakeStage.groovy
@@ -16,6 +16,9 @@
 
 package com.netflix.spinnaker.orca.bakery.pipeline
 
+import com.google.common.base.Joiner
+import com.netflix.spinnaker.kork.exceptions.ConstraintViolationException
+import com.netflix.spinnaker.kork.dynamicconfig.DynamicConfigService
 import com.netflix.spinnaker.orca.pipeline.graph.StageGraphBuilder
 
 import java.time.Clock
@@ -36,6 +39,9 @@ import groovy.transform.CompileStatic
 import groovy.util.logging.Slf4j
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.stereotype.Component
+
+import java.util.stream.Collectors
+
 import static com.netflix.spinnaker.orca.pipeline.model.SyntheticStageOwner.STAGE_BEFORE
 import static java.time.Clock.systemUTC
 import static java.time.ZoneOffset.UTC
@@ -108,6 +114,28 @@ class BakeStage implements StageDefinitionBuilder {
   @Component
   @CompileStatic
   static class CompleteParallelBakeTask implements Task {
+    public static final List<String> DEPLOYMENT_DETAILS_CONTEXT_FIELDS = [
+      "ami",
+      "imageId",
+      "imageName",
+      "amiSuffix",
+      "baseLabel",
+      "baseOs",
+      "refId",
+      "storeType",
+      "vmType",
+      "region",
+      "package",
+      "cloudProviderType",
+      "cloudProvider"
+    ]
+    DynamicConfigService dynamicConfigService
+
+    @Autowired
+    CompleteParallelBakeTask(DynamicConfigService dynamicConfigService) {
+      this.dynamicConfigService = dynamicConfigService
+    }
+
     TaskResult execute(Stage stage) {
       def bakeInitializationStages = stage.execution.stages.findAll {
         it.parentStageId == stage.parentStageId && it.status == ExecutionStatus.RUNNING
@@ -120,7 +148,7 @@ class BakeStage implements StageDefinitionBuilder {
       def globalContext = [
         deploymentDetails: relatedBakeStages.findAll{it.context.ami || it.context.imageId}.collect { Stage bakeStage ->
           def deploymentDetails = [:]
-          ["ami", "imageId", "amiSuffix", "baseLabel", "baseOs", "refId", "storeType", "vmType", "region", "package", "cloudProviderType", "cloudProvider"].each {
+          DEPLOYMENT_DETAILS_CONTEXT_FIELDS.each {
             if (bakeStage.context.containsKey(it)) {
               deploymentDetails.put(it, bakeStage.context.get(it))
             }
@@ -129,7 +157,30 @@ class BakeStage implements StageDefinitionBuilder {
           return deploymentDetails
         }
       ]
+
+      if (failOnImageNameMismatchEnabled()) {
+         List<String> distinctImageNames = globalContext.deploymentDetails.stream()
+          .map({details -> details['imageName']})
+          .distinct()
+          .collect(Collectors.toList())
+
+        if (distinctImageNames.size() > 1) {
+          throw new ConstraintViolationException(
+            "Image names found in different regions do not match: ${Joiner.on(", ").join(distinctImageNames)}. "
+            + "Re-run the bake to protect against deployment failures.")
+        }
+      }
+
       TaskResult.builder(ExecutionStatus.SUCCEEDED).outputs(globalContext).build()
     }
+
+    private boolean failOnImageNameMismatchEnabled() {
+      try {
+        return dynamicConfigService.isEnabled("stages.bake.failOnImageNameMismatch", false)
+      } catch (Exception e) {
+        log.error("Unable to retrieve config value for stages.bake.failOnImageNameMismatch. Assuming false.", e)
+        return false
+      }
+    }
   }
 }
diff --git a/orca-bakery/src/test/groovy/com/netflix/spinnaker/orca/bakery/pipeline/BakeStageSpec.groovy b/orca-bakery/src/test/groovy/com/netflix/spinnaker/orca/bakery/pipeline/BakeStageSpec.groovy
@@ -16,6 +16,8 @@
 
 package com.netflix.spinnaker.orca.bakery.pipeline
 
+import com.netflix.spinnaker.kork.dynamicconfig.DynamicConfigService
+import com.netflix.spinnaker.kork.exceptions.ConstraintViolationException
 
 import java.time.Clock
 import com.netflix.spinnaker.orca.ExecutionStatus
@@ -31,6 +33,8 @@ import static java.time.ZoneOffset.UTC
 import static java.time.temporal.ChronoUnit.*
 
 class BakeStageSpec extends Specification {
+  def dynamicConfigService = Mock(DynamicConfigService)
+
   @Unroll
   def "should build contexts corresponding to locally specified bake region and all target deploy regions"() {
     given:
@@ -116,6 +120,40 @@ class BakeStageSpec extends Specification {
     }
   }
 
+  def "should fail if image names don't match across regions (unless user opts out)"() {
+    given:
+    def pipeline = pipeline {
+      stage {
+        id = "1"
+        type = "bake"
+        context = [
+          "region": "us-east-1",
+          "regions": ["us-east-1", "us-west-2", "eu-east-1"],
+        ]
+        status = ExecutionStatus.RUNNING
+      }
+    }
+
+    def bakeStage = pipeline.stageById("1")
+    def graph = StageGraphBuilder.beforeStages(bakeStage)
+    new BakeStage(regionCollector: new RegionCollector()).beforeStages(bakeStage, graph)
+    def parallelStages = graph.build()
+
+    parallelStages.eachWithIndex { it, idx ->
+      it.context.ami = "${idx}"
+      it.context.imageName = "image#${idx}"
+    }
+    pipeline.stages.addAll(parallelStages)
+
+    dynamicConfigService.isEnabled("stages.bake.failOnImageNameMismatch", false) >> { true }
+
+    when:
+    new BakeStage.CompleteParallelBakeTask(dynamicConfigService).execute(bakeStage)
+
+    then:
+    thrown(ConstraintViolationException)
+  }
+
   private
   static List<Map> deployAz(String cloudProvider, String prefix, String... regions) {
     if (prefix == "clusters") {