Pipeline webhook trigger "Payload Constraints" does not support mutli-level json payload .

[JamieChe]

Issue Summary:

Pipeline webhook trigger "Payload Constraints" does not support multi-level json payload .

Cloud Provider(s):

kubernetes

Environment:

spinnaker 1.9.5

Feature Area:

Pipeline automated triggers

Description:

I want to trigger pipeline with webook with payload constraints . But when the payload json is multi-level, the constraint matching will only match the first level json key and value.
I checked the code https://github.com/spinnaker/echo/blob/master/echo-pipelinetriggers/src/main/java/com/netflix/spinnaker/echo/pipelinetriggers/artifacts/ArtifactMatcher.java, found it doesn't check json payload recursively.
Moreover, the value match is a subsequence match using match(input).find(), but not a full string match. Not sure whether it's a expected behavior in payload constraints feature.

Steps to Reproduce:

Set test webhook payload to trigger pipeline, and hope pipeline be triggered when project=boobar.
{
"foo": {
"project": "boobar",
"user": "dummy user"
},
"title": "dummy title"
}

When setting payload constraints foo.project=boobar or project=boobar, pipeline will not be triggered.
But setting foo="project=boobar"(second level key-value as string) or foo=boobar (subsequence match), pipeline is triggered.

Additional Details:

[lwander]

(unfortunately) this is by design. We are happy to accept a proposal that addresses this! Something like jsonpath to build constraints is an idea...

[JamieChe]

Thanks for the information. But I've tested with jsonpath, like key=$.foo.project, value=boobar, still doesn't work.
Anything wrong with the format?

[lwander]

Sorry if I wasn't clear: this isn't supported, but it could be implemented with something like jsonpath. If you or anyone in the community has a proposal, we can help you/them implement it.

[spinnakerbot]

This issue hasn't been updated in 45 days, so we are tagging it as 'stale'. If you want to remove this label, comment:

@spinnakerbot remove-label stale

[spinnakerbot]

This issue is tagged as 'stale' and hasn't been updated in 45 days, so we are tagging it as 'to-be-closed'. It will be closed in 45 days unless updates are made. If you want to remove this label, comment:

@spinnakerbot remove-label to-be-closed

[Alegrowin]

@spinnakerbot remove-label to-be-closed

[spinnakerbot]

This issue is tagged as 'stale' and hasn't been updated in 45 days, so we are tagging it as 'to-be-closed'. It will be closed in 45 days unless updates are made. If you want to remove this label, comment:

@spinnakerbot remove-label to-be-closed

[Alegrowin]

@spinnakerbot remove-label to-be-closed

[spinnakerbot]

This issue is tagged as 'stale' and hasn't been updated in 45 days, so we are tagging it as 'to-be-closed'. It will be closed in 45 days unless updates are made. If you want to remove this label, comment:

@spinnakerbot remove-label to-be-closed

[acaproulx]

@spinnakerbot remove-label to-be-closed

[spinnakerbot]

This issue is tagged as 'stale' and hasn't been updated in 45 days, so we are tagging it as 'to-be-closed'. It will be closed in 45 days unless updates are made. If you want to remove this label, comment:

@spinnakerbot remove-label to-be-closed

[Alegrowin]

@spinnakerbot remove-label to-be-closed

[Sluggerman]

We need this too, otherwise it is quite limiting feature. Anyone working on this?

[shengyu]

I think Payload Constraints would be more useful if it supports SpEL.

[jfillo]

I just ran into this issue. I tried adding a payload constraint of parameters.foo=bar and the constraint was not recognized even though parameter foo with value bar is passed when triggering my pipeline

[emjburns]

We're happy to review an RFC and offer guidance if someone would like to work on this! @jfillo, any interest in contributing support for multi-level payload matching?

[TonyMcTony]

Implementing this would allow for Bitbucket webhooks to actually be functional, as Bitbucket does not allow any customization of the payload.

[bgreeleyquibi]

I was working on something similar and noticed that trying to read a nested property in a JSON object is also not supported for pub sub as well.

I see you mentioned that this is by design, @lwander. Just out of curiosity, what drove the decision?

[ademariag]

Encountered the same issue. Would be great to have it to support jsonpath

[TonyMcTony]

@StevenPG @TonyMcTony did you find any workaround to integrate Bitbucket?

