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

Add additional endpoints, remove non-protected endpoints #20669

Merged
merged 7 commits into from
May 24, 2023

Conversation

jonathanfrappier
Copy link
Contributor

@jonathanfrappier jonathanfrappier commented May 18, 2023

🎟️ Jira ticket

πŸ” Deploy preview

Description

  • sys/auth/:path/tune is listed
    sys/auth/:path/tune | GET, POST, DELETE | Manage the auth methods (enable, read, delete, and tune)
  • Removed - auth/token/create-orphan is not a root-protected endpoint.
  • Added missing / - There is a slash missing from sys/config/auditing/request-headers:name
  • Added - sys/config-ui is not a valid endpoint. This should be sys/config/ui/headers
  • Added - sys/config/ui/headers/:name
  • Removed - sys/internal/specs/openapi is not root protected!
  • Removed - sys/step-down is listed as root protected but is not
  • Added - sys/remount is root protected but missing from the table
  • Added - sys/storage/raft/snapshot-auto/config
  • Added - /sys/storage/raft/snapshot-auto/config/:name
  • Spot checked several verbs, including sys/raw which was pointed out as an example of being incorrect, but according to our documentation is correct in the table.

Update:

  • Added - sys/internal/inspect/router/
  • Already in table specific to endpoints requiring sudo - sys/leases
  • Opted to leave out since deprecated - sys/revoke-force/:prefix and sys/revoke-prefix/:prefix
  • Already in table - beyond scope of this specific doc for further explanation - auth/token/create

yhyakuna

This comment was marked as outdated.

@maxb

This comment was marked as outdated.

@maxb

This comment was marked as outdated.

@jonathanfrappier

This comment was marked as outdated.

maxb

This comment was marked as outdated.

@jonathanfrappier

This comment was marked as outdated.

@jonathanfrappier
Copy link
Contributor Author

Checked with engineering on open questions:

  • Updated sys/raw row
  • Updated sys/internal/inspect/router to specify :tag with required tags
  • dr/reindex & performance/reindex are not valid. replication/reindex already listed
  • sys/leases/lookup does not require sudo. sys/leases already listed
  • sys/plugins/catalog/:type/:name documented as intended, leaving table to match.

@jonathanfrappier
Copy link
Contributor Author

I created a separate ticket to address auth/token/create. I have left it in the table currently as there are cases, according to the existing docs, that it could require sudo. New ticket shares your suggestions on additional clarity but need more time to research and collaborate with engineering.

Comment on lines +829 to +830
| [sys/raw:path](/vault/api-docs/system/raw) | GET, POST, DELETE | Used to access the raw underlying store in Vault |
| [sys/raw:prefix](/vault/api-docs/system/raw#list-raw) | GET, LIST | Returns a list keys for a given path prefix |
Copy link
Contributor

Choose a reason for hiding this comment

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

This new representation of sys/raw does not communicate well to users.

There is a single API, which can be best described as sys/raw/:path, accepting GET, LIST, POST, DELETE:

Suggested change
| [sys/raw:path](/vault/api-docs/system/raw) | GET, POST, DELETE | Used to access the raw underlying store in Vault |
| [sys/raw:prefix](/vault/api-docs/system/raw#list-raw) | GET, LIST | Returns a list keys for a given path prefix |
| [sys/raw/:path](/vault/api-docs/system/raw) | GET, LIST, POST, DELETE | Used to access the raw underlying store in Vault |

It doesn't make sense to single out LIST to a separate row, use different path-parameter placeholders for the two rows, duplicate GET over both of them, or omit the slash character before the colons.

Copy link
Contributor Author

@jonathanfrappier jonathanfrappier May 24, 2023

Choose a reason for hiding this comment

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

Thanks for checking in. I matched the table to our docs which list /sys/raw/:prefix and sys/raw:path on https://developer.hashicorp.com/vault/api-docs/system/raw

@jonathanfrappier jonathanfrappier merged commit c6970cd into main May 24, 2023
@jonathanfrappier jonathanfrappier deleted the docs/fix-root-protected-paths-policies branch May 24, 2023 21:32
yhyakuna pushed a commit that referenced this pull request May 24, 2023
* Add additional endpoints, remove non-protected endpoints

* Add step-down per engineering

* Match HTTP verb to individual doc pages

* Add /sys/internal/inspect/router to table

* Apply additional suggestions

* Updates based on engineering feedback

* Adding unsaved changes
yhyakuna added a commit that referenced this pull request May 24, 2023
…0765)

* Add additional endpoints, remove non-protected endpoints

* Add step-down per engineering

* Match HTTP verb to individual doc pages

* Add /sys/internal/inspect/router to table

* Apply additional suggestions

* Updates based on engineering feedback

* Adding unsaved changes

Co-authored-by: Jonathan Frappier <92055993+jonathanfrappier@users.noreply.github.com>
@maxb
Copy link
Contributor

maxb commented May 25, 2023

  • sys/leases/lookup does not require sudo.

Oops, my bad there - you're right, it doesn't, I misread the match patterns and data from the OpenAPI spec.

In order to track the remaining open issues, I've opened #20780

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.

3 participants