[8.5](backport #1912) LoadLimits does not override existing values (#…

…1914) * LoadLimits does not override existing values (#1912) Fleet-server will use any specified cache or server limit values over whatever is returned by the default/agent number loader. For example, if A max body size is specifically set to a value such as 5MB, and the default returned by the LoadLimits is 1MB, the 5MB value is used. (cherry picked from commit 1b2834c) # Conflicts: # CHANGELOG.next.asciidoc * Fix merge conflict Co-authored-by: Michel Laterman <[email protected]> Co-authored-by: michel-laterman <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
elastic · Sep 27, 2022 · 59355d6 · 59355d6
1 parent f4c0322
commit 59355d6
Show file tree

Hide file tree

Showing 4 changed files with 86 additions and 35 deletions.
diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc
@@ -11,6 +11,7 @@
 - Improve authc debug logging. {pull}1870[1870]
 - Add error detail to catch-all HTTP error response. {pull}1854[1854]
 - Fix issue were errors where being ignored written to elasticsearch. {pull}1896[1896]
+- LoadServerLimits will not overwrite specified limits when loading default/agent number specified values. {issue}1841[1841] {pull}1912[1912]
 
 ==== New Features
 

diff --git a/internal/pkg/config/cache.go b/internal/pkg/config/cache.go
@@ -30,14 +30,29 @@ func (c *Cache) InitDefaults() {
 	c.LoadLimits(loadLimits(0))
 }
 
+// LoadLimits loads envLimits for any attribute that is not defined in Cache
 func (c *Cache) LoadLimits(limits *envLimits) {
 	l := limits.Cache
 
-	c.NumCounters = l.NumCounters
-	c.MaxCost = l.MaxCost
-	c.ActionTTL = defaultActionTTL
-	c.EnrollKeyTTL = defaultEnrollKeyTTL
-	c.ArtifactTTL = defaultArtifactTTL
-	c.APIKeyTTL = defaultAPIKeyTTL
-	c.APIKeyJitter = defaultAPIKeyJitter
+	if c.NumCounters == 0 {
+		c.NumCounters = l.NumCounters
+	}
+	if c.MaxCost == 0 {
+		c.MaxCost = l.MaxCost
+	}
+	if c.ActionTTL == 0 {
+		c.ActionTTL = defaultActionTTL
+	}
+	if c.EnrollKeyTTL == 0 {
+		c.EnrollKeyTTL = defaultEnrollKeyTTL
+	}
+	if c.ArtifactTTL == 0 {
+		c.ArtifactTTL = defaultArtifactTTL
+	}
+	if c.APIKeyTTL == 0 {
+		c.APIKeyTTL = defaultAPIKeyTTL
+	}
+	if c.APIKeyJitter == 0 {
+		c.APIKeyJitter = defaultAPIKeyJitter
+	}
 }
diff --git a/internal/pkg/config/config_test.go b/internal/pkg/config/config_test.go
@@ -181,6 +181,38 @@ func TestConfig(t *testing.T) {
 	}
 }
 
