Fallout-build / Fallout-build/Fallout

Review ADR-0001 / ADR-0002 — CD primitive patterns + secret conventions

Open
#167 0 comments 0 reactions 0 assignees View on GitHub
enhancement target/vNext
Dominant language
C#
Stars
154
Forks
19
Avg merge
1d 22h
Merged PRs (30d)
15

Description

Two ADRs were drafted in the same pass during early CD planning:
- [ADR-0001 — CD primitives: attributes for config, tasks for state](../blob/main/docs/adr/0001-cd-primitives-attributes-vs-tasks.md)
- [ADR-0002 — Cross-provider auth and secret conventions](../blob/main/docs/adr/0002-cross-provider-auth-and-secret-conventions.md)

Both are **Proposed**. This issue tracks the review pass that promotes them to **Accepted** (or supersedes them with a new ADR).

## What this is
Architectural framing for the v13 CD work, written before substantial code lands. Reviewing now lets the first concrete CD implementation slot into a settled model rather than retrofit one.

## What review should cover

**ADR-0001 — the two patterns split**
- Is the file-shaped vs API-shaped split the right cut? (Alternative: collapse everything to tasks, lose the declarative environment story.)
- Hybrid attribute (`[GitHubEnvironment]`, `[OctopusProject]`) — capped at *stable* config, dynamic stuff in tasks. Is that line defensible, or do we need a sharper rule?
- The monorepo-fan-out sketch (12 agents + 3 UIs + N libraries) — does inventory-in-static-fields hold at that scale, or is there pressure for a richer DSL?
- Octopus-as-v12-sample-plugin — agreed it's the right validation vehicle for the plugin SDK?

**ADR-0002 — the resolution chain + naming**
- ✅ Canonical `SCREAMING_SNAKE` derived from PascalCase field name — lock this in across all providers?
- ✅ Plugins receive resolved values, never raw stores — agreed this is the right trust boundary for v12 SDK?
- ⚠️ **Log masking** — the ADR flags that no explicit `RegisterSensitiveValue` / output scrubber is wired to `[Secret]`. Needs verification before either ADR moves to Accepted. If missing, a small framework PR adds it; if present, the ADR should cite the file.
- ⚠️ **Windows / Linux `CredentialStore`** — macOS-only today. Non-blocking for the ADRs but worth surfacing (in ADR-0002's Open Questions); a separate issue (#180) should land before plugin authors hit it.
- Naming escape hatch (`[Parameter("custom_name")]`) — enough for the cases that matter, or do we need provider-side mapping dictionaries?

**Cross-cutting**
- Any CD shapes not sketched that would invalidate either ADR? Specifically: K8s-native deployments (Helm releases, ArgoCD applications), serverless (AWS Lambda, Cloudflare Workers), package-manager promotion (NuGet → public feed gates).

## Outcome
- Either ADR → **Accepted** via this review, date updated.
- Or one/both → **Superseded by NNNN**, with the replacing ADR explaining the divergence.

Don't silently rewrite either ADR. Per `docs/adr/README.md`, ADRs are history; corrections happen via new ADRs.

## Related
- v13 milestone (this one)
- v12 milestone — plugin SDK consumes both ADRs' decisions
- RFC #106 — CD vision (parent)
- RFC #113 — deployment agent (consumes the ADR-0002 resolution chain)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.