[Agent] Support for config constraints #17112

michalpristas · 2020-03-19T11:59:05Z

What does this PR do?

Followup tohttps://github.com//issues/15690.
This PR introduces contraints as described in this issue: #16409

This PR enhances emitter not only to use decorators to enrich configuration but filters to modify ast by dropping some nodes.

It introduces also constraints filter which loads all datasets and for those which contain constraint node parses this nodes and uses existing boolexp implementation to evaluate expressions.
It also allows user to use some basic variables such as agent.id, agent.version and such.

Nodes containing constraints section and where constraint is evaluated as FALSE, these nodes are dropped.

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have made corresponding change to the default configuration files
I have added tests that prove my fix is effective or that my feature works

Fixes: #16409

elasticmachine · 2020-03-19T11:59:07Z

Pinging @elastic/ingest-management (Project:fleet)

ph · 2020-03-23T19:36:38Z

x-pack/agent/pkg/agent/application/emitter.go

+			if err := filter(ast); err != nil {
+				return errors.New(err, "failed to filter configuration", errors.TypeConfig)
+			}
+		}


Maybe we should log when a constrains fails?

I think I did not correctly express my requirement here, my bad.

The above code do log when a constraint fails, but I think we should also log when a constraints matches and we filter content.

ph

A few small changes:

Adding logger when a constrains disable configuration?
Initialize the var store and the methods once?

ph · 2020-03-23T19:41:00Z

x-pack/agent/pkg/agent/application/filters/constraints_filter.go

+	osKernelKey     = "os.kernel"
+	osPlatformKey   = "os.platform"
+	osVersionKey    = "os.version"
+)


Can you add a documentation with the variables that are accessible in the configuration?
This will be useful when writing the documentation and also when we implement them in UI.

ph · 2020-03-23T19:42:17Z

x-pack/agent/pkg/agent/application/filters/constraints_filter.go

+	regs := boolexp.NewMethodsReg()
+	if err := regs.Register(validateVersionFuncName, regValidateVersion); err != nil {
+		return false, err
+	}


This could be defined once and reused?

ph · 2020-03-23T19:44:09Z

x-pack/agent/pkg/agent/application/filters/constraints_filter.go

