From bf5e107eb347c2fc1c8fcd2f4190e2c3aed05ae1 Mon Sep 17 00:00:00 2001 From: Chrison Simtian Date: Mon, 25 May 2026 20:16:54 +1200 Subject: [PATCH 1/2] docs(adr): seed ADR home + propose ADR-0001 (CD primitives) and ADR-0002 (auth/secrets) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduces docs/adr/ as the home for Architecture Decision Records, with a README that locks the MADR-lite format, status lifecycle, and filename convention. Adds the first two ADRs — both Proposed, dated 2026-05-24 — as the architectural framing for the v13 CD work: - ADR-0001 splits CD primitives into attribute-driven file generators (config that lives in git) vs tasks-driven REST calls (state that lives in an external system's DB), with a sparingly-used hybrid for stable config that lives in an external DB. Validates the split against GitHub Releases/Environments, Octopus, and the RFC #113 deployment agent. - ADR-0002 codifies a shared auth/secret convention across providers: canonical SCREAMING_SNAKE from PascalCase field names, plugins receive resolved values only (never raw stores), [Secret] as the trust marker, and a documented resolution chain (CLI > env var > provider secret > encrypted parameters > Keychain > prompt). Lands as Proposed so review (#167) is where the status flips to Accepted or supersedes either with a follow-up ADR. Per docs/adr/README.md, ADRs are history — corrections happen via new ADRs, not in-place edits. Refs #167 --- .../0001-cd-primitives-attributes-vs-tasks.md | 440 ++++++++++++++++++ ...ss-provider-auth-and-secret-conventions.md | 154 ++++++ docs/adr/README.md | 36 ++ 3 files changed, 630 insertions(+) create mode 100644 docs/adr/0001-cd-primitives-attributes-vs-tasks.md create mode 100644 docs/adr/0002-cross-provider-auth-and-secret-conventions.md create mode 100644 docs/adr/README.md diff --git a/docs/adr/0001-cd-primitives-attributes-vs-tasks.md b/docs/adr/0001-cd-primitives-attributes-vs-tasks.md new file mode 100644 index 000000000..753378e13 --- /dev/null +++ b/docs/adr/0001-cd-primitives-attributes-vs-tasks.md @@ -0,0 +1,440 @@ +# ADR-0001 — CD primitives: attributes for config, tasks for state + +- **Status:** Proposed +- **Date:** 2026-05-24 +- **Deciders:** Fallout maintainers +- **Relates to:** RFC [#106](https://github.com/ChrisonSimtian/Fallout/issues/106) (CD platform vision), RFC [#113](https://github.com/ChrisonSimtian/Fallout/issues/113) (deployment agent), milestone [v13](https://github.com/ChrisonSimtian/Fallout/milestone/8), epic [#139](https://github.com/ChrisonSimtian/Fallout/issues/139) (package-manager distribution) + +## Context + +Fallout already has a well-established CI integration pattern: a class-level `[GitHubActions(...)]` attribute is reflected into a POCO at startup, the POCO is serialized to `.github/workflows/*.yml` by a hand-rolled writer, the file is committed. See `src/Fallout.Common/CI/GitHubActions/GitHubActionsAttribute.cs` for the canonical implementation, `src/Fallout.Build/CICD/GenerateBuildServerConfigurationsAttribute.cs` for the middleware that drives generation, and `build/Build.CI.GitHubActions.cs` for live usage. + +The v13 milestone introduces **continuous delivery** — release tracking, environment promotion, approval gates, deployment-target execution. The natural question for contributors is: *do we keep using the attribute-driven file-generation pattern, or does CD need something different?* + +Three concrete drivers force the question: + +1. **GitHub Releases & Environments** (immediate need — once v13 lands). +2. **Octopus Deploy** (target: 2019 on-prem server — concrete consumer ask). +3. **The deployment agent of RFC #113** — needs to be told what to deploy where, by what. + +These three share a property the CI side doesn't have: **their state lives in an external system's database**, not in a file you commit. You don't "generate a Release"; you POST one. You don't "generate an Environment"; you reconcile its config via the API. The existing attribute → writer pattern is wrong-shaped for that. + +## Decision + +CD primitives split into **two** patterns, picked by whether the primitive's state is *file-shaped* or *API-shaped*: + +| Pattern | When | Mechanism | +|---|---|---| +| **Attribute → file writer** (existing) | Output is a config file committed to git | `[Attribute]` → POCO → `Write(CustomFileWriter)` → `.yml` / `.toml` / `.json` on disk | +| **Tasks → REST client** (new) | Output is state in an external system's database | `*Tasks` static class with methods that issue authenticated HTTP calls; called from C# targets | +| **Attribute + sync target** (hybrid, sparingly) | Configuration that is *stable* but lives in an external DB (e.g. Environments, Octopus Projects) | `[Attribute]` declares the config; a `Sync*` target reads the attributes via reflection and PUTs them through the API | + +The hybrid is the bridge — declarative authoring, imperative reconciliation. Use it only where the *definition* is stable across runs (an Environment's reviewers don't change every release). Releases and Deployments, which are inherently per-version, stay pure tasks. + +### Why not all-attribute? + +You'd need a file writer that "generates Releases." But Releases live in GitHub/Octopus, not on disk — there's no file to write. Forcing the pattern means inventing fictional artifacts (`.github/releases/v1.2.3.json`?) that nothing consumes. + +### Why not all-task? + +Environments and Octopus Projects *are* configuration — required reviewers, lifecycle definitions, channel rules. Writing 30 lines of imperative API calls every time you want to add a reviewer is the YAML problem in disguise. The hybrid keeps declarative authoring where it earns its weight. + +## Sketches + +### 1. GitHub Releases — pure tasks + +```csharp +// src/Fallout.Common/Tools/GitHub/Releases/GitHubReleaseTasks.cs + +public static partial class GitHubReleaseTasks +{ + public static async Task CreateGitHubRelease( + Configure configurator) + { + var settings = configurator(new CreateGitHubReleaseSettings()); + + var response = await GitHubHttpClient.Value + .CreateRequest(HttpMethod.Post, $"repos/{settings.Repository}/releases") + .WithBearerToken(settings.Token) + .WithJsonContent(new + { + tag_name = settings.TagName, + name = settings.Name, + body = settings.Body, + draft = settings.Draft, + prerelease = settings.PreRelease, + make_latest = settings.MakeLatest ? "true" : "false" + }) + .GetResponseAsync(); + + return await response.AsJsonAsync(); + } + + public static async Task UploadReleaseAsset( + long releaseId, + AbsolutePath asset, + Configure configurator = null) + { + var settings = (configurator ?? (s => s))(new UploadReleaseAssetSettings()); + // POST to uploads.github.com/repos/{repo}/releases/{id}/assets?name={file} + // ... binary upload, returns asset metadata + } + + public static async Task GetLatestRelease(string repository, string token) { /* ... */ } + public static async Task DeleteGitHubRelease(long releaseId, string repository, string token) { /* ... */ } +} + +public class CreateGitHubReleaseSettings : ToolSettings +{ + public string Repository { get; internal set; } + public string Token { get; internal set; } + public string TagName { get; internal set; } + public string Name { get; internal set; } + public string Body { get; internal set; } + public bool Draft { get; internal set; } + public bool PreRelease { get; internal set; } + public bool MakeLatest { get; internal set; } = true; +} + +public static class CreateGitHubReleaseSettingsExtensions +{ + public static T SetTagName(this T settings, string tagName) where T : CreateGitHubReleaseSettings + => settings.With(s => s.TagName = tagName); + // ... idiomatic fluent setters matching the existing Tool wrapper convention +} +``` + +Used from a target — composes with everything else (`DependsOn`, `Requires`, `OnlyWhen*`): + +```csharp +[Parameter, Secret] readonly string GitHubToken; + +Target PublishGitHubRelease => _ => _ + .DependsOn(Pack) + .OnlyWhenStatic(() => GitRepository.IsOnMainBranch()) + .Requires(() => GitHubToken) + .Executes(async () => + { + var release = await GitHubReleaseTasks.CreateGitHubRelease(s => s + .SetRepository("ChrisonSimtian/Fallout") + .SetToken(GitHubToken) + .SetTagName($"v{MajorMinorPatchVersion}") + .SetName($"Fallout {MajorMinorPatchVersion}") + .SetBody(ChangelogTasks.ExtractNotes(ChangelogFile, MajorMinorPatchVersion))); + + await PackagesDirectory.GlobFiles("*.nupkg").ForEachAsync(asset => + GitHubReleaseTasks.UploadReleaseAsset(release.Id, asset)); + }); +``` + +### 2. GitHub Environments — hybrid (attribute + sync) + +```csharp +// src/Fallout.Common/CD/GitHubEnvironments/GitHubEnvironmentAttribute.cs + +[AttributeUsage(AttributeTargets.Class, AllowMultiple = true)] +public class GitHubEnvironmentAttribute : DeploymentConfigurationAttributeBase +{ + public GitHubEnvironmentAttribute(string name) => Name = name; + public string Name { get; } + public string[] RequiredReviewers { get; set; } = new string[0]; + public uint WaitTimerMinutes { get; set; } + public string DeploymentBranchPolicy { get; set; } // "main" / "protected" / "selected" + public string[] AllowedBranchPatterns { get; set; } = new string[0]; + public string[] ImportSecrets { get; set; } = new string[0]; // creates env-scoped secrets + + public override Type ProviderType => typeof(GitHubEnvironmentProvider); +} +``` + +```csharp +// build/Build.CD.cs + +[GitHubEnvironment("staging", + WaitTimerMinutes = 0, + DeploymentBranchPolicy = "selected", + AllowedBranchPatterns = new[] { "main", "release/*" })] +[GitHubEnvironment("production", + RequiredReviewers = new[] { "@chrisonsimtian/release-managers" }, + WaitTimerMinutes = 30, + DeploymentBranchPolicy = "selected", + AllowedBranchPatterns = new[] { "main" }, + ImportSecrets = new[] { "NuGetApiKey", "OctopusApiKey" })] +partial class Build { } +``` + +```csharp +Target SyncGitHubEnvironments => _ => _ + .Requires(() => GitHubToken) + .Executes(async () => + { + foreach (var env in this.GetType().GetCustomAttributes()) + await GitHubEnvironmentTasks.UpsertEnvironment(env, "ChrisonSimtian/Fallout", GitHubToken); + }); +``` + +Same shape as `[GenerateBuildServerConfigurations]`, but the "generator" emits API calls instead of file writes. Run on demand; not part of the build hot path. + +The CI side gains **one** new property to close the loop: + +```csharp +[GitHubActions("deploy-production", GitHubActionsImage.UbuntuLatest, + OnPushBranches = new[] { "main" }, + InvokedTargets = new[] { nameof(Deploy) }, + Environment = "production")] // ← references the environment by name +``` + +Definition lives in API-land; reference lives in YAML. Clean split. + +### 3. Octopus Deploy 2019 — same patterns, larger surface, shipped as a v12 plugin + +Octopus is a richer domain (Projects, Releases, Channels, Lifecycles, Environments, Targets, Variables, Tenants), but the same two patterns cover it. + +**Placement:** unlike `Fallout.Common.GitHub` (which is first-party, in-tree), Octopus integration is expected to ship as the **first sample plugin** on top of the v12 plugin SDK (milestone [#7](https://github.com/ChrisonSimtian/Fallout/milestone/7)). The maintainer's own Octopus 2019 server has full API support, so the plugin doubles as a dogfooding vehicle for the SDK. This is a design-validation win: if a real consumer with a non-trivial provider can be built as a clean external plugin, the SDK has succeeded; if it forces awkward escape hatches, the SDK isn't ready. **The patterns below are framework-shape; the *namespace* and *package* are v12-plugin-shape.** + +```csharp +// Pure tasks — per-release imperative +public static partial class OctopusTasks +{ + public static Task PushPackage(AbsolutePath package, OctopusEndpoint endpoint); + public static Task CreateRelease(Configure configurator); + public static Task DeployRelease(Configure configurator); + public static Task WaitForDeployment(string deploymentId, OctopusEndpoint endpoint, TimeSpan timeout); + public static Task PromoteRelease(string releaseId, string toEnvironment, OctopusEndpoint endpoint); +} + +// Hybrid — Project + Channel + Lifecycle are stable config, declared once +[OctopusProject("Agent.WebScraper", LifecycleId = "Lifecycles-Standard")] +[OctopusProject("Agent.Indexer", LifecycleId = "Lifecycles-Standard")] +// ... 10 more +partial class Build { } + +Target SyncOctopusProjects => _ => _ + .Requires(() => OctopusApiKey) + .Executes(async () => { /* read attributes, PUT to /api/projects */ }); +``` + +The mental shift vs GitHub: **Octopus owns the deployment process**. Fallout's job is "build, push to feed, create release, optionally trigger deployment, optionally wait." The actual *what runs on which machine* is Octopus's deployment process definition — also code-defined (under Octopus's own `Octopus.Cli` / config-as-code), but a separate concern. + +## The user's scenario — one big build, many deployments + +> *"I have a solution where I have one big build but then multiple deployments across multiple projects: a dozen agents, two to three web UIs, and a handful of NuGet packages."* + +This is the **monorepo-with-coordinated-release** pattern. The deciding question is whether all artifacts share a version stream (simpler) or each evolves independently (more complex). Default: shared version stream — every artifact gets `MajorMinorPatchVersion` from `Nerdbank.GitVersioning`, every Octopus Release uses that same version. + +### Mental model + +```mermaid +flowchart TD + Compile[Compile] --> Test[Test] + Test --> Pack[Pack] + + subgraph "One build, three artifact families" + Pack --> PackAgents["12× agent .nupkg
artifacts/agents/"] + Pack --> PackUIs["3× web UI .nupkg
artifacts/ui/"] + Pack --> PackLibs["N× library .nupkg
artifacts/nuget/"] + end + + PackAgents --> PushOcto["OctopusTasks.PushPackage
(loop over agents + UIs)"] + PackUIs --> PushOcto + PackLibs --> PushNuGet["DotNetNuGetPush
internal feed only"] + + PushOcto --> CreateRel["OctopusTasks.CreateRelease
fan-out: 1 per deployable project
(12 agents + 3 UIs = 15 releases)"] + + CreateRel --> DeployDev["Deploy → Dev
(auto, via lifecycle)"] + DeployDev -.->|approval| DeployTest["Deploy → Test"] + DeployTest -.->|approval| DeployProd["Deploy → Prod"] + + style PackLibs fill:#e8f5e9 + style PushNuGet fill:#e8f5e9 + style PackAgents fill:#fff3e0 + style PackUIs fill:#fff3e0 +``` + +Note the **green path** for libraries: those never enter Octopus, they go straight to a NuGet feed. Mixing them into Octopus would create 5+ throwaway projects whose "deployment" is meaningless. Keep the lanes separate from the start. + +### Concrete Build.cs sketch + +```csharp +[Octopus(Server = "https://octopus.internal", ApiKey = nameof(OctopusApiKey))] +partial class Build : FalloutBuild +{ + [Parameter, Secret] readonly string OctopusApiKey; + [Parameter, Secret] readonly string InternalNuGetApiKey; + + static readonly DeployableProject[] Agents = new[] + { + new DeployableProject("Agent.WebScraper", "src/Agents/Agent.WebScraper"), + new DeployableProject("Agent.Indexer", "src/Agents/Agent.Indexer"), + new DeployableProject("Agent.Reporter", "src/Agents/Agent.Reporter"), + // ... 9 more + }; + + static readonly DeployableProject[] WebUIs = new[] + { + new DeployableProject("Customer.UI", "src/UI/Customer.UI"), + new DeployableProject("Admin.UI", "src/UI/Admin.UI"), + new DeployableProject("Ops.UI", "src/UI/Ops.UI"), + }; + + static readonly DeployableProject[] Libraries = new[] + { + new DeployableProject("Common.Contracts", "src/Common.Contracts"), + new DeployableProject("Common.Telemetry", "src/Common.Telemetry"), + // ... + }; + + IEnumerable Deployables => Agents.Concat(WebUIs); + + Target Pack => _ => _ + .DependsOn(Test) + .Executes(() => + { + Deployables.Concat(Libraries).ForEach(p => + DotNetTasks.DotNetPack(s => s + .SetProject(p.ProjectPath) + .SetVersion(MajorMinorPatchVersion) + .SetOutputDirectory(ArtifactsDir / p.Kind))); + }); + + Target PushToOctopus => _ => _ + .DependsOn(Pack) + .OnlyWhenStatic(() => GitRepository.IsOnMainBranch()) + .Requires(() => OctopusApiKey) + .Executes(async () => + { + await Deployables.ForEachAsync(p => + OctopusTasks.PushPackage( + ArtifactsDir / p.Kind / $"{p.Name}.{MajorMinorPatchVersion}.nupkg", + OctopusEndpoint)); + }); + + Target PushToNuGet => _ => _ + .DependsOn(Pack) + .OnlyWhenStatic(() => GitRepository.IsOnMainBranch()) + .Requires(() => InternalNuGetApiKey) + .Executes(async () => + { + await Libraries.ForEachAsync(p => + DotNetTasks.DotNetNuGetPush(s => s + .SetTargetPath(ArtifactsDir / p.Kind / $"{p.Name}.{MajorMinorPatchVersion}.nupkg") + .SetSource(InternalNuGetFeed) + .SetApiKey(InternalNuGetApiKey))); + }); + + Target CreateOctopusReleases => _ => _ + .DependsOn(PushToOctopus) + .Requires(() => OctopusApiKey) + .Executes(async () => + { + await Deployables.ForEachAsync(p => + OctopusTasks.CreateRelease(s => s + .SetEndpoint(OctopusEndpoint) + .SetProject(p.Name) + .SetVersion(MajorMinorPatchVersion) + .SetReleaseNotes(ChangelogTasks.ExtractNotes(ChangelogFile, MajorMinorPatchVersion)) + .EnableIgnoreExisting())); + }); + + Target DeployToDev => _ => _ + .DependsOn(CreateOctopusReleases) + .Executes(async () => + { + await Deployables.ForEachAsync(p => + OctopusTasks.DeployRelease(s => s + .SetEndpoint(OctopusEndpoint) + .SetProject(p.Name) + .SetVersion(MajorMinorPatchVersion) + .SetEnvironment("Dev"))); + }); +} + +record DeployableProject(string Name, string ProjectPath) +{ + public string Kind => /* "agents" | "ui" | "library" — derived from path */; +} +``` + +The shape is deliberately mundane. C# expresses the inventory; LINQ does the fan-out; the framework does dependency ordering. **No new conceptual machinery — just the existing target + task primitives applied to a richer external API.** + +## The two-stack architecture (the diagram you asked for) + +```mermaid +flowchart TB + subgraph "Author surface (Build.cs)" + BuildCs[partial class Build : FalloutBuild] + end + + subgraph "CI side — file-shaped" + CIAttr["[GitHubActions]"] + CIWriter["CustomFileWriter
(serialize POCO → YAML)"] + Workflows[.github/workflows/*.yml] + CIAttr --> CIWriter --> Workflows + end + + subgraph "CD side — API-shaped" + Tasks["*Tasks classes
(GitHubReleaseTasks, OctopusTasks)"] + HybridAttr["[GitHubEnvironment]
[OctopusProject]"] + SyncTargets["Sync*Targets"] + HttpClient[HttpClient + auth] + ExternalAPI[(GitHub API
Octopus Server API)] + Tasks --> HttpClient + HybridAttr --> SyncTargets --> HttpClient + HttpClient --> ExternalAPI + end + + BuildCs --> CIAttr + BuildCs --> Tasks + BuildCs --> HybridAttr + + style Workflows fill:#e3f2fd + style ExternalAPI fill:#fff3e0 +``` + +Both stacks share the **upper half** — `Build.cs`, the `FalloutBuild` class, the middleware pipeline, reflection-based discovery. They diverge in the **lower half** — file emission for things you commit, REST calls for things that live in someone else's database. + +## Consequences + +### Positive + +- **Two patterns is the right number.** "All attribute" forces fictional artifacts (release JSON files); "all task" loses declarative environment definitions. The split mirrors the real shape of the domain. +- **Test isolation.** REST clients sit behind interfaces (`IGitHubReleaseClient`, `IOctopusClient`); targets calling them are unit-testable with a mock. The CI side doesn't need this because it's already pure-function (POCO → string). +- **Provider symmetry.** Both `Fallout.Common.GitHub` and `Fallout.Common.Octopus` end up the same shape — Tasks class + (optionally) a hybrid attribute + a host class. New providers slot in without touching the build kernel. +- **Composes with v11 middleware.** `IOnTargetSucceeded`, `IOnBuildFinished` hooks can wrap deployment policy (post-Pack release listener, post-deploy notification) — exactly the role v12's plugin SDK extends. +- **Clear lane for the deployment agent.** RFC #113's `fallout-agent` becomes the *executor* invoked by the deployment-task layer, not a parallel concept. The agent doesn't need to understand Octopus or GitHub Releases — it just runs scripts. + +### Negative + +- **Two patterns to teach.** Contributors will sometimes pick the wrong one. The mitigation is naming: `*Tasks` always means runtime imperative, `[*Attribute]` always means declarative metadata. If a contributor reaches for an attribute and finds themselves writing imperative API logic, that's a signal they want a task. +- **Auth model proliferation.** Each provider brings its own auth idioms (GitHub token, Octopus API key, GitLab token, …). We need a single `[Parameter, Secret]` convention and a discipline of "secrets live in attributes' `ImportSecrets`, not in code." +- **State drift.** API-shaped state can diverge from what the build declared (someone edits an environment in the GitHub UI). The sync target is *reconcile-on-demand*, not continuous. Documenting "your `[GitHubEnvironment]` attribute is the source of truth, run `SyncGitHubEnvironments` after edits" is mandatory. +- **The hybrid attribute is the easiest to misuse.** Tempting to keep adding fields to it — `[GitHubEnvironment(Name, Reviewers, WaitTimer, BranchPolicy, Secrets, DeploymentURL, ProtectionRule, RuleSet, …)]`. Cap it at "stable configuration that rarely changes per release." Dynamic things go in tasks. + +## Alternatives considered + +### A. All-attribute, file-everywhere + +Force CD primitives into the existing pattern by inventing files (`.fallout/releases/v1.2.3.json`, `.fallout/environments/production.yaml`) that a separate "applier" CLI reads and POSTs to GitHub/Octopus. + +**Rejected** because the files have no consumers other than the applier — they're make-believe artifacts. Worse, they re-introduce the "config file as source of truth" problem the framework's code-first philosophy explicitly rejects. The author would refactor by editing JSON files, not C#. + +### B. All-task, no declarative + +Drop the hybrid; every provider is just a Tasks class. Environments are managed by writing imperative `UpsertEnvironment(...)` calls in a target. + +**Rejected** because Environments and Octopus Projects *are* configuration. Twelve agent projects written as `await OctopusTasks.UpsertProject("Agent.WebScraper", lifecycle: "..."); await OctopusTasks.UpsertProject("Agent.Indexer", ...);` is a YAML problem in disguise. Lose the ability to inspect "what environments does this build define?" at a glance. + +### C. Adopt an existing client library (Octokit, Octopus.Client) + +Skip the hand-rolled HTTP wrappers; depend on the official client libraries. + +**Deferred.** The hand-rolled `_httpClient.Value.CreateRequest(...)` pattern in `GitHubActions.Client.cs` is small (~30 lines per endpoint) and avoids pulling in a transitive dependency tree per provider. Octokit-specifically also has [historical churn](https://github.com/octokit/octokit.net/issues) that consumers don't want propagating. Revisit if the wrapper surface grows past ~20 endpoints per provider; below that, hand-rolling pays for itself. + +## References + +- RFC [#106](https://github.com/ChrisonSimtian/Fallout/issues/106) — Continuous Delivery vision +- RFC [#113](https://github.com/ChrisonSimtian/Fallout/issues/113) — CD deployment agent +- Existing CI pattern: `src/Fallout.Common/CI/GitHubActions/`, `src/Fallout.Build/CICD/GenerateBuildServerConfigurationsAttribute.cs` +- Existing task wrapper pattern: `src/Fallout.Common/Tools/GitHub/GitHubTasks.cs` +- Existing runtime API client: `src/Fallout.Common/CI/GitHubActions/GitHubActions.Client.cs` +- Architecture context: `docs/architecture.md` diff --git a/docs/adr/0002-cross-provider-auth-and-secret-conventions.md b/docs/adr/0002-cross-provider-auth-and-secret-conventions.md new file mode 100644 index 000000000..03ca4d1fb --- /dev/null +++ b/docs/adr/0002-cross-provider-auth-and-secret-conventions.md @@ -0,0 +1,154 @@ +# ADR-0002 — Cross-provider auth and secret conventions + +- **Status:** Proposed +- **Date:** 2026-05-24 +- **Deciders:** Fallout maintainers +- **Relates to:** [ADR-0001](0001-cd-primitives-attributes-vs-tasks.md), RFC [#106](https://github.com/ChrisonSimtian/Fallout/issues/106), milestone [v12](https://github.com/ChrisonSimtian/Fallout/milestone/7) (plugin SDK), milestone [v13](https://github.com/ChrisonSimtian/Fallout/milestone/8) (CD vision) + +## Context + +[ADR-0001](0001-cd-primitives-attributes-vs-tasks.md) introduces multiple CD providers — first-party GitHub adapters in-tree, third-party adapters (Octopus, GitLab, …) shipped as v12 plugins. Each provider has its own secret store with its own naming conventions, scoping rules, and injection mechanisms. Without a shared convention, every provider invents its own and consumers end up writing the same field three times with three names. + +Fallout already has mature primitives that are scattered and worth surfacing: + +- **`[Parameter]`** (`src/Fallout.Build/ParameterAttribute.cs:34`) — declarative field injection from CLI args + env vars. +- **`[Secret]`** (`src/Fallout.Build/ParameterAttribute.cs:76`) — marker attribute; semantically "do not log this value." Currently a passive marker — actual log-scrubbing behaviour needs explicit framework-side support (see Open Questions). +- **`CredentialStore`** (`src/Fallout.Build/Utilities/CredentialStore.cs`) — macOS Keychain wrapper. Linux/Windows fall back to prompt-only. +- **Encrypted parameters file** — `parameters.json` (+ named profiles `parameters..json`), AES-encrypted with a password held in the Keychain or prompted for. Managed interactively via `dotnet fallout :secrets` (`src/Fallout.GlobalTool/Program.Secrets.cs`). +- **`ImportSecrets`** on `[GitHubActions]` (`src/Fallout.Common/CI/GitHubActions/GitHubActionsAttribute.cs:65`) — declares which `[Parameter, Secret]` fields the workflow should inject from GitHub's secret store, mapped via `SplitCamelHumpsWithKnownWords().JoinUnderscore().ToUpperInvariant()` (line 217). +- **Provider-sourced injection** — `AzureKeyVaultSecretAttribute`, `AppVeyorSecretAttribute` already exist. They're a third category: **runtime-resolved value providers** that look up the secret from an external store on startup, no env-var hop required. + +The drivers for codifying this now: + +1. Octopus 2019 (in-flight as v12 plugin) brings a fourth secret-store flavour (Sensitive Variables) on top of GitHub Actions secrets, encrypted parameters file, and Azure Key Vault. A consistent story across all four is cheaper to establish before the second provider lands than after the fifth. +2. Plugins (v12) introduce a **trust boundary**. A third-party plugin must not be free to invent its own secret-handling shortcut, leak values through logs, or pull from arbitrary stores. The convention is also a security policy. +3. CI/CD asymmetry: CI secrets (registry credentials, signing certs) and CD secrets (deploy keys, target-host credentials) often want different scopes. Today both flow through the same `[Parameter, Secret]` — fine, but the *attribute* should be able to express the difference if a consumer asks for it. + +## Decision + +### Core rules + +1. **The C# field is the canonical identifier.** A secret is declared exactly once, as a `[Parameter, Secret]` field on the build class. Everything else — env-var names, CI-provider lookups, plugin access — derives from this name. + + ```csharp + [Parameter, Secret] readonly string OctopusApiKey; + ``` + +2. **One canonical wire form: `SCREAMING_SNAKE`.** Derived from the C# name via the existing `SplitCamelHumpsWithKnownWords().JoinUnderscore().ToUpperInvariant()` helper (today on `GitHubActionsAttribute.GetSecretValue`, line 217). `OctopusApiKey` → `OCTOPUS_API_KEY` everywhere — GitHub secret name, env-var key, Octopus Variable Set entry, GitLab CI/CD Variable. **Mandatory across providers.** Escape hatch: `[Parameter("CUSTOM_NAME")]` if a downstream system locks you to a non-derivable name (this is exactly why the maintainer's `release.yml` is hand-written today — see `build/Build.CI.GitHubActions.cs:41-46`). + +3. **Resolution order is provider-independent.** When the build starts, every `[Parameter]` is resolved by walking this chain in order, taking the first non-null: + + 1. CLI argument (`-OctopusApiKey `) + 2. Environment variable (`OCTOPUS_API_KEY`) + 3. Value-provider attribute output (e.g. `[AzureKeyVaultSecret(...)]` fetches from Key Vault) + 4. Encrypted parameters file (`parameters.json` / `parameters..json`) + 5. `CredentialStore` lookup (macOS Keychain today; Windows DPAPI / Linux libsecret are open work) + 6. Interactive prompt (only when `Host.IsInteractive` — never in CI) + + Same chain regardless of which CI runs the build. Same chain regardless of whether the provider integration is first-party or plugin. + +4. **Provider attributes handle *injection*, never *storage*.** `[GitHubActions(ImportSecrets = new[] { nameof(OctopusApiKey) })]` writes the env-var injection into the workflow YAML. That's the provider's entire role. The framework's value-injection layer picks it up via step 2 of the chain above. Symmetric for any other CI: `[GitLabPipeline(ImportSecrets = ...)]`, `[AzurePipelines(ImportSecrets = ...)]`, etc. + +5. **Plugins receive resolved values, never raw stores.** The v12 plugin SDK exposes secrets to plugins as `[Parameter, Secret]`-typed values on the build object, already resolved. A plugin **cannot**: + - Read the encrypted parameters file directly. + - Call `CredentialStore.TryGetPassword` (or its successors). + - Add a step-3 value provider that calls out to a network store under its own auth. + + A plugin that needs a new secret declares it as `[Parameter, Secret]` on the build class, and the framework resolves it through the standard chain before the plugin sees it. This is enforced by the SDK surface — `IPlugin` (or equivalent) receives an `IBuildContext` that doesn't expose the raw stores. Plugins authored *for* a remote secret store (a hypothetical `Fallout.Plugin.HashiCorpVault`) can add a value-provider attribute (step 3), but the value still flows through the resolution chain — it does not bypass it. + +6. **Log masking is a framework-level service, not provider-level.** Every value resolved into a `[Secret]`-marked field is registered with a central `SensitiveValueRegistry` at injection time. The logging middleware scrubs registered values from all output streams (stdout, stderr, target logs, build summary) before they're written. **This needs verification** — see Open Questions. + +### Cross-provider mapping + +For a single declaration: + +```csharp +[Parameter, Secret] readonly string OctopusApiKey; +``` + +…the canonical name is `OCTOPUS_API_KEY` (derived once, used everywhere): + +| Surface | Where the value lives | How the build sees it | +|---|---|---| +| **Local dev** | `parameters.json` (encrypted, password in Keychain) | Decrypted at startup, injected by `[Parameter]` | +| **CLI override** | passed inline | `-OctopusApiKey ` or env `OCTOPUS_API_KEY=…` | +| **GitHub Actions** | Repo or environment secret named `OCTOPUS_API_KEY` | `[GitHubActions(ImportSecrets = new[] { nameof(OctopusApiKey) })]` emits `env: OctopusApiKey: ${{ secrets.OCTOPUS_API_KEY }}` | +| **Octopus Deploy** | Sensitive Variable in the project's variable set | `[OctopusProject(..., ImportSecrets = new[] { nameof(OctopusApiKey) })]` emits the variable mapping when project syncs | +| **GitLab CI** | CI/CD Variable (project or group scope), masked + protected | `[GitLabPipeline(ImportSecrets = ...)]` writes the `variables:` block | +| **Azure Pipelines** | Variable group (or pipeline secret) | `[AzurePipelines(ImportSecrets = ...)]` writes the `secret: true` declaration | +| **Azure Key Vault** | Vault secret named `OCTOPUS_API_KEY` (or arbitrary, with explicit `[AzureKeyVaultSecret(SecretName = "...")]`) | Resolved at startup (step 3 of the chain) — no env-var hop | + +One declaration, one canonical name, one resolution chain. Adding a new provider doesn't change the build code — only the attribute on the build class. + +### Anti-patterns + +The convention only works if these stay banned: + +1. **Hardcoded secret in code** — `var key = "abc123";`. Trivially detectable; should fail review. +2. **Secret committed to git in any form except the encrypted parameters file.** That file is encrypted; nothing else is. `.env` files, `secrets.yaml` without `git-crypt`/`sops`, comment-block "for local dev only" — all banned. +3. **Plugin calling its own HTTP client with hard-coded provider auth.** Plugins consume `[Parameter]` values; if a plugin needs Octopus API access, the consuming build declares `[Parameter, Secret] OctopusApiKey` and the plugin reads it via the SDK-exposed build object. +4. **Plaintext CLI arg in CI logs.** Treat `-OctopusApiKey ` as a footgun. CI runs should always go through `ImportSecrets`, never positional args. Local dev can use `--from-stdin` if the value isn't already in the parameters file. +5. **Value-provider attributes that mask intent.** `[AzureKeyVaultSecret] readonly string OctopusApiKey` is fine if the secret really is in Key Vault. It is not fine to use it as a way to silently change the resolution source per environment — that just hides the dependency. +6. **Re-deriving the canonical name in user code.** If you find yourself writing `.ToUpperInvariant().Replace("…", "_")`, you're rebuilding the convention. The framework owns the derivation; consumers reference it via `nameof(MyField)` only. + +## Open questions + +These are decisions the ADR does not close; they need follow-up work or a follow-up ADR. + +- **Log-masking implementation status.** The `[Secret]` attribute exists as a marker; I did not find an explicit `RegisterSensitiveValue` / output scrubber. **Action:** confirm whether masking is wired up (and where), or implement it as a small middleware that hooks `IOnBuildCreated` to register every `[Secret]`-marked value with the logging layer. Tracked under v13 alongside the first CD task work. +- **Windows + Linux credential stores.** `CredentialStore.SavePassword` / `TryGetPassword` only handle `PlatformFamily.OSX`. Windows DPAPI (`ProtectedData`) and Linux libsecret are missing — non-macOS users currently fall back to prompts every run. **Action:** separate issue, not blocking the convention but worth a tracking entry under v12 (since plugin authors on Windows will hit this first). +- **Per-environment scoping.** GitHub Environments and Octopus environments both support env-scoped secrets. The convention above treats secret name as flat. Future: `[GitHubEnvironment("production", ImportSecrets = new[] { nameof(ProdDeployKey) })]` declares the secret as environment-scoped on the GitHub side; the C# field stays one declaration. Not in scope for this ADR; flagged for the CD work to bake in from day one. +- **Secret rotation.** No framework concept today for "this secret should be rotated every N days." Probably never the framework's job (the secret store owns lifecycle) — but worth documenting that the build does not enforce rotation; that's a consumer policy. +- **External secret-manager integrations.** Step 3 of the resolution chain is the extension point for remote stores. Azure Key Vault is the reference implementation today; 1Password, Bitwarden Secrets Manager, HashiCorp Vault, AWS Secrets Manager, GCP Secret Manager, Doppler, Infisical are tracked candidates. Each ships as a v12 plugin, not as in-tree code. Running list at issue [#168](https://github.com/ChrisonSimtian/Fallout/issues/168). + +## Consequences + +### Positive + +- **One mental model.** Declare a `[Parameter, Secret]` field, optionally add it to `ImportSecrets` on the relevant provider attribute. Done. The chain handles everything else. +- **Provider portability.** Switching from GitHub Actions to GitLab (or running the same build locally) requires zero code changes; only the provider attribute changes. +- **Plugin sandbox.** The plugin SDK can present a narrow `IBuildContext` that exposes resolved parameter values but no secret stores. Third-party plugins have a clear, auditable surface. (Critical for the Octopus-plugin-as-validation point in ADR-0001.) +- **Composes with existing infrastructure.** `dotnet fallout :secrets`, the encrypted parameters file, the Keychain integration, and `AzureKeyVaultSecretAttribute` all keep their existing semantics. The ADR codifies what's there + adds the cross-provider naming rule. + +### Negative + +- **Naming inflexibility.** Some consumers will hit a downstream system that *requires* a specific secret name not derivable from PascalCase. They pay for the escape hatch (`[Parameter("custom_name")]`) and an inline comment. Acceptable cost. +- **Plugin authors hit guardrails they may not expect.** "I want to call Vault from my plugin" needs a value-provider attribute, not raw plugin code. This is a feature, not a bug, but needs to be documented in the plugin authoring guide. +- **Linux/Windows local dev is worse than macOS.** Until DPAPI / libsecret support lands, those platforms prompt for the parameters-file password on every run. Surface this in CONTRIBUTING.md so it doesn't surprise contributors. +- **Two attributes for the same secret on the same field.** `[Parameter, Secret] readonly string OctopusApiKey` plus `ImportSecrets = new[] { nameof(OctopusApiKey) }` on the CI attribute. Slightly verbose. The alternative ("the field's `[Secret]` flag auto-imports into all CI providers") is rejected: it makes scope implicit and surprises consumers when a PR-triggered workflow can read a production deploy key. + +## Alternatives considered + +### A. Provider-namespaced Secret attributes (`[GitHubSecret]`, `[OctopusSecret]`) + +Make secret declarations explicitly provider-scoped: `[GitHubSecret] readonly string OctopusApiKey` would only resolve when running under GitHub Actions. + +**Rejected.** The same secret often serves multiple uses — `OctopusApiKey` is a GitHub Actions secret (for CI runs that push to Octopus), an Octopus Sensitive Variable (for deployment-time scripts that call back to Octopus), and a local-dev secret. Provider-namespacing forces multiple declarations or arbitrary "primary provider" choices. The canonical-name approach gets the same effect (one declaration, multiple injection points) without the duplication. + +### B. Plugins manage their own secrets + +Trust plugins to call the credential store / value providers directly. + +**Rejected.** Trust boundary. A plugin that handles its own secrets can leak them through logs, telemetry, or unintended HTTP requests, and the build host has no audit point. Forcing all secret access through the resolution chain gives the framework one place to put masking, audit, and access policy. + +### C. Native provider naming (no normalisation) + +Each provider names its secrets however it wants; the build attribute maps explicitly. `[GitHubActions(ImportSecrets = new Dictionary { { nameof(OctopusApiKey), "OCTO_DEPLOY_KEY" } })]`. + +**Rejected.** Loses the property that consumers can search the codebase for `OctopusApiKey` and find every place it's used. Adds a name-mapping table per provider. The escape hatch (`[Parameter("explicit_name")]`) covers the rare cases where it matters, without forcing every secret through a mapping. + +### D. Adopt a third-party secret library (DotNetEnv, Microsoft.Extensions.Configuration secrets) + +Skip the existing infrastructure; layer on a standard .NET configuration provider. + +**Deferred.** The existing `CredentialStore` + encrypted parameters file is the right shape for build automation specifically — it survives the absence of a host (.NET Generic Host isn't appropriate for a build). The existing system also has a UX (`dotnet fallout :secrets`) that a generic config library wouldn't match. Revisit if maintenance cost rises; otherwise keep. + +## References + +- [ADR-0001](0001-cd-primitives-attributes-vs-tasks.md) — the two CD patterns this layers on +- `src/Fallout.Build/ParameterAttribute.cs:34,76` — `ParameterAttribute` and `SecretAttribute` definitions +- `src/Fallout.Build/Utilities/CredentialStore.cs` — macOS Keychain wrapper +- `src/Fallout.GlobalTool/Program.Secrets.cs` — encrypted parameters file CLI +- `src/Fallout.Common/CI/GitHubActions/GitHubActionsAttribute.cs:217` — canonical name derivation (`SplitCamelHumpsWithKnownWords().JoinUnderscore().ToUpperInvariant()`) +- `src/Fallout.Common/Tools/AzureKeyVault/AzureKeyVaultSecretAttribute.cs` — runtime value-provider precedent (step 3 of the resolution chain) +- RFC [#113](https://github.com/ChrisonSimtian/Fallout/issues/113) — deployment agent (needs the resolution chain to authenticate to coordinators) diff --git a/docs/adr/README.md b/docs/adr/README.md new file mode 100644 index 000000000..49ba91d6c --- /dev/null +++ b/docs/adr/README.md @@ -0,0 +1,36 @@ +# Architecture Decision Records + +This directory holds ADRs — short, dated records of architectural decisions, the context that drove them, and their consequences. Format is [MADR](https://adr.github.io/madr/)-lite: title, status, context, decision, consequences, alternatives. + +## When to write one + +Write an ADR when a decision: + +- Changes the shape of a public API or extension point. +- Picks a pattern that other code is expected to follow (e.g. "all CD primitives use tasks, not file generators"). +- Locks in a non-obvious trade-off that a future contributor would otherwise relitigate. +- Closes out an RFC issue with a concrete commitment. + +Do **not** write an ADR for routine implementation choices, refactors, or bug fixes — commit messages and the PR description carry those. + +## Filename convention + +`NNNN-kebab-case-title.md` — e.g. `0001-cd-primitives-attributes-vs-tasks.md`. Numbers are sequential, never reused. A superseded ADR keeps its number; the new one references it. + +## Status lifecycle + +- **Proposed** — written, under discussion. +- **Accepted** — agreed by maintainers (typically via PR review). +- **Superseded by NNNN** — replaced; the new ADR explains why. +- **Deprecated** — no longer applies, but kept for historical context. + +## Status field is the contract + +If you change a decision, do NOT silently rewrite the old ADR — add a new one and mark the old `Superseded by NNNN`. ADRs are *history*, not living documentation. Living docs go elsewhere in `docs/`. + +## Index + +| # | Title | Status | +|---|---|---| +| [0001](0001-cd-primitives-attributes-vs-tasks.md) | CD primitives — attributes for config, tasks for state | Proposed | +| [0002](0002-cross-provider-auth-and-secret-conventions.md) | Cross-provider auth and secret conventions | Proposed | From f9df8c7fc593016ea9da63debe4290d12e98fe44 Mon Sep 17 00:00:00 2001 From: Chrison Simtian Date: Mon, 25 May 2026 20:40:10 +1200 Subject: [PATCH 2/2] ci: retrigger after #179 (sibling ubuntu-latest workflow)