Sub teams for build?

[joyeecheung]

Moving the discussion in https://github.com/orgs/nodejs/teams/collaborators/discussions/30 to public.

Per the discussions in nodejs/reliability#2 I created nodejs/build-files and nodejs/build-infra teams so that people can stop pinging nodejs/build for build file changes.

The difference is: if someone from the build team would need SSH access or account access or sending PRs to the build repo to fix this, ping nodejs/build-infra, if someone would need to open a PR to the build files in core to fix this, ping nodejs/build-files

The build files refer to Makefile, vcbuild.bat, configure and .gypi/.gyp files in the Node core.
I added people from the infra admins section of the build README to build-infra. For the build-files team, feel free to add yourself or send a request to join.
I will open two PRs later:

• Use ncu-team to sync the infra admins section of the build README with members from build-infra
• Change @nodejs/build to @nodejs/build-files in .github/CODEOWNERS of core

I have talked with @maclover7 and @refack and we think a build-infra team for the infra admins probably does not make sense either.

So is there a better idea about how to restructure the sub teams to make the pings more effective? I think the build-files team still make sense since pinging nodejs/build for reviews of Makefile/configure etc. is kind of weird.

[joyeecheung]

For reference, in the README we have

• Build WG Members (sync'ed with nodejs/build): have root access to the test CI machines
• Infra Admins (I've sync'ed it with nodejs/build-infra): have access to infra machines, service provider accounts, certificates, etc.
• Release Admins (no corresponding team): have access to release machines
• GitHub Bot Admins (sync'ed with nodejs/github-bot, not part of the nodejs/build)

For access secrets, we also have

• test (not in sync with nodejs/build)
• infra (in sync with the Infra Admins section)
• release (not in sync with the Release Admins section)
• github-bot (not in sync with the github-bot team)

The owner of some public keys there have not been active for quite some time now. This could be a bigger problem than pings not being delivered properly since there's access to machines and credentials involved.

On the other hand, we tend to have more people who actually have access than people in the team/README, because people may not want to be pinged by they still need access to work.

In practice, infra and release admins should probably be the same group of people. Release admins is not really the release team as far as I understand, the actual nodejs/release team is split into nodejs/LTS,nodejs/citgm and nodejs/releasers team

access.md also mentions nodejs/jenkins-admins who has admin access to jenkins, but it's not a subteam of nodejs/build.

[richardlau]

Perhaps release admins was intended to be those that have admin access to the release CI (as opposed to the regular CI) (unless that group in practice is the same as nodejs/jenkins-admins)?

[richardlau]

To repeat my opinion from nodejs/reliability#2 (comment) I don't think the build-files team belongs as a subteam of the build WG (but is still worth having under core). The only time we should ping a build WG (sub) team on a build file related PR is if the change also requires changes to a Jenkins job or changes to a machine configuration.

[refack]

I agree there is a difference, but there is still some overlap between knowledge of the build-files and the CI cluster.
For example the test cluster uses some make targets (run-ci, test-ci-js, build-ci...) and vcbuild.bat
While breaking these will fail a test CI job, it might break a release build at a later time. (Ref: nodejs/node#13900 (review))

[joyeecheung]

@refack I think for those targets we just need to ping the build as well? Pinging two teams at a time is easy anyway ;)

[refack]

The owner of some public keys there have not been active for quite some time now. This could be a bigger problem than pings not being delivered properly since there's access to machines and credentials involved.

AFAIK @rvagg and @joaocgreis audit these lists on occasion, and prune inactive members.

On the other hand, we tend to have more people who actually have access than people in the team/README, because people may not want to be pinged by they still need access to work.

Theoretically the WG has a process for tracking this, so IMHO these lists should be synched (or at least actual gpg access should be a subset of teams)

In practice, infra and release admins should probably be the same group of people.

👍
I believe this is a "deprecated" feature. For as long as I have been a member I don't know of a difference.

---

There was an idea to set up a KeyBox server to manage all this, but again time...

[refack]

refack I think for those targets we just need to ping the build as well? Pinging two teams at a time is easy anyway ;)

Probably, I just think it's good for build WG members to join nodejs/build-files... Also as much as we can document this knowledge, and make the job configs transparent, it should help.

[maclover7]

Action item for meeting: to approve/not approve the following changes to GitHub team layout. Will not result in any changes to permissions, just uses new GitHub feature to make org structure easier to understand.

Current:

nodejs/build
nodejs/jenkins-admins
nodejs/jenkins-release-admins

Proposed (using Github's "child" team UI):

nodejs/build
  --> nodejs/jenkins-admins
    --> nodejs/jenkins-release-admins

[joyeecheung]

Another item: merge "Release Admins" into "Infra Admins" (they are basically the same group of people at the moment) as well as merging the release into infra secrets folder.

There are currently no teams for the release admins, and the "Release Admins" is not related to nodejs/ release or nodejs/releasers AFAICT. People who have access to the two secret folders are the same, the only difference is in the README.

There is nodejs/build-infra which is in sync with the Infra Admins. We could keep the team in sync in the README using ncu-team as well.

[mhdawson]

I think Release Admins may have only recently been created so that we could add people to Infra Admins without adding them to Release Admins. We may just not have done that yet. @rvagg is that correct?

[joyeecheung]

Heads up: I am going to put the jenkins-admins under build and jenkins-release-admins under jenkins-admins since there were no objections in the last build meeting.

[mhdawson]

@rvagg if you can please comment on the refactoring of the admins.

[rvagg]

Yes, sorry, I'm currently quite preoccupied with work, sorry I haven't been keeping on top of this stuff!

Yes, secrets/release and secrets/infra are owned by the same core group, they're just critical bits that we need to keep tighter controls of like signing certs. I don't mind the folders being merged if that's required to make things more obvious.

Jenkins admins is only new and I have no objections no properly organising their inheritance in GitHub, I tried that originally but failed cause I don't fully understand this teams inheritance stuff so am happy for others to take control of that!

The rest all sounds good. I'm also not opposed to refreshing membership of the more core groups / access controls. We have bandwidth problems right not with those of us who have access to those resources and I don't want to see that become a problem. Most recently we added @gibfahn but he's since had his availability decreased too, due to a job change. Probably something to discuss and finalise via email but suggestions here would be welcome.

[joaocgreis]

I'm not sure merging secrets/release and secrets/infra would be good. Even if the people who currently have access are the same, I believe the secrets/release folder can be used to give access to the people building releases, if anyone becomes active at that level.

build-files sounds good to have. For changes in vcbuild, please mention platform-windows because that file is Windows specific. For changes that might affect releases, if there is any doubt that it'll work well, a mention to build sounds reasonable. Those of us with release access can easily start a test build.

[github-actions]

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.