+		return false, err
+	}
+
+	if err := initVarStore(store); err != nil {


Looking a the fields used I think this could be defined once and reused?

The following requires a reboot to change:

// Agent store.vars[agentIDKey] = agentInfo.AgentID() store.vars[agentVersionKey] = release.Version() // Host store.vars[hostArchKey] = info.Architecture // Operating system store.vars[osFamilyKey] = runtime.GOOS store.vars[osKernelKey] = info.KernelVersion store.vars[osPlatformKey] = info.OS.Family store.vars[osVersionKey] = info.OS.Version

ph · 2020-03-24T13:24:07Z

x-pack/agent/pkg/boolexp/boolexp_test.go

@@ -36,6 +36,7 @@ func TestBoolexp(t *testing.T) {
 	}{
 		// Variables
 		{expression: "%{[hello.var]} == 'hello'", result: true},
+		{expression: "%{[hello.var]} != 'hello'", result: false},


oops I've missed that :)

…nts-new-master

ph · 2020-03-24T16:44:28Z

x-pack/agent/pkg/agent/application/filters/constraints_filter.go

+		if isOK, err := evaluateConstraint(constraint); !isOK || err != nil {
+			if err == nil {
+				// log only constraint not matching
+				log.Infof("constraint '%s' not matching for datasource: %v", constraint, datasourceNode.String())


We might want to reduce what we want to output to the log to the type of the datasource and id? This is just to make sure we do not expose credentials in the log this could be a problem with metricbeat where datasource have a higher change of containing secrets.

constriant is at datasource level and as such it does not contain type, ID is optional for now. i think namespace, output is a identifying configuration and id if present will be logged

sound good ^

ruflin

Should this have somewhere a changelog entry?

Could you update the PR description with more details on what is actually done in this PR and how? At the moment it references to 3 different issues.

ruflin · 2020-03-25T06:43:19Z

x-pack/agent/pkg/agent/application/info/agent_id.go

@@ -80,6 +80,10 @@ func getInfoFromStore(s ioStore) (*persistentAgentInfo, error) {
 			errors.M(errors.MetaKeyPath, AgentConfigFile))
 	}

+	if err := reader.Close(); err != nil {


How is this related to this PR?

this fixes an issue on windows i experienced on tests by introducing more extensive use of agentInfo.ID()

ruflin · 2020-03-25T06:43:25Z

x-pack/agent/pkg/agent/application/info/agent_id.go

@@ -118,6 +122,10 @@ func updateAgentInfo(s ioStore, agentInfo *persistentAgentInfo) error {
 			errors.M(errors.MetaKeyPath, AgentConfigFile))
 	}

+	if err := reader.Close(); err != nil {


How is this related to this PR?

same as ^^^

ph · 2020-03-25T13:28:03Z

Should this have somewhere a changelog entry?

Could you update the PR description with more details on what is actually done in this PR and how? At the moment it references to 3 different issues.

Good comment, I've missed that.. I need to update my local checklist for PR review.

ph

LGTM, Missing changelog entry.

[Agent] Support for config constraints (#17112) (cherry picked from commit e0370f6) Co-authored-by: Michal Pristas <[email protected]>

support for constraints

a25f53f

michalpristas added Project:fleet labels Mar 19, 2020

michalpristas requested a review from ph March 19, 2020 11:59

michalpristas self-assigned this Mar 19, 2020

michalpristas added 2 commits March 19, 2020 14:43

comments

a5329ae

mage vendor

d41fe1d

ph reviewed Mar 23, 2020

View reviewed changes

ph suggested changes Mar 23, 2020

View reviewed changes

michalpristas added 3 commits March 24, 2020 10:05

Merge branch 'master' into agent-constraints-new-master

91386b2

docs and optimisations

0144529

fix test case

b6ecc06

ph reviewed Mar 24, 2020

View reviewed changes

michalpristas added 3 commits March 24, 2020 14:35

Merge branch 'master' of github.com:elastic/beats into agent-constrai…

3491fb3

…nts-new-master

uncommited changes

8f65c02

fixed descriptors closing

981c2c4

michalpristas force-pushed the agent-constraints-new-master branch from 807fa81 to 981c2c4 Compare March 24, 2020 14:36

logger

4e9e4da

ph reviewed Mar 24, 2020

View reviewed changes

ruflin reviewed Mar 25, 2020

View reviewed changes

michalpristas added 2 commits March 25, 2020 10:35

log only ns, out and id

d9be8e9

Merge branch 'master' into agent-constraints-new-master

13d368f

ph approved these changes Mar 25, 2020

View reviewed changes

changelog

c17b99a

ph approved these changes Mar 25, 2020

View reviewed changes

michalpristas merged commit e0370f6 into elastic:master Mar 26, 2020

ph added the needs_backport PR is waiting to be backported to other branches. label Apr 7, 2020

ph mentioned this pull request Apr 7, 2020

Cherry-pick #17112 to 7.x: [Agent] Support for config constraints #17583

Merged

5 tasks

ph added v7.8.0 and removed needs_backport PR is waiting to be backported to other branches. labels Apr 7, 2020

ph added a commit that referenced this pull request Apr 7, 2020

[Agent] Support for config constraints (#17112) (#17583)

701dae5

[Agent] Support for config constraints (#17112) (cherry picked from commit e0370f6) Co-authored-by: Michal Pristas <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Agent] Support for config constraints #17112

[Agent] Support for config constraints #17112

michalpristas commented Mar 19, 2020 •

edited

Loading

elasticmachine commented Mar 19, 2020

ph Mar 23, 2020

ph Mar 24, 2020

ph left a comment

ph Mar 23, 2020

ph Mar 23, 2020

ph Mar 23, 2020

ph Mar 24, 2020

ph Mar 24, 2020

michalpristas Mar 25, 2020

ph Mar 25, 2020

ruflin left a comment

ruflin Mar 25, 2020

michalpristas Mar 25, 2020

ruflin Mar 25, 2020

michalpristas Mar 25, 2020

ph commented Mar 25, 2020

ph left a comment

[Agent] Support for config constraints #17112

[Agent] Support for config constraints #17112

Conversation

michalpristas commented Mar 19, 2020 • edited Loading

What does this PR do?

Checklist

elasticmachine commented Mar 19, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ph left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruflin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ph commented Mar 25, 2020

ph left a comment

Choose a reason for hiding this comment

michalpristas commented Mar 19, 2020 •

edited

Loading