Copilot commented on code in PR #547:
URL: https://github.com/apache/ranger/pull/547#discussion_r2114809295
##########
PyTest-KMS-HDFS/test_kms/utils.py:
##########
@@ -0,0 +1,14 @@
+#no scope mismatch due to utils
Review Comment:
[nitpick] This comment is unclear—consider clarifying the intent or removing
it if it isn't adding meaningful context.
##########
test-pytest.sh:
##########
@@ -0,0 +1,110 @@
+#!/bin/bash
+
+#handle input
+if [ $# -lt 1 ]; then
+ echo "no arguments passed , using default DB: postgres"
+ DB_TYPE=postgres
+ EXTRA_SERVICES=()
+
+else
+ DB_TYPE=$1
+ shift
+ EXTRA_SERVICES=("$@")
+fi
+
+# Remove all containers and clean up docker space
+docker rm -f $(docker ps -aq --filter "name=ranger")
+docker system prune --all --force --volumes
+
+#path setup
+RANGER_DOCKER_PATH="dev-support/ranger-docker"
+TESTS_PATH="$HOME/Desktop/PyTest-KMS-EP"
Review Comment:
The `TESTS_PATH` points to `PyTest-KMS-EP` but the test directory is
`PyTest-KMS-HDFS`; update the path to the correct test folder.
```suggestion
TESTS_PATH="$HOME/Desktop/PyTest-KMS-HDFS"
```
##########
PyTest-KMS-HDFS/test_kms/test_blacklisting.py:
##########
@@ -0,0 +1,263 @@
+import xml.etree.ElementTree as ET
+import requests
+import pytest
+import time
+import docker
+import tarfile
+import io
+
+DBKS_SITE_PATH =
"/opt/ranger/ranger-3.0.0-SNAPSHOT-kms/ews/webapp/WEB-INF/classes/conf/dbks-site.xml"
+KMS_SERVICE_NAME = "dev_kms"
+BASE_URL = "http://localhost:9292/kms/v1"
+RANGER_AUTH = ('keyadmin', 'rangerR0cks!') # Ranger key admin user
+BASE_URL_RANGER = "http://localhost:6080/service/public/v2/api/policy"
+HEADERS={"Content-Type": "application/json","Accept":"application/json"}
+KMS_CONTAINER_NAME = "ranger-kms"
+
+user1="nobody"
+
+client = docker.from_env()
+container = client.containers.get(KMS_CONTAINER_NAME)
+
+
+# **************** create KMS policy for user1
--------------------------------------
[email protected](scope="module", autouse=True)
+def create_initial_kms_policy():
+ policy_data = {
+ "policyName": "blacklist-policy",
+ "service": KMS_SERVICE_NAME,
+ "resources": {
+ "keyname": {
+ "values": ["blacklist-*"], # Match any key starting with
'blacklist-'
+ "isExcludes": False,
+ "isRecursive": False
+ }
+ },
+ "policyItems": [{
+ "accesses": [
+ {"type": "CREATE", "isAllowed": True},
+ {"type": "ROLLOVER", "isAllowed": True},
+ {"type": "DELETE", "isAllowed": True}
+ ],
+ "users": [user1]
+ }]
+ }
+
+ response = requests.post(BASE_URL_RANGER, auth=RANGER_AUTH,
json=policy_data)
+ time.sleep(30) # Wait for policy propagation
Review Comment:
Fixed `sleep` delays can slow down tests and add flakiness; consider polling
the service for readiness instead of a blind 30s wait.
##########
PyTest-KMS-HDFS/test_hdfs/test_config.py:
##########
@@ -0,0 +1,79 @@
+
+##Contains all constant values regarding USER, PATH, HDFS
Commands----------------------
+
+
+HDFS_USER = "hdfs"
+HIVE_USER = "hive"
+HBASE_USER= "hbase"
+KEY_ADMIN="keyadmin"
+HEADERS={"Content-Type": "application/json","Accept":"application/json"}
+PARAMS={"user.name":"keyadmin"}
+BASE_URL="http://localhost:9292/kms/v1"
+HADOOP_CONTAINER = "ranger-hadoop"
+HDFS_USER = "hdfs"
Review Comment:
Duplicate definition of `HDFS_USER`; remove the redundant assignment to
reduce confusion.
```suggestion
# Line 13 removed as it is redundant.
```
##########
PyTest-KMS-HDFS/test_kms/utils.py:
##########
@@ -0,0 +1,14 @@
+#no scope mismatch due to utils
+import subprocess
+
+KMS_CONTAINER_NAME = "ranger-kms" # Replace with your actual KMS
container name
+KMS_LOG_FILE = "/var/log/ranger/kms/ranger-kms-ranger-kms.example.com-root.log"
+
+def fetch_logs():
+ try:
+ cmd = f"docker exec {KMS_CONTAINER_NAME} tail -n 100 {KMS_LOG_FILE}"
+ logs = subprocess.check_output(cmd, shell=True, text=True)
+ error_logs = [line for line in logs.split("\n") if "ERROR" in line or
"Exception" in line]
+ return "\n".join(error_logs) if error_logs else "No recent errors in
logs."
+ except Exception as e:
+ return f"Failed to fetch logs from container: {str(e)}"
Review Comment:
[nitpick] Catching all exceptions can mask specific errors; consider
catching only the exceptions you expect (e.g., `subprocess.CalledProcessError`).
```suggestion
except subprocess.CalledProcessError as e:
return f"Failed to fetch logs from container. Command failed with
exit code {e.returncode}: {e.output}"
```
--
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]