[Agent] Enable post install hooks #17241

michalpristas · 2020-03-25T13:44:36Z

What does this PR do?

This PR introduces new collection of steps just like we have with rules.
This collection is part of baked in spec file and is called post_install
When installation needs to be run it executed Install method of respective installer and after that it runs collection of these hooks.

At the moment two steps were introduced, rename and remove;

Example of the config:

name: Metricbeat
cmd: metricbeat
args: ["-E", "setup.ilm.enabled=false"]
configurable: grpc
post_install:
  - move_file:
      path: "modules.d/system.yml"
      target: "modules.d/system.yml.disabled"

Why is it important?

Metricbeat is collecting system metrics by default and is sending them to default ES index. We want to avoid this default behavior as default behavior is specified by configuration coming from fleet.

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.

Fixes: #17197
Related: elastic/kibana#60319

elasticmachine · 2020-03-25T13:44:39Z

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

ph · 2020-03-25T20:07:07Z

x-pack/agent/pkg/agent/transpiler/steps.go

+	// NOTE(ph): this is a bit of a hack because I want to make sure
+	// the unpack strategy stay in the struct implementation and yaml
+	// doesn't have a RawMessage similar to the JSON package, so partial unpack
+	// is not possible.


Yeah I this is what I hate the most about the yaml package, there is a way to work around that go-yaml/yaml#13 (comment)

I think you can remove my name next to NOTE :)

ph · 2020-03-25T20:11:22Z

x-pack/agent/pkg/agent/transpiler/steps.go

+// Execute executes delete file step.
+func (r *DeleteFileStep) Execute(rootDir string) error {
+	path := filepath.Join(rootDir, filepath.FromSlash(r.Path))
+	err := os.Remove(path)


We probably want to add some defensive programming here, even if we control the spec, we never know when this will not be the case. I think we should convert the Filepath.Join() into an absolute path and make sure we are in the "rootdir" and not somewhere else.

I believe this is the case, we want to sandbox every command into a specific directly. I assume all our commands operate in the extracted directory.

this is something i was thinking about but did not want to sound too paraniod

Better to be paranoid than sorry :)

ph · 2020-03-26T20:14:35Z

I think this works, I need to think a bit more in the context of endpoint and the driver/endpoint installation because I assume this will be also a post install hook that will trigger the installation with a custom installation script.

Maybe we will need to have different hooks defined per platforms? No need to do it now. but this is something we have in mind.

michalpristas · 2020-03-27T08:17:54Z

Jenkins test this

michalpristas · 2020-03-27T08:18:13Z

@ph i think we can introduce constraints here as well at some later point

michalpristas · 2020-03-27T12:01:10Z

needs update on tests, failures are related to change

ph

Minors changes, but the looks good to me.

ph · 2020-03-30T19:13:57Z

x-pack/agent/pkg/agent/transpiler/steps.go

+	if runtime.GOOS == "windows" {
+		absRoot = strings.ToLower(absRoot)
+		absPath = strings.ToLower(absPath)
+	}


This is also true on macOS, by default HFS+ is case insensitive. So the following will work.

mkdir hello cd HelLO

I've been bitten so many times on weird git issues with case insensitive filesystem, the things is if you turn on the case sensitivity on HFS+ it will make many things fails.

ph · 2020-03-30T19:16:17Z

x-pack/agent/pkg/agent/transpiler/steps.go

+	// NOTE(ph): this is a bit of a hack because I want to make sure
+	// the unpack strategy stay in the struct implementation and yaml
+	// doesn't have a RawMessage similar to the JSON package, so partial unpack
+	// is not possible.


I think you can remove my name next to NOTE :)

ph · 2020-03-30T19:19:21Z

x-pack/agent/pkg/agent/transpiler/steps_test.go

+			{"/a/b", "/a/b/../c", "\\a\\c", false},
+			{"/a/b", "../c", "\\a\\c", false},
+		},
+	}


Can we add tests for the case sensitivity on windows/darwin?

ph

LGTM

[Agent] Enable post install hooks (elastic#17241) (cherry picked from commit 5c2eb42)

[Agent] Enable post install hooks (#17241) (cherry picked from commit 5c2eb42) Co-authored-by: Michal Pristas <michal.pristas@gmail.com>

michalpristas added 3 commits March 24, 2020 14:34

added steps

042d9c6

Merge branch 'master' into agent-post-install

a32b3a7

post install hooks working

0af63c8

michalpristas added enhancement Project:fleet labels Mar 25, 2020

michalpristas requested a review from ph March 25, 2020 13:44

michalpristas added 3 commits March 25, 2020 14:45

changelog

1fe0bf4

apply to execute

e73d37c

updated comments

453546e

ph reviewed Mar 25, 2020

View reviewed changes

michalpristas added 4 commits March 26, 2020 10:11

changelog conflicts

ea370ff

more defensive steps

4f30025

mage vendor

069e438

serialization tedst

98437b8

ruflin assigned michalpristas Mar 26, 2020

ph added the Team:ingest-management label Mar 27, 2020

michalpristas added 2 commits March 27, 2020 13:41

run specific tests

ca18a46

mage vendor

cd80ffe

ph approved these changes Mar 30, 2020

View reviewed changes

michalpristas added 3 commits March 31, 2020 11:58

case sensitivity check

64a61d0

mage vendor

b7a06ea

tests: lowers resulting path in insensitive FSs

4584bdd

ph approved these changes Mar 31, 2020

View reviewed changes

michalpristas added 2 commits April 1, 2020 09:36

Merge branch 'master' into agent-post-install

f3a0ba7

mage vendor

aab3104

michalpristas merged commit 5c2eb42 into elastic:master Apr 1, 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 #17241 to 7.x: [Agent] Enable post install hooks #17588

Merged

6 tasks

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

ph pushed a commit to ph/beats that referenced this pull request Apr 7, 2020

[Agent] Enable post install hooks (elastic#17241)

ad9aab9

[Agent] Enable post install hooks (elastic#17241) (cherry picked from commit 5c2eb42)

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

[Agent] Enable post install hooks (#17241) (#17588)

d4d6d4f

[Agent] Enable post install hooks (#17241) (cherry picked from commit 5c2eb42) Co-authored-by: Michal Pristas <michal.pristas@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Agent] Enable post install hooks #17241

[Agent] Enable post install hooks #17241

michalpristas commented Mar 25, 2020 •

edited by ruflin

Loading

elasticmachine commented Mar 25, 2020

ph Mar 25, 2020

ph Mar 30, 2020

ph Mar 25, 2020

michalpristas Mar 26, 2020

ph Mar 26, 2020

ph commented Mar 26, 2020

michalpristas commented Mar 27, 2020

michalpristas commented Mar 27, 2020

michalpristas commented Mar 27, 2020

ph left a comment

ph Mar 30, 2020

ph Mar 30, 2020

ph Mar 30, 2020

ph Mar 30, 2020

ph left a comment

[Agent] Enable post install hooks #17241

[Agent] Enable post install hooks #17241

Conversation

michalpristas commented Mar 25, 2020 • edited by ruflin Loading

What does this PR do?

Why is it important?

Checklist

elasticmachine commented Mar 25, 2020

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 26, 2020

michalpristas commented Mar 27, 2020

michalpristas commented Mar 27, 2020

michalpristas commented Mar 27, 2020

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

ph left a comment

Choose a reason for hiding this comment

michalpristas commented Mar 25, 2020 •

edited by ruflin

Loading