OpenAPI defect introduced by extracting request/response bodies to schema references

[maxb]

Describe the bug
#14217 changed OpenAPI generation to extract the details of the JSON format of requests and responses, from being in-line with the relevant operations, to being defined in a separate portion of the document, and referenced by name.

Critically, the names assigned to these request/response schemas, are not sufficiently unique. This leads to some silently overwriting others, and generating an OpenAPI document that contains lies about actual requests/responses.

To Reproduce
Steps to reproduce the behavior:

1. Start up a vault server -dev - use a 1.13-dev make dev-ui build for this, as it seems 1.12 isn't showing responses in the API explorer at all.
2. Enable an approle auth method
3. Open up the API explorer
4. Observe the GET​ /auth​/{approle_mount_path}​/role/{role_name} endpoint, specifically that it claims its response will look identical to the GET​ /auth​/{approle_mount_path}​/role endpoint - these are entirely different endpoints, but the response for one has overwritten the other during OpenAPI generation.

Additional context
With the introduction of plugin versioning in Vault 1.12, there is now a new related problem - you could be running multiple different versions of a plugin, which have different parameters for their APIs. The current approach of seeking to flatten all request/response formats into named references becomes even more problematic - you'd have to disambiguate between different versions of request/response for the same endpoint in plugin mounts running different versions.

For these reasons, I believe the change to use request/response schemas should be reverted, returning to inline definition, which will more reliably associate the correct parameters with each endpoint.

[averche]

Hi @maxb! You are correct, this is indeed an issue with the current request/response naming function. I believe it should be resolved in #18321, which has a shared naming pattern for operationIds, request names and response names. For the examples you mentioned, the following operationIds and response names will be generated:

GET​ /auth​/{approle_mount_path}​/role
    OperationID: AppRoleListRoles
  Response name: AppRoleListRolesResponse
 
GET​ /auth​/{approle_mount_path}​/role/{role_name}`
    OperationID: AppRoleReadRole
  Response name: AppRoleReadRoleResponse

For a bit of background, we are working on generating client libraries from this OpenAPI spec. Without extracting request and response bodies (and giving them sensible names), openapi generators have no way of knowing what to call these request/response objects in the generated libraries. This results in anonymous structs/classes being generated for requests and responses with meaningless names like RequestStruct1.

[maxb]

Right - but that does need to be balanced with avoiding naming clashes, that result in the incorrect Request/Response objects being associated with some APIs. Let's continue the thread of conversation in #18321, to avoid splitting it between an issue and a PR.

[averche]

Hi @maxb! The #19319 (OpenAPI naming changes) and ~40 related PR's have been successfully merged for all the builtin plugins (both within this repo and in hashicorp/vault-plugin-*). As a result, all the OperationID's for built-in plugins and the corresponding request/response names are now unique:

GET​ /auth​/{approle_mount_path}​/role
    OperationID: app-role-list-roles
  Response name: AppRoleListRolesResponse
 
GET​ /auth​/{approle_mount_path}​/role/{role_name}`
    OperationID: app-role-read-role
  Response name: AppRoleReadRoleResponse

In general,

 request name = titleCase(operationID) + "Request"
response name = titleCase(operationID) + "Response"

Thanks again for bringing up this issue and contributing to the OpenAPI-related work!