From ad42f720c268fe2fa14519f73314f8e3b7126832 Mon Sep 17 00:00:00 2001
From: Rob Fletcher <robfletcher@users.noreply.github.com>
Date: Mon, 13 Apr 2020 10:21:36 -0700
Subject: [PATCH] fix(baking): don't rebake when we find a region-only diff
 (#1008)

---
 .../keel/bakery/artifact/ImageHandler.kt          | 15 ++++++++++-----
 .../keel/bakery/artifact/ImageHandlerTests.kt     | 14 ++++----------
 2 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/keel-bakery-plugin/src/main/kotlin/com/netflix/spinnaker/keel/bakery/artifact/ImageHandler.kt b/keel-bakery-plugin/src/main/kotlin/com/netflix/spinnaker/keel/bakery/artifact/ImageHandler.kt
index 1cfaeb5715..57d54aae98 100644
--- a/keel-bakery-plugin/src/main/kotlin/com/netflix/spinnaker/keel/bakery/artifact/ImageHandler.kt
+++ b/keel-bakery-plugin/src/main/kotlin/com/netflix/spinnaker/keel/bakery/artifact/ImageHandler.kt
@@ -3,6 +3,7 @@ package com.netflix.spinnaker.keel.bakery.artifact
 import com.netflix.frigga.ami.AppVersion
 import com.netflix.spinnaker.igor.ArtifactService
 import com.netflix.spinnaker.keel.actuation.ArtifactHandler
+import com.netflix.spinnaker.keel.api.ResourceDiff
 import com.netflix.spinnaker.keel.api.actuation.Task
 import com.netflix.spinnaker.keel.api.actuation.TaskLauncher
 import com.netflix.spinnaker.keel.api.artifacts.DebianArtifact
@@ -46,7 +47,9 @@ class ImageHandler(
         val current = artifact.findLatestAmi()
         val diff = DefaultResourceDiff(desired, current)
 
-        if (diff.hasChanges()) {
+        if (current != null && diff.isRegionsOnly()) {
+          publisher.publishEvent(ImageRegionMismatchDetected(current, artifact.vmOptions.regions))
+        } else if (diff.hasChanges()) {
           if (current == null) {
             log.info("No AMI found for {}", artifact.name)
           } else {
@@ -63,10 +66,6 @@ class ImageHandler(
         } else {
           log.debug("Existing image for {} is up-to-date", artifact.name)
         }
-
-        if (current != null && diff.affectedRootPropertyNames == setOf("regions")) {
-          publisher.publishEvent(ImageRegionMismatchDetected(current, artifact.vmOptions.regions))
-        }
       }
     }
   }
@@ -176,6 +175,12 @@ class ImageHandler(
       }
     } ?: defaultCredentials
 
+  /**
+   * @return `true` if the only changes in the diff are to the regions of an image.
+   */
+  private fun ResourceDiff<Image>.isRegionsOnly(): Boolean =
+    current != null && affectedRootPropertyNames.all { it == "regions" }
+
   private val log by lazy { LoggerFactory.getLogger(javaClass) }
 }
 
diff --git a/keel-bakery-plugin/src/test/kotlin/com/netflix/spinnaker/keel/bakery/artifact/ImageHandlerTests.kt b/keel-bakery-plugin/src/test/kotlin/com/netflix/spinnaker/keel/bakery/artifact/ImageHandlerTests.kt
index 7f57a2b8b2..cbb84543c9 100644
--- a/keel-bakery-plugin/src/test/kotlin/com/netflix/spinnaker/keel/bakery/artifact/ImageHandlerTests.kt
+++ b/keel-bakery-plugin/src/test/kotlin/com/netflix/spinnaker/keel/bakery/artifact/ImageHandlerTests.kt
@@ -363,16 +363,10 @@ internal class ImageHandlerTests : JUnit5Minutests {
                 runHandler(artifact)
               }
 
-              test("a bake is launched") {
-                expectThat(bakeTask)
-                  .isCaptured()
-                  .captured
-                  .hasSize(1)
-                  .first()
-                  .and {
-                    get("type").isEqualTo("bake")
-                    get("regions").isEqualTo(artifact.vmOptions.regions)
-                  }
+              test("no bake is launched") {
+                verify(exactly = 0) {
+                  taskLauncher.submitJob(any(), any(), any(), any(), any(), any(), any(), any<List<Map<String, Any?>>>())
+                }
               }
 
               test("an event is triggered because we want to track region mismatches") {