From 8bcdd731ac091dcfb6b20dc875249989a5ade3de Mon Sep 17 00:00:00 2001 From: Martin Redolatti Date: Thu, 2 May 2024 14:19:38 -0300 Subject: [PATCH 1/3] ensure caching works for `s` parameter --- splitio/commitversion.go | 2 +- splitio/proxy/caching/caching.go | 30 ++++++++++++++------------- splitio/proxy/caching/caching_test.go | 14 +++++++++++++ 3 files changed, 31 insertions(+), 15 deletions(-) diff --git a/splitio/commitversion.go b/splitio/commitversion.go index 9efa5bb7..dbf9e8ca 100644 --- a/splitio/commitversion.go +++ b/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 = "1ffbd68" +const CommitVersion = "94c6f52" diff --git a/splitio/proxy/caching/caching.go b/splitio/proxy/caching/caching.go index 89d153ec..7752ac2c 100644 --- a/splitio/proxy/caching/caching.go +++ b/splitio/proxy/caching/caching.go @@ -51,23 +51,25 @@ func MakeProxyCache() *gincache.Middleware { return gincache.New(&gincache.Options{ SuccessfulOnly: true, // we're not interested in caching non-200 responses Size: cacheSize, - KeyFactory: func(ctx *gin.Context) string { - - var encodingPrefix string - if strings.Contains(ctx.Request.Header.Get("Accept-Encoding"), "gzip") { - encodingPrefix = "gzip::" - } - - if strings.HasPrefix(ctx.Request.URL.Path, "/api/auth") || strings.HasPrefix(ctx.Request.URL.Path, "/api/v2/auth") { - // For auth requests, since we don't support streaming yet, we only need a single entry in the table, - // so we strip the query-string which contains the user-list - return encodingPrefix + ctx.Request.URL.Path - } - return encodingPrefix + ctx.Request.URL.Path + ctx.Request.URL.RawQuery - }, + KeyFactory: keyFactoryFN, // we make each request handler responsible for generating the surrogates. // this way we can use segment names as surrogates for mysegments & segment changes // with a lot less work SurrogateFactory: func(ctx *gin.Context) []string { return ctx.GetStringSlice(SurrogateContextKey) }, }) } + +func keyFactoryFN(ctx *gin.Context) string { + + var encodingPrefix string + if strings.Contains(ctx.Request.Header.Get("Accept-Encoding"), "gzip") { + encodingPrefix = "gzip::" + } + + if strings.HasPrefix(ctx.Request.URL.Path, "/api/auth") || strings.HasPrefix(ctx.Request.URL.Path, "/api/v2/auth") { + // For auth requests, since we don't support streaming yet, we only need a single entry in the table, + // so we strip the query-string which contains the user-list + return encodingPrefix + ctx.Request.URL.Path + } + return encodingPrefix + ctx.Request.URL.Path + ctx.Request.URL.RawQuery +} diff --git a/splitio/proxy/caching/caching_test.go b/splitio/proxy/caching/caching_test.go index 5d46574b..ee73138e 100644 --- a/splitio/proxy/caching/caching_test.go +++ b/splitio/proxy/caching/caching_test.go @@ -1,12 +1,26 @@ package caching import ( + "net/http" + "net/url" "testing" + "github.com/gin-gonic/gin" "github.com/splitio/go-split-commons/v5/dtos" "github.com/stretchr/testify/assert" ) +func TestCacheKeysDoNotOverlap(t *testing.T) { + + url1, _ := url.Parse("http://proxy.split.io/api/spitChanges?since=-1") + c1 := &gin.Context{Request: &http.Request{URL: url1}} + + url2, _ := url.Parse("http://proxy.split.io/api/spitChanges?s=1.1&since=-1") + c2 := &gin.Context{Request: &http.Request{URL: url2}} + + assert.NotEqual(t, keyFactoryFN(c1), keyFactoryFN(c2)) +} + func TestSegmentSurrogates(t *testing.T) { assert.Equal(t, segmentPrefix+"segment1", MakeSurrogateForSegmentChanges("segment1")) assert.NotEqual(t, MakeSurrogateForSegmentChanges("segment1"), MakeSurrogateForSegmentChanges("segment2")) From d59ba443002203b1f87eec4a863feea9cc8fd520 Mon Sep 17 00:00:00 2001 From: Martin Redolatti Date: Thu, 2 May 2024 15:50:09 -0300 Subject: [PATCH 2/3] proxy: handle `s` in splitChanges endpoint --- go.mod | 2 +- go.sum | 4 + splitio/commitversion.go | 2 +- splitio/proxy/controllers/sdk.go | 34 ++++++ splitio/proxy/controllers/sdk_test.go | 153 ++++++++++++++++++++++++-- 5 files changed, 186 insertions(+), 9 deletions(-) diff --git a/go.mod b/go.mod index 04bf054f..ec0d0b3f 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,7 @@ require ( github.com/gin-gonic/gin v1.9.1 github.com/google/uuid v1.3.0 github.com/splitio/gincache v1.0.1 - github.com/splitio/go-split-commons/v5 v5.2.2-0.20240502153343-221c57a08ca1 + github.com/splitio/go-split-commons/v5 v5.2.2-0.20240502174052-97f050270ae1 github.com/splitio/go-toolkit/v5 v5.4.0 github.com/stretchr/testify v1.8.4 go.etcd.io/bbolt v1.3.6 diff --git a/go.sum b/go.sum index 7d32e1ed..4f8f3e80 100644 --- a/go.sum +++ b/go.sum @@ -90,8 +90,12 @@ github.com/rogpeppe/go-internal v1.8.0 h1:FCbCCtXNOY3UtUuHUYaghJg4y7Fd14rXifAYUA github.com/rogpeppe/go-internal v1.8.0/go.mod h1:WmiCO8CzOY8rg0OYDC4/i/2WRWAB6poM+XZ2dLUbcbE= github.com/splitio/gincache v1.0.1 h1:dLYdANY/BqH4KcUMCe/LluLyV5WtuE/LEdQWRE06IXU= github.com/splitio/gincache v1.0.1/go.mod h1:CcgJDSM9Af75kyBH0724v55URVwMBuSj5x1eCWIOECY= +github.com/splitio/go-split-commons/v5 v5.2.2-0.20240426184453-77f3af03a811 h1:f9MsUfeCGeI3RLmz64hDLC9lyXIM7OGgwb6d7EY7M0M= +github.com/splitio/go-split-commons/v5 v5.2.2-0.20240426184453-77f3af03a811/go.mod h1:344KP05ULARzjRfnC4VtGSyu5l3kmIM375WUIzrURs0= github.com/splitio/go-split-commons/v5 v5.2.2-0.20240502153343-221c57a08ca1 h1:xAayvpboEuhr/vX8DOnNqVLNTsR0mWHcbVxwkEI780g= github.com/splitio/go-split-commons/v5 v5.2.2-0.20240502153343-221c57a08ca1/go.mod h1:344KP05ULARzjRfnC4VtGSyu5l3kmIM375WUIzrURs0= +github.com/splitio/go-split-commons/v5 v5.2.2-0.20240502174052-97f050270ae1 h1:ApuAWj2XxHobGXuoThhpPhuLI95Zkx1YmaQyKzSoQZw= +github.com/splitio/go-split-commons/v5 v5.2.2-0.20240502174052-97f050270ae1/go.mod h1:344KP05ULARzjRfnC4VtGSyu5l3kmIM375WUIzrURs0= github.com/splitio/go-toolkit/v5 v5.4.0 h1:g5WFpRhQomnXCmvfsNOWV4s5AuUrWIZ+amM68G8NBKM= github.com/splitio/go-toolkit/v5 v5.4.0/go.mod h1:xYhUvV1gga9/1029Wbp5pjnR6Cy8nvBpjw99wAbsMko= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= diff --git a/splitio/commitversion.go b/splitio/commitversion.go index f43968a2..c10965dc 100644 --- a/splitio/commitversion.go +++ b/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 = "94c6f52" \ No newline at end of file +const CommitVersion = "59410e1" diff --git a/splitio/proxy/controllers/sdk.go b/splitio/proxy/controllers/sdk.go index 5487d76d..86c40d10 100644 --- a/splitio/proxy/controllers/sdk.go +++ b/splitio/proxy/controllers/sdk.go @@ -9,7 +9,10 @@ import ( "github.com/gin-gonic/gin" "github.com/splitio/go-split-commons/v5/dtos" + "github.com/splitio/go-split-commons/v5/engine/grammar" + "github.com/splitio/go-split-commons/v5/engine/grammar/matchers" "github.com/splitio/go-split-commons/v5/service" + "github.com/splitio/go-split-commons/v5/service/api/specs" "github.com/splitio/go-toolkit/v5/logging" "golang.org/x/exp/slices" @@ -18,6 +21,10 @@ import ( "github.com/splitio/split-synchronizer/v5/splitio/proxy/storage" ) +const ( + labelUnsupportedMatcher = "targeting rule type unsupported by sdk" +) + // SdkServerController bundles all request handler for sdk-server apis type SdkServerController struct { logger logging.LoggerInterface @@ -25,6 +32,7 @@ type SdkServerController struct { proxySplitStorage storage.ProxySplitStorage proxySegmentStorage storage.ProxySegmentStorage fsmatcher flagsets.FlagSetMatcher + versionFilter specs.SplitVersionFilter } // NewSdkServerController instantiates a new sdk server controller @@ -42,6 +50,7 @@ func NewSdkServerController( proxySplitStorage: proxySplitStorage, proxySegmentStorage: proxySegmentStorage, fsmatcher: fsmatcher, + versionFilter: specs.NewSplitVersionFilter(), } } @@ -77,6 +86,13 @@ func (c *SdkServerController) SplitChanges(ctx *gin.Context) { ctx.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return } + + spec, _ := ctx.GetQuery("s") + if spec != specs.FLAG_V1_1 { + spec = specs.FLAG_V1_0 + } + splits.Splits = c.patchUnsupportedMatchers(splits.Splits, spec) + ctx.JSON(http.StatusOK, splits) ctx.Set(caching.SurrogateContextKey, []string{caching.SplitSurrogate}) ctx.Set(caching.StickyContextKey, true) @@ -143,3 +159,21 @@ func (c *SdkServerController) fetchSplitChangesSince(since int64, sets []string) fetchOptions := service.MakeFlagRequestParams().WithChangeNumber(since).WithFlagSetsFilter(strings.Join(sets, ",")) // at this point the sets have been sanitized & sorted return c.fetcher.Fetch(fetchOptions) } + +func (c *SdkServerController) patchUnsupportedMatchers(splits []dtos.SplitDTO, version string) []dtos.SplitDTO { + for si := range splits { + for ci := range splits[si].Conditions { + for mi := range splits[si].Conditions[ci].MatcherGroup.Matchers { + if c.versionFilter.ShouldFilter(splits[si].Conditions[ci].MatcherGroup.Matchers[mi].MatcherType, version) { + splits[si].Conditions[ci].ConditionType = grammar.ConditionTypeWhitelist + splits[si].Conditions[ci].MatcherGroup.Matchers[mi].MatcherType = matchers.MatcherTypeAllKeys + splits[si].Conditions[ci].MatcherGroup.Matchers[mi].String = nil + splits[si].Conditions[ci].Label = labelUnsupportedMatcher + splits[si].Conditions[ci].Partitions = []dtos.PartitionDTO{{Treatment: "control", Size: 100}} + } + } + } + } + + return splits +} diff --git a/splitio/proxy/controllers/sdk_test.go b/splitio/proxy/controllers/sdk_test.go index 3091061b..2f55552d 100644 --- a/splitio/proxy/controllers/sdk_test.go +++ b/splitio/proxy/controllers/sdk_test.go @@ -3,14 +3,17 @@ package controllers import ( "encoding/json" "errors" - "io/ioutil" + "io" "net/http" "net/http/httptest" "testing" "github.com/gin-gonic/gin" "github.com/splitio/go-split-commons/v5/dtos" + "github.com/splitio/go-split-commons/v5/engine/grammar" + "github.com/splitio/go-split-commons/v5/engine/grammar/matchers" "github.com/splitio/go-split-commons/v5/service" + "github.com/splitio/go-split-commons/v5/service/api/specs" "github.com/splitio/go-toolkit/v5/logging" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -52,7 +55,7 @@ func TestSplitChangesRecentSince(t *testing.T) { assert.Equal(t, 200, resp.Code) - body, err := ioutil.ReadAll(resp.Body) + body, err := io.ReadAll(resp.Body) assert.Nil(t, err) var s dtos.SplitChangesDTO @@ -103,7 +106,7 @@ func TestSplitChangesOlderSince(t *testing.T) { assert.Equal(t, 200, resp.Code) - body, err := ioutil.ReadAll(resp.Body) + body, err := io.ReadAll(resp.Body) assert.Nil(t, err) var s dtos.SplitChangesDTO @@ -192,7 +195,7 @@ func TestSplitChangesWithFlagSets(t *testing.T) { assert.Equal(t, 200, resp.Code) - body, err := ioutil.ReadAll(resp.Body) + body, err := io.ReadAll(resp.Body) assert.Nil(t, err) var s dtos.SplitChangesDTO @@ -239,7 +242,7 @@ func TestSplitChangesWithFlagSetsStrict(t *testing.T) { assert.Equal(t, 200, resp.Code) - body, err := ioutil.ReadAll(resp.Body) + body, err := io.ReadAll(resp.Body) assert.Nil(t, err) var s dtos.SplitChangesDTO @@ -252,6 +255,142 @@ func TestSplitChangesWithFlagSetsStrict(t *testing.T) { splitFetcher.AssertExpectations(t) } +func TestSplitChangesNewMatcherOldSpec(t *testing.T) { + gin.SetMode(gin.TestMode) + + var splitStorage psmocks.ProxySplitStorageMock + splitStorage.On("ChangesSince", int64(-1), []string(nil)). + Return(&dtos.SplitChangesDTO{ + Since: -1, + Till: 1, + Splits: []dtos.SplitDTO{ + { + Name: "s1", + Status: "ACTIVE", + Conditions: []dtos.ConditionDTO{ + { + MatcherGroup: dtos.MatcherGroupDTO{Matchers: []dtos.MatcherDTO{{MatcherType: matchers.MatcherTypeGreaterThanOrEqualToSemver}}}, + Partitions: []dtos.PartitionDTO{{Treatment: "on", Size: 100}}, + Label: "some label", + }, + }}, + }, + }, nil). + Once() + + var splitFetcher splitFetcherMock + + resp := httptest.NewRecorder() + ctx, router := gin.CreateTestContext(resp) + logger := logging.NewLogger(nil) + group := router.Group("/api") + controller := NewSdkServerController( + logger, + &splitFetcher, + &splitStorage, + nil, + flagsets.NewMatcher(false, nil), + ) + controller.Register(group) + + ctx.Request, _ = http.NewRequest(http.MethodGet, "/api/splitChanges?since=-1", nil) + ctx.Request.Header.Set("Authorization", "Bearer someApiKey") + ctx.Request.Header.Set("SplitSDKVersion", "go-1.1.1") + ctx.Request.Header.Set("SplitSDKMachineIp", "1.2.3.4") + ctx.Request.Header.Set("SplitSDKMachineName", "ip-1-2-3-4") + router.ServeHTTP(resp, ctx.Request) + + assert.Equal(t, 200, resp.Code) + + body, err := io.ReadAll(resp.Body) + assert.Nil(t, err) + + var s dtos.SplitChangesDTO + err = json.Unmarshal(body, &s) + assert.Nil(t, err) + assert.Equal(t, 1, len(s.Splits)) + assert.Equal(t, int64(-1), s.Since) + assert.Equal(t, int64(1), s.Till) + + cond := s.Splits[0].Conditions[0] + assert.Equal(t, grammar.ConditionTypeWhitelist, cond.ConditionType) + assert.Equal(t, matchers.MatcherTypeAllKeys, cond.MatcherGroup.Matchers[0].MatcherType) + assert.Equal(t, labelUnsupportedMatcher, cond.Label) + assert.Equal(t, []dtos.PartitionDTO{{Treatment: "control", Size: 100}}, cond.Partitions) + + splitStorage.AssertExpectations(t) + splitFetcher.AssertExpectations(t) +} + +func TestSplitChangesNewMatcherNewSpec(t *testing.T) { + gin.SetMode(gin.TestMode) + + var splitStorage psmocks.ProxySplitStorageMock + splitStorage.On("ChangesSince", int64(-1), []string(nil)). + Return(&dtos.SplitChangesDTO{ + Since: -1, + Till: 1, + Splits: []dtos.SplitDTO{ + { + Name: "s1", + Status: "ACTIVE", + Conditions: []dtos.ConditionDTO{ + { + MatcherGroup: dtos.MatcherGroupDTO{Matchers: []dtos.MatcherDTO{{MatcherType: matchers.MatcherTypeGreaterThanOrEqualToSemver}}}, + Partitions: []dtos.PartitionDTO{{Treatment: "on", Size: 100}}, + Label: "some label", + }, + }}, + }, + }, nil). + Once() + + var splitFetcher splitFetcherMock + + resp := httptest.NewRecorder() + ctx, router := gin.CreateTestContext(resp) + logger := logging.NewLogger(nil) + group := router.Group("/api") + controller := NewSdkServerController( + logger, + &splitFetcher, + &splitStorage, + nil, + flagsets.NewMatcher(false, nil), + ) + controller.Register(group) + + ctx.Request, _ = http.NewRequest(http.MethodGet, "/api/splitChanges?since=-1", nil) + ctx.Request.Header.Set("Authorization", "Bearer someApiKey") + ctx.Request.Header.Set("SplitSDKVersion", "go-1.1.1") + ctx.Request.Header.Set("SplitSDKMachineIp", "1.2.3.4") + ctx.Request.Header.Set("SplitSDKMachineName", "ip-1-2-3-4") + q := ctx.Request.URL.Query() + q.Add("s", specs.FLAG_V1_1) + ctx.Request.URL.RawQuery = q.Encode() + router.ServeHTTP(resp, ctx.Request) + + assert.Equal(t, 200, resp.Code) + + body, err := io.ReadAll(resp.Body) + assert.Nil(t, err) + + var s dtos.SplitChangesDTO + err = json.Unmarshal(body, &s) + assert.Nil(t, err) + assert.Equal(t, 1, len(s.Splits)) + assert.Equal(t, int64(-1), s.Since) + assert.Equal(t, int64(1), s.Till) + + cond := s.Splits[0].Conditions[0] + assert.Equal(t, matchers.MatcherTypeGreaterThanOrEqualToSemver, cond.MatcherGroup.Matchers[0].MatcherType) + assert.Equal(t, "some label", cond.Label) + assert.Equal(t, []dtos.PartitionDTO{{Treatment: "on", Size: 100}}, cond.Partitions) + + splitStorage.AssertExpectations(t) + splitFetcher.AssertExpectations(t) +} + func TestSegmentChanges(t *testing.T) { gin.SetMode(gin.TestMode) @@ -280,7 +419,7 @@ func TestSegmentChanges(t *testing.T) { assert.Equal(t, 200, resp.Code) - body, err := ioutil.ReadAll(resp.Body) + body, err := io.ReadAll(resp.Body) assert.Nil(t, err) var s dtos.SegmentChangesDTO @@ -353,7 +492,7 @@ func TestMySegments(t *testing.T) { router.ServeHTTP(resp, ctx.Request) assert.Equal(t, 200, resp.Code) - body, err := ioutil.ReadAll(resp.Body) + body, err := io.ReadAll(resp.Body) assert.Nil(t, err) var ms MSC From 8a8bf6b9dffec7b602163a1fa9cdef3262b4f9f5 Mon Sep 17 00:00:00 2001 From: Martin Redolatti Date: Thu, 2 May 2024 16:01:54 -0300 Subject: [PATCH 3/3] update go.sum --- go.sum | 4 ---- 1 file changed, 4 deletions(-) diff --git a/go.sum b/go.sum index 4f8f3e80..31ef8150 100644 --- a/go.sum +++ b/go.sum @@ -90,10 +90,6 @@ github.com/rogpeppe/go-internal v1.8.0 h1:FCbCCtXNOY3UtUuHUYaghJg4y7Fd14rXifAYUA github.com/rogpeppe/go-internal v1.8.0/go.mod h1:WmiCO8CzOY8rg0OYDC4/i/2WRWAB6poM+XZ2dLUbcbE= github.com/splitio/gincache v1.0.1 h1:dLYdANY/BqH4KcUMCe/LluLyV5WtuE/LEdQWRE06IXU= github.com/splitio/gincache v1.0.1/go.mod h1:CcgJDSM9Af75kyBH0724v55URVwMBuSj5x1eCWIOECY= -github.com/splitio/go-split-commons/v5 v5.2.2-0.20240426184453-77f3af03a811 h1:f9MsUfeCGeI3RLmz64hDLC9lyXIM7OGgwb6d7EY7M0M= -github.com/splitio/go-split-commons/v5 v5.2.2-0.20240426184453-77f3af03a811/go.mod h1:344KP05ULARzjRfnC4VtGSyu5l3kmIM375WUIzrURs0= -github.com/splitio/go-split-commons/v5 v5.2.2-0.20240502153343-221c57a08ca1 h1:xAayvpboEuhr/vX8DOnNqVLNTsR0mWHcbVxwkEI780g= -github.com/splitio/go-split-commons/v5 v5.2.2-0.20240502153343-221c57a08ca1/go.mod h1:344KP05ULARzjRfnC4VtGSyu5l3kmIM375WUIzrURs0= github.com/splitio/go-split-commons/v5 v5.2.2-0.20240502174052-97f050270ae1 h1:ApuAWj2XxHobGXuoThhpPhuLI95Zkx1YmaQyKzSoQZw= github.com/splitio/go-split-commons/v5 v5.2.2-0.20240502174052-97f050270ae1/go.mod h1:344KP05ULARzjRfnC4VtGSyu5l3kmIM375WUIzrURs0= github.com/splitio/go-toolkit/v5 v5.4.0 h1:g5WFpRhQomnXCmvfsNOWV4s5AuUrWIZ+amM68G8NBKM=