Andrew Wang
added a comment - 17/Jul/14 21:39 Hi Tucu, thanks for working on this, I took a look and had some review comments:
ProxyUsers#refreshSUGC, we should init the new sip before assigning it to the volatile variable. This way it's not visible before it's ready.
DefaultImpersonationProvider:
Good time to add some comments about the regexes, namely example matches
Would be good to Precondition check that init has been called wherever we use configPrefix or related
Some basic checking of configPrefix in init as well, e.g. not empty, not null
ImpersonationProvider needs class annotations. Just a reminder, if this is a public interface, adding a new method is incompatible. I do see it in branch-2 already.

Andrew Wang
added a comment - 17/Jul/14 23:46 Didn't see these handled:
Would be good to Precondition check that init has been called wherever we use configPrefix or related
Some basic checking of configPrefix in init as well, e.g. not empty, not null
+1 pending these

Alejandro Abdelnur
added a comment - 18/Jul/14 06:09 new patch addressing one of the comments i've missed in previous patch.
Would be good to Precondition check that init has been called wherever we use configPrefix or related
The ProxyUsers initialization logic ensures this happens.
—
This is belongs to a string of patches built on top of each other:
HADOOP-10817 proxyuser logic supporting custom prefix properties
HADOOP-10799 HTTP delegation token built in client/server authentication classes
HADOOP-10800 HttpFS using HADOOP-10799 and removing custom code
HADOOP-10835 HTTP proxyuser logic built in client/server authentication classes
HADOOP-10836 HttpFS using HADOOP-10835 and removing custom code
HADOOP-10770 KMS delegation support using HADOOP-10799
HADOOP-10698 KMS proxyuser support using HADOOP-10835