Skip to content

feat: add HCP Vault Secrets API docs #2050

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
Merged
zchsh merged 5 commits into from
Jul 11, 2023
Merged

feat: add HCP Vault Secrets API docs #2050

zchsh merged 5 commits into from
Jul 11, 2023

Conversation

@zchsh
Copy link
Contributor

@zchsh zchsh commented Jul 6, 2023 •
edited
Loading

🔗 Relevant links

🗒️ What

This PR adds API documentation for HCP Vault Secrets at /hcp/api-docs/vault-secrets.

🤷 Why

To better expose documentation for this API to users.

🛠️ How

Applies existing work with HCP Packer API docs to the analogous Swagger file for HCP Vault Secrets.

📸 Design Screenshots

Note: this is a net new page

localhost_3000_hcp_api-docs_vault-secrets

🧪 Testing

💭 Anything else?

  • For now, we've stuck to the existing API docs template, without further modifications
  • Intent is to favour rebuilding the API docs template from scratch over tweaking the current template

@vercel
Copy link

vercel bot commented Jul 6, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
dev-portal ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 11, 2023 2:46pm

@github-actions
Copy link

github-actions bot commented Jul 6, 2023 •
edited
Loading

📦 Next.js Bundle Analysis

This analysis was generated by the next.js bundle analysis action 🤖

New Page Added

The following page was added to the bundle from the code in this PR:

Page Size (compressed) First Load
/hcp/api-docs/vault-secrets/[[...page]] 153.49 KB 348.45 KB

@zchsh zchsh marked this pull request as ready for review July 6, 2023 13:05
@zchsh zchsh requested a review from a team as a code owner July 6, 2023 13:05
emilypersson1
emilypersson1 approved these changes Jul 6, 2023
View reviewed changes
Copy link
Contributor

@emilypersson1 emilypersson1 left a comment

Choose a reason for hiding this comment

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

nice, thank you!

@kartiklunkad26
Copy link

kartiklunkad26 commented Jul 6, 2023

@zchsh working on updating the title & tags/name but wanted to ask about the tags/name. What is the purpose of the sub-heading?

Is the intent that multiple sub-sections can exist? For the tags/name, honestly we wouldn't want a sub-heading at all

@zchsh
Copy link
Contributor Author

zchsh commented Jul 6, 2023

@kartiklunkad26 Great question! The intent behind showing that subheading is to display the service name, which made sense in the context of the Boundary API docs for which this template was originally built (see https://developer.hashicorp.com/boundary/api-docs/account-service)... but obviously isn't quite as applicable in this case!

If we'd like, I can remove the Secret Service heading as a one-off modification for the HCP Vault Secrets API docs 👍

@kartiklunkad26
Copy link

kartiklunkad26 commented Jul 6, 2023

Perfect!

@zchsh
Copy link
Contributor Author

zchsh commented Jul 6, 2023

@kartiklunkad26 I've update this PR to remove the subheading, and updated the screenshot in the PR description. Should take just a couple minutes for the preview to rebuild!

kendallstrautman
kendallstrautman reviewed Jul 6, 2023
View reviewed changes
src/views/api-docs-view/components/path-truncation-aside/index.tsx
* Intended to be used with a `<PathTruncationAside />` in each operation intro.
*/
function truncateVaultSecretsOperationPath(path: string) {
return path.replace(
Copy link
Contributor

@kendallstrautman kendallstrautman Jul 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

⛏️ Could we add a bit more clarity as to what the regex should match on in the comment?

Copy link
Contributor Author

@zchsh zchsh Jul 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

🙌 Great call! Added in a comment on this regex matching a standard prefix 👍

kendallstrautman
kendallstrautman reviewed Jul 6, 2023
View reviewed changes
kendallstrautman
kendallstrautman approved these changes Jul 6, 2023
View reviewed changes
Copy link
Contributor

@kendallstrautman kendallstrautman left a comment

Choose a reason for hiding this comment

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

LGTM!

zchsh and others added 2 commits July 6, 2023 12:16
@zchsh
docs: add comment on path truncation
0c5e64d
@zchsh @kendallstrautman
style: use cleaner serviceData.name check
955ae4b 
Co-authored-by: Kendall Strautman Swarthout <36613477+kendallstrautman@users.noreply.github.com>
@ovflowd ovflowd mentioned this pull request Jul 9, 2023
Merged
@zchsh zchsh merged commit 74e0b84 into main Jul 11, 2023
@zchsh zchsh deleted the zs.hcp-vault-secrets-api-docs branch July 11, 2023 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@emilypersson1 emilypersson1 emilypersson1 approved these changes

@kartiklunkad26 kartiklunkad26 Awaiting requested review from kartiklunkad26

+1 more reviewer

@kendallstrautman kendallstrautman kendallstrautman approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@zchsh @kartiklunkad26 @kendallstrautman @emilypersson1