Improve OpenAPI request/response naming strategy

changelog/18321.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+openapi: Improve request/response naming strategy
+```

sdk/framework/backend.go

@@ -530,14 +530,9 @@ func (b *Backend) handleRootHelp(req *logical.Request) (*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")
+	// If specified in the request, the mount path will be used as part of the
+	// request/response body names and operation id's in the OpenAPI document.
+	mountPathWithPrefix := req.GetString("mount_path_with_prefix")
 
 	// Build OpenAPI response for the entire backend
 	vaultVersion := "unknown"
@@ -550,7 +545,7 @@ func (b *Backend) handleRootHelp(req *logical.Request) (*logical.Response, error
 	}
 
 	doc := NewOASDocument(vaultVersion)
-	if err := documentPaths(b, requestResponsePrefix, doc); err != nil {
+	if err := documentPaths(b, mountPathWithPrefix, doc); err != nil {
 		b.Logger().Warn("error generating OpenAPI", "error", err)
 	}
 

sdk/framework/openapi.go

@@ -215,9 +215,9 @@ var (
 )
 
 // documentPaths parses all paths in a framework.Backend into OpenAPI paths.
-func documentPaths(backend *Backend, requestResponsePrefix string, doc *OASDocument) error {
+func documentPaths(backend *Backend, mountPathWithPrefix string, doc *OASDocument) error {
 	for _, p := range backend.Paths {
-		if err := documentPath(p, backend.SpecialPaths(), requestResponsePrefix, backend.BackendType, doc); err != nil {
+		if err := documentPath(p, backend.SpecialPaths(), mountPathWithPrefix, backend.BackendType, doc); err != nil {
 			return err
 		}
 	}
@@ -226,7 +226,7 @@ func documentPaths(backend *Backend, requestResponsePrefix string, doc *OASDocum
 }
 
 // documentPath parses a framework.Path into one or more OpenAPI paths.
-func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix string, backendType logical.BackendType, doc *OASDocument) error {
+func documentPath(p *Path, specialPaths *logical.Paths, mountPathWithPrefix string, backendType logical.BackendType, doc *OASDocument) error {
 	var sudoPaths []string
 	var unauthPaths []string
 
@@ -265,12 +265,8 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st
 		// Body fields will be added to individual operations.
 		pathFields, bodyFields := splitFields(p.Fields, path)
 
-		defaultMountPath := requestResponsePrefix
-		if requestResponsePrefix == "kv" {
-			defaultMountPath = "secret"
-		}
-
-		if defaultMountPath != "system" && defaultMountPath != "identity" {
+		if mountPathWithPrefix != "sys" && mountPathWithPrefix != "identity" {
+			defaultMountPath := mountPathWithPrefix[strings.LastIndex(mountPathWithPrefix, "/")+1:]
 			p := OASParameter{
 				Name:        fmt.Sprintf("%s_mount_path", defaultMountPath),
 				Description: "Path where the backend was mounted; the endpoint path will be offset by the mount path",
@@ -324,6 +320,12 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st
 		// Process each supported operation by building up an Operation object
 		// with descriptions, properties and examples from the framework.Path data.
 		for opType, opHandler := range operations {
+			operationID, updateOperationOnly := constructOperationID(opType, mountPathWithPrefix, path)
+
+			if updateOperationOnly && opType != logical.UpdateOperation {
+				continue
+			}
+
 			props := opHandler.Properties()
 			if props.Unpublished {
 				continue
@@ -348,6 +350,7 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st
 			op.Summary = props.Summary
 			op.Description = props.Description
 			op.Deprecated = props.Deprecated
+			op.OperationID = operationID
 
 			// Add any fields not present in the path as body parameters for POST.
 			if opType == logical.CreateOperation || opType == logical.UpdateOperation {
@@ -395,7 +398,7 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st
 
 				// Set the final request body. Only JSON request data is supported.
 				if len(s.Properties) > 0 || s.Example != nil {
-					requestName := constructRequestResponseName(path, requestResponsePrefix, "Request")
+					requestName := operationID + "Request"
 					doc.Components.Schemas[requestName] = s
 					op.RequestBody = &OASRequestBody{
 						Required: true,
@@ -502,7 +505,7 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st
 					}
 
 					if len(resp.Fields) != 0 {
-						responseName := constructRequestResponseName(path, requestResponsePrefix, "Response")
+						responseName := operationID + "Response"
 						doc.Components.Schemas[responseName] = responseSchema
 						content = OASContent{
 							"application/json": &OASMediaTypeObject{
@@ -534,31 +537,73 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st
 	return nil
 }
 
-// constructRequestResponseName joins the given path with prefix & suffix into
-// a CamelCase request or response name.
+// constructOperationID joins the given inputs into a title case operationID,
+// which is also used as a root for request and response names.
 //
-// For example, path=/config/lease/{name}, prefix="secret", suffix="request"
-// will result in "SecretConfigLeaseRequest"
-func constructRequestResponseName(path, prefix, suffix string) string {
-	var b strings.Builder
+// For many mount + path combinations, which would otherwise result in
+// difficult-to-read names, the function uses a custom lookup table to
+// construct the name instead.
+//
+// Examples of generated operation ID's:
+//   - KVv2Write
+//   - KVv2Read
+//   - GoogleCloudLogin
+//   - GoogleCloudWriteRole
+//
+// The function also returns "update operation only" indicator, it is set to
+// true if the operation ID should result in a single update (POST) operation
+// for this path. For example, the following 3 operations, will be collapsed
+// into a single operation "Deregister" and will be mapped into a single
+// update (POST) operation:
+//
+//	   PUT /v1/gcpkms/keys/deregister/:key
+//	  POST /v1/gcpkms/keys/deregister/:key
+//	DELETE /v1/gcpkms/keys/deregister/:key
+func constructOperationID(operation logical.Operation, mount, path string) (string, bool) {
+	// For most request names "write" seems to be more applicable in place of "update"
+	var operationStr string
+	if operation == logical.UpdateOperation {
+		operationStr = "write"
+	} else {
+		operationStr = string(operation)
+	}
 
-	title := cases.Title(language.English)
+	updateOperationOnly := false
 
-	b.WriteString(title.String(prefix))
+	var parts []string
 
-	// 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(title.String(token))
+	if mapping, ok := knownPathMappings[knownPathKey{mount: mount, path: path}]; ok {
+		// If the mapping exists, construct a [prefix][operation][suffix] name
+		if mapping.prefix != "" {
+			parts = append(parts, mapping.prefix)
 		}
+
+		if mapping.operation != "" {
+			parts = append(parts, mapping.operation)
+		} else {
+			parts = append(parts, nonWordRe.Split(strings.ToLower(operationStr), -1)...)
+		}
+
+		if mapping.suffix != "" {
+			parts = append(parts, mapping.suffix)
+		}
+
+		updateOperationOnly = mapping.updateOperationOnly
+	} else {
+		// Else, fall back to [mount][operation][path] split by non-word characters
+		parts = append(parts, nonWordRe.Split(strings.ToLower(mount), -1)...)
+		parts = append(parts, nonWordRe.Split(strings.ToLower(operationStr), -1)...)
+		parts = append(parts, nonWordRe.Split(strings.ToLower(path), -1)...)
 	}
 
-	b.WriteString(suffix)
+	// Title case everything & join the result into a string
+	title := cases.Title(language.English, cases.NoLower)
 
-	return b.String()
+	for i, s := range parts {
+		parts[i] = title.String(s)
+	}
+
+	return strings.Join(parts, ""), updateOperationOnly
 }
 
 func specialPathMatch(path string, specialPaths []string) bool {
@@ -784,6 +829,9 @@ func cleanResponse(resp *logical.Response) *cleanedResponse {
 //	postSysToolsRandomUrlbytes_2
 //
 // An optional user-provided suffix ("context") may also be appended.
+//
+// Deprecated: operationID's are now populated using constructOperationID();
+// This function is here for backwards compatibility with older plugins
 func (d *OASDocument) CreateOperationIDs(context string) {
 	// title caser
 	title := cases.Title(language.English)
@@ -814,6 +862,10 @@ func (d *OASDocument) CreateOperationIDs(context string) {
 				continue
 			}
 
+			if oasOperation.OperationID != "" {
+				continue
+			}
+
 			// Discard "_mount_path" from any {thing_mount_path} parameters
 			path = strings.Replace(path, "_mount_path", "", 1)