fix(webhooks): Construct Webhook status check URL honoring the origin…

orca-webhook/src/main/groovy/com/netflix/spinnaker/orca/webhook/service/WebhookService.groovy

@@ -70,7 +70,7 @@ class WebhookService {
     )
     HttpHeaders headers = buildHttpHeaders(customHeaders)
     HttpEntity<Object> httpEntity = new HttpEntity<>(headers)
-    return restTemplateProvider.getRestTemplate().exchange(validatedUri, HttpMethod.GET, httpEntity, Object)
+    return restTemplateProvider.getRestTemplate(url).exchange(validatedUri, HttpMethod.GET, httpEntity, Object)
   }
 
   List<WebhookProperties.PreconfiguredWebhook> getPreconfiguredWebhooks() {

orca-webhook/src/main/groovy/com/netflix/spinnaker/orca/webhook/tasks/CreateWebhookTask.groovy

@@ -21,7 +21,6 @@ import com.fasterxml.jackson.core.JsonParseException
 import com.fasterxml.jackson.databind.JsonMappingException
 import com.fasterxml.jackson.databind.ObjectMapper
 import com.jayway.jsonpath.JsonPath
-import com.jayway.jsonpath.PathNotFoundException
 import com.netflix.spinnaker.orca.ExecutionStatus
 import com.netflix.spinnaker.orca.RetryableTask
 import com.netflix.spinnaker.orca.TaskResult
@@ -32,13 +31,19 @@ import com.netflix.spinnaker.orca.webhook.service.WebhookService
 import groovy.util.logging.Slf4j
 import org.apache.http.HttpHeaders
 import org.springframework.beans.factory.annotation.Autowired
+import org.springframework.http.ResponseEntity
 import org.springframework.stereotype.Component
 import org.springframework.web.client.HttpStatusCodeException
 
+import java.util.regex.Matcher
+import java.util.regex.Pattern
+
 @Slf4j
 @Component
 class CreateWebhookTask implements RetryableTask {
 
+  private static final Pattern URL_SCHEME = Pattern.compile("(.*)://(.*)")
+
   long backoffPeriod = 10000
   long timeout = 300000
 
@@ -119,24 +124,16 @@ class CreateWebhookTask implements RetryableTask {
 
     if (statusCode.is2xxSuccessful() || statusCode.is3xxRedirection()) {
       if (stageData.waitForCompletion) {
-        def statusUrl = null
-        def statusUrlResolution = stageData.statusUrlResolution
-        switch (statusUrlResolution) {
-          case WebhookProperties.StatusUrlResolution.getMethod:
-            statusUrl = stageData.url
-            break
-          case WebhookProperties.StatusUrlResolution.locationHeader:
-            statusUrl = response.headers.getFirst(HttpHeaders.LOCATION)
-            break
-          case WebhookProperties.StatusUrlResolution.webhookResponse:
-            try {
-              statusUrl = JsonPath.compile(stageData.statusUrlJsonPath as String).read(response.body)
-            } catch (PathNotFoundException e) {
-              outputs.webhook << [error: e.message]
-              return TaskResult.builder(ExecutionStatus.TERMINAL).context(outputs).build()
-            }
+        String statusUrl
+        try {
+          statusUrl = determineWebHookStatusCheckUrl(response, stageData)
+        } catch (Exception e) {
+          outputs.webhook << [error: 'Exception while resolving status check URL: ' + e.message]
+          log.error('Exception received while determining status check url', e)
+          return TaskResult.builder(ExecutionStatus.TERMINAL).context(outputs).build()
         }
-        if (!statusUrl || !(statusUrl instanceof String)) {
+
+        if (!statusUrl) {
           outputs.webhook << [
             error         : "The status URL couldn't be resolved, but 'Wait for completion' was checked",
             statusEndpoint: statusUrl
@@ -162,4 +159,41 @@ class CreateWebhookTask implements RetryableTask {
       return TaskResult.builder(ExecutionStatus.TERMINAL).context(outputs).build()
     }
   }
+
+  private String determineWebHookStatusCheckUrl(ResponseEntity response, WebhookStage.StageData stageData) {
+
+    String statusCheckUrl
+
+    switch (stageData.statusUrlResolution) {
+      case WebhookProperties.StatusUrlResolution.getMethod:
+        statusCheckUrl = stageData.url
+        break
+      case WebhookProperties.StatusUrlResolution.locationHeader:
+        statusCheckUrl = response.headers.getFirst(HttpHeaders.LOCATION)
+        break
+      case WebhookProperties.StatusUrlResolution.webhookResponse:
+        statusCheckUrl = JsonPath.compile(stageData.statusUrlJsonPath as String).read(response.body)
+        break
+    }
+    log.info('Web hook status check url as resolved: {}', statusCheckUrl)
+
+    // Preserve the protocol scheme of original webhook that was called, when calling for status check of a webhook.
+    if (statusCheckUrl != stageData.url) {
+      Matcher statusUrlMatcher = URL_SCHEME.matcher(statusCheckUrl)
+      URI statusCheckUri = URI.create(statusCheckUrl).normalize()
+      String statusCheckHost = statusCheckUri.getHost()
+
+      URI webHookUri = URI.create(stageData.url).normalize()
+      String webHookHost = webHookUri.getHost()
+      if (webHookHost == statusCheckHost &&
+          webHookUri.getScheme() != statusCheckUri.getScheme() && statusUrlMatcher.find()) {
+        // Same hosts keep the original protocol scheme of the webhook that was originally set.
+        statusCheckUrl = webHookUri.getScheme() + '://' + statusUrlMatcher.group(2)
+        log.info('Adjusted Web hook status check url: {}', statusCheckUrl)
+      }
+    }
+
+    return statusCheckUrl
+  }
+
 }

orca-webhook/src/main/groovy/com/netflix/spinnaker/orca/webhook/tasks/MonitorWebhookTask.groovy

@@ -160,7 +160,6 @@ class MonitorWebhookTask implements OverridableTimeoutRetryableTask {
       return TaskResult.builder(statusMap[result.toString().toUpperCase()]).context(responsePayload).build()
     }
 
-    stage.context
     return TaskResult.builder(ExecutionStatus.RUNNING).context(response ? responsePayload : originalResponse).build()
   }
 

orca-webhook/src/test/groovy/com/netflix/spinnaker/orca/webhook/tasks/CreateWebhookTaskSpec.groovy

@@ -20,6 +20,7 @@ package com.netflix.spinnaker.orca.webhook.tasks
 import com.netflix.spinnaker.orca.ExecutionStatus
 import com.netflix.spinnaker.orca.pipeline.model.Execution
 import com.netflix.spinnaker.orca.pipeline.model.Stage
+import com.netflix.spinnaker.orca.webhook.config.WebhookProperties
 import com.netflix.spinnaker.orca.webhook.service.WebhookService
 import org.springframework.http.HttpHeaders
 import org.springframework.http.HttpMethod
@@ -28,6 +29,8 @@ import org.springframework.http.ResponseEntity
 import org.springframework.web.client.HttpServerErrorException
 import spock.lang.Specification
 import spock.lang.Subject
+import spock.lang.Unroll
+
 import java.nio.charset.Charset
 
 class CreateWebhookTaskSpec extends Specification {
@@ -454,8 +457,7 @@ class CreateWebhookTaskSpec extends Specification {
         statusCode: HttpStatus.CREATED,
         statusCodeValue: HttpStatus.CREATED.value(),
         body: body,
-        error: "The status URL couldn't be resolved, but 'Wait for completion' was checked",
-        statusEndpoint: ["this", "is", "a", "list"]
+        error: "Exception while resolving status check URL: Illegal character in path at index 0: [this, is, a, list]"
       ]
     ]
     stage.context.statusEndpoint == null
@@ -558,6 +560,50 @@ class CreateWebhookTaskSpec extends Specification {
     ]
   }
 
+  @Unroll
+  def 'should honor the web hook #webHookUrl protocol/scheme for status check URL #responseStatusCheckUrl'() {
+    setup:
+    Map<String, Object> stageContext = [method: HttpMethod.POST,
+                                        payload: ['test': 'test'],
+                                        waitForCompletion: true,
+                                        statusUrlJsonPath:  '$.statusCheckUrl']
+    def stage = new Stage(pipeline, "webhook", "Protocol scheme webhook", stageContext)
+
+    when:
+    stage.context.statusUrlResolution = statusUrlResolution
+    stage.context.url = webHookUrl
+
+    def headers = new HttpHeaders()
+    if (statusUrlResolution == WebhookProperties.StatusUrlResolution.locationHeader) {
+      headers.add(HttpHeaders.LOCATION, responseStatusCheckUrl)
+    }
+
+    createWebhookTask.webhookService = Stub(WebhookService) {
+      exchange(
+        stage.context.method as HttpMethod,
+        stage.context.url as String,
+        stage.context.payload,
+        null
+      ) >> new ResponseEntity<Map>(['statusCheckUrl': responseStatusCheckUrl] as Map, headers, HttpStatus.OK)
+
+    }
+
+    def result = createWebhookTask.execute(stage)
+
+    then:
+    result.status == ExecutionStatus.SUCCEEDED
+    stage.context.statusEndpoint == resultstatusCheckUrl
+
+    where:
+    webHookUrl                   | statusUrlResolution                                    | responseStatusCheckUrl             || resultstatusCheckUrl
+    'proto-abc://test.com'       | WebhookProperties.StatusUrlResolution.getMethod        | 'https://test.com'                 || 'proto-abc://test.com'
+    'proto-abc://test.com'       | WebhookProperties.StatusUrlResolution.webhookResponse  | 'https://test.com'                 || 'proto-abc://test.com'
+    'proto-abc://test.com'       | WebhookProperties.StatusUrlResolution.locationHeader   | 'https://test.com/api/status/123'  || 'proto-abc://test.com/api/status/123'
+    'proto-abc://test.com'       | WebhookProperties.StatusUrlResolution.getMethod        | 'https://blah.com'                 || 'proto-abc://test.com'
+    'proto-abc://test.com'       | WebhookProperties.StatusUrlResolution.webhookResponse  | 'https://blah.com'                 || 'https://blah.com'
+    'proto-abc://test.com'       | WebhookProperties.StatusUrlResolution.locationHeader   | 'https://blah.com/api/status/123'  || 'https://blah.com/api/status/123'
+  }
+
   private HttpServerErrorException throwHttpException(HttpStatus statusCode, String body) {
     if (body != null) {
       throw new HttpServerErrorException(