fuel-dev team mailing list archive

[Igor Kalnitsky <ikalnitsky@xxxxxxxxxxxx>]

Hi guys,

Indeed, our validators don't cover all input cases and it's bad. Moreover,
our validators is too complicated to support and we have some plans to
refactor them.

Here the blueprints we have:

- https://blueprints.launchpad.net/fuel/+spec/nailgun-validators-to-objects
- https://blueprints.launchpad.net/fuel/+spec/nailgun-validators-testing
- https://blueprints.launchpad.net/fuel/+spec/nailgun-validators-json

So.. In order to reduce our job, I propose to implement this
blueprint<https://blueprints.launchpad.net/fuel/+spec/nailgun-validators-testing>first,
and then to cover input cases as much as possible.

What do you think about it?

- Igor


On Tue, May 20, 2014 at 3:30 PM, Aleksey Kasatkin <akasatkin@xxxxxxxxxxxx>wrote:

> Vitaly,
> That's all about validation of input data indeed. There is no sense to
> validate own output.
> You had proposals about declarative description of validation logic in
> April in Moscow. Can you provide links to materials on this topic?
>
>
> Aleksey Kasatkin
>
> S. Software Developer | Mirantis, Inc. | http://www.mirantis.com
> cell: +380938330852 | skype: alexeyk_ru
>
>
> On Tue, May 20, 2014 at 3:10 PM, Vitaly Kramskikh <vkramskikh@xxxxxxxxxxxx
> > wrote:
>
>> 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.
>>
>
>
> --
> 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
>
>