fix(webhook): Pass stage configuration data to rest template provider… (

#3965) * fix(webhook): Pass stage configuration data to rest template providers for more customization needs * Add enum to represent the webhook task type Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
spinnaker · Oct 14, 2020 · eef8e09 · eef8e09
1 parent d1da56e
commit eef8e09
Show file tree

Hide file tree

Showing 8 changed files with 215 additions and 174 deletions.
diff --git a/...c/main/groovy/com/netflix/spinnaker/orca/webhook/service/DefaultRestTemplateProvider.java b/...c/main/groovy/com/netflix/spinnaker/orca/webhook/service/DefaultRestTemplateProvider.java
@@ -16,14 +16,15 @@
 
 package com.netflix.spinnaker.orca.webhook.service;
 
+import com.netflix.spinnaker.orca.webhook.pipeline.WebhookStage;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.core.annotation.Order;
 import org.springframework.stereotype.Component;
 import org.springframework.web.client.RestTemplate;
 
 @Order
 @Component
-public class DefaultRestTemplateProvider implements RestTemplateProvider {
+public class DefaultRestTemplateProvider implements RestTemplateProvider<WebhookStage.StageData> {
   private final RestTemplate restTemplate;
 
   @Autowired
@@ -32,12 +33,17 @@ public DefaultRestTemplateProvider(RestTemplate restTemplate) {
   }
 
   @Override
-  public boolean supports(String targetUrl) {
+  public boolean supports(String targetUrl, WebhookStage.StageData stageData) {
     return true;
   }
 
   @Override
   public RestTemplate getRestTemplate(String targetUrl) {
     return restTemplate;
   }
+
+  @Override
+  public Class<WebhookStage.StageData> getStageDataType() {
+    return WebhookStage.StageData.class;
+  }
 }
diff --git a/...hook/src/main/groovy/com/netflix/spinnaker/orca/webhook/service/RestTemplateProvider.java b/...hook/src/main/groovy/com/netflix/spinnaker/orca/webhook/service/RestTemplateProvider.java
@@ -16,11 +16,22 @@
 
 package com.netflix.spinnaker.orca.webhook.service;
 
+import com.netflix.spinnaker.orca.webhook.pipeline.WebhookStage;
 import org.springframework.web.client.RestTemplate;
 
-public interface RestTemplateProvider {
-  /** @return true if this {@code RestTemplateProvider} supports the given url */
-  boolean supports(String targetUrl);
+/**
+ * Concrete implementations will provide a rest template customized to call webhook related
+ * endpoints.
+ *
+ * @param <T> stage configuration data.
+ */
+public interface RestTemplateProvider<T extends WebhookStage.StageData> {
+
+  /**
+   * @return true if this {@code RestTemplateProvider} supports the given url and stage
+   *     configuration
+   */
+  boolean supports(String targetUrl, T stageData);
 
   /**
    * Provides an opportunity for a {@code RestTemplateProvider} to modify any aspect of the target
@@ -30,10 +41,13 @@ public interface RestTemplateProvider {
    *
    * @return a potentially modified target url
    */
-  default String getTargetUrl(String targetUrl) {
+  default String getTargetUrl(String targetUrl, T stageData) {
     return targetUrl;
   }
 
   /** @return a configured {@code RestTemplate} */
   RestTemplate getRestTemplate(String targetUrl);
+
+  /** @return type of stage data that the template provider is expecting */
+  Class<T> getStageDataType();
 }
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
@@ -17,16 +17,27 @@
 
 package com.netflix.spinnaker.orca.webhook.service
 
+import com.netflix.spinnaker.kork.exceptions.SpinnakerException
+import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution
 import com.netflix.spinnaker.orca.config.UserConfiguredUrlRestrictions
 import com.netflix.spinnaker.orca.webhook.config.WebhookProperties
+import com.netflix.spinnaker.orca.webhook.pipeline.WebhookStage
+import groovy.util.logging.Slf4j
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.http.HttpEntity
 import org.springframework.http.HttpHeaders
 import org.springframework.http.HttpMethod
 import org.springframework.http.ResponseEntity
 import org.springframework.stereotype.Service
+import org.springframework.web.client.HttpStatusCodeException
+import org.springframework.web.client.RestTemplate
 
+/**
+ * Service that interacts with webhook endpoints for initiating a webhook call, checking the status of webhook
+ * or cancelling a webhook call.
+ */
 @Service
+@Slf4j
 class WebhookService {
 
   // These headers create a security vulnerability.
@@ -36,7 +47,7 @@ class WebhookService {
     "X-SPINNAKER-USER-ORIGIN",
     "X-SPINNAKER-REQUEST-ID",
     "X-SPINNAKER-EXECUTION-ID"
-  ];
+  ]
 
   @Autowired
   private List<RestTemplateProvider> restTemplateProviders = []
@@ -47,30 +58,90 @@ class WebhookService {
   @Autowired
   private WebhookProperties preconfiguredWebhookProperties
 
-  ResponseEntity<Object> exchange(HttpMethod httpMethod, String url, Object payload, Object customHeaders) {
-    def restTemplateProvider = restTemplateProviders.find {
-      it.supports(url)
+  ResponseEntity<Object> callWebhook(StageExecution stageExecution) {
+    RestTemplateData restTemplateData = getRestTemplateData(WebhookTaskType.CREATE, stageExecution)
+    if (restTemplateData == null) {
+      throw new SpinnakerException("Unable to determine rest template to call webhook")
+    }
+    return  restTemplateData.exchange()
+  }
+
+  ResponseEntity<Object> getWebhookStatus(StageExecution stageExecution) {
+    RestTemplateData restTemplateData = getRestTemplateData(WebhookTaskType.MONITOR, stageExecution)
+    if (restTemplateData == null) {
+      throw new SpinnakerException("Unable to determine rest template to monitor webhook")
+    }
+    return restTemplateData.exchange()
+  }
+
+  ResponseEntity<Object> cancelWebhook(StageExecution stageExecution) {
+    RestTemplateData restTemplateData = getRestTemplateData(WebhookTaskType.CANCEL, stageExecution)
+
+    // Only do cancellation if we can determine the rest template to use.
+    if (restTemplateData == null) {
+      log.warn("Cannot determine rest template to cancel the webhook")
+      return null
+    }
+    WebhookStage.StageData stageData = restTemplateData.stageData
+    try {
+      log.info("Sending best effort webhook cancellation to ${stageData.cancelEndpoint}")
+      ResponseEntity<Object> response = restTemplateData.exchange()
+      log.debug(
+          "Received status code {} from cancel endpoint {} in execution {} in stage {}",
+          response.statusCode,
+          stageData.cancelEndpoint,
+          stageExecution.execution.id,
+          stageExecution.id
+      )
+    } catch (HttpStatusCodeException e) {
+      log.warn("Failed to cancel webhook ${stageData.cancelEndpoint} with statusCode=${e.getStatusCode().value()}", e)
+    } catch (Exception e) {
+      log.warn("Failed to cancel webhook ${stageData.cancelEndpoint}", e)
     }
 
-    URI validatedUri = userConfiguredUrlRestrictions.validateURI(
-      restTemplateProvider.getTargetUrl(url)
-    )
-    HttpHeaders headers = buildHttpHeaders(customHeaders)
-    HttpEntity<Object> payloadEntity = new HttpEntity<>(payload, headers)
-    return restTemplateProvider.getRestTemplate(url).exchange(validatedUri, httpMethod, payloadEntity, Object)
   }
 
-  ResponseEntity<Object> getStatus(String url, Object customHeaders) {
-    def restTemplateProvider = restTemplateProviders.find {
-      it.supports(url)
+  private RestTemplateData getRestTemplateData(WebhookTaskType taskType, StageExecution stageExecution) {
+    String destinationUrl = null
+    for (RestTemplateProvider provider : restTemplateProviders) {
+      WebhookStage.StageData stageData = stageExecution.mapTo(provider.getStageDataType())
+      HttpHeaders headers = buildHttpHeaders(stageData.customHeaders)
+      HttpMethod httpMethod = HttpMethod.GET
+      HttpEntity<Object> payloadEntity = null
+      switch (taskType) {
+        case WebhookTaskType.CREATE:
+          destinationUrl = stageData.url
+          payloadEntity = new HttpEntity<>(stageData.payload, headers)
+          httpMethod = stageData.method
+          break
+        case WebhookTaskType.MONITOR:
+          destinationUrl = stageData.statusEndpoint
+          payloadEntity = new HttpEntity<>(null, headers)
+          break
+        case WebhookTaskType.CANCEL:
+          destinationUrl = stageData.cancelEndpoint
+          payloadEntity = new HttpEntity<>(stageData.cancelPayload, headers)
+          httpMethod = stageData.cancelMethod
+          break
+        default:
+          destinationUrl = ''
+          break
+      }
+
+      // Return on the first match
+      if (destinationUrl != null && !destinationUrl.isEmpty() && provider.supports(destinationUrl, stageData)) {
+        URI validatedUri = userConfiguredUrlRestrictions.validateURI(provider.getTargetUrl(destinationUrl, stageData))
+        RestTemplate restTemplate = provider.getRestTemplate(destinationUrl)
+        return new RestTemplateData(restTemplate, validatedUri, httpMethod, payloadEntity, stageData)
+      }
     }
 
-    URI validatedUri = userConfiguredUrlRestrictions.validateURI(
-      restTemplateProvider.getTargetUrl(url)
-    )
-    HttpHeaders headers = buildHttpHeaders(customHeaders)
-    HttpEntity<Object> httpEntity = new HttpEntity<>(headers)
-    return restTemplateProvider.getRestTemplate(url).exchange(validatedUri, HttpMethod.GET, httpEntity, Object)
+    // No providers found.
+    log.warn('Unable to find rest template provider for url: {} , executionId: {}, webhookTaskType: {}',
+        destinationUrl,
+        stageExecution.id,
+        taskType)
+    return null
   }
 
   List<WebhookProperties.PreconfiguredWebhook> getPreconfiguredWebhooks() {
@@ -81,7 +152,7 @@ class WebhookService {
     HttpHeaders headers = new HttpHeaders()
     customHeaders?.each { key, value ->
       if (headerDenyList.contains(key.toUpperCase())) {
-        return;
+        return
       }
       if (value instanceof List<String>) {
         headers.put(key as String, value as List<String>)
@@ -91,4 +162,37 @@ class WebhookService {
     }
     return headers
   }
+
+  private static class RestTemplateData {
+
+    final RestTemplate restTemplate
+    final URI validatedUri
+    final HttpMethod httpMethod
+    final HttpEntity<Object> payloadEntity
+    final WebhookStage.StageData stageData
+
+    RestTemplateData(RestTemplate restTemplate,
+                     URI validatedUri,
+                     HttpMethod httpMethod,
+                     HttpEntity<Object> payloadEntity,
+                     WebhookStage.StageData stageData) {
+      this.restTemplate = restTemplate
+      this.validatedUri = validatedUri
+      this.httpMethod = httpMethod
+      this.payloadEntity = payloadEntity
+      this.stageData = stageData
+    }
+
+    ResponseEntity<Object> exchange() {
+      return this.restTemplate.exchange(validatedUri, httpMethod, payloadEntity, Object)
+    }
+
+  }
+
+  private static enum WebhookTaskType {
+    CREATE,
+    MONITOR,
+    CANCEL
+  }
+
 }
diff --git a/...webhook/src/main/groovy/com/netflix/spinnaker/orca/webhook/tasks/CreateWebhookTask.groovy b/...webhook/src/main/groovy/com/netflix/spinnaker/orca/webhook/tasks/CreateWebhookTask.groovy
@@ -48,12 +48,10 @@ class CreateWebhookTask implements RetryableTask {
 
   @Override
   TaskResult execute(StageExecution stage) {
-    WebhookStage.StageData stageData = stage.mapTo(WebhookStage.StageData)
 
     WebhookResponseProcessor responseProcessor = new WebhookResponseProcessor(objectMapper, stage, webhookProperties)
-
     try {
-      def response = webhookService.exchange(stageData.method, stageData.url, stageData.payload, stageData.customHeaders)
+      def response = webhookService.callWebhook(stage)
       return responseProcessor.process(response, null)
     } catch (Exception e) {
       return responseProcessor.process(null, e)

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
@@ -79,7 +79,7 @@ class MonitorWebhookTask implements OverridableTimeoutRetryableTask {
 
     def response
     try {
-      response = webhookService.getStatus(stageData.statusEndpoint, stageData.customHeaders)
+      response = webhookService.getWebhookStatus(stage)
       log.debug(
         "Received status code {} from status endpoint {} in execution {} in stage {}",
         response.statusCode,
@@ -176,28 +176,7 @@ class MonitorWebhookTask implements OverridableTimeoutRetryableTask {
   }
 
   @Override void onCancel(@Nonnull StageExecution stage) {
-    WebhookStage.StageData stageData = stage.mapTo(WebhookStage.StageData)
-
-    // Only do cancellation if we made the initial webhook request and the user specified a cancellation endpoint
-    if (Strings.isNullOrEmpty(stageData.cancelEndpoint) || Strings.isNullOrEmpty(stageData.webhook?.statusCode)) {
-      return
-    }
-
-    try {
-      log.info("Sending best effort webhook cancellation to ${stageData.cancelEndpoint}")
-      def response = webhookService.exchange(stageData.cancelMethod, stageData.cancelEndpoint, stageData.cancelPayload, stageData.customHeaders)
-      log.debug(
-        "Received status code {} from cancel endpoint {} in execution {} in stage {}",
-        response.statusCode,
-        stageData.cancelEndpoint,
-        stage.execution.id,
-        stage.id
-      )
-    } catch (HttpStatusCodeException e) {
-      log.warn("Failed to cancel webhook ${stageData.cancelEndpoint} with statusCode=${e.getStatusCode().value()}", e)
-    } catch (Exception e) {
-      log.warn("Failed to cancel webhook ${stageData.cancelEndpoint}", e)
-    }
+    webhookService.cancelWebhook(stage)
   }
 
   private static Map<String, ExecutionStatus> createStatusMap(String successStatuses, String canceledStatuses, String terminalStatuses) {