fuel-dev team mailing list archive

[Vitaly Kramskikh <vkramskikh@xxxxxxxxxxxx>]

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 <akasatkin@xxxxxxxxxxxx>:

> 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 <
> ikolodyazhny@xxxxxxxxxxxx> 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     : fuel-dev@xxxxxxxxxxxxxxxxxxx
>> Unsubscribe : https://launchpad.net/~fuel-dev
>> More help   : https://help.launchpad.net/ListHelp
>>
>>
>


-- 
Vitaly Kramskikh,
Software Engineer,
Mirantis, Inc.