diff --git a/orca-webhook/src/main/groovy/com/netflix/spinnaker/orca/webhook/service/WebhookService.groovy b/orca-webhook/src/main/groovy/com/netflix/spinnaker/orca/webhook/service/WebhookService.groovy index 56e531b2b6..d26652eaed 100644 --- a/orca-webhook/src/main/groovy/com/netflix/spinnaker/orca/webhook/service/WebhookService.groovy +++ b/orca-webhook/src/main/groovy/com/netflix/spinnaker/orca/webhook/service/WebhookService.groovy @@ -70,7 +70,7 @@ class WebhookService { ) HttpHeaders headers = buildHttpHeaders(customHeaders) HttpEntity httpEntity = new HttpEntity<>(headers) - return restTemplateProvider.getRestTemplate().exchange(validatedUri, HttpMethod.GET, httpEntity, Object) + return restTemplateProvider.getRestTemplate(url).exchange(validatedUri, HttpMethod.GET, httpEntity, Object) } List getPreconfiguredWebhooks() { diff --git a/orca-webhook/src/main/groovy/com/netflix/spinnaker/orca/webhook/tasks/CreateWebhookTask.groovy b/orca-webhook/src/main/groovy/com/netflix/spinnaker/orca/webhook/tasks/CreateWebhookTask.groovy index 4a98ed709d..55a85395b9 100644 --- a/orca-webhook/src/main/groovy/com/netflix/spinnaker/orca/webhook/tasks/CreateWebhookTask.groovy +++ b/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 + } + } diff --git a/orca-webhook/src/main/groovy/com/netflix/spinnaker/orca/webhook/tasks/MonitorWebhookTask.groovy b/orca-webhook/src/main/groovy/com/netflix/spinnaker/orca/webhook/tasks/MonitorWebhookTask.groovy index 7fef72e34f..4e7514c44b 100644 --- a/orca-webhook/src/main/groovy/com/netflix/spinnaker/orca/webhook/tasks/MonitorWebhookTask.groovy +++ b/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() } diff --git a/orca-webhook/src/test/groovy/com/netflix/spinnaker/orca/webhook/tasks/CreateWebhookTaskSpec.groovy b/orca-webhook/src/test/groovy/com/netflix/spinnaker/orca/webhook/tasks/CreateWebhookTaskSpec.groovy index 5bd7351d33..f68e57a2ac 100644 --- a/orca-webhook/src/test/groovy/com/netflix/spinnaker/orca/webhook/tasks/CreateWebhookTaskSpec.groovy +++ b/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 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(['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(