Skip to content

hosting-cli(gcp): add --service-account flag#6556

Open
Kastier1 wants to merge 1 commit into
mainfrom
simon/gcp-cloud-run-service-account
Open

hosting-cli(gcp): add --service-account flag#6556
Kastier1 wants to merge 1 commit into
mainfrom
simon/gcp-cloud-run-service-account

Conversation

@Kastier1
Copy link
Copy Markdown
Contributor

@Kastier1 Kastier1 commented May 22, 2026

Summary

  • Adds --service-account to reflex deploy --gcp. When set, the value is forwarded as CLOUD_RUN_SERVICE_ACCOUNT env var to the deploy script, which appends --service-account=<value> to gcloud run deploy.
  • Today, Cloud Run services deployed via this command run as the project's default compute SA (broad permissions; security antipattern for prod). This lets users pin a least-privilege per-service SA.
  • When --service-account is omitted, the env var is not included in env_overrides and the deploy script falls back to the existing default-compute-SA behavior — fully backwards compatible.
  • --help calls out that the deploying principal needs roles/iam.serviceAccountUser on the target SA, since the gcloud 403 otherwise is cryptic.

Requires the matching backend change so the deploy script honors CLOUD_RUN_SERVICE_ACCOUNT: reflex-dev/flexgen#3746

Test plan

  • `pytest tests/units/reflex_cli/v2/test_gcp.py` — 25 passed (2 new tests: `test_gcp_deploy_forwards_service_account`, `test_gcp_deploy_omits_service_account_when_unset`)
  • After the flexgen PR merges, smoke-test `reflex deploy --gcp --service-account my-sa@my-project.iam.gserviceaccount.com ...` and confirm the deployed service runs as the specified SA in the Cloud Run console.

🤖 Generated with Claude Code

Forward the new CLOUD_RUN_SERVICE_ACCOUNT env var to the deploy script
so the Cloud Run service can run as a user-specified IAM SA instead of
the project's default compute SA (broad permissions; a known security
antipattern for production workloads).

Flag is optional. When unset, the key is omitted from env_overrides
entirely — the deploy script defaults it to empty, and the bash
${VAR:+...} expansion drops `--service-account` from `gcloud run
deploy`, preserving today's behavior.

Caveat surfaced in --help: the deploying principal needs
roles/iam.serviceAccountUser on the target SA, or gcloud will 403.

Requires the matching backend change in flexgen so the deploy script
honors CLOUD_RUN_SERVICE_ACCOUNT.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Kastier1 Kastier1 requested a review from a team as a code owner May 22, 2026 16:07
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 22, 2026

Greptile Summary

This PR adds a --service-account flag to reflex deploy --gcp, forwarding the value as CLOUD_RUN_SERVICE_ACCOUNT to the deploy script so Cloud Run services can run as a least-privilege per-service SA instead of the project's default compute SA. The change is backwards-compatible — omitting the flag preserves existing behaviour.

  • New ENV_SERVICE_ACCOUNT constant, click option, and parameter wired into deploy_env with a conditional guard; the backend script change to consume CLOUD_RUN_SERVICE_ACCOUNT is tracked separately.
  • Two new unit tests cover the forwarding path and the key-absent path; both follow the established fixture pattern in the test file.

Confidence Score: 4/5

Safe to merge on the CLI side; the only behavioral edge case is silent no-op for an empty-string argument, which would confuse users but cause no data loss.

The implementation is small, additive, and well-tested. The single concern is that if service_account: silently ignores --service-account "" rather than warning the user, which could produce confusing behaviour in the field.

packages/reflex-hosting-cli/src/reflex_cli/v2/gcp.py — specifically the conditional guard around service_account injection

Important Files Changed

Filename Overview
packages/reflex-hosting-cli/src/reflex_cli/v2/gcp.py Adds --service-account flag: new ENV_SERVICE_ACCOUNT constant, click option, function parameter, and conditional env injection. The if service_account: check silently drops an empty-string argument instead of warning the user.
tests/units/reflex_cli/v2/test_gcp.py Adds two well-scoped tests: one verifying the SA is forwarded in env_overrides when provided, one verifying the key is absent when omitted. Both follow the established test pattern.

Reviews (1): Last reviewed commit: "hosting-cli(gcp): add --service-account ..." | Re-trigger Greptile

Comment on lines +296 to +297
if service_account:
deploy_env[ENV_SERVICE_ACCOUNT] = service_account
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 If a user passes --service-account "" (an explicit empty string), the falsy check silently skips the injection and falls back to the default compute SA with no warning. The user gets no indication their flag was ignored. Using is not None is more precise here — it fires only for the truly-absent case and preserves the semantics of an explicit (even if invalid) user input triggering either a forward or an early error.

Suggested change
if service_account:
deploy_env[ENV_SERVICE_ACCOUNT] = service_account
if service_account is not None:
if not service_account:
console.error("--service-account cannot be an empty string.")
raise click.exceptions.Exit(2)
deploy_env[ENV_SERVICE_ACCOUNT] = service_account

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for this one, i agree. a common case would be passing --service-account "$MY_SERVICE_ACCOUNT" in some CI job, and not realizing that the CI environment isn't defining MY_SERVICE_ACCOUNT`, so it's getting passed as empty string and resolving to the original behavior, despite user intent.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 22, 2026

Merging this PR will not alter performance

✅ 24 untouched benchmarks


Comparing simon/gcp-cloud-run-service-account (2fe82e0) with main (d611a5d)

Open in CodSpeed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants