From 857ec3f424dba0982b0d8a2d905ebbf3c556a852 Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Mon, 20 Feb 2023 19:40:52 -0500 Subject: [PATCH 01/33] add prefix & suffix display attributes --- sdk/framework/path.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/sdk/framework/path.go b/sdk/framework/path.go index e6221af92d52..e01d32bd3016 100644 --- a/sdk/framework/path.go +++ b/sdk/framework/path.go @@ -224,6 +224,12 @@ type DisplayAttributes struct { // Action is the verb to use for the operation. Action string `json:"action,omitempty"` + // OperationPrefix is used to construct OpenAPI OperationID, if specified + OperationPrefix string `json:"operationPrefix"` + + // OperationSuffix is used to construct OpenAPI OperationID, if specified + OperationSuffix string `json:"operationSuffix"` + // EditType is the optional type of form field needed for a property // This is only necessary for a "textarea" or "file" EditType string `json:"editType,omitempty"` From beed664c9c169914a616bc3b6db47ba655dfffb6 Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Wed, 22 Feb 2023 01:09:16 -0500 Subject: [PATCH 02/33] add DisplayAttrs to PathParameters --- sdk/framework/path.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sdk/framework/path.go b/sdk/framework/path.go index e01d32bd3016..6f2b2e851436 100644 --- a/sdk/framework/path.go +++ b/sdk/framework/path.go @@ -264,6 +264,7 @@ type PathOperation struct { Deprecated bool ForwardPerformanceSecondary bool ForwardPerformanceStandby bool + DisplayAttrs *DisplayAttributes } func (p *PathOperation) Handler() OperationFunc { @@ -280,6 +281,7 @@ func (p *PathOperation) Properties() OperationProperties { Deprecated: p.Deprecated, ForwardPerformanceSecondary: p.ForwardPerformanceSecondary, ForwardPerformanceStandby: p.ForwardPerformanceStandby, + DisplayAttrs: p.DisplayAttrs, } } From df763a5e05ca9a5224b3ec7e0a7e44b665ab6bb2 Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Wed, 22 Feb 2023 01:09:57 -0500 Subject: [PATCH 03/33] add constructOperationID func --- sdk/framework/openapi.go | 134 ++++++++++++++++++++++++++++++--------- 1 file changed, 105 insertions(+), 29 deletions(-) diff --git a/sdk/framework/openapi.go b/sdk/framework/openapi.go index 57906672827a..91d7156c8ac3 100644 --- a/sdk/framework/openapi.go +++ b/sdk/framework/openapi.go @@ -315,6 +315,8 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st continue } + operationID := constructOperationID(path, opType, p.DisplayAttrs, props.DisplayAttrs) + if opType == logical.CreateOperation { pi.CreateSupported = true @@ -334,6 +336,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 { @@ -381,7 +384,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, @@ -488,7 +491,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{ @@ -520,33 +523,6 @@ 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. -// -// 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 - - title := cases.Title(language.English) - - b.WriteString(title.String(prefix)) - - // 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)) - } - } - - b.WriteString(suffix) - - return b.String() -} - func specialPathMatch(path string, specialPaths []string) bool { // Test for exact or prefix match of special paths. for _, sp := range specialPaths { @@ -558,6 +534,98 @@ func specialPathMatch(path string, specialPaths []string) bool { return false } +// In general, the name will be constructed as follows: +// +// operation id: [prefix][verb][suffix] +// request name: [prefix][verb][suffix]Request +// response name: [prefix][verb][suffix]Response +func constructOperationID(path string, operation logical.Operation, attributesPath, attributesOperation *DisplayAttributes) string { + // default-initialize for convenience + var ( + attrsPath DisplayAttributes + attrsOperation DisplayAttributes + ) + if attributesPath != nil { + attrsPath = *attributesPath + } + if attributesOperation != nil { + attrsOperation = *attributesOperation + } + + title := cases.Title(language.English, cases.NoLower) + + var ( + prefix string + verb string + suffix string + ) + + // prefix + switch { + case attrsOperation.OperationPrefix != "": + prefix = attrsOperation.OperationPrefix + + case attrsPath.OperationPrefix != "": + prefix = attrsPath.OperationPrefix + } + + // verb + switch { + case attrsOperation.Action != "": + verb = attrsOperation.Action + + case operation == logical.UpdateOperation: + verb = "Write" + + default: + verb = title.String(string(operation)) + } + + // suffix + switch { + case attrsOperation.OperationSuffix != "": + suffix = attrsOperation.OperationSuffix + + case attrsPath.OperationSuffix != "": + suffix = attrsPath.OperationSuffix + } + + // disambiguate parameterized paths + if suffix != "" { + var ( + parameters []string + parts []string = strings.Split(path, "/") + ) + for _, e := range parts { + if strings.HasPrefix(e, "{") && strings.HasSuffix(e, "}") { + parameters = append(parameters, e) + } + } + if len(parameters) != 0 && strings.HasSuffix(suffix, "s") { + last := parameters[len(parameters)-1] + + // remove the last "s" character to make this not plural + if last == "{name}" || last == "{role}" || last == "{method_id}" || last == "{issuer_ref}" { + suffix = suffix[:len(suffix)-1] + } + } + } + + // else, fall back to using the path to construct the operation id + if prefix == "" && suffix == "" { + parts := nonWordRe.Split(strings.ToLower(path), -1) + + // title case everything & join the result into a string + for i, s := range parts { + parts[i] = title.String(s) + } + + suffix = strings.Join(parts, "") + } + + return fmt.Sprintf("%s%s%s", prefix, verb, suffix) +} + // expandPattern expands a regex pattern by generating permutations of any optional parameters // and changing named parameters into their {openapi} equivalents. func expandPattern(pattern string) ([]string, error) { @@ -883,6 +951,10 @@ 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 and +// will soon be reomoved. func (d *OASDocument) CreateOperationIDs(context string) { opIDCount := make(map[string]int) var paths []string @@ -910,6 +982,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) From 72c5c47f92c4b305466cafad303d9cc00b202e10 Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Wed, 22 Feb 2023 17:34:07 -0500 Subject: [PATCH 04/33] Fixes & comments --- sdk/framework/openapi.go | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/sdk/framework/openapi.go b/sdk/framework/openapi.go index 91d7156c8ac3..3ee240fc6e36 100644 --- a/sdk/framework/openapi.go +++ b/sdk/framework/openapi.go @@ -315,7 +315,7 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st continue } - operationID := constructOperationID(path, opType, p.DisplayAttrs, props.DisplayAttrs) + operationID := constructOperationID(path, opType, p.DisplayAttrs, props.DisplayAttrs, requestResponsePrefix) if opType == logical.CreateOperation { pi.CreateSupported = true @@ -534,12 +534,19 @@ func specialPathMatch(path string, specialPaths []string) bool { return false } -// In general, the name will be constructed as follows: +// constructOperationID joins the given inputs into a title case operation id, +// which is also used as a prefix for request and response names. // -// operation id: [prefix][verb][suffix] -// request name: [prefix][verb][suffix]Request -// response name: [prefix][verb][suffix]Response -func constructOperationID(path string, operation logical.Operation, attributesPath, attributesOperation *DisplayAttributes) string { +// The OperationPrefix / OperationSuffix / Action found in display attributes +// will be used, if provided. Otherwise, the function falls back to using the +// path and the operation. +// +// Examples of generated operation identifiers: +// - KVv2Write +// - KVv2Read +// - GoogleCloudLogin +// - GoogleCloudWriteRole +func constructOperationID(path string, operation logical.Operation, attributesPath, attributesOperation *DisplayAttributes, defaultPrefix string) string { // default-initialize for convenience var ( attrsPath DisplayAttributes @@ -611,16 +618,19 @@ func constructOperationID(path string, operation logical.Operation, attributesPa } } - // else, fall back to using the path to construct the operation id + // fall back to using the path to construct the operation id if prefix == "" && suffix == "" { + // split, title case everything, and join the result into a string parts := nonWordRe.Split(strings.ToLower(path), -1) - // title case everything & join the result into a string for i, s := range parts { parts[i] = title.String(s) } suffix = strings.Join(parts, "") + + // TODO: remove this after adding display attributes to builtin plugins + prefix = title.String(defaultPrefix) } return fmt.Sprintf("%s%s%s", prefix, verb, suffix) From ce4ca4a274a233e37ef2a579a56b7f6ad1bdf576 Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Thu, 23 Feb 2023 11:50:26 -0500 Subject: [PATCH 05/33] Add test and fix logic --- sdk/framework/openapi.go | 94 +++++++++--------- sdk/framework/openapi_test.go | 177 ++++++++++++++++++++++------------ 2 files changed, 161 insertions(+), 110 deletions(-) diff --git a/sdk/framework/openapi.go b/sdk/framework/openapi.go index 3ee240fc6e36..00e802ebefd4 100644 --- a/sdk/framework/openapi.go +++ b/sdk/framework/openapi.go @@ -315,7 +315,7 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st continue } - operationID := constructOperationID(path, opType, p.DisplayAttrs, props.DisplayAttrs, requestResponsePrefix) + operationID := constructOperationID(path, p.DisplayAttrs, opType, props.DisplayAttrs, requestResponsePrefix) if opType == logical.CreateOperation { pi.CreateSupported = true @@ -546,58 +546,38 @@ func specialPathMatch(path string, specialPaths []string) bool { // - KVv2Read // - GoogleCloudLogin // - GoogleCloudWriteRole -func constructOperationID(path string, operation logical.Operation, attributesPath, attributesOperation *DisplayAttributes, defaultPrefix string) string { - // default-initialize for convenience - var ( - attrsPath DisplayAttributes - attrsOperation DisplayAttributes - ) - if attributesPath != nil { - attrsPath = *attributesPath - } - if attributesOperation != nil { - attrsOperation = *attributesOperation - } - - title := cases.Title(language.English, cases.NoLower) - +func constructOperationID( + path string, + pathAttributes *DisplayAttributes, + operation logical.Operation, + operationAttributes *DisplayAttributes, + defaultPrefix string, +) string { var ( prefix string - verb string suffix string + verb string ) - // prefix - switch { - case attrsOperation.OperationPrefix != "": - prefix = attrsOperation.OperationPrefix - - case attrsPath.OperationPrefix != "": - prefix = attrsPath.OperationPrefix + if operationAttributes != nil { + prefix = operationAttributes.OperationPrefix + suffix = operationAttributes.OperationSuffix + verb = operationAttributes.Action } - // verb - switch { - case attrsOperation.Action != "": - verb = attrsOperation.Action - - case operation == logical.UpdateOperation: - verb = "Write" - - default: - verb = title.String(string(operation)) - } - - // suffix - switch { - case attrsOperation.OperationSuffix != "": - suffix = attrsOperation.OperationSuffix - - case attrsPath.OperationSuffix != "": - suffix = attrsPath.OperationSuffix + if pathAttributes != nil { + if prefix == "" { + prefix = pathAttributes.OperationPrefix + } + if suffix == "" { + suffix = pathAttributes.OperationSuffix + } + if verb == "" { + verb = pathAttributes.Action + } } - // disambiguate parameterized paths + // disambiguate the suffix for the paths generated through `expandPattern` if suffix != "" { var ( parameters []string @@ -618,9 +598,18 @@ func constructOperationID(path string, operation logical.Operation, attributesPa } } - // fall back to using the path to construct the operation id - if prefix == "" && suffix == "" { - // split, title case everything, and join the result into a string + title := cases.Title(language.English, cases.NoLower) + + // fall back to using the path + operation to construct the operation id + needPrefix := prefix == "" && (suffix == "" || verb == "") + needSuffix := suffix == "" && (prefix == "" || verb == "") + needVerb := verb == "" + + if needPrefix { + prefix = defaultPrefix + } + + if needSuffix { parts := nonWordRe.Split(strings.ToLower(path), -1) for i, s := range parts { @@ -628,12 +617,17 @@ func constructOperationID(path string, operation logical.Operation, attributesPa } suffix = strings.Join(parts, "") + } - // TODO: remove this after adding display attributes to builtin plugins - prefix = title.String(defaultPrefix) + if needVerb { + if operation == logical.UpdateOperation { + verb = "Write" + } else { + verb = string(operation) + } } - return fmt.Sprintf("%s%s%s", prefix, verb, suffix) + return title.String(prefix) + title.String(verb) + title.String(suffix) } // expandPattern expands a regex pattern by generating permutations of any optional parameters diff --git a/sdk/framework/openapi_test.go b/sdk/framework/openapi_test.go index 3d5789e9dca9..7ee59e968274 100644 --- a/sdk/framework/openapi_test.go +++ b/sdk/framework/openapi_test.go @@ -480,66 +480,6 @@ func TestOpenAPI_Paths(t *testing.T) { }) } -func TestOpenAPI_OperationID(t *testing.T) { - path1 := &Path{ - Pattern: "foo/" + GenericNameRegex("id"), - Fields: map[string]*FieldSchema{ - "id": {Type: TypeString}, - }, - Operations: map[logical.Operation]OperationHandler{ - logical.ReadOperation: &PathOperation{}, - logical.UpdateOperation: &PathOperation{}, - logical.DeleteOperation: &PathOperation{}, - }, - } - - path2 := &Path{ - Pattern: "Foo/" + GenericNameRegex("id"), - Fields: map[string]*FieldSchema{ - "id": {Type: TypeString}, - }, - Operations: map[logical.Operation]OperationHandler{ - logical.ReadOperation: &PathOperation{}, - }, - } - - for _, context := range []string{"", "bar"} { - doc := NewOASDocument("version") - err := documentPath(path1, nil, "kv", logical.TypeLogical, doc) - if err != nil { - t.Fatal(err) - } - err = documentPath(path2, nil, "kv", logical.TypeLogical, doc) - if err != nil { - t.Fatal(err) - } - doc.CreateOperationIDs(context) - - tests := []struct { - path string - op string - opID string - }{ - {"/Foo/{id}", "get", "getFooId"}, - {"/foo/{id}", "get", "getFooId_2"}, - {"/foo/{id}", "post", "postFooId"}, - {"/foo/{id}", "delete", "deleteFooId"}, - } - - for _, test := range tests { - actual := getPathOp(doc.Paths[test.path], test.op).OperationID - expected := test.opID - if context != "" { - expected += "_" + context - } - - if actual != expected { - t.Fatalf("expected %v, got %v", expected, actual) - } - } - } -} - func TestOpenAPI_CustomDecoder(t *testing.T) { p := &Path{ Pattern: "foo", @@ -628,6 +568,123 @@ func TestOpenAPI_CleanResponse(t *testing.T) { } } +func TestOpenAPI_constructOperationID(t *testing.T) { + tests := map[string]struct { + path string + pathAttributes *DisplayAttributes + operation logical.Operation + operationAttributes *DisplayAttributes + defaultPrefix string + expected string + }{ + "empty": { + path: "", + pathAttributes: nil, + operation: logical.Operation(""), + operationAttributes: nil, + defaultPrefix: "", + expected: "", + }, + "simple-read": { + path: "path/to/thing", + pathAttributes: nil, + operation: logical.ReadOperation, + operationAttributes: nil, + defaultPrefix: "test", + expected: "TestReadPathToThing", + }, + "simple-write": { + path: "path/to/thing", + pathAttributes: nil, + operation: logical.UpdateOperation, + operationAttributes: nil, + defaultPrefix: "test", + expected: "TestWritePathToThing", + }, + "operation-prefix": { + path: "path/to/thing", + pathAttributes: &DisplayAttributes{OperationPrefix: "MyPrefix"}, + operation: logical.UpdateOperation, + operationAttributes: nil, + defaultPrefix: "test", + expected: "MyPrefixWritePathToThing", + }, + "operation-prefix-override": { + path: "path/to/thing", + pathAttributes: &DisplayAttributes{OperationPrefix: "MyPrefix"}, + operation: logical.UpdateOperation, + operationAttributes: &DisplayAttributes{OperationPrefix: "BetterPrefix"}, + defaultPrefix: "test", + expected: "BetterPrefixWritePathToThing", + }, + "operation-prefix-and-suffix": { + path: "path/to/thing", + pathAttributes: &DisplayAttributes{OperationPrefix: "MyPrefix", OperationSuffix: "MySuffix"}, + operation: logical.UpdateOperation, + operationAttributes: nil, + defaultPrefix: "test", + expected: "MyPrefixWriteMySuffix", + }, + "operation-prefix-and-suffix-override": { + path: "path/to/thing", + pathAttributes: &DisplayAttributes{OperationPrefix: "MyPrefix", OperationSuffix: "MySuffix"}, + operation: logical.UpdateOperation, + operationAttributes: &DisplayAttributes{OperationPrefix: "BetterPrefix", OperationSuffix: "BetterSuffix"}, + defaultPrefix: "test", + expected: "BetterPrefixWriteBetterSuffix", + }, + "operation-prefix-action-suffix": { + path: "path/to/thing", + pathAttributes: &DisplayAttributes{OperationPrefix: "MyPrefix", Action: "Create", OperationSuffix: "MySuffix"}, + operation: logical.UpdateOperation, + operationAttributes: &DisplayAttributes{OperationPrefix: "BetterPrefix", OperationSuffix: "BetterSuffix"}, + defaultPrefix: "test", + expected: "BetterPrefixCreateBetterSuffix", + }, + "operation-prefix-action-suffix-override": { + path: "path/to/thing", + pathAttributes: &DisplayAttributes{OperationPrefix: "MyPrefix", Action: "Create", OperationSuffix: "MySuffix"}, + operation: logical.UpdateOperation, + operationAttributes: &DisplayAttributes{OperationPrefix: "BetterPrefix", Action: "Login", OperationSuffix: "BetterSuffix"}, + defaultPrefix: "test", + expected: "BetterPrefixLoginBetterSuffix", + }, + "operation-prefix-action": { + path: "path/to/thing", + pathAttributes: nil, + operation: logical.UpdateOperation, + operationAttributes: &DisplayAttributes{OperationPrefix: "BetterPrefix", Action: "Login"}, + defaultPrefix: "test", + expected: "BetterPrefixLogin", + }, + "operation-action-suffix": { + path: "path/to/thing", + pathAttributes: nil, + operation: logical.UpdateOperation, + operationAttributes: &DisplayAttributes{Action: "Login", OperationSuffix: "BetterSuffix"}, + defaultPrefix: "test", + expected: "LoginBetterSuffix", + }, + } + + for name, test := range tests { + name, test := name, test + t.Run(name, func(t *testing.T) { + t.Parallel() + actual := constructOperationID( + test.path, + test.pathAttributes, + test.operation, + test.operationAttributes, + test.defaultPrefix, + ) + if actual != test.expected { + t.Fatalf("expected: %s; got: %s", test.expected, actual) + } + }) + } +} + func testPath(t *testing.T, path *Path, sp *logical.Paths, expectedJSON string) { t.Helper() From fcdd2d3a1b15eb5dd9e2e9ffbb85198024d11a22 Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Thu, 23 Feb 2023 12:27:49 -0500 Subject: [PATCH 06/33] fix existing test data --- sdk/framework/testdata/legacy.json | 17 ++++++----- sdk/framework/testdata/operations.json | 34 +++++++++++++++------ sdk/framework/testdata/operations_list.json | 13 +++++--- sdk/framework/testdata/responses.json | 17 ++++++----- 4 files changed, 52 insertions(+), 29 deletions(-) diff --git a/sdk/framework/testdata/legacy.json b/sdk/framework/testdata/legacy.json index f526f1e2aade..85b459988141 100644 --- a/sdk/framework/testdata/legacy.json +++ b/sdk/framework/testdata/legacy.json @@ -24,9 +24,11 @@ } ], "get": { - "operationId": "getLookupId", + "operationId": "KvReadLookupId", "summary": "Synopsis", - "tags": ["secrets"], + "tags": [ + "secrets" + ], "responses": { "200": { "description": "OK" @@ -34,15 +36,17 @@ } }, "post": { - "operationId": "postLookupId", + "operationId": "KvWriteLookupId", "summary": "Synopsis", - "tags": ["secrets"], + "tags": [ + "secrets" + ], "requestBody": { "required": true, "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/KvLookupRequest" + "$ref": "#/components/schemas/KvWriteLookupIdRequest" } } } @@ -57,7 +61,7 @@ }, "components": { "schemas": { - "KvLookupRequest": { + "KvWriteLookupIdRequest": { "type": "object", "properties": { "token": { @@ -69,4 +73,3 @@ } } } - diff --git a/sdk/framework/testdata/operations.json b/sdk/framework/testdata/operations.json index e1db6744018d..7311fdbc24e6 100644 --- a/sdk/framework/testdata/operations.json +++ b/sdk/framework/testdata/operations.json @@ -37,8 +37,10 @@ } ], "get": { - "operationId": "getFooId", - "tags": ["secrets"], + "operationId": "KvReadFooId", + "tags": [ + "secrets" + ], "summary": "My Summary", "description": "My Description", "responses": { @@ -58,8 +60,10 @@ ] }, "post": { - "operationId": "postFooId", - "tags": ["secrets"], + "operationId": "KvWriteFooId", + "tags": [ + "secrets" + ], "summary": "Update Summary", "description": "Update Description", "requestBody": { @@ -67,7 +71,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/KvFooRequest" + "$ref": "#/components/schemas/KvWriteFooIdRequest" } } } @@ -82,9 +86,11 @@ }, "components": { "schemas": { - "KvFooRequest": { + "KvWriteFooIdRequest": { "type": "object", - "required": ["age"], + "required": [ + "age" + ], "properties": { "flavors": { "type": "array", @@ -96,7 +102,11 @@ "age": { "type": "integer", "description": "the age", - "enum": [1, 2, 3], + "enum": [ + 1, + 2, + 3 + ], "x-vault-displayAttrs": { "name": "Age", "sensitive": true, @@ -113,9 +123,13 @@ "x-abc-token": { "type": "string", "description": "a header value", - "enum": ["a", "b", "c"] + "enum": [ + "a", + "b", + "c" + ] }, - "maximum" : { + "maximum": { "type": "integer", "description": "a maximum value", "format": "int64" diff --git a/sdk/framework/testdata/operations_list.json b/sdk/framework/testdata/operations_list.json index e89622a3c40a..de6caa94c96b 100644 --- a/sdk/framework/testdata/operations_list.json +++ b/sdk/framework/testdata/operations_list.json @@ -36,8 +36,10 @@ } ], "get": { - "operationId": "getFooId", - "tags": ["secrets"], + "operationId": "KvListFooId", + "tags": [ + "secrets" + ], "summary": "List Summary", "description": "List Description", "responses": { @@ -53,7 +55,9 @@ "in": "query", "schema": { "type": "string", - "enum": ["true"] + "enum": [ + "true" + ] } } ] @@ -61,7 +65,6 @@ } }, "components": { - "schemas": { - } + "schemas": {} } } diff --git a/sdk/framework/testdata/responses.json b/sdk/framework/testdata/responses.json index 4e442cfb49ba..ac6877908924 100644 --- a/sdk/framework/testdata/responses.json +++ b/sdk/framework/testdata/responses.json @@ -14,8 +14,10 @@ "description": "Synopsis", "x-vault-unauthenticated": true, "delete": { - "operationId": "deleteFoo", - "tags": ["secrets"], + "operationId": "KvDeleteFoo", + "tags": [ + "secrets" + ], "summary": "Delete stuff", "responses": { "204": { @@ -24,8 +26,10 @@ } }, "get": { - "operationId": "getFoo", - "tags": ["secrets"], + "operationId": "KvReadFoo", + "tags": [ + "secrets" + ], "summary": "My Summary", "description": "My Description", "responses": { @@ -34,7 +38,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/KvFooResponse" + "$ref": "#/components/schemas/KvReadFooResponse" } } } @@ -45,7 +49,7 @@ }, "components": { "schemas": { - "KvFooResponse": { + "KvReadFooResponse": { "type": "object", "properties": { "field_a": { @@ -61,4 +65,3 @@ } } } - From f3a4dbe9158b3345d660f5a8963f7577c224a762 Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Thu, 23 Feb 2023 12:33:28 -0500 Subject: [PATCH 07/33] ommitempty --- sdk/framework/path.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/framework/path.go b/sdk/framework/path.go index 6f2b2e851436..8393f1bf9ca8 100644 --- a/sdk/framework/path.go +++ b/sdk/framework/path.go @@ -225,10 +225,10 @@ type DisplayAttributes struct { Action string `json:"action,omitempty"` // OperationPrefix is used to construct OpenAPI OperationID, if specified - OperationPrefix string `json:"operationPrefix"` + OperationPrefix string `json:"operationPrefix,omitempty"` // OperationSuffix is used to construct OpenAPI OperationID, if specified - OperationSuffix string `json:"operationSuffix"` + OperationSuffix string `json:"operationSuffix,omitempty"` // EditType is the optional type of form field needed for a property // This is only necessary for a "textarea" or "file" From 5c727697b5b898224a7543e34b76edbb706b79a4 Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Thu, 23 Feb 2023 14:17:56 -0500 Subject: [PATCH 08/33] changelog --- changelog/19319.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/19319.txt diff --git a/changelog/19319.txt b/changelog/19319.txt new file mode 100644 index 000000000000..4702344afb08 --- /dev/null +++ b/changelog/19319.txt @@ -0,0 +1,3 @@ +```release-note:improvement +openapi: Improve operationId/request/response naming strategy +``` From d3e16aaf967520a92cf20dd3090321603964683a Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Sun, 26 Feb 2023 16:18:11 -0500 Subject: [PATCH 09/33] better suffix disambiguation --- sdk/framework/openapi.go | 55 +++++++++++++++++++++-------------- sdk/framework/openapi_test.go | 46 +++++++++++++++++++++++++++-- 2 files changed, 76 insertions(+), 25 deletions(-) diff --git a/sdk/framework/openapi.go b/sdk/framework/openapi.go index 00e802ebefd4..b1f9562a2ea3 100644 --- a/sdk/framework/openapi.go +++ b/sdk/framework/openapi.go @@ -244,7 +244,7 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st } } - for _, path := range paths { + for pathIndex, path := range paths { // Construct a top level PathItem which will be populated as the path is processed. pi := OASPathItem{ Description: cleanString(p.HelpSynopsis), @@ -315,8 +315,6 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st continue } - operationID := constructOperationID(path, p.DisplayAttrs, opType, props.DisplayAttrs, requestResponsePrefix) - if opType == logical.CreateOperation { pi.CreateSupported = true @@ -333,6 +331,15 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st op := NewOASOperation() + operationID := constructOperationID( + path, + pathIndex, + p.DisplayAttrs, + opType, + props.DisplayAttrs, + requestResponsePrefix, + ) + op.Summary = props.Summary op.Description = props.Description op.Deprecated = props.Deprecated @@ -548,6 +555,7 @@ func specialPathMatch(path string, specialPaths []string) bool { // - GoogleCloudWriteRole func constructOperationID( path string, + pathIndex int, pathAttributes *DisplayAttributes, operation logical.Operation, operationAttributes *DisplayAttributes, @@ -577,24 +585,27 @@ func constructOperationID( } } - // disambiguate the suffix for the paths generated through `expandPattern` - if suffix != "" { - var ( - parameters []string - parts []string = strings.Split(path, "/") - ) - for _, e := range parts { - if strings.HasPrefix(e, "{") && strings.HasSuffix(e, "}") { - parameters = append(parameters, e) - } - } - if len(parameters) != 0 && strings.HasSuffix(suffix, "s") { - last := parameters[len(parameters)-1] - - // remove the last "s" character to make this not plural - if last == "{name}" || last == "{role}" || last == "{method_id}" || last == "{issuer_ref}" { - suffix = suffix[:len(suffix)-1] - } + // A single suffix string can contain multiple pipe-delimited strings. To + // determine the actual suffix, we attempt to match it by the index of the + // paths returned from `expandPattern(...)`. For example: + // + // aws/ + // Pattern: `^(creds|sts)/(?P\w(([\w-.@]+)?\w)?)$` + // DisplayAttrs: { + // OperationSuffix: "Credentials|STSCredentials" + // } + // + // Will expand into two paths and corresponding suffixes: + // + // path 0: "creds/{name}" suffix: Credentials + // path 1: "sts/{name}" suffix: STSCredentials + // + if suffixes := strings.Split(suffix, "|"); len(suffixes) > 1 || pathIndex > 0 { + // if the index is out of bounds, fall back to the old logic + if pathIndex >= len(suffixes) { + suffix = "" + } else { + suffix = suffixes[pathIndex] } } @@ -602,7 +613,7 @@ func constructOperationID( // fall back to using the path + operation to construct the operation id needPrefix := prefix == "" && (suffix == "" || verb == "") - needSuffix := suffix == "" && (prefix == "" || verb == "") + needSuffix := suffix == "" && (prefix == "" || verb == "" || pathIndex > 0) needVerb := verb == "" if needPrefix { diff --git a/sdk/framework/openapi_test.go b/sdk/framework/openapi_test.go index 7ee59e968274..38c753ecd6f8 100644 --- a/sdk/framework/openapi_test.go +++ b/sdk/framework/openapi_test.go @@ -571,6 +571,7 @@ func TestOpenAPI_CleanResponse(t *testing.T) { func TestOpenAPI_constructOperationID(t *testing.T) { tests := map[string]struct { path string + pathIndex int pathAttributes *DisplayAttributes operation logical.Operation operationAttributes *DisplayAttributes @@ -579,6 +580,7 @@ func TestOpenAPI_constructOperationID(t *testing.T) { }{ "empty": { path: "", + pathIndex: 0, pathAttributes: nil, operation: logical.Operation(""), operationAttributes: nil, @@ -587,6 +589,7 @@ func TestOpenAPI_constructOperationID(t *testing.T) { }, "simple-read": { path: "path/to/thing", + pathIndex: 0, pathAttributes: nil, operation: logical.ReadOperation, operationAttributes: nil, @@ -595,6 +598,7 @@ func TestOpenAPI_constructOperationID(t *testing.T) { }, "simple-write": { path: "path/to/thing", + pathIndex: 0, pathAttributes: nil, operation: logical.UpdateOperation, operationAttributes: nil, @@ -603,6 +607,7 @@ func TestOpenAPI_constructOperationID(t *testing.T) { }, "operation-prefix": { path: "path/to/thing", + pathIndex: 0, pathAttributes: &DisplayAttributes{OperationPrefix: "MyPrefix"}, operation: logical.UpdateOperation, operationAttributes: nil, @@ -611,6 +616,7 @@ func TestOpenAPI_constructOperationID(t *testing.T) { }, "operation-prefix-override": { path: "path/to/thing", + pathIndex: 0, pathAttributes: &DisplayAttributes{OperationPrefix: "MyPrefix"}, operation: logical.UpdateOperation, operationAttributes: &DisplayAttributes{OperationPrefix: "BetterPrefix"}, @@ -619,6 +625,7 @@ func TestOpenAPI_constructOperationID(t *testing.T) { }, "operation-prefix-and-suffix": { path: "path/to/thing", + pathIndex: 0, pathAttributes: &DisplayAttributes{OperationPrefix: "MyPrefix", OperationSuffix: "MySuffix"}, operation: logical.UpdateOperation, operationAttributes: nil, @@ -627,6 +634,7 @@ func TestOpenAPI_constructOperationID(t *testing.T) { }, "operation-prefix-and-suffix-override": { path: "path/to/thing", + pathIndex: 0, pathAttributes: &DisplayAttributes{OperationPrefix: "MyPrefix", OperationSuffix: "MySuffix"}, operation: logical.UpdateOperation, operationAttributes: &DisplayAttributes{OperationPrefix: "BetterPrefix", OperationSuffix: "BetterSuffix"}, @@ -635,7 +643,8 @@ func TestOpenAPI_constructOperationID(t *testing.T) { }, "operation-prefix-action-suffix": { path: "path/to/thing", - pathAttributes: &DisplayAttributes{OperationPrefix: "MyPrefix", Action: "Create", OperationSuffix: "MySuffix"}, + pathIndex: 0, + pathAttributes: &DisplayAttributes{OperationPrefix: "MyPrefix", OperationSuffix: "MySuffix", Action: "Create"}, operation: logical.UpdateOperation, operationAttributes: &DisplayAttributes{OperationPrefix: "BetterPrefix", OperationSuffix: "BetterSuffix"}, defaultPrefix: "test", @@ -643,14 +652,16 @@ func TestOpenAPI_constructOperationID(t *testing.T) { }, "operation-prefix-action-suffix-override": { path: "path/to/thing", - pathAttributes: &DisplayAttributes{OperationPrefix: "MyPrefix", Action: "Create", OperationSuffix: "MySuffix"}, + pathIndex: 0, + pathAttributes: &DisplayAttributes{OperationPrefix: "MyPrefix", OperationSuffix: "MySuffix", Action: "Create"}, operation: logical.UpdateOperation, - operationAttributes: &DisplayAttributes{OperationPrefix: "BetterPrefix", Action: "Login", OperationSuffix: "BetterSuffix"}, + operationAttributes: &DisplayAttributes{OperationPrefix: "BetterPrefix", OperationSuffix: "BetterSuffix", Action: "Login"}, defaultPrefix: "test", expected: "BetterPrefixLoginBetterSuffix", }, "operation-prefix-action": { path: "path/to/thing", + pathIndex: 0, pathAttributes: nil, operation: logical.UpdateOperation, operationAttributes: &DisplayAttributes{OperationPrefix: "BetterPrefix", Action: "Login"}, @@ -659,12 +670,40 @@ func TestOpenAPI_constructOperationID(t *testing.T) { }, "operation-action-suffix": { path: "path/to/thing", + pathIndex: 0, pathAttributes: nil, operation: logical.UpdateOperation, operationAttributes: &DisplayAttributes{Action: "Login", OperationSuffix: "BetterSuffix"}, defaultPrefix: "test", expected: "LoginBetterSuffix", }, + "pipe-delimited-suffix-0": { + path: "path/to/thing", + pathIndex: 0, + pathAttributes: nil, + operation: logical.UpdateOperation, + operationAttributes: &DisplayAttributes{OperationPrefix: "BetterPrefix", OperationSuffix: "Suffix0|Suffix1"}, + defaultPrefix: "test", + expected: "BetterPrefixWriteSuffix0", + }, + "pipe-delimited-suffix-1": { + path: "path/to/thing", + pathIndex: 1, + pathAttributes: nil, + operation: logical.UpdateOperation, + operationAttributes: &DisplayAttributes{OperationPrefix: "BetterPrefix", OperationSuffix: "Suffix0|Suffix1"}, + defaultPrefix: "test", + expected: "BetterPrefixWriteSuffix1", + }, + "pipe-delimited-suffix-2-fallback": { + path: "path/to/thing", + pathIndex: 2, + pathAttributes: nil, + operation: logical.UpdateOperation, + operationAttributes: &DisplayAttributes{OperationPrefix: "BetterPrefix", OperationSuffix: "Suffix0|Suffix1"}, + defaultPrefix: "test", + expected: "BetterPrefixWritePathToThing", + }, } for name, test := range tests { @@ -673,6 +712,7 @@ func TestOpenAPI_constructOperationID(t *testing.T) { t.Parallel() actual := constructOperationID( test.path, + test.pathIndex, test.pathAttributes, test.operation, test.operationAttributes, From 22a1e74a3ded3a4cc1fa357023c119397425a631 Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Sun, 26 Feb 2023 16:35:24 -0500 Subject: [PATCH 10/33] Update comment --- sdk/framework/openapi.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/sdk/framework/openapi.go b/sdk/framework/openapi.go index b1f9562a2ea3..dd58ca32e3e8 100644 --- a/sdk/framework/openapi.go +++ b/sdk/framework/openapi.go @@ -967,9 +967,8 @@ func cleanResponse(resp *logical.Response) *cleanedResponse { // // 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 and -// will soon be reomoved. +// Deprecated: operationID's are now populated using `constructOperationID`. +// This function is here for backwards compatibility with older plugins. func (d *OASDocument) CreateOperationIDs(context string) { opIDCount := make(map[string]int) var paths []string From 70037172c8b782b1d6f5a7b0ff98dc60d6cffda8 Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Mon, 27 Feb 2023 16:10:54 -0500 Subject: [PATCH 11/33] hyphenate instead of TitleCase --- sdk/framework/openapi.go | 49 +++++++++--------- sdk/framework/openapi_test.go | 56 ++++++++++----------- sdk/framework/testdata/legacy.json | 8 +-- sdk/framework/testdata/operations.json | 8 +-- sdk/framework/testdata/operations_list.json | 2 +- sdk/framework/testdata/responses.json | 8 +-- 6 files changed, 67 insertions(+), 64 deletions(-) diff --git a/sdk/framework/openapi.go b/sdk/framework/openapi.go index dd58ca32e3e8..f96e80ebaeb8 100644 --- a/sdk/framework/openapi.go +++ b/sdk/framework/openapi.go @@ -14,8 +14,6 @@ import ( "github.com/hashicorp/vault/sdk/helper/wrapping" "github.com/hashicorp/vault/sdk/logical" "github.com/mitchellh/mapstructure" - "golang.org/x/text/cases" - "golang.org/x/text/language" ) // OpenAPI specification (OAS): https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md @@ -391,7 +389,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 := operationID + "Request" + requestName := operationID + "-request" doc.Components.Schemas[requestName] = s op.RequestBody = &OASRequestBody{ Required: true, @@ -498,7 +496,7 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st } if len(resp.Fields) != 0 { - responseName := operationID + "Response" + responseName := operationID + "-response" doc.Components.Schemas[responseName] = responseSchema content = OASContent{ "application/json": &OASMediaTypeObject{ @@ -541,18 +539,19 @@ func specialPathMatch(path string, specialPaths []string) bool { return false } -// constructOperationID joins the given inputs into a title case operation id, -// which is also used as a prefix for request and response names. +// constructOperationID joins the given inputs into a hyphen-separated +// lower-case operation id, which is also used as a prefix for request and +// response names. // // The OperationPrefix / OperationSuffix / Action found in display attributes // will be used, if provided. Otherwise, the function falls back to using the // path and the operation. // // Examples of generated operation identifiers: -// - KVv2Write -// - KVv2Read -// - GoogleCloudLogin -// - GoogleCloudWriteRole +// - kvv2-write +// - kvv2-read +// - google-cloud-login +// - google-cloud-write-role func constructOperationID( path string, pathIndex int, @@ -561,6 +560,7 @@ func constructOperationID( operationAttributes *DisplayAttributes, defaultPrefix string, ) string { + var ( prefix string suffix string @@ -592,13 +592,13 @@ func constructOperationID( // aws/ // Pattern: `^(creds|sts)/(?P\w(([\w-.@]+)?\w)?)$` // DisplayAttrs: { - // OperationSuffix: "Credentials|STSCredentials" + // OperationSuffix: "credentials|sts-credentials" // } // // Will expand into two paths and corresponding suffixes: // - // path 0: "creds/{name}" suffix: Credentials - // path 1: "sts/{name}" suffix: STSCredentials + // path 0: "creds/{name}" suffix: credentials + // path 1: "sts/{name}" suffix: sts-credentials // if suffixes := strings.Split(suffix, "|"); len(suffixes) > 1 || pathIndex > 0 { // if the index is out of bounds, fall back to the old logic @@ -609,7 +609,16 @@ func constructOperationID( } } - title := cases.Title(language.English, cases.NoLower) + // hyphenate is a helper that hyphenates the given slice except the empty elements + hyphenate := func(parts []string) string { + filtered := make([]string, 0, len(parts)) + for _, e := range parts { + if e != "" { + filtered = append(filtered, e) + } + } + return strings.ToLower(strings.Join(filtered, "-")) + } // fall back to using the path + operation to construct the operation id needPrefix := prefix == "" && (suffix == "" || verb == "") @@ -621,24 +630,18 @@ func constructOperationID( } if needSuffix { - parts := nonWordRe.Split(strings.ToLower(path), -1) - - for i, s := range parts { - parts[i] = title.String(s) - } - - suffix = strings.Join(parts, "") + suffix = hyphenate(nonWordRe.Split(strings.ToLower(path), -1)) } if needVerb { if operation == logical.UpdateOperation { - verb = "Write" + verb = "write" } else { verb = string(operation) } } - return title.String(prefix) + title.String(verb) + title.String(suffix) + return hyphenate([]string{prefix, verb, suffix}) } // expandPattern expands a regex pattern by generating permutations of any optional parameters diff --git a/sdk/framework/openapi_test.go b/sdk/framework/openapi_test.go index 38c753ecd6f8..680051401a73 100644 --- a/sdk/framework/openapi_test.go +++ b/sdk/framework/openapi_test.go @@ -594,7 +594,7 @@ func TestOpenAPI_constructOperationID(t *testing.T) { operation: logical.ReadOperation, operationAttributes: nil, defaultPrefix: "test", - expected: "TestReadPathToThing", + expected: "test-read-path-to-thing", }, "simple-write": { path: "path/to/thing", @@ -603,106 +603,106 @@ func TestOpenAPI_constructOperationID(t *testing.T) { operation: logical.UpdateOperation, operationAttributes: nil, defaultPrefix: "test", - expected: "TestWritePathToThing", + expected: "test-write-path-to-thing", }, "operation-prefix": { path: "path/to/thing", pathIndex: 0, - pathAttributes: &DisplayAttributes{OperationPrefix: "MyPrefix"}, + pathAttributes: &DisplayAttributes{OperationPrefix: "my-prefix"}, operation: logical.UpdateOperation, operationAttributes: nil, defaultPrefix: "test", - expected: "MyPrefixWritePathToThing", + expected: "my-prefix-write-path-to-thing", }, "operation-prefix-override": { path: "path/to/thing", pathIndex: 0, - pathAttributes: &DisplayAttributes{OperationPrefix: "MyPrefix"}, + pathAttributes: &DisplayAttributes{OperationPrefix: "my-prefix"}, operation: logical.UpdateOperation, - operationAttributes: &DisplayAttributes{OperationPrefix: "BetterPrefix"}, + operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix"}, defaultPrefix: "test", - expected: "BetterPrefixWritePathToThing", + expected: "better-prefix-write-path-to-thing", }, "operation-prefix-and-suffix": { path: "path/to/thing", pathIndex: 0, - pathAttributes: &DisplayAttributes{OperationPrefix: "MyPrefix", OperationSuffix: "MySuffix"}, + pathAttributes: &DisplayAttributes{OperationPrefix: "my-prefix", OperationSuffix: "my-suffix"}, operation: logical.UpdateOperation, operationAttributes: nil, defaultPrefix: "test", - expected: "MyPrefixWriteMySuffix", + expected: "my-prefix-write-my-suffix", }, "operation-prefix-and-suffix-override": { path: "path/to/thing", pathIndex: 0, - pathAttributes: &DisplayAttributes{OperationPrefix: "MyPrefix", OperationSuffix: "MySuffix"}, + pathAttributes: &DisplayAttributes{OperationPrefix: "my-prefix", OperationSuffix: "my-suffix"}, operation: logical.UpdateOperation, - operationAttributes: &DisplayAttributes{OperationPrefix: "BetterPrefix", OperationSuffix: "BetterSuffix"}, + operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix", OperationSuffix: "better-suffix"}, defaultPrefix: "test", - expected: "BetterPrefixWriteBetterSuffix", + expected: "better-prefix-write-better-suffix", }, "operation-prefix-action-suffix": { path: "path/to/thing", pathIndex: 0, - pathAttributes: &DisplayAttributes{OperationPrefix: "MyPrefix", OperationSuffix: "MySuffix", Action: "Create"}, + pathAttributes: &DisplayAttributes{OperationPrefix: "my-prefix", OperationSuffix: "my-suffix", Action: "Create"}, operation: logical.UpdateOperation, - operationAttributes: &DisplayAttributes{OperationPrefix: "BetterPrefix", OperationSuffix: "BetterSuffix"}, + operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix", OperationSuffix: "better-suffix"}, defaultPrefix: "test", - expected: "BetterPrefixCreateBetterSuffix", + expected: "better-prefix-create-better-suffix", }, "operation-prefix-action-suffix-override": { path: "path/to/thing", pathIndex: 0, - pathAttributes: &DisplayAttributes{OperationPrefix: "MyPrefix", OperationSuffix: "MySuffix", Action: "Create"}, + pathAttributes: &DisplayAttributes{OperationPrefix: "my-prefix", OperationSuffix: "my-suffix", Action: "Create"}, operation: logical.UpdateOperation, - operationAttributes: &DisplayAttributes{OperationPrefix: "BetterPrefix", OperationSuffix: "BetterSuffix", Action: "Login"}, + operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix", OperationSuffix: "better-suffix", Action: "Login"}, defaultPrefix: "test", - expected: "BetterPrefixLoginBetterSuffix", + expected: "better-prefix-login-better-suffix", }, "operation-prefix-action": { path: "path/to/thing", pathIndex: 0, pathAttributes: nil, operation: logical.UpdateOperation, - operationAttributes: &DisplayAttributes{OperationPrefix: "BetterPrefix", Action: "Login"}, + operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix", Action: "Login"}, defaultPrefix: "test", - expected: "BetterPrefixLogin", + expected: "better-prefix-login", }, "operation-action-suffix": { path: "path/to/thing", pathIndex: 0, pathAttributes: nil, operation: logical.UpdateOperation, - operationAttributes: &DisplayAttributes{Action: "Login", OperationSuffix: "BetterSuffix"}, + operationAttributes: &DisplayAttributes{Action: "Login", OperationSuffix: "better-suffix"}, defaultPrefix: "test", - expected: "LoginBetterSuffix", + expected: "login-better-suffix", }, "pipe-delimited-suffix-0": { path: "path/to/thing", pathIndex: 0, pathAttributes: nil, operation: logical.UpdateOperation, - operationAttributes: &DisplayAttributes{OperationPrefix: "BetterPrefix", OperationSuffix: "Suffix0|Suffix1"}, + operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix", OperationSuffix: "suffix0|suffix1"}, defaultPrefix: "test", - expected: "BetterPrefixWriteSuffix0", + expected: "better-prefix-write-suffix0", }, "pipe-delimited-suffix-1": { path: "path/to/thing", pathIndex: 1, pathAttributes: nil, operation: logical.UpdateOperation, - operationAttributes: &DisplayAttributes{OperationPrefix: "BetterPrefix", OperationSuffix: "Suffix0|Suffix1"}, + operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix", OperationSuffix: "suffix0|suffix1"}, defaultPrefix: "test", - expected: "BetterPrefixWriteSuffix1", + expected: "better-prefix-write-suffix1", }, "pipe-delimited-suffix-2-fallback": { path: "path/to/thing", pathIndex: 2, pathAttributes: nil, operation: logical.UpdateOperation, - operationAttributes: &DisplayAttributes{OperationPrefix: "BetterPrefix", OperationSuffix: "Suffix0|Suffix1"}, + operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix", OperationSuffix: "suffix0|suffix1"}, defaultPrefix: "test", - expected: "BetterPrefixWritePathToThing", + expected: "better-prefix-write-path-to-thing", }, } diff --git a/sdk/framework/testdata/legacy.json b/sdk/framework/testdata/legacy.json index 85b459988141..748a1d7ba8b0 100644 --- a/sdk/framework/testdata/legacy.json +++ b/sdk/framework/testdata/legacy.json @@ -24,7 +24,7 @@ } ], "get": { - "operationId": "KvReadLookupId", + "operationId": "kv-read-lookup-id", "summary": "Synopsis", "tags": [ "secrets" @@ -36,7 +36,7 @@ } }, "post": { - "operationId": "KvWriteLookupId", + "operationId": "kv-write-lookup-id", "summary": "Synopsis", "tags": [ "secrets" @@ -46,7 +46,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/KvWriteLookupIdRequest" + "$ref": "#/components/schemas/kv-write-lookup-id-request" } } } @@ -61,7 +61,7 @@ }, "components": { "schemas": { - "KvWriteLookupIdRequest": { + "kv-write-lookup-id-request": { "type": "object", "properties": { "token": { diff --git a/sdk/framework/testdata/operations.json b/sdk/framework/testdata/operations.json index 7311fdbc24e6..df8fed8bddfb 100644 --- a/sdk/framework/testdata/operations.json +++ b/sdk/framework/testdata/operations.json @@ -37,7 +37,7 @@ } ], "get": { - "operationId": "KvReadFooId", + "operationId": "kv-read-foo-id", "tags": [ "secrets" ], @@ -60,7 +60,7 @@ ] }, "post": { - "operationId": "KvWriteFooId", + "operationId": "kv-write-foo-id", "tags": [ "secrets" ], @@ -71,7 +71,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/KvWriteFooIdRequest" + "$ref": "#/components/schemas/kv-write-foo-id-request" } } } @@ -86,7 +86,7 @@ }, "components": { "schemas": { - "KvWriteFooIdRequest": { + "kv-write-foo-id-request": { "type": "object", "required": [ "age" diff --git a/sdk/framework/testdata/operations_list.json b/sdk/framework/testdata/operations_list.json index de6caa94c96b..a08208b24fa4 100644 --- a/sdk/framework/testdata/operations_list.json +++ b/sdk/framework/testdata/operations_list.json @@ -36,7 +36,7 @@ } ], "get": { - "operationId": "KvListFooId", + "operationId": "kv-list-foo-id", "tags": [ "secrets" ], diff --git a/sdk/framework/testdata/responses.json b/sdk/framework/testdata/responses.json index ac6877908924..ed097261e0c3 100644 --- a/sdk/framework/testdata/responses.json +++ b/sdk/framework/testdata/responses.json @@ -14,7 +14,7 @@ "description": "Synopsis", "x-vault-unauthenticated": true, "delete": { - "operationId": "KvDeleteFoo", + "operationId": "kv-delete-foo", "tags": [ "secrets" ], @@ -26,7 +26,7 @@ } }, "get": { - "operationId": "KvReadFoo", + "operationId": "kv-read-foo", "tags": [ "secrets" ], @@ -38,7 +38,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/KvReadFooResponse" + "$ref": "#/components/schemas/kv-read-foo-response" } } } @@ -49,7 +49,7 @@ }, "components": { "schemas": { - "KvReadFooResponse": { + "kv-read-foo-response": { "type": "object", "properties": { "field_a": { From 4713d814e80e6c970420c3591df1fb2d1485adfe Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Mon, 27 Feb 2023 16:13:18 -0500 Subject: [PATCH 12/33] fmt --- sdk/framework/openapi.go | 1 - 1 file changed, 1 deletion(-) diff --git a/sdk/framework/openapi.go b/sdk/framework/openapi.go index f96e80ebaeb8..12d5cdd3e55a 100644 --- a/sdk/framework/openapi.go +++ b/sdk/framework/openapi.go @@ -560,7 +560,6 @@ func constructOperationID( operationAttributes *DisplayAttributes, defaultPrefix string, ) string { - var ( prefix string suffix string From 1b5afe3c4f72c69b92b405683d1c681215f98b3f Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Mon, 27 Feb 2023 18:22:20 -0500 Subject: [PATCH 13/33] User OperationVerb since Action conflicts --- sdk/framework/openapi.go | 10 +++++----- sdk/framework/openapi_test.go | 18 +++++++++--------- sdk/framework/path.go | 12 ++++++++++-- 3 files changed, 24 insertions(+), 16 deletions(-) diff --git a/sdk/framework/openapi.go b/sdk/framework/openapi.go index 12d5cdd3e55a..5b0ec6955a4c 100644 --- a/sdk/framework/openapi.go +++ b/sdk/framework/openapi.go @@ -543,9 +543,9 @@ func specialPathMatch(path string, specialPaths []string) bool { // lower-case operation id, which is also used as a prefix for request and // response names. // -// The OperationPrefix / OperationSuffix / Action found in display attributes -// will be used, if provided. Otherwise, the function falls back to using the -// path and the operation. +// The OperationPrefix / -Verb / -Suffix found in display attributes will be +// used, if provided. Otherwise, the function falls back to using the path and +// the operation. // // Examples of generated operation identifiers: // - kvv2-write @@ -569,7 +569,7 @@ func constructOperationID( if operationAttributes != nil { prefix = operationAttributes.OperationPrefix suffix = operationAttributes.OperationSuffix - verb = operationAttributes.Action + verb = operationAttributes.OperationVerb } if pathAttributes != nil { @@ -580,7 +580,7 @@ func constructOperationID( suffix = pathAttributes.OperationSuffix } if verb == "" { - verb = pathAttributes.Action + verb = pathAttributes.OperationVerb } } diff --git a/sdk/framework/openapi_test.go b/sdk/framework/openapi_test.go index 680051401a73..475c99aaecda 100644 --- a/sdk/framework/openapi_test.go +++ b/sdk/framework/openapi_test.go @@ -641,39 +641,39 @@ func TestOpenAPI_constructOperationID(t *testing.T) { defaultPrefix: "test", expected: "better-prefix-write-better-suffix", }, - "operation-prefix-action-suffix": { + "operation-prefix-verb-suffix": { path: "path/to/thing", pathIndex: 0, - pathAttributes: &DisplayAttributes{OperationPrefix: "my-prefix", OperationSuffix: "my-suffix", Action: "Create"}, + pathAttributes: &DisplayAttributes{OperationPrefix: "my-prefix", OperationSuffix: "my-suffix", OperationVerb: "Create"}, operation: logical.UpdateOperation, operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix", OperationSuffix: "better-suffix"}, defaultPrefix: "test", expected: "better-prefix-create-better-suffix", }, - "operation-prefix-action-suffix-override": { + "operation-prefix-verb-suffix-override": { path: "path/to/thing", pathIndex: 0, - pathAttributes: &DisplayAttributes{OperationPrefix: "my-prefix", OperationSuffix: "my-suffix", Action: "Create"}, + pathAttributes: &DisplayAttributes{OperationPrefix: "my-prefix", OperationSuffix: "my-suffix", OperationVerb: "Create"}, operation: logical.UpdateOperation, - operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix", OperationSuffix: "better-suffix", Action: "Login"}, + operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix", OperationSuffix: "better-suffix", OperationVerb: "Login"}, defaultPrefix: "test", expected: "better-prefix-login-better-suffix", }, - "operation-prefix-action": { + "operation-prefix-verb": { path: "path/to/thing", pathIndex: 0, pathAttributes: nil, operation: logical.UpdateOperation, - operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix", Action: "Login"}, + operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix", OperationVerb: "Login"}, defaultPrefix: "test", expected: "better-prefix-login", }, - "operation-action-suffix": { + "operation-verb-suffix": { path: "path/to/thing", pathIndex: 0, pathAttributes: nil, operation: logical.UpdateOperation, - operationAttributes: &DisplayAttributes{Action: "Login", OperationSuffix: "better-suffix"}, + operationAttributes: &DisplayAttributes{OperationVerb: "Login", OperationSuffix: "better-suffix"}, defaultPrefix: "test", expected: "login-better-suffix", }, diff --git a/sdk/framework/path.go b/sdk/framework/path.go index 8393f1bf9ca8..1a9b1875acf9 100644 --- a/sdk/framework/path.go +++ b/sdk/framework/path.go @@ -224,10 +224,18 @@ type DisplayAttributes struct { // Action is the verb to use for the operation. Action string `json:"action,omitempty"` - // OperationPrefix is used to construct OpenAPI OperationID, if specified + // OperationPrefix is a hyphenated lower-case string used to construct + // OpenAPI OperationID, if specified. It's usually the name of the plugin. OperationPrefix string `json:"operationPrefix,omitempty"` - // OperationSuffix is used to construct OpenAPI OperationID, if specified + // OperationPrefix is a hyphenated lower-case string used to construct + // OpenAPI OperationID, if specified. It is typically an action to be + // performed (e.g. "write", "read", "login", etc.) + OperationVerb string `json:"operationVerb,omitempty"` + + // OperationPrefix is a hyphenated lower-case string used to construct + // OpenAPI OperationID, if specified. It is typically the name of the + // resource on which the action is performed, e.g. "cert", "config". OperationSuffix string `json:"operationSuffix,omitempty"` // EditType is the optional type of form field needed for a property From 54ad97a5a92dcc737268166e601990685e177e0a Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Mon, 27 Feb 2023 18:29:20 -0500 Subject: [PATCH 14/33] reorder vars --- sdk/framework/openapi.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/sdk/framework/openapi.go b/sdk/framework/openapi.go index 5b0ec6955a4c..fe5c35294dba 100644 --- a/sdk/framework/openapi.go +++ b/sdk/framework/openapi.go @@ -562,26 +562,26 @@ func constructOperationID( ) string { var ( prefix string - suffix string verb string + suffix string ) if operationAttributes != nil { prefix = operationAttributes.OperationPrefix - suffix = operationAttributes.OperationSuffix verb = operationAttributes.OperationVerb + suffix = operationAttributes.OperationSuffix } if pathAttributes != nil { if prefix == "" { prefix = pathAttributes.OperationPrefix } - if suffix == "" { - suffix = pathAttributes.OperationSuffix - } if verb == "" { verb = pathAttributes.OperationVerb } + if suffix == "" { + suffix = pathAttributes.OperationSuffix + } } // A single suffix string can contain multiple pipe-delimited strings. To @@ -621,17 +621,13 @@ func constructOperationID( // fall back to using the path + operation to construct the operation id needPrefix := prefix == "" && (suffix == "" || verb == "") - needSuffix := suffix == "" && (prefix == "" || verb == "" || pathIndex > 0) needVerb := verb == "" + needSuffix := suffix == "" && (prefix == "" || verb == "" || pathIndex > 0) if needPrefix { prefix = defaultPrefix } - if needSuffix { - suffix = hyphenate(nonWordRe.Split(strings.ToLower(path), -1)) - } - if needVerb { if operation == logical.UpdateOperation { verb = "write" @@ -640,6 +636,10 @@ func constructOperationID( } } + if needSuffix { + suffix = hyphenate(nonWordRe.Split(strings.ToLower(path), -1)) + } + return hyphenate([]string{prefix, verb, suffix}) } From 5ac38b33f99409b50a163a916a6323c80e9eef58 Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Mon, 27 Feb 2023 20:51:51 -0500 Subject: [PATCH 15/33] openapi: Add display attributes for Okta auth --- builtin/credential/okta/backend.go | 5 +++-- builtin/credential/okta/path_config.go | 10 +++++++--- builtin/credential/okta/path_groups.go | 23 +++++++++++++++-------- builtin/credential/okta/path_login.go | 6 ++++++ builtin/credential/okta/path_users.go | 23 +++++++++++++++-------- 5 files changed, 46 insertions(+), 21 deletions(-) diff --git a/builtin/credential/okta/backend.go b/builtin/credential/okta/backend.go index 58ba6b523f9d..caf398fa480b 100644 --- a/builtin/credential/okta/backend.go +++ b/builtin/credential/okta/backend.go @@ -15,8 +15,9 @@ import ( ) const ( - mfaPushMethod = "push" - mfaTOTPMethod = "token:software:totp" + operationPrefixOkta = "okta" + mfaPushMethod = "push" + mfaTOTPMethod = "token:software:totp" ) func Factory(ctx context.Context, conf *logical.BackendConfig) (logical.Backend, error) { diff --git a/builtin/credential/okta/path_config.go b/builtin/credential/okta/path_config.go index 7fc93efb87c7..3a59e41a4e73 100644 --- a/builtin/credential/okta/path_config.go +++ b/builtin/credential/okta/path_config.go @@ -24,6 +24,13 @@ const ( func pathConfig(b *backend) *framework.Path { p := &framework.Path{ Pattern: `config`, + + DisplayAttrs: &framework.DisplayAttributes{ + OperationPrefix: operationPrefixOkta, + OperationSuffix: "config", + Action: "Configure", + }, + Fields: map[string]*framework.FieldSchema{ "organization": { Type: framework.TypeString, @@ -89,9 +96,6 @@ func pathConfig(b *backend) *framework.Path { ExistenceCheck: b.pathConfigExistenceCheck, HelpSynopsis: pathConfigHelp, - DisplayAttrs: &framework.DisplayAttributes{ - Action: "Configure", - }, } tokenutil.AddTokenFields(p.Fields) diff --git a/builtin/credential/okta/path_groups.go b/builtin/credential/okta/path_groups.go index f9ff0225ac98..6f155b633313 100644 --- a/builtin/credential/okta/path_groups.go +++ b/builtin/credential/okta/path_groups.go @@ -13,22 +13,33 @@ func pathGroupsList(b *backend) *framework.Path { return &framework.Path{ Pattern: "groups/?$", + DisplayAttrs: &framework.DisplayAttributes{ + OperationPrefix: operationPrefixOkta, + OperationSuffix: "groups", + Navigation: true, + ItemType: "Group", + }, + Callbacks: map[logical.Operation]framework.OperationFunc{ logical.ListOperation: b.pathGroupList, }, HelpSynopsis: pathGroupHelpSyn, HelpDescription: pathGroupHelpDesc, - DisplayAttrs: &framework.DisplayAttributes{ - Navigation: true, - ItemType: "Group", - }, } } func pathGroups(b *backend) *framework.Path { return &framework.Path{ Pattern: `groups/(?P.+)`, + + DisplayAttrs: &framework.DisplayAttributes{ + OperationPrefix: operationPrefixOkta, + OperationSuffix: "group", + Action: "Create", + ItemType: "Group", + }, + Fields: map[string]*framework.FieldSchema{ "name": { Type: framework.TypeString, @@ -49,10 +60,6 @@ func pathGroups(b *backend) *framework.Path { HelpSynopsis: pathGroupHelpSyn, HelpDescription: pathGroupHelpDesc, - DisplayAttrs: &framework.DisplayAttributes{ - Action: "Create", - ItemType: "Group", - }, } } diff --git a/builtin/credential/okta/path_login.go b/builtin/credential/okta/path_login.go index 0f8967576bb7..bba478146c15 100644 --- a/builtin/credential/okta/path_login.go +++ b/builtin/credential/okta/path_login.go @@ -20,6 +20,12 @@ const ( func pathLogin(b *backend) *framework.Path { return &framework.Path{ Pattern: `login/(?P.+)`, + + DisplayAttrs: &framework.DisplayAttributes{ + OperationPrefix: operationPrefixOkta, + OperationVerb: "login", + }, + Fields: map[string]*framework.FieldSchema{ "username": { Type: framework.TypeString, diff --git a/builtin/credential/okta/path_users.go b/builtin/credential/okta/path_users.go index bd5fdc0ebbe0..63d11b03f433 100644 --- a/builtin/credential/okta/path_users.go +++ b/builtin/credential/okta/path_users.go @@ -11,22 +11,33 @@ func pathUsersList(b *backend) *framework.Path { return &framework.Path{ Pattern: "users/?$", + DisplayAttrs: &framework.DisplayAttributes{ + OperationPrefix: operationPrefixOkta, + OperationSuffix: "users", + Navigation: true, + ItemType: "User", + }, + Callbacks: map[logical.Operation]framework.OperationFunc{ logical.ListOperation: b.pathUserList, }, HelpSynopsis: pathUserHelpSyn, HelpDescription: pathUserHelpDesc, - DisplayAttrs: &framework.DisplayAttributes{ - Navigation: true, - ItemType: "User", - }, } } func pathUsers(b *backend) *framework.Path { return &framework.Path{ Pattern: `users/(?P.+)`, + + DisplayAttrs: &framework.DisplayAttributes{ + OperationPrefix: operationPrefixOkta, + OperationSuffix: "user", + Action: "Create", + ItemType: "User", + }, + Fields: map[string]*framework.FieldSchema{ "name": { Type: framework.TypeString, @@ -52,10 +63,6 @@ func pathUsers(b *backend) *framework.Path { HelpSynopsis: pathUserHelpSyn, HelpDescription: pathUserHelpDesc, - DisplayAttrs: &framework.DisplayAttributes{ - Action: "Create", - ItemType: "User", - }, } } From caf83524f62bc58667500520fde14cf6074f46af Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Mon, 27 Feb 2023 21:01:06 -0500 Subject: [PATCH 16/33] add verify display attrs --- builtin/credential/okta/path_login.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/builtin/credential/okta/path_login.go b/builtin/credential/okta/path_login.go index bba478146c15..87f5a4119b91 100644 --- a/builtin/credential/okta/path_login.go +++ b/builtin/credential/okta/path_login.go @@ -195,6 +195,10 @@ func (b *backend) pathLoginRenew(ctx context.Context, req *logical.Request, d *f func pathVerify(b *backend) *framework.Path { return &framework.Path{ Pattern: `verify/(?P.+)`, + DisplayAttrs: &framework.DisplayAttributes{ + OperationPrefix: operationPrefixOkta, + OperationVerb: "verify", + }, Fields: map[string]*framework.FieldSchema{ "nonce": { Type: framework.TypeString, From 3200825d040b8e4bcea1497e70aa0c9564109cd2 Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Mon, 27 Feb 2023 21:03:18 -0500 Subject: [PATCH 17/33] changelog --- changelog/19391.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/19391.txt diff --git a/changelog/19391.txt b/changelog/19391.txt new file mode 100644 index 000000000000..2c5da13bd249 --- /dev/null +++ b/changelog/19391.txt @@ -0,0 +1,3 @@ +```release-note:improvement +openapi: Add display attributes for Okta auth +``` From 3483f88af59655454408b6a1b86fc6e982158109 Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Tue, 28 Feb 2023 19:38:13 -0500 Subject: [PATCH 18/33] rm changelog --- changelog/19391.txt | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 changelog/19391.txt diff --git a/changelog/19391.txt b/changelog/19391.txt deleted file mode 100644 index 2c5da13bd249..000000000000 --- a/changelog/19391.txt +++ /dev/null @@ -1,3 +0,0 @@ -```release-note:improvement -openapi: Add display attributes for Okta auth -``` From 26b144e170c34a6aba873f0b0b998ccdb696f6ab Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Wed, 1 Mar 2023 16:50:04 -0500 Subject: [PATCH 19/33] allow verb-only --- sdk/framework/openapi.go | 8 +++++--- sdk/framework/openapi_test.go | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/sdk/framework/openapi.go b/sdk/framework/openapi.go index fe5c35294dba..cd83cc733b7a 100644 --- a/sdk/framework/openapi.go +++ b/sdk/framework/openapi.go @@ -620,9 +620,11 @@ func constructOperationID( } // fall back to using the path + operation to construct the operation id - needPrefix := prefix == "" && (suffix == "" || verb == "") - needVerb := verb == "" - needSuffix := suffix == "" && (prefix == "" || verb == "" || pathIndex > 0) + var ( + needPrefix = prefix == "" && verb == "" + needVerb = verb == "" + needSuffix = suffix == "" && (verb == "" || pathIndex > 0) + ) if needPrefix { prefix = defaultPrefix diff --git a/sdk/framework/openapi_test.go b/sdk/framework/openapi_test.go index 475c99aaecda..bc63f2cdd7a9 100644 --- a/sdk/framework/openapi_test.go +++ b/sdk/framework/openapi_test.go @@ -605,6 +605,24 @@ func TestOpenAPI_constructOperationID(t *testing.T) { defaultPrefix: "test", expected: "test-write-path-to-thing", }, + "operation-verb": { + path: "path/to/thing", + pathIndex: 0, + pathAttributes: &DisplayAttributes{OperationVerb: "do-something"}, + operation: logical.UpdateOperation, + operationAttributes: nil, + defaultPrefix: "test", + expected: "do-something", + }, + "operation-verb-override": { + path: "path/to/thing", + pathIndex: 0, + pathAttributes: &DisplayAttributes{OperationVerb: "do-something"}, + operation: logical.UpdateOperation, + operationAttributes: &DisplayAttributes{OperationVerb: "do-something-else"}, + defaultPrefix: "test", + expected: "do-something-else", + }, "operation-prefix": { path: "path/to/thing", pathIndex: 0, From 49eda184bfd300a0fa87287dcc08dd06d6bdcfaa Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Wed, 1 Mar 2023 16:57:27 -0500 Subject: [PATCH 20/33] better comments --- sdk/framework/path.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/sdk/framework/path.go b/sdk/framework/path.go index 1a9b1875acf9..ec123644bf5d 100644 --- a/sdk/framework/path.go +++ b/sdk/framework/path.go @@ -225,17 +225,19 @@ type DisplayAttributes struct { Action string `json:"action,omitempty"` // OperationPrefix is a hyphenated lower-case string used to construct - // OpenAPI OperationID, if specified. It's usually the name of the plugin. + // OpenAPI OperationID. It is typically the name of the plugin. OperationPrefix string `json:"operationPrefix,omitempty"` // OperationPrefix is a hyphenated lower-case string used to construct - // OpenAPI OperationID, if specified. It is typically an action to be - // performed (e.g. "write", "read", "login", etc.) + // OpenAPI OperationID. It is typically an action to be performed + // (e.g. "generate", "sign", "login", etc.) OperationVerb string `json:"operationVerb,omitempty"` // OperationPrefix is a hyphenated lower-case string used to construct - // OpenAPI OperationID, if specified. It is typically the name of the - // resource on which the action is performed, e.g. "cert", "config". + // OpenAPI OperationID. It is typically the name of the resource on which + // the action is performed, e.g. "cert", "config". A pipe (|) separator can + // be used to list different suffixes for various permutations of the + // `Path.Pattern` regular expression. OperationSuffix string `json:"operationSuffix,omitempty"` // EditType is the optional type of form field needed for a property From 72c4acd2d43353cdec4b04330a778b764ef535b0 Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Fri, 3 Mar 2023 16:25:36 -0500 Subject: [PATCH 21/33] more comments, better example --- sdk/framework/openapi.go | 16 +++++++++------- sdk/framework/path.go | 10 ++++++---- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/sdk/framework/openapi.go b/sdk/framework/openapi.go index cd83cc733b7a..01e3ea757b80 100644 --- a/sdk/framework/openapi.go +++ b/sdk/framework/openapi.go @@ -588,16 +588,18 @@ func constructOperationID( // determine the actual suffix, we attempt to match it by the index of the // paths returned from `expandPattern(...)`. For example: // - // aws/ - // Pattern: `^(creds|sts)/(?P\w(([\w-.@]+)?\w)?)$` + // pki/ + // Pattern: "keys/generate/(internal|exported|kms)", // DisplayAttrs: { - // OperationSuffix: "credentials|sts-credentials" - // } + // ... + // OperationSuffix: "internal-key|exported-key|kms-key", + // }, // - // Will expand into two paths and corresponding suffixes: + // will expand into three paths and corresponding suffixes: // - // path 0: "creds/{name}" suffix: credentials - // path 1: "sts/{name}" suffix: sts-credentials + // path 0: "keys/generate/internal" suffix: internal-key + // path 1: "keys/generate/exported" suffix: exported-key + // path 2: "keys/generate/kms" suffix: kms-key // if suffixes := strings.Split(suffix, "|"); len(suffixes) > 1 || pathIndex > 0 { // if the index is out of bounds, fall back to the old logic diff --git a/sdk/framework/path.go b/sdk/framework/path.go index ec123644bf5d..d9723e3d604e 100644 --- a/sdk/framework/path.go +++ b/sdk/framework/path.go @@ -230,14 +230,16 @@ type DisplayAttributes struct { // OperationPrefix is a hyphenated lower-case string used to construct // OpenAPI OperationID. It is typically an action to be performed - // (e.g. "generate", "sign", "login", etc.) + // (e.g. "generate", "sign", "login", etc.). If not specified, the verb + // defaults to `logical.Operation.String()` (e.g. "read", "delete", etc.). OperationVerb string `json:"operationVerb,omitempty"` // OperationPrefix is a hyphenated lower-case string used to construct // OpenAPI OperationID. It is typically the name of the resource on which - // the action is performed, e.g. "cert", "config". A pipe (|) separator can - // be used to list different suffixes for various permutations of the - // `Path.Pattern` regular expression. + // the action is performed (e.g. "role", "credentials", etc.). A pipe (|) + // separator can be used to list different suffixes for various permutations + // of the `Path.Pattern` regular expression. If not specified, the suffix + // defaults to the `Path.Pattern` split by dashes. OperationSuffix string `json:"operationSuffix,omitempty"` // EditType is the optional type of form field needed for a property From d6c2a45a344ca94551cf4ac322436068dccb18a6 Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Sun, 12 Mar 2023 12:31:23 -0400 Subject: [PATCH 22/33] better name for helper --- sdk/framework/openapi.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sdk/framework/openapi.go b/sdk/framework/openapi.go index 01e3ea757b80..0514f4a9d321 100644 --- a/sdk/framework/openapi.go +++ b/sdk/framework/openapi.go @@ -610,8 +610,8 @@ func constructOperationID( } } - // hyphenate is a helper that hyphenates the given slice except the empty elements - hyphenate := func(parts []string) string { + // a helper that hyphenates & lower-cases the slice except the empty elements + toLowerHyphenate := func(parts []string) string { filtered := make([]string, 0, len(parts)) for _, e := range parts { if e != "" { @@ -641,10 +641,10 @@ func constructOperationID( } if needSuffix { - suffix = hyphenate(nonWordRe.Split(strings.ToLower(path), -1)) + suffix = toLowerHyphenate(nonWordRe.Split(path, -1)) } - return hyphenate([]string{prefix, verb, suffix}) + return toLowerHyphenate([]string{prefix, verb, suffix}) } // expandPattern expands a regex pattern by generating permutations of any optional parameters From f68771435a2242d5852e46af7e8f638989c1447d Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Mon, 13 Mar 2023 17:22:20 -0400 Subject: [PATCH 23/33] config -> configure --- builtin/credential/okta/path_config.go | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/builtin/credential/okta/path_config.go b/builtin/credential/okta/path_config.go index 3a59e41a4e73..27b280acd41c 100644 --- a/builtin/credential/okta/path_config.go +++ b/builtin/credential/okta/path_config.go @@ -27,7 +27,6 @@ func pathConfig(b *backend) *framework.Path { DisplayAttrs: &framework.DisplayAttributes{ OperationPrefix: operationPrefixOkta, - OperationSuffix: "config", Action: "Configure", }, @@ -87,10 +86,25 @@ func pathConfig(b *backend) *framework.Path { }, }, - Callbacks: map[logical.Operation]framework.OperationFunc{ - logical.ReadOperation: b.pathConfigRead, - logical.CreateOperation: b.pathConfigWrite, - logical.UpdateOperation: b.pathConfigWrite, + Operations: map[logical.Operation]framework.OperationHandler{ + logical.ReadOperation: &framework.PathOperation{ + Callback: b.pathConfigRead, + DisplayAttrs: &framework.DisplayAttributes{ + OperationSuffix: "config", + }, + }, + logical.CreateOperation: &framework.PathOperation{ + Callback: b.pathConfigWrite, + DisplayAttrs: &framework.DisplayAttributes{ + OperationSuffix: "config", + }, + }, + logical.UpdateOperation: &framework.PathOperation{ + Callback: b.pathConfigWrite, + DisplayAttrs: &framework.DisplayAttributes{ + OperationVerb: "configure", + }, + }, }, ExistenceCheck: b.pathConfigExistenceCheck, From 22fa3e976925d0084f6ce70c60cee5095a5dfed8 Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Mon, 13 Mar 2023 17:24:11 -0400 Subject: [PATCH 24/33] config -> configure --- builtin/credential/okta/path_config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/credential/okta/path_config.go b/builtin/credential/okta/path_config.go index 27b280acd41c..88679decb570 100644 --- a/builtin/credential/okta/path_config.go +++ b/builtin/credential/okta/path_config.go @@ -96,7 +96,7 @@ func pathConfig(b *backend) *framework.Path { logical.CreateOperation: &framework.PathOperation{ Callback: b.pathConfigWrite, DisplayAttrs: &framework.DisplayAttributes{ - OperationSuffix: "config", + OperationVerb: "configure", }, }, logical.UpdateOperation: &framework.PathOperation{ From 156d62333f969e1c7b775e8b1d62ec9933823987 Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Mon, 20 Mar 2023 20:11:33 -0400 Subject: [PATCH 25/33] configuration --- builtin/credential/okta/path_config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/credential/okta/path_config.go b/builtin/credential/okta/path_config.go index 88679decb570..1289ba451059 100644 --- a/builtin/credential/okta/path_config.go +++ b/builtin/credential/okta/path_config.go @@ -90,7 +90,7 @@ func pathConfig(b *backend) *framework.Path { logical.ReadOperation: &framework.PathOperation{ Callback: b.pathConfigRead, DisplayAttrs: &framework.DisplayAttributes{ - OperationSuffix: "config", + OperationSuffix: "configuration", }, }, logical.CreateOperation: &framework.PathOperation{ From 2c504ae59c27ce1a727a7871e1f53352bf4cee05 Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Mon, 20 Mar 2023 20:31:21 -0400 Subject: [PATCH 26/33] log-in --- builtin/credential/okta/path_login.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/credential/okta/path_login.go b/builtin/credential/okta/path_login.go index 87f5a4119b91..f6eca73c08ba 100644 --- a/builtin/credential/okta/path_login.go +++ b/builtin/credential/okta/path_login.go @@ -23,7 +23,7 @@ func pathLogin(b *backend) *framework.Path { DisplayAttrs: &framework.DisplayAttributes{ OperationPrefix: operationPrefixOkta, - OperationVerb: "login", + OperationVerb: "log-in", }, Fields: map[string]*framework.FieldSchema{ From af41a737ad175189642980935b230d071ae240eb Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Tue, 21 Mar 2023 22:19:10 -0400 Subject: [PATCH 27/33] allow empty multi-field suffixes --- sdk/framework/openapi.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/sdk/framework/openapi.go b/sdk/framework/openapi.go index 0514f4a9d321..d1dba914a6f2 100644 --- a/sdk/framework/openapi.go +++ b/sdk/framework/openapi.go @@ -601,10 +601,13 @@ func constructOperationID( // path 1: "keys/generate/exported" suffix: exported-key // path 2: "keys/generate/kms" suffix: kms-key // + pathIndexOutOfRange := false + if suffixes := strings.Split(suffix, "|"); len(suffixes) > 1 || pathIndex > 0 { // if the index is out of bounds, fall back to the old logic if pathIndex >= len(suffixes) { suffix = "" + pathIndexOutOfRange = true } else { suffix = suffixes[pathIndex] } @@ -625,7 +628,7 @@ func constructOperationID( var ( needPrefix = prefix == "" && verb == "" needVerb = verb == "" - needSuffix = suffix == "" && (verb == "" || pathIndex > 0) + needSuffix = suffix == "" && (verb == "" || pathIndexOutOfRange) ) if needPrefix { From 243c898b05c89f1cd3d32f23aa27b98c640d4ee7 Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Thu, 23 Mar 2023 18:16:34 -0400 Subject: [PATCH 28/33] add withoutOperationHints --- sdk/framework/openapi.go | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/sdk/framework/openapi.go b/sdk/framework/openapi.go index f575055fe7f4..776f2b1cef5b 100644 --- a/sdk/framework/openapi.go +++ b/sdk/framework/openapi.go @@ -250,7 +250,7 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st pi.Sudo = specialPathMatch(path, sudoPaths) pi.Unauthenticated = specialPathMatch(path, unauthPaths) - pi.DisplayAttrs = p.DisplayAttrs + pi.DisplayAttrs = withoutOperationHints(p.DisplayAttrs) // If the newer style Operations map isn't defined, create one from the legacy fields. operations := p.Operations @@ -292,7 +292,7 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st Pattern: t.pattern, Enum: field.AllowedValues, Default: field.Default, - DisplayAttrs: field.DisplayAttrs, + DisplayAttrs: withoutOperationHints(field.DisplayAttrs), }, Required: required, Deprecated: field.Deprecated, @@ -371,7 +371,7 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st Enum: field.AllowedValues, Default: field.Default, Deprecated: field.Deprecated, - DisplayAttrs: field.DisplayAttrs, + DisplayAttrs: withoutOperationHints(field.DisplayAttrs), } if openapiField.baseType == "array" { p.Items = &OASSchema{ @@ -485,7 +485,7 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st Enum: field.AllowedValues, Default: field.Default, Deprecated: field.Deprecated, - DisplayAttrs: field.DisplayAttrs, + DisplayAttrs: withoutOperationHints(field.DisplayAttrs), } if openapiField.baseType == "array" { p.Items = &OASSchema{ @@ -982,6 +982,19 @@ func splitFields(allFields map[string]*FieldSchema, pattern string) (pathFields, return pathFields, bodyFields } +// withoutOperationHints returns a copy of the given DisplayAttributes without +// OperationPrefix / OperationVerb / OperationSuffix since we don't need these +// fields in the final output. +func withoutOperationHints(in *DisplayAttributes) *DisplayAttributes { + copy := *in + + copy.OperationPrefix = "" + copy.OperationVerb = "" + copy.OperationSuffix = "" + + return © +} + // cleanedResponse is identical to logical.Response but with nulls // removed from from JSON encoding type cleanedResponse struct { From d37caaa61411efe236c5c4064dc1a204c5592103 Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Thu, 23 Mar 2023 18:24:38 -0400 Subject: [PATCH 29/33] nil check --- sdk/framework/openapi.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sdk/framework/openapi.go b/sdk/framework/openapi.go index 776f2b1cef5b..94aba14c93db 100644 --- a/sdk/framework/openapi.go +++ b/sdk/framework/openapi.go @@ -986,6 +986,10 @@ func splitFields(allFields map[string]*FieldSchema, pattern string) (pathFields, // OperationPrefix / OperationVerb / OperationSuffix since we don't need these // fields in the final output. func withoutOperationHints(in *DisplayAttributes) *DisplayAttributes { + if in == nil { + return nil + } + copy := *in copy.OperationPrefix = "" From cb5aa04f2cde3d5f5a776cdcb4705ceb6e255bb8 Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Thu, 23 Mar 2023 18:34:20 -0400 Subject: [PATCH 30/33] empty obj check --- sdk/framework/openapi.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sdk/framework/openapi.go b/sdk/framework/openapi.go index 94aba14c93db..2e8cafe3afa5 100644 --- a/sdk/framework/openapi.go +++ b/sdk/framework/openapi.go @@ -996,6 +996,11 @@ func withoutOperationHints(in *DisplayAttributes) *DisplayAttributes { copy.OperationVerb = "" copy.OperationSuffix = "" + // return nil if all fields are empty to avoid empty JSON objects + if copy == (DisplayAttributes{}) { + return nil + } + return © } From 7aacc2c255b70f690d376f3af328831d27b3979c Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Thu, 30 Mar 2023 17:42:38 -0400 Subject: [PATCH 31/33] write -> create-or-update --- sdk/framework/openapi.go | 20 ++++++--- sdk/framework/openapi_test.go | 60 +++++++++++++++++++------- sdk/framework/testdata/legacy.json | 6 +-- sdk/framework/testdata/operations.json | 6 +-- 4 files changed, 65 insertions(+), 27 deletions(-) diff --git a/sdk/framework/openapi.go b/sdk/framework/openapi.go index 2e8cafe3afa5..64770aff8f0d 100644 --- a/sdk/framework/openapi.go +++ b/sdk/framework/openapi.go @@ -305,6 +305,13 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st return strings.ToLower(pi.Parameters[i].Name) < strings.ToLower(pi.Parameters[j].Name) }) + for opType := range operations { + if opType == logical.CreateOperation { + pi.CreateSupported = true + break + } + } + // 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 { @@ -314,8 +321,6 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st } if opType == logical.CreateOperation { - pi.CreateSupported = true - // If both Create and Update are defined, only process Update. if operations[logical.UpdateOperation] != nil { continue @@ -335,6 +340,7 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st p.DisplayAttrs, opType, props.DisplayAttrs, + pi.CreateSupported, requestResponsePrefix, ) @@ -599,6 +605,7 @@ func constructOperationID( pathAttributes *DisplayAttributes, operation logical.Operation, operationAttributes *DisplayAttributes, + createSupported bool, defaultPrefix string, ) string { var ( @@ -677,9 +684,12 @@ func constructOperationID( } if needVerb { - if operation == logical.UpdateOperation { - verb = "write" - } else { + switch { + case operation == logical.UpdateOperation && createSupported == true: + verb = "create-or-update" + case operation == logical.UpdateOperation && createSupported == false: + verb = "update" + default: verb = string(operation) } } diff --git a/sdk/framework/openapi_test.go b/sdk/framework/openapi_test.go index cd0105cdeee7..2fdf609d6024 100644 --- a/sdk/framework/openapi_test.go +++ b/sdk/framework/openapi_test.go @@ -659,6 +659,7 @@ func TestOpenAPI_constructOperationID(t *testing.T) { pathAttributes *DisplayAttributes operation logical.Operation operationAttributes *DisplayAttributes + createSupported bool defaultPrefix string expected string }{ @@ -668,6 +669,7 @@ func TestOpenAPI_constructOperationID(t *testing.T) { pathAttributes: nil, operation: logical.Operation(""), operationAttributes: nil, + createSupported: false, defaultPrefix: "", expected: "", }, @@ -677,17 +679,29 @@ func TestOpenAPI_constructOperationID(t *testing.T) { pathAttributes: nil, operation: logical.ReadOperation, operationAttributes: nil, + createSupported: false, defaultPrefix: "test", expected: "test-read-path-to-thing", }, - "simple-write": { + "simple-update": { path: "path/to/thing", pathIndex: 0, pathAttributes: nil, operation: logical.UpdateOperation, operationAttributes: nil, + createSupported: false, defaultPrefix: "test", - expected: "test-write-path-to-thing", + expected: "test-update-path-to-thing", + }, + "simple-create-or-update": { + path: "path/to/thing", + pathIndex: 0, + pathAttributes: nil, + operation: logical.UpdateOperation, + operationAttributes: nil, + createSupported: true, + defaultPrefix: "test", + expected: "test-create-or-update-path-to-thing", }, "operation-verb": { path: "path/to/thing", @@ -695,6 +709,7 @@ func TestOpenAPI_constructOperationID(t *testing.T) { pathAttributes: &DisplayAttributes{OperationVerb: "do-something"}, operation: logical.UpdateOperation, operationAttributes: nil, + createSupported: false, defaultPrefix: "test", expected: "do-something", }, @@ -704,6 +719,7 @@ func TestOpenAPI_constructOperationID(t *testing.T) { pathAttributes: &DisplayAttributes{OperationVerb: "do-something"}, operation: logical.UpdateOperation, operationAttributes: &DisplayAttributes{OperationVerb: "do-something-else"}, + createSupported: false, defaultPrefix: "test", expected: "do-something-else", }, @@ -713,35 +729,39 @@ func TestOpenAPI_constructOperationID(t *testing.T) { pathAttributes: &DisplayAttributes{OperationPrefix: "my-prefix"}, operation: logical.UpdateOperation, operationAttributes: nil, + createSupported: false, defaultPrefix: "test", - expected: "my-prefix-write-path-to-thing", + expected: "my-prefix-update-path-to-thing", }, "operation-prefix-override": { path: "path/to/thing", pathIndex: 0, pathAttributes: &DisplayAttributes{OperationPrefix: "my-prefix"}, - operation: logical.UpdateOperation, + operation: logical.DeleteOperation, operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix"}, + createSupported: false, defaultPrefix: "test", - expected: "better-prefix-write-path-to-thing", + expected: "better-prefix-delete-path-to-thing", }, "operation-prefix-and-suffix": { path: "path/to/thing", pathIndex: 0, pathAttributes: &DisplayAttributes{OperationPrefix: "my-prefix", OperationSuffix: "my-suffix"}, - operation: logical.UpdateOperation, + operation: logical.ListOperation, operationAttributes: nil, + createSupported: false, defaultPrefix: "test", - expected: "my-prefix-write-my-suffix", + expected: "my-prefix-list-my-suffix", }, "operation-prefix-and-suffix-override": { path: "path/to/thing", pathIndex: 0, pathAttributes: &DisplayAttributes{OperationPrefix: "my-prefix", OperationSuffix: "my-suffix"}, - operation: logical.UpdateOperation, + operation: logical.ReadOperation, operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix", OperationSuffix: "better-suffix"}, + createSupported: false, defaultPrefix: "test", - expected: "better-prefix-write-better-suffix", + expected: "better-prefix-read-better-suffix", }, "operation-prefix-verb-suffix": { path: "path/to/thing", @@ -749,15 +769,17 @@ func TestOpenAPI_constructOperationID(t *testing.T) { pathAttributes: &DisplayAttributes{OperationPrefix: "my-prefix", OperationSuffix: "my-suffix", OperationVerb: "Create"}, operation: logical.UpdateOperation, operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix", OperationSuffix: "better-suffix"}, + createSupported: false, defaultPrefix: "test", expected: "better-prefix-create-better-suffix", }, "operation-prefix-verb-suffix-override": { path: "path/to/thing", pathIndex: 0, - pathAttributes: &DisplayAttributes{OperationPrefix: "my-prefix", OperationSuffix: "my-suffix", OperationVerb: "Create"}, + pathAttributes: &DisplayAttributes{OperationPrefix: "my-prefix", OperationSuffix: "my-suffix", OperationVerb: "create"}, operation: logical.UpdateOperation, - operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix", OperationSuffix: "better-suffix", OperationVerb: "Login"}, + operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix", OperationSuffix: "better-suffix", OperationVerb: "login"}, + createSupported: false, defaultPrefix: "test", expected: "better-prefix-login-better-suffix", }, @@ -766,7 +788,8 @@ func TestOpenAPI_constructOperationID(t *testing.T) { pathIndex: 0, pathAttributes: nil, operation: logical.UpdateOperation, - operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix", OperationVerb: "Login"}, + operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix", OperationVerb: "login"}, + createSupported: false, defaultPrefix: "test", expected: "better-prefix-login", }, @@ -775,7 +798,8 @@ func TestOpenAPI_constructOperationID(t *testing.T) { pathIndex: 0, pathAttributes: nil, operation: logical.UpdateOperation, - operationAttributes: &DisplayAttributes{OperationVerb: "Login", OperationSuffix: "better-suffix"}, + operationAttributes: &DisplayAttributes{OperationVerb: "login", OperationSuffix: "better-suffix"}, + createSupported: false, defaultPrefix: "test", expected: "login-better-suffix", }, @@ -785,8 +809,9 @@ func TestOpenAPI_constructOperationID(t *testing.T) { pathAttributes: nil, operation: logical.UpdateOperation, operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix", OperationSuffix: "suffix0|suffix1"}, + createSupported: false, defaultPrefix: "test", - expected: "better-prefix-write-suffix0", + expected: "better-prefix-update-suffix0", }, "pipe-delimited-suffix-1": { path: "path/to/thing", @@ -794,8 +819,9 @@ func TestOpenAPI_constructOperationID(t *testing.T) { pathAttributes: nil, operation: logical.UpdateOperation, operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix", OperationSuffix: "suffix0|suffix1"}, + createSupported: true, defaultPrefix: "test", - expected: "better-prefix-write-suffix1", + expected: "better-prefix-create-or-update-suffix1", }, "pipe-delimited-suffix-2-fallback": { path: "path/to/thing", @@ -803,8 +829,9 @@ func TestOpenAPI_constructOperationID(t *testing.T) { pathAttributes: nil, operation: logical.UpdateOperation, operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix", OperationSuffix: "suffix0|suffix1"}, + createSupported: false, defaultPrefix: "test", - expected: "better-prefix-write-path-to-thing", + expected: "better-prefix-update-path-to-thing", }, } @@ -818,6 +845,7 @@ func TestOpenAPI_constructOperationID(t *testing.T) { test.pathAttributes, test.operation, test.operationAttributes, + test.createSupported, test.defaultPrefix, ) if actual != test.expected { diff --git a/sdk/framework/testdata/legacy.json b/sdk/framework/testdata/legacy.json index 748a1d7ba8b0..7c15e6df28a3 100644 --- a/sdk/framework/testdata/legacy.json +++ b/sdk/framework/testdata/legacy.json @@ -36,7 +36,7 @@ } }, "post": { - "operationId": "kv-write-lookup-id", + "operationId": "kv-update-lookup-id", "summary": "Synopsis", "tags": [ "secrets" @@ -46,7 +46,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/kv-write-lookup-id-request" + "$ref": "#/components/schemas/kv-update-lookup-id-request" } } } @@ -61,7 +61,7 @@ }, "components": { "schemas": { - "kv-write-lookup-id-request": { + "kv-update-lookup-id-request": { "type": "object", "properties": { "token": { diff --git a/sdk/framework/testdata/operations.json b/sdk/framework/testdata/operations.json index df8fed8bddfb..674e3bea0a3a 100644 --- a/sdk/framework/testdata/operations.json +++ b/sdk/framework/testdata/operations.json @@ -60,7 +60,7 @@ ] }, "post": { - "operationId": "kv-write-foo-id", + "operationId": "kv-create-or-update-foo-id", "tags": [ "secrets" ], @@ -71,7 +71,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/kv-write-foo-id-request" + "$ref": "#/components/schemas/kv-create-or-update-foo-id-request" } } } @@ -86,7 +86,7 @@ }, "components": { "schemas": { - "kv-write-foo-id-request": { + "kv-create-or-update-foo-id-request": { "type": "object", "required": [ "age" From ed3a56d959e56ed4a400b88c24b999894999ea1b Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Fri, 31 Mar 2023 18:58:52 -0400 Subject: [PATCH 32/33] Revert "write -> create-or-update" This reverts commit 7aacc2c255b70f690d376f3af328831d27b3979c. --- sdk/framework/openapi.go | 20 +++------ sdk/framework/openapi_test.go | 60 +++++++------------------- sdk/framework/testdata/legacy.json | 6 +-- sdk/framework/testdata/operations.json | 6 +-- 4 files changed, 27 insertions(+), 65 deletions(-) diff --git a/sdk/framework/openapi.go b/sdk/framework/openapi.go index 64770aff8f0d..2e8cafe3afa5 100644 --- a/sdk/framework/openapi.go +++ b/sdk/framework/openapi.go @@ -305,13 +305,6 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st return strings.ToLower(pi.Parameters[i].Name) < strings.ToLower(pi.Parameters[j].Name) }) - for opType := range operations { - if opType == logical.CreateOperation { - pi.CreateSupported = true - break - } - } - // 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 { @@ -321,6 +314,8 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st } if opType == logical.CreateOperation { + pi.CreateSupported = true + // If both Create and Update are defined, only process Update. if operations[logical.UpdateOperation] != nil { continue @@ -340,7 +335,6 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st p.DisplayAttrs, opType, props.DisplayAttrs, - pi.CreateSupported, requestResponsePrefix, ) @@ -605,7 +599,6 @@ func constructOperationID( pathAttributes *DisplayAttributes, operation logical.Operation, operationAttributes *DisplayAttributes, - createSupported bool, defaultPrefix string, ) string { var ( @@ -684,12 +677,9 @@ func constructOperationID( } if needVerb { - switch { - case operation == logical.UpdateOperation && createSupported == true: - verb = "create-or-update" - case operation == logical.UpdateOperation && createSupported == false: - verb = "update" - default: + if operation == logical.UpdateOperation { + verb = "write" + } else { verb = string(operation) } } diff --git a/sdk/framework/openapi_test.go b/sdk/framework/openapi_test.go index 2fdf609d6024..cd0105cdeee7 100644 --- a/sdk/framework/openapi_test.go +++ b/sdk/framework/openapi_test.go @@ -659,7 +659,6 @@ func TestOpenAPI_constructOperationID(t *testing.T) { pathAttributes *DisplayAttributes operation logical.Operation operationAttributes *DisplayAttributes - createSupported bool defaultPrefix string expected string }{ @@ -669,7 +668,6 @@ func TestOpenAPI_constructOperationID(t *testing.T) { pathAttributes: nil, operation: logical.Operation(""), operationAttributes: nil, - createSupported: false, defaultPrefix: "", expected: "", }, @@ -679,29 +677,17 @@ func TestOpenAPI_constructOperationID(t *testing.T) { pathAttributes: nil, operation: logical.ReadOperation, operationAttributes: nil, - createSupported: false, defaultPrefix: "test", expected: "test-read-path-to-thing", }, - "simple-update": { + "simple-write": { path: "path/to/thing", pathIndex: 0, pathAttributes: nil, operation: logical.UpdateOperation, operationAttributes: nil, - createSupported: false, defaultPrefix: "test", - expected: "test-update-path-to-thing", - }, - "simple-create-or-update": { - path: "path/to/thing", - pathIndex: 0, - pathAttributes: nil, - operation: logical.UpdateOperation, - operationAttributes: nil, - createSupported: true, - defaultPrefix: "test", - expected: "test-create-or-update-path-to-thing", + expected: "test-write-path-to-thing", }, "operation-verb": { path: "path/to/thing", @@ -709,7 +695,6 @@ func TestOpenAPI_constructOperationID(t *testing.T) { pathAttributes: &DisplayAttributes{OperationVerb: "do-something"}, operation: logical.UpdateOperation, operationAttributes: nil, - createSupported: false, defaultPrefix: "test", expected: "do-something", }, @@ -719,7 +704,6 @@ func TestOpenAPI_constructOperationID(t *testing.T) { pathAttributes: &DisplayAttributes{OperationVerb: "do-something"}, operation: logical.UpdateOperation, operationAttributes: &DisplayAttributes{OperationVerb: "do-something-else"}, - createSupported: false, defaultPrefix: "test", expected: "do-something-else", }, @@ -729,39 +713,35 @@ func TestOpenAPI_constructOperationID(t *testing.T) { pathAttributes: &DisplayAttributes{OperationPrefix: "my-prefix"}, operation: logical.UpdateOperation, operationAttributes: nil, - createSupported: false, defaultPrefix: "test", - expected: "my-prefix-update-path-to-thing", + expected: "my-prefix-write-path-to-thing", }, "operation-prefix-override": { path: "path/to/thing", pathIndex: 0, pathAttributes: &DisplayAttributes{OperationPrefix: "my-prefix"}, - operation: logical.DeleteOperation, + operation: logical.UpdateOperation, operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix"}, - createSupported: false, defaultPrefix: "test", - expected: "better-prefix-delete-path-to-thing", + expected: "better-prefix-write-path-to-thing", }, "operation-prefix-and-suffix": { path: "path/to/thing", pathIndex: 0, pathAttributes: &DisplayAttributes{OperationPrefix: "my-prefix", OperationSuffix: "my-suffix"}, - operation: logical.ListOperation, + operation: logical.UpdateOperation, operationAttributes: nil, - createSupported: false, defaultPrefix: "test", - expected: "my-prefix-list-my-suffix", + expected: "my-prefix-write-my-suffix", }, "operation-prefix-and-suffix-override": { path: "path/to/thing", pathIndex: 0, pathAttributes: &DisplayAttributes{OperationPrefix: "my-prefix", OperationSuffix: "my-suffix"}, - operation: logical.ReadOperation, + operation: logical.UpdateOperation, operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix", OperationSuffix: "better-suffix"}, - createSupported: false, defaultPrefix: "test", - expected: "better-prefix-read-better-suffix", + expected: "better-prefix-write-better-suffix", }, "operation-prefix-verb-suffix": { path: "path/to/thing", @@ -769,17 +749,15 @@ func TestOpenAPI_constructOperationID(t *testing.T) { pathAttributes: &DisplayAttributes{OperationPrefix: "my-prefix", OperationSuffix: "my-suffix", OperationVerb: "Create"}, operation: logical.UpdateOperation, operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix", OperationSuffix: "better-suffix"}, - createSupported: false, defaultPrefix: "test", expected: "better-prefix-create-better-suffix", }, "operation-prefix-verb-suffix-override": { path: "path/to/thing", pathIndex: 0, - pathAttributes: &DisplayAttributes{OperationPrefix: "my-prefix", OperationSuffix: "my-suffix", OperationVerb: "create"}, + pathAttributes: &DisplayAttributes{OperationPrefix: "my-prefix", OperationSuffix: "my-suffix", OperationVerb: "Create"}, operation: logical.UpdateOperation, - operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix", OperationSuffix: "better-suffix", OperationVerb: "login"}, - createSupported: false, + operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix", OperationSuffix: "better-suffix", OperationVerb: "Login"}, defaultPrefix: "test", expected: "better-prefix-login-better-suffix", }, @@ -788,8 +766,7 @@ func TestOpenAPI_constructOperationID(t *testing.T) { pathIndex: 0, pathAttributes: nil, operation: logical.UpdateOperation, - operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix", OperationVerb: "login"}, - createSupported: false, + operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix", OperationVerb: "Login"}, defaultPrefix: "test", expected: "better-prefix-login", }, @@ -798,8 +775,7 @@ func TestOpenAPI_constructOperationID(t *testing.T) { pathIndex: 0, pathAttributes: nil, operation: logical.UpdateOperation, - operationAttributes: &DisplayAttributes{OperationVerb: "login", OperationSuffix: "better-suffix"}, - createSupported: false, + operationAttributes: &DisplayAttributes{OperationVerb: "Login", OperationSuffix: "better-suffix"}, defaultPrefix: "test", expected: "login-better-suffix", }, @@ -809,9 +785,8 @@ func TestOpenAPI_constructOperationID(t *testing.T) { pathAttributes: nil, operation: logical.UpdateOperation, operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix", OperationSuffix: "suffix0|suffix1"}, - createSupported: false, defaultPrefix: "test", - expected: "better-prefix-update-suffix0", + expected: "better-prefix-write-suffix0", }, "pipe-delimited-suffix-1": { path: "path/to/thing", @@ -819,9 +794,8 @@ func TestOpenAPI_constructOperationID(t *testing.T) { pathAttributes: nil, operation: logical.UpdateOperation, operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix", OperationSuffix: "suffix0|suffix1"}, - createSupported: true, defaultPrefix: "test", - expected: "better-prefix-create-or-update-suffix1", + expected: "better-prefix-write-suffix1", }, "pipe-delimited-suffix-2-fallback": { path: "path/to/thing", @@ -829,9 +803,8 @@ func TestOpenAPI_constructOperationID(t *testing.T) { pathAttributes: nil, operation: logical.UpdateOperation, operationAttributes: &DisplayAttributes{OperationPrefix: "better-prefix", OperationSuffix: "suffix0|suffix1"}, - createSupported: false, defaultPrefix: "test", - expected: "better-prefix-update-path-to-thing", + expected: "better-prefix-write-path-to-thing", }, } @@ -845,7 +818,6 @@ func TestOpenAPI_constructOperationID(t *testing.T) { test.pathAttributes, test.operation, test.operationAttributes, - test.createSupported, test.defaultPrefix, ) if actual != test.expected { diff --git a/sdk/framework/testdata/legacy.json b/sdk/framework/testdata/legacy.json index 7c15e6df28a3..748a1d7ba8b0 100644 --- a/sdk/framework/testdata/legacy.json +++ b/sdk/framework/testdata/legacy.json @@ -36,7 +36,7 @@ } }, "post": { - "operationId": "kv-update-lookup-id", + "operationId": "kv-write-lookup-id", "summary": "Synopsis", "tags": [ "secrets" @@ -46,7 +46,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/kv-update-lookup-id-request" + "$ref": "#/components/schemas/kv-write-lookup-id-request" } } } @@ -61,7 +61,7 @@ }, "components": { "schemas": { - "kv-update-lookup-id-request": { + "kv-write-lookup-id-request": { "type": "object", "properties": { "token": { diff --git a/sdk/framework/testdata/operations.json b/sdk/framework/testdata/operations.json index 674e3bea0a3a..df8fed8bddfb 100644 --- a/sdk/framework/testdata/operations.json +++ b/sdk/framework/testdata/operations.json @@ -60,7 +60,7 @@ ] }, "post": { - "operationId": "kv-create-or-update-foo-id", + "operationId": "kv-write-foo-id", "tags": [ "secrets" ], @@ -71,7 +71,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/kv-create-or-update-foo-id-request" + "$ref": "#/components/schemas/kv-write-foo-id-request" } } } @@ -86,7 +86,7 @@ }, "components": { "schemas": { - "kv-create-or-update-foo-id-request": { + "kv-write-foo-id-request": { "type": "object", "required": [ "age" From 0748a1a00f32b9537869bd2dc255f98bb6b340a0 Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Tue, 4 Apr 2023 12:01:57 -0400 Subject: [PATCH 33/33] title case response/request names --- sdk/framework/openapi.go | 18 ++++++++++++++++-- sdk/framework/openapi_test.go | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/sdk/framework/openapi.go b/sdk/framework/openapi.go index 2e8cafe3afa5..3c99fe508e78 100644 --- a/sdk/framework/openapi.go +++ b/sdk/framework/openapi.go @@ -14,6 +14,8 @@ import ( "github.com/hashicorp/vault/sdk/helper/wrapping" "github.com/hashicorp/vault/sdk/logical" "github.com/mitchellh/mapstructure" + "golang.org/x/text/cases" + "golang.org/x/text/language" ) // OpenAPI specification (OAS): https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md @@ -389,7 +391,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 := operationID + "-request" + requestName := hyphenatedToTitleCase(operationID) + "Request" doc.Components.Schemas[requestName] = s op.RequestBody = &OASRequestBody{ Required: true, @@ -496,7 +498,7 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st } if len(resp.Fields) != 0 { - responseName := operationID + "-response" + responseName := hyphenatedToTitleCase(operationID) + "Response" doc.Components.Schemas[responseName] = responseSchema content = OASContent{ "application/json": &OASMediaTypeObject{ @@ -1004,6 +1006,18 @@ func withoutOperationHints(in *DisplayAttributes) *DisplayAttributes { return © } +func hyphenatedToTitleCase(in string) string { + var b strings.Builder + + title := cases.Title(language.English, cases.NoLower) + + for _, word := range strings.Split(in, "-") { + b.WriteString(title.String(word)) + } + + return b.String() +} + // cleanedResponse is identical to logical.Response but with nulls // removed from from JSON encoding type cleanedResponse struct { diff --git a/sdk/framework/openapi_test.go b/sdk/framework/openapi_test.go index cd0105cdeee7..9e2763f1241e 100644 --- a/sdk/framework/openapi_test.go +++ b/sdk/framework/openapi_test.go @@ -827,6 +827,41 @@ func TestOpenAPI_constructOperationID(t *testing.T) { } } +func TestOpenAPI_hyphenatedToTitleCase(t *testing.T) { + tests := map[string]struct { + in string + expected string + }{ + "simple": { + in: "test", + expected: "Test", + }, + "two-words": { + in: "two-words", + expected: "TwoWords", + }, + "three-words": { + in: "one-two-three", + expected: "OneTwoThree", + }, + "not-hyphenated": { + in: "something_like_this", + expected: "Something_like_this", + }, + } + + for name, test := range tests { + name, test := name, test + t.Run(name, func(t *testing.T) { + t.Parallel() + actual := hyphenatedToTitleCase(test.in) + if actual != test.expected { + t.Fatalf("expected: %s; got: %s", test.expected, actual) + } + }) + } +} + func testPath(t *testing.T, path *Path, sp *logical.Paths, expectedJSON string) { t.Helper()