Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions pkg/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
23 changes: 21 additions & 2 deletions pkg/featureflag/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,17 +1,36 @@
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",
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",
],
)

go_test(
name = "featureflag_test",
srcs = ["feature_gate_example_test.go"],
deps = [
":featureflag",
"//pkg/server/license/licensepb",
"//pkg/settings/cluster",
"//pkg/util/leaktest",
"//pkg/util/log",
"@com_github_stretchr_testify//require",
],
)
192 changes: 192 additions & 0 deletions pkg/featureflag/feature_gate.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,192 @@
// 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 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
// 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 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 = 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 {
return func(g *FeatureGate) {
g.name = name
}
}

// WithExperimental marks the gate as experimental.
Comment thread
dt marked this conversation as resolved.
//
// 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
}
}

// WithCloudOnly marks the gate as available only in cloud deployments.
//
// 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
}
}

// 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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not a blocking concern right now but eventually I think this would be cleaner if we could have featureflag depend on the license setting; perhaps that means the setting is in the wrong place if pkg/server/license is depending on featureflag and we'd want to move the setting itself to a util pkg separate from the the higher level code in pkg/server that is probably what's pulling in featureflag.

but again, not blocking for now -- just something i'd maybe have in a TODO.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, the hook is there purely to break the cycle.

Moving the license setting (or at least the GetLicense accessor) to a lower-level package that featureflag can import directly would be cleaner. I'll add a TODO for this. It'll make the code more straightforward once we sort out the package layering.


// 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 stubs and 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
}
88 changes: 88 additions & 0 deletions pkg/featureflag/feature_gate_example_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
// 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/cluster"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/stretchr/testify/require"
)

// 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
// 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.
require.NoError(t, cfGate.Enabled(ctx, st))

// Disabling the operator setting denies the feature even though the license
// gate remains permissive.
cfGate.Setting().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))
}
1 change: 1 addition & 0 deletions pkg/server/license/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
8 changes: 8 additions & 0 deletions pkg/server/license/license.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down
20 changes: 20 additions & 0 deletions pkg/server/license/licensepb/license.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Impl nit: I think we should switch this to a compact []byte packed a bitflag (using emum IDs for shifts) since 4 bytes or 8 bytes per bool is pretty bloated for something that gets moved around by humans as a string on their terminal or clipboard. wire-compat with manage-service doesn't seem worth a worse UX for our users (of having big, multi-kb license strings) so we could either just not worry about identical wire format or, better yet, managed-service could switch to the superior representation as well if/when we decide we want them to match.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense! The bitflag approach is definitely more efficient. My main question is whether we should make that change now or defer it.

With only 8 features today, the difference in license string length is roughly 10 characters, so it’s not really a UX concern yet.

If we ship the repeated Feature encoding first and decide to move to a bitmask later, we’ll either need to support decoding both formats or reissue licenses. Since there’s currently no migration cost, I’m leaning toward doing the bitflag encoding now.

Does that align with your thinking?

@dt dt Jun 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only 8 features today

Alternative framing : we haven't even launched this yet or incorporated it into every DB team's day to day workflows and we're already at 8 -- and 15% increase in the user-visible/user-handled license strings. It seems likely that we'll want teams to put every feature/option/aspect of a feature that we might want to at some point unbundle/make an add-on/repackage behind its own discrete flag to ensure those decisions can be made externally at lic-gen time, so we should probably expect this list to only grow from here.

currently no migration cost, I’m leaning toward doing the bitflag encoding now.

Agreed: it's much easier to make the switch now before we externally release anything than do it later when it'll become a Migration™. There's ~no reason not to just do it now, it's already a win for the lic size and will only be moreso later.

}
Loading