[ 
https://issues.apache.org/jira/browse/HDDS-804?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16699769#comment-16699769
 ] 

Xiaoyu Yao commented on HDDS-804:
---------------------------------

Thanks [~ajayydv] for the patch. It looks good to me overall. Here are few 
comments:

 

*OzoneSecretManager.java*

 

Line 54: can we pass the LOG from subclass instead of keeping all under the 
base abstract class?

 

Line 89: should we use securityConfig instead of conf here?

 

Line 125: should we use the private key identified by identifier.getKeyID() to 
create the password here?

Otherwise, if the key rolled the password created by the currentKey might not 
match with the signature in the token, which

Is created by previousKey.

 

*OzoneDelegationTOkenSecretManager.java*

 

Line 173: can we guard the log with if (LOG.isDebugEnabled()))

 

 

Line 304: should we update the currentKeyId after loading all the master key?

 

Line 345: should this be super.startThreads()? Or rename the base class methods?

 

Line 388: set store = null after close.

 

Line 406: should we catch the exceptions from store.removeToken so that if one 
token

removal fails, other expired tokens can be removed? This applies to the 
ExpiredTokenRemover

as well since it shared the same removeExpiredToken() routine.

 

 

*OzoneBlockTokenSecretManager.java*

Line 48-50: can we use the KIND_NAME defined in OzoneBlockTokenIdentifier class?
  

Line 79: the comment needs to be fixed.

 

Line 163: NIT: the description does not match the code.

 

Line 178: NIT: add an extra empty line here

> Block token: Add secret token manager
> -------------------------------------
>
>                 Key: HDDS-804
>                 URL: https://issues.apache.org/jira/browse/HDDS-804
>             Project: Hadoop Distributed Data Store
>          Issue Type: Sub-task
>          Components: Security
>            Reporter: Ajay Kumar
>            Assignee: Ajay Kumar
>            Priority: Major
>         Attachments: HDDS-804-HDDS-4.00.patch, HDDS-804-HDDS-4.01.patch, 
> HDDS-804-HDDS-4.02.patch, HDDS-804-HDDS-4.03.patch
>
>
> Add secret manager to process block tokens in OzoneManager.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to