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

non-root user #537

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

rseleven
Copy link

Running a Container With a Non Root User

Copy link
Collaborator

@Bert-R Bert-R left a comment

Choose a reason for hiding this comment

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

The intend is good, but the current approach does not work:

  • The content is not added properly (see my other comment)
  • Kafdrop does not run anymore. This is because the working directory is set to /, which is only writable by root. Due to that, the touch operations of kafdrop.sh all fail. The right way to fix that is by adding the application in /app and make that the working directory. However, that requires changes in Dockerfile, kafdrop.sh and the README

ADD kafdrop.sh /
ADD kafdrop*tar.gz /
COPY kafdrop.sh /
COPY kafdrop*tar.gz /
Copy link
Collaborator

Choose a reason for hiding this comment

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

This won't work: the ``ADD` command extracts the zip file. Now it's just copied.

dependabot bot and others added 2 commits June 29, 2023 19:15
…#538)

Bumps [spring-boot-starter-parent](https://github.com/spring-projects/spring-boot) from 3.1.0 to 3.1.1.
- [Release notes](https://github.com/spring-projects/spring-boot/releases)
- [Commits](spring-projects/spring-boot@v3.1.0...v3.1.1)

---
updated-dependencies:
- dependency-name: org.springframework.boot:spring-boot-starter-parent
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…ic8.docker-maven-plugin:0.43.0

fix "Dockerfile"
@rseleven
Copy link
Author

  • replace-dependencies: com.spotify.docker-maven-plugin:1.22 -> io.fabric8.docker-maven-plugin:0.43.0
  • fix "Dockerfile"
  • fix "kafdrop.sh"

when using com.spotify.docker-maven-plugin:1.22 there were problems with https://docs.docker.com/build/buildkit/

Copy link
Collaborator

@Bert-R Bert-R left a comment

Choose a reason for hiding this comment

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

Thanks for the follow-up!
Unfortunately, the build didn't work anymore because the newly selected plug-in uses different configuration parameters.

I've looked into it and feel we can work with a much simplified Dockerfile, while still using the 'old' Spotify Docker plug-in. See my comments.

To test locally, you would need to do a local chmod +x src/main/docker/kafdrop.sh until @davideicardi has reviewed and merged #539.

Comment on lines -242 to -244
<groupId>com.spotify</groupId>
<groupId>io.fabric8</groupId>
<artifactId>docker-maven-plugin</artifactId>
<version>1.2.2</version>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please revert this change, as the current change is incomplete, leading to these warnings that make the image build fail:

[WARNING] Parameter 'dockerDirectory' is unknown for plugin 'docker-maven-plugin:0.43.0:build (default-cli)'
[WARNING] Parameter 'imageName' is unknown for plugin 'docker-maven-plugin:0.43.0:build (default-cli)'
[WARNING] Parameter 'forceTags' is unknown for plugin 'docker-maven-plugin:0.43.0:build (default-cli)'
[WARNING] Parameter 'resources' is unknown for plugin 'docker-maven-plugin:0.43.0:build (default-cli)'
[WARNING] Parameter 'imageTags' is unknown for plugin 'docker-maven-plugin:0.43.0:build (default-cli)'

With that, pom.xml can be removed from your change set.


RUN chmod +x /kafdrop.sh
RUN mkdir /opt/kafdrop && \
tar \
Copy link
Collaborator

Choose a reason for hiding this comment

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

This became a lot more complex than it used to be. Let's simplify this.

  • Through ADD, Docker can extract the zip to the /opt/kafdrop. That causes a bit of an ugly directory structure, but nobody needs to go through it, so it doesn't matter
  • The chmod can be prevented by adding the execution permission in Git. I created Make kafdrop.sh executable #539 for that

With these changes, the Dockerfile can be come this simple:

FROM eclipse-temurin:17.0.7_7-jdk

WORKDIR /opt/kafdrop
ADD kafdrop*tar.gz .
COPY kafdrop.sh .

RUN groupadd -g 1000 kafdrop && \
    useradd -u 1000 -g kafdrop kafdrop && \
    chown --recursive kafdrop:kafdrop /opt/kafdrop

USER kafdrop

ENV PATH="/opt/java/openjdk/bin:$PATH"

EXPOSE 9000

ENTRYPOINT ["/opt/kafdrop/kafdrop.sh"]

@@ -65,4 +65,4 @@ ARGS="--add-opens=java.base/sun.nio.ch=ALL-UNNAMED -Xss256K \
$HEAP_ARGS \
$JVM_OPTS"

exec java $ARGS -jar /kafdrop*/kafdrop*jar ${CMD_ARGS}
exec java $ARGS -jar /opt/kafdrop/kafdrop*.jar ${CMD_ARGS}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Due to the above mentioned changes, this path needs to be changed:

Suggested change
exec java $ARGS -jar /opt/kafdrop/kafdrop*.jar ${CMD_ARGS}
exec java $ARGS -jar /opt/kafdrop/kafdrop*/kafdrop*jar ${CMD_ARGS}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants