feat(slack): adding frequency options and test passed/failed notifica…

…tions (#1808) * feat(slack): adding frequncy options and test passed/failed notifications * fix(comments): add more comments * fix(nit): fixing nits
spinnaker · Feb 16, 2021 · 81597fa · 81597fa
1 parent fe1f542
commit 81597fa
Show file tree

Hide file tree

Showing 20 changed files with 412 additions and 158 deletions.
diff --git a/keel-core/src/main/kotlin/com/netflix/spinnaker/keel/events/NotificationEvent.kt b/keel-core/src/main/kotlin/com/netflix/spinnaker/keel/events/NotificationEvent.kt
@@ -9,7 +9,7 @@ import com.netflix.spinnaker.keel.notifications.Notification
 import com.netflix.spinnaker.keel.notifications.NotificationScope
 import com.netflix.spinnaker.keel.notifications.NotificationType
 
-abstract class NotificationEvent{
+abstract class NotificationEvent {
   abstract val scope: NotificationScope
   abstract val type: NotificationType
 }
@@ -25,15 +25,15 @@ abstract class RepeatedNotificationEvent {
 data class UnhealthyNotification(
   override val ref: String,
   override val message: Notification
-  ) :RepeatedNotificationEvent() {
-    override val type = NotificationType.RESOURCE_UNHEALTHY
-    override val scope = NotificationScope.RESOURCE
-  }
+) : RepeatedNotificationEvent() {
+  override val type = NotificationType.RESOURCE_UNHEALTHY
+  override val scope = NotificationScope.RESOURCE
+}
 
 data class PinnedNotification(
   val config: DeliveryConfig,
   val pin: EnvironmentArtifactPin
-): NotificationEvent() {
+) : NotificationEvent() {
   override val type = NotificationType.ARTIFACT_PINNED
   override val scope = NotificationScope.ARTIFACT
 }
@@ -43,7 +43,7 @@ data class UnpinnedNotification(
   val pinnedEnvironment: PinnedEnvironment?,
   val targetEnvironment: String,
   val user: String
-): NotificationEvent() {
+) : NotificationEvent() {
   override val type = NotificationType.ARTIFACT_UNPINNED
   override val scope = NotificationScope.ARTIFACT
 }
@@ -52,7 +52,7 @@ data class MarkAsBadNotification(
   val config: DeliveryConfig,
   val user: String,
   val veto: EnvironmentArtifactVeto
-): NotificationEvent() {
+) : NotificationEvent() {
   override val type = NotificationType.ARTIFACT_MARK_AS_BAD
   override val scope = NotificationScope.ARTIFACT
 }
@@ -62,7 +62,7 @@ data class ArtifactDeployedNotification(
   val artifactVersion: String,
   val deliveryArtifact: DeliveryArtifact,
   val targetEnvironment: String
-): NotificationEvent() {
-  override val type = NotificationType.ARTIFACT_DEPLOYMENT
+) : NotificationEvent() {
+  override val type = NotificationType.ARTIFACT_DEPLOYMENT_SUCCEDEED
   override val scope = NotificationScope.ARTIFACT
 }
diff --git a/keel-core/src/main/kotlin/com/netflix/spinnaker/keel/notifications/NotificationType.kt b/keel-core/src/main/kotlin/com/netflix/spinnaker/keel/notifications/NotificationType.kt
@@ -8,9 +8,15 @@ enum class NotificationType {
   ARTIFACT_PINNED,
   ARTIFACT_UNPINNED,
   ARTIFACT_MARK_AS_BAD,
-  ARTIFACT_DEPLOYMENT,
+  ARTIFACT_DEPLOYMENT_FAILED,
+  ARTIFACT_DEPLOYMENT_SUCCEDEED,
   APPLICATION_PAUSED,
   APPLICATION_RESUMED,
   LIFECYCLE_EVENT,
-  MANUAL_JUDGMENT
+  MANUAL_JUDGMENT_AWAIT,
+  MANUAL_JUDGMENT_REJECTED,
+  MANUAL_JUDGMENT_APPROVED,
+  TEST_PASSED,
+  TEST_FAILED,
+  DELIVEY_CONFIG_UPDATED
 }
diff --git a/keel-slack/src/main/kotlin/com/netflix/spinnaker/keel/slack/NotificationEventListener.kt b/keel-slack/src/main/kotlin/com/netflix/spinnaker/keel/slack/NotificationEventListener.kt
@@ -2,6 +2,10 @@ package com.netflix.spinnaker.keel.slack
 
 import com.netflix.spinnaker.keel.api.DeliveryConfig
 import com.netflix.spinnaker.keel.api.Environment
+import com.netflix.spinnaker.keel.api.NotificationFrequency
+import com.netflix.spinnaker.keel.api.NotificationFrequency.normal
+import com.netflix.spinnaker.keel.api.NotificationFrequency.quiet
+import com.netflix.spinnaker.keel.api.NotificationFrequency.verbose
 import com.netflix.spinnaker.keel.api.NotificationType
 import com.netflix.spinnaker.keel.api.artifacts.DeliveryArtifact
 import com.netflix.spinnaker.keel.api.constraints.ConstraintStatus
@@ -16,10 +20,25 @@ import com.netflix.spinnaker.keel.events.PinnedNotification
 import com.netflix.spinnaker.keel.events.UnpinnedNotification
 import com.netflix.spinnaker.keel.lifecycle.LifecycleEvent
 import com.netflix.spinnaker.keel.lifecycle.LifecycleEventStatus
+import com.netflix.spinnaker.keel.notifications.NotificationType.APPLICATION_PAUSED
+import com.netflix.spinnaker.keel.notifications.NotificationType.APPLICATION_RESUMED
+import com.netflix.spinnaker.keel.notifications.NotificationType.ARTIFACT_DEPLOYMENT_FAILED
+import com.netflix.spinnaker.keel.notifications.NotificationType.ARTIFACT_DEPLOYMENT_SUCCEDEED
+import com.netflix.spinnaker.keel.notifications.NotificationType.ARTIFACT_MARK_AS_BAD
+import com.netflix.spinnaker.keel.notifications.NotificationType.ARTIFACT_PINNED
+import com.netflix.spinnaker.keel.notifications.NotificationType.ARTIFACT_UNPINNED
+import com.netflix.spinnaker.keel.notifications.NotificationType.DELIVEY_CONFIG_UPDATED
+import com.netflix.spinnaker.keel.notifications.NotificationType.LIFECYCLE_EVENT
+import com.netflix.spinnaker.keel.notifications.NotificationType.MANUAL_JUDGMENT_APPROVED
+import com.netflix.spinnaker.keel.notifications.NotificationType.MANUAL_JUDGMENT_AWAIT
+import com.netflix.spinnaker.keel.notifications.NotificationType.MANUAL_JUDGMENT_REJECTED
+import com.netflix.spinnaker.keel.notifications.NotificationType.TEST_FAILED
+import com.netflix.spinnaker.keel.notifications.NotificationType.TEST_PASSED
 import com.netflix.spinnaker.keel.persistence.KeelRepository
 import com.netflix.spinnaker.keel.slack.handlers.SlackNotificationHandler
 import com.netflix.spinnaker.keel.slack.handlers.supporting
 import com.netflix.spinnaker.keel.telemetry.ArtifactVersionVetoed
+import com.netflix.spinnaker.keel.telemetry.VerificationCompleted
 import org.slf4j.LoggerFactory
 import org.springframework.context.event.EventListener
 import org.springframework.stereotype.Component
@@ -62,7 +81,7 @@ class NotificationEventListener(
           application = config.application,
           time = clock.instant()
         ),
-        Type.ARTIFACT_PINNED,
+        ARTIFACT_PINNED,
         pin.targetEnvironment)
     }
 
