Open Stack

On Thu, 18 Sep 2014 13:45:42 +0200
Flavio Percoco <flavio at redhat.com> wrote:

> On 09/18/2014 01:28 PM, Sean Dague wrote:
> > On 09/18/2014 07:19 AM, Ken'ichi Ohmichi wrote:
> >> 2014-09-18 19:57 GMT+09:00 Sean Dague <sean at dague.net>:
> >>> On 09/18/2014 06:38 AM, Christopher Yeoh wrote:
> >>>> On Sat, 13 Sep 2014 06:48:19 -0400
> >>>> Sean Dague <sean at dague.net> wrote:
> >>>>>>>
> >>>>>>> We have proposed that the allowed characters for all resource
> >>>>>>> names in Nova (flavors, aggregates, etc.) be expanded to all
> >>>>>>> printable unicode characters and horizontal spaces:
> >>>>>>> https://review.openstack.org/#/c/119741
> >>>>>>>
> >>>>>>> Currently, the only allowed characters in most resource names
> >>>>>>> are alphanumeric, space, and [.-_].
> >>>>>>>
> >>>>>>>
> >>>>>>> We have proposed this change for two principal reasons:
> >>>>>>>
> >>>>>>> 1. We have customers who have migrated data forward since
> >>>>>>> Essex, when no restrictions were in place, and thus have
> >>>>>>> characters in resource names that are disallowed in the
> >>>>>>> current version of OpenStack. This is only likely to be
> >>>>>>> useful to people migrating from Essex or earlier, since the
> >>>>>>> current restrictions were added in Folsom.
> >>>>>>>
> >>>>>>> 2. It's pretty much always a bad idea to add unnecessary
> >>>>>>> restrictions without a good reason. While we don't have an
> >>>>>>> immediate need to use, for example, the ever-useful
> >>>>>>> http://codepoints.net/U+1F4A9 in a flavor name, it's hard to
> >>>>>>> come up with a reason people *shouldn't* be allowed to use it.
> >>>>>>>
> >>>>>>> That said, apparently people have had a need to not be
> >>>>>>> allowed to use some characters, but it's not clear why:
> >>>>>>> https://bugs.launchpad.net/nova/+bug/977187 So I guess if
> >>>>>>> anyone knows any reason why these printable characters should
> >>>>>>> not be joined in holy resource naming, speak now or forever
> >>>>>>> hold your peace.
> >>>>>>
> >>>>>> I also could not find the reason of current restriction on the
> >>>>>> bug report, and I'd like to know it as the history.
> >>>>>> On v2 API(not v2.1), each resource name contains the following
> >>>>>> restriction for its name:
> >>>>>>
> >>>>>>   Resource  | Length  | Pattern
> >>>>>>  -----------+---------+----------------------------------
> >>>>>>   aggregate | 1-255   | nothing
> >>>>>>   backup    | nothing | nothing
> >>>>>>   flavor    | 1-255   | '^[a-zA-Z0-9. _-]*[a-zA-Z0-9_-]+
> >>>>>>             |         |   [a-zA-Z0-9. _-]*$'
> >>>>>>   keypair   | 1-255   | '^[a-zA-Z0-9 _-]+$'
> >>>>>>   server    | 1-255   | nothing
> >>>>>>   cell      | 1-255   | don't contain "." and "!"
> >>>>>>
> >>>>>> On v2.1 API, we have applied the same restriction rule[1] for
> >>>>>> whole resource names for API consistency, so maybe we need to
> >>>>>> consider this topic for whole names.
> >>>>>>
> >>>>>> [1]:
> >>>>>> https://github.com/openstack/nova/blob/master/nova/api/validation/parameter_types.py#L44
> >>>>>
> >>>>> Honestly, I bet this had to do with how the database used to be
> >>>>> set up.
> >>>>>
> >>>>
> >>>> So it turns out that utf8 support in MySQL does not support
> >>>> UTF-8 4 byte multibyte characters (only 1-3 bytes). For example
> >>>> if you do a create image call with an image name to glance with
> >>>> a 4 byte multibyte character in the name it will 500. I'd guess
> >>>> we do something similar in places with the Nova API where we
> >>>> have inadequate input validation. If you want 4 byte support you
> >>>> need to use utf8mb4 instead.
> >>>
> >>> Oh... fun. :(
> >>>
> >>>> I don't know if postgresql has any restrictions (I don't think it
> >>>> does) or if db2 does too. But I don't think we can/should make
> >>>> it a complete free for all. It should at most be what most
> >>>> databases support.
> >>>>
> >>>> I think its a big enough change that this late in the cycle we
> >>>> should push it off to Kilo. It's always much easier to loosen
> >>>> input validation than tighten it (or have to have an "oops"
> >>>> revert on an officially released Nova). Perhaps some tests to
> >>>> verify that everything we allow past the input validation checks
> >>>> we can actually store.
> >>>
> >>> So, honestly, that seems like a pendulum swing in an odd way.
> >>>
> >>> Havana "use anything you want!"
> >>> Icehouse ?
> >>> Juno "strict asci!"
> >>> Kilo "utf8"
> >>
> >> Correct validation history is
> >>
> >> Essex: "use anything you want!"
> >> Folsom: "strict asci!"
> >> [..]
> >> Juno: "strict asci!"
> >>
> >> So I don't think we should make the input validation loose right
> >> now to avoid a pendulum swing.
> > 
> > Ok, great. That history makes me ok with status quo. I didn't
> > realize it went back so far.
> > 
> >>> Can't we just catch the db exception correctly in glance and not
> >>> have it explode? And then allow it. Exploding with a 500 on a bad
> >>> name seems the wrong thing to do anyway.
> >>>
> >>> That would also mean that if the user changed their database to
> >>> support utf8mb4 (which they might want to do if it was important
> >>> to them) it would all work.
> >>>
> >>> I think some release notes would be fine to explain the current
> >>> situation and limitations.
> >>>
> >>> Basically, lets skate towards the puck here, realizing some
> >>> corner cases exist, but that we're moving in the direction we
> >>> want to be, not back tracking.
> >>
> >> One idea is that: How about using base64 encoding/decoding if
> >> non-ascii chars come? REST API layer would encode resource names
> >> if necessary before passing it to DB layer, and we don't need to
> >> consider backend DB features. The disadvantage is that available
> >> name length becomes short if non-ascii chars. But maybe that would
> >> be acceptable..
> > 
> > Honestly, we should utf8 through to the db. If the user's db doesn't
> > support it... that's their implementation issue.
> 
> +1
> 
> > 
> > Also... glance should not explode. That seems like a critical bug.
> > Has anyone filed that?
> 
> I agree it should not explode. That seems bad. I'll repeat Sean's
> question: Has anyone filed that? Otherwise, I think I'll go ahead and
> create a bug for it.
> 

Yes I did report it. Only reason I mentioned glance was that I'd seen
the backtrace from glance myself yesterday, whereas I'm pretty sure
it'll fail with Nova in the same way but I haven't actually tested it
yet.

https://bugs.launchpad.net/glance/+bug/1370954

Chris