From f674f0ea3250f65c775c880a7a33a70fb16e7756 Mon Sep 17 00:00:00 2001 From: Anton Averchenkov <84287187+averche@users.noreply.github.com> Date: Thu, 23 Mar 2023 16:33:44 -0400 Subject: [PATCH] Fix response structure validation tests for non-2xx responses (#19726) --- builtin/logical/pki/backend_test.go | 8 +--- .../testhelpers/schema/response_validation.go | 46 ++++++++++++------- .../schema/response_validation_test.go | 30 ++++++++++++ 3 files changed, 61 insertions(+), 23 deletions(-) diff --git a/builtin/logical/pki/backend_test.go b/builtin/logical/pki/backend_test.go index 17327f00d8ad..dd74cc0f6218 100644 --- a/builtin/logical/pki/backend_test.go +++ b/builtin/logical/pki/backend_test.go @@ -5254,12 +5254,8 @@ func TestBackend_IfModifiedSinceHeaders(t *testing.T) { }, } cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{ - HandlerFunc: vaulthttp.Handler, - // XXX: Schema response validation does not take into account - // response codes, preventing non-200 responses from working - // properly. - // - // RequestResponseCallback: schema.ResponseValidatingCallback(t), + HandlerFunc: vaulthttp.Handler, + RequestResponseCallback: schema.ResponseValidatingCallback(t), }) cluster.Start() defer cluster.Cleanup() diff --git a/sdk/helper/testhelpers/schema/response_validation.go b/sdk/helper/testhelpers/schema/response_validation.go index febb857ecea0..430d1754a56f 100644 --- a/sdk/helper/testhelpers/schema/response_validation.go +++ b/sdk/helper/testhelpers/schema/response_validation.go @@ -14,11 +14,11 @@ import ( "github.com/hashicorp/vault/sdk/logical" ) -// ValidateResponseData is a test helper that validates whether the given -// response data map conforms to the response schema (schema.Fields). It cycles -// through the data map and validates conversions in the schema. In "strict" -// mode, this function will also ensure that the data map has all schema's -// requred fields and does not have any fields outside of the schema. +// ValidateResponse is a test helper that validates whether the given response +// object conforms to the response schema (schema.Fields). It cycles through +// the data map and validates conversions in the schema. In "strict" mode, this +// function will also ensure that the data map has all schema-required fields +// and does not have any fields outside of the schema. func ValidateResponse(t *testing.T, schema *framework.Response, response *logical.Response, strict bool) { t.Helper() @@ -29,11 +29,11 @@ func ValidateResponse(t *testing.T, schema *framework.Response, response *logica } } -// ValidateResponse is a test helper that validates whether the given response -// object conforms to the response schema (schema.Fields). It cycles through -// the data map and validates conversions in the schema. In "strict" mode, this -// function will also ensure that the data map has all schema-required fields -// and does not have any fields outside of the schema. +// ValidateResponseData is a test helper that validates whether the given +// response data map conforms to the response schema (schema.Fields). It cycles +// through the data map and validates conversions in the schema. In "strict" +// mode, this function will also ensure that the data map has all schema's +// requred fields and does not have any fields outside of the schema. func ValidateResponseData(t *testing.T, schema *framework.Response, data map[string]interface{}, strict bool) { t.Helper() @@ -53,6 +53,16 @@ func validateResponseDataImpl(schema *framework.Response, data map[string]interf return nil } + // Certain responses may come through with non-2xx status codes. While + // these are not always errors (e.g. 3xx redirection codes), we don't + // consider them for the purposes of schema validation + if status, exists := data[logical.HTTPStatusCode]; exists { + s, ok := status.(int) + if ok && (s < 200 || s > 299) { + return nil + } + } + // Marshal the data to JSON and back to convert the map's values into // JSON strings expected by Validate() and ValidateStrict(). This is // not efficient and is done for testing purposes only. @@ -100,7 +110,8 @@ func validateResponseDataImpl(schema *framework.Response, data map[string]interf return fd.Validate() } -// FindResponseSchema is a test helper to extract response schema from the given framework path / operation +// FindResponseSchema is a test helper to extract response schema from the +// given framework path / operation. func FindResponseSchema(t *testing.T, paths []*framework.Path, pathIdx int, operation logical.Operation) *framework.Response { t.Helper() @@ -149,8 +160,8 @@ func GetResponseSchema(t *testing.T, path *framework.Path, operation logical.Ope return &schemaResponses[0] } -// ResponseValidatingCallback can be used in setting up a [vault.TestCluster] that validates every response against the -// openapi specifications +// ResponseValidatingCallback can be used in setting up a [vault.TestCluster] +// that validates every response against the openapi specifications. // // [vault.TestCluster]: https://pkg.go.dev/github.com/hashicorp/vault/vault#TestCluster func ResponseValidatingCallback(t *testing.T) func(logical.Backend, *logical.Request, *logical.Response) { @@ -164,15 +175,16 @@ func ResponseValidatingCallback(t *testing.T) func(logical.Backend, *logical.Req if b == nil { t.Fatalf("non-nil backend required") } + backend, ok := b.(PathRouter) if !ok { t.Fatalf("could not cast %T to have `Route(string) *framework.Path`", b) } - // the full request path includes the backend - // but when passing to the backend, we have to trim the mount point - // `sys/mounts/secret` -> `mounts/secret` - // `auth/token/create` -> `create` + // The full request path includes the backend but when passing to the + // backend, we have to trim the mount point: + // `sys/mounts/secret` -> `mounts/secret` + // `auth/token/create` -> `create` requestPath := strings.TrimPrefix(req.Path, req.MountPoint) route := backend.Route(requestPath) diff --git a/sdk/helper/testhelpers/schema/response_validation_test.go b/sdk/helper/testhelpers/schema/response_validation_test.go index 98880d007606..4f4aa8b1cc3c 100644 --- a/sdk/helper/testhelpers/schema/response_validation_test.go +++ b/sdk/helper/testhelpers/schema/response_validation_test.go @@ -275,6 +275,36 @@ func TestValidateResponse(t *testing.T) { errorExpected: false, }, + "string schema field, response has non-200 http_status_code, strict": { + schema: &framework.Response{ + Fields: map[string]*framework.FieldSchema{ + "foo": { + Type: framework.TypeString, + }, + }, + }, + response: map[string]interface{}{ + "http_status_code": 304, + }, + strict: true, + errorExpected: false, + }, + + "string schema field, response has non-200 http_status_code, not strict": { + schema: &framework.Response{ + Fields: map[string]*framework.FieldSchema{ + "foo": { + Type: framework.TypeString, + }, + }, + }, + response: map[string]interface{}{ + "http_status_code": 304, + }, + strict: false, + errorExpected: false, + }, + "schema has http_raw_body, strict": { schema: &framework.Response{ Fields: map[string]*framework.FieldSchema{