Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions docs/ADRs/0044-deprecate-per-org-installation-mode.md
Original file line number Diff line number Diff line change
Expand Up @@ -333,8 +333,8 @@ ADR 0045 is accepted and its implementation (Phases 1–4) is complete.
`OrgVariableExists`, `DeleteOrgVariable`, `SetOrgVariableRepos`,
`GetOrgVariableRepos`, `CreateOrgSecret`, `OrgSecretExists`,
`DeleteOrgSecret`, `SetOrgSecretRepos`, `GetOrgSecretRepos`,
`ListOrgRepos`). Keep `ListOrgInstallations` (per-repo app discovery)
and `GetOrgPlan` (plan detection).
`ListOrgRepos`). Keep `ListOrgInstallations` (now on `GitHubExtensions`,
per-repo app discovery) and `GetOrgPlan` (plan detection).
- Remove the `FULLSEND_PER_REPO_INSTALL` guard variable and all
guard-checking logic (no longer needed when per-org enrollment
does not exist).
Expand Down Expand Up @@ -374,7 +374,8 @@ release checklist. The summary below captures the high-level ordering.
- PR 9: Remove per-org scaffold templates; rename per-repo scaffolds.
- PR 10: Remove org-level forge methods (10 secret/variable methods,
`ListOrgRepos`); remove `PerRepoGuardVar`; keep
`ListOrgInstallations` (per-repo app discovery) and `GetOrgPlan`.
`ListOrgInstallations` (now on `GitHubExtensions`, per-repo app
discovery) and `GetOrgPlan`.
- PR 11: Simplify dispatch provisioner; retain `Config.GitHubOrgs`
(needed by `EnsureOrgInMint`).
- PR 12: Simplify appsetup; remove per-org branching.
Expand Down
2 changes: 1 addition & 1 deletion docs/ADRs/0067-gitlab-cron-polling-event-dispatch.md
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ secret storage.
protected branches API
- `CreatePipelineSchedule` / `DeletePipelineSchedule` — GitLab-native; GitHub
returns `ErrNotSupported`
- `UpdateVariable` — for poll watermark management
- `UpdateCIVariable` — for poll watermark management

A new `ErrNotSupported` sentinel (complementing the existing forge
sentinel errors) allows forge
Expand Down
26 changes: 14 additions & 12 deletions docs/plans/deprecate-per-org-install.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ The per-org model touches these subsystems:
| Layer stack | `internal/layers/configrepo.go`, `dispatch.go`, `enrollment.go`, `secrets.go`, `inference.go`, `vendorbinary.go` | `ConfigRepoLayer`, `DispatchTokenLayer`, `EnrollmentLayer`, `buildLayerStack()` |
| Config | `internal/config/config.go` | `OrgConfig`, `NewOrgConfig`, `ParseOrgConfig`, `EnabledRepos`, `DisabledRepos`, `DefaultAgentRoles` |
| Scaffold | `internal/scaffold/fullsend-repo/` | `dispatch.yml`, thin callers, `shim-workflow-call.yaml`, `repo-maintenance.yml`, `CustomizedDirs()` |
| Forge interface | `internal/forge/forge.go`, `github/github.go`, `fake.go` | 11 org-level methods to remove (5 secret, 5 variable, `ListOrgRepos`), 2 retained (`ListOrgInstallations`, `GetOrgPlan`), `PerRepoGuardVar` |
| Forge interface | `internal/forge/forge.go`, `github/github.go`, `fake.go` | 11 org-level methods to remove (5 secret, 5 variable, `ListOrgRepos`), 2 retained (`ListOrgInstallations` via `GitHubExtensions`, `GetOrgPlan`), `PerRepoGuardVar` |
| Dispatch | `internal/dispatch/dispatch.go`, `gcf/provisioner.go` | `OrgSecretNames`, `OrgVariableNames`, PEM-sharing logic, org-wide WIF |
| Appsetup | `internal/appsetup/appsetup.go` | `findExistingInstallation` (org-level app discovery) |
| Mint | `internal/mintcore/claims.go` | `.fullsend` workflow ref pattern, `ValidateOrgAllowed` |
Expand Down Expand Up @@ -641,9 +641,9 @@ SetOrgVariableRepos(ctx context.Context, org, name string, repoIDs []int64) erro
GetOrgVariableRepos(ctx context.Context, org, name string) ([]int64, error)

