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.

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

adds keyless signatures documentation #56

Merged
merged 4 commits into from
Feb 8, 2023

Conversation

ChrisJBurns
Copy link
Contributor

Signed-off-by: ChrisJBurns 29541485+ChrisJBurns@users.noreply.github.com

@netlify
Copy link

netlify bot commented Jan 2, 2023

Deploy Preview for docssigstore ready!

Name Link
🔨 Latest commit 596e026
🔍 Latest deploy log https://app.netlify.com/sites/docssigstore/deploys/63e2cf802c35690008a469e2
😎 Deploy Preview https://deploy-preview-56--docssigstore.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@ChrisJBurns
Copy link
Contributor Author

ChrisJBurns commented Jan 2, 2023

@znewman01 Here's the keyless.md migration 👍

I've just copied and pasted the markdown, not sure if you wanted a refactor of the content itself? I assumed that the content in the keyless.md is good to go as is

znewman01
znewman01 previously approved these changes Jan 2, 2023
Copy link
Contributor

@znewman01 znewman01 left a comment

Choose a reason for hiding this comment

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

Looks great, just a wholesale migration is a good start

CC @ltagliaferri for approval

Copy link
Collaborator

@ltagliaferri ltagliaferri left a comment

Choose a reason for hiding this comment

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

I left a few comments for changes, the most important ones are the ones around the GA as we should avoid merging in outdated info here.

content/en/cosign/sign.md Outdated Show resolved Hide resolved
content/en/cosign/keyless.md Outdated Show resolved Hide resolved
content/en/cosign/keyless.md Outdated Show resolved Hide resolved
content/en/cosign/keyless.md Outdated Show resolved Hide resolved
content/en/cosign/keyless.md Outdated Show resolved Hide resolved
content/en/cosign/keyless.md Outdated Show resolved Hide resolved
content/en/cosign/keyless.md Outdated Show resolved Hide resolved
content/en/cosign/keyless.md Outdated Show resolved Hide resolved
content/en/cosign/keyless.md Outdated Show resolved Hide resolved
@ChrisJBurns
Copy link
Contributor Author

@ltagliaferri Addressed PR comments 👍

content/en/cosign/keyless.md Outdated Show resolved Hide resolved
content/en/cosign/keyless.md Outdated Show resolved Hide resolved
content/en/cosign/keyless.md Outdated Show resolved Hide resolved
content/en/cosign/keyless.md Outdated Show resolved Hide resolved
content/en/cosign/keyless.md Outdated Show resolved Hide resolved
content/en/cosign/keyless.md Show resolved Hide resolved
content/en/cosign/keyless.md Show resolved Hide resolved

### Custom root Cert

You can override the public good instance root CA using the environment variable `SIGSTORE_ROOT_FILE`, e.g.
Copy link
Contributor

Choose a reason for hiding this comment

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

We should update this section with the flags for the CT and Rekor keys too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will need to figure out what those flags are ... will update when committed and pushed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have had a look at the current cosign that I've got locally from brew and I can't see any keys for rekor when doing cosign sign -help. Which keys are you referencing exactly? I can quickly put them in if possible

content/en/cosign/keyless.md Outdated Show resolved Hide resolved
content/en/cosign/sign.md Outdated Show resolved Hide resolved
@ChrisJBurns
Copy link
Contributor Author

ChrisJBurns commented Jan 5, 2023

@haydentherapper I'll have to spend a bit more time on this PR. I've only copied and pasted the content from the existing KEYLESS.md in the cosign repo, so am unsure on the details of keyless signatures. Additionally, I've not actually run anything locally around keyless signatures so I'll have to do a bit of learning in order to fill the gaps, as well as giving it a try locally to get the latest updates for output.

@haydentherapper
Copy link
Contributor

I think it's fine to copy this over as-is and not add content, but I would remove outdated content.

Signed-off-by: ChrisJBurns <29541485+ChrisJBurns@users.noreply.github.com>
@ChrisJBurns
Copy link
Contributor Author

@haydentherapper have pushed up changes that (I think) addresses some of your comments. I've resolved them so you can see which ones I've attempted to resolve and left the others unresolved with a comment as they will take a bit more time for me to do.

content/en/cosign/keyless.md Outdated Show resolved Hide resolved
content/en/cosign/keyless.md Outdated Show resolved Hide resolved
content/en/cosign/keyless.md Outdated Show resolved Hide resolved
Signed-off-by: ChrisJBurns <29541485+ChrisJBurns@users.noreply.github.com>
@ChrisJBurns
Copy link
Contributor Author

ChrisJBurns commented Jan 28, 2023

@haydentherapper @znewman01 @ltagliaferri Have pushed some newer changes, made a quick start at the top using ttl.sh and crane. Also updated the output for the sign and verify examples. I got alot of output in the optional field, but just kept it to null in the example if this it ok? I can squash commits when approvals start coming in 👍

Lastly, I didn't get this in my output for verify, so I'm assuming the latest cosign binary in brew doesn't have it yet (1.13.1). But added the updated output that was pushed in this commit

znewman01
znewman01 previously approved these changes Feb 6, 2023
Copy link
Contributor

@znewman01 znewman01 left a comment

Choose a reason for hiding this comment

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

I know you signed up for "copy paste a markdown file between two repos" and got a heck of a lot more than that, thanks!

content/en/cosign/keyless.md Outdated Show resolved Hide resolved
content/en/cosign/keyless.md Outdated Show resolved Hide resolved
content/en/cosign/keyless.md Outdated Show resolved Hide resolved
Signed-off-by: ChrisJBurns <29541485+ChrisJBurns@users.noreply.github.com>
@ChrisJBurns
Copy link
Contributor Author

@znewman01 Just pushed up amendments, let me know when you get a chance 👍

Copy link
Contributor

@znewman01 znewman01 left a comment

Choose a reason for hiding this comment

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

Awesome, thanks so much for this! Docs are really important for helping users navigate what we often take for granted 😄

@ltagliaferri ltagliaferri self-requested a review February 8, 2023 14:26
@ltagliaferri ltagliaferri merged commit 054265d into sigstore:main Feb 8, 2023
@ChrisJBurns
Copy link
Contributor Author

ChrisJBurns commented Feb 8, 2023

@znewman01 No problems, glad to finally get it done 😄 I'll submit the deprecation PR for the cosign/keyless.md file ASAP

Copy link

@WoNation-Unity WoNation-Unity left a comment

Choose a reason for hiding this comment

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

Pipeline

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants