Skip to content

Commit ea5231f

Browse files
feat(featureflag): OpenFeature + flagd wrapper, fail-closed (P1) (#43)
* feat(featureflag): OpenFeature + flagd wrapper with fail-CLOSED defaults P1 of the feature-flag stack (ADR docs/sessions/2026-06-04/ADR-feature-flags.md). A common/featureflag package shaped like storageprovider/queueprovider: - Provider interface (BoolEnabled/etc.) + EvalContext (targeting key + teamID/ tier/env) for api/worker/provisioner to share one flag source. - Fail CLOSED: every eval returns the caller's default on ANY error (provider down, missing flag, sync failure, nil ctx) — the deliberate inverse of the "fail open on Redis" rule; unbuilt features default OFF. - Factory selects backend by config: `static` (in-memory, default — no network, CI-safe) and `flagd` (OpenFeature flagd resolver). Unknown/unregistered/ construction-failure/nil-provider all DEGRADE TO STATIC, never error the service. - Registry-iterating contract test (TestRegistry_AllBackendsFailClosed) asserts every backend fails closed; REGISTRY.md documents flag-hygiene convention. Coverage: featureflag 98.6% / flagd 97.4% / static 100%. Not yet wired into services (per-service PR follows; api #245 Team gate refactor next). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(featureflag): drop flagd backend from P1 to clear GO-2026-4279 The bundled flagd backend pulled github.com/open-feature/flagd/core@v0.11.2 (GO-2026-4279, multiple Go-runtime CVEs; fixed in v0.13.1), failing govulncheck + osv-scan. The flagd server isn't deployed yet anyway, so the backend impl is premature. P1 now ships only the interface + static provider + factory (config 'flagd' degrades to static, fail-closed) + the registry-iterating fail-closed contract test — zero vulnerable deps. flagd backend returns in a follow-up PR alongside the flagd deployment, pinned to a patched flagd/core. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(featureflag): cover empty-allowlist-entry skip + StringVariant type-mismatch Closes the last 2 uncovered branches in static_core.go (matches() empty-id continue; StringVariant type-assertion guard) → featureflag pkg 100% of statements, satisfying the 100%-patch-coverage gate. Both are fail-closed proofs: an empty allowlist entry must not match an empty context, and a type-mismatched string read returns the caller default + ErrTypeMismatch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 14b7d00 commit ea5231f

14 files changed

Lines changed: 1397 additions & 7 deletions

featureflag/contract_test.go

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
package featureflag_test
2+
3+
// contract_test.go — registry-iterating contract test for the feature-flag
4+
// abstraction (CLAUDE.md rule 18).
5+
//
6+
// Every backend registers itself with the global registry at package-init via
7+
// featureflag.Register(name, builder). This test iterates the LIVE registry
8+
// rather than a hand-typed slice, so a third backend added later is
9+
// automatically held to the same fail-closed contract.
10+
//
11+
// THE central invariant (the inverse of the repo's "fail open on Redis" rule):
12+
// every backend MUST return the caller-supplied DEFAULT on every failure mode —
13+
// - the flag is missing
14+
// - the underlying provider errors
15+
// - the eval context is nil/empty (no targeting info)
16+
// - the context is cancelled / nil
17+
// Failing closed means an unbuilt feature is OFF for everyone unless explicitly
18+
// targeted on. This file is the load-bearing proof of that guarantee.
19+
20+
import (
21+
"context"
22+
"testing"
23+
24+
"github.com/stretchr/testify/assert"
25+
"github.com/stretchr/testify/require"
26+
27+
"instant.dev/common/featureflag"
28+
29+
// side-effect imports register each backend with the registry.
30+
// NOTE: the flagd backend is deferred to a follow-up PR (it ships with the
31+
// flagd deployment and pulls the OpenFeature+flagd dep tree, which carries
32+
// GO-2026-4279 until flagd/core >= v0.13.1). Config "flagd" currently
33+
// degrades to the static backend (fail-closed) via the factory. Until then
34+
// the registry holds only `static`, and the contract below still iterates it.
35+
_ "instant.dev/common/featureflag/static"
36+
)
37+
38+
// configForBackend returns the minimum Config to construct each backend such
39+
// that it has NO usable flag source — so every evaluation MUST fall back to the
40+
// caller default (static: supply no flags). Must fail closed.
41+
func configForBackend(name string) featureflag.Config {
42+
return featureflag.Config{Backend: name}
43+
}
44+
45+
// TestRegistry_AllBackendsFailClosed is the rule-18 contract test. It iterates
46+
// every registered backend and asserts the fail-closed guarantee across all the
47+
// failure surfaces, for all three typed accessors.
48+
func TestRegistry_AllBackendsFailClosed(t *testing.T) {
49+
registered := featureflag.ListRegistered()
50+
require.GreaterOrEqual(t, len(registered), 1,
51+
"expected at least 1 backend registered (static; flagd deferred to a follow-up PR); got %v", registered)
52+
53+
for _, name := range registered {
54+
name := name
55+
t.Run(name, func(t *testing.T) {
56+
p, err := featureflag.Factory(configForBackend(name))
57+
// Factory NEVER hard-errors (it degrades to static); err is advisory.
58+
require.NotNil(t, p, "Factory(%q) returned nil provider", name)
59+
t.Cleanup(func() { _ = p.Close() })
60+
_ = err
61+
62+
ctx := context.Background()
63+
64+
// --- missing flag: every accessor returns its caller default ---
65+
t.Run("missing_flag", func(t *testing.T) {
66+
gotBool, _ := p.BoolVariant(ctx, "no_such_flag", true, featureflag.EvalContext{})
67+
assert.True(t, gotBool, "%s: missing bool flag must return caller default (true)", name)
68+
gotBool2, _ := p.BoolVariant(ctx, "no_such_flag", false, featureflag.EvalContext{})
69+
assert.False(t, gotBool2, "%s: missing bool flag must return caller default (false)", name)
70+
71+
gotStr, _ := p.StringVariant(ctx, "no_such_flag", "DEFAULT", featureflag.EvalContext{})
72+
assert.Equal(t, "DEFAULT", gotStr, "%s: missing string flag must return caller default", name)
73+
74+
gotInt, _ := p.IntVariant(ctx, "no_such_flag", 42, featureflag.EvalContext{})
75+
assert.Equal(t, int64(42), gotInt, "%s: missing int flag must return caller default", name)
76+
})
77+
78+
// --- nil / empty eval context: still returns default ---
79+
t.Run("empty_eval_context", func(t *testing.T) {
80+
got, _ := p.BoolVariant(ctx, "feature_team_billing", false, featureflag.EvalContext{})
81+
assert.False(t, got, "%s: empty eval context must not enable a gated feature", name)
82+
83+
gotNilAttrs, _ := p.BoolVariant(ctx, "feature_team_billing", false, featureflag.EvalContext{
84+
Attributes: nil,
85+
})
86+
assert.False(t, gotNilAttrs, "%s: nil attributes must not enable a gated feature", name)
87+
})
88+
89+
// --- nil context: must fail closed, never panic ---
90+
t.Run("nil_context", func(t *testing.T) {
91+
//nolint:staticcheck // SA1012: deliberately passing nil ctx to prove fail-closed
92+
got, err := p.BoolVariant(nil, "anything", false, featureflag.EvalContext{})
93+
assert.False(t, got, "%s: nil context must return caller default", name)
94+
assert.Error(t, err, "%s: nil context should surface an advisory error", name)
95+
})
96+
97+
// --- cancelled context: must fail closed ---
98+
t.Run("cancelled_context", func(t *testing.T) {
99+
cctx, cancel := context.WithCancel(context.Background())
100+
cancel()
101+
got, err := p.BoolVariant(cctx, "anything", false, featureflag.EvalContext{})
102+
assert.False(t, got, "%s: cancelled context must return caller default", name)
103+
assert.Error(t, err, "%s: cancelled context should surface an advisory error", name)
104+
})
105+
106+
// --- Name + Close contract ---
107+
t.Run("name_and_close", func(t *testing.T) {
108+
assert.NotEmpty(t, p.Name(), "%s: Name() must be non-empty", name)
109+
assert.NoError(t, p.Close(), "%s: Close() must be a clean no-op", name)
110+
assert.NoError(t, p.Close(), "%s: Close() must be idempotent", name)
111+
})
112+
})
113+
}
114+
}
115+
116+
// TestFactory_UnknownBackendDegradesToStatic verifies the DELIBERATE difference
117+
// from storageprovider/queueprovider: an unknown backend does NOT hard-fail the
118+
// service. It degrades to the static (fail-closed) backend and returns an
119+
// advisory error — because a flag system that refuses to boot is strictly worse
120+
// than one serving OFF defaults.
121+
func TestFactory_UnknownBackendDegradesToStatic(t *testing.T) {
122+
p, err := featureflag.Factory(featureflag.Config{Backend: "made-up-backend"})
123+
require.NotNil(t, p, "unknown backend must still yield a usable provider")
124+
assert.ErrorIs(t, err, featureflag.ErrUnknownBackend, "should surface advisory ErrUnknownBackend")
125+
// The degraded provider must serve fail-closed defaults.
126+
got, _ := p.BoolVariant(context.Background(), "anything", false, featureflag.EvalContext{})
127+
assert.False(t, got, "degraded provider must fail closed")
128+
_ = p.Close()
129+
}
130+
131+
// TestNormalizeBackend covers the alias table — the SUT is the table itself, so
132+
// this is hand-typed (rule 18's carve-out: the table IS the registry here).
133+
func TestNormalizeBackend(t *testing.T) {
134+
cases := map[string]string{
135+
"": featureflag.BackendStatic, // empty defaults to static
136+
"static": featureflag.BackendStatic,
137+
"STATIC": featureflag.BackendStatic,
138+
"memory": featureflag.BackendStatic,
139+
"in-memory": featureflag.BackendStatic,
140+
"inmem": featureflag.BackendStatic,
141+
"file": featureflag.BackendStatic,
142+
"flagd": featureflag.BackendFlagd,
143+
"openfeature": featureflag.BackendFlagd,
144+
"open-feature": featureflag.BackendFlagd,
145+
"grpc": featureflag.BackendFlagd,
146+
"nonsense": "",
147+
}
148+
for in, want := range cases {
149+
assert.Equal(t, want, featureflag.NormalizeBackend(in), "NormalizeBackend(%q)", in)
150+
}
151+
}

featureflag/errors.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
package featureflag
2+
3+
import "fmt"
4+
5+
// errPanic wraps a recovered panic value as an error so the fail-closed
6+
// wrapper can return it alongside the caller default. The value is never
7+
// re-panicked — a misbehaving provider must NEVER crash a request path; the
8+
// feature simply reads as its default (OFF).
9+
func errPanic(r any) error {
10+
if err, ok := r.(error); ok {
11+
return fmt.Errorf("featureflag: provider panicked: %w", err)
12+
}
13+
return fmt.Errorf("featureflag: provider panicked: %v", r)
14+
}

featureflag/factory.go

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
package featureflag
2+
3+
import (
4+
"fmt"
5+
"strings"
6+
)
7+
8+
// Canonical backend identifiers. These are the strings every layer (api,
9+
// worker, provisioner, k8s ConfigMaps) compares against.
10+
const (
11+
// BackendStatic is the pure in-memory backend (default). Reads a flag
12+
// definition map / file; NO network; tests + CI need no running flagd.
13+
BackendStatic = "static"
14+
15+
// BackendFlagd is the OpenFeature SDK + flagd backend (gRPC streaming).
16+
// IMPL DEFERRED to a follow-up PR (ships with the flagd deployment; its
17+
// OpenFeature+flagd dep tree carries GO-2026-4279 until flagd/core >=
18+
// v0.13.1). Until registered, config "flagd" degrades to BackendStatic
19+
// (fail-closed) via the factory — same safety, no vulnerable deps in-tree.
20+
BackendFlagd = "flagd"
21+
)
22+
23+
// Config is the operator-facing configuration for the feature-flag backend.
24+
// The api / worker / provisioner wire this from env vars
25+
// (FEATURE_FLAG_BACKEND + per-backend knobs) and pass it to Factory() at boot.
26+
type Config struct {
27+
// Backend selects the implementation. One of "static", "flagd". Aliases
28+
// ("memory"/"inmem"/"file" -> "static"; "openfeature"/"grpc" -> "flagd").
29+
// Empty defaults to "static" — the safest, dependency-free backend.
30+
Backend string
31+
32+
// StaticFlags is the in-memory flag definition consumed by the static
33+
// backend. Keyed by flag key. When both StaticFlags and StaticFilePath are
34+
// empty the static backend serves an empty flag set (every flag => default,
35+
// i.e. fully fail-closed). Ignored by the flagd backend.
36+
StaticFlags map[string]StaticFlag
37+
38+
// StaticFilePath, when set, loads the static flag definition from a
39+
// flagd-format JSON file at boot (the in-cluster ConfigMap mount path).
40+
// StaticFlags wins over StaticFilePath on key collision. Ignored by flagd.
41+
StaticFilePath string
42+
43+
// Flagd-specific. Host + Port of the flagd gRPC endpoint. Defaults:
44+
// host "localhost", port 8013 (flagd rpc resolver default).
45+
FlagdHost string
46+
FlagdPort uint16
47+
48+
// FlagdInProcess selects the in-process resolver (flagd syncs the flag set
49+
// over gRPC and evaluates locally — lowest latency) instead of the default
50+
// rpc resolver (each eval is a gRPC round-trip). Both are sub-second.
51+
FlagdInProcess bool
52+
}
53+
54+
// NormalizeBackend maps the operator-facing value (with historical aliases)
55+
// onto one of the canonical backend strings. An unrecognised non-empty value
56+
// returns "" so Factory can decide how to degrade.
57+
func NormalizeBackend(raw string) string {
58+
switch strings.ToLower(strings.TrimSpace(raw)) {
59+
case "", "static", "memory", "in-memory", "inmem", "file":
60+
return BackendStatic
61+
case "flagd", "openfeature", "open-feature", "grpc":
62+
return BackendFlagd
63+
default:
64+
return ""
65+
}
66+
}
67+
68+
// Factory selects and constructs the right [Provider] for cfg, ALREADY WRAPPED
69+
// by [Wrap] so the returned provider is guaranteed fail-closed.
70+
//
71+
// Unlike storageprovider/queueprovider — where an unknown backend hard-fails so
72+
// a service never silently degrades to a less-secure store — featureflag
73+
// degrades DELIBERATELY: an unbuilt feature defaulting OFF is the SAFE state, so
74+
// if the requested backend is unknown or its construction fails, Factory falls
75+
// back to the static backend (fail-closed defaults) and returns a nil error.
76+
// A flag system that refuses to boot would take the whole service down; that is
77+
// strictly worse than serving defaults. The returned error is non-nil only as
78+
// an ADVISORY (so the caller can log/alert that it degraded) — the provider is
79+
// always usable.
80+
func Factory(cfg Config) (Provider, error) {
81+
name := NormalizeBackend(cfg.Backend)
82+
if name == "" {
83+
// Unknown backend: degrade to static, surface advisory error.
84+
p, _ := newStatic(cfg)
85+
return Wrap(p), fmt.Errorf("%w: %q (degraded to static, fail-closed defaults)", ErrUnknownBackend, cfg.Backend)
86+
}
87+
88+
ctor, ok := lookupBuilder(name)
89+
if !ok {
90+
// Backend recognised but its impl package wasn't imported (e.g. flagd
91+
// excluded from a slim build). Degrade to static.
92+
p, _ := newStatic(cfg)
93+
return Wrap(p), fmt.Errorf("featureflag: backend %q not registered — did you import the impl package? (degraded to static)", name)
94+
}
95+
96+
p, err := ctor(cfg)
97+
if err != nil || p == nil {
98+
// Construction failed (flagd unreachable at boot, bad config, ...).
99+
// Degrade to static rather than failing the service.
100+
sp, _ := newStatic(cfg)
101+
return Wrap(sp), fmt.Errorf("featureflag: backend %q failed to construct (%v) — degraded to static", name, err)
102+
}
103+
return Wrap(p), nil
104+
}
105+
106+
// Builder is the constructor signature each backend registers via Register from
107+
// its package init(). Keeping flagd's OpenFeature + gRPC transitive deps in a
108+
// subpackage means `common` consumers that only use the static backend don't
109+
// pull the flagd SDK into their import graph (same pattern as queueprovider).
110+
type Builder func(cfg Config) (Provider, error)
111+
112+
var builders = map[string]Builder{}
113+
114+
// Register adds a Builder under name. Called from each backend package's
115+
// init(). Idempotent — a second registration with the same name overwrites the
116+
// first (used in tests to inject a fake).
117+
func Register(name string, b Builder) {
118+
builders[NormalizeBackend(name)] = b
119+
}
120+
121+
func lookupBuilder(name string) (Builder, bool) {
122+
b, ok := builders[name]
123+
return b, ok
124+
}
125+
126+
// ListRegistered returns the names of every backend currently registered. Used
127+
// by the registry-iterating contract test (CLAUDE.md rule 18).
128+
func ListRegistered() []string {
129+
out := make([]string, 0, len(builders))
130+
for k := range builders {
131+
out = append(out, k)
132+
}
133+
return out
134+
}
135+
136+
// newStatic is the in-package fallback constructor used by Factory's degrade
137+
// paths. It does NOT depend on the static subpackage being imported, so the
138+
// fail-closed fallback works even in a build that never imported any backend.
139+
// It builds the same concrete provider the static subpackage registers.
140+
func newStatic(cfg Config) (Provider, error) {
141+
return buildStatic(cfg)
142+
}
143+
144+
// NewStaticBuilder returns the [Builder] for the in-memory static backend. The
145+
// featureflag/static subpackage calls this from its init() to register the
146+
// backend under [BackendStatic]. Exported (rather than registering from the
147+
// root package directly) so a slim build that never imports the static
148+
// subpackage keeps an empty registry, while Factory's degrade path still has a
149+
// hard fallback via buildStatic.
150+
func NewStaticBuilder() Builder {
151+
return buildStatic
152+
}

0 commit comments

Comments
 (0)