// Org installations — DO NOT remove:
// ListOrgInstallations is also used by per-repo install path
// (detectSharedApps in admin.go, findExistingInstallation and
// ensureInstalled in appsetup.go).
// ListOrgInstallations (now on GitHubExtensions) is also used by per-repo
// install path (detectSharedApps in admin.go, findExistingInstallation
// and ensureInstalled in appsetup.go).
```

Remove `PerRepoGuardVar` constant and all its callers (these are
Expand All @@ -667,8 +667,9 @@ The guard variable is no longer needed when per-org enrollment does not
exist. All callers are explicitly covered: per-repo paths in this PR,
`enrollment.go` in PR 7, and `reconcile-repos.sh` in PR 9.

Keep `ListOrgInstallations()` — used by per-repo app discovery
(`detectSharedApps`, `findExistingInstallation`, `ensureInstalled`).
Keep `ListOrgInstallations()` (now on `GitHubExtensions`) — used by
per-repo app discovery (`detectSharedApps`, `findExistingInstallation`,
`ensureInstalled`).
Remove `ListOrgRepos()` — all callers (`admin.go` enrollment discovery,
`github.go` org setup, `dispatch.go` layer) are per-org code paths
removed in earlier PRs; no per-repo callers exist.
Expand All @@ -683,7 +684,7 @@ Remove implementations of the 10 removed interface methods:
- `CreateOrUpdateOrgVariable`, `OrgVariableExists`, `DeleteOrgVariable`,
`SetOrgVariableRepos`, `GetOrgVariableRepos` (~200 lines)

Keep `ListOrgInstallations` implementation (retained in interface).
Keep `ListOrgInstallations` implementation (retained on `GitHubExtensions`).

**`internal/forge/fake.go`:**

Expand All @@ -701,8 +702,8 @@ Remove:
- Any test files that use fake org methods: update to remove.

**After merge:** Forge interface has no org-level secret/variable methods.
`ListOrgInstallations` retained for per-repo app discovery. ~400 lines
removed from GitHub implementation.
`ListOrgInstallations` retained (on `GitHubExtensions`) for per-repo app
discovery. ~400 lines removed from GitHub implementation.

---

Expand Down Expand Up @@ -753,8 +754,9 @@ Keep `Provision()`, `StoreAgentPEM()`, `Name()`.
**`internal/appsetup/appsetup.go`:**

- Keep `findExistingInstallation()` and `ensureInstalled()` — these use
`ListOrgInstallations()` which is retained (app installation discovery
is an org-level GitHub API operation regardless of fullsend install mode).
`ListOrgInstallations()` via `GitHubExtensions` type assertion (app
installation discovery is an org-level GitHub API operation regardless
of fullsend install mode).
- Simplify `Run()`: remove any per-org-specific branching (e.g. org-level
app creation paths that differ from per-repo). The shared app discovery
flow (`detectSharedApps` → `findExistingInstallation`) works for both
Expand All @@ -771,7 +773,7 @@ Keep `Provision()`, `StoreAgentPEM()`, `Name()`.
cases.

**After merge:** Appsetup retains shared app discovery
(`findExistingInstallation`, `ensureInstalled` via `ListOrgInstallations`)
(`findExistingInstallation`, `ensureInstalled` via `GitHubExtensions.ListOrgInstallations`)
while per-org-specific branching is removed.

---
Expand Down
42 changes: 21 additions & 21 deletions docs/plans/gitlab-cron-polling-implementation.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,14 @@ Add to `internal/forge/forge.go`:

```go
IsProtectedBranch(ctx context.Context, owner, repo, branch string) (bool, error)
CreatePipelineSchedule(ctx context.Context, owner, repo, ref, description, cron string, variables map[string]string) (scheduleID string, err error)
DeletePipelineSchedule(ctx context.Context, owner, repo, scheduleID string) error
CreatePipelineSchedule(ctx context.Context, owner, repo, ref, description, cron string, variables map[string]string) (int64, error)
DeletePipelineSchedule(ctx context.Context, owner, repo string, scheduleID int64) error
ListPipelineSchedules(ctx context.Context, owner, repo string) ([]PipelineSchedule, error)
UpdateVariable(ctx context.Context, owner, repo, key, value string) error
CreateProtectedVariable(ctx context.Context, owner, repo, key, value string) error
UpdateCIVariable(ctx context.Context, owner, repo, name, value string, protected bool) error
CreateProtectedCIVariable(ctx context.Context, owner, repo, name, value string) error
```

These methods are forge-neutral by design. `IsProtectedBranch` maps to GitHub's branch protection API and GitLab's protected branches API. `CreatePipelineSchedule` and `DeletePipelineSchedule` are GitLab-native; the GitHub implementation returns `ErrNotSupported`. `UpdateVariable` maps to GitLab's CI/CD variable API. `CreateProtectedVariable` creates a CI/CD variable with `Protected: true, Masked: false` — used for poll state variables (watermark, label state) that must not be accessible on non-protected branches but whose values are not secrets.
These methods are forge-neutral by design. `IsProtectedBranch` maps to GitHub's branch protection API and GitLab's protected branches API. `CreatePipelineSchedule` and `DeletePipelineSchedule` are GitLab-native; the GitHub implementation returns `ErrNotSupported`. `UpdateCIVariable` maps to GitLab's CI/CD variable API. `CreateProtectedCIVariable` creates a CI/CD variable with `Protected: true, Masked: false` — used for poll state variables (watermark, label state) that must not be accessible on non-protected branches but whose values are not secrets. The "CI" prefix distinguishes these from the existing `RepoVariable` methods which model GitHub Actions variables.

### New sentinel error

Expand Down Expand Up @@ -206,8 +206,8 @@ Single-token constructor for the bot project access token. The token is used for
| `CreatePipelineSchedule` | `PipelineSchedules.CreatePipelineSchedule` | GitLab-specific |
| `DeletePipelineSchedule` | `PipelineSchedules.DeletePipelineSchedule` | GitLab-specific |
| `ListPipelineSchedules` | `PipelineSchedules.ListProjectPipelineSchedules` | For uninstall cleanup |
| `UpdateVariable` | `ProjectVariables.UpdateVariable` | For poll watermark |
| `CreateProtectedVariable` | `ProjectVariables.CreateVariable` | With `Protected: true`, `Masked: false` — for poll state |
| `UpdateCIVariable` | `ProjectVariables.UpdateVariable` | For poll watermark |
| `CreateProtectedCIVariable` | `ProjectVariables.CreateVariable` | With `Protected: true`, `Masked: false` — for poll state |
| `DispatchWorkflow` | → `ErrNotSupported` | GitHub-only |
| `ListOrgInstallations` | → `GitHubExtensions` (not on base interface) | GitHub-only |
| `GetAppClientID` | → `GitHubExtensions` (not on base interface) | GitHub-only |
Expand Down Expand Up @@ -1061,7 +1061,7 @@ func (p *Poller) persistLabelState(ctx context.Context, owner, repo string, stat
if err != nil {
return fmt.Errorf("marshal label state: %w", err)
}
if err := p.client.UpdateVariable(ctx, owner, repo, "FULLSEND_LABEL_STATE", string(stateBytes)); err != nil {
if err := p.client.UpdateCIVariable(ctx, owner, repo, "FULLSEND_LABEL_STATE", string(stateBytes), true); err != nil {
return fmt.Errorf("persist label state: %w", err)
}
return nil
Expand Down Expand Up @@ -1093,7 +1093,7 @@ func (p *Poller) watermarkVarName() string {
}

func (p *Poller) updateWatermark(ctx context.Context, owner, repo string, t time.Time) error {
return p.client.UpdateVariable(ctx, owner, repo, p.watermarkVarName(), t.Format(time.RFC3339))
return p.client.UpdateCIVariable(ctx, owner, repo, p.watermarkVarName(), t.Format(time.RFC3339), true)
}
```

