From c20e5110bf2277a83e648b310d2bb1d441e95e6a Mon Sep 17 00:00:00 2001 From: Vishal Vaibhav <168088244+vishalv1994@users.noreply.github.com> Date: Mon, 1 Jun 2026 23:09:08 +0530 Subject: [PATCH 1/7] licensepb: add Feature enum and License.features field Add a top-level Feature enum and a repeated Feature features field (number 12, wire-compatible with the managed-service proto's 8-11) so a license can carry the set of entitlements it grants. This is the first building block for license-aware feature gates. Co-Authored-By: Claude Opus 4.7 --- pkg/server/license/licensepb/license.proto | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/pkg/server/license/licensepb/license.proto b/pkg/server/license/licensepb/license.proto index 3ae1e5643a03..3bafac77000a 100644 --- a/pkg/server/license/licensepb/license.proto +++ b/pkg/server/license/licensepb/license.proto @@ -9,6 +9,21 @@ option go_package = "github.com/cockroachdb/cockroach/pkg/server/license/license import "gogoproto/gogo.proto"; +// Feature enumerates the entitlements a license can grant. A license carries +// the subset of these features that the holder is permitted to use, allowing +// the binary to gate functionality based on what the customer has purchased. +enum Feature { + FEATURE_UNSPECIFIED = 0; + CHANGEFEED = 1; + BACKUP = 2; + RESTORE = 3; + EXPORT = 4; + IMPORT = 5; + MULTIREGION = 6; + LDR_BIDIRECTIONAL = 7; + PCR_MULTIREGION = 8; +} + message License { reserved 1; int64 valid_until_unix_sec = 2; @@ -43,4 +58,9 @@ message License { // dependencies, as the generated code is also used in other repositories. bytes license_id = 6; bytes organization_id = 7; + + // features lists the entitlements this license grants its holder. Field + // number 12 is chosen to stay wire-compatible with the managed-service + // proto, which already uses field numbers 8 through 11. + repeated Feature features = 12; } From 07a01184dcbeb599e2e5b5077009f6d09b28b626 Mon Sep 17 00:00:00 2001 From: Vishal Vaibhav <168088244+vishalv1994@users.noreply.github.com> Date: Mon, 1 Jun 2026 23:14:00 +0530 Subject: [PATCH 2/7] featureflag: add license-aware FeatureGate (Register/Enabled) Add a FeatureGate type that combines a license-entitlement check (read directly from the license proto via a hook) with an optional operator cluster setting, plus experimental/cloud-only stubs. This is the core of the license-aware feature-gate prototype; existing call sites are not yet migrated. Co-Authored-By: Claude Opus 4.7 --- pkg/featureflag/BUILD.bazel | 8 +- pkg/featureflag/feature_gate.go | 180 ++++++++++++++++++++++++++++++++ 2 files changed, 187 insertions(+), 1 deletion(-) create mode 100644 pkg/featureflag/feature_gate.go diff --git a/pkg/featureflag/BUILD.bazel b/pkg/featureflag/BUILD.bazel index cca656d94c94..bd87dd15e3ed 100644 --- a/pkg/featureflag/BUILD.bazel +++ b/pkg/featureflag/BUILD.bazel @@ -2,16 +2,22 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( name = "featureflag", - srcs = ["feature_flags.go"], + srcs = [ + "feature_flags.go", + "feature_gate.go", + ], importpath = "github.com/cockroachdb/cockroach/pkg/featureflag", visibility = ["//visibility:public"], deps = [ + "//pkg/server/license/licensepb", "//pkg/server/telemetry", "//pkg/settings", + "//pkg/settings/cluster", "//pkg/sql/pgwire/pgcode", "//pkg/sql/pgwire/pgerror", "//pkg/sql/sqltelemetry", "//pkg/util/log", "//pkg/util/metric", + "@com_github_cockroachdb_errors//:errors", ], ) diff --git a/pkg/featureflag/feature_gate.go b/pkg/featureflag/feature_gate.go new file mode 100644 index 000000000000..db4ac5e4298b --- /dev/null +++ b/pkg/featureflag/feature_gate.go @@ -0,0 +1,180 @@ +// Copyright 2026 The Cockroach Authors. +// +// Use of this software is governed by the CockroachDB Software License +// included in the /LICENSE file. + +package featureflag + +import ( + "context" + "slices" + + "github.com/cockroachdb/cockroach/pkg/server/license/licensepb" + "github.com/cockroachdb/cockroach/pkg/server/telemetry" + "github.com/cockroachdb/cockroach/pkg/settings" + "github.com/cockroachdb/cockroach/pkg/settings/cluster" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" + "github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry" + "github.com/cockroachdb/errors" +) + +// FeatureGate combines a license-entitlement check with an optional operator +// cluster setting (and stubs for experimental/cloud-only gating) into a single +// enforcement point for a feature. +// +// The license-entitlement check reads the installed license proto directly (via +// GetLicenseHook) and asks whether the gate's feature is among the entitlements +// the license grants. This deliberately avoids relying on denormalized cluster +// settings so that entitlements remain a single source of truth. +// +// The operator setting, when supplied via WithSetting, is one of the existing +// feature.*.enabled cluster-setting bools. The gate does not register a new +// setting; it reuses the one the caller already owns, so that operators retain +// the ability to turn a feature off even when the license permits it. +// +// Lifecycle: a FeatureGate is constructed once, typically in a package-level +// var via Register, and is immutable thereafter. It is evaluated per-use by +// calling Enabled, which performs the ordered license-then-operator checks. +type FeatureGate struct { + // feature is the license entitlement this gate guards. + feature licensepb.Feature + + // setting is the operator cluster setting that can additionally disable + // this feature, or nil if the gate has no operator setting. + setting *settings.BoolSetting + + // experimental marks the gate as experimental. This is currently a stub: + // Enabled does not yet enforce any experimental semantics. + experimental bool + + // cloudOnly marks the gate as available only in cloud deployments. This is + // currently a stub: Enabled does not yet enforce any cloud-only semantics. + cloudOnly bool + + // name is the human-readable name used in error messages. It defaults to + // the feature's enum String() when WithName is not supplied. + name string +} + +// Option configures a FeatureGate during construction. Options follow the +// functional-options pattern and are applied in order by Register. +type Option func(*FeatureGate) + +// Register constructs a FeatureGate for the given license feature, applies the +// supplied options, and returns it. It is typically called once to initialize a +// package-level var. If no WithName option is supplied, the gate's display name +// defaults to the feature's enum String(). +func Register(feature licensepb.Feature, opts ...Option) *FeatureGate { + g := &FeatureGate{ + feature: feature, + } + for _, opt := range opts { + opt(g) + } + if g.name == "" { + g.name = feature.String() + } + return g +} + +// WithSetting attaches an existing operator cluster setting to the gate. The +// setting is one of the existing feature.*.enabled bools; the gate reuses it +// rather than registering a new one. When the setting is false, Enabled denies +// the feature even if the license permits it. +func WithSetting(s *settings.BoolSetting) Option { + return func(g *FeatureGate) { + g.setting = s + } +} + +// WithName overrides the display name used in error messages. Without it, the +// gate's name defaults to the feature's enum String(). +func WithName(name string) Option { + return func(g *FeatureGate) { + g.name = name + } +} + +// WithExperimental marks the gate as experimental. +// +// TODO: the evaluation semantics for experimental gates (build tag? version +// gate? injected setting?) are an open design question for David. Enabled does +// not yet enforce experimental gating. +func WithExperimental() Option { + return func(g *FeatureGate) { + g.experimental = true + } +} + +// WithCloudOnly marks the gate as available only in cloud deployments. +// +// TODO: the evaluation semantics for cloud-only gates are an open design +// question for David. Enabled does not yet enforce cloud-only gating. +func WithCloudOnly() Option { + return func(g *FeatureGate) { + g.cloudOnly = true + } +} + +// GetLicenseHook returns the currently installed license, or nil if none is +// installed. It is populated by an init() in pkg/server/license to avoid an +// import cycle (server/license imports featureflag, not the reverse). When +// nil, the license gate is permissive. +var GetLicenseHook func(st *cluster.Settings) (*licensepb.License, error) + +// Enabled evaluates the gate against the current cluster state and returns nil +// if the feature is permitted, or an error describing why it is denied. +// +// The checks are evaluated in order: +// +// 1. License entitlement. If no license is installed (or no hook is wired up), +// the gate is permissive. If a license is present, the feature must appear +// in the license's entitlement list; otherwise the call is denied with an +// InsufficientPrivilege error. +// 2. Operator setting. If the gate has an operator setting and it is disabled, +// the call is denied with an OperatorIntervention error and a denial +// telemetry counter is incremented. +// +// Experimental and cloud-only gating are not yet enforced. +func (g *FeatureGate) Enabled(ctx context.Context, st *cluster.Settings) error { + // License gate first. + // + // During this prototype, the absence of a license is permissive: with no + // hook wired up or no license installed we allow the feature and fall + // through to the operator-setting gate. + if GetLicenseHook != nil { + lic, err := GetLicenseHook(st) + if err != nil { + return errors.Wrap(err, "reading license") + } + if lic != nil { + // TODO: an installed license whose Features list is empty predates + // the entitlements field. This prototype treats that as permissive + // (allow). Whether empty-features should be strict (deny) or + // permissive (allow) is an open decision for David. + if len(lic.Features) > 0 && !slices.Contains(lic.Features, g.feature) { + err := pgerror.Newf( + pgcode.InsufficientPrivilege, + "feature %s is not included in your license", + g.name, + ) + return errors.WithHint(err, "upgrade your license to enable this feature") + } + } + } + + // Operator setting gate second. + if g.setting != nil { + if !g.setting.Get(&st.SV) { + telemetry.Inc(sqltelemetry.FeatureDeniedByFeatureFlagCounter) + return pgerror.Newf( + pgcode.OperatorIntervention, + "feature %s was disabled by the database administrator", + g.name, + ) + } + } + + return nil +} From c11dfde2ff16b4908f362545673eb654063f20c5 Mon Sep 17 00:00:00 2001 From: Vishal Vaibhav <168088244+vishalv1994@users.noreply.github.com> Date: Mon, 1 Jun 2026 23:16:57 +0530 Subject: [PATCH 3/7] server/license: wire GetLicense into featureflag hook Populate featureflag.GetLicenseHook from an init() so feature gates can read license entitlements. The dependency direction is server/license -> featureflag, avoiding an import cycle. Co-Authored-By: Claude Opus 4.7 --- pkg/server/license/BUILD.bazel | 1 + pkg/server/license/license.go | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/pkg/server/license/BUILD.bazel b/pkg/server/license/BUILD.bazel index 7322950e4e33..ec7a12efb2f4 100644 --- a/pkg/server/license/BUILD.bazel +++ b/pkg/server/license/BUILD.bazel @@ -14,6 +14,7 @@ go_library( importpath = "github.com/cockroachdb/cockroach/pkg/server/license", visibility = ["//visibility:public"], deps = [ + "//pkg/featureflag", "//pkg/keys", "//pkg/roachpb", "//pkg/server/license/licensepb", diff --git a/pkg/server/license/license.go b/pkg/server/license/license.go index 6fd2b03d4722..2eb1897fbd0f 100644 --- a/pkg/server/license/license.go +++ b/pkg/server/license/license.go @@ -10,6 +10,7 @@ import ( "sync/atomic" "time" + "github.com/cockroachdb/cockroach/pkg/featureflag" "github.com/cockroachdb/cockroach/pkg/server/license/licensepb" "github.com/cockroachdb/cockroach/pkg/settings" "github.com/cockroachdb/cockroach/pkg/settings/cluster" @@ -21,6 +22,13 @@ import ( "github.com/cockroachdb/errors" ) +func init() { + // Wire the license read path into the featureflag package so feature + // gates can consult license entitlements without creating an import + // cycle (featureflag must not import server/license). + featureflag.GetLicenseHook = GetLicense +} + // LicenseTTLMetadata is the metric metadata for seconds until license expiry. var LicenseTTLMetadata = metric.Metadata{ // This metric name isn't namespaced for backwards compatibility. The From 0c8d8356c455bfcb36b3d7fa7177a84ea0d8f0aa Mon Sep 17 00:00:00 2001 From: Vishal Vaibhav <168088244+vishalv1994@users.noreply.github.com> Date: Mon, 1 Jun 2026 23:19:50 +0530 Subject: [PATCH 4/7] featureflag: add compiling example test for FeatureGate Demonstrate the feature-gate API shape with a license-only gate (multi-region) and a license+setting gate (changefeed), exercising Register, WithSetting, and Enabled without touching real call sites. Co-Authored-By: Claude Opus 4.7 --- pkg/featureflag/BUILD.bazel | 16 ++++- pkg/featureflag/feature_gate_example_test.go | 66 ++++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 pkg/featureflag/feature_gate_example_test.go diff --git a/pkg/featureflag/BUILD.bazel b/pkg/featureflag/BUILD.bazel index bd87dd15e3ed..4597fb74981c 100644 --- a/pkg/featureflag/BUILD.bazel +++ b/pkg/featureflag/BUILD.bazel @@ -1,4 +1,4 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_library") +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "featureflag", @@ -21,3 +21,17 @@ go_library( "@com_github_cockroachdb_errors//:errors", ], ) + +go_test( + name = "featureflag_test", + srcs = ["feature_gate_example_test.go"], + deps = [ + ":featureflag", + "//pkg/server/license/licensepb", + "//pkg/settings", + "//pkg/settings/cluster", + "//pkg/util/leaktest", + "//pkg/util/log", + "@com_github_stretchr_testify//require", + ], +) diff --git a/pkg/featureflag/feature_gate_example_test.go b/pkg/featureflag/feature_gate_example_test.go new file mode 100644 index 000000000000..96a66f7325b3 --- /dev/null +++ b/pkg/featureflag/feature_gate_example_test.go @@ -0,0 +1,66 @@ +// Copyright 2026 The Cockroach Authors. +// +// Use of this software is governed by the CockroachDB Software License +// included in the /LICENSE file. + +// This file is a compiling demonstration of the feature-gate API shape for the +// prototype. It exercises both kinds of gate that featureflag.Register can +// build: a license-only gate (modeled on multi-region) and a license+setting +// gate (modeled on changefeed), and shows how Enabled evaluates each. It is +// deliberately self-contained and does not touch any real call sites; the +// setting it registers is a throwaway used solely to drive the operator gate. +package featureflag_test + +import ( + "context" + "testing" + + "github.com/cockroachdb/cockroach/pkg/featureflag" + "github.com/cockroachdb/cockroach/pkg/server/license/licensepb" + "github.com/cockroachdb/cockroach/pkg/settings" + "github.com/cockroachdb/cockroach/pkg/settings/cluster" + "github.com/cockroachdb/cockroach/pkg/util/leaktest" + "github.com/cockroachdb/cockroach/pkg/util/log" + "github.com/stretchr/testify/require" +) + +// changefeedEnabled is a throwaway operator setting that stands in for one of +// the real feature.*.enabled bools. It is registered at package scope because +// settings registration must happen at init and panics on duplicate names. +var changefeedEnabled = settings.RegisterBoolSetting( + settings.ApplicationLevel, + "test.featuregate.changefeed.enabled", + "test-only setting for the feature-gate example", + true, +) + +// TestFeatureGateExample demonstrates the feature-gate API shape by building +// and evaluating both kinds of gate. Because pkg/server/license is not imported +// here, GetLicenseHook stays nil and the license gate is permissive, which lets +// the test focus on the operator-setting path. +func TestFeatureGateExample(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + + ctx := context.Background() + st := cluster.MakeTestingClusterSettings() + + // A license-only gate (like multi-region) has no operator setting. With no + // license installed the license gate is permissive, so Enabled allows it. + mrGate := featureflag.Register(licensepb.Feature_MULTIREGION) + require.NoError(t, mrGate.Enabled(ctx, st)) + + // A license+setting gate (like changefeed) additionally consults an operator + // setting. With the setting defaulting true and no license installed, both + // gates allow the feature. + cfGate := featureflag.Register( + licensepb.Feature_CHANGEFEED, + featureflag.WithSetting(changefeedEnabled), + ) + require.NoError(t, cfGate.Enabled(ctx, st)) + + // Disabling the operator setting denies the feature even though the license + // gate remains permissive. + changefeedEnabled.Override(ctx, &st.SV, false) + require.Error(t, cfGate.Enabled(ctx, st)) +} From ef770b06a14dfc0a35e38681981fc737c7712c16 Mon Sep 17 00:00:00 2001 From: Vishal Vaibhav <168088244+vishalv1994@users.noreply.github.com> Date: Mon, 1 Jun 2026 23:20:49 +0530 Subject: [PATCH 5/7] pkg: register featureflag_test in aggregate BUILD lists Generated by ./dev generate bazel as a consequence of adding the featureflag example test target. Co-Authored-By: Claude Opus 4.7 --- pkg/BUILD.bazel | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel index 7931c0b5e698..65ef2b8937d6 100644 --- a/pkg/BUILD.bazel +++ b/pkg/BUILD.bazel @@ -173,6 +173,7 @@ ALL_TESTS = [ "//pkg/crosscluster/replicationutils:replicationutils_test", "//pkg/crosscluster/streamclient/randclient:randclient_test", "//pkg/crosscluster/streamclient:streamclient_test", + "//pkg/featureflag:featureflag_test", "//pkg/geo/geogen:geogen_test", "//pkg/geo/geogfn:geogfn_test", "//pkg/geo/geographiclib:geographiclib_test", @@ -1505,6 +1506,7 @@ GO_TARGETS = [ "//pkg/crosscluster:crosscluster", "//pkg/docs:docs", "//pkg/featureflag:featureflag", + "//pkg/featureflag:featureflag_test", "//pkg/gen/genbzl:genbzl", "//pkg/gen/genbzl:genbzl_lib", "//pkg/geo/geodist:geodist", From fd7040ee487fc08546336def8db7c3f9439a5bea Mon Sep 17 00:00:00 2001 From: Vishal Vaibhav <168088244+vishalv1994@users.noreply.github.com> Date: Mon, 1 Jun 2026 23:25:17 +0530 Subject: [PATCH 6/7] featureflag: address review - clarify WithSetting doc, test license denial Reword the WithSetting comment to describe current behavior rather than narrate an alternative, and add a test that overrides GetLicenseHook to exercise the license-entitlement deny path (a license granting BACKUP but not CHANGEFEED), demonstrating the core gating mechanism. Co-Authored-By: Claude Opus 4.7 --- pkg/featureflag/feature_gate.go | 6 ++-- pkg/featureflag/feature_gate_example_test.go | 29 ++++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/pkg/featureflag/feature_gate.go b/pkg/featureflag/feature_gate.go index db4ac5e4298b..3ddc4feb8c09 100644 --- a/pkg/featureflag/feature_gate.go +++ b/pkg/featureflag/feature_gate.go @@ -79,9 +79,9 @@ func Register(feature licensepb.Feature, opts ...Option) *FeatureGate { } // WithSetting attaches an existing operator cluster setting to the gate. The -// setting is one of the existing feature.*.enabled bools; the gate reuses it -// rather than registering a new one. When the setting is false, Enabled denies -// the feature even if the license permits it. +// gate does not register a new setting; the caller supplies one of the existing +// feature.*.enabled bools. When the setting is false, Enabled denies the +// feature even if the license permits it. func WithSetting(s *settings.BoolSetting) Option { return func(g *FeatureGate) { g.setting = s diff --git a/pkg/featureflag/feature_gate_example_test.go b/pkg/featureflag/feature_gate_example_test.go index 96a66f7325b3..17ec86464765 100644 --- a/pkg/featureflag/feature_gate_example_test.go +++ b/pkg/featureflag/feature_gate_example_test.go @@ -64,3 +64,32 @@ func TestFeatureGateExample(t *testing.T) { changefeedEnabled.Override(ctx, &st.SV, false) require.Error(t, cfGate.Enabled(ctx, st)) } + +// TestFeatureGateLicenseDenial exercises the license-entitlement path by +// overriding GetLicenseHook to return a license that grants only a subset of +// features. This is normally wired by pkg/server/license, but the prototype +// keeps the hook exported so a license-gated decision can be demonstrated in +// isolation. +func TestFeatureGateLicenseDenial(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + + ctx := context.Background() + st := cluster.MakeTestingClusterSettings() + + // Install a license that entitles BACKUP but not CHANGEFEED. + featureflag.GetLicenseHook = func(*cluster.Settings) (*licensepb.License, error) { + return &licensepb.License{ + Features: []licensepb.Feature{licensepb.Feature_BACKUP}, + }, nil + } + defer func() { featureflag.GetLicenseHook = nil }() + + // An entitled feature passes the license gate. + backupGate := featureflag.Register(licensepb.Feature_BACKUP) + require.NoError(t, backupGate.Enabled(ctx, st)) + + // A feature absent from the license is denied, even with no operator setting. + changefeedGate := featureflag.Register(licensepb.Feature_CHANGEFEED) + require.Error(t, changefeedGate.Enabled(ctx, st)) +} From 6ddd2badf78ea71e534d1705effb5814fbfccc44 Mon Sep 17 00:00:00 2001 From: Vishal Vaibhav <168088244+vishalv1994@users.noreply.github.com> Date: Wed, 3 Jun 2026 00:59:22 +0530 Subject: [PATCH 7/7] featureflag: WithSetting registers the operator setting internally Address review feedback: WithSetting now takes the setting name (string) and calls settings.RegisterBoolSetting on the caller's behalf, removing the need for each call site to separately register a feature.*.enabled bool. A new Setting() accessor exposes the registered *BoolSetting for direct reads or test overrides. Co-Authored-By: roachdev-claude --- pkg/featureflag/BUILD.bazel | 1 - pkg/featureflag/feature_gate.go | 44 +++++++++++++------- pkg/featureflag/feature_gate_example_test.go | 21 ++++------ 3 files changed, 35 insertions(+), 31 deletions(-) diff --git a/pkg/featureflag/BUILD.bazel b/pkg/featureflag/BUILD.bazel index 4597fb74981c..6ab3f37fc6fb 100644 --- a/pkg/featureflag/BUILD.bazel +++ b/pkg/featureflag/BUILD.bazel @@ -28,7 +28,6 @@ go_test( deps = [ ":featureflag", "//pkg/server/license/licensepb", - "//pkg/settings", "//pkg/settings/cluster", "//pkg/util/leaktest", "//pkg/util/log", diff --git a/pkg/featureflag/feature_gate.go b/pkg/featureflag/feature_gate.go index 3ddc4feb8c09..570abb90f853 100644 --- a/pkg/featureflag/feature_gate.go +++ b/pkg/featureflag/feature_gate.go @@ -28,10 +28,9 @@ import ( // the license grants. This deliberately avoids relying on denormalized cluster // settings so that entitlements remain a single source of truth. // -// The operator setting, when supplied via WithSetting, is one of the existing -// feature.*.enabled cluster-setting bools. The gate does not register a new -// setting; it reuses the one the caller already owns, so that operators retain -// the ability to turn a feature off even when the license permits it. +// The operator setting, when supplied via WithSetting, is a feature.*.enabled +// cluster-setting bool that the gate registers on the caller's behalf. Operators +// retain the ability to turn a feature off even when the license permits it. // // Lifecycle: a FeatureGate is constructed once, typically in a package-level // var via Register, and is immutable thereafter. It is evaluated per-use by @@ -78,16 +77,29 @@ func Register(feature licensepb.Feature, opts ...Option) *FeatureGate { return g } -// WithSetting attaches an existing operator cluster setting to the gate. The -// gate does not register a new setting; the caller supplies one of the existing -// feature.*.enabled bools. When the setting is false, Enabled denies the -// feature even if the license permits it. -func WithSetting(s *settings.BoolSetting) Option { +// WithSetting registers a feature.*.enabled cluster-setting bool with the given +// name and attaches it to the gate. The setting defaults to +// FeatureFlagEnabledDefault (true). When the setting is false, Enabled denies +// the feature even if the license permits it. Use Setting() to access the +// registered *BoolSetting for direct reads or test overrides. +func WithSetting(name string) Option { return func(g *FeatureGate) { - g.setting = s + g.setting = settings.RegisterBoolSetting( + settings.ApplicationLevel, + settings.InternalKey(name), + "set to true to enable the feature, false to disable; default is true", + FeatureFlagEnabledDefault, + settings.WithPublic, + ) } } +// Setting returns the operator cluster setting attached to this gate, or nil if +// the gate was constructed without WithSetting. +func (g *FeatureGate) Setting() *settings.BoolSetting { + return g.setting +} + // WithName overrides the display name used in error messages. Without it, the // gate's name defaults to the feature's enum String(). func WithName(name string) Option { @@ -98,9 +110,9 @@ func WithName(name string) Option { // WithExperimental marks the gate as experimental. // -// TODO: the evaluation semantics for experimental gates (build tag? version -// gate? injected setting?) are an open design question for David. Enabled does -// not yet enforce experimental gating. +// TODO(vishalv): stub — not enforced by Enabled yet. The real implementation +// would allow experimental features in non-release builds and require an +// explicit unsafe-experimental-mode interlock in release builds. func WithExperimental() Option { return func(g *FeatureGate) { g.experimental = true @@ -109,8 +121,8 @@ func WithExperimental() Option { // WithCloudOnly marks the gate as available only in cloud deployments. // -// TODO: the evaluation semantics for cloud-only gates are an open design -// question for David. Enabled does not yet enforce cloud-only gating. +// TODO(vishalv): stub — not enforced by Enabled yet. The real implementation +// would check the MSO environment variable to determine cloud context. func WithCloudOnly() Option { return func(g *FeatureGate) { g.cloudOnly = true @@ -136,7 +148,7 @@ var GetLicenseHook func(st *cluster.Settings) (*licensepb.License, error) // the call is denied with an OperatorIntervention error and a denial // telemetry counter is incremented. // -// Experimental and cloud-only gating are not yet enforced. +// Experimental and cloud-only gating are stubs and not yet enforced. func (g *FeatureGate) Enabled(ctx context.Context, st *cluster.Settings) error { // License gate first. // diff --git a/pkg/featureflag/feature_gate_example_test.go b/pkg/featureflag/feature_gate_example_test.go index 17ec86464765..daff685e4c33 100644 --- a/pkg/featureflag/feature_gate_example_test.go +++ b/pkg/featureflag/feature_gate_example_test.go @@ -17,21 +17,18 @@ import ( "github.com/cockroachdb/cockroach/pkg/featureflag" "github.com/cockroachdb/cockroach/pkg/server/license/licensepb" - "github.com/cockroachdb/cockroach/pkg/settings" "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/util/leaktest" "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/stretchr/testify/require" ) -// changefeedEnabled is a throwaway operator setting that stands in for one of -// the real feature.*.enabled bools. It is registered at package scope because -// settings registration must happen at init and panics on duplicate names. -var changefeedEnabled = settings.RegisterBoolSetting( - settings.ApplicationLevel, - "test.featuregate.changefeed.enabled", - "test-only setting for the feature-gate example", - true, +// cfGate is a license+setting gate modeled on changefeed. It is declared at +// package scope because WithSetting calls settings.RegisterBoolSetting, which +// must happen at init time so that cluster settings objects pick up the default. +var cfGate = featureflag.Register( + licensepb.Feature_CHANGEFEED, + featureflag.WithSetting("test.featuregate.changefeed.enabled"), ) // TestFeatureGateExample demonstrates the feature-gate API shape by building @@ -53,15 +50,11 @@ func TestFeatureGateExample(t *testing.T) { // A license+setting gate (like changefeed) additionally consults an operator // setting. With the setting defaulting true and no license installed, both // gates allow the feature. - cfGate := featureflag.Register( - licensepb.Feature_CHANGEFEED, - featureflag.WithSetting(changefeedEnabled), - ) require.NoError(t, cfGate.Enabled(ctx, st)) // Disabling the operator setting denies the feature even though the license // gate remains permissive. - changefeedEnabled.Override(ctx, &st.SV, false) + cfGate.Setting().Override(ctx, &st.SV, false) require.Error(t, cfGate.Enabled(ctx, st)) }