+func TestLoadServerLimits(t *testing.T) {
+	t.Run("empty loads limits", func(t *testing.T) {
+		c := &Config{Inputs: []Input{{}}}
+		err := c.LoadServerLimits()
+		assert.NoError(t, err)
+		assert.Equal(t, int64(defaultCheckinMaxBody), c.Inputs[0].Server.Limits.CheckinLimit.MaxBody)
+		assert.Equal(t, defaultActionTTL, c.Inputs[0].Cache.ActionTTL)
+	})
+	t.Run("existing values are not overridden", func(t *testing.T) {
+		c := &Config{
+			Inputs: []Input{{
+				Server: Server{
+					Limits: ServerLimits{
+						CheckinLimit: Limit{
+							MaxBody: 5 * defaultCheckinMaxBody,
+						},
+					},
+				},
+				Cache: Cache{
+					ActionTTL: time.Minute,
+				},
+			}},
+		}
+		err := c.LoadServerLimits()
+		assert.NoError(t, err)
+		assert.Equal(t, int64(5*defaultCheckinMaxBody), c.Inputs[0].Server.Limits.CheckinLimit.MaxBody)
+		assert.Equal(t, defaultCheckinBurst, c.Inputs[0].Server.Limits.CheckinLimit.Burst)
+		assert.Equal(t, time.Minute, c.Inputs[0].Cache.ActionTTL)
+	})
+
+}
+
 // Stub out the defaults so that the above is easier to maintain
 
 func defaultCache() Cache {

diff --git a/internal/pkg/config/limits.go b/internal/pkg/config/limits.go
@@ -36,38 +36,41 @@ func (c *ServerLimits) InitDefaults() {
 func (c *ServerLimits) LoadLimits(limits *envLimits) {
 	l := limits.Server
 
-	c.MaxHeaderByteSize = 8192 // 8k
-	c.MaxConnections = l.MaxConnections
-	c.PolicyThrottle = l.PolicyThrottle
+	if c.MaxHeaderByteSize == 0 {
+		c.MaxHeaderByteSize = 8192 // 8k
+	}
+	if c.MaxConnections == 0 {
+		c.MaxConnections = l.MaxConnections
+	}
+	if c.PolicyThrottle == 0 {
+		c.PolicyThrottle = l.PolicyThrottle
+	}
+
+	c.CheckinLimit = mergeEnvLimit(c.CheckinLimit, l.CheckinLimit)
+	c.ArtifactLimit = mergeEnvLimit(c.ArtifactLimit, l.ArtifactLimit)
+	c.EnrollLimit = mergeEnvLimit(c.EnrollLimit, l.EnrollLimit)
+	c.AckLimit = mergeEnvLimit(c.AckLimit, l.AckLimit)
+	c.StatusLimit = mergeEnvLimit(c.StatusLimit, l.StatusLimit)
+}
 
-	c.CheckinLimit = Limit{
-		Interval: l.CheckinLimit.Interval,
-		Burst:    l.CheckinLimit.Burst,
-		Max:      l.CheckinLimit.Max,
-		MaxBody:  l.CheckinLimit.MaxBody,
+func mergeEnvLimit(L Limit, l limit) Limit {
+	result := Limit{
+		Interval: L.Interval,
+		Burst:    L.Burst,
+		Max:      L.Max,
+		MaxBody:  L.MaxBody,
 	}
-	c.ArtifactLimit = Limit{
-		Interval: l.ArtifactLimit.Interval,
-		Burst:    l.ArtifactLimit.Burst,
-		Max:      l.ArtifactLimit.Max,
-		MaxBody:  l.ArtifactLimit.MaxBody,
+	if result.Interval == 0 {
+		result.Interval = l.Interval
 	}
-	c.EnrollLimit = Limit{
-		Interval: l.EnrollLimit.Interval,
-		Burst:    l.EnrollLimit.Burst,
-		Max:      l.EnrollLimit.Max,
-		MaxBody:  l.EnrollLimit.MaxBody,
+	if result.Burst == 0 {
+		result.Burst = l.Burst
 	}
-	c.AckLimit = Limit{
-		Interval: l.AckLimit.Interval,
-		Burst:    l.AckLimit.Burst,
-		Max:      l.AckLimit.Max,
-		MaxBody:  l.AckLimit.MaxBody,
+	if result.Max == 0 {
+		result.Max = l.Max
 	}
-	c.StatusLimit = Limit{
-		Interval: l.StatusLimit.Interval,
-		Burst:    l.StatusLimit.Burst,
-		Max:      l.StatusLimit.Max,
-		MaxBody:  l.StatusLimit.MaxBody,
+	if result.MaxBody == 0 {
+		result.MaxBody = l.MaxBody
 	}
+	return result
 }