Bug 1117176

Hi Ravi,

Currently at the "Add User/Group" we have the following dialog:


  Search: ______________V    ___________________ [GO]
              Authz                 Term

It would be nice to add the new field of "namespace", each authz extension supports a collection of namespaces, a search should be limited to a specific namespace.

Currently, to keep UI as it is we just iterate through all namespaces, which may take a lot of unneeded time.

New search should be:

  Search: ______________V _____________V    ___________________ [GO]
              Authz         Namespace             Term

Please note that namespace can be long term, so although dropbox should be width enough, the dropbox caption should be at fixed size, overflow should be hidden.

Thanks!

Sorry, comment#1 and subject change was in wrong bug.

I am unsure we would like to show groups, the overhead is huge!

(In reply to Alon Bar-Lev from comment #3)
> I am unsure we would like to show groups, the overhead is huge!

Can you elaborate?

Also, is this indeed a regression?

(In reply to Oved Ourfali from comment #4)
> (In reply to Alon Bar-Lev from comment #3)
> > I am unsure we would like to show groups, the overhead is huge!
> 
> Can you elaborate?
> 
> Also, is this indeed a regression?

we are not user management application, the restapi is used only to allow to get user unique id in order to add it to our application.

enlisting user groups of user takes a lot of query for ldap for example, and is not needed.

user can use own user management solution (ldap or whatever) to see what groups a user belongs to.

(In reply to Alon Bar-Lev from comment #5)
> (In reply to Oved Ourfali from comment #4)
> > (In reply to Alon Bar-Lev from comment #3)
> > > I am unsure we would like to show groups, the overhead is huge!
> > 
> > Can you elaborate?
> > 
> > Also, is this indeed a regression?
> 
> we are not user management application, the restapi is used only to allow to
> get user unique id in order to add it to our application.
> 
> enlisting user groups of user takes a lot of query for ldap for example, and
> is not needed.
> 
> user can use own user management solution (ldap or whatever) to see what
> groups a user belongs to.

In the past we did fill them?

Also, they are accessible in https://engine/ovirt-engine/api/users but not for a specific user?

(In reply to Oved Ourfali from comment #6)
> (In reply to Alon Bar-Lev from comment #5)
> > (In reply to Oved Ourfali from comment #4)
> > > (In reply to Alon Bar-Lev from comment #3)
> > > > I am unsure we would like to show groups, the overhead is huge!
> > > 
> > > Can you elaborate?
> > > 
> > > Also, is this indeed a regression?
> > 
> > we are not user management application, the restapi is used only to allow to
> > get user unique id in order to add it to our application.
> > 
> > enlisting user groups of user takes a lot of query for ldap for example, and
> > is not needed.
> > 
> > user can use own user management solution (ldap or whatever) to see what
> > groups a user belongs to.
> 
> In the past we did fill them?

I do not think it is important.

> Also, they are accessible in https://engine/ovirt-engine/api/users but not
> for a specific user?

they should be available at /groups.... this is the place for groups...

(In reply to Alon Bar-Lev from comment #7)
> (In reply to Oved Ourfali from comment #6)
> > (In reply to Alon Bar-Lev from comment #5)
> > > (In reply to Oved Ourfali from comment #4)
> > > > (In reply to Alon Bar-Lev from comment #3)
> > > > > I am unsure we would like to show groups, the overhead is huge!
> > > > 
> > > > Can you elaborate?
> > > > 
> > > > Also, is this indeed a regression?
> > > 
> > > we are not user management application, the restapi is used only to allow to
> > > get user unique id in order to add it to our application.
> > > 
> > > enlisting user groups of user takes a lot of query for ldap for example, and
> > > is not needed.
> > > 
> > > user can use own user management solution (ldap or whatever) to see what
> > > groups a user belongs to.
> > 
> > In the past we did fill them?
> 
> I do not think it is important.
> 

It is. As we need to add backward compatibility to our consideration here.

> > Also, they are accessible in https://engine/ovirt-engine/api/users but not
> > for a specific user?
> 
> they should be available at /groups.... this is the place for groups...

I agree.

So the status as I see it is that we do allow to show groups under https://engine/ovirt-engine/api/users (seeing the groups of each user), but not when accessed under domains.

Doesn't seem important to me, but it is indeed a regression.

Juan + Barak - please share your thoughts.

We don't know what users are doing with the information they get from /domains/{domain:id}/users. They may have applications that rely on the groups presented there, so if we used to show the groups there we need to keep showing them.

In 4.0 we can remove this functionality if deemed unnecessary.

The solution to support that is easy - the users should be queried with the RESOLVE_GROUPS flag.
However, this will hurt the performance of the search.
Barak, 
please advise.

I would go ahead with the fix suggested by Yair,
But mark it for removal in 4.0

Juan you need to track/manage the 4.0 items to be removed from the API.

reply publicly again:

1. there is no logical scenario in which a user is to be added into application and /domain/x/users is enumerated while the group of that user is to enumerated, please do remember we enumerate EXTERNAL resources. recursive query of groups are extremely expensive, and should be avoided in almost any cost. we worked very hard to remove them from the product, leaving them only when user actually login when it is actually required.

2. groups should and probably always were enumerated bu querying /domain/x/groups, this is why we have the groups, and probably what 3rd parties are doing.

3. this behavior was implemented because of lack of knowledge about ldap and optimization, and we were hit bugs regarding both, which are far more important that keeping behavior that is outside of sensible usage.

4. we already broke the mapping between internal and external users, and have no side effects yet, this issue follow the same principal, a data that had been there and should not have been used.

5. correct approach is to remove the recursive group query when listing external users.

6. if we are to leave legacy implementation we should add it disabled per default and that can be turned on explicitly using vdc_options.

7. the effort of (6) can be done in z-stream if we have a report that it is indeed needed, as there is no sense in investing any resource in that per (1), there is no change in api schema in either case.

I have no problem with the #6, but I don't think we can avoid delivering it for GA (= not in zstream), I agree it's not changing the schema per say as ..../domains/<ID>/users/<UID>/groups/ will be empty.

But if someone expects it to have the correct data it is a problem.

Again - I think that #6 above you have suggested is an easy fix and I would go with that for GA.

(In reply to Barak from comment #13)
> I have no problem with the #6, but I don't think we can avoid delivering it
> for GA (= not in zstream), I agree it's not changing the schema per say as
> ..../domains/<ID>/users/<UID>/groups/ will be empty.
> 
> But if someone expects it to have the correct data it is a problem.

again, please present a sensible sequence of use case when this is required. aka when should someone expect that.

> Again - I think that #6 above you have suggested is an easy fix and I would
> go with that for GA.

I think we should not invest void efforts.

(In reply to Alon Bar-Lev from comment #14)
> (In reply to Barak from comment #13)
> > I have no problem with the #6, but I don't think we can avoid delivering it
> > for GA (= not in zstream), I agree it's not changing the schema per say as
> > ..../domains/<ID>/users/<UID>/groups/ will be empty.
> > 
> > But if someone expects it to have the correct data it is a problem.
> 
> again, please present a sensible sequence of use case when this is required.
> aka when should someone expect that.

even when someone (wrongfully) query those parts - he does not have to add a user/group there in order for things to break for him.

> 
> > Again - I think that #6 above you have suggested is an easy fix and I would
> > go with that for GA.
> 
> I think we should not invest void efforts.

According to comment #10 this is not a big effort.

(In reply to Barak from comment #15)
> (In reply to Alon Bar-Lev from comment #14)
> > (In reply to Barak from comment #13)
> > > I have no problem with the #6, but I don't think we can avoid delivering it
> > > for GA (= not in zstream), I agree it's not changing the schema per say as
> > > ..../domains/<ID>/users/<UID>/groups/ will be empty.
> > > 
> > > But if someone expects it to have the correct data it is a problem.
> > 
> > again, please present a sensible sequence of use case when this is required.
> > aka when should someone expect that.
> 
> even when someone (wrongfully) query those parts - he does not have to add a
> user/group there in order for things to break for him.

he will get empty result, which is valid response, nothing will break.

> 
> > 
> > > Again - I think that #6 above you have suggested is an easy fix and I would
> > > go with that for GA.
> > 
> > I think we should not invest void efforts.
> 
> According to comment #10 this is not a big effort.

again, this is a void effort.

(In reply to Alon Bar-Lev from comment #16)
> > 
> > > 
> > > > Again - I think that #6 above you have suggested is an easy fix and I would
> > > > go with that for GA.
> > > 
> > > I think we should not invest void efforts.
> > 
> > According to comment #10 this is not a big effort.
> 
> again, this is a void effort.

need to clarify this:

providing this legacy behavior disabled per default, or providing the option to enable the legacy behavior at first z-stream has the same impact.

first user faces issue, then after its root cause is determine he gets a solution either by enable vdc_option or waiting for next z-stream and enable vdc_option. the chance we get such is minimal, as the response of external user has no groups is valid.

(In reply to Alon Bar-Lev from comment #17)
> (In reply to Alon Bar-Lev from comment #16)
> > > 
> > > > 
> > > > > Again - I think that #6 above you have suggested is an easy fix and I would
> > > > > go with that for GA.
> > > > 
> > > > I think we should not invest void efforts.
> > > 
> > > According to comment #10 this is not a big effort.
> > 
> > again, this is a void effort.
> 
> need to clarify this:
> 
> providing this legacy behavior disabled per default, or providing the option
> to enable the legacy behavior at first z-stream has the same impact.
> 
> first user faces issue, then after its root cause is determine he gets a
> solution either by enable vdc_option or waiting for next z-stream and enable
> vdc_option. the chance we get such is minimal, as the response of external
> user has no groups is valid.

The difference is the dependency in zstream.

In case it is only config for GA , a customer encounter this and enable this ... and he is good.

(In reply to Barak from comment #18)
> (In reply to Alon Bar-Lev from comment #17)
> > (In reply to Alon Bar-Lev from comment #16)
> > > > 
> > > > > 
> > > > > > Again - I think that #6 above you have suggested is an easy fix and I would
> > > > > > go with that for GA.
> > > > > 
> > > > > I think we should not invest void efforts.
> > > > 
> > > > According to comment #10 this is not a big effort.
> > > 
> > > again, this is a void effort.
> > 
> > need to clarify this:
> > 
> > providing this legacy behavior disabled per default, or providing the option
> > to enable the legacy behavior at first z-stream has the same impact.
> > 
> > first user faces issue, then after its root cause is determine he gets a
> > solution either by enable vdc_option or waiting for next z-stream and enable
> > vdc_option. the chance we get such is minimal, as the response of external
> > user has no groups is valid.
> 
> The difference is the dependency in zstream.
> 
> In case it is only config for GA , a customer encounter this and enable this
> ... and he is good.

The chance that there is an effect is small, and worth taking the chance in order to reduce dirty code within code base. Adding such dirty code should have a good reason. A good reason was not provided so far (real use case sequence), only speculations of someone do "wrongfully" behavior (comment#15) in this case he will just get no groups for user, without any other side effect, which is a valid response, so such "wrongfully" application will get a list of users with no groups, display the users without group information... all ok.

I do try to understand, and have failed so far, and I give up.

I just hope this is not to make internal people happy, this I do not accept.

(In reply to Alon Bar-Lev from comment #19)
> (In reply to Barak from comment #18)
> > (In reply to Alon Bar-Lev from comment #17)
> > > (In reply to Alon Bar-Lev from comment #16)
> > > > > 
> > > > > > 
> > > > > > > Again - I think that #6 above you have suggested is an easy fix and I would
> > > > > > > go with that for GA.
> > > > > > 
> > > > > > I think we should not invest void efforts.
> > > > > 
> > > > > According to comment #10 this is not a big effort.
> > > > 
> > > > again, this is a void effort.
> > > 
> > > need to clarify this:
> > > 
> > > providing this legacy behavior disabled per default, or providing the option
> > > to enable the legacy behavior at first z-stream has the same impact.
> > > 
> > > first user faces issue, then after its root cause is determine he gets a
> > > solution either by enable vdc_option or waiting for next z-stream and enable
> > > vdc_option. the chance we get such is minimal, as the response of external
> > > user has no groups is valid.
> > 
> > The difference is the dependency in zstream.
> > 
> > In case it is only config for GA , a customer encounter this and enable this
> > ... and he is good.
> 
> The chance that there is an effect is small, and worth taking the chance in
> order to reduce dirty code within code base. Adding such dirty code should
> have a good reason. A good reason was not provided so far (real use case
> sequence), only speculations of someone do "wrongfully" behavior
> (comment#15) in this case he will just get no groups for user, without any
> other side effect, which is a valid response, so such "wrongfully"
> application will get a list of users with no groups, display the users
> without group information... all ok.
> 
> I do try to understand, and have failed so far, and I give up.
> 
> I just hope this is not to make internal people happy, this I do not accept.

I'm against adding a configuration for that, as it is adding more ugly code for something we don't want in the first place.

I see two options here:
1. Implement and mark as deprecated? (do we have a way of marking something as deprecated in the REST API?)
2. Leaving it empty as Alon suggested.

This discussion can be endless, and adding the support is a small effort, that's why I've suggested going with the solution Yair proposed.

It sounds to me like this option shouldn't be used at all, and there is a good explanation behind it. 

Barak - If the claim that "it shouldn't be used, and probably not used by anyone, and that even if it is then we have good claims why it doesn't make sense" convinces you, then let's close this one as WONTFIX, providing some release note. If it doesn't, then let's fix it.

I know we talked offline about just fixing it, but the more I think about it the less sense it makes.

Nelli - what automation does it block?
If one wants to know the domain user groups, then he should go to his directory server, and not to the oVirt API.
I suggest to remove this Keyword.

removing AutomationBlocker keyword since i don't see any reference for automtion jobs.

O.K. guys you (and Itamar) have convinced me ...
This use case was modeled wrong .. we can remove it ... meaning leave it as is.
Or should we remove the /groups as well ?

(In reply to Barak from comment #23)
> Or should we remove the /groups as well ?

/domains/x/groups should return a list of groups from the authz provider, again non recursive.

as far as I checked it works correctly, if does not work should open a new bug.

closing this bug.

thanks for discussion!