Modernising ACLs for Gerrit 3.7
Hi, The changes to update copy-conditionals and submit-requirements have multiplied a bit, so I thought it might be good to have a bit of a written-down plan to get us ready for Gerrit 3.7. I think we should probably start with a manual update to All-Projects, as we don't really manage that in Ansible. The two reviews to update the bootstrap documentation should serve as review for the production change: https://review.opendev.org/c/opendev/system-config/+/876236 https://review.opendev.org/c/opendev/system-config/+/876237 I've consolidated those changes an actual diff to our production All-Projects/project.config shown in https://paste.opendev.org/show/bmy36m7TO2b4cpndO5gO/ I have that in a checkout, so if we're happy with that, I can manually push it (or will rework for comments). I think this gives us a good place to start and covers both the copy-conditions and submit-requirements changes for the top-level All-Projects. Updating the copy-conditions for the project ACL's is already reviewed in https://review.opendev.org/c/openstack/project-config/+/867931 I think I'll feel better about that having already done All-Projects and seen that working, but that just needs to be applied. Then we need to move onto submit-requirements for the project ACL's. Firstly https://review.opendev.org/c/openstack/project-config/+/875992/1 is just mechnical to handle the arguments in the normalize tool. The first change https://review.opendev.org/c/openstack/project-config/+/875804/3 updates labels using "function = NoOp" to also have a no-op submit-requirement. We know from testing that labels must still specify a function. Strictly, I don't think the s-r is necessary; but it does seem that we can use this to add a description explaining what the label is doing. I also think this future-proofs us against the removal of label functions (although it is currently unclear how future gerrit will deal with this). The second change is similar https://review.opendev.org/c/openstack/project-config/+/875993/2 except this is a large selection of Backport-Candidate labels using "function = NoBlock", which is alised to NoOp. For consistency, this is all changed to NoOp and, again, given a submit-requirement as well. Following that is https://review.opendev.org/c/openstack/project-config/+/875994/2 which is a mechanical change again for the normalize tool, and then the two final changes https://review.opendev.org/c/openstack/project-config/+/875995 https://review.opendev.org/c/openstack/project-config/+/875996 convert more complex labels that have blocking semantics submit-requirements. I think these just want careful review to ensure I've translated what they're doing correctly. At that point, I think we're fully migrated off the deprecated ACL options, clearing our path for a Gerrit 3.7 upgrade, general notes for which are kept at https://etherpad.opendev.org/p/gerrit-upgrade-3.7 -i
On Fri, Mar 03, 2023 at 02:30:40PM +1100, Ian Wienand wrote:
The changes to update copy-conditionals and submit-requirements have multiplied a bit, so I thought it might be good to have a bit of a written-down plan to get us ready for Gerrit 3.7.
Thanks for the feedback (particularly clarkb) so here's a small update just to get things straight The changes to All-Projects have fixed an issue with the copyConditional matching NO_CHANGE (which would allow updating commit messages, which we don't want). https://review.opendev.org/c/opendev/system-config/+/876236 https://review.opendev.org/c/opendev/system-config/+/876237 Those are incoporated into an updated patch for the production All-Projects at https://paste.opendev.org/show/brAj40R1mJbQZSXAXEQ5/ I have tested this on a held node https://104.130.253.50/c/x/test-project/+/3 it's going to look something like https://imgur.com/a/7aWTHix with the code-review, workflow and verfied as submit-requirements, with "trigger" requirements like "Review-Priority" listed separately. There was some discussion about when it was OK to push this. Personally I think probably when we are ready, but with us watching for a quick revert if required -- the idea is that for users it should be a no-op; so the rollback condition would be anything not working as it does before the update. I was thinking that because the long-term implications of "function = NoOp" being deprecated are unclear, we should use a submit-requirement for our no-op labels like Backport-Candidate, etc. Upon playing with this more, I think that's not a good idea. You can do this, with a submit-requirement that "is:true" so it always passes. However, it looks confusing in the UI. If you have a "-1 Not a candidate", then you get a "passing" submit-requirement for the Backport-Candidate with a -1. Its too confusing to deal with. It is unclear how this sort of thing will be handled into the future by gerrit. The "function" is deprecated, but if it will be completely removed we don't know; see https://groups.google.com/g/repo-discuss/c/tZwIp3Hx-wA/m/rJgmT_OsAQAJ Thus I've abandonded https://review.opendev.org/c/openstack/project-config/+/875804 and simlified to just convert everything to NoOp with https://review.opendev.org/c/openstack/project-config/+/875993 At least then we are only dealing with one type of function in our ACL's consistently ... The ACL's that *are* using blocking submit rules have had a couple of things pointed out by clarkb fixed, and are converted with https://review.opendev.org/c/openstack/project-config/+/875995 https://review.opendev.org/c/openstack/project-config/+/875996 I think this series of patches puts us in a good position for a 3.7 upgrade. -i
participants (1)
-
Ian Wienand