Never change version precision of actions chosen by users#5891
Conversation
377bc6f to
65e48c5
Compare
|
Ok, this discussion actually happened here: #4953 (comment). @mctofu was considering both the approach in this PR and the one used currently, but in the end it was decided after discussing with @brrygrdn to go with the current one. However, I think the situation described here was not considered at the time, namely, author not intentionally changing the version policy, just taking some time to create the new major version tag, enough for Dependabot to run in the middle and create an unexpected update. So I'd like to reevaluate our approach, I think it's more consistent and simpler this way, and less subject to surprise users. I think changing existing behavior should require strong agreement, so if not everyone agrees here I'm happy to close and possibly think of something else. |
65e48c5 to
fc5233b
Compare
fc5233b to
bacb7ca
Compare
mctofu
left a comment
There was a problem hiding this comment.
👍 Thanks for fixing this! The precision change when there's a delay in publishing the version was definitely not expected or intended.
brrygrdn
left a comment
There was a problem hiding this comment.
TL;DR - I think this behaviour is preferable 👍🏻
Reading over the original conversation, I think my bias towards creating any PR at all was based on the idea that we (currently) aren't prompted to try to create a PR again when a new version of the Action is released, so if we miss the opportunity to create a PR when the alert goes out we likely won't create one without user intervention (i.e. they push the workflow / dependabot config / press a button on the Alert )
Looking at it with fresh eyes, I think this behaviour is probably better as:
- The alert that the repo is vulnerable will still exist so if the user wants to change precision to get the fix asap, they can
- I also overlooked the convention in actions to 'move' the major tag to the new minor version so this was likely causing a lot more unhelpful PRs than I considered at the time.
|
Thanks @brrygrdn! I had actually not considered this in the context of security updates, just regular version updates! It's true that if the new version fixes a vulnerability, the new flow will be slower to fix the vulnerability since it requires either manual intervention of the user, or waiting for the action's author to move the major tag. But it seems like a rare case, because it first needs the action's release workflow to allow this interim time where version is released but tag not yet moved, and also because security fixes usually come as patch level releases, so it would not be subject to this in general. I will ship this now! |
I'm changing some intentionally added specs, but these specs were added precisely to fix "precision being changed" issues. I think these cases were added to have a nice fallback behavior when tags with the same precision are not available. However, in practice: * Users still don't want this. * The edge cases only happen temporary when actions authors have released a version but not yet done the dance of moving the major version tag to point to the (new) latest patch or minor level version of the action. So I think it's best to simplify this code to filter out versions not fully matching existing precision, and then choose the latest over what's left.
bacb7ca to
b57b558
Compare
Never changing the existing version precision is our default behavior, however there's one edge case where Dependabot may actually propose a PR changing the precision.
This is only in the interim between the time where a new major version of the action has been released, but the action's author has not yet created a new major version tag pointing to the new release.
If Dependabot happens to run between the above two events, it will create a PR changing the precision from vX to vX.Y.Z, just because it's unable to find any versions with the same precision.
I think this in never what users expect, and it introduces inconsistent behavior in Dependabot, not dependent of the state of the manifest files, but on external circumstances which is confusing. On top of that, the issue will be automatically "fixed" next time Dependabot runs, once the actions author has created the proper major version tag.
I'm aware that I'm changing expectations of existing specs here, but I don't think this is intended after all. These specs where initially added here, precisely to fix "precision being changed" issues. The few specs that I'm tweaking I think may have been added to have a nice fallback behavior when tags with the same precision are not available. However, in practice users still don't want this since the valid edge cases where this happens are only temporary.
So I think it's best to simplify this code to filter out versions not fully matching existing precision, and then choose the latest over what's left.
Fixes #5887.
Closes #2304.