@@ -95,7 +114,7 @@ class NotificationEventListener(
           user = user,
           targetEnvironment = targetEnvironment
         ),
-        Type.ARTIFACT_UNPINNED,
+        ARTIFACT_UNPINNED,
         targetEnvironment)
 
       log.debug("no environment $targetEnvironment was found in the config named ${config.name}")
@@ -125,7 +144,7 @@ class NotificationEventListener(
           application = config.name,
           comment = veto.comment
         ),
-        Type.ARTIFACT_MARK_AS_BAD,
+        ARTIFACT_MARK_AS_BAD,
         veto.targetEnvironment
       )
     }
@@ -142,7 +161,7 @@ class NotificationEventListener(
           time = clock.instant(),
           application = application
         ),
-        Type.APPLICATION_PAUSED)
+        APPLICATION_PAUSED)
     }
 
   }
@@ -158,7 +177,7 @@ class NotificationEventListener(
           time = clock.instant(),
           application = application
         ),
-        Type.APPLICATION_RESUMED)
+        APPLICATION_RESUMED)
     }
   }
 
@@ -184,7 +203,7 @@ class NotificationEventListener(
             eventType = type,
             application = config.application
           ),
-          Type.LIFECYCLE_EVENT,
+          LIFECYCLE_EVENT,
           artifact = deliveryArtifact)
       }
     }
@@ -210,7 +229,7 @@ class NotificationEventListener(
           priorVersion = priorVersion,
           status = DeploymentStatus.SUCCEEDED
         ),
-        Type.ARTIFACT_DEPLOYMENT,
+        ARTIFACT_DEPLOYMENT_SUCCEDEED,
         targetEnvironment)
     }
   }
@@ -238,7 +257,7 @@ class NotificationEventListener(
           targetEnvironment = veto.targetEnvironment,
           status = DeploymentStatus.FAILED
         ),
-        Type.ARTIFACT_DEPLOYMENT,
+        ARTIFACT_DEPLOYMENT_FAILED,
         veto.targetEnvironment)
     }
   }
@@ -257,7 +276,7 @@ class NotificationEventListener(
 
         val deliveryArtifact = config.artifacts.find {
           it.reference == currentState.artifactReference
-        } .also {
+        }.also {
           if (it == null) log.debug("Artifact with reference ${currentState.artifactReference}  not found in delivery config")
         } ?: return
 
@@ -266,7 +285,7 @@ class NotificationEventListener(
           log.debug("$deliveryArtifact version ${currentState.artifactVersion} not found. Can't send manual judgement notification.")
           return
         }
-        val currentArtifact = repository.getArtifactVersionByPromotionStatus(config, currentState.environmentName , deliveryArtifact, PromotionStatus.CURRENT)
+        val currentArtifact = repository.getArtifactVersionByPromotionStatus(config, currentState.environmentName, deliveryArtifact, PromotionStatus.CURRENT)
 
         sendSlackMessage(
           config,
@@ -279,12 +298,57 @@ class NotificationEventListener(
             deliveryArtifact = deliveryArtifact,
             stateUid = currentState.uid
           ),
-          Type.MANUAL_JUDGMENT,
+          MANUAL_JUDGMENT_AWAIT,
           environment.name)
       }
     }
   }
 
+  @EventListener(VerificationCompleted::class)
+  fun onVerificationCompletedNotification(notification: VerificationCompleted) {
+    log.debug("Received verification completed event: $notification")
+    with(notification) {
+      if (status != ConstraintStatus.PASS && status != ConstraintStatus.FAIL) {
+        log.debug("Not sending notification for verification completed with status $status it's not pass/fail. Ignoring notification for" +
+          "application $application")
+        return
+      }
+      val config = repository.getDeliveryConfig(notification.deliveryConfigName)
+
+      val deliveryArtifact = config.artifacts.find {
+        it.reference == notification.artifactReference
+      }.also {
+        if (it == null) log.debug("Artifact with reference ${notification.artifactReference}  not found in delivery config")
+      } ?: return
+
+      val artifactVersion = repository.getArtifactVersion(deliveryArtifact, notification.artifactVersion, null)
+      if (artifactVersion == null) {
+        log.debug("artifact version is null for application ${config.application}. Can't send verification completed notification.")
+        return
+      }
+
+      val type = when (status) {
+        ConstraintStatus.PASS -> TEST_PASSED
+        ConstraintStatus.FAIL -> TEST_FAILED
+        //We shouldn't get here as we checked prior that status is either fail/pass
+        else -> TEST_PASSED
+      }
+
+      sendSlackMessage(
+        config,
+        SlackVerificationCompletedNotification(
+          time = clock.instant(),
+          application = config.application,
+          artifact = artifactVersion.copy(reference = deliveryArtifact.reference),
+          targetEnvironment = environmentName,
+          deliveryArtifact = deliveryArtifact,
+          status = status
+        ),
+        type,
+        environmentName)
+    }
+  }
+
 
   private inline fun <reified T : SlackNotificationEvent> sendSlackMessage(config: DeliveryConfig, message: T, type: Type,
                                                                            targetEnvironment: String? = null,
@@ -306,10 +370,25 @@ class NotificationEventListener(
 
     environments.flatMap { it.notifications }
       .filter { it.type == NotificationType.slack }
+      .filter { translateFrequencyToEvents(it.frequency).contains(type) }
       .groupBy { it.address }
       .forEach { (channel, _) ->
         handler.sendMessage(message, channel)
       }
   }
+
+
+  fun translateFrequencyToEvents(frequency: NotificationFrequency): List<Type> {
+    val quietNotifications = listOf(ARTIFACT_MARK_AS_BAD, ARTIFACT_PINNED, ARTIFACT_UNPINNED, LIFECYCLE_EVENT, APPLICATION_PAUSED,
+      APPLICATION_RESUMED, MANUAL_JUDGMENT_AWAIT, ARTIFACT_DEPLOYMENT_FAILED, TEST_FAILED)
+    val normalNotifications = quietNotifications + listOf(ARTIFACT_DEPLOYMENT_SUCCEDEED, DELIVEY_CONFIG_UPDATED, TEST_PASSED)
+    val verboseNotifications = normalNotifications + listOf(MANUAL_JUDGMENT_REJECTED, MANUAL_JUDGMENT_APPROVED)
+
+    return when (frequency) {
+      verbose -> verboseNotifications
+      normal -> normalNotifications
+      quiet -> quietNotifications
+    }
+  }
 }
 
