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.

Sign up for GitHub

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

Jump to bottom

doc: use git-secure-tag for release tags #7603

Closed
wants to merge 3 commits into from
Closed

doc: use git-secure-tag for release tags #7603

wants to merge 3 commits into from

Conversation

indutny
Copy link
Member

@indutny indutny commented Jul 8, 2016 •
edited
Loading

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc

Description of change

git-secure-tag recursively constructs an SHA-512 digest out of the
git tree, and puts the hash from the tree's root into the tag
annotation. This hash provides better integrity guarantees than the
default SHA-1 merkle tree that git uses.

Fix: #7579

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Jul 8, 2016
@indutny
Copy link
Member Author

indutny commented Jul 8, 2016

cc @rvagg @Fishrock123 @rmg

@indutny indutny mentioned this pull request Jul 8, 2016
Closed
@MylesBorins
Copy link
Contributor

@indutny does this work with subkeys?

@indutny
Copy link
Member Author

indutny commented Jul 8, 2016

@thealphanerd It supports -s and -u options, just as git tag.

@indutny
Copy link
Member Author

indutny commented Jul 8, 2016

It does not support -d, -F, and -a, though.

@indutny
Copy link
Member Author

indutny commented Jul 8, 2016

Updated description, initial version was misleading. Sorry!

@rmg
Copy link
Contributor

rmg commented Jul 8, 2016

I liked the idea before seeing the docs. Now that I see it is literally a drop in replacement with just a few extra keystrokes thrown in and still spits out a signed git tag in the end, I think I like it even more :-)

rvagg
rvagg reviewed Jul 8, 2016
View reviewed changes
doc/releases.md
Create a tag using the following command:

```
$ git tag <vx.y.z> <commit-sha> -sm 'YYYY-MM-DD Node.js vx.y.z (Release Type) Release'
$ git secure-tag <vx.y.z> <commit-sha> -s -m 'YYYY-MM-DD Node.js vx.y.z (Release Type) Release'
Copy link
Member

Choose a reason for hiding this comment

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

does -sm work (doesn't need to change here, just interested if my keystroke memory will still apply)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, it actually works. I'll change it back.

Copy link
Member Author

Choose a reason for hiding this comment

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

Pushed.

@rvagg
Copy link
Member

rvagg commented Jul 8, 2016

need to make sure we don't have any objectors in @nodejs/release

lgtm

@jasnell
Copy link
Member

jasnell commented Jul 8, 2016

Emphatic LGTM on this.

@Fishrock123
Copy link
Contributor

Should we make a test release with it to ensure everything behaves as expected?

Also, does this work properly with the verification in tools/release.sh?

@rvagg
Copy link
Member

rvagg commented Jul 8, 2016

Yes, tools/release.sh is just checking that the key is signed by you, not how it was signed or what it contains so it should be fine. Full releases are the only ones that we normally sign and manually promote, the rest just happen automatically. Perhaps this PR can hold off until the next release, whichever branch that is, to see how it goes.

This was referenced Jul 10, 2016
Closed
Closed
@indutny
Copy link
Member Author

indutny commented Jul 11, 2016

@thealphanerd is going to do a v4 RC release. Does anyone have an objection to using git-secure-tag for it?

@MylesBorins MylesBorins self-assigned this Jul 11, 2016
@rvagg
Copy link
Member

rvagg commented Jul 12, 2016

iirc we aren't tagging RC releases these days, only proper releases with a manual promotion, I have no objections to using this in the next v4 though as long as it's understood that if something comes up that causes this to hold up the release (whatever that might be!) then it can be dropped

@indutny
Copy link
Member Author

indutny commented Jul 12, 2016

Cool, thank you for the heads up!

@indutny
Copy link
Member Author

indutny commented Jul 23, 2016

@rvagg should we land this PR?

@MylesBorins
Copy link
Contributor

@indutny I think we were waiting to do a release with the tool first.

looks like this PR needs a rebase

@indutny
Copy link
Member Author

indutny commented Jul 23, 2016

@thealphanerd we just did a release without a tool, and it is not clear how release team will learn about tool if we won't land this first :)

@indutny
doc: use git-secure-tag for release tags
3b37d4b 
`git-secure-tag` recursively constructs an SHA-512 digest out of the
git tree, and puts the hash from the tree's root into the tag
annotation. This hash provides better integrity guarantees than the
default SHA-1 merkle tree that git uses.

Fix: nodejs#7579
@MylesBorins
Copy link
Contributor

that's fair. I was under the impression we were waiting for the LTS on this. @nodejs/release are we planning a v6 release for next week? Can we do make the tag with this tool?

@indutny
Copy link
Member Author

indutny commented Jul 23, 2016

Rebased.

@ChALkeR ChALkeR added the security Issues and PRs related to security. label Jul 23, 2016
@jasnell
Copy link
Member

jasnell commented Aug 4, 2016

where did we land on this? :-)

@indutny
Copy link
Member Author

indutny commented Aug 5, 2016

We didn't, but I think we should.

ChALkeR
ChALkeR reviewed Aug 5, 2016
View reviewed changes
doc/releases.md
@@ -216,10 +216,16 @@ Once you have produced builds that you're happy with, create a new tag. By waiti

Tag summaries have a predictable format, look at a recent tag to see, `git tag -v v6.0.0`. The message should look something like `2016-04-26 Node.js v6.0.0 (Current) Release`.

Install `git-secure-tag` npm module:

```sh
Copy link
Member

