require_self_approval: true should prevent suggesting PR author for approval

[BenTheElder]

[This is re-filed from https://github.com/kubernetes/test-infra/issues/29827]

What happened:

TLDR in a repo with implicit_self_approval off, prow will sometimes suggest the author as the approver, it should suggest someone else.

https://kubernetes.slack.com/archives/CDECRSC5U/p1669738130214399

What you expected to happen:

suggest non-pr-author for finding an approver

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

.... you'll need implicit_self_approval disabled, yourself in OWNERS, and then file a PR until this happens.

Please provide links to example occurrences, if any:

lots of sample links in the slack thread linked above

Anything else we need to know?:

/sig testing

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle rotten
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

[petr-muller]

Approver logic is tricky but filtering out author name from the list of options does not seem too scary, so let's try marking it as a good first issue. I hope I'm not throwing someone under the bus 😬

[Bharadwajshivam28]

Hey @BenTheElder @petr-muller can I look into this issue? also if yes then can you suggest me some steps to get started on this?

[petr-muller]

@Bharadwajshivam28 Sure, feel free to give it a shot!

Here are some breadcrumbs. The problem is that Prow sometimes asks ...please ask for approval from AUTHOR... even when require_self_approval is set to true, indicating that authors with approval permissions do not get an automated approved label, and it is likely desirable for other approver to approve the PR (although the author can still approve it themselves, but if that behavior is desirable then the repo should disable require_self_approval.

The message seems to be built here from a Approvers struct (partially from its AssignedCCs member, partially from SuggestedCCs), you can follow its call chain to figure out how the Approver struct that ends us passed here is built and prevent the author from being added here if require_self_approval: true (unless they are the only approver, I guess).

prow/pkg/plugins/approve/approvers/owners.go

Lines 685 to 691 in 24e7653

	**Once this PR has been reviewed and has the lgtm label**, please ask for approval from {{range $index, $cc := .ap.AssignedCCs}}{{if $index}}, {{end}}{{printf "[%s](https://github.com/%s)" $cc $cc}}{{end}} and additionally assign {{range $index, $cc := .ap.SuggestedCCs}}{{if $index}}, {{end}}{{printf "[%s](https://github.com/%s)" $cc $cc}}{{end}} for approval. For more information see [the Kubernetes Code Review Process]({{ .prProcessLink }}).
	{{- else -}}
	**Once this PR has been reviewed and has the lgtm label**, please assign {{range $index, $cc := .ap.SuggestedCCs}}{{if $index}}, {{end}}{{printf "[%s](https://github.com/%s)" $cc $cc}}{{end}} for approval. For more information see [the Kubernetes Code Review Process]({{ .prProcessLink }}).
	{{- end}}
	{{- else -}}
	{{- if len .ap.AssignedCCs -}}
	**Once this PR has been reviewed and has the lgtm label**, please ask for approval from {{range $index, $cc := .ap.AssignedCCs}}{{if $index}}, {{end}}{{printf "[%s](https://github.com/%s)" $cc $cc}}{{end}}. For more information see [the Kubernetes Code Review Process]({{ .prProcessLink }}).

I retitled this issue to mention the require_self_approval: true config instead, which is the current name of that config option. The original implicit_self_approve was renamed with inverted defaults to current require_self_approval in 2019.