Expand Down Expand Up @@ -1673,19 +1673,19 @@ func runGitLabPerRepoInstall(ctx context.Context, target string, opts installOpt
"chore: add fullsend CI/CD pipeline", scaffoldFiles)

// 12. Set protected CI/CD variables.
// Use CreateProtectedVariable (Protected: true, Masked: false) for
// Use CreateProtectedCIVariable (Protected: true, Masked: false) for
// configuration identifiers — CreateRepoSecret (Protected + Masked)
// requires values >= 8 characters (e.g. "wif" would fail) and masks
// GCP resource names in logs, hindering debugging.
client.CreateProtectedVariable(ctx, owner, repo, "FULLSEND_CREDENTIAL_MODE", credentialMode)
client.CreateProtectedCIVariable(ctx, owner, repo, "FULLSEND_CREDENTIAL_MODE", credentialMode)
if credentialMode == "wif" {
client.CreateProtectedVariable(ctx, owner, repo, "FULLSEND_WIF_PROVIDER", wifProviderResourceName)
client.CreateProtectedVariable(ctx, owner, repo, "FULLSEND_SA", serviceAccountEmail)
client.CreateProtectedVariable(ctx, owner, repo, "FULLSEND_BOT_TOKEN_SECRET", secretManagerSecretName)
client.CreateProtectedVariable(ctx, owner, repo, "FULLSEND_GCP_PROJECT_ID", opts.gcpProject)
client.CreateProtectedCIVariable(ctx, owner, repo, "FULLSEND_WIF_PROVIDER", wifProviderResourceName)
client.CreateProtectedCIVariable(ctx, owner, repo, "FULLSEND_SA", serviceAccountEmail)
client.CreateProtectedCIVariable(ctx, owner, repo, "FULLSEND_BOT_TOKEN_SECRET", secretManagerSecretName)
client.CreateProtectedCIVariable(ctx, owner, repo, "FULLSEND_GCP_PROJECT_ID", opts.gcpProject)
}
client.CreateProtectedVariable(ctx, owner, repo, "FULLSEND_FORGE", "gitlab")
client.CreateProtectedVariable(ctx, owner, repo, "FULLSEND_PER_REPO_INSTALL", "true")
client.CreateProtectedCIVariable(ctx, owner, repo, "FULLSEND_FORGE", "gitlab")
client.CreateProtectedCIVariable(ctx, owner, repo, "FULLSEND_PER_REPO_INSTALL", "true")

