Skip to content

Commit

Permalink
Use a dedicated GH API call to get the e-mail address (#1951)
Browse files Browse the repository at this point in the history
Depending on the privacy settings, the `/user` endpoint might or might
not contain the primary e-mail even if the user makes an authenticated
call on behalf of themselves. Let's use the `/email` API endpoint
instead.

Fixes: #1950
  • Loading branch information
jhrozek authored Dec 18, 2023
1 parent 888f7cf commit fa5bee5
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 5 deletions.
26 changes: 25 additions & 1 deletion internal/engine/actions/remediate/pull_request/pull_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,11 @@ func (r *Remediator) runGit(
return fmt.Errorf("cannot get authenticated user: %w", err)
}

email, err := getPrimaryEmail(ctx, r.ghCli)
if err != nil {
return fmt.Errorf("cannot get primary email: %w", err)
}

logger.Debug().Str("branch", branchBaseName(title)).Msg("Checking out branch")
err = wt.Checkout(&git.CheckoutOptions{
Branch: plumbing.NewBranchReferenceName(branchBaseName(title)),
Expand All @@ -288,7 +293,7 @@ func (r *Remediator) runGit(
_, err = wt.Commit(title, &git.CommitOptions{
Author: &object.Signature{
Name: u.GetName(),
Email: u.GetEmail(),
Email: email,
When: time.Now(),
},
})
Expand Down Expand Up @@ -484,3 +489,22 @@ func prFromBranchAlreadyExists(

return len(openPrs) > 0, nil
}

func getPrimaryEmail(ctx context.Context, cli provifv1.GitHub) (string, error) {
emails, err := cli.ListEmails(ctx, &github.ListOptions{})
if err != nil {
return "", fmt.Errorf("cannot get email: %w", err)
}

fallback := ""
for _, email := range emails {
if fallback == "" {
fallback = email.GetEmail()
}
if email.GetPrimary() {
return email.GetEmail(), nil
}
}

return fallback, nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,13 @@ func happyPathMockSetup(mockGitHub *mock_ghclient.MockGitHub) {
Email: github.String("[email protected]"),
Login: github.String("stacklok-bot"),
}, nil)
mockGitHub.EXPECT().
ListEmails(gomock.Any(), gomock.Any()).Return([]*github.UserEmail{
{
Email: github.String("[email protected]"),
Primary: github.Bool(true),
},
}, nil)
mockGitHub.EXPECT().
GetToken().Return("token")
mockGitHub.EXPECT().
Expand Down Expand Up @@ -464,6 +471,14 @@ func TestPullRequestRemediate(t *testing.T) {
Email: github.String("[email protected]"),
Login: github.String("stacklok-bot"),
}, nil)
// likewise we need to update the branch with a valid e-mail
mockGitHub.EXPECT().
ListEmails(gomock.Any(), gomock.Any()).Return([]*github.UserEmail{
{
Email: github.String("[email protected]"),
Primary: github.Bool(true),
},
}, nil)
mockGitHub.EXPECT().
GetToken().Return("token")
// this is the last call we expect to make. It returns existing PRs from this branch, so we
Expand Down
9 changes: 9 additions & 0 deletions internal/providers/github/github_rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -614,3 +614,12 @@ func (c *RestClient) CreateComment(ctx context.Context, owner, repo string, numb
})
return err
}

// ListEmails lists all emails for the authenticated user.
func (c *RestClient) ListEmails(ctx context.Context, opts *github.ListOptions) ([]*github.UserEmail, error) {
emails, _, err := c.client.Users.ListEmails(ctx, opts)
if err != nil {
return nil, err
}
return emails, nil
}
15 changes: 15 additions & 0 deletions internal/providers/github/mock/github.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions pkg/api/openapi/minder/v1/minder.swagger.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/api/protobuf/go/minder/v1/minder.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pkg/providers/v1/providers.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ type GitHub interface {
CreatePullRequest(ctx context.Context, owner, repo, title, body, head, base string) (*github.PullRequest, error)
ListPullRequests(ctx context.Context, owner, repo string, opt *github.PullRequestListOptions) ([]*github.PullRequest, error)
CreateComment(ctx context.Context, owner, repo string, number int, comment string) error
ListEmails(ctx context.Context, opts *github.ListOptions) ([]*github.UserEmail, error)
}

// ParseAndValidate parses the given provider configuration and validates it.
Expand Down

0 comments on commit fa5bee5

Please sign in to comment.