[ACM] Only allow cache expiration values in full seconds. (#2443) (#2451)

* [ACM] Only allow cache expiration values in full seconds.

fixes #2438

Author: simitt

beater/config.go

@@ -27,17 +27,19 @@ import (
 
 	"github.com/pkg/errors"
 
-	"github.com/elastic/apm-server/sourcemap"
 	"github.com/elastic/beats/libbeat/common"
 	"github.com/elastic/beats/libbeat/common/transport/tlscommon"
 	"github.com/elastic/beats/libbeat/paths"
+
+	"github.com/elastic/apm-server/sourcemap"
 )
 
 const (
 	// DefaultPort of APM Server
 	DefaultPort = "8200"
 
-	defaultAPMPipeline = "apm"
+	defaultAPMPipeline  = "apm"
+	errMsgInvalidACMCfg = "invalid value for `apm-server.agent.config.cache.expiration`, only accepting full seconds"
 )
 
 type Config struct {
@@ -157,6 +159,9 @@ func newConfig(version string, ucfg *common.Config) (*Config, error) {
 		return nil, errors.Wrap(err, "Error processing configuration")
 	}
 
+	if float64(int(c.AgentConfig.Cache.Expiration.Seconds())) != c.AgentConfig.Cache.Expiration.Seconds() {
+		return nil, errors.New(errMsgInvalidACMCfg)
+	}
 	if c.RumConfig.isEnabled() {
 		if _, err := regexp.Compile(c.RumConfig.LibraryPattern); err != nil {
 			return nil, errors.New(fmt.Sprintf("Invalid regex for `library_pattern`: %v", err.Error()))

beater/config_test.go

@@ -369,3 +369,26 @@ func TestTLSSettings(t *testing.T) {
 		}
 	})
 }
+
+func TestAgentConfig(t *testing.T) {
+	t.Run("InvalidValueTooSmall", func(t *testing.T) {
+		cfg, err := newConfig("9.9.9",
+			common.MustNewConfigFrom(map[string]string{"agent.config.cache.expiration": "123ms"}))
+		require.Error(t, err)
+		assert.Nil(t, cfg)
+	})
+
+	t.Run("InvalidUnit", func(t *testing.T) {
+		cfg, err := newConfig("9.9.9",
+			common.MustNewConfigFrom(map[string]string{"agent.config.cache.expiration": "1230ms"}))
+		require.Error(t, err)
+		assert.Nil(t, cfg)
+	})
+
+	t.Run("Valid", func(t *testing.T) {
+		cfg, err := newConfig("9.9.9",
+			common.MustNewConfigFrom(map[string]string{"agent.config.cache.expiration": "123000ms"}))
+		require.NoError(t, err)
+		assert.Equal(t, time.Second*123, cfg.AgentConfig.Cache.Expiration)
+	})
+}

changelogs/7.3.asciidoc

@@ -20,7 +20,7 @@ https://github.com/elastic/apm-server/compare/v7.2.1\...v7.3.0[View commits]
 [float]
 ==== Added
 - Support adding transaction and span information to metrics  {pull}2265[2265],{pull}2287[2287].
-- Initial support for remote agent configuration, requires Kibana {pull}2289[2289],{pull}2301[2301],{pull}2386[2386],{pull}2407[2407],{pull}2421[2421].
+- Initial support for remote agent configuration, requires Kibana {pull}2289[2289],{pull}2301[2301],{pull}2386[2386],{pull}2407[2407],{pull}2421[2421],{pull}2443[2443].
 - Add basic caching to remote agent configuration {pull}2337[2337].
 - Enable APM pipeline by default {pull}2301[2301].
 - Add fields required by breakdown graphs APM pipeline by default {pull}2315[2315],{pull}2397[2397].

tests/system/test_integration_acm.py

@@ -13,7 +13,7 @@ class AgentConfigurationIntegrationTest(ElasticTest):
     config_overrides = {
         "logging_json": "true",
         "kibana_enabled": "true",
-        "acm_cache_expiration": "1ms",  # no cache
+        "acm_cache_expiration": "1s",
     }
 
     def create_service_config(self, settings, name, env=None, _id="new"):
@@ -160,7 +160,7 @@ def test_config_requests(self):
         assert updated_config_with_env_rsp.status_code == 200, updated_config_with_env_rsp.status_code
         # TODO (gr): remove when cache can be disabled via config
         # wait for cache to purge
-        time.sleep(.10)  # sleep much more than acm_cache_expiration to reduce flakiness
+        time.sleep(1.1)  # sleep much more than acm_cache_expiration to reduce flakiness
 
         # TODO (gr): include If-None-Match header - https://github.com/elastic/apm-server/issues/2434
         r5_post_update = requests.get(self.agent_config_url,