types icon indicating copy to clipboard operation
types copied to clipboard

feat(rulesets): add release to ruleset event types

Open Gradsta opened this issue 2 years ago • 3 comments

This is one in a series of PRs that enables Vela users to define pipeline steps for GitHub release events. This feature was originally requested in #518 in the community repository.

While a release requires a tag, the opposite cannot be said--tags are independent events and can exist without the presence of a release. As such, it can be useful to distinguish between the different event types. For my particular use case, I'd like to use a Gradle plugin that uses tags to automatically version my repository. In this setup, merging a PR to the main branch would serve as the trigger to increment the version (i.e., tagging the applicable commit) and deploy to stage. Creating a release and selecting the newly-created tag would serve as the trigger to deploy to production. This requires that the release event type be supported by Vela.

Related PRs

Note: The checks in the PRs listed above are failing as they cannot find the types introduced in this PR. I'm assuming that will be resolved if/when this PR is merged. I'm going to leave those PRs in draft status for the time being.

Gradsta avatar Nov 04 '22 15:11 Gradsta

Codecov Report

Merging #272 (faef798) into main (9661c89) will increase coverage by 0.01%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #272      +/-   ##
==========================================
+ Coverage   96.93%   96.95%   +0.01%     
==========================================
  Files          57       57              
  Lines        6301     6331      +30     
==========================================
+ Hits         6108     6138      +30     
  Misses        143      143              
  Partials       50       50              
Impacted Files Coverage Δ
database/repo.go 100.00% <100.00%> (ø)
library/build.go 100.00% <100.00%> (ø)
library/repo.go 100.00% <100.00%> (ø)
library/secret.go 100.00% <100.00%> (ø)
webhook.go 100.00% <100.00%> (ø)
yaml/ruleset.go 100.00% <100.00%> (ø)

codecov[bot] avatar Nov 04 '22 15:11 codecov[bot]

Just a general comment here: I'm not sure we want to listen to every single action related to the release event. If we do, then we need to add the capability for the user to control that on their end, i.e. some sort of AllowReleaseActions enum or something. I ran into this issue when I tried to implement various pull_request actions. Vela would run entire pipelines when someone changed the title of a PR because it was an edited action.

I like the change, but I think in order to implement this effectively, either A) determine which actions are essential and only "listen" to those (perhaps only released?), or B) create some sort of system for filtering the noise of all the other actions for the end user. I admittedly have been kicking option B down the road in regard to PRs, comments, etc.

ecrupper avatar Nov 04 '22 18:11 ecrupper

@ecrupper I see what you're saying. It seems obvious now that the different action types could cause issues. When a release is created, as an example, multiple release webhook events—all with different actions—could be sent at once depending on the release options selected. Presumably, this would kick off three separate builds which would be undesired

I like the change, but I think in order to implement this effectively, either A) determine which actions are essential and only "listen" to those (perhaps only released?), or B) create some sort of system for filtering the noise of all the other actions for the end user. I admittedly have been kicking option B down the road in regard to PRs, comments, etc.

Option A makes sense to me and would likely cover most people's use cases. I'll modify the PR. I do like the idea of giving the user control a la option B, though that's probably a more long-term solution.

Gradsta avatar Nov 04 '22 20:11 Gradsta

Hey @Gradsta, sorry for not moving on this sooner. First, thanks for making the edits per Easton's callouts. We are still interested in this change.

Unfortunately, we aren't able to merge this at the moment due to an an existing race condition that is touched on here: https://github.com/go-vela/community/issues/213. This issue still exists and we see it occasionally in cases where folks kick off multiple events at the same time.

I think it's fair to say that a majority of release events will be accompanied by tag/push events, so the likelihood of that causing an issue in the form of one or the other event not getting picked up is real.

It's something we hope to address sooner than later - until then, we will hold on this. Just wanted to provide an update.

wass3rw3rk avatar Apr 26 '23 15:04 wass3rw3rk

going to move this into draft for now to avoid confusion on this being ready.

wass3rw3rk avatar Jun 21 '23 16:06 wass3rw3rk

Apparently we only followed up in the UI PR. Apologies for not updating all PRs. Please see UI PR for our updates and decision to close. Happy to re-engage if you want to discuss.

https://github.com/go-vela/ui/pull/596#issuecomment-1802092643

chrispdriscoll avatar Apr 10 '24 14:04 chrispdriscoll