// 13. Initialize poll watermarks (protected — must not be accessible
// to pipelines on non-protected branches to prevent tampering).
Expand All @@ -1696,9 +1696,9 @@ func runGitLabPerRepoInstall(ctx context.Context, target string, opts installOpt
// full-poll re-discovers it) is harmless — resource_group serialization
// ensures at most one pipeline at a time; see ADR 0067.
initTime := time.Now().Format(time.RFC3339)
client.CreateProtectedVariable(ctx, owner, repo, "FULLSEND_LAST_POLL_AT_FAST", initTime)
client.CreateProtectedVariable(ctx, owner, repo, "FULLSEND_LAST_POLL_AT_FULL", initTime)
client.CreateProtectedVariable(ctx, owner, repo, "FULLSEND_LABEL_STATE", "{}")
client.CreateProtectedCIVariable(ctx, owner, repo, "FULLSEND_LAST_POLL_AT_FAST", initTime)
client.CreateProtectedCIVariable(ctx, owner, repo, "FULLSEND_LAST_POLL_AT_FULL", initTime)
client.CreateProtectedCIVariable(ctx, owner, repo, "FULLSEND_LABEL_STATE", "{}")

// 14. Set up inference WIF (if --inference-project provided)

Expand Down Expand Up @@ -1816,7 +1816,7 @@ Add implementations to `internal/forge/fake.go` for:
- `IsProtectedBranch` — configurable return value
- `CreatePipelineSchedule` — record call, return fake schedule ID
- `DeletePipelineSchedule` — record call
- `UpdateVariable` — record call
- `UpdateCIVariable` — record call

## Security-Critical Code Paths

Expand Down
17 changes: 5 additions & 12 deletions docs/problems/gitlab-implementation.md
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ This keeps the dispatch scanning logic identical across GitHub and GitLab.

## Forge Interface Evolution

**Challenge**: ADR-0005 promises "Adding a new forge requires implementing `forge.Client` — no changes to layers, CLI, or app setup code." However, the current `forge.Client` interface contains GitHub-specific methods (`ListOrgInstallations`, `GetAppClientID`) and operations (`DispatchWorkflow`) that don't map directly to GitLab.
**Challenge**: ADR-0005 promises "Adding a new forge requires implementing `forge.Client` — no changes to layers, CLI, or app setup code." However, the `forge.Client` interface previously contained GitHub-specific methods (`ListOrgInstallations`, `GetAppClientID`) that don't map to GitLab. These have since been moved to the `forge.GitHubExtensions` interface, and callers use type assertions. Operations like `DispatchWorkflow` remain on the base interface.

### Proposed Forge-Neutral Interface Additions

Expand Down Expand Up @@ -418,23 +418,16 @@ DeleteWebhook(ctx context.Context, owner, repo, webhookID string) error

### Existing GitHub-Specific Methods

- `ListOrgInstallations(ctx, org) ([]Installation, error)` — GitHub App-specific. GitLab equivalent would list Project Access Tokens, but tokens are scoped per-project not org-wide. This method may need to become forge-specific or return an empty list for non-GitHub forges.
- `GetAppClientID(ctx, slug) (string, error)` — GitHub App-specific. No GitLab equivalent. This should be deprecated or moved to a GitHub-specific extension interface.
- `ListOrgInstallations(ctx, org) ([]Installation, error)` — GitHub App-specific. **Now moved to `forge.GitHubExtensions`**. GitLab equivalent would list Project Access Tokens, but tokens are scoped per-project not org-wide. Callers type-assert to `GitHubExtensions` and handle the fallback.
- `GetAppClientID(ctx, slug) (string, error)` — GitHub App-specific. No GitLab equivalent. **Now moved to `forge.GitHubExtensions`**. Callers type-assert and fall back gracefully.
- `DispatchWorkflow(ctx, owner, repo, workflowFile, ref, inputs)` — GitHub Actions-specific (targets a specific workflow file). Replaced by forge-neutral `TriggerPipeline` above.

### Backward Compatibility and Migration Strategy

To prevent interface bloat while maintaining backward compatibility:

1. **Deprecation phase**: Mark GitHub-specific methods with deprecation comments and update callers to use forge-neutral equivalents (`TriggerPipeline` instead of `DispatchWorkflow`, etc.). This phase allows gradual migration without breaking existing code.
2. **Extension interfaces**: Move forge-specific methods that have no neutral equivalent (e.g., `GetAppClientID`) to optional extension interfaces:
```go
type GitHubForgeClient interface {
Client
GetAppClientID(ctx context.Context, slug string) (string, error)
}
```
Callers that need GitHub-specific behavior can type-assert to the extension interface.
1. **Extension interfaces (implemented)**: GitHub-specific methods (`ListOrgInstallations`, `GetAppClientID`) have been moved to the `forge.GitHubExtensions` optional extension interface. Callers type-assert and handle non-GitHub forges gracefully.
2. **Deprecation phase**: Mark remaining GitHub-specific methods with deprecation comments and update callers to use forge-neutral equivalents (`TriggerPipeline` instead of `DispatchWorkflow`, etc.). This phase allows gradual migration without breaking existing code.
3. **Breaking change timeline**: After all internal callers migrate to forge-neutral methods, remove deprecated methods in a major version bump. Document this timeline in the interface godoc (e.g., "deprecated: use TriggerPipeline, will be removed in v2.0.0").

This strategy limits interface growth to forge-neutral primitives while preserving GitHub-specific functionality via opt-in extension interfaces.
Expand Down
Loading
Loading