Proposal: Improved system for filtering operations out of the generated library

[maxb]

I propose that this project is in need of a nicer way to exclude specific operations from the generated library.

Currently, operationIds ending with 2 or 3 are filtered, but accomplishing this has required implementing the filter list in multiple ways, in many places:

vault-client-go/generate/templates/api.handlebars

Lines 20 to 23 in c9387ae

	{{#endsWith nickname "2"}}
	{{else}}
	{{#endsWith nickname "3"}}
	{{else}}

vault-client-go/generate/templates/api_doc.handlebars

Line 7 in c9387ae

{{#with operations}}{{#each operation}}{{#endsWith operationId "2"}}{{else}}{{#endsWith operationId "3"}}{{else}}[**{{operationId}}**]({{classname}}.md#{{operationId}}) | **{{httpMethod}}** {{path}} | {{summary}}

vault-client-go/generate/templates/api_doc.handlebars

Line 11 in c9387ae

{{#each operation}}{{#endsWith operationId "2"}}{{else}}{{#endsWith operationId "3"}}{{else}}

vault-client-go/.openapi-generator-ignore

Lines 7 to 15 in c9387ae

	model_*2_request.go
	model_*2_response.go
	model_*3_request.go
	model_*3_response.go
	docs/*2Request.md
	docs/*2Response.md
	docs/*3Request.md
	docs/*3Response.md

It is already very unappealing to extend this list further, and yet, there are more endpoints I think are in the interests of the library's users, to exclude, such as:

• sys/monitor - since this endpoint returns a streaming response, it cannot be realistically handled other than by custom hand written code
• Legacy deprecated Identity endpoints - these aren't even documented, and no-one would even know about them without looking at the code or OpenAPI - it doesn't make sense for these to appear in a modern Vault library
  • identity/alias
  • identity/alias/id
  • identity/alias/id/{id}
  • identity/persona
  • identity/persona/id
  • identity/persona/id/{id}

Other cases may arise.

Ideally we would just add these to a simple exclude list in a format that could be easily shared with vault-client-dotnet or future languages.

The upstream openapi-generator project does not have an answer for us. It only supports include lists for things to generate, not exclude lists, and then only as a single string, not reading from a file.

I propose, therefore, that we need to create a small filter script, that reads the raw OpenAPI spec fetched from Vault, selectively deletes operations by matching against an operationId exclude list, and feeds the filtered OpenAPI spec to openapi-generator.

Such a filter script could be very easily implemented in Python, using exclusively its built in standard library. Also, Python is a common language interpreter available in many developer environments. Therefore, Python feels like the right tool for the job for this filter script. Go is not an appealing tool for the job, as its builtin JSON library isn't well suited to dynamic processing, and insists on randomizing the order of object keys.

Please advise what you think of this, and whether it's worth progressing to a PR.

[averche]

Hello again, @maxb and thank you for the detailed proposal!

The exclusion of *2 and *3 suffixes was indeed a hacky fix introduced in #154. To add to the list of the endpoints you mentioned above, we also now have ACME endpoints under PKI, which are not supposed to be generated in the libraries.

Internally, we have been discussing adding yet another vendor field in the openapi spec at the operation level, e.g. x-vault-excludeFromCodeGeneration (similar to the existing x-vault-unauthenticated and x-vault-sudo). Alternatively, we could add the field to the existing x-vault-displayAttributes object.

I like your proposal as well! Once the OpenAPI spec is a bit more polished, we are planning to publish it in a separate repo with its own CI pipelines and two separate specs (a raw one and one specific to the generated vault-client-* libraries). The python script and the corresponding exclusion list could be migrated to that new repo once it is ready.

We'll have a further discussion in the team to determine which approach is best. Meanwhile, we'd be happy to hear your thoughts as well!

[maxb]

Internally, we have been discussing adding yet another vendor field in the openapi spec at the operation level, e.g. x-vault-excludeFromCodeGeneration

I have two concerns with this approach:

• It commits you to making a single include/exclude decision for all code generation purposes. Suppose that one day in the future you have released vault-client-go v1.0, and later some endpoints in Vault are deprecated (but still work). You may decide to retain the methods in vault-client-go until the next major version of the library, to preserve compatibility. At the same time you may be preparing to release vault-client-some-new-language v1.0. You may well not want to include deprecated methods in a brand new library, where no compatibility constraints exist.

• It commits you to a many-PR process for making changes... first update the plugin repo, second pull a new plugin version into Vault core, third pull a new OpenAPI spec into the library. I think it would be useful for the client library to have control, without needing to work through the release process of other repositories.

[averche]

It commits you to making a single include/exclude decision for all code generation purposes.

I think that's a desired outcome. We are moving towards a single source-of-truth OpenAPI document, whether it is generated by a python script or has flags to prevent generation.

Suppose that one day in the future you have released vault-client-go v1.0, and later some endpoints in Vault are deprecated (but still work). You may decide to retain the methods in vault-client-go until the next major version of the library, to preserve compatibility.

What you described applies well to a deprecation workflow. OpenAPI already supports deprecated: true at the operation level. We can use it to mark individual endpoints as deprecated (scheduled to be removed in the future). If / when they are removed at the OpenAPI level, we can release a new major version of all vault-client-* libraries.

The suggested x-vault-excludeFromCodeGeneration is less for the deprecated endpoints and more for duplicates (e.g. TokenLookupSelf2, TokenLookupSelf3) and operations we may want to deliberately exclude from all client libraries (e.g. Monitor, ACME-related endpoints, etc.) even though they are not deprecated.

At the same time you may be preparing to release vault-client-some-new-language v1.0. You may well not want to include deprecated methods in a brand new library, where no compatibility constraints exist.

I think it still could make sense to keep deprecated methods in a new library until they have been fully removed from the OpenAPI spec to keep the libraries consistent between each other.

It commits you to a many-PR process for making changes... first update the plugin repo, second pull a new plugin version into Vault core, third pull a new OpenAPI spec into the library. I think it would be useful for the client library to have control, without needing to work through the release process of other repositories.

Yes, this has been an issue we've been dealing with for populating the response structures and operation ID's. It does add more operational overhead but I think it may be justified in the end. The exclusion flag is not likely to change once set. We could also have logic in openapi.go to add the flag for all "*2", "*3" and "*acme*" endpoints at the time of generating the API (effectively maintain the exclusion list there), though this solution may be too flaky.