diff --git a/keel-slack/src/main/kotlin/com/netflix/spinnaker/keel/slack/SlackNotificationEvent.kt b/keel-slack/src/main/kotlin/com/netflix/spinnaker/keel/slack/SlackNotificationEvent.kt
@@ -2,6 +2,7 @@ package com.netflix.spinnaker.keel.slack
 
 import com.netflix.spinnaker.keel.api.artifacts.DeliveryArtifact
 import com.netflix.spinnaker.keel.api.artifacts.PublishedArtifact
+import com.netflix.spinnaker.keel.api.constraints.ConstraintStatus
 import com.netflix.spinnaker.keel.core.api.EnvironmentArtifactPin
 import com.netflix.spinnaker.keel.core.api.UID
 import com.netflix.spinnaker.keel.lifecycle.LifecycleEventType
@@ -78,6 +79,15 @@ data class SlackManualJudgmentNotification(
   override val application: String
 ) : SlackNotificationEvent(time, application)
 
+data class SlackVerificationCompletedNotification(
+  val artifact: PublishedArtifact,
+  override val time: Instant,
+  val targetEnvironment: String,
+  val deliveryArtifact: DeliveryArtifact,
+  val status: ConstraintStatus,
+  override val application: String
+) : SlackNotificationEvent(time, application)
+
 
 enum class DeploymentStatus {
   SUCCEEDED, FAILED;

diff --git a/keel-slack/src/main/kotlin/com/netflix/spinnaker/keel/slack/SlackService.kt b/keel-slack/src/main/kotlin/com/netflix/spinnaker/keel/slack/SlackService.kt
@@ -7,16 +7,13 @@ import com.netflix.spinnaker.config.SlackConfiguration
 import com.netflix.spinnaker.keel.notifications.NotificationType
 import com.slack.api.Slack
 import com.slack.api.model.block.LayoutBlock
-import com.slack.api.webhook.Payload.PayloadBuilder
-import com.slack.api.webhook.WebhookPayloads.payload
 import org.slf4j.LoggerFactory
 import org.springframework.boot.context.properties.EnableConfigurationProperties
 import org.springframework.core.env.Environment
 import org.springframework.stereotype.Component
 
 /**
- * This notifier is responsible for actually sending the Slack notification,
- * based on the [channel] and the [blocks] it gets from the different handlers.
+ * This notifier is responsible for actually sending or fetching data from Slack directly.
  */
 @Component
 @EnableConfigurationProperties(SlackConfiguration::class)
@@ -34,22 +31,28 @@ class SlackService(
   private val isSlackEnabled: Boolean
     get() = springEnv.getProperty("keel.notifications.slack", Boolean::class.java, true)
 
+  /**
+   * Sends slack notification to [channel], which the specified [blocks].
+   * In case of an error with creating the blocks, or for notification preview, the fallback text will be sent.
+   */
   fun sendSlackNotification(channel: String, blocks: List<LayoutBlock>,
-                            application: String, type: NotificationType) {
+                            application: String, type: List<NotificationType>,
+                            fallbackText: String) {
     if (isSlackEnabled) {
       log.debug("Sending slack notification $type for application $application in channel $channel")
 
       val response = slack.methods(configToken).chatPostMessage { req ->
         req
           .channel(channel)
           .blocks(blocks)
+          .text(fallbackText)
       }
 
       if (response.isOk) {
         spectator.counter(
           SLACK_MESSAGE_SENT,
           listOf(
-            BasicTag("notificationType", type.name),
+            BasicTag("notificationType", type.first().name),
             BasicTag("application", application)
           )
         ).safeIncrement()
@@ -67,6 +70,9 @@ class SlackService(
     }
   }
 
+  /**
+   * Get slack username by the user's [email]. Return the original email if username is not found.
+   */
   fun getUsernameByEmail(email: String): String {
     log.debug("lookup user id for email $email")
     val response = slack.methods(configToken).usersLookupByEmail { req ->
@@ -84,6 +90,9 @@ class SlackService(
     return email
   }
 
+  /**
+   * Get user's email address by slack [userId]. Return the original userId if email is not found.
+   */
   fun getEmailByUserId(userId: String): String {
     log.debug("lookup user email for username $userId")
     val response = slack.methods(configToken).usersInfo { req ->
@@ -103,18 +112,6 @@ class SlackService(
     return userId
   }
 
-  // Update a notification based on the response Url, using blocks (the actual notification).
-  // If something failed, the fallback text will be displayed
-  fun respondToCallback(responseUrl: String, blocks: List<LayoutBlock>, fallbackText: String) {
-    val response = slack.send(responseUrl, payload { p: PayloadBuilder ->
-      p
-        .text(fallbackText)
-        .blocks(blocks)
-    })
-
-    log.debug("slack respondToCallback returned ${response.code}")
-  }
-
 
   companion object {
     private const val SLACK_MESSAGE_SENT = "keel.slack.message.sent"