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.

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

Elastic Agent ACI compliant image #3778

Merged
merged 19 commits into from
Dec 1, 2023

Conversation

michalpristas
Copy link
Contributor

@michalpristas michalpristas commented Nov 16, 2023

What does this PR do?

Bringing back work which allows

  • image to run on Azure Container Instances (no root group)
  • run cloud defend (heartbeat capabilities set)
  • smaller size (got rid of double chown and reduced size of npm modules setup for synthethics)
  • allow container to run as different user (get rid of world permission stripping)

Size comparison

branch
docker images | grep cloud
docker.elastic.co/beats-ci/elastic-agent-cloud. 8.12.0-SNAPSHOT  331c23d10995  5 minutes ago. 3.08GB

main
docker images | grep cloud
docker.elastic.co/beats-ci/elastic-agent-cloud  8.12.0-SNAPSHOT  a6e4e5b9ff81  6 minutes ago. 3.08GB

@michalpristas michalpristas added Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team Team:Elastic-Agent Label for the Agent team backport-skip skip-changelog labels Nov 16, 2023
@michalpristas michalpristas self-assigned this Nov 16, 2023
@elasticmachine
Copy link
Contributor

elasticmachine commented Nov 16, 2023

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-11-20T08:12:34.979+0000

  • Duration: 49 min 55 sec

❕ Flaky test report

No test was executed to be analysed.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages.

  • run integration tests : Run the Elastic Agent Integration tests.

  • run end-to-end tests : Generate the packages and run the E2E Tests.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@michalpristas
Copy link
Contributor Author

still working on -complete image size

@michalpristas michalpristas marked this pull request as ready for review November 20, 2023 08:27
@michalpristas michalpristas requested a review from a team as a code owner November 20, 2023 08:27
@michalpristas michalpristas requested review from faec and pchila November 20, 2023 08:27
@elasticmachine
Copy link
Contributor

Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane)

Copy link
Member

@pchila pchila left a comment

Choose a reason for hiding this comment

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

Aside from some unnecessary change of ownership in the earlier stage, the change looks good.

Just out of curiosity: how hard would it be to always create the elastic-agent user and group, associate all the files to that and as last instruction add a USER instruction ? (ideally with the value being an argument instead of a template)

The user could be specified at runtime by using something like docker run --user elastic-agent ... if we want to run as non-root without having to decide at image build time

Comment on lines 9 to 27
{{- if ne .user "root" }}
RUN groupadd --gid 1000 {{ .BeatName }} && \
useradd -M --uid 1000 --gid 1000 --groups 0 {{ .user }} && \
true
{{- end }}

RUN mkdir -p {{ $beatHome }}/data {{ $beatHome }}/data/elastic-agent-{{ commit_short }}/logs && \
chown -R root:root {{ $beatHome }} && \
COPY --chown={{ .user }}:{{ .user }} beat {{ $beatHome }}

RUN true && \
{{- if ne .user "root" }}
usermod -d {{ $beatHome}} {{ .user }} && \
{{- end}}
# ECE needs to create config here under non-1000 user
chmod 0777 {{ $beatHome}} && \
mkdir -p {{ $beatHome }}/data {{ $beatHome }}/data/elastic-agent-{{ commit_short }}/logs && \
find {{ $beatHome }} -type d -exec chmod 0755 {} \; && \
find {{ $beatHome }} -type f -exec chmod 0644 {} \; && \
find {{ $beatHome }}/data -type d -exec chmod 0770 {} \; && \
find {{ $beatHome }}/data -type f -exec chmod 0660 {} \; && \
find {{ $beatHome }}/data -type d -exec chmod 0777 {} \; && \
find {{ $beatHome }}/data -type f -exec chmod 0666 {} \; && \
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to create the user and the correct ownership in the intermediate stage ? Wouldn't it be enough to copy with the correct ownership and user in the last stage (line ~127) ?

Copy link
Contributor Author

@michalpristas michalpristas Nov 20, 2023

Choose a reason for hiding this comment

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

hmm that;s a nice idea, will check, not sure it will let us differentiate /d/f though

@michalpristas michalpristas changed the title Image custom uid Elastic Agent ACI compliant image Nov 20, 2023
@@ -27,7 +38,7 @@ RUN mkdir -p {{ $beatHome }}/data {{ $beatHome }}/data/elastic-agent-{{ commit_s
(chmod 0755 {{ $beatHome }}/data/elastic-agent-*/components/pf-elastic-collector || true) && \
(chmod 0755 {{ $beatHome }}/data/elastic-agent-*/components/pf-elastic-symbolizer || true) && \
(chmod 0755 {{ $beatHome }}/data/elastic-agent-*/components/pf-host-agent || true) && \
find {{ $beatHome }}/data/elastic-agent-{{ commit_short }}/components -name "*.yml*" -type f -exec chown root:root {} \; && \
find {{ $beatHome }}/data/elastic-agent-{{ commit_short }}/components -name "*.yml*" -type f -exec chown {{ .user }}:{{ .user }} {} \; && \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is a no, modules permissions are checked to be owned by root

@cmacknz cmacknz requested a review from blakerouse November 23, 2023 15:06
@cmacknz
Copy link
Member

cmacknz commented Nov 23, 2023

Did we confirm this image can be deployed to Elastic Cloud successfully and that ECE stack can come up.

I would also like @blakerouse to review this before merge to see if there is anything that will cause problems for the non-root/unprivileged agent work.

@michalpristas
Copy link
Contributor Author

i created cloud instance using our regular testing/cloud env. this was fine.
for the use case lastly reported i haven't found a way to deploy custom image (QA labs does not seem to work)

@michalpristas
Copy link
Contributor Author

i'm seeing some weird behavior but i see it on main as well.
i need to test this on another machine to rule out my env. i don't trust my mac

@michalpristas
Copy link
Contributor Author

seems like some env issues on my side, rebuilt on my linux machine and it seems to produce system metrics normally.

Copy link
Contributor

@blakerouse blakerouse left a comment

Choose a reason for hiding this comment

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

Looks good to me

Copy link

SonarQube Quality Gate

Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@michalpristas michalpristas merged commit 8d4e3da into elastic:main Dec 1, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-skip skip-changelog Team:Elastic-Agent Label for the Agent team Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team
Projects
None yet
6 participants