From c642cc67dcc0b76a900d0b076e0a83e0297408ae Mon Sep 17 00:00:00 2001 From: Ralph Bean Date: Thu, 9 Apr 2026 17:13:13 -0400 Subject: [PATCH 1/5] Add inference provider credential provisioning (Vertex AI) Adds an InferenceLayer to the install stack that provisions and stores inference provider credentials as repo secrets on .fullsend. Supports three modes: create SA + key, verify existing SA + key, or use a pre-made key directly. Coded behind an abstract inference.Provider interface to support future providers. Closes #152 Co-Authored-By: Claude Opus 4.6 --- docs/ADRs/0006-ordered-layer-model.md | 2 +- docs/architecture.md | 2 +- .../v1/adr-0011-org-config-yaml/SPEC.md | 13 + .../config.schema.json | 11 + .../adr-0014-github-apps-and-secrets/SPEC.md | 11 +- docs/plans/vertex-inference-provisioning.md | 283 ++++++++++++++++++ e2e/admin/admin_test.go | 69 ++++- internal/cli/admin.go | 60 +++- internal/config/config.go | 35 ++- internal/config/config_test.go | 98 +++++- internal/inference/inference.go | 20 ++ internal/inference/vertex/gcp.go | 134 +++++++++ internal/inference/vertex/vertex.go | 104 +++++++ internal/inference/vertex/vertex_test.go | 162 ++++++++++ internal/layers/configrepo_test.go | 1 + internal/layers/inference.go | 127 ++++++++ internal/layers/inference_test.go | 176 +++++++++++ 17 files changed, 1275 insertions(+), 33 deletions(-) create mode 100644 docs/plans/vertex-inference-provisioning.md create mode 100644 internal/inference/inference.go create mode 100644 internal/inference/vertex/gcp.go create mode 100644 internal/inference/vertex/vertex.go create mode 100644 internal/inference/vertex/vertex_test.go create mode 100644 internal/layers/inference.go create mode 100644 internal/layers/inference_test.go diff --git a/docs/ADRs/0006-ordered-layer-model.md b/docs/ADRs/0006-ordered-layer-model.md index 2be929c695..f41f3e6470 100644 --- a/docs/ADRs/0006-ordered-layer-model.md +++ b/docs/ADRs/0006-ordered-layer-model.md @@ -25,7 +25,7 @@ Installing fullsend into an org involves multiple concerns with ordering depende Each installation concern is a `Layer` implementing `Install`, `Uninstall`, and `Analyze`. Layers are composed into an ordered `Stack`. Install runs layers forward; uninstall runs them in reverse; analyze runs them forward and collects reports. -The current stack order is: config-repo → workflows → secrets → dispatch-token → enrollment. +The current stack order is: config-repo → workflows → secrets → inference → dispatch-token → enrollment. Each layer is idempotent — re-running install skips already-completed work. Uninstall collects all errors rather than stopping on the first, so partial teardown still makes progress. Each layer declares the OAuth scopes it needs via `RequiredScopes`, enabling a preflight check that fails early when the token lacks required permissions. diff --git a/docs/architecture.md b/docs/architecture.md index 47da0e37dd..5316a4c3cb 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -38,7 +38,7 @@ Infrastructure platform choice and configuration are specified in the adopting o **Decided:** - Forge abstraction: all forge operations go through the `forge.Client` interface, keeping the rest of the codebase forge-agnostic ([ADR 0005](ADRs/0005-forge-abstraction-layer.md)). -- Installation model: ordered layer stack (install forward, uninstall reverse, analyze for status reporting) with idempotent operations ([ADR 0006](ADRs/0006-ordered-layer-model.md)). +- Installation model: ordered layer stack (install forward, uninstall reverse, analyze for status reporting) with idempotent operations. Current stack: config-repo → workflows → secrets → inference → dispatch-token → enrollment ([ADR 0006](ADRs/0006-ordered-layer-model.md)). - Cross-repo dispatch: `workflow_dispatch` with an org-level dispatch token replaces `workflow_call`, keeping App PEM secrets in the config repo ([ADR 0008](ADRs/0008-workflow-dispatch-for-cross-repo-dispatch.md)). - Shim workflow security: `pull_request_target` prevents PR authors from modifying the shim to exfiltrate the dispatch token ([ADR 0009](ADRs/0009-pull-request-target-in-shim-workflows.md)). diff --git a/docs/normative/admin-install/v1/adr-0011-org-config-yaml/SPEC.md b/docs/normative/admin-install/v1/adr-0011-org-config-yaml/SPEC.md index 0fba0d73ca..452211d096 100644 --- a/docs/normative/admin-install/v1/adr-0011-org-config-yaml/SPEC.md +++ b/docs/normative/admin-install/v1/adr-0011-org-config-yaml/SPEC.md @@ -30,6 +30,18 @@ Schema version of this document. For this SPEC, the value MUST be exactly `1`. Dispatch backend for agent work. The only defined value in v1 is `github-actions`. +### `inference` (mapping, optional) + +Configures the inference provider used by agents for LLM API access. + +#### `inference.provider` (string, required when `inference` is present) + +Inference backend for agent LLM calls. Defined values in v1: + +- `vertex` — Google Cloud Vertex AI + +When `inference` is present and `provider` is `vertex`, the install process provisions GCP service account credentials and stores them as repository secrets on `.fullsend` (see [ADR 0014](../adr-0014-github-apps-and-secrets/SPEC.md) §5). + ### `defaults` (mapping, required) Organization-wide defaults applied to repositories unless overridden elsewhere in this file. @@ -84,6 +96,7 @@ A document is **valid v1** if and only if: 3. `dispatch.platform` is `github-actions`. 4. `defaults.max_implementation_retries` ≥ 0. 5. Every entry in `defaults.roles` is one of the four defined role literals. +6. If `inference` is present, `inference.provider` is one of the defined provider literals (`vertex`). Per-repo `roles`, when present, are not checked by the v1 reference validator; tooling SHOULD still restrict values to the same role literals for consistency. diff --git a/docs/normative/admin-install/v1/adr-0011-org-config-yaml/config.schema.json b/docs/normative/admin-install/v1/adr-0011-org-config-yaml/config.schema.json index 63dafabdb5..c67407baed 100644 --- a/docs/normative/admin-install/v1/adr-0011-org-config-yaml/config.schema.json +++ b/docs/normative/admin-install/v1/adr-0011-org-config-yaml/config.schema.json @@ -22,6 +22,17 @@ } } }, + "inference": { + "type": "object", + "additionalProperties": false, + "required": ["provider"], + "properties": { + "provider": { + "type": "string", + "enum": ["vertex"] + } + } + }, "defaults": { "type": "object", "additionalProperties": false, 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 17e2020daf..3801336a60 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 @@ -47,13 +47,16 @@ For each role, the setup runner (`appsetup.Setup.Run`): All of the following are **repository-level** Actions secrets and variables on **`.fullsend`** in the target organization. -| Kind | Name pattern | Value | -|----------|----------------------------------------|-------| -| Secret | `FULLSEND__APP_PRIVATE_KEY` | PEM text of the GitHub App private key | -| Variable | `FULLSEND__APP_ID` | Decimal string of the GitHub App numeric ID | +| Kind | Name pattern | Value | Layer | +|----------|----------------------------------------|-------|-------| +| Secret | `FULLSEND__APP_PRIVATE_KEY` | PEM text of the GitHub App private key | secrets | +| Variable | `FULLSEND__APP_ID` | Decimal string of the GitHub App numeric ID | secrets | +| Secret | `GOOGLE_APPLICATION_CREDENTIALS` | GCP service account key JSON (when inference provider is `vertex`) | inference | +| Secret | `GCP_PROJECT_ID` | GCP project identifier (when inference provider is `vertex`) | 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 and variable as above; if PEM is empty (reuse path), the implementation **must** skip writing that role’s secret/variable. +- 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 both `GOOGLE_APPLICATION_CREDENTIALS` and `GCP_PROJECT_ID`. ## 6. Analyze / health semantics for the secrets layer diff --git a/docs/plans/vertex-inference-provisioning.md b/docs/plans/vertex-inference-provisioning.md new file mode 100644 index 0000000000..21aa947878 --- /dev/null +++ b/docs/plans/vertex-inference-provisioning.md @@ -0,0 +1,283 @@ +# Plan: Inference Provider Credential Provisioning (Vertex AI) + +## Problem + +Agents running in GitHub Actions need credentials to call an inference API (currently GCP Vertex AI). During `fullsend admin install`, we need to: + +1. Provision or accept GCP service account credentials +2. Upload the credential JSON as `GOOGLE_APPLICATION_CREDENTIALS` repo secret on `.fullsend` (NOT org-scoped) +3. Upload the GCP project ID as `GCP_PROJECT_ID` repo secret on `.fullsend` +4. Record the chosen inference provider in `config.yaml` + +This must be coded behind an abstract interface so we can support other inference providers (e.g., Bedrock, OpenAI) in the future. + +## Three Modes of Operation + +| Mode | Input | Actions | +|------|-------|---------| +| 1. Minimal | GCP project ID only | Create service account → create key → upload key + project ID | +| 2. Existing SA | GCP project ID + SA name | Verify SA exists → create key → upload key + project ID | +| 3. Pre-made key | GCP project ID + key JSON | Upload key + project ID | + +## Config Change + +Add `inference` section to `config.yaml`: + +```yaml +version: "1" +dispatch: + platform: github-actions +inference: + provider: vertex +defaults: + # ... +``` + +## Implementation Plan + +### Phase 1: Abstractions and Config + +#### 1a. `internal/config/config.go` — Add inference config + +- Add `InferenceConfig` struct with `Provider string` field. +- Add `InferenceConfig` field to `OrgConfig`. +- Add validation: provider must be one of `ValidProviders()` (initially just `"vertex"`). +- Update `NewOrgConfig` to accept an inference provider parameter. +- Update `config_test.go` with new validation cases. + +#### 1b. `internal/inference/inference.go` — Provider interface + +New package `internal/inference/` with: + +```go +// Provider provisions inference credentials during install. +type Provider interface { + // Name returns the provider identifier (e.g. "vertex"). + Name() string + + // Provision acquires credentials and returns secrets to store. + // Returns a map of secret-name → secret-value pairs to store + // as repo secrets on .fullsend. + Provision(ctx context.Context) (map[string]string, error) + + // Validate checks that stored credentials are functional (for Analyze). + Validate(ctx context.Context) error +} +``` + +#### 1c. `internal/inference/vertex/vertex.go` — Vertex implementation + +New package `internal/inference/vertex/` with: + +```go +type Config struct { + ProjectID string // required: GCP project ID + ServiceAccountName string // optional: existing SA name (mode 2) + CredentialJSON string // optional: pre-made key JSON (mode 3) +} + +type Vertex struct { + cfg Config + gcpAPI GCPClient // interface for testability +} +``` + +**GCPClient interface** (for mocking in unit tests): + +```go +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) +} +``` + +**LiveGCPClient**: Real implementation using GCP IAM REST API (or `google.golang.org/api/iam/v1`). + +**Provision logic:** + +``` +if cfg.CredentialJSON != "" { + // Mode 3: key provided directly + return {"GOOGLE_APPLICATION_CREDENTIALS": cfg.CredentialJSON, "GCP_PROJECT_ID": cfg.ProjectID} +} + +saName := cfg.ServiceAccountName +if saName == "" { + // Mode 1: create a new service account + saName = "fullsend-agent" + gcpAPI.CreateServiceAccount(ctx, cfg.ProjectID, saName, "Fullsend agent inference") +} else { + // Mode 2: verify existing SA + gcpAPI.GetServiceAccount(ctx, cfg.ProjectID, saName) +} + +// Create key for the SA (modes 1 and 2) +saEmail := saName + "@" + cfg.ProjectID + ".iam.gserviceaccount.com" +keyJSON := gcpAPI.CreateServiceAccountKey(ctx, cfg.ProjectID, saEmail) +return {"GOOGLE_APPLICATION_CREDENTIALS": keyJSON, "GCP_PROJECT_ID": cfg.ProjectID} +``` + +### Phase 2: Layer + +#### 2a. `internal/layers/inference.go` — InferenceLayer + +New layer that runs after SecretsLayer (position 4, before DispatchTokenLayer): + +```go +type InferenceLayer struct { + org string + client forge.Client + provider inference.Provider + ui *ui.Printer +} +``` + +- **Install**: Calls `provider.Provision(ctx)` to get secrets, then stores each as a repo secret on `.fullsend` via `client.CreateRepoSecret()`. +- **Uninstall**: No-op (secrets deleted with repo, same as SecretsLayer). +- **Analyze**: Checks that expected secrets exist (`GOOGLE_APPLICATION_CREDENTIALS`, `GCP_PROJECT_ID` for vertex). +- **RequiredScopes**: `["repo"]` for install/analyze. + +**Layer ordering** (updated): + +1. ConfigRepoLayer +2. WorkflowsLayer +3. SecretsLayer (agent app keys) +4. **InferenceLayer** (inference provider credentials) ← NEW +5. DispatchTokenLayer +6. EnrollmentLayer + +Rationale: InferenceLayer needs `.fullsend` repo to exist (created by ConfigRepoLayer) and stores repo-level secrets (like SecretsLayer). It must run before EnrollmentLayer since enrolled repos will need these secrets available. + +#### 2b. Unit tests: `internal/layers/inference_test.go` + +Test all three modes using a fake GCPClient and forge.FakeClient: +- Mode 1: no SA name, no key → creates SA, creates key, uploads secrets +- Mode 2: SA name provided → verifies SA, creates key, uploads secrets +- Mode 3: key provided → uploads secrets directly (no GCP calls) +- Error cases: SA not found, key creation failure, secret upload failure +- Analyze: all present, none present, partial + +### Phase 3: CLI Integration + +#### 3a. `internal/cli/admin.go` — New flags and wiring + +Add flags to `install` command: +- `--gcp-project` (string): GCP project ID +- `--gcp-service-account` (string): existing SA name (optional) +- `--gcp-credentials-file` (string): path to pre-made key JSON file (optional) + +The flags determine which mode is used. Validation: +- If `--gcp-credentials-file` is set, `--gcp-service-account` is ignored (mode 3). +- `--gcp-project` is required when any GCP flag is provided. + +Wire the inference provider into `buildLayerStack()` — add `inference.Provider` parameter. + +The inference provider choice (`vertex`) gets written into `config.yaml` via the `InferenceConfig` field on `OrgConfig`. + +#### 3b. Update `buildLayerStack` and all call sites + +Add the InferenceLayer to the stack between SecretsLayer and DispatchTokenLayer. Update: +- `buildLayerStack()` in `admin.go` +- `runDryRun()` — build with a no-op/analyze-only provider +- `runUninstall()` — include InferenceLayer (no-op uninstall) +- `runAnalyze()` — include InferenceLayer for status checking +- `buildTestLayerStack()` in e2e tests + +### Phase 4: Tests + +#### 4a. Unit tests for `internal/inference/vertex/` + +- `vertex_test.go`: Test Provision() for all 3 modes with a fake GCPClient. +- Test SA name defaulting, email construction, error handling. + +#### 4b. Unit tests for `internal/layers/inference_test.go` + +- Follow existing pattern from `secrets_test.go`. +- Use `forge.FakeClient` for secret storage verification. +- Use a fake inference.Provider that returns canned secrets. + +#### 4c. Unit tests for `internal/config/config_test.go` + +- Validate inference provider field parsing and validation. +- Test marshal/unmarshal round-trip with inference config. + +#### 4d. E2E tests + +**In CI**, the `E2E_HALFSEND_VERTEX_KEY` secret contains a pre-made GCP credential JSON. The e2e test will use **mode 3** (key provided directly), since: +- We can't create/delete service accounts in CI (would need org-level GCP permissions). +- A pre-created SA with a pre-made key is the safe, idempotent approach. + +**E2E changes:** + +- `e2e/admin/testutil.go`: Add `E2E_HALFSEND_VERTEX_KEY` env var loading. Also add `E2E_GCP_PROJECT_ID` (or derive from the key JSON). +- `e2e/admin/admin_test.go`: + - In `runFullInstall()`: Create a vertex provider in mode 3 using the env var, add InferenceLayer to the test stack. + - In `verifyInstalled()`: Assert `GOOGLE_APPLICATION_CREDENTIALS` and `GCP_PROJECT_ID` repo secrets exist. + - In `verifyNotInstalled()`: Assert secrets are gone (deleted with repo). + - In `buildTestLayerStack()`: Accept and include inference provider/layer. +- The e2e test should `t.Skip` the inference portion if `E2E_HALFSEND_VERTEX_KEY` is not set (so existing CI doesn't break until the secret is wired into the workflow). + +**Makefile**: Add `E2E_HALFSEND_VERTEX_KEY` to the env vars passed through to `go test` in the `e2e-test` target. + +### Phase 5: Documentation Updates + +#### 5a. `docs/ADRs/0006-ordered-layer-model.md` — Update layer stack ordering + +This is the canonical ADR defining the layer model. The Consequences section lists the current stack as `config-repo → workflows → secrets → dispatch-token → enrollment`. Update to include InferenceLayer at position 4: + +`config-repo → workflows → secrets → inference → dispatch-token → enrollment` + +#### 5b. `docs/architecture.md` — Update architecture overview + +References the layer stack ordering inline. Update the installation model description to reflect the new 6-layer stack. + +#### 5c. `docs/normative/admin-install/v1/adr-0011-org-config-yaml/` — Extend config schema + +- `SPEC.md`: Add `inference` as a new root-level key with `provider` field. Document that `provider` must be one of the valid providers (initially `vertex`). +- `config.schema.json`: Add `inference` object to the JSON schema with `provider` enum property. + +#### 5d. `docs/normative/admin-install/v1/adr-0014-github-apps-and-secrets/SPEC.md` — Extend credential surface + +Currently documents only GitHub App secrets (`FULLSEND__APP_PRIVATE_KEY`, `FULLSEND__APP_ID`). Add a new section or extend the secrets table to cover inference provider secrets stored on `.fullsend`: + +- `GOOGLE_APPLICATION_CREDENTIALS` — GCP service account key JSON (repo secret) +- `GCP_PROJECT_ID` — GCP project identifier (repo secret) + +Note: these are repo-level secrets on `.fullsend`, not org-level, consistent with the existing credential isolation pattern. + +## File Summary + +| File | Action | +|------|--------| +| `internal/config/config.go` | Modify — add InferenceConfig | +| `internal/config/config_test.go` | Modify — add inference validation tests | +| `internal/inference/inference.go` | Create — Provider interface | +| `internal/inference/vertex/vertex.go` | Create — Vertex provider + GCPClient interface | +| `internal/inference/vertex/vertex_test.go` | Create — Unit tests | +| `internal/inference/vertex/gcp.go` | Create — LiveGCPClient (real GCP API calls) | +| `internal/layers/inference.go` | Create — InferenceLayer | +| `internal/layers/inference_test.go` | Create — Unit tests | +| `internal/cli/admin.go` | Modify — flags, wiring, buildLayerStack | +| `internal/forge/fake.go` | No change needed (already supports repo secrets) | +| `e2e/admin/testutil.go` | Modify — add vertex key env var | +| `e2e/admin/admin_test.go` | Modify — add inference to install/verify | +| `Makefile` | Modify — pass E2E_HALFSEND_VERTEX_KEY | +| `docs/ADRs/0006-ordered-layer-model.md` | Modify — add InferenceLayer to stack ordering | +| `docs/architecture.md` | Modify — update layer stack reference | +| `docs/normative/.../adr-0011-.../SPEC.md` | Modify — add `inference` section | +| `docs/normative/.../adr-0011-.../config.schema.json` | Modify — add `inference` to schema | +| `docs/normative/.../adr-0014-.../SPEC.md` | Modify — add inference secrets to credential surface | + +## Open Questions + +1. **SA naming convention**: Should mode 1 use `fullsend-agent` as the default SA name, or something org-specific like `fullsend-{org}`? +2. **Key rotation**: Should we handle the case where the SA already has 10 keys (GCP limit)? Could list and delete oldest. +3. **GCP auth for SA creation**: Modes 1 and 2 need the installer's own GCP credentials (`gcloud auth` or `GOOGLE_APPLICATION_CREDENTIALS` on the local machine). Should we use Application Default Credentials, or require a separate flag? +4. **Vertex AI API enablement**: Should we check/enable the Vertex AI API on the project, or just document it as a prerequisite? +5. **Multiple inference providers**: The config supports `provider: vertex` but should we plan for `provider: bedrock` shape now, or keep the interface minimal and extend later? diff --git a/e2e/admin/admin_test.go b/e2e/admin/admin_test.go index 95c39c6bac..45ced3c359 100644 --- a/e2e/admin/admin_test.go +++ b/e2e/admin/admin_test.go @@ -4,6 +4,7 @@ package admin import ( "context" + "encoding/json" "fmt" "os" "strings" @@ -19,6 +20,8 @@ import ( "github.com/fullsend-ai/fullsend/internal/config" "github.com/fullsend-ai/fullsend/internal/forge" gh "github.com/fullsend-ai/fullsend/internal/forge/github" + "github.com/fullsend-ai/fullsend/internal/inference" + "github.com/fullsend-ai/fullsend/internal/inference/vertex" "github.com/fullsend-ai/fullsend/internal/layers" "github.com/fullsend-ai/fullsend/internal/ui" ) @@ -141,7 +144,8 @@ func TestAdminInstallUninstall(t *testing.T) { hasPrivate := hasPrivateRepos(allRepos) // Second install should reuse existing dispatch token (empty string). - stack := buildTestLayerStack(testOrg, env.client, orgCfg, env.printer, user, hasPrivate, enabledRepos, defaultBranches, agentCreds, "", enrolledRepoIDs) + // Inference provider is nil for idempotent re-install (already provisioned). + stack := buildTestLayerStack(testOrg, env.client, orgCfg, env.printer, user, hasPrivate, enabledRepos, defaultBranches, agentCreds, "", enrolledRepoIDs, nil) err = stack.InstallAll(ctx) require.NoError(t, err, "second InstallAll should succeed") verifyInstalled(t, env, orgCfg, enabledRepos, defaultBranches, agentCreds) @@ -212,7 +216,26 @@ func runFullInstall(t *testing.T, env *e2eEnv) ([]layers.AgentCredentials, *conf agents[i] = ac.AgentEntry } - orgCfg := config.NewOrgConfig(repoNames, enabledRepos, defaultRoles, agents) + // Build inference provider if vertex key is available (mode 3). + var inferenceProvider inference.Provider + var inferenceProviderName string + if vertexKey := os.Getenv("E2E_HALFSEND_VERTEX_KEY"); vertexKey != "" { + gcpProjectID := os.Getenv("E2E_GCP_PROJECT_ID") + if gcpProjectID == "" { + // Try to extract project_id from the key JSON. + gcpProjectID = extractProjectID(t, vertexKey) + } + inferenceProvider = vertex.New(vertex.Config{ + ProjectID: gcpProjectID, + CredentialJSON: vertexKey, + }, nil) + inferenceProviderName = "vertex" + t.Logf("Inference provider: vertex (project: %s)", gcpProjectID) + } else { + t.Log("E2E_HALFSEND_VERTEX_KEY not set, skipping inference layer") + } + + orgCfg := config.NewOrgConfig(repoNames, enabledRepos, defaultRoles, agents, inferenceProviderName) user, err := env.client.GetAuthenticatedUser(ctx) require.NoError(t, err, "getting authenticated user") @@ -252,7 +275,7 @@ func runFullInstall(t *testing.T, env *e2eEnv) ([]layers.AgentCredentials, *conf // Build full layer stack with the dispatch token and install all layers. // Config-repo and workflows are idempotent, so re-running them is harmless. - stack := buildTestLayerStack(testOrg, env.client, orgCfg, env.printer, user, hasPrivate, enabledRepos, defaultBranches, agentCreds, dispatchToken, enrolledRepoIDs) + stack := buildTestLayerStack(testOrg, env.client, orgCfg, env.printer, user, hasPrivate, enabledRepos, defaultBranches, agentCreds, dispatchToken, enrolledRepoIDs, inferenceProvider) err = stack.InstallAll(ctx) require.NoError(t, err, "installing layers") @@ -262,11 +285,12 @@ func runFullInstall(t *testing.T, env *e2eEnv) ([]layers.AgentCredentials, *conf func runUninstall(t *testing.T, env *e2eEnv) { t.Helper() - emptyCfg := config.NewOrgConfig(nil, nil, nil, nil) + emptyCfg := config.NewOrgConfig(nil, nil, nil, nil, "") stack := layers.NewStack( layers.NewConfigRepoLayer(testOrg, env.client, emptyCfg, env.printer, false), layers.NewWorkflowsLayer(testOrg, env.client, env.printer, ""), layers.NewSecretsLayer(testOrg, env.client, nil, env.printer), + layers.NewInferenceLayer(testOrg, env.client, nil, env.printer), layers.NewDispatchTokenLayer(testOrg, env.client, "", nil, env.printer), layers.NewEnrollmentLayer(testOrg, env.client, nil, nil, env.printer), ) @@ -278,11 +302,12 @@ func runUninstall(t *testing.T, env *e2eEnv) { // (expected when resources are already deleted). func runUninstallAllowNotFound(t *testing.T, env *e2eEnv) { t.Helper() - emptyCfg := config.NewOrgConfig(nil, nil, nil, nil) + emptyCfg := config.NewOrgConfig(nil, nil, nil, nil, "") stack := layers.NewStack( layers.NewConfigRepoLayer(testOrg, env.client, emptyCfg, env.printer, false), layers.NewWorkflowsLayer(testOrg, env.client, env.printer, ""), layers.NewSecretsLayer(testOrg, env.client, nil, env.printer), + layers.NewInferenceLayer(testOrg, env.client, nil, env.printer), layers.NewDispatchTokenLayer(testOrg, env.client, "", nil, env.printer), layers.NewEnrollmentLayer(testOrg, env.client, nil, nil, env.printer), ) @@ -337,6 +362,15 @@ 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{"GOOGLE_APPLICATION_CREDENTIALS", "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) + } + } + // Dispatch token org secret exists. dispatchExists, err := env.client.OrgSecretExists(ctx, testOrg, "FULLSEND_DISPATCH_TOKEN") assert.NoError(t, err, "checking dispatch token org secret") @@ -362,7 +396,7 @@ func verifyInstalled(t *testing.T, env *e2eEnv, orgCfg *config.OrgConfig, enable require.NoError(t, err) hasPrivate := hasPrivateRepos(allRepos) - analyzeStack := buildTestLayerStack(testOrg, env.client, orgCfg, env.printer, user, hasPrivate, enabledRepos, defaultBranches, agentCreds, "", nil) + analyzeStack := buildTestLayerStack(testOrg, env.client, orgCfg, env.printer, user, hasPrivate, enabledRepos, defaultBranches, agentCreds, "", nil, nil) reports, err := analyzeStack.AnalyzeAll(ctx) require.NoError(t, err, "analyzing layers") for _, report := range reports { @@ -394,11 +428,12 @@ func verifyNotInstalled(t *testing.T, env *e2eEnv) { assert.NoError(t, err, "checking dispatch token after uninstall") assert.False(t, dispatchExists, "FULLSEND_DISPATCH_TOKEN org secret should be deleted") - emptyCfg := config.NewOrgConfig(nil, nil, nil, nil) + emptyCfg := config.NewOrgConfig(nil, nil, nil, nil, "") stack := layers.NewStack( layers.NewConfigRepoLayer(testOrg, env.client, emptyCfg, env.printer, false), layers.NewWorkflowsLayer(testOrg, env.client, env.printer, ""), layers.NewSecretsLayer(testOrg, env.client, nil, env.printer), + layers.NewInferenceLayer(testOrg, env.client, nil, env.printer), layers.NewDispatchTokenLayer(testOrg, env.client, "", nil, env.printer), layers.NewEnrollmentLayer(testOrg, env.client, nil, nil, env.printer), ) @@ -431,11 +466,13 @@ func buildTestLayerStack( agentCreds []layers.AgentCredentials, dispatchToken string, enrolledRepoIDs []int64, + inferenceProvider inference.Provider, ) *layers.Stack { return layers.NewStack( layers.NewConfigRepoLayer(org, client, cfg, printer, hasPrivate), layers.NewWorkflowsLayer(org, client, printer, user), layers.NewSecretsLayer(org, client, agentCreds, printer), + layers.NewInferenceLayer(org, client, inferenceProvider, printer), layers.NewDispatchTokenLayer(org, client, dispatchToken, enrolledRepoIDs, printer), layers.NewEnrollmentLayer(org, client, enabledRepos, defaultBranches, printer), ) @@ -465,3 +502,21 @@ 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 36abe9acb6..f00912814d 100644 --- a/internal/cli/admin.go +++ b/internal/cli/admin.go @@ -15,6 +15,8 @@ import ( "github.com/fullsend-ai/fullsend/internal/config" "github.com/fullsend-ai/fullsend/internal/forge" gh "github.com/fullsend-ai/fullsend/internal/forge/github" + "github.com/fullsend-ai/fullsend/internal/inference" + "github.com/fullsend-ai/fullsend/internal/inference/vertex" "github.com/fullsend-ai/fullsend/internal/layers" "github.com/fullsend-ai/fullsend/internal/ui" ) @@ -79,6 +81,9 @@ func newInstallCmd() *cobra.Command { var agents string var dryRun bool var skipAppSetup bool + var gcpProject string + var gcpServiceAccount string + var gcpCredentialsFile string cmd := &cobra.Command{ Use: "install ", @@ -111,8 +116,24 @@ func newInstallCmd() *cobra.Command { roles[i] = strings.TrimSpace(roles[i]) } + // Build inference provider from GCP flags. + var inferenceProvider inference.Provider + var inferenceProviderName string + if gcpProject != "" { + vcfg := vertex.Config{ProjectID: gcpProject, ServiceAccountName: gcpServiceAccount} + if gcpCredentialsFile != "" { + credData, readErr := os.ReadFile(gcpCredentialsFile) + if readErr != nil { + return fmt.Errorf("reading credentials file: %w", readErr) + } + vcfg.CredentialJSON = string(credData) + } + inferenceProvider = vertex.New(vcfg, vertex.NewLiveGCPClient()) + inferenceProviderName = "vertex" + } + if dryRun { - return runDryRun(ctx, client, printer, org, repos, roles) + return runDryRun(ctx, client, printer, org, repos, roles, inferenceProvider, inferenceProviderName) } // Collect agent credentials via app setup. @@ -125,7 +146,7 @@ func newInstallCmd() *cobra.Command { agentCreds = creds } - return runInstall(ctx, client, printer, org, repos, roles, agentCreds) + return runInstall(ctx, client, printer, org, repos, roles, agentCreds, inferenceProvider, inferenceProviderName) }, } @@ -133,6 +154,9 @@ func newInstallCmd() *cobra.Command { cmd.Flags().StringVar(&agents, "agents", "fullsend,triage,coder,review", "comma-separated agent roles") cmd.Flags().BoolVar(&dryRun, "dry-run", false, "preview changes without making them") cmd.Flags().BoolVar(&skipAppSetup, "skip-app-setup", false, "skip GitHub App creation/setup") + cmd.Flags().StringVar(&gcpProject, "gcp-project", "", "GCP project ID for Vertex AI inference") + 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)") return cmd } @@ -220,7 +244,7 @@ func newAnalyzeCmd() *cobra.Command { } // runDryRun builds a layer stack with empty credentials and analyzes. -func runDryRun(ctx context.Context, client forge.Client, printer *ui.Printer, org string, enabledRepos, roles []string) error { +func runDryRun(ctx context.Context, client forge.Client, printer *ui.Printer, org string, enabledRepos, roles []string, inferenceProvider inference.Provider, inferenceProviderName string) error { printer.Header("Dry run - analyzing what install would do") printer.Blank() @@ -234,7 +258,7 @@ func runDryRun(ctx context.Context, client forge.Client, printer *ui.Printer, or hasPrivate := hasPrivateRepos(allRepos) // Build config with empty agents for analysis. - cfg := config.NewOrgConfig(repoNames, enabledRepos, roles, nil) + cfg := config.NewOrgConfig(repoNames, enabledRepos, roles, nil, inferenceProviderName) user, err := client.GetAuthenticatedUser(ctx) if err != nil { @@ -250,7 +274,7 @@ func runDryRun(ctx context.Context, client forge.Client, printer *ui.Printer, or } enrolledRepoIDs := collectEnrolledRepoIDs(allRepos, enabledRepos) - stack := buildLayerStack(org, client, cfg, printer, user, hasPrivate, enabledRepos, defaultBranches, agentCreds, "", enrolledRepoIDs) + stack := buildLayerStack(org, client, cfg, printer, user, hasPrivate, enabledRepos, defaultBranches, agentCreds, "", enrolledRepoIDs, inferenceProvider) if err := runPreflight(ctx, stack, layers.OpInstall, client, printer); err != nil { return err @@ -301,7 +325,7 @@ func runAppSetup(ctx context.Context, client forge.Client, printer *ui.Printer, } // runInstall performs the full installation. -func runInstall(ctx context.Context, client forge.Client, printer *ui.Printer, org string, enabledRepos, roles []string, agentCreds []layers.AgentCredentials) error { +func runInstall(ctx context.Context, client forge.Client, printer *ui.Printer, org string, enabledRepos, roles []string, agentCreds []layers.AgentCredentials, inferenceProvider inference.Provider, inferenceProviderName string) error { printer.Header("Discovering repositories") allRepos, err := client.ListOrgRepos(ctx, org) @@ -325,7 +349,7 @@ func runInstall(ctx context.Context, client forge.Client, printer *ui.Printer, o agents[i] = ac.AgentEntry } - cfg := config.NewOrgConfig(repoNames, enabledRepos, roles, agents) + cfg := config.NewOrgConfig(repoNames, enabledRepos, roles, agents, inferenceProviderName) user, err := client.GetAuthenticatedUser(ctx) if err != nil { @@ -334,7 +358,7 @@ func runInstall(ctx context.Context, client forge.Client, printer *ui.Printer, o // Build stack with empty dispatch token for preflight — we check scopes // before prompting the user so we fail early on missing admin:org. - stack := buildLayerStack(org, client, cfg, printer, user, hasPrivate, enabledRepos, defaultBranches, agentCreds, "", enrolledRepoIDs) + stack := buildLayerStack(org, client, cfg, printer, user, hasPrivate, enabledRepos, defaultBranches, agentCreds, "", enrolledRepoIDs, inferenceProvider) if err := runPreflight(ctx, stack, layers.OpInstall, client, printer); err != nil { return err @@ -367,7 +391,7 @@ func runInstall(ctx context.Context, client forge.Client, printer *ui.Printer, o } // Rebuild stack with the actual dispatch token. - stack = buildLayerStack(org, client, cfg, printer, user, hasPrivate, enabledRepos, defaultBranches, agentCreds, dispatchToken, enrolledRepoIDs) + stack = buildLayerStack(org, client, cfg, printer, user, hasPrivate, enabledRepos, defaultBranches, agentCreds, dispatchToken, enrolledRepoIDs, inferenceProvider) printer.Header("Installing layers") printer.Blank() @@ -412,11 +436,12 @@ func runUninstall(ctx context.Context, client forge.Client, printer *ui.Printer, // Build a minimal stack for uninstall. // Only ConfigRepoLayer matters for uninstall since other layers are no-ops. - emptyCfg := config.NewOrgConfig(nil, nil, nil, nil) + emptyCfg := config.NewOrgConfig(nil, nil, nil, nil, "") stack := layers.NewStack( layers.NewConfigRepoLayer(org, client, emptyCfg, printer, false), layers.NewWorkflowsLayer(org, client, printer, ""), layers.NewSecretsLayer(org, client, nil, printer), + layers.NewInferenceLayer(org, client, nil, printer), layers.NewDispatchTokenLayer(org, client, "", nil, printer), layers.NewEnrollmentLayer(org, client, nil, nil, printer), ) @@ -524,14 +549,23 @@ func runAnalyze(ctx context.Context, client forge.Client, printer *ui.Printer, o }) } - cfg := config.NewOrgConfig(repoNames, nil, defaultRoles, nil) + cfg := config.NewOrgConfig(repoNames, nil, defaultRoles, nil, "") user, err := client.GetAuthenticatedUser(ctx) if err != nil { return fmt.Errorf("getting authenticated user: %w", err) } - stack := buildLayerStack(org, client, cfg, printer, user, hasPrivate, nil, defaultBranches, agentCreds, "", nil) + // Try to detect inference provider from existing config. + var inferenceProvider inference.Provider + if cfgData, cfgErr := client.GetFileContent(ctx, org, forge.ConfigRepoName, "config.yaml"); cfgErr == nil { + if existingCfg, parseErr := config.ParseOrgConfig(cfgData); parseErr == nil && existingCfg.Inference.Provider != "" { + // For analyze, we only need SecretNames() — no GCP client needed. + inferenceProvider = vertex.New(vertex.Config{ProjectID: "analyze"}, nil) + } + } + + stack := buildLayerStack(org, client, cfg, printer, user, hasPrivate, nil, defaultBranches, agentCreds, "", nil, inferenceProvider) if err := runPreflight(ctx, stack, layers.OpAnalyze, client, printer); err != nil { return err @@ -554,11 +588,13 @@ func buildLayerStack( agentCreds []layers.AgentCredentials, dispatchToken string, enrolledRepoIDs []int64, + inferenceProvider inference.Provider, ) *layers.Stack { return layers.NewStack( layers.NewConfigRepoLayer(org, client, cfg, printer, hasPrivate), layers.NewWorkflowsLayer(org, client, printer, user), layers.NewSecretsLayer(org, client, agentCreds, printer), + layers.NewInferenceLayer(org, client, inferenceProvider, printer), layers.NewDispatchTokenLayer(org, client, dispatchToken, enrolledRepoIDs, printer), layers.NewEnrollmentLayer(org, client, enabledRepos, defaultBranches, printer), ) diff --git a/internal/config/config.go b/internal/config/config.go index d8767c1fd4..4e192fb3c6 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -21,6 +21,11 @@ type DispatchConfig struct { Platform string `yaml:"platform"` } +// InferenceConfig configures the inference provider used by agents. +type InferenceConfig struct { + Provider string `yaml:"provider"` +} + // RepoDefaults holds default settings applied to all repos. type RepoDefaults struct { Roles []string `yaml:"roles"` @@ -36,11 +41,12 @@ type RepoConfig struct { // OrgConfig is the top-level configuration for a fullsend organization. type OrgConfig struct { - Version string `yaml:"version"` - Dispatch DispatchConfig `yaml:"dispatch"` - Defaults RepoDefaults `yaml:"defaults"` - Agents []AgentEntry `yaml:"agents"` - Repos map[string]RepoConfig `yaml:"repos"` + Version string `yaml:"version"` + Dispatch DispatchConfig `yaml:"dispatch"` + Inference InferenceConfig `yaml:"inference,omitempty"` + Defaults RepoDefaults `yaml:"defaults"` + Agents []AgentEntry `yaml:"agents"` + Repos map[string]RepoConfig `yaml:"repos"` } // ValidRoles returns the set of recognized agent roles. @@ -48,6 +54,11 @@ func ValidRoles() []string { return []string{"fullsend", "triage", "coder", "review"} } +// ValidProviders returns the set of recognized inference providers. +func ValidProviders() []string { + return []string{"vertex"} +} + // DefaultAgentRoles returns the standard set of agent roles used // when no custom roles are specified. This is the same as ValidRoles // but exists as a separate function for semantic clarity. @@ -56,7 +67,7 @@ func DefaultAgentRoles() []string { } // NewOrgConfig creates a new OrgConfig with sensible defaults. -func NewOrgConfig(allRepos, enabledRepos, roles []string, agents []AgentEntry) *OrgConfig { +func NewOrgConfig(allRepos, enabledRepos, roles []string, agents []AgentEntry, inferenceProvider string) *OrgConfig { repos := make(map[string]RepoConfig, len(allRepos)) for _, r := range allRepos { repos[r] = RepoConfig{ @@ -64,7 +75,7 @@ func NewOrgConfig(allRepos, enabledRepos, roles []string, agents []AgentEntry) * } } - return &OrgConfig{ + cfg := &OrgConfig{ Version: "1", Dispatch: DispatchConfig{ Platform: "github-actions", @@ -77,6 +88,10 @@ func NewOrgConfig(allRepos, enabledRepos, roles []string, agents []AgentEntry) * Agents: agents, Repos: repos, } + if inferenceProvider != "" { + cfg.Inference = InferenceConfig{Provider: inferenceProvider} + } + return cfg } // ParseOrgConfig parses YAML bytes into an OrgConfig. @@ -120,6 +135,12 @@ func (c *OrgConfig) Validate() error { return fmt.Errorf("invalid role %q: must be one of %s", role, strings.Join(valid, ", ")) } } + if c.Inference.Provider != "" { + validProviders := ValidProviders() + if !slices.Contains(validProviders, c.Inference.Provider) { + return fmt.Errorf("invalid inference provider %q: must be one of %s", c.Inference.Provider, strings.Join(validProviders, ", ")) + } + } return nil } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 7dfebf8866..8dc907c00b 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -25,7 +25,7 @@ func TestNewOrgConfig(t *testing.T) { {Role: "fullsend", Name: "test", Slug: "test-slug"}, } - cfg := NewOrgConfig(allRepos, enabledRepos, roles, agents) + cfg := NewOrgConfig(allRepos, enabledRepos, roles, agents, "") assert.Equal(t, "1", cfg.Version) assert.Equal(t, "github-actions", cfg.Dispatch.Platform) @@ -234,3 +234,99 @@ repos: assert.True(t, cfg.Repos["repo-x"].Enabled) assert.False(t, cfg.Repos["repo-y"].Enabled) } + +func TestNewOrgConfig_WithInferenceProvider(t *testing.T) { + cfg := NewOrgConfig(nil, nil, nil, nil, "vertex") + assert.Equal(t, "vertex", cfg.Inference.Provider) +} + +func TestNewOrgConfig_WithoutInferenceProvider(t *testing.T) { + cfg := NewOrgConfig(nil, nil, nil, nil, "") + assert.Empty(t, cfg.Inference.Provider) +} + +func TestOrgConfigValidate_ValidInferenceProvider(t *testing.T) { + cfg := &OrgConfig{ + Version: "1", + Dispatch: DispatchConfig{Platform: "github-actions"}, + Inference: InferenceConfig{Provider: "vertex"}, + Defaults: RepoDefaults{ + Roles: []string{"fullsend"}, + MaxImplementationRetries: 2, + }, + } + err := cfg.Validate() + assert.NoError(t, err) +} + +func TestOrgConfigValidate_InvalidInferenceProvider(t *testing.T) { + cfg := &OrgConfig{ + Version: "1", + Dispatch: DispatchConfig{Platform: "github-actions"}, + Inference: InferenceConfig{Provider: "openai"}, + Defaults: RepoDefaults{ + Roles: []string{"fullsend"}, + MaxImplementationRetries: 2, + }, + } + err := cfg.Validate() + assert.Error(t, err) + assert.Contains(t, err.Error(), "openai") +} + +func TestOrgConfigValidate_EmptyInferenceProvider(t *testing.T) { + cfg := &OrgConfig{ + Version: "1", + Dispatch: DispatchConfig{Platform: "github-actions"}, + Defaults: RepoDefaults{ + Roles: []string{"fullsend"}, + MaxImplementationRetries: 2, + }, + } + err := cfg.Validate() + assert.NoError(t, err) +} + +func TestParseOrgConfig_WithInference(t *testing.T) { + yamlData := ` +version: "1" +dispatch: + platform: github-actions +inference: + provider: vertex +defaults: + roles: + - fullsend + max_implementation_retries: 2 + auto_merge: false +agents: [] +repos: {} +` + cfg, err := ParseOrgConfig([]byte(yamlData)) + require.NoError(t, err) + assert.Equal(t, "vertex", cfg.Inference.Provider) +} + +func TestOrgConfigMarshal_WithInference(t *testing.T) { + cfg := &OrgConfig{ + Version: "1", + Dispatch: DispatchConfig{Platform: "github-actions"}, + Inference: InferenceConfig{Provider: "vertex"}, + Defaults: RepoDefaults{ + Roles: []string{"fullsend"}, + MaxImplementationRetries: 2, + }, + Agents: []AgentEntry{}, + Repos: map[string]RepoConfig{}, + } + + data, err := cfg.Marshal() + require.NoError(t, err) + assert.Contains(t, string(data), "inference:") + assert.Contains(t, string(data), "provider: vertex") +} + +func TestValidProviders(t *testing.T) { + providers := ValidProviders() + assert.Equal(t, []string{"vertex"}, providers) +} diff --git a/internal/inference/inference.go b/internal/inference/inference.go new file mode 100644 index 0000000000..4aa82f7487 --- /dev/null +++ b/internal/inference/inference.go @@ -0,0 +1,20 @@ +// Package inference defines the interface for provisioning inference provider +// credentials during fullsend installation. +package inference + +import "context" + +// Provider provisions inference credentials during install. +type Provider interface { + // Name returns the provider identifier (e.g. "vertex"). + Name() string + + // Provision acquires credentials and returns secrets to store. + // Returns a map of secret-name → secret-value pairs to store + // as repo secrets on .fullsend. + Provision(ctx context.Context) (map[string]string, error) + + // SecretNames returns the names of secrets this provider manages, + // used by Analyze to check whether credentials are present. + SecretNames() []string +} diff --git a/internal/inference/vertex/gcp.go b/internal/inference/vertex/gcp.go new file mode 100644 index 0000000000..ddf1c39b47 --- /dev/null +++ b/internal/inference/vertex/gcp.go @@ -0,0 +1,134 @@ +package vertex + +import ( + "context" + "encoding/base64" + "encoding/json" + "fmt" + "io" + "net/http" + "os/exec" + "strings" +) + +// LiveGCPClient implements GCPClient using the GCP IAM REST API. +// It obtains access tokens via `gcloud auth print-access-token`. +type LiveGCPClient struct{} + +// NewLiveGCPClient creates a new LiveGCPClient. +func NewLiveGCPClient() *LiveGCPClient { + return &LiveGCPClient{} +} + +// accessToken obtains a GCP access token from the gcloud CLI. +func (c *LiveGCPClient) accessToken(ctx context.Context) (string, error) { + cmd := exec.CommandContext(ctx, "gcloud", "auth", "print-access-token") + out, err := cmd.Output() + if err != nil { + return "", fmt.Errorf("getting GCP access token: %w (ensure 'gcloud auth login' has been run)", err) + } + token := strings.TrimSpace(string(out)) + if token == "" { + return "", fmt.Errorf("gcloud returned empty access token") + } + return token, nil +} + +// doRequest creates and executes an authenticated HTTP request. +func (c *LiveGCPClient) doRequest(ctx context.Context, method, url, body string) (*http.Response, error) { + token, err := c.accessToken(ctx) + if err != nil { + return nil, err + } + + var bodyReader io.Reader + if body != "" { + bodyReader = strings.NewReader(body) + } + + req, err := http.NewRequestWithContext(ctx, method, url, bodyReader) + if err != nil { + return nil, fmt.Errorf("creating request: %w", err) + } + req.Header.Set("Authorization", "Bearer "+token) + if body != "" { + req.Header.Set("Content-Type", "application/json") + } + + return http.DefaultClient.Do(req) +} + +// GetServiceAccount checks that a service account exists in the project. +func (c *LiveGCPClient) GetServiceAccount(ctx context.Context, projectID, saName string) error { + email := saName + "@" + projectID + ".iam.gserviceaccount.com" + url := fmt.Sprintf("https://iam.googleapis.com/v1/projects/%s/serviceAccounts/%s", projectID, email) + + resp, err := c.doRequest(ctx, http.MethodGet, url, "") + 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(resp.Body) + return fmt.Errorf("unexpected status %d checking service account: %s", resp.StatusCode, body) + } + return nil +} + +// CreateServiceAccount creates a new service account in the project. +func (c *LiveGCPClient) CreateServiceAccount(ctx context.Context, projectID, saName, displayName string) error { + url := fmt.Sprintf("https://iam.googleapis.com/v1/projects/%s/serviceAccounts", projectID) + payload := fmt.Sprintf(`{"accountId":%q,"serviceAccount":{"displayName":%q}}`, saName, displayName) + + resp, err := c.doRequest(ctx, http.MethodPost, url, 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(resp.Body) + return fmt.Errorf("unexpected status %d creating service account: %s", resp.StatusCode, 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) { + url := fmt.Sprintf("https://iam.googleapis.com/v1/projects/%s/serviceAccounts/%s/keys", projectID, saEmail) + payload := `{"keyAlgorithm":"KEY_ALG_RSA_2048","privateKeyType":"TYPE_GOOGLE_CREDENTIALS_FILE"}` + + resp, err := c.doRequest(ctx, http.MethodPost, url, 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(resp.Body) + return nil, fmt.Errorf("unexpected status %d creating key: %s", resp.StatusCode, 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 new file mode 100644 index 0000000000..ada11a39ac --- /dev/null +++ b/internal/inference/vertex/vertex.go @@ -0,0 +1,104 @@ +// Package vertex implements the inference.Provider interface for Google Cloud +// Vertex AI. It supports three modes of credential provisioning: +// +// 1. GCP project ID only → create service account + key +// 2. GCP project ID + SA name → verify SA exists, create key +// 3. GCP project ID + credential JSON → use key directly +package vertex + +import ( + "context" + "fmt" +) + +const ( + // SecretCredentials is the repo secret name for the GCP service account key. + SecretCredentials = "GOOGLE_APPLICATION_CREDENTIALS" + + // SecretProjectID is the repo secret name for the GCP project ID. + SecretProjectID = "GCP_PROJECT_ID" + + // 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 + ServiceAccountName string // optional: existing SA name (mode 2) + CredentialJSON string // optional: pre-made key JSON (mode 3) +} + +// Provider implements inference.Provider for Vertex AI. +type Provider struct { + cfg Config + gcpAPI GCPClient +} + +// New creates a Vertex Provider with the given config and GCP client. +func New(cfg Config, gcpAPI GCPClient) *Provider { + return &Provider{cfg: cfg, gcpAPI: gcpAPI} +} + +// Name returns "vertex". +func (p *Provider) Name() string { + return "vertex" +} + +// SecretNames returns the secret names this provider manages. +func (p *Provider) SecretNames() []string { + return []string{SecretCredentials, SecretProjectID} +} + +// Provision acquires GCP credentials and returns them as secrets. +func (p *Provider) Provision(ctx context.Context) (map[string]string, error) { + if p.cfg.ProjectID == "" { + return nil, fmt.Errorf("GCP project ID is required") + } + + // Mode 3: credential JSON provided directly. + if p.cfg.CredentialJSON != "" { + return map[string]string{ + SecretCredentials: p.cfg.CredentialJSON, + SecretProjectID: p.cfg.ProjectID, + }, nil + } + + 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 new file mode 100644 index 0000000000..fd9e52f239 --- /dev/null +++ b/internal/inference/vertex/vertex_test.go @@ -0,0 +1,162 @@ +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 +} + +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 + } + 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"}, 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", 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", 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 := `{"type":"service_account","project_id":"my-project","private_key":"..."}` + p := New(Config{ProjectID: "my-project", 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, credJSON, secrets[SecretCredentials]) + assert.Equal(t, "my-project", secrets[SecretProjectID]) +} + +func TestProvision_MissingProjectID(t *testing.T) { + gcp := newFakeGCPClient() + p := New(Config{}, gcp) + + _, 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"}, 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"}, gcp) + + _, err := p.Provision(context.Background()) + require.Error(t, err) + assert.Contains(t, err.Error(), "quota exceeded") +} + +func TestName(t *testing.T) { + p := New(Config{}, nil) + 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) +} diff --git a/internal/layers/configrepo_test.go b/internal/layers/configrepo_test.go index 2d4fa65151..f7b1fed533 100644 --- a/internal/layers/configrepo_test.go +++ b/internal/layers/configrepo_test.go @@ -21,6 +21,7 @@ func newTestConfig(t *testing.T) *config.OrgConfig { []string{"repo-a"}, []string{"coder"}, []config.AgentEntry{{Role: "coder", Name: "Bot", Slug: "bot-slug"}}, + "", ) } diff --git a/internal/layers/inference.go b/internal/layers/inference.go new file mode 100644 index 0000000000..511c6da504 --- /dev/null +++ b/internal/layers/inference.go @@ -0,0 +1,127 @@ +package layers + +import ( + "context" + "fmt" + + "github.com/fullsend-ai/fullsend/internal/forge" + "github.com/fullsend-ai/fullsend/internal/inference" + "github.com/fullsend-ai/fullsend/internal/ui" +) + +// InferenceLayer manages inference provider credentials in the .fullsend repo. +type InferenceLayer struct { + org string + client forge.Client + provider inference.Provider + ui *ui.Printer +} + +var _ Layer = (*InferenceLayer)(nil) + +// NewInferenceLayer creates a new InferenceLayer. +func NewInferenceLayer(org string, client forge.Client, provider inference.Provider, printer *ui.Printer) *InferenceLayer { + return &InferenceLayer{ + org: org, + client: client, + provider: provider, + ui: printer, + } +} + +// Name returns the layer name. +func (l *InferenceLayer) Name() string { + return "inference" +} + +// RequiredScopes returns the scopes needed for the given operation. +func (l *InferenceLayer) RequiredScopes(op Operation) []string { + switch op { + case OpInstall, OpAnalyze: + return []string{"repo"} + default: + return nil + } +} + +// Install provisions inference credentials and stores them as repo secrets. +func (l *InferenceLayer) Install(ctx context.Context) error { + if l.provider == nil { + l.ui.StepInfo("no inference provider configured, skipping") + return nil + } + + l.ui.StepStart(fmt.Sprintf("provisioning %s credentials", l.provider.Name())) + + secrets, err := l.provider.Provision(ctx) + if err != nil { + l.ui.StepFail(fmt.Sprintf("failed to provision %s credentials", l.provider.Name())) + return fmt.Errorf("provisioning %s: %w", l.provider.Name(), err) + } + + for name, value := range secrets { + l.ui.StepStart(fmt.Sprintf("storing %s", name)) + if err := l.client.CreateRepoSecret(ctx, l.org, forge.ConfigRepoName, name, value); err != nil { + l.ui.StepFail(fmt.Sprintf("failed to store %s", name)) + return fmt.Errorf("creating secret %s: %w", name, err) + } + l.ui.StepDone(fmt.Sprintf("stored %s", name)) + } + + l.ui.StepDone(fmt.Sprintf("%s credentials provisioned", l.provider.Name())) + return nil +} + +// Uninstall is a no-op. Secrets are removed when the .fullsend repo is deleted. +func (l *InferenceLayer) Uninstall(_ context.Context) error { + return nil +} + +// Analyze checks whether inference credentials exist in the .fullsend repo. +func (l *InferenceLayer) Analyze(ctx context.Context) (*LayerReport, error) { + report := &LayerReport{Name: l.Name()} + + if l.provider == nil { + report.Status = StatusInstalled + report.Details = append(report.Details, "no inference provider configured") + return report, nil + } + + secretNames := l.provider.SecretNames() + var present, missing []string + + for _, name := range secretNames { + exists, err := l.client.RepoSecretExists(ctx, l.org, forge.ConfigRepoName, name) + if err != nil { + return nil, fmt.Errorf("checking secret %s: %w", name, err) + } + if exists { + present = append(present, name) + } else { + missing = append(missing, name) + } + } + + switch { + case len(missing) == 0: + report.Status = StatusInstalled + for _, name := range present { + report.Details = append(report.Details, fmt.Sprintf("%s exists", name)) + } + case len(present) == 0: + report.Status = StatusNotInstalled + for _, name := range missing { + report.WouldInstall = append(report.WouldInstall, fmt.Sprintf("create %s", name)) + } + default: + report.Status = StatusDegraded + for _, name := range present { + report.Details = append(report.Details, fmt.Sprintf("%s exists", name)) + } + for _, name := range missing { + report.WouldFix = append(report.WouldFix, fmt.Sprintf("create missing %s", name)) + } + } + + return report, nil +} diff --git a/internal/layers/inference_test.go b/internal/layers/inference_test.go new file mode 100644 index 0000000000..b6ba5b27fc --- /dev/null +++ b/internal/layers/inference_test.go @@ -0,0 +1,176 @@ +package layers + +import ( + "bytes" + "context" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/fullsend-ai/fullsend/internal/forge" + "github.com/fullsend-ai/fullsend/internal/inference" + "github.com/fullsend-ai/fullsend/internal/ui" +) + +// fakeProvider is a test double for inference.Provider. +type fakeProvider struct { + name string + secretNames []string + secrets map[string]string + err error +} + +func (f *fakeProvider) Name() string { return f.name } +func (f *fakeProvider) SecretNames() []string { return f.secretNames } +func (f *fakeProvider) Provision(_ context.Context) (map[string]string, error) { return f.secrets, f.err } + +func newInferenceLayer(t *testing.T, client *forge.FakeClient, provider inference.Provider) (*InferenceLayer, *bytes.Buffer) { + t.Helper() + var buf bytes.Buffer + printer := ui.New(&buf) + layer := NewInferenceLayer("test-org", client, provider, printer) + return layer, &buf +} + +func vertexProvider() *fakeProvider { + return &fakeProvider{ + name: "vertex", + secretNames: []string{"GOOGLE_APPLICATION_CREDENTIALS", "GCP_PROJECT_ID"}, + secrets: map[string]string{ + "GOOGLE_APPLICATION_CREDENTIALS": `{"type":"service_account"}`, + "GCP_PROJECT_ID": "my-project", + }, + } +} + +func TestInferenceLayer_Name(t *testing.T) { + layer, _ := newInferenceLayer(t, &forge.FakeClient{}, nil) + assert.Equal(t, "inference", layer.Name()) +} + +func TestInferenceLayer_Install_StoresSecrets(t *testing.T) { + client := forge.NewFakeClient() + provider := vertexProvider() + layer, _ := newInferenceLayer(t, client, provider) + + err := layer.Install(context.Background()) + require.NoError(t, err) + + require.Len(t, client.CreatedSecrets, 2) + + secretMap := make(map[string]string) + for _, s := range client.CreatedSecrets { + assert.Equal(t, "test-org", s.Owner) + assert.Equal(t, ".fullsend", s.Repo) + secretMap[s.Name] = s.Value + } + + assert.Equal(t, `{"type":"service_account"}`, secretMap["GOOGLE_APPLICATION_CREDENTIALS"]) + assert.Equal(t, "my-project", secretMap["GCP_PROJECT_ID"]) +} + +func TestInferenceLayer_Install_NilProvider(t *testing.T) { + client := forge.NewFakeClient() + layer, _ := newInferenceLayer(t, client, nil) + + err := layer.Install(context.Background()) + require.NoError(t, err) + + assert.Empty(t, client.CreatedSecrets) +} + +func TestInferenceLayer_Install_ProvisionError(t *testing.T) { + client := forge.NewFakeClient() + provider := vertexProvider() + provider.err = errors.New("gcp auth failed") + provider.secrets = nil + layer, _ := newInferenceLayer(t, client, provider) + + err := layer.Install(context.Background()) + require.Error(t, err) + assert.Contains(t, err.Error(), "gcp auth failed") +} + +func TestInferenceLayer_Install_SecretWriteError(t *testing.T) { + client := forge.NewFakeClient() + client.Errors["CreateRepoSecret"] = errors.New("permission denied") + provider := vertexProvider() + layer, _ := newInferenceLayer(t, client, provider) + + err := layer.Install(context.Background()) + require.Error(t, err) + assert.Contains(t, err.Error(), "permission denied") +} + +func TestInferenceLayer_Uninstall_Noop(t *testing.T) { + client := forge.NewFakeClient() + provider := vertexProvider() + layer, _ := newInferenceLayer(t, client, provider) + + err := layer.Uninstall(context.Background()) + require.NoError(t, err) + assert.Empty(t, client.CreatedSecrets) +} + +func TestInferenceLayer_Analyze_AllPresent(t *testing.T) { + client := forge.NewFakeClient() + client.Secrets["test-org/.fullsend/GOOGLE_APPLICATION_CREDENTIALS"] = true + client.Secrets["test-org/.fullsend/GCP_PROJECT_ID"] = true + provider := vertexProvider() + layer, _ := newInferenceLayer(t, client, provider) + + report, err := layer.Analyze(context.Background()) + require.NoError(t, err) + + assert.Equal(t, "inference", report.Name) + assert.Equal(t, StatusInstalled, report.Status) + assert.Len(t, report.Details, 2) + assert.Empty(t, report.WouldInstall) +} + +func TestInferenceLayer_Analyze_NonePresent(t *testing.T) { + client := forge.NewFakeClient() + provider := vertexProvider() + layer, _ := newInferenceLayer(t, client, provider) + + report, err := layer.Analyze(context.Background()) + require.NoError(t, err) + + assert.Equal(t, StatusNotInstalled, report.Status) + assert.Len(t, report.WouldInstall, 2) +} + +func TestInferenceLayer_Analyze_Partial(t *testing.T) { + client := forge.NewFakeClient() + client.Secrets["test-org/.fullsend/GCP_PROJECT_ID"] = true + // GOOGLE_APPLICATION_CREDENTIALS missing + provider := vertexProvider() + layer, _ := newInferenceLayer(t, client, provider) + + report, err := layer.Analyze(context.Background()) + require.NoError(t, err) + + assert.Equal(t, StatusDegraded, report.Status) + assert.NotEmpty(t, report.Details) + assert.NotEmpty(t, report.WouldFix) +} + +func TestInferenceLayer_Analyze_NilProvider(t *testing.T) { + client := forge.NewFakeClient() + layer, _ := newInferenceLayer(t, client, nil) + + report, err := layer.Analyze(context.Background()) + require.NoError(t, err) + + assert.Equal(t, StatusInstalled, report.Status) + assert.Contains(t, report.Details[0], "no inference provider configured") +} + +func TestInferenceLayer_RequiredScopes(t *testing.T) { + layer, _ := newInferenceLayer(t, &forge.FakeClient{}, nil) + assert.Equal(t, []string{"repo"}, layer.RequiredScopes(OpInstall)) + assert.Equal(t, []string{"repo"}, layer.RequiredScopes(OpAnalyze)) + assert.Nil(t, layer.RequiredScopes(OpUninstall)) +} From e72f07c4aabeada65e7e149abb9c1a3c5448637a Mon Sep 17 00:00:00 2001 From: Ralph Bean Date: Sun, 12 Apr 2026 10:26:16 -0400 Subject: [PATCH 2/5] Address review feedback on inference provisioning - Replace gcloud subprocess with Go SDK (golang.org/x/oauth2/google) to eliminate PATH-hijack risk - Add dedicated HTTP client with 30s timeout instead of http.DefaultClient - Escape user-supplied URL path parameters and validate GCP naming rules - Extract only error.message from GCP API responses to prevent metadata leakage - Rename secret from GOOGLE_APPLICATION_CREDENTIALS to FULLSEND_GCP_SA_KEY_JSON to avoid collision with the GCP SDK env var (which expects a file path) - Add idempotency check: skip provisioning if secrets already exist, preventing SA key accumulation against GCP's 10-key limit - Error if --gcp-service-account or --gcp-credentials-file set without --gcp-project instead of silently ignoring them - Preserve existing inference config on re-install when GCP flags omitted Co-Authored-By: Claude Opus 4.6 --- .../adr-0014-github-apps-and-secrets/SPEC.md | 4 +- docs/plans/vertex-inference-provisioning.md | 14 +-- e2e/admin/admin_test.go | 2 +- go.mod | 2 + go.sum | 4 + internal/cli/admin.go | 23 +++++ internal/inference/vertex/gcp.go | 96 ++++++++++++++----- internal/inference/vertex/vertex.go | 6 +- internal/layers/inference.go | 21 ++++ internal/layers/inference_test.go | 26 ++++- 10 files changed, 157 insertions(+), 41 deletions(-) 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 3801336a60..0f3a4430a2 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,12 +51,12 @@ 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__APP_ID` | Decimal string of the GitHub App numeric ID | secrets | -| Secret | `GOOGLE_APPLICATION_CREDENTIALS` | GCP service account key JSON (when inference provider is `vertex`) | inference | +| Secret | `FULLSEND_GCP_SA_KEY_JSON` | GCP service account key JSON (when inference provider is `vertex`) | inference | | Secret | `GCP_PROJECT_ID` | GCP project identifier (when inference provider is `vertex`) | 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 and variable as above; if PEM is empty (reuse path), the implementation **must** skip writing that role’s secret/variable. -- 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 both `GOOGLE_APPLICATION_CREDENTIALS` and `GCP_PROJECT_ID`. +- 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 both `FULLSEND_GCP_SA_KEY_JSON` and `GCP_PROJECT_ID`. ## 6. Analyze / health semantics for the secrets layer diff --git a/docs/plans/vertex-inference-provisioning.md b/docs/plans/vertex-inference-provisioning.md index 21aa947878..1dda5463b8 100644 --- a/docs/plans/vertex-inference-provisioning.md +++ b/docs/plans/vertex-inference-provisioning.md @@ -5,7 +5,7 @@ Agents running in GitHub Actions need credentials to call an inference API (currently GCP Vertex AI). During `fullsend admin install`, we need to: 1. Provision or accept GCP service account credentials -2. Upload the credential JSON as `GOOGLE_APPLICATION_CREDENTIALS` repo secret on `.fullsend` (NOT org-scoped) +2. Upload the credential JSON as `FULLSEND_GCP_SA_KEY_JSON` repo secret on `.fullsend` (NOT org-scoped) 3. Upload the GCP project ID as `GCP_PROJECT_ID` repo secret on `.fullsend` 4. Record the chosen inference provider in `config.yaml` @@ -104,7 +104,7 @@ type GCPClient interface { ``` if cfg.CredentialJSON != "" { // Mode 3: key provided directly - return {"GOOGLE_APPLICATION_CREDENTIALS": cfg.CredentialJSON, "GCP_PROJECT_ID": cfg.ProjectID} + return {"FULLSEND_GCP_SA_KEY_JSON": cfg.CredentialJSON, "GCP_PROJECT_ID": cfg.ProjectID} } saName := cfg.ServiceAccountName @@ -120,7 +120,7 @@ if saName == "" { // Create key for the SA (modes 1 and 2) saEmail := saName + "@" + cfg.ProjectID + ".iam.gserviceaccount.com" keyJSON := gcpAPI.CreateServiceAccountKey(ctx, cfg.ProjectID, saEmail) -return {"GOOGLE_APPLICATION_CREDENTIALS": keyJSON, "GCP_PROJECT_ID": cfg.ProjectID} +return {"FULLSEND_GCP_SA_KEY_JSON": keyJSON, "GCP_PROJECT_ID": cfg.ProjectID} ``` ### Phase 2: Layer @@ -140,7 +140,7 @@ type InferenceLayer struct { - **Install**: Calls `provider.Provision(ctx)` to get secrets, then stores each as a repo secret on `.fullsend` via `client.CreateRepoSecret()`. - **Uninstall**: No-op (secrets deleted with repo, same as SecretsLayer). -- **Analyze**: Checks that expected secrets exist (`GOOGLE_APPLICATION_CREDENTIALS`, `GCP_PROJECT_ID` for vertex). +- **Analyze**: Checks that expected secrets exist (`FULLSEND_GCP_SA_KEY_JSON`, `GCP_PROJECT_ID` for vertex). - **RequiredScopes**: `["repo"]` for install/analyze. **Layer ordering** (updated): @@ -218,7 +218,7 @@ Add the InferenceLayer to the stack between SecretsLayer and DispatchTokenLayer. - `e2e/admin/testutil.go`: Add `E2E_HALFSEND_VERTEX_KEY` env var loading. Also add `E2E_GCP_PROJECT_ID` (or derive from the key JSON). - `e2e/admin/admin_test.go`: - In `runFullInstall()`: Create a vertex provider in mode 3 using the env var, add InferenceLayer to the test stack. - - In `verifyInstalled()`: Assert `GOOGLE_APPLICATION_CREDENTIALS` and `GCP_PROJECT_ID` repo secrets exist. + - In `verifyInstalled()`: Assert `FULLSEND_GCP_SA_KEY_JSON` and `GCP_PROJECT_ID` repo secrets exist. - In `verifyNotInstalled()`: Assert secrets are gone (deleted with repo). - In `buildTestLayerStack()`: Accept and include inference provider/layer. - The e2e test should `t.Skip` the inference portion if `E2E_HALFSEND_VERTEX_KEY` is not set (so existing CI doesn't break until the secret is wired into the workflow). @@ -246,7 +246,7 @@ References the layer stack ordering inline. Update the installation model descri Currently documents only GitHub App secrets (`FULLSEND__APP_PRIVATE_KEY`, `FULLSEND__APP_ID`). Add a new section or extend the secrets table to cover inference provider secrets stored on `.fullsend`: -- `GOOGLE_APPLICATION_CREDENTIALS` — GCP service account key JSON (repo secret) +- `FULLSEND_GCP_SA_KEY_JSON` — GCP service account key JSON (repo secret) - `GCP_PROJECT_ID` — GCP project identifier (repo secret) Note: these are repo-level secrets on `.fullsend`, not org-level, consistent with the existing credential isolation pattern. @@ -278,6 +278,6 @@ Note: these are repo-level secrets on `.fullsend`, not org-level, consistent wit 1. **SA naming convention**: Should mode 1 use `fullsend-agent` as the default SA name, or something org-specific like `fullsend-{org}`? 2. **Key rotation**: Should we handle the case where the SA already has 10 keys (GCP limit)? Could list and delete oldest. -3. **GCP auth for SA creation**: Modes 1 and 2 need the installer's own GCP credentials (`gcloud auth` or `GOOGLE_APPLICATION_CREDENTIALS` on the local machine). Should we use Application Default Credentials, or require a separate flag? +3. **GCP auth for SA creation**: ~~Should we use Application Default Credentials, or require a separate flag?~~ **Decided:** Use Application Default Credentials via `golang.org/x/oauth2/google.FindDefaultCredentials()`. This eliminates the PATH-hijack risk of shelling out to `gcloud` and is the standard Go approach. 4. **Vertex AI API enablement**: Should we check/enable the Vertex AI API on the project, or just document it as a prerequisite? 5. **Multiple inference providers**: The config supports `provider: vertex` but should we plan for `provider: bedrock` shape now, or keep the interface minimal and extend later? diff --git a/e2e/admin/admin_test.go b/e2e/admin/admin_test.go index 45ced3c359..f95e66a19a 100644 --- a/e2e/admin/admin_test.go +++ b/e2e/admin/admin_test.go @@ -364,7 +364,7 @@ func verifyInstalled(t *testing.T, env *e2eEnv, orgCfg *config.OrgConfig, enable // Inference secrets exist if vertex key was provided. if os.Getenv("E2E_HALFSEND_VERTEX_KEY") != "" { - for _, secretName := range []string{"GOOGLE_APPLICATION_CREDENTIALS", "GCP_PROJECT_ID"} { + for _, secretName := range []string{"FULLSEND_GCP_SA_KEY_JSON", "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) diff --git a/go.mod b/go.mod index e90bd5ef20..7fa8bd94e9 100644 --- a/go.mod +++ b/go.mod @@ -14,6 +14,7 @@ require ( ) require ( + cloud.google.com/go/compute/metadata v0.3.0 // indirect github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect github.com/charmbracelet/colorprofile v0.2.3-0.20250311203215-f60798e515dc // indirect github.com/charmbracelet/x/ansi v0.8.0 // indirect @@ -33,5 +34,6 @@ require ( github.com/rivo/uniseg v0.4.7 // indirect github.com/spf13/pflag v1.0.9 // indirect github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect + golang.org/x/oauth2 v0.36.0 // indirect golang.org/x/sys v0.42.0 // indirect ) diff --git a/go.sum b/go.sum index 85b5a618a1..23a5ad0942 100644 --- a/go.sum +++ b/go.sum @@ -1,3 +1,5 @@ +cloud.google.com/go/compute/metadata v0.3.0 h1:Tz+eQXMEqDIKRsmY3cHTL6FVaynIjX2QxYC4trgAKZc= +cloud.google.com/go/compute/metadata v0.3.0/go.mod h1:zFmK7XCadkQkj6TtorcaGlCW1hT1fIilQDwofLpJ20k= github.com/aymanbagabas/go-osc52/v2 v2.0.1 h1:HwpRHbFMcZLEVr42D4p7XBqjyuxQH5SMiErDT4WkJ2k= github.com/aymanbagabas/go-osc52/v2 v2.0.1/go.mod h1:uYgXzlJ7ZpABp8OJ+exZzJJhRNQ2ASbcXHWsFqH8hp8= github.com/charmbracelet/colorprofile v0.2.3-0.20250311203215-f60798e515dc h1:4pZI35227imm7yK2bGPcfpFEmuY1gc2YSTShr4iJBfs= @@ -76,6 +78,8 @@ golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg= golang.org/x/net v0.51.0 h1:94R/GTO7mt3/4wIKpcR5gkGmRLOuE/2hNGeWq/GBIFo= golang.org/x/net v0.51.0/go.mod h1:aamm+2QF5ogm02fjy5Bb7CQ0WMt1/WVM7FtyaTLlA9Y= +golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs= +golang.org/x/oauth2 v0.36.0/go.mod h1:YDBUJMTkDnJS+A4BP4eZBjCqtokkg1hODuPjwiGPO7Q= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= diff --git a/internal/cli/admin.go b/internal/cli/admin.go index f00912814d..4d53a4459c 100644 --- a/internal/cli/admin.go +++ b/internal/cli/admin.go @@ -116,6 +116,11 @@ func newInstallCmd() *cobra.Command { roles[i] = strings.TrimSpace(roles[i]) } + // Validate GCP flag dependencies. + if gcpProject == "" && (gcpServiceAccount != "" || gcpCredentialsFile != "") { + return fmt.Errorf("--gcp-service-account and --gcp-credentials-file require --gcp-project to be set") + } + // Build inference provider from GCP flags. var inferenceProvider inference.Provider var inferenceProviderName string @@ -130,6 +135,9 @@ func newInstallCmd() *cobra.Command { } inferenceProvider = vertex.New(vcfg, vertex.NewLiveGCPClient()) inferenceProviderName = "vertex" + } else { + // Preserve existing inference config if no GCP flags provided. + inferenceProviderName = loadExistingInferenceProvider(ctx, client, org) } if dryRun { @@ -677,6 +685,21 @@ func printAnalysis(ctx context.Context, stack *layers.Stack, printer *ui.Printer return nil } +// loadExistingInferenceProvider reads the inference provider name from +// an existing config.yaml in .fullsend, if available. This prevents +// re-installs without --gcp-project from silently erasing the inference section. +func loadExistingInferenceProvider(ctx context.Context, client forge.Client, org string) string { + data, err := client.GetFileContent(ctx, org, forge.ConfigRepoName, "config.yaml") + if err != nil { + return "" + } + cfg, err := config.ParseOrgConfig(data) + if err != nil { + return "" + } + return cfg.Inference.Provider +} + // 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 index ddf1c39b47..8f677f43b4 100644 --- a/internal/inference/vertex/gcp.go +++ b/internal/inference/vertex/gcp.go @@ -7,35 +7,48 @@ import ( "fmt" "io" "net/http" - "os/exec" + "net/url" + "regexp" "strings" + "time" + + "golang.org/x/oauth2/google" ) +// gcpIDPattern validates GCP project IDs and service account names. +var gcpIDPattern = regexp.MustCompile(`^[a-z][a-z0-9-]{4,28}[a-z0-9]$`) + // LiveGCPClient implements GCPClient using the GCP IAM REST API. -// It obtains access tokens via `gcloud auth print-access-token`. -type LiveGCPClient struct{} +// It obtains access tokens via Application Default Credentials. +type LiveGCPClient struct { + httpClient *http.Client +} // NewLiveGCPClient creates a new LiveGCPClient. func NewLiveGCPClient() *LiveGCPClient { - return &LiveGCPClient{} + return &LiveGCPClient{ + httpClient: &http.Client{Timeout: 30 * time.Second}, + } } -// accessToken obtains a GCP access token from the gcloud CLI. +// accessToken obtains a GCP access token using Application Default Credentials. func (c *LiveGCPClient) accessToken(ctx context.Context) (string, error) { - cmd := exec.CommandContext(ctx, "gcloud", "auth", "print-access-token") - out, err := cmd.Output() + creds, err := google.FindDefaultCredentials(ctx, "https://www.googleapis.com/auth/cloud-platform") if err != nil { - return "", fmt.Errorf("getting GCP access token: %w (ensure 'gcloud auth login' has been run)", err) + return "", fmt.Errorf("finding GCP credentials: %w (ensure 'gcloud auth application-default login' has been run or GOOGLE_APPLICATION_CREDENTIALS is set)", err) } - token := strings.TrimSpace(string(out)) - if token == "" { - return "", fmt.Errorf("gcloud returned empty access token") + tok, err := creds.TokenSource.Token() + if err != nil { + return "", fmt.Errorf("obtaining GCP access token: %w", err) } - return token, nil + if tok.AccessToken == "" { + return "", fmt.Errorf("GCP credentials returned empty access token") + } + return tok.AccessToken, nil } // doRequest creates and executes an authenticated HTTP request. -func (c *LiveGCPClient) doRequest(ctx context.Context, method, url, body string) (*http.Response, error) { +func (c *LiveGCPClient) doRequest(ctx context.Context, method, reqURL, body string) (*http.Response, error) { token, err := c.accessToken(ctx) if err != nil { return nil, err @@ -46,7 +59,7 @@ func (c *LiveGCPClient) doRequest(ctx context.Context, method, url, body string) bodyReader = strings.NewReader(body) } - req, err := http.NewRequestWithContext(ctx, method, url, bodyReader) + req, err := http.NewRequestWithContext(ctx, method, reqURL, bodyReader) if err != nil { return nil, fmt.Errorf("creating request: %w", err) } @@ -55,15 +68,37 @@ func (c *LiveGCPClient) doRequest(ctx context.Context, method, url, body string) req.Header.Set("Content-Type", "application/json") } - return http.DefaultClient.Do(req) + return c.httpClient.Do(req) +} + +// extractGCPErrorMessage parses a GCP API error response and returns only +// the error message, avoiding leakage of sensitive metadata. +func extractGCPErrorMessage(body []byte) string { + var gcpErr struct { + Error struct { + Message string `json:"message"` + } `json:"error"` + } + if json.Unmarshal(body, &gcpErr) == nil && gcpErr.Error.Message != "" { + return gcpErr.Error.Message + } + return "(error details unavailable)" } // 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" - url := fmt.Sprintf("https://iam.googleapis.com/v1/projects/%s/serviceAccounts/%s", projectID, email) + reqURL := fmt.Sprintf("https://iam.googleapis.com/v1/projects/%s/serviceAccounts/%s", + url.PathEscape(projectID), url.PathEscape(email)) - resp, err := c.doRequest(ctx, http.MethodGet, url, "") + resp, err := c.doRequest(ctx, http.MethodGet, reqURL, "") if err != nil { return fmt.Errorf("checking service account: %w", err) } @@ -74,17 +109,25 @@ func (c *LiveGCPClient) GetServiceAccount(ctx context.Context, projectID, saName } if resp.StatusCode != http.StatusOK { body, _ := io.ReadAll(resp.Body) - return fmt.Errorf("unexpected status %d checking service account: %s", resp.StatusCode, body) + return fmt.Errorf("unexpected status %d checking service account: %s", resp.StatusCode, extractGCPErrorMessage(body)) } return nil } // CreateServiceAccount creates a new service account in the project. func (c *LiveGCPClient) CreateServiceAccount(ctx context.Context, projectID, saName, displayName string) error { - url := fmt.Sprintf("https://iam.googleapis.com/v1/projects/%s/serviceAccounts", projectID) + 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)) payload := fmt.Sprintf(`{"accountId":%q,"serviceAccount":{"displayName":%q}}`, saName, displayName) - resp, err := c.doRequest(ctx, http.MethodPost, url, payload) + resp, err := c.doRequest(ctx, http.MethodPost, reqURL, payload) if err != nil { return fmt.Errorf("creating service account: %w", err) } @@ -96,17 +139,22 @@ func (c *LiveGCPClient) CreateServiceAccount(ctx context.Context, projectID, saN } if resp.StatusCode != http.StatusOK { body, _ := io.ReadAll(resp.Body) - return fmt.Errorf("unexpected status %d creating service account: %s", resp.StatusCode, body) + return fmt.Errorf("unexpected status %d creating service account: %s", resp.StatusCode, extractGCPErrorMessage(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) { - url := fmt.Sprintf("https://iam.googleapis.com/v1/projects/%s/serviceAccounts/%s/keys", projectID, saEmail) + if !gcpIDPattern.MatchString(projectID) { + return nil, fmt.Errorf("invalid GCP project ID %q", projectID) + } + + 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.doRequest(ctx, http.MethodPost, url, payload) + resp, err := c.doRequest(ctx, http.MethodPost, reqURL, payload) if err != nil { return nil, fmt.Errorf("creating service account key: %w", err) } @@ -114,7 +162,7 @@ func (c *LiveGCPClient) CreateServiceAccountKey(ctx context.Context, projectID, if resp.StatusCode != http.StatusOK { body, _ := io.ReadAll(resp.Body) - return nil, fmt.Errorf("unexpected status %d creating key: %s", resp.StatusCode, body) + return nil, fmt.Errorf("unexpected status %d creating key: %s", resp.StatusCode, extractGCPErrorMessage(body)) } var result struct { diff --git a/internal/inference/vertex/vertex.go b/internal/inference/vertex/vertex.go index ada11a39ac..8087880fa9 100644 --- a/internal/inference/vertex/vertex.go +++ b/internal/inference/vertex/vertex.go @@ -12,8 +12,10 @@ import ( ) const ( - // SecretCredentials is the repo secret name for the GCP service account key. - SecretCredentials = "GOOGLE_APPLICATION_CREDENTIALS" + // 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. SecretProjectID = "GCP_PROJECT_ID" diff --git a/internal/layers/inference.go b/internal/layers/inference.go index 511c6da504..6a36dc6523 100644 --- a/internal/layers/inference.go +++ b/internal/layers/inference.go @@ -45,12 +45,33 @@ 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). func (l *InferenceLayer) Install(ctx context.Context) error { if l.provider == nil { l.ui.StepInfo("no inference provider configured, skipping") return nil } + // Check if secrets already exist to avoid re-provisioning. + allExist := true + for _, name := range l.provider.SecretNames() { + exists, err := l.client.RepoSecretExists(ctx, l.org, forge.ConfigRepoName, name) + if err != nil { + // Can't verify — proceed with provisioning. + allExist = false + break + } + if !exists { + allExist = false + break + } + } + if allExist { + l.ui.StepDone(fmt.Sprintf("%s credentials already provisioned, skipping", l.provider.Name())) + return nil + } + l.ui.StepStart(fmt.Sprintf("provisioning %s credentials", l.provider.Name())) secrets, err := l.provider.Provision(ctx) diff --git a/internal/layers/inference_test.go b/internal/layers/inference_test.go index b6ba5b27fc..2aeab8c200 100644 --- a/internal/layers/inference_test.go +++ b/internal/layers/inference_test.go @@ -37,9 +37,9 @@ func newInferenceLayer(t *testing.T, client *forge.FakeClient, provider inferenc func vertexProvider() *fakeProvider { return &fakeProvider{ name: "vertex", - secretNames: []string{"GOOGLE_APPLICATION_CREDENTIALS", "GCP_PROJECT_ID"}, + secretNames: []string{"FULLSEND_GCP_SA_KEY_JSON", "GCP_PROJECT_ID"}, secrets: map[string]string{ - "GOOGLE_APPLICATION_CREDENTIALS": `{"type":"service_account"}`, + "FULLSEND_GCP_SA_KEY_JSON": `{"type":"service_account"}`, "GCP_PROJECT_ID": "my-project", }, } @@ -67,7 +67,7 @@ func TestInferenceLayer_Install_StoresSecrets(t *testing.T) { secretMap[s.Name] = s.Value } - assert.Equal(t, `{"type":"service_account"}`, secretMap["GOOGLE_APPLICATION_CREDENTIALS"]) + assert.Equal(t, `{"type":"service_account"}`, secretMap["FULLSEND_GCP_SA_KEY_JSON"]) assert.Equal(t, "my-project", secretMap["GCP_PROJECT_ID"]) } @@ -104,6 +104,22 @@ func TestInferenceLayer_Install_SecretWriteError(t *testing.T) { assert.Contains(t, err.Error(), "permission denied") } +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/GCP_PROJECT_ID"] = true + provider := vertexProvider() + layer, buf := newInferenceLayer(t, client, provider) + + err := layer.Install(context.Background()) + require.NoError(t, err) + + // Should not have created any new secrets. + assert.Empty(t, client.CreatedSecrets) + // Should indicate skipping in output. + assert.Contains(t, buf.String(), "already provisioned") +} + func TestInferenceLayer_Uninstall_Noop(t *testing.T) { client := forge.NewFakeClient() provider := vertexProvider() @@ -116,7 +132,7 @@ func TestInferenceLayer_Uninstall_Noop(t *testing.T) { func TestInferenceLayer_Analyze_AllPresent(t *testing.T) { client := forge.NewFakeClient() - client.Secrets["test-org/.fullsend/GOOGLE_APPLICATION_CREDENTIALS"] = true + client.Secrets["test-org/.fullsend/FULLSEND_GCP_SA_KEY_JSON"] = true client.Secrets["test-org/.fullsend/GCP_PROJECT_ID"] = true provider := vertexProvider() layer, _ := newInferenceLayer(t, client, provider) @@ -145,7 +161,7 @@ func TestInferenceLayer_Analyze_NonePresent(t *testing.T) { func TestInferenceLayer_Analyze_Partial(t *testing.T) { client := forge.NewFakeClient() client.Secrets["test-org/.fullsend/GCP_PROJECT_ID"] = true - // GOOGLE_APPLICATION_CREDENTIALS missing + // FULLSEND_GCP_SA_KEY_JSON missing provider := vertexProvider() layer, _ := newInferenceLayer(t, client, provider) From db84d32130719b36a4d03497b7c29c2273529b74 Mon Sep 17 00:00:00 2001 From: Ralph Bean Date: Sun, 12 Apr 2026 10:32:57 -0400 Subject: [PATCH 3/5] Fix nil-pointer crash in analyze path and validate saEmail - Guard against nil gcpAPI in Provision() for modes 1/2, returning a clear error instead of panicking if called without a GCP client - Add saEmailPattern validation in CreateServiceAccountKey to prevent URL injection via crafted email parameters - Add tests for nil GCP client in all three modes Co-Authored-By: Claude Opus 4.6 --- internal/inference/vertex/gcp.go | 6 ++++++ internal/inference/vertex/vertex.go | 4 ++++ internal/inference/vertex/vertex_test.go | 26 ++++++++++++++++++++++++ 3 files changed, 36 insertions(+) diff --git a/internal/inference/vertex/gcp.go b/internal/inference/vertex/gcp.go index 8f677f43b4..406b013d81 100644 --- a/internal/inference/vertex/gcp.go +++ b/internal/inference/vertex/gcp.go @@ -18,6 +18,9 @@ import ( // 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 obtains access tokens via Application Default Credentials. type LiveGCPClient struct { @@ -149,6 +152,9 @@ func (c *LiveGCPClient) CreateServiceAccountKey(ctx context.Context, projectID, 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)) diff --git a/internal/inference/vertex/vertex.go b/internal/inference/vertex/vertex.go index 8087880fa9..3f43d731db 100644 --- a/internal/inference/vertex/vertex.go +++ b/internal/inference/vertex/vertex.go @@ -78,6 +78,10 @@ func (p *Provider) Provision(ctx context.Context) (map[string]string, error) { }, nil } + if p.gcpAPI == nil { + return nil, fmt.Errorf("GCP client is required for modes 1 and 2 (no credential JSON provided)") + } + saName := p.cfg.ServiceAccountName if saName == "" { // Mode 1: create a new service account. diff --git a/internal/inference/vertex/vertex_test.go b/internal/inference/vertex/vertex_test.go index fd9e52f239..797a6cb89c 100644 --- a/internal/inference/vertex/vertex_test.go +++ b/internal/inference/vertex/vertex_test.go @@ -150,6 +150,32 @@ func TestProvision_CreateKeyError(t *testing.T) { assert.Contains(t, err.Error(), "quota exceeded") } +func TestProvision_NilGCPClient_Mode1(t *testing.T) { + p := New(Config{ProjectID: "my-project"}, 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", ServiceAccountName: "my-sa"}, nil) + + _, 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 := `{"type":"service_account"}` + p := New(Config{ProjectID: "my-project", CredentialJSON: credJSON}, nil) + + secrets, err := p.Provision(context.Background()) + require.NoError(t, err) + assert.Equal(t, credJSON, secrets[SecretCredentials]) +} + func TestName(t *testing.T) { p := New(Config{}, nil) assert.Equal(t, "vertex", p.Name()) From f30c25e7e504ae452586ab016d1a462daebd926f Mon Sep 17 00:00:00 2001 From: Ralph Bean Date: Thu, 16 Apr 2026 16:42:43 -0400 Subject: [PATCH 4/5] Address medium and low review feedback on inference provisioning - Rename GCP_PROJECT_ID to FULLSEND_GCP_PROJECT_ID for prefix consistency - Validate credential file JSON structure before upload - Add NewAnalyzeOnly() constructor to replace fragile sentinel - Add 409 Conflict idempotency test for SA creation - Use []byte for credential JSON and zero after use - Add symlink protection on credential file path - Use json.Marshal instead of fmt.Sprintf for SA payload - Deduplicate runAnalyze by reusing loadExistingInferenceProvider - Bound HTTP response body reads with io.LimitReader Co-Authored-By: Claude Opus 4.6 --- .../adr-0014-github-apps-and-secrets/SPEC.md | 4 +- docs/plans/vertex-inference-provisioning.md | 14 +++--- e2e/admin/admin_test.go | 2 +- internal/cli/admin.go | 43 ++++++++++++++++--- internal/inference/vertex/gcp.go | 19 ++++++-- internal/inference/vertex/vertex.go | 17 +++++--- internal/inference/vertex/vertex_test.go | 40 +++++++++++++++-- internal/layers/inference_test.go | 14 +++--- 8 files changed, 116 insertions(+), 37 deletions(-) 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 0f3a4430a2..8b09d57a17 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 @@ -52,11 +52,11 @@ 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__APP_ID` | Decimal string of the GitHub App numeric ID | secrets | | Secret | `FULLSEND_GCP_SA_KEY_JSON` | GCP service account key JSON (when inference provider is `vertex`) | inference | -| Secret | `GCP_PROJECT_ID` | GCP project identifier (when inference provider is `vertex`) | inference | +| Secret | `FULLSEND_GCP_PROJECT_ID` | GCP project identifier (when inference provider is `vertex`) | 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 and variable as above; if PEM is empty (reuse path), the implementation **must** skip writing that role’s secret/variable. -- 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 both `FULLSEND_GCP_SA_KEY_JSON` and `GCP_PROJECT_ID`. +- 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 both `FULLSEND_GCP_SA_KEY_JSON` and `FULLSEND_GCP_PROJECT_ID`. ## 6. Analyze / health semantics for the secrets layer diff --git a/docs/plans/vertex-inference-provisioning.md b/docs/plans/vertex-inference-provisioning.md index 1dda5463b8..4910391ba7 100644 --- a/docs/plans/vertex-inference-provisioning.md +++ b/docs/plans/vertex-inference-provisioning.md @@ -6,7 +6,7 @@ Agents running in GitHub Actions need credentials to call an inference API (curr 1. Provision or accept GCP service account credentials 2. Upload the credential JSON as `FULLSEND_GCP_SA_KEY_JSON` repo secret on `.fullsend` (NOT org-scoped) -3. Upload the GCP project ID as `GCP_PROJECT_ID` repo secret on `.fullsend` +3. Upload the GCP project ID as `FULLSEND_GCP_PROJECT_ID` repo secret on `.fullsend` 4. Record the chosen inference provider in `config.yaml` This must be coded behind an abstract interface so we can support other inference providers (e.g., Bedrock, OpenAI) in the future. @@ -104,7 +104,7 @@ type GCPClient interface { ``` if cfg.CredentialJSON != "" { // Mode 3: key provided directly - return {"FULLSEND_GCP_SA_KEY_JSON": cfg.CredentialJSON, "GCP_PROJECT_ID": cfg.ProjectID} + return {"FULLSEND_GCP_SA_KEY_JSON": cfg.CredentialJSON, "FULLSEND_GCP_PROJECT_ID": cfg.ProjectID} } saName := cfg.ServiceAccountName @@ -120,7 +120,7 @@ if saName == "" { // Create key for the SA (modes 1 and 2) saEmail := saName + "@" + cfg.ProjectID + ".iam.gserviceaccount.com" keyJSON := gcpAPI.CreateServiceAccountKey(ctx, cfg.ProjectID, saEmail) -return {"FULLSEND_GCP_SA_KEY_JSON": keyJSON, "GCP_PROJECT_ID": cfg.ProjectID} +return {"FULLSEND_GCP_SA_KEY_JSON": keyJSON, "FULLSEND_GCP_PROJECT_ID": cfg.ProjectID} ``` ### Phase 2: Layer @@ -140,7 +140,7 @@ type InferenceLayer struct { - **Install**: Calls `provider.Provision(ctx)` to get secrets, then stores each as a repo secret on `.fullsend` via `client.CreateRepoSecret()`. - **Uninstall**: No-op (secrets deleted with repo, same as SecretsLayer). -- **Analyze**: Checks that expected secrets exist (`FULLSEND_GCP_SA_KEY_JSON`, `GCP_PROJECT_ID` for vertex). +- **Analyze**: Checks that expected secrets exist (`FULLSEND_GCP_SA_KEY_JSON`, `FULLSEND_GCP_PROJECT_ID` for vertex). - **RequiredScopes**: `["repo"]` for install/analyze. **Layer ordering** (updated): @@ -215,10 +215,10 @@ Add the InferenceLayer to the stack between SecretsLayer and DispatchTokenLayer. **E2E changes:** -- `e2e/admin/testutil.go`: Add `E2E_HALFSEND_VERTEX_KEY` env var loading. Also add `E2E_GCP_PROJECT_ID` (or derive from the key JSON). +- `e2e/admin/testutil.go`: Add `E2E_HALFSEND_VERTEX_KEY` env var loading. Also add `E2E_FULLSEND_GCP_PROJECT_ID` (or derive from the key JSON). - `e2e/admin/admin_test.go`: - In `runFullInstall()`: Create a vertex provider in mode 3 using the env var, add InferenceLayer to the test stack. - - In `verifyInstalled()`: Assert `FULLSEND_GCP_SA_KEY_JSON` and `GCP_PROJECT_ID` repo secrets exist. + - In `verifyInstalled()`: Assert `FULLSEND_GCP_SA_KEY_JSON` and `FULLSEND_GCP_PROJECT_ID` repo secrets exist. - In `verifyNotInstalled()`: Assert secrets are gone (deleted with repo). - In `buildTestLayerStack()`: Accept and include inference provider/layer. - The e2e test should `t.Skip` the inference portion if `E2E_HALFSEND_VERTEX_KEY` is not set (so existing CI doesn't break until the secret is wired into the workflow). @@ -247,7 +247,7 @@ References the layer stack ordering inline. Update the installation model descri Currently documents only GitHub App secrets (`FULLSEND__APP_PRIVATE_KEY`, `FULLSEND__APP_ID`). Add a new section or extend the secrets table to cover inference provider secrets stored on `.fullsend`: - `FULLSEND_GCP_SA_KEY_JSON` — GCP service account key JSON (repo secret) -- `GCP_PROJECT_ID` — GCP project identifier (repo secret) +- `FULLSEND_GCP_PROJECT_ID` — GCP project identifier (repo secret) Note: these are repo-level secrets on `.fullsend`, not org-level, consistent with the existing credential isolation pattern. diff --git a/e2e/admin/admin_test.go b/e2e/admin/admin_test.go index f95e66a19a..d8fc2112b5 100644 --- a/e2e/admin/admin_test.go +++ b/e2e/admin/admin_test.go @@ -364,7 +364,7 @@ func verifyInstalled(t *testing.T, env *e2eEnv, orgCfg *config.OrgConfig, enable // Inference secrets exist if vertex key was provided. if os.Getenv("E2E_HALFSEND_VERTEX_KEY") != "" { - for _, secretName := range []string{"FULLSEND_GCP_SA_KEY_JSON", "GCP_PROJECT_ID"} { + for _, secretName := range []string{"FULLSEND_GCP_SA_KEY_JSON", "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) diff --git a/internal/cli/admin.go b/internal/cli/admin.go index 4d53a4459c..8150a05c64 100644 --- a/internal/cli/admin.go +++ b/internal/cli/admin.go @@ -3,6 +3,7 @@ package cli import ( "bufio" "context" + "encoding/json" "fmt" "net/url" "os" @@ -127,11 +128,27 @@ func newInstallCmd() *cobra.Command { if gcpProject != "" { vcfg := vertex.Config{ProjectID: gcpProject, 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) } - vcfg.CredentialJSON = string(credData) + // Zero credential bytes when done to limit exposure in memory. + defer func() { + for i := range credData { + credData[i] = 0 + } + }() + if err := validateCredentialJSON(credData); err != nil { + return err + } + vcfg.CredentialJSON = credData } inferenceProvider = vertex.New(vcfg, vertex.NewLiveGCPClient()) inferenceProviderName = "vertex" @@ -564,13 +581,10 @@ func runAnalyze(ctx context.Context, client forge.Client, printer *ui.Printer, o return fmt.Errorf("getting authenticated user: %w", err) } - // Try to detect inference provider from existing config. + // Detect inference provider from existing config. var inferenceProvider inference.Provider - if cfgData, cfgErr := client.GetFileContent(ctx, org, forge.ConfigRepoName, "config.yaml"); cfgErr == nil { - if existingCfg, parseErr := config.ParseOrgConfig(cfgData); parseErr == nil && existingCfg.Inference.Provider != "" { - // For analyze, we only need SecretNames() — no GCP client needed. - inferenceProvider = vertex.New(vertex.Config{ProjectID: "analyze"}, nil) - } + if providerName := loadExistingInferenceProvider(ctx, client, org); providerName != "" { + inferenceProvider = vertex.NewAnalyzeOnly() } stack := buildLayerStack(org, client, cfg, printer, user, hasPrivate, nil, defaultBranches, agentCreds, "", nil, inferenceProvider) @@ -700,6 +714,21 @@ func loadExistingInferenceProvider(ctx context.Context, client forge.Client, org return cfg.Inference.Provider } +// 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 index 406b013d81..199e8eb31f 100644 --- a/internal/inference/vertex/gcp.go +++ b/internal/inference/vertex/gcp.go @@ -111,7 +111,7 @@ func (c *LiveGCPClient) GetServiceAccount(ctx context.Context, projectID, saName return fmt.Errorf("service account %s not found in project %s", saName, projectID) } if resp.StatusCode != http.StatusOK { - body, _ := io.ReadAll(resp.Body) + body, _ := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) return fmt.Errorf("unexpected status %d checking service account: %s", resp.StatusCode, extractGCPErrorMessage(body)) } return nil @@ -128,7 +128,18 @@ func (c *LiveGCPClient) CreateServiceAccount(ctx context.Context, projectID, saN reqURL := fmt.Sprintf("https://iam.googleapis.com/v1/projects/%s/serviceAccounts", url.PathEscape(projectID)) - payload := fmt.Sprintf(`{"accountId":%q,"serviceAccount":{"displayName":%q}}`, saName, displayName) + 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.doRequest(ctx, http.MethodPost, reqURL, payload) if err != nil { @@ -141,7 +152,7 @@ func (c *LiveGCPClient) CreateServiceAccount(ctx context.Context, projectID, saN return nil } if resp.StatusCode != http.StatusOK { - body, _ := io.ReadAll(resp.Body) + body, _ := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) return fmt.Errorf("unexpected status %d creating service account: %s", resp.StatusCode, extractGCPErrorMessage(body)) } return nil @@ -167,7 +178,7 @@ func (c *LiveGCPClient) CreateServiceAccountKey(ctx context.Context, projectID, defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - body, _ := io.ReadAll(resp.Body) + body, _ := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) return nil, fmt.Errorf("unexpected status %d creating key: %s", resp.StatusCode, extractGCPErrorMessage(body)) } diff --git a/internal/inference/vertex/vertex.go b/internal/inference/vertex/vertex.go index 3f43d731db..1316fb372d 100644 --- a/internal/inference/vertex/vertex.go +++ b/internal/inference/vertex/vertex.go @@ -18,7 +18,8 @@ const ( SecretCredentials = "FULLSEND_GCP_SA_KEY_JSON" // SecretProjectID is the repo secret name for the GCP project ID. - SecretProjectID = "GCP_PROJECT_ID" + // Uses the FULLSEND_ prefix for consistency with other secrets. + SecretProjectID = "FULLSEND_GCP_PROJECT_ID" // defaultSAName is the service account name created in mode 1. defaultSAName = "fullsend-agent" @@ -40,7 +41,7 @@ type GCPClient interface { type Config struct { ProjectID string // required ServiceAccountName string // optional: existing SA name (mode 2) - CredentialJSON string // optional: pre-made key JSON (mode 3) + CredentialJSON []byte // optional: pre-made key JSON (mode 3) } // Provider implements inference.Provider for Vertex AI. @@ -54,6 +55,12 @@ func New(cfg Config, gcpAPI GCPClient) *Provider { return &Provider{cfg: cfg, gcpAPI: gcpAPI} } +// NewAnalyzeOnly creates a Provider that only supports SecretNames() and Name(). +// Calling Provision() on this provider returns an error. +func NewAnalyzeOnly() *Provider { + return &Provider{} +} + // Name returns "vertex". func (p *Provider) Name() string { return "vertex" @@ -71,15 +78,15 @@ func (p *Provider) Provision(ctx context.Context) (map[string]string, error) { } // Mode 3: credential JSON provided directly. - if p.cfg.CredentialJSON != "" { + if len(p.cfg.CredentialJSON) > 0 { return map[string]string{ - SecretCredentials: p.cfg.CredentialJSON, + SecretCredentials: string(p.cfg.CredentialJSON), SecretProjectID: p.cfg.ProjectID, }, nil } if p.gcpAPI == nil { - return nil, fmt.Errorf("GCP client is required for modes 1 and 2 (no credential JSON provided)") + return nil, fmt.Errorf("GCP client is required for provisioning") } saName := p.cfg.ServiceAccountName diff --git a/internal/inference/vertex/vertex_test.go b/internal/inference/vertex/vertex_test.go index 797a6cb89c..6b7d5e3e75 100644 --- a/internal/inference/vertex/vertex_test.go +++ b/internal/inference/vertex/vertex_test.go @@ -18,6 +18,7 @@ type fakeGCPClient struct { getErr error createErr error createKeyErr error + alreadyExists bool // simulate 409 Conflict (SA already exists → success) } func newFakeGCPClient() *fakeGCPClient { @@ -42,6 +43,10 @@ func (f *fakeGCPClient) CreateServiceAccount(_ context.Context, projectID, saNam 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 @@ -107,7 +112,7 @@ func TestProvision_Mode2_SANotFound(t *testing.T) { func TestProvision_Mode3_PreMadeKey(t *testing.T) { gcp := newFakeGCPClient() - credJSON := `{"type":"service_account","project_id":"my-project","private_key":"..."}` + credJSON := []byte(`{"type":"service_account","project_id":"my-project","private_key":"..."}`) p := New(Config{ProjectID: "my-project", CredentialJSON: credJSON}, gcp) secrets, err := p.Provision(context.Background()) @@ -117,8 +122,24 @@ func TestProvision_Mode3_PreMadeKey(t *testing.T) { assert.Empty(t, gcp.createdSAs) assert.Empty(t, gcp.createdKeys) - assert.Equal(t, credJSON, secrets[SecretCredentials]) + 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"}, gcp) + + 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, "my-project", secrets[SecretProjectID]) + assert.Equal(t, string(gcp.keyData), secrets[SecretCredentials]) } func TestProvision_MissingProjectID(t *testing.T) { @@ -168,12 +189,23 @@ func TestProvision_NilGCPClient_Mode2(t *testing.T) { func TestProvision_NilGCPClient_Mode3_OK(t *testing.T) { // Mode 3 should work fine without a GCP client. - credJSON := `{"type":"service_account"}` + credJSON := []byte(`{"type":"service_account"}`) p := New(Config{ProjectID: "my-project", CredentialJSON: credJSON}, nil) secrets, err := p.Provision(context.Background()) require.NoError(t, err) - assert.Equal(t, credJSON, secrets[SecretCredentials]) + assert.Equal(t, string(credJSON), secrets[SecretCredentials]) +} + +func TestProvision_AnalyzeOnly(t *testing.T) { + p := NewAnalyzeOnly() + + 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") } func TestName(t *testing.T) { diff --git a/internal/layers/inference_test.go b/internal/layers/inference_test.go index 2aeab8c200..b5363db816 100644 --- a/internal/layers/inference_test.go +++ b/internal/layers/inference_test.go @@ -37,10 +37,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", "GCP_PROJECT_ID"}, + secretNames: []string{"FULLSEND_GCP_SA_KEY_JSON", "FULLSEND_GCP_PROJECT_ID"}, secrets: map[string]string{ - "FULLSEND_GCP_SA_KEY_JSON": `{"type":"service_account"}`, - "GCP_PROJECT_ID": "my-project", + "FULLSEND_GCP_SA_KEY_JSON": `{"type":"service_account"}`, + "FULLSEND_GCP_PROJECT_ID": "my-project", }, } } @@ -68,7 +68,7 @@ func TestInferenceLayer_Install_StoresSecrets(t *testing.T) { } assert.Equal(t, `{"type":"service_account"}`, secretMap["FULLSEND_GCP_SA_KEY_JSON"]) - assert.Equal(t, "my-project", secretMap["GCP_PROJECT_ID"]) + assert.Equal(t, "my-project", secretMap["FULLSEND_GCP_PROJECT_ID"]) } func TestInferenceLayer_Install_NilProvider(t *testing.T) { @@ -107,7 +107,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/GCP_PROJECT_ID"] = true + client.Secrets["test-org/.fullsend/FULLSEND_GCP_PROJECT_ID"] = true provider := vertexProvider() layer, buf := newInferenceLayer(t, client, provider) @@ -133,7 +133,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/GCP_PROJECT_ID"] = true + client.Secrets["test-org/.fullsend/FULLSEND_GCP_PROJECT_ID"] = true provider := vertexProvider() layer, _ := newInferenceLayer(t, client, provider) @@ -160,7 +160,7 @@ func TestInferenceLayer_Analyze_NonePresent(t *testing.T) { func TestInferenceLayer_Analyze_Partial(t *testing.T) { client := forge.NewFakeClient() - client.Secrets["test-org/.fullsend/GCP_PROJECT_ID"] = true + client.Secrets["test-org/.fullsend/FULLSEND_GCP_PROJECT_ID"] = true // FULLSEND_GCP_SA_KEY_JSON missing provider := vertexProvider() layer, _ := newInferenceLayer(t, client, provider) From 825b2eacf3ea7176dc1c5ddc608b8bc3fad50c28 Mon Sep 17 00:00:00 2001 From: Ralph Bean Date: Fri, 17 Apr 2026 08:44:28 -0400 Subject: [PATCH 5/5] Fix e2e build: convert vertexKey string to []byte for CredentialJSON Co-Authored-By: Claude Opus 4.6 --- e2e/admin/admin_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/admin/admin_test.go b/e2e/admin/admin_test.go index d8fc2112b5..df727eb3d2 100644 --- a/e2e/admin/admin_test.go +++ b/e2e/admin/admin_test.go @@ -227,7 +227,7 @@ func runFullInstall(t *testing.T, env *e2eEnv) ([]layers.AgentCredentials, *conf } inferenceProvider = vertex.New(vertex.Config{ ProjectID: gcpProjectID, - CredentialJSON: vertexKey, + CredentialJSON: []byte(vertexKey), }, nil) inferenceProviderName = "vertex" t.Logf("Inference provider: vertex (project: %s)", gcpProjectID)