[openstack-dev] [Neutron] UniqueConstraint for name and tenant_id in security group
Christopher Yeoh
cbkyeoh at gmail.com
Tue Dec 16 05:07:31 UTC 2014
So I think this is something we really should get agreement on across the
open stack API first before flipping back and forth on a case by case
basis.
Personally I think we should be using uuids for uniqueness and leave any
extra restrictions to a ui layer if really required. If we try to have name
uniqueness then "test " should be considered the same as " test" as " test
" and it introduces all sorts of slightly different combos that look the
same except under very close comparison. Add unicode for extra fun.
Chris
On Tue, 16 Dec 2014 at 7:24 am, Maru Newby <marun at redhat.com> wrote:
>
> On Dec 15, 2014, at 9:13 AM, Assaf Muller <amuller at redhat.com> wrote:
>
> >
> >
> > ----- Original Message -----
> >> -----BEGIN PGP SIGNED MESSAGE-----
> >> Hash: SHA512
> >>
> >> I was (rightfully) asked to share my comments on the matter that I
> >> left in gerrit here. See below.
> >>
> >> On 12/12/14 22:40, Sean Dague wrote:
> >>> On 12/12/2014 01:05 PM, Maru Newby wrote:
> >>>>
> >>>> On Dec 11, 2014, at 2:27 PM, Sean Dague <sean at dague.net> wrote:
> >>>>
> >>>>> On 12/11/2014 04:16 PM, Jay Pipes wrote:
> >>>>>> On 12/11/2014 04:07 PM, Vishvananda Ishaya wrote:
> >>>>>>> On Dec 11, 2014, at 1:04 PM, Jay Pipes <jaypipes at gmail.com>
> >>>>>>> wrote:
> >>>>>>>> On 12/11/2014 04:01 PM, Vishvananda Ishaya wrote:
> >>>>>>>>>
> >>>>>>>>> On Dec 11, 2014, at 8:00 AM, Henry Gessau
> >>>>>>>>> <gessau at cisco.com> wrote:
> >>>>>>>>>
> >>>>>>>>>> On Thu, Dec 11, 2014, Mark McClain <mark at mcclain.xyz>
> >>>>>>>>>> wrote:
> >>>>>>>>>>>
> >>>>>>>>>>>> On Dec 11, 2014, at 8:43 AM, Jay Pipes
> >>>>>>>>>>>> <jaypipes at gmail.com <mailto:jaypipes at gmail.com>>
> >>>>>>>>>>>> wrote:
> >>>>>>>>>>>>
> >>>>>>>>>>>> I'm generally in favor of making name attributes
> >>>>>>>>>>>> opaque, utf-8 strings that are entirely
> >>>>>>>>>>>> user-defined and have no constraints on them. I
> >>>>>>>>>>>> consider the name to be just a tag that the user
> >>>>>>>>>>>> places on some resource. It is the resource's ID
> >>>>>>>>>>>> that is unique.
> >>>>>>>>>>>>
> >>>>>>>>>>>> I do realize that Nova takes a different approach
> >>>>>>>>>>>> to *some* resources, including the security group
> >>>>>>>>>>>> name.
> >>>>>>>>>>>>
> >>>>>>>>>>>> End of the day, it's probably just a personal
> >>>>>>>>>>>> preference whether names should be unique to a
> >>>>>>>>>>>> tenant/user or not.
> >>>>>>>>>>>>
> >>>>>>>>>>>> Maru had asked me my opinion on whether names
> >>>>>>>>>>>> should be unique and I answered my personal
> >>>>>>>>>>>> opinion that no, they should not be, and if
> >>>>>>>>>>>> Neutron needed to ensure that there was one and
> >>>>>>>>>>>> only one default security group for a tenant,
> >>>>>>>>>>>> that a way to accomplish such a thing in a
> >>>>>>>>>>>> race-free way, without use of SELECT FOR UPDATE,
> >>>>>>>>>>>> was to use the approach I put into the pastebin
> >>>>>>>>>>>> on the review above.
> >>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> I agree with Jay. We should not care about how a
> >>>>>>>>>>> user names the resource. There other ways to
> >>>>>>>>>>> prevent this race and Jay’s suggestion is a good
> >>>>>>>>>>> one.
> >>>>>>>>>>
> >>>>>>>>>> However we should open a bug against Horizon because
> >>>>>>>>>> the user experience there is terrible with duplicate
> >>>>>>>>>> security group names.
> >>>>>>>>>
> >>>>>>>>> The reason security group names are unique is that the
> >>>>>>>>> ec2 api supports source rule specifications by
> >>>>>>>>> tenant_id (user_id in amazon) and name, so not
> >>>>>>>>> enforcing uniqueness means that invocation in the ec2
> >>>>>>>>> api will either fail or be non-deterministic in some
> >>>>>>>>> way.
> >>>>>>>>
> >>>>>>>> So we should couple our API evolution to EC2 API then?
> >>>>>>>>
> >>>>>>>> -jay
> >>>>>>>
> >>>>>>> No I was just pointing out the historical reason for
> >>>>>>> uniqueness, and hopefully encouraging someone to find the
> >>>>>>> best behavior for the ec2 api if we are going to keep the
> >>>>>>> incompatibility there. Also I personally feel the ux is
> >>>>>>> better with unique names, but it is only a slight
> >>>>>>> preference.
> >>>>>>
> >>>>>> Sorry for snapping, you made a fair point.
> >>>>>
> >>>>> Yeh, honestly, I agree with Vish. I do feel that the UX of
> >>>>> that constraint is useful. Otherwise you get into having to
> >>>>> show people UUIDs in a lot more places. While those are good
> >>>>> for consistency, they are kind of terrible to show to people.
> >>>>
> >>>> While there is a good case for the UX of unique names - it also
> >>>> makes orchestration via tools like puppet a heck of a lot simpler
> >>>> - the fact is that most OpenStack resources do not require unique
> >>>> names. That being the case, why would we want security groups to
> >>>> deviate from this convention?
> >>>
> >>> Maybe the other ones are the broken ones?
> >>>
> >>> Honestly, any sanely usable system makes names unique inside a
> >>> container. Like files in a directory.
> >>
> >> Correct. Or take git: it does not use hashes to identify objects, right?
> >>
> >>> In this case the tenant is the container, which makes sense.
> >>>
> >>> It is one of many places that OpenStack is not consistent. But I'd
> >>> rather make things consistent and more usable than consistent and
> >>> less.
> >>
> >> Are we only proposing to make security group name unique? I assume
> >> that, since that's what we currently have in review. The change would
> >> make API *more* inconsistent, not less, since other objects still use
> >> uuid for identification.
> >>
> >> You may say that we should move *all* neutron objects to the new
> >> identification system by name. But what's the real benefit?
> >>
> >> If there are problems in UX (client, horizon, ...), we should fix the
> >> view and not data models used. If we decide we want users to avoid
> >> using objects with the same names, fine, let's add warnings in UI
> >> (probably with an option to disable it so that we don't push the
> >> validation into their throats).
> >>
> >> Finally, I have concern about us changing user visible object
> >> attributes like names during db migrations, as it's proposed in the
> >> patch discussed here. I think such behaviour can be quite unexpected
> >> for some users, if not breaking their workflow and/or scripts.
> >>
> >> My belief is that responsible upstream does not apply ad-hoc changes
> >> to API to fix a race condition that is easily solvable in other ways
> >> (see Assaf's proposal to introduce a new DefaultSecurityGroups table
> >> in patchset 12 comments).
> >>
> >
> > As usual you explain yourself better than I can... I think my main
> > original objection to the patch is that it feels like an accidental
> > API change to fix a bug. If you want unique naming:
> > 1) We need to be consistent across different resources
> > 2) It needs to be in a dedicate change, and perhaps blueprint
> >
> > Since there's conceivable alternative solutions to the bug that aren't
> > substantially more costly or complicated, I don't see why we would pursue
> > the proposed approach.
>
>
> +1
>
> Regardless of the merits of security groups having unique names, I don’t
> think it is a change that should be slipped in as part of a bugfix. If we
> want to see this kind of API-modifying change introduced in Neutron (or any
> other OpenStack project), there is a process that needs to be followed.
>
>
>
> Maru
>
>
> >
> >> As for the whole object identification scheme change, for this to
> >> work, it probably needs a spec and a long discussion on any possible
> >> complications (and benefits) when applying a change like that.
> >>
> >> For reference and convenience of readers, leaving the link to the
> >> patch below: https://review.openstack.org/#/c/135006/
> >>
> >>
> >>
> >>>
> >>> -Sean
> >>>
> >> -----BEGIN PGP SIGNATURE-----
> >> Version: GnuPG/MacGPG2 v2.0.22 (Darwin)
> >>
> >> iQEcBAEBCgAGBQJUjxI1AAoJEC5aWaUY1u579M8H/RC+M7/9YYDClWRjhQLBNqEq
> >> 0pMxURZi8lTyDmi+cXA7wq1QzgOwUqWnJnOMYzq8nt9wLh8cgasjU5YXZokrqDLw
> >> zEu/a1Cd9Alp4iGYQ6upw94BptGrMvk+XwTedMX9zMLf0od1k8Gzp5xYH/GXInN3
> >> E+wje40Huia0MmLu4i2GMr/13gD2aYhMeGxZtDMcxQsF0DBh0gy8OM9pfKgIiXVM
> >> T65nFbXUY1/PuAdzYwMto5leuWZH03YIddXlzkQbcZoH4PGgNEE3eKl1ctQSMGoo
> >> bz3l522VimQvVnP/XiM6xBjFqsnPM5Tc7Ylu942l+NfpfcAM5QB6Ihvw5kQI0uw=
> >> =gIsu
> >> -----END PGP SIGNATURE-----
> >>
> >> _______________________________________________
> >> OpenStack-dev mailing list
> >> OpenStack-dev at lists.openstack.org
> >> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> >>
> >
> > _______________________________________________
> > OpenStack-dev mailing list
> > OpenStack-dev at lists.openstack.org
> > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
>
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20141216/2ebbb200/attachment.html>
More information about the OpenStack-dev
mailing list