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

[bug] [investigate] DCO sign off on branch merges #682

Open
asraa opened this issue Feb 28, 2023 · 5 comments
Open

[bug] [investigate] DCO sign off on branch merges #682

asraa opened this issue Feb 28, 2023 · 5 comments
Labels
bug Something isn't working

Comments

@asraa
Copy link
Contributor

asraa commented Feb 28, 2023

Description

Once a ceremony is complete on a branch (e.g. ceremony/2023-02-22), a final automated workflow creates a PR that merges the ceremony branch to main.

This creates a PR with the ceremony commits, for example, see this PR: #676

Which is opened by the sigstore-bot from the workflow and contains the individual commits from the ceremony branch to merge into main.

After manually adding DCO to the Update Snapshot and Timestamp PR, we still find that the DCO action fails to validate the commits due to a mismatch on the sign-off committer name and email and the actual one in the sign-off.

Commit sha: [2acbcce](https://github.com/sigstore/root-signing/pull/676/commits/2acbcce13ca5768d538c7e39cc27dcc91b1c65fd), Author: Bob Callaway, Committer: GitHub; Expected "Bob Callaway [[email protected]](mailto:[email protected])", but got "Bob Callaway [[email protected]](mailto:[email protected])".
Commit sha: [9afb8e4](https://github.com/sigstore/root-signing/pull/676/commits/9afb8e44fb255da5445f25fc209cabe1e60c6879), Author: dlorenc, Committer: GitHub; Expected "dlorenc [[email protected]](mailto:[email protected])", but got "Dan Lorenc [[email protected]](mailto:[email protected])".
Commit sha: [eb13041](https://github.com/sigstore/root-signing/pull/676/commits/eb130410d2a894b076ee590d7cdffcac742f3e82), Author: Fredrik Skogman, Committer: GitHub; Expected "Fredrik Skogman [[email protected]](mailto:[email protected])", but got "Brian DeHamer [[email protected]](mailto:[email protected])".
Commit sha: [fda3a54](https://github.com/sigstore/root-signing/pull/676/commits/fda3a54e2c0f558c15969dd7fba87b0f864e4928), Author: Marina Moore, Committer: GitHub; Expected "Marina Moore [[email protected]](mailto:[email protected])", but got "Marina Moore [[email protected]](mailto:[email protected])".

It seems like the committer is the GH user of the person who merged the commit (or in the case of a maintainer, theirs?) and the actual person who signed off is the sign-off email used in the commit.

(1) I'm not sure why DCO is doing such a strict validation.
(2) I know we could squash and create a merge commit with sign-off, but then we lose the individual commits preserved
(3) We can "Set DCO to pass" manually on these and merge.
(4) It feels like there must be a better way for DCO to detect this.

@kommendorkapten

Version

@asraa asraa added the bug Something isn't working label Feb 28, 2023
@kommendorkapten
Copy link
Member

When merging a PR, we can see this:
Screenshot 2023-02-28 at 19 49 22

The message clearly says: "This commit will be authored by [email protected]", I wonder if we update the merge commit message to be singned off by that email, if it will work as expected when the DCO test is done?

Also, would it be ok to change the signoff message?

@asraa
Copy link
Contributor Author

asraa commented Feb 28, 2023

Seems like a weird process, but I think it would solve the manual merges.

For auto-review bot, I can switch to using https://cli.github.com/manual/gh_pr_merge, which seems to have an --author-email. I can maybe detect the bot ID?

@asraa
Copy link
Contributor Author

asraa commented Feb 28, 2023

At least for now we can try with the manual merge + use the noreply!

@kommendorkapten
Copy link
Member

I think I know about the issue!
See the picture below.
For all activities that happens via GitHub's web UI, such as merges, the primary email configured in GitHub is used. This does not have to be the same email address that is used when signing off on a commit. If these are not in sync, the signoff will not match for the merge commit.

Screenshot 2023-03-01 at 15 31 29

@kommendorkapten
Copy link
Member

So my understanding is that if the primary email address in GitHub is the same as the email address used during signoff, the merge commit will pass the DCO test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants