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

Update API docs for PKI multi-issuer functionality #15238

Merged
merged 9 commits into from
May 11, 2022

Conversation

cipherboy
Copy link
Contributor

Signed-off-by: Alexander Scheel <[email protected]>

Copy link
Contributor

@kitography kitography left a comment

Choose a reason for hiding this comment

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

So, this is a draft, and my nits are probably unhelpful - you'll fix them anyway, I'm sure.

I guess I'm a little concerned that we are throwing a lot at someone who probably doesn't want to deal with multiple issuers (at least at first). It would make a lot of sense to me to include a tiny issuer section, maybe:

*) Read Issuer (sample Default)
*) Import Issuer / Generate Issuer
*) Set Default Issuer

And then either include a jump directly to Roles (the Issuing certificate section), or put that section first, and put a larger "advanced issuer configuration" later.

It would be cool if Revoke Certificate was paired with reading the CRL.

```

## Read Certificate
<a name="read-ca-certificate"></a>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we remove this, or what does this do? Nothing points to #read-ca-certificate anymore

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This allows bookmarks to the (existing, soon to be old) docs persist and link to the right thing in the new updated format. :-)

- [Sign Verbatim](#sign-verbatim)
- [Tidy](#tidy)
- [Tidy Status](#tidy-status)
- [Notice About New Multi-Issuer Functionality](#notice-about-new-multi-issuer-functionality)
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe I'm doing this wrong, but none of these links seem to work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you cd website && make it should launch a Vercel server that'll let you view it at e.g., http://localhost:3000/api-docs/secret/pki and the links definitely work for me. Maybe not all of them, but most of them definitely do and this one does.

I think GH's markdown rendering just doesn't do the linking the same as Vercel or maybe they don't render anchors for PR pages? Not sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

- [Delete Key](#delete-key)
- [Delete All Issuers and Keys](#delete-all-issuers-and-keys)
- [Managing Authority Information](#managing-authority-information)
- [List Roles](#list-roles)
Copy link
Contributor

Choose a reason for hiding this comment

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

list roles is listed twice (line 29)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this was intentional to give a logical grouping of information (list/create/read/delete as allotted by the usage) per section. The first-most section contains all the information about it and later ones refer back.

@cipherboy cipherboy force-pushed the cipherboy-pki-pod-rotation-docs branch from 6344dd2 to ab2b027 Compare May 10, 2022 13:22
cipherboy and others added 2 commits May 10, 2022 09:24
This substantially restructures the PKI secret engine's docs for two
purposes:

 1. To provide an explicit grouping of APIs by user usage and roles,
 2. To add all of the new APIs, hopefully with as minimal duplication
    as possible.

Signed-off-by: Alexander Scheel <[email protected]>
 - Add [1] links next to the DER/PEM format entries within various PKI
   response tables. These link to a new section explaining that the vault
   cli does not support DER/PEM response formats
 - Remove repetition of vault cli blurb in various description fields.
 - Fix up some typos
@cipherboy cipherboy force-pushed the cipherboy-pki-pod-rotation-docs branch from ab2b027 to f075c47 Compare May 10, 2022 15:29
@cipherboy cipherboy marked this pull request as ready for review May 10, 2022 15:30
@cipherboy cipherboy requested a review from taoism4504 as a code owner May 10, 2022 15:30
This was referenced Jun 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants