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. @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] [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 >>>> >>>
