Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Isolate zk client config for suppressing authentication #9438

Merged
merged 7 commits into from Jul 11, 2019
Merged

Isolate zk client config for suppressing authentication #9438

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
496875b
Isolate zk client config for suppressing authentication
Jul 2, 2019
955ed6f
Remove restartable testing server hack
Jul 9, 2019
bb99e9e
Bump curator to 4.2.0
Jul 9, 2019
53e9123
Unify zookeeper creation
Jul 9, 2019
542e81e
Use configuration for isolating config
Jul 10, 2019
e6c81fa
doc update
Jul 10, 2019
31fa30a
Update prop definition with scope and consistency level
Jul 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion core/common/pom.xml
View file
Open in desktop
Expand Up @@ -57,7 +57,8 @@
</dependency>
<dependency>
<groupId>io.netty</groupId>
<artifactId>netty-all</artifactId>
<artifactId>netty</artifactId>
<version>3.7.0.Final</version>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why change the version only for this specific module?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This module uses ConcurrentIdentityMap implementation from netty which is dropped after this version. And that's the only use of netty from this module.

Actually we don't need an explicit netty dependency anymore. I can have another PR to remove netty dependencies throughout the code. (Locally I tested that removing netty dependencies, except this one, has no effect on build.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you create a tech dept issue for this?

Copy link
Contributor

@calvinjia calvinjia Jul 11, 2019 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we can remove the direct dependency on Netty that would be helpful. (In a separate effort of course)

Copy link
Contributor Author

@ggezer ggezer Jul 11, 2019 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created #9449

</dependency>
<dependency>
<groupId>io.swagger</groupId>
Expand Down
7 changes: 5 additions & 2 deletions core/common/src/main/java/alluxio/master/MasterInquireClient.java
View file
Open in desktop
Expand Up @@ -16,6 +16,7 @@
import alluxio.exception.status.UnavailableException;
import alluxio.master.SingleMasterInquireClient.SingleMasterConnectDetails;
import alluxio.master.ZkMasterInquireClient.ZkMasterConnectDetails;
import alluxio.security.authentication.AuthType;
import alluxio.uri.Authority;
import alluxio.util.ConfigurationUtils;
import alluxio.util.network.NetworkAddressUtils;
Expand Down Expand Up @@ -81,7 +82,8 @@ public static MasterInquireClient create(AlluxioConfiguration conf) {
return ZkMasterInquireClient.getClient(conf.get(PropertyKey.ZOOKEEPER_ADDRESS),
conf.get(PropertyKey.ZOOKEEPER_ELECTION_PATH),
conf.get(PropertyKey.ZOOKEEPER_LEADER_PATH),
conf.getInt(PropertyKey.ZOOKEEPER_LEADER_INQUIRY_RETRY_COUNT));
conf.getInt(PropertyKey.ZOOKEEPER_LEADER_INQUIRY_RETRY_COUNT),
conf.getEnum(PropertyKey.SECURITY_AUTHENTICATION_TYPE, AuthType.class));
} else {
List<InetSocketAddress> addresses = ConfigurationUtils.getMasterRpcAddresses(conf);
if (addresses.size() > 1) {
Expand All @@ -97,7 +99,8 @@ public static MasterInquireClient createForJobMaster(AlluxioConfiguration conf)
return ZkMasterInquireClient.getClient(conf.get(PropertyKey.ZOOKEEPER_ADDRESS),
conf.get(PropertyKey.ZOOKEEPER_JOB_ELECTION_PATH),
conf.get(PropertyKey.ZOOKEEPER_JOB_LEADER_PATH),
conf.getInt(PropertyKey.ZOOKEEPER_LEADER_INQUIRY_RETRY_COUNT));
conf.getInt(PropertyKey.ZOOKEEPER_LEADER_INQUIRY_RETRY_COUNT),
conf.getEnum(PropertyKey.SECURITY_AUTHENTICATION_TYPE, AuthType.class));
ggezer marked this conversation as resolved.
Show resolved Hide resolved
} else {
List<InetSocketAddress> addresses = ConfigurationUtils.getJobMasterRpcAddresses(conf);
if (addresses.size() > 1) {
Expand Down
37 changes: 30 additions & 7 deletions core/common/src/main/java/alluxio/master/ZkMasterInquireClient.java
View file
Open in desktop
Expand Up @@ -13,6 +13,7 @@

import alluxio.Constants;
import alluxio.exception.status.UnavailableException;
import alluxio.security.authentication.AuthType;
import alluxio.uri.Authority;
import alluxio.uri.ZookeeperAuthority;
import alluxio.util.CommonUtils;
Expand All @@ -23,7 +24,10 @@
import org.apache.curator.framework.CuratorFramework;
import org.apache.curator.framework.CuratorFrameworkFactory;
import org.apache.curator.retry.ExponentialBackoffRetry;
import org.apache.curator.utils.ZookeeperFactory;
import org.apache.zookeeper.Watcher;
import org.apache.zookeeper.ZooKeeper;
import org.apache.zookeeper.client.ZKClientConfig;
import org.apache.zookeeper.data.Stat;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -53,22 +57,37 @@ public final class ZkMasterInquireClient implements MasterInquireClient, Closeab
private final CuratorFramework mClient;
private final int mInquireRetryCount;

/**
* Zookeeper factory for curator that explicitly disables authentication.
ggezer marked this conversation as resolved.
Show resolved Hide resolved
*/
private class NoSaslZookeeperFactory implements ZookeeperFactory {

@Override
public ZooKeeper newZooKeeper(String connectString, int sessionTimeout, Watcher watcher,
boolean canBeReadOnly) throws Exception {
ZKClientConfig zkConfig = new ZKClientConfig();
zkConfig.setProperty(ZKClientConfig.ENABLE_CLIENT_SASL_KEY, "false");
return new ZooKeeper(connectString, sessionTimeout, watcher, zkConfig);
}
}

/**
* Gets the client.
*
* @param zookeeperAddress the address for Zookeeper
* @param electionPath the path of the master election
* @param leaderPath the path of the leader
* @param inquireRetryCount the number of times to retry connections
* @param authType Alluxio auth type
* @return the client
*/
public static synchronized ZkMasterInquireClient getClient(String zookeeperAddress,
String electionPath, String leaderPath, int inquireRetryCount) {
String electionPath, String leaderPath, int inquireRetryCount, AuthType authType) {
ZkMasterConnectDetails connectDetails =
new ZkMasterConnectDetails(zookeeperAddress, leaderPath);
if (!sCreatedClients.containsKey(connectDetails)) {
sCreatedClients.put(connectDetails, new ZkMasterInquireClient(connectDetails, electionPath,
inquireRetryCount));
sCreatedClients.put(connectDetails,
new ZkMasterInquireClient(connectDetails, electionPath, inquireRetryCount, authType));
}
return sCreatedClients.get(connectDetails);
}
Expand All @@ -78,16 +97,20 @@ public static synchronized ZkMasterInquireClient getClient(String zookeeperAddre
*
* @param connectDetails connect details
* @param electionPath the path of the master election
* @param inquireRetryCount the number of times to retry connections
* @param authType Alluxio auth type
*/
private ZkMasterInquireClient(ZkMasterConnectDetails connectDetails, String electionPath,
int inquireRetryCount) {
int inquireRetryCount, AuthType authType) {
mConnectDetails = connectDetails;
mElectionPath = electionPath;

LOG.info("Creating new zookeeper client for {}", connectDetails);
// Start the client lazily.
mClient = CuratorFrameworkFactory.newClient(connectDetails.getZkAddress(),
new ExponentialBackoffRetry(Constants.SECOND_MS, 3));
CuratorFrameworkFactory.Builder curatorBuilder = CuratorFrameworkFactory.builder();
curatorBuilder.connectString(connectDetails.getZkAddress());
curatorBuilder.retryPolicy(new ExponentialBackoffRetry(Constants.SECOND_MS, 3));
curatorBuilder.zookeeperFactory(new NoSaslZookeeperFactory());
madanadit marked this conversation as resolved.
Show resolved Hide resolved
mClient = curatorBuilder.build();

mInquireRetryCount = inquireRetryCount;
}
Expand Down
10 changes: 6 additions & 4 deletions core/common/src/test/java/alluxio/master/ZkMasterInquireClientTest.java
View file
Open in desktop
Expand Up @@ -14,6 +14,7 @@
import alluxio.AlluxioTestDirectory;
import alluxio.Constants;
import alluxio.exception.status.UnavailableException;
import alluxio.security.authentication.AuthType;

import org.apache.curator.framework.CuratorFramework;
import org.apache.curator.framework.CuratorFrameworkFactory;
Expand All @@ -33,6 +34,7 @@ public class ZkMasterInquireClientTest {
private static final String LOOPBACK_IP = "127.0.0.1";
private static final int TESTING_SERVER_PORT = 11111;
private static final int INQUIRE_RETRY_COUNT = 2;
private static final AuthType INQUIRE_AUTH_TYPE = AuthType.SIMPLE;

private TestingServer mZkServer;

Expand All @@ -53,7 +55,7 @@ public void after() throws Exception {
public void testNoParticipant() throws Exception {
// Create zk inquire client.
MasterInquireClient zkInquirer = ZkMasterInquireClient.getClient(mZkServer.getConnectString(),
ELECTION_PATH, LEADER_PATH, INQUIRE_RETRY_COUNT);
ELECTION_PATH, LEADER_PATH, INQUIRE_RETRY_COUNT, INQUIRE_AUTH_TYPE);
// Create curator client for manipulating the leader path.
CuratorFramework client = CuratorFrameworkFactory.newClient(mZkServer.getConnectString(),
new ExponentialBackoffRetry(Constants.SECOND_MS, INQUIRE_RETRY_COUNT));
Expand All @@ -76,7 +78,7 @@ public void testNoParticipant() throws Exception {
public void testNoPath() throws Exception {
// Create zk inquire client.
MasterInquireClient zkInquirer = ZkMasterInquireClient.getClient(mZkServer.getConnectString(),
ELECTION_PATH, LEADER_PATH, INQUIRE_RETRY_COUNT);
ELECTION_PATH, LEADER_PATH, INQUIRE_RETRY_COUNT, INQUIRE_AUTH_TYPE);
// Query should fail with no leader path created.
boolean queryFailed = false;
try {
Expand All @@ -92,7 +94,7 @@ public void testNoPath() throws Exception {
public void testSingleLeader() throws Exception {
// Create zk inquire client.
MasterInquireClient zkInquirer = ZkMasterInquireClient.getClient(mZkServer.getConnectString(),
ELECTION_PATH, LEADER_PATH, INQUIRE_RETRY_COUNT);
ELECTION_PATH, LEADER_PATH, INQUIRE_RETRY_COUNT, INQUIRE_AUTH_TYPE);
// Create curator client for manipulating the leader path.
CuratorFramework client = CuratorFrameworkFactory.newClient(mZkServer.getConnectString(),
new ExponentialBackoffRetry(Constants.SECOND_MS, INQUIRE_RETRY_COUNT));
Expand All @@ -110,7 +112,7 @@ public void testSingleLeader() throws Exception {
public void testMultipleLeaders() throws Exception {
// Create zk inquire client.
MasterInquireClient zkInquirer = ZkMasterInquireClient.getClient(mZkServer.getConnectString(),
ELECTION_PATH, LEADER_PATH, INQUIRE_RETRY_COUNT);
ELECTION_PATH, LEADER_PATH, INQUIRE_RETRY_COUNT, INQUIRE_AUTH_TYPE);
// Create curator client for manipulating the leader path.
CuratorFramework client = CuratorFrameworkFactory.newClient(mZkServer.getConnectString(),
new ExponentialBackoffRetry(Constants.SECOND_MS, INQUIRE_RETRY_COUNT));
Expand Down
6 changes: 3 additions & 3 deletions minicluster/src/main/java/alluxio/master/MultiMasterLocalAlluxioCluster.java
View file
Open in desktop
Expand Up @@ -26,9 +26,9 @@
import alluxio.util.CommonUtils;
import alluxio.util.WaitForOptions;
import alluxio.util.io.PathUtils;
import alluxio.zookeeper.RestartableTestingServer;

import com.google.common.base.Throwables;
import org.apache.curator.test.TestingServer;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -49,7 +49,7 @@ public final class MultiMasterLocalAlluxioCluster extends AbstractLocalAlluxioCl
private static final Logger LOG = LoggerFactory.getLogger(MultiMasterLocalAlluxioCluster.class);
private static final int WAIT_MASTER_START_TIMEOUT_MS = 5000;

private RestartableTestingServer mCuratorServer = null;
private TestingServer mCuratorServer = null;
private int mNumOfMasters = 0;

private final List<LocalAlluxioMaster> mMasters = new ArrayList<>();
Expand All @@ -73,7 +73,7 @@ public MultiMasterLocalAlluxioCluster(int numMasters, int numWorkers) {

try {
mCuratorServer =
new RestartableTestingServer(-1, AlluxioTestDirectory.createTemporaryDirectory("zk"));
new TestingServer(-1, AlluxioTestDirectory.createTemporaryDirectory("zk"));
LOG.info("Started testing zookeeper: {}", mCuratorServer.getConnectString());
} catch (Exception e) {
throw Throwables.propagate(e);
Expand Down
10 changes: 6 additions & 4 deletions minicluster/src/main/java/alluxio/multi/process/MultiProcessCluster.java
View file
Open in desktop
Expand Up @@ -39,16 +39,17 @@
import alluxio.master.journal.JournalType;
import alluxio.multi.process.PortCoordination.ReservedPort;
import alluxio.network.PortUtils;
import alluxio.security.authentication.AuthType;
import alluxio.util.CommonUtils;
import alluxio.util.WaitForOptions;
import alluxio.util.io.PathUtils;
import alluxio.util.network.NetworkAddressUtils;
import alluxio.zookeeper.RestartableTestingServer;

import com.google.common.base.Preconditions;
import com.google.common.io.Closer;
import org.apache.commons.io.Charsets;
import org.apache.commons.io.FileUtils;
import org.apache.curator.test.TestingServer;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -116,7 +117,7 @@ public final class MultiProcessCluster {
/** Addresses of all masters. Should have the same size as {@link #mMasters}. */
private List<MasterNetAddress> mMasterAddresses;
private State mState;
private RestartableTestingServer mCuratorServer;
private TestingServer mCuratorServer;
private FileSystemContext mFilesystemContext;
/**
* Tracks whether the test has succeeded. If mSuccess is never updated before {@link #destroy()},
Expand Down Expand Up @@ -192,7 +193,7 @@ public synchronized void start() throws Exception {
break;
case ZOOKEEPER_HA:
mCuratorServer = mCloser.register(
new RestartableTestingServer(-1, AlluxioTestDirectory.createTemporaryDirectory("zk")));
new TestingServer(-1, AlluxioTestDirectory.createTemporaryDirectory("zk")));
mProperties.put(PropertyKey.MASTER_JOURNAL_TYPE, JournalType.UFS.toString());
mProperties.put(PropertyKey.ZOOKEEPER_ENABLED, "true");
mProperties.put(PropertyKey.ZOOKEEPER_ADDRESS, mCuratorServer.getConnectString());
Expand Down Expand Up @@ -652,7 +653,8 @@ public synchronized MasterInquireClient getMasterInquireClient() {
return ZkMasterInquireClient.getClient(mCuratorServer.getConnectString(),
ServerConfiguration.get(PropertyKey.ZOOKEEPER_ELECTION_PATH),
ServerConfiguration.get(PropertyKey.ZOOKEEPER_LEADER_PATH),
ServerConfiguration.getInt(PropertyKey.ZOOKEEPER_LEADER_INQUIRY_RETRY_COUNT));
ServerConfiguration.getInt(PropertyKey.ZOOKEEPER_LEADER_INQUIRY_RETRY_COUNT),
ServerConfiguration.getEnum(PropertyKey.SECURITY_AUTHENTICATION_TYPE, AuthType.class));
default:
throw new IllegalStateException("Unknown deploy mode: " + mDeployMode.toString());
}
Expand Down
43 changes: 0 additions & 43 deletions minicluster/src/main/java/alluxio/zookeeper/RestartableTestingServer.java
View file
Open in desktop

This file was deleted.

7 changes: 3 additions & 4 deletions pom.xml
View file
Open in desktop
Expand Up @@ -134,8 +134,7 @@
</repositories>

<properties>
<apache.curator.version>4.0.1</apache.curator.version>
<apache.curator-test.version>2.12.0</apache.curator-test.version>
<apache.curator.version>4.2.0</apache.curator.version>
<aws.amazonaws.version>1.11.215</aws.amazonaws.version>
<build.path>build</build.path>
<copycat.version>1.2.12</copycat.version>
Expand Down Expand Up @@ -571,7 +570,7 @@
<dependency>
<groupId>org.apache.zookeeper</groupId>
<artifactId>zookeeper</artifactId>
<version>3.4.6</version>
<version>3.5.5</version>
</dependency>
<dependency>
<groupId>org.eclipse.jetty</groupId>
Expand Down Expand Up @@ -692,7 +691,7 @@
<dependency>
<groupId>org.apache.curator</groupId>
<artifactId>curator-test</artifactId>
<version>${apache.curator-test.version}</version>
<version>${apache.curator.version}</version>
<scope>test</scope>
</dependency>
<dependency>
Expand Down
4 changes: 4 additions & 0 deletions shell/pom.xml
View file
Open in desktop
Expand Up @@ -59,6 +59,10 @@
<groupId>org.apache.httpcomponents</groupId>
<artifactId>httpclient</artifactId>
</dependency>
<dependency>
<groupId>jline</groupId>
<artifactId>jline</artifactId>
</dependency>

<!-- Internal dependencies -->
<dependency>
Expand Down