New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Webhook stage, integrate against anything #1512

Closed
amanya opened this Issue Mar 27, 2017 · 13 comments

Comments

Projects
None yet
4 participants
@amanya

amanya commented Mar 27, 2017

Create a new stage for calling an external webhook / service, that will be useful for having a flexible way of interacting with technologies that have not yet been integrated with Spinnaker, for example running AWS lambdas or running Marathon deployments.

Description

We'll add a new pipeline stage type that will call a user-defined endpoint, passing to it a json document with fields customized in the stage's ui using pipeline expressions.

When configuring the stage, we will have a checkbox to "wait for results". If it is not checked, the stage will be succeeded if the webhook's status code is 2xx or failed otherwise.

If "wait for results" is checked, Spinnaker will expect a 2xx return code along with a callback url that can be returned in the "Location" header or in the body of the response. This url will be used to get the status of the task we triggered, it will return a JSON like:

{
    "status": "[NOT_STARTED|RUNNING|SUCCEEDED|TERMINAL|...]",
    "info": "Some information about the process"
}

This information will be used to update the status of the execution and give feedback to the user.

To give this the maximum flexibility, there will be a configurable (JSONPath notation perhaps) way of specifying what response element to look at, and what values map to success/failure.

As a next step, it will be nice to have config-driven definitions of external services that would then be presented as choices when using this new stage.

Implementation details

We decided that the best fit for putting this is Orca, because it has similar tasks as the one we want to implement, for example in the bake stage it triggers a task with parameters, waits for it to complete and update the execution accordingly. Also the jenkins and run script stages are good examples of this.

There has been some discussion about this feature before opening this issue in this google document.

@amanya

This comment has been minimized.

Show comment
Hide comment
@amanya

amanya Mar 27, 2017

This is a screenshot of the inital works in the stage's configuration view:

image

amanya commented Mar 27, 2017

This is a screenshot of the inital works in the stage's configuration view:

image

@robzienert

This comment has been minimized.

Show comment
Hide comment
@robzienert

robzienert Mar 28, 2017

Member

Looks like great progress; I know it's still in the works, but would like to see the textbox for "Status URL path" to be hidden, and slotted below its label when visible.

Member

robzienert commented Mar 28, 2017

Looks like great progress; I know it's still in the works, but would like to see the textbox for "Status URL path" to be hidden, and slotted below its label when visible.

jervi added a commit to jervi/orca that referenced this issue Mar 30, 2017

jervi added a commit to jervi/orca that referenced this issue Mar 30, 2017

jervi added a commit to jervi/orca that referenced this issue Mar 30, 2017

jervi added a commit to jervi/orca that referenced this issue Mar 30, 2017

@amanya

This comment has been minimized.

Show comment
Hide comment
@amanya

amanya Mar 31, 2017

TODO list for next iterations:

  • Define new custom stages based on the webhook stage by configuring them in orca.yml. Those will be selectable in the ui as if they were regular stages.
  • "try it out" button that will show the results of the webhook call next to it so that the json path values could be tested. Similar to to how expresssions evaluate to results.
  • Beautify the payload in the execution view. Limit the number of lines to be shown with an option to show the rest (clamp)
  • Store a log of all calls to the status endpoint and show a link to view it in the same way as the bake log.
  • Pass custom headers to the webhook call

amanya commented Mar 31, 2017

TODO list for next iterations:

  • Define new custom stages based on the webhook stage by configuring them in orca.yml. Those will be selectable in the ui as if they were regular stages.
  • "try it out" button that will show the results of the webhook call next to it so that the json path values could be tested. Similar to to how expresssions evaluate to results.
  • Beautify the payload in the execution view. Limit the number of lines to be shown with an option to show the rest (clamp)
  • Store a log of all calls to the status endpoint and show a link to view it in the same way as the bake log.
  • Pass custom headers to the webhook call

jervi added a commit to jervi/orca that referenced this issue Mar 31, 2017

jervi added a commit to jervi/orca that referenced this issue Mar 31, 2017

amanya added a commit to amanya/deck that referenced this issue Mar 31, 2017

feat(stages/webhook): Webhook stage spinnaker/spinnaker#1512
Create a new stage for calling an external webhook / service, that will
be useful for having a flexible way of interacting with technologies
that have not yet been integrated with Spinnaker, for example running
AWS lambdas or running Marathon deployments.

amanya added a commit to amanya/deck that referenced this issue Mar 31, 2017

feat(stages/webhook): Webhook stage spinnaker/spinnaker#1512
Create a new stage for calling an external webhook / service, that will
be useful for having a flexible way of interacting with technologies
that have not yet been integrated with Spinnaker, for example running
AWS lambdas or running Marathon deployments.

amanya added a commit to amanya/deck that referenced this issue Mar 31, 2017

