[
https://issues.apache.org/jira/browse/HADOOP-15692?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16594121#comment-16594121
]
Steve Loughran commented on HADOOP-15692:
-----------------------------------------
I'm busy learning about DTs myself (HADOOP-14556) so am now roughly in a
position to review this —at the same time, I'm still learning, so it's good to
see your design and try to pick up ideas. What I'm thinking of implementing as
pluging mechanism is similar to yours, though you are ahead of me in
implementation.
h2. General
Some general questions about overall design.
* Is the expectation that there will be one token provider for every client
configuration? As I've been looking at for S3A having per-fs configs here.
* related: is there ever going to be the need to have >1 link to the same FS
URI for the same client? Daryn's patch for S3A has support for that, which I've
been adopting, even though I'm not sure how I'd imagine using.
I'd like to propose making CustomDelegationTokenManager class a subclass of
AbstractService, using its initialize/start/stop. AbfsDelegationTokenManager
can then subclass CompositeService or simply invoke the lifecycle events of its
tokenManager field as appropriate, as can AzureBlobFileSystem. This is what I
intend to do with my s3a plugin. Why do this?
* Gives everything a unified lifecycle, *including an operation to stop all
threads running*
* Designed for easy subclassing; stable and robust lifecycle implementation.
If someone wanted to implement a plugin which did like the restricted roles of
AWS, where when a DT was requested a token which was returned which offered
only access to the specific container, would this plugin design support it in
future? The use case I'm looking at is: shared Hive cluster with user requests
including DTs granting access only to the given containers. If the Oauth tokens
don't offer such restrictions, I'm not so worried about this, but do consider
this use rather than just "shared Oauth pool service".
h2. Specific
h3. AbfsConfiguration
* can you keep imports in alphabetical order; the renamed
CustomTokenProviderAdapter import needs to move
* You can do a static import of FileSystemConfigurations if that would help the
code readability
h3. AzureBlobFileSystem
L126: add a log entry to log the status of security & DT config; will be useful
for debugging.
L833. Is the javadoc consistent with the current config option/behaviour (i.e.
you need two switches for DTs)?
L840: Feel free to use ? : here rather than the if/else clause
h3. CustomDelegationTokenManager
L40: Add the FS URI to the constructor call. Eventually you'll need it, at
which point all existing implementations will stop linking
L65: maybe have it return false if cancelling isn't supported? Because it's not
in the same category of error as other IOEs.
h3. CustomTokenProviderAdaptee
I know Adaptee is technically correct, but I keep reading it as a typo for the
much more common Adapter.
L74: Recommend `java.time.ZonedDateTime` over `Date`; the latter is too
ambiguous on a global scale. If I in GMT+1 auth with an Azure service in GMT+2,
I still need to know the expiry time relative to me.
h3, DelegationTokenIdentifier order
importing: should be
{code}
java.*
javax.*
--
all non org.apache.
--
org.apache.*
--
statics
{code}
I know, it's rarely changed in existing code, but it's good to get right in the
new stuff. thanks.
L54: use a ref to the constant, to guarantee consistency/spelling
h3. AbfsTokenRenewer
same comment about import ordering.
> ABFS: extensible support for custom oauth
> -----------------------------------------
>
> Key: HADOOP-15692
> URL: https://issues.apache.org/jira/browse/HADOOP-15692
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs/azure
> Reporter: Thomas Marquardt
> Assignee: Da Zhou
> Priority: Major
> Attachments: HADOOP-15692-HADOOP-15407-001.patch
>
>
> ABFS supports oauth in various forms and needs to export interfaces for
> customization of FileSystem.getDelegationToken and getAccessToken.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
