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

Change OpenAPI code generator to extract request objects #14217

Merged
merged 17 commits into from
Mar 12, 2022

Conversation

averche
Copy link
Contributor

@averche averche commented Feb 23, 2022

Current generated OpenAPI json

The current generated spec has request bodies anonymously defined inline:

        "requestBody": {
          "content": {
            "application/json": {
              "schema": {
                "type": "object",
                "properties": {
                  "token": {
                    "type": "string",
                    "description": "My token"
                  }
                }
              }
            }
          }

New generated OpenAPI spec:

The new generated spec has the request bodies extracted out under /components/schemas and referenced with $ref tags. The advantage of this style is that the spec can now be used for code generation. The generated request structs will have human-readable names (e.g. KvLookupRequest in the example below):

   { {
        "requestBody": {
          "content": {
            "application/json": {
              "schema": {
                "$ref": "#/components/schemas/KvLookupRequest"
                  }
                }
              }
            }
          }
       }
  "components": {
    "schemas": {
      "KvLookupRequest": {
        "type": "object",
        "properties": {
          "token": {
            "type": "string",
            "description": "My token"
          }
        }
      }
    }
  }

Implementation

The struct schemas are extracted individually for each plugin in openapi.go and then combined in vault/logical_system.go. Since the plugins don't have names (types) associated with them until they are mounted, we pass the plugin name to each plugin through its help endpoint within the "data" map.

@averche averche requested a review from a team February 23, 2022 16:25
@vercel vercel bot temporarily deployed to Preview – vault February 23, 2022 16:39 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook February 23, 2022 16:39 Inactive
scripts/gen_openapi.sh Outdated Show resolved Hide resolved
@vercel vercel bot temporarily deployed to Preview – vault-storybook February 23, 2022 18:08 Inactive
@vercel vercel bot temporarily deployed to Preview – vault February 23, 2022 18:08 Inactive
// the plugin is enabled (mounted). If specified in the request, the type
// will be used as part of the request/response body names in the OAS
// document
var requestResponsePrefix string
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a possibility of this variable staying as an empty string? And if so will that cause any problems with the spec generation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is possible if whoever calls the plugin's /help endpoint does not specify the "requestResponsePrefix". In this case, the generated request name will be missing that prefix. For example "KvLookupRequest" would be replaced with just "LookupRequest". I'm not really sure what the best way of dealing with this is (or if it should be of concern). In the OpenAPI generation workflow, the prefix will always be set.

@@ -32,6 +32,9 @@ func NewOASDocument() *OASDocument {
},
},
Paths: make(map[string]*OASPathItem),
Components: OASComponents{
Schemas: make(map[string]*OASSchema),
Copy link
Collaborator

@digivava digivava Feb 23, 2022

Choose a reason for hiding this comment

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

Good job utilizing this more modern OAS concept! (reference schema components)

sdk/framework/openapi.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@digivava digivava left a comment

Choose a reason for hiding this comment

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

Small typo in a comment but otherwise it looks good to me, good job thinking through how best to get the mount type in there.

@vercel vercel bot temporarily deployed to Preview – vault-storybook February 23, 2022 20:54 Inactive
@vercel vercel bot temporarily deployed to Preview – vault February 23, 2022 20:54 Inactive
Copy link
Contributor

@VinnyHC VinnyHC left a comment

Choose a reason for hiding this comment

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

This looks good to me! I briefly walked through the API Explorer in the FE and that also seems to be happy with your changes. 🎉

@vercel vercel bot temporarily deployed to Preview – vault-storybook February 25, 2022 20:54 Inactive
@vercel vercel bot temporarily deployed to Preview – vault February 25, 2022 20:54 Inactive
@vercel vercel bot temporarily deployed to Preview – vault February 25, 2022 21:01 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook February 25, 2022 21:01 Inactive
@averche averche marked this pull request as ready for review February 25, 2022 22:17
@averche averche requested a review from kalafut February 25, 2022 22:20
sdk/framework/backend.go Outdated Show resolved Hide resolved
@vercel vercel bot temporarily deployed to Preview – vault March 4, 2022 15:50 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook March 4, 2022 15:50 Inactive
@averche averche requested a review from kalafut March 4, 2022 16:26
sdk/framework/backend.go Outdated Show resolved Hide resolved
Copy link
Contributor

@kalafut kalafut left a comment

Choose a reason for hiding this comment

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

LGTM. Does the generated doc still pass the swagger.io inspector OK?

@averche
Copy link
Contributor Author

averche commented Mar 7, 2022

LGTM. Does the generated doc still pass the swagger.io inspector OK?

Yes, tested it in swagger.io 👍

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.

5 participants