Closed migmartri closed 1 month ago
{
"schemaVersion": "v1",
"materials": [
{
"type": "CONTAINER_IMAGE",
"name": "skynet-control-plane",
"output": true,
"annotations": [
{
"name": "component",
"value": "control-plane"
},
{
"name": "asset"
}
]
},
{ "type:": "ARTIFACT", "name": "rootfs" },
]
}
Doing a research on the topic I've found the following:
unknown field "type:"
.type:
on that material the actual type would be of type type: MATERIAL_TYPE_UNSPECIFIED
which is the default value of the enum.protovalidate
has a nice feature where you can skip on the middleware the validation of certain messages doing the following change, which will leave the validation to the developer in a later stage.
diff --git a/app/controlplane/internal/server/grpc.go b/app/controlplane/internal/server/grpc.go
index 5754ff3..ec4a610 100644
--- a/app/controlplane/internal/server/grpc.go
+++ b/app/controlplane/internal/server/grpc.go
@@ -97,7 +97,11 @@ func NewGRPCServer(opts *Opts) (*grpc.Server, error) {
grpc.Middleware(craftMiddleware(opts)...),
grpc.UnaryInterceptor(
grpc_prometheus.UnaryServerInterceptor,
What this means is that, when reaching the server, the validations are run thanks to protovalidate and raise `validation error: - v1.materials[4].type: value must not be in list [0] [enum.not_in]` and this is because the incoming proto although well formed is not valid upon our constraints.
We cannot perform the same operation as the CLI does under the hood since what is received on the server is correct although not valid so going from `proto => json => proto` won't work because the type would still be `type: MATERIAL_TYPE_UNSPECIFIED`, if we later run the validation, then we will fail.
The solution then, would be on the approach of improving the error message. Unfortunately the buf validation library although wide, does not provide much more configuration except the `cel` option:
Something like this will perform in the same way as the enum validation we have in place. From here:
```proto
MaterialType type = 1 [
(buf.validate.field).enum = {
not_in: [0]
},
(buf.validate.field).enum.defined_only = true
];
To here:
MaterialType type = 1 [(buf.validate.field).cel = {
id: "type"
message: "Type must be one of the allowed values"
expression: "this != 0 && this in [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]"
}];
As you can see one of the drawback is, that we need to keep track of the list of values.
To sum up, we need to find a way of improving the general return of errors in the API level.
what are the next steps on this then?
I'd like to double check the possibility of improving validation error messages server side. It's very discouraging that at first this is not trivial. So next step is to look into how hard is to have custom error messages applied to the proto validations.
When creating a contract through the API, if it contains a typo on the actual fields of the json, it throws an error from the buf validation, we should return a better error as in the CLI:
Interesting enough, using grpcurl produces the same error: