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

Tolerate default commit signing with SSH in tests #1124

Merged

Conversation

jtnord
Copy link
Member

@jtnord jtnord commented Apr 8, 2024

Tolerate default commit signing with SSH in tests

work around https://bugs.eclipse.org/bugs/show_bug.cgi?id=581483

If the current git setup has SSH based code signing enabled (gpg.format=ssh)[1] then the tests fail (even though not signing) as jgit fails to parse the config with an error like:

java.lang.IllegalArgumentException: Invalid value: gpg.format=ssh
        at org.eclipse.jgit.lib.DefaultTypedConfigGetter.getEnum(DefaultTypedConfigGetter.java:103)
        at org.eclipse.jgit.lib.Config.getEnum(Config.java:454)
        at org.eclipse.jgit.lib.GpgConfig.<init>(GpgConfig.java:86)
        at org.eclipse.jgit.api.CommitCommand.processOptions(CommitCommand.java:662)
        at org.eclipse.jgit.api.CommitCommand.call(CommitCommand.java:189)
        at org.jenkinsci.plugins.gitclient.JGitAPIImpl.commit(JGitAPIImpl.java:487)
        at org.jenkinsci.plugins.gitclient.GitClientMaintenanceTest.commitFile(GitClientMaintenanceTest.java:230)
        at org.jenkinsci.plugins.gitclient.GitClientMaintenanceTest.commitOneFile(GitClientMaintenanceTest.java:224)
        at org.jenkinsci.plugins.gitclient.GitClientMaintenanceTest.commitSeveralFiles(GitClientMaintenanceTest.java:218)
        at org.jenkinsci.plugins.gitclient.GitClientMaintenanceTest.test_gc_maintenance(GitClientMaintenanceTest.java:333)
        at java.base/java.lang.reflect.Method.invoke(Method.java:566)

[1] https://git-scm.com/docs/git-config#Documentation/git-config.txt-gpgformat

This has only been tested on my local windows development environment. before this change, tests failed as above.

After this change the only failing test is

 GitClientTest.test_git_ssh_executable_found_on_windows:3393 » Runtime ssh executable not found. The git plugin only supports official git client https://git-scm.com/download/win

which is a lie, but somewhat expected as this installation of the official git client did not install ssh or any mysys tools.

Checklist

  • Unit tests pass locally with my changes

Types of changes

What types of changes does your code introduce?

  • Test fix (non-breaking change which updates dependencies or improves infrastructure)

@jtnord jtnord requested a review from a team as a code owner April 8, 2024 15:46
@github-actions github-actions bot added the tests Automated test addition or improvement label Apr 8, 2024
@jtnord jtnord force-pushed the be-lotterant-of-ssh-commit-signing branch from 8f792ad to b964feb Compare April 8, 2024 19:34
If the current git setup has SSH based code signing enabled
(gpg.format=ssh)[1] then the tests fail (even though not signing) as
jgit fails to parse the config with an error like:

```
java.lang.IllegalArgumentException: Invalid value: gpg.format=ssh
        at org.eclipse.jgit.lib.DefaultTypedConfigGetter.getEnum(DefaultTypedConfigGetter.java:103)
        at org.eclipse.jgit.lib.Config.getEnum(Config.java:454)
        at org.eclipse.jgit.lib.GpgConfig.<init>(GpgConfig.java:86)
        at org.eclipse.jgit.api.CommitCommand.processOptions(CommitCommand.java:662)
        at org.eclipse.jgit.api.CommitCommand.call(CommitCommand.java:189)
        at org.jenkinsci.plugins.gitclient.JGitAPIImpl.commit(JGitAPIImpl.java:487)
        at org.jenkinsci.plugins.gitclient.GitClientMaintenanceTest.commitFile(GitClientMaintenanceTest.java:230)
        at org.jenkinsci.plugins.gitclient.GitClientMaintenanceTest.commitOneFile(GitClientMaintenanceTest.java:224)
        at org.jenkinsci.plugins.gitclient.GitClientMaintenanceTest.commitSeveralFiles(GitClientMaintenanceTest.java:218)
        at org.jenkinsci.plugins.gitclient.GitClientMaintenanceTest.test_gc_maintenance(GitClientMaintenanceTest.java:333)
        at java.base/java.lang.reflect.Method.invoke(Method.java:566)
```

[1] https://git-scm.com/docs/git-config#Documentation/git-config.txt-gpgformat
@jtnord jtnord force-pushed the be-lotterant-of-ssh-commit-signing branch from b964feb to f7e2c3c Compare April 8, 2024 19:36
@jtnord jtnord marked this pull request as draft April 8, 2024 21:02
Only write local git config if in local repo