Yes, we ended up with using webhooks from Jira automation (https://community.atlassian.com/t5/Jira-articles/Automation-for-Jira-Send-web-request-using-Jira-REST-API/ba-p/1443828), there you can define the webhook payload yourself using smart values from Jira: https://support.atlassian.com/jira-software-cloud/docs/smart-values-development/

Alternatively, if your usage scenario is that you want the pipeline to trigger on changes only to specific files in the git repository, you can go with something like this: https://spinnaker.io/guides/user/pipeline/triggers-with-artifactsrewrite/github/#configure-the-github-artifact (check the note there).

[shubhamc183]

Thanks a lot, @TonyMcTony!

Will check this out.

[pwaterz]

@edgarulg did you implement the jsonpath option? It looks like it's committed, and in the latest release, but no idea how to use it.

[edgarulg]

@pwaterz yes I implemented the jsonpath support. Here is the PR with the implementation: spinnaker/echo#1078

Basically the Payload Constraints supports a JsonPath expression in the key & the value can be a single String or Regex. Here is an example:

Having this payload from a webhook

{
"key1": "example",
"key2": {
   "key3": "test",
   "key4":"another-text"
   }
}

Your Payload Constrains could looks like the following:
Key | Value
key1 : example
$.key2.key4 : *text

so the webhook will be allowed to trigger because it matched the first level key value and the jsonpath expression.

It has backward compatibility, so first it will try to match the Payload Constrains as key value in the first level of the payload (this is the current behavior), when it cannot be matched it will take the key and use it as a JsonPath expression to allow multi level search in payload.

It supports any value that can be cast to an String and List of Strings only. You can find more use cases here: https://github.com/spinnaker/echo/blob/3b8057c752816050e29a1977d817e90c6d4d6388/echo-pipelinetriggers/src/test/groovy/com/netflix/spinnaker/echo/pipelinetriggers/artifacts/ArtifactMatcherSpec.groovy#L55

I need to open another PR in deck to add this details on the Spinnaker UI but happy to answer any question that you have.

[pwaterz]

I'm running 1.25.0 which I think includes this, and it doesn't seem to work. Ex:

{
  "source": {
    "gitSource": {
      "url": "https://github.com/namespace/repo.git",
      "revision": "1234"
    }
  }
}

$.source.gitSource.url

[edgarulg]

@pwaterz sorry for the late response, Could you provide me the entire payload constrains and the payload that you are using to reproduce the issue? I will test it and write a fix for that.

Thanks!

[pwaterz]

{
  "source": {
    "gitSource": {
      "url": "https://github.com/namespace/repo.git",
      "revision": "1234"
    }
  }
}

This is a stripped down version of the payload. It's a pubsub message from google cloud build.

key: $.source.gitSource.url
value: https://github.com/namespace/repo.git

Also note I've tried matching top level key/values in the payloads and those aren't matching either.

[edgarulg]

@pwaterz are you using a pubsub trigger? for now, the jsonpath support feature is present only for Webhook trigger. I may include this feature for the pubsub trigger as well. I will double check that the functionality works as expect.

[pwaterz]

Ahh yeah I am using it for pubsub. It would be awesome to have it for pubsub! I've worked around it for now by transforming the current pubsub message to a flat json object, but it would great to not have to do that.

[spinnakerbot]

This issue hasn't been updated in 45 days, so we are tagging it as 'stale'. If you want to remove this label, comment:

@spinnakerbot remove-label stale

[spinnakerbot]

This issue is tagged as 'stale' and hasn't been updated in 45 days, so we are tagging it as 'to-be-closed'. It will be closed in 45 days unless updates are made. If you want to remove this label, comment:

@spinnakerbot remove-label to-be-closed

[Alegrowin]

@spinnakerbot remove-label to-be-closed

[spinnakerbot]

This issue is tagged as 'stale' and hasn't been updated in 45 days, so we are tagging it as 'to-be-closed'. It will be closed in 45 days unless updates are made. If you want to remove this label, comment:

@spinnakerbot remove-label to-be-closed

[papanito]

@spinnakerbot remove-label to-be-closed

[spinnakerbot]

"to-be-closed" has not been applied, and cannot be removed.

[kcasas]

@pwaterz You can workaround this by utilizing the regex on the value field. So instead of

key: $.source.gitSource.url
value: https://github.com/namespace/repo.git

use

key: source
value: .*https:\/\/github\.com\/namespace\/repo\.git.*

[spinnakerbot]

This issue is tagged as 'stale' and hasn't been updated in 45 days, so we are tagging it as 'to-be-closed'. It will be closed in 45 days unless updates are made. If you want to remove this label, comment:

@spinnakerbot remove-label to-be-closed

[papanito]

@spinnakerbot remove-label to-be-closed

[spinnakerbot]

This issue is tagged as 'stale' and hasn't been updated in 45 days, so we are tagging it as 'to-be-closed'. It will be closed in 45 days unless updates are made. If you want to remove this label, comment:

@spinnakerbot remove-label to-be-closed

[spinnakerbot]

This issue is tagged as 'to-be-closed' and hasn't been updated in 45 days, so we are closing it. You can always reopen this issue if needed.