Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions beater/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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()))
Expand Down
23 changes: 23 additions & 0 deletions beater/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
2 changes: 1 addition & 1 deletion changelogs/7.3.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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].
Expand Down
4 changes: 2 additions & 2 deletions tests/system/test_integration_acm.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"):
Expand Down Expand Up @@ -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,
Expand Down