Revert "Add mount path into the default generated openapi.json spec (#17839)" (#17890)

This reverts commit 02064eccb4.
This commit is contained in:
Anton Averchenkov
2022-11-10 18:39:53 -05:00
committed by GitHub
parent 467067371d
commit 20f66ef7dd
13 changed files with 142 additions and 126 deletions

View File

@@ -37,9 +37,9 @@ const (
// path matches that path or not (useful specifically for the paths that // path matches that path or not (useful specifically for the paths that
// contain templated fields.) // contain templated fields.)
var sudoPaths = map[string]*regexp.Regexp{ var sudoPaths = map[string]*regexp.Regexp{
"/auth/{mount_path}/accessors/": regexp.MustCompile(`^/auth/.+/accessors/$`), "/auth/token/accessors/": regexp.MustCompile(`^/auth/token/accessors/$`),
"/{mount_path}/root": regexp.MustCompile(`^/.+/root$`), "/pki/root": regexp.MustCompile(`^/pki/root$`),
"/{mount_path}/root/sign-self-issued": regexp.MustCompile(`^/.+/root/sign-self-issued$`), "/pki/root/sign-self-issued": regexp.MustCompile(`^/pki/root/sign-self-issued$`),
"/sys/audit": regexp.MustCompile(`^/sys/audit$`), "/sys/audit": regexp.MustCompile(`^/sys/audit$`),
"/sys/audit/{path}": regexp.MustCompile(`^/sys/audit/.+$`), "/sys/audit/{path}": regexp.MustCompile(`^/sys/audit/.+$`),
"/sys/auth/{path}": regexp.MustCompile(`^/sys/auth/.+$`), "/sys/auth/{path}": regexp.MustCompile(`^/sys/auth/.+$`),

View File

@@ -1,3 +0,0 @@
```release-note:improvement
openapi: Add {mount_path} parameter to secret & auth paths in the generated openapi.json spec.
```

View File

@@ -539,9 +539,16 @@ func (b *Backend) handleRootHelp(req *logical.Request) (*logical.Response, error
// names in the OAS document. // names in the OAS document.
requestResponsePrefix := req.GetString("requestResponsePrefix") requestResponsePrefix := req.GetString("requestResponsePrefix")
// Generic mount paths will primarily be used for code generation purposes.
// This will result in dynamic mount paths being placed instead of
// hardcoded default paths. For example /auth/approle/login would be replaced
// with /auth/{mountPath}/login. This will be replaced for all secrets
// engines and auth methods that are enabled.
genericMountPaths, _ := req.Get("genericMountPaths").(bool)
// Build OpenAPI response for the entire backend // Build OpenAPI response for the entire backend
doc := NewOASDocument() doc := NewOASDocument()
if err := documentPaths(b, requestResponsePrefix, doc); err != nil { if err := documentPaths(b, requestResponsePrefix, genericMountPaths, doc); err != nil {
b.Logger().Warn("error generating OpenAPI", "error", err) b.Logger().Warn("error generating OpenAPI", "error", err)
} }

View File

@@ -13,8 +13,6 @@ import (
"github.com/hashicorp/vault/sdk/logical" "github.com/hashicorp/vault/sdk/logical"
"github.com/hashicorp/vault/sdk/version" "github.com/hashicorp/vault/sdk/version"
"github.com/mitchellh/mapstructure" "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 // OpenAPI specification (OAS): https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md
@@ -208,16 +206,16 @@ var (
altRootsRe = regexp.MustCompile(`^\(([\w\-_]+(?:\|[\w\-_]+)+)\)(/.*)$`) // Pattern starting with alts, e.g. "(root1|root2)/(?P<name>regex)" altRootsRe = regexp.MustCompile(`^\(([\w\-_]+(?:\|[\w\-_]+)+)\)(/.*)$`) // Pattern starting with alts, e.g. "(root1|root2)/(?P<name>regex)"
cleanCharsRe = regexp.MustCompile("[()^$?]") // Set of regex characters that will be stripped during cleaning cleanCharsRe = regexp.MustCompile("[()^$?]") // Set of regex characters that will be stripped during cleaning
cleanSuffixRe = regexp.MustCompile(`/\?\$?$`) // Path suffix patterns that will be stripped during cleaning cleanSuffixRe = regexp.MustCompile(`/\?\$?$`) // Path suffix patterns that will be stripped during cleaning
nonWordRe = regexp.MustCompile(`[^a-zA-Z0-9]+`) // Match a sequence of non-word characters nonWordRe = regexp.MustCompile(`[^\w]+`) // Match a sequence of non-word characters
pathFieldsRe = regexp.MustCompile(`{(\w+)}`) // Capture OpenAPI-style named parameters, e.g. "lookup/{urltoken}", pathFieldsRe = regexp.MustCompile(`{(\w+)}`) // Capture OpenAPI-style named parameters, e.g. "lookup/{urltoken}",
reqdRe = regexp.MustCompile(`\(?\?P<(\w+)>[^)]*\)?`) // Capture required parameters, e.g. "(?P<name>regex)" reqdRe = regexp.MustCompile(`\(?\?P<(\w+)>[^)]*\)?`) // Capture required parameters, e.g. "(?P<name>regex)"
wsRe = regexp.MustCompile(`\s+`) // Match whitespace, to be compressed during cleaning wsRe = regexp.MustCompile(`\s+`) // Match whitespace, to be compressed during cleaning
) )
// documentPaths parses all paths in a framework.Backend into OpenAPI paths. // documentPaths parses all paths in a framework.Backend into OpenAPI paths.
func documentPaths(backend *Backend, requestResponsePrefix string, doc *OASDocument) error { func documentPaths(backend *Backend, requestResponsePrefix string, genericMountPaths bool, doc *OASDocument) error {
for _, p := range backend.Paths { for _, p := range backend.Paths {
if err := documentPath(p, backend.SpecialPaths(), requestResponsePrefix, backend.BackendType, doc); err != nil { if err := documentPath(p, backend.SpecialPaths(), requestResponsePrefix, genericMountPaths, backend.BackendType, doc); err != nil {
return err return err
} }
} }
@@ -226,7 +224,7 @@ func documentPaths(backend *Backend, requestResponsePrefix string, doc *OASDocum
} }
// documentPath parses a framework.Path into one or more OpenAPI paths. // documentPath parses a framework.Path into one or more OpenAPI paths.
func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix string, backendType logical.BackendType, doc *OASDocument) error { func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix string, genericMountPaths bool, backendType logical.BackendType, doc *OASDocument) error {
var sudoPaths []string var sudoPaths []string
var unauthPaths []string var unauthPaths []string
@@ -235,11 +233,6 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st
unauthPaths = specialPaths.Unauthenticated unauthPaths = specialPaths.Unauthenticated
} }
defaultMountPath := requestResponsePrefix
if requestResponsePrefix == "kv" {
defaultMountPath = "secret"
}
// Convert optional parameters into distinct patterns to be processed independently. // Convert optional parameters into distinct patterns to be processed independently.
paths := expandPattern(p.Pattern) paths := expandPattern(p.Pattern)
@@ -270,17 +263,16 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st
// Body fields will be added to individual operations. // Body fields will be added to individual operations.
pathFields, bodyFields := splitFields(p.Fields, path) pathFields, bodyFields := splitFields(p.Fields, path)
if genericMountPaths && requestResponsePrefix != "system" && requestResponsePrefix != "identity" {
// Add mount path as a parameter // Add mount path as a parameter
if defaultMountPath != "system" && defaultMountPath != "identity" {
p := OASParameter{ p := OASParameter{
Name: "mount_path", Name: "mountPath",
Description: "Path where the backend was mounted; the endpoint path will be offset by the mount path", Description: "Path that the backend was mounted at",
In: "path", In: "path",
Schema: &OASSchema{ Schema: &OASSchema{
Type: "string", Type: "string",
Default: defaultMountPath,
}, },
Required: false, Required: true,
} }
pi.Parameters = append(pi.Parameters, p) pi.Parameters = append(pi.Parameters, p)
@@ -349,7 +341,6 @@ func documentPath(p *Path, specialPaths *logical.Paths, requestResponsePrefix st
op.Summary = props.Summary op.Summary = props.Summary
op.Description = props.Description op.Description = props.Description
op.Deprecated = props.Deprecated op.Deprecated = props.Deprecated
op.OperationID = constructOperationID(string(opType), path, defaultMountPath)
// Add any fields not present in the path as body parameters for POST. // Add any fields not present in the path as body parameters for POST.
if opType == logical.CreateOperation || opType == logical.UpdateOperation { if opType == logical.CreateOperation || opType == logical.UpdateOperation {
@@ -524,19 +515,6 @@ func constructRequestName(requestResponsePrefix string, path string) string {
return b.String() return b.String()
} }
func constructOperationID(method, path, defaultMountPath string) string {
// title caser
title := cases.Title(language.English)
// Space-split on non-words, title case everything, recombine
id := nonWordRe.ReplaceAllLiteralString(strings.ToLower(path), " ")
id = fmt.Sprintf("%s %s", defaultMountPath, id)
id = title.String(id)
id = strings.ReplaceAll(id, " ", "")
return method + id
}
func specialPathMatch(path string, specialPaths []string) bool { func specialPathMatch(path string, specialPaths []string) bool {
// Test for exact or prefix match of special paths. // Test for exact or prefix match of special paths.
for _, sp := range specialPaths { for _, sp := range specialPaths {
@@ -749,3 +727,61 @@ func cleanResponse(resp *logical.Response) *cleanedResponse {
Headers: resp.Headers, Headers: resp.Headers,
} }
} }
// CreateOperationIDs generates unique operationIds for all paths/methods.
// The transform will convert path/method into camelcase. e.g.:
//
// /sys/tools/random/{urlbytes} -> postSysToolsRandomUrlbytes
//
// In the unlikely case of a duplicate ids, a numeric suffix is added:
//
// postSysToolsRandomUrlbytes_2
//
// An optional user-provided suffix ("context") may also be appended.
func (d *OASDocument) CreateOperationIDs(context string) {
opIDCount := make(map[string]int)
var paths []string
// traverse paths in a stable order to ensure stable output
for path := range d.Paths {
paths = append(paths, path)
}
sort.Strings(paths)
for _, path := range paths {
pi := d.Paths[path]
for _, method := range []string{"get", "post", "delete"} {
var oasOperation *OASOperation
switch method {
case "get":
oasOperation = pi.Get
case "post":
oasOperation = pi.Post
case "delete":
oasOperation = pi.Delete
}
if oasOperation == nil {
continue
}
// Space-split on non-words, title case everything, recombine
opID := nonWordRe.ReplaceAllString(strings.ToLower(path), " ")
opID = strings.Title(opID)
opID = method + strings.ReplaceAll(opID, " ", "")
// deduplicate operationIds. This is a safeguard, since generated IDs should
// already be unique given our current path naming conventions.
opIDCount[opID]++
if opIDCount[opID] > 1 {
opID = fmt.Sprintf("%s_%d", opID, opIDCount[opID])
}
if context != "" {
opID += "_" + context
}
oasOperation.OperationID = opID
}
}
}

View File

@@ -271,7 +271,7 @@ func TestOpenAPI_SpecialPaths(t *testing.T) {
Root: test.rootPaths, Root: test.rootPaths,
Unauthenticated: test.unauthPaths, Unauthenticated: test.unauthPaths,
} }
err := documentPath(&path, sp, "kv", logical.TypeLogical, doc) err := documentPath(&path, sp, "kv", false, logical.TypeLogical, doc)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@@ -517,35 +517,42 @@ func TestOpenAPI_OperationID(t *testing.T) {
}, },
} }
for _, context := range []string{"", "bar"} {
doc := NewOASDocument() doc := NewOASDocument()
err := documentPath(path1, nil, "kv", logical.TypeLogical, doc) err := documentPath(path1, nil, "kv", false, logical.TypeLogical, doc)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
err = documentPath(path2, nil, "kv", logical.TypeLogical, doc) err = documentPath(path2, nil, "kv", false, logical.TypeLogical, doc)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
doc.CreateOperationIDs(context)
tests := []struct { tests := []struct {
path string path string
op string op string
opID string opID string
}{ }{
{"/Foo/{id}", "get", "readSecretFooId"}, {"/Foo/{id}", "get", "getFooId"},
{"/foo/{id}", "post", "updateSecretFooId"}, {"/foo/{id}", "get", "getFooId_2"},
{"/foo/{id}", "delete", "deleteSecretFooId"}, {"/foo/{id}", "post", "postFooId"},
{"/foo/{id}", "delete", "deleteFooId"},
} }
for _, test := range tests { for _, test := range tests {
actual := getPathOp(doc.Paths[test.path], test.op).OperationID actual := getPathOp(doc.Paths[test.path], test.op).OperationID
expected := test.opID expected := test.opID
if context != "" {
expected += "_" + context
}
if actual != expected { if actual != expected {
t.Fatalf("expected %v, got %v", expected, actual) t.Fatalf("expected %v, got %v", expected, actual)
} }
} }
} }
}
func TestOpenAPI_CustomDecoder(t *testing.T) { func TestOpenAPI_CustomDecoder(t *testing.T) {
p := &Path{ p := &Path{
@@ -576,7 +583,7 @@ func TestOpenAPI_CustomDecoder(t *testing.T) {
} }
docOrig := NewOASDocument() docOrig := NewOASDocument()
err := documentPath(p, nil, "kv", logical.TypeLogical, docOrig) err := documentPath(p, nil, "kv", false, logical.TypeLogical, docOrig)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@@ -639,9 +646,10 @@ func testPath(t *testing.T, path *Path, sp *logical.Paths, expectedJSON string)
t.Helper() t.Helper()
doc := NewOASDocument() doc := NewOASDocument()
if err := documentPath(path, sp, "kv", logical.TypeLogical, doc); err != nil { if err := documentPath(path, sp, "kv", false, logical.TypeLogical, doc); err != nil {
t.Fatal(err) t.Fatal(err)
} }
doc.CreateOperationIDs("")
docJSON, err := json.MarshalIndent(doc, "", " ") docJSON, err := json.MarshalIndent(doc, "", " ")
if err != nil { if err != nil {

View File

@@ -317,7 +317,7 @@ func (p *Path) helpCallback(b *Backend) OperationFunc {
// Build OpenAPI response for this path // Build OpenAPI response for this path
doc := NewOASDocument() doc := NewOASDocument()
if err := documentPath(p, b.SpecialPaths(), requestResponsePrefix, b.BackendType, doc); err != nil { if err := documentPath(p, b.SpecialPaths(), requestResponsePrefix, false, b.BackendType, doc); err != nil {
b.Logger().Warn("error generating OpenAPI", "error", err) b.Logger().Warn("error generating OpenAPI", "error", err)
} }

View File

@@ -21,23 +21,12 @@
"type": "string" "type": "string"
}, },
"required": true "required": true
},
{
"name": "mount_path",
"description": "Path where the backend was mounted; the endpoint path will be offset by the mount path",
"in": "path",
"schema": {
"type": "string",
"default": "secret"
}
} }
], ],
"get": { "get": {
"operationId": "getLookupId",
"summary": "Synopsis", "summary": "Synopsis",
"operationId": "readSecretLookupId", "tags": ["secrets"],
"tags": [
"secrets"
],
"responses": { "responses": {
"200": { "200": {
"description": "OK" "description": "OK"
@@ -45,11 +34,9 @@
} }
}, },
"post": { "post": {
"operationId": "postLookupId",
"summary": "Synopsis", "summary": "Synopsis",
"operationId": "updateSecretLookupId", "tags": ["secrets"],
"tags": [
"secrets"
],
"requestBody": { "requestBody": {
"content": { "content": {
"application/json": { "application/json": {

View File

@@ -34,19 +34,10 @@
"type": "string" "type": "string"
}, },
"required": true "required": true
},
{
"name": "mount_path",
"description": "Path where the backend was mounted; the endpoint path will be offset by the mount path",
"in": "path",
"schema": {
"type": "string",
"default": "secret"
}
} }
], ],
"get": { "get": {
"operationId": "readSecretFooId", "operationId": "getFooId",
"tags": ["secrets"], "tags": ["secrets"],
"summary": "My Summary", "summary": "My Summary",
"description": "My Description", "description": "My Description",
@@ -67,7 +58,7 @@
] ]
}, },
"post": { "post": {
"operationId": "updateSecretFooId", "operationId": "postFooId",
"tags": ["secrets"], "tags": ["secrets"],
"summary": "Update Summary", "summary": "Update Summary",
"description": "Update Description", "description": "Update Description",

View File

@@ -33,19 +33,10 @@
"type": "string" "type": "string"
}, },
"required": true "required": true
},
{
"name": "mount_path",
"description": "Path where the backend was mounted; the endpoint path will be offset by the mount path",
"in": "path",
"schema": {
"type": "string",
"default": "secret"
}
} }
], ],
"get": { "get": {
"operationId": "listSecretFooId", "operationId": "getFooId",
"tags": ["secrets"], "tags": ["secrets"],
"summary": "List Summary", "summary": "List Summary",
"description": "List Description", "description": "List Description",

View File

@@ -12,20 +12,9 @@
"paths": { "paths": {
"/foo": { "/foo": {
"description": "Synopsis", "description": "Synopsis",
"parameters": [
{
"name": "mount_path",
"description": "Path where the backend was mounted; the endpoint path will be offset by the mount path",
"in": "path",
"schema": {
"type": "string",
"default": "secret"
}
}
],
"x-vault-unauthenticated": true, "x-vault-unauthenticated": true,
"delete": { "delete": {
"operationId": "deleteSecretFoo", "operationId": "deleteFoo",
"tags": ["secrets"], "tags": ["secrets"],
"summary": "Delete stuff", "summary": "Delete stuff",
"responses": { "responses": {
@@ -35,7 +24,7 @@
} }
}, },
"get": { "get": {
"operationId": "readSecretFoo", "operationId": "getFoo",
"tags": ["secrets"], "tags": ["secrets"],
"summary": "My Summary", "summary": "My Summary",
"description": "My Description", "description": "My Description",

View File

@@ -4408,10 +4408,14 @@ func (b *SystemBackend) pathInternalOpenAPI(ctx context.Context, req *logical.Re
return nil, err return nil, err
} }
context := d.Get("context").(string)
// Set up target document and convert to map[string]interface{} which is what will // Set up target document and convert to map[string]interface{} which is what will
// be received from plugin backends. // be received from plugin backends.
doc := framework.NewOASDocument() doc := framework.NewOASDocument()
genericMountPaths, _ := d.Get("generic_mount_paths").(bool)
procMountGroup := func(group, mountPrefix string) error { procMountGroup := func(group, mountPrefix string) error {
for mount, entry := range resp.Data[group].(map[string]interface{}) { for mount, entry := range resp.Data[group].(map[string]interface{}) {
@@ -4429,7 +4433,7 @@ func (b *SystemBackend) pathInternalOpenAPI(ctx context.Context, req *logical.Re
req := &logical.Request{ req := &logical.Request{
Operation: logical.HelpOperation, Operation: logical.HelpOperation,
Storage: req.Storage, Storage: req.Storage,
Data: map[string]interface{}{"requestResponsePrefix": pluginType}, Data: map[string]interface{}{"requestResponsePrefix": pluginType, "genericMountPaths": genericMountPaths},
} }
resp, err := backend.HandleRequest(ctx, req) resp, err := backend.HandleRequest(ctx, req)
@@ -4483,8 +4487,8 @@ func (b *SystemBackend) pathInternalOpenAPI(ctx context.Context, req *logical.Re
} }
} }
if mount != "sys/" && mount != "identity/" { if genericMountPaths && mount != "sys/" && mount != "identity/" {
s := fmt.Sprintf("/%s{mount_path}/%s", mountPrefix, path) s := fmt.Sprintf("/%s{mountPath}/%s", mountPrefix, path)
doc.Paths[s] = obj doc.Paths[s] = obj
} else { } else {
doc.Paths["/"+mountPrefix+mount+path] = obj doc.Paths["/"+mountPrefix+mount+path] = obj
@@ -4506,6 +4510,8 @@ func (b *SystemBackend) pathInternalOpenAPI(ctx context.Context, req *logical.Re
return nil, err return nil, err
} }
doc.CreateOperationIDs(context)
buf, err := json.Marshal(doc) buf, err := json.Marshal(doc)
if err != nil { if err != nil {
return nil, err return nil, err

View File

@@ -3598,10 +3598,10 @@ func TestSystemBackend_OASGenericMount(t *testing.T) {
path string path string
tag string tag string
}{ }{
{"/auth/{mount_path}/lookup", "auth"}, {"/auth/{mountPath}/lookup", "auth"},
{"/{mount_path}/{path}", "secrets"}, {"/{mountPath}/{path}", "secrets"},
{"/identity/group/id", "identity"}, {"/identity/group/id", "identity"},
{"/{mount_path}/.*", "secrets"}, {"/{mountPath}/.*", "secrets"},
{"/sys/policy", "system"}, {"/sys/policy", "system"},
} }
@@ -3683,10 +3683,10 @@ func TestSystemBackend_OpenAPI(t *testing.T) {
path string path string
tag string tag string
}{ }{
{"/auth/{mount_path}/lookup", "auth"}, {"/auth/token/lookup", "auth"},
{"/{mount_path}/{path}", "secrets"}, {"/cubbyhole/{path}", "secrets"},
{"/identity/group/id", "identity"}, {"/identity/group/id", "identity"},
{"/{mount_path}/.*", "secrets"}, // TODO update after kv repo update {"/secret/.*", "secrets"}, // TODO update after kv repo update
{"/sys/policy", "system"}, {"/sys/policy", "system"},
} }

View File

@@ -31,6 +31,10 @@ This endpoint returns a single OpenAPI document describing all paths visible to
| :----- | :---------------------------- | | :----- | :---------------------------- |
| `GET` | `/sys/internal/specs/openapi` | | `GET` | `/sys/internal/specs/openapi` |
### Parameters
- `generic_mount_paths` `(bool: false)` Used to specify whether to use generic mount paths. If set, the mount paths will be replaced with a dynamic parameter: `{mountPath}`
### Sample Request ### Sample Request