fix(webhooks): Construct Webhook status check URL honoring the origin… #3187
Conversation
srekapalli
commented
Sep 23, 2019
srekapalli
commented
- Change here is to make the status check URL of a webhook to have the same protocol/scheme of the webhook.
- RestTemplate provider keys on the URL as a whole and it is important to keep the scheme as is from the original webhook so that the same RestTemplate provider as webhook will be used for making the status check call.
| @@ -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) | |||
There was a problem hiding this comment.
Method expects a URL to be provided so adding it though Groovy doesn't complain about it during compilation time
| @@ -160,7 +160,6 @@ class MonitorWebhookTask implements OverridableTimeoutRetryableTask { | |||
| return TaskResult.builder(statusMap[result.toString().toUpperCase()]).context(responsePayload).build() | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Removed unnecessary statement.
| @@ -162,4 +157,41 @@ class CreateWebhookTask implements RetryableTask { | |||
| return TaskResult.builder(ExecutionStatus.TERMINAL).context(outputs).build() | |||
| } | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Below method adjusts the status check URL if the original webhook domain and status check URL domain given back matches.
orca-webhook/src/main/groovy/com/netflix/spinnaker/orca/webhook/tasks/CreateWebhookTask.groovy
Outdated
Show resolved
Hide resolved
| @Slf4j | ||
| @Component | ||
| class CreateWebhookTask implements RetryableTask { | ||
|
|
||
| private static final Pattern URL_SCHEME = Pattern.compile("(.*)://(.*)") |
There was a problem hiding this comment.
was trying to see if there are ways to make the URL class do the "hard" work here but can't find anything.
There was a problem hiding this comment.
@srekapalli I believe you mentioned there are some other URIs involved in the webhook flow besides status? How are those going to be handled
@cfieber : Actually they were all going through the same webHookService implementation and only status URL is derived from the webhook call response and that is the reason we needed special treatment for that. I wondered initially that I need to do the same for 'cancel' and 'progress' URL's but we don't need to. |
