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

Fix gpg signing #292

Closed
wants to merge 1 commit into from
Closed

Conversation

unidevel
Copy link
Contributor

@unidevel unidevel commented Feb 24, 2025

useGpgCmd seems never work on both my machine and the action env, not sure why.

So replace it with useInMemoryPgpKeys without using the gpg command, also added 2 checkings to avoid errors when running build/test in dev env(without the gradle.properties, or not set some of the signing related properties)

Tested with https://github.com/unix280/tempto/actions/runs/13507065551

Signature can be found in my test repo https://m2.unidevel.cn/#/releases/io/prestodb/tempto/tempto-core/1.54

@unidevel unidevel requested a review from aaneja February 24, 2025 20:28
def keyFile = secretKeyRingFilePath ? Paths.get(secretKeyRingFilePath) : null
if (keyFile && Files.exists(keyFile) && !Files.isDirectory(keyFile)) {
def secretKey = new String(Files.readAllBytes(keyFile), "UTF-8")
useInMemoryPgpKeys(signingKeyId, secretKey, signingPassword)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use null value to make the parameter optional, e.g. user set the signing.keyId=, if use empty string here, it will report error like secret key not found, but if use the null value here, it will use the first secret key in the secret file

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am assuming that the signing failed previously because we need to import the key into the actions runner's GPG keyring. It failed because no key was found on the keyring. Could we try the import as a setup step in the action instead of calling the useInMemoryPgpKeys? I think it just needs a gpg --import keyFile.gpg or something like that.

Have you tried using nektos/act for testing?

@unidevel unidevel marked this pull request as ready for review February 24, 2025 20:33
def keyFile = secretKeyRingFilePath ? Paths.get(secretKeyRingFilePath) : null
if (keyFile && Files.exists(keyFile) && !Files.isDirectory(keyFile)) {
def secretKey = new String(Files.readAllBytes(keyFile), "UTF-8")
useInMemoryPgpKeys(signingKeyId, secretKey, signingPassword)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am assuming that the signing failed previously because we need to import the key into the actions runner's GPG keyring. It failed because no key was found on the keyring. Could we try the import as a setup step in the action instead of calling the useInMemoryPgpKeys? I think it just needs a gpg --import keyFile.gpg or something like that.

Have you tried using nektos/act for testing?

@unidevel
Copy link
Contributor Author

I am assuming that the signing failed previously because we need to import the key into the actions runner's GPG keyring. It failed because no key was found on the keyring. Could we try the import as a setup step in the action instead of calling the useInMemoryPgpKeys? I think it just needs a gpg --import keyFile.gpg or something like that.

The problem is that gradle sign will use gpg --batch which can't accept any passpharse, unless it is cached before.

That's why I'm using useInMemoryPgpKeys which has the minimal change comparing with useGpgCmd

Have you tried using nektos/act for testing?

Good to know this, will try it in future, thanks

Copy link
Contributor

@ZacBlanco ZacBlanco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes. After taking another look at the signing plugin docs I think this looks good.

@unidevel
Copy link
Contributor Author

@ZacBlanco Thanks for your comments, I found another way to cache the passphase which is simpler than this, so I'll close this one.

@unidevel unidevel closed this Feb 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants