fuel-dev team mailing list archive

[Igor Kalnitsky <ikalnitsky@xxxxxxxxxxxx>]

Aleksey,

As I've pointed early, we have a blueprint to write unit-tests for
validators, not integrational. It's a good idea, I think. If we write more
tests now (as integrational), we will write the same tests later as
unit-tests.

So my point is: if the
blueprint<https://blueprints.launchpad.net/fuel/+spec/nailgun-validators-testing>will
milestoned for 5.1, i propose to write more tests after it will be
completed.


- Igor



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

> Igor,
>
> AFAIC, validators refactoring is another task that should be done in
> parallel.
>
> In my opinion, documenting of API (parameters JSONs) should be done as a
> first step to improve input cases coverage.
> AFAIK, we have the same task for the orchestrator: there is no
> documentation on its input yet (just some fragments and samples).
>
>
> Aleksey Kasatkin
>
> S. Software Developer | Mirantis, Inc. | http://www.mirantis.com
> cell: +380938330852 | skype: alexeyk_ru
>
>
> On Tue, May 20, 2014 at 3:50 PM, Igor Kalnitsky <ikalnitsky@xxxxxxxxxxxx>wrote:
>
>> 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
>>>
>>>
>>
>