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 -------------------------------------- +@pytest.fixture(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: dev-unsubscr...@ranger.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org