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

Fix gen_openapi.sh script to load plugins #17752

Merged
merged 3 commits into from
Nov 1, 2022
Merged

Conversation

averche
Copy link
Contributor

@averche averche commented Oct 31, 2022

The current gen_openapi.sh has been broken since #16846, which changed the format of registry.go. This PR fixes the regular expressions used to parse the file.

Note: this is a temporary solution to fix the immediate failure and the script is still very brittle. In the future, a better solution should be considered (e.g. introduce a new endpoint to return the contents of registry.go)

Fixes: VAULT-9672

@averche averche requested a review from a team October 31, 2022 21:48
Copy link
Contributor

@dhuckins dhuckins left a comment

Choose a reason for hiding this comment

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

lgtm!

this is a lot of bash tho... (and starting the server is interesting) have we considered moving this script to go?

@averche
Copy link
Contributor Author

averche commented Nov 1, 2022

this is a lot of bash tho... (and starting the server is interesting) have we considered moving this script to go?

I'm not a big fan of this script. Moving some or all of it into Go would be ideal. I think we are starting vault so that we can hit the openapi endpoint. Perhaps the same can be accomplished with NewTestCluster() or maybe in docker?

@dhuckins
Copy link
Contributor

dhuckins commented Nov 1, 2022

so the actual spec is generated on the fly?
(how do you find endpoints in vault?)

@averche
Copy link
Contributor Author

averche commented Nov 1, 2022

Yes, it is generated on the fly. I believe a lot of the path/endpoint logic is in this funciton.

@averche averche enabled auto-merge (squash) November 1, 2022 21:10
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.

2 participants