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

Improve OpenAPI request/response naming strategy #18321

Closed
wants to merge 32 commits into from
Closed
Changes from 1 commit
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
45aee60
Improve OpenAPI request/response naming strategy
averche Dec 12, 2022
691e29b
default mappings
averche Dec 12, 2022
1fad3a1
change the mapped names
averche Dec 14, 2022
ed954bf
add operationImplied flag
averche Dec 14, 2022
a1058a5
Add a test
averche Dec 14, 2022
7a9b75d
changelog
averche Dec 14, 2022
59a692f
requestResponsePrefix -> defaultMountPath
averche Dec 14, 2022
7a39ca9
update -> write
averche Dec 14, 2022
3b90f9a
fix test
averche Dec 14, 2022
91a5134
undo test change
averche Dec 14, 2022
19d5ef0
move the map into openapi_path_mappings.go
averche Dec 19, 2022
2375e5e
Update table with prefix/operation/suffix struct
averche Dec 20, 2022
0d14048
Fix function and test
averche Dec 20, 2022
4d5ccb9
test
averche Dec 20, 2022
ac7fe27
mountPathWithPrefix
averche Dec 20, 2022
0be96b8
mountPathWithPrefix
averche Dec 20, 2022
a0a65bf
comment
averche Dec 20, 2022
8e34fda
Adjust certain path mappings
averche Dec 21, 2022
77684f5
undo script changes
averche Dec 21, 2022
dc6fb71
Merge branch 'main' into ui/openapi-request-response-names
averche Dec 21, 2022
dd5767c
spaces
averche Dec 21, 2022
1e6bfe2
don't need path lib
averche Dec 21, 2022
70ec341
dedup
averche Dec 21, 2022
5165291
change KVv1/KVv2 naming structure
averche Jan 2, 2023
f1b79a1
Merge branch 'main' into ui/openapi-request-response-names
averche Jan 3, 2023
d8d4413
Add back CreateOperationIDs
averche Jan 4, 2023
9ad60e9
comment
averche Jan 4, 2023
66a741b
rename constructOperationID and address PR feedback
averche Jan 5, 2023
9088087
fix tests
averche Jan 5, 2023
f1d7eff
add updateOperationOnly flag
averche Jan 5, 2023
27da1e3
fix test
averche Jan 5, 2023
98dae14
remove kv -> secret hack
averche Jan 5, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Improve OpenAPI request/response naming strategy
  • Loading branch information
averche committed Dec 12, 2022
commit 45aee6053b2a74595713ff47b81782b5a2d597b9
133 changes: 48 additions & 85 deletions sdk/framework/openapi.go
Original file line number Diff line number Diff line change
@@ -239,6 +239,14 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st
paths := expandPattern(p.Pattern)

