You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2020/05/18 10:28:28 UTC
[GitHub] [hive] pkumarsinha commented on a change in pull request #1016: HIVE-23433 : Add Deny Policy on Replicated Database
pkumarsinha commented on a change in pull request #1016:
URL: https://github.com/apache/hive/pull/1016#discussion_r426523080
##########
File path: ql/src/test/org/apache/hadoop/hive/ql/exec/repl/TestRangerLoadTask.java
##########
@@ -148,4 +152,68 @@ public void testSuccessRangerDumpMetrics() throws Exception {
.getAllValues().get(1).toString().contains("{\"sourceDbName\":\"srcdb\",\"targetDbName\""
+ ":\"tgtdb\",\"actualNumPolicies\":1,\"loadEndTime\""));
}
+
+ @Test
+ public void testSuccessAddDenyRangerPolicies() throws Exception {
+ String rangerResponse = "{\"metaDataInfo\":{\"Host name\":\"ranger.apache.org\","
+ + "\"Exported by\":\"hive\",\"Export time\":\"May 5, 2020, 8:55:03 AM\",\"Ranger apache version\""
+ + ":\"2.0.0.7.2.0.0-61\"},\"policies\":[{\"service\":\"hive\",\"name\":\"db-level\",\"policyType\":0,"
+ + "\"description\":\"\",\"isAuditEnabled\":true,\"resources\":{\"database\":{\"values\":[\"aa\"],"
+ + "\"isExcludes\":false,\"isRecursive\":false},\"column\":{\"values\":[\"id\"],\"isExcludes\":false,"
+ + "\"isRecursive\":false},\"table\":{\"values\":[\"*\"],\"isExcludes\":false,\"isRecursive\":false}},"
+ + "\"policyItems\":[{\"accesses\":[{\"type\":\"select\",\"isAllowed\":true},{\"type\":\"update\","
+ + "\"isAllowed\":true}],\"users\":[\"admin\"],\"groups\":[\"public\"],\"conditions\":[],"
+ + "\"delegateAdmin\":false}],\"denyPolicyItems\":[],\"allowExceptions\":[],\"denyExceptions\":[],"
+ + "\"dataMaskPolicyItems\":[],\"rowFilterPolicyItems\":[],\"id\":40,\"guid\":"
+ + "\"4e2b3406-7b9a-4004-8cdf-7a239c8e2cae\",\"isEnabled\":true,\"version\":1}]}";
+ RangerExportPolicyList rangerPolicyList = new Gson().fromJson(rangerResponse, RangerExportPolicyList.class);
+ Mockito.when(conf.getVar(REPL_AUTHORIZATION_PROVIDER_SERVICE_ENDPOINT)).thenReturn("rangerEndpoint");
+ Mockito.when(work.getSourceDbName()).thenReturn("srcdb");
+ Mockito.when(work.getTargetDbName()).thenReturn("tgtdb");
+ Mockito.when(conf.getVar(REPL_RANGER_SERVICE_NAME)).thenReturn("hive");
+ Path rangerDumpPath = new Path("/tmp");
+ Mockito.when(work.getCurrentDumpPath()).thenReturn(rangerDumpPath);
+ mockClient.saveRangerPoliciesToFile(rangerPolicyList,
Review comment:
Is this line needed? you anyway returning policyList on readRangerPoliciesFromJsonFile call: 178
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ranger/RangerRestClientImpl.java
##########
@@ -349,6 +354,90 @@ public boolean checkConnection(String url) {
return (clientResp.getStatus() < HttpServletResponse.SC_UNAUTHORIZED);
}
+ @Override
+ public List<RangerPolicy> addDenyPolicies(List<RangerPolicy> rangerPolicies, String rangerServiceName,
+ String sourceDb, String targetDb) {
+ List<RangerPolicy> rangerPoliciesToImport = new ArrayList<RangerPolicy>();
Review comment:
Why is this new one needed? Do we need to preserve the list obtained from exported policies?
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/repl/RangerLoadTask.java
##########
@@ -104,9 +104,13 @@ public int execute() {
LOG.info("There are no ranger policies to import");
rangerPolicies = new ArrayList<>();
}
- List<RangerPolicy> updatedRangerPolicies = rangerRestClient.changeDataSet(rangerPolicies, work.getSourceDbName(),
- work.getTargetDbName());
- long importCount = 0;
+ List<RangerPolicy> rangerPoliciesWithDenyPolicy = rangerRestClient.addDenyPolicies(rangerPolicies,
Review comment:
Shouldn't this be controlled by a config? We have seen in the past where the customer needed to disable this.
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ranger/RangerRestClientImpl.java
##########
@@ -349,6 +354,90 @@ public boolean checkConnection(String url) {
return (clientResp.getStatus() < HttpServletResponse.SC_UNAUTHORIZED);
}
+ @Override
+ public List<RangerPolicy> addDenyPolicies(List<RangerPolicy> rangerPolicies, String rangerServiceName,
+ String sourceDb, String targetDb) {
+ List<RangerPolicy> rangerPoliciesToImport = new ArrayList<RangerPolicy>();
+ if (CollectionUtils.isNotEmpty(rangerPolicies)) {
+ for (RangerPolicy rangerPolicy : rangerPolicies) {
+ rangerPoliciesToImport.add(rangerPolicy);
+ }
+ }
+
+ RangerPolicy denyRangerPolicy = null;
+
+ if (sourceDb.endsWith("/")) {
+ sourceDb = StringUtils.removePattern(sourceDb, "/+$");
Review comment:
Why is this needed?
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ranger/RangerRestClientImpl.java
##########
@@ -349,6 +354,90 @@ public boolean checkConnection(String url) {
return (clientResp.getStatus() < HttpServletResponse.SC_UNAUTHORIZED);
}
+ @Override
+ public List<RangerPolicy> addDenyPolicies(List<RangerPolicy> rangerPolicies, String rangerServiceName,
+ String sourceDb, String targetDb) {
+ List<RangerPolicy> rangerPoliciesToImport = new ArrayList<RangerPolicy>();
+ if (CollectionUtils.isNotEmpty(rangerPolicies)) {
+ for (RangerPolicy rangerPolicy : rangerPolicies) {
+ rangerPoliciesToImport.add(rangerPolicy);
+ }
+ }
+
+ RangerPolicy denyRangerPolicy = null;
+
+ if (sourceDb.endsWith("/")) {
+ sourceDb = StringUtils.removePattern(sourceDb, "/+$");
+ }
+ if (targetDb.endsWith("/")) {
+ targetDb = StringUtils.removePattern(targetDb, "/+$");
+ }
+ if (!StringUtils.isEmpty(rangerServiceName)) {
+ denyRangerPolicy = new RangerPolicy();
+ denyRangerPolicy.setService(rangerServiceName);
+ denyRangerPolicy.setName(sourceDb + "_replication deny policy for " + targetDb);
+ }
+ if (denyRangerPolicy != null) {
+ Map<String, RangerPolicy.RangerPolicyResource> rangerPolicyResourceMap = new HashMap<String, RangerPolicy.RangerPolicyResource>();
+ RangerPolicy.RangerPolicyResource rangerPolicyResource = new RangerPolicy.RangerPolicyResource();
+ List<String> resourceNameList = new ArrayList<String>();
+
+ List<RangerPolicy.RangerPolicyItem> denyPolicyItemsForPublicGroup = denyRangerPolicy.getDenyPolicyItems();
+ RangerPolicy.RangerPolicyItem denyPolicyItem = new RangerPolicy.RangerPolicyItem();
+ List<RangerPolicy.RangerPolicyItemAccess> denyPolicyItemAccesses = new ArrayList<RangerPolicy.RangerPolicyItemAccess>();
+
+ List<RangerPolicy.RangerPolicyItem> denyExceptionsItemsForBeaconUser = denyRangerPolicy.getDenyExceptions();
+ RangerPolicy.RangerPolicyItem denyExceptionsPolicyItem = new RangerPolicy.RangerPolicyItem();
+ List<RangerPolicy.RangerPolicyItemAccess> denyExceptionsPolicyItemAccesses = new ArrayList<RangerPolicy.RangerPolicyItemAccess>();
+
+ resourceNameList.add(sourceDb);
+ rangerPolicyResource.setValues(resourceNameList);
+ RangerPolicy.RangerPolicyResource rangerPolicyResourceColumn =new RangerPolicy.RangerPolicyResource();
+ rangerPolicyResourceColumn.setValues(new ArrayList<String>(){{add("*"); }});
+ RangerPolicy.RangerPolicyResource rangerPolicyResourceTable =new RangerPolicy.RangerPolicyResource();
+ rangerPolicyResourceTable.setValues(new ArrayList<String>(){{add("*"); }});
+ rangerPolicyResourceMap.put("database", rangerPolicyResource);
+ rangerPolicyResourceMap.put("table", rangerPolicyResourceTable);
+ rangerPolicyResourceMap.put("column", rangerPolicyResourceColumn);
+ denyRangerPolicy.setResources(rangerPolicyResourceMap);
+
+ denyPolicyItemAccesses.add(new RangerPolicy.RangerPolicyItemAccess("create", true));
Review comment:
Line: 404:430 can be refactored as all the lines are essentially doing the same thing with different access types.
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ranger/RangerRestClientImpl.java
##########
@@ -349,6 +354,90 @@ public boolean checkConnection(String url) {
return (clientResp.getStatus() < HttpServletResponse.SC_UNAUTHORIZED);
}
+ @Override
+ public List<RangerPolicy> addDenyPolicies(List<RangerPolicy> rangerPolicies, String rangerServiceName,
+ String sourceDb, String targetDb) {
+ List<RangerPolicy> rangerPoliciesToImport = new ArrayList<RangerPolicy>();
+ if (CollectionUtils.isNotEmpty(rangerPolicies)) {
+ for (RangerPolicy rangerPolicy : rangerPolicies) {
+ rangerPoliciesToImport.add(rangerPolicy);
+ }
+ }
+
+ RangerPolicy denyRangerPolicy = null;
+
+ if (sourceDb.endsWith("/")) {
+ sourceDb = StringUtils.removePattern(sourceDb, "/+$");
+ }
+ if (targetDb.endsWith("/")) {
+ targetDb = StringUtils.removePattern(targetDb, "/+$");
+ }
+ if (!StringUtils.isEmpty(rangerServiceName)) {
+ denyRangerPolicy = new RangerPolicy();
+ denyRangerPolicy.setService(rangerServiceName);
+ denyRangerPolicy.setName(sourceDb + "_replication deny policy for " + targetDb);
+ }
+ if (denyRangerPolicy != null) {
+ Map<String, RangerPolicy.RangerPolicyResource> rangerPolicyResourceMap = new HashMap<String, RangerPolicy.RangerPolicyResource>();
+ RangerPolicy.RangerPolicyResource rangerPolicyResource = new RangerPolicy.RangerPolicyResource();
+ List<String> resourceNameList = new ArrayList<String>();
+
+ List<RangerPolicy.RangerPolicyItem> denyPolicyItemsForPublicGroup = denyRangerPolicy.getDenyPolicyItems();
+ RangerPolicy.RangerPolicyItem denyPolicyItem = new RangerPolicy.RangerPolicyItem();
+ List<RangerPolicy.RangerPolicyItemAccess> denyPolicyItemAccesses = new ArrayList<RangerPolicy.RangerPolicyItemAccess>();
+
+ List<RangerPolicy.RangerPolicyItem> denyExceptionsItemsForBeaconUser = denyRangerPolicy.getDenyExceptions();
+ RangerPolicy.RangerPolicyItem denyExceptionsPolicyItem = new RangerPolicy.RangerPolicyItem();
+ List<RangerPolicy.RangerPolicyItemAccess> denyExceptionsPolicyItemAccesses = new ArrayList<RangerPolicy.RangerPolicyItemAccess>();
+
+ resourceNameList.add(sourceDb);
+ rangerPolicyResource.setValues(resourceNameList);
+ RangerPolicy.RangerPolicyResource rangerPolicyResourceColumn =new RangerPolicy.RangerPolicyResource();
+ rangerPolicyResourceColumn.setValues(new ArrayList<String>(){{add("*"); }});
+ RangerPolicy.RangerPolicyResource rangerPolicyResourceTable =new RangerPolicy.RangerPolicyResource();
+ rangerPolicyResourceTable.setValues(new ArrayList<String>(){{add("*"); }});
+ rangerPolicyResourceMap.put("database", rangerPolicyResource);
+ rangerPolicyResourceMap.put("table", rangerPolicyResourceTable);
+ rangerPolicyResourceMap.put("column", rangerPolicyResourceColumn);
+ denyRangerPolicy.setResources(rangerPolicyResourceMap);
+
+ denyPolicyItemAccesses.add(new RangerPolicy.RangerPolicyItemAccess("create", true));
+ denyPolicyItemAccesses.add(new RangerPolicy.RangerPolicyItemAccess("update", true));
+ denyPolicyItemAccesses.add(new RangerPolicy.RangerPolicyItemAccess("drop", true));
+ denyPolicyItemAccesses.add(new RangerPolicy.RangerPolicyItemAccess("alter", true));
+ denyPolicyItemAccesses.add(new RangerPolicy.RangerPolicyItemAccess("index", true));
+ denyPolicyItemAccesses.add(new RangerPolicy.RangerPolicyItemAccess("lock", true));
+ denyPolicyItemAccesses.add(new RangerPolicy.RangerPolicyItemAccess("write", true));
+ denyPolicyItemAccesses.add(new RangerPolicy.RangerPolicyItemAccess("ReplAdmin", true));
+ denyPolicyItem.setAccesses(denyPolicyItemAccesses);
+ denyPolicyItemsForPublicGroup.add(denyPolicyItem);
+ List<String> denyPolicyItemsGroups = new ArrayList<String>();
+ denyPolicyItemsGroups.add("public");
+ denyPolicyItem.setGroups(denyPolicyItemsGroups);
+ denyRangerPolicy.setDenyPolicyItems(denyPolicyItemsForPublicGroup);
+
+ denyExceptionsPolicyItemAccesses.add(new RangerPolicy.RangerPolicyItemAccess("create", true));
+ denyExceptionsPolicyItemAccesses.add(new RangerPolicy.RangerPolicyItemAccess("update", true));
+ denyExceptionsPolicyItemAccesses.add(new RangerPolicy.RangerPolicyItemAccess("drop", true));
+ denyExceptionsPolicyItemAccesses.add(new RangerPolicy.RangerPolicyItemAccess("alter", true));
+ denyExceptionsPolicyItemAccesses.add(new RangerPolicy.RangerPolicyItemAccess("index", true));
+ denyExceptionsPolicyItemAccesses.add(new RangerPolicy.RangerPolicyItemAccess("lock", true));
+ denyExceptionsPolicyItemAccesses.add(new RangerPolicy.RangerPolicyItemAccess("write", true));
+ denyExceptionsPolicyItemAccesses.add(new RangerPolicy.RangerPolicyItemAccess("select", true));
+ denyExceptionsPolicyItemAccesses.add(new RangerPolicy.RangerPolicyItemAccess("read", true));
+ denyExceptionsPolicyItemAccesses.add(new RangerPolicy.RangerPolicyItemAccess("ReplAdmin", true));
Review comment:
Aren't we handling ReplAdmin in another patch?
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ranger/RangerRestClientImpl.java
##########
@@ -349,6 +354,90 @@ public boolean checkConnection(String url) {
return (clientResp.getStatus() < HttpServletResponse.SC_UNAUTHORIZED);
}
+ @Override
+ public List<RangerPolicy> addDenyPolicies(List<RangerPolicy> rangerPolicies, String rangerServiceName,
+ String sourceDb, String targetDb) {
+ List<RangerPolicy> rangerPoliciesToImport = new ArrayList<RangerPolicy>();
+ if (CollectionUtils.isNotEmpty(rangerPolicies)) {
+ for (RangerPolicy rangerPolicy : rangerPolicies) {
+ rangerPoliciesToImport.add(rangerPolicy);
+ }
+ }
+
+ RangerPolicy denyRangerPolicy = null;
+
+ if (sourceDb.endsWith("/")) {
+ sourceDb = StringUtils.removePattern(sourceDb, "/+$");
+ }
+ if (targetDb.endsWith("/")) {
+ targetDb = StringUtils.removePattern(targetDb, "/+$");
+ }
+ if (!StringUtils.isEmpty(rangerServiceName)) {
Review comment:
Deny policy is added only when !StringUtils.isEmpty(rangerServiceName), this check should be done at the beginning?
Also, REPL_RANGER_SERVICE_NAME is a mandatory config during ranger repl dump. Shouldn't this be mandatory at load as well? Wondering how does service mapping work in that case.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org