Skip to content

chore: Add ca-certificates to the worker's runtime image to support reading signed S3 URLs in the CLP connector (fixes #56). #55

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

Merged
anlowee merged 3 commits into from
Aug 19, 2025
Merged

chore: Add ca-certificates to the worker's runtime image to support reading signed S3 URLs in the CLP connector (fixes #56). #55

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f19f0bc
Add ca-certificates
anlowee Aug 18, 2025
b306071
Address comments
anlowee Aug 19, 2025
ada065a
Update presto-native-execution/scripts/dockerfiles/prestissimo-runtim…
anlowee Aug 19, 2025
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
15 changes: 9 additions & 6 deletions presto-native-execution/scripts/dockerfiles/prestissimo-runtime.dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,15 @@ FROM ${BASE_IMAGE}
ENV BUILD_BASE_DIR=_build
ENV BUILD_DIR=""

# Temporary fix for https://github.com/prestodb/presto/issues/25531
# TODO: Update this code when there's a proper fix
RUN apt-get update && \
DEBIAN_FRONTEND=noninteractive apt-get install -y tzdata && \
apt-get clean && \
rm -rf /var/lib/apt/lists/*
# NOTE:
# - We need `ca-certificates` to support reads from signed S3 URLs.
# - We need `tzdata` as a temporary workaround for https://github.com/prestodb/presto/issues/25531
RUN apt-get update \
&& DEBIAN_FRONTEND=noninteractive apt-get install -y \
ca-certificates \
tzdata \
&& apt-get clean \
&& rm -rf /var/lib/apt/lists/*
Comment on lines +45 to +50

@coderabbitai coderabbitai Bot Aug 19, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Hadolint: add --no-install-recommends (apt) and consider version pinning (optional)

  • For Debian/Ubuntu, prefer --no-install-recommends to reduce image size (DL3015). Included in the portable snippet above.
  • Optional: pin package versions for full reproducibility (DL3008). Only do this if you’re prepared to maintain version bumps; otherwise, leave unpinned.
🧰 Tools
🪛 Hadolint (2.12.0)

[info] 45-45: Avoid additional packages by specifying --no-install-recommends

(DL3015)

[warning] 45-45: Pin versions in apt get install. Instead of apt-get install <package> use apt-get install <package>=<version>

(DL3008)

🤖 Prompt for AI Agents 
In presto-native-execution/scripts/dockerfiles/prestissimo-runtime.dockerfile
around lines 45 to 50, the apt-get install invocation should add
--no-install-recommends to avoid pulling recommended packages and reduce image
size; update the command to call apt-get install -y --no-install-recommends
ca-certificates tzdata while keeping DEBIAN_FRONTEND=noninteractive, apt-get
clean and removal of /var/lib/apt/lists/*, and optionally pin package versions
(e.g. ca-certificates=<version> tzdata=<version>) only if you are prepared to
maintain them for reproducibility.


COPY --chmod=0775 --from=prestissimo-image /prestissimo/${BUILD_BASE_DIR}/${BUILD_DIR}/presto_cpp/main/presto_server /usr/bin/
COPY --chmod=0775 --from=prestissimo-image /runtime-libraries/* /usr/lib64/prestissimo-libs/
Expand Down