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

Backport of Fix multiple OpenAPI generation issues with new AST-based generator into release/1.11.x #19717

Conversation

hc-github-team-secure-vault-core
Copy link
Collaborator

Backport

This PR is auto-generated from #18554 to be assessed for backporting due to the inclusion of the label backport/1.11.x.

WARNING automatic cherry-pick of commits failed. Commits will require human attention.

merge conflict error: POST https://api.github.com/repos/hashicorp/vault/merges: 409 Merge conflict []

The below text is copied from the body of the original PR.


Currently, the Vault OpenAPI generator attempts to use a variety of ad-hoc regexps, to simplify the path regexps that are the canonical specification of URL paths in Vault backends, and convert them into OpenAPI URL patterns. Regexps are not well suited to parsing regexps, and this approach is fragile and incomplete.

Happily, the Go regexp library provides easy access to its string-to-AST parser, so we can just ask it to parse the regexps for us, and then walk the resulting AST for a more reliable translation from regexps to OpenAPI patterns - this PR makes that change.

Whilst developing this change, it came to light that there are incorrectly unescaped full stops in some existing patterns - so the dot in /.well-known/ could in fact be any character. These are now properly escaped.

As a result of these changes, the OpenAPI document changes in the following ways:

In the AWS secrets engine, the /creds/{name} endpoint was being incorrectly documented as /creds - fixed (closes #18488).

In the GCP secrets engine, the /roleset and /static-account endpoints were missing - fixed.

In the KV v2 secrets engine, the /.* endpoint, which already had no visible operations since they are all marked as Unpublished, is now removed from the OpenAPI document entirely, since '/.* is not a valid OpenAPI path pattern.

In the PKI secrets engine, widespread inaccuracies are fixed (closes #18484):

The following endpoints, which were incorrectly undocumented, are added:

/cert/crl
/cert/delta-crl
/crl/delta
/crl/delta/pem
/crl/pem
/issuer/{issuer_ref}
/issuer/{issuer_ref}/crl
/issuer/{issuer_ref}/crl/delta
/issuer/{issuer_ref}/crl/delta/der
/issuer/{issuer_ref}/crl/delta/pem
/issuer/{issuer_ref}/crl/der
/issuer/{issuer_ref}/crl/pem
/issuer/{issuer_ref}/der
/issuer/{issuer_ref}/json
/issuer/{issuer_ref}/pem
/issuers/import/bundle
/issuers/import/cert
/keys/generate/exported
/keys/generate/internal
/keys/generate/kms

The following endpoints, which were documented but don't actually exist, are removed:

//delta
//delta/pem
//der
//json
//pem
/bundle
/cert
/delta-crl
/internal|exported
/kms
/{issuer_ref}/crl/pem|/der|/delta/pem
/{issuer_ref}/der|/pem

In the RADIUS auth method, /login endpoint no longer documents an incorrect urlusername field in the request body.

In the /sys/plugins/catalog/{type}/{name} endpoint, the type parameter is now no longer also incorrectly included in the request body.

In the /sys/tools/hash endpoints, the urlalgorithm parameter is now no longer incorrectly included in the request body.

In the /sys/tools/random and /transit/random endpoints, the source and urlbytes parameters are now no longer incorrectly included in the request body.

In various /transit/ endpoints, the urlalgorithm parameter is now no longer incorrectly included in the request body.

In the /transit/restore endpoints, the name parameter is now no longer incorrectly included in the request body.

These "parameter is now no longer incorrectly included in the request body" fixes are helpful, but accidental, and dependent on another bug - the request schema for multiple expansions for the same endpoint overwrites each other, with last generated wins, and the generation is simply happening in a more convenient order in this version of the code.


Overview of commits

@hashicorp-cla
Copy link

hashicorp-cla commented Mar 23, 2023

CLA assistant check
All committers have signed the CLA.

@hc-github-team-secure-vault-core hc-github-team-secure-vault-core force-pushed the backport/openapi-generation-regexp-syntax/deadly-lucky-tetra branch 2 times, most recently from 36709b9 to 9ef9b62 Compare March 23, 2023 17:15
…18554)

* Regexp metacharacter `.` should be escaped when used literally

The paths including `/.well-known/` in the Vault API could currently
technically be invoked with any random character in place of the dot.

* Replace implementation of OpenAPI path translator with regexp AST-based one

* Add changelog

* Typo fix from PR review - thanks!

Co-authored-by: Anton Averchenkov <84287187+averche@users.noreply.github.com>

* Add comment based on review feedback

* Change style of error handling as suggested in code review

* Make a further tweak to the handling of the error case

* Add more tests, testing cases which fail with the previous implementation

* Resolve issue with a test, and improve comment

---------

Co-authored-by: Anton Averchenkov <84287187+averche@users.noreply.github.com>
@averche averche force-pushed the backport/openapi-generation-regexp-syntax/deadly-lucky-tetra branch from edef8ef to e4380af Compare March 23, 2023 17:59
@averche averche marked this pull request as ready for review March 23, 2023 20:48
@averche averche enabled auto-merge (squash) March 23, 2023 21:11
@averche averche merged commit 5071c47 into release/1.11.x Mar 23, 2023
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.

4 participants