feat(stages/webhook): Webhook stage spinnaker/spinnaker#1512
Create a new stage for calling an external webhook / service, that will
be useful for having a flexible way of interacting with technologies
that have not yet been integrated with Spinnaker, for example running
AWS lambdas or running Marathon deployments.

amanya added a commit to amanya/deck that referenced this issue Mar 31, 2017

feat(stages/webhook): Webhook stage spinnaker/spinnaker#1512
Create a new stage for calling an external webhook / service, that will
be useful for having a flexible way of interacting with technologies
that have not yet been integrated with Spinnaker, for example running
AWS lambdas or running Marathon deployments.

jervi added a commit to jervi/orca that referenced this issue Mar 31, 2017

feat(stages/webhook): Webhoook stage
This PR is first implementation of the webhook stage discussed in issue spinnaker/spinnaker#1512.
Corresponds with Deck PR spinnaker/deck#3447
@robzienert

This comment has been minimized.

Show comment
Hide comment
@robzienert

robzienert Apr 1, 2017

Member
Member

robzienert commented Apr 1, 2017

jervi added a commit to jervi/orca that referenced this issue Apr 4, 2017

feat(stages/webhook): Webhoook stage
This PR is first implementation of the webhook stage discussed in issue spinnaker/spinnaker#1512.
Corresponds with Deck PR spinnaker/deck#3447

jervi added a commit to jervi/orca that referenced this issue Apr 5, 2017

feat(stages/webhook): Webhoook stage
This PR is first implementation of the webhook stage discussed in issue spinnaker/spinnaker#1512.
Corresponds with Deck PR spinnaker/deck#3447

jervi added a commit to jervi/orca that referenced this issue Apr 6, 2017

feat(stages/webhook): Webhoook stage
This PR is first implementation of the webhook stage discussed in issue spinnaker/spinnaker#1512.
Corresponds with Deck PR spinnaker/deck#3447

robzienert added a commit to spinnaker/orca that referenced this issue Apr 6, 2017

