From 02bd0eca5337a42de47a3b18fe929996eb2ff5e8 Mon Sep 17 00:00:00 2001 From: Bharath Vissapragada Date: Mon, 3 Feb 2020 10:15:32 -0800 Subject: [PATCH] HBASE-23752: Fix remaining test failures from nightly runs (#1102) TestFromClientSideWithCoprocessor: Initialization bug causing parameterized runs to fail. TestCustomSaslAuthenticationProvider: Test config had to be fixed because it was written pre-master registry implementation. TestSnapshotScannerHDFSAclController: Cluster restart did not reset the cached connection state. Signed-off-by: Nick Dimiduk Signed-off-by: Josh Elser --- .../TestFromClientSideWithCoprocessor.java | 7 +- .../TestSnapshotScannerHDFSAclController.java | 6 +- .../TestCustomSaslAuthenticationProvider.java | 92 +++++++++++++++---- 3 files changed, 81 insertions(+), 24 deletions(-) diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSideWithCoprocessor.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSideWithCoprocessor.java index d78976e63af..723b9971add 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSideWithCoprocessor.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSideWithCoprocessor.java @@ -49,10 +49,7 @@ public class TestFromClientSideWithCoprocessor extends TestFromClientSide { } public TestFromClientSideWithCoprocessor(Class registry, int numHedgedReqs) throws Exception { - if (TEST_UTIL == null) { - // It is ok to initialize once because the test is parameterized for a single dimension. - initialize(registry, numHedgedReqs, NoOpScanPolicyObserver.class, - MultiRowMutationEndpoint.class); - } + initialize(registry, numHedgedReqs, NoOpScanPolicyObserver.class, + MultiRowMutationEndpoint.class); } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController.java index 7ef4cf83ee7..7424bc44676 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController.java @@ -910,6 +910,11 @@ public class TestSnapshotScannerHDFSAclController { TEST_UTIL.restartHBaseCluster(1); TEST_UTIL.waitUntilNoRegionsInTransition(); + // reset the cached configs after restart + conf = TEST_UTIL.getConfiguration(); + admin = TEST_UTIL.getAdmin(); + helper = new SnapshotScannerHDFSAclHelper(conf, admin.getConnection()); + Path tmpNsDir = helper.getPathHelper().getTmpNsDir(namespace); assertTrue(fs.exists(tmpNsDir)); // check all regions in tmp table2 dir are archived @@ -917,7 +922,6 @@ public class TestSnapshotScannerHDFSAclController { // create table1 and snapshot TestHDFSAclHelper.createTableAndPut(TEST_UTIL, table); - admin = TEST_UTIL.getAdmin(); aclTable = TEST_UTIL.getConnection().getTable(PermissionStorage.ACL_TABLE_NAME); admin.snapshot(snapshot, table); TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, grantUser, snapshot, 6); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/provider/TestCustomSaslAuthenticationProvider.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/provider/TestCustomSaslAuthenticationProvider.java index 957810e692e..96508178e1e 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/provider/TestCustomSaslAuthenticationProvider.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/provider/TestCustomSaslAuthenticationProvider.java @@ -21,7 +21,6 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; - import java.io.ByteArrayInputStream; import java.io.DataInput; import java.io.DataInputStream; @@ -30,13 +29,14 @@ import java.io.File; import java.io.IOException; import java.net.InetAddress; import java.security.PrivilegedExceptionAction; +import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.List; import java.util.Map; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; - import javax.security.auth.callback.Callback; import javax.security.auth.callback.CallbackHandler; import javax.security.auth.callback.NameCallback; @@ -47,7 +47,6 @@ import javax.security.sasl.RealmCallback; import javax.security.sasl.RealmChoiceCallback; import javax.security.sasl.Sasl; import javax.security.sasl.SaslClient; - import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.Cell; @@ -57,6 +56,7 @@ import org.apache.hadoop.hbase.HBaseTestingUtility; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.LocalHBaseCluster; import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.TableNameTestRule; import org.apache.hadoop.hbase.client.Admin; import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder; import org.apache.hadoop.hbase.client.Connection; @@ -68,7 +68,10 @@ import org.apache.hadoop.hbase.client.RetriesExhaustedException; import org.apache.hadoop.hbase.client.Table; import org.apache.hadoop.hbase.client.TableDescriptorBuilder; import org.apache.hadoop.hbase.coprocessor.CoprocessorHost; +import org.apache.hadoop.hbase.exceptions.MasterRegistryFetchException; import org.apache.hadoop.hbase.ipc.BlockingRpcClient; +import org.apache.hadoop.hbase.ipc.NettyRpcClient; +import org.apache.hadoop.hbase.ipc.NettyRpcServer; import org.apache.hadoop.hbase.ipc.RpcClientFactory; import org.apache.hadoop.hbase.ipc.RpcServerFactory; import org.apache.hadoop.hbase.ipc.SimpleRpcServer; @@ -93,6 +96,7 @@ import org.apache.hadoop.security.token.SecretManager; import org.apache.hadoop.security.token.SecretManager.InvalidToken; import org.apache.hadoop.security.token.Token; import org.apache.hadoop.security.token.TokenIdentifier; +import org.junit.After; import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; @@ -100,10 +104,11 @@ import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; import org.junit.experimental.categories.Category; -import org.junit.rules.TestName; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; import org.slf4j.Logger; import org.slf4j.LoggerFactory; - +import org.apache.hbase.thirdparty.com.google.common.base.Throwables; import org.apache.hadoop.hbase.shaded.protobuf.generated.RPCProtos.UserInformation; /** @@ -112,6 +117,7 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.RPCProtos.UserInformati * This tests holds a "user database" in memory as a hashmap. Clients provide their password * in the client Hadoop configuration. The servers validate this password via the "user database". */ +@RunWith(Parameterized.class) @Category({MediumTests.class, SecurityTests.class}) public class TestCustomSaslAuthenticationProvider { private static final Logger LOG = LoggerFactory.getLogger( @@ -126,6 +132,27 @@ public class TestCustomSaslAuthenticationProvider { private static final String USER1_PASSWORD = "foobarbaz"; private static final String USER2_PASSWORD = "bazbarfoo"; + @Parameterized.Parameters(name = "{index}: rpcClientImpl={0}, rpcServerImpl={1}") + public static Collection parameters() { + List params = new ArrayList<>(); + List rpcClientImpls = Arrays.asList( + BlockingRpcClient.class.getName(), NettyRpcClient.class.getName()); + List rpcServerImpls = Arrays.asList( + SimpleRpcServer.class.getName(), NettyRpcServer.class.getName()); + for (String rpcClientImpl : rpcClientImpls) { + for (String rpcServerImpl : rpcServerImpls) { + params.add(new Object[] { rpcClientImpl, rpcServerImpl }); + } + } + return params; + } + + @Parameterized.Parameter(0) + public String rpcClientImpl; + + @Parameterized.Parameter(1) + public String rpcServerImpl; + private static Map createUserDatabase() { Map db = new ConcurrentHashMap<>(); db.put("user1", USER1_PASSWORD); @@ -400,7 +427,7 @@ public class TestCustomSaslAuthenticationProvider { } } - static LocalHBaseCluster createCluster(HBaseTestingUtility util, File keytabFile, + static void createBaseCluster(HBaseTestingUtility util, File keytabFile, MiniKdc kdc) throws Exception { String servicePrincipal = "hbase/localhost"; String spnegoPrincipal = "HTTP/localhost"; @@ -416,8 +443,6 @@ public class TestCustomSaslAuthenticationProvider { util.startMiniDFSCluster(1); Path rootdir = util.getDataTestDirOnTestFS("TestGenerateDelegationToken"); FSUtils.setRootDir(util.getConfiguration(), rootdir); - LocalHBaseCluster cluster = new LocalHBaseCluster(util.getConfiguration(), 1); - return cluster; } private static final HBaseTestingUtility UTIL = new HBaseTestingUtility(); @@ -431,10 +456,6 @@ public class TestCustomSaslAuthenticationProvider { UTIL.getDataTestDir("keytab").toUri().getPath()); final MiniKdc kdc = UTIL.setupMiniKdc(KEYTAB_FILE); - // Switch back to NIO for now. - CONF.set(RpcClientFactory.CUSTOM_RPC_CLIENT_IMPL_CONF_KEY, BlockingRpcClient.class.getName()); - CONF.set(RpcServerFactory.CUSTOM_RPC_SERVER_IMPL_CONF_KEY, SimpleRpcServer.class.getName()); - // Adds our test impls instead of creating service loader entries which // might inadvertently get them loaded on a real cluster. CONF.setStrings(SaslClientAuthenticationProviders.EXTRA_PROVIDERS_KEY, @@ -443,9 +464,23 @@ public class TestCustomSaslAuthenticationProvider { InMemoryServerProvider.class.getName()); CONF.set(SaslClientAuthenticationProviders.SELECTOR_KEY, InMemoryProviderSelector.class.getName()); + createBaseCluster(UTIL, KEYTAB_FILE, kdc); + } - CLUSTER = createCluster(UTIL, KEYTAB_FILE, kdc); + @Before + public void setUpBeforeTest() throws Exception { + CONF.unset(HConstants.CLIENT_CONNECTION_REGISTRY_IMPL_CONF_KEY); + CONF.set(RpcClientFactory.CUSTOM_RPC_CLIENT_IMPL_CONF_KEY, rpcClientImpl); + CONF.set(RpcServerFactory.CUSTOM_RPC_SERVER_IMPL_CONF_KEY, rpcServerImpl); + if (rpcClientImpl.equals(BlockingRpcClient.class.getName())) { + // Set the connection registry to ZKConnectionRegistry since hedging is not supported on + // blocking rpc clients. + CONF.set(HConstants.CLIENT_CONNECTION_REGISTRY_IMPL_CONF_KEY, + HConstants.ZK_CONNECTION_REGISTRY_CLASS); + } + CLUSTER = new LocalHBaseCluster(CONF, 1); CLUSTER.startup(); + createTable(); } @AfterClass @@ -457,14 +492,21 @@ public class TestCustomSaslAuthenticationProvider { UTIL.shutdownMiniZKCluster(); } + @After + public void shutDownCluster() throws IOException { + if (CLUSTER != null) { + UTIL.deleteTable(name.getTableName()); + CLUSTER.shutdown(); + } + } + @Rule - public TestName name = new TestName(); + public TableNameTestRule name = new TableNameTestRule(); TableName tableName; String clusterId; - @Before public void createTable() throws Exception { - tableName = TableName.valueOf(name.getMethodName()); + tableName = name.getTableName(); // Create a table and write a record as the service user (hbase) UserGroupInformation serviceUgi = UserGroupInformation.loginUserFromKeytabAndReturnUGI( @@ -517,7 +559,7 @@ public class TestCustomSaslAuthenticationProvider { }); } - @Test(expected = RetriesExhaustedException.class) + @Test public void testNegativeAuthentication() throws Exception { // Validate that we can read that record back out as the user with our custom auth'n final Configuration clientConf = new Configuration(CONF); @@ -527,12 +569,26 @@ public class TestCustomSaslAuthenticationProvider { user1.addToken(createPasswordToken("user1", "definitely not the password", clusterId)); user1.doAs(new PrivilegedExceptionAction() { @Override public Void run() throws Exception { + // Depending on the registry in use, the following code can throw exceptions at different + // places. Master registry fails at the createConnection() step because the RPC to the + // master fails with sasl auth. With ZK registry, connection creation succeeds (since there + // is no RPC to HBase services involved) but the subsequent get() fails. The root cause + // should still be a SaslException in both the cases. try (Connection conn = ConnectionFactory.createConnection(clientConf); Table t = conn.getTable(tableName)) { t.get(new Get(Bytes.toBytes("r1"))); fail("Should not successfully authenticate with HBase"); - return null; + } catch (MasterRegistryFetchException mfe) { + Throwable cause = mfe.getCause(); + assertTrue(cause.getMessage(), cause.getMessage().contains("SaslException")); + } catch (RetriesExhaustedException re) { + assertTrue(re.getMessage(), re.getMessage().contains("SaslException")); + } catch (Exception e) { + // Any other exception is unexpected. + fail("Unexpected exception caught, was expecting a authentication error: " + + Throwables.getStackTraceAsString(e)); } + return null; } }); }