HBASE-27312 Update create-release to work with maven-gpg-plugin-3.0.1 and gnupg >= 2.1.x #4716
Conversation
ndimiduk
requested review from
busbey,
saintstack,
joshelser,
apurtell,
Apache9 and
huaxiangsun
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
| @@ -105,6 +105,9 @@ $ scp ~/gpg.example.apache.pub example.gce.host: | |||
| # gpg-agent's extra socket (this will restrict what commands the remote node is allowed to have | |||
| # your agent handle. Note that the gpg guide above can help you set this up in your ssh config | |||
| # rather than typing it in ssh like this every time. | |||
| # Note that as of maven-gpg-plugin, with gnupg >= 2.1, the plugin uses `--pinentry-mode error`, | |||
There was a problem hiding this comment.
this should have been "as of maven-gpg-plugin 3.0.1"
There was a problem hiding this comment.
Want to say more here that the scripts have undone use of extra socket... And add in the paragraph you have at the head of this PR where you note implications of our skirting the 'restricted' socket?
There was a problem hiding this comment.
Yeah, lets see if we add a launch flag like Duo suggests. I'll update the readme accordingly.
| @@ -320,7 +320,7 @@ else | |||
| # agent socket and agent extra socket to your local gpg-agent's extra socket. See the README.txt | |||
| # for an example. | |||
| GPG_PROXY_MOUNT=(--mount \ | |||
| "type=bind,src=$(gpgconf --list-dir agent-extra-socket),dst=/home/${USER}/.gnupg/S.gpg-agent") | |||
| "type=bind,src=$(gpgconf --list-dir agent-socket),dst=/home/${USER}/.gnupg/S.gpg-agent") | |||
There was a problem hiding this comment.
I suppose so. Would be good to try it on linux but going by Mac osx experience, would be surprised if the restricted extra socket worked with the maven gpg plugin pinentry-mode setting.... So yeah, lets make this change.
| @@ -30,7 +30,7 @@ | |||
| DRY_RUN=${DRY_RUN:-1} #default to dry run | |||
| DEBUG=${DEBUG:-0} | |||
| GPG=${GPG:-gpg} | |||
| GPG_ARGS=(--no-autostart --batch) | |||
| GPG_ARGS=(-v --no-autostart --batch --pinentry-mode error) | |||
There was a problem hiding this comment.
I can leave out the -v, but the --pinentry-mode error is how the maven-gpg-plugin invokes the command. It'll error out rather than giving the user a prompt from pin entry. So, it checks that the socket works, but it will fail if the user has not unlocked the key before starting the build. Maybe we keep this but add another test invocation run from the host environment, in non-batch and allow for pin entry?
There was a problem hiding this comment.
Looking at this again, you're probably correct that we don't want this added to all invocations. Probably only at the place where we test the environment before proceeding.
| "${MVN[@]}" install assembly:single -DskipTests -Dcheckstyle.skip=true "${PUBLISH_PROFILES[@]}" | ||
| cmd=("${MVN[@]}" install assembly:single -DskipTests -Dcheckstyle.skip=true "${PUBLISH_PROFILES[@]}") | ||
| echo "${cmd[*]}" | ||
| "${cmd[@]}" |
There was a problem hiding this comment.
These changes are probably not needed? Or not related to what this PR is about?
There was a problem hiding this comment.
Not needed but helpful when reading logs. Without this, I can't tell which step is ending in failure. I'm fine with pulling them out to a separate PR if you prefer.
| @@ -105,6 +105,9 @@ $ scp ~/gpg.example.apache.pub example.gce.host: | |||
| # gpg-agent's extra socket (this will restrict what commands the remote node is allowed to have | |||
| # your agent handle. Note that the gpg guide above can help you set this up in your ssh config | |||
| # rather than typing it in ssh like this every time. | |||
| # Note that as of maven-gpg-plugin, with gnupg >= 2.1, the plugin uses `--pinentry-mode error`, | |||
There was a problem hiding this comment.
Want to say more here that the scripts have undone use of extra socket... And add in the paragraph you have at the head of this PR where you note implications of our skirting the 'restricted' socket?
|
I also changed the script locally to use I own the build machine so I can put the private key on the machine. But IIRC, what @busbey suggested in the past, is to use agent forwarding so you do not need to put your private key on the build machine. See here https://wiki.gnupg.org/AgentForwarding So maybe we could make this configurable? By default we will use local key, but with a special command line argument we could still use the extra socket. Thanks. |
When local machine (host) runs Linux, I believe that the script mounts the local When local machine (host) in MacOS (and presumably also when running Windows), the docker daemon is running inside of a VM (guest), so we have to do this agent-forwarding thing. The suggestion is to use the restricted socket, When local machine is not the build machine, like a build machine (linux) running in public cloud, you should probably just forward the So in all cases there's a gpg-agent involved. I suppose yes, we could add a configuration option, something like WDYT? |
+1
|
ndimiduk
force-pushed
the
27312-create-release-gpg-plugin-3.0.1-gnupg-2.1
branch
from
9a81694 to
4920f7e
Compare
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
Any updates here? @ndimiduk
We want to implement this logic in this PR? Or another PR? Thanks. |
|
Any progress here? @ndimiduk With HBASE-25983, we are ready to move the release process to jdk11. And for HBASE-27359, we also need to change the release scripts to generate different artifacts for hadoop2 and hadoop3. Can we land the improvements to the release scripts here now and start the above works? Thanks. |
|
I've not attempted release candidates anytime recently, so the status here is not changed. There's a couple reviewer questions about the wisdom of exposing the socket with elevated privileges. The changes here that allowed me to proceed may not make sense for other environments. For example, if someone is building an RC on a 3PC, they may not want the full privileged socket exposed to the remote environment. However, I don't know if any of our release managers are building on "untrusted" hardware. |
I think we could introduce a flag to control the behavior? i.e, mounting agent-socket or agent-extra-socket, and we could still make the default to mount agent-extra-socket, which is the same with the current behavior. Anyway, if this is not to be landed in the new future, I will try to land other improvements first. Thanks @ndimiduk ! |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
Given that with a modern GnuPG and maven-gpg-plugin, this change is necessary for signing/releasing to work, I think that we should merge this patch as is. I don't see a use-case where someone would be able to use the standard socket. We should, perhaps, warn/error in the presence of an old GnuPG. I also checked the released issues in maven-gpg-plugin 3.1.0 (the latest release), and I see no mention of socket use or |
|
@saintstack @Apache9 what do you think? |
|
I filed and linked to https://issues.apache.org/jira/browse/MGPG-92. |
|
What do you mean by 'standard socket'? |
… and gnupg >= 2.1.x
ndimiduk
force-pushed
the
27312-create-release-gpg-plugin-3.0.1-gnupg-2.1
branch
from
4920f7e to
2e31996
Compare
The best summary description that I've found of the various gpg-agent sockets is https://unix.stackexchange.com/a/605639. It refers to entries in the Arch Linux wiki, so it may not be authoritative.
Yes, this is my point -- since we upgraded to this combination of gpg and maven-gpg-plugin, builds require use of the |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
I'm a bit confusing, here you said we must use the 'extra' socket but what you have done in this PR is to remove the usage of 'exrta' socket? Sorry I can not follow. |
You're absolutely correct. I've swapped the meaning of "standard" and "extra" in my mind. Sorry to add confusion, I've been away from this issue for a while. |
|
Ah, OK, no problem. +1. |
This is... a solution. It's probably fine for running on hardware you own with a source repository you trust. I'm not sure it's wise when running on a remote build machine...