From 91c2333d5cc631b68ab6cc36506ada0cf4c84369 Mon Sep 17 00:00:00 2001 From: Wayne Sun Date: Tue, 12 May 2026 16:44:45 -0400 Subject: [PATCH 1/3] feat: switch GCP auth to direct WIF (no intermediate service account) Replace SA-based Workload Identity Federation with direct WIF where the WIF pool principalSet gets IAM permissions on GCP resources directly. This eliminates the intermediate service account for Vertex AI access, following the google-github-actions/auth recommended pattern. - Add SetProjectIAMBinding to GCFClient for project-level IAM bindings - Grant roles/aiplatform.user to org principalSets in provisioner - Remove --gcp-wif-sa-email CLI flag and WIFServiceAccount config field - Remove service_account parameter from setup-gcp composite action - Remove gcp_wif_sa_email secret from all 6 workflow files - Make service_account_impersonation_url conditional in sandbox creds - Rename SA from fullsend-dispatch to fullsend-mint Signed-off-by: Wayne Sun --- docs/guides/admin/installation.md | 39 ++------ .../adr-0014-github-apps-and-secrets/SPEC.md | 3 +- .../plans/2026-05-04-retro-agent.md | 1 - internal/cli/admin.go | 10 +- internal/cli/admin_test.go | 3 +- internal/dispatch/gcf/gcp.go | 60 +++++++++--- internal/dispatch/gcf/gcp_test.go | 91 +++++++++++++++++++ internal/dispatch/gcf/provisioner.go | 43 ++++++--- internal/dispatch/gcf/provisioner_test.go | 67 +++++++++++++- internal/inference/vertex/vertex.go | 15 +-- internal/inference/vertex/vertex_test.go | 36 ++------ .../.github/actions/setup-gcp/action.yml | 4 - .../fullsend-repo/.github/workflows/code.yml | 1 - .../fullsend-repo/.github/workflows/fix.yml | 1 - .../.github/workflows/prioritize.yml | 1 - .../fullsend-repo/.github/workflows/retro.yml | 1 - .../.github/workflows/review.yml | 1 - .../.github/workflows/triage.yml | 1 - .../scripts/prepare-sandbox-credentials.sh | 15 +-- internal/scaffold/scaffold_test.go | 5 +- 20 files changed, 267 insertions(+), 131 deletions(-) diff --git a/docs/guides/admin/installation.md b/docs/guides/admin/installation.md index 03a885e8f7..5c49415165 100644 --- a/docs/guides/admin/installation.md +++ b/docs/guides/admin/installation.md @@ -71,44 +71,27 @@ gcloud iam workload-identity-pools providers create-oidc github \ --workload-identity-pool=github-actions \ --issuer-uri="https://token.actions.githubusercontent.com" \ --attribute-mapping="google.subject=assertion.sub,attribute.repository_owner=assertion.repository_owner,attribute.repository=assertion.repository" \ - --attribute-condition="assertion.repository_owner == '$ORG_NAME'" \ + --attribute-condition="assertion.repository == '$ORG_NAME/.fullsend'" \ --project="$GCP_PROJECT" ``` -The `attribute-condition` restricts which GitHub Actions workflows can exchange OIDC tokens for GCP credentials. +The `attribute-condition` restricts which GitHub Actions workflows can exchange OIDC tokens for GCP credentials. Scoping to `$ORG_NAME/.fullsend` ensures only the `.fullsend` config repo can authenticate — workflows in other repos cannot obtain Vertex AI credentials. -- **Org-wide** (`repository_owner`): any repo in the org can authenticate. Simpler to maintain but means a compromised or misconfigured workflow in *any* repo could obtain Vertex AI credentials. -- **Repo-scoped** (`repository`): only the `.fullsend` repo can authenticate. Limits blast radius — recommended for orgs where not all repos are equally trusted. - -For repo-scoped access, replace the `attribute-condition` above with: - -```bash ---attribute-condition="assertion.repository == '$ORG_NAME/.fullsend'" -``` - -If you choose repo-scoped access, also update the `--member` in step 1c to match: - -```bash ---member="principalSet://iam.googleapis.com/projects/$PROJECT_NUMBER/locations/global/workloadIdentityPools/github-actions/attribute.repository/$ORG_NAME/.fullsend" -``` - -**1c. Grant the service account impersonation permission** +**1c. Grant Vertex AI access to the WIF principal** ```bash export PROJECT_NUMBER=$(gcloud projects describe "$GCP_PROJECT" --format='value(projectNumber)') -gcloud iam service-accounts add-iam-policy-binding \ - "fullsend-agent@$GCP_PROJECT.iam.gserviceaccount.com" \ - --role="roles/iam.workloadIdentityUser" \ - --member="principalSet://iam.googleapis.com/projects/$PROJECT_NUMBER/locations/global/workloadIdentityPools/github-actions/attribute.repository_owner/$ORG_NAME" \ - --project="$GCP_PROJECT" +gcloud projects add-iam-policy-binding "$GCP_PROJECT" \ + --role="roles/aiplatform.user" \ + --member="principalSet://iam.googleapis.com/projects/$PROJECT_NUMBER/locations/global/workloadIdentityPools/github-actions/attribute.repository/$ORG_NAME/.fullsend" \ + --condition=None ``` **1d. Note the WIF provider resource name** ```bash export WIF_PROVIDER="projects/$PROJECT_NUMBER/locations/global/workloadIdentityPools/github-actions/providers/github" -export WIF_SA_EMAIL="fullsend-agent@$GCP_PROJECT.iam.gserviceaccount.com" ``` ### Option B: Service account key (legacy) @@ -151,11 +134,10 @@ fullsend admin install "$ORG_NAME" \ --gcp-project "$GCP_PROJECT" \ --gcp-region global \ --gcp-wif-provider "$WIF_PROVIDER" \ - --gcp-wif-sa-email "$WIF_SA_EMAIL" \ --mint-project "$GCP_PROJECT" ``` -`--mint-project` specifies the GCP project where the OIDC token mint Cloud Function is deployed. It can be the same project as `--gcp-project` or a separate project. The installer automatically provisions a Cloud Function, WIF pool (`fullsend-pool`), WIF provider (`github-oidc`), and Secret Manager secrets in the mint project. A service account (`fullsend-dispatch`) is also created as the Cloud Function's runtime identity to access Secret Manager — this is internal infrastructure and does not require any admin setup. +`--mint-project` specifies the GCP project where the OIDC token mint Cloud Function is deployed. It can be the same project as `--gcp-project` or a separate project. The installer automatically provisions a Cloud Function, WIF pool (`fullsend-pool`), WIF provider (`github-oidc`), and Secret Manager secrets in the mint project. A service account (`fullsend-mint`) is also created as the Cloud Function's runtime identity to access Secret Manager — this is internal infrastructure and does not require any admin setup. Additional mint flags: @@ -180,7 +162,6 @@ fullsend admin install "$FIRST_ORG" \ --gcp-project "$GCP_PROJECT" \ --gcp-region global \ --gcp-wif-provider "$WIF_PROVIDER" \ - --gcp-wif-sa-email "$WIF_SA_EMAIL" \ --mint-project "$GCP_PROJECT" \ --public ``` @@ -194,7 +175,6 @@ fullsend admin install "$ADDITIONAL_ORG" \ --gcp-project "$GCP_PROJECT" \ --gcp-region global \ --gcp-wif-provider "$WIF_PROVIDER" \ - --gcp-wif-sa-email "$WIF_SA_EMAIL" \ --mint-url "$MINT_URL" ``` @@ -225,8 +205,7 @@ If you already have fullsend installed with a service account key: --gcp-project "$GCP_PROJECT" \ --gcp-region global \ --gcp-wif-provider "$WIF_PROVIDER" \ - --gcp-wif-sa-email "$WIF_SA_EMAIL" \ - --mint-project "$GCP_PROJECT" + --mint-project "$GCP_PROJECT" ``` 3. Verify a workflow run succeeds with WIF auth (check for "Authenticated using Workload Identity Federation" in the auth step output) 4. Delete the old SA key: `gcloud iam service-accounts keys delete --iam-account=...` diff --git a/docs/normative/admin-install/v1/adr-0014-github-apps-and-secrets/SPEC.md b/docs/normative/admin-install/v1/adr-0014-github-apps-and-secrets/SPEC.md index 80c12665cf..38f92781bb 100644 --- a/docs/normative/admin-install/v1/adr-0014-github-apps-and-secrets/SPEC.md +++ b/docs/normative/admin-install/v1/adr-0014-github-apps-and-secrets/SPEC.md @@ -53,13 +53,12 @@ All of the following are **repository-level** Actions secrets and variables on * | Variable | `FULLSEND__CLIENT_ID` | GitHub App Client ID (e.g. `Iv23_...`), per [GitHub recommendation](https://github.blog/changelog/2024-05-01-github-apps-can-now-use-the-client-id-to-fetch-installation-tokens/) | secrets | | Secret | `FULLSEND_GCP_SA_KEY_JSON` | GCP service account key JSON (SA key mode only) | inference | | Secret | `FULLSEND_GCP_WIF_PROVIDER` | Full WIF provider resource name (WIF mode only) | inference | -| Secret | `FULLSEND_GCP_WIF_SA_EMAIL` | Service account email for WIF impersonation (WIF mode only) | inference | | Secret | `FULLSEND_GCP_PROJECT_ID` | GCP project identifier (when inference provider is `vertex`) | inference | | Variable | `FULLSEND_GCP_REGION` | GCP region for Vertex AI (e.g. `us-east5`) | inference | - `` is the agent role in **ASCII uppercase** (e.g. `FULLSEND_TRIAGE_APP_PRIVATE_KEY`). - For each role processed in install, if PEM is non-empty, the implementation **must** create/update the secret; if PEM is empty (reuse path), the implementation **must** skip writing that role’s secret. The Client ID variable is **always** written (even on reuse) to ensure it stays current. -- Inference secrets are only created when an inference provider is configured in `config.yaml` (see [ADR 0011](../adr-0011-org-config-yaml/SPEC.md)). When `inference.provider` is `vertex`, the implementation **must** store `FULLSEND_GCP_PROJECT_ID` and either the SA key secret (`FULLSEND_GCP_SA_KEY_JSON`) or the WIF secrets (`FULLSEND_GCP_WIF_PROVIDER` and `FULLSEND_GCP_WIF_SA_EMAIL`). The two auth modes are mutually exclusive — WIF secrets and the SA key secret **must not** coexist. The WIF provider name and SA email are stored as secrets (not variables) so their values are masked in GitHub Actions logs. +- Inference secrets are only created when an inference provider is configured in `config.yaml` (see [ADR 0011](../adr-0011-org-config-yaml/SPEC.md)). When `inference.provider` is `vertex`, the implementation **must** store `FULLSEND_GCP_PROJECT_ID` and either the SA key secret (`FULLSEND_GCP_SA_KEY_JSON`) or the WIF provider secret (`FULLSEND_GCP_WIF_PROVIDER`). The two auth modes are mutually exclusive — WIF secrets and the SA key secret **must not** coexist. The WIF provider name is stored as a secret (not a variable) so its value is masked in GitHub Actions logs. ## 6. Analyze / health semantics for the secrets layer diff --git a/docs/superpowers/plans/2026-05-04-retro-agent.md b/docs/superpowers/plans/2026-05-04-retro-agent.md index dcc40444cb..57cea9f89a 100644 --- a/docs/superpowers/plans/2026-05-04-retro-agent.md +++ b/docs/superpowers/plans/2026-05-04-retro-agent.md @@ -816,7 +816,6 @@ jobs: uses: google-github-actions/auth@v3 with: workload_identity_provider: ${{ secrets.FULLSEND_GCP_WIF_PROVIDER }} - service_account: ${{ secrets.FULLSEND_GCP_WIF_SA_EMAIL }} - name: Authenticate to Google Cloud (SA key) if: vars.FULLSEND_GCP_AUTH_MODE != 'wif' diff --git a/internal/cli/admin.go b/internal/cli/admin.go index 5f7625011c..50db845a02 100644 --- a/internal/cli/admin.go +++ b/internal/cli/admin.go @@ -99,7 +99,6 @@ func newInstallCmd() *cobra.Command { var gcpServiceAccount string var gcpCredentialsFile string var gcpWIFProvider string - var gcpWIFSAEmail string var mintProvider string var mintProject string var mintRegion string @@ -156,8 +155,8 @@ func newInstallCmd() *cobra.Command { } // Validate GCP flag dependencies. - if gcpProject == "" && (gcpServiceAccount != "" || gcpCredentialsFile != "" || gcpRegion != "" || gcpWIFProvider != "" || gcpWIFSAEmail != "") { - return fmt.Errorf("--gcp-service-account, --gcp-credentials-file, --gcp-wif-provider, --gcp-wif-sa-email, and --gcp-region require --gcp-project to be set") + if gcpProject == "" && (gcpServiceAccount != "" || gcpCredentialsFile != "" || gcpRegion != "" || gcpWIFProvider != "") { + return fmt.Errorf("--gcp-service-account, --gcp-credentials-file, --gcp-wif-provider, and --gcp-region require --gcp-project to be set") } if gcpProject != "" && gcpRegion == "" { return fmt.Errorf("--gcp-region is required when --gcp-project is set") @@ -168,9 +167,6 @@ func newInstallCmd() *cobra.Command { if gcpWIFProvider != "" && gcpServiceAccount != "" { return fmt.Errorf("--gcp-wif-provider and --gcp-service-account are mutually exclusive") } - if (gcpWIFProvider != "") != (gcpWIFSAEmail != "") { - return fmt.Errorf("--gcp-wif-provider and --gcp-wif-sa-email must be provided together") - } // Build inference provider from GCP flags. var inferenceProvider inference.Provider @@ -180,7 +176,6 @@ func newInstallCmd() *cobra.Command { if gcpWIFProvider != "" { vcfg.Mode = vertex.AuthModeWIF vcfg.WIFProvider = gcpWIFProvider - vcfg.WIFServiceAccount = gcpWIFSAEmail } else { vcfg.ServiceAccountName = gcpServiceAccount if gcpCredentialsFile != "" { @@ -292,7 +287,6 @@ func newInstallCmd() *cobra.Command { cmd.Flags().StringVar(&gcpServiceAccount, "gcp-service-account", "", "existing GCP service account name (optional, used with --gcp-project)") cmd.Flags().StringVar(&gcpCredentialsFile, "gcp-credentials-file", "", "path to pre-made GCP service account key JSON (optional, used with --gcp-project)") cmd.Flags().StringVar(&gcpWIFProvider, "gcp-wif-provider", "", "full Workload Identity Federation provider resource name (e.g. projects/PROJECT_NUMBER/locations/global/workloadIdentityPools/POOL/providers/PROVIDER)") - cmd.Flags().StringVar(&gcpWIFSAEmail, "gcp-wif-sa-email", "", "GCP service account email for WIF impersonation (required with --gcp-wif-provider)") cmd.Flags().StringVar(&mintProvider, "mint-provider", "gcf", "token mint provider (gcf)") cmd.Flags().StringVar(&mintProject, "mint-project", "", "cloud project for token mint (e.g. GCP project ID)") cmd.Flags().StringVar(&mintRegion, "mint-region", "us-central1", "cloud region for token mint") diff --git a/internal/cli/admin_test.go b/internal/cli/admin_test.go index 7e454c7772..ff8f5bc58c 100644 --- a/internal/cli/admin_test.go +++ b/internal/cli/admin_test.go @@ -58,8 +58,9 @@ func TestInstallCmd_Flags(t *testing.T) { wifProviderFlag := cmd.Flags().Lookup("gcp-wif-provider") require.NotNil(t, wifProviderFlag, "expected --gcp-wif-provider flag") + // --gcp-wif-sa-email removed (direct WIF, no intermediate SA) wifSAEmailFlag := cmd.Flags().Lookup("gcp-wif-sa-email") - require.NotNil(t, wifSAEmailFlag, "expected --gcp-wif-sa-email flag") + assert.Nil(t, wifSAEmailFlag, "--gcp-wif-sa-email flag should have been removed") // --repo flag should not exist (issue #495) repoFlag := cmd.Flags().Lookup("repo") diff --git a/internal/dispatch/gcf/gcp.go b/internal/dispatch/gcf/gcp.go index 6dfe480a0d..2025e33c92 100644 --- a/internal/dispatch/gcf/gcp.go +++ b/internal/dispatch/gcf/gcp.go @@ -76,6 +76,9 @@ type GCFClient interface { // IAM binding (Secret Manager resources) SetSecretIAMBinding(ctx context.Context, resource, member, role string) error + // IAM binding (project-level) + SetProjectIAMBinding(ctx context.Context, projectID, member, role string) error + // Cloud Run IAM (for function invoker policy) SetCloudRunInvoker(ctx context.Context, projectID, region, serviceName string) error @@ -370,7 +373,7 @@ func (c *LiveGCFClient) SetSecretIAMBinding(ctx context.Context, resource, membe setURL := fmt.Sprintf("https://secretmanager.googleapis.com/v1/%s:setIamPolicy", resource) for attempt := range maxRetries { - err := c.trySetIAMBinding(ctx, getURL, setURL, member, role) + err := c.trySetIAMBinding(ctx, http.MethodGet, "", getURL, setURL, member, role) if err == nil { return nil } @@ -386,8 +389,48 @@ func (c *LiveGCFClient) SetSecretIAMBinding(ctx context.Context, resource, membe return fmt.Errorf("IAM policy update failed after %d retries", maxRetries) } -func (c *LiveGCFClient) trySetIAMBinding(ctx context.Context, getURL, setURL, member, role string) error { - resp, err := c.Client.DoRequest(ctx, http.MethodGet, getURL, "") +type conflictError struct{ status int } + +func (e *conflictError) Error() string { + return fmt.Sprintf("IAM policy conflict (status %d)", e.status) +} + +func isConflict(err error) bool { + var ce *conflictError + return errors.As(err, &ce) +} + +// SetProjectIAMBinding sets an IAM binding on a GCP project. +// Uses read-modify-write with retry on 409 Conflict (etag mismatch). +func (c *LiveGCFClient) SetProjectIAMBinding(ctx context.Context, projectID, member, role string) error { + const maxRetries = 3 + getURL := fmt.Sprintf("https://cloudresourcemanager.googleapis.com/v1/projects/%s:getIamPolicy", + url.PathEscape(projectID)) + setURL := fmt.Sprintf("https://cloudresourcemanager.googleapis.com/v1/projects/%s:setIamPolicy", + url.PathEscape(projectID)) + + for attempt := range maxRetries { + err := c.trySetIAMBinding(ctx, http.MethodPost, "{}", getURL, setURL, member, role) + if err == nil { + return nil + } + if !isConflict(err) || attempt == maxRetries-1 { + return err + } + select { + case <-ctx.Done(): + return ctx.Err() + case <-time.After(time.Duration(200*(attempt+1)) * time.Millisecond): + } + } + return fmt.Errorf("project IAM policy update failed after %d retries", maxRetries) +} + +// trySetIAMBinding performs a single read-modify-write IAM policy update. +// getMethod/getBody control the getIamPolicy request (GET+"" for Secret Manager, +// POST+"{}" for Cloud Resource Manager). setIamPolicy always uses POST. +func (c *LiveGCFClient) trySetIAMBinding(ctx context.Context, getMethod, getBody, getURL, setURL, member, role string) error { + resp, err := c.Client.DoRequest(ctx, getMethod, getURL, getBody) if err != nil { return fmt.Errorf("getting IAM policy: %w", err) } @@ -456,17 +499,6 @@ func (c *LiveGCFClient) trySetIAMBinding(ctx context.Context, getURL, setURL, me return nil } -type conflictError struct{ status int } - -func (e *conflictError) Error() string { - return fmt.Sprintf("IAM policy conflict (status %d)", e.status) -} - -func isConflict(err error) bool { - var ce *conflictError - return errors.As(err, &ce) -} - // SetCloudRunInvoker ensures allUsers has roles/run.invoker on the Cloud Run // service backing a Cloud Function. Uses read-modify-write with retry on 409 // (etag conflict) to preserve existing bindings. The function's own OIDC diff --git a/internal/dispatch/gcf/gcp_test.go b/internal/dispatch/gcf/gcp_test.go index a453df7a05..80e5ae2d41 100644 --- a/internal/dispatch/gcf/gcp_test.go +++ b/internal/dispatch/gcf/gcp_test.go @@ -369,6 +369,97 @@ func TestLiveGCFClient_SetSecretIAMBinding(t *testing.T) { }) } +// --- SetProjectIAMBinding --- + +func TestLiveGCFClient_SetProjectIAMBinding(t *testing.T) { + t.Run("adds new binding", func(t *testing.T) { + callCount := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + callCount++ + assert.Equal(t, http.MethodPost, r.Method) + if callCount == 1 { + assert.Contains(t, r.URL.Path, ":getIamPolicy") + w.WriteHeader(http.StatusOK) + fmt.Fprintln(w, `{"bindings":[],"etag":"v1"}`) + return + } + assert.Contains(t, r.URL.Path, ":setIamPolicy") + var body map[string]interface{} + json.NewDecoder(r.Body).Decode(&body) + policy := body["policy"].(map[string]interface{}) + assert.Equal(t, "v1", policy["etag"]) + bindings := policy["bindings"].([]interface{}) + assert.Len(t, bindings, 1) + b := bindings[0].(map[string]interface{}) + assert.Equal(t, "roles/aiplatform.user", b["role"]) + members := b["members"].([]interface{}) + assert.Contains(t, members, "principalSet://iam.googleapis.com/projects/123/locations/global/workloadIdentityPools/pool/attribute.repository_owner/my-org") + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + err := newTestClient(srv).SetProjectIAMBinding(context.Background(), + "my-project", + "principalSet://iam.googleapis.com/projects/123/locations/global/workloadIdentityPools/pool/attribute.repository_owner/my-org", + "roles/aiplatform.user") + require.NoError(t, err) + assert.Equal(t, 2, callCount) + }) + + t.Run("member already bound", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + fmt.Fprintln(w, `{"bindings":[{"role":"roles/aiplatform.user","members":["principalSet://example"]}]}`) + })) + defer srv.Close() + + err := newTestClient(srv).SetProjectIAMBinding(context.Background(), + "my-project", "principalSet://example", "roles/aiplatform.user") + require.NoError(t, err) + }) + + t.Run("retries on 409 conflict", func(t *testing.T) { + callCount := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + callCount++ + if callCount <= 2 { + if callCount%2 == 1 { + w.WriteHeader(http.StatusOK) + fmt.Fprintln(w, `{"bindings":[],"etag":"v1"}`) + return + } + w.WriteHeader(http.StatusConflict) + fmt.Fprintln(w, `{"error":{"message":"conflict"}}`) + return + } + if callCount == 3 { + w.WriteHeader(http.StatusOK) + fmt.Fprintln(w, `{"bindings":[],"etag":"v2"}`) + return + } + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + err := newTestClient(srv).SetProjectIAMBinding(context.Background(), + "proj", "member", "role") + require.NoError(t, err) + assert.Equal(t, 4, callCount) + }) + + t.Run("getIamPolicy error", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusForbidden) + fmt.Fprintln(w, `{"error":{"message":"denied"}}`) + })) + defer srv.Close() + + err := newTestClient(srv).SetProjectIAMBinding(context.Background(), "proj", "m", "role") + require.Error(t, err) + assert.Contains(t, err.Error(), "getting IAM policy returned 403") + }) +} + // --- SetCloudRunInvoker --- func TestLiveGCFClient_SetCloudRunInvoker(t *testing.T) { diff --git a/internal/dispatch/gcf/provisioner.go b/internal/dispatch/gcf/provisioner.go index 63608d7f94..d69cdcbd10 100644 --- a/internal/dispatch/gcf/provisioner.go +++ b/internal/dispatch/gcf/provisioner.go @@ -61,7 +61,7 @@ var gcpRegionPattern = regexp.MustCompile(`^[a-z]+-[a-z]+[0-9]+$`) var rolePattern = regexp.MustCompile(`^[a-z][a-z0-9_-]*$`) const ( - saName = "fullsend-dispatch" + saName = "fullsend-mint" defaultPool = "fullsend-pool" defaultProvider = "github-oidc" defaultRegion = "us-central1" @@ -194,10 +194,11 @@ func (p *Provisioner) StoreAgentPEM(ctx context.Context, org, role string, pemDa // 1. Look up project number // 2. Create/verify service account // 3. Create/verify WIF pool + provider -// 4. Store all agent PEMs in Secret Manager -// 5. Grant SA access to all role secrets -// 6. Deploy Cloud Function -// 7. Return FULLSEND_MINT_URL +// 4. Grant Vertex AI access to each org's WIF principalSet (direct WIF) +// 5. Store all agent PEMs in Secret Manager +// 6. Grant SA access to all role secrets +// 7. Deploy Cloud Function +// 8. Return FULLSEND_MINT_URL // // When MintURL is set, reuses an existing mint: // 1. Store all agent PEMs in Secret Manager @@ -362,6 +363,16 @@ func (p *Provisioner) provisionSelfManaged(ctx context.Context) (map[string]stri return nil, fmt.Errorf("creating WIF provider: %w", err) } + // Step 4b: Grant Vertex AI access to each installing org's .fullsend repo + // at the project level (direct WIF — no intermediate service account). + for _, org := range installingOrgs { + principal := fmt.Sprintf("principalSet://iam.googleapis.com/projects/%s/locations/global/workloadIdentityPools/%s/attribute.repository/%s/.fullsend", + projectNumber, p.cfg.WIFPoolName, org) + if err := p.gcpAPI.SetProjectIAMBinding(ctx, p.cfg.ProjectID, principal, "roles/aiplatform.user"); err != nil { + return nil, fmt.Errorf("granting Vertex AI access for org %s: %w", org, err) + } + } + // Step 5a: Store new agent PEMs only for installing orgs. for _, org := range installingOrgs { for _, role := range sortedByteMapKeys(p.cfg.AgentPEMs) { @@ -573,21 +584,24 @@ func deriveAllowedRoles(roleAppIDsJSON string) string { return strings.Join(roles, ",") } -// buildAttributeCondition constructs a WIF CEL condition from an org list. +// buildAttributeCondition constructs a WIF CEL condition scoped to each org's +// .fullsend repo (not org-wide) to limit which workflows can authenticate. func buildAttributeCondition(orgs []string) string { if len(orgs) == 1 { - return fmt.Sprintf("assertion.repository_owner == '%s'", orgs[0]) + return fmt.Sprintf("assertion.repository == '%s/.fullsend'", orgs[0]) } quoted := make([]string, len(orgs)) for i, org := range orgs { - quoted[i] = fmt.Sprintf("'%s'", org) + quoted[i] = fmt.Sprintf("'%s/.fullsend'", org) } - return fmt.Sprintf("assertion.repository_owner in [%s]", strings.Join(quoted, ", ")) + return fmt.Sprintf("assertion.repository in [%s]", strings.Join(quoted, ", ")) } +const fullsendRepoSuffix = "/.fullsend" + // parseConditionOrgs extracts GitHub org names from a WIF attribute condition. -// Supports both single-org ("assertion.repository_owner == 'org1'") and -// multi-org ("assertion.repository_owner in ['org1', 'org2']") formats. +// Supports both repo-scoped ("assertion.repository == 'org/.fullsend'") and +// legacy org-scoped ("assertion.repository_owner == 'org'") formats. func parseConditionOrgs(condition string) []string { var orgs []string for _, part := range strings.Split(condition, "'") { @@ -595,7 +609,12 @@ func parseConditionOrgs(condition string) []string { if part == "" { continue } - if githubOrgPattern.MatchString(part) { + if strings.HasSuffix(part, fullsendRepoSuffix) { + org := strings.TrimSuffix(part, fullsendRepoSuffix) + if githubOrgPattern.MatchString(org) { + orgs = append(orgs, org) + } + } else if githubOrgPattern.MatchString(part) { orgs = append(orgs, part) } } diff --git a/internal/dispatch/gcf/provisioner_test.go b/internal/dispatch/gcf/provisioner_test.go index 75a3f1b6fd..f4d08b4707 100644 --- a/internal/dispatch/gcf/provisioner_test.go +++ b/internal/dispatch/gcf/provisioner_test.go @@ -68,6 +68,15 @@ type fakeGCFClient struct { // Captured env vars from the last CreateFunction or UpdateFunction call. lastCreateFunctionEnvVars map[string]string + + // Captured project IAM binding arguments. + projectIAMBindings []projectIAMBinding +} + +type projectIAMBinding struct { + ProjectID string + Member string + Role string } func newFakeGCFClient() *fakeGCFClient { @@ -120,6 +129,10 @@ func (f *fakeGCFClient) AddSecretVersion(_ context.Context, _ string, secretID s func (f *fakeGCFClient) SetSecretIAMBinding(_ context.Context, _, _, _ string) error { return f.record("SetSecretIAMBinding") } +func (f *fakeGCFClient) SetProjectIAMBinding(_ context.Context, projectID, member, role string) error { + f.projectIAMBindings = append(f.projectIAMBindings, projectIAMBinding{projectID, member, role}) + return f.record("SetProjectIAMBinding") +} func (f *fakeGCFClient) SetCloudRunInvoker(_ context.Context, _, _, _ string) error { return f.record("SetCloudRunInvoker") } @@ -328,6 +341,7 @@ func TestProvisioner_Provision_FullFlow(t *testing.T) { "CreateWIFPool", "GetWIFProvider", "CreateWIFProvider", + "SetProjectIAMBinding", "GetSecret", "CreateSecret", "AddSecretVersion", @@ -344,6 +358,13 @@ func TestProvisioner_Provision_FullFlow(t *testing.T) { require.Contains(t, vars, "FULLSEND_MINT_URL") assert.Equal(t, "https://fullsend-mint-abc123.run.app", vars["FULLSEND_MINT_URL"]) + // Verify project IAM binding arguments. + require.Len(t, fake.projectIAMBindings, 1) + assert.Equal(t, "my-project", fake.projectIAMBindings[0].ProjectID) + assert.Equal(t, "roles/aiplatform.user", fake.projectIAMBindings[0].Role) + assert.Contains(t, fake.projectIAMBindings[0].Member, "principalSet://iam.googleapis.com/") + assert.Contains(t, fake.projectIAMBindings[0].Member, "attribute.repository/test-org/.fullsend") + // Verify PEMs were zeroed. for role, pem := range p.cfg.AgentPEMs { for _, b := range pem { @@ -1038,6 +1059,46 @@ func TestProvisioner_Provision_AddSecretVersionError(t *testing.T) { assert.Contains(t, err.Error(), "version error") } +func TestProvisioner_Provision_SetProjectIAMBindingError(t *testing.T) { + fake := newFakeGCFClient() + fake.errs["SetProjectIAMBinding"] = fmt.Errorf("project iam denied") + + p := newTestProvisioner(Config{ + ProjectID: "test-project-id", + GitHubOrgs: []string{"org"}, + AgentPEMs: singleRolePEMs(), + AgentAppIDs: singleRoleAppIDs(), + FunctionSourceDir: fakeFunctionSourceDir(t), + }, fake) + + _, err := p.Provision(context.Background()) + require.Error(t, err) + assert.Contains(t, err.Error(), "granting Vertex AI access for org org") + assert.Contains(t, err.Error(), "project iam denied") +} + +func TestProvisioner_Provision_MultiOrg_ProjectIAMBindings(t *testing.T) { + fake := newFakeGCFClient() + fake.functionInfoAfterCreate = &FunctionInfo{URI: "https://mint.run.app"} + + p := newTestProvisioner(Config{ + ProjectID: "shared-project", + GitHubOrgs: []string{"org-a", "org-b"}, + AgentPEMs: singleRolePEMs(), + AgentAppIDs: singleRoleAppIDs(), + FunctionSourceDir: fakeFunctionSourceDir(t), + }, fake) + + _, err := p.Provision(context.Background()) + require.NoError(t, err) + + require.Len(t, fake.projectIAMBindings, 2) + assert.Contains(t, fake.projectIAMBindings[0].Member, "attribute.repository/org-a/.fullsend") + assert.Contains(t, fake.projectIAMBindings[1].Member, "attribute.repository/org-b/.fullsend") + assert.Equal(t, "roles/aiplatform.user", fake.projectIAMBindings[0].Role) + assert.Equal(t, "roles/aiplatform.user", fake.projectIAMBindings[1].Role) +} + func TestProvisioner_Provision_SetIAMBindingError(t *testing.T) { fake := newFakeGCFClient() fake.errs["SetSecretIAMBinding"] = fmt.Errorf("iam error") @@ -1152,7 +1213,7 @@ func TestProvisioner_Provision_MultiOrg_WIFCondition(t *testing.T) { _, err := p.Provision(context.Background()) require.NoError(t, err) - assert.Equal(t, "assertion.repository_owner in ['acme', 'widgetco']", + assert.Equal(t, "assertion.repository in ['acme/.fullsend', 'widgetco/.fullsend']", fake.lastWIFProviderConfig.AttributeCondition) } @@ -1171,7 +1232,7 @@ func TestProvisioner_Provision_SingleOrg_WIFCondition(t *testing.T) { _, err := p.Provision(context.Background()) require.NoError(t, err) - assert.Equal(t, "assertion.repository_owner == 'acme'", + assert.Equal(t, "assertion.repository == 'acme/.fullsend'", fake.lastWIFProviderConfig.AttributeCondition) } @@ -1257,7 +1318,7 @@ func TestProvisioner_Provision_MultiOrg_MergeDoesNotOverwriteExistingPEMs(t *tes } // WIF condition should include both orgs. - assert.Equal(t, "assertion.repository_owner in ['existing-org', 'new-org']", + assert.Equal(t, "assertion.repository in ['existing-org/.fullsend', 'new-org/.fullsend']", fake.lastWIFProviderConfig.AttributeCondition) // ROLE_APP_IDS should preserve existing-org's entries and add new-org's. diff --git a/internal/inference/vertex/vertex.go b/internal/inference/vertex/vertex.go index 173bc16dfa..3740af7746 100644 --- a/internal/inference/vertex/vertex.go +++ b/internal/inference/vertex/vertex.go @@ -44,10 +44,6 @@ const ( // Stored as a secret so the value is masked in GitHub Actions logs. SecretWIFProvider = "FULLSEND_GCP_WIF_PROVIDER" - // SecretWIFServiceAccount is the repo secret for the SA email used with WIF. - // Stored as a secret so the value is masked in GitHub Actions logs. - SecretWIFServiceAccount = "FULLSEND_GCP_WIF_SA_EMAIL" - // defaultSAName is the service account name created in mode 1. defaultSAName = "fullsend-agent" ) @@ -72,7 +68,6 @@ type Config struct { ServiceAccountName string // optional: existing SA name (sa_key mode 2) CredentialJSON []byte // optional: pre-made key JSON (sa_key mode 3) WIFProvider string // WIF mode: full provider resource name - WIFServiceAccount string // WIF mode: service account email } // Provider implements inference.Provider for Vertex AI. @@ -106,7 +101,7 @@ func (p *Provider) Name() string { // SecretNames returns the secret names this provider manages. func (p *Provider) SecretNames() []string { if p.cfg.Mode == AuthModeWIF { - return []string{SecretWIFProvider, SecretWIFServiceAccount, SecretProjectID} + return []string{SecretWIFProvider, SecretProjectID} } return []string{SecretCredentials, SecretProjectID} } @@ -138,13 +133,9 @@ func (p *Provider) provisionWIF() (map[string]string, error) { if p.cfg.WIFProvider == "" { return nil, fmt.Errorf("WIF provider resource name is required") } - if p.cfg.WIFServiceAccount == "" { - return nil, fmt.Errorf("WIF service account email is required") - } return map[string]string{ - SecretWIFProvider: p.cfg.WIFProvider, - SecretWIFServiceAccount: p.cfg.WIFServiceAccount, - SecretProjectID: p.cfg.ProjectID, + SecretWIFProvider: p.cfg.WIFProvider, + SecretProjectID: p.cfg.ProjectID, }, nil } diff --git a/internal/inference/vertex/vertex_test.go b/internal/inference/vertex/vertex_test.go index 2e2cee1d93..5e162100eb 100644 --- a/internal/inference/vertex/vertex_test.go +++ b/internal/inference/vertex/vertex_test.go @@ -211,7 +211,7 @@ func TestProvision_AnalyzeOnly(t *testing.T) { t.Run("wif mode", func(t *testing.T) { p := NewAnalyzeOnly(AuthModeWIF) assert.Equal(t, "vertex", p.Name()) - assert.Equal(t, []string{SecretWIFProvider, SecretWIFServiceAccount, SecretProjectID}, p.SecretNames()) + assert.Equal(t, []string{SecretWIFProvider, SecretProjectID}, p.SecretNames()) }) t.Run("empty defaults to sa_key", func(t *testing.T) { @@ -222,28 +222,25 @@ func TestProvision_AnalyzeOnly(t *testing.T) { func TestProvision_WIF(t *testing.T) { p := New(Config{ - ProjectID: "my-project", - Region: "global", - Mode: AuthModeWIF, - WIFProvider: "projects/123/locations/global/workloadIdentityPools/pool/providers/gh", - WIFServiceAccount: "sa@my-project.iam.gserviceaccount.com", + ProjectID: "my-project", + Region: "global", + Mode: AuthModeWIF, + WIFProvider: "projects/123/locations/global/workloadIdentityPools/pool/providers/gh", }, nil) secrets, err := p.Provision(context.Background()) require.NoError(t, err) assert.Equal(t, "projects/123/locations/global/workloadIdentityPools/pool/providers/gh", secrets[SecretWIFProvider]) - assert.Equal(t, "sa@my-project.iam.gserviceaccount.com", secrets[SecretWIFServiceAccount]) assert.Equal(t, "my-project", secrets[SecretProjectID]) - assert.Len(t, secrets, 3) + assert.Len(t, secrets, 2) } func TestProvision_WIF_MissingProvider(t *testing.T) { p := New(Config{ - ProjectID: "my-project", - Region: "global", - Mode: AuthModeWIF, - WIFServiceAccount: "sa@my-project.iam.gserviceaccount.com", + ProjectID: "my-project", + Region: "global", + Mode: AuthModeWIF, }, nil) _, err := p.Provision(context.Background()) @@ -251,23 +248,10 @@ func TestProvision_WIF_MissingProvider(t *testing.T) { assert.Contains(t, err.Error(), "WIF provider resource name is required") } -func TestProvision_WIF_MissingSA(t *testing.T) { - p := New(Config{ - ProjectID: "my-project", - Region: "global", - Mode: AuthModeWIF, - WIFProvider: "projects/123/locations/global/workloadIdentityPools/pool/providers/gh", - }, nil) - - _, err := p.Provision(context.Background()) - require.Error(t, err) - assert.Contains(t, err.Error(), "WIF service account email is required") -} - func TestSecretNames_WIF(t *testing.T) { p := New(Config{Mode: AuthModeWIF}, nil) names := p.SecretNames() - assert.Equal(t, []string{SecretWIFProvider, SecretWIFServiceAccount, SecretProjectID}, names) + assert.Equal(t, []string{SecretWIFProvider, SecretProjectID}, names) } func TestName(t *testing.T) { diff --git a/internal/scaffold/fullsend-repo/.github/actions/setup-gcp/action.yml b/internal/scaffold/fullsend-repo/.github/actions/setup-gcp/action.yml index cef9494a66..f918c11b4c 100644 --- a/internal/scaffold/fullsend-repo/.github/actions/setup-gcp/action.yml +++ b/internal/scaffold/fullsend-repo/.github/actions/setup-gcp/action.yml @@ -8,9 +8,6 @@ inputs: gcp_wif_provider: description: 'Workload Identity Federation provider (required if gcp_auth_mode is "wif")' required: false - gcp_wif_sa_email: - description: 'Service account email for WIF (required if gcp_auth_mode is "wif")' - required: false gcp_sa_key_json: description: 'Service account key JSON (required if gcp_auth_mode is not "wif")' required: false @@ -27,7 +24,6 @@ runs: uses: google-github-actions/auth@v3 with: workload_identity_provider: ${{ inputs.gcp_wif_provider }} - service_account: ${{ inputs.gcp_wif_sa_email }} - name: Authenticate to Google Cloud (SA key) if: inputs.gcp_auth_mode != 'wif' diff --git a/internal/scaffold/fullsend-repo/.github/workflows/code.yml b/internal/scaffold/fullsend-repo/.github/workflows/code.yml index 2d26ce7f52..5479dbddc4 100644 --- a/internal/scaffold/fullsend-repo/.github/workflows/code.yml +++ b/internal/scaffold/fullsend-repo/.github/workflows/code.yml @@ -69,7 +69,6 @@ jobs: with: gcp_auth_mode: ${{ vars.FULLSEND_GCP_AUTH_MODE }} gcp_wif_provider: ${{ secrets.FULLSEND_GCP_WIF_PROVIDER }} - gcp_wif_sa_email: ${{ secrets.FULLSEND_GCP_WIF_SA_EMAIL }} gcp_sa_key_json: ${{ secrets.FULLSEND_GCP_SA_KEY_JSON }} - name: Setup agent environment diff --git a/internal/scaffold/fullsend-repo/.github/workflows/fix.yml b/internal/scaffold/fullsend-repo/.github/workflows/fix.yml index 1b19ebbc92..4e91d25b9b 100644 --- a/internal/scaffold/fullsend-repo/.github/workflows/fix.yml +++ b/internal/scaffold/fullsend-repo/.github/workflows/fix.yml @@ -256,7 +256,6 @@ jobs: with: gcp_auth_mode: ${{ vars.FULLSEND_GCP_AUTH_MODE }} gcp_wif_provider: ${{ secrets.FULLSEND_GCP_WIF_PROVIDER }} - gcp_wif_sa_email: ${{ secrets.FULLSEND_GCP_WIF_SA_EMAIL }} gcp_sa_key_json: ${{ secrets.FULLSEND_GCP_SA_KEY_JSON }} - name: Setup agent environment diff --git a/internal/scaffold/fullsend-repo/.github/workflows/prioritize.yml b/internal/scaffold/fullsend-repo/.github/workflows/prioritize.yml index c89ae79ec9..947010f88d 100644 --- a/internal/scaffold/fullsend-repo/.github/workflows/prioritize.yml +++ b/internal/scaffold/fullsend-repo/.github/workflows/prioritize.yml @@ -52,7 +52,6 @@ jobs: with: gcp_auth_mode: ${{ vars.FULLSEND_GCP_AUTH_MODE }} gcp_wif_provider: ${{ secrets.FULLSEND_GCP_WIF_PROVIDER }} - gcp_wif_sa_email: ${{ secrets.FULLSEND_GCP_WIF_SA_EMAIL }} gcp_sa_key_json: ${{ secrets.FULLSEND_GCP_SA_KEY_JSON }} - name: Setup agent environment diff --git a/internal/scaffold/fullsend-repo/.github/workflows/retro.yml b/internal/scaffold/fullsend-repo/.github/workflows/retro.yml index f50d632875..6b73b9bf95 100644 --- a/internal/scaffold/fullsend-repo/.github/workflows/retro.yml +++ b/internal/scaffold/fullsend-repo/.github/workflows/retro.yml @@ -59,7 +59,6 @@ jobs: with: gcp_auth_mode: ${{ vars.FULLSEND_GCP_AUTH_MODE }} gcp_wif_provider: ${{ secrets.FULLSEND_GCP_WIF_PROVIDER }} - gcp_wif_sa_email: ${{ secrets.FULLSEND_GCP_WIF_SA_EMAIL }} gcp_sa_key_json: ${{ secrets.FULLSEND_GCP_SA_KEY_JSON }} - name: Setup agent environment diff --git a/internal/scaffold/fullsend-repo/.github/workflows/review.yml b/internal/scaffold/fullsend-repo/.github/workflows/review.yml index 81f7c3bbb4..ebc9578db3 100644 --- a/internal/scaffold/fullsend-repo/.github/workflows/review.yml +++ b/internal/scaffold/fullsend-repo/.github/workflows/review.yml @@ -60,7 +60,6 @@ jobs: with: gcp_auth_mode: ${{ vars.FULLSEND_GCP_AUTH_MODE }} gcp_wif_provider: ${{ secrets.FULLSEND_GCP_WIF_PROVIDER }} - gcp_wif_sa_email: ${{ secrets.FULLSEND_GCP_WIF_SA_EMAIL }} gcp_sa_key_json: ${{ secrets.FULLSEND_GCP_SA_KEY_JSON }} - name: Setup agent environment diff --git a/internal/scaffold/fullsend-repo/.github/workflows/triage.yml b/internal/scaffold/fullsend-repo/.github/workflows/triage.yml index 5ad3bd2a59..5488b7354d 100644 --- a/internal/scaffold/fullsend-repo/.github/workflows/triage.yml +++ b/internal/scaffold/fullsend-repo/.github/workflows/triage.yml @@ -59,7 +59,6 @@ jobs: with: gcp_auth_mode: ${{ vars.FULLSEND_GCP_AUTH_MODE }} gcp_wif_provider: ${{ secrets.FULLSEND_GCP_WIF_PROVIDER }} - gcp_wif_sa_email: ${{ secrets.FULLSEND_GCP_WIF_SA_EMAIL }} gcp_sa_key_json: ${{ secrets.FULLSEND_GCP_SA_KEY_JSON }} - name: Setup agent environment diff --git a/internal/scaffold/fullsend-repo/scripts/prepare-sandbox-credentials.sh b/internal/scaffold/fullsend-repo/scripts/prepare-sandbox-credentials.sh index 57d6c40f76..c8a4e2eef5 100755 --- a/internal/scaffold/fullsend-repo/scripts/prepare-sandbox-credentials.sh +++ b/internal/scaffold/fullsend-repo/scripts/prepare-sandbox-credentials.sh @@ -32,16 +32,11 @@ if [[ "$CRED_TYPE" == "external_account" ]]; then SANDBOX_CREDS="$RUNNER_TEMP/sandbox-gcp-credentials.json" jq '{ - type: .type, - audience: .audience, - subject_token_type: .subject_token_type, - token_url: .token_url, - service_account_impersonation_url: .service_account_impersonation_url, - credential_source: { - file: "/tmp/workspace/.gcp-oidc-token", - format: .credential_source.format - } - }' "$CRED_CONFIG" > "$SANDBOX_CREDS" + type, audience, subject_token_type, token_url, + credential_source: { file: "/tmp/workspace/.gcp-oidc-token", format: .credential_source.format } + } + (if .service_account_impersonation_url then + {service_account_impersonation_url} + else {} end)' "$CRED_CONFIG" > "$SANDBOX_CREDS" OIDC_AUTH_FILE="$RUNNER_TEMP/gcp-oidc-auth" printf '%s' "$OIDC_AUTH" > "$OIDC_AUTH_FILE" diff --git a/internal/scaffold/scaffold_test.go b/internal/scaffold/scaffold_test.go index f747575208..d7cc3b819a 100644 --- a/internal/scaffold/scaffold_test.go +++ b/internal/scaffold/scaffold_test.go @@ -291,6 +291,7 @@ func TestCodeWorkflowContent(t *testing.T) { assert.Contains(t, s, "./.github/actions/validate-enrollment") assert.NotContains(t, s, "create-github-app-token") assert.NotContains(t, s, "FULLSEND_CODER_CLIENT_ID") + assert.NotContains(t, s, "GCP_WIF_SA_EMAIL") // Verify concurrency group prevents overlapping runs for same issue assert.Contains(t, s, "concurrency:") assert.Contains(t, s, "fullsend-code-") @@ -370,7 +371,7 @@ func TestSetupGcpActionContent(t *testing.T) { assert.Contains(t, s, "inputs:") assert.Contains(t, s, "gcp_auth_mode:") assert.Contains(t, s, "gcp_wif_provider:") - assert.Contains(t, s, "gcp_wif_sa_email:") + assert.NotContains(t, s, "gcp_wif_sa_email:") assert.Contains(t, s, "gcp_sa_key_json:") // Verify pre-mask step assert.Contains(t, s, "Pre-mask GCP credential file path") @@ -379,7 +380,7 @@ func TestSetupGcpActionContent(t *testing.T) { assert.Contains(t, s, "if: inputs.gcp_auth_mode == 'wif'") assert.Contains(t, s, "google-github-actions/auth@v3") assert.Contains(t, s, "workload_identity_provider:") - assert.Contains(t, s, "service_account:") + assert.NotContains(t, s, "service_account:") // Verify SA key authentication path assert.Contains(t, s, "if: inputs.gcp_auth_mode != 'wif'") assert.Contains(t, s, "credentials_json:") From c7eef6b9bf17eb134e96612837741ec42b13360d Mon Sep 17 00:00:00 2001 From: Wayne Sun Date: Tue, 12 May 2026 23:00:32 -0400 Subject: [PATCH 2/3] feat: pass project_id to auth action and GOOGLE_CLOUD_PROJECT into sandbox Direct WIF (no intermediate service account) requires project_id in the google-github-actions/auth call so the runner sets GOOGLE_CLOUD_PROJECT. Inside the OpenShell sandbox this env var is lost, so pass it through gcp-vertex.env to let the Google Auth library skip the CRM projects.get fallback that WIF principals cannot call. Signed-off-by: Wayne Sun --- .../fullsend-repo/.github/actions/setup-gcp/action.yml | 4 ++++ internal/scaffold/fullsend-repo/.github/workflows/code.yml | 1 + internal/scaffold/fullsend-repo/.github/workflows/fix.yml | 1 + .../scaffold/fullsend-repo/.github/workflows/prioritize.yml | 1 + internal/scaffold/fullsend-repo/.github/workflows/retro.yml | 1 + internal/scaffold/fullsend-repo/.github/workflows/review.yml | 1 + internal/scaffold/fullsend-repo/.github/workflows/triage.yml | 1 + internal/scaffold/fullsend-repo/env/gcp-vertex.env | 1 + 8 files changed, 11 insertions(+) diff --git a/internal/scaffold/fullsend-repo/.github/actions/setup-gcp/action.yml b/internal/scaffold/fullsend-repo/.github/actions/setup-gcp/action.yml index f918c11b4c..e20368df7b 100644 --- a/internal/scaffold/fullsend-repo/.github/actions/setup-gcp/action.yml +++ b/internal/scaffold/fullsend-repo/.github/actions/setup-gcp/action.yml @@ -8,6 +8,9 @@ inputs: gcp_wif_provider: description: 'Workload Identity Federation provider (required if gcp_auth_mode is "wif")' required: false + gcp_project_id: + description: 'GCP project ID — passed to google-github-actions/auth so the runner env has GOOGLE_CLOUD_PROJECT for direct WIF' + required: false gcp_sa_key_json: description: 'Service account key JSON (required if gcp_auth_mode is not "wif")' required: false @@ -24,6 +27,7 @@ runs: uses: google-github-actions/auth@v3 with: workload_identity_provider: ${{ inputs.gcp_wif_provider }} + project_id: ${{ inputs.gcp_project_id }} - name: Authenticate to Google Cloud (SA key) if: inputs.gcp_auth_mode != 'wif' diff --git a/internal/scaffold/fullsend-repo/.github/workflows/code.yml b/internal/scaffold/fullsend-repo/.github/workflows/code.yml index 5479dbddc4..4d40e143b4 100644 --- a/internal/scaffold/fullsend-repo/.github/workflows/code.yml +++ b/internal/scaffold/fullsend-repo/.github/workflows/code.yml @@ -69,6 +69,7 @@ jobs: with: gcp_auth_mode: ${{ vars.FULLSEND_GCP_AUTH_MODE }} gcp_wif_provider: ${{ secrets.FULLSEND_GCP_WIF_PROVIDER }} + gcp_project_id: ${{ secrets.FULLSEND_GCP_PROJECT_ID }} gcp_sa_key_json: ${{ secrets.FULLSEND_GCP_SA_KEY_JSON }} - name: Setup agent environment diff --git a/internal/scaffold/fullsend-repo/.github/workflows/fix.yml b/internal/scaffold/fullsend-repo/.github/workflows/fix.yml index 4e91d25b9b..920fba6e39 100644 --- a/internal/scaffold/fullsend-repo/.github/workflows/fix.yml +++ b/internal/scaffold/fullsend-repo/.github/workflows/fix.yml @@ -256,6 +256,7 @@ jobs: with: gcp_auth_mode: ${{ vars.FULLSEND_GCP_AUTH_MODE }} gcp_wif_provider: ${{ secrets.FULLSEND_GCP_WIF_PROVIDER }} + gcp_project_id: ${{ secrets.FULLSEND_GCP_PROJECT_ID }} gcp_sa_key_json: ${{ secrets.FULLSEND_GCP_SA_KEY_JSON }} - name: Setup agent environment diff --git a/internal/scaffold/fullsend-repo/.github/workflows/prioritize.yml b/internal/scaffold/fullsend-repo/.github/workflows/prioritize.yml index 947010f88d..be10b7eb07 100644 --- a/internal/scaffold/fullsend-repo/.github/workflows/prioritize.yml +++ b/internal/scaffold/fullsend-repo/.github/workflows/prioritize.yml @@ -52,6 +52,7 @@ jobs: with: gcp_auth_mode: ${{ vars.FULLSEND_GCP_AUTH_MODE }} gcp_wif_provider: ${{ secrets.FULLSEND_GCP_WIF_PROVIDER }} + gcp_project_id: ${{ secrets.FULLSEND_GCP_PROJECT_ID }} gcp_sa_key_json: ${{ secrets.FULLSEND_GCP_SA_KEY_JSON }} - name: Setup agent environment diff --git a/internal/scaffold/fullsend-repo/.github/workflows/retro.yml b/internal/scaffold/fullsend-repo/.github/workflows/retro.yml index 6b73b9bf95..71eea89abe 100644 --- a/internal/scaffold/fullsend-repo/.github/workflows/retro.yml +++ b/internal/scaffold/fullsend-repo/.github/workflows/retro.yml @@ -59,6 +59,7 @@ jobs: with: gcp_auth_mode: ${{ vars.FULLSEND_GCP_AUTH_MODE }} gcp_wif_provider: ${{ secrets.FULLSEND_GCP_WIF_PROVIDER }} + gcp_project_id: ${{ secrets.FULLSEND_GCP_PROJECT_ID }} gcp_sa_key_json: ${{ secrets.FULLSEND_GCP_SA_KEY_JSON }} - name: Setup agent environment diff --git a/internal/scaffold/fullsend-repo/.github/workflows/review.yml b/internal/scaffold/fullsend-repo/.github/workflows/review.yml index ebc9578db3..12fd80457c 100644 --- a/internal/scaffold/fullsend-repo/.github/workflows/review.yml +++ b/internal/scaffold/fullsend-repo/.github/workflows/review.yml @@ -60,6 +60,7 @@ jobs: with: gcp_auth_mode: ${{ vars.FULLSEND_GCP_AUTH_MODE }} gcp_wif_provider: ${{ secrets.FULLSEND_GCP_WIF_PROVIDER }} + gcp_project_id: ${{ secrets.FULLSEND_GCP_PROJECT_ID }} gcp_sa_key_json: ${{ secrets.FULLSEND_GCP_SA_KEY_JSON }} - name: Setup agent environment diff --git a/internal/scaffold/fullsend-repo/.github/workflows/triage.yml b/internal/scaffold/fullsend-repo/.github/workflows/triage.yml index 5488b7354d..85450ef724 100644 --- a/internal/scaffold/fullsend-repo/.github/workflows/triage.yml +++ b/internal/scaffold/fullsend-repo/.github/workflows/triage.yml @@ -59,6 +59,7 @@ jobs: with: gcp_auth_mode: ${{ vars.FULLSEND_GCP_AUTH_MODE }} gcp_wif_provider: ${{ secrets.FULLSEND_GCP_WIF_PROVIDER }} + gcp_project_id: ${{ secrets.FULLSEND_GCP_PROJECT_ID }} gcp_sa_key_json: ${{ secrets.FULLSEND_GCP_SA_KEY_JSON }} - name: Setup agent environment diff --git a/internal/scaffold/fullsend-repo/env/gcp-vertex.env b/internal/scaffold/fullsend-repo/env/gcp-vertex.env index 9277d498cf..1852aea605 100644 --- a/internal/scaffold/fullsend-repo/env/gcp-vertex.env +++ b/internal/scaffold/fullsend-repo/env/gcp-vertex.env @@ -2,3 +2,4 @@ export CLAUDE_CODE_USE_VERTEX=1 export ANTHROPIC_VERTEX_PROJECT_ID=${ANTHROPIC_VERTEX_PROJECT_ID} export CLOUD_ML_REGION=${CLOUD_ML_REGION} export GOOGLE_APPLICATION_CREDENTIALS=/tmp/workspace/.gcp-credentials.json +export GOOGLE_CLOUD_PROJECT=${GOOGLE_CLOUD_PROJECT} From b6f3b48e0bd732f9e2ff663823c5ceb4902e0d6c Mon Sep 17 00:00:00 2001 From: Wayne Sun Date: Wed, 13 May 2026 11:34:42 -0400 Subject: [PATCH 3/3] refactor: remove SA key auth path, WIF is the only GCP auth mode Drop gcp_sa_key_json input, SA key provisioning, AuthModeSAKey, GCPClient interface, gcp.go, and extractProjectID. Vertex provider now only handles direct WIF with workload_identity_provider + project_id. Update all test fixtures to use WIF secret names. Signed-off-by: Wayne Sun --- docs/guides/admin/installation.md | 78 +----- .../adr-0014-github-apps-and-secrets/SPEC.md | 5 +- e2e/admin/admin_test.go | 44 +-- internal/cli/admin.go | 77 +----- internal/inference/vertex/gcp.go | 143 ---------- internal/inference/vertex/vertex.go | 129 +-------- internal/inference/vertex/vertex_test.go | 260 ++---------------- internal/layers/inference.go | 2 +- internal/layers/inference_test.go | 14 +- .../.github/actions/setup-gcp/action.yml | 31 +-- .../fullsend-repo/.github/workflows/code.yml | 2 - .../fullsend-repo/.github/workflows/fix.yml | 2 - .../.github/workflows/prioritize.yml | 2 - .../fullsend-repo/.github/workflows/retro.yml | 2 - .../.github/workflows/review.yml | 2 - .../.github/workflows/triage.yml | 2 - internal/scaffold/scaffold_test.go | 16 +- 17 files changed, 87 insertions(+), 724 deletions(-) delete mode 100644 internal/inference/vertex/gcp.go diff --git a/docs/guides/admin/installation.md b/docs/guides/admin/installation.md index 5c49415165..1e9efa28d6 100644 --- a/docs/guides/admin/installation.md +++ b/docs/guides/admin/installation.md @@ -36,31 +36,14 @@ The `--gcp-region` flag is required when `--gcp-project` is set. Use `global` fo ## 1. Set up GCP authentication -Fullsend supports two methods for authenticating to Vertex AI. **Workload Identity Federation (WIF) is recommended** — it eliminates long-lived credentials entirely. +Fullsend uses [Workload Identity Federation (WIF)](https://cloud.google.com/iam/docs/workload-identity-federation) to authenticate GitHub Actions to Vertex AI. WIF eliminates long-lived credentials — GitHub Actions exchange short-lived OIDC tokens for GCP access tokens. See the [google-github-actions/auth documentation](https://github.com/google-github-actions/auth#direct-workload-identity-federation) for background on direct WIF. -### Option A: Workload Identity Federation (recommended) - -WIF lets GitHub Actions exchange short-lived OIDC tokens for GCP access tokens. No service account keys are stored. - -**1a. Create a service account** +**1a. Create a Workload Identity Pool and OIDC Provider** ```bash export GCP_PROJECT="" export ORG_NAME="" -gcloud iam service-accounts create fullsend-agent \ - --display-name="Fullsend agent inference" \ - --project="$GCP_PROJECT" - -gcloud projects add-iam-policy-binding "$GCP_PROJECT" \ - --member="serviceAccount:fullsend-agent@$GCP_PROJECT.iam.gserviceaccount.com" \ - --role="roles/aiplatform.user" \ - --condition=None -``` - -**1b. Create a Workload Identity Pool and OIDC Provider** - -```bash gcloud iam workload-identity-pools create github-actions \ --location=global \ --display-name="GitHub Actions" \ @@ -77,44 +60,24 @@ gcloud iam workload-identity-pools providers create-oidc github \ The `attribute-condition` restricts which GitHub Actions workflows can exchange OIDC tokens for GCP credentials. Scoping to `$ORG_NAME/.fullsend` ensures only the `.fullsend` config repo can authenticate — workflows in other repos cannot obtain Vertex AI credentials. -**1c. Grant Vertex AI access to the WIF principal** +**1b. Grant Vertex AI access to the WIF principal** ```bash export PROJECT_NUMBER=$(gcloud projects describe "$GCP_PROJECT" --format='value(projectNumber)') +export WIF_PRINCIPAL="principalSet://iam.googleapis.com/projects/$PROJECT_NUMBER/locations/global/workloadIdentityPools/github-actions/attribute.repository/$ORG_NAME/.fullsend" gcloud projects add-iam-policy-binding "$GCP_PROJECT" \ --role="roles/aiplatform.user" \ - --member="principalSet://iam.googleapis.com/projects/$PROJECT_NUMBER/locations/global/workloadIdentityPools/github-actions/attribute.repository/$ORG_NAME/.fullsend" \ + --member="$WIF_PRINCIPAL" \ --condition=None ``` -**1d. Note the WIF provider resource name** +**1c. Note the WIF provider resource name** ```bash export WIF_PROVIDER="projects/$PROJECT_NUMBER/locations/global/workloadIdentityPools/github-actions/providers/github" ``` -### Option B: Service account key (legacy) - -Create a service account with the `Vertex AI User` role and download its key: - -```bash -export GCP_PROJECT="" -export ORG_NAME="" - -gcloud iam service-accounts create "$ORG_NAME" \ - --display-name="Fullsend for $ORG_NAME" \ - --project="$GCP_PROJECT" - -gcloud projects add-iam-policy-binding "$GCP_PROJECT" \ - --member="serviceAccount:$ORG_NAME@$GCP_PROJECT.iam.gserviceaccount.com" \ - --role="roles/aiplatform.user" \ - --condition=None - -gcloud iam service-accounts keys create sa-key.json \ - --iam-account="$ORG_NAME@$GCP_PROJECT.iam.gserviceaccount.com" -``` - ## 2. Run the installer The installer is interactive. It will open multiple browser windows to create and install a GitHub App for each agent role. Follow the prompts in each window to complete the app setup. @@ -182,35 +145,6 @@ fullsend admin install "$ADDITIONAL_ORG" \ > **Note:** Multi-org with `--public` requires all orgs to share the same GitHub Apps. Private apps (the default) are single-org only. -**With SA key (legacy):** - -```bash -fullsend admin install "$ORG_NAME" \ - --gcp-project "$GCP_PROJECT" \ - --gcp-region global \ - --gcp-credentials-file sa-key.json \ - --mint-project "$GCP_PROJECT" -rm sa-key.json -``` - -### Migrating from SA key to WIF - -If you already have fullsend installed with a service account key: - -1. Create the WIF resources (steps 1a–1d in Option A above) -2. Re-run the installer with WIF flags (the installer updates secrets in-place): - ```bash - fullsend admin install "$ORG_NAME" \ - --skip-app-setup \ - --gcp-project "$GCP_PROJECT" \ - --gcp-region global \ - --gcp-wif-provider "$WIF_PROVIDER" \ - --mint-project "$GCP_PROJECT" - ``` -3. Verify a workflow run succeeds with WIF auth (check for "Authenticated using Workload Identity Federation" in the auth step output) -4. Delete the old SA key: `gcloud iam service-accounts keys delete --iam-account=...` -5. Remove the `FULLSEND_GCP_SA_KEY_JSON` secret from the `.fullsend` repo settings once the scaffolded agent workflows have been updated to use WIF (re-running `fullsend admin install` with `--skip-app-setup` updates the workflows) - ## 3. Merge enrollment PRs If you chose to enroll repositories during install, the installer dispatches a workflow that creates an enrollment PR in each enrolled repo. These PRs add a shim workflow (`.github/workflows/fullsend.yaml`) that wires events to the agent pipeline. diff --git a/docs/normative/admin-install/v1/adr-0014-github-apps-and-secrets/SPEC.md b/docs/normative/admin-install/v1/adr-0014-github-apps-and-secrets/SPEC.md index 38f92781bb..787c795faa 100644 --- a/docs/normative/admin-install/v1/adr-0014-github-apps-and-secrets/SPEC.md +++ b/docs/normative/admin-install/v1/adr-0014-github-apps-and-secrets/SPEC.md @@ -51,14 +51,13 @@ All of the following are **repository-level** Actions secrets and variables on * |----------|----------------------------------------|-------|-------| | Secret | `FULLSEND__APP_PRIVATE_KEY` | PEM text of the GitHub App private key | secrets | | Variable | `FULLSEND__CLIENT_ID` | GitHub App Client ID (e.g. `Iv23_...`), per [GitHub recommendation](https://github.blog/changelog/2024-05-01-github-apps-can-now-use-the-client-id-to-fetch-installation-tokens/) | secrets | -| Secret | `FULLSEND_GCP_SA_KEY_JSON` | GCP service account key JSON (SA key mode only) | inference | -| Secret | `FULLSEND_GCP_WIF_PROVIDER` | Full WIF provider resource name (WIF mode only) | inference | +| Secret | `FULLSEND_GCP_WIF_PROVIDER` | Full WIF provider resource name | inference | | Secret | `FULLSEND_GCP_PROJECT_ID` | GCP project identifier (when inference provider is `vertex`) | inference | | Variable | `FULLSEND_GCP_REGION` | GCP region for Vertex AI (e.g. `us-east5`) | inference | - `` is the agent role in **ASCII uppercase** (e.g. `FULLSEND_TRIAGE_APP_PRIVATE_KEY`). - For each role processed in install, if PEM is non-empty, the implementation **must** create/update the secret; if PEM is empty (reuse path), the implementation **must** skip writing that role’s secret. The Client ID variable is **always** written (even on reuse) to ensure it stays current. -- Inference secrets are only created when an inference provider is configured in `config.yaml` (see [ADR 0011](../adr-0011-org-config-yaml/SPEC.md)). When `inference.provider` is `vertex`, the implementation **must** store `FULLSEND_GCP_PROJECT_ID` and either the SA key secret (`FULLSEND_GCP_SA_KEY_JSON`) or the WIF provider secret (`FULLSEND_GCP_WIF_PROVIDER`). The two auth modes are mutually exclusive — WIF secrets and the SA key secret **must not** coexist. The WIF provider name is stored as a secret (not a variable) so its value is masked in GitHub Actions logs. +- Inference secrets are only created when an inference provider is configured in `config.yaml` (see [ADR 0011](../adr-0011-org-config-yaml/SPEC.md)). When `inference.provider` is `vertex`, the implementation **must** store `FULLSEND_GCP_PROJECT_ID` and `FULLSEND_GCP_WIF_PROVIDER`. The WIF provider name is stored as a secret (not a variable) so its value is masked in GitHub Actions logs. ## 6. Analyze / health semantics for the secrets layer diff --git a/e2e/admin/admin_test.go b/e2e/admin/admin_test.go index 832f46a8ff..f2199f22ef 100644 --- a/e2e/admin/admin_test.go +++ b/e2e/admin/admin_test.go @@ -163,7 +163,7 @@ func TestAdminInstallUninstall(t *testing.T) { // ========================================= // Phase 2.5: Triage dispatch smoke test // ========================================= - if os.Getenv("E2E_HALFSEND_VERTEX_KEY") != "" { + if os.Getenv("E2E_HALFSEND_WIF_PROVIDER") != "" { t.Log("=== Phase 2.5: Triage Dispatch Smoke Test ===") vendorBinaryForE2E(t, env) runTriageDispatchSmokeTest(t, env) @@ -271,29 +271,27 @@ func runFullInstall(t *testing.T, env *e2eEnv) ([]layers.AgentCredentials, *conf agents[i] = ac.AgentEntry } - // Build inference provider if vertex key is available (mode 3). + // Build inference provider if WIF provider is available. var inferenceProvider inference.Provider var inferenceProviderName string - if vertexKey := os.Getenv("E2E_HALFSEND_VERTEX_KEY"); vertexKey != "" { + if wifProvider := os.Getenv("E2E_HALFSEND_WIF_PROVIDER"); wifProvider != "" { gcpProjectID := os.Getenv("E2E_GCP_PROJECT_ID") if gcpProjectID == "" { - // Try to extract project_id from the key JSON. - gcpProjectID = extractProjectID(t, vertexKey) + t.Fatal("E2E_GCP_PROJECT_ID is required when E2E_HALFSEND_WIF_PROVIDER is set") } gcpRegion := os.Getenv("E2E_GCP_REGION") if gcpRegion == "" { gcpRegion = "global" } inferenceProvider = vertex.New(vertex.Config{ - ProjectID: gcpProjectID, - Region: gcpRegion, - CredentialJSON: []byte(vertexKey), - }, nil) - // Region is stored as a variable, not a secret. + ProjectID: gcpProjectID, + Region: gcpRegion, + WIFProvider: wifProvider, + }) inferenceProviderName = "vertex" t.Logf("Inference provider: vertex (project: %s)", gcpProjectID) } else { - t.Log("E2E_HALFSEND_VERTEX_KEY not set, skipping inference layer") + t.Log("E2E_HALFSEND_WIF_PROVIDER not set, skipping inference layer") } orgCfg := config.NewOrgConfig(repoNames, enabledRepos, defaultRoles, agents, inferenceProviderName) @@ -439,9 +437,9 @@ func verifyInstalled(t *testing.T, env *e2eEnv, orgCfg *config.OrgConfig, enable assert.True(t, exists, "variable %s should exist", varName) } - // Inference secrets exist if vertex key was provided. - if os.Getenv("E2E_HALFSEND_VERTEX_KEY") != "" { - for _, secretName := range []string{"FULLSEND_GCP_SA_KEY_JSON", "FULLSEND_GCP_PROJECT_ID"} { + // Inference secrets exist if WIF provider was configured. + if os.Getenv("E2E_HALFSEND_WIF_PROVIDER") != "" { + for _, secretName := range []string{"FULLSEND_GCP_WIF_PROVIDER", "FULLSEND_GCP_PROJECT_ID"} { exists, secErr := env.client.RepoSecretExists(ctx, testOrg, forge.ConfigRepoName, secretName) assert.NoError(t, secErr, "checking inference secret %s", secretName) assert.True(t, exists, "inference secret %s should exist", secretName) @@ -858,21 +856,3 @@ func hasPrivateRepos(repos []forge.Repository) bool { } return false } - -// extractProjectID attempts to extract project_id from a GCP service account -// key JSON string. Falls back to "unknown" if parsing fails. -func extractProjectID(t *testing.T, keyJSON string) string { - t.Helper() - var key struct { - ProjectID string `json:"project_id"` - } - if err := json.Unmarshal([]byte(keyJSON), &key); err != nil { - t.Logf("warning: could not parse project_id from vertex key: %v", err) - return "unknown" - } - if key.ProjectID == "" { - t.Log("warning: vertex key has empty project_id") - return "unknown" - } - return key.ProjectID -} diff --git a/internal/cli/admin.go b/internal/cli/admin.go index 50db845a02..2d5f95d934 100644 --- a/internal/cli/admin.go +++ b/internal/cli/admin.go @@ -3,7 +3,6 @@ package cli import ( "bufio" "context" - "encoding/json" "fmt" "io" "os" @@ -96,8 +95,6 @@ func newInstallCmd() *cobra.Command { var enrollNoneFlag bool var gcpProject string var gcpRegion string - var gcpServiceAccount string - var gcpCredentialsFile string var gcpWIFProvider string var mintProvider string var mintProject string @@ -155,53 +152,26 @@ func newInstallCmd() *cobra.Command { } // Validate GCP flag dependencies. - if gcpProject == "" && (gcpServiceAccount != "" || gcpCredentialsFile != "" || gcpRegion != "" || gcpWIFProvider != "") { - return fmt.Errorf("--gcp-service-account, --gcp-credentials-file, --gcp-wif-provider, and --gcp-region require --gcp-project to be set") + if gcpProject == "" && (gcpRegion != "" || gcpWIFProvider != "") { + return fmt.Errorf("--gcp-wif-provider and --gcp-region require --gcp-project to be set") } if gcpProject != "" && gcpRegion == "" { return fmt.Errorf("--gcp-region is required when --gcp-project is set") } - if gcpWIFProvider != "" && gcpCredentialsFile != "" { - return fmt.Errorf("--gcp-wif-provider and --gcp-credentials-file are mutually exclusive: use WIF or SA key, not both") - } - if gcpWIFProvider != "" && gcpServiceAccount != "" { - return fmt.Errorf("--gcp-wif-provider and --gcp-service-account are mutually exclusive") + if gcpProject != "" && gcpWIFProvider == "" { + return fmt.Errorf("--gcp-wif-provider is required when --gcp-project is set") } // Build inference provider from GCP flags. var inferenceProvider inference.Provider var inferenceProviderName string if gcpProject != "" { - vcfg := vertex.Config{ProjectID: gcpProject, Region: gcpRegion} - if gcpWIFProvider != "" { - vcfg.Mode = vertex.AuthModeWIF - vcfg.WIFProvider = gcpWIFProvider - } else { - vcfg.ServiceAccountName = gcpServiceAccount - if gcpCredentialsFile != "" { - info, statErr := os.Lstat(gcpCredentialsFile) - if statErr != nil { - return fmt.Errorf("checking credentials file: %w", statErr) - } - if !info.Mode().IsRegular() { - return fmt.Errorf("credentials file %s must be a regular file", gcpCredentialsFile) - } - credData, readErr := os.ReadFile(gcpCredentialsFile) - if readErr != nil { - return fmt.Errorf("reading credentials file: %w", readErr) - } - defer func() { - for i := range credData { - credData[i] = 0 - } - }() - if err := validateCredentialJSON(credData); err != nil { - return err - } - vcfg.CredentialJSON = credData - } + vcfg := vertex.Config{ + ProjectID: gcpProject, + Region: gcpRegion, + WIFProvider: gcpWIFProvider, } - inferenceProvider = vertex.New(vcfg, vertex.NewLiveGCPClient()) + inferenceProvider = vertex.New(vcfg) inferenceProviderName = "vertex" } else { // Preserve existing inference config if no GCP flags provided. @@ -284,8 +254,6 @@ func newInstallCmd() *cobra.Command { cmd.Flags().BoolVar(&enrollNoneFlag, "enroll-none", false, "skip repository enrollment without prompting") cmd.Flags().StringVar(&gcpProject, "gcp-project", "", "GCP project ID for Vertex AI inference") cmd.Flags().StringVar(&gcpRegion, "gcp-region", "", "GCP region for Vertex AI (e.g. global, required with --gcp-project)") - cmd.Flags().StringVar(&gcpServiceAccount, "gcp-service-account", "", "existing GCP service account name (optional, used with --gcp-project)") - cmd.Flags().StringVar(&gcpCredentialsFile, "gcp-credentials-file", "", "path to pre-made GCP service account key JSON (optional, used with --gcp-project)") cmd.Flags().StringVar(&gcpWIFProvider, "gcp-wif-provider", "", "full Workload Identity Federation provider resource name (e.g. projects/PROJECT_NUMBER/locations/global/workloadIdentityPools/POOL/providers/PROVIDER)") cmd.Flags().StringVar(&mintProvider, "mint-provider", "gcf", "token mint provider (gcf)") cmd.Flags().StringVar(&mintProject, "mint-project", "", "cloud project for token mint (e.g. GCP project ID)") @@ -902,17 +870,10 @@ func runAnalyze(ctx context.Context, client forge.Client, printer *ui.Printer, o return fmt.Errorf("getting authenticated user: %w", err) } - // Detect inference provider and auth mode from existing config. + // Detect inference provider from existing config. var inferenceProvider inference.Provider if providerName := loadExistingInferenceProvider(ctx, client, org); providerName != "" { - mode := vertex.AuthModeSAKey - wifExists, err := client.RepoSecretExists(ctx, org, forge.ConfigRepoName, vertex.SecretWIFProvider) - if err != nil { - printer.StepWarn(fmt.Sprintf("Could not check WIF secret: %v (defaulting to SA key mode)", err)) - } else if wifExists { - mode = vertex.AuthModeWIF - } - inferenceProvider = vertex.NewAnalyzeOnly(mode) + inferenceProvider = vertex.NewAnalyzeOnly() } dispatcher := gcf.NewProvisioner(gcf.Config{}, nil) @@ -1122,22 +1083,6 @@ func loadExistingEnabledRepos(ctx context.Context, client forge.Client, org stri } return cfg.EnabledRepos() } - -// validateCredentialJSON checks that raw bytes look like a GCP service account key. -func validateCredentialJSON(data []byte) error { - var keyFile struct { - Type string `json:"type"` - ProjectID string `json:"project_id"` - } - if err := json.Unmarshal(data, &keyFile); err != nil { - return fmt.Errorf("credentials file is not valid JSON: %w", err) - } - if keyFile.Type != "service_account" { - return fmt.Errorf("credentials file type is %q, expected \"service_account\"", keyFile.Type) - } - return nil -} - // loadKnownSlugs tries to read agent slugs from an existing config. func loadKnownSlugs(ctx context.Context, client forge.Client, org string) map[string]string { data, err := client.GetFileContent(ctx, org, forge.ConfigRepoName, "config.yaml") diff --git a/internal/inference/vertex/gcp.go b/internal/inference/vertex/gcp.go deleted file mode 100644 index 0716af8978..0000000000 --- a/internal/inference/vertex/gcp.go +++ /dev/null @@ -1,143 +0,0 @@ -package vertex - -import ( - "context" - "encoding/base64" - "encoding/json" - "fmt" - "io" - "net/http" - "net/url" - "regexp" - - "github.com/fullsend-ai/fullsend/internal/gcp" -) - -// gcpIDPattern validates GCP project IDs and service account names. -var gcpIDPattern = regexp.MustCompile(`^[a-z][a-z0-9-]{4,28}[a-z0-9]$`) - -// saEmailPattern validates GCP service account email addresses. -var saEmailPattern = regexp.MustCompile(`^[a-z][a-z0-9-]{4,28}[a-z0-9]@[a-z][a-z0-9-]{4,28}[a-z0-9]\.iam\.gserviceaccount\.com$`) - -// LiveGCPClient implements GCPClient using the GCP IAM REST API. -// It embeds *gcp.Client for shared ADC auth and HTTP helper logic. -type LiveGCPClient struct { - *gcp.Client -} - -// NewLiveGCPClient creates a new LiveGCPClient. -func NewLiveGCPClient() *LiveGCPClient { - return &LiveGCPClient{ - Client: gcp.NewClient(), - } -} - -// GetServiceAccount checks that a service account exists in the project. -func (c *LiveGCPClient) GetServiceAccount(ctx context.Context, projectID, saName string) error { - if !gcpIDPattern.MatchString(projectID) { - return fmt.Errorf("invalid GCP project ID %q", projectID) - } - if !gcpIDPattern.MatchString(saName) { - return fmt.Errorf("invalid service account name %q", saName) - } - - email := saName + "@" + projectID + ".iam.gserviceaccount.com" - reqURL := fmt.Sprintf("https://iam.googleapis.com/v1/projects/%s/serviceAccounts/%s", - url.PathEscape(projectID), url.PathEscape(email)) - - resp, err := c.Client.DoRequest(ctx, http.MethodGet, reqURL, "") - if err != nil { - return fmt.Errorf("checking service account: %w", err) - } - defer resp.Body.Close() - - if resp.StatusCode == http.StatusNotFound { - return fmt.Errorf("service account %s not found in project %s", saName, projectID) - } - if resp.StatusCode != http.StatusOK { - body, _ := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) - return fmt.Errorf("unexpected status %d checking service account: %s", resp.StatusCode, gcp.ExtractErrorMessage(body)) - } - return nil -} - -// CreateServiceAccount creates a new service account in the project. -func (c *LiveGCPClient) CreateServiceAccount(ctx context.Context, projectID, saName, displayName string) error { - if !gcpIDPattern.MatchString(projectID) { - return fmt.Errorf("invalid GCP project ID %q", projectID) - } - if !gcpIDPattern.MatchString(saName) { - return fmt.Errorf("invalid service account name %q", saName) - } - - reqURL := fmt.Sprintf("https://iam.googleapis.com/v1/projects/%s/serviceAccounts", - url.PathEscape(projectID)) - payloadObj := struct { - AccountID string `json:"accountId"` - ServiceAccount struct { - DisplayName string `json:"displayName"` - } `json:"serviceAccount"` - }{AccountID: saName} - payloadObj.ServiceAccount.DisplayName = displayName - payloadBytes, err := json.Marshal(payloadObj) - if err != nil { - return fmt.Errorf("marshaling request: %w", err) - } - payload := string(payloadBytes) - - resp, err := c.Client.DoRequest(ctx, http.MethodPost, reqURL, payload) - if err != nil { - return fmt.Errorf("creating service account: %w", err) - } - defer resp.Body.Close() - - if resp.StatusCode == http.StatusConflict { - // SA already exists — treat as success for idempotency. - return nil - } - if resp.StatusCode != http.StatusOK { - body, _ := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) - return fmt.Errorf("unexpected status %d creating service account: %s", resp.StatusCode, gcp.ExtractErrorMessage(body)) - } - return nil -} - -// CreateServiceAccountKey generates a new JSON key for the service account. -func (c *LiveGCPClient) CreateServiceAccountKey(ctx context.Context, projectID, saEmail string) ([]byte, error) { - if !gcpIDPattern.MatchString(projectID) { - return nil, fmt.Errorf("invalid GCP project ID %q", projectID) - } - if !saEmailPattern.MatchString(saEmail) { - return nil, fmt.Errorf("invalid service account email %q", saEmail) - } - - reqURL := fmt.Sprintf("https://iam.googleapis.com/v1/projects/%s/serviceAccounts/%s/keys", - url.PathEscape(projectID), url.PathEscape(saEmail)) - payload := `{"keyAlgorithm":"KEY_ALG_RSA_2048","privateKeyType":"TYPE_GOOGLE_CREDENTIALS_FILE"}` - - resp, err := c.Client.DoRequest(ctx, http.MethodPost, reqURL, payload) - if err != nil { - return nil, fmt.Errorf("creating service account key: %w", err) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - body, _ := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) - return nil, fmt.Errorf("unexpected status %d creating key: %s", resp.StatusCode, gcp.ExtractErrorMessage(body)) - } - - var result struct { - PrivateKeyData string `json:"privateKeyData"` - } - if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { - return nil, fmt.Errorf("decoding key response: %w", err) - } - - // privateKeyData is base64-encoded JSON credentials. - decoded, err := base64.StdEncoding.DecodeString(result.PrivateKeyData) - if err != nil { - return nil, fmt.Errorf("decoding private key data: %w", err) - } - - return decoded, nil -} diff --git a/internal/inference/vertex/vertex.go b/internal/inference/vertex/vertex.go index 3740af7746..82c462341a 100644 --- a/internal/inference/vertex/vertex.go +++ b/internal/inference/vertex/vertex.go @@ -1,9 +1,5 @@ // Package vertex implements the inference.Provider interface for Google Cloud -// Vertex AI. It supports two auth modes: -// -// - SA key (default): long-lived service account key JSON, with sub-modes -// for creating, verifying, or using a pre-made key. -// - WIF: Workload Identity Federation with GitHub OIDC — no stored keys. +// Vertex AI using Workload Identity Federation with GitHub OIDC. package vertex import ( @@ -11,86 +7,38 @@ import ( "fmt" ) -// AuthMode selects between service-account key and Workload Identity Federation. -type AuthMode string - -const ( - // AuthModeSAKey uses a long-lived service account key JSON file. - AuthModeSAKey AuthMode = "sa_key" - - // AuthModeWIF uses Workload Identity Federation with GitHub OIDC. - AuthModeWIF AuthMode = "wif" -) - const ( - // SecretCredentials is the repo secret name for the GCP service account key JSON. - // Uses the FULLSEND_ prefix to avoid confusion with the GCP SDK env var - // GOOGLE_APPLICATION_CREDENTIALS, which expects a file path, not JSON content. - SecretCredentials = "FULLSEND_GCP_SA_KEY_JSON" - // SecretProjectID is the repo secret name for the GCP project ID. - // Uses the FULLSEND_ prefix for consistency with other secrets. SecretProjectID = "FULLSEND_GCP_PROJECT_ID" // VariableRegion is the repo variable name for the GCP region. VariableRegion = "FULLSEND_GCP_REGION" - // VariableAuthMode tells workflows which GCP auth method to use. - // Must be a variable (not a secret) because GitHub's dispatch-time - // validator does not recognise the secrets context. - VariableAuthMode = "FULLSEND_GCP_AUTH_MODE" - // SecretWIFProvider is the repo secret for the full WIF provider resource name. - // Stored as a secret so the value is masked in GitHub Actions logs. SecretWIFProvider = "FULLSEND_GCP_WIF_PROVIDER" - - // defaultSAName is the service account name created in mode 1. - defaultSAName = "fullsend-agent" ) -// GCPClient abstracts GCP IAM operations for testability. -type GCPClient interface { - // GetServiceAccount checks that a service account exists. - GetServiceAccount(ctx context.Context, projectID, saName string) error - - // CreateServiceAccount creates a new service account. - CreateServiceAccount(ctx context.Context, projectID, saName, displayName string) error - - // CreateServiceAccountKey generates a new JSON key for a service account. - CreateServiceAccountKey(ctx context.Context, projectID, saEmail string) ([]byte, error) -} - // Config holds the inputs for Vertex credential provisioning. type Config struct { - ProjectID string // required - Region string // required: GCP region (e.g. global) - Mode AuthMode // "sa_key" (default) or "wif" - ServiceAccountName string // optional: existing SA name (sa_key mode 2) - CredentialJSON []byte // optional: pre-made key JSON (sa_key mode 3) - WIFProvider string // WIF mode: full provider resource name + ProjectID string // required + Region string // required: GCP region (e.g. global) + WIFProvider string // full WIF provider resource name } // Provider implements inference.Provider for Vertex AI. type Provider struct { - cfg Config - gcpAPI GCPClient + cfg Config } -// New creates a Vertex Provider with the given config and GCP client. -func New(cfg Config, gcpAPI GCPClient) *Provider { - if cfg.Mode == "" { - cfg.Mode = AuthModeSAKey - } - return &Provider{cfg: cfg, gcpAPI: gcpAPI} +// New creates a Vertex Provider with the given config. +func New(cfg Config) *Provider { + return &Provider{cfg: cfg} } // NewAnalyzeOnly creates a Provider that only supports SecretNames() and Name(). // Calling Provision() on this provider returns an error. -func NewAnalyzeOnly(mode AuthMode) *Provider { - if mode == "" { - mode = AuthModeSAKey - } - return &Provider{cfg: Config{Mode: mode}} +func NewAnalyzeOnly() *Provider { + return &Provider{} } // Name returns "vertex". @@ -100,17 +48,12 @@ func (p *Provider) Name() string { // SecretNames returns the secret names this provider manages. func (p *Provider) SecretNames() []string { - if p.cfg.Mode == AuthModeWIF { - return []string{SecretWIFProvider, SecretProjectID} - } - return []string{SecretCredentials, SecretProjectID} + return []string{SecretWIFProvider, SecretProjectID} } // Variables returns non-secret name/value pairs to store as repo variables. func (p *Provider) Variables() map[string]string { - vars := map[string]string{ - VariableAuthMode: string(p.cfg.Mode), - } + vars := map[string]string{} if p.cfg.Region != "" { vars[VariableRegion] = p.cfg.Region } @@ -122,14 +65,6 @@ func (p *Provider) Provision(ctx context.Context) (map[string]string, error) { if p.cfg.ProjectID == "" { return nil, fmt.Errorf("GCP project ID is required") } - - if p.cfg.Mode == AuthModeWIF { - return p.provisionWIF() - } - return p.provisionSAKey(ctx) -} - -func (p *Provider) provisionWIF() (map[string]string, error) { if p.cfg.WIFProvider == "" { return nil, fmt.Errorf("WIF provider resource name is required") } @@ -138,43 +73,3 @@ func (p *Provider) provisionWIF() (map[string]string, error) { SecretProjectID: p.cfg.ProjectID, }, nil } - -func (p *Provider) provisionSAKey(ctx context.Context) (map[string]string, error) { - // Mode 3: credential JSON provided directly. - if len(p.cfg.CredentialJSON) > 0 { - return map[string]string{ - SecretCredentials: string(p.cfg.CredentialJSON), - SecretProjectID: p.cfg.ProjectID, - }, nil - } - - if p.gcpAPI == nil { - return nil, fmt.Errorf("GCP client is required for provisioning") - } - - saName := p.cfg.ServiceAccountName - if saName == "" { - // Mode 1: create a new service account. - saName = defaultSAName - if err := p.gcpAPI.CreateServiceAccount(ctx, p.cfg.ProjectID, saName, "Fullsend agent inference"); err != nil { - return nil, fmt.Errorf("creating service account %s: %w", saName, err) - } - } else { - // Mode 2: verify existing service account. - if err := p.gcpAPI.GetServiceAccount(ctx, p.cfg.ProjectID, saName); err != nil { - return nil, fmt.Errorf("verifying service account %s: %w", saName, err) - } - } - - // Create key for the service account (modes 1 and 2). - saEmail := saName + "@" + p.cfg.ProjectID + ".iam.gserviceaccount.com" - keyJSON, err := p.gcpAPI.CreateServiceAccountKey(ctx, p.cfg.ProjectID, saEmail) - if err != nil { - return nil, fmt.Errorf("creating key for %s: %w", saEmail, err) - } - - return map[string]string{ - SecretCredentials: string(keyJSON), - SecretProjectID: p.cfg.ProjectID, - }, nil -} diff --git a/internal/inference/vertex/vertex_test.go b/internal/inference/vertex/vertex_test.go index 5e162100eb..e1c98ec384 100644 --- a/internal/inference/vertex/vertex_test.go +++ b/internal/inference/vertex/vertex_test.go @@ -2,283 +2,75 @@ package vertex import ( "context" - "fmt" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -// fakeGCPClient is a test double for GCPClient. -type fakeGCPClient struct { - existingSAs map[string]bool // key: "projectID/saName" - createdSAs []string // "projectID/saName" - createdKeys []string // "projectID/saEmail" - keyData []byte - getErr error - createErr error - createKeyErr error - alreadyExists bool // simulate 409 Conflict (SA already exists → success) -} - -func newFakeGCPClient() *fakeGCPClient { - return &fakeGCPClient{ - existingSAs: make(map[string]bool), - keyData: []byte(`{"type":"service_account","project_id":"test-project"}`), - } -} - -func (f *fakeGCPClient) GetServiceAccount(_ context.Context, projectID, saName string) error { - if f.getErr != nil { - return f.getErr - } - key := projectID + "/" + saName - if !f.existingSAs[key] { - return fmt.Errorf("service account %s not found in project %s", saName, projectID) - } - return nil -} - -func (f *fakeGCPClient) CreateServiceAccount(_ context.Context, projectID, saName, _ string) error { - if f.createErr != nil { - return f.createErr - } - // Simulate 409 Conflict → success (SA already exists, idempotent). - if f.alreadyExists { - return nil - } - f.createdSAs = append(f.createdSAs, projectID+"/"+saName) - f.existingSAs[projectID+"/"+saName] = true - return nil -} - -func (f *fakeGCPClient) CreateServiceAccountKey(_ context.Context, projectID, saEmail string) ([]byte, error) { - if f.createKeyErr != nil { - return nil, f.createKeyErr - } - f.createdKeys = append(f.createdKeys, projectID+"/"+saEmail) - return f.keyData, nil -} - -func TestProvision_Mode1_CreateSAAndKey(t *testing.T) { - gcp := newFakeGCPClient() - p := New(Config{ProjectID: "my-project", Region: "global"}, gcp) - - secrets, err := p.Provision(context.Background()) - require.NoError(t, err) - - // Should have created a service account. - require.Len(t, gcp.createdSAs, 1) - assert.Equal(t, "my-project/fullsend-agent", gcp.createdSAs[0]) - - // Should have created a key. - require.Len(t, gcp.createdKeys, 1) - assert.Equal(t, "my-project/fullsend-agent@my-project.iam.gserviceaccount.com", gcp.createdKeys[0]) - - // Should return both secrets. - assert.Equal(t, string(gcp.keyData), secrets[SecretCredentials]) - assert.Equal(t, "my-project", secrets[SecretProjectID]) -} - -func TestProvision_Mode2_ExistingSA(t *testing.T) { - gcp := newFakeGCPClient() - gcp.existingSAs["my-project/my-sa"] = true - p := New(Config{ProjectID: "my-project", Region: "global", ServiceAccountName: "my-sa"}, gcp) - - secrets, err := p.Provision(context.Background()) - require.NoError(t, err) - - // Should NOT have created a service account. - assert.Empty(t, gcp.createdSAs) - - // Should have created a key for the existing SA. - require.Len(t, gcp.createdKeys, 1) - assert.Equal(t, "my-project/my-sa@my-project.iam.gserviceaccount.com", gcp.createdKeys[0]) - - assert.Equal(t, string(gcp.keyData), secrets[SecretCredentials]) - assert.Equal(t, "my-project", secrets[SecretProjectID]) -} - -func TestProvision_Mode2_SANotFound(t *testing.T) { - gcp := newFakeGCPClient() - // SA does not exist. - p := New(Config{ProjectID: "my-project", Region: "global", ServiceAccountName: "missing-sa"}, gcp) - - _, err := p.Provision(context.Background()) - require.Error(t, err) - assert.Contains(t, err.Error(), "missing-sa") - assert.Contains(t, err.Error(), "not found") -} - -func TestProvision_Mode3_PreMadeKey(t *testing.T) { - gcp := newFakeGCPClient() - credJSON := []byte(`{"type":"service_account","project_id":"my-project","private_key":"..."}`) - p := New(Config{ProjectID: "my-project", Region: "global", CredentialJSON: credJSON}, gcp) - - secrets, err := p.Provision(context.Background()) - require.NoError(t, err) - - // No GCP API calls should have been made. - assert.Empty(t, gcp.createdSAs) - assert.Empty(t, gcp.createdKeys) - - assert.Equal(t, string(credJSON), secrets[SecretCredentials]) - assert.Equal(t, "my-project", secrets[SecretProjectID]) -} - -func TestProvision_Mode1_SA409Conflict(t *testing.T) { - gcp := newFakeGCPClient() - // Simulate the SA already existing (409 Conflict → treated as success). - gcp.alreadyExists = true - p := New(Config{ProjectID: "my-project", Region: "global"}, gcp) +func TestProvision_WIF(t *testing.T) { + p := New(Config{ + ProjectID: "my-project", + Region: "global", + WIFProvider: "projects/123/locations/global/workloadIdentityPools/pool/providers/gh", + }) secrets, err := p.Provision(context.Background()) require.NoError(t, err) - // SA was not re-created (409 path), but key was still generated. - assert.Empty(t, gcp.createdSAs) // no new SA recorded - assert.NotEmpty(t, gcp.createdKeys) + assert.Equal(t, "projects/123/locations/global/workloadIdentityPools/pool/providers/gh", secrets[SecretWIFProvider]) assert.Equal(t, "my-project", secrets[SecretProjectID]) - assert.Equal(t, string(gcp.keyData), secrets[SecretCredentials]) + assert.Len(t, secrets, 2) } func TestProvision_MissingProjectID(t *testing.T) { - gcp := newFakeGCPClient() - p := New(Config{}, gcp) + p := New(Config{}) _, err := p.Provision(context.Background()) require.Error(t, err) assert.Contains(t, err.Error(), "project ID") } -func TestProvision_CreateSAError(t *testing.T) { - gcp := newFakeGCPClient() - gcp.createErr = fmt.Errorf("permission denied") - p := New(Config{ProjectID: "my-project", Region: "global"}, gcp) - - _, err := p.Provision(context.Background()) - require.Error(t, err) - assert.Contains(t, err.Error(), "permission denied") -} - -func TestProvision_CreateKeyError(t *testing.T) { - gcp := newFakeGCPClient() - gcp.createKeyErr = fmt.Errorf("quota exceeded") - p := New(Config{ProjectID: "my-project", Region: "global"}, gcp) - - _, err := p.Provision(context.Background()) - require.Error(t, err) - assert.Contains(t, err.Error(), "quota exceeded") -} - -func TestProvision_NilGCPClient_Mode1(t *testing.T) { - p := New(Config{ProjectID: "my-project", Region: "global"}, nil) - - _, err := p.Provision(context.Background()) - require.Error(t, err) - assert.Contains(t, err.Error(), "GCP client is required") -} - -func TestProvision_NilGCPClient_Mode2(t *testing.T) { - p := New(Config{ProjectID: "my-project", Region: "global", ServiceAccountName: "my-sa"}, nil) +func TestProvision_MissingWIFProvider(t *testing.T) { + p := New(Config{ + ProjectID: "my-project", + Region: "global", + }) _, err := p.Provision(context.Background()) require.Error(t, err) - assert.Contains(t, err.Error(), "GCP client is required") -} - -func TestProvision_NilGCPClient_Mode3_OK(t *testing.T) { - // Mode 3 should work fine without a GCP client. - credJSON := []byte(`{"type":"service_account"}`) - p := New(Config{ProjectID: "my-project", Region: "global", CredentialJSON: credJSON}, nil) - - secrets, err := p.Provision(context.Background()) - require.NoError(t, err) - assert.Equal(t, string(credJSON), secrets[SecretCredentials]) + assert.Contains(t, err.Error(), "WIF provider resource name is required") } func TestProvision_AnalyzeOnly(t *testing.T) { - t.Run("sa_key mode", func(t *testing.T) { - p := NewAnalyzeOnly(AuthModeSAKey) - assert.Equal(t, "vertex", p.Name()) - assert.Equal(t, []string{SecretCredentials, SecretProjectID}, p.SecretNames()) - - _, err := p.Provision(context.Background()) - require.Error(t, err) - assert.Contains(t, err.Error(), "project ID is required") - }) - - t.Run("wif mode", func(t *testing.T) { - p := NewAnalyzeOnly(AuthModeWIF) - assert.Equal(t, "vertex", p.Name()) - assert.Equal(t, []string{SecretWIFProvider, SecretProjectID}, p.SecretNames()) - }) - - t.Run("empty defaults to sa_key", func(t *testing.T) { - p := NewAnalyzeOnly("") - assert.Equal(t, []string{SecretCredentials, SecretProjectID}, p.SecretNames()) - }) -} - -func TestProvision_WIF(t *testing.T) { - p := New(Config{ - ProjectID: "my-project", - Region: "global", - Mode: AuthModeWIF, - WIFProvider: "projects/123/locations/global/workloadIdentityPools/pool/providers/gh", - }, nil) - - secrets, err := p.Provision(context.Background()) - require.NoError(t, err) - - assert.Equal(t, "projects/123/locations/global/workloadIdentityPools/pool/providers/gh", secrets[SecretWIFProvider]) - assert.Equal(t, "my-project", secrets[SecretProjectID]) - assert.Len(t, secrets, 2) -} - -func TestProvision_WIF_MissingProvider(t *testing.T) { - p := New(Config{ - ProjectID: "my-project", - Region: "global", - Mode: AuthModeWIF, - }, nil) + p := NewAnalyzeOnly() + assert.Equal(t, "vertex", p.Name()) + assert.Equal(t, []string{SecretWIFProvider, SecretProjectID}, p.SecretNames()) _, err := p.Provision(context.Background()) require.Error(t, err) - assert.Contains(t, err.Error(), "WIF provider resource name is required") + assert.Contains(t, err.Error(), "project ID is required") } -func TestSecretNames_WIF(t *testing.T) { - p := New(Config{Mode: AuthModeWIF}, nil) +func TestSecretNames(t *testing.T) { + p := New(Config{}) names := p.SecretNames() assert.Equal(t, []string{SecretWIFProvider, SecretProjectID}, names) } func TestName(t *testing.T) { - p := New(Config{}, nil) + p := New(Config{}) assert.Equal(t, "vertex", p.Name()) } -func TestSecretNames(t *testing.T) { - p := New(Config{}, nil) - names := p.SecretNames() - assert.Equal(t, []string{SecretCredentials, SecretProjectID}, names) -} - func TestVariables_WithRegion(t *testing.T) { - p := New(Config{Region: "global"}, nil) + p := New(Config{Region: "global"}) vars := p.Variables() - assert.Equal(t, map[string]string{VariableAuthMode: "sa_key", VariableRegion: "global"}, vars) + assert.Equal(t, map[string]string{VariableRegion: "global"}, vars) } func TestVariables_WithoutRegion(t *testing.T) { - p := New(Config{}, nil) - vars := p.Variables() - assert.Equal(t, map[string]string{VariableAuthMode: "sa_key"}, vars) -} - -func TestVariables_WIFMode(t *testing.T) { - p := New(Config{Mode: AuthModeWIF, Region: "global"}, nil) + p := New(Config{}) vars := p.Variables() - assert.Equal(t, map[string]string{VariableAuthMode: "wif", VariableRegion: "global"}, vars) + assert.Equal(t, map[string]string{}, vars) } diff --git a/internal/layers/inference.go b/internal/layers/inference.go index 8a3d173eeb..1bda7c003b 100644 --- a/internal/layers/inference.go +++ b/internal/layers/inference.go @@ -46,7 +46,7 @@ func (l *InferenceLayer) RequiredScopes(op Operation) []string { // Install provisions inference credentials and stores them as repo secrets. // If all expected secrets already exist, provisioning is skipped to maintain -// idempotency (avoids accumulating SA keys against GCP's 10-key limit). +// idempotency (avoids unnecessary re-provisioning). func (l *InferenceLayer) Install(ctx context.Context) error { if l.provider == nil { l.ui.StepInfo("no inference provider configured, skipping") diff --git a/internal/layers/inference_test.go b/internal/layers/inference_test.go index 8ee69da904..bfc26e7a67 100644 --- a/internal/layers/inference_test.go +++ b/internal/layers/inference_test.go @@ -39,10 +39,10 @@ func newInferenceLayer(t *testing.T, client *forge.FakeClient, provider inferenc func vertexProvider() *fakeProvider { return &fakeProvider{ name: "vertex", - secretNames: []string{"FULLSEND_GCP_SA_KEY_JSON", "FULLSEND_GCP_PROJECT_ID"}, + secretNames: []string{"FULLSEND_GCP_WIF_PROVIDER", "FULLSEND_GCP_PROJECT_ID"}, secrets: map[string]string{ - "FULLSEND_GCP_SA_KEY_JSON": `{"type":"service_account"}`, - "FULLSEND_GCP_PROJECT_ID": "my-project", + "FULLSEND_GCP_WIF_PROVIDER": "projects/123/locations/global/workloadIdentityPools/pool/providers/gh", + "FULLSEND_GCP_PROJECT_ID": "my-project", }, variables: map[string]string{ "FULLSEND_GCP_REGION": "global", @@ -72,7 +72,7 @@ func TestInferenceLayer_Install_StoresSecrets(t *testing.T) { secretMap[s.Name] = s.Value } - assert.Equal(t, `{"type":"service_account"}`, secretMap["FULLSEND_GCP_SA_KEY_JSON"]) + assert.Equal(t, "projects/123/locations/global/workloadIdentityPools/pool/providers/gh", secretMap["FULLSEND_GCP_WIF_PROVIDER"]) assert.Equal(t, "my-project", secretMap["FULLSEND_GCP_PROJECT_ID"]) // Variables should also have been set. @@ -116,7 +116,7 @@ func TestInferenceLayer_Install_SecretWriteError(t *testing.T) { func TestInferenceLayer_Install_SkipsWhenSecretsExist(t *testing.T) { client := forge.NewFakeClient() - client.Secrets["test-org/.fullsend/FULLSEND_GCP_SA_KEY_JSON"] = true + client.Secrets["test-org/.fullsend/FULLSEND_GCP_WIF_PROVIDER"] = true client.Secrets["test-org/.fullsend/FULLSEND_GCP_PROJECT_ID"] = true provider := vertexProvider() layer, buf := newInferenceLayer(t, client, provider) @@ -146,7 +146,7 @@ func TestInferenceLayer_Uninstall_Noop(t *testing.T) { func TestInferenceLayer_Analyze_AllPresent(t *testing.T) { client := forge.NewFakeClient() - client.Secrets["test-org/.fullsend/FULLSEND_GCP_SA_KEY_JSON"] = true + client.Secrets["test-org/.fullsend/FULLSEND_GCP_WIF_PROVIDER"] = true client.Secrets["test-org/.fullsend/FULLSEND_GCP_PROJECT_ID"] = true client.VariablesExist["test-org/.fullsend/FULLSEND_GCP_REGION"] = true provider := vertexProvider() @@ -176,7 +176,7 @@ func TestInferenceLayer_Analyze_NonePresent(t *testing.T) { func TestInferenceLayer_Analyze_Partial(t *testing.T) { client := forge.NewFakeClient() client.Secrets["test-org/.fullsend/FULLSEND_GCP_PROJECT_ID"] = true - // FULLSEND_GCP_SA_KEY_JSON missing + // FULLSEND_GCP_WIF_PROVIDER missing provider := vertexProvider() layer, _ := newInferenceLayer(t, client, provider) diff --git a/internal/scaffold/fullsend-repo/.github/actions/setup-gcp/action.yml b/internal/scaffold/fullsend-repo/.github/actions/setup-gcp/action.yml index e20368df7b..16f5814432 100644 --- a/internal/scaffold/fullsend-repo/.github/actions/setup-gcp/action.yml +++ b/internal/scaffold/fullsend-repo/.github/actions/setup-gcp/action.yml @@ -1,18 +1,12 @@ name: Setup GCP -description: Authenticate to Google Cloud, mask credentials, and prepare sandbox credentials +description: Authenticate to Google Cloud via Workload Identity Federation, mask credentials, and prepare sandbox credentials inputs: - gcp_auth_mode: - description: 'GCP authentication mode: "wif" or "sa-key"' - required: true gcp_wif_provider: - description: 'Workload Identity Federation provider (required if gcp_auth_mode is "wif")' - required: false + description: 'Workload Identity Federation provider resource name' + required: true gcp_project_id: - description: 'GCP project ID — passed to google-github-actions/auth so the runner env has GOOGLE_CLOUD_PROJECT for direct WIF' - required: false - gcp_sa_key_json: - description: 'Service account key JSON (required if gcp_auth_mode is not "wif")' + description: 'GCP project ID — passed to google-github-actions/auth so the runner env has GOOGLE_CLOUD_PROJECT' required: false runs: @@ -23,28 +17,11 @@ runs: run: echo "::add-mask::${GITHUB_WORKSPACE}/gha-creds-" - name: Authenticate to Google Cloud (WIF) - if: inputs.gcp_auth_mode == 'wif' uses: google-github-actions/auth@v3 with: workload_identity_provider: ${{ inputs.gcp_wif_provider }} project_id: ${{ inputs.gcp_project_id }} - - name: Authenticate to Google Cloud (SA key) - if: inputs.gcp_auth_mode != 'wif' - uses: google-github-actions/auth@v3 - with: - credentials_json: ${{ inputs.gcp_sa_key_json }} - - # GCP_OIDC_TOKEN_FILE is expected by google-github-actions/auth when using - # WIF. For non-WIF (SA key), we set it to an empty file to avoid undefined - # variable errors in downstream steps that may reference it. - - name: Set GCP_OIDC_TOKEN_FILE for non-WIF - if: inputs.gcp_auth_mode != 'wif' - shell: bash - run: | - touch "$RUNNER_TEMP/empty-oidc-token" - echo "GCP_OIDC_TOKEN_FILE=$RUNNER_TEMP/empty-oidc-token" >> "${GITHUB_ENV}" - - name: Mask GCP credential file paths shell: bash run: | diff --git a/internal/scaffold/fullsend-repo/.github/workflows/code.yml b/internal/scaffold/fullsend-repo/.github/workflows/code.yml index 4d40e143b4..b919d24540 100644 --- a/internal/scaffold/fullsend-repo/.github/workflows/code.yml +++ b/internal/scaffold/fullsend-repo/.github/workflows/code.yml @@ -67,10 +67,8 @@ jobs: - name: Setup GCP and prepare credentials uses: ./.github/actions/setup-gcp with: - gcp_auth_mode: ${{ vars.FULLSEND_GCP_AUTH_MODE }} gcp_wif_provider: ${{ secrets.FULLSEND_GCP_WIF_PROVIDER }} gcp_project_id: ${{ secrets.FULLSEND_GCP_PROJECT_ID }} - gcp_sa_key_json: ${{ secrets.FULLSEND_GCP_SA_KEY_JSON }} - name: Setup agent environment env: diff --git a/internal/scaffold/fullsend-repo/.github/workflows/fix.yml b/internal/scaffold/fullsend-repo/.github/workflows/fix.yml index 920fba6e39..0c7ac32c93 100644 --- a/internal/scaffold/fullsend-repo/.github/workflows/fix.yml +++ b/internal/scaffold/fullsend-repo/.github/workflows/fix.yml @@ -254,10 +254,8 @@ jobs: - name: Setup GCP and prepare credentials uses: ./.github/actions/setup-gcp with: - gcp_auth_mode: ${{ vars.FULLSEND_GCP_AUTH_MODE }} gcp_wif_provider: ${{ secrets.FULLSEND_GCP_WIF_PROVIDER }} gcp_project_id: ${{ secrets.FULLSEND_GCP_PROJECT_ID }} - gcp_sa_key_json: ${{ secrets.FULLSEND_GCP_SA_KEY_JSON }} - name: Setup agent environment env: diff --git a/internal/scaffold/fullsend-repo/.github/workflows/prioritize.yml b/internal/scaffold/fullsend-repo/.github/workflows/prioritize.yml index be10b7eb07..2d2e9ab3d3 100644 --- a/internal/scaffold/fullsend-repo/.github/workflows/prioritize.yml +++ b/internal/scaffold/fullsend-repo/.github/workflows/prioritize.yml @@ -50,10 +50,8 @@ jobs: - name: Setup GCP and prepare credentials uses: ./.github/actions/setup-gcp with: - gcp_auth_mode: ${{ vars.FULLSEND_GCP_AUTH_MODE }} gcp_wif_provider: ${{ secrets.FULLSEND_GCP_WIF_PROVIDER }} gcp_project_id: ${{ secrets.FULLSEND_GCP_PROJECT_ID }} - gcp_sa_key_json: ${{ secrets.FULLSEND_GCP_SA_KEY_JSON }} - name: Setup agent environment env: diff --git a/internal/scaffold/fullsend-repo/.github/workflows/retro.yml b/internal/scaffold/fullsend-repo/.github/workflows/retro.yml index 71eea89abe..d744b8ee68 100644 --- a/internal/scaffold/fullsend-repo/.github/workflows/retro.yml +++ b/internal/scaffold/fullsend-repo/.github/workflows/retro.yml @@ -57,10 +57,8 @@ jobs: - name: Setup GCP and prepare credentials uses: ./.github/actions/setup-gcp with: - gcp_auth_mode: ${{ vars.FULLSEND_GCP_AUTH_MODE }} gcp_wif_provider: ${{ secrets.FULLSEND_GCP_WIF_PROVIDER }} gcp_project_id: ${{ secrets.FULLSEND_GCP_PROJECT_ID }} - gcp_sa_key_json: ${{ secrets.FULLSEND_GCP_SA_KEY_JSON }} - name: Setup agent environment env: diff --git a/internal/scaffold/fullsend-repo/.github/workflows/review.yml b/internal/scaffold/fullsend-repo/.github/workflows/review.yml index 12fd80457c..078e4279a9 100644 --- a/internal/scaffold/fullsend-repo/.github/workflows/review.yml +++ b/internal/scaffold/fullsend-repo/.github/workflows/review.yml @@ -58,10 +58,8 @@ jobs: - name: Setup GCP and prepare credentials uses: ./.github/actions/setup-gcp with: - gcp_auth_mode: ${{ vars.FULLSEND_GCP_AUTH_MODE }} gcp_wif_provider: ${{ secrets.FULLSEND_GCP_WIF_PROVIDER }} gcp_project_id: ${{ secrets.FULLSEND_GCP_PROJECT_ID }} - gcp_sa_key_json: ${{ secrets.FULLSEND_GCP_SA_KEY_JSON }} - name: Setup agent environment env: diff --git a/internal/scaffold/fullsend-repo/.github/workflows/triage.yml b/internal/scaffold/fullsend-repo/.github/workflows/triage.yml index 85450ef724..a0df81c6c3 100644 --- a/internal/scaffold/fullsend-repo/.github/workflows/triage.yml +++ b/internal/scaffold/fullsend-repo/.github/workflows/triage.yml @@ -57,10 +57,8 @@ jobs: - name: Setup GCP and prepare credentials uses: ./.github/actions/setup-gcp with: - gcp_auth_mode: ${{ vars.FULLSEND_GCP_AUTH_MODE }} gcp_wif_provider: ${{ secrets.FULLSEND_GCP_WIF_PROVIDER }} gcp_project_id: ${{ secrets.FULLSEND_GCP_PROJECT_ID }} - gcp_sa_key_json: ${{ secrets.FULLSEND_GCP_SA_KEY_JSON }} - name: Setup agent environment env: diff --git a/internal/scaffold/scaffold_test.go b/internal/scaffold/scaffold_test.go index d7cc3b819a..512f1cbcd1 100644 --- a/internal/scaffold/scaffold_test.go +++ b/internal/scaffold/scaffold_test.go @@ -369,24 +369,20 @@ func TestSetupGcpActionContent(t *testing.T) { s := string(content) // Verify inputs (composite actions cannot access vars/secrets directly) assert.Contains(t, s, "inputs:") - assert.Contains(t, s, "gcp_auth_mode:") assert.Contains(t, s, "gcp_wif_provider:") + assert.Contains(t, s, "gcp_project_id:") assert.NotContains(t, s, "gcp_wif_sa_email:") - assert.Contains(t, s, "gcp_sa_key_json:") + assert.NotContains(t, s, "gcp_auth_mode:") + assert.NotContains(t, s, "gcp_sa_key_json:") + assert.NotContains(t, s, "credentials_json:") // Verify pre-mask step assert.Contains(t, s, "Pre-mask GCP credential file path") assert.Contains(t, s, "GITHUB_WORKSPACE}/gha-creds-") - // Verify WIF authentication path - assert.Contains(t, s, "if: inputs.gcp_auth_mode == 'wif'") + // Verify WIF authentication assert.Contains(t, s, "google-github-actions/auth@v3") assert.Contains(t, s, "workload_identity_provider:") + assert.Contains(t, s, "project_id:") assert.NotContains(t, s, "service_account:") - // Verify SA key authentication path - assert.Contains(t, s, "if: inputs.gcp_auth_mode != 'wif'") - assert.Contains(t, s, "credentials_json:") - // Verify OIDC token workaround for non-WIF - assert.Contains(t, s, "RUNNER_TEMP/empty-oidc-token") - assert.Contains(t, s, "GCP_OIDC_TOKEN_FILE") // Verify credential masking assert.Contains(t, s, "Mask GCP credential file paths") assert.Contains(t, s, "::add-mask::")