cloudfoundry / korifi

Cloud Foundry on Kubernetes
Apache License 2.0
311 stars 61 forks source link

[Bug]: API validation errors are misleading #1947

Open gcapizzi opened 1 year ago

gcapizzi commented 1 year ago

What happened?

The validation errors returned by the Korifi API are not very good. In particular:

Here's an example of an invalid app creation call:

cf curl "/v3/apps" -X POST -d '{ "name": "my_app" }'

and here's the error message returned:

{
  "errors": [
    {
      "detail": "Data is a required field",
      "title": "CF-UnprocessableEntity",
      "code": 10008
    }
  ]
}

The error is referring to the relationships.space.data field.

What you expected to happen

An acceptable error message would be:

{
  "errors": [
    {
      "detail": "`relationships.space.data` is required",
      "title": "CF-UnprocessableEntity",
      "code": 10008
    }
  ]
}

Acceptance Criteria

GIVEN I have made an invalid request to the Korifi API WHEN I read the error message THEN I can immediately tell what's wrong with my request, so that I can fix it

How to reproduce it (as minimally and precisely as possible)

See example above.

Anything else we need to know?

This might not be fixable while still using go-playground/validator, as it's a library designed to validate Go structs and knows nothing about JSON. An alternative could be using JSON Schema or simply implement our validations manually.

gcapizzi commented 1 year ago

I have tested santhosh-tekuri/jsonschema, a JSON Schema library, as a possible alternative.

Given the following schema for an app:

{
  "title": "AppCreate",
  "type": "object",
  "required": ["name", "relationships"],
  "properties": {
    "name": {
      "type": "string",
      "description": "the app name"
    },
    "relationships": {
      "type": "object",
      "required": ["space"],
      "properties": {
        "space": {
          "required": ["data"],
          "properties": {
            "data": {
              "type": "object",
              "required": ["guid"],
              "properties": {
                "guid": {
                  "type": "string",
                  "description": "the space guid"
                }
              }
            }
          }
        }
      }
    }
  }
}

and the following invalid app payload:

{
  "name": "foo",
  "relationships": {
    "bar": {}
  }
}

the library offers the following error formats:

gcapizzi commented 1 year ago

I have also spiked a possible declarative validation library at eirini-forks/validus.

Given the following validation:

appValidation := validation.AllOf(
    validation.JSONField("Name", validation.Required()),
    validation.JSONField("Relationships",
        validation.JSONField("Space",
            validation.JSONField("Data",
                validation.JSONField("GUID", validation.Required()),
            ),
        ),
    ),
)

and the following app creation payload:

app := AppCreate {
    Name:          "the-app-guid",
    Relationships: AppRelationships{},
}

it returns the following error:

Field 'relationships' is invalid: Field 'space' is invalid: Field 'data' is invalid: value is nil
gcapizzi commented 1 year ago

Other interesting candidate: https://github.com/gookit/validate