-
Notifications
You must be signed in to change notification settings - Fork 12
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
RFC 0045 - Github Trust Levels #45
base: main
Are you sure you want to change the base?
Conversation
I'd never thought of levels as "projects". Nor do I recall a "project" having multiple levels (or really being defined at all). Making sure we have a consistent model will be to discussion. The mental model I have (and brought to security/risk with me) is that "levels" are traditionally assigned to a branch, and specify both the permissions required to modify the code base and the pool of workers used for building (to provide isolation between the levels). In that model, "project" was not defined, and no inter-level services were provided to a branch. I think it's important to have the conceptual model agreed upon before we discuss "remapping levels". Since this is looking forward to gecko-on-git, I'd suggest a table explaining each concept, with a column for the "hg nomenclature" and the "git nomenclature". If we really are talking different concepts here, I'm going to strongly push for a term other than "level", as that term is already referenced in too many docs and tools to be redefined. I'll add some background to specific sections in the current draft as well. (P.S. there's another unfinished discussion about repurposing "level 2" in the TC/GitHub context to refer to environments that are not strictly level 3, but do produce shipable artifacts. E.g. how do we define the separate pool needed for ML training.) |
could explicitly fork each one and list them all out as a separate level 2 | ||
project (like we do now). But it would be much nicer if these could be | ||
replaced by branches on the main repo. Supporting this will require allowing | ||
different branches to have different levels. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI, the reason the mercurial repos uses separate repositories for some branches was not (entirely) about "poor branch semantics in hg". A primary reason was that there was no good method to enforce different levels of write access with the way the hg server operates. The same is true of git, and the only reason the branches could be hosted in the same repository as m-c et all is that neither git nor GitHub will be controlling who's allowed to write to what. Phabricator will enforce all the ACLs.
Regardless of how branches are stored, each branch has a "level" associated with it, and can only be "built" on machinery in a specific pool.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A primary reason was that there was no good method to enforce different levels of write access with the way the hg server operates. The same is true of git
But don't branch protections in Github provide this? You can block people with write access from landing directly to the branch. In the case of Gecko, I assume we'd set up a branch protection that blocks everyone except the Lando bot for branches that are traditionally L3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope -- branch protections are not a git feature, but a GitHub feature. My understanding is no one will be using the GitHub interface directly. But this is a tricky area, and plans may not yet be "firm enough" to really say
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, but branch protections aren't just for the Github interface, they apply to the Git server. So e.g, if you add a branch protection to block pushes on main
, then run git push origin main
the Git server will reject your push.
So while it's true they aren't a general Git feature, they are a feature of Github's Git servers :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This approach would need more discussion, as:
- it may lead to a more complex access model than desired for m-c
- why not use the gecko-projects approach
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since writing this I learned that using gecko-projects
is the plan, so this is a moot point.
But personally I'd prefer branches on the main repo (defaulting to level 1), as that seems more self-serve and easy to set up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have greater faith in humanity than I! 😉
Self-serve and shared "free" resources more often leads to recreating the tragedy of the commons in my experience. 😒
1. L1 is used for public (e.g untrusted) pull requests. | ||
2. L2 is used for trusted pull requests (e.g with `collaborator` or `public_restricted` policies), as well as | ||
pushes to unprotected or non-release branches. | ||
3. L3 is used for pushes to protected or release branches. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This, imo, is a very different use of the "level" system. I think this will be an interesting discussion that may lead to a re-definition of levels. 😉 Now would be the best time to do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mental model I have (and brought to security/risk with me) is that "levels" are traditionally assigned to a branch, and specify both the permissions required to modify the code base and the pool of workers used for building (to provide isolation between the levels).
The above is my definition of level as well. I'm interested to dig into why you think this proposal constitutes a change to that definition. (Though we shouldn't dig in too much, because this whole idea is moot if there isn't a good way to implement it :p)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
interesting -- probably a zoom chat -- maybe I'm looking at "target branch" to set level, and you're looking at "source branch". And/or I'm only considering m-c where phabricator is controlling access, and you're looking at "native GitHub" projects as well. (Both need coverage.)
Thanks for the comments, you are very observant :p Just for the record, I like to push work up somewhere visible early and the fact this PR is in the draft definitely means it's not in a final form and may even be scrapped. When working through a potential implementation, I already ran into some issue (see #46).. so I'm not even sure it's possible! |
I should also note that this RFC is coming out of an action item from a discussion we had that touched on Github + levels. The main goal for now is to get the idea into writing, I have no intention of actively championing this. |
Oh, I understand the context and totally appreciate the transparency! We're going to (finally) be reconciling the old hg world in the context of newer folks used to a GitHub only world. Besides getting the technical details right, there will be a huge communication effort. I'd go so far as to say that maybe we should scrap all new mentions of "L[1234]" and come up with new terms to avoid no-longer-valid assumptions based on the older terminology. For example, I think TC views the levels more as "security domains" which are more based on the intended usage of any artifacts. (L3=general release; L1=dev's use), as TC has never had control or over or knowledge of who-could-modify-the-repo. Also, we have many more roles that were shoe horned into the level model. (E.g. sheriffs have L4, but they never "create commits" in that role - they manipulate commits.) I don't want to expand scope, but I will note that there is discussion of a "rapid release" model on the services side. They also are re-assigning roles (and thus permissions) and thus causing us to rethink how we talk about who-can-do-what, as "dev" vs "sre" no longer cut it - it's more nuanced. |
I like this idea, levels are quite confusing even to seasoned Taskgraph veterans. |
Just to cross link, we discussed another use case for non-L1/L3 branches as part of the Firefox Translations RRA recently. The tl;dr is that we want on-push tasks for that repository to have the following qualities
I believe this is almost the same as the L2 definition in this proposal, except the branch in question is a protected branch (at least from a GitHub configuration standpoint). |
I think that even falls in the proposal:
|
It highlights a potential inconsistency though. As L3 is:
A branch can be both protected and a non-release branch, which technically meets the requirements of L2 and L3 as currently stated. (I agree that non-release probably takes precedence here though...) This makes me wonder if protected/unprotected shouldn't be part of this definition, or at the very least should be an implementation requirement for L3 branches rather than part of the definition. |
I made a mistake by opening this PR too early and now the conversation has fractured. I'm going to lock conversation here for now. Please add comments to the issue instead: I'll re-open comments here once it's time. |
Rendered