Skip to content

Commit

Permalink
fix(webhooks): Construct Webhook status check URL honoring the origin…
Browse files Browse the repository at this point in the history 
…al webhook protocol/scheme (#3187)
  • Loading branch information
@srekapalli @marchello2000
srekapalli authored and marchello2000 committed Sep 23, 2019
1 parent 1ae7811 commit 3bc32da
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 22 deletions.
2 changes: 1 addition & 1 deletion ...-webhook/src/main/groovy/com/netflix/spinnaker/orca/webhook/service/WebhookService.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
70 changes: 52 additions & 18 deletions ...webhook/src/main/groovy/com/netflix/spinnaker/orca/webhook/tasks/CreateWebhookTask.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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
}

}
1 change: 0 additions & 1 deletion ...ebhook/src/main/groovy/com/netflix/spinnaker/orca/webhook/tasks/MonitorWebhookTask.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand Down
50 changes: 48 additions & 2 deletions ...ook/src/test/groovy/com/netflix/spinnaker/orca/webhook/tasks/CreateWebhookTaskSpec.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down

0 comments on commit 3bc32da

Please sign in to comment.