fix: invalid scoped-sync responses for empty flags (#1352)

Fixes an issue where invalid flag payloads were returned on sync
requests with scopes if the flag set was empty.

Below is an example of the bug.

```shell
$ grpcurl -import-path /home/todd/temp -proto sync.proto -plaintext localhost:8015 flagd.sync.v1.FlagSyncService/FetchAllFlags
{
  "flagConfiguration": "{\"flags\":{}}"
}
$ grpcurl -import-path /home/todd/temp -proto sync.proto -plaintext -d '{"selector":"../config/samples/example_flags.flagd.json"}' localhost:8015 flagd.sync.v1.FlagSyncService/FetchAllFlags 
{}
```

---------

Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
This commit is contained in:
Todd Baert 2024-07-05 14:25:34 -04:00 committed by GitHub
parent 450cbc84ca
commit 51371d25e2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 59 additions and 23 deletions

View File

@ -12,6 +12,11 @@ import (
"github.com/open-feature/flagd/core/pkg/store"
)
//nolint:errchkjson
var emptyConfigBytes, _ = json.Marshal(map[string]map[string]string{
"flags": {},
})
// Multiplexer abstract subscription handling and storage processing.
// Flag configurations will be lazy loaded using reFill logic upon the calls to publish.
type Multiplexer struct {
@ -162,6 +167,10 @@ func (r *Multiplexer) SourcesAsMetadata() string {
// reFill local configuration values
func (r *Multiplexer) reFill() error {
clear(r.selectorFlags)
// start all sources with empty config
for _, source := range r.sources {
r.selectorFlags[source] = string(emptyConfigBytes)
}
all, err := r.store.GetAll(context.Background())
if err != nil {
@ -170,7 +179,7 @@ func (r *Multiplexer) reFill() error {
bytes, err := json.Marshal(map[string]interface{}{"flags": all})
if err != nil {
return fmt.Errorf("error from marshallin: %w", err)
return fmt.Errorf("error marshalling: %w", err)
}
r.allFlags = string(bytes)
@ -188,6 +197,7 @@ func (r *Multiplexer) reFill() error {
}
}
// for all flags, sort them into their correct selector
for source, flags := range collector {
bytes, err := json.Marshal(map[string]interface{}{"flags": flags})
if err != nil {

View File

@ -6,8 +6,12 @@ import (
"strings"
"testing"
"time"
"github.com/stretchr/testify/assert"
)
const emptyConfigString = "{\"flags\":{}}"
func TestRegistration(t *testing.T) {
// given
mux, err := NewMux(getSimpleFlagStore())
@ -20,6 +24,7 @@ func TestRegistration(t *testing.T) {
testName string
id interface{}
source string
flagStringValidator func(flagString string, testSource string, testName string)
connection chan payload
expectError bool
}{
@ -32,18 +37,35 @@ func TestRegistration(t *testing.T) {
testName: "subscribe to source A",
id: context.Background(),
source: "A",
flagStringValidator: func(flagString string, testSource string, testName string) {
assert.Contains(t, flagString, fmt.Sprintf("\"source\":\"%s\"", testSource))
},
connection: make(chan payload, 1),
},
{
testName: "subscribe to source B",
id: context.Background(),
source: "B",
flagStringValidator: func(flagString string, testSource string, testName string) {
assert.Contains(t, flagString, fmt.Sprintf("\"source\":\"%s\"", testSource))
},
connection: make(chan payload, 1),
},
{
testName: "subscribe to empty",
id: context.Background(),
source: "C",
connection: make(chan payload, 1),
flagStringValidator: func(flagString string, testSource string, testName string) {
assert.Equal(t, flagString, emptyConfigString)
},
expectError: false,
},
{
testName: "subscribe to non-existing",
id: context.Background(),
source: "C",
source: "D",
connection: make(chan payload, 1),
expectError: true,
},
@ -75,13 +97,8 @@ func TestRegistration(t *testing.T) {
break
}
if initSync.flags == "" {
t.Fatal("expected non empty flag payload, but got empty")
}
// validate source of flag
if test.source != "" && !strings.Contains(initSync.flags, fmt.Sprintf("\"source\":\"%s\"", test.source)) {
t.Fatal("expected initial flag response to contain flags from source, but failed to find source")
if test.flagStringValidator != nil {
test.flagStringValidator(initSync.flags, test.source, test.testName)
}
})
}
@ -173,4 +190,13 @@ func TestGetAllFlags(t *testing.T) {
t.Fatal("expected flags to be scoped")
return
}
// when - get all for a flagless-scope
flags, err = mux.GetAllFlags("C")
if err != nil {
t.Fatal("error when retrieving all flags")
return
}
assert.Equal(t, flags, emptyConfigString)
}

View File

@ -128,7 +128,7 @@ func TestSyncServiceEndToEnd(t *testing.T) {
t.Fatal("expected sources entry in the metadata, but got nil")
}
if asMap["sources"] != "A,B" {
if asMap["sources"] != "A,B,C" {
t.Fatal("incorrect sources entry in metadata")
}

View File

@ -5,7 +5,7 @@ import (
"github.com/open-feature/flagd/core/pkg/store"
)
// getSimpleFlagStore returns a flag store pre-filled with flags from sources A & B
// getSimpleFlagStore returns a flag store pre-filled with flags from sources A & B & C, which C empty
func getSimpleFlagStore() (*store.Flags, []string) {
variants := map[string]any{
"true": true,
@ -28,5 +28,5 @@ func getSimpleFlagStore() (*store.Flags, []string) {
Source: "B",
})
return flagStore, []string{"A", "B"}
return flagStore, []string{"A", "B", "C"}
}