feat(stages/webhook): Webhoook stage (#1257)
This PR is first implementation of the webhook stage discussed in issue spinnaker/spinnaker#1512.
Corresponds with Deck PR spinnaker/deck#3447

amanya added a commit to amanya/deck that referenced this issue Apr 7, 2017

feat(stages/webhook): Webhook stage spinnaker/spinnaker#1512
Create a new stage for calling an external webhook / service, that will
be useful for having a flexible way of interacting with technologies
that have not yet been integrated with Spinnaker, for example running
AWS lambdas or running Marathon deployments.

amanya added a commit to amanya/deck that referenced this issue Apr 7, 2017

feat(stages/webhook): Webhook stage spinnaker/spinnaker#1512
Create a new stage for calling an external webhook / service, that will
be useful for having a flexible way of interacting with technologies
that have not yet been integrated with Spinnaker, for example running
AWS lambdas or running Marathon deployments.
@Nagarajj

This comment has been minimized.

Show comment
Hide comment
@Nagarajj

Nagarajj Apr 12, 2017

This would be helpful, would there be a way to pass headers to the web service url. It seems to assume no headers to be present.

@service
class WebhookService {

@Autowired
RestTemplate restTemplate

ResponseEntity exchange(HttpMethod httpMethod, String url, Object payload) {
HttpEntity payloadEntity = new HttpEntity<>(payload)
return restTemplate.exchange(url, httpMethod, payloadEntity, Object)
}

ResponseEntity getStatus(String url) {
return restTemplate.getForEntity(url, Object)
}
}

Nagarajj commented Apr 12, 2017

This would be helpful, would there be a way to pass headers to the web service url. It seems to assume no headers to be present.

@service
class WebhookService {

@Autowired
RestTemplate restTemplate

ResponseEntity exchange(HttpMethod httpMethod, String url, Object payload) {
HttpEntity payloadEntity = new HttpEntity<>(payload)
return restTemplate.exchange(url, httpMethod, payloadEntity, Object)
}

ResponseEntity getStatus(String url) {
return restTemplate.getForEntity(url, Object)
}
}

@amanya

This comment has been minimized.

Show comment
Hide comment
@amanya

amanya Apr 13, 2017

@Nagarajj this is a feature other people requested, we'll probably implement it soon :)

amanya commented Apr 13, 2017

@Nagarajj this is a feature other people requested, we'll probably implement it soon :)

nzthiago added a commit to Microsoft/deck that referenced this issue Apr 17, 2017

feat(stages/webhook): Webhook stage spinnaker/spinnaker#1512
Create a new stage for calling an external webhook / service, that will
be useful for having a flexible way of interacting with technologies
that have not yet been integrated with Spinnaker, for example running
AWS lambdas or running Marathon deployments.

Nagarajj pushed a commit to Nagarajj/deck that referenced this issue Apr 27, 2017

Janardhana , Nagaraj Janardhana , Nagaraj
feat(stages/webhook): Add Headers to Webhook stage spinnaker/spinnake…
…r#1512

Add headers to webservice calls made as part of web hook stage
Also fix issue with "statusUrlResolution" not being initialized by default

Nagarajj pushed a commit to Nagarajj/orca that referenced this issue Apr 27, 2017

Janardhana , Nagaraj Janardhana , Nagaraj
feat(stages/webhook): Add Header support to Webhook stage spinnaker/s…
…pinnaker#1512

Adds headers support to the web hook stage core
Also, adds support for collection with single element resulting from jsonPath expression returning list in response.
Corresponds to deck PR spinnaker/deck#3596
@Nagarajj

This comment has been minimized.

Show comment
Hide comment
@Nagarajj

Nagarajj Apr 27, 2017

@amanya I have made changes to addHeaders to WebService requests. Have submitted a PR, please see if that makes sense.

  1. Headers are added to both the webService calls. Headers could be things such as "Authorization" headers for authentication (required by both "execute" and "getStatus" requests).
  2. Added support for JSONPath returning list response such as selecting last element in array "$.[-1:].status" or find running element in response "$.[?(@.status == "RUNNING")].status"
  3. Fixed a minor existing UI bug where default "statusUrlResolution" of "getMethod" was not set as it was not initialized.

Below is the screenshot of change
webhook-headers

Nagarajj commented Apr 27, 2017

@amanya I have made changes to addHeaders to WebService requests. Have submitted a PR, please see if that makes sense.

  1. Headers are added to both the webService calls. Headers could be things such as "Authorization" headers for authentication (required by both "execute" and "getStatus" requests).
  2. Added support for JSONPath returning list response such as selecting last element in array "$.[-1:].status" or find running element in response "$.[?(@.status == "RUNNING")].status"
  3. Fixed a minor existing UI bug where default "statusUrlResolution" of "getMethod" was not set as it was not initialized.

Below is the screenshot of change
webhook-headers

@amanya

This comment has been minimized.

Show comment
Hide comment
@amanya

amanya Apr 27, 2017

@Nagarajj Ups, we already did something very similar: spinnaker/deck#3590

Would you wait for our PR to be merged and then add your changes in a new one? Or do you think it's worth updating ours? Your code seems much simpler but I can't look at in in depth right now.

amanya commented Apr 27, 2017

@Nagarajj Ups, we already did something very similar: spinnaker/deck#3590

Would you wait for our PR to be merged and then add your changes in a new one? Or do you think it's worth updating ours? Your code seems much simpler but I can't look at in in depth right now.

@Nagarajj

This comment has been minimized.

Show comment
Hide comment
@Nagarajj

Nagarajj Apr 27, 2017

@amanya sure, will wait for your PR to be merged. One Q ? will the headers support both "execute" and "getStatus" webservice calls. Would be helpful if it supports both. Rest of the issues are simpler and will raise a seperate PR for those. Thanks.

Nagarajj commented Apr 27, 2017

@amanya sure, will wait for your PR to be merged. One Q ? will the headers support both "execute" and "getStatus" webservice calls. Would be helpful if it supports both. Rest of the issues are simpler and will raise a seperate PR for those. Thanks.

@amanya

This comment has been minimized.

Show comment
Hide comment
@amanya

amanya Apr 28, 2017

@Nagarajj No, we only implemented for the execute call, feel free to add it for the getStatus as well, good idea!

amanya commented Apr 28, 2017

@Nagarajj No, we only implemented for the execute call, feel free to add it for the getStatus as well, good idea!

@Nagarajj

This comment has been minimized.

Show comment
Hide comment
@Nagarajj

Nagarajj Apr 29, 2017

@amanya, should we share the headers across both the calls or provide ability to define separate headers for the getStatus call.

The use case we are trying to solve for, ability to define "Authorization" header which is required for both execute and getStatus call. Not sure of an use case where a separate set of headers would be required across execute and getStatus.

Let know your thoughts and i will get going on thins.

Nagarajj commented Apr 29, 2017

@amanya, should we share the headers across both the calls or provide ability to define separate headers for the getStatus call.

The use case we are trying to solve for, ability to define "Authorization" header which is required for both execute and getStatus call. Not sure of an use case where a separate set of headers would be required across execute and getStatus.

Let know your thoughts and i will get going on thins.

@erran

This comment has been minimized.

Show comment
Hide comment
@erran

erran Jul 18, 2017

Is this issue still on going after the Webhook stage and predefined webhook stages after spinnaker/orca#1329?

erran commented Jul 18, 2017

Is this issue still on going after the Webhook stage and predefined webhook stages after spinnaker/orca#1329?

@amanya

This comment has been minimized.

Show comment
Hide comment
@amanya

amanya Jul 21, 2017

@erran no, I think we can close it, thanks for the heads up!

amanya commented Jul 21, 2017

@erran no, I think we can close it, thanks for the heads up!

@amanya amanya closed this Jul 21, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment