glance member-add doesn't validate input

[Dan Yocum]

Hi,

In ESSEX it is possible to enter invalid data in the glance database when adding members to images.

For instance, I can issue the following command and it completes with success:

glance member-add foo

Even though 'foo' is neither a project nor a registered user in the cloud.

[Brian Waldon (bcwaldon)]

This happens because Glance doesn't have a master list of tenants to check against, and it additionally doesn't decide what a proper tenant ID format is. Would you expect Glance to call out to Keystone in this case?

[Dan Yocum (yocum)]

That's what I would expect, yes.

Am I wrong in expecting this? If so, how is it possible to indicate what member or project an image belongs to?

Is the usage case that prompted this - one of our developers brought his own VM to me to launch in our cloud, but he doesn't want other members of the project to be able to launch it. I figured that since there's a column in horizon that indicates if an image is public or not, I drew the conclusion that I would be able to make images public to specific members (or tenants?). My suspicion was further verified when I found the 'glance member-add <ID> <MEMBER>' option.

So I tried it. And it didn't do anything useful that I could see.

So, from your comment, am I to infer that <MEMBER> is actually tenant and not an individual user? I can see where it would be useful to limit an image to either a particular tenant or an individual user.

'keystone tenant-list' will provide the list of current tenants on the cloud, in the uuid format that everyone seems to be using.

[Brian Waldon (bcwaldon)]

First to clear up the confusion about the definition of a member: an image member is an keystone tenant - it cannot represent a keystone user.

Now, I do think this is a fair thing to ask for, but we have stayed away from it so far for a few reasons:
1) It is costly to call out to Keystone each time we want to add an image member. This problem is treatable, but not without a relatively high level of effort.
2) A deployer may want to seed image members before creating tenants in keystone.

We are spending some time rethinking image sharing for the v2.1 API, so I definitely want to take all of this feedback into account. I will probably end up closing this bug and rolling your thoughts into the related blueprint here: https://blueprints.launchpad.net/glance/+spec/glance-api-v2-image-sharing.

[Dan Yocum (yocum)]

Member == tenant. That's good to know, thanks. Can you change the help text to reflect that?

I'm not sure it's as costly as you might think - of course, the scales that I'm thinking about are much smaller than say, Rackspace or Amazon sized clouds. I think the the majority of the private/hybrid cloud deployers are only going to have a few tenants (<<100) and a few VM flavors (<20). Again, that's my gut feeling. Even if these are 2 orders of magnitude larger, the calls to any database is not going to be bad. If it is a concern use some client side cache - this is data that's not going to change very often.

Maybe I'm missing something, but I just don't see case #2 happening.

I pretty much figured this wouldn't get addressed for Essex and maybe not even in Folsom, but yes, I'll follow the blueprint discussion and pipe up when something piques my interest.

Thanks!

[Brian Waldon (bcwaldon)]

My biggest objection to checking the list of tenants behind the scenes is my general uneasiness towards tying independent distributed services together. I feel like this would be asking for trouble. I'm not sure I see the value in doing this over the compromise of being able to create image members with tenants that do not exist.

[Flavio Percoco (flaper87)]

I agree with Brian and I'd like to keep glance and keystone as independent as possible.

Adding this will make glance more "breakable" by any change that happens in keystone. Perhaps, we should just let users know in a more verbose way that glance doesn't verifies whether that tenant exists or not.

[Dan Yocum (yocum)]

All I'm saying is that the input into the db needs to be validated before it's actually inserted. If the authentication service is "Bob's Pretty Good Auth" then glance should check against that service to verify that that the user exists and is a member of a given project.

Hold on, what are we talking about again? Jeez - it's been 3mo since we last talked about this. ;-)

[John Bresnahan (jbresnah)]

Another issue that would need to be addressed here is consistency. You can validate a member by calling out to keystone when it is inserted, but what if immediately after adding that member to an image the tenant is deleted from keystone? I think having the possibility of an error when any given member attempts to access the image is ultimately unavoidable.

[John Bresnahan (jbresnah)]

IIUC the integration would have to be very tight for that happen. As it
stands keystone could be backed by one db and glance another. I do not
think it is practical to keep them entirely in sync.

On 04/11/2013 11:47 AM, <email address hidden> wrote:
> Then there should be a sql cascade to remove any user+tenant entries
> from the keystone user_tenant_membership table. That's just good
> practice, imo.
>
> On 04/11/2013 04:24 PM, John Bresnahan wrote:
>> Another issue that would need to be addressed here is consistency. You
>> can validate a member by calling out to keystone when it is inserted,
>> but what if immediately after adding that member to an image the tenant
>> is deleted from keystone? I think having the possibility of an error
>> when any given member attempts to access the image is ultimately
>> unavoidable.
>>
>

[Flavio Percoco (flaper87)]

Hey,

@dan

TBH, I don't think it is a good idea to tight glance to keystone - even though it is possible - for a couple of reasons.

1) That would make Glance even more fragile to possible changes in keystone
2) I don't think Glance should worry about the existence of the tenant in keystone, I mean, if the user adds a tenant and later it is removed from keystone - as pointed out by John - it wouldn't be possible to clean it up in glance, unless keystone sends a notification to glance - which would make the integration even tighter.
3) If the tenant doesn't exist in keystone, users wont be able to login to that tenant which means that it wont be used as a member filter in glance, ever. In this case, users will have to clean up the members themselves unless - as pointed in point 2 - keystone notifies Glance and other services.

IMHO, Glance should be as independent as possible from other services in order to focus on the service it is providing, unless we change the way auth works in glance, I don't think there's a good and not so tight way to do this.

Perhaps, this is something keystone should be taking care of. Dunno.

[Mark Washenberger (markwash)]

Is missing validation causing a big problem somewhere? Or would the validation be more of a "nice to have" feature?

[Flavio Percoco (flaper87)]

AFAICT, Apart of being able to associate images to non-existing tenants this isn't a big issue - which can be fixed / changed by admins / owners.

TBH, It could be both, "opinion" or "won't fix" - and I'm more inclined to the later. There are many things that should be considered before implementing something like this - John pointed some of those. I personally don't see a huge advantage on having this, I could be wrong, though.

[Dan Yocum (yocum)]

I don't see my last reply, so I'll post it again, below. FWIW, Flavio, I agree with you - there are other, pressing matters and as long as this doesn't cause an error, then it can be pushed down the todo list.

Here's my last reply that got swallowed by the Internets:

Hi Flavio,

You make good points, so let me make this suggestion: how about a glance
process that does housecleaning on a regular basis that checks against
whatever authorization scheme is being used for valid member+tenant
combinations. I know John is somewhat familiar with the GUMS* <->
VOMS** interaction, so I'll use that as an example.

VOMS is analogous to keystone in this case, but it only maintains
member+tenant combinations.

GUMS is analogous to glance in that it can map a member+tenant
combination to a local resource, i.e., VM image.

GUMS makes a call-out to VOMS periodically (hourly) and queries the VOMS
service for member+tenant combination. When a member+tenant becomes
invalid in VOMS, GUMS removes that record from it's local database.

Dan

* https://twiki.grid.iu.edu/bin/view/Integration/GumsAdmins
** http://vdt.cs.wisc.edu/VOMS-documentation.html

[Flavio Percoco (flaper87)]

Hey,

Yep, that makes a bit of sense to me, even though, I would still prefer to suggest keystone guys to notify when a tenant has been deleted. I can't think of any security issue around that, OTOH. I guess we would need a scrubber listening to keystone notifications anyway.

Along above lines, we could also extend glance-scrubber to do more than just deleting images but this definitely needs further discussion / thoughts.

Would you like to propose keystone notifications n the os-dev list - or the next keystone meeting - and see what they think about that?

Cheers,
FF

P.S: I think we should move this discussion onto a blueprint / pad. Would you like to create one?

[Dan Yocum (yocum)]

Think this task may dovetail nicely into this blueprint:

https://wiki.openstack.org/wiki/Glance-api-v2-image-sharing

The proposal I would make is the following:

A new glance add-on service should periodically poll the upstream authentication and authorization service (i.e., keystone, ldap, et al.) to obtain member privileges to VM images. This service should update the glance database with a user+tenant+role -> image "map." A service that provides similar functionality is the Grid User Mapping Service (GUMS) used widely in the Open Science Grid (OSG).

[Flavio Percoco (flaper87)]

Cool!

That's a good start for a brainstorm on the topic. Add as much details as possible.

I'd say we can close this bug now and track everything in the blueprint!
Thanks a lot!