Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: For embedded postgres appsmith user is unable to create tables because of the missing permission #36661

Closed
1 task done
abhvsn opened this issue Oct 2, 2024 · 0 comments · Fixed by #36664
Closed
1 task done
Assignees
Labels
Bug Something isn't working DB Infrastructure Pod Pod to handle database infrastructure High This issue blocks a user from building or impacts a lot of users Move to Postgres Issues required to be solved for the move to Postgres as repository layer Needs Triaging Needs attention from maintainers to triage Production QA Pod Issues under the QA Pod QA Needs QA attention

Comments

@abhvsn
Copy link
Contributor

abhvsn commented Oct 2, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Description

When user opt for postgres version, startup flow makes sure to create appropriate DB, user and schema. In the current implementation we are not granting any permission to the role that is being created explicitly and is dependent on inheriting the create table and related CRUD ops on these tables. This can lead to permission issues with the appsmith user which is being used by Appsmith instance.

Ref logs:

postgres stdout | 2024-10-02 09:26:09.381 UTC [1781] ERROR:  permission denied for schema appsmith at character 14
postgres stdout | 2024-10-02 09:26:09.381 UTC [1781] STATEMENT:  CREATE TABLE "appsmith"."flyway_schema_history" (
postgres stdout | 	    "installed_rank" INT NOT NULL,
postgres stdout | 	    "version" VARCHAR(50),
postgres stdout | 	    "description" VARCHAR(200) NOT NULL,
postgres stdout | 	    "type" VARCHAR(20) NOT NULL,
postgres stdout | 	    "script" VARCHAR(1000) NOT NULL,
postgres stdout | 	    "checksum" INTEGER,
postgres stdout | 	    "installed_by" VARCHAR(100) NOT NULL,
postgres stdout | 	    "installed_on" TIMESTAMP NOT NULL DEFAULT now(),
postgres stdout | 	    "execution_time" INTEGER NOT NULL,
postgres stdout | 	    "success" BOOLEAN NOT NULL
postgres stdout | 	)

To fix this we need to add the explicit permission for appsmith user:

psql -h "$PG_DB_HOST" -p "$PG_DB_PORT" -U postgres -d "$PG_DB_NAME" -c "GRANT ALL PRIVILEGES ON SCHEMA appsmith TO ${PG_DB_USER};"
psql -h "$PG_DB_HOST" -p "$PG_DB_PORT" -U postgres -d "$PG_DB_NAME" -c "GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA appsmith TO ${PG_DB_USER};"
psql -h "$PG_DB_HOST" -p "$PG_DB_PORT" -U postgres -d "$PG_DB_NAME" -c "ALTER DEFAULT PRIVILEGES IN SCHEMA appsmith GRANT ALL PRIVILEGES ON TABLES TO ${PG_DB_USER};"
psql -h "$PG_DB_HOST" -p "$PG_DB_PORT" -U postgres -c "GRANT CONNECT ON DATABASE appsmith TO ${PG_DB_USER};"

Steps To Reproduce

  1. Pull latest image
  2. Update the env to use Postgres url
  3. Restart the container

Public Sample App

No response

Environment

Production

Severity

High (Blocker to building or releasing)

Issue video log

No response

Version

v1.42

@abhvsn abhvsn added Bug Something isn't working Needs Triaging Needs attention from maintainers to triage labels Oct 2, 2024
@Nikhil-Nandagopal Nikhil-Nandagopal added High This issue blocks a user from building or impacts a lot of users Production labels Oct 2, 2024
@abhvsn abhvsn added the Move to Postgres Issues required to be solved for the move to Postgres as repository layer label Oct 2, 2024
@github-actions github-actions bot added the DB Infrastructure Pod Pod to handle database infrastructure label Oct 2, 2024
abhvsn added a commit that referenced this issue Oct 4, 2024
## Description
PR to add the necessary grants to `appsmith` user when user opts for Postgres embedded DB.

fixes #36661

## Automation

/test Sanity

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/11155064003>
> Commit: 1fb82e5
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=11155064003&attempt=2"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Sanity`
> Spec:
> <hr>Thu, 03 Oct 2024 04:22:54 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [x] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **New Features**
- Introduced default values for PostgreSQL database connection
parameters.
  - Added a new function to manage user permissions on database schemas.

- **Improvements**
- Enhanced the existing database initialization process to include
permission granting after schema verification.
- Updated documentation within the script for better clarity on new
functionalities.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
@appsmith-bot appsmith-bot added the QA Needs QA attention label Oct 4, 2024
@github-actions github-actions bot added the QA Pod Issues under the QA Pod label Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working DB Infrastructure Pod Pod to handle database infrastructure High This issue blocks a user from building or impacts a lot of users Move to Postgres Issues required to be solved for the move to Postgres as repository layer Needs Triaging Needs attention from maintainers to triage Production QA Pod Issues under the QA Pod QA Needs QA attention
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants