[
https://issues.apache.org/jira/browse/HDDS-804?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16700848#comment-16700848
]
Ajay Kumar commented on HDDS-804:
---------------------------------
[~xyao], [~linyiqun] Thanks for comments:
{quote}Line 54: can we pass the LOG from subclass instead of keeping all under
the base abstract class?{quote}
Do you mean pass the logger as dependency in constructor or move all the
logging to child classes?
{quote}Line 89: should we use securityConfig instead of conf here?{quote}
Done
{quote}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.{quote}
TokenIndentifier doesn't have any getKeyId method. Alternatively we can keep
the createPassword same as this method is for generating the hash for given
identifier using current key and update retrieve password if required? (May be
try for other available keys if verification for first key fails, assuming
there are no more than couple of keys at given point of time? )
{quote}Line 173: can we guard the log with if (LOG.isDebugEnabled())){quote}
Done
{quote}Line 304: should we update the currentKeyId after loading all the master
key?{quote}
Done
{quote}Line 345: should this be super.startThreads()? Or rename the base class
methods?{quote}
There is no startThreads method in any of our SecretManager classes now.
{quote}Line 388: set store = null after close.{quote}
store is final variable so can't reassign.
{quote}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.{quote}
Done
{quote}Line 48-50: can we use the KIND_NAME defined in
OzoneBlockTokenIdentifier class?{quote}
Done
{quote}Line 79: the comment needs to be fixed.{quote}
Done
{quote}Line 163: NIT: the description does not match the code.{quote}
Fixed
{quote}Line 178: NIT: add an extra empty line here{quote}
Done
{quote}Can we use a consistent way for token expiration?{quote}
Changed all calls to {{Time.monotonicNow()}}.
{quote}Also we will print an incorrect date by
Time.formatTime(Time.monotonicNow()).{quote}
I might be missing something here but i think this will work as monotonicNow
returns time in millis.
> 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, HDDS-804-HDDS-4.04.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]
