From 98a0609b59a50f39c66d536170f13958b1ec29ec Mon Sep 17 00:00:00 2001 From: David Hadas Date: Fri, 6 Jan 2023 17:05:20 +0200 Subject: [PATCH] Merge Fuse with Learn (#120) * record numSamples * removed fuse * removed fuse * rm fuse in envelop * fix e2e test * revert e2e crd * better log for loadConfig * fixed Mutex issues * fixed Mutex issues --- cmd/guard-service/services.go | 59 ++++++++++-------- pkg/apis/guard/v1alpha1/asciiFlags.go | 11 +--- pkg/apis/guard/v1alpha1/body.go | 20 ------ pkg/apis/guard/v1alpha1/count.go | 62 +++++-------------- pkg/apis/guard/v1alpha1/envelop.go | 9 --- pkg/apis/guard/v1alpha1/flagSlice.go | 12 +--- pkg/apis/guard/v1alpha1/guardianSpec.go | 1 + pkg/apis/guard/v1alpha1/httpHeaders.go | 8 --- pkg/apis/guard/v1alpha1/httpMediaType.go | 9 --- pkg/apis/guard/v1alpha1/httpReq.go | 16 ----- pkg/apis/guard/v1alpha1/httpResp.go | 10 --- pkg/apis/guard/v1alpha1/ipSet.go | 34 +--------- pkg/apis/guard/v1alpha1/keyval.go | 56 +++-------------- pkg/apis/guard/v1alpha1/limit.go | 8 --- pkg/apis/guard/v1alpha1/pod.go | 13 ---- pkg/apis/guard/v1alpha1/pod_test.go | 8 +-- pkg/apis/guard/v1alpha1/queryString.go | 8 --- pkg/apis/guard/v1alpha1/sessionData.go | 15 +---- pkg/apis/guard/v1alpha1/set.go | 25 ++------ pkg/apis/guard/v1alpha1/simpleVal.go | 17 ----- pkg/apis/guard/v1alpha1/structuredProfiler.go | 36 +++-------- pkg/apis/guard/v1alpha1/url.go | 9 --- pkg/apis/guard/v1alpha1/v1alpha1.go | 7 +-- pkg/apis/guard/v1alpha1/v1alpha1_test.go | 16 +++-- pkg/guard-gate/client.go | 10 ++- pkg/guard-utils/stats.go | 4 +- 26 files changed, 97 insertions(+), 386 deletions(-) diff --git a/cmd/guard-service/services.go b/cmd/guard-service/services.go index bd83ae84..583d3ba0 100644 --- a/cmd/guard-service/services.go +++ b/cmd/guard-service/services.go @@ -24,7 +24,10 @@ import ( pi "knative.dev/security-guard/pkg/pluginterfaces" ) -var maxPileCount = uint32(1000) +const ( + pileMergeLimit = uint32(1000) + numSamplesLimit = uint32(1000000) +) // A cached record kept by guard-service for each deployed service type serviceRecord struct { @@ -72,7 +75,6 @@ func (s *services) tick() { // Tick should not include any asynchronous work // Move all asynchronous work (e.g. KubeApi work) to go routines s.mutex.Lock() - defer s.mutex.Unlock() if len(s.tickerKeys) == 0 { // Assign more work to be done now and in future ticks @@ -90,18 +92,28 @@ func (s *services) tick() { maxIterations = 100 } - // try to learn one record - i := 0 + // find a record to learn + i := 0 // i is the index of the record to learn + var record *serviceRecord for ; i < maxIterations; i++ { - if record, exists := s.cache[s.tickerKeys[i]]; exists { - // we still have this record, lets learn it - if s.learnPile(record) { - // we learned one record + r, exists := s.cache[s.tickerKeys[i]] + if exists { + if r.pile.Count != 0 { + // we will learn this record! + record = r + // (during the next tick we should try the next one) i++ break } } } + s.mutex.Unlock() + // Must unlock s.mutex before s.learnPile + + if record != nil { + // lets learn it + s.learnPile(record) + } // remove the keys we processed from the key slice s.tickerKeys = s.tickerKeys[i:] @@ -132,6 +144,7 @@ func (s *services) get(ns string, sid string, cmFlag bool) *serviceRecord { // try to get from cache record := s.cache[service] s.mutex.Unlock() + // Must unlock s.mutex before s.kmgr.Watch, s.kmgr.GetGuardian, s.set // watch any unknown namespace if !knownNamespace { @@ -153,6 +166,7 @@ func (s *services) set(ns string, sid string, cmFlag bool, guardianSpec *spec.Gu service := serviceKey(ns, sid, cmFlag) s.mutex.Lock() + defer s.mutex.Unlock() record, exists := s.cache[service] if !exists { record = new(serviceRecord) @@ -162,7 +176,6 @@ func (s *services) set(ns string, sid string, cmFlag bool, guardianSpec *spec.Gu record.cmFlag = cmFlag s.cache[service] = record } - s.mutex.Unlock() record.guardianSpec = guardianSpec pi.Log.Debugf("cache record for %s.%s", ns, sid) @@ -184,7 +197,9 @@ func (s *services) merge(record *serviceRecord, pile *spec.SessionDataPile) { record.pileMutex.Lock() record.pile.Merge(pile) record.pileMutex.Unlock() - if record.pile.Count > maxPileCount { + // Must unlock pileMutex before s.learnPile + + if record.pile.Count > pileMergeLimit { s.learnPile(record) } } @@ -192,29 +207,23 @@ func (s *services) merge(record *serviceRecord, pile *spec.SessionDataPile) { // update the record guardianSpec by learning a new config and fusing with the record existing config // update KubeAPI as well. // return true if we try to learn and access kubeApi, false if count is zero and we have nothing to do -func (s *services) learnPile(record *serviceRecord) bool { - if record.pile.Count == 0 { - return false +func (s *services) learnPile(record *serviceRecord) { + if record.guardianSpec.Learned == nil { + record.guardianSpec.Learned = new(spec.SessionDataConfig) } - config := new(spec.SessionDataConfig) record.pileMutex.Lock() - config.Learn(&record.pile) + record.guardianSpec.Learned.Learn(&record.pile) + record.guardianSpec.NumSamples += record.pile.Count + if record.guardianSpec.NumSamples > numSamplesLimit { + record.guardianSpec.NumSamples = numSamplesLimit + } record.pile.Clear() record.pileMutex.Unlock() - - if record.guardianSpec.Learned != nil { - config.Fuse(record.guardianSpec.Learned) - } - - // update the cached record - record.guardianSpec.Learned = config - record.guardianSpec.Learned.Active = true + // Must unlock record.pileMutex before s.persist // update the kubeApi record go s.persist(record) - - return true } func (s *services) persist(record *serviceRecord) { diff --git a/pkg/apis/guard/v1alpha1/asciiFlags.go b/pkg/apis/guard/v1alpha1/asciiFlags.go index 3a40a52b..f5d9d3ad 100644 --- a/pkg/apis/guard/v1alpha1/asciiFlags.go +++ b/pkg/apis/guard/v1alpha1/asciiFlags.go @@ -134,16 +134,7 @@ func (config *AsciiFlagsConfig) learnI(valPile ValuePile) { // pile is RO and unchanged - never uses pile internal objects func (config *AsciiFlagsConfig) Learn(pile AsciiFlagsPile) { - *config = AsciiFlagsConfig(pile) -} - -func (config *AsciiFlagsConfig) fuseI(otherValConfig ValueConfig) { - config.Fuse(*otherValConfig.(*AsciiFlagsConfig)) -} - -// otherConfig is RO and unchanged - never uses otherConfig internal objects -func (config *AsciiFlagsConfig) Fuse(otherConfig AsciiFlagsConfig) { - *config |= otherConfig + *config |= AsciiFlagsConfig(pile) } func (config *AsciiFlagsConfig) Prepare() { diff --git a/pkg/apis/guard/v1alpha1/body.go b/pkg/apis/guard/v1alpha1/body.go index b938c3a2..86ad7cda 100644 --- a/pkg/apis/guard/v1alpha1/body.go +++ b/pkg/apis/guard/v1alpha1/body.go @@ -150,26 +150,6 @@ func (config *BodyConfig) Learn(pile *BodyPile) { } } -func (config *BodyConfig) fuseI(otherValConfig ValueConfig) { - config.Fuse(otherValConfig.(*BodyConfig)) -} - -// otherConfig is RO and unchanged - never uses otherConfig internal objects -func (config *BodyConfig) Fuse(otherConfig *BodyConfig) { - if otherConfig.Structured != nil { - if config.Structured == nil { - config.Structured = new(StructuredConfig) - } - config.Structured.Fuse(otherConfig.Structured) - } - if otherConfig.Unstructured != nil { - if config.Unstructured == nil { - config.Unstructured = new(SimpleValConfig) - } - config.Unstructured.Fuse(otherConfig.Unstructured) - } -} - func (config *BodyConfig) Prepare() { if config.Structured != nil { config.Structured.Prepare() diff --git a/pkg/apis/guard/v1alpha1/count.go b/pkg/apis/guard/v1alpha1/count.go index db5ba0f9..fab70b5b 100644 --- a/pkg/apis/guard/v1alpha1/count.go +++ b/pkg/apis/guard/v1alpha1/count.go @@ -62,22 +62,6 @@ type countRange struct { Max uint8 `json:"max"` } -func (cRange *countRange) fuseTwoRanges(otherRange *countRange) bool { - if cRange.Max < otherRange.Min || cRange.Min > otherRange.Max { - // no overlap - nothing to do! - return false - } - - // There is overlap of some sort - if cRange.Min > otherRange.Min { - cRange.Min = otherRange.Min - } - if cRange.Max < otherRange.Max { - cRange.Max = otherRange.Max - } - return true -} - // Exposes ValueConfig interface type CountConfig []countRange @@ -116,14 +100,14 @@ func (config *CountConfig) learnI(valPile ValuePile) { // Learn now offers the simplest single rule support // pile is RO and unchanged - never uses pile internal objects -// Future: Improve Learn +// Future: Improve Learn - e.g. by supporting more then one range func (config *CountConfig) Learn(pile CountPile) { - min := uint8(0) - max := uint8(0) - if len(pile) > 0 { - min = pile[0] - max = pile[0] + if len(pile) == 0 { + return } + + min := pile[0] + max := pile[0] for _, v := range pile { if min > v { min = v @@ -132,33 +116,17 @@ func (config *CountConfig) Learn(pile CountPile) { max = v } } - *config = append(*config, countRange{min, max}) -} -func (config *CountConfig) fuseI(otherValConfig ValueConfig) { - config.Fuse(*otherValConfig.(*CountConfig)) -} + if *config == nil { + *config = append(*config, countRange{min, max}) + return + } -// Fuse CountConfig in-place -// otherValConfig is RO and unchanged - never uses otherValConfig internal objects -// The implementation look to opportunistically merge new entries to existing ones -// The implementation does now squash entries even if after the Fuse they may be squashed -// This is done to achieve Fuse in-place -// Future: Improve Fuse - e.g. by keeping extra entries in Range [0,0] and reusing them instead of adding new entries -func (config *CountConfig) Fuse(otherConfig CountConfig) { - var fused bool - for _, other := range otherConfig { - fused = false - for idx, this := range *config { - if fused = this.fuseTwoRanges(&other); fused { - (*config)[idx] = this - break - } - } - if !fused { - // Creating new countRange avoids both objects pointing to the same object - *config = append(*config, countRange{other.Min, other.Max}) - } + if min < (*config)[0].Min { + (*config)[0].Min = min + } + if max > (*config)[0].Max { + (*config)[0].Max = max } } diff --git a/pkg/apis/guard/v1alpha1/envelop.go b/pkg/apis/guard/v1alpha1/envelop.go index fd8c92fb..df2f6b9c 100644 --- a/pkg/apis/guard/v1alpha1/envelop.go +++ b/pkg/apis/guard/v1alpha1/envelop.go @@ -102,15 +102,6 @@ func (config *EnvelopConfig) Learn(pile *EnvelopPile) { config.ResponseTime.Learn(pile.ResponseTime) } -func (config *EnvelopConfig) fuseI(otherValConfig ValueConfig) { - config.Fuse(otherValConfig.(*EnvelopConfig)) -} - -func (config *EnvelopConfig) Fuse(otherConfig *EnvelopConfig) { - config.CompletionTime.Fuse(&otherConfig.CompletionTime) - config.ResponseTime.Fuse(&otherConfig.ResponseTime) -} - func (config *EnvelopConfig) Prepare() { config.CompletionTime.Prepare() config.ResponseTime.Prepare() diff --git a/pkg/apis/guard/v1alpha1/flagSlice.go b/pkg/apis/guard/v1alpha1/flagSlice.go index aba43435..8df760fe 100644 --- a/pkg/apis/guard/v1alpha1/flagSlice.go +++ b/pkg/apis/guard/v1alpha1/flagSlice.go @@ -101,17 +101,7 @@ func (config *FlagSliceConfig) learnI(valPile ValuePile) { // otherPile is RO and unchanged - never uses otherPile internal objects func (config *FlagSliceConfig) Learn(pile FlagSlicePile) { - *config = make(FlagSliceConfig, len(pile)) - copy(*config, pile) -} - -func (config *FlagSliceConfig) fuseI(otherValConfig ValueConfig) { - config.Fuse(*otherValConfig.(*FlagSliceConfig)) -} - -// otherConfig is RO and unchanged - never uses otherConfig internal objects -func (config *FlagSliceConfig) Fuse(otherConfig FlagSliceConfig) { - *config = mergeFlagSlices(*config, otherConfig) + *config = mergeFlagSlices(*config, pile) } func (config *FlagSliceConfig) Prepare() { diff --git a/pkg/apis/guard/v1alpha1/guardianSpec.go b/pkg/apis/guard/v1alpha1/guardianSpec.go index 683c6178..b9b61cff 100644 --- a/pkg/apis/guard/v1alpha1/guardianSpec.go +++ b/pkg/apis/guard/v1alpha1/guardianSpec.go @@ -27,6 +27,7 @@ type Ctrl struct { type GuardianSpec struct { Configured *SessionDataConfig `json:"configured"` // configrued criteria Learned *SessionDataConfig `json:"learned,omitempty"` // Learned citeria + NumSamples uint32 `json:"samples"` // Number of Samples Learned Control *Ctrl `json:"control"` // Control } diff --git a/pkg/apis/guard/v1alpha1/httpHeaders.go b/pkg/apis/guard/v1alpha1/httpHeaders.go index 312a6fab..e56c13b2 100644 --- a/pkg/apis/guard/v1alpha1/httpHeaders.go +++ b/pkg/apis/guard/v1alpha1/httpHeaders.go @@ -93,14 +93,6 @@ func (config *HeadersConfig) Learn(pile *HeadersPile) { config.Kv.Learn(pile.Kv) } -func (config *HeadersConfig) fuseI(otherValConfig ValueConfig) { - config.Fuse(otherValConfig.(*HeadersConfig)) -} - -func (config *HeadersConfig) Fuse(otherConfig *HeadersConfig) { - config.Kv.Fuse(&otherConfig.Kv) -} - func (config *HeadersConfig) Prepare() { config.Kv.Prepare() } diff --git a/pkg/apis/guard/v1alpha1/httpMediaType.go b/pkg/apis/guard/v1alpha1/httpMediaType.go index 1596a8ce..8151850f 100644 --- a/pkg/apis/guard/v1alpha1/httpMediaType.go +++ b/pkg/apis/guard/v1alpha1/httpMediaType.go @@ -104,15 +104,6 @@ func (config *MediaTypeConfig) Learn(pile *MediaTypePile) { config.Params.Learn(&pile.Params) } -func (config *MediaTypeConfig) fuseI(otherValConfig ValueConfig) { - config.Fuse(otherValConfig.(*MediaTypeConfig)) -} - -func (config *MediaTypeConfig) Fuse(otherConfig *MediaTypeConfig) { - config.TypeTokens.Fuse(&otherConfig.TypeTokens) - config.Params.Fuse(&otherConfig.Params) -} - func (config *MediaTypeConfig) Prepare() { config.TypeTokens.Prepare() config.Params.Prepare() diff --git a/pkg/apis/guard/v1alpha1/httpReq.go b/pkg/apis/guard/v1alpha1/httpReq.go index f6e2632e..1dc90500 100644 --- a/pkg/apis/guard/v1alpha1/httpReq.go +++ b/pkg/apis/guard/v1alpha1/httpReq.go @@ -180,22 +180,6 @@ func (config *ReqConfig) Learn(pile *ReqPile) { config.Url.Learn(&pile.Url) } -func (config *ReqConfig) fuseI(otherValConfig ValueConfig) { - config.Fuse(otherValConfig.(*ReqConfig)) -} - -func (config *ReqConfig) Fuse(otherConfig *ReqConfig) { - config.ClientIp.Fuse(&otherConfig.ClientIp) - config.HopIp.Fuse(&otherConfig.HopIp) - config.Method.Fuse(&otherConfig.Method) - config.Proto.Fuse(&otherConfig.Proto) - config.MediaType.Fuse(&otherConfig.MediaType) - config.ContentLength.Fuse(otherConfig.ContentLength) - config.Headers.Fuse(&otherConfig.Headers) - config.Qs.Fuse(&otherConfig.Qs) - config.Url.Fuse(&otherConfig.Url) -} - func (config *ReqConfig) Prepare() { config.ClientIp.Prepare() config.HopIp.Prepare() diff --git a/pkg/apis/guard/v1alpha1/httpResp.go b/pkg/apis/guard/v1alpha1/httpResp.go index ed675ace..d31312c4 100644 --- a/pkg/apis/guard/v1alpha1/httpResp.go +++ b/pkg/apis/guard/v1alpha1/httpResp.go @@ -117,16 +117,6 @@ func (config *RespConfig) Learn(pile *RespPile) { config.ContentLength.Learn(pile.ContentLength) } -func (config *RespConfig) fuseI(otherValConfig ValueConfig) { - config.Fuse(otherValConfig.(*RespConfig)) -} - -func (config *RespConfig) Fuse(otherConfig *RespConfig) { - config.Headers.Fuse(&otherConfig.Headers) - config.MediaType.Fuse(&otherConfig.MediaType) - config.ContentLength.Fuse(otherConfig.ContentLength) -} - func (config *RespConfig) Prepare() { config.Headers.Prepare() config.MediaType.Prepare() diff --git a/pkg/apis/guard/v1alpha1/ipSet.go b/pkg/apis/guard/v1alpha1/ipSet.go index cd6ad67a..48634c04 100644 --- a/pkg/apis/guard/v1alpha1/ipSet.go +++ b/pkg/apis/guard/v1alpha1/ipSet.go @@ -226,13 +226,14 @@ LoopProfileIPs: // Learn currently offers a rough and simple CIDR support // Learn try to add IPs to current CIDRs by inflating the CIDRs. // When no CIDR can be inflated to include the IP, Learn adds a new CIDR for this IP +// Future: Improve Learn to squash consecutive cidrs + func (config *IpSetConfig) learnI(valPile ValuePile) { config.Learn(valPile.(*IpSetPile)) } // pile is RO and unchanged - never uses pile internal objects func (config *IpSetConfig) Learn(pile *IpSetPile) { - *config = nil LoopPileIPs: for _, ip := range pile.List { for _, cidr := range *config { @@ -251,36 +252,5 @@ LoopPileIPs: } } -func (config *IpSetConfig) fuseI(otherValConfig ValueConfig) { - config.Fuse(otherValConfig.(*IpSetConfig)) -} - -// Fuse CidrSetConfig -// otherConfig is RO and unchanged - never uses otherConfig internal objects -// The implementation look to opportunistically skip new entries -// The implementation does not squash new and old entries -// Future: Improve Fuse to squash consecutive cidrs -func (config *IpSetConfig) Fuse(otherConfig *IpSetConfig) { -LoopOtherCidrs: - for _, otherCidr := range *otherConfig { - for idx, myCidr := range *config { - if myCidr.InflateBy(otherCidr.IP) { - if myCidr.InflateBy(otherCidr.lastIP()) { - continue LoopOtherCidrs - } - } - if myCidr.Include(otherCidr) { - continue LoopOtherCidrs - } - if otherCidr.Include(myCidr) { - (*config)[idx] = otherCidr - continue LoopOtherCidrs - } - } - // Add a copy of the otherCidr to my list of CIDRs - *config = append(*config, *otherCidr.DeepCopy()) - } -} - func (config *IpSetConfig) Prepare() { } diff --git a/pkg/apis/guard/v1alpha1/keyval.go b/pkg/apis/guard/v1alpha1/keyval.go index ebd0fffb..5a647f4d 100644 --- a/pkg/apis/guard/v1alpha1/keyval.go +++ b/pkg/apis/guard/v1alpha1/keyval.go @@ -172,61 +172,21 @@ func (config *KeyValConfig) learnI(valPile ValuePile) { // aggregating all known keys which have common low security fingerprint into // OtherKeynames and OtherVals func (config *KeyValConfig) Learn(pile *KeyValPile) { - config.OtherVals = nil - config.OtherKeynames = nil - - if pile == nil { - config.Vals = nil + if pile == nil || len(*pile) == 0 { return } // learn known keys - config.Vals = make(map[string]*SimpleValConfig, len(*pile)) - for k, v := range *pile { - svc := new(SimpleValConfig) - svc.Learn(v) - config.Vals[k] = svc - } -} - -func (config *KeyValConfig) fuseI(otherValConfig ValueConfig) { - config.Fuse(otherValConfig.(*KeyValConfig)) -} - -// otherConfig is RO and unchanged - never uses otherConfig internal objects -func (config *KeyValConfig) Fuse(otherConfig *KeyValConfig) { - if otherConfig == nil { - return - } if config.Vals == nil { - config.Vals = make(map[string]*SimpleValConfig, len(otherConfig.Vals)) - } - // fuse known keys - for k, v := range otherConfig.Vals { - svc, exists := config.Vals[k] - if exists { - svc.Fuse(v) - } else { - svc := new(SimpleValConfig) - svc.Fuse(v) - config.Vals[k] = svc - } - } - - // fuse keynames of unknown keys - if otherConfig.OtherKeynames != nil { - if config.OtherKeynames == nil { - config.OtherKeynames = new(SimpleValConfig) - } - config.OtherKeynames.Fuse(otherConfig.OtherKeynames) + config.Vals = make(map[string]*SimpleValConfig, len(*pile)) } - - // fuse key values of unknown keys - if otherConfig.OtherVals != nil { - if config.OtherVals == nil { - config.OtherVals = new(SimpleValConfig) + for k, v := range *pile { + svc, ok := config.Vals[k] + if !ok { + svc = new(SimpleValConfig) + config.Vals[k] = svc } - config.OtherVals.Fuse(otherConfig.OtherVals) + svc.Learn(v) } } diff --git a/pkg/apis/guard/v1alpha1/limit.go b/pkg/apis/guard/v1alpha1/limit.go index 3b6e5607..2271f935 100644 --- a/pkg/apis/guard/v1alpha1/limit.go +++ b/pkg/apis/guard/v1alpha1/limit.go @@ -117,13 +117,5 @@ func (config *LimitConfig) Learn(pile LimitPile) { } } -func (config *LimitConfig) fuseI(otherValConfig ValueConfig) { - config.Fuse(otherValConfig.(*LimitConfig)) -} - -func (config *LimitConfig) Fuse(otherConfig *LimitConfig) { - -} - func (config *LimitConfig) Prepare() { } diff --git a/pkg/apis/guard/v1alpha1/pod.go b/pkg/apis/guard/v1alpha1/pod.go index 97df44fe..cf270795 100644 --- a/pkg/apis/guard/v1alpha1/pod.go +++ b/pkg/apis/guard/v1alpha1/pod.go @@ -239,19 +239,6 @@ func (config *PodConfig) Learn(pile *PodPile) { config.Udplite6Peers.Learn(&pile.Udplite6Peers) } -func (config *PodConfig) fuseI(otherValConfig ValueConfig) { - config.Fuse(otherValConfig.(*PodConfig)) -} - -func (config *PodConfig) Fuse(otherConfig *PodConfig) { - config.Tcp4Peers.Fuse(&otherConfig.Tcp4Peers) - config.Udp4Peers.Fuse(&otherConfig.Udp4Peers) - config.Udplite4Peers.Fuse(&otherConfig.Udplite4Peers) - config.Tcp6Peers.Fuse(&otherConfig.Tcp6Peers) - config.Udp6Peers.Fuse(&otherConfig.Udp6Peers) - config.Udplite6Peers.Fuse(&otherConfig.Udplite6Peers) -} - func (config *PodConfig) Prepare() { config.Tcp4Peers.Prepare() config.Udp4Peers.Prepare() diff --git a/pkg/apis/guard/v1alpha1/pod_test.go b/pkg/apis/guard/v1alpha1/pod_test.go index 7e7659bd..d7695fd3 100644 --- a/pkg/apis/guard/v1alpha1/pod_test.go +++ b/pkg/apis/guard/v1alpha1/pod_test.go @@ -67,8 +67,6 @@ func TestPod_V1(t *testing.T) { var config1 PodConfig var config1a PodConfig var config2 PodConfig - var config2a PodConfig - var config34 PodConfig var config56 PodConfig updateAll(data1) @@ -116,8 +114,7 @@ func TestPod_V1(t *testing.T) { pile1.Add(&profile1a) pile2.Add(&profile2a) config1a.Learn(&pile1) - config2a.Learn(&pile2) - config1a.Fuse(&config2a) + config1a.Learn(&pile2) if d := config1a.Decide(&profile1); d != nil { t.Errorf(d.String("Decide expected to ok but returned error")) } @@ -133,13 +130,12 @@ func TestPod_V1(t *testing.T) { pile2.Clear() pile1.Add(&profile3) pile2.Add(&profile4) - config34.Learn(&pile2) + config56.Learn(&pile2) pile1.Clear() pile2.Clear() pile1.Add(&profile5) pile2.Add(&profile6) config56.Learn(&pile2) - config56.Fuse(&config34) if d := config56.Decide(&profile3); d != nil { t.Errorf(d.String("Decide expected to ok but returned error")) diff --git a/pkg/apis/guard/v1alpha1/queryString.go b/pkg/apis/guard/v1alpha1/queryString.go index 7d3f831b..a5b1cd9f 100644 --- a/pkg/apis/guard/v1alpha1/queryString.go +++ b/pkg/apis/guard/v1alpha1/queryString.go @@ -93,14 +93,6 @@ func (config *QueryConfig) Learn(pile *QueryPile) { config.Kv.Learn(pile.Kv) } -func (config *QueryConfig) fuseI(otherValConfig ValueConfig) { - config.Fuse(otherValConfig.(*QueryConfig)) -} - -func (config *QueryConfig) Fuse(otherConfig *QueryConfig) { - config.Kv.Fuse(&otherConfig.Kv) -} - func (config *QueryConfig) Prepare() { config.Kv.Prepare() } diff --git a/pkg/apis/guard/v1alpha1/sessionData.go b/pkg/apis/guard/v1alpha1/sessionData.go index 8ef6da7f..76c42217 100644 --- a/pkg/apis/guard/v1alpha1/sessionData.go +++ b/pkg/apis/guard/v1alpha1/sessionData.go @@ -140,6 +140,7 @@ func (config *SessionDataConfig) learnI(valPile ValuePile) { } func (config *SessionDataConfig) Learn(pile *SessionDataPile) { + config.Active = true config.Req.Learn(&pile.Req) config.Resp.Learn(&pile.Resp) config.ReqBody.Learn(&pile.ReqBody) @@ -148,20 +149,6 @@ func (config *SessionDataConfig) Learn(pile *SessionDataPile) { config.Pod.Learn(&pile.Pod) } -func (config *SessionDataConfig) fuseI(otherValConfig ValueConfig) { - config.Fuse(otherValConfig.(*SessionDataConfig)) -} - -func (config *SessionDataConfig) Fuse(otherConfig *SessionDataConfig) { - config.Active = config.Active || otherConfig.Active - config.Req.Fuse(&otherConfig.Req) - config.Resp.Fuse(&otherConfig.Resp) - config.ReqBody.Fuse(&otherConfig.ReqBody) - config.RespBody.Fuse(&otherConfig.RespBody) - config.Envelop.Fuse(&otherConfig.Envelop) - config.Pod.Fuse(&otherConfig.Pod) -} - func (config *SessionDataConfig) Prepare() { config.Req.Prepare() config.Resp.Prepare() diff --git a/pkg/apis/guard/v1alpha1/set.go b/pkg/apis/guard/v1alpha1/set.go index 287d2137..8ce89d5e 100644 --- a/pkg/apis/guard/v1alpha1/set.go +++ b/pkg/apis/guard/v1alpha1/set.go @@ -142,29 +142,14 @@ func (config *SetConfig) learnI(valPile ValuePile) { // pile is RO and unchanged - never uses pile internal objects func (config *SetConfig) Learn(pile *SetPile) { - config.List = make([]string, len(pile.List)) - config.m = make(map[string]bool, len(pile.List)) - - for i, v := range pile.List { - config.m[v] = true - config.List[i] = v + if config.List == nil { + config.List = make([]string, len(pile.List)) } -} - -func (config *SetConfig) fuseI(otherValConfig ValueConfig) { - config.Fuse(otherValConfig.(*SetConfig)) -} - -// otherConfig is RO and unchanged - never uses otherConfig internal objects -func (config *SetConfig) Fuse(otherConfig *SetConfig) { if config.m == nil { - config.m = make(map[string]bool, len(config.List)+len(otherConfig.List)) - // Populate the map from the information in List - for _, v := range config.List { - config.m[v] = true - } + config.m = make(map[string]bool, len(pile.List)) } - for _, v := range otherConfig.List { + + for _, v := range pile.List { if !config.m[v] { config.m[v] = true config.List = append(config.List, v) diff --git a/pkg/apis/guard/v1alpha1/simpleVal.go b/pkg/apis/guard/v1alpha1/simpleVal.go index ce1114df..1d69d1b1 100644 --- a/pkg/apis/guard/v1alpha1/simpleVal.go +++ b/pkg/apis/guard/v1alpha1/simpleVal.go @@ -287,7 +287,6 @@ type SimpleValConfig struct { Sequences LimitConfig `json:"sequences"` Flags AsciiFlagsConfig `json:"flags"` UnicodeFlags FlagSliceConfig `json:"unicodeFlags"` - //Mandatory bool `json:"mandatory"` } func (config *SimpleValConfig) learnI(valPile ValuePile) { @@ -305,22 +304,6 @@ func (config *SimpleValConfig) Learn(pile *SimpleValPile) { config.UnicodeFlags.Learn(pile.UnicodeFlags) } -func (config *SimpleValConfig) fuseI(otherValConfig ValueConfig) { - config.Fuse(otherValConfig.(*SimpleValConfig)) -} - -func (config *SimpleValConfig) Fuse(otherConfig *SimpleValConfig) { - config.Digits.Fuse(&otherConfig.Digits) - config.Letters.Fuse(&otherConfig.Letters) - config.Spaces.Fuse(&otherConfig.Spaces) - config.SpecialChars.Fuse(&otherConfig.SpecialChars) - config.NonReadables.Fuse(&otherConfig.NonReadables) - config.Unicodes.Fuse(&otherConfig.Unicodes) - config.Sequences.Fuse(&otherConfig.Sequences) - config.Flags.Fuse(otherConfig.Flags) - config.UnicodeFlags.Fuse(otherConfig.UnicodeFlags) -} - func (config *SimpleValConfig) decideI(valProfile ValueProfile) *Decision { return config.Decide(valProfile.(*SimpleValProfile)) } diff --git a/pkg/apis/guard/v1alpha1/structuredProfiler.go b/pkg/apis/guard/v1alpha1/structuredProfiler.go index e867027e..65091e3b 100644 --- a/pkg/apis/guard/v1alpha1/structuredProfiler.go +++ b/pkg/apis/guard/v1alpha1/structuredProfiler.go @@ -248,53 +248,33 @@ func (config *StructuredConfig) learnI(valPile ValuePile) { // pile is RO and unchanged - never uses pile internal objects func (config *StructuredConfig) Learn(pile *StructuredPile) { - config.Kind = pile.Kind - switch config.Kind { - case KindObject: - config.Kv = make(map[string]*StructuredConfig, len(pile.Kv)) - for k, v := range pile.Kv { - config.Kv[k] = new(StructuredConfig) - config.Kv[k].Learn(v) - } - case KindArray, KindBoolean, KindNumber, KindString: - config.Val = new(SimpleValConfig) - config.Val.Learn(pile.Val) - } -} - -func (config *StructuredConfig) fuseI(otherValConfig ValueConfig) { - config.Fuse(otherValConfig.(*StructuredConfig)) -} - -// otherConfig is RO and unchanged - never uses otherConfig internal objects -func (config *StructuredConfig) Fuse(otherConfig *StructuredConfig) { if config.Kind == KindEmpty { - config.Kind = otherConfig.Kind - switch otherConfig.Kind { + config.Kind = pile.Kind + switch pile.Kind { case KindObject: - config.Kv = make(map[string]*StructuredConfig, len(otherConfig.Kv)) + config.Kv = make(map[string]*StructuredConfig, len(pile.Kv)) config.Val = nil case KindArray, KindBoolean, KindNumber, KindString: config.Val = new(SimpleValConfig) config.Kv = nil } } - if config.Kind != otherConfig.Kind { + if config.Kind != pile.Kind { config.Kind = KindMulti return } - switch otherConfig.Kind { + switch config.Kind { case KindObject: - for k, v := range otherConfig.Kv { + for k, v := range pile.Kv { vConfig, exists := config.Kv[k] if !exists { vConfig = new(StructuredConfig) config.Kv[k] = vConfig } - vConfig.Fuse(v) + vConfig.Learn(v) } case KindArray, KindBoolean, KindNumber, KindString: - config.Val.Fuse(otherConfig.Val) + config.Val.Learn(pile.Val) } } diff --git a/pkg/apis/guard/v1alpha1/url.go b/pkg/apis/guard/v1alpha1/url.go index f209bad5..f5f50ffe 100644 --- a/pkg/apis/guard/v1alpha1/url.go +++ b/pkg/apis/guard/v1alpha1/url.go @@ -102,15 +102,6 @@ func (config *UrlConfig) Learn(pile *UrlPile) { config.Val.Learn(&pile.Val) } -func (config *UrlConfig) fuseI(otherValConfig ValueConfig) { - config.Fuse(otherValConfig.(*UrlConfig)) -} - -func (config *UrlConfig) Fuse(otherConfig *UrlConfig) { - config.Segments.Fuse(otherConfig.Segments) - config.Val.Fuse(&otherConfig.Val) -} - func (config *UrlConfig) decideI(valProfile ValueProfile) *Decision { return config.Decide(valProfile.(*UrlProfile)) diff --git a/pkg/apis/guard/v1alpha1/v1alpha1.go b/pkg/apis/guard/v1alpha1/v1alpha1.go index 33ec4596..156f6ec9 100644 --- a/pkg/apis/guard/v1alpha1/v1alpha1.go +++ b/pkg/apis/guard/v1alpha1/v1alpha1.go @@ -155,16 +155,11 @@ type ValuePile interface { // A Config defining what Value should adhere to type ValueConfig interface { - // learnI config from a pile - destroy any prior state of Config + // learnI from a pile to this config // pile should not be used after it is Learned by config // Config may absorb some or the pile internal structures learnI(pile ValuePile) - // fuseI otherConfig to this config - // otherConfig should not be used after it is fused to a config - // Config may absorb some or the otherConfig internal structures - fuseI(otherConfig ValueConfig) - // decideI if profile meets config // returns nil if profile is approved by config // otherwise, returns *Decision with details about all failures diff --git a/pkg/apis/guard/v1alpha1/v1alpha1_test.go b/pkg/apis/guard/v1alpha1/v1alpha1_test.go index 13c00757..220d7932 100644 --- a/pkg/apis/guard/v1alpha1/v1alpha1_test.go +++ b/pkg/apis/guard/v1alpha1/v1alpha1_test.go @@ -54,7 +54,6 @@ func ValueTests_Test(t *testing.T, profiles []ValueProfile, piles []ValuePile, c // Initial Tests piles[9].Clear() piles[3].mergeI(piles[4]) - configs[3].fuseI(configs[4]) configs[5].learnI(piles[5]) configs[6].decideI(profiles[0]) piles[6].Clear() @@ -76,11 +75,11 @@ func ValueTests_Test(t *testing.T, profiles []ValueProfile, piles []ValuePile, c // Test ConfigValue for i, pile := range piles { - configs[i].learnI(pile) - configs[0].fuseI(configs[i]) - configs[0].fuseI(configs[i]) + configs[0].learnI(pile) + configs[0].learnI(pile) + if d := configs[0].decideI(profiles[i]); d != nil { - t.Errorf("config.Decide(profile) wrong decission: %s\nFor profile %s\nwhen using config %s\n", d.String(""), profiles[i], configs[0]) + t.Errorf("config.Decide(profile) wrong decision: %s\nFor profile %s\nwhen using config %s\n", d.String(""), profiles[i], configs[0]) } } }) @@ -115,8 +114,7 @@ func ValueTests_Test_WithMarshal(t *testing.T, profiles []ValueProfile, piles [] } // Test ConfigValue config.learnI(pile) - config.fuseI(config) - config.fuseI(config) + config.learnI(pile) if d := config.decideI(profile); d != nil { t.Errorf(d.String("config.Decide(profile) wrong decission:")) @@ -270,8 +268,8 @@ func ValueTests_TestFuse(t *testing.T, profiles []ValueProfile, piles []ValuePil // test ConfigValue configs[0].learnI(piles[0]) - configs[1].learnI(piles[1]) - configs[0].fuseI(configs[1]) + configs[0].learnI(piles[1]) + if d := configs[0].decideI(profiles[0]); d != nil { t.Errorf(d.String("config.Decide(profile) wrong decission: ")) } diff --git a/pkg/guard-gate/client.go b/pkg/guard-gate/client.go index b33d4836..fcab3316 100644 --- a/pkg/guard-gate/client.go +++ b/pkg/guard-gate/client.go @@ -40,6 +40,8 @@ type httpClientInterface interface { Do(req *http.Request) (*http.Response, error) } +const pileLimit = 1000 + type httpClient struct { client http.Client token string @@ -136,6 +138,7 @@ func (srv *gateClient) reportPile() { srv.pileMutex.Lock() postBody, marshalErr := json.Marshal(srv.pile) srv.pileMutex.Unlock() + // Must unlock srv.pileMutex before http.NewRequest if marshalErr != nil { // should never happen @@ -180,14 +183,19 @@ func (srv *gateClient) addToPile(profile *spec.SessionDataProfile) { srv.pileMutex.Lock() srv.pile.Add(profile) srv.pileMutex.Unlock() + // Must unlock srv.pileMutex before srv.reportPile + + if srv.pile.Count > pileLimit { + srv.reportPile() + } pi.Log.Debugf("Learn - add to pile! pileCount %d", srv.pile.Count) } func (srv *gateClient) clearPile() { srv.pileMutex.Lock() + defer srv.pileMutex.Unlock() srv.pile.Clear() - srv.pileMutex.Unlock() } func (srv *gateClient) loadGuardian() *spec.GuardianSpec { diff --git a/pkg/guard-utils/stats.go b/pkg/guard-utils/stats.go index 4ac1a628..edf9bcc2 100644 --- a/pkg/guard-utils/stats.go +++ b/pkg/guard-utils/stats.go @@ -33,13 +33,13 @@ func (s *Stat) Init() { func (s *Stat) Add(key string) { s.mutex.Lock() + defer s.mutex.Unlock() s.statistics[key]++ - s.mutex.Unlock() } func (s *Stat) Log() string { s.mutex.Lock() + defer s.mutex.Unlock() str := fmt.Sprintf("%v", s.statistics) - s.mutex.Unlock() return str }