[Elastic Agent] Fix issues with dynamic inputs and conditions by blakerouse · Pull Request #23886 · elastic/beats

blakerouse · 2021-02-05T19:52:56Z

What does this PR do?

Fixes 2 issues that where affecting the usage of dynamic inputs and conditions.

Loading of the configuration was not excluding the inputs from the go-ucfg variable expansion. Inside of inputs on variable expansion is done by the dynamic inputs code.
EQL string methods where too strict on type checks, this lowers that to try to convert any type to a string before doing the comparison. This is important in cases where a variable results to an un-expected type, like *eql.null.

Why is it important?

Usage of dynamic inputs work as expected.

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
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Related issues

elasticmachine · 2021-02-05T19:53:49Z

Pinging @elastic/agent (Team:Agent)

elasticmachine · 2021-02-05T19:56:29Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Build Cause: Pull request #23886 updated
Start Time: 2021-02-10T17:06:24.295+0000
Duration: 26 min 9 sec
Commit: df7a3c5

Test stats 🧪

Test	Results
Failed	0
Passed	5862
Skipped	16
Total	5878

Trends 🧪

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test	Results
Failed	0
Passed	5862
Skipped	16
Total	5878

michalpristas

tested ok with standalone config, code lgtm
looks like config from #23685 works

michalpristas · 2021-02-08T09:03:42Z

x-pack/elastic-agent/pkg/config/config.go

can we generalize this a bit? e.g have a list of keywords where we want to skip ucfg resolution.
i think extraction to func and var list of keys would be enough

elasticmachine · 2021-02-08T09:12:22Z

Pinging @elastic/ingest-management (Team:Ingest Management)

michalpristas · 2021-02-08T10:18:27Z

/pacakge

ph · 2021-02-08T12:51:56Z

/package

blakerouse · 2021-02-08T14:03:09Z

/package

blakerouse · 2021-02-08T14:03:39Z

@michalpristas I cleaned up the config loading more. See what you think.

michalpristas

I think it looks ok. i like having load defines inside config package

michalpristas · 2021-02-08T14:23:48Z

x-pack/elastic-agent/pkg/config/config.go

with multiple VarSkipKeys last one wins right? should we allow multiple entries by combining them?

Yes last wins. If we combine them, then no way to overwrite.... idk its a choice

i'm bringing this up just to verify it's a decision not a bug

ChrsMark · 2021-02-09T11:58:19Z

Thank you for fixing this one @blakerouse . Could you provide an example of valid configuration based on your fixes (dynamic input + env vars), or point me to a docs page which includes such examples? It's still not clear to me what is the proper way to combine dynamic inputs and env vars, do you still need env. prefix when refer to env variables in configuration like at

data_stream:
  dataset: kubernetes.container
  type: metrics
  metricsets:
  - container
  add_metadata: true
  bearer_token_file: /var/run/secrets/kubernetes.io/serviceaccount/token
  hosts:
  - 'https://${env.NODE_NAME}:10250'

?

blakerouse · 2021-02-09T12:53:19Z

@ChrsMark Yes you still need to use ${env.NODE_NODE}. Inside of inputs is only resolved by the providers so it must start with the name of the provider first. So env.* in the case you want to get it from the environment variable.

blakerouse · 2021-02-09T13:52:53Z

/package

ChrsMark · 2021-02-10T13:36:17Z

Hey, is there anything missing with this so as to have it in and unblock #23679?

…ssue with EQL string methods.

blakerouse · 2021-02-10T17:10:55Z

/package

blakerouse · 2021-02-11T14:50:11Z

Tested locally with same built package and both Fleet mode and standalone work. So E2E test failed for an unknown reason.

…c#23886) * Fix loading of configurations to not parse variables in inputs. Fix issue with EQL string methods. * Run fmt. * Add changelog entry. * Cleanup the config loading more. * Fix NewConfigFrom to handle maps. (cherry picked from commit de7906b)

#24004) * Fix loading of configurations to not parse variables in inputs. Fix issue with EQL string methods. * Run fmt. * Add changelog entry. * Cleanup the config loading more. * Fix NewConfigFrom to handle maps. (cherry picked from commit de7906b)

#24005) * Fix loading of configurations to not parse variables in inputs. Fix issue with EQL string methods. * Run fmt. * Add changelog entry. * Cleanup the config loading more. * Fix NewConfigFrom to handle maps. (cherry picked from commit de7906b)

blakerouse added bug needs_backport PR is waiting to be backported to other branches. Team:Elastic-Agent Label for the Agent team labels Feb 5, 2021

blakerouse self-assigned this Feb 5, 2021

botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Feb 5, 2021

blakerouse marked this pull request as ready for review February 5, 2021 19:53

ph added the review label Feb 5, 2021

michalpristas approved these changes Feb 8, 2021

View reviewed changes

botelastic bot added the Team:Ingest Management label Feb 8, 2021

michalpristas approved these changes Feb 8, 2021

View reviewed changes

blakerouse added 5 commits February 10, 2021 12:05

Fix loading of configurations to not parse variables in inputs. Fix i…

ba0683f

…ssue with EQL string methods.

Run fmt.

91a0ba6

Add changelog entry.

b2baa3b

Cleanup the config loading more.

c62de28

Fix NewConfigFrom to handle maps.

df7a3c5

blakerouse force-pushed the fix-agent-dynamic-inputs branch from 2bfca38 to df7a3c5 Compare February 10, 2021 17:05

blakerouse merged commit de7906b into elastic:master Feb 11, 2021

blakerouse deleted the fix-agent-dynamic-inputs branch February 11, 2021 14:50

blakerouse mentioned this pull request Feb 11, 2021

Cherry-pick #23886 to 7.x: [Elastic Agent] Fix issues with dynamic inputs and conditions #24004

Merged

4 tasks

blakerouse added v7.12.0 and removed needs_backport PR is waiting to be backported to other branches. labels Feb 11, 2021

blakerouse mentioned this pull request Feb 11, 2021

Cherry-pick #23886 to 7.11: [Elastic Agent] Fix issues with dynamic inputs and conditions #24005

Merged

4 tasks

blakerouse added the v7.11.1 label Feb 11, 2021

ChrsMark mentioned this pull request Feb 16, 2021

Add Agent standalone k8s manifest #23679

Merged

22 tasks

This was referenced Feb 16, 2021

[Ingest Management] Fix reloading of log level for services #24055

Merged

Cherry-pick #24055 to 7.x: Fix reloading of log level for services #24061

Merged

Cherry-pick #24055 to 7.11: Fix reloading of log level for services #24062

Merged

This was referenced Mar 30, 2021

agent.TestKubernetesIntegrationRecipe is failing elastic/cloud-on-k8s#4360

Closed

Standalone Elastic Agent 7.11.2 Kubernetes integration issue #24842

Closed

Conversation

blakerouse commented Feb 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Why is it important?

Checklist

Related issues

Uh oh!

elasticmachine commented Feb 5, 2021

Uh oh!

elasticmachine commented Feb 5, 2021 • edited by jenkins-beats-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💚 Build Succeeded

Build stats

Test stats 🧪

Trends 🧪

💚 Flaky test report

Test stats 🧪

Uh oh!

michalpristas left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

michalpristas Feb 8, 2021

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Feb 8, 2021

Uh oh!

michalpristas commented Feb 8, 2021

Uh oh!

ph commented Feb 8, 2021

Uh oh!

blakerouse commented Feb 8, 2021

Uh oh!

blakerouse commented Feb 8, 2021

Uh oh!

michalpristas left a comment

Choose a reason for hiding this comment

Uh oh!

michalpristas Feb 8, 2021

Choose a reason for hiding this comment

Uh oh!

blakerouse Feb 8, 2021

Choose a reason for hiding this comment

Uh oh!

michalpristas Feb 8, 2021

Choose a reason for hiding this comment

Uh oh!

ChrsMark commented Feb 9, 2021

Uh oh!

blakerouse commented Feb 9, 2021

Uh oh!

blakerouse commented Feb 9, 2021

Uh oh!

ChrsMark commented Feb 10, 2021

Uh oh!

blakerouse commented Feb 10, 2021

Uh oh!

blakerouse commented Feb 11, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

blakerouse commented Feb 5, 2021 •

edited

Loading

elasticmachine commented Feb 5, 2021 •

edited by jenkins-beats-ci bot

Loading

michalpristas left a comment •

edited

Loading