for _, path := range paths {

log.L().Warn(
fmt.Sprintf(
`"{prefix: "%s", path: "%s"}: "%s",`,
requestResponsePrefix,
path,
constructRequestIdentifier(logical.Operation(""), path, requestResponsePrefix, "")))

// Construct a top level PathItem which will be populated as the path is processed.
pi := OASPathItem{
Description: cleanString(p.HelpSynopsis),
@@ -395,7 +403,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 := constructRequestIdentifier(opType, path, requestResponsePrefix, "request")
doc.Components.Schemas[requestName] = s
op.RequestBody = &OASRequestBody{
Required: true,
@@ -502,7 +510,7 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st
}

if len(resp.Fields) != 0 {
responseName := constructRequestResponseName(path, requestResponsePrefix, "Response")
responseName := constructRequestIdentifier(opType, path, requestResponsePrefix, "response")
doc.Components.Schemas[responseName] = responseSchema
content = OASContent{
"application/json": &OASMediaTypeObject{
@@ -534,31 +542,50 @@ 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.
// When constructing a request or response name from a prefix + path, certain
// paths result in very long names or names with duplicate parts. For such paths,
// we use custom path mappings instead.
type customPathKey struct {
prefix string
path string
}

var customPathMappings = map[customPathKey]string{}

// constructRequestIdentifier joins the given inputs into a title case string,
// e.g. 'UpdateSecretConfigLeaseRequest'. This function is used to generate:
//
// 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
// - operation id
// - request name
// - response name
//
// For certain prefix + path combinations, which would otherwise result in an
// ugly string, the function uses a custom lookup table to construct part of
// the string instead.
func constructRequestIdentifier(operation logical.Operation, path, prefix, suffix string) string {
var parts []string

// Split the operation by non-word characters
parts = append(parts, nonWordRe.Split(strings.ToLower(string(operation)), -1)...)

// Append either the known mapping or prefix + path split by non-word characters
if mapping, ok := customPathMappings[customPathKey{prefix: prefix, path: path}]; ok {
parts = append(parts, mapping)
} else {
parts = append(parts, nonWordRe.Split(strings.ToLower(prefix), -1)...)
parts = append(parts, nonWordRe.Split(strings.ToLower(path), -1)...)
}

title := cases.Title(language.English)
parts = append(parts, suffix)

b.WriteString(title.String(prefix))
// Title case everything & join the result into a string
title := cases.Title(language.English)

// 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))
}
for i, s := range parts {
parts[i] = title.String(s)
}

b.WriteString(suffix)

return b.String()
return strings.Join(parts, "")
}

func specialPathMatch(path string, specialPaths []string) bool {
@@ -773,67 +800,3 @@ func cleanResponse(resp *logical.Response) *cleanedResponse {
Headers: resp.Headers,
}
}

// CreateOperationIDs generates unique operationIds for all paths/methods.
// The transform will convert path/method into camelcase. e.g.:
//
// /sys/tools/random/{urlbytes} -> postSysToolsRandomUrlbytes
//
// In the unlikely case of a duplicate ids, a numeric suffix is added:
//
// postSysToolsRandomUrlbytes_2
//
// An optional user-provided suffix ("context") may also be appended.
func (d *OASDocument) CreateOperationIDs(context string) {
// title caser
title := cases.Title(language.English)

opIDCount := make(map[string]int)
var paths []string

// traverse paths in a stable order to ensure stable output
for path := range d.Paths {
paths = append(paths, path)
}
sort.Strings(paths)

for _, path := range paths {
pi := d.Paths[path]
for _, method := range []string{"get", "post", "delete"} {
var oasOperation *OASOperation
switch method {
case "get":
oasOperation = pi.Get
case "post":
oasOperation = pi.Post
case "delete":
oasOperation = pi.Delete
}

if oasOperation == nil {
continue
}

// Discard "_mount_path" from any {thing_mount_path} parameters
path = strings.Replace(path, "_mount_path", "", 1)

// Space-split on non-words, title case everything, recombine
opID := nonWordRe.ReplaceAllString(strings.ToLower(path), " ")
opID = title.String(opID)
opID = method + strings.ReplaceAll(opID, " ", "")

// deduplicate operationIds. This is a safeguard, since generated IDs should
// already be unique given our current path naming conventions.
opIDCount[opID]++
if opIDCount[opID] > 1 {
opID = fmt.Sprintf("%s_%d", opID, opIDCount[opID])
}

if context != "" {
opID += "_" + context
}

oasOperation.OperationID = opID
}
}
}
13 changes: 2 additions & 11 deletions vault/logical_system.go
Original file line number Diff line number Diff line change
@@ -4434,19 +4434,12 @@ func (b *SystemBackend) pathInternalOpenAPI(ctx context.Context, req *logical.Re
return nil, err
}

context := d.Get("context").(string)

// Set up target document and convert to map[string]interface{} which is what will
// be received from plugin backends.
doc := framework.NewOASDocument(version.Version)

procMountGroup := func(group, mountPrefix string) error {
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)
}
for mount := range resp.Data[group].(map[string]interface{}) {

backend := b.Core.router.MatchingBackend(ctx, mountPrefix+mount)

@@ -4457,7 +4450,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},
Data: map[string]interface{}{"requestResponsePrefix": strings.Trim(mountPrefix+mount, "/")},
}

resp, err := backend.HandleRequest(ctx, req)
@@ -4538,8 +4531,6 @@ func (b *SystemBackend) pathInternalOpenAPI(ctx context.Context, req *logical.Re
return nil, err
}

doc.CreateOperationIDs(context)

buf, err := json.Marshal(doc)
if err != nil {
return nil, err