Attaching a patch. I verified OIV output the information of Delegation Tokens with the patch locally. Here is the output:

<SecretManagerSection>
<currentId>2</currentId><tokenSequenceNumber>1</tokenSequenceNumber><DelegationKey><Id>1</Id><ExpiryDate>1398857460505</ExpiryDate></DelegationKey>
<DelegationKey><Id>2</Id><ExpiryDate>1398943860505</ExpiryDate></DelegationKey>
<PersistToken><Version>0</Version><Owner>aajisaka</Owner><Renewer>JobTracker</Renewer><RealUser></RealUser><IssueDate>1398857452426</IssueDate><MaxDate>1398857462426</MaxDate><SequenceNumber>1</SequenceNumber><MasterKeyId>2</MasterKeyId><ExpiryDate>1398857457426</ExpiryDate></PersistToken>
</SecretManagerSection>

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12642632/HDFS-6310.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

-1 tests included. The patch doesn't appear to include any new or modified tests.
Please justify why no new tests are needed for this patch.
Also please list what manual steps were performed to verify this patch.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 javadoc. There were no new javadoc warning messages.

+1 eclipse:eclipse. The patch built with eclipse:eclipse.

+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

+1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs.

+1 contrib tests. The patch passed contrib unit tests.

Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6775//testReport/
Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6775//console

This message is automatically generated.

Delegation tokens are intentionally left out in PBImageXmlWriter as they are sensitive information.

PBImageXmlWriter was designed to print all the information of fsimage, wasn't it? I think it's better to print the tokens because pre-2.4.0 OIV prints them.

Delegation tokens are intentionally left out [\....] as they are sensitive information.

The actual keys are excluded. If you think the rest contain sensitive information, please clarify.

I don't feel the concern with outputting the secret manger state is valid. If the user has access to the fsimage to run oiv, then they obviously can extract the state in other ways.

The oiv tool is useful in debugging the state of the fsimage. Selectively omitting some of the state impedes debugging. For instance, at least a year ago we discovered that expired tokens were never being purged from the fsimage. We had clusters whose fsimage contained 2+ years of tokens which added significant time to NN startup. The problem was evident with the use of oiv.

It concerns me that a documented tool (oiv), with external tools built around it, is being indiscriminately made incompatible within minor releases.

The actual keys are excluded. If you think the rest contain sensitive information, please clarify.

I don't feel the concern with outputting the secret manger state is valid. If the user has access to the fsimage to run oiv, then they obviously can extract the state in other ways. The oiv tool is useful in debugging the state of the fsimage. Selectively omitting some of the state impedes debugging.

As long as the key is out this should be fine. What I don't want is that an attacker can print out the token using oiv and then use the token directly, which might give an attacker a handy way to attack the system.

It concerns me that a documented tool (oiv), with external tools built around it, is being indiscriminately made incompatible within minor releases.

I guess we might need to clarify what compatibility means in this context. The format of the XML closely have been closely matching the internal layout of the fsimage since at least 2.2. The oiv of PB-based fsimage follows this tradition.

As long as the key is out this should be fine. What I don't want is that an attacker can print out the token using oiv and then use the token directly, which might give an attacker a handy way to attack the system.

If the attacker has access to the image, it's already game over whether oiv accurately dumps the image or not. They can extract the tokens and keys in other ways so why impede legitimate debugging?

I guess we might need to clarify what compatibility means in this context.

My incompatible concern isn't strictly related to this jira so we probably shouldn't debate it here. Just an explanation: It's a general concern that any existing tools built around the output are being broken. Perhaps this is fine for a major release, but within minor releases I'm not so sure.

Examples: Is the official documentation for using pig still valid? Does twitter's tool still work?

http://hadoop.apache.org/docs/r2.4.0/hadoop-project-dist/hadoop-hdfs/HdfsImageViewer.html
https://github.com/twitter/hdfs-du

If the attacker has access to the image, it's already game over whether oiv accurately dumps the image or not.

I agree with you.
Haohui Mai, what do you think? If you agree with that, could you review the patch?

The patch artifact directory has been removed!
This is a fatal error for test-patch.sh. Aborting.
Jenkins (node H4) information at https://builds.apache.org/job/PreCommit-HDFS-Build/10729/ may provide some hints.

-1 overall

This message was automatically generated.

-1 overall

This message was automatically generated.