Update source formatting

Wrap the comment
@MarkEWaite
Copy link
Contributor

@jtnord I pushed changes that allow the pull request to pass tests in my environments (Linux, Windows, FreeBSD, etc.) and should pass tests on ci.jenkins.io. Could you test the most recent changes on your Windows environment?

@MarkEWaite MarkEWaite changed the title Be tollerant of SSH commit signing in tests Tolerate default commit signing with SSH in tests Apr 13, 2024
The `--local` argument will cause command line git to fail if the command
is not performed inside a valid repository. That is a good thing because
the tests should avoid changing global configuration where ever possible.
@MarkEWaite
Copy link
Contributor

@jtnord I'm out of office for the next week but may have time for further reviews and changes. I see a pattern of repeating code in those tests that I would like to fix, if you're OK with me using this draft pull request to improve the tests. I would need you to tell me if the tests that I've modified still pass on your Windows computer.

@jtnord
Copy link
Member Author

jtnord commented Apr 15, 2024

@MarkEWaite all[1] tests pass, however something is still missing the --local and messing with the actual clone of the repo.

after running tests my clone has the following new config options ->

commit.gpgsign=false
tag.gpgsign=false
gpg.format=openpgp

Which is unrelated to the tolleration of ssh commit signing, but if you are also looking to cleanup / refactor local setting may be something to look at at the same time,

[1] with the exception of GitClientTest.test_git_ssh_executable_found_on_windows:3393 which is somewhat expected for my environment given the test.

@MarkEWaite
Copy link
Contributor

after running tests my clone has the following new config options ->

commit.gpgsign=false
tag.gpgsign=false
gpg.format=openpgp

Which is unrelated to the toleration of ssh commit signing, but if you are also looking to cleanup / refactor local setting may be something to look at at the same time,

Thanks. I've previously accepted persistent changes in the configuration of the working repository to support tests, but this may be the ideal time to fix that poor behavior.

@jtnord
Copy link
Member Author

jtnord commented Apr 29, 2024

@MarkEWaite pklease @mention me when you would like me to run the tests locally again.

@MarkEWaite
Copy link
Contributor

@jtnord I've fixed the duplications that I had created in the tests. I've run the tests successfully on my Linux machine from a user configuration that has GPG signing enabled globally and I've run them successfully on my Linux and Windows machines where GPG signing is not enabled. Could you check them as well?

@olamy I don't want to disrupt your work on the switch from trilead to Apache Mina. Once this has been reviewed and verified by @jtnord, I'm happy to wait until your work on the switch from trilead to Apache Mina is complete If you find that these changes won't disrupt your work on the switch from trilead to Apache Mina, they can be merged as soon as they are confirmed to pass tests in all the desired configurations.

Thanks again to both of you!

@olamy
Copy link
Member

olamy commented May 9, 2024

@MarkEWaite no worries at all. the changes in this PR are different from my current work from now
Better to do this now as I may change those classes (*Test*) as well later next week :)

@MarkEWaite MarkEWaite marked this pull request as ready for review May 9, 2024 04:03
@MarkEWaite MarkEWaite merged commit d409dff into jenkinsci:master May 9, 2024
15 checks passed
@jtnord jtnord deleted the be-lotterant-of-ssh-commit-signing branch May 9, 2024 09:44
@jtnord
Copy link
Member Author

jtnord commented May 9, 2024

tests all pass locally with the exception of the known GitClientTest.test_git_ssh_executable_found_on_windows

However it is still modifying my local config.

[user]
	name = git-client-user
	email = git-client-user@example.com
[commit]
	gpgsign = false
[tag]
	gpgSign = false
[gpg]
	format = openpgp
[ERROR] Errors:
[ERROR]   GitClientTest.test_git_ssh_executable_found_on_windows:3371 » Runtime ssh executable not found. The git plugin only supports official git client https://git-scm.com/download/win
❯ ssh -V
OpenSSH_for_Windows_8.6p1, LibreSSL 3.4.3

❯ where ssh
C:\Windows\System32\OpenSSH\ssh.exe

@MarkEWaite
Copy link
Contributor

MarkEWaite commented May 9, 2024

However it is still modifying my local config.

Thanks. I saw that in my local development but didn't fix it in this pull request. Will fix it in a future pull request.

@olamy
Copy link
Member

olamy commented May 10, 2024

yup confirmed as well.
grhh need to go to almost every test changing the config :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Automated test addition or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants