From c4250780085ec746a02fc570767cc2fcc2d6bcbe Mon Sep 17 00:00:00 2001 From: Anton Averchenkov <84287187+averche@users.noreply.github.com> Date: Fri, 11 Mar 2022 19:00:26 -0500 Subject: [PATCH] Change OpenAPI code generator to extract request objects (#14217) --- changelog/14217.txt | 3 + sdk/framework/backend.go | 15 ++++- sdk/framework/openapi.go | 51 +++++++++++--- sdk/framework/openapi_test.go | 10 +-- sdk/framework/path.go | 10 ++- sdk/framework/testdata/legacy.json | 21 ++++-- sdk/framework/testdata/operations.json | 73 +++++++++++---------- sdk/framework/testdata/operations_list.json | 4 ++ sdk/framework/testdata/responses.json | 4 ++ vault/logical_system.go | 14 +++- vault/logical_system_test.go | 3 + 11 files changed, 150 insertions(+), 58 deletions(-) create mode 100644 changelog/14217.txt diff --git a/changelog/14217.txt b/changelog/14217.txt new file mode 100644 index 000000000..de42fca36 --- /dev/null +++ b/changelog/14217.txt @@ -0,0 +1,3 @@ +```release-note:improvement +sdk: Change OpenAPI code generator to extract request objects into /components/schemas and reference them by name. +``` diff --git a/sdk/framework/backend.go b/sdk/framework/backend.go index cdee46f48..ecb88f5c4 100644 --- a/sdk/framework/backend.go +++ b/sdk/framework/backend.go @@ -198,7 +198,7 @@ func (b *Backend) HandleRequest(ctx context.Context, req *logical.Request) (*log // If the path is empty and it is a help operation, handle that. if req.Path == "" && req.Operation == logical.HelpOperation { - return b.handleRootHelp() + return b.handleRootHelp(req) } // Find the matching route @@ -457,7 +457,7 @@ func (b *Backend) route(path string) (*Path, map[string]string) { return nil, nil } -func (b *Backend) handleRootHelp() (*logical.Response, error) { +func (b *Backend) handleRootHelp(req *logical.Request) (*logical.Response, error) { // Build a mapping of the paths and get the paths alphabetized to // make the output prettier. pathsMap := make(map[string]*Path) @@ -486,9 +486,18 @@ func (b *Backend) handleRootHelp() (*logical.Response, error) { return nil, err } + // Plugins currently don't have a direct knowledge of their own "type" + // (e.g. "kv", "cubbyhole"). It defaults to the name of the executable but + // can be overridden when the plugin is mounted. Since we need this type to + // form the request & response full names, we are passing it as an optional + // request parameter to the plugin's root help endpoint. If specified in + // the request, the type will be used as part of the request/response body + // names in the OAS document. + requestResponsePrefix := req.GetString("requestResponsePrefix") + // Build OpenAPI response for the entire backend doc := NewOASDocument() - if err := documentPaths(b, doc); err != nil { + if err := documentPaths(b, requestResponsePrefix, doc); err != nil { b.Logger().Warn("error generating OpenAPI", "error", err) } diff --git a/sdk/framework/openapi.go b/sdk/framework/openapi.go index fb760774b..87240ecc8 100644 --- a/sdk/framework/openapi.go +++ b/sdk/framework/openapi.go @@ -32,6 +32,9 @@ func NewOASDocument() *OASDocument { }, }, Paths: make(map[string]*OASPathItem), + Components: OASComponents{ + Schemas: make(map[string]*OASSchema), + }, } } @@ -78,9 +81,14 @@ func NewOASDocumentFromMap(input map[string]interface{}) (*OASDocument, error) { } type OASDocument struct { - Version string `json:"openapi" mapstructure:"openapi"` - Info OASInfo `json:"info"` - Paths map[string]*OASPathItem `json:"paths"` + Version string `json:"openapi" mapstructure:"openapi"` + Info OASInfo `json:"info"` + Paths map[string]*OASPathItem `json:"paths"` + Components OASComponents `json:"components"` +} + +type OASComponents struct { + Schemas map[string]*OASSchema `json:"schemas"` } type OASInfo struct { @@ -148,6 +156,7 @@ type OASMediaTypeObject struct { } type OASSchema struct { + Ref string `json:"$ref,omitempty"` Type string `json:"type,omitempty"` Description string `json:"description,omitempty"` Properties map[string]*OASSchema `json:"properties,omitempty"` @@ -204,9 +213,9 @@ var ( ) // documentPaths parses all paths in a framework.Backend into OpenAPI paths. -func documentPaths(backend *Backend, doc *OASDocument) error { +func documentPaths(backend *Backend, requestResponsePrefix string, doc *OASDocument) error { for _, p := range backend.Paths { - if err := documentPath(p, backend.SpecialPaths(), backend.BackendType, doc); err != nil { + if err := documentPath(p, backend.SpecialPaths(), requestResponsePrefix, backend.BackendType, doc); err != nil { return err } } @@ -215,7 +224,7 @@ func documentPaths(backend *Backend, doc *OASDocument) error { } // documentPath parses a framework.Path into one or more OpenAPI paths. -func documentPath(p *Path, specialPaths *logical.Paths, backendType logical.BackendType, doc *OASDocument) error { +func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix string, backendType logical.BackendType, doc *OASDocument) error { var sudoPaths []string var unauthPaths []string @@ -224,7 +233,7 @@ func documentPath(p *Path, specialPaths *logical.Paths, backendType logical.Back unauthPaths = specialPaths.Unauthenticated } - // Convert optional parameters into distinct patterns to be process independently. + // Convert optional parameters into distinct patterns to be processed independently. paths := expandPattern(p.Pattern) for _, path := range paths { @@ -358,10 +367,12 @@ func documentPath(p *Path, specialPaths *logical.Paths, backendType logical.Back // Set the final request body. Only JSON request data is supported. if len(s.Properties) > 0 || s.Example != nil { + requestName := constructRequestName(requestResponsePrefix, path) + doc.Components.Schemas[requestName] = s op.RequestBody = &OASRequestBody{ Content: OASContent{ "application/json": &OASMediaTypeObject{ - Schema: s, + Schema: &OASSchema{Ref: fmt.Sprintf("#/components/schemas/%s", requestName)}, }, }, } @@ -459,6 +470,30 @@ func documentPath(p *Path, specialPaths *logical.Paths, backendType logical.Back return nil } +// constructRequestName joins the given prefix with the path elements into a +// CamelCaseRequest string. +// +// For example, prefix="kv" & path=/config/lease/{name} => KvConfigLeaseRequest +func constructRequestName(requestResponsePrefix string, path string) string { + var b strings.Builder + + b.WriteString(strings.Title(requestResponsePrefix)) + + // split the path by / _ - separators + for _, token := range strings.FieldsFunc(path, func(r rune) bool { + return r == '/' || r == '_' || r == '-' + }) { + // exclude request fields + if !strings.ContainsAny(token, "{}") { + b.WriteString(strings.Title(token)) + } + } + + b.WriteString("Request") + + return b.String() +} + func specialPathMatch(path string, specialPaths []string) bool { // Test for exact or prefix match of special paths. for _, sp := range specialPaths { diff --git a/sdk/framework/openapi_test.go b/sdk/framework/openapi_test.go index 7e514481d..fa14d2eb8 100644 --- a/sdk/framework/openapi_test.go +++ b/sdk/framework/openapi_test.go @@ -271,7 +271,7 @@ func TestOpenAPI_SpecialPaths(t *testing.T) { Root: test.rootPaths, Unauthenticated: test.unauthPaths, } - err := documentPath(&path, sp, logical.TypeLogical, doc) + err := documentPath(&path, sp, "kv", logical.TypeLogical, doc) if err != nil { t.Fatal(err) } @@ -515,11 +515,11 @@ func TestOpenAPI_OperationID(t *testing.T) { for _, context := range []string{"", "bar"} { doc := NewOASDocument() - err := documentPath(path1, nil, logical.TypeLogical, doc) + err := documentPath(path1, nil, "kv", logical.TypeLogical, doc) if err != nil { t.Fatal(err) } - err = documentPath(path2, nil, logical.TypeLogical, doc) + err = documentPath(path2, nil, "kv", logical.TypeLogical, doc) if err != nil { t.Fatal(err) } @@ -579,7 +579,7 @@ func TestOpenAPI_CustomDecoder(t *testing.T) { } docOrig := NewOASDocument() - err := documentPath(p, nil, logical.TypeLogical, docOrig) + err := documentPath(p, nil, "kv", logical.TypeLogical, docOrig) if err != nil { t.Fatal(err) } @@ -642,7 +642,7 @@ func testPath(t *testing.T, path *Path, sp *logical.Paths, expectedJSON string) t.Helper() doc := NewOASDocument() - if err := documentPath(path, sp, logical.TypeLogical, doc); err != nil { + if err := documentPath(path, sp, "kv", logical.TypeLogical, doc); err != nil { t.Fatal(err) } doc.CreateOperationIDs("") diff --git a/sdk/framework/path.go b/sdk/framework/path.go index 4dc8ca303..b316d2cc1 100644 --- a/sdk/framework/path.go +++ b/sdk/framework/path.go @@ -301,9 +301,17 @@ func (p *Path) helpCallback(b *Backend) OperationFunc { return nil, errwrap.Wrapf("error executing template: {{err}}", err) } + // The plugin type (e.g. "kv", "cubbyhole") is only assigned at the time + // the plugin is enabled (mounted). If specified in the request, the type + // will be used as part of the request/response names in the OAS document + var requestResponsePrefix string + if v, ok := req.Data["requestResponsePrefix"]; ok { + requestResponsePrefix = v.(string) + } + // Build OpenAPI response for this path doc := NewOASDocument() - if err := documentPath(p, b.SpecialPaths(), b.BackendType, doc); err != nil { + if err := documentPath(p, b.SpecialPaths(), requestResponsePrefix, b.BackendType, doc); err != nil { b.Logger().Warn("error generating OpenAPI", "error", err) } diff --git a/sdk/framework/testdata/legacy.json b/sdk/framework/testdata/legacy.json index 2fcb26882..cb1f7ebd3 100644 --- a/sdk/framework/testdata/legacy.json +++ b/sdk/framework/testdata/legacy.json @@ -41,13 +41,7 @@ "content": { "application/json": { "schema": { - "type": "object", - "properties": { - "token": { - "type": "string", - "description": "My token" - } - } + "$ref": "#/components/schemas/KvLookupRequest" } } } @@ -59,6 +53,19 @@ } } } + }, + "components": { + "schemas": { + "KvLookupRequest": { + "type": "object", + "properties": { + "token": { + "type": "string", + "description": "My token" + } + } + } + } } } diff --git a/sdk/framework/testdata/operations.json b/sdk/framework/testdata/operations.json index 4c140f92b..94d54087b 100644 --- a/sdk/framework/testdata/operations.json +++ b/sdk/framework/testdata/operations.json @@ -66,39 +66,7 @@ "content": { "application/json": { "schema": { - "type": "object", - "required": ["age"], - "properties": { - "flavors": { - "type": "array", - "description": "the flavors", - "items": { - "type": "string" - } - }, - "age": { - "type": "integer", - "description": "the age", - "enum": [1, 2, 3], - "x-vault-displayAttrs": { - "name": "Age", - "sensitive": true, - "group": "Some Group", - "value": 7 - } - }, - "name": { - "type": "string", - "description": "the name", - "default": "Larry", - "pattern": "\\w([\\w-.]*\\w)?" - }, - "x-abc-token": { - "type": "string", - "description": "a header value", - "enum": ["a", "b", "c"] - } - } + "$ref": "#/components/schemas/KvFooRequest" } } } @@ -110,5 +78,44 @@ } } } + }, + "components": { + "schemas": { + "KvFooRequest": { + "type": "object", + "required": ["age"], + "properties": { + "flavors": { + "type": "array", + "description": "the flavors", + "items": { + "type": "string" + } + }, + "age": { + "type": "integer", + "description": "the age", + "enum": [1, 2, 3], + "x-vault-displayAttrs": { + "name": "Age", + "sensitive": true, + "group": "Some Group", + "value": 7 + } + }, + "name": { + "type": "string", + "description": "the name", + "default": "Larry", + "pattern": "\\w([\\w-.]*\\w)?" + }, + "x-abc-token": { + "type": "string", + "description": "a header value", + "enum": ["a", "b", "c"] + } + } + } + } } } diff --git a/sdk/framework/testdata/operations_list.json b/sdk/framework/testdata/operations_list.json index bea40f61a..e89622a3c 100644 --- a/sdk/framework/testdata/operations_list.json +++ b/sdk/framework/testdata/operations_list.json @@ -59,5 +59,9 @@ ] } } + }, + "components": { + "schemas": { + } } } diff --git a/sdk/framework/testdata/responses.json b/sdk/framework/testdata/responses.json index bd332054c..b0e197bab 100644 --- a/sdk/framework/testdata/responses.json +++ b/sdk/framework/testdata/responses.json @@ -46,6 +46,10 @@ } } } + }, + "components": { + "schemas": { + } } } diff --git a/vault/logical_system.go b/vault/logical_system.go index 3f8d04459..84566349c 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -4027,7 +4027,13 @@ func (b *SystemBackend) pathInternalOpenAPI(ctx context.Context, req *logical.Re doc := framework.NewOASDocument() procMountGroup := func(group, mountPrefix string) error { - for mount := range resp.Data[group].(map[string]interface{}) { + for mount, entry := range resp.Data[group].(map[string]interface{}) { + + var pluginType string + if t, ok := entry.(map[string]interface{})["type"]; ok { + pluginType = t.(string) + } + backend := b.Core.router.MatchingBackend(ctx, mountPrefix+mount) if backend == nil { @@ -4037,6 +4043,7 @@ func (b *SystemBackend) pathInternalOpenAPI(ctx context.Context, req *logical.Re req := &logical.Request{ Operation: logical.HelpOperation, Storage: req.Storage, + Data: map[string]interface{}{"requestResponsePrefix": pluginType}, } resp, err := backend.HandleRequest(ctx, req) @@ -4092,6 +4099,11 @@ func (b *SystemBackend) pathInternalOpenAPI(ctx context.Context, req *logical.Re doc.Paths["/"+mountPrefix+mount+path] = obj } + + // Merge backend schema components + for e, schema := range backendDoc.Components.Schemas { + doc.Components.Schemas[e] = schema + } } return nil } diff --git a/vault/logical_system_test.go b/vault/logical_system_test.go index 4d8fa62ec..5775a2b40 100644 --- a/vault/logical_system_test.go +++ b/vault/logical_system_test.go @@ -3404,6 +3404,9 @@ func TestSystemBackend_OpenAPI(t *testing.T) { }, }, "paths": map[string]interface{}{}, + "components": map[string]interface{}{ + "schemas": map[string]interface{}{}, + }, } if diff := deep.Equal(oapi, exp); diff != nil {