Skip to content
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

Create a Cloud Account access policy for ci-infra #300

Merged
merged 5 commits into from
Jan 7, 2025

Conversation

zxiiro
Copy link
Collaborator

@zxiiro zxiiro commented Nov 19, 2024

The goal of this Pull Request is to create an access policy that the PyTorch CI Infra team can enforce and grant access to community contributors whom require access to the PyTorch Foundation's AWS Account.

Closes: #298

The goal of this Pull Request is to create an access policy that the
PyTorch CI Infra team can enforce and grant access to community
contributors whom require access to the PyTorch Foundation's AWS
Account.

Closes: #298
Signed-off-by: Thanh Ha <[email protected]>
@zxiiro zxiiro self-assigned this Nov 19, 2024
Comment on lines 21 to 23
* Permissions will be reviewed and extended on a quarterly cadence by existing
pytorch-infra-administrators with advisory of regular attendees of the
PyTorch Infrastructure weekly meeting or the PyTorch TAC.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normally I would expect this role to be appointed by a governing body such as the TAC or TSC. I think PyTorch's TAC is currently not setup for this kind of governance. I think the regular attendees of the current weekly Monday CI Sync meeting is likely the folks whom should make decisions on this.

cloud-account-access.md Show resolved Hide resolved
cloud-account-access.md Outdated Show resolved Hide resolved
Comment on lines 51 to 52
* Permissions automatically expire after 6 months if not renewed.
* Permissions will be reviewed an extended on a quarterly cadence by existing
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you want to propose a 6 month or a quarterly cadence?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a strong opinion on the cadence however my intent here is to have a bit of overlap. So in this example 6 month expiry but 3 month extension. This gives us 3 months overlap to extend the permissions before it expires.

My thought here is if someone goes on vacation or we forget to review the extension PR for a few weeks. The users don't lose access immediately. 3 months to review a PR though is likely way too long. Maybe something like quarterly expiry with 1 month overlap to review extensions would make more sense.

I'd like to hear what opinions others have here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha. What mechanism might be used to review the permissions? Are you thinking of a separate meeting held every 3 months?

It makes sense during such meetings to review any permissions expected to expire within X weeks of the next meeting (to account for the next meeting getting pushed out a bit for any reason). No strong opinion on the meeting cadence as long as this bit is accounted for.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ZainRizvi I think a meeting likely isn't necessary most of the time if it's just reviewing to extend permissions. So I was imagining a GHA that would automatically create a PR against the project and pytorch-admins would review the PR when it comes in async. A meeting could optionally be scheduled if admins need further discussion as necessary.

This all assumes we can create that GHA that will automatically extend permissions in a PR.

cloud-account-access.md Show resolved Hide resolved
@jwagantall
Copy link
Collaborator

@zxiiro Overall looks good to me with the comments provided by other reviewers addressed.
Could you please click on "Resolve conversation" for the completed actions?

@zxiiro zxiiro marked this pull request as ready for review December 9, 2024 20:47
@jeanschmidt
Copy link
Contributor

I don't have any other particular comment to add, I believe we're in sync on what we've been discussing in recent meetings

@zxiiro zxiiro merged commit 57b807d into main Jan 7, 2025
8 checks passed
@zxiiro zxiiro deleted the zxiiro/cloud-account-access branch January 7, 2025 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable security login best practices for AWS login
4 participants