Task/sdk controller fagsets

Makefile

@@ -96,6 +96,10 @@ images_release: # entrypoints
 	@echo "$(DOCKER) push splitsoftware/split-proxy:$(version)"
 	@echo "$(DOCKER) push splitsoftware/split-proxy:latest"
 
+## display unit test coverage derived from last test run (use `make test display-coverage` for up-to-date results)
+display-coverage: coverage.out
+	go tool cover -html=coverage.out
+
 # --------------------------------------------------------------------------
 #
 # Internal targets:
@@ -106,6 +110,8 @@ images_release: # entrypoints
 go.sum: go.mod
 	$(GO) mod tidy
 
+coverage.out: test_coverage
+
 # because of windows .exe suffix, we need a macro on the right side, which needs to be executed
 # after the `%` evaluation, therefore, in a second expansion
 .SECONDEXPANSION:

cmd/proxy/main.go

@@ -34,7 +34,10 @@ func setupConfig(cliArgs *cconf.CliFlags) (*conf.Main, error) {
 	}
 
 	cconf.PopulateFromArguments(&proxyConf, cliArgs.RawConfig)
-	return &proxyConf, nil
+
+	var err error
+	proxyConf.FlagSetsFilter, err = cconf.ValidateFlagsets(proxyConf.FlagSetsFilter)
+	return &proxyConf, err
 }
 
 func main() {
@@ -57,8 +60,13 @@ func main() {
 
 	cfg, err := setupConfig(cliArgs)
 	if err != nil {
-		fmt.Println("error processing config: ", err)
-		os.Exit(exitCodeConfigError)
+		var fsErr cconf.FlagSetValidationError
+		if errors.As(err, &fsErr) {
+			fmt.Println("error processing flagsets: ", err.Error())
+		} else {
+			fmt.Println("error processing config: ", err)
+			os.Exit(exitCodeConfigError)
+		}
 	}
 
 	logger := log.BuildFromConfig(&cfg.Logging, "Split-Proxy", &cfg.Integrations.Slack)

cmd/synchronizer/main.go

@@ -3,9 +3,7 @@ package main
 import (
 	"errors"
 	"fmt"
-	"github.com/splitio/go-split-commons/v5/flagsets"
 	"os"
-	"strings"
 
 	"github.com/splitio/split-synchronizer/v5/splitio"
 	"github.com/splitio/split-synchronizer/v5/splitio/common"
@@ -24,18 +22,6 @@ func parseCliArgs() *cconf.CliFlags {
 	return cconf.ParseCliArgs(&conf.Main{})
 }
 
-type flagSetValidationError struct {
-	wrapped []error
-}
-
-func (f flagSetValidationError) Error() string {
-	var errors []string
-	for _, err := range f.wrapped {
-		errors = append(errors, err.Error())
-	}
-	return strings.Join(errors, ".|| ")
-}
-
 func setupConfig(cliArgs *cconf.CliFlags) (*conf.Main, error) {
 	syncConf := conf.Main{}
 	cconf.PopulateDefaults(&syncConf)
@@ -50,13 +36,7 @@ func setupConfig(cliArgs *cconf.CliFlags) (*conf.Main, error) {
 	cconf.PopulateFromArguments(&syncConf, cliArgs.RawConfig)
 
 	var err error
-	sanitizedFlagSets, fsErr := flagsets.SanitizeMany(syncConf.FlagSetsFilter)
-	if fsErr != nil {
-		err = flagSetValidationError{wrapped: fsErr}
-	}
-	if sanitizedFlagSets != nil {
-		syncConf.FlagSetsFilter = sanitizedFlagSets
-	}
+	syncConf.FlagSetsFilter, err = cconf.ValidateFlagsets(syncConf.FlagSetsFilter)
 	return &syncConf, err
 }
 
@@ -80,7 +60,7 @@ func main() {
 
 	cfg, err := setupConfig(cliArgs)
 	if err != nil {
-		var fsErr flagSetValidationError
+		var fsErr cconf.FlagSetValidationError
 		if errors.As(err, &fsErr) {
 			fmt.Println("error processing flagset: ", err.Error())
 		} else {

go.mod

@@ -38,6 +38,7 @@ require (
 	github.com/pelletier/go-toml/v2 v2.0.8 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/redis/go-redis/v9 v9.0.4 // indirect
+	github.com/stretchr/objx v0.5.0 // indirect
 	github.com/twitchyliquid64/golang-asm v0.15.1 // indirect
 	github.com/ugorji/go/codec v1.2.11 // indirect
 	golang.org/x/arch v0.3.0 // indirect

go.sum

@@ -96,6 +96,7 @@ github.com/splitio/go-toolkit/v5 v5.3.2-0.20231106173125-49e72b9823dc h1:14jdJE/
 github.com/splitio/go-toolkit/v5 v5.3.2-0.20231106173125-49e72b9823dc/go.mod h1:xYhUvV1gga9/1029Wbp5pjnR6Cy8nvBpjw99wAbsMko=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
+github.com/stretchr/objx v0.5.0 h1:1zr/of2m5FGMsad5YfcqgdqdWrIhu+EBEJRhR1U7z/c=
 github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
 github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=

splitio/commitversion.go

@@ -5,4 +5,4 @@ This file is created automatically, please do not edit
 */
 
 // CommitVersion is the version of the last commit previous to release
-const CommitVersion = "03a9105"
+const CommitVersion = "5b37ef1"

splitio/common/conf/validators.go

@@ -0,0 +1,28 @@
+package conf
+
+import (
+	"strings"
+
+	"github.com/splitio/go-split-commons/v5/flagsets"
+)
+
+type FlagSetValidationError struct {
+	wrapped []error
+}
+
+func (f FlagSetValidationError) Error() string {
+	var errors []string
+	for _, err := range f.wrapped {
+		errors = append(errors, err.Error())
+	}
+	return strings.Join(errors, ".|| ")
+}
+
+func ValidateFlagsets(sets []string) ([]string, error) {
+	var toRet error
+	sanitizedFlagSets, fsErr := flagsets.SanitizeMany(sets)
+	if fsErr != nil {
+		toRet = FlagSetValidationError{wrapped: fsErr}
+	}
+	return sanitizedFlagSets, toRet
+}

splitio/common/conf/validators_test.go

@@ -0,0 +1,24 @@
+package conf
+
+import (
+	"testing"
+
+	"github.com/splitio/go-split-commons/v5/dtos"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestFlagSetValidationError(t *testing.T) {
+
+	sanitized, err := ValidateFlagsets([]string{"Flagset1", " flagset2 ", "123#@flagset"})
+	assert.NotNil(t, err)
+	assert.Equal(t, []string{"flagset1", "flagset2"}, sanitized)
+
+	asFVE := err.(FlagSetValidationError)
+	assert.Equal(t, 3, len(asFVE.wrapped))
+	assert.ElementsMatch(t, []error{
+		dtos.FlagSetValidatonError{Message: "Flag Set name Flagset1 should be all lowercase - converting string to lowercase"},
+		dtos.FlagSetValidatonError{Message: "Flag Set name  flagset2  has extra whitespace, trimming"},
+		dtos.FlagSetValidatonError{Message: "you passed 123#@flagset, Flag Set must adhere to the regular expressions ^[a-z0-9][_a-z0-9]{0,49}$. This means a Flag Set must " +
+			"start with a letter or number, be in lowercase, alphanumeric and have a max length of 50 characters. 123#@flagset was discarded."},
+	}, asFVE.wrapped)
+}

splitio/producer/initialization.go

@@ -123,7 +123,7 @@ func Start(logger logging.LoggerInterface, cfg *conf.Main) error {
 	eventEvictionMonitor := evcalc.New(1)
 
 	workers := synchronizer.Workers{
-		SplitUpdater: split.NewSplitUpdater(storages.SplitStorage, splitAPI.SplitFetcher, logger, syncTelemetryStorage, appMonitor, flagsets.NewFlagSetFilter(nil)), // TODO(mredolatti)
+		SplitUpdater: split.NewSplitUpdater(storages.SplitStorage, splitAPI.SplitFetcher, logger, syncTelemetryStorage, appMonitor, flagSetsFilter),
 		SegmentUpdater: segment.NewSegmentUpdater(storages.SplitStorage, storages.SegmentStorage, splitAPI.SegmentFetcher,
 			logger, syncTelemetryStorage, appMonitor),
 		ImpressionsCountRecorder: impressionscount.NewRecorderSingle(impressionsCounter, splitAPI.ImpressionRecorder,

splitio/proxy/caching/caching.go

@@ -34,18 +34,6 @@ func MakeSurrogateForSegmentChanges(segmentName string) string {
 
 // MakeSurrogateForMySegments creates a list surrogate keys for all the segments involved
 func MakeSurrogateForMySegments(mysegments []dtos.MySegmentDTO) []string {
-	if len(mysegments) == 0 {
-		return nil
-	}
-
-	/*
-		surrogates := make([]string, 0, len(mysegments))
-		for idx := range mysegments {
-			surrogates = append(surrogates, segmentPrefix+mysegments[idx].Name)
-		}
-		return surrogates
-	*/
-
 	// Since we are now evicting individually for every updated key, we don't need surrogates for mySegments
 	return nil
 }

splitio/proxy/caching/caching_test.go

@@ -4,31 +4,20 @@ import (
 	"testing"
 
 	"github.com/splitio/go-split-commons/v5/dtos"
-	"github.com/splitio/go-toolkit/v5/testhelpers"
+	"github.com/stretchr/testify/assert"
 )
 
-func TestSegment(t *testing.T) {
-
-	if MakeSurrogateForSegmentChanges("segment1") != segmentPrefix+"segment1" {
-		t.Error("wrong segment changes surrogate.")
-	}
+func TestSegmentSurrogates(t *testing.T) {
+	assert.Equal(t, segmentPrefix+"segment1", MakeSurrogateForSegmentChanges("segment1"))
+	assert.NotEqual(t, MakeSurrogateForSegmentChanges("segment1"), MakeSurrogateForSegmentChanges("segment2"))
 }
 
 func TestMySegmentKeyGeneration(t *testing.T) {
 	entries := MakeMySegmentsEntries("k1")
-	if entries[0] != "/api/mySegments/k1" {
-		t.Error("invalid mySegments cache entry")
-	}
-	if entries[1] != "gzip::/api/mySegments/k1" {
-		t.Error("invalid mySegments cache entry")
-	}
+	assert.Equal(t, "/api/mySegments/k1", entries[0])
+	assert.Equal(t, "gzip::/api/mySegments/k1", entries[1])
 }
 
-func TestMySegments(t *testing.T) {
-	testhelpers.AssertStringSliceEquals(
-		t,
-		MakeSurrogateForMySegments([]dtos.MySegmentDTO{{Name: "segment1"}, {Name: "segment2"}}),
-		[]string{},
-		"wrong my segments surrogate keys",
-	)
+func TestMySegmentsSurrogates(t *testing.T) {
+	assert.Equal(t, []string(nil), MakeSurrogateForMySegments([]dtos.MySegmentDTO{{Name: "segment1"}, {Name: "segment2"}}))
 }

splitio/proxy/caching/workers.go

@@ -31,7 +31,7 @@ func NewCacheAwareSplitSync(
 	flagSetsFilter flagsets.FlagSetFilter,
 ) *CacheAwareSplitSynchronizer {
 	return &CacheAwareSplitSynchronizer{
-		wrapped:      split.NewSplitUpdater(splitStorage, splitFetcher, logger, runtimeTelemetry, appMonitor, flagsets.NewFlagSetFilter(nil)), // TODO(mredolatti): fix this
+		wrapped:      split.NewSplitUpdater(splitStorage, splitFetcher, logger, runtimeTelemetry, appMonitor, flagSetsFilter),
 		splitStorage: splitStorage,
 		cacheFlusher: cacheFlusher,
 	}