periobolos keep going if user not found - error code 422

[jlewi]

A follow on to #14321. As mentioned in that periobolos seems to error out updating the org if a GitHub id doesn't exist. I would like it to keep going.

In that issue it was mentioned this was fixed in newer images. I just tried the latest image
gcr.io/k8s-prow/peribolos@sha256:cb771295078f0a7353f375d39242ecafadcfc46650efb8a5f0adfa10b3f8c050

and it looks like I'm still hitting the issue. Here's the error

{"component":"peribolos","error":"status code 422 not one of [200], body: {\"message\":\"The request could not be processed.\",\"documentation_url\":\"https://developer.github.com/v3/orgs/members/#add-or-update-organization-membership\"}","file":"prow/cmd/peribolos/main.go:484","func":"main.configureOrgMembers.func1","level":"warning","msg":"UpdateOrgMembership(kubeflow, ajayalfred, false) failed","time":"2020-05-21T04:13:58Z"}

Full logs are attached.
logs.txt

It looks like peribolos tries to recover from a not found error

test-infra/prow/cmd/peribolos/main.go

Lines 443 to 447 in e1cbc3c

	if github.IsNotFound(err) {
	// this could be caused by someone removing their account
	// or a typo in the configuration but should not crash the sync
	err = nil
	}

It looks like github.IsNotFound is checking for 404s.

test-infra/prow/github/client.go

Line 613 in 2905fd2

if strings.Contains(errorMsg, "status code 404") {

But it looks like the error code in this case is 422

/cc @clarketm

[nikhita]

This might not have been the intended behavior but I actually like having peribolos fail because it tells us if a member has updated their github username. If we don't get a signal for failure, some malicious actor could claim ownership of the old username and do bad things in our repos.

If we end up enabling this behavior, can we please gate it behind a flag and not make it enabled by default?

/area prow/peribolos
/cc @cblecker @mrbobbytables @spiffxp

[kurokochin]

/assign

[fejta-bot]

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-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

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

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[fejta-bot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

[k8s-ci-robot]

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[chizhg]

/reopen