Uploaded image for project: 'Flink'
  1. Flink
  2. FLINK-5839 Flink Security problem collection
  3. FLINK-6117

'zookeeper.sasl.disable' not takes effet when starting CuratorFramework

    Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.2.0
    • Fix Version/s: 1.3.0
    • Component/s: Client, JobManager
    • Labels:
    • Environment:

      Ubuntu, non-secured

    • Flags:
      Patch

      Description

      The value of 'zookeeper.sasl.disable' not used in the right way when starting CuratorFramework.

      Here are all the settings relevant to high-availability in my flink-conf.yaml:

      high-availability: zookeeper
      high-availability.zookeeper.quorum: localhost:2181
      high-availability.zookeeper.storageDir: hdfs:///flink/ha/

      Obviously, no explicit value is set for 'zookeeper.sasl.disable' so default value of 'true'(ConfigConstants.DEFAULT_ZOOKEEPER_SASL_DISABLE) would be applied. But when FlinkYarnSessionCli & FlinkApplicationMasterRunner start,
      both logs show that they attempt connecting to zookeeper in 'SASL' mode.

      logs are like this:

      2017-03-18 23:53:10,498 INFO org.apache.zookeeper.ZooKeeper - Initiating client connection, connectString=localhost:2181 sessionTimeout=60000 watcher=org.apache.flink.shaded.org.apache.curator.ConnectionState@5949eba8
      2017-03-18 23:53:10,498 INFO org.apache.zookeeper.ZooKeeper - Initiating client connection, connectString=localhost:2181 sessionTimeout=60000 watcher=org.apache.flink.shaded.org.apache.curator.ConnectionState@5949eba8
      2017-03-18 23:53:10,522 WARN org.apache.zookeeper.ClientCnxn - SASL configuration failed: javax.security.auth.login.LoginException: No JAAS configuration section named 'Client' was found in specified JAAS configuration file: '/tmp/jaas-3047036396963510842.conf'. Will continue connection to Zookeeper server without SASL authentication, if Zookeeper server allows it.
      2017-03-18 23:53:10,522 WARN org.apache.zookeeper.ClientCnxn - SASL configuration failed: javax.security.auth.login.LoginException: No JAAS configuration section named 'Client' was found in specified JAAS configuration file: '/tmp/jaas-3047036396963510842.conf'. Will continue connection to Zookeeper server without SASL authentication, if Zookeeper server allows it.
      2017-03-18 23:53:10,530 INFO org.apache.zookeeper.ClientCnxn - Opening socket connection to server localhost/127.0.0.1:2181
      2017-03-18 23:53:10,530 INFO org.apache.zookeeper.ClientCnxn - Opening socket connection to server localhost/127.0.0.1:2181
      2017-03-18 23:53:10,534 ERROR org.apache.flink.shaded.org.apache.curator.ConnectionState - Authentication failed

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user gaqzcb opened a pull request:

          https://github.com/apache/flink/pull/3566

          FLINK-6117makes setting of 'zookeeper.sasl.disable' work correctly

          Thanks for contributing to Apache Flink. Before you open your pull request, please take the following check list into consideration.
          If your changes take all of the items into account, feel free to open your pull request. For more information and/or questions please refer to the [How To Contribute guide](http://flink.apache.org/how-to-contribute.html).
          In addition to going through the list, please provide a meaningful description of your changes.

          • [ ] General
          • The pull request references the related JIRA issue ("[FLINK-XXX] Jira title text")
          • The pull request addresses only one issue
          • Each commit in the PR has a meaningful commit message (including the JIRA id)
          • [ ] Documentation
          • Documentation has been added for new functionality
          • Old documentation affected by the pull request has been updated
          • JavaDoc for public methods has been added
          • [ ] Tests & Build
          • Functionality added by the pull request is covered by tests
          • `mvn clean verify` has been executed successfully locally or a Travis build has passed

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

          $ git pull https://github.com/gaqzcb/flink-1 FLINK-6117

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

          https://github.com/apache/flink/pull/3566.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 #3566


          commit d6ff39b39c4cd4eda319fb1e169528b4c5613c3f
          Author: zcb <2056268568@qq.com>
          Date: 2017-03-19T08:31:34Z

          makes setting of 'zookeeper.sasl.disable' work correctly


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user gaqzcb opened a pull request: https://github.com/apache/flink/pull/3566 FLINK-6117 makes setting of 'zookeeper.sasl.disable' work correctly Thanks for contributing to Apache Flink. Before you open your pull request, please take the following check list into consideration. If your changes take all of the items into account, feel free to open your pull request. For more information and/or questions please refer to the [How To Contribute guide] ( http://flink.apache.org/how-to-contribute.html ). In addition to going through the list, please provide a meaningful description of your changes. [ ] General The pull request references the related JIRA issue (" [FLINK-XXX] Jira title text") The pull request addresses only one issue Each commit in the PR has a meaningful commit message (including the JIRA id) [ ] Documentation Documentation has been added for new functionality Old documentation affected by the pull request has been updated JavaDoc for public methods has been added [ ] Tests & Build Functionality added by the pull request is covered by tests `mvn clean verify` has been executed successfully locally or a Travis build has passed You can merge this pull request into a Git repository by running: $ git pull https://github.com/gaqzcb/flink-1 FLINK-6117 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/3566.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 #3566 commit d6ff39b39c4cd4eda319fb1e169528b4c5613c3f Author: zcb <2056268568@qq.com> Date: 2017-03-19T08:31:34Z makes setting of 'zookeeper.sasl.disable' work correctly
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user vijikarthi commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3566#discussion_r106829135

          — Diff: flink-runtime/src/main/java/org/apache/flink/runtime/util/ZooKeeperUtils.java —
          @@ -89,6 +90,7 @@ public static CuratorFramework startCuratorFramework(Configuration configuration

          boolean disableSaslClient = configuration.getBoolean(ConfigConstants.ZOOKEEPER_SASL_DISABLE,
          ConfigConstants.DEFAULT_ZOOKEEPER_SASL_DISABLE);
          + System.setProperty(ZooKeeperSaslClient.ENABLE_CLIENT_SASL_KEY, String.valueOf(!disableSaslClient));
          — End diff –

          Please move this logic to `ZooKeeperModule` class https://github.com/apache/flink/blob/master/flink-runtime/src/main/java/org/apache/flink/runtime/security/modules/ZooKeeperModule.java#L49

          Show
          githubbot ASF GitHub Bot added a comment - Github user vijikarthi commented on a diff in the pull request: https://github.com/apache/flink/pull/3566#discussion_r106829135 — Diff: flink-runtime/src/main/java/org/apache/flink/runtime/util/ZooKeeperUtils.java — @@ -89,6 +90,7 @@ public static CuratorFramework startCuratorFramework(Configuration configuration boolean disableSaslClient = configuration.getBoolean(ConfigConstants.ZOOKEEPER_SASL_DISABLE, ConfigConstants.DEFAULT_ZOOKEEPER_SASL_DISABLE); + System.setProperty(ZooKeeperSaslClient.ENABLE_CLIENT_SASL_KEY, String.valueOf(!disableSaslClient)); — End diff – Please move this logic to `ZooKeeperModule` class https://github.com/apache/flink/blob/master/flink-runtime/src/main/java/org/apache/flink/runtime/security/modules/ZooKeeperModule.java#L49
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user gaqzcb commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3566#discussion_r106839886

          — Diff: flink-runtime/src/main/java/org/apache/flink/runtime/util/ZooKeeperUtils.java —
          @@ -89,6 +90,7 @@ public static CuratorFramework startCuratorFramework(Configuration configuration

          boolean disableSaslClient = configuration.getBoolean(ConfigConstants.ZOOKEEPER_SASL_DISABLE,
          ConfigConstants.DEFAULT_ZOOKEEPER_SASL_DISABLE);
          + System.setProperty(ZooKeeperSaslClient.ENABLE_CLIENT_SASL_KEY, String.valueOf(!disableSaslClient));
          — End diff –

          thanks @vijikarthi , I will do this a bit later.

          Show
          githubbot ASF GitHub Bot added a comment - Github user gaqzcb commented on a diff in the pull request: https://github.com/apache/flink/pull/3566#discussion_r106839886 — Diff: flink-runtime/src/main/java/org/apache/flink/runtime/util/ZooKeeperUtils.java — @@ -89,6 +90,7 @@ public static CuratorFramework startCuratorFramework(Configuration configuration boolean disableSaslClient = configuration.getBoolean(ConfigConstants.ZOOKEEPER_SASL_DISABLE, ConfigConstants.DEFAULT_ZOOKEEPER_SASL_DISABLE); + System.setProperty(ZooKeeperSaslClient.ENABLE_CLIENT_SASL_KEY, String.valueOf(!disableSaslClient)); — End diff – thanks @vijikarthi , I will do this a bit later.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user zhengcanbin opened a pull request:

          https://github.com/apache/flink/pull/3575

          FLINK-6117makes setting of 'zookeeper.sasl.disable' work correctly

          Thanks for contributing to Apache Flink. Before you open your pull request, please take the following check list into consideration.
          If your changes take all of the items into account, feel free to open your pull request. For more information and/or questions please refer to the [How To Contribute guide](http://flink.apache.org/how-to-contribute.html).
          In addition to going through the list, please provide a meaningful description of your changes.

          • [ ] General
          • The pull request references the related JIRA issue ("[FLINK-XXX] Jira title text")
          • The pull request addresses only one issue
          • Each commit in the PR has a meaningful commit message (including the JIRA id)
          • [ ] Documentation
          • Documentation has been added for new functionality
          • Old documentation affected by the pull request has been updated
          • JavaDoc for public methods has been added
          • [ ] Tests & Build
          • Functionality added by the pull request is covered by tests
          • `mvn clean verify` has been executed successfully locally or a Travis build has passed

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

          $ git pull https://github.com/zhengcanbin/flink FLINK-6117

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

          https://github.com/apache/flink/pull/3575.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 #3575


          commit 7cc94adb1d8478d12c52f8ea69177a7610e459d0
          Author: zcb <2056268568@qq.com>
          Date: 2017-03-20T15:46:04Z

          FLINK-6117makes setting of 'zookeeper.sasl.disable' work correctly


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user zhengcanbin opened a pull request: https://github.com/apache/flink/pull/3575 FLINK-6117 makes setting of 'zookeeper.sasl.disable' work correctly Thanks for contributing to Apache Flink. Before you open your pull request, please take the following check list into consideration. If your changes take all of the items into account, feel free to open your pull request. For more information and/or questions please refer to the [How To Contribute guide] ( http://flink.apache.org/how-to-contribute.html ). In addition to going through the list, please provide a meaningful description of your changes. [ ] General The pull request references the related JIRA issue (" [FLINK-XXX] Jira title text") The pull request addresses only one issue Each commit in the PR has a meaningful commit message (including the JIRA id) [ ] Documentation Documentation has been added for new functionality Old documentation affected by the pull request has been updated JavaDoc for public methods has been added [ ] Tests & Build Functionality added by the pull request is covered by tests `mvn clean verify` has been executed successfully locally or a Travis build has passed You can merge this pull request into a Git repository by running: $ git pull https://github.com/zhengcanbin/flink FLINK-6117 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/3575.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 #3575 commit 7cc94adb1d8478d12c52f8ea69177a7610e459d0 Author: zcb <2056268568@qq.com> Date: 2017-03-20T15:46:04Z FLINK-6117 makes setting of 'zookeeper.sasl.disable' work correctly
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zhengcanbin commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3566#discussion_r106940624

          — Diff: flink-runtime/src/main/java/org/apache/flink/runtime/util/ZooKeeperUtils.java —
          @@ -89,6 +90,7 @@ public static CuratorFramework startCuratorFramework(Configuration configuration

          boolean disableSaslClient = configuration.getBoolean(ConfigConstants.ZOOKEEPER_SASL_DISABLE,
          ConfigConstants.DEFAULT_ZOOKEEPER_SASL_DISABLE);
          + System.setProperty(ZooKeeperSaslClient.ENABLE_CLIENT_SASL_KEY, String.valueOf(!disableSaslClient));
          — End diff –

          @vijikarthi, I post another pull Request #3575 with my new formal github account.
          I will close Request #3566.

          Show
          githubbot ASF GitHub Bot added a comment - Github user zhengcanbin commented on a diff in the pull request: https://github.com/apache/flink/pull/3566#discussion_r106940624 — Diff: flink-runtime/src/main/java/org/apache/flink/runtime/util/ZooKeeperUtils.java — @@ -89,6 +90,7 @@ public static CuratorFramework startCuratorFramework(Configuration configuration boolean disableSaslClient = configuration.getBoolean(ConfigConstants.ZOOKEEPER_SASL_DISABLE, ConfigConstants.DEFAULT_ZOOKEEPER_SASL_DISABLE); + System.setProperty(ZooKeeperSaslClient.ENABLE_CLIENT_SASL_KEY, String.valueOf(!disableSaslClient)); — End diff – @vijikarthi, I post another pull Request #3575 with my new formal github account. I will close Request #3566.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user gaqzcb closed the pull request at:

          https://github.com/apache/flink/pull/3566

          Show
          githubbot ASF GitHub Bot added a comment - Github user gaqzcb closed the pull request at: https://github.com/apache/flink/pull/3566
          Hide
          canbinzheng CanBin Zheng added a comment -

          @Vijay Srinivasaraghavan

          It seems that I have no permission to assign my issue to myself, how to do this?

          Show
          canbinzheng CanBin Zheng added a comment - @ Vijay Srinivasaraghavan It seems that I have no permission to assign my issue to myself, how to do this?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zhengcanbin commented on the issue:

          https://github.com/apache/flink/pull/3575

          Hi, @fhueske All checks have failed, is there any problem with my patch code?

          Show
          githubbot ASF GitHub Bot added a comment - Github user zhengcanbin commented on the issue: https://github.com/apache/flink/pull/3575 Hi, @fhueske All checks have failed, is there any problem with my patch code?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zhengcanbin commented on the issue:

          https://github.com/apache/flink/pull/3575

          Hi @hadronzoo, checks have failed, from the CI build Details I can't find problems with my patch, can you review this patch?

          Show
          githubbot ASF GitHub Bot added a comment - Github user zhengcanbin commented on the issue: https://github.com/apache/flink/pull/3575 Hi @hadronzoo, checks have failed, from the CI build Details I can't find problems with my patch, can you review this patch?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zhengcanbin commented on the issue:

          https://github.com/apache/flink/pull/3575

          I checked in every failed raw log and that error is something similar
          ![image](https://cloud.githubusercontent.com/assets/26545140/24150102/ca94436e-0e7f-11e7-8f96-502cc03b9c35.png)
          this is not problem from my side, @hadronzoo

          Show
          githubbot ASF GitHub Bot added a comment - Github user zhengcanbin commented on the issue: https://github.com/apache/flink/pull/3575 I checked in every failed raw log and that error is something similar ! [image] ( https://cloud.githubusercontent.com/assets/26545140/24150102/ca94436e-0e7f-11e7-8f96-502cc03b9c35.png ) this is not problem from my side, @hadronzoo
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zhengcanbin closed the pull request at:

          https://github.com/apache/flink/pull/3575

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

          GitHub user zhengcanbin opened a pull request:

          https://github.com/apache/flink/pull/3600

          FLINK-6117Make setting of 'zookeeper.sasl.disable' work correctly

          The value of 'zookeeper.sasl.disable' not used in the right way when starting CuratorFramework. When 'zookeeper.sasl.disable' is set to true, both of FlinkYarnSessionCli & FlinkApplicationMasterRunner logs show that they attempt connecting to zookeeper in SASL mode.

          On my side, this patch works and All tests passed with 'mvn clean verify', can someone review this?

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

          $ git pull https://github.com/zhengcanbin/flink flink6117

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

          https://github.com/apache/flink/pull/3600.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 #3600


          commit d149f361b7fdc75ae80e253d4762b084bec32f77
          Author: zcb <2056268568@qq.com>
          Date: 2017-03-22T19:44:10Z

          Make setting of 'zookeeper.sasl.disable' work correctly


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user zhengcanbin opened a pull request: https://github.com/apache/flink/pull/3600 FLINK-6117 Make setting of 'zookeeper.sasl.disable' work correctly The value of 'zookeeper.sasl.disable' not used in the right way when starting CuratorFramework. When 'zookeeper.sasl.disable' is set to true, both of FlinkYarnSessionCli & FlinkApplicationMasterRunner logs show that they attempt connecting to zookeeper in SASL mode. On my side, this patch works and All tests passed with 'mvn clean verify', can someone review this? You can merge this pull request into a Git repository by running: $ git pull https://github.com/zhengcanbin/flink flink6117 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/3600.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 #3600 commit d149f361b7fdc75ae80e253d4762b084bec32f77 Author: zcb <2056268568@qq.com> Date: 2017-03-22T19:44:10Z Make setting of 'zookeeper.sasl.disable' work correctly
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zhengcanbin commented on the issue:

          https://github.com/apache/flink/pull/3600

          Hi, this patch works and passes all tests on my side, but now all checks have failed, I check all the five raw logs, obviously those errors are not from my side.
          ![image](https://cloud.githubusercontent.com/assets/26545140/24238979/167209d4-0fe7-11e7-9e58-8e556ba793ba.png)

          ![image](https://cloud.githubusercontent.com/assets/26545140/24238993/23a546a2-0fe7-11e7-8620-3de1de103b26.png)

          ![image](https://cloud.githubusercontent.com/assets/26545140/24239009/2c6770da-0fe7-11e7-877c-bb53f2f53758.png)

          I found it's so hard to pass the CI build, is there something wrong with the servers?
          Who can help look over?

          Show
          githubbot ASF GitHub Bot added a comment - Github user zhengcanbin commented on the issue: https://github.com/apache/flink/pull/3600 Hi, this patch works and passes all tests on my side, but now all checks have failed, I check all the five raw logs, obviously those errors are not from my side. ! [image] ( https://cloud.githubusercontent.com/assets/26545140/24238979/167209d4-0fe7-11e7-9e58-8e556ba793ba.png ) ! [image] ( https://cloud.githubusercontent.com/assets/26545140/24238993/23a546a2-0fe7-11e7-8620-3de1de103b26.png ) ! [image] ( https://cloud.githubusercontent.com/assets/26545140/24239009/2c6770da-0fe7-11e7-877c-bb53f2f53758.png ) I found it's so hard to pass the CI build, is there something wrong with the servers? Who can help look over?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user StephanEwen commented on the issue:

          https://github.com/apache/flink/pull/3600

          Yes, CI seems to be unstable.

          Can you try to push again to the branch (maybe change the commit message, force push then) to retrigger the CI?

          Show
          githubbot ASF GitHub Bot added a comment - Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/3600 Yes, CI seems to be unstable. Can you try to push again to the branch (maybe change the commit message, force push then) to retrigger the CI?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user StephanEwen commented on the issue:

          https://github.com/apache/flink/pull/3600

          @EronWright @vijikarthi If you find the time, what do you think about this patch?

          Show
          githubbot ASF GitHub Bot added a comment - Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/3600 @EronWright @vijikarthi If you find the time, what do you think about this patch?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user EronWright commented on the issue:

          https://github.com/apache/flink/pull/3600

          To recap, when the ZK server is configured to use SASL, the client is challenged accordingly. The client checks for a login entry in the JAAS config to obtain a kerberos credential, and issues a warning if one isn't found. SASL may be explicitly disabled on the client using the system property `zookeeper.sasl.clientconfig`.

          Flink provides a configuration setting with which to disable SASL on the client, which causes the system property to be set. I cannot think of why the default value would be 'true'. The behavior of 1.2.0 is that the default is 'false' (because it effectively wasn't implemented). Maybe the default should remain 'false'?

          Show
          githubbot ASF GitHub Bot added a comment - Github user EronWright commented on the issue: https://github.com/apache/flink/pull/3600 To recap, when the ZK server is configured to use SASL, the client is challenged accordingly. The client checks for a login entry in the JAAS config to obtain a kerberos credential, and issues a warning if one isn't found. SASL may be explicitly disabled on the client using the system property `zookeeper.sasl.clientconfig`. Flink provides a configuration setting with which to disable SASL on the client, which causes the system property to be set. I cannot think of why the default value would be 'true'. The behavior of 1.2.0 is that the default is 'false' (because it effectively wasn't implemented). Maybe the default should remain 'false'?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zhengcanbin commented on the issue:

          https://github.com/apache/flink/pull/3600

          @EronWright @StephanEwen
          In ZookeeperSaslClient.java, 'zookeeper.sasl.client' is true by default, so I agree to set 'zookeeper.sasl.disable' to false by default, for consistency.

          Show
          githubbot ASF GitHub Bot added a comment - Github user zhengcanbin commented on the issue: https://github.com/apache/flink/pull/3600 @EronWright @StephanEwen In ZookeeperSaslClient.java, 'zookeeper.sasl.client' is true by default, so I agree to set 'zookeeper.sasl.disable' to false by default, for consistency.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user shijinkui commented on the issue:

          https://github.com/apache/flink/pull/3600

          @Rucongzhang Please review it whether it meet your requirement.

          Show
          githubbot ASF GitHub Bot added a comment - Github user shijinkui commented on the issue: https://github.com/apache/flink/pull/3600 @Rucongzhang Please review it whether it meet your requirement.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zhengcanbin commented on the issue:

          https://github.com/apache/flink/pull/3600

          @EronWright @vijikarthi

          Show
          githubbot ASF GitHub Bot added a comment - Github user zhengcanbin commented on the issue: https://github.com/apache/flink/pull/3600 @EronWright @vijikarthi
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Rucongzhang commented on the issue:

          https://github.com/apache/flink/pull/3600

          @zhengcanbin ,@StephanEwen ,@shijinkui. After push this issue, we must illustrate in flink configuration. If we want to use kerberos authentication with ZK server, we must set zookeeper.sasl.disable:false and set the security.kerberos.login.contexts:zookeeper. But before, we only set security.kerberos.login.contexts:zookeeper. Thanks!

          Show
          githubbot ASF GitHub Bot added a comment - Github user Rucongzhang commented on the issue: https://github.com/apache/flink/pull/3600 @zhengcanbin ,@StephanEwen ,@shijinkui. After push this issue, we must illustrate in flink configuration. If we want to use kerberos authentication with ZK server, we must set zookeeper.sasl.disable:false and set the security.kerberos.login.contexts:zookeeper. But before, we only set security.kerberos.login.contexts:zookeeper. Thanks!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user gaqzcb commented on the issue:

          https://github.com/apache/flink/pull/3600

          @EronWright @vijikarthi If you find the time, pls review this patch.

          Show
          githubbot ASF GitHub Bot added a comment - Github user gaqzcb commented on the issue: https://github.com/apache/flink/pull/3600 @EronWright @vijikarthi If you find the time, pls review this patch.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user StephanEwen commented on the issue:

          https://github.com/apache/flink/pull/3600

          I am a bit confused here, this seems to make things more complicated.

          Before, if you wanted to use ZK and Kerberos, you only added `zookeeper` to `security.kerberos.login.contexts`. Now you need additionally to set `zookeeper.sasl.disable` to `false`?

          Why don't we keep `zookeeper.sasl.disable` as `false` by default? If I understand it correctly, it is anyways only ever relevant when the ZooKeeper login context has been enabled...

          Show
          githubbot ASF GitHub Bot added a comment - Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/3600 I am a bit confused here, this seems to make things more complicated. Before, if you wanted to use ZK and Kerberos, you only added `zookeeper` to `security.kerberos.login.contexts`. Now you need additionally to set `zookeeper.sasl.disable` to `false`? Why don't we keep `zookeeper.sasl.disable` as `false` by default? If I understand it correctly, it is anyways only ever relevant when the ZooKeeper login context has been enabled...
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user vijikarthi commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3600#discussion_r108284815

          — Diff: flink-core/src/main/java/org/apache/flink/configuration/SecurityOptions.java —
          @@ -55,6 +55,10 @@
          // ZooKeeper Security Options
          // ------------------------------------------------------------------------

          + public static final ConfigOption<Boolean> ZOOKEEPER_SASL_DISABLE =
          + key("zookeeper.sasl.disable")
          + .defaultValue(true);
          — End diff –

          Can the default value be false (meaning SASL client is always enabled) to be in consistent with ZK SASL client module?

          Show
          githubbot ASF GitHub Bot added a comment - Github user vijikarthi commented on a diff in the pull request: https://github.com/apache/flink/pull/3600#discussion_r108284815 — Diff: flink-core/src/main/java/org/apache/flink/configuration/SecurityOptions.java — @@ -55,6 +55,10 @@ // ZooKeeper Security Options // ------------------------------------------------------------------------ + public static final ConfigOption<Boolean> ZOOKEEPER_SASL_DISABLE = + key("zookeeper.sasl.disable") + .defaultValue(true); — End diff – Can the default value be false (meaning SASL client is always enabled) to be in consistent with ZK SASL client module?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user vijikarthi commented on the issue:

          https://github.com/apache/flink/pull/3600

          The default ZK SASL client behavior is to enable SASL client and to be in consistent it makes sense for us to leave the default option enabled.

          Show
          githubbot ASF GitHub Bot added a comment - Github user vijikarthi commented on the issue: https://github.com/apache/flink/pull/3600 The default ZK SASL client behavior is to enable SASL client and to be in consistent it makes sense for us to leave the default option enabled.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user EronWright commented on the issue:

          https://github.com/apache/flink/pull/3600

          The thread is becoming confused as to what the default behavior should be. In brief I think the code should say:
          ```
          public static final ConfigOption<Boolean> ZOOKEEPER_SASL_DISABLE =
          key("zookeeper.sasl.disable")
          .defaultValue(false);
          ```

          The above would leave SASL enabled by default, as is consistent with Flink 1.2.0.

          Show
          githubbot ASF GitHub Bot added a comment - Github user EronWright commented on the issue: https://github.com/apache/flink/pull/3600 The thread is becoming confused as to what the default behavior should be. In brief I think the code should say: ``` public static final ConfigOption<Boolean> ZOOKEEPER_SASL_DISABLE = key("zookeeper.sasl.disable") .defaultValue(false); ``` The above would leave SASL enabled by default, as is consistent with Flink 1.2.0.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Rucongzhang commented on the issue:

          https://github.com/apache/flink/pull/3600

          @StephanEwen , yes, now if you want to use kerberos , you need additionally to set zookeeper.sasl.disable to false. And I agree with you, the default value of zookeeper.sasl.disable can set to false. Then, If you want to use the kerberos, can only set ZooKeeper login context . Thanks!

          Show
          githubbot ASF GitHub Bot added a comment - Github user Rucongzhang commented on the issue: https://github.com/apache/flink/pull/3600 @StephanEwen , yes, now if you want to use kerberos , you need additionally to set zookeeper.sasl.disable to false. And I agree with you, the default value of zookeeper.sasl.disable can set to false. Then, If you want to use the kerberos, can only set ZooKeeper login context . Thanks!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zhengcanbin commented on the issue:

          https://github.com/apache/flink/pull/3600

          @StephanEwen @vijikarthi @EronWright @Rucongzhang
          Thanks all! I have changed this default behaviour, now `zookeeper.sasl.disable` is `false` by default.

          Show
          githubbot ASF GitHub Bot added a comment - Github user zhengcanbin commented on the issue: https://github.com/apache/flink/pull/3600 @StephanEwen @vijikarthi @EronWright @Rucongzhang Thanks all! I have changed this default behaviour, now `zookeeper.sasl.disable` is `false` by default.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user vijikarthi commented on the issue:

          https://github.com/apache/flink/pull/3600

          The changes looks good to me.

          Show
          githubbot ASF GitHub Bot added a comment - Github user vijikarthi commented on the issue: https://github.com/apache/flink/pull/3600 The changes looks good to me.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zhengcanbin commented on the issue:

          https://github.com/apache/flink/pull/3600

          @StephanEwen @EronWright

          Show
          githubbot ASF GitHub Bot added a comment - Github user zhengcanbin commented on the issue: https://github.com/apache/flink/pull/3600 @StephanEwen @EronWright
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user StephanEwen commented on the issue:

          https://github.com/apache/flink/pull/3600

          Thanks @zhengcanbin and @vijikarthi

          Merging this...

          Show
          githubbot ASF GitHub Bot added a comment - Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/3600 Thanks @zhengcanbin and @vijikarthi Merging this...
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/flink/pull/3600

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/3600
          Hide
          StephanEwen Stephan Ewen added a comment -

          Fixed in eef85e095a8a0e4c4553631b74ba7b9f173cebf0

          Show
          StephanEwen Stephan Ewen added a comment - Fixed in eef85e095a8a0e4c4553631b74ba7b9f173cebf0

            People

            • Assignee:
              canbinzheng CanBin Zheng
              Reporter:
              canbinzheng CanBin Zheng
            • Votes:
              1 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

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

                  Development