Skip to content

Commit

Permalink
fix(webhook): Capture response headers in the stage output (spinnaker…
Browse files Browse the repository at this point in the history
…#3976)

* fix(webhook): Capture response headers into the stage output

* fix(test): Increasing the timeout for verifying mock operations
  • Loading branch information
srekapalli authored Oct 23, 2020
1 parent a3d5193 commit d73fc8b
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ class KubernetesPreconfiguredJobSpec : JUnit5Minutests {
.contains("\"ref\":\"/pipelines")
}

verify(timeout = 1000) { katoRestService.requestOperations(any(), "kubernetes", match { it.toString().contains("alias=preconfiguredJob") }) }
verify(timeout = 1500) { katoRestService.requestOperations(any(), "kubernetes", match { it.toString().contains("alias=preconfiguredJob") }) }
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ class MonitorWebhookTask implements OverridableTimeoutRetryableTask {
stage.execution.id,
stage.id
)
} catch (HttpStatusCodeException e) {
} catch (HttpStatusCodeException e) {
def statusCode = e.getStatusCode()
def statusValue = statusCode.value()

Expand All @@ -104,6 +104,9 @@ class MonitorWebhookTask implements OverridableTimeoutRetryableTask {
log.error(errorMessage, e)
Map<String, ?> outputs = originalResponse
outputs.webhook.monitor << [error: errorMessage]
if (e.getResponseHeaders() != null && !e.getResponseHeaders().isEmpty()) {
outputs.webhook.monitor << [headers: e.getResponseHeaders().toSingleValueMap()]
}
return TaskResult.builder(ExecutionStatus.TERMINAL).context(outputs).build()
} catch (Exception e) {
if (e instanceof UnknownHostException || e.cause instanceof UnknownHostException) {
Expand All @@ -130,6 +133,10 @@ class MonitorWebhookTask implements OverridableTimeoutRetryableTask {
statusCodeValue: response.statusCode.value()
]

if (!response.getHeaders().isEmpty()) {
responsePayload.webhook.monitor << [headers: response.getHeaders().toSingleValueMap()]
}

if (Strings.isNullOrEmpty(stageData.statusJsonPath)) {
return TaskResult.builder(ExecutionStatus.SUCCEEDED).context(responsePayload).build()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ TaskResult processReceivedHttpStatusException(HttpStatusCodeException e) {
Map<String, Object> webHookOutput = (Map<String, Object>) stageOutput.get("webhook");
webHookOutput.put("statusCode", e.getStatusCode());
webHookOutput.put("statusCodeValue", e.getStatusCode().value());
if (e.getResponseHeaders() != null) {
webHookOutput.put("headers", e.getResponseHeaders().toSingleValueMap());
}
if (!StringUtils.isEmpty(e.getResponseBodyAsString())) {
webHookOutput.put("body", processResponseBodyAsJson(e.getResponseBodyAsString()));
}
Expand Down Expand Up @@ -165,6 +168,9 @@ TaskResult processResponse(ResponseEntity response) {
if (response.getBody() != null) {
webHookOutput.put("body", response.getBody());
}
if (!response.getHeaders().isEmpty()) {
webHookOutput.put("headers", response.getHeaders().toSingleValueMap());
}
HttpStatus status = response.getStatusCode();

if (status.is2xxSuccessful() || status.is3xxRedirection()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ class CreateWebhookTaskSpec extends Specification {
result.status == ExecutionStatus.SUCCEEDED
result.context as Map == [
webhook: [
headers:[Location: "https://my-service.io/api/status/123"],
statusCode: HttpStatus.CREATED,
statusCodeValue: HttpStatus.CREATED.value(),
body: [success: true],
Expand Down Expand Up @@ -544,6 +545,7 @@ class CreateWebhookTaskSpec extends Specification {
if (statusUrlResolution == WebhookProperties.StatusUrlResolution.locationHeader) {
headers.add(HttpHeaders.LOCATION, responseStatusCheckUrl)
}
headers.add(HttpHeaders.CONTENT_TYPE, "application/json")

createWebhookTask.webhookService = Stub(WebhookService) {
callWebhook(stage) >> new ResponseEntity<Map>(['statusCheckUrl': responseStatusCheckUrl] as Map, headers, HttpStatus.OK)
Expand All @@ -555,6 +557,7 @@ class CreateWebhookTaskSpec extends Specification {
then:
result.status == ExecutionStatus.SUCCEEDED
stage.context.statusEndpoint == resultstatusCheckUrl
result.context.webhook.headers.size() >= 1

where:
webHookUrl | statusUrlResolution | responseStatusCheckUrl || resultstatusCheckUrl
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import com.netflix.spinnaker.orca.pipeline.model.PipelineExecutionImpl
import com.netflix.spinnaker.orca.pipeline.model.StageExecutionImpl
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.HttpStatus
import org.springframework.http.ResponseEntity
import org.springframework.web.client.HttpServerErrorException
Expand Down Expand Up @@ -170,8 +171,10 @@ class MonitorWebhookTaskSpec extends Specification {

def "should do a get request to the defined statusEndpoint"() {
setup:
def headers = new HttpHeaders()
headers.add(HttpHeaders.CONTENT_TYPE, "application/json")
monitorWebhookTask.webhookService = Mock(WebhookService) {
1 * getWebhookStatus(_) >> new ResponseEntity<Map>([status:"RUNNING"], HttpStatus.OK)
1 * getWebhookStatus(_) >> new ResponseEntity<Map>([status:"RUNNING"], headers, HttpStatus.OK)
}
def stage = new StageExecutionImpl(pipeline, "webhook", [
statusEndpoint: 'https://my-service.io/api/status/123',
Expand All @@ -188,7 +191,7 @@ class MonitorWebhookTaskSpec extends Specification {

then:
result.status == ExecutionStatus.RUNNING
result.context.webhook.monitor == [body: [status: "RUNNING"], statusCode: HttpStatus.OK, statusCodeValue: HttpStatus.OK.value()]
result.context.webhook.monitor == [headers: ["Content-Type": "application/json"], body: [status: "RUNNING"], statusCode: HttpStatus.OK, statusCodeValue: HttpStatus.OK.value()]
}

def "should find correct element using statusJsonPath parameter"() {
Expand Down

0 comments on commit d73fc8b

Please sign in to comment.