jir-ogc by jirhiker · Pull Request #556 · DataIntegrationGroup/OcotilloAPI

jirhiker · 2026-02-27T17:03:02Z

Why

This PR addresses the following problem / context:

Use bullet points here

How

Implementation summary - the following was changed / added / removed:

Use bullet points here

Notes

Any special considerations, workarounds, or follow-up work to note?

Use bullet points here

…d allow password handling

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2581f610d3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-02-27T17:05:00Z

db/engine.py

+        password = os.environ.get("CLOUD_SQL_PASSWORD")
        database = os.environ.get("CLOUD_SQL_DATABASE")
-        use_iam_auth = get_bool_env("CLOUD_SQL_IAM_AUTH", True)
+        use_iam_auth = get_bool_env("CLOUD_SQL_IAM_AUTH", False)


Preserve IAM auth as default for Cloud SQL connections

Changing CLOUD_SQL_IAM_AUTH to default False causes DB_DRIVER=cloudsql to silently switch from IAM tokens to password auth when the env var is omitted; in IAM-only deployments (no CLOUD_SQL_PASSWORD configured), connection setup now passes a missing password and fails at startup. This is a regression from prior behavior (default IAM) and can break existing Cloud SQL runtime paths unless every environment is updated to explicitly set CLOUD_SQL_IAM_AUTH=true.

Useful? React with 👍 / 👎.

Copilot

Pull request overview

This PR changes Cloud SQL connection behavior to support password-based authentication (via CLOUD_SQL_PASSWORD) in addition to IAM token auth, and removes the previous implicit default that forced IAM auth.

Changes:

Default CLOUD_SQL_IAM_AUTH to False in Cloud SQL connection paths and use CLOUD_SQL_PASSWORD when IAM auth is disabled.
Remove the transfer script’s implicit default that set CLOUD_SQL_IAM_AUTH=true for Cloud SQL.
Update Alembic Cloud SQL connection handling to mirror the password-vs-IAM selection.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
`transfers/transfer.py`	Removes implicit IAM-auth defaulting when `DB_DRIVER` is Cloud SQL.
`db/engine.py`	Adds password-based Cloud SQL support and changes default IAM auth behavior.
`alembic/env.py`	Aligns Alembic Cloud SQL connections with the new IAM/password selection logic.

Comments suppressed due to low confidence (1)

alembic/env.py:66

Returning a SQLAlchemy URL containing the raw CLOUD_SQL_PASSWORD can leak credentials via logs, stack traces, or Alembic debug output. For Cloud SQL, the URL here is only used for Alembic configuration/offline rendering; consider always omitting the password from the URL (or using a redacted/placeholder value) and relying on the Connector creator in run_migrations_online for the real password.

        if use_iam_auth:
            return f"postgresql+pg8000://{user}@/{database}"
        return f"postgresql+pg8000://{user}:{password}@/{database}"

Copilot · 2026-02-27T17:05:59Z

db/engine.py

-        connect_kwargs["password"] = get_iam_login_token()
+        if use_iam_auth:
+            connect_kwargs["password"] = get_iam_login_token()
+        else:


If use_iam_auth is False, connect_kwargs["password"] is set from CLOUD_SQL_PASSWORD without any validation. When CLOUD_SQL_PASSWORD is unset/empty, this will fail later with a driver/connector error that’s hard to diagnose. Add an explicit check and raise a clear RuntimeError when password-based auth is selected but CLOUD_SQL_PASSWORD is missing.

Suggested change

else:

else:

if not password:

raise RuntimeError(

"CLOUD_SQL_PASSWORD must be set and non-empty when "

"CLOUD_SQL_IAM_AUTH is disabled (password-based auth)."

)

Copilot · 2026-02-27T17:06:00Z

db/engine.py

+            if use_iam_auth:
+                connect_kwargs["password"] = get_iam_login_token()
+            else:
+                connect_kwargs["password"] = password


If use_iam_auth is False, connect_kwargs["password"] is set from CLOUD_SQL_PASSWORD without validation. Add a clear error when password-based auth is selected but CLOUD_SQL_PASSWORD is missing/blank to avoid opaque connector failures.

Copilot · 2026-02-27T17:06:00Z

alembic/env.py

+            if use_iam_auth:
+                connect_kwargs["password"] = get_iam_login_token()
+            else:
+                connect_kwargs["password"] = password


If use_iam_auth is False, password-based auth is used but there’s no validation that CLOUD_SQL_PASSWORD is set. Add an explicit check with a clear error when password is missing/blank to prevent hard-to-debug connector failures.

Copilot · 2026-02-27T17:06:00Z

transfers/transfer.py

 if (
    not (os.getenv("DB_DRIVER") or "").strip()
    and (os.getenv("CLOUD_SQL_INSTANCE_NAME") or "").strip()
 ):
    os.environ["DB_DRIVER"] = "cloudsql"


When DB_DRIVER is auto-set to "cloudsql" (based on CLOUD_SQL_INSTANCE_NAME), the transfer process will now default to password-based auth because db/engine.py defaults CLOUD_SQL_IAM_AUTH to False. If CLOUD_SQL_PASSWORD is not set (common in managed runtimes using IAM), this will lead to a confusing connection failure. Consider restoring a safe default here (e.g., set CLOUD_SQL_IAM_AUTH=true when DB_DRIVER=cloudsql and CLOUD_SQL_PASSWORD is empty) or explicitly validate that either IAM auth is enabled or a password is provided before proceeding.

Copilot · 2026-02-27T17:06:01Z

db/engine.py

+        password = os.environ.get("CLOUD_SQL_PASSWORD")
        database = os.environ.get("CLOUD_SQL_DATABASE")
-        use_iam_auth = get_bool_env("CLOUD_SQL_IAM_AUTH", True)
+        use_iam_auth = get_bool_env("CLOUD_SQL_IAM_AUTH", False)


CLOUD_SQL_IAM_AUTH now defaults to False. In environments that previously relied on the implicit default (or on transfers/transfer.py setting it), this changes the default auth mode from IAM to password and can break Cloud SQL connections without an explicit CLOUD_SQL_PASSWORD. Consider defaulting to True (security/compatibility), or default based on whether CLOUD_SQL_PASSWORD is set.

Suggested change

use_iam_auth = get_bool_env("CLOUD_SQL_IAM_AUTH", False)

use_iam_auth = get_bool_env("CLOUD_SQL_IAM_AUTH", password is None)

feat: disable default IAM authentication for Cloud SQL connections an…

2581f61

…d allow password handling

Copilot AI review requested due to automatic review settings February 27, 2026 17:03

Copilot started reviewing on behalf of jirhiker February 27, 2026 17:03 View session

jirhiker merged commit a4343ee into staging Feb 27, 2026
10 checks passed

chatgpt-codex-connector bot reviewed Feb 27, 2026

View reviewed changes

Copilot AI reviewed Feb 27, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

jir-ogc#556

jir-ogc#556
jirhiker merged 1 commit intostagingfrom
jir-ogc

jirhiker commented Feb 27, 2026

Uh oh!

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

chatgpt-codex-connector bot Feb 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-        else:
+        else:
+            if not password:
+                raise RuntimeError(
+                    "CLOUD_SQL_PASSWORD must be set and non-empty when "
+                    "CLOUD_SQL_IAM_AUTH is disabled (password-based auth)."
+                )

	use_iam_auth = get_bool_env("CLOUD_SQL_IAM_AUTH", False)
	use_iam_auth = get_bool_env("CLOUD_SQL_IAM_AUTH", password is None)

Conversation

jirhiker commented Feb 27, 2026

Why

How

Notes

Uh oh!

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants