-
Notifications
You must be signed in to change notification settings - Fork 970
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
Support for webhook integration with Pluggable SCM materials #8170
Comments
Seems to relate to #5328 |
@chadlwilson The webhook notification API endpoints are analogous to the notification API endpoints, with the only difference being the matching you mentioned. The webhook notification code (around here and here for instance) is responsible for parsing the webhook payload, getting the parts related to the URL and then make guesses about the possible ways that the same URL could have been configured in GoCD, so that the material subsystem in GoCD can try and match all materials against those possible URLs and trigger pipelines related to those materials. As you said, it does look like this will need a change to the SCM plugin API. The problem is the payload. If you look at the SCM plugin notification API endpoint, the body of the request needs to have the With this case that you bring up, the SCM plugin will probably need to be given all the configurations (across the whole server) which match its type and it will be asked to make a determination about which materials match. In an ideal world, if there was something in between (say a small server) which could translate the webhook payload to a valid SCM notification call, that would work. That can be a workaround for now, but the change you're mentioning looks like it will need a plugin API change, as you said. |
Thanks for the reply @arvindsv ! I spent some time looking at the codebase at the weekend and trying to figure out how I might be able to introduce such support, but couldn't quite find the right place because
That's as far as I got before going down the thought process of "hmm, maybe as soon as the On the assumption that supporting this would be desirable, would you be able to help me understand what I might be getting into in submitting a draft PR for this by letting me know would I have to introduce a new Scm API version
|
@chadlwilson Of course! Happy to help. I'll think about this a bit more. I might not be the person with the most up-to-date information about the codebase. :) So, we might need to get some help from the others. I'll reply once I've given this more thought. |
@chadlwilson I thought about it. I'm going to write down some things here, which might help us have a discussion. Assumption: We need to do something which is generic, which cannot be specific to one kind of plugin. Thoughts:
cc: @maheshp @kritika-singh3 -- since my knowledge of the codebase is a little dated. |
My thoughts,
|
That seems neither here not there, right? Now the plugin API is tightly tied to something like "GitHub" and I can see that having to change all the time. If that's the case, we might as well send the whole JSON to the plugin and ask it to parse. Maybe the notification endpoint can be something like: At least that way plugin capabilities remains generic. Problem with this: If a GoCD server has both a normal git material and a pluggable SCM material, then you will need to choose which URL to use to notify, and that decides which pipelines are triggered, which is a significant downside. |
After my conversation with @arvindsv we considered the below approach to implement this feature,
{
"supported_webhooks": [
{
"provider": "GitHub",
"events": ["pull"]
}
]
}
|
This issue has been automatically marked as stale because it has not had activity in the last 90 days. |
I think would be good to keep this open, as a possible feature at some point. Unfortunately the complexity of implementing it is/was a bit beyond my available spare time to digest & invest/digest. Is there a way we can label issues as features so they don't get assumed to be stale/non-reproducible bugs by the bot? |
This has been marked to be ignored by the stalebot, but I don't see much point in keeping something open forever. However, this one will remain open for now. :) |
(long post) Carried out a quick spike for this with the following changes: On GoCD server side
Note:
On Plugin sideTested the changes with gocd git path material plugin.
Questions to be discussed/decided upon
Thoughts/Suggestions/Feedback welcomed! |
|
@kritika-singh3 on second thoughts, if we can use the Can we explore this option? |
Can we think of a solution with the best of both options: Flow on GoCD end:
Thoughts/Reasoning involved: @arvindsv's point that supporting multiple plugins might not be a normal scenario is right. Hence, instead of taking a list of ids in the same query param, preferred would be to allow the same param to be specified multiple times. @maheshp 's point that we should be able to support pluggable SCMs without having to update the plugin is a valid point. Hence, supporting But there can be certain scenarios for which a plugin decision maybe required. For e.g. triggering when a tag with name Hope I was clear enought. LMK if you think this is an overkill solution. |
My opinion: I think it would be better to keep it simple, and extend only if necessary later (meaning, multiple params). |
Maybe we should just start with adding support for material updates based on |
Ohk. Sounds good. |
Done as part of #8646 |
Rather late to the party - but thank you for all the work on this @maheshp , @kritika-singh3, @arvindsv and @marques-work . Some appreciation for your work over at TWChennai/gocd-git-path-material-plugin#27 (comment) (although I haven't had the opportunity to try it out myself yet, sadly!) |
Issue Type
Summary
Currently, GoCD supports incoming webhooks from a number of sources (GitHub, GitLab, Bitbucket Server + Cloud) all relating to Git materials, however my understanding is that it uses some heuristics to match the material and basically hard-codes the material search to filter for the inbuilt Git material.
There are a few SCM plugins in the ecosystem that also support Git, however I don't think there is a way for the webhooks to select these Git materials (and I'm not convinced there would be a way without an enhancement to the
scm
API that would allowIn particular, the Git Path Material plugin could benefit from this; as polling dozens of materials is expensive, and as noted here, can end up breaching API request limits.
Any other info
Even if such support existed, I'm not familiar enough with how material triggers work to know whether this would work with the git-path-material-plugin, since the plugin basically filters the
git log
when answering thelatest-revision
request.Opening this for discussion/ideas about whether there might be a way to make this work.
The text was updated successfully, but these errors were encountered: