This sounds good to me. On Thu, Mar 7, 2019 at 3:32 PM Michael Luckey <[email protected]> wrote:
> Thanks for your comments. > > So to continue here, I ll prepare a PR implementing C: > > Pass the sign key to the relevant scripts and use that for signing. There > is something similar already implemented [1] > > We might discuss on that, whether it will work for us or if we need to > implement something different. > > This should affect at least 'build_release_candidate.sh' and > 'sign_hash_python_wheels.sh'. The release manager is responsible for > selecting the proper key. Currently there is no 'state passed between the > scripts', so the release manager will have to specify this repeatedly. This > could probably be improved later on. > This might become a problem. Is it possible for us to tackle this sooner than later? > > @Ahmet Altay <[email protected]> Could you elaborate which global state > you are referring to? Is it only that git global configuration of the > signing key? [2] > I was referring to things not related to signing. I do not want to digress this thread but briefly I was referring to global installations of binaries with sudo and changes to bashrc file. We can work on those improvements separately. > > [1] > https://github.com/apache/beam/blob/master/release/src/main/scripts/preparation_before_release.sh#L42-L45 > [2] > https://github.com/apache/beam/blob/master/release/src/main/scripts/preparation_before_release.sh#L47-L48 > > On Thu, Mar 7, 2019 at 6:03 PM Ahmet Altay <[email protected]> wrote: > >> >> >> On Wed, Mar 6, 2019 at 8:09 PM Kenneth Knowles <[email protected]> wrote: >> >>> What is the precise purpose of each signature? My naïve impression is >>> that what matters is that users can verify that the release and tag are >>> authentically products of the release manager, so just whatever keys are >>> used have to be in the KEYS file. The test/verification of this is more >>> important that the interface to the scripts. >>> >> >> This is my understanding as well. >> >> >> >>> I would prefer to leave the script implementation and the user's git/gpg >>> configuration (or lack thereof) flexible, while giving default guidance >>> that makes it easy for a release manager. >>> >> >> I am conflicted here. During the release I had issues with scripts making >> unwanted changes to my global configurations. On that sense I agree with >> leaving it flexible and not pushing hard changes to user's configuration. >> On the other hand, ideally the release scripts should just do the right >> things with the push of a button. Each flexibility in there could mean >> release managers to read the documentation properly, make the required >> changes (e.g. signing) in the way they choose. That might lead >> inconsistencies across releases or missed steps. Arguably we will catch >> those issues at validation and it might not be a significant issue. >> >> >>> >>> Kenn >>> >>> On Wed, Mar 6, 2019 at 7:29 PM Ahmet Altay <[email protected]> wrote: >>> >>>> Your observations about the release process are correct. >>>> >>>> (B) sounds like a better option to me because it will prevent creation >>>> of one more global setting. However both solutions are workable. >>>> >>>> On Wed, Mar 6, 2019 at 8:08 AM Robert Bradshaw <[email protected]> >>>> wrote: >>>> >>>>> I would not be opposed to make the choice of signing key a required >>>>> argument for the relevant release script(s). >>>>> >>>> >>>> I agree with this. My question would be, how do we ensure that same key >>>> is passed to different scripts. Today release consists of using multiple >>>> scripts, it would be a simple mistake for the release manager to use a >>>> different key for different scripts. >>>> >>>> >>>>> >>>>> On Wed, Mar 6, 2019 at 3:44 PM Michael Luckey <[email protected]> >>>>> wrote: >>>>> > >>>>> > Hi, >>>>> > >>>>> > After upgrade to gradle 5 @altay (volunteering/selected as release >>>>> manager) was hit by an issue [1] which prevented signing of artefacts. He >>>>> was unfortunately forced to rollback to gradle 4 to do the release. >>>>> > >>>>> > After fixing a configuration issue within beam it seemingly revealed >>>>> an underlying regression in gradle's signing plugin itself [2]. >>>>> > >>>>> > If I understand correctly, beams current setup works along the >>>>> following line: On initial configuration any release manager will setup >>>>> the >>>>> to be used key for git only [3], but we never did something similar on >>>>> gradles behalf. Which results in the signing plugin (delegating to gpg cmd >>>>> line) using whatever gpg considers to be the default key, whether >>>>> explicitly configured with gpg.conf or implicitly. >>>>> > >>>>> > 1. Am I right in assuming that these keys not necessarily have to >>>>> match? I.e. that the key used for signing the release tag ('git tag -s') >>>>> is >>>>> not necessarily the key used for signing the artifacts? >>>>> > 2. Am I right to assume, that we want/require them to be the same? >>>>> I.e. the key which is uploaded to Beam KEYS file? >>>>> > >>>>> > Now after grade 5 stopped defaulting to gpg's default key, we >>>>> somehow need to explicitly specify the key to use for the signing plugin. >>>>> The simplest solution would be to just add a note to the release guide how >>>>> to solve that issue on dev side. Which likely will lead to some >>>>> frustration >>>>> as it is easy to miss. >>>>> > >>>>> > So I would like to integrate something into the used scripting. >>>>> > >>>>> > Options: >>>>> > >>>>> > A: During one time setup, developer is forced to select the proper >>>>> key. This key will be set to (global) git configuration [4]. We could add >>>>> this key also to gradle.gradleHome gradle.properties file as >>>>> 'signing.gnupg.keyName' which then would be used by gradles signing >>>>> plugin. >>>>> > >>>>> > Obvious drawback here would be that this is a global configuration >>>>> (ok, the same problem we have already for git), which might not be >>>>> appropriate for all devs. >>>>> > >>>>> > B: Read the 'git config user.signingkey' on script execution and >>>>> pass this as '-Psigning.gnupg.keyName' parameter to the gradle run. Of >>>>> course this will only work, iff the git config is set. So would it be save >>>>> to assume such? >>>>> > >>>>> > Drawback here, of course, is someone not using the release script >>>>> missing to set the signing key. >>>>> > >>>>> > Of course, both will not solve any issue with source.zip releases >>>>> and pythons signing key, which, as far as I can tell also rely on gpgs >>>>> default key which might conflict with 1. above? >>>>> > >>>>> > Any thoughts about this? >>>>> > >>>>> > michel >>>>> > >>>>> > >>>>> > >>>>> > >>>>> > [1] https://issues.apache.org/jira/browse/BEAM-6726 >>>>> > [2] https://github.com/gradle/gradle/issues/8657 >>>>> > [3] >>>>> https://github.com/apache/beam/blob/master/website/src/contribute/release-guide.md >>>>> > [4] >>>>> https://github.com/apache/beam/blob/master/release/src/main/scripts/preparation_before_release.sh#L44-L48 >>>>> >>>>
