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

feat(docker): add curl and jq binaries #798

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

miton18
Copy link
Contributor

@miton18 miton18 commented Jul 4, 2024

This is the work of @chmuche
CleverCloud/clever-tools-dockerhub#6

but applied on the docker template

@miton18 miton18 requested a review from a team as a code owner July 4, 2024 09:05
Copy link

github-actions bot commented Jul 4, 2024

🔎 A preview has been automatically published:

  • 🐧 linux 2f96af9f86770835265b58d6b0e2e68a11e77e6337fac17571de6906a0a254ab
  • 🍏 macos 86227f61b0653c6c9291f148fcadb672ed0408ccba104a4fdf9c57e2b36ee170
  • 🪟 win 80bb575af8a24c7c3f30bd10e79edd173f723e8b7b1374a407fa229c66d69e83

This preview will be deleted once this PR is closed.

Copy link
Collaborator

@pdesoyres-cc pdesoyres-cc left a comment

Choose a reason for hiding this comment

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

Nice, but with some nitpicks and questions.

templates/dockerhub/Dockerfile Outdated Show resolved Hide resolved
@@ -28,9 +28,11 @@ LABEL version="<%= version %>" \
VOLUME ["/actions"]
WORKDIR /actions

COPY --from=build /tmp/fakeroot/ /
RUN mkdir -p /etc/ssl/certs
COPY --from=build /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/ca-certificates.crt
Copy link
Collaborator

Choose a reason for hiding this comment

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

question: I don't understand this one: I read that we copy a file at the same place, does this make sens?

Copy link

Choose a reason for hiding this comment

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

Inside the base image, busybox:glibc there is not cert or packet manager. So we copy the certs from the build image with the --from

## The loader search ld-linux-x86-64.so.2 in /lib64 but the folder does not exist
RUN ln -s lib lib64
COPY --from=build /tmp/fakeroot/ /
COPY --from=ghcr.io/tarampampam/curl:8.6.0 /bin/curl /usr/bin/curl
Copy link
Collaborator

Choose a reason for hiding this comment

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

question: why not use apt-get instead?

Copy link

Choose a reason for hiding this comment

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

And copy it from the build image ?
By the way, I add curl to this image and I can get script or call we hoot inside my CI.

RUN ln -s lib lib64
COPY --from=build /tmp/fakeroot/ /
COPY --from=ghcr.io/tarampampam/curl:8.6.0 /bin/curl /usr/bin/curl
COPY --from=ghcr.io/jqlang/jq /jq /usr/bin/jq
Copy link
Collaborator

Choose a reason for hiding this comment

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

question: why not use apt-get instead?

@@ -0,0 +1,38 @@
FROM debian AS build
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is this file for?

Copy link

Choose a reason for hiding this comment

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

Splitting docker image, that way the final image is smaller and without security risk.

Copy link

@chmuche chmuche left a comment

Choose a reason for hiding this comment

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

I the main author of the PR, but in the wrong repo. I hope my answer are ok.

@@ -28,9 +28,11 @@ LABEL version="<%= version %>" \
VOLUME ["/actions"]
WORKDIR /actions

COPY --from=build /tmp/fakeroot/ /
RUN mkdir -p /etc/ssl/certs
COPY --from=build /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/ca-certificates.crt
Copy link

Choose a reason for hiding this comment

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

Inside the base image, busybox:glibc there is not cert or packet manager. So we copy the certs from the build image with the --from

## The loader search ld-linux-x86-64.so.2 in /lib64 but the folder does not exist
RUN ln -s lib lib64
COPY --from=build /tmp/fakeroot/ /
COPY --from=ghcr.io/tarampampam/curl:8.6.0 /bin/curl /usr/bin/curl
Copy link

Choose a reason for hiding this comment

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

And copy it from the build image ?
By the way, I add curl to this image and I can get script or call we hoot inside my CI.

@@ -0,0 +1,38 @@
FROM debian AS build
Copy link

Choose a reason for hiding this comment

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

Splitting docker image, that way the final image is smaller and without security risk.

This is the work of @chmuche
CleverCloud/clever-tools-dockerhub#6

but applied on the dockertemplate
@miton18 miton18 force-pushed the feat/docker-curl branch from 1755ab0 to f165509 Compare July 5, 2024 11:54
@hsablonniere hsablonniere added this to the 3.12 milestone Jan 23, 2025
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.

4 participants