Aleksey, As for that particular bug, there was an UI issue, so UI sent disk names which didn't exist on a node and backend code didn't handle this case properly, so that caused 500 error. It has nothing to do with validation of user input, it is a check of existence of every model mentioned in a request and it must always be performed on backend. UI and CLI have nothing to validate, as they construct this data, and UI was constructing it improperly. We had/have cases where absence of this kind of validation leads to 500 error on every request to node handler, so nailgun becomes unusable.
As for validation of user input, I think we need to describe declaratively as many validation rules as possible to minimize amounts of validation logic in backend and UI. Refactoring of networks is a good example - now we have a set of flags for every network so size of validation code reduced dramatically and we don't have hardcoded network names anywhere. The same can be done for roles: https://review.openstack.org/#/c/89601/6/nailgun/nailgun/fixtures/openstack.yaml. So UI and backend should use these configs to validate data. Also complex validation logic which cannot be described declaratively should be validated on backend only (like intersection of networks). I think CLI should not have any validation at all and just display server response in case of 4xx error. 2014-05-20 13:56 GMT+04:00 Aleksey Kasatkin <[email protected]>: > Yes, the validation of input data in Nailgun doesn't cover all possible > data integrity problems. We just send data from UI and CLI using right > format. It's true for networking data as well (e.g. networks roles are not > validated, networks can be duplicated). > AFAIC, it's very simple to get 500 error using curl. Data validation in > Nailgun should be strengthened if we want to get the same usability for > API as for UI and CLI. > It is partly implemented with introduction of JSON schemas for objects. > Additional validation logic should be added, especially for complex data > sets like disks and networking data. > We enhance data validation occasionally but API is not documented enough > (no description on parameters JSONs). We should get it covered with > documentation at first and make proper validation according to that doc > then (as it was with Networking limitations: see > https://etherpad.openstack.org/p/limitations-of-networking-configuration). > > Validation for CLI can be done similarly. > > Vitaly, please comment on this problem from UI point of view. What can be > done to formalize validation in UI and to make it systematic? > > > Aleksey Kasatkin > > S. Software Developer | Mirantis, Inc. | http://www.mirantis.com > cell: +380938330852 | skype: alexeyk_ru > > > On Mon, May 19, 2014 at 3:35 PM, Ivan Kolodyazhny < > [email protected]> wrote: > >> Hi Fuel devs! >> >> I'm looking on fix https://bugs.launchpad.net/fuel/+bug/1319306. This >> bug is fixed only for Web UI. But it should be fixed on back-end site too. >> Let's add validation for it. AFAIK, at least we've got similar errors with >> roles assignment validation earlier. >> >> I agree, that we need as much as possible validations on our Web UI. But >> back-end validation is very important too. It must be implemented because >> Web UI is only one of possible users of our API. A good REST API must not >> raises 500 errors. 4xx HTTP status codes should be used for any case of >> invalid data. It's a good practice and it makes API usage less risky. >> >> I propose to implement validation on every API client and API handlers: >> >> - API handlers - all input data must be validated; >> - Web UI - all input data should be validated as much as possible; >> - Fuel (CLI) Client - at least, all required parameters should be >> validated. >> >> >> Let's discuss my proposals and file bugs and blueprints if needed. >> >> -- >> Regards, >> Ivan Kolodyazhny, >> Software Engineer, Mirantis, Inc. >> >> -- >> Mailing list: https://launchpad.net/~fuel-dev >> Post to : [email protected] >> Unsubscribe : https://launchpad.net/~fuel-dev >> More help : https://help.launchpad.net/ListHelp >> >> > -- Vitaly Kramskikh, Software Engineer, Mirantis, Inc.
-- Mailing list: https://launchpad.net/~fuel-dev Post to : [email protected] Unsubscribe : https://launchpad.net/~fuel-dev More help : https://help.launchpad.net/ListHelp
