From 6eb44a11c7a72565c545d6d1811e9131d87410f7 Mon Sep 17 00:00:00 2001 From: Abhijeet <41686026+abhvsn@users.noreply.github.com> Date: Fri, 13 Dec 2024 10:51:55 +0530 Subject: [PATCH] chore: Add `appsmith` user existence check for auth tests (#38069) ## Description PR to add the check for `appsmith` user existence before any assertions in pg-auth-test to remove the flakiness. ### :mag: Cypress test results > [!WARNING] > Tests have not run on the HEAD 0ac8736872f1d8b51b384b644dd0f3b21f725cb0 yet >
Fri, 13 Dec 2024 05:18:03 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No ## Summary by CodeRabbit - **Bug Fixes** - Enhanced testing for PostgreSQL authentication to ensure the Appsmith user exists before access checks. - **Tests** - Updated existing test functions to include user existence verification, improving the robustness of the testing process. - Introduced new functions to verify user existence and check the readiness of the Appsmith instance and PostgreSQL. - Streamlined logic for readiness checks, enhancing overall testing efficiency. --- deploy/docker/tests/pg-test-utils.sh | 70 +++++++++++++++++ deploy/docker/tests/test-pg-auth.sh | 112 +++++++-------------------- 2 files changed, 100 insertions(+), 82 deletions(-) create mode 100644 deploy/docker/tests/pg-test-utils.sh diff --git a/deploy/docker/tests/pg-test-utils.sh b/deploy/docker/tests/pg-test-utils.sh new file mode 100644 index 0000000000..1cbde82eb7 --- /dev/null +++ b/deploy/docker/tests/pg-test-utils.sh @@ -0,0 +1,70 @@ +#!/bin/bash + +# Default container name if not provided +container_name=${container_name:-appsmith-docker-test} + +# Function to check if the Appsmith instance is up +is_appsmith_instance_ready() { + local max_retries=200 + local retry_count=0 + local response_code + + while [ $retry_count -lt $max_retries ]; do + response_code=$(curl -s -o /dev/null -w "%{http_code}" http://localhost/health) + if [[ $response_code -eq 200 ]]; then + echo "Appsmith instance is ready." + return 0 + fi + echo "Waiting for Appsmith instance to be ready... (Attempt: $((retry_count + 1)))" + retry_count=$((retry_count + 1)) + sleep 2 + done + return 1 +} + +# Function to wait until the postgres is ready +wait_for_postgres() { + local max_retries=200 + local retry_count=0 + + while [ $retry_count -lt $max_retries ]; do + if docker exec "${container_name}" pg_isready; then + echo "Postgres is ready." + return 0 + fi + echo "Waiting for Postgres to be ready... (Attempt: $((retry_count + 1)))" + retry_count=$((retry_count + 1)) + sleep 2 + done +} + +# Function to check if the user exists in the database +check_user_exists() { + local user + user=$1 + local max_retries=200 + local retry_count=0 + while [ $retry_count -lt $max_retries ]; do + if docker exec "${container_name}" bash -c "psql -p 5432 -U postgres -c \"SELECT 1 FROM pg_roles WHERE rolname='$user';\" | grep -q 1"; then + echo "$user user exists." + return 0 + fi + echo "Waiting for $user user to be created... (Attempt: $((retry_count + 1)))" + retry_count=$((retry_count + 1)) + sleep 1 + done + echo "$user user does not exist." + return 1 +} + +# Function to check if the Appsmith user has read access to databases +check_user_datasource_access_with_host_port_wo_auth() { + docker exec "${container_name}" bash -c "psql -h 127.0.0.1 -p 5432 -U postgres -c '\l'" + return $? +} + +# Function to check if the Appsmith user has read access to databases +check_user_datasource_access_with_local_port_wo_auth() { + docker exec "${container_name}" bash -c "psql -p 5432 -U postgres -c '\l'" + return $? +} \ No newline at end of file diff --git a/deploy/docker/tests/test-pg-auth.sh b/deploy/docker/tests/test-pg-auth.sh index 854de8a962..c5237b1c20 100755 --- a/deploy/docker/tests/test-pg-auth.sh +++ b/deploy/docker/tests/test-pg-auth.sh @@ -1,8 +1,8 @@ #!/bin/bash set -o errexit -# set -x source ./composes.sh +source ./pg-test-utils.sh # Function to update the APPSMITH_DB_URL in docker.env @@ -13,41 +13,6 @@ update_db_url() { docker exec "${container_name}" bash -c "sed -i 's|^APPSMITH_POSTGRES_DB_URL=|APPSMITH_DB_URL=|' /appsmith-stacks/configuration/docker.env" } -# Function to check if the Appsmith instance is up -is_appsmith_instance_ready() { - local max_retries=200 - local retry_count=0 - local response_code - - while [ $retry_count -lt $max_retries ]; do - response_code=$(curl -s -o /dev/null -w "%{http_code}" http://localhost/health) - if [[ $response_code -eq 200 ]]; then - echo "Appsmith instance is ready." - return 0 - fi - echo "Waiting for Appsmith instance to be ready... (Attempt: $((retry_count + 1)))" - retry_count=$((retry_count + 1)) - sleep 2 - done - return 1 -} - -# Function to wait until the postgres is ready -wait_for_postgres() { - local max_retries=200 - local retry_count=0 - - while [ $retry_count -lt $max_retries ]; do - if docker exec "${container_name}" pg_isready; then - echo "Postgres is ready." - return 0 - fi - echo "Waiting for Postgres to be ready... (Attempt: $((retry_count + 1)))" - retry_count=$((retry_count + 1)) - sleep 2 - done -} - # Function to read the password from the PostgreSQL URL in docker.env.sh get_appsmith_password() { local password @@ -96,18 +61,6 @@ EOF return 1 } -# Function to check if the Appsmith user has read access to databases -check_user_datasource_access_with_host_port_wo_auth() { - docker exec "${container_name}" bash -c "psql -h 127.0.0.1 -p 5432 -U postgres -c '\l'" - return $? -} - -# Function to check if the Appsmith user has read access to databases -check_user_datasource_access_with_local_port_wo_auth() { - docker exec "${container_name}" bash -c "psql -p 5432 -U postgres -c '\l'" - return $? -} - # Test to check if the postgres auth is enabled after upgrading from 1.50 to local image # Expectation: # 1. Appsmith instance should be able to upgrade from v1.50 to local image @@ -167,31 +120,23 @@ test_postgres_auth_enabled_upgrade_from_150tolocal() { RETRYSECONDS=5 retry_count=0 - while true; do - retry_count=$((retry_count + 1)) - if docker exec "${container_name}" pg_isready && - [ "$(docker exec "${container_name}" bash -c 'cat /appsmith-stacks/data/postgres/main/PG_VERSION')" = "14" ]; then - break - fi - if [ $retry_count -le $MAX_RETRIES ]; then - echo "Waiting for postgres to be up..." - sleep $RETRYSECONDS - else - echo "Test ${FUNCNAME[0]} Failed" - exit 1 - fi - done + # Wait until postgres to come up + wait_for_postgres # Check if the Appsmith instance is up if is_appsmith_instance_ready; then - - # Check if the Appsmith user has read access to databases - if check_user_datasource_access_with_auth; then - echo "Test ${FUNCNAME[0]} Passed ✅" - else - echo "Test ${FUNCNAME[0]} Failed ❌" - exit 1 - fi + if ! check_user_exists appsmith; then + echo "Appsmith user does not exist" + echo "Test ${FUNCNAME[0]} Failed ❌" + exit 1 + fi + # Check if the Appsmith user has read access to databases + if check_user_datasource_access_with_auth; then + echo "Test ${FUNCNAME[0]} Passed ✅" + else + echo "Test ${FUNCNAME[0]} Failed ❌" + exit 1 + fi else echo "Appsmith instance failed to start." echo "Test ${FUNCNAME[0]} Failed ❌" @@ -253,23 +198,26 @@ test_postgres_auth_enabled_restart_localtolocal() { echo "Starting Appsmith local to check the auth" compose_appsmith_local - wait_for_postgres # Check if the Appsmith instance is up if is_appsmith_instance_ready; then - - # Check if the Appsmith user has read access to databases - if check_user_datasource_access_with_auth; then - echo "Test ${FUNCNAME[0]} Passed ✅" - else - echo "Test ${FUNCNAME[0]} Failed ❌" - exit 1 - fi + if ! check_user_exists appsmith; then + echo "Appsmith user does not exist" + echo "Test ${FUNCNAME[0]} Failed ❌" + exit 1 + fi + # Check if the Appsmith user has read access to databases + if check_user_datasource_access_with_auth; then + echo "Test ${FUNCNAME[0]} Passed ✅" + else + echo "Test ${FUNCNAME[0]} Failed ❌" + exit 1 + fi else - echo "Appsmith instance failed to start." - echo "Test ${FUNCNAME[0]} Failed ❌" - exit 1 + echo "Appsmith instance failed to start." + echo "Test ${FUNCNAME[0]} Failed ❌" + exit 1 fi }