Skip to content

feat: add configuration property to define which key is used if a string without equals is used as a selector #1798

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
+89 −65
Open

feat: add configuration property to define which key is used if a string without equals is used as a selector #1798

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b4b4bd1
feat: add configuration property to define which key is used if a str…
aepfli Sep 9, 2025
71a7ba5
fixup: improve config property description
aepfli Sep 9, 2025
726d586
fixup: improve test
aepfli Sep 9, 2025
67d630e
fixup: simplify logic
aepfli Sep 9, 2025
5f25f9e
fixup: fixing missed usages and update docs
aepfli Sep 9, 2025
54989a2
fixup: fixing docs
aepfli Sep 9, 2025
dcbf242
fixup: gemini fixes
aepfli Sep 23, 2025
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
6 changes: 3 additions & 3 deletions core/pkg/evaluator/fractional_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ func TestFractionalEvaluation(t *testing.T) {
"headerColor": {
State: "ENABLED",
DefaultVariant: "red",
Variants: colorVariants,
Variants: colorVariants,
Targeting: []byte(
`{
"fractional": [
Expand All @@ -401,7 +401,7 @@ func TestFractionalEvaluation(t *testing.T) {
for name, tt := range tests {
t.Run(name, func(t *testing.T) {
log := logger.NewLogger(nil, false)
s, err := store.NewStore(log, sources)
s, err := store.NewStore(log, store.StoreConfig{Sources: sources})
if err != nil {
t.Fatalf("NewStore failed: %v", err)
}
Expand Down Expand Up @@ -530,7 +530,7 @@ func BenchmarkFractionalEvaluation(b *testing.B) {
for name, tt := range tests {
b.Run(name, func(b *testing.B) {
log := logger.NewLogger(nil, false)
s, err := store.NewStore(log, sources)
s, err := store.NewStore(log, store.StoreConfig{Sources: sources})
if err != nil {
b.Fatalf("NewStore failed: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion core/pkg/evaluator/semver_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -791,7 +791,7 @@ func TestJSONEvaluator_semVerEvaluation(t *testing.T) {
for name, tt := range tests {
t.Run(name, func(t *testing.T) {
log := logger.NewLogger(nil, false)
s, err := store.NewStore(log, sources)
s, err := store.NewStore(log, store.StoreConfig{Sources: sources})
if err != nil {
t.Fatalf("NewStore failed: %v", err)
}
Expand Down
4 changes: 2 additions & 2 deletions core/pkg/evaluator/string_comparison_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ func TestJSONEvaluator_startsWithEvaluation(t *testing.T) {
for name, tt := range tests {
t.Run(name, func(t *testing.T) {
log := logger.NewLogger(nil, false)
s, err := store.NewStore(log, sources)
s, err := store.NewStore(log, store.StoreConfig{Sources: sources})
if err != nil {
t.Fatalf("NewStore failed: %v", err)
}
Expand Down Expand Up @@ -320,7 +320,7 @@ func TestJSONEvaluator_endsWithEvaluation(t *testing.T) {
for name, tt := range tests {
t.Run(name, func(t *testing.T) {
log := logger.NewLogger(nil, false)
s, err := store.NewStore(log, sources)
s, err := store.NewStore(log, store.StoreConfig{Sources: sources})
if err != nil {
t.Fatalf("NewStore failed: %v", err)
}
Expand Down
15 changes: 9 additions & 6 deletions core/pkg/store/query.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,21 +36,24 @@
// For example, to select flags from source "./mySource" and flagSetId "1234", use the expression:
// "source=./mySource,flagSetId=1234"
func NewSelector(selectorExpression string) Selector {
return NewSelectorWithFallback(selectorExpression, "")
}
func NewSelectorWithFallback(selectorExpression string, fallbackExpressionKey string) Selector {
return Selector{
indexMap: expressionToMap(selectorExpression),
indexMap: expressionToMap(selectorExpression, fallbackExpressionKey),
}
}

func expressionToMap(sExp string) map[string]string {
func expressionToMap(sExp string, fallbackExpressionKey string) map[string]string {
selectorMap := make(map[string]string)
if sExp == "" {
return selectorMap
}

if strings.Index(sExp, "=") == -1 {

Check failure on line 52 in core/pkg/store/query.go

View workflow job for this annotation

GitHub Actions / lint 

S1003: should use !strings.Contains(sExp, "=") instead (staticcheck)
// if no '=' is found, treat the whole string as as source (backwards compatibility)
// we may may support interpreting this as a flagSetId in the future as an option
selectorMap[sourceIndex] = sExp
if fallbackExpressionKey == "" {
fallbackExpressionKey = sourceIndex
}
selectorMap[fallbackExpressionKey] = sExp
return selectorMap
}

Expand Down
15 changes: 11 additions & 4 deletions core/pkg/store/query_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,10 @@ func TestSelector_ToMetadata(t *testing.T) {

func TestNewSelector(t *testing.T) {
tests := []struct {
name string
input string
wantMap map[string]string
name string
input string
fallbackExpressionKey string
wantMap map[string]string
}{
{
name: "source and flagSetId",
Expand All @@ -175,6 +176,12 @@ func TestNewSelector(t *testing.T) {
input: "mysource",
wantMap: map[string]string{"source": "mysource"},
},
{
name: "no equals, treat as flagSetId",
input: "flagSetId",
fallbackExpressionKey: "flagSetId",
wantMap: map[string]string{"flagSetId": "flagSetId"},
},
{
name: "empty string",
input: "",
Expand All @@ -184,7 +191,7 @@ func TestNewSelector(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
s := NewSelector(tt.input)
s := NewSelectorWithFallback(tt.input, tt.fallbackExpressionKey)
if !reflect.DeepEqual(s.indexMap, tt.wantMap) {
t.Errorf("NewSelector(%q) indexMap = %v, want %v", tt.input, s.indexMap, tt.wantMap)
}
Expand Down
25 changes: 16 additions & 9 deletions core/pkg/store/store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,22 @@ type IStore interface {
var _ IStore = (*Store)(nil)

type Store struct {
db *memdb.MemDB
logger *logger.Logger
sources []string
db *memdb.MemDB
logger *logger.Logger
sources []string
expressionFallbackKey string
Copy link

Choose a reason for hiding this comment

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

critical

The new field expressionFallbackKey is stored in the Store struct but it's never actually used. This means the new selector-fallback-key configuration has no effect.

To fix this, you could introduce a method on the Store struct to create selectors, which would use the configured fallback key. For example:

func (s *Store) NewSelector(selectorExpression string) Selector {
    return NewSelectorWithFallback(selectorExpression, s.expressionFallbackKey)
}

Then, you would need to update all call sites of store.NewSelector() to use this new method on a *Store instance (e.g., myStore.NewSelector(...)).

Copy link
Member Author

Choose a reason for hiding this comment

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

this is not an issue, we are actually updating this where we need it.

// deprecated: has no effect and will be removed soon.
FlagSources []string
}

type StoreConfig struct {
Sources []string
SelectorFallbackKey string
}

// NewStore creates a new in-memory store with the given sources.
// The order of sources in the slice determines their priority, when queries result in duplicate flags (queries without source or flagSetId), the higher priority source "wins".
func NewStore(logger *logger.Logger, sources []string) (*Store, error) {
func NewStore(logger *logger.Logger, storeConfig StoreConfig) (*Store, error) {

// a unique index must exist for each set of constraints - for example, to look up by key and source, we need a compound index on key+source, etc
// we maybe want to generate these dynamically in the future to support more robust querying, but for now we will hardcode the ones we need
Expand Down Expand Up @@ -124,18 +130,19 @@ func NewStore(logger *logger.Logger, sources []string) (*Store, error) {
}

// clone the sources to avoid modifying the original slice
s := slices.Clone(sources)
s := slices.Clone(storeConfig.Sources)

return &Store{
sources: s,
db: db,
logger: logger,
sources: s,
db: db,
logger: logger,
expressionFallbackKey: storeConfig.SelectorFallbackKey,
}, nil
}

// Deprecated: use NewStore instead - will be removed very soon.
func NewFlags() *Store {
state, err := NewStore(logger.NewLogger(nil, false), noValidatedSources)
state, err := NewStore(logger.NewLogger(nil, false), StoreConfig{Sources: noValidatedSources})

if err != nil {
panic(fmt.Sprintf("unable to create flag store: %v", err))
Expand Down
20 changes: 10 additions & 10 deletions core/pkg/store/store_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func TestUpdateFlags(t *testing.T) {
{
name: "both nil",
setup: func(t *testing.T) IStore {
s, err := NewStore(logger.NewLogger(nil, false), sources)
s, err := NewStore(logger.NewLogger(nil, false), StoreConfig{Sources: sources})
if err != nil {
t.Fatalf("NewStore failed: %v", err)
}
Expand All @@ -42,7 +42,7 @@ func TestUpdateFlags(t *testing.T) {
{
name: "both empty flags",
setup: func(t *testing.T) IStore {
s, err := NewStore(logger.NewLogger(nil, false), sources)
s, err := NewStore(logger.NewLogger(nil, false), StoreConfig{Sources: sources})
if err != nil {
t.Fatalf("NewStore failed: %v", err)
}
Expand All @@ -55,7 +55,7 @@ func TestUpdateFlags(t *testing.T) {
{
name: "empty new",
setup: func(t *testing.T) IStore {
s, err := NewStore(logger.NewLogger(nil, false), sources)
s, err := NewStore(logger.NewLogger(nil, false), StoreConfig{Sources: sources})
if err != nil {
t.Fatalf("NewStore failed: %v", err)
}
Expand All @@ -68,7 +68,7 @@ func TestUpdateFlags(t *testing.T) {
{
name: "update from source 1 (old flag removed)",
setup: func(t *testing.T) IStore {
s, err := NewStore(logger.NewLogger(nil, false), sources)
s, err := NewStore(logger.NewLogger(nil, false), StoreConfig{Sources: sources})
if err != nil {
t.Fatalf("NewStore failed: %v", err)
}
Expand All @@ -88,7 +88,7 @@ func TestUpdateFlags(t *testing.T) {
{
name: "update from source 1 (new flag added)",
setup: func(t *testing.T) IStore {
s, err := NewStore(logger.NewLogger(nil, false), sources)
s, err := NewStore(logger.NewLogger(nil, false), StoreConfig{Sources: sources})
if err != nil {
t.Fatalf("NewStore failed: %v", err)
}
Expand All @@ -109,7 +109,7 @@ func TestUpdateFlags(t *testing.T) {
{
name: "flag set inheritance",
setup: func(t *testing.T) IStore {
s, err := NewStore(logger.NewLogger(nil, false), sources)
s, err := NewStore(logger.NewLogger(nil, false), StoreConfig{Sources: sources})
if err != nil {
t.Fatalf("NewStore failed: %v", err)
}
Expand Down Expand Up @@ -218,7 +218,7 @@ func TestGet(t *testing.T) {
"dupe": {Key: "dupe", DefaultVariant: "off", Metadata: model.Metadata{"flagSetId": flagSetIdC}},
}

store, err := NewStore(logger.NewLogger(nil, false), sources)
store, err := NewStore(logger.NewLogger(nil, false), StoreConfig{Sources: sources})
if err != nil {
t.Fatalf("NewStore failed: %v", err)
}
Expand Down Expand Up @@ -303,7 +303,7 @@ func TestGetAllNoWatcher(t *testing.T) {
"dupe": {Key: "dupe", DefaultVariant: "off", Metadata: model.Metadata{"flagSetId": flagSetIdC}},
}

store, err := NewStore(logger.NewLogger(nil, false), sources)
store, err := NewStore(logger.NewLogger(nil, false), StoreConfig{Sources: sources})
if err != nil {
t.Fatalf("NewStore failed: %v", err)
}
Expand Down Expand Up @@ -375,7 +375,7 @@ func TestWatch(t *testing.T) {
"flagC": {Key: "flagC", DefaultVariant: "off"},
}

store, err := NewStore(logger.NewLogger(nil, false), sources)
store, err := NewStore(logger.NewLogger(nil, false), StoreConfig{Sources: sources})
if err != nil {
t.Fatalf("NewStore failed: %v", err)
}
Expand Down Expand Up @@ -453,7 +453,7 @@ func TestQueryMetadata(t *testing.T) {
"flagB": {Key: "flagB", DefaultVariant: "on"},
}

store, err := NewStore(logger.NewLogger(nil, false), sources)
store, err := NewStore(logger.NewLogger(nil, false), StoreConfig{Sources: sources})
if err != nil {
t.Fatalf("NewStore failed: %v", err)
}
Expand Down
1 change: 1 addition & 0 deletions docs/reference/flagd-cli/flagd_start.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ flagd start [flags]
-K, --otel-key-path string tls key path to use with OpenTelemetry collector
-I, --otel-reload-interval duration how long between reloading the otel tls certificate from disk (default 1h0m0s)
-p, --port int32 Port to listen on (default 8013)
--selector-fallback-key string Fallback key for a selector expression that does not contain an '='. Defaults to 'source'.
-c, --server-cert-path string Server side tls certificate path
-k, --server-key-path string Server side tls key path
-d, --socket-path string Flagd unix socket path. With grpc the evaluations service will become available on this address. With http(s) the grpc-gateway proxy will use this address internally.
Expand Down
48 changes: 26 additions & 22 deletions flagd/cmd/start.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,28 +18,29 @@ import (
)

const (
corsFlagName = "cors-origin"
logFormatFlagName = "log-format"
managementPortFlagName = "management-port"
metricsExporter = "metrics-exporter"
ofrepPortFlagName = "ofrep-port"
otelCollectorURI = "otel-collector-uri"
otelCertPathFlagName = "otel-cert-path"
otelKeyPathFlagName = "otel-key-path"
otelCAPathFlagName = "otel-ca-path"
otelReloadIntervalFlagName = "otel-reload-interval"
portFlagName = "port"
serverCertPathFlagName = "server-cert-path"
serverKeyPathFlagName = "server-key-path"
socketPathFlagName = "socket-path"
sourcesFlagName = "sources"
syncPortFlagName = "sync-port"
syncSocketPathFlagName = "sync-socket-path"
uriFlagName = "uri"
disableSyncMetadata = "disable-sync-metadata"
contextValueFlagName = "context-value"
headerToContextKeyFlagName = "context-from-header"
streamDeadlineFlagName = "stream-deadline"
corsFlagName = "cors-origin"
logFormatFlagName = "log-format"
managementPortFlagName = "management-port"
metricsExporter = "metrics-exporter"
ofrepPortFlagName = "ofrep-port"
otelCollectorURI = "otel-collector-uri"
otelCertPathFlagName = "otel-cert-path"
otelKeyPathFlagName = "otel-key-path"
otelCAPathFlagName = "otel-ca-path"
otelReloadIntervalFlagName = "otel-reload-interval"
portFlagName = "port"
serverCertPathFlagName = "server-cert-path"
serverKeyPathFlagName = "server-key-path"
socketPathFlagName = "socket-path"
sourcesFlagName = "sources"
syncPortFlagName = "sync-port"
syncSocketPathFlagName = "sync-socket-path"
uriFlagName = "uri"
disableSyncMetadata = "disable-sync-metadata"
contextValueFlagName = "context-value"
headerToContextKeyFlagName = "context-from-header"
streamDeadlineFlagName = "stream-deadline"
selectorFallbackKeyFlagName = "selector-fallback-key"
)

func init() {
Expand Down Expand Up @@ -91,6 +92,7 @@ func init() {
"header values to context values, where key is Header name, value is context key")
flags.Duration(streamDeadlineFlagName, 0, "Set a server-side deadline for flagd sync and event streams (default 0, means no deadline).")
flags.Bool(disableSyncMetadata, false, "Disables the getMetadata endpoint of the sync service. Defaults to false, but will default to true in later versions.")
flags.String(selectorFallbackKeyFlagName, "", "Fallback key for a selector expression that does not contain an '='. Defaults to 'source'.")

bindFlags(flags)
}
Expand All @@ -117,6 +119,7 @@ func bindFlags(flags *pflag.FlagSet) {
_ = viper.BindPFlag(headerToContextKeyFlagName, flags.Lookup(headerToContextKeyFlagName))
_ = viper.BindPFlag(streamDeadlineFlagName, flags.Lookup(streamDeadlineFlagName))
_ = viper.BindPFlag(disableSyncMetadata, flags.Lookup(disableSyncMetadata))
_ = viper.BindPFlag(selectorFallbackKeyFlagName, flags.Lookup(selectorFallbackKeyFlagName))
}

// startCmd represents the start command
Expand Down Expand Up @@ -191,6 +194,7 @@ var startCmd = &cobra.Command{
StreamDeadline: viper.GetDuration(streamDeadlineFlagName),
DisableSyncMetadata: viper.GetBool(disableSyncMetadata),
SyncProviders: syncProviders,
SelectorFallbackKey: viper.GetString(selectorFallbackKeyFlagName),
ContextValues: contextValuesToMap,
HeaderToContextKeyMappings: headerToContextKeyMappings,
})
Expand Down
4 changes: 3 additions & 1 deletion flagd/pkg/runtime/from_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ type Config struct {
SyncProviders []sync.SourceConfig
CORS []string

SelectorFallbackKey string

ContextValues map[string]any
HeaderToContextKeyMappings map[string]string
}
Expand Down Expand Up @@ -86,7 +88,7 @@ func FromConfig(logger *logger.Logger, version string, config Config) (*Runtime,
}

// build flag store, collect flag sources & fill sources details
store, err := store.NewStore(logger, sources)
store, err := store.NewStore(logger, store.StoreConfig{Sources: sources, SelectorFallbackKey: config.SelectorFallbackKey})
if err != nil {
return nil, fmt.Errorf("error creating flag store: %w", err)
}
Expand Down
6 changes: 3 additions & 3 deletions flagd/pkg/service/flag-evaluation/connect_service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ func TestConnectServiceNotify(t *testing.T) {
eval := mock.NewMockIEvaluator(ctrl)
sources := []string{"source1", "source2"}
log := logger.NewLogger(nil, false)
s, err := store.NewStore(log, sources)
s, err := store.NewStore(log, store.StoreConfig{Sources: sources})
if err != nil {
t.Fatalf("NewStore failed: %v", err)
}
Expand Down Expand Up @@ -219,7 +219,7 @@ func TestConnectServiceNotify(t *testing.T) {
func TestConnectServiceWatcher(t *testing.T) {
sources := []string{"source1", "source2"}
log := logger.NewLogger(nil, false)
s, err := store.NewStore(log, sources)
s, err := store.NewStore(log, store.StoreConfig{Sources: sources})

if err != nil {
t.Fatalf("NewStore failed: %v", err)
Expand Down Expand Up @@ -269,7 +269,7 @@ func TestConnectServiceShutdown(t *testing.T) {
eval := mock.NewMockIEvaluator(ctrl)
sources := []string{"source1", "source2"}
log := logger.NewLogger(nil, false)
s, err := store.NewStore(log, sources)
s, err := store.NewStore(log, store.StoreConfig{Sources: sources})
if err != nil {
t.Fatalf("NewStore failed: %v", err)
}
Expand Down
Loading
Loading