Fix response structure validation tests for non-2xx responses (#19726)

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,
+		HandlerFunc:             vaulthttp.Handler,
-		// XXX: Schema response validation does not take into account
+		RequestResponseCallback: schema.ResponseValidatingCallback(t),
-		// response codes, preventing non-200 responses from working
-		// properly.
-		//
-		// RequestResponseCallback: schema.ResponseValidatingCallback(t),
 	})
 	cluster.Start()
 	defer cluster.Cleanup()

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
+// ValidateResponse is a test helper that validates whether the given response
-// response data map conforms to the response schema (schema.Fields). It cycles
+// object conforms to the response schema (schema.Fields). It cycles through
-// through the data map and validates conversions in the schema. In "strict"
+// the data map and validates conversions in the schema. In "strict" mode, this
-// mode, this function will also ensure that the data map has all schema's
+// function will also ensure that the data map has all schema-required fields
-// requred fields and does not have any fields outside of the schema.
+// 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
+// ValidateResponseData is a test helper that validates whether the given
-// object conforms to the response schema (schema.Fields). It cycles through
+// response data map conforms to the response schema (schema.Fields). It cycles
-// the data map and validates conversions in the schema. In "strict" mode, this
+// through the data map and validates conversions in the schema. In "strict"
-// function will also ensure that the data map has all schema-required fields
+// mode, this function will also ensure that the data map has all schema's
-// and does not have any fields outside of the schema.
+// 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
+// ResponseValidatingCallback can be used in setting up a [vault.TestCluster]
-// openapi specifications
+// 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
+		// The full request path includes the backend but when passing to the
-		// but when passing to the backend, we have to trim the mount point
+		// backend, we have to trim the mount point:
-		// `sys/mounts/secret` -> `mounts/secret`
+		//   `sys/mounts/secret` -> `mounts/secret`
-		// `auth/token/create` -> `create`
+		//   `auth/token/create` -> `create`
 		requestPath := strings.TrimPrefix(req.Path, req.MountPoint)
 
 		route := backend.Route(requestPath)

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{