This is an automated email from the ASF dual-hosted git repository. linaataustin pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/sentry.git
The following commit(s) were added to refs/heads/master by this push: new f5cad4f SENTRY-2545: Rolling back Privilege Cache to SimplePrivilegeCache does not work f5cad4f is described below commit f5cad4fb15fa9b3aaf9d691e2df3065860b11353 Author: lina.li <lina...@cloudera.com> AuthorDate: Sat Dec 28 23:08:21 2019 -0600 SENTRY-2545: Rolling back Privilege Cache to SimplePrivilegeCache does not work --- .../hive/authz/HiveAuthzBindingHookBase.java | 19 +-- .../metastore/MetastoreAuthzBindingBase.java | 19 +-- .../cache/TestSimpleFilteredPrivilegeCache.java | 4 +- .../provider/cache/TestSimplePrivilegeCache.java | 146 +++++++++++++++++++++ .../hive/AbstractTestWithStaticConfiguration.java | 3 + .../e2e/hive/TestEndToEndWithSimpleCache.java | 27 ++++ .../TestMetastoreEndToEndWithSimpleCache.java | 31 +++++ 7 files changed, 231 insertions(+), 18 deletions(-) diff --git a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java index a4b664b..703ac5d 100644 --- a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java +++ b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java @@ -872,17 +872,20 @@ public abstract class HiveAuthzBindingHookBase extends AbstractSemanticAnalyzerH // load the privilege cache class that takes privilege factory as input Constructor<?> cacheConstructor = Class.forName(privilegeCacheName).getDeclaredConstructor(Set.class, PrivilegeFactory.class); - if (cacheConstructor != null) { + cacheConstructor.setAccessible(true); + return (PrivilegeCache) cacheConstructor. + newInstance(userPrivileges, inPrivilegeFactory); + } catch (NoSuchMethodException ex) { + try { + // load the privilege cache class that does not use privilege factory + Constructor<?> cacheConstructor = Class.forName(privilegeCacheName).getDeclaredConstructor(Set.class); cacheConstructor.setAccessible(true); return (PrivilegeCache) cacheConstructor. - newInstance(userPrivileges, inPrivilegeFactory); + newInstance(userPrivileges); + } catch (Exception ex2) { + LOG.error("Exception at creating privilege cache after second try", ex2); + throw ex; } - - // load the privilege cache class that does not use privilege factory - cacheConstructor = Class.forName(privilegeCacheName).getDeclaredConstructor(Set.class); - cacheConstructor.setAccessible(true); - return (PrivilegeCache) cacheConstructor. - newInstance(userPrivileges); } catch (Exception ex) { LOG.error("Exception at creating privilege cache", ex); throw ex; diff --git a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java index bc7a554..ea4788a 100644 --- a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java +++ b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java @@ -531,17 +531,20 @@ public abstract class MetastoreAuthzBindingBase extends MetaStorePreEventListene // load the privilege cache class that takes privilege factory as input Constructor<?> cacheConstructor = Class.forName(privilegeCacheName).getDeclaredConstructor(Set.class, PrivilegeFactory.class); - if (cacheConstructor != null) { + cacheConstructor.setAccessible(true); + return (PrivilegeCache) cacheConstructor. + newInstance(userPrivileges, inPrivilegeFactory); + } catch (NoSuchMethodException ex) { + try { + // load the privilege cache class that does not use privilege factory + Constructor<?> cacheConstructor = Class.forName(privilegeCacheName).getDeclaredConstructor(Set.class); cacheConstructor.setAccessible(true); return (PrivilegeCache) cacheConstructor. - newInstance(userPrivileges, inPrivilegeFactory); + newInstance(userPrivileges); + } catch (Exception ex2) { + LOG.error("Exception at creating privilege cache after second try", ex2); + throw ex; } - - // load the privilege cache class that does not use privilege factory - cacheConstructor = Class.forName(privilegeCacheName).getDeclaredConstructor(Set.class); - cacheConstructor.setAccessible(true); - return (PrivilegeCache) cacheConstructor. - newInstance(userPrivileges); } catch (Exception ex) { LOG.error("Exception at creating privilege cache", ex); throw ex; diff --git a/sentry-provider/sentry-provider-cache/src/test/java/org/apache/sentry/provider/cache/TestSimpleFilteredPrivilegeCache.java b/sentry-provider/sentry-provider-cache/src/test/java/org/apache/sentry/provider/cache/TestSimpleFilteredPrivilegeCache.java index 4615bd4..9d4a7b0 100644 --- a/sentry-provider/sentry-provider-cache/src/test/java/org/apache/sentry/provider/cache/TestSimpleFilteredPrivilegeCache.java +++ b/sentry-provider/sentry-provider-cache/src/test/java/org/apache/sentry/provider/cache/TestSimpleFilteredPrivilegeCache.java @@ -141,7 +141,7 @@ public class TestSimpleFilteredPrivilegeCache { } long end = System.currentTimeMillis(); - System.out.println("SimplePrivilegeCache - total time on list string: " + (end - start) + " ms"); + System.out.println("SimpleFilteredPrivilegeCache - total time on list string: " + (end - start) + " ms"); } @Test @@ -161,7 +161,7 @@ public class TestSimpleFilteredPrivilegeCache { } long end = System.currentTimeMillis(); - System.out.println("SimplePrivilegeCache - total time on list obj: " + (end - start) + " ms"); + System.out.println("SimpleFilteredPrivilegeCache - total time on list obj: " + (end - start) + " ms"); } Set<String> generatePrivilegeStrings(int dbCount, int tableCount) { diff --git a/sentry-provider/sentry-provider-cache/src/test/java/org/apache/sentry/provider/cache/TestSimplePrivilegeCache.java b/sentry-provider/sentry-provider-cache/src/test/java/org/apache/sentry/provider/cache/TestSimplePrivilegeCache.java new file mode 100644 index 0000000..2926484 --- /dev/null +++ b/sentry-provider/sentry-provider-cache/src/test/java/org/apache/sentry/provider/cache/TestSimplePrivilegeCache.java @@ -0,0 +1,146 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.sentry.provider.cache; + +import static org.junit.Assert.assertEquals; + +import com.google.common.collect.Sets; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import org.apache.sentry.core.common.Authorizable; +import org.apache.sentry.core.common.utils.KeyValue; +import org.apache.sentry.core.common.utils.SentryConstants; +import org.apache.sentry.core.model.db.Database; +import org.apache.sentry.core.model.db.Server; +import org.apache.sentry.core.model.db.Table; +import org.apache.sentry.policy.common.CommonPrivilege; +import org.junit.Ignore; +import org.junit.Test; + +public class TestSimplePrivilegeCache { + @Test + public void testListPrivilegesCaseSensitivity() { + CommonPrivilege dbSelect = create(new KeyValue("Server", "Server1"), + new KeyValue("db", "db1"), new KeyValue("action", "SELECT")); + + SimplePrivilegeCache cache = new SimplePrivilegeCache(Sets.newHashSet(dbSelect.toString())); + assertEquals(1, cache.listPrivileges(null, null, null).size()); + } + + @Test + public void testListPrivilegesWildCard() { + CommonPrivilege t1D1Select = create(new KeyValue("Server", "server1"), + new KeyValue("db", "db1"), new KeyValue("table", "t1"), new KeyValue("action", "SELECT")); + CommonPrivilege t1D2Select = create(new KeyValue("Server", "server1"), + new KeyValue("db", "db2"), new KeyValue("table", "t1"), new KeyValue("action", "SELECT")); + CommonPrivilege t2Select = create(new KeyValue("Server", "server1"), + new KeyValue("db", "db1"), new KeyValue("table", "t2"), new KeyValue("action", "SELECT")); + CommonPrivilege wildCardTable = create(new KeyValue("Server", "server1"), + new KeyValue("db", "db1"), new KeyValue("table", "*"), new KeyValue("action", "SELECT")); + CommonPrivilege allTable = create(new KeyValue("Server", "server1"), + new KeyValue("db", "db1"), new KeyValue("table", "ALL"), new KeyValue("action", "SELECT")); + CommonPrivilege allDatabase = create(new KeyValue("Server", "server1"), + new KeyValue("db", "*")); + CommonPrivilege colSelect = create(new KeyValue("Server", "server1"), + new KeyValue("db", "db1"), new KeyValue("table", "t1"), new KeyValue("column", "c1"), new KeyValue("action", "SELECT")); + + SimplePrivilegeCache cache = new SimplePrivilegeCache(Sets.newHashSet(t1D1Select.toString(), + t1D2Select.toString(), t2Select.toString(), wildCardTable.toString(), allTable.toString(), + allDatabase.toString(), colSelect.toString())); + + assertEquals(7, cache.listPrivileges(null, null, null).size()); + } + + @Test + public void testListPrivilegesURI() { + CommonPrivilege uri1Select = create(new KeyValue("Server", "server1"), + new KeyValue("uri", "hdfs:///uri/path1")); + CommonPrivilege uri2Select = create(new KeyValue("Server", "server1"), + new KeyValue("uri", "hdfs:///uri/path2")); + + SimplePrivilegeCache cache = new SimplePrivilegeCache(Sets.newHashSet(uri1Select.toString(), + uri2Select.toString())); + + assertEquals(2, cache.listPrivileges(null, null, null).size()); + } + + @Test + @Ignore("This test should be run manually.") + public void testListPrivilegesPerf() { + int priDbCount = 1000; + int priTableCount = 10; + int authDbCount = 1200; + int authTableCount = 1; + Set<String> privileges = generatePrivilegeStrings(priDbCount, priTableCount); + SimplePrivilegeCache cache = new SimplePrivilegeCache(privileges); + List<Authorizable[]> authorizables = generateAuthoriables(12000, 10); + System.out.println( + "SimplePrivilegeCache - {privileges: db - " + priDbCount + ", table - " + priTableCount + "}, {authorizable: db - " + authDbCount + ", table - " + authTableCount + "}"); + + long start = System.currentTimeMillis(); + for (Authorizable[] authorizableHierarchy : authorizables) { + Set<String> priStrings = cache.listPrivileges(null, null, null); + for (String priString : priStrings) { + CommonPrivilege currPri = create(priString); + currPri.getParts(); + } + } + long end = System.currentTimeMillis(); + + System.out.println("SimplePrivilegeCache - total time on list string: " + (end - start) + " ms"); + } + + Set<String> generatePrivilegeStrings(int dbCount, int tableCount) { + Set<String> priStrings = new HashSet<>(); + for (int i = 0; i < dbCount; i ++) { + for (int j = 0; j < tableCount; j ++) { + String priString = "Server=server1->Database=db" + i + "->Table=table" + j; + priStrings.add(priString); + } + } + + return priStrings; + } + + List<Authorizable[]> generateAuthoriables(int dbCount, int tableCount) { + List<Authorizable[]> authorizables = new ArrayList<>(); + + for (int i = 0; i < dbCount; i ++) { + for (int j = 0; j < tableCount; j ++) { + Authorizable[] authorizable = new Authorizable[3]; + authorizable[0] = new Server("server1"); + authorizable[1] = new Database("db" + i); + authorizable[2] = new Table("table" + j); + + authorizables.add(authorizable); + } + } + + return authorizables; + } + + + static CommonPrivilege create(KeyValue... keyValues) { + return create(SentryConstants.AUTHORIZABLE_JOINER.join(keyValues)); + } + + static CommonPrivilege create(String s) { + return new CommonPrivilege(s); + } +} diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java index 86e7d14..a3ae0fc 100644 --- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java +++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java @@ -35,6 +35,7 @@ import java.util.HashSet; import com.google.common.collect.Sets; import org.apache.hive.hcatalog.listener.DbNotificationListener; +import org.apache.sentry.binding.hive.conf.HiveAuthzConf; import org.apache.sentry.binding.hive.conf.HiveAuthzConf.AuthzConfVars; import org.apache.sentry.binding.metastore.messaging.json.SentryJSONMessageFactory; import org.apache.sentry.api.common.ApiConstants.ClientConfig; @@ -147,6 +148,7 @@ public abstract class AbstractTestWithStaticConfiguration extends RulesForE2ETes protected static boolean showDbOnSelectOnly = false; protected static boolean showTableOnSelectOnly = false; protected static boolean restrictDefaultDatabase = false; + protected static String cacheClassName = HiveAuthzConf.AuthzConfVars.AUTHZ_PRIVILEGE_CACHE.getDefault(); protected static File baseDir; protected static File logDir; @@ -320,6 +322,7 @@ public abstract class AbstractTestWithStaticConfiguration extends RulesForE2ETes properties.put(AuthzConfVars.SHOWDATABASES_ON_SELECT_ONLY.getVar(), String.valueOf(showDbOnSelectOnly)); properties.put(AuthzConfVars.SHOWTABLES_ON_SELECT_ONLY.getVar(), String.valueOf(showTableOnSelectOnly)); properties.put(AuthzConfVars.AUTHZ_RESTRICT_DEFAULT_DB.getVar(), String.valueOf(restrictDefaultDatabase)); + properties.put(AuthzConfVars.AUTHZ_PRIVILEGE_CACHE.getVar(), cacheClassName); if (useSentryService && (!startSentry)) { configureHiveAndMetastoreForSentry(); diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestEndToEndWithSimpleCache.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestEndToEndWithSimpleCache.java new file mode 100644 index 0000000..fe98def --- /dev/null +++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestEndToEndWithSimpleCache.java @@ -0,0 +1,27 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.sentry.tests.e2e.hive; + +import org.junit.BeforeClass; + +public class TestEndToEndWithSimpleCache extends TestEndToEnd { + @BeforeClass + public static void setupTestStaticConfiguration() throws Exception { + cacheClassName = "org.apache.sentry.provider.cache.SimplePrivilegeCache"; + TestEndToEnd.setupTestStaticConfiguration(); + } +} diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreEndToEndWithSimpleCache.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreEndToEndWithSimpleCache.java new file mode 100644 index 0000000..4e9d4a6 --- /dev/null +++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreEndToEndWithSimpleCache.java @@ -0,0 +1,31 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.sentry.tests.e2e.metastore; + +import org.junit.BeforeClass; + +/** + * This test when privilege cache uses simple cache, the metastore authorization works end to end + */ +public class TestMetastoreEndToEndWithSimpleCache extends TestMetastoreEndToEnd { + @BeforeClass + public static void setupTestStaticConfiguration() throws Exception { + cacheClassName = "org.apache.sentry.provider.cache.SimplePrivilegeCache"; + TestMetastoreEndToEnd.setupTestStaticConfiguration(); + } +}