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

Uma Maheswara Rao G commented on HADOOP-10735:
----------------------------------------------

Thanks a lot Yi for update on patch! Along with Colin comments, I have few 
comments on new patch!

{code}
 private static List<Class<? extends CryptoCodec>> getCodecClasses(
+      Configuration conf) {
+    List<Class<? extends CryptoCodec>> result = Lists.newArrayList();
+    String codecString = conf.get(HADOOP_SECURITY_CRYPTO_CODEC_CLASS_KEY);
+    if (codecString != null) {
+      for (String c : Splitter.on(',').trimResults().omitEmptyStrings().
+          split(codecString)) {
+        try {
+          Class<?> cls = conf.getClassByName(c);
+          result.add(cls.asSubclass(CryptoCodec.class));
+        } catch (ClassCastException e) {
+          throw new IllegalArgumentException("Class " + c + 
+              " is not a CryptoCodec.", e);
+        } catch (ClassNotFoundException e) {
+          throw new IllegalArgumentException("Crypto codec " + c + 
+              " not found.", e);
+        }
+      }
+    }
+    if (result.isEmpty()) {
+      result.add(OpensslAesCtrCryptoCodec.class);
+      result.add(JceAesCtrCryptoCodec.class);
+    }
+    
+    return result;
   }
{code}
Can't we use getClasses from config instead? atleast this peice of code should 
be qualified as util method.

It is good that you already added config doc regarding to order.

Why cann't it be HadoopIllegalArgEception instead of IAException?

{code}
  if (result.isEmpty()) {
+      result.add(OpensslAesCtrCryptoCodec.class);
+      result.add(JceAesCtrCryptoCodec.class);
+    }
{code}
We could return much earlier this...If no configuration items found, we could 
add and return.


> Fall back AesCtrCryptoCodec implementation from OpenSSL to JCE if non native 
> support.
> -------------------------------------------------------------------------------------
>
>                 Key: HADOOP-10735
>                 URL: https://issues.apache.org/jira/browse/HADOOP-10735
>             Project: Hadoop Common
>          Issue Type: Sub-task
>          Components: security
>    Affects Versions: fs-encryption (HADOOP-10150 and HDFS-6134)
>            Reporter: Yi Liu
>            Assignee: Yi Liu
>             Fix For: fs-encryption (HADOOP-10150 and HDFS-6134)
>
>         Attachments: HADOOP-10735.001.patch, HADOOP-10735.002.patch, 
> HADOOP-10735.003.patch, HADOOP-10735.004.patch
>
>
> If there is no native support or OpenSSL version is too low not supporting 
> AES-CTR, but {{OpensslAesCtrCryptoCodec}} is configured, we need to fall back 
> it to JCE implementation.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Reply via email to