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
Merged
3 changes: 3 additions & 0 deletions changelog/14217.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
sdk: Change OpenAPI code generator to extract request objects into /components/schemas and reference them by name.
```
15 changes: 12 additions & 3 deletions sdk/framework/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}

Expand Down
51 changes: 43 additions & 8 deletions sdk/framework/openapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

},
}
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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"`
Expand Down Expand Up @@ -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 {
averche marked this conversation as resolved.
Show resolved Hide resolved
return err
}
}
Expand All @@ -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

Expand All @@ -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 {
Expand Down Expand Up @@ -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)},
},
},
}
Expand Down Expand Up @@ -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 {
Expand Down
10 changes: 5 additions & 5 deletions sdk/framework/openapi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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("")
Expand Down
10 changes: 9 additions & 1 deletion sdk/framework/path.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
21 changes: 14 additions & 7 deletions sdk/framework/testdata/legacy.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,7 @@
"content": {
"application/json": {
"schema": {
"type": "object",
"properties": {
"token": {
"type": "string",
"description": "My token"
}
}
"$ref": "#/components/schemas/KvLookupRequest"
}
}
}
Expand All @@ -59,6 +53,19 @@
}
}
}
},
"components": {
"schemas": {
"KvLookupRequest": {
"type": "object",
"properties": {
"token": {
"type": "string",
"description": "My token"
}
}
}
}
}
}

73 changes: 40 additions & 33 deletions sdk/framework/testdata/operations.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
}
Expand All @@ -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"]
}
}
}
}
}
}
4 changes: 4 additions & 0 deletions sdk/framework/testdata/operations_list.json
Original file line number Diff line number Diff line change
Expand Up @@ -59,5 +59,9 @@
]
}
}
},
"components": {
"schemas": {
}
}
}
4 changes: 4 additions & 0 deletions sdk/framework/testdata/responses.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@
}
}
}
},
"components": {
"schemas": {
}
}
}

14 changes: 13 additions & 1 deletion vault/logical_system.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
Expand Down Expand Up @@ -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
}
Expand Down
3 changes: 3 additions & 0 deletions vault/logical_system_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3368,6 +3368,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 {
Expand Down