Skip to content

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

@ChrisonSimtian

Description

@ChrisonSimtian

Two ADRs were drafted in the same pass during early CD planning:

Both are currently 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 mental 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 the inventory-in-static-fields pattern hold up at that scale, or is there pressure for a richer DSL?
  • The Octopus-as-v12-sample-plugin placement — 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 — locking 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 I didn't find an explicit `RegisterSensitiveValue` / output scrubber wired to `[Secret]`. Needs verification before either ADR can move to Accepted. If it's missing, a small framework PR adds it; if it's present, the ADR should cite the file.
  • ⚠️ Windows / Linux `CredentialStore` — macOS-only today. Non-blocking for the ADRs but worth surfacing (already mentioned in ADR-0002's Open Questions); a separate issue should land before plugin authors hit it.
  • Naming escape hatch (`[Parameter("custom_name")]`) — sufficient for the cases that matter, or do we need provider-side mapping dictionaries?

Cross-cutting

  • Are there CD shapes I haven't sketched that would invalidate either ADR? Specifically: K8s-native deployments (Helm releases, ArgoCD applications), serverless deploys (AWS Lambda, Cloudflare Workers), package-manager promotion (NuGet → public feed gates).

Outcome

  • Either ADR → status Accepted via this PR's review, with the date updated.
  • Or one/both → status Superseded by NNNN, with the replacing ADR explaining the divergence.

Do not silently rewrite either ADR. Per the convention in `docs/adr/README.md`, ADRs are history; corrections happen via new ADRs.

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions