Skip to content

Commit f8c5d7b

Browse files
authored
feat(frost/signing): RFC-21 Phase 5.3 -- readiness-gate env-var guard (#3979)
## Summary **Closes Phase 5 of RFC-21.** Adds the explicit-operator-opt-in gate that protects production builds with the \`frost_roast_retry\` tag from entering the orchestration path without an explicit operator decision. Production builds with the build tag enabled now refuse to wire orchestration unless the operator sets \`KEEP_CORE_FROST_ROAST_RETRY_ENABLED=true\`. The pattern matches the existing \`KEEP_CORE_FROST_TBTC_SIGNER_ACCEPT_SCAFFOLD_KEY_GROUP\` env var from PR #3960: a build tag enables the code path, an env var enables the wiring, both must agree for the feature to be live. Stacked on #3978 (Phase 5.2). ## What lands ### New file: \`pkg/frost/signing/roast_retry_readiness.go\` (untagged) | Surface | Notes | |---|---| | \`RoastRetryReadinessOptInEnvVar\` constant | \`KEEP_CORE_FROST_ROAST_RETRY_ENABLED\` | | \`ErrRoastRetryReadinessOptOut\` sentinel | for \`errors.Is\` detection | | \`EnsureRoastRetryReadinessOptIn() error\` | returns nil if env var is \`"true"\` (case-insensitive, whitespace-trimmed); error otherwise | | \`RoastRetryReadinessOptInEnabled() bool\` | boolean helper | | Per-call (not cached) | operators can flip without restart | ### \`roast_retry_orchestration.go\` extended \`BeginOrchestrationForSession\` now calls \`EnsureRoastRetryReadinessOptIn\` **before** consulting the registry. The env var is the load-bearing gate; missing opt-in short-circuits orchestration even when the registry has a real coordinator. ## Why per-call rather than init-time The scaffold-opt-in env var from PR #3960 is also evaluated per call. Operators can flip it during a debugging session without restart. Same reasoning here -- the readiness env var is a kill switch, not a build constant. ## Test coverage | File | Cases | |---|---| | \`roast_retry_readiness_test.go\` (untagged, 7 cases) | accepts \`"true"\`; case-insensitive; whitespace-trimmed; rejects unset (with sentinel + env-var-name in error); rejects other values; bool helper mirrors error; **env var name matches RFC-21 spec (locks the name)** | | \`roast_retry_orchestration_frost_roast_retry_test.go\` (extended) | New \`TestBeginOrchestrationForSession_ErrorsWhenReadinessOptInUnset\` -- asserts a registered coordinator alone is not enough; missing env var still short-circuits orchestration | ## Verification | Command | Result | |---|---| | \`go build ./...\` | clean | | \`go test ./pkg/frost/signing/...\` | pass (default build) | | \`go test -tags 'frost_roast_retry' ./pkg/frost/signing/...\` | pass | | \`go test -race -tags 'frost_native frost_tbtc_signer frost_roast_retry' ./pkg/frost/...\` | pass (5 packages) | | \`staticcheck -checks '-SA1019' ./pkg/frost/...\` | silent | | \`go vet ./pkg/frost/...\` | clean | | \`gofmt -l ./pkg/frost/signing/\` | silent | ## Phase 5 complete | PR | Scope | State | |---|---|---| | 5.1 (#3977) | Retry adapter scaffolding | open | | 5.2 (#3978) | Orchestration helpers + TTL sweep | open | | **5.3 (this)** | **Readiness-gate env-var guard** | **open** | Phase 6 will wire the production call sites and migrate the three FROST/tbtc-signer receive loops onto the adapter in a single coordinated change. ## Test plan - [ ] CI green. - [ ] Reviewer confirms the case-insensitive / whitespace-trimmed parsing is acceptable. (Alternatives: strict \`"true"\` only.) - [ ] Reviewer confirms the per-call evaluation is acceptable. (Alternatives: read once at init.)
2 parents 1bbe669 + 3af8ba2 commit f8c5d7b

4 files changed

Lines changed: 182 additions & 0 deletions

File tree

pkg/frost/signing/roast_retry_orchestration.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,12 @@ func BeginOrchestrationForSession(
2828
sessionID string,
2929
ctx attempt.AttemptContext,
3030
) (roast.AttemptHandle, func(), error) {
31+
if err := EnsureRoastRetryReadinessOptIn(); err != nil {
32+
return roast.AttemptHandle{}, nil, fmt.Errorf(
33+
"roast orchestration: %w",
34+
err,
35+
)
36+
}
3137
deps, ok := RegisteredRoastRetryCoordinator()
3238
if !ok {
3339
return roast.AttemptHandle{}, nil, fmt.Errorf(

pkg/frost/signing/roast_retry_orchestration_frost_roast_retry_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ func newOrchestrationTestContext(t *testing.T) attempt.AttemptContext {
3131
}
3232

3333
func TestBeginOrchestrationForSession_HappyPath(t *testing.T) {
34+
t.Setenv(RoastRetryReadinessOptInEnvVar, "true")
3435
ResetRoastRetryRegistrationForTest()
3536
ResetSessionHandleRegistryForTest()
3637
t.Cleanup(ResetRoastRetryRegistrationForTest)
@@ -71,11 +72,14 @@ func TestBeginOrchestrationForSession_HappyPath(t *testing.T) {
7172
}
7273

7374
func TestBeginOrchestrationForSession_ErrorsWhenRegistryEmpty(t *testing.T) {
75+
t.Setenv(RoastRetryReadinessOptInEnvVar, "true")
7476
ResetRoastRetryRegistrationForTest()
7577
ResetSessionHandleRegistryForTest()
7678
t.Cleanup(ResetRoastRetryRegistrationForTest)
7779
t.Cleanup(ResetSessionHandleRegistryForTest)
7880

81+
// Readiness env var is set; the registry is empty -- we expect
82+
// the registry-empty error, not the env-var error.
7983
_, _, err := BeginOrchestrationForSession("session-X", newOrchestrationTestContext(t))
8084
if err == nil {
8185
t.Fatal("expected error when registry is empty")
@@ -85,7 +89,35 @@ func TestBeginOrchestrationForSession_ErrorsWhenRegistryEmpty(t *testing.T) {
8589
}
8690
}
8791

92+
func TestBeginOrchestrationForSession_ErrorsWhenReadinessOptInUnset(t *testing.T) {
93+
// Explicitly unset, in case the test runner inherits the env var
94+
// from outside.
95+
t.Setenv(RoastRetryReadinessOptInEnvVar, "")
96+
ResetRoastRetryRegistrationForTest()
97+
ResetSessionHandleRegistryForTest()
98+
t.Cleanup(ResetRoastRetryRegistrationForTest)
99+
t.Cleanup(ResetSessionHandleRegistryForTest)
100+
101+
// Even with a registered coordinator, the readiness env var
102+
// short-circuits orchestration. This is the load-bearing safety
103+
// property: production builds with the frost_roast_retry tag
104+
// still cannot enter the orchestration path without an explicit
105+
// operator decision.
106+
RegisterRoastRetryCoordinator(RoastRetryDeps{
107+
Coordinator: roast.NewInMemoryCoordinator(),
108+
Signer: roast.NoOpSigner(),
109+
Verifier: roast.NoOpSignatureVerifier(),
110+
SelfMember: 1,
111+
})
112+
113+
_, _, err := BeginOrchestrationForSession("session-no-optin", newOrchestrationTestContext(t))
114+
if !errors.Is(err, ErrRoastRetryReadinessOptOut) {
115+
t.Fatalf("expected ErrRoastRetryReadinessOptOut, got %v", err)
116+
}
117+
}
118+
88119
func TestBeginOrchestrationForSession_ErrorsWhenCoordinatorNil(t *testing.T) {
120+
t.Setenv(RoastRetryReadinessOptInEnvVar, "true")
89121
ResetRoastRetryRegistrationForTest()
90122
ResetSessionHandleRegistryForTest()
91123
t.Cleanup(ResetRoastRetryRegistrationForTest)
@@ -108,6 +140,7 @@ func TestBeginOrchestrationForSession_ErrorsWhenCoordinatorNil(t *testing.T) {
108140
}
109141

110142
func TestBeginOrchestrationForSession_PropagatesBeginAttemptError(t *testing.T) {
143+
t.Setenv(RoastRetryReadinessOptInEnvVar, "true")
111144
ResetRoastRetryRegistrationForTest()
112145
ResetSessionHandleRegistryForTest()
113146
t.Cleanup(ResetRoastRetryRegistrationForTest)
@@ -213,6 +246,7 @@ func TestStartSessionHandleSweeper_IsIdempotent(t *testing.T) {
213246
}
214247

215248
func TestRegisterRoastRetryCoordinator_StartsSweeper(t *testing.T) {
249+
t.Setenv(RoastRetryReadinessOptInEnvVar, "true")
216250
ResetRoastRetryRegistrationForTest()
217251
ResetSessionHandleRegistryForTest()
218252
t.Cleanup(ResetRoastRetryRegistrationForTest)
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
package signing
2+
3+
import (
4+
"errors"
5+
"fmt"
6+
"os"
7+
"strings"
8+
)
9+
10+
// RoastRetryReadinessOptInEnvVar is the environment variable name
11+
// operators must set to "true" to opt in to RFC-21 ROAST retry
12+
// activation. The variable is read per call -- not cached -- so an
13+
// operator can flip it during a debugging session without
14+
// restarting the node.
15+
//
16+
// Pattern matches the existing
17+
// KEEP_CORE_FROST_TBTC_SIGNER_ACCEPT_SCAFFOLD_KEY_GROUP env var
18+
// from PR #3960: a build tag enables the code path, an env var
19+
// enables the wiring, both must agree for the feature to be live.
20+
const RoastRetryReadinessOptInEnvVar = "KEEP_CORE_FROST_ROAST_RETRY_ENABLED"
21+
22+
// ErrRoastRetryReadinessOptOut is the error
23+
// EnsureRoastRetryReadinessOptIn returns when the env var is unset
24+
// or set to anything other than "true". Use errors.Is to detect.
25+
var ErrRoastRetryReadinessOptOut = errors.New(
26+
"roast retry readiness: operator opt-in env var is not set to true",
27+
)
28+
29+
// EnsureRoastRetryReadinessOptIn reads the
30+
// RoastRetryReadinessOptInEnvVar environment variable and returns
31+
// nil if it is set to the string "true" (case-insensitive,
32+
// whitespace-trimmed). Returns ErrRoastRetryReadinessOptOut
33+
// otherwise.
34+
//
35+
// Callers in the orchestration layer invoke this before
36+
// RegisterRoastRetryCoordinator so production builds with the
37+
// frost_roast_retry build tag still refuse to wire orchestration
38+
// without an explicit operator decision.
39+
//
40+
// The function is per-call (not cached) so operators can flip the
41+
// env var dynamically during debugging.
42+
func EnsureRoastRetryReadinessOptIn() error {
43+
if !RoastRetryReadinessOptInEnabled() {
44+
return fmt.Errorf(
45+
"%w: set %s=true to enable",
46+
ErrRoastRetryReadinessOptOut,
47+
RoastRetryReadinessOptInEnvVar,
48+
)
49+
}
50+
return nil
51+
}
52+
53+
// RoastRetryReadinessOptInEnabled reports whether the readiness
54+
// env var is currently set to "true". Cheap to call; use this when
55+
// you need a boolean (e.g., to gate a log message) and
56+
// EnsureRoastRetryReadinessOptIn when you need an error.
57+
func RoastRetryReadinessOptInEnabled() bool {
58+
value := strings.TrimSpace(os.Getenv(RoastRetryReadinessOptInEnvVar))
59+
return strings.EqualFold(value, "true")
60+
}
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
package signing
2+
3+
import (
4+
"errors"
5+
"strings"
6+
"testing"
7+
)
8+
9+
func TestEnsureRoastRetryReadinessOptIn_AcceptsTrue(t *testing.T) {
10+
t.Setenv(RoastRetryReadinessOptInEnvVar, "true")
11+
if err := EnsureRoastRetryReadinessOptIn(); err != nil {
12+
t.Fatalf("expected nil error, got %v", err)
13+
}
14+
}
15+
16+
func TestEnsureRoastRetryReadinessOptIn_AcceptsTrueCaseInsensitive(t *testing.T) {
17+
cases := []string{"true", "True", "TRUE", "tRuE"}
18+
for _, value := range cases {
19+
t.Run(value, func(t *testing.T) {
20+
t.Setenv(RoastRetryReadinessOptInEnvVar, value)
21+
if err := EnsureRoastRetryReadinessOptIn(); err != nil {
22+
t.Fatalf("expected nil error for %q, got %v", value, err)
23+
}
24+
})
25+
}
26+
}
27+
28+
func TestEnsureRoastRetryReadinessOptIn_AcceptsTrimmedWhitespace(t *testing.T) {
29+
t.Setenv(RoastRetryReadinessOptInEnvVar, " true ")
30+
if err := EnsureRoastRetryReadinessOptIn(); err != nil {
31+
t.Fatalf("expected nil error for whitespace-padded 'true', got %v", err)
32+
}
33+
}
34+
35+
func TestEnsureRoastRetryReadinessOptIn_RejectsUnset(t *testing.T) {
36+
t.Setenv(RoastRetryReadinessOptInEnvVar, "")
37+
err := EnsureRoastRetryReadinessOptIn()
38+
if !errors.Is(err, ErrRoastRetryReadinessOptOut) {
39+
t.Fatalf("expected ErrRoastRetryReadinessOptOut, got %v", err)
40+
}
41+
if !strings.Contains(err.Error(), RoastRetryReadinessOptInEnvVar) {
42+
t.Fatalf(
43+
"error must mention the env var name to guide operators; got %v",
44+
err,
45+
)
46+
}
47+
}
48+
49+
func TestEnsureRoastRetryReadinessOptIn_RejectsOtherValues(t *testing.T) {
50+
cases := []string{"false", "1", "yes", "TRUE_", "tru", "anything"}
51+
for _, value := range cases {
52+
t.Run(value, func(t *testing.T) {
53+
t.Setenv(RoastRetryReadinessOptInEnvVar, value)
54+
err := EnsureRoastRetryReadinessOptIn()
55+
if !errors.Is(err, ErrRoastRetryReadinessOptOut) {
56+
t.Fatalf("expected error for %q, got nil", value)
57+
}
58+
})
59+
}
60+
}
61+
62+
func TestRoastRetryReadinessOptInEnabled_MirrorsEnsureResult(t *testing.T) {
63+
t.Setenv(RoastRetryReadinessOptInEnvVar, "true")
64+
if !RoastRetryReadinessOptInEnabled() {
65+
t.Fatal("expected true when env var set to true")
66+
}
67+
t.Setenv(RoastRetryReadinessOptInEnvVar, "false")
68+
if RoastRetryReadinessOptInEnabled() {
69+
t.Fatal("expected false when env var set to false")
70+
}
71+
}
72+
73+
func TestRoastRetryReadinessOptInEnvVar_MatchesRFC(t *testing.T) {
74+
const expected = "KEEP_CORE_FROST_ROAST_RETRY_ENABLED"
75+
if RoastRetryReadinessOptInEnvVar != expected {
76+
t.Fatalf(
77+
"env var name drifted: got %q want %q (must match RFC-21 Phase 5)",
78+
RoastRetryReadinessOptInEnvVar,
79+
expected,
80+
)
81+
}
82+
}

0 commit comments

Comments
 (0)