search

drogue-iot / drogue-cloud

Cloud Native IoT
https://drogue.io
Apache License 2.0
114 stars 30 forks source link

Applications are not checked for validity on save #264

Open JulianFeinauer opened 2 years ago

JulianFeinauer commented 2 years ago

It seems that you can currently enter an invalid Application definition (as yaml) and thats get stored without an Issue.

Expected Behavior: This should produce an error internaly and return an error code and not get saved

ctron commented 2 years ago

True. Currently that is "on purpose". As we only enforce the metadata section and the fact that there are a .status and .spec field of the type map.

As discussed in the chat, it makes of course sense to validate (before storing). Some of the content however cannot be validated by Drogue Cloud, as it doesn't know about the content. A possible solution would be to configure (opt-in) web hook validation on the application level, which would be able to validate.

So the logic could be:

Any rejection would reject the whole operations. Maybe a webhook could also alter the content (which might make it tricky to parallelize).

I think it would make sense to use Knative serving style endpoints again (as we do with payload validation/enrichment).

PR's welcome :) … I would also say that a PR could start with some internal validation first. Assuming this is wrapped by an async operation (which can later be extended easily with webhooks).

JulianFeinauer commented 2 years ago

You have to help me out a bit with my currently not-perfect understanding on drogues architecture. Are there services one application could have but others not? Arent they running on the same instance or is this micro-serviced away? What do applications share?

lulf commented 2 years ago

@JulianFeinauer At the moment, it is possible to add custom fields within .spec. and .status for the Application and Device types.

One such example application is https://github.com/drogue-iot/drogue-ajour, which extends them with additional information related to firmware updates, and can be deployed by the 'user' of Drogue Cloud.

ctron commented 2 years ago

The service responsible for writing the application to the persistent storage is the device-management-service. That contains two layers (Rust modules): endpoints which defines the REST endpoints and forward the requests to some persistence service, and service which defines the persistence services and their implementations (currently postgres only).

I guess it doesn't make sense to start any database operation before validating the data. So I would assume the validation part triggered by the endpoints layer before executing calls on the service layer.

JulianFeinauer commented 2 years ago

yes that makes sense to put it in endpoints, indeed. After @lulf response I realized that there may be different kinds of validations. The validation I was looking for, was a situation where there is a Dialog (from Drogue Core) which will later be serialized from the application. But some kind of issues (in my case there was an ENUM and i used a string not in the enum) this leads to the dialog not being parseable and thus silent failure.

Which is a totally different kind of validation then to check for custom fields.

So we should also perhaps consider how to do that.

ctron commented 2 years ago

A dialog?

JulianFeinauer commented 2 years ago

Sorry, a dialect, i mean.