fix(misc): Allow monitor only webhook tasks to default to webhook url…

… if status endpoint is not provided (#3995) Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
spinnaker · Nov 12, 2020 · b53347d · b53347d
1 parent dab03bb
commit b53347d
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 7 deletions.
diff --git a/...-webhook/src/main/groovy/com/netflix/spinnaker/orca/webhook/service/WebhookService.groovy b/...-webhook/src/main/groovy/com/netflix/spinnaker/orca/webhook/service/WebhookService.groovy
@@ -115,7 +115,7 @@ class WebhookService {
           httpMethod = stageData.method
           break
         case WebhookTaskType.MONITOR:
-          destinationUrl = stageData.statusEndpoint
+          destinationUrl = stageData.statusEndpoint?.trim() ?: stageData.url
           payloadEntity = new HttpEntity<>(null, headers)
           break
         case WebhookTaskType.CANCEL:

diff --git a/...ebhook/src/main/groovy/com/netflix/spinnaker/orca/webhook/tasks/MonitorWebhookTask.groovy b/...ebhook/src/main/groovy/com/netflix/spinnaker/orca/webhook/tasks/MonitorWebhookTask.groovy
@@ -29,7 +29,7 @@ import com.netflix.spinnaker.orca.webhook.pipeline.WebhookStage
 import com.netflix.spinnaker.orca.webhook.service.WebhookService
 import groovy.util.logging.Slf4j
 import org.springframework.beans.factory.annotation.Autowired
-import org.springframework.beans.factory.annotation.Value
+import org.apache.commons.lang3.StringUtils
 import org.springframework.stereotype.Component
 import org.springframework.web.client.HttpStatusCodeException
 
@@ -68,10 +68,14 @@ class MonitorWebhookTask implements OverridableTimeoutRetryableTask {
   TaskResult execute(StageExecution stage) {
     WebhookStage.StageData stageData = stage.mapTo(WebhookStage.StageData)
 
-    if (Strings.isNullOrEmpty(stageData.statusEndpoint)) {
+    if (StringUtils.isBlank(stageData.statusEndpoint) && !stageData.monitorOnly) {
       throw new IllegalStateException("Missing required parameter: statusEndpoint = ${stageData.statusEndpoint}")
     }
 
+    if (stageData.monitorOnly && StringUtils.isAllBlank(stageData.statusEndpoint, stageData.url)) {
+      throw new IllegalStateException("Missing required parameter. Either webhook url or statusEndpoint are required")
+    }
+
     // Preserve the responses we got from createWebhookTask, but reset the monitor subkey as we will overwrite it new data
     def originalResponse = stage.context.getOrDefault("webhook", [:])
     originalResponse["monitor"] = [:]

diff --git a/...ok/src/test/groovy/com/netflix/spinnaker/orca/webhook/tasks/MonitorWebhookTaskSpec.groovy b/...ok/src/test/groovy/com/netflix/spinnaker/orca/webhook/tasks/MonitorWebhookTaskSpec.groovy
@@ -39,10 +39,12 @@ class MonitorWebhookTaskSpec extends Specification {
   MonitorWebhookTask monitorWebhookTask = new MonitorWebhookTask(null, new WebhookProperties())
 
   @Unroll
-  def "should fail if required parameter #parameter is missing"() {
+  def "should fail if required parameter(url: #url, statusUrl: #statusEndpoint) is missing"() {
     setup:
     def stage = new StageExecutionImpl(pipeline, "webhook", [
-      statusEndpoint: 'https://my-service.io/api/status/123',
+      statusEndpoint: statusEndpoint,
+      url: webhookurl,
+      monitorOnly: monitorOnly,
       statusJsonPath: '$.status',
       successStatuses: 'SUCCESS',
       canceledStatuses: 'CANCELED',
@@ -51,15 +53,16 @@ class MonitorWebhookTaskSpec extends Specification {
     ])
 
     when:
-    stage.context.remove parameter
     monitorWebhookTask.execute stage
 
     then:
     def ex = thrown IllegalStateException
     ex.message.startsWith("Missing required parameter")
 
     where:
-    parameter << ["statusEndpoint"]
+    statusEndpoint   |  webhookurl        | monitorOnly
+    null             |  null              | true
+    null             | 'http://test.net'  | false
   }
 
   def "should fail if no parameters are supplied"() {