mneethiraj commented on code in PR #847:
URL: https://github.com/apache/ranger/pull/847#discussion_r2921217121
##########
agents-common/src/test/java/org/apache/ranger/plugin/audit/TestRangerDefaultAuditHandler.java:
##########
@@ -286,11 +286,13 @@ public void test07_getDatasetsAndProjects_fromContext() {
public void test08_getAdditionalInfo_nullAndNonNull() {
RangerDefaultAuditHandler handler = new RangerDefaultAuditHandler();
RangerAccessRequestImpl req = new RangerAccessRequestImpl();
+ RangerAccessResult res = new RangerAccessResult(0, "svc", null,
req);
Review Comment:
`res` and `res1` are not used. Consider reverting changes in
`TestRangerDefaultAuditHandler`.
##########
dev-support/ranger-docker/scripts/audit-server/ranger-audit-consumer-hdfs.sh:
##########
@@ -0,0 +1,138 @@
+#!/bin/bash
+
+# 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.
+
+set -e
+
+AUDIT_CONSUMER_HOME_DIR="${AUDIT_CONSUMER_HOME_DIR:-/opt/ranger-audit-consumer-hdfs}"
Review Comment:
`/opt/ranger-audit-consumer-hdfs` => `/opt/ranger/audit-consumer-hdfs` - to
be consistent with installation location of other Ranger services -
usersync/tagsync/kms.
##########
ranger-audit-server/ranger-audit-common/src/main/java/org/apache/ranger/audit/consumer/kafka/AuditConsumer.java:
##########
@@ -0,0 +1,78 @@
+/*
Review Comment:
To be consistent with directory names of other servers (kms, tagsync,
ugsync), I suggest following renaming:
- ranger-audit-server => audit-server
- ranger-audit-server/ranger-audit-common => audit-server/common
- ranger-audit-server/ranger-audit-consumer-hdfs =>
audit-server/consumer-hdfs
- ranger-audit-server/ranger-audit-consumer-solr =>
audit-server/consumer-solr
##########
agents-common/src/main/java/org/apache/ranger/plugin/service/RangerBasePlugin.java:
##########
@@ -400,7 +401,20 @@ public void init() {
if (!providerFactory.isInitDone()) {
if (pluginConfig.getProperties() != null) {
- providerFactory.init(pluginConfig.getProperties(), getAppId());
+ Properties baseProps = pluginConfig.getProperties();
+ Properties auditProps = new Properties();
+ auditProps.putAll(baseProps);
+ String serviceType = getServiceType();
+ if (StringUtils.isNotEmpty(serviceType)) {
+ auditProps.setProperty("ranger.plugin.audit.service.type",
serviceType);
Review Comment:
`ranger.plugin.audit.service.type` and `ranger.plugin.audit.app.id` don't
seem to be used any more. Please review and revert changes in
`RangerBasePlugin`.
##########
ranger-audit-server/ranger-audit-common/src/main/java/org/apache/ranger/audit/consumer/kafka/AuditConsumerBase.java:
##########
@@ -0,0 +1,101 @@
+/*
+ * 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.ranger.audit.consumer.kafka;
+
+import org.apache.kafka.clients.admin.AdminClientConfig;
+import org.apache.kafka.clients.consumer.ConsumerConfig;
+import org.apache.kafka.clients.consumer.KafkaConsumer;
+import org.apache.ranger.audit.provider.MiscUtil;
+import org.apache.ranger.audit.server.AuditServerConstants;
+import org.apache.ranger.audit.utils.AuditMessageQueueUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Properties;
+
+public class AuditConsumerBase {
+ private static final Logger LOG =
LoggerFactory.getLogger(AuditConsumerBase.class);
+
+ public Properties consumerProps = new Properties();
Review Comment:
- consider marking all fields as `final`
- consider reducing the visibility from `public` to `private` (or
`protected` - if the derived classes should have direct access to these fields).
##########
dev-support/ranger-docker/scripts/admin/create-ranger-services.py:
##########
@@ -27,6 +27,11 @@ def service_not_exists(service):
'default-policy.1.resource.path.is-recursive': 'true',
'default-policy.1.policyItem.1.users':
'hive',
'default-policy.1.policyItem.1.accessTypes':
'read,write,execute',
+ 'default-policy.2.name': 'ranger-audit-path',
+ 'default-policy.2.resource.path':
'/ranger/audit/*,/ranger/audit',
Review Comment:
Is `/ranger/audit/*` necessary, given `is-recursive` is set to `true` below?
##########
dev-support/ranger-docker/scripts/kdc/entrypoint.sh:
##########
@@ -74,6 +74,19 @@ function create_keytabs() {
create_principal_and_keytab rangeradmin ranger
create_principal_and_keytab rangerlookup ranger
+ create_principal_and_keytab HTTP ranger-audit
Review Comment:
Principals and keytabs for audit server are created in lines 81 & 82; lines
77 and 78 seem unnecessary.
##########
dev-support/ranger-docker/scripts/hadoop/ranger-hdfs-plugin-install.properties:
##########
@@ -90,3 +90,7 @@
SSL_KEYSTORE_FILE_PATH=/etc/hadoop/conf/ranger-plugin-keystore.jks
SSL_KEYSTORE_PASSWORD=myKeyFilePassword
SSL_TRUSTSTORE_FILE_PATH=/etc/hadoop/conf/ranger-plugin-truststore.jks
SSL_TRUSTSTORE_PASSWORD=changeit
+
+XAAUDIT.AUDITSERVER.ENABLE=true
+XAAUDIT.AUDITSERVER.URL=http://ranger-audit-server.rangernw:7081
+XAAUDIT.AUDITSERVER.FILE_SPOOL_DIR=/var/log/hadoop/hdfs/audit/http/spool
Review Comment:
`http` => `audit-server`
##########
dev-support/ranger-docker/scripts/audit-server/ranger-audit-consumer-hdfs.sh:
##########
@@ -0,0 +1,138 @@
+#!/bin/bash
+
+# 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.
+
+set -e
+
+AUDIT_CONSUMER_HOME_DIR="${AUDIT_CONSUMER_HOME_DIR:-/opt/ranger-audit-consumer-hdfs}"
+AUDIT_CONSUMER_CONF_DIR="${AUDIT_CONSUMER_CONF_DIR:-/opt/ranger-audit-consumer-hdfs/conf}"
+AUDIT_CONSUMER_LOG_DIR="${AUDIT_CONSUMER_LOG_DIR:-/var/log/ranger/ranger-audit-consumer-hdfs}"
Review Comment:
`/var/log/ranger/ranger-audit-consumer-hdfs` =>
`/var/log/ranger/audit-consumer-hdfs` - to be consistent with logs location for
other Ranger services - usersync/tagsync/kms.
##########
ranger-audit-server/ranger-audit-common/src/main/java/org/apache/ranger/audit/server/AuditConfig.java:
##########
@@ -0,0 +1,90 @@
+/*
+ * 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.ranger.audit.server;
Review Comment:
Are contents of pacage `org.apache.ranger.audit.server` necessary in
`ranger-audit-common` module? If not, consider moving to ranger-audit-server
module. If needed, consider renaming the package.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
