From 9696600e59ccd5e295ee4525a5e50d8dccc9f3d2 Mon Sep 17 00:00:00 2001 From: Anton Averchenkov <84287187+averche@users.noreply.github.com> Date: Fri, 13 Jan 2023 14:55:56 -0500 Subject: [PATCH] Add response schema validation methods & test helpers (#18635) This pull request adds 3 functions (and corresponding tests): `testhelpers/response_validation.go`: - `ValidateResponse` - `ValidateResponseData` field_data.go: - `ValidateStrict` (has the "strict" validation logic) The functions are primarily meant to be used in tests to ensure that the responses are consistent with the defined response schema. An example of how the functions can be used in tests can be found in #18636. ### Background This PR is part of the ongoing work to add structured responses in Vault OpenAPI (VLT-234) --- changelog/18635.txt | 3 + sdk/framework/field_data.go | 36 ++- sdk/framework/field_data_test.go | 112 +++++++- .../testhelpers/schema/response_validation.go | 78 +++++ .../schema/response_validation_test.go | 272 ++++++++++++++++++ 5 files changed, 499 insertions(+), 2 deletions(-) create mode 100644 changelog/18635.txt create mode 100644 sdk/helper/testhelpers/schema/response_validation.go create mode 100644 sdk/helper/testhelpers/schema/response_validation_test.go diff --git a/changelog/18635.txt b/changelog/18635.txt new file mode 100644 index 000000000..43f3fdf67 --- /dev/null +++ b/changelog/18635.txt @@ -0,0 +1,3 @@ +```release-note:improvement +sdk: Add response schema validation method framework/FieldData.ValidateStrict and two test helpers (ValidateResponse, ValidateResponseData) +``` diff --git a/sdk/framework/field_data.go b/sdk/framework/field_data.go index 99e3fb7ab..d9e6fa365 100644 --- a/sdk/framework/field_data.go +++ b/sdk/framework/field_data.go @@ -25,7 +25,7 @@ type FieldData struct { Schema map[string]*FieldSchema } -// Validate cycles through raw data and validate conversions in +// Validate cycles through raw data and validates conversions in // the schema, so we don't get an error/panic later when // trying to get data out. Data not in the schema is not // an error at this point, so we don't worry about it. @@ -53,6 +53,40 @@ func (d *FieldData) Validate() error { return nil } +// ValidateStrict cycles through raw data and validates conversions in the +// schema. In addition to the checks done by Validate, this function ensures +// that the raw data has all of the schema's required fields and does not +// have any fields outside of the schema. It will return a non-nil error if: +// +// 1. a conversion (parsing of the field's value) fails +// 2. a raw field does not exist in the schema (unless the schema is nil) +// 3. a required schema field is missing from the raw data +// +// This function is currently used for validating response schemas in tests. +func (d *FieldData) ValidateStrict() error { + // the schema is nil, nothing to validate + if d.Schema == nil { + return nil + } + + for field := range d.Raw { + if _, _, err := d.GetOkErr(field); err != nil { + return fmt.Errorf("field %q: %w", field, err) + } + } + + for field, schema := range d.Schema { + if !schema.Required { + continue + } + if _, ok := d.Raw[field]; !ok { + return fmt.Errorf("missing required field %q", field) + } + } + + return nil +} + // Get gets the value for the given field. If the key is an invalid field, // FieldData will panic. If you want a safer version of this method, use // GetOk. If the field k is not set, the default value (if set) will be diff --git a/sdk/framework/field_data_test.go b/sdk/framework/field_data_test.go index d7cbd9761..b152e84fa 100644 --- a/sdk/framework/field_data_test.go +++ b/sdk/framework/field_data_test.go @@ -1157,8 +1157,118 @@ func TestFieldDataGetFirst(t *testing.T) { t.Fatal("should have gotten buzz for fizz") } - result, ok = data.GetFirst("cats") + _, ok = data.GetFirst("cats") if ok { t.Fatal("shouldn't have gotten anything for cats") } } + +func TestValidateStrict(t *testing.T) { + cases := map[string]struct { + Schema map[string]*FieldSchema + Raw map[string]interface{} + ExpectError bool + }{ + "string type, string value": { + map[string]*FieldSchema{ + "foo": {Type: TypeString}, + }, + map[string]interface{}{ + "foo": "bar", + }, + false, + }, + + "string type, int value": { + map[string]*FieldSchema{ + "foo": {Type: TypeString}, + }, + map[string]interface{}{ + "foo": 42, + }, + false, + }, + + "string type, unset value": { + map[string]*FieldSchema{ + "foo": {Type: TypeString}, + }, + map[string]interface{}{}, + false, + }, + + "string type, unset required value": { + map[string]*FieldSchema{ + "foo": { + Type: TypeString, + Required: true, + }, + }, + map[string]interface{}{}, + true, + }, + + "value not in schema": { + map[string]*FieldSchema{ + "foo": { + Type: TypeString, + Required: true, + }, + }, + map[string]interface{}{ + "foo": 42, + "bar": 43, + }, + true, + }, + + "value not in schema, empty schema": { + map[string]*FieldSchema{}, + map[string]interface{}{ + "foo": 42, + "bar": 43, + }, + true, + }, + + "value not in schema, nil schema": { + nil, + map[string]interface{}{ + "foo": 42, + "bar": 43, + }, + false, + }, + + "type time, invalid value": { + map[string]*FieldSchema{ + "foo": {Type: TypeTime}, + }, + map[string]interface{}{ + "foo": "2021-13-11T09:08:07+02:00", + }, + true, + }, + } + + for name, tc := range cases { + name, tc := name, tc + t.Run(name, func(t *testing.T) { + t.Parallel() + + data := &FieldData{ + Raw: tc.Raw, + Schema: tc.Schema, + } + + err := data.ValidateStrict() + + if err == nil && tc.ExpectError == true { + t.Fatalf("expected an error, got nil") + } + if err != nil && tc.ExpectError == false { + t.Fatalf("unexpected error: %v", err) + } + }) + } +} diff --git a/sdk/helper/testhelpers/schema/response_validation.go b/sdk/helper/testhelpers/schema/response_validation.go new file mode 100644 index 000000000..9597e8481 --- /dev/null +++ b/sdk/helper/testhelpers/schema/response_validation.go @@ -0,0 +1,78 @@ +package schema + +import ( + "encoding/json" + "fmt" + "testing" + + "github.com/hashicorp/vault/sdk/framework" + "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. +func ValidateResponse(t *testing.T, schema *framework.Response, response *logical.Response, strict bool) { + t.Helper() + + if response != nil { + ValidateResponseData(t, schema, response.Data, strict) + } else { + ValidateResponseData(t, schema, nil, strict) + } +} + +// 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 ValidateResponseData(t *testing.T, schema *framework.Response, data map[string]interface{}, strict bool) { + t.Helper() + + if err := validateResponseDataImpl( + schema, + data, + strict, + ); err != nil { + t.Fatalf("validation error: %v; response data: %#v", err, data) + } +} + +// validateResponseDataImpl is extracted so that it can be tested +func validateResponseDataImpl(schema *framework.Response, data map[string]interface{}, strict bool) error { + // nothing to validate + if schema == nil { + 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. + jsonBytes, err := json.Marshal(data) + if err != nil { + return fmt.Errorf("failed to convert input to json: %w", err) + } + + var dataWithStringValues map[string]interface{} + if err := json.Unmarshal( + jsonBytes, + &dataWithStringValues, + ); err != nil { + return fmt.Errorf("failed to unmashal data: %w", err) + } + + // Validate + fd := framework.FieldData{ + Raw: dataWithStringValues, + Schema: schema.Fields, + } + + if strict { + return fd.ValidateStrict() + } + + return fd.Validate() +} diff --git a/sdk/helper/testhelpers/schema/response_validation_test.go b/sdk/helper/testhelpers/schema/response_validation_test.go new file mode 100644 index 000000000..976389c44 --- /dev/null +++ b/sdk/helper/testhelpers/schema/response_validation_test.go @@ -0,0 +1,272 @@ +package schema + +import ( + "testing" + "time" + + "github.com/hashicorp/vault/sdk/framework" +) + +func TestValidateResponse(t *testing.T) { + cases := map[string]struct { + schema *framework.Response + response map[string]interface{} + strict bool + errorExpected bool + }{ + "nil schema, nil response, strict": { + schema: nil, + response: nil, + strict: true, + errorExpected: false, + }, + + "nil schema, nil response, not strict": { + schema: nil, + response: nil, + strict: false, + errorExpected: false, + }, + + "nil schema, good response, strict": { + schema: nil, + response: map[string]interface{}{ + "foo": "bar", + }, + strict: true, + errorExpected: false, + }, + + "nil schema, good response, not strict": { + schema: nil, + response: map[string]interface{}{ + "foo": "bar", + }, + strict: true, + errorExpected: false, + }, + + "nil schema fields, good response, strict": { + schema: &framework.Response{}, + response: map[string]interface{}{ + "foo": "bar", + }, + strict: true, + errorExpected: false, + }, + + "nil schema fields, good response, not strict": { + schema: &framework.Response{}, + response: map[string]interface{}{ + "foo": "bar", + }, + strict: true, + errorExpected: false, + }, + + "string schema field, string response, strict": { + schema: &framework.Response{ + Fields: map[string]*framework.FieldSchema{ + "foo": { + Type: framework.TypeString, + }, + }, + }, + response: map[string]interface{}{ + "foo": "bar", + }, + strict: true, + errorExpected: false, + }, + + "string schema field, string response, not strict": { + schema: &framework.Response{ + Fields: map[string]*framework.FieldSchema{ + "foo": { + Type: framework.TypeString, + }, + }, + }, + response: map[string]interface{}{ + "foo": "bar", + }, + strict: false, + errorExpected: false, + }, + + "string schema not required field, empty response, strict": { + schema: &framework.Response{ + Fields: map[string]*framework.FieldSchema{ + "foo": { + Type: framework.TypeString, + Required: false, + }, + }, + }, + response: map[string]interface{}{}, + strict: true, + errorExpected: false, + }, + + "string schema required field, empty response, strict": { + schema: &framework.Response{ + Fields: map[string]*framework.FieldSchema{ + "foo": { + Type: framework.TypeString, + Required: true, + }, + }, + }, + response: map[string]interface{}{}, + strict: true, + errorExpected: true, + }, + + "string schema required field, empty response, not strict": { + schema: &framework.Response{ + Fields: map[string]*framework.FieldSchema{ + "foo": { + Type: framework.TypeString, + Required: true, + }, + }, + }, + response: map[string]interface{}{}, + strict: false, + errorExpected: false, + }, + + "string schema required field, nil response, strict": { + schema: &framework.Response{ + Fields: map[string]*framework.FieldSchema{ + "foo": { + Type: framework.TypeString, + Required: true, + }, + }, + }, + response: nil, + strict: true, + errorExpected: true, + }, + + "string schema required field, nil response, not strict": { + schema: &framework.Response{ + Fields: map[string]*framework.FieldSchema{ + "foo": { + Type: framework.TypeString, + Required: true, + }, + }, + }, + response: nil, + strict: false, + errorExpected: false, + }, + + "empty schema, string response, strict": { + schema: &framework.Response{ + Fields: map[string]*framework.FieldSchema{}, + }, + response: map[string]interface{}{ + "foo": "bar", + }, + strict: true, + errorExpected: true, + }, + + "empty schema, string response, not strict": { + schema: &framework.Response{ + Fields: map[string]*framework.FieldSchema{}, + }, + response: map[string]interface{}{ + "foo": "bar", + }, + strict: false, + errorExpected: false, + }, + + "time schema, string response, strict": { + schema: &framework.Response{ + Fields: map[string]*framework.FieldSchema{ + "time": { + Type: framework.TypeTime, + Required: true, + }, + }, + }, + response: map[string]interface{}{ + "time": "2024-12-11T09:08:07Z", + }, + strict: true, + errorExpected: false, + }, + + "time schema, string response, not strict": { + schema: &framework.Response{ + Fields: map[string]*framework.FieldSchema{ + "time": { + Type: framework.TypeTime, + Required: true, + }, + }, + }, + response: map[string]interface{}{ + "time": "2024-12-11T09:08:07Z", + }, + strict: false, + errorExpected: false, + }, + + "time schema, time response, strict": { + schema: &framework.Response{ + Fields: map[string]*framework.FieldSchema{ + "time": { + Type: framework.TypeTime, + Required: true, + }, + }, + }, + response: map[string]interface{}{ + "time": time.Date(2024, 12, 11, 9, 8, 7, 0, time.UTC), + }, + strict: true, + errorExpected: false, + }, + + "time schema, time response, not strict": { + schema: &framework.Response{ + Fields: map[string]*framework.FieldSchema{ + "time": { + Type: framework.TypeTime, + Required: true, + }, + }, + }, + response: map[string]interface{}{ + "time": time.Date(2024, 12, 11, 9, 8, 7, 0, time.UTC), + }, + strict: false, + errorExpected: false, + }, + } + + for name, tc := range cases { + name, tc := name, tc + t.Run(name, func(t *testing.T) { + t.Parallel() + + err := validateResponseDataImpl( + tc.schema, + tc.response, + tc.strict, + ) + if err == nil && tc.errorExpected == true { + t.Fatalf("expected an error, got nil") + } + if err != nil && tc.errorExpected == false { + t.Fatalf("unexpected error: %v", err) + } + }) + } +}