-
Notifications
You must be signed in to change notification settings - Fork 83
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
Cosign reorg #323
Cosign reorg #323
Conversation
✅ Deploy Preview for docssigstore ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
…tion. Quickstart surfaced to top level. Signing, verifying, key management, and system configuration nested under cosign category. Signed-off-by: hayleycd <cook.hayley@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The top level reorg looks good to me. Left some small comments inline.
@@ -10,7 +10,7 @@ This support is enabled through the [PIV protocol](https://csrc.nist.gov/project | |||
and the [go-piv](https://github.com/go-piv/piv-go) library, which is not included in the standard release. Use `make cosign-pivkey-pkcs11key`, or `go build -tags=pivkey,pkcs11key ./cmd/cosign`, to build `cosign` with support for hardware tokens. | |||
|
|||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the markdownlint bot is complaining about these borders, they're probably not needed if this is being changed from a note to a regular section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @cmurphy! I am happy to fix this, but to give you some more context:
- There are ten errors that showed up in the markdown linter. This is one of them.
- However, I had previously fixed ten other errors. You see them in places like here.
- When I fixed those ten there were another ten found.
Basically I am not sure how many errors will eventually show up in chunks of ten, and I asked @haydentherapper whether it was worth chasing all of these down when I am not actually bringing in new errors and the check didn't technically fail. I opened issue #325 to investigate this.
Let me know what you think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only pointed this one out because it looks like it was introduced by this change - the note header went from a bolded NOTE to an actual level-2 header, and there is now an error about the heading being surrounded by a blank line, which would not have existed previously. I certainly agree that it's not worth chasing all the preexisting errors down, but it would be better not to introduce new ones.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cmurphy Gotcha! That makes sense. It just got lost in the noise. I will fix.
|
||
Language specific clients (like [sigstore-python](https://github.com/sigstore/sigstore-python)) are other options for signing and verifying, but Cosign is a great, language agnostic place to start. | ||
|
||
This quickstart will walk you through how to sign and verify a blob and a container. Although keyless signing is recommended, this quickstart will also show you how to sign using a generated key. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this is out of scope for this reorg PR, but since we're recommending keyless signing maybe that's what the focus of this quickstart should be?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cmurphy So it does go through keyless signing first, and then signing with a generated key at the end. Would you like for me to remove the generated key section?
I think I could reword things so that it is clearer that the first part of the quickstart is for keyless signing and is preferred. However, a quickstart does not need to highlight every feature so I am comfortable removing the generated key section all together.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I was initially thinking to just remove the non-keyless signing part since I imagine the quickstart as a very brief, easy introduction, but I'd also be fine with just rewording this to de-emphasize signing with a generated key. If we go with the latter, either removing the second sentence entirely or changing it to something like
This quickstart will walk you through how to sign and verify a blob and a container. Although keyless signing is recommended, this quickstart will also show you how to sign using a generated key. | |
This quickstart will walk you through how to sign and verify a blob and a container. It will introduce keyless signing, which is the recommended method, as well as signing with a generated key, which is necessary for some use cases. |
I think it's mainly starting the sentence with "Although" that's throwing me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that there is a lot going on in this quickstart, so I opted to remove the section. Let me know what you think.
Signed-off-by: hayleycd <cook.hayley@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great!
content/en/cosign/signing/gitsign.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gitsign is a unique case, it's more like Cosign in that it's a tool built on top of an SDK to sign a specific format (commits, rather than containers/blobs like Cosign).
What do you think about an additional section for Sigstore tooling that is not Cosign? Something like:
- Cosign
- Sign
- Verify
- Etc
- Client libraries
- sigstore-python
- sigstore-java
- etc
- Sigstore tools
- gitsign
- model signing (https://github.com/sigstore/model-transparency)
- In the future, more tools
If we were to ever refactor Cosign to be smaller in scope to only support container signing, it would then just move under Sigstore tooling
, though I don't expect that to happen. But it would give us a category to expand over time as more use cases for Sigstore signing arise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@haydentherapper I like this idea. Would you like to see it incorporated in this pull request, or in a future one? GitSign is easy enough. It would take a little more to move model signing in just because it is currently not in the docs, but it isn't a big deal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy to do this as a follow up. We can also create a placeholder and ask one of the maintainers from model signing to help fill in some details. I'll create an issue.
|
||
Cosign is a command line utility that is used to sign software artifacts and verify signatures using Sigstore. | ||
|
||
Language specific clients (like [sigstore-python](https://github.com/sigstore/sigstore-python)) are other options for signing and verifying, but Cosign is a great, language agnostic place to start. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might slightly rephrase to add that the language specific clients are SDKs to build tooling on top of, and though some may offer basic CLIs, Cosign is the recommended tool for most signing and verifying.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@haydentherapper Thanks! That makes sense. I have updated the wording a bit. This part will change again once the language clients are added to the docs. We could also cut this section for now and reintroduce something once there is something more to link to within the documentation (I want to link to the upcoming 'language client' section of the docs).
Signed-off-by: hayleycd <cook.hayley@gmail.com>
Closes #322.
Summary
Reorganizing documentation ahead of including language client information
Release Note
NONE