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

multi-gitter run does not sign commits #261

Open
jetersen opened this issue Jul 18, 2022 · 14 comments · May be fixed by #492
Open

multi-gitter run does not sign commits #261

jetersen opened this issue Jul 18, 2022 · 14 comments · May be fixed by #492
Labels
enhancement New feature or request

Comments

@jetersen
Copy link
Contributor

jetersen commented Jul 18, 2022

Describe the bug
Should be able to understand signing commits

To Reproduce
Steps to reproduce the behavior:

  1. Run multi-gitter run -xxx
  2. git verify-commit HEAD

Expected behavior
Commits are signed

Additional context
Changing git-type to cmd does not work

git-type: cmd
@jetersen jetersen added the bug Something isn't working label Jul 18, 2022
@lemeurherve
Copy link

AFAIU a SignKey should be added after the Author here, but I didn't found how to declare an openpgp.Entity

image

@jetersen jetersen changed the title multi-gitter run using go dep does not sign commits multi-gitter run does not sign commits Jul 18, 2022
@lindell
Copy link
Owner

lindell commented Jul 19, 2022

@jetersen When running with --git-type=cmd, did you have git configured to always sign commits? (git config --global commit.gpgsign true).

If implemented, I would be happy to merge a PR that adds support for signing natively in both git modes. But that should be behind a flag since it's not the default git behavior. (behind the -S flag).

@lindell lindell added enhancement New feature or request and removed bug Something isn't working labels Jul 19, 2022
@jetersen
Copy link
Contributor Author

@lindell
Copy link
Owner

lindell commented Jul 19, 2022

I think I've tried that too.

Can you please confirm that this is actually the case?

When I try to run it I get this:
image

I don't have gpg set up on in this terminal, so the error is expected, but it tries to sign the commit.

@jetersen
Copy link
Contributor Author

I can give it a shot :)

@lemeurherve
Copy link

I think I've tried that too.

Can you please confirm that this is actually the case?

When I try to run it I get this: image

If I remember correctly it's the same error I've got. I'll try again later today (hopefully) and let you know too.

@jetersen
Copy link
Contributor Author

jetersen commented Jul 19, 2022

@lemeurherve it worked for me: jenkinsci/jenkins-infra-test-plugin#40

Question is do you have a password on your key?
Have you enabled gpg agent to store password temporarily.

Perhaps you need to configure for interactivity so you could enter your password? Not sure if git would detect the tty being available.

One trick you can do regarding gpg commit signing is create a subkey without password. If you want to keep to master key secure.
I tried that but decided against it after some issues with other applications.

@jetersen
Copy link
Contributor Author

While testing though I found conditional logic bug, for some reason it says the feature branch existed even though it was a new fork. This was using git-type: cmd had to change conflict strategy to replace

} else if featureBranchExist && r.ConflictStrategy == ConflictStrategySkip {

concurrent: 1
git-type: cmd
conflict-strategy: replace
fork: true
fork-owner: jetersen-cloud
log-file: '-'
log-format: text
log-level: debug
repo:
  - jenkinsci/jenkins-infra-test-plugin
branch: test-multi-gitter
commit-message: Testing Multi Gitter
pr-title: Testing multi-gitter
pr-body: |
  Testing out the multi-gitter with gpg signing.

  Sweet 💣

@jetersen
Copy link
Contributor Author

jetersen commented Jul 19, 2022

Although it would be nice to support -S sign and -s signoff as flags in git-type: cmd

@lindell
Copy link
Owner

lindell commented Jul 19, 2022

@jetersen In regards to the newly created fork and the branch already existing. Could the branch name have existed in the original repo, before the fork? In that case the branch would have been copied to the fork, and it would exist.

@lindell
Copy link
Owner

lindell commented Jul 19, 2022

Although it would be nice to support -S sign and -s signoff as flags in git-type: cmd

I've been thinking of creating a new flag/setting called something like "--extra-commit-args" that would only work with git-type: cmd. That way it would be possible to use these, and other features before it's implemented for real in multi-gitter, or things that might not ever be implemented.

@jetersen
Copy link
Contributor Author

--extra-commit-args makes sense :)

@lemeurherve
Copy link

I think I've tried that too.

Can you please confirm that this is actually the case?
When I try to run it I get this: image

If I remember correctly it's the same error I've got. I'll try again later today (hopefully) and let you know too.

I've made another test today, and it's working as intended, I must have missed the passphrase prompt the first time, sorry for the noise.

NB: +1 for the --extra-commit-args!

@Sam13
Copy link

Sam13 commented May 15, 2024

@jetersen When running with --git-type=cmd, did you have git configured to always sign commits? (git config --global commit.gpgsign true).

@lindell I had to remove author-email and author-name from my multi-gitter configuration file to get signed GIT commits although correctly configured via git config.
Probably this section of the GIT configuration is not used when environment variables are set for author and commiter?

cmd.Env = append(cmd.Env,

@ChrisStatham ChrisStatham linked a pull request Jul 17, 2024 that will close this issue
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants