kubernetes / test-infra

Test infrastructure for the Kubernetes project.
Apache License 2.0
3.82k stars 2.63k forks source link

[peribolos] github org invites aren't resent when they expire #20872

Closed dprotaso closed 3 months ago

dprotaso commented 3 years ago

What happened: A user added themselves to a peribolos config and an invite was sent. The invite expired and peribolos didn't resend the invite.

What you expected to happen: Peribolos checks the invite status and if expired resends the org invitation.

How to reproduce it (as minimally and precisely as possible):

Please provide links to example occurrences, if any:

Anything else we need to know?:

BenTheElder commented 3 years ago

/area prow/peribolos /area prow

BenTheElder commented 3 years ago

IMHO this is less of a bug and more of a feature request? I don't think it's that this feature is broken, I think it doesn't exist? I bet the owners would be happy to see this implemented. /sig testing /sig contributor-experience cc @mrbobbytables @cblecker

dprotaso commented 3 years ago

Feel free to reclassify it as needed

On Tue, Feb 16, 2021 at 17:56 Benjamin Elder notifications@github.com wrote:

IMHO this is less of a bug and more of a feature request? I don't think it's that this feature is broken, I think it doesn't exist? I bet the owners would be happy to see this implemented. /sig testing /sig contributor-experience cc @mrbobbytables https://github.com/mrbobbytables @cblecker https://github.com/cblecker

— You are receiving this because you authored the thread.

Reply to this email directly, view it on GitHub https://github.com/kubernetes/test-infra/issues/20872#issuecomment-780171130, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAERAQYDL7VESJAOMIYLVDS7LZZ3ANCNFSM4XXFFRAQ .

nikhita commented 3 years ago

cc @cblecker @mrbobbytables

nikhita commented 3 years ago

Marking this issue as help wanted. /help

If you are a new contributor who would like to pick this up, here's how you can go about this:

Background about peribolos:

Feature Request:

Background about the code:

https://github.com/kubernetes/test-infra/blob/e9bafa1cc6be4c8fae4046cef5aeccaa28b3a648/prow/cmd/peribolos/main.go#L798-L814

https://github.com/kubernetes/test-infra/blob/e9bafa1cc6be4c8fae4046cef5aeccaa28b3a648/prow/cmd/peribolos/main.go#L1284-L1285

Changes to make:

I haven't thought about this too much, so there is perhaps a better way to work with the GitHub API for this feature. I'd recommend taking a deeper look at how the GitHub API works with invitations to understand this - https://docs.github.com/en/rest/reference/orgs#members

This approach would probably lead to an extra API call. This should not be problematic because it'll only be triggered on a peribolos run, which isn't too often.

I'd also recommend trying out with a dummy org to verify that this feature works.

If you have questions or need help, please ask in the #prow channel on slack.

k8s-ci-robot commented 3 years ago

@nikhita: This request has been marked as needing help from a contributor.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed by commenting with the /remove-help command.

In response to [this](https://github.com/kubernetes/test-infra/issues/20872): >Marking this issue as help wanted. >/help > >If you are a new contributor who would like to pick this up, here's how you can go about this: > >**Background about peribolos**: > >- The tool [peribolos](https://github.com/kubernetes/test-infra/tree/master/prow/cmd/peribolos) is used to manage the GitHub org configuration. For example, it allows defining GitHub org members, teams, team members, etc is a yaml file. >- If new members are listed in the yaml file, peribolos automatically sends an invitation to these new members to join the GitHub org. >- Similarly, if members are removed from the yaml file, peribolos also removes them from the GitHub org. >- The configuration for the @kubernetes GitHub org is defined in https://github.com/kubernetes/org/blob/main/config/kubernetes/org.yaml. The team configuration is in individual SIG/WG directories in https://github.com/kubernetes/org/tree/main/config/kubernetes. >- The logs for peribolos runs are at https://testgrid.k8s.io/sig-contribex-org#ci-peribolos (ci-peribolos runs every 24 hours) and https://testgrid.k8s.io/sig-contribex-org#post-peribolos (post-peribolos runs after a PR is merged). These jobs are defined at https://github.com/kubernetes/test-infra/blob/master/config/jobs/kubernetes/test-infra/test-infra-trusted.yaml. > >**Feature Request**: > >- When a new member is invited to a GitHub org, they have seven days to accept the invite. If they don't accept the invite within seven days, the invite is "expired". >- To avoid expiring the invite, we need to add a new feature in peribolos such that it will look for expired invites and send a new invitation to the member. > >**Background about the code**: > >- The peribolos code lists all invitations in `orgInvitations` but doesn't check for expired/failed invitations. > >https://github.com/kubernetes/test-infra/blob/e9bafa1cc6be4c8fae4046cef5aeccaa28b3a648/prow/cmd/peribolos/main.go#L798-L814 > >- It _does_ check for pending invitations (if the member has not accepted the invite yet) and avoids sending a new invitation in this case. > >https://github.com/kubernetes/test-infra/blob/e9bafa1cc6be4c8fae4046cef5aeccaa28b3a648/prow/cmd/peribolos/main.go#L1284-L1285 > >**Changes to make**: > >I haven't thought about this too much, so there is perhaps a better way to work with the GitHub API for this feature. I'd recommend taking a deeper look at how the GitHub API works with invitations to understand this - https://docs.github.com/en/rest/reference/orgs#members > >This approach would probably lead to an extra API call. This should not be problematic because it'll only be triggered on a peribolos run, which isn't too often. > >I'd also recommend trying out with a dummy org to verify that this feature works. > >- Check if the ["list failed invitations"](https://docs.github.com/en/rest/reference/orgs#list-failed-organization-invitations) GitHub API call correctly looks for expired invitations. > >- You might need to add a new method in the prow github client for listing failed invitations `ListFailedOrgInvitations` similar to `ListOrgInvitations`: https://github.com/kubernetes/test-infra/blob/5d56b77ab5fefb5b614a4966b4f92b5fe18b0dd7/prow/github/client.go#L1335 > >- Use `ListFailedOrgInvitations` in the `inviteClient`: https://github.com/kubernetes/test-infra/blob/e9bafa1cc6be4c8fae4046cef5aeccaa28b3a648/prow/cmd/peribolos/main.go#L794-L796 > >- Add tests for [peribolos](https://github.com/kubernetes/test-infra/blob/master/prow/cmd/peribolos/main_test.go) and the [github client](https://github.com/kubernetes/test-infra/blob/master/prow/github/client_test.go). > > Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
daemon1024 commented 3 years ago

I would like to work on this 🤗😄

/assign

fejta-bot commented 3 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale

dprotaso commented 3 years ago

/lifecycle frozen

dprotaso commented 3 years ago

@daemon1024 do you still plan on working on this issue?

daemon1024 commented 3 years ago

Sorry for providing no updates. I tried to work on this but the 7 days wait for testing hampered the process.

I made some attempts at https://github.com/daemon1024/test-infra/commit/b57e191aed9b381617e0d82474dc8f9ead744adb. If this helps.

I will unassign myself from this since I don't have bandwidth to work on this right now. Apologies again.

dprotaso commented 3 years ago

I tried to work on this but the 7 days wait for testing hampered the process.

What's the 7 day wait for testing?

daemon1024 commented 3 years ago

Need to wait for an invite to expire. The time period for an invite to expire is 7 days.

bhumijgupta commented 3 years ago

I would like to pick this up /assign

bhumijgupta commented 3 years ago

From my observations, after 7 days, the failed invitations are not returned by /orgs/{org}/invitations endpoint and are successfully returned by /orgs/{org}/failed_invitations

MadhavJivrajani commented 3 years ago

@bhumijgupta are you still working on this?

BenTheElder commented 3 months ago

This is now at https://github.com/kubernetes-sigs/prow/tree/main/cmd/peribolos and we should track issues in that repo.

This issue has had no activity for years now, so I'm going to close it. If there is further interest please re-file in kubernetes-sigs/prow and link back to this issue for historical context.

For now we have options to work around this, and we have very limited bandwidth to work on features.