clouditor / clouditor

The Clouditor is a tool to support continuous cloud assurance. Developed by Fraunhofer AISEC.
https://clouditor.io
Apache License 2.0
74 stars 20 forks source link

Optional-defined fields with validation rules are saved in the storage and can violate Response Validation #1051

Open oxisto opened 1 year ago

oxisto commented 1 year ago

Discussed in https://github.com/clouditor/clouditor/discussions/1010

Originally posted by **lebogg** March 14, 2023 Even when we use optional for a proto field (see example below), the storage will save the respective tuple with a zero value of this field (e.g. empty string in the raw field). When we then call `ListEvidences`, e.g., we (in theory) can have a validation error since we have filled the field `raw` with empty string but now validation rules apply (min length 1 in this example). Thus, we would get errors when we check these evidences with corresponding validation methods. e.g. with the [validation on the response](https://github.com/clouditor/clouditor/blob/38f261d852424768157c72b16e802e7c9977a215/api/evidence/evidence_store.pb.validate.go#L635) (`func (m *ListEvidencesResponse) Validate()`). This currently happens in the new tests of #999, where we use the response validation method. The question is: Do we just ignore it; adjust the .proto files; or adjust storage operations to make nil entries if that is even possible. Example: https://github.com/clouditor/clouditor/blob/a718fcf839ed9a5696dcf28e74c20a77b16b6fe4/api/evidence/evidence.proto#L57

This seems strange because the database can handle NULL fields, so sin this case null/nil should be stored in the DB instead of "". Maybe we need to set a different "default value" (see https://gorm.io/docs/models.html#Fields-Tags, tag default).

oxisto commented 1 year ago

We should check, whether nullable/optional fields are really stored as "null" in the database (and not as an empty string for example)