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

Enterprise-only endpoints have incorrect generated client functions #208

Closed
maxb opened this issue Jul 25, 2023 · 6 comments
Closed

Enterprise-only endpoints have incorrect generated client functions #208

maxb opened this issue Jul 25, 2023 · 6 comments

Comments

@maxb
Copy link
Contributor

maxb commented Jul 25, 2023

Currently, this client library is generated from an OpenAPI spec fetched from Vault OSS.

That has been tremendously helpful to me as a community contributor, as otherwise I'd be significantly impeded in doing local development.

Vault OSS now has stub definitions of Enterprise-only APIs for the sys/ backend only. Originally, I think the intent was so that client code would get a more useful error over the network when executing an Enterprise-only operation against an OSS server. But, the presence of the stubs mean the endpoints also become visible in OpenAPI. However the stubs do not contain enough information to generate correct client methods.

Options:

  • Switch to generating the library using Enterprise OpenAPI spec. Easy for HashiCorp to implement, but would put up a wall against community contributors like me.

  • Take the Enterprise-only endpoints out of the client library for now. We could do this easily by changing the stub code to add an enterprise-stub operation ID suffix, and excluding it.

  • Expand complexity of stubs to include copies of more of the framework.Path struct fields from Enterprise. Ideally everything except the callbacks.

@averche
Copy link
Collaborator

averche commented Jul 25, 2023

The longer term vision is to automate the generation & syncing of the OpenAPI spec into the libraries along with the true enterprise endpoints (which still need to be cleaned up with proper display attributes). In the short term, we can just exclude them as unsupported.

Take the Enterprise-only endpoints out of the client library for now. We could do this easily by changing the stub code to add an enterprise-stub operation ID suffix, and excluding it.

I like this idea! Probably better to set the OperationPrefix instead though as setting the suffix will lead to conflicting operation ID's.

@maxb
Copy link
Contributor Author

maxb commented Jul 25, 2023

setting the suffix will lead to conflicting operation ID's.

Um.... how? Prefix and suffix should be equivalent for this purpose, and we're already doing other excludes via the suffix (2, 3, etc.)

@averche
Copy link
Collaborator

averche commented Jul 25, 2023

When unset, the OperationSuffix will default to lower-case hyphenated path string, which, in combination with the unique verb is pretty much guaranteed to be unique. If the suffix is overwritten with a single sting (e.g. "enterprise-stub", it will likely result in collisions between different paths).

Unless you intended to append "enterprise-stub" to the existing suffixes, then it should work and there will be no additional collisions.

@maxb
Copy link
Contributor Author

maxb commented Jul 25, 2023

Unless you intended to append "enterprise-stub" to the existing suffixes, then it should work and there will be no additional collisions.

Ah, right. That's what I was envisaging, but I now see the point that it might be a bit tricky to compute the implicit value to append the additional suffix to it. I'll see what can be done.

@maxb
Copy link
Contributor Author

maxb commented Jul 26, 2023

You are right, doing this as a prefix is much easier - it's not worth the extra code to make this a suffix.

averche pushed a commit to hashicorp/vault that referenced this issue Jul 27, 2023
…tubs (#22072)

* Add a distinguishing prefix to OpenAPI operation IDs for enterprise stubs

As discussed in hashicorp/vault-client-go#208

* Dropping system from operation IDs per PR feedback.
@maxb
Copy link
Contributor Author

maxb commented Jul 27, 2023

Enterprise stub-based functions have now been removed from the library.

@maxb maxb closed this as completed Jul 27, 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

No branches or pull requests

2 participants