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

Docs move keyless info to overview #156

Merged

Conversation

jonvnadelberg
Copy link
Collaborator

@jonvnadelberg jonvnadelberg commented Apr 25, 2023

Fixes #155

Summary

Removes duplicate information and clarifies information in overview.md

Release Note

NONE

Documentation

Doc only fix

Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
@netlify
Copy link

netlify bot commented Apr 25, 2023

Deploy Preview for docssigstore ready!

Name Link
🔨 Latest commit b70c8a9
🔍 Latest deploy log https://app.netlify.com/sites/docssigstore/deploys/6452deb416f93600085f2f0b
😎 Deploy Preview https://deploy-preview-156--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.

@jonvnadelberg
Copy link
Collaborator Author

@olivekl Please review.

I didn't change the name of the keyless.md file, but did change it's purpose. Should we create a whole new file with the same content and get rid of keyless.md?

Copy link
Contributor

@haydentherapper haydentherapper left a comment

Choose a reason for hiding this comment

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

Overall, I don't think we need to change anything in overview.md. Summarizing my suggestions:

  • Let's rename keyless.md to "public_deployment.md", and update the title. That gives us a place to add content about the public deployment
  • Let's create "timestamping.md" and move the section on timestamps to it.
  • Move "Custom Components" section under "verify.md"
  • After that, revert all changes done to overview.md

That will result in duplicated content being deleted from "keyless.md", and removing the "ttl.sh" example which I personally don't think is needed.

content/en/cosign/keyless.md Outdated Show resolved Hide resolved
content/en/cosign/overview.md Outdated Show resolved Hide resolved
content/en/cosign/overview.md Outdated Show resolved Hide resolved
content/en/cosign/overview.md Show resolved Hide resolved
content/en/cosign/overview.md Outdated Show resolved Hide resolved
content/en/cosign/overview.md Outdated Show resolved Hide resolved
@dmitris
Copy link
Contributor

dmitris commented Apr 26, 2023

I wonder where the "non-Fulcio keyless" use case (the one with short-lived keys and provided certificate-chain + identity) would be documented, thinking of sigstore/cosign#2845 and #153?

@haydentherapper
Copy link
Contributor

@dmitris - In verify.md, as you proposed. "keyless.md" and "openid_signing.md" are being cleaned up to remove duplication.

@jonvnadelberg
Copy link
Collaborator Author

jonvnadelberg commented Apr 26, 2023 via email

@haydentherapper
Copy link
Contributor

I would love for someone else to chime in too, but my thought is that ttl.sh is just for testing and not something someone should do for real-world usage. crane is already discussed in the signing docs too. I was thinking either we'd remove ttl.sh entirely, or we have a dedicated subsection in the quickstart guide showing its uagae, but make it clear this is just an example.

@dmitris
Copy link
Contributor

dmitris commented Apr 27, 2023

I was thinking either we'd remove ttl.sh entirely
I learned about ttl.sh from the examples currently in https://docs.sigstore.dev/cosign/keyless/#quickstart and it seems great for ad-hoc testing, it would be too bad if that pointer disappear completely

jonvnadelberg and others added 11 commits May 1, 2023 06:13
Co-authored-by: ltagliaferri <lisa.tagliaferri@gmail.com>
Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
Co-authored-by: ltagliaferri <lisa.tagliaferri@gmail.com>
Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
Co-authored-by: ltagliaferri <lisa.tagliaferri@gmail.com>
Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
Co-authored-by: ltagliaferri <lisa.tagliaferri@gmail.com>
Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
Co-authored-by: ltagliaferri <lisa.tagliaferri@gmail.com>
Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
Co-authored-by: ltagliaferri <lisa.tagliaferri@gmail.com>
Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
Co-authored-by: ltagliaferri <lisa.tagliaferri@gmail.com>
Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
add note saying that for the example we are using script variable.

Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
Co-authored-by: ltagliaferri <lisa.tagliaferri@gmail.com>
Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
add context to revert section

Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
haydentherapper
haydentherapper previously approved these changes May 2, 2023
@haydentherapper
Copy link
Contributor

Looks great!

1. `rm -r ~/.sigstore`
1. `curl -O https://raw.githubusercontent.com/sigstore/root-signing/main/staging/repository/1.root.json`
1. `cosign initialize --mirror=https://tuf-repo-cdn.sigstore.dev --root=1.root.json`
1. `COSIGN_EXPERIMENTAL=1 cosign sign --oidc-issuer "https://oauth2.sigstage.dev/auth" --fulcio-url "https://fulcio.sigstage.dev" --rekor-url "https://rekor.sigstage.dev" ${IMAGE_DIGEST}`
Copy link
Member

Choose a reason for hiding this comment

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

we can drop the experimental and need to add the --yes flag as well in the sign

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed on dropping experimental. I'd not include "--yes" though, because we don't want the examples to be skipping through that prompt

Copy link
Member

Choose a reason for hiding this comment

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

makes sense

@jonvnadelberg
Copy link
Collaborator Author

A few comments / suggestions / questions here. Thanks!

changes implemented

Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
add id info to verify, remove "experimental"

Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
remove typo.

Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
change OIDC issuer to accounts.example.com

Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
typo

Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
add https to oidc issuer example.

Signed-off-by: jonvnadelberg <121979961+jonvnadelberg@users.noreply.github.com>
@haydentherapper
Copy link
Contributor

@ltagliaferri Can you take another look?

@haydentherapper
Copy link
Contributor

Bumping for review

@ltagliaferri ltagliaferri merged commit b8fc14d into sigstore:main May 9, 2023
@jonvnadelberg jonvnadelberg deleted the docs_move_keyless_info_to_overview branch June 8, 2023 14:50
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.

docs: Integrate some content of keyless.md into overview.md
5 participants