@ChALkeR ChALkeR Aug 5, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

console, ref: #7971, #7727.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ack, thank you!

@indutny
Copy link
Member Author

indutny commented Aug 5, 2016

We have two LGTMs here, and general consensus. Going to land it in a bit if no objections will be mentioned.

@jasnell
Copy link
Member

jasnell commented Aug 5, 2016

SGTM!

@evanlucas
Copy link
Contributor

LGTM

@indutny
Copy link
Member Author

indutny commented Aug 5, 2016

Landed in 0f3f76c, thank you everyone! cc @nodejs/release

indutny added a commit that referenced this pull request Aug 5, 2016
@indutny
doc: use git-secure-tag for release tags
0f3f76c 
`git-secure-tag` recursively constructs an SHA-512 digest out of the
git tree, and puts the hash from the tree's root into the tag
annotation. This hash provides better integrity guarantees than the
default SHA-1 merkle tree that git uses.

Fix: #7579
PR-URL: #7603
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
@jbergstroem
Copy link
Member

Post-land-LGTM -- I think this is a great addition.

yorkie
yorkie reviewed Aug 5, 2016
View reviewed changes
doc/releases.md
@@ -216,10 +216,16 @@ Once you have produced builds that you're happy with, create a new tag. By waiti

Tag summaries have a predictable format, look at a recent tag to see, `git tag -v v6.0.0`. The message should look something like `2016-04-26 Node.js v6.0.0 (Current) Release`.

Install `git-secure-tag` npm module:

```console
Copy link
Contributor

Choose a reason for hiding this comment

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

use sh to keep consistent with others?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is made in anticipation of other PR that will change the rest to console.

@jasnell
Copy link
Member

jasnell commented Aug 8, 2016

Whoops, looks like we forgot to close this when it landed.

@jasnell jasnell closed this Aug 8, 2016
@cjihrig cjihrig mentioned this pull request Aug 8, 2016
Closed
cjihrig pushed a commit that referenced this pull request Aug 10, 2016
@indutny @cjihrig
doc: use git-secure-tag for release tags
3d1a064 
`git-secure-tag` recursively constructs an SHA-512 digest out of the
git tree, and puts the hash from the tree's root into the tag
annotation. This hash provides better integrity guarantees than the
default SHA-1 merkle tree that git uses.

Fix: #7579
PR-URL: #7603
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
@cjihrig cjihrig mentioned this pull request Aug 11, 2016
Merged
MylesBorins pushed a commit that referenced this pull request Sep 9, 2016
@indutny
doc: use git-secure-tag for release tags
bf33d2f 
`git-secure-tag` recursively constructs an SHA-512 digest out of the
git tree, and puts the hash from the tree's root into the tag
annotation. This hash provides better integrity guarantees than the
default SHA-1 merkle tree that git uses.

Fix: #7579
PR-URL: #7603
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 28, 2016
@indutny
doc: use git-secure-tag for release tags
23989b0 
`git-secure-tag` recursively constructs an SHA-512 digest out of the
git tree, and puts the hash from the tree's root into the tag
annotation. This hash provides better integrity guarantees than the
default SHA-1 merkle tree that git uses.

Fix: #7579
PR-URL: #7603
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
rvagg pushed a commit that referenced this pull request Oct 18, 2016
@indutny @rvagg
doc: use git-secure-tag for release tags
652e7d6 
`git-secure-tag` recursively constructs an SHA-512 digest out of the
git tree, and puts the hash from the tree's root into the tag
annotation. This hash provides better integrity guarantees than the
default SHA-1 merkle tree that git uses.

Fix: #7579
PR-URL: #7603
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 26, 2016
@indutny @MylesBorins
doc: use git-secure-tag for release tags
009df78 
`git-secure-tag` recursively constructs an SHA-512 digest out of the
git tree, and puts the hash from the tree's root into the tag
annotation. This hash provides better integrity guarantees than the
default SHA-1 merkle tree that git uses.

Fix: #7579
PR-URL: #7603
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Oct 26, 2016
Closed
@MylesBorins MylesBorins removed their assignment Dec 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
doc Issues and PRs related to the documentations. security Issues and PRs related to security.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use EVTag for release tags