fix: Add password based auth for postgres #37068
Conversation
WalkthroughThe changes in this pull request enhance the PostgreSQL initialization process within the Appsmith deployment. Key modifications include the addition of a custom Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (1)
deploy/docker/fs/opt/appsmith/entrypoint.sh (1)
Add connection test for appsmith user before database shutdown
Add a verification step before stopping Postgres to ensure the appsmith user can connect with password authentication:
su postgres -c "env PATH='$PATH' psql -h 127.0.0.1 -U appsmith -d ${APPSMITH_PG_DATABASE} -c '\q'"
- Location:
deploy/docker/fs/opt/appsmith/entrypoint.shincreate_appsmith_pg_dbfunction, before thepg_ctl stopcommand🔗 Analysis chain
Line range hint
437-483: Verify appsmith user connection after setupAdd a connection test for the appsmith user after database setup to ensure the authentication changes work as expected.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if appsmith_hba.conf exists and contains proper authentication rules # Test: Check if appsmith_hba.conf exists if ! rg -q "appsmith_hba.conf" .; then echo "Error: appsmith_hba.conf not found in the repository" exit 1 fi # Test: Check authentication rules in appsmith_hba.conf rg -A 5 "^host.*appsmith.*password"Length of output: 65
Script:
#!/bin/bash # Let's check the actual content of the postgres configuration files fd "appsmith_hba.conf" . -x cat {} # Also check if there's any connection verification logic in the entrypoint script rg -B 2 -A 2 "psql.*appsmith" deploy/docker/fs/opt/appsmith/entrypoint.shLength of output: 1423
Script:
#!/bin/bash # Let's check if there's any connection test after database creation rg -B 5 -A 10 "create_appsmith_pg_db" deploy/docker/fs/opt/appsmith/entrypoint.sh # Also check if there's any psql command execution after database setup rg -B 2 -A 5 "su postgres.*psql" deploy/docker/fs/opt/appsmith/entrypoint.shLength of output: 1648
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
- deploy/docker/fs/opt/appsmith/entrypoint.sh (2 hunks)
- deploy/docker/fs/opt/appsmith/postgres/appsmith_hba.conf (1 hunks)
🧰 Additional context used
📓 Learnings (1)
deploy/docker/fs/opt/appsmith/entrypoint.sh (2)
Learnt from: abhvsn PR: appsmithorg/appsmith#36670 File: deploy/docker/fs/opt/appsmith/entrypoint.sh:465-483 Timestamp: 2024-10-07T09:13:14.613Z Learning: In the `deploy/docker/fs/opt/appsmith/entrypoint.sh` script, `APPSMITH_PG_DATABASE` is a constant with the value `appsmith`, so we can assume it remains constant and doesn't require additional quoting in SQL commands.Learnt from: abhvsn PR: appsmithorg/appsmith#36670 File: deploy/docker/fs/opt/appsmith/entrypoint.sh:465-483 Timestamp: 2024-10-08T15:32:34.115Z Learning: In the `deploy/docker/fs/opt/appsmith/entrypoint.sh` script, `APPSMITH_PG_DATABASE` is a constant with the value `appsmith`, so we can assume it remains constant and doesn't require additional quoting in SQL commands.
🔇 Additional comments (5)
deploy/docker/fs/opt/appsmith/postgres/appsmith_hba.conf (5)
1-10: Documentation is clear and helpful.The comments effectively explain the purpose and implications of the configuration.
15-16: Verify IPv4 configuration aligns with deployment architecture.The configuration correctly implements password authentication for the appsmith user. However, limiting connections to only localhost (127.0.0.1) might be restrictive if the application needs to accept connections from other containers or hosts.
Consider if you need to allow connections from other network ranges based on your container networking setup.
18-19: IPv6 configuration mirrors IPv4 settings.The configuration maintains consistency between IPv4 and IPv6 settings.
13-13: Review security implications of trust authentication for postgres user.While using
trustmethod for local connections is common in containerized environments, consider if this presents any security risks in your deployment context.#!/bin/bash # Check if there are any other configuration files that might override these settings fd -t f -e conf -e config . | grep -i "pg.*hba"
22-24: 🛠️ Refactor suggestionReview necessity of replication configuration.
The replication configuration might be unnecessary unless you're planning to implement PostgreSQL replication.
Consider removing the replication configuration if it's not needed for your use case:
-# Allow replication connections from localhost, by a user with the -# replication privilege. -local replication all md5 -host replication all 127.0.0.1/32 md5 -host replication all ::1/128 md5
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
deploy/docker/fs/opt/appsmith/pg-utils.sh (1)
110-114: Add error handling for database creation.The database creation looks good, but consider capturing and handling potential errors.
- psql -p "$PG_DB_PORT" -U "$POSTGRES_ADMIN_USER" -c "CREATE DATABASE $PG_DB_NAME;" + if ! psql -p "$PG_DB_PORT" -U "$POSTGRES_ADMIN_USER" -c "CREATE DATABASE $PG_DB_NAME;"; then + echo "Failed to create database $PG_DB_NAME" + exit 1 + fideploy/docker/fs/opt/appsmith/entrypoint.sh (1)
479-479: Add timeout to pg_isready command.While the explicit host is good, the command should include a timeout to prevent hanging. Consider adding the
-t(timeout) parameter.- until su postgres -c "env PATH='$PATH' pg_isready -h 127.0.0.1"; do + until su postgres -c "env PATH='$PATH' pg_isready -h 127.0.0.1 -t 5"; do
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
- deploy/docker/fs/opt/appsmith/entrypoint.sh (2 hunks)
- deploy/docker/fs/opt/appsmith/pg-utils.sh (3 hunks)
- deploy/docker/fs/opt/appsmith/postgres/appsmith_hba.conf (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- deploy/docker/fs/opt/appsmith/postgres/appsmith_hba.conf
🧰 Additional context used
📓 Learnings (1)
deploy/docker/fs/opt/appsmith/entrypoint.sh (3)
Learnt from: abhvsn PR: appsmithorg/appsmith#37068 File: deploy/docker/fs/opt/appsmith/entrypoint.sh:475-475 Timestamp: 2024-10-25T08:05:20.269Z Learning: In `deploy/docker/fs/opt/appsmith/entrypoint.sh`, `pg_isready` does not check authentication, so even if there is no `pg_hba.conf` entry for host connections for the `postgres` user, we don't need to worry.Learnt from: abhvsn PR: appsmithorg/appsmith#36670 File: deploy/docker/fs/opt/appsmith/entrypoint.sh:465-483 Timestamp: 2024-10-07T09:13:14.613Z Learning: In the `deploy/docker/fs/opt/appsmith/entrypoint.sh` script, `APPSMITH_PG_DATABASE` is a constant with the value `appsmith`, so we can assume it remains constant and doesn't require additional quoting in SQL commands.Learnt from: abhvsn PR: appsmithorg/appsmith#36670 File: deploy/docker/fs/opt/appsmith/entrypoint.sh:465-483 Timestamp: 2024-10-08T15:32:34.115Z Learning: In the `deploy/docker/fs/opt/appsmith/entrypoint.sh` script, `APPSMITH_PG_DATABASE` is a constant with the value `appsmith`, so we can assume it remains constant and doesn't require additional quoting in SQL commands.
🔇 Additional comments (3)
deploy/docker/fs/opt/appsmith/pg-utils.sh (2)
9-9: LGTM! Variable naming follows shell conventions.
133-135: LGTM! Clear function naming and proper parameter passing.deploy/docker/fs/opt/appsmith/entrypoint.sh (1)
436-440: LGTM! PostgreSQL configuration setup looks secure.The implementation follows PostgreSQL security best practices with proper file ownership and permissions.
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/11514664672. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
deploy/docker/fs/opt/appsmith/entrypoint.sh (1)
436-440: Add error handling for configuration file operations.The file operations look good, with correct permissions (600) and ownership (postgres:postgres) for PostgreSQL security. However, we should add error handling to ensure the configuration file exists and operations succeed.
Apply this diff to add error handling:
+ if [[ ! -f /opt/appsmith/postgres/appsmith_hba.conf ]]; then + echo "Error: PostgreSQL HBA configuration file not found" + return 1 + fi cp /opt/appsmith/postgres/appsmith_hba.conf "$POSTGRES_DB_PATH/pg_hba.conf" + if [[ $? -ne 0 ]]; then + echo "Error: Failed to copy PostgreSQL HBA configuration" + return 1 + fi # PostgreSQL requires strict file permissions for the pg_hba.conf file. Add file permission settings after copying the configuration file. # 600 is the recommended permission for pg_hba.conf file for read and write access to the owner only. chown postgres:postgres "$POSTGRES_DB_PATH/pg_hba.conf" chmod 600 "$POSTGRES_DB_PATH/pg_hba.conf"
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- deploy/docker/fs/opt/appsmith/entrypoint.sh (1 hunks)
🧰 Additional context used
📓 Learnings (1)
deploy/docker/fs/opt/appsmith/entrypoint.sh (3)
Learnt from: abhvsn PR: appsmithorg/appsmith#37068 File: deploy/docker/fs/opt/appsmith/entrypoint.sh:475-475 Timestamp: 2024-10-25T08:05:20.269Z Learning: In `deploy/docker/fs/opt/appsmith/entrypoint.sh`, `pg_isready` does not check authentication, so even if there is no `pg_hba.conf` entry for host connections for the `postgres` user, we don't need to worry.Learnt from: abhvsn PR: appsmithorg/appsmith#36670 File: deploy/docker/fs/opt/appsmith/entrypoint.sh:465-483 Timestamp: 2024-10-07T09:13:14.613Z Learning: In the `deploy/docker/fs/opt/appsmith/entrypoint.sh` script, `APPSMITH_PG_DATABASE` is a constant with the value `appsmith`, so we can assume it remains constant and doesn't require additional quoting in SQL commands.Learnt from: abhvsn PR: appsmithorg/appsmith#36670 File: deploy/docker/fs/opt/appsmith/entrypoint.sh:465-483 Timestamp: 2024-10-08T15:32:34.115Z Learning: In the `deploy/docker/fs/opt/appsmith/entrypoint.sh` script, `APPSMITH_PG_DATABASE` is a constant with the value `appsmith`, so we can assume it remains constant and doesn't require additional quoting in SQL commands.
🔇 Additional comments (1)
deploy/docker/fs/opt/appsmith/entrypoint.sh (1)
436-436: Verify the HBA configuration content.Let's verify the content of the new HBA configuration file to ensure it properly configures password-based authentication for the
appsmithuser.✅ Verification successful
HBA configuration is properly configured for secure authentication ✅
The PostgreSQL HBA configuration file correctly implements:
- Password-based authentication (scram-sha-256) for the
appsmithuser- Restricted access to localhost only (127.0.0.1/32 and ::1/128)
- Proper access control for both appsmith and postgres databases
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check the content of the PostgreSQL HBA configuration file # Expected: Should find entries for password authentication if [[ -f /opt/appsmith/postgres/appsmith_hba.conf ]]; then echo "=== PostgreSQL HBA Configuration ===" cat /opt/appsmith/postgres/appsmith_hba.conf else echo "Error: PostgreSQL HBA configuration file not found" exit 1 fiLength of output: 171
Script:
#!/bin/bash # Let's check the actual content in the repository fd appsmith_hba.conf # If found, let's see its content fd appsmith_hba.conf --exec cat {}Length of output: 1500
|
Deploy-Preview-URL: https://ce-37068.dp.appsmith.com |
…org/appsmith into feat/add-auth-for-postgres-user
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Outside diff range and nitpick comments (1)
deploy/docker/tests/test-pg-auth.sh (1)
2-2: Consider removing the commented-out debug option
The line # set -x is commented out. If debugging is not needed, consider removing it to keep the script clean.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
- deploy/docker/tests/.gitignore (1 hunks)
- deploy/docker/tests/pg-upgrade/composes.sh (1 hunks)
- deploy/docker/tests/test-pg-auth.sh (1 hunks)
- scripts/local_testing.sh (4 hunks)
✅ Files skipped from review due to trivial changes (1)
- deploy/docker/tests/.gitignore
🔇 Additional comments (7)
deploy/docker/tests/pg-upgrade/composes.sh (1)
49-68: Apply the same refactoring pattern here.
This function can use the same refactoring pattern suggested for compose_appsmith_latest.
scripts/local_testing.sh (3)
13-13: LGTM: Help message accurately reflects new parameters.
The help message has been updated to correctly document the new tag parameter in the command syntax.
78-80: LGTM: Proper default tag handling.
Good practice to set a default tag value when none is provided.
116-120: Verify Docker base image compatibility.
The Docker build command now uses the tag parameter correctly, but we should verify that all referenced base images exist.
✅ Verification successful
Since Docker commands cannot be executed in the sandbox environment, let me try to verify the base image references in the codebase to understand their usage and configuration.
Base images are correctly referenced and used in the codebase
The base images appsmith/base-ce:release and appsmith/base-ee:release are properly configured in the build system:
- The main Dockerfile uses an ARG BASE parameter which is correctly passed from the build command
- The base images are consistently used across the codebase, as seen in route-tests configuration
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the base images are available
docker pull appsmith/base-ce:release
docker pull appsmith/base-ee:release
Length of output: 225
Script:
#!/bin/bash
# Search for references to base images in Docker files and scripts
rg -g '!node_modules' -g '!dist' -g '!build' "appsmith/base-[ce|ee]" -A 2
# Look for Dockerfile configurations
fd Dockerfile --exec cat {}
Length of output: 3529
deploy/docker/tests/test-pg-auth.sh (3)
5-5: Verify the existence of 'composes.sh'
Ensure that the ./pg-upgrade/composes.sh script exists and is correctly sourced to avoid runtime errors.
51-52: Handle possible authentication failures
The command may fail if local connections require a password. Ensure the authentication configuration allows passwordless local connections if intended.
116-119: Ensure 'pg_isready' is available
Verify that pg_isready exists within the 'appsmith' container to prevent command not found errors.
…org/appsmith into feat/add-auth-for-postgres-user
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (3)
deploy/docker/tests/test-pg-auth.sh (3)
18-30: Improve retry mechanism configurationConsider these enhancements:
- Move magic numbers to configurable variables
- Add exponential backoff to prevent overwhelming the system
- local max_retries=200 + local max_retries=${MAX_HEALTH_RETRIES:-200} + local retry_delay=${HEALTH_RETRY_DELAY:-1} 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 return 0 fi echo "Waiting for Appsmith instance to be ready... (Attempt: $((retry_count + 1)))" retry_count=$((retry_count + 1)) - sleep 1 + sleep $((retry_delay * retry_count))
111-111: Enhance cleanup processThe cleanup process should be more thorough and handle potential failures.
+cleanup() { + echo "Cleaning up containers and volumes..." + docker compose down --timeout 30 || true + docker compose rm -fsv appsmith || true + # Optionally clean volumes if needed + # docker volume prune -f +} + +trap cleanup EXITAlso applies to: 140-141
181-182: Add command-line options for test configurationConsider adding options to control test behavior (e.g., cleanup, timeouts, verbosity).
+# Parse command line options +while getopts "cv:t:" opt; do + case $opt in + c) SKIP_CLEANUP=true ;; + v) VERBOSE=$OPTARG ;; + t) TIMEOUT=$OPTARG ;; + \?) echo "Invalid option: -$OPTARG" >&2; exit 1 ;; + esac +done + test_postgres_auth_enabled_upgrade_from_147tolocal
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
deploy/docker/tests/pg-upgrade/composes.sh(1 hunks)deploy/docker/tests/test-pg-auth.sh(1 hunks)
🧰 Additional context used
📓 Learnings (1)
deploy/docker/tests/pg-upgrade/composes.sh (2)
Learnt from: abhvsn
PR: appsmithorg/appsmith#37068
File: deploy/docker/tests/pg-upgrade/composes.sh:1-4
Timestamp: 2024-10-29T08:52:10.043Z
Learning: In the `deploy/docker/tests/pg-upgrade/composes.sh` script, it's acceptable to leave `set -o errexit` commented out because the script is used for testing purposes and errors are handled in the consumer script.
Learnt from: abhvsn
PR: appsmithorg/appsmith#37068
File: deploy/docker/tests/pg-upgrade/composes.sh:70-74
Timestamp: 2024-10-29T08:52:09.822Z
Learning: In the testing scripts, the `cleanup` function is called before running tests, so additional safety checks are not necessary.
🔇 Additional comments (5)
deploy/docker/tests/test-pg-auth.sh (1)
11-14: Add synchronization check before URL update
The URL update assumes the configuration file is ready. Add a check to ensure the file exists and is accessible before modification.
deploy/docker/tests/pg-upgrade/composes.sh (4)
5-24: LGTM!
The generate_compose_file function looks good. It centralizes the Docker Compose configuration generation, making the code more DRY and maintainable.
32-39: Verify error handling.
Similar to the previous comment, the compose_appsmith_latest function should handle potential errors from docker compose pull and docker compose up -d. Consider adding error checks and logging.
63-67: ****
Based on the learnings, the cleanup function is called before running tests, so additional safety checks are not necessary.
69-74: LGTM!
The check_appsmith_edition function correctly determines the Appsmith edition based on the Git remote URL.
…dd-auth-for-postgres-user
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/11703065186. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (4)
deploy/docker/tests/test-pg-auth.sh (4)
1-6: Enhance script robustness with additional shell options.Consider adding these shell options for better error handling and debugging:
#!/bin/bash set -o errexit +set -o nounset +set -o pipefail # set -x + +# shellcheck disable=SC1091 source ./composes.sh
17-32: Parameterize the health check URL.Consider making the health check URL configurable to support different environments and ports.
+readonly HEALTH_CHECK_URL=${HEALTH_CHECK_URL:-"http://localhost/health"} +readonly MAX_RETRIES=${MAX_RETRIES:-200} + 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) + while [ $retry_count -lt $MAX_RETRIES ]; do + response_code=$(curl -s -o /dev/null -w "%{http_code}" "${HEALTH_CHECK_URL}")
41-89: Standardize error messages and logging format.The error messages use inconsistent symbols (✅, ❌) and formats. Consider standardizing the output format for better parsing and readability.
+readonly SUCCESS="✅" +readonly FAILURE="❌" + +log_status() { + local test_name="$1" + local status="$2" + echo "${test_name}: ${status}" +} + check_user_datasource_access_with_auth() { # ... existing code ... if [[ $appsmith_user_local_access -ne 0 && $appsmith_user_remote_access -eq 0 ]]; then - echo "appsmith user does not have read access to databases with local unix socket: ✅" - echo "appsmith user has read access to databases with local tcp socket: ✅" + log_status "appsmith user local unix socket access denied" "${SUCCESS}" + log_status "appsmith user local tcp socket access granted" "${SUCCESS}"
184-186: Add script documentation.Consider adding a header comment block describing the script's purpose, usage, and requirements.
+#!/bin/bash +# Description: Tests PostgreSQL authentication after upgrading from Appsmith v1.47 +# Usage: ./test-pg-auth.sh +# Requirements: +# - Docker +# - Docker Compose +# - curl +# - PostgreSQL client tools + container_name="appsmith-docker-test" test_postgres_auth_enabled_upgrade_from_147tolocal
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
deploy/docker/tests/test-pg-auth.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck
deploy/docker/tests/test-pg-auth.sh
[warning] 144-144: The surrounding quotes actually unquote this. Remove or escape them.
(SC2027)
|
Deploy-Preview-URL: https://ce-37068.dp.appsmith.com |
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Outside diff range and nitpick comments (2)
deploy/docker/tests/composes.sh (1)
78-81: Improve variable declarations and validate paths.Move variables to the top and add path validation:
+# Configuration +container_name="appsmith-docker-test" +stacks_path="/tmp/$container_name-stacks" +docker_compose_path="docker-compose.yml" + +# Validate paths +if [[ ! -w "$(dirname "$stacks_path")" ]]; then + echo "Error: Directory $(dirname "$stacks_path") is not writable" + exit 1 +fi + +if [[ ! -w "$(dirname "$docker_compose_path")" ]]; then + echo "Error: Directory $(dirname "$docker_compose_path") is not writable" + exit 1 +fi + -container_name="appsmith-docker-test" -# mkdir -p /tmp/$container_name -stacks_path="/tmp/$container_name-stacks" -docker_compose_path="docker-compose.yml"deploy/docker/tests/test-pg-auth.sh (1)
17-32: Parameterize health check URL and add timeoutThe health check URL and curl timeout should be configurable:
+readonly HEALTH_CHECK_URL=${HEALTH_CHECK_URL:-"http://localhost/health"} +readonly CURL_TIMEOUT=${CURL_TIMEOUT:-5} + 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) + response_code=$(curl -s -o /dev/null -w "%{http_code}" --max-time "$CURL_TIMEOUT" "$HEALTH_CHECK_URL")
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
deploy/docker/tests/composes.sh(1 hunks)deploy/docker/tests/test-pg-auth.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck
deploy/docker/tests/test-pg-auth.sh
[warning] 144-144: The surrounding quotes actually unquote this. Remove or escape them.
(SC2027)
| psql -p ${port} -U ${POSTGRES_ADMIN_USER} -d ${db} -c "GRANT ALL PRIVILEGES ON SCHEMA ${schema} TO ${user};" | ||
| psql -p ${port} -U ${POSTGRES_ADMIN_USER} -d ${db} -c "GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA ${schema} TO ${user};" | ||
| psql -p ${port} -U ${POSTGRES_ADMIN_USER} -d ${db} -c "ALTER DEFAULT PRIVILEGES IN SCHEMA ${schema} GRANT ALL PRIVILEGES ON TABLES TO ${user};" | ||
| psql -p ${port} -U ${POSTGRES_ADMIN_USER} -d ${db} -c "GRANT CONNECT ON DATABASE ${db} TO ${user};" | ||
| } |
There was a problem hiding this comment.
let's add "-h /var/.sql" check the default unix socket path. let's be explicit that we are forcing unix socket use here
| if check_user_datasource_access_wo_auth; then | ||
| echo "postgres user has read access to databases: ✅" | ||
| else | ||
| echo "postgres user does not have read access to databases: ❌" |
There was a problem hiding this comment.
this will never happen for 1.47. add a comment here
| } | ||
|
|
||
| container_name="appsmith-docker-test" | ||
|
|
Description
Manual testing for the auth mechanism with
appsmithandpostgresuser:Fixes https://github.com/appsmithorg/appsmith-ee/issues/5261
/test Sanity
🔍 Cypress test results
Caution
🔴 🔴 🔴 Some tests have failed.
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11814765103
Commit: 6fe0e0a
Cypress dashboard.
Tags: @tag.Sanity
Spec:
The following are new failures, please fix them before merging the PR:
- cypress/e2e/Sanity/Datasources/RestApiDatasource_spec.js
List of identified flaky tests.Wed, 13 Nov 2024 10:25:28 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores