hadoop-common-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Larry McCay (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HADOOP-9797) Pluggable and compatible UGI change
Date Wed, 31 Jul 2013 18:57:49 GMT

    [ https://issues.apache.org/jira/browse/HADOOP-9797?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13725592#comment-13725592

Larry McCay commented on HADOOP-9797:

As for the design changes, I think that you introduce some nice abstraction that is missing
in UGI. In general the pattern of using an additional check everywhere isSecurityEnabled is
called is unfortunate - but I understand why you've take that approach for backward compatibility.
I think that this pattern can be leveraged for REST endpoints as well - within AuthFilter.

A couple things that bother me a bit:
1. It is even more difficult to distinguish the difference between TokenAuth and the existing
TOKEN(AuthMethod.TOKEN) in the code. I think that in our previous discussions we had the context
of those discussions to keep it all straight. For the uninitiated developer trying to discern
what this code does - I think it is a problem. Unfortunately, I don't have an alternative
term to propose yet.

2. LiteUgi: I think that this is probably mostly a classname issue. Lite brings certain connotations
to mind that I don't think make sense for this class. It appears to be a base/abstract class
for deriving concrete UGI implementations from but at the same time it is returned from methods.
It is also used in method names. I think that what we really need here is an abstract implementation
of a new UGI interface. The interface should be returned by related methods instead of an
abstract class and method names should not include the abstract/base classname. I also don't
think Lite is an appropriate name - I kept thinking that it was somehow related to Simple
or some other "lighter" context. This would facilitate the ability to have UGI impls that
don't need to extend that particular abstract class.

3. Using the LiteUgi name in methods - just calling this out as a separate issue from the
classname in #2 above.

I also share Daryn's opinion that it is a large change. Refactorings such as these are difficult
to decompose into smaller steps but doing so would allow the review to be done more easily.
Additionally, smaller changes would need to be reverted in the case of problems being introduced
through one of the patches.

I will continue to dig through the patch to provide a more detailed review - but I thought
that I would share my high level thinking at this time.

> Pluggable and compatible UGI change
> -----------------------------------
>                 Key: HADOOP-9797
>                 URL: https://issues.apache.org/jira/browse/HADOOP-9797
>             Project: Hadoop Common
>          Issue Type: Sub-task
>          Components: security
>            Reporter: Kai Zheng
>            Assignee: Kai Zheng
>              Labels: rhino
>             Fix For: 3.0.0
>         Attachments: HADOOP-9797-v1.patch
> As already widely discussed current UGI related classes needs to be improved in many
aspects. This is to improve and make UGI so that it can be: 
> * Pluggable, new authentication method with its login module can be dynamically registered
and plugged without having to change the UGI class;
> * Extensible, login modules with their options can be dynamically extended and customized
so that can be reusable elsewhere, like in TokenAuth;
> * No Kerberos relevant, remove any Kerberos relevant functionalities out of it to make
it simple and suitable for other login mechanisms; 
> * Of appropriate abstraction and API, with improved abstraction and API it’s possible
to allow authentication implementations not using JAAS modules;
> * Compatible, should be compatible with previous deployment and authentication methods,
so the existing APIs won’t be removed and some of them are just to be deprecated.

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

View raw message