Skip to content

Fixes #2050: Use the scratch image instead of ubi9-minimal in order t… #2170

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

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

Conversation

ganeshmurthy
Copy link
Contributor

…o reduce the size of the images

@ganeshmurthy ganeshmurthy marked this pull request as draft June 26, 2025 19:31
@ganeshmurthy ganeshmurthy self-assigned this Jun 26, 2025
Copy link
Contributor

@skitt skitt left a comment

Choose a reason for hiding this comment

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

The extraction process can be simplified, see the suggestion. I’m wary of taking things this far however because this will produce an image where scanners will only recognise the Go binary; vulnerabilities in the libraries will go undetected.

Dockerfile.cli Outdated
Comment on lines 20 to 41
RUN mkdir -p /runtime-deps && \
ldd /go/src/app/skupper | while IFS= read -r line; do \
# The vdso library (example linux-vdso.so.1), is supplied by linux, skip it
echo "$line" | grep -q "linux-vdso" && continue; \
# Extract library path - note that some lines have => and some
# lines have the direct full path, we need to handle both formats.
lib_path=""; \
if echo "$line" | grep -q '=>'; then \
# Format: "libname.so => /path/to/lib (address)"
lib_path=$(echo "$line" | awk '{print $3}'); \
elif echo "$line" | grep -qE '^[[:space:]]*/'; then \
# Format: "/path/to/ld-linux (address)" - dynamic linker
lib_path=$(echo "$line" | awk '{print $1}'); \
fi; \
# Copy the file into the runtime-deps only if path is valid
if [ -f "$lib_path" ] && [ "$lib_path" != "" ]; then \
dest_dir="/runtime-deps$(dirname "$lib_path")"; \
mkdir -p "$dest_dir"; \
cp "$lib_path" "$dest_dir/"; \
echo "Copied: $lib_path"; \
fi; \
done
Copy link
Contributor

Choose a reason for hiding this comment

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

You can rely on the fact that relevant entries are absolute paths:

Suggested change
RUN mkdir -p /runtime-deps && \
ldd /go/src/app/skupper | while IFS= read -r line; do \
# The vdso library (example linux-vdso.so.1), is supplied by linux, skip it
echo "$line" | grep -q "linux-vdso" && continue; \
# Extract library path - note that some lines have => and some
# lines have the direct full path, we need to handle both formats.
lib_path=""; \
if echo "$line" | grep -q '=>'; then \
# Format: "libname.so => /path/to/lib (address)"
lib_path=$(echo "$line" | awk '{print $3}'); \
elif echo "$line" | grep -qE '^[[:space:]]*/'; then \
# Format: "/path/to/ld-linux (address)" - dynamic linker
lib_path=$(echo "$line" | awk '{print $1}'); \
fi; \
# Copy the file into the runtime-deps only if path is valid
if [ -f "$lib_path" ] && [ "$lib_path" != "" ]; then \
dest_dir="/runtime-deps$(dirname "$lib_path")"; \
mkdir -p "$dest_dir"; \
cp "$lib_path" "$dest_dir/"; \
echo "Copied: $lib_path"; \
fi; \
done
RUN mkdir -p /runtime-deps && \
ldd /go/src/app/skupper | grep -o '/\S*' | while IFS= read -r lib_path; do \
dest_dir="/runtime-deps/${lib_path%/*}"; \
mkdir -p "$dest_dir"; \
cp "$lib_path" "$dest_dir"; \
echo "Copied: $lib_path"; \
done

@ganeshmurthy
Copy link
Contributor Author

The extraction process can be simplified, see the suggestion. I’m wary of taking things this far however because this will produce an image where scanners will only recognise the Go binary; vulnerabilities in the libraries will go undetected.

Thank you Stephen for looking into this. You make a very good point that Clair scan (or any other scanner) will scan only the go binary and might not find CVEs that might actually affect the binary. I had not thought about that. A better approach would be to also copy the dependencies. I will look into this. Thanks again.

@skitt
Copy link
Contributor

skitt commented Jun 27, 2025

The extraction process can be simplified, see the suggestion. I’m wary of taking things this far however because this will produce an image where scanners will only recognise the Go binary; vulnerabilities in the libraries will go undetected.

Thank you Stephen for looking into this. You make a very good point that Clair scan (or any other scanner) will scan only the go binary and might not find CVEs that might actually affect the binary. I had not thought about that. A better approach would be to also copy the dependencies. I will look into this. Thanks again.

Yup, you could look for the packages that ship the required libraries and only install those (and their dependencies).

@ganeshmurthy ganeshmurthy marked this pull request as ready for review July 1, 2025 02:14
@skitt
Copy link
Contributor

skitt commented Jul 1, 2025

@ganeshmurthy you might as well drop the commits dealing with /runtime-deps entirely…

@ganeshmurthy
Copy link
Contributor Author

@ganeshmurthy you might as well drop the commits dealing with /runtime-deps entirely…

I will squash all the commits so any commits that create/delete the /runtime-deps will be gone.

BTW, @c-kruse noted that the reason I needed to copy the dependent libs was because of the issue with 'jq' which you fixed, thanks for that as well !

…mal in order to reduce the size of the images

Signed-off-by: Ganesh Murthy       <[email protected]>
@ganeshmurthy
Copy link
Contributor Author

It looks like the end to end test has failed. Can you @granzoto please take a look and let me know if the failure is due to this PR ? Thanks.

Copy link
Contributor

@c-kruse c-kruse left a comment

Choose a reason for hiding this comment

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

As far as I can tell this should work!

I'll leave it to @fgiorgetti and @nluaces - who may know better if we expect a shell or some other non-skupper executables in these containers somewhere like the system commands or skupper-ansible or something.

@nluaces
Copy link
Member

nluaces commented Jul 2, 2025

I'll leave it to @fgiorgetti and @nluaces - who may know better if we expect a shell or some other non-skupper executables in these containers somewhere like the system commands or skupper-ansible or something.

I think this concern is valid. It is needed to check first that everything works as it is now.

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