> On Jan. 28, 2016, 7:39 p.m., Anne Yu wrote: > > sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/CommonPrivilege.java, > > line 51 > > <https://reviews.apache.org/r/42287/diff/2/?file=1224483#file1224483line51> > > > > Just high level design question, not intend to block the check-in, but > > would be appreciated if can help me understand more: > > > > implyMethodMap: what does method mean here, could you elaborate, is > > this a kind of action? to me, url is a string also. could you also add more > > java doc. > > > > actionMap: is there a possibily that more privileges could be mapped > > here? i meant since it's an integer, each bit might have meaings here (for > > kalfka, solr etc). Do you catch the full information somewhere? Or just put > > more java doc here. Thanks!
Thanks for the review, the following is the description about the high level design, but for the implementation, I'll do more improvement, because using Map<String, String> isn't good way for scalability. 1. Currently, in DBWildcardPrivilege, to compare the dbName, tableName, columnName, String.equals() is responsible for it, but for url, PathUtils.impliesURI() is responsible for it. The implyMethodMap is created to resolve this problem, store the flag for resource, for example, db -> string, url -> url, when CommonPrivilege do the imply(), it get resourceName db, check the implyMethodMap, then, String.equals() will be used to compare the value. If get the resourceName url, PathUtils.impliesURI() will be used. 2. For the actionMap, it isn't for all components, every component should have its own actionMap. I think int have 32 bits, and it should be enough for any component. Currently, the patch is just for the CommonPrivilege, more information for the actionMap will be added to the specific component in the following JIRA. In my mind, these 2 maps should be defined in eg, sentry-binding-hive, sentry-binding-solr. - Colin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42287/#review116806 ----------------------------------------------------------- On Jan. 28, 2016, 1:57 a.m., Colin Ma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42287/ > ----------------------------------------------------------- > > (Updated Jan. 28, 2016, 1:57 a.m.) > > > Review request for sentry. > > > Repository: sentry > > > Description > ------- > > Create CommonPrivilege for every component. > The main change is for the interface Privilege, implies(Privilege p) ---> > implies(Privilege privilege, Map<String, String> implyMethodMap, Map<String, > Integer> actionMap) > The actionMap is stored the mapping between action name and action value, eg: > for Hive: select ---> 1 (binary: 0000001) > insert ---> 3 (binary: 0000011) > create ---> 4 (binary: 0000100) > all ---> 7 (binary: 0000111) > when compare the action, use the action value with the & operator > The implyMethodMap is stored the mapping between resource type and imply > method, eg: > for Hive: db ---> string (imply as string value) > url ---> url (imply as url value) > > > Diffs > ----- > > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryIniPolicyFileFormatter.java > 79164da > > sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestSentryIniPolicyFileFormatter.java > 655417b > sentry-policy/sentry-policy-common/pom.xml 68ada23 > > sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/CommonPrivilege.java > PRE-CREATION > > sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/KeyValue.java > PRE-CREATION > > sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/PrivilegeConstants.java > PRE-CREATION > > sentry-policy/sentry-policy-common/src/test/java/org/apache/sentry/policy/common/TestCommonPrivilege.java > PRE-CREATION > > sentry-policy/sentry-policy-common/src/test/java/org/apache/sentry/policy/common/TestKeyValue.java > PRE-CREATION > > sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/AbstractDBPrivilegeValidator.java > e940fc3 > > sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/DBModelAuthorizables.java > f07eb11 > > sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/DBWildcardPrivilege.java > dfc2872 > > sentry-policy/sentry-policy-db/src/test/java/org/apache/sentry/policy/db/TestDBWildcardPrivilege.java > bf5cec5 > > sentry-policy/sentry-policy-indexer/src/main/java/org/apache/sentry/policy/indexer/AbstractIndexerPrivilegeValidator.java > 8520d1a > > sentry-policy/sentry-policy-indexer/src/main/java/org/apache/sentry/policy/indexer/IndexerModelAuthorizables.java > e561962 > > sentry-policy/sentry-policy-indexer/src/main/java/org/apache/sentry/policy/indexer/IndexerWildcardPrivilege.java > ab6b27f > > sentry-policy/sentry-policy-indexer/src/test/java/org/apache/sentry/policy/indexer/TestIndexerWildcardPrivilege.java > 5348f95 > > sentry-policy/sentry-policy-search/src/main/java/org/apache/sentry/policy/search/AbstractSearchPrivilegeValidator.java > 781e722 > > sentry-policy/sentry-policy-search/src/main/java/org/apache/sentry/policy/search/SearchModelAuthorizables.java > dcf17a2 > > sentry-policy/sentry-policy-search/src/main/java/org/apache/sentry/policy/search/SearchWildcardPrivilege.java > c522412 > > sentry-policy/sentry-policy-search/src/test/java/org/apache/sentry/policy/search/TestSearchWildcardPrivilege.java > 125f358 > > sentry-policy/sentry-policy-sqoop/src/main/java/org/apache/sentry/policy/sqoop/ServerNameRequiredMatch.java > bbbcedd > > sentry-policy/sentry-policy-sqoop/src/main/java/org/apache/sentry/policy/sqoop/SqoopModelAuthorizables.java > 223fb55 > > sentry-policy/sentry-policy-sqoop/src/main/java/org/apache/sentry/policy/sqoop/SqoopWildcardPrivilege.java > 139cf7f > > sentry-policy/sentry-policy-sqoop/src/test/java/org/apache/sentry/policy/sqoop/TestSqoopWildcardPrivilege.java > 1f03f05 > > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/KeyValue.java > 984fe46 > > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ProviderConstants.java > c6f7e2c > > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java > fef4bd9 > > sentry-provider/sentry-provider-common/src/test/java/org/apache/sentry/provider/common/TestKeyValue.java > 1ae4c0c > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeObject.java > c6e4aa6 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java > 45f9ce4 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryGMPrivilege.java > 56bbb8f > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > 530bdc7 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/CommandUtil.java > ffccec2 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/ListPrivilegesCmd.java > 98fae95 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java > 46798a0 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceImportExport.java > 9d0a2d6 > > sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/LocalGroupMappingService.java > 1c12f11 > > sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java > 3a648a5 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java > 614856f > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPolicyImportExport.java > 2482eb4 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/SentryPolicyProviderForDb.java > d0994b6 > > Diff: https://reviews.apache.org/r/42287/diff/ > > > Testing > ------- > > > Thanks, > > Colin Ma > >
