chore(code): Upstream changes from Netflix (spinnaker#1951)

* feat(notifications): add compact notifications

Squashed commit of the following:

commit 046f4cb3637e80e64c21c61118e853172d733024
Author: Emily Burns <[email protected]>
Date:   Wed Jun 16 12:31:34 2021 -0700

    fix(mj): make mj not have a header

commit c2d68ed679680e55b9b021c4e76d7b41b3bc6f1e
Author: Emily Burns <[email protected]>
Date:   Wed Jun 16 11:21:24 2021 -0700

    fix(notifications): even smaller, no headers, mj

commit 237ca444226e3b4ffb801f65e8c13f72a4a8e467
Author: Emily Burns <[email protected]>
Date:   Tue Jun 15 20:40:14 2021 -0700

    feat(notifications): add compact notifications

(cherry picked from commit 9b39e64db7316044b6e8e2972dd9d5e0862891c4)

* fix(dgs): make task props non-nullable

Squashed commit of the following:

commit e3fb199e7e8b105913cb6a56bdca90eb8fd3b07b
Author: Rani <[email protected]>
Date:   Tue Jun 15 14:08:29 2021 -0700

    fix(dgs): make tasks non-nullable

(cherry picked from commit 2d17a45e5c5e3b410dcc2d63636fc9bdc59e04c2)

* feat(notifications): Add MdNotification to the GraphQL API + delivery config import failed notification

Squashed commit of the following:

commit 483cc50de686478eeacae7fa0cdfa8ba01a0fece
Author: Luis Pollo <[email protected]>
Date:   Thu Jun 17 10:48:31 2021 -0700

    fix(pr): Add missing indices, fix test

commit b21d781b3cc304fccf7a48a5192918c56b6af4f1
Author: Luis Pollo <[email protected]>
Date:   Thu Jun 17 09:10:42 2021 -0700

    fix(notifications): Fixed GraphQL model

commit 4da9436f44f76889fbdd984726b47c166f98aa5c
Author: Luis Pollo <[email protected]>
Date:   Thu Jun 17 08:52:09 2021 -0700

    feat(notifications): Add dismissedAt timestamp + renames

commit 184f2eec351190d7e65017d5577ff326edc5d801
Author: Luis Pollo <[email protected]>
Date:   Thu Jun 17 08:32:19 2021 -0700

    chore(log): Add logging around dismissible notifications

commit 8507f476b4d3c59a609d871ae19199d20a67c778
Author: Luis Pollo <[email protected]>
Date:   Thu Jun 17 08:27:59 2021 -0700

    fix(pr): Make SqlDismissibleNotificationRepository a @bean

commit 2dd5a946ee09cbd38d045bd00d8c43b8d0b8ec6f
Author: Luis Pollo <[email protected]>
Date:   Thu Jun 17 08:04:39 2021 -0700

    chore(pr): More minor tweaks

commit b16a95c6c318b1b0064a5d9c791aa72a209d42f7
Author: Luis Pollo <[email protected]>
Date:   Thu Jun 17 07:50:14 2021 -0700

    chore(pr): Minor tweaks

commit ebc430c5524fe7e676ed659dafeeac0d128eed07
Author: Luis Pollo <[email protected]>
Date:   Thu Jun 17 07:39:25 2021 -0700

    fix(pr): Revert unrelated changes

commit 0d0c35594f5bb0cd0b99b18a492e4d3dbb7f8d5c
Author: Luis Pollo <[email protected]>
Date:   Wed Jun 16 22:13:07 2021 -0700

    feat(scm): Generate dismissable notification for delivery config import failure

commit 5340fcc79e0cf6ad0c57c6980717554f0c281a94
Author: Luis Pollo <[email protected]>
Date:   Wed Jun 16 22:12:42 2021 -0700

    feat(notifications): Add GraphQL support for dismissable notifications

commit 9a23bde5e499da3d658e98c34d298f9b22c38387
Author: Luis Pollo <[email protected]>
Date:   Wed Jun 16 21:04:34 2021 -0700

    feat(notifications): Introduce dismissable notifications

(cherry picked from commit 0dfb4ab20f061f4041e96fc5cbf4ac284b824003)

* fix(capacity): pass desired capacity to Orca but don't include in diff

We need to always pass desired capacity when cloning a server group, but not diff on it if there are scaling policies in play.

(cherry picked from commit 3f589263b46f536b0b0d187f517f52dc64b5eec7)

* feat(verifications): Pass resource endpoints to verification test containers

Squashed commit of the following:

commit 9b49dbd4629a50db3f4653e39e68e5747fcf3be4
Author: Luis Pollo <[email protected]>
Date:   Fri Jun 18 17:25:48 2021 -0700

    fix(test): Fix broken test

commit fcb29856c67b333c4d2f6cc5d7e88c2939cec46c
Author: Luis Pollo <[email protected]>
Date:   Fri Jun 18 17:11:27 2021 -0700

    feat(verifications): Rename + test container endpoint vars

commit 3f38c0fa91967bfae2a254083f765247c7b3128b
Author: Luis Pollo <[email protected]>
Date:   Fri Jun 18 09:39:55 2021 -0700

    feat(verifications): Add resource endpoints to verification test containers

(cherry picked from commit 63ad3de9e60015f70c67975c1e5b637601ade958)

* debug(caches): Add more debug logs to application cache

(cherry picked from commit 832fc83064ac7dbeaa104837d488480c7b2c6df5)

* fix(certs): segment certificate cache by account

(cherry picked from commit 8db40855f0a36c6048b907b0b1da2f80fc8e3ea2)

* fix(network): Fix load balancer endpoint discovery + friendly resource names

Squashed commit of the following:

commit e9d869220e4ad2b5b82c08e1fa438a39406f0e0d
Author: Luis Pollo <[email protected]>
Date:   Mon Jun 21 16:00:50 2021 -0700

    fix(tests): Fix tests

commit 876271571141db6550d655649facb22cc0d4b00b
Author: Luis Pollo <[email protected]>
Date:   Sat Jun 19 08:56:57 2021 -0700

    fix(network): Fix load balancer endpoint discovery + friendly resource names

(cherry picked from commit 1a15a7f17518d2f2590d9f68edc7eca3270a3613)

* fix(dgs): minor fixes in notifications query

Squashed commit of the following:

commit c786e3f7471deb104b136c90ef7278eb9ee1652f
Author: Rani <[email protected]>
Date:   Tue Jun 22 07:19:15 2021 +0300

    fix(dgs): convert level to enum

commit 2f8c17aed02b74609e59af6bede77dee75e4a277
Author: Rani <[email protected]>
Date:   Tue Jun 22 07:15:53 2021 +0300

    fix(dgs): make notification objects non-nullable (but keep the array nullable)

(cherry picked from commit de9fc62364d5b79494c65d4c32d5d7a75e15d876)

Co-authored-by: Emily Burns <[email protected]>
Co-authored-by: Rob Fletcher <[email protected]>
Co-authored-by: Rani Horev <[email protected]>

Author: 4 people

keel-api/src/main/kotlin/com/netflix/spinnaker/keel/api/NotificationConfig.kt

@@ -17,11 +17,14 @@
  */
 package com.netflix.spinnaker.keel.api
 
+import com.netflix.spinnaker.keel.api.NotificationDisplay.NORMAL
+
 // todo eb: this does not allow you to customize the notification message, but we can add that later.
 data class NotificationConfig(
   val type: NotificationType,
   val address: String, // either slack channel or email address
-  val frequency: NotificationFrequency
+  val frequency: NotificationFrequency,
+  val display: NotificationDisplay = NORMAL
 )
 
 enum class NotificationFrequency {
@@ -33,3 +36,7 @@ enum class NotificationFrequency {
 enum class NotificationType {
   slack, email
 }
+
+enum class NotificationDisplay {
+  COMPACT, NORMAL
+}

keel-api/src/main/kotlin/com/netflix/spinnaker/keel/api/ResourceKind.kt

@@ -11,6 +11,9 @@ data class ResourceKind(
     }
   }
 
+  val friendlyName: String
+    get() = "${group.toTitleCase()} ${kind.toTitleCase()}"
+
   override fun toString(): String {
     return "$group/$kind@v$version"
   }
@@ -26,4 +29,7 @@ data class ResourceKind(
         ?.let { (group, kind, version) -> ResourceKind(group, kind, version) }
         ?: error("$value is not a valid resource kind")
   }
+
+  private fun String.toTitleCase() =
+    replace('-', ' ').capitalize()
 }

keel-clouddriver/src/main/kotlin/com/netflix/spinnaker/keel/clouddriver/CloudDriverCache.kt

@@ -33,7 +33,7 @@ interface CloudDriverCache {
   fun credentialBy(name: String): Credential
   fun defaultKeyPairForAccount(account: String) =
     credentialBy(account).attributes["defaultKeyPair"] as String
-  fun certificateByName(name: String): Certificate
+  fun certificateByAccountAndName(account: String, name: String): Certificate
   fun certificateByArn(arn: String): Certificate
 }
 

keel-clouddriver/src/main/kotlin/com/netflix/spinnaker/keel/clouddriver/CloudDriverService.kt

@@ -91,6 +91,14 @@ interface CloudDriverService {
     @Path("application") application: String
   ): List<AmazonLoadBalancer>
 
+  @GET("/aws/loadBalancers/{account}/{region}/{name}")
+  suspend fun getAmazonLoadBalancer(
+    @Header("X-SPINNAKER-USER") user: String,
+    @Path("account") account: String,
+    @Path("region") region: String,
+    @Path("name") name: String
+  ): List<AmazonLoadBalancer>
+
   @GET("/{provider}/loadBalancers/{account}/{region}/{name}")
   suspend fun getClassicLoadBalancer(
     @Header("X-SPINNAKER-USER") user: String,

keel-clouddriver/src/main/kotlin/com/netflix/spinnaker/keel/clouddriver/MemoryCloudDriverCache.kt

@@ -154,13 +154,20 @@ class MemoryCloudDriverCache(
         .getOrElse { ex -> throw CacheLoadingException("Error loading subnetsByPurpose cache", ex) }
     }
 
-  private val certificatesByName: AsyncLoadingCache<String, Certificate> =
+  private data class CertificateKey(val account: String, val name: String) {
+    constructor(certificate: Certificate) : this(
+      certificate.accountName,
+      certificate.serverCertificateName
+    )
+  }
+
+  private val certificatesByAccountAndName: AsyncLoadingCache<CertificateKey, Certificate> =
     cacheFactory
-      .asyncBulkLoadingCache("certificatesByName") {
+      .asyncBulkLoadingCache("certificatesByAccountAndName") {
         runCatching {
           cloudDriver
             .getCertificates()
-            .associateBy { it.serverCertificateName }
+            .associateBy { CertificateKey(it) }
         }
           .getOrElse { ex -> throw CacheLoadingException("Error loading certificatesByName cache", ex) }
       }
@@ -216,9 +223,9 @@ class MemoryCloudDriverCache(
       subnetsByPurpose.get(Triple(account, region, purpose)).await() ?: notFound("Subnet with purpose \"$purpose\" not found in $account:$region")
     }
 
-  override fun certificateByName(name: String): Certificate =
+  override fun certificateByAccountAndName(account: String, name: String): Certificate =
     runBlocking {
-      certificatesByName.get(name).await() ?: notFound("Certificate with name $name not found")
+      certificatesByAccountAndName.get(CertificateKey(account, name)).await() ?: notFound("Certificate with name $name not found")
     }
 
   override fun certificateByArn(arn: String): Certificate =

keel-clouddriver/src/main/kotlin/com/netflix/spinnaker/keel/clouddriver/model/AmazonLoadBalancer.kt

@@ -1,5 +1,6 @@
 package com.netflix.spinnaker.keel.clouddriver.model
 
+import com.fasterxml.jackson.annotation.JsonAlias
 import com.fasterxml.jackson.annotation.JsonSubTypes
 import com.fasterxml.jackson.annotation.JsonTypeInfo
 import com.fasterxml.jackson.annotation.JsonTypeInfo.As
@@ -25,4 +26,6 @@ interface AmazonLoadBalancer {
   val vpcId: String
   val subnets: Set<String>
   val scheme: String?
+  @get:JsonAlias("dnsname")
+  val dnsName: String
 }

keel-clouddriver/src/main/kotlin/com/netflix/spinnaker/keel/clouddriver/model/ApplicationLoadBalancerModel.kt

@@ -13,6 +13,7 @@ data class ApplicationLoadBalancerModel(
   override val vpcId: String,
   override val subnets: Set<String>,
   override val scheme: String?,
+  override val dnsName: String,
   val idleTimeout: Int,
   val securityGroups: Set<String>,
   val listeners: List<ApplicationLoadBalancerListener>,

keel-clouddriver/src/main/kotlin/com/netflix/spinnaker/keel/clouddriver/model/Capacity.kt

@@ -3,5 +3,5 @@ package com.netflix.spinnaker.keel.clouddriver.model
 data class Capacity(
   val min: Int,
   val max: Int,
-  val desired: Int?
+  val desired: Int
 )

keel-clouddriver/src/main/kotlin/com/netflix/spinnaker/keel/clouddriver/model/Certificate.kt

@@ -1,3 +1,3 @@
 package com.netflix.spinnaker.keel.clouddriver.model
 
-data class Certificate(val serverCertificateName: String, val arn: String)
+data class Certificate(val serverCertificateName: String, val accountName: String, val arn: String)

keel-clouddriver/src/main/kotlin/com/netflix/spinnaker/keel/clouddriver/model/ClassicLoadBalancerModel.kt

@@ -30,6 +30,7 @@ data class ClassicLoadBalancerModel(
 
   override val subnets: Set<String>,
   override val scheme: String?,
+  override val dnsName: String,
   val idleTimeout: Int,
   val securityGroups: Set<String>,
   val listenerDescriptions: List<ClassicLoadBalancerListenerDescription>,

keel-clouddriver/src/main/kotlin/com/netflix/spinnaker/keel/clouddriver/model/NetworkLoadBalancerModel.kt

@@ -9,6 +9,7 @@ data class NetworkLoadBalancerModel(
   override val vpcId: String,
   override val subnets: Set<String>,
   override val scheme: String?,
+  override val dnsName: String,
   override val availabilityZones: Set<String>,
   val targetGroups: List<TargetGroup>
 ) : AmazonLoadBalancer {

keel-clouddriver/src/test/kotlin/com/netflix/spinnaker/keel/clouddriver/MemoryCloudDriverCacheTest.kt

@@ -57,8 +57,10 @@ internal class MemoryCloudDriverCacheTest {
   )
 
   val certificates = listOf(
-    Certificate("cert-1", "arn:cert-1"),
-    Certificate("cert-2", "arn:cert-2")
+    Certificate("cert-1", "prod", "arn:prod:cert-1"),
+    Certificate("cert-2", "prod", "arn:prod:cert-2"),
+    Certificate("cert-1", "test", "arn:test:cert-1"),
+    Certificate("cert-2", "test", "arn:test:cert-2")
   )
 
   @Test
@@ -224,26 +226,44 @@ internal class MemoryCloudDriverCacheTest {
 
   @ParameterizedTest
   @ValueSource(strings = ["cert-1", "cert-2"])
-  fun `certificates are looked up from CloudDriver when requested by name`(name: String) {
+  fun `certificates are looked up from CloudDriver when requested by account and name`(name: String) {
     every { cloudDriver.getCertificates() } returns certificates
 
-    expectThat(subject.certificateByName(name))
+    expectThat(subject.certificateByAccountAndName("test", name))
       .get { serverCertificateName } isEqualTo name
   }
 
+  @ParameterizedTest
+  @ValueSource(strings = ["prod", "test"])
+  fun `certificates are correctly distinguished by account`(account: String) {
+    every { cloudDriver.getCertificates() } returns certificates
+
+    expectThat(subject.certificateByAccountAndName(account, "cert-1")) {
+      get { serverCertificateName } isEqualTo "cert-1"
+      get { account } isEqualTo account
+    }
+  }
+
   @Test
   fun `an unknown certificate name throws an exception`() {
     every { cloudDriver.getCertificates() } returns certificates
 
-    expectThrows<ResourceNotFound> { subject.certificateByName("does-not-exist") }
+    expectThrows<ResourceNotFound> { subject.certificateByAccountAndName("test", "does-not-exist") }
+  }
+
+  @Test
+  fun `an unknown account for a known certificate name throws an exception`() {
+    every { cloudDriver.getCertificates() } returns certificates
+
+    expectThrows<ResourceNotFound> { subject.certificateByAccountAndName("fnord", "cert-1") }
   }
 
   @ParameterizedTest
   @ValueSource(strings = ["cert-1", "cert-2"])
   fun `subsequent requests for a certificate by name hit the cache`(name: String) {
     every { cloudDriver.getCertificates() } returns certificates
 
-    repeat(4) { subject.certificateByName(name) }
+    repeat(4) { subject.certificateByAccountAndName("test", name) }
 
     verify(exactly = 1) { cloudDriver.getCertificates() }
   }
@@ -256,7 +276,7 @@ internal class MemoryCloudDriverCacheTest {
 
     listOf("cert-1", "cert-2")
       .forEach {
-        subject.certificateByName(it)
+        subject.certificateByAccountAndName("test", it)
         // this test can be vulnerable to an occasional race condition where the 2nd read misses the
         // cache (seemingly due to the way the bulk load works). This seems to be sufficient to
         // prevent it
@@ -267,7 +287,7 @@ internal class MemoryCloudDriverCacheTest {
   }
 
   @ParameterizedTest
-  @ValueSource(strings = ["arn:cert-1", "arn:cert-2"])
+  @ValueSource(strings = ["arn:prod:cert-1", "arn:prod:cert-2", "arn:test:cert-1", "arn:test:cert-2"])
   fun `certificates are looked up from CloudDriver when requested by ARN`(arn: String) {
     every { cloudDriver.getCertificates() } returns certificates
 
@@ -283,7 +303,7 @@ internal class MemoryCloudDriverCacheTest {
   }
 
   @ParameterizedTest
-  @ValueSource(strings = ["arn:cert-1", "arn:cert-2"])
+  @ValueSource(strings = ["arn:prod:cert-1", "arn:prod:cert-2", "arn:test:cert-1", "arn:test:cert-2"])
   fun `subsequent requests for a certificate by ARN hit the cache`(arn: String) {
     every { cloudDriver.getCertificates() } returns certificates
 
@@ -298,7 +318,7 @@ internal class MemoryCloudDriverCacheTest {
   fun `all certs are cached at once when requested by ARN`() {
     every { cloudDriver.getCertificates() } returns certificates
 
-    listOf("arn:cert-1", "arn:cert-2")
+    listOf("arn:prod:cert-1", "arn:prod:cert-2")
       .forEach {
         subject.certificateByArn(it)
         // this test can be vulnerable to an occasional race condition where the 2nd read misses the

keel-core/src/main/kotlin/com/netflix/spinnaker/keel/constraints/ManualJudgementConstraintAttributes.kt

@@ -1,5 +1,7 @@
 package com.netflix.spinnaker.keel.constraints
 
+import com.netflix.spinnaker.keel.api.NotificationDisplay
+import com.netflix.spinnaker.keel.api.NotificationDisplay.NORMAL
 import com.netflix.spinnaker.keel.api.artifacts.DeliveryArtifact
 import com.netflix.spinnaker.keel.api.artifacts.PublishedArtifact
 import com.netflix.spinnaker.keel.api.constraints.ConstraintStateAttributes
@@ -15,5 +17,7 @@ data class OriginalSlackMessageDetail(
   val currentArtifact: PublishedArtifact? = null,
   val pinnedArtifact: PublishedArtifact? = null,
   val deliveryArtifact: DeliveryArtifact,
-  val targetEnvironment: String
+  val targetEnvironment: String,
+  val author: String? = null,
+  val display: NotificationDisplay? = NORMAL
 )

keel-core/src/main/kotlin/com/netflix/spinnaker/keel/notifications/DeliveryConfigImportFailed.kt

@@ -0,0 +1,32 @@
+package com.netflix.spinnaker.keel.notifications
+
+import com.netflix.spinnaker.keel.api.UID
+import com.netflix.spinnaker.keel.events.EventLevel
+import com.netflix.spinnaker.keel.events.EventLevel.ERROR
+import java.time.Instant
+
+/**
+ * A [DismissibleNotification] that indicates a delivery config failed to import.
+ */
+data class DeliveryConfigImportFailed(
+  override val triggeredAt: Instant,
+  override val application: String,
+  val repoType: String,
+  val projectKey: String,
+  val repoSlug: String,
+  val commitHash: String,
+  override val link: String? = null,
+  override var uid: UID? = null
+) : DismissibleNotification() {
+  override val level: EventLevel = ERROR
+  override val triggeredBy: String = "Managed Delivery"
+  override val message: String
+    get() {
+      val repo = "repository $repoType/$projectKey/$repoSlug (commit ${commitHash.short})"
+        .let { if (link != null) "[$it]($link)" else it }
+      return "Delivery config for application $application failed to import from $repo"
+    }
+
+  private val String.short: String
+    get() = substring(0, 7)
+}

keel-core/src/main/kotlin/com/netflix/spinnaker/keel/notifications/DismissibleNotification.kt

@@ -0,0 +1,33 @@
+package com.netflix.spinnaker.keel.notifications
+
+import com.fasterxml.jackson.annotation.JsonSubTypes
+import com.fasterxml.jackson.annotation.JsonTypeInfo
+import com.netflix.spinnaker.keel.api.UID
+import com.netflix.spinnaker.keel.events.EventLevel
+import com.netflix.spinnaker.keel.events.EventLevel.INFO
+import java.time.Instant
+
+/**
+ * A user-facing notification that can be dismissed.
+ */
+@JsonTypeInfo(
+  use = JsonTypeInfo.Id.NAME,
+  property = "type",
+  include = JsonTypeInfo.As.PROPERTY
+)
+@JsonSubTypes(
+  JsonSubTypes.Type(value = DeliveryConfigImportFailed::class),
+)
+abstract class DismissibleNotification {
+  open val level: EventLevel = INFO
+  abstract val triggeredAt: Instant
+  abstract val application: String
+  abstract val message: String
+  open val isActive: Boolean = true
+  open val environment: String? = null
+  open val link: String? = null
+  open val triggeredBy: String? = null
+  open val dismissedBy: String? = null
+  open val dismissedAt: Instant? = null
+  open var uid: UID? = null // read from the database
+}

keel-core/src/main/kotlin/com/netflix/spinnaker/keel/persistence/DismissibleNotificationRepository.kt

@@ -0,0 +1,42 @@
+package com.netflix.spinnaker.keel.persistence
+
+import com.netflix.spinnaker.keel.api.UID
+import com.netflix.spinnaker.keel.events.EventLevel
+import com.netflix.spinnaker.keel.notifications.DismissibleNotification
+import java.time.Instant
+
+/**
+ * Repository for [DismissibleNotification].
+ */
+interface DismissibleNotificationRepository {
+  companion object {
+    const val DEFAULT_MAX_NOTIFICATIONS = 10
+  }
+
+  /**
+   * Records a [DismissibleNotification].
+   *
+   * @return the [UID] of the newly created notification.
+   */
+  fun storeNotification(notification: DismissibleNotification): UID
+
+  /**
+   * Retrieves the history of [DismissibleNotification]s for [application], newest to oldest.
+   *
+   * @param application the name of the application.
+   * @param onlyActive whether to filter for only notifications that are active.
+   * @param levels optional filter for event levels. Defaults to all.
+   * @param limit the maximum number of notifications to return.
+   */
+  fun notificationHistory(
+    application: String,
+    onlyActive: Boolean = false,
+    levels: Set<EventLevel> = emptySet(),
+    limit: Int = DEFAULT_MAX_NOTIFICATIONS
+  ): List<DismissibleNotification>
+
+  /**
+   * Sets [DismissibleNotification.isActive] to false in the corresponding database record.
+   */
+  fun dismissNotification(notificationUid: UID, user: String): Boolean
+}

keel-ec2-api/src/main/kotlin/com/netflix/spinnaker/keel/api/ec2/ApplicationLoadBalancerSpec.kt

@@ -34,8 +34,6 @@ data class ApplicationLoadBalancerSpec(
 
   override val id: String = "${locations.account}:$moniker"
 
-  override val displayName = "Application Load Balancer $moniker"
-
   override val dependsOn: Set<Dependency>
     get() = locations.regions.flatMap { region ->
       dependencies.securityGroupNames.map { Dependency(SECURITY_GROUP, region.name, it) }