Skip to content
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.

Sign up for GitHub

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

[Heartbeat] Unpack beats to enable capabilities inside container #30200

Closed
wants to merge 4 commits into from
Closed

[Heartbeat] Unpack beats to enable capabilities inside container #30200

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f8809a5
Unpack beats inside agent container
emilioalvap Feb 3, 2022
e799d53
Change ownership to elastic-agent user
emilioalvap Feb 3, 2022
a006bff
Update elastic-agent docker template
emilioalvap Feb 15, 2022
d732b36
Remove config setting
emilioalvap Feb 22, 2022
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
1 change: 1 addition & 0 deletions dev-tools/packaging/packages.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,7 @@ shared:
user: '{{ .BeatName }}'
linux_capabilities: ''
image_name: ''
beats_install_path: "install"
files:
'elastic-agent.yml':
source: 'elastic-agent.docker.yml'
Expand Down
12 changes: 11 additions & 1 deletion dev-tools/packaging/templates/docker/Dockerfile.elastic-agent.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ FROM {{ .from }}
ENV BEAT_SETUID_AS={{ .user }}

{{- if contains .from "ubi-minimal" }}
RUN for iter in {1..10}; do microdnf update -y && microdnf install -y findutils shadow-utils && microdnf clean all && exit_code=0 && break || exit_code=$? && echo "microdnf error: retry $iter in 10s" && sleep 10; done; (exit $exit_code)
RUN for iter in {1..10}; do microdnf update -y && microdnf install -y tar gzip findutils shadow-utils && microdnf clean all && exit_code=0 && break || exit_code=$? && echo "microdnf error: retry $iter in 10s" && sleep 10; done; (exit $exit_code)
{{- else }}

RUN for iter in {1..10}; do \
Expand Down Expand Up @@ -181,6 +181,16 @@ RUN mkdir /app
RUN chown {{ .user }} /app
{{- end }}
{{- end }}

RUN mkdir -p {{ $beatHome }}/data/{{.BeatName}}-{{ commit_short }}/{{ .beats_install_path }} && \
for beatPath in {{ $beatHome }}/data/{{.BeatName}}-{{ commit_short }}/downloads/*.tar.gz; do \
tar xf $beatPath -C {{ $beatHome }}/data/{{.BeatName}}-{{ commit_short }}/{{ .beats_install_path }}; \
done && \
chown -R {{ .user }}:{{ .user }} {{ $beatHome }}/data/{{.BeatName}}-{{ commit_short }}/{{ .beats_install_path }} && \
chown -R root:root {{ $beatHome }}/data/{{.BeatName}}-{{ commit_short }}/{{ .beats_install_path }}/*/*.yml && \
chmod 0644 {{ $beatHome }}/data/{{.BeatName}}-{{ commit_short }}/{{ .beats_install_path }}/*/*.yml && \
Comment on lines +190 to +191
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@blakerouse @andrewvc These two are workarounds for a couple of issues I've found, I'd welcome your suggestions on how to fix those.

First one is related to permission checking in libbeat. This method is checking that config file owner is either root or executing user, which is not the case if container is extracted as elastic-agent and executed as root. At the moment, k8s templates for elastic-agent containers in docs portal use root user to execute pods, possibly for ECK templates too. The workaround is to change config file ownership to root.

The second one is related to this post-build check. Config files extracted from beat .tars do not follow the specified permission mask:

package_test.go:258: file usr/share/elastic-agent/data/elastic-agent-d732b3/install/osquerybeat-8.2.0-linux-x86_64/osquerybeat.yml has wrong permissions: expected=-rw-r--r-- actual=-rw-------

I wasn't sure what the rationale is behind this check and what the impact could be for another beats.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@emilioalvap I think the first workaround is okay, as Elastic Agent never writes this file.

As for the post-build check I see you do a chmod 0644 for the *.yml, can that be changed to chmod 0600 or would that break it?

Copy link
Collaborator Author

@emilioalvap emilioalvap Mar 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@blakerouse Some of the beats' *.yml files come with 0600 and that is actually what breaks the test. 0644 seems to be the permission mask applied to *.yml files outside zipped beats. So now that we are unpacking beats, the test that used to check elastic.-agent *.yml files is also checking other beats config files.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@emilioalvap Still confused? So are you saying that its working with the change and the test is passing or we need to change either one?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@blakerouse Sure, let me clarify. I added the following to make the test pass:

chmod 0644 {{ $beatHome }}/data/{{.BeatName}}-{{ commit_short }}/{{ .beats_install_path }}/*/*.yml

But I wanted to make sure that this change wouldn't impact other beats in a way that I cannot foresee, as its changing file permission for all beats unpacked in the build phase.

setcap cap_net_raw,cap_setuid+p {{ $beatHome }}/data/{{.BeatName}}-{{ commit_short }}/{{ .beats_install_path }}/heartbeat-*/heartbeat
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the cap_setuid+p for? Why is that needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heartbeat requires it for switching user id at runtime

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To add some detail here @blakerouse heartbeat runs node as a subprocess, and node hates to run as root. It's also just a best practice to setuid out of root if you can regardless.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. It was just more of a question so I can understand. Thanks for the explanation.


USER {{ .user }}

{{- if (and (contains .image_name "-complete") (not (contains .from "ubi-minimal"))) }}
Expand Down