Support AWS RDS IAM Authentication for Redash database - #7694
winebarrel wants to merge 10 commits into
Conversation
584229f to
3cb352b
Compare
- upstream migrated Poetry -> uv (removed poetry.lock, added uv.lock) - moved boto3/botocore from all_ds group to main [project].dependencies - regenerated uv.lock
Greptile SummaryThis PR adds opt-in AWS RDS IAM authentication support for Redash's own metadata database, controlled by the
Confidence Score: 5/5
|
| Filename | Overview |
|---|---|
| redash/settings/init.py | Adds REDASH_DATABASE_AWS_IAM_AUTH boolean setting parsed from the REDASH_DATABASE_AWS_IAM_AUTH env var, defaulting to false. Clean addition with a documentation link. |
| redash/models/base.py | Adds a conditional IAM auth block: lazily imports boto3, creates a module-level RDS client singleton, and registers a SQLAlchemy do_connect event listener that injects a fresh IAM auth token as the password on each new physical connection. Previous review concerns (per-connection client creation, SSL enforcement, dependency footprint) have all been addressed. |
| uv.lock | Version bump only: redash 26.7.0.dev0 → 26.8.0.dev0. No new top-level dependencies added; boto3/botocore remain in the optional all_ds group. |
Sequence Diagram
sequenceDiagram
participant App as Redash App
participant SA as SQLAlchemy Engine
participant Hook as do_connect Hook
participant RDS as _RDS_CLIENT (boto3)
participant DB as RDS PostgreSQL
Note over App: Module load (REDASH_DATABASE_AWS_IAM_AUTH=true)
App->>RDS: boto3.client("rds") → _RDS_CLIENT singleton
App->>SA: db.engine.connect()
SA->>Hook: do_connect(_dialect, _conn_rec, _cargs, cparams)
Hook->>RDS: generate_db_auth_token(DBHostname, Port, DBUsername)
Note over RDS: Locally signed presigned URL<br/>(no network call)
RDS-->>Hook: auth_token (valid 15 min)
Hook->>Hook: "cparams["password"] = auth_token"
SA->>DB: "psycopg2.connect(**cparams) with IAM token as password"
DB-->>SA: Connection established
Reviews (9): Last reviewed commit: "Merge branch 'master' into support-rds-i..." | Re-trigger Greptile
boto3.client("rds") was instantiated inside the do_connect hook, so a new
client (endpoint resolution, credential chain lookup, config parsing) was
built on every physical DB connection. Hoist it to a module-level RDS_CLIENT
singleton and close over it, leaving only generate_db_auth_token per connection.
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="redash/models/base.py">
<violation number="1" location="redash/models/base.py:49">
P1: RQ worker jobs inherit this boto3 client across forks, which can cause incorrect response ordering and intermittent RDS token-generation or database-connection failures. Creating the client lazily after the fork (or otherwise once per worker process) preserves the connection reuse without sharing a client across processes.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
RDS IAM DB authentication requires the token to be sent over an encrypted channel. Default sslmode to 'require' in the do_connect hook (via setdefault, so an operator-configured stricter mode like verify-full is preserved).
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="redash/models/base.py">
<violation number="1" location="redash/models/base.py:61">
P1: IAM authentication can still attempt a non-encrypted connection when `REDASH_DATABASE_URL` specifies `sslmode=disable` (or another non-strict mode), because `setdefault` only handles a missing key. Preserve only `verify-ca`/`verify-full`; set every other value to `require`.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Instead of promoting boto3/botocore to top-level dependencies (which adds the full AWS SDK to every install), keep them in the optional all_ds group and import boto3 lazily inside the REDASH_DATABASE_AWS_IAM_AUTH guard. Enabling IAM auth now requires installing the all_ds extras (or boto3 directly).
setdefault only covered a missing sslmode, so an explicit weak mode in REDASH_DATABASE_URL (e.g. sslmode=disable/prefer) would let IAM auth attempt an unencrypted connection. Force sslmode=require for anything other than the stricter verify-ca/verify-full.
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="pyproject.toml">
<violation number="1">
P2: With boto3 moved out of the core dependencies, a base (non-all_ds) install enabling REDASH_DATABASE_AWS_IAM_AUTH will crash at startup with a bare ImportError, because base.py imports boto3 unguarded. The Postgres/Athena runners guard the same import with try/except (IAM_ENABLED flag), so consider guarding base.py's import too and failing with a clear message (or requiring all_ds) instead of a hard crash.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Don't force sslmode in the do_connect hook. SSL is not an intrinsic requirement of IAM auth (it's controlled server-side by rds.force_ssl), sslmode is static connection config rather than something the hook needs to set, and forcing it would override an operator's explicit choice. Leave transport security to the DB URL and rds.force_ssl.
It's an internal detail of the connection hook with no external references; mark it private to match the file's convention (e.g. _gfk_types).
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
What type of PR is this?
Description
This will enable the use of AWS RDS IAM authentication with Redash Database.
see https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/UsingWithRDS.IAMDBAuth.html
Using IAM authentication allows you to connect to the database more securely than with password authentication.
How is this tested?
I added the following settings and confirmed that I could connect to the RDS for testing.
# NOTE: It is not a required environment variable. AWS_ACCESS_KEY_ID=... AWS_SECRET_ACCESS_KEY=...Related Tickets & Documents
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
N/A