Added support for GPG signing

[jaredpetersen]

Round 2 for adding support for GPG to setup-java per issue #43. You can see my previous attempt in PR #59.

This modifies the generated settings.xml file so that Maven GPG plugin can use it, calls gpg to import a private key (using the runner temp directory), and then removes it in a newly-implemented post step. All of the GPG key logic is located in a separate module rather than shoving it into auth.ts like my first attempt.

I also added a XML library to make it easier to conditionally generate XML (and has the added benefit of removing the custom XML parsing logic that currently exists). This will also make it easier to do settings.xml modifications later if needed.

The generated settings.xml file looks the following without GPG enabled:

<settings xmlns="http://maven.apache.org/SETTINGS/1.0.0"
  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
  xsi:schemaLocation="http://maven.apache.org/SETTINGS/1.0.0 https://maven.apache.org/xsd/settings-1.0.0.xsd">
  <servers>
    <server>
      <id>github</id>
      <username>${env.GITHUB_ACTOR}</username>
      <password>${env.GITHUB_TOKEN}</password>
    </server>
  </servers>
</settings>

and like the following with GPG enabled:

<settings xmlns="http://maven.apache.org/SETTINGS/1.0.0"
  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
  xsi:schemaLocation="http://maven.apache.org/SETTINGS/1.0.0 https://maven.apache.org/xsd/settings-1.0.0.xsd">
  <servers>
    <server>
      <id>github</id>
      <username>${env.GITHUB_ACTOR}</username>
      <password>${env.GITHUB_TOKEN}</password>
    </server>
    <server>
      <id>gpg.passphrase</id>
      <passphrase>${env.GPG_PASSPHRASE}</passphrase>
    </server>
  </servers>
</settings>

You can see some successful cross-platform test runs here:

• Default, GPG not in use
• GPG enabled, default parameters
• GPG enabled, override parameters

[jaredpetersen]

CC @joshmgross & @clarkbw

[clarkbw]

🤩 cleaned this up a lot! I’ll try to do a review on Tuesday when I’m back.

[jaredpetersen]

Great! Enjoy your weekend!

[clarkbw]

One thing to look into. Overall it looks good to me.

[jaredpetersen]

Merged master into my fork since there were merge conflicts. Let me know if there's anything I can do on my end to push this forward, @konradpabjan 🙂

[clarkbw]

With setSecret fixed it looks good to me.

[jaredpetersen]

@konradpabjan Any updates on your end?

[konradpabjan]

All good! No objections @jaredpetersen so we can move forward 😃

Apologies for the huge wait. A little unfortunate that we can't make the cleanup step go away for users that don't use GPG signing but it's not a deal breaker.

[konradpabjan]

Looks good to me! 👍 Thanks for the PR!

Asking a few more folks for 👀 and then we can merge this in

[jaredpetersen]

Sounds good! I will try to address your feedback tonight. The links to examples from other github actions repositories are very much appreciated and will save me a lot of time.

[jaredpetersen]

Okay, addressed the feedback. Should be good to go for another round of review.

Did another set of runs on my own repo to verify that everything is still working as well.

• Default
• Default GPG
• GPG override

[jaredpetersen]

Test timeout fixed but I think the GITHUB_TOKEN and GITHUB_ACTOR feedback deserves another look @konradpabjan.

[yacaovsnc]

We should not log these, should we?

[konradpabjan]

This is actually fine, see the output: https://github.com/jaredpetersen/setup-java-gpg/runs/876184775?check_suite_focus=true#step:3:18

In settings.xml you can have interpolation: https://maven.apache.org/settings.html#quick-overview with things such as ${env.HOME} so that nothing gets actually printed to the file. That's what we're doing, just with our own env variables such as $GITHUB_TOKEN

[yacaovsnc]

If all we are printing out are variable names, then we really don't need to print this right? It also creates a mental burden for others to check whether we are printing sensitive information or not. 😄

My vote is to never have anything remotely resembles print(password) anywhere. 😃 We could print password is not null if that info is needed.

Since this isn't a new problem, this does not block the PR.

[yaananth]

I think this might help to confirm for example if passphrase is set or not, with ${gpgPassphrase ? '$' + gpgPassphrase : null} But agreed on the other just print(pwd) kinds :)

[jaredpetersen]

So just to be clear -- we're OK with this current output?

[yaananth]

What could be the error message here? If we fail to clean up, should we surface that as an actionable item to end user so that they can manually clean it up (relevant to self-hosted runners)

[jaredpetersen]

This is just going to output whatever we get from running the failed GPG command. So for example if it failed to find the key for whatever reason, it's going to tell us

>> gpg --batch --yes --delete-keys hello
gpg: key "hello" not found: Not found
gpg: hello: delete key failed: Not found

Or if it failed, it would just be whatever @action/exec surfaces for errors (which may look a little raw).

I can try to simulate what that might look like a little more concretely tonight if I push up a bad cleanup change.

[jaredpetersen]

It might be safer to just say "failed to delete key" rather than the actual GPG output so that we don't leak anything accidentally. If this fails on a hosted runner, they could run the delete command themselves to get a better understanding of what exactly went wrong.

I think I'll go down that route instead.

[yaananth]

Thanks for adding this and also cleaning up code along the way 🙇