Uploaded image for project: 'Hadoop YARN'
  1. Hadoop YARN
  2. YARN-6457

Allow custom SSL configuration to be supplied in WebApps

    Details

      Description

      Currently a custom SSL store cannot be passed on to WebApps which forces the embedded web-server to use the default keystore set up in ssl-server.xml for the whole Hadoop cluster. There are cases where the Hadoop app needs to use its own/custom keystore.

      1. YARN-6457.01.patch
        2 kB
        Sanjay M Pujare
      2. YARN-6457.00.patch
        2 kB
        Haibo Chen

        Issue Links

          Activity

          Hide
          sanjaypujare Sanjay M Pujare added a comment -

          Discussed this enhancement with Keys Botzum and Bill ODonnell

          Show
          sanjaypujare Sanjay M Pujare added a comment - Discussed this enhancement with Keys Botzum and Bill ODonnell
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user sanjaypujare opened a pull request:

          https://github.com/apache/hadoop/pull/213

          YARN-6457 use existing conf object as sslConf object in WebApps for the builder to use for the HttpServer2

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/sanjaypujare/hadoop YARN-6457.sanjay

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/hadoop/pull/213.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #213


          commit daa1fc2cc42617b362cae17c886d706d48a0b84f
          Author: DTAdmin <sanjay@datatorrent.com>
          Date: 2017-04-09T23:19:30Z

          YARN-6457 use existing conf object as sslConf object in WebApps for the builder to use for the HttpServer2


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user sanjaypujare opened a pull request: https://github.com/apache/hadoop/pull/213 YARN-6457 use existing conf object as sslConf object in WebApps for the builder to use for the HttpServer2 You can merge this pull request into a Git repository by running: $ git pull https://github.com/sanjaypujare/hadoop YARN-6457 .sanjay Alternatively you can review and apply these changes as the patch at: https://github.com/apache/hadoop/pull/213.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #213 commit daa1fc2cc42617b362cae17c886d706d48a0b84f Author: DTAdmin <sanjay@datatorrent.com> Date: 2017-04-09T23:19:30Z YARN-6457 use existing conf object as sslConf object in WebApps for the builder to use for the HttpServer2
          Hide
          haibochen Haibo Chen added a comment -

          YARN-4562 allows custom keystore configuration by passing the config object to WebAppUtils.loadSslConfiguration(). If you mark ssl.server.keystore.location as final, that should prevent the default ssl-server.xml to override your settings.

          Show
          haibochen Haibo Chen added a comment - YARN-4562 allows custom keystore configuration by passing the config object to WebAppUtils.loadSslConfiguration(). If you mark ssl.server.keystore.location as final, that should prevent the default ssl-server.xml to override your settings.
          Hide
          sanjaypujare Sanjay M Pujare added a comment -

          Haibo Chen thanks. Couple of comments:

          • are you sure the "final" works the way you have described? The default ssl-server.xml could mark its property final in which case my value in my config object won't take effect (which is the impression I get when I see "Administrators typically define parameters as final in core-site.xml for values that user applications may not alter." at https://hadoop.apache.org/docs/r2.7.2/api/org/apache/hadoop/conf/Configuration.html
          • the fix for YARN-4562 you mentioned seems to be in 2.9.0, 3.0.0-alpha1 whereas I need it in 2.7.1 or even 2.6.0. Can we incorporate the fix in these earlier releases?
          Show
          sanjaypujare Sanjay M Pujare added a comment - Haibo Chen thanks. Couple of comments: are you sure the "final" works the way you have described? The default ssl-server.xml could mark its property final in which case my value in my config object won't take effect (which is the impression I get when I see "Administrators typically define parameters as final in core-site.xml for values that user applications may not alter." at https://hadoop.apache.org/docs/r2.7.2/api/org/apache/hadoop/conf/Configuration.html the fix for YARN-4562 you mentioned seems to be in 2.9.0, 3.0.0-alpha1 whereas I need it in 2.7.1 or even 2.6.0. Can we incorporate the fix in these earlier releases?
          Hide
          haibochen Haibo Chen added a comment -

          You are right. Looks like final is only available for use by administrators. Sorry for the misinformation.
          I don't think this fix can go into 2.7.1 or 2.6.0 release. We can backport YARN-4562 to 2.7 and fix this issue on top of that. Then you can get it only if you upgrade to a later maintenance release. Do you want to create a patch against trunk?

          Show
          haibochen Haibo Chen added a comment - You are right. Looks like final is only available for use by administrators. Sorry for the misinformation. I don't think this fix can go into 2.7.1 or 2.6.0 release. We can backport YARN-4562 to 2.7 and fix this issue on top of that. Then you can get it only if you upgrade to a later maintenance release. Do you want to create a patch against trunk?
          Hide
          sanjaypujare Sanjay M Pujare added a comment -

          I can create the PR against master (if that's what you mean by "creating a patch against trunk"). Or could you point me to how to create this patch (contributor guidelines probably?)

          Show
          sanjaypujare Sanjay M Pujare added a comment - I can create the PR against master (if that's what you mean by "creating a patch against trunk"). Or could you point me to how to create this patch (contributor guidelines probably?)
          Hide
          haibochen Haibo Chen added a comment -

          The master branch is 'trunk', so you can just create a PR against it. The traditional workflow can be found at https://wiki.apache.org/hadoop/HowToContribute

          Show
          haibochen Haibo Chen added a comment - The master branch is 'trunk', so you can just create a PR against it. The traditional workflow can be found at https://wiki.apache.org/hadoop/HowToContribute
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user sanjaypujare opened a pull request:

          https://github.com/apache/hadoop/pull/216

          YARN-6457 use existing conf object as sslConf object in WebApps for the builder to use for the HttpServer2

          Merged from 2.7.1 branch

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/sanjaypujare/hadoop YARN-6457.trunk.sanjay

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/hadoop/pull/216.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #216


          commit 8218dab40b96984a383aa4ad9fa6ae2b053cff4a
          Author: Sanjay Pujare <sanjay@node24.morado.com>
          Date: 2017-04-21T23:00:13Z

          YARN-6457 use existing conf object as sslConf object in WebApps for the builder to use for the HttpServer2


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user sanjaypujare opened a pull request: https://github.com/apache/hadoop/pull/216 YARN-6457 use existing conf object as sslConf object in WebApps for the builder to use for the HttpServer2 Merged from 2.7.1 branch You can merge this pull request into a Git repository by running: $ git pull https://github.com/sanjaypujare/hadoop YARN-6457 .trunk.sanjay Alternatively you can review and apply these changes as the patch at: https://github.com/apache/hadoop/pull/216.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #216 commit 8218dab40b96984a383aa4ad9fa6ae2b053cff4a Author: Sanjay Pujare <sanjay@node24.morado.com> Date: 2017-04-21T23:00:13Z YARN-6457 use existing conf object as sslConf object in WebApps for the builder to use for the HttpServer2
          Hide
          sanjaypujare Sanjay M Pujare added a comment -

          Haibo Chen I have the new PR https://github.com/apache/hadoop/pull/216 . Pls let me know, thanks!

          Show
          sanjaypujare Sanjay M Pujare added a comment - Haibo Chen I have the new PR https://github.com/apache/hadoop/pull/216 . Pls let me know, thanks!
          Hide
          sanjaypujare Sanjay M Pujare added a comment -

          Haibo Chen just a reminder - do you see any issues with the PR? Is there anything you want me to do or is it ready to be merged? Thanks

          Show
          sanjaypujare Sanjay M Pujare added a comment - Haibo Chen just a reminder - do you see any issues with the PR? Is there anything you want me to do or is it ready to be merged? Thanks
          Hide
          haibochen Haibo Chen added a comment -

          My apologies for being slow looking at this! With the change, If ssl.server.keystore.location is set by users, ssl-server.xml will no longer be loaded.
          This is an issue if cluster admins have relied on the ssl-server.xml to force the ssl configurations (by making the properties final in the file).
          Now users can just work around that by specifying ssl.server.keystore.location. I wonder if we could do things similar in SSLFactory, that is,
          we allow users to configure 'hadoop.ssl.server.conf', which defaults to ssl-server.xml. Users can then specify the new configuration property and
          upload their own ssl-server.xml file to distributed cache. If cluster admins wants to force ssl configurations, they can make hadoop.ssl.server.conf
          final in yarn-site.xml. Does that work for your use case?

          Show
          haibochen Haibo Chen added a comment - My apologies for being slow looking at this! With the change, If ssl.server.keystore.location is set by users, ssl-server.xml will no longer be loaded. This is an issue if cluster admins have relied on the ssl-server.xml to force the ssl configurations (by making the properties final in the file). Now users can just work around that by specifying ssl.server.keystore.location. I wonder if we could do things similar in SSLFactory, that is, we allow users to configure 'hadoop.ssl.server.conf', which defaults to ssl-server.xml. Users can then specify the new configuration property and upload their own ssl-server.xml file to distributed cache. If cluster admins wants to force ssl configurations, they can make hadoop.ssl.server.conf final in yarn-site.xml. Does that work for your use case?
          Hide
          sanjaypujare Sanjay M Pujare added a comment -

          Haibo Chen I understand the issue you have raised but I see couple of problems with your suggestion:

          • in the current code in WebAppUtils.java in the function loadSslConfiguration(HttpServer2.Builder, Configuration) it doesn't get the value of hadoop.ssl.server.conf key but the default value YarnConfiguration.YARN_SSL_SERVER_RESOURCE_DEFAULT (i.e. ssl-server.xml) is hardcoded in the loadResource call. Unless you are proposing fixing this, your suggestion won't work
          • the Hadoop app (in our case) reads the same set of config files as the other Hadoop components so it is going read the yarn-site.xml file and use the same value of hadoop.ssl.server.conf but of course the app can get the value from somewhere else and override it in the Confguration object before passing it to WebApps builder. But in that case doesn't it defeat the purpose of marking it final in yarn-site.xml?

          Also we have coded and tested our fix against the change in the PR so we would like to go ahead with this fix (assuming it passes all the reviews)

          Show
          sanjaypujare Sanjay M Pujare added a comment - Haibo Chen I understand the issue you have raised but I see couple of problems with your suggestion: in the current code in WebAppUtils.java in the function loadSslConfiguration(HttpServer2.Builder, Configuration) it doesn't get the value of hadoop.ssl.server.conf key but the default value YarnConfiguration.YARN_SSL_SERVER_RESOURCE_DEFAULT (i.e. ssl-server.xml) is hardcoded in the loadResource call. Unless you are proposing fixing this, your suggestion won't work the Hadoop app (in our case) reads the same set of config files as the other Hadoop components so it is going read the yarn-site.xml file and use the same value of hadoop.ssl.server.conf but of course the app can get the value from somewhere else and override it in the Confguration object before passing it to WebApps builder. But in that case doesn't it defeat the purpose of marking it final in yarn-site.xml? Also we have coded and tested our fix against the change in the PR so we would like to go ahead with this fix (assuming it passes all the reviews)
          Hide
          sanjaypujare Sanjay M Pujare added a comment -

          Haibo Chen thinking more about it, I can see how this can be made to work. I am thinking the following code in WebAppUtils.loadSslConfiguration(Builder, Configuration) will do the trick:

          if (sslConf == null)

          { sslConf = new Configuration(false); sslConf.addResource(YarnConfiguration.YARN_SSL_SERVER_RESOURCE_DEFAULT); }

          else {
          String customSslServerConf = sslConf.get("hadoop.ssl.server.conf"); // TODO define the string in YarnConfigruation or elsewhere
          if (customSslServerConf != null)

          { sslConf.addResource(new Path(customSslServerConf)); }

          else

          { sslConf.addResource(YarnConfiguration.YARN_SSL_SERVER_RESOURCE_DEFAULT); }

          }

          Let me know if this is what you had in mind.

          Show
          sanjaypujare Sanjay M Pujare added a comment - Haibo Chen thinking more about it, I can see how this can be made to work. I am thinking the following code in WebAppUtils.loadSslConfiguration(Builder, Configuration) will do the trick: if (sslConf == null) { sslConf = new Configuration(false); sslConf.addResource(YarnConfiguration.YARN_SSL_SERVER_RESOURCE_DEFAULT); } else { String customSslServerConf = sslConf.get("hadoop.ssl.server.conf"); // TODO define the string in YarnConfigruation or elsewhere if (customSslServerConf != null) { sslConf.addResource(new Path(customSslServerConf)); } else { sslConf.addResource(YarnConfiguration.YARN_SSL_SERVER_RESOURCE_DEFAULT); } } Let me know if this is what you had in mind.
          Hide
          haibochen Haibo Chen added a comment - - edited

          Yep. That is what I had in mind. I think the code can be simplified a bit

          
          

          if (sslConf == null)

          { sslConf = new Configuration(false); } boolean needsClientAuth = YarnConfiguration.YARN_SSL_CLIENT_HTTPS_NEED_AUTH_DEFAULT; String sslConfResource = conf.get("hadoop.ssl.server.conf", YarnConfiguration.YARN_SSL_SERVER_RESOURCE_DEFAULT); sslConf.addResource(sslConfResource); ... {code:java}
          Show
          haibochen Haibo Chen added a comment - - edited Yep. That is what I had in mind. I think the code can be simplified a bit if (sslConf == null) { sslConf = new Configuration(false); } boolean needsClientAuth = YarnConfiguration.YARN_SSL_CLIENT_HTTPS_NEED_AUTH_DEFAULT; String sslConfResource = conf.get("hadoop.ssl.server.conf", YarnConfiguration.YARN_SSL_SERVER_RESOURCE_DEFAULT); sslConf.addResource(sslConfResource); ... {code:java}
          Hide
          sanjaypujare Sanjay M Pujare added a comment - - edited

          Haibo Chen I would have suggested the same code you have but to my disappointment I noticed that Configuration.addResource(String) and Configuration.addResource(Path) behave differently and Configuration.addResource(String) didn't work for me and that is why I suggested my code. If there is no issue with backward compatibility I can modify your code as follows:

          if (sslConf == null)
          { sslConf = new Configuration(false); }
          boolean needsClientAuth = YarnConfiguration.YARN_SSL_CLIENT_HTTPS_NEED_AUTH_DEFAULT;
          String sslConfResource = conf.get("hadoop.ssl.server.conf", 
          YarnConfiguration.YARN_SSL_SERVER_RESOURCE_DEFAULT);
          sslConf.addResource(new Path(sslConfResource));
          

          Also, is it okay to use "hadoop.ssl.server.conf" as it is without defining it as a constant in YarnConfiguration?

          Show
          sanjaypujare Sanjay M Pujare added a comment - - edited Haibo Chen I would have suggested the same code you have but to my disappointment I noticed that Configuration.addResource(String) and Configuration.addResource(Path) behave differently and Configuration.addResource(String) didn't work for me and that is why I suggested my code. If there is no issue with backward compatibility I can modify your code as follows: if (sslConf == null ) { sslConf = new Configuration( false ); } boolean needsClientAuth = YarnConfiguration.YARN_SSL_CLIENT_HTTPS_NEED_AUTH_DEFAULT; String sslConfResource = conf.get( "hadoop.ssl.server.conf" , YarnConfiguration.YARN_SSL_SERVER_RESOURCE_DEFAULT); sslConf.addResource( new Path(sslConfResource)); Also, is it okay to use "hadoop.ssl.server.conf" as it is without defining it as a constant in YarnConfiguration?
          Hide
          PramodSSImmaneni Pramod Immaneni added a comment -

          If the method relies only on the configuration passed from outside, then the user could sidestep any final settings for the ssl configuration and pass only the custom ssl setting, since the user has total control on the construction of the configuration object. Instead, if the method were to apply the configuration passed in from outside as an addendum on top of the internal configuration object it is creating today then the setting can take effect. So what I am suggesting is this

           public static HttpServer2.Builder loadSslConfiguration(
                HttpServer2.Builder builder, Configuration conf) {
                Configuration sslConf = new Configuration(false);
                boolean needsClientAuth = YarnConfiguration.YARN_SSL_CLIENT_HTTPS_NEED_AUTH_DEFAULT;
                sslConf.addResource(YarnConfiguration.YARN_SSL_SERVER_RESOURCE_DEFAULT);
                sslConf.addResource(conf);
                ....
          
          Show
          PramodSSImmaneni Pramod Immaneni added a comment - If the method relies only on the configuration passed from outside, then the user could sidestep any final settings for the ssl configuration and pass only the custom ssl setting, since the user has total control on the construction of the configuration object. Instead, if the method were to apply the configuration passed in from outside as an addendum on top of the internal configuration object it is creating today then the setting can take effect. So what I am suggesting is this public static HttpServer2.Builder loadSslConfiguration( HttpServer2.Builder builder, Configuration conf) { Configuration sslConf = new Configuration( false ); boolean needsClientAuth = YarnConfiguration.YARN_SSL_CLIENT_HTTPS_NEED_AUTH_DEFAULT; sslConf.addResource(YarnConfiguration.YARN_SSL_SERVER_RESOURCE_DEFAULT); sslConf.addResource(conf); ....
          Hide
          sanjaypujare Sanjay M Pujare added a comment -

          I agree with Pramod Immaneni with a small change (check for conf being non-null)

          public static HttpServer2.Builder loadSslConfiguration(
                HttpServer2.Builder builder, Configuration conf) {
                Configuration sslConf = new Configuration(false);
                boolean needsClientAuth = YarnConfiguration.YARN_SSL_CLIENT_HTTPS_NEED_AUTH_DEFAULT;
                sslConf.addResource(YarnConfiguration.YARN_SSL_SERVER_RESOURCE_DEFAULT);
                if (conf != null) {
                      sslConf.addResource(conf);
                }
                ....
          
          Show
          sanjaypujare Sanjay M Pujare added a comment - I agree with Pramod Immaneni with a small change (check for conf being non-null) public static HttpServer2.Builder loadSslConfiguration( HttpServer2.Builder builder, Configuration conf) { Configuration sslConf = new Configuration( false ); boolean needsClientAuth = YarnConfiguration.YARN_SSL_CLIENT_HTTPS_NEED_AUTH_DEFAULT; sslConf.addResource(YarnConfiguration.YARN_SSL_SERVER_RESOURCE_DEFAULT); if (conf != null ) { sslConf.addResource(conf); } ....
          Hide
          sanjaypujare Sanjay M Pujare added a comment -

          Hi Haibo Chen, I have verified the latest fix suggested by Pramod Immaneni that it works. If you are okay, I will modify my PR and it can then be merged. Pls let me know

          Show
          sanjaypujare Sanjay M Pujare added a comment - Hi Haibo Chen , I have verified the latest fix suggested by Pramod Immaneni that it works. If you are okay, I will modify my PR and it can then be merged. Pls let me know
          Hide
          haibochen Haibo Chen added a comment -

          Pramod Immaneni, Sanjay M Pujare, the approach looks great to me! Just one nit that not directly related to this issue, but would be nice to fix. We can move 'boolean needsClientAuth..' statement right before the return statement where it is used. Please create a new PR, and I'll commit it.

          Show
          haibochen Haibo Chen added a comment - Pramod Immaneni , Sanjay M Pujare , the approach looks great to me! Just one nit that not directly related to this issue, but would be nice to fix. We can move 'boolean needsClientAuth..' statement right before the return statement where it is used. Please create a new PR, and I'll commit it.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user sanjaypujare commented on the issue:

          https://github.com/apache/hadoop/pull/216

          closed as another PR will replace this

          Show
          githubbot ASF GitHub Bot added a comment - Github user sanjaypujare commented on the issue: https://github.com/apache/hadoop/pull/216 closed as another PR will replace this
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user sanjaypujare closed the pull request at:

          https://github.com/apache/hadoop/pull/216

          Show
          githubbot ASF GitHub Bot added a comment - Github user sanjaypujare closed the pull request at: https://github.com/apache/hadoop/pull/216
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user sanjaypujare opened a pull request:

          https://github.com/apache/hadoop/pull/219

          YARN-6457 use existing conf object as a resource for sslConf object in WebApps for the builder to use in HttpServer2

          use the passed conf object as a resource in local sslConf so it can be overridden

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/sanjaypujare/hadoop YARN-6457.trunk.sanjay1

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/hadoop/pull/219.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #219



          Show
          githubbot ASF GitHub Bot added a comment - GitHub user sanjaypujare opened a pull request: https://github.com/apache/hadoop/pull/219 YARN-6457 use existing conf object as a resource for sslConf object in WebApps for the builder to use in HttpServer2 use the passed conf object as a resource in local sslConf so it can be overridden You can merge this pull request into a Git repository by running: $ git pull https://github.com/sanjaypujare/hadoop YARN-6457 .trunk.sanjay1 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/hadoop/pull/219.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #219
          Hide
          sanjaypujare Sanjay M Pujare added a comment -

          Haibo Chen thanks. I have a new PR https://github.com/apache/hadoop/pull/219 . Pls let me know if you have questions/comments. Thanks

          Show
          sanjaypujare Sanjay M Pujare added a comment - Haibo Chen thanks. I have a new PR https://github.com/apache/hadoop/pull/219 . Pls let me know if you have questions/comments. Thanks
          Hide
          haibochen Haibo Chen added a comment -

          Attaching a patch from the Pull Request to initiate pre-commit check on the change

          Show
          haibochen Haibo Chen added a comment - Attaching a patch from the Pull Request to initiate pre-commit check on the change
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 20s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s 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 mvninstall 15m 2s trunk passed
          +1 compile 0m 30s trunk passed
          +1 checkstyle 0m 23s trunk passed
          +1 mvnsite 0m 32s trunk passed
          +1 mvneclipse 0m 15s trunk passed
          -1 findbugs 1m 1s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common in trunk has 1 extant Findbugs warnings.
          +1 javadoc 0m 30s trunk passed
          +1 mvninstall 0m 26s the patch passed
          +1 compile 0m 25s the patch passed
          +1 javac 0m 25s the patch passed
          +1 checkstyle 0m 17s the patch passed
          +1 mvnsite 0m 28s the patch passed
          +1 mvneclipse 0m 13s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 3s the patch passed
          -1 javadoc 0m 27s hadoop-yarn-common in the patch failed.
          +1 unit 2m 24s hadoop-yarn-common in the patch passed.
          +1 asflicense 0m 18s The patch does not generate ASF License warnings.
          25m 57s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:14b5c93
          JIRA Issue YARN-6457
          GITHUB PR https://github.com/apache/hadoop/pull/219
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 0bb11fa2ee4a 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / cef2815
          Default Java 1.8.0_121
          findbugs v3.1.0-RC1
          findbugs https://builds.apache.org/job/PreCommit-YARN-Build/15857/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common-warnings.html
          javadoc https://builds.apache.org/job/PreCommit-YARN-Build/15857/artifact/patchprocess/patch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/15857/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/15857/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 20s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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 mvninstall 15m 2s trunk passed +1 compile 0m 30s trunk passed +1 checkstyle 0m 23s trunk passed +1 mvnsite 0m 32s trunk passed +1 mvneclipse 0m 15s trunk passed -1 findbugs 1m 1s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common in trunk has 1 extant Findbugs warnings. +1 javadoc 0m 30s trunk passed +1 mvninstall 0m 26s the patch passed +1 compile 0m 25s the patch passed +1 javac 0m 25s the patch passed +1 checkstyle 0m 17s the patch passed +1 mvnsite 0m 28s the patch passed +1 mvneclipse 0m 13s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 3s the patch passed -1 javadoc 0m 27s hadoop-yarn-common in the patch failed. +1 unit 2m 24s hadoop-yarn-common in the patch passed. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 25m 57s Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue YARN-6457 GITHUB PR https://github.com/apache/hadoop/pull/219 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 0bb11fa2ee4a 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / cef2815 Default Java 1.8.0_121 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-YARN-Build/15857/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common-warnings.html javadoc https://builds.apache.org/job/PreCommit-YARN-Build/15857/artifact/patchprocess/patch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/15857/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common Console output https://builds.apache.org/job/PreCommit-YARN-Build/15857/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          haibochen Haibo Chen added a comment -

          The findbugs warnings are not related. Mind fixing the javadoc issue? namely,
          1) "@param sslConf the Configuration instance to use during loading of SSL conf"
          Now that we have renamed sslConf to conf, we need to rename it in javadoc as well. The description is not accurate either. How about "the Configuration instance to load custom SSL settings from"?
          2) There is no @return in the method javadoc, let's add that.

          Show
          haibochen Haibo Chen added a comment - The findbugs warnings are not related. Mind fixing the javadoc issue? namely, 1) "@param sslConf the Configuration instance to use during loading of SSL conf" Now that we have renamed sslConf to conf, we need to rename it in javadoc as well. The description is not accurate either. How about "the Configuration instance to load custom SSL settings from"? 2) There is no @return in the method javadoc, let's add that.
          Hide
          haibochen Haibo Chen added a comment -

          If it is convenient for you, you can generate a patch (name it to YARN-6457.01.patch) and upload it here to kick off the precommit job.

          Show
          haibochen Haibo Chen added a comment - If it is convenient for you, you can generate a patch (name it to YARN-6457 .01.patch) and upload it here to kick off the precommit job.
          Hide
          sanjaypujare Sanjay M Pujare added a comment -

          Will do...

          Show
          sanjaypujare Sanjay M Pujare added a comment - Will do...
          Hide
          sanjaypujare Sanjay M Pujare added a comment -

          Attaching next patch from the Pull Request to initiate pre-commit check on the change

          Show
          sanjaypujare Sanjay M Pujare added a comment - Attaching next patch from the Pull Request to initiate pre-commit check on the change
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 24s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s 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 mvninstall 13m 42s trunk passed
          +1 compile 0m 29s trunk passed
          +1 checkstyle 0m 19s trunk passed
          +1 mvnsite 0m 30s trunk passed
          +1 mvneclipse 0m 15s trunk passed
          -1 findbugs 0m 58s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common in trunk has 1 extant Findbugs warnings.
          +1 javadoc 0m 29s trunk passed
          +1 mvninstall 0m 27s the patch passed
          +1 compile 0m 26s the patch passed
          +1 javac 0m 26s the patch passed
          -0 checkstyle 0m 16s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common: The patch generated 1 new + 11 unchanged - 0 fixed = 12 total (was 11)
          +1 mvnsite 0m 28s the patch passed
          +1 mvneclipse 0m 13s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 3s the patch passed
          +1 javadoc 0m 28s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common generated 0 new + 4574 unchanged - 1 fixed = 4574 total (was 4575)
          +1 unit 2m 22s hadoop-yarn-common in the patch passed.
          +1 asflicense 0m 20s The patch does not generate ASF License warnings.
          24m 34s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:14b5c93
          JIRA Issue YARN-6457
          GITHUB PR https://github.com/apache/hadoop/pull/219
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux e672c4556faa 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / cef2815
          Default Java 1.8.0_121
          findbugs v3.1.0-RC1
          findbugs https://builds.apache.org/job/PreCommit-YARN-Build/15862/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common-warnings.html
          checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/15862/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.txt
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/15862/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/15862/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 24s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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 mvninstall 13m 42s trunk passed +1 compile 0m 29s trunk passed +1 checkstyle 0m 19s trunk passed +1 mvnsite 0m 30s trunk passed +1 mvneclipse 0m 15s trunk passed -1 findbugs 0m 58s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common in trunk has 1 extant Findbugs warnings. +1 javadoc 0m 29s trunk passed +1 mvninstall 0m 27s the patch passed +1 compile 0m 26s the patch passed +1 javac 0m 26s the patch passed -0 checkstyle 0m 16s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common: The patch generated 1 new + 11 unchanged - 0 fixed = 12 total (was 11) +1 mvnsite 0m 28s the patch passed +1 mvneclipse 0m 13s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 3s the patch passed +1 javadoc 0m 28s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common generated 0 new + 4574 unchanged - 1 fixed = 4574 total (was 4575) +1 unit 2m 22s hadoop-yarn-common in the patch passed. +1 asflicense 0m 20s The patch does not generate ASF License warnings. 24m 34s Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue YARN-6457 GITHUB PR https://github.com/apache/hadoop/pull/219 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux e672c4556faa 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / cef2815 Default Java 1.8.0_121 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-YARN-Build/15862/artifact/patchprocess/branch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common-warnings.html checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/15862/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-common.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/15862/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common Console output https://builds.apache.org/job/PreCommit-YARN-Build/15862/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          sanjaypujare Sanjay M Pujare added a comment -

          Haibo Chen the Javadoc issue seems to have been fixed. Looks like we are good to go....

          Show
          sanjaypujare Sanjay M Pujare added a comment - Haibo Chen the Javadoc issue seems to have been fixed. Looks like we are good to go....
          Hide
          haibochen Haibo Chen added a comment -

          Patch LGTM+1, will fix the minor checkstyle issue when I commit.

          Show
          haibochen Haibo Chen added a comment - Patch LGTM+1, will fix the minor checkstyle issue when I commit.
          Hide
          haibochen Haibo Chen added a comment -

          Thanks Sanjay M Pujare for your contribution and Pramod Immaneni for the proposed solution! I committed the patch to branch-2 and trunk

          Show
          haibochen Haibo Chen added a comment - Thanks Sanjay M Pujare for your contribution and Pramod Immaneni for the proposed solution! I committed the patch to branch-2 and trunk
          Hide
          haibochen Haibo Chen added a comment -

          Sanjay M Pujare, can you go to YARN-4562 to request a backport of it, if you want the fix in 2.7? I can do a backport of this patch afterwards.

          Show
          haibochen Haibo Chen added a comment - Sanjay M Pujare , can you go to YARN-4562 to request a backport of it, if you want the fix in 2.7? I can do a backport of this patch afterwards.
          Hide
          sanjaypujare Sanjay M Pujare added a comment -

          Haibo Chen Will do (I was about to ask that... ).

          Show
          sanjaypujare Sanjay M Pujare added a comment - Haibo Chen Will do (I was about to ask that... ).
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11698 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11698/)
          YARN-6457. Allow custom SSL configuration to be supplied in WebApps. (haibochen: rev 1769b12a773dc6c83f13663a77da33fa78878730)

          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/webapp/util/WebAppUtils.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11698 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11698/ ) YARN-6457 . Allow custom SSL configuration to be supplied in WebApps. (haibochen: rev 1769b12a773dc6c83f13663a77da33fa78878730) (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/webapp/util/WebAppUtils.java
          Hide
          haibochen Haibo Chen added a comment -

          Sanjay M Pujare I have backported this to 2.7.4+ and 2.8.1+ as well

          Show
          haibochen Haibo Chen added a comment - Sanjay M Pujare I have backported this to 2.7.4+ and 2.8.1+ as well
          Hide
          ajisakaa Akira Ajisaka added a comment -

          Hi Haibo Chen, would you backport this to branch-2.8.1 as well?

          Show
          ajisakaa Akira Ajisaka added a comment - Hi Haibo Chen , would you backport this to branch-2.8.1 as well?
          Hide
          haibochen Haibo Chen added a comment -

          Will do. Did not realize that 2.8.1 has been cut. Thanks for the reminder!

          Show
          haibochen Haibo Chen added a comment - Will do. Did not realize that 2.8.1 has been cut. Thanks for the reminder!
          Hide
          vrozov Vlad Rozov added a comment -

          What is the reason that Yarn/WebApps do not use SSLFactory.SSL_SERVER_CONF_DEFAULT and other conf settings like SSLFactory.SSL_SERVER_CONF_KEY ("hadoop.ssl.server.conf"). According to SSLFactory java doc "it is used to configure HTTPS in Hadoop HTTP based endpoints, client and server" and instead hardcodes SSL conf to "ssl-server.xml"? In addition, in trunk, HttpServer2.Builder provides setSSLConf() method, that may simplify WebAppsUtils implementation.

          Show
          vrozov Vlad Rozov added a comment - What is the reason that Yarn/WebApps do not use SSLFactory.SSL_SERVER_CONF_DEFAULT and other conf settings like SSLFactory.SSL_SERVER_CONF_KEY ("hadoop.ssl.server.conf"). According to SSLFactory java doc "it is used to configure HTTPS in Hadoop HTTP based endpoints, client and server" and instead hardcodes SSL conf to "ssl-server.xml"? In addition, in trunk, HttpServer2.Builder provides setSSLConf() method, that may simplify WebAppsUtils implementation.
          Hide
          haibochen Haibo Chen added a comment -

          Thanks Vlad Rozov for pointing out SSLFactory. I have noticed that SSLFactory.SSL_SERVER_CONF_DEFAULT and the like have been duplicated, both in YARN and HDFS. I did not suggest it with the intent to keep this patch small. Can you file a jira so that we take a comprehensive look at fixing the duplication across all hadoop component?

          Show
          haibochen Haibo Chen added a comment - Thanks Vlad Rozov for pointing out SSLFactory. I have noticed that SSLFactory.SSL_SERVER_CONF_DEFAULT and the like have been duplicated, both in YARN and HDFS. I did not suggest it with the intent to keep this patch small. Can you file a jira so that we take a comprehensive look at fixing the duplication across all hadoop component?
          Hide
          vrozov Vlad Rozov added a comment -

          Haibo Chen Please see YARN-6579.

          Show
          vrozov Vlad Rozov added a comment - Haibo Chen Please see YARN-6579 .
          Hide
          vrozov Vlad Rozov added a comment -

          Another question. As far as I can see, all SSL configuration parameters may be set in yarn-site.xml or any other configuration files that Yarn reads by default and be overwritten (if not declared as final) by settings in ssl-server.xml. Is it desired behavior?

          Show
          vrozov Vlad Rozov added a comment - Another question. As far as I can see, all SSL configuration parameters may be set in yarn-site.xml or any other configuration files that Yarn reads by default and be overwritten (if not declared as final) by settings in ssl-server.xml. Is it desired behavior?
          Hide
          haibochen Haibo Chen added a comment -

          I think yes. Prior to this patch, all SSL configuration parameters need to be specified in ssl-server.xml, that is, ssl-server configurations are effectively final. There may be cluster setups that rely on that fact. Allowing custom SSL configuration should not alter that behavior

          Show
          haibochen Haibo Chen added a comment - I think yes. Prior to this patch, all SSL configuration parameters need to be specified in ssl-server.xml, that is, ssl-server configurations are effectively final. There may be cluster setups that rely on that fact. Allowing custom SSL configuration should not alter that behavior
          Hide
          vrozov Vlad Rozov added a comment -

          What if "ssl.server.truststore.location" is set in yarn-site.xml and is not specified in ssl-server.xml? Should the setting in yarn-site.xml still be in effect?

          Show
          vrozov Vlad Rozov added a comment - What if "ssl.server.truststore.location" is set in yarn-site.xml and is not specified in ssl-server.xml? Should the setting in yarn-site.xml still be in effect?
          Hide
          haibochen Haibo Chen added a comment -

          Yes. only configuration properties specified in ssl-server.xml and marked as final cannot be overridden

          Show
          haibochen Haibo Chen added a comment - Yes. only configuration properties specified in ssl-server.xml and marked as final cannot be overridden
          Hide
          vrozov Vlad Rozov added a comment -

          Never mind, I see it now. WebAppsUtils creates Configuration with loadDefaults set to false, so it will not load settings from default including yarn-site.xml.

          Show
          vrozov Vlad Rozov added a comment - Never mind, I see it now. WebAppsUtils creates Configuration with loadDefaults set to false, so it will not load settings from default including yarn-site.xml.
          Hide
          vinodkv Vinod Kumar Vavilapalli added a comment -

          2.8.1 became a security release. Moving fix-version to 2.8.2 after the fact.

          Show
          vinodkv Vinod Kumar Vavilapalli added a comment - 2.8.1 became a security release. Moving fix-version to 2.8.2 after the fact.
          Hide
          rkanter Robert Kanter added a comment -

          Vlad Rozov, Sanjay M Pujare we were doing some testing and found that this change breaks a setup with HDFS HA + SSL + Hadoop Credstore. In that setup, the RM will fail to startup with a stack trace like this:

          Error starting ResourceManager
          java.lang.IllegalArgumentException: java.net.UnknownHostException: ns1
          	at org.apache.hadoop.security.SecurityUtil.buildTokenService(SecurityUtil.java:444)
          	at org.apache.hadoop.hdfs.NameNodeProxiesClient.createProxyWithClientProtocol(NameNodeProxiesClient.java:132)
          	at org.apache.hadoop.hdfs.DFSClient.<init>(DFSClient.java:341)
          	at org.apache.hadoop.hdfs.DFSClient.<init>(DFSClient.java:285)
          	at org.apache.hadoop.hdfs.DistributedFileSystem.initialize(DistributedFileSystem.java:163)
          	at org.apache.hadoop.fs.FileSystem.createFileSystem(FileSystem.java:3258)
          	at org.apache.hadoop.fs.FileSystem.access$200(FileSystem.java:123)
          	at org.apache.hadoop.fs.FileSystem$Cache.getInternal(FileSystem.java:3307)
          	at org.apache.hadoop.fs.FileSystem$Cache.get(FileSystem.java:3275)
          	at org.apache.hadoop.fs.FileSystem.get(FileSystem.java:476)
          	at org.apache.hadoop.fs.FileSystem.get(FileSystem.java:467)
          	at org.apache.hadoop.fs.Path.getFileSystem(Path.java:361)
          	at org.apache.hadoop.security.alias.JavaKeyStoreProvider.initFileSystem(JavaKeyStoreProvider.java:89)
          	at org.apache.hadoop.security.alias.AbstractJavaKeyStoreProvider.<init>(AbstractJavaKeyStoreProvider.java:85)
          	at org.apache.hadoop.security.alias.JavaKeyStoreProvider.<init>(JavaKeyStoreProvider.java:49)
          	at org.apache.hadoop.security.alias.JavaKeyStoreProvider.<init>(JavaKeyStoreProvider.java:41)
          	at org.apache.hadoop.security.alias.JavaKeyStoreProvider$Factory.createProvider(JavaKeyStoreProvider.java:100)
          	at org.apache.hadoop.security.alias.CredentialProviderFactory.getProviders(CredentialProviderFactory.java:73)
          	at org.apache.hadoop.conf.Configuration.getPasswordFromCredentialProviders(Configuration.java:2157)
          	at org.apache.hadoop.conf.Configuration.getPassword(Configuration.java:2095)
          	at org.apache.hadoop.yarn.webapp.util.WebAppUtils.getPassword(WebAppUtils.java:431)
          	at org.apache.hadoop.yarn.webapp.util.WebAppUtils.loadSslConfiguration(WebAppUtils.java:409)
          	at org.apache.hadoop.yarn.webapp.WebApps$Builder.build(WebApps.java:312)
          	at org.apache.hadoop.yarn.webapp.WebApps$Builder.start(WebApps.java:401)
          	at org.apache.hadoop.yarn.server.resourcemanager.ResourceManager.startWepApp(ResourceManager.java:1119)
          	at org.apache.hadoop.yarn.server.resourcemanager.ResourceManager.serviceStart(ResourceManager.java:1229)
          	at org.apache.hadoop.service.AbstractService.start(AbstractService.java:194)
          	at org.apache.hadoop.yarn.server.resourcemanager.ResourceManager.main(ResourceManager.java:1425)
          Caused by: java.net.UnknownHostException: ns1
          	... 28 more
          

          I did some digging, and the problem is that with HDFS HA, we have a logical name (i.e. "ns1") instead of an actual hostname. So when the Credstore (i.e. Configuration.getPassword) tries to read the password, it needs to resolve the logical name into a hostname; however, that information is now missing because of this JIRA. If I change it so that we do new Configuration() instead of new Configuration(false), so we'll load hdfs-site (and others), and that fixes the problem.

          Why do we need to set loadDefaults to false?

          Show
          rkanter Robert Kanter added a comment - Vlad Rozov , Sanjay M Pujare we were doing some testing and found that this change breaks a setup with HDFS HA + SSL + Hadoop Credstore. In that setup, the RM will fail to startup with a stack trace like this: Error starting ResourceManager java.lang.IllegalArgumentException: java.net.UnknownHostException: ns1 at org.apache.hadoop.security.SecurityUtil.buildTokenService(SecurityUtil.java:444) at org.apache.hadoop.hdfs.NameNodeProxiesClient.createProxyWithClientProtocol(NameNodeProxiesClient.java:132) at org.apache.hadoop.hdfs.DFSClient.<init>(DFSClient.java:341) at org.apache.hadoop.hdfs.DFSClient.<init>(DFSClient.java:285) at org.apache.hadoop.hdfs.DistributedFileSystem.initialize(DistributedFileSystem.java:163) at org.apache.hadoop.fs.FileSystem.createFileSystem(FileSystem.java:3258) at org.apache.hadoop.fs.FileSystem.access$200(FileSystem.java:123) at org.apache.hadoop.fs.FileSystem$Cache.getInternal(FileSystem.java:3307) at org.apache.hadoop.fs.FileSystem$Cache.get(FileSystem.java:3275) at org.apache.hadoop.fs.FileSystem.get(FileSystem.java:476) at org.apache.hadoop.fs.FileSystem.get(FileSystem.java:467) at org.apache.hadoop.fs.Path.getFileSystem(Path.java:361) at org.apache.hadoop.security.alias.JavaKeyStoreProvider.initFileSystem(JavaKeyStoreProvider.java:89) at org.apache.hadoop.security.alias.AbstractJavaKeyStoreProvider.<init>(AbstractJavaKeyStoreProvider.java:85) at org.apache.hadoop.security.alias.JavaKeyStoreProvider.<init>(JavaKeyStoreProvider.java:49) at org.apache.hadoop.security.alias.JavaKeyStoreProvider.<init>(JavaKeyStoreProvider.java:41) at org.apache.hadoop.security.alias.JavaKeyStoreProvider$Factory.createProvider(JavaKeyStoreProvider.java:100) at org.apache.hadoop.security.alias.CredentialProviderFactory.getProviders(CredentialProviderFactory.java:73) at org.apache.hadoop.conf.Configuration.getPasswordFromCredentialProviders(Configuration.java:2157) at org.apache.hadoop.conf.Configuration.getPassword(Configuration.java:2095) at org.apache.hadoop.yarn.webapp.util.WebAppUtils.getPassword(WebAppUtils.java:431) at org.apache.hadoop.yarn.webapp.util.WebAppUtils.loadSslConfiguration(WebAppUtils.java:409) at org.apache.hadoop.yarn.webapp.WebApps$Builder.build(WebApps.java:312) at org.apache.hadoop.yarn.webapp.WebApps$Builder.start(WebApps.java:401) at org.apache.hadoop.yarn.server.resourcemanager.ResourceManager.startWepApp(ResourceManager.java:1119) at org.apache.hadoop.yarn.server.resourcemanager.ResourceManager.serviceStart(ResourceManager.java:1229) at org.apache.hadoop.service.AbstractService.start(AbstractService.java:194) at org.apache.hadoop.yarn.server.resourcemanager.ResourceManager.main(ResourceManager.java:1425) Caused by: java.net.UnknownHostException: ns1 ... 28 more I did some digging, and the problem is that with HDFS HA, we have a logical name (i.e. "ns1") instead of an actual hostname. So when the Credstore (i.e. Configuration.getPassword ) tries to read the password, it needs to resolve the logical name into a hostname; however, that information is now missing because of this JIRA. If I change it so that we do new Configuration() instead of new Configuration(false) , so we'll load hdfs-site (and others), and that fixes the problem. Why do we need to set loadDefaults to false ?
          Hide
          vrozov Vlad Rozov added a comment - - edited

          Robert Kanter I don't see how this JIRA affects your use case, was not loadDefaults set to false prior to the patch as well? In case you plan to change loadDefaults, please see my prior comments regarding "ssl.server.truststore.location"

          Show
          vrozov Vlad Rozov added a comment - - edited Robert Kanter I don't see how this JIRA affects your use case, was not loadDefaults set to false prior to the patch as well? In case you plan to change loadDefaults , please see my prior comments regarding "ssl.server.truststore.location"
          Hide
          rkanter Robert Kanter added a comment -

          was not loadDefaults set to false prior to the patch as well?

          It was. However, it only called that code before when the passed in Configuration was null. Now it always does it.

          if (sslConf == null) {		
             sslConf = new Configuration(false);
          }
          

          vs

          Configuration sslConf = new Configuration(false);
          

          The passed in config in the code path I'm interested in is not null, so it actually did not create the new Configuration in the original version.

          In any case, I tried using HDFS HA + SSL + Hadoop Credstore after reverting YARN-6457 (so the original code was used), and everything works fine. So this JIRA definitely affects this use case.

          In case you plan to change loadDefaults, please see my prior comments regarding "ssl.server.truststore.location"

          Could you please clarify? I looked back at the earlier comments and I'm still not understanding the issue.

          Show
          rkanter Robert Kanter added a comment - was not loadDefaults set to false prior to the patch as well? It was. However, it only called that code before when the passed in Configuration was null . Now it always does it. if (sslConf == null ) { sslConf = new Configuration( false ); } vs Configuration sslConf = new Configuration( false ); The passed in config in the code path I'm interested in is not null , so it actually did not create the new Configuration in the original version. In any case, I tried using HDFS HA + SSL + Hadoop Credstore after reverting YARN-6457 (so the original code was used), and everything works fine. So this JIRA definitely affects this use case. In case you plan to change loadDefaults, please see my prior comments regarding "ssl.server.truststore.location" Could you please clarify? I looked back at the earlier comments and I'm still not understanding the issue.
          Hide
          vrozov Vlad Rozov added a comment -

          If passed conf is not null, should not the following code handle your case?

          if (conf != null) {
            sslConf.addResource(conf);
          }
          

          How HDFS HA + SSL + Hadoop credential store worked prior to YARN-4562 was fixed?

          The issue is that prior to this and YARN-4562 fix, "ssl.server.truststore.location" and other properties that are specific to ssl-server.xml were ignored if set in yarn-site.xml and only loaded from ssl-server.xml. Whether it was an intentional behavior or a bug, needs to be discussed. The behavior should not change simply as a side effect of this JIRA fix.

          Show
          vrozov Vlad Rozov added a comment - If passed conf is not null , should not the following code handle your case? if (conf != null ) { sslConf.addResource(conf); } How HDFS HA + SSL + Hadoop credential store worked prior to YARN-4562 was fixed? The issue is that prior to this and YARN-4562 fix, "ssl.server.truststore.location" and other properties that are specific to ssl-server.xml were ignored if set in yarn-site.xml and only loaded from ssl-server.xml . Whether it was an intentional behavior or a bug, needs to be discussed. The behavior should not change simply as a side effect of this JIRA fix.
          Hide
          rkanter Robert Kanter added a comment -

          That didn't seem to be handling it. Let me try it again and see what I find.

          Also, we're setting all ssl.* properties in ssl-server.xml.

          Show
          rkanter Robert Kanter added a comment - That didn't seem to be handling it. Let me try it again and see what I find. Also, we're setting all ssl.* properties in ssl-server.xml.
          Hide
          vrozov Vlad Rozov added a comment -

          Also, we're setting all ssl.* properties in ssl-server.xml.

          My point is that the behavior can't be changed implicitly, it needs to be documented (in JIRA) and changed explicitly if necessary.

          Show
          vrozov Vlad Rozov added a comment - Also, we're setting all ssl.* properties in ssl-server.xml. My point is that the behavior can't be changed implicitly, it needs to be documented (in JIRA) and changed explicitly if necessary.
          Hide
          rkanter Robert Kanter added a comment -

          Sorry for raising the alarm - we're still looking into this, but its looking like a configuration issue on our end, so I think we're fine here.

          Show
          rkanter Robert Kanter added a comment - Sorry for raising the alarm - we're still looking into this, but its looking like a configuration issue on our end, so I think we're fine here.

            People

            • Assignee:
              sanjaypujare Sanjay M Pujare
              Reporter:
              sanjaypujare Sanjay M Pujare
            • Votes:
              0 Vote for this issue
              Watchers:
              11 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 96h
                96h
                Remaining:
                Remaining Estimate - 96h
                96h
                Logged:
                Time Spent - Not Specified
                Not Specified

                  Development