Change OpenAPI code generator to extract request objects (#14217)

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.
+```

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)
 	}
 

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"`
+	Version    string                  `json:"openapi" mapstructure:"openapi"`
-	Info    OASInfo                 `json:"info"`
+	Info       OASInfo                 `json:"info"`
-	Paths   map[string]*OASPathItem `json:"paths"`
+	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 {

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("")

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)
 		}
 

sdk/framework/testdata/legacy.json

@@ -41,13 +41,7 @@
           "content": {
             "application/json": {
               "schema": {
-                "type": "object",
+                "$ref": "#/components/schemas/KvLookupRequest"
-                "properties": {
-                  "token": {
-                    "type": "string",
-                    "description": "My token"
-                  }
-                }
               }
             }
           }
@@ -59,6 +53,19 @@
         }
       }
     }
+  },
+  "components": {
+    "schemas": {
+      "KvLookupRequest": {
+        "type": "object",
+        "properties": {
+          "token": {
+            "type": "string",
+            "description": "My token"
+          }
+        }
+      }
+    }
   }
 }
 

sdk/framework/testdata/operations.json

@@ -66,39 +66,7 @@
           "content": {
             "application/json": {
               "schema": {
-                "type": "object",
+                "$ref": "#/components/schemas/KvFooRequest"
-                "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"]
-                  }
-                }
               }
             }
           }
@@ -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"]
+          }
+        }
+      }
+    }
   }
 }

sdk/framework/testdata/operations_list.json

@@ -59,5 +59,9 @@
         ]
       }
     }
+  },
+  "components": {
+    "schemas": {
+    }
   }
 }

sdk/framework/testdata/responses.json

@@ -46,6 +46,10 @@
         }
       }
     }
+  },
+  "components": {
+    "schemas": {
+    }
   }
 }
 

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
 	}

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 {