ZOOKEEPER-3561: Generalize target authentication scheme for ZooKeeper authentication enforcement.

[arshadmohammad]

Added enforce.auth.enabled and enforce.auth.scheme to enforce any authentication scheme.

[arshadmohammad]

As enforcing the SASL authentication scheme feature is already released. Keeping the feature as it is. Provided generalization on top of it

[hanm]

Looks good.

CI build fails due to check style failure. Let's fix these to get a green build.

[anmolnar]

Great stuff, lgtm.
Just a few nitpicks.

[arshadmohammad]

One ci feedback is pending from long time. Reopening the PR to trigger the ci again.

[eolivelli]

LGTM !
very useful

[eolivelli]

@arshadmohammad in order to force Jenkins to restart the job click on the click on the link "Details",login with your Apache id/password and click on the button to restart the job
https://ci-hadoop.apache.org/blue/organizations/jenkins/zookeeper-precommit-github-pr/detail/PR-1500/2/pipeline/

[ztzg]

Generally LGTM, but we have an existing use-case which requires multiple schemes. (I am willing to contribute such a change in a subsequent PR, however.)

[hanm]

There is one C unit test failure reported by https://ci-hadoop.apache.org/blue/organizations/jenkins/zookeeper-precommit-github-pr/detail/PR-1500/4/pipeline

`[2020-10-16T21:54:25.858Z] [exec] /home/jenkins/jenkins-home/workspace/eper-precommit-github-pr_PR-1500/zookeeper-client/zookeeper-client-c/tests/TestSASLAuth.cc:120: Assertion: assertion failed [Expression: ctx.waitForConnected(zk)]

[2020-10-16T21:54:25.858Z] [exec] Failures !!!

[2020-10-16T21:54:25.858Z] [exec] Run: 89 Failure total: 1 Failures: 1 Errors: 0

[2020-10-16T21:54:25.858Z] [exec] FAIL: zktest-mt

[2020-10-16T21:54:25.858Z] [exec] ==========================================

[2020-10-16T21:54:25.858Z] [exec] 1 of 2 tests failed

[2020-10-16T21:54:25.858Z] [exec] Please report to user@zookeeper.apache.org

[2020-10-16T21:54:25.858Z] [exec] ==========================================

[2020-10-16T21:54:25.858Z] [exec] Makefile:1850: recipe for target 'check-TESTS' failed

[2020-10-16T21:54:25.858Z] [exec] make[1]: Leaving directory '/home/jenkins/jenkins-home/workspace/eper-precommit-github-pr_PR-1500/zookeeper-client/zookeeper-client-c/target/c'

[2020-10-16T21:54:25.858Z] [exec] Makefile:2106: recipe for target 'check-am' failed

[2020-10-16T21:54:25.858Z] [exec] make[1]: *** [check-TESTS] Error 1

[2020-10-16T21:54:25.858Z] [exec] make: *** [check-am] Error 2`

@arshadmohammad do you mind check if that's a flaky test? Once we clear this I will merge this PR. I am also triggering another build to see if we can get a green build.

[ztzg]

Hi @arshadmohammad,

Addressed comments, Added support to configure multiple authentication schemes to enforce authentication

Fantastic; thanks! (Though I did not mean to imply you had to do the job!)

@hanm wrote:

There is one C unit test failure reported by https://ci-hadoop.apache.org/blue/organizations/jenkins/zookeeper-precommit-github-pr/detail/PR-1500/4/pipeline

`[2020-10-16T21:54:25.858Z] [exec] /home/jenkins/jenkins-home/workspace/eper-precommit-github-pr_PR-1500/zookeeper-client/zookeeper-client-c/tests/TestSASLAuth.cc:120: Assertion: assertion failed [Expression: ctx.waitForConnected(zk)]

@arshadmohammad do you mind check if that's a flaky test?

Not due to flakiness: the test used to work because the old sessionRequireClientSASLAuth flag was able to do its job without having a SASL provider configured (even though the combination makes no sense in practice). Suggested fix:

--- a/zookeeper-client/zookeeper-client-c/tests/zkServer.sh
+++ b/zookeeper-client/zookeeper-client-c/tests/zkServer.sh
@@ -128,9 +128,9 @@ PROPERTIES="$EXTRA_JVM_ARGS -Dzookeeper.extendedTypesEnabled=true -Dznode.contai
 if [ "x$1" == "xstartRequireSASLAuth" ]
 then
     PROPERTIES="-Dzookeeper.sessionRequireClientSASLAuth=true $PROPERTIES"
+    PROPERTIES="$PROPERTIES -Dzookeeper.authProvider.1=org.apache.zookeeper.server.auth.SASLAuthenticationProvider"
     if [ "x$2" != "x" ]
     then
-        PROPERTIES="$PROPERTIES -Dzookeeper.authProvider.1=org.apache.zookeeper.server.auth.SASLAuthenticationProvider"
         PROPERTIES="$PROPERTIES -Djava.security.auth.login.config=$2"
     fi
     if [ "x$3" != "x" ]

[arshadmohammad]

Not due to flakiness: the test used to work because the old sessionRequireClientSASLAuth flag was able to do its job without
having a SASL provider configured (even though the combination makes no sense in practice). Suggested fix:
Thanks @ztzg for analyzing and suggesting the fix. Updated the PR

[hanm]

got a green build now, merging.