-
Notifications
You must be signed in to change notification settings - Fork 1.8k
packaging: add debian trixie support #10728
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
base: master
Are you sure you want to change the base?
Conversation
WalkthroughAdds Debian 13 (Trixie) across packaging: new build targets, Debian Docker base stages, repository publication entries, documentation, and a Debian 13 smoke-test Dockerfile. No exported APIs changed; control flow only updated to include the new distro in existing loops. Changes
Sequence Diagram(s)sequenceDiagram
actor Maintainer
participant UpdateScript as update-repos.sh
participant AWS as AWS S3
participant RepoGen as update-apt-repo.sh
Maintainer->>UpdateScript: Run
rect #E6F4EA
UpdateScript->>UpdateScript: Iterate DEB_REPO_PATHS (includes debian/trixie)
end
UpdateScript->>AWS: Sync artifacts for debian/trixie
UpdateScript->>RepoGen: Update APT metadata for debian/trixie
sequenceDiagram
participant Builder as Debian13 Smoke Docker
participant Official as packages.fluentbit.io (release)
participant Staging as packages.fluentbit.io (staging)
Builder->>Official: Add key & repo, apt install fluent-bit
Builder->>Builder: Enable service, copy /test.sh
Builder->>Builder: Remove official apt source (prep)
rect #FFF5E6
Builder->>Staging: Add staging key & repo, apt update
Builder->>Staging: apt install fluent-bit (staging upgrade)
end
Builder->>Builder: Enable service, copy /test.sh
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes Assessment against linked issues
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Nitpick comments (6)
packaging/update-apt-repo.sh (1)
13-13
: Keep the documented repo list accurate: add Ubuntu Noble.The comment enumerating valid DEB_REPO values omits ubuntu/noble, which is already handled in update-repos.sh. Keep this in sync to avoid confusion.
-# "debian/bookworm" "debian/bullseye" "debian/buster" "debian/trixie" "ubuntu/xenial" "ubuntu/bionic" "ubuntu/focal" "ubuntu/jammy" "raspbian/buster" "raspbian/bullseye" +# "debian/bookworm" "debian/bullseye" "debian/buster" "debian/trixie" "ubuntu/xenial" "ubuntu/bionic" "ubuntu/focal" "ubuntu/jammy" "ubuntu/noble" "raspbian/buster" "raspbian/bullseye"packaging/README.md (1)
24-25
: Optional: include the codename for Debian 13 for consistency/readability.Ubuntu rows include codename; consider doing the same for Debian to aid readers scanning the table.
-| Debian | 13 | x86_64 | debian/trixie | -| Debian | 13 | arm64v8 | debian/trixie.arm64v8 | +| Debian | 13 / Trixie | x86_64 | debian/trixie | +| Debian | 13 / Trixie | arm64v8 | debian/trixie.arm64v8 |If you adopt this, consider updating the Debian 12 rows to “12 / Bookworm” in a follow-up for full consistency.
packaging/distros/debian/Dockerfile (1)
182-189
: Optional: use --no-install-recommends to keep images leaner.Not required, but it’s a common best practice to reduce image size.
- apt-get install -y curl ca-certificates build-essential \ + apt-get install -y --no-install-recommends curl ca-certificates build-essential \packaging/testing/smoke/packages/Dockerfile.debian13 (3)
17-18
: curl | sh from master is brittle; consider pinning and verifyingPiping an unpinned remote script to sh is risky and non-reproducible. Pin to a tag/commit and verify checksum, or inline the minimal apt steps here to install the official repo on trixie.
- At minimum, preinstall ca-certificates and curl, and verify the script checksum.
Example:
RUN apt-get update && apt-get install -y ca-certificates curl # Optional: add checksum verification of install.sh before executing
28-30
: STAGING_VERSION is set but unused hereENV STAGING_VERSION is not used in this Dockerfile. If test.sh consumes it, fine; otherwise, remove to avoid confusion.
18-18
: systemctl enable in build stages may be flaky in containersRunning systemctl in Docker build stages can fail if systemd isn’t present. Dokken images often include systemd, but build-time enable isn’t always necessary for smoke tests.
- Consider making it non-fatal: append || true.
- Alternatively, rely on deb-systemd-helper via package postinst, or start/enable only at runtime in the test harness.
Example:
-RUN systemctl enable fluent-bit +RUN systemctl enable fluent-bit || trueAlso applies to: 42-42
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packaging/README.md
(1 hunks)packaging/build-config.json
(1 hunks)packaging/distros/debian/Dockerfile
(1 hunks)packaging/testing/smoke/packages/Dockerfile.debian13
(1 hunks)packaging/update-apt-repo.sh
(1 hunks)packaging/update-repos.sh
(1 hunks)
🧰 Additional context used
🪛 Checkov (3.2.334)
packaging/distros/debian/Dockerfile
[HIGH] 181-192: Ensure that certificate validation isn't disabled with curl
(CKV2_DOCKER_2)
[HIGH] 207-218: Ensure that certificate validation isn't disabled with curl
(CKV2_DOCKER_2)
🔇 Additional comments (4)
packaging/update-repos.sh (2)
53-57
: LGTM: added Debian 13 (trixie) to DEB_REPO_PATHS.The new entry follows the existing pattern and will flow through the same publishing path as other Debian repos.
53-65
: Heads-up: packaging now includes debian/trixie — confirm S3 prefix and the sqlite call-site typesQuick summary:
- packaging/update-repos.sh does include "debian/trixie" (so the script will attempt to sync/publish it).
- The repo contains calls to sqlite3_last_insert_rowid in src/flb_sqldb.c and src/flb_blob_db.c; the DB wrapper types live in include/fluent-bit/flb_sqldb.h and include/fluent-bit/flb_blob_db.h.
- I could not verify the S3 prefix (AWS_S3_BUCKET_STAGING) from CI — your environment must run the S3 check with credentials.
Files/locations to check:
- packaging/update-repos.sh — lines adding "debian/trixie"
- src/flb_blob_db.c — usage: sqlite3_last_insert_rowid(context->db) (around line ~912)
- src/flb_sqldb.c — usage: return sqlite3_last_insert_rowid(db->handler) (around line ~170)
- include/fluent-bit/flb_blob_db.h — struct flb_blob_db definition (contains the db member)
- include/fluent-bit/flb_sqldb.h — struct flb_sqldb / handler definition
Suggested next steps (manual / CI checks):
- Verify s3://$AWS_S3_BUCKET_STAGING/debian/trixie exists before enabling AWS_SYNC in manual runs.
- Confirm types being passed to sqlite3_last_insert_rowid are sqlite3* (or adapt wrappers). Packaging/CI for Debian 13 (trixie) is likely to surface build/link-time/compile failures if pointer types differ — fix code in src/flb_blob_db.c/src/flb_sqldb.c if needed.
Rewritten snippet context (unchanged in file):
DEB_REPO_PATHS=( "debian/bookworm" "debian/bullseye" "debian/buster" "debian/trixie" "ubuntu/jammy" "ubuntu/noble" "raspbian/bookworm" ) ... for DEB_REPO in "${DEB_REPO_PATHS[@]}"; do ... /bin/bash -eux "$SCRIPT_DIR/update-apt-repo.sh" done
Tag:
packaging/build-config.json (2)
99-106
: Targets added correctly; base-stage names exist.
- debian/trixie maps to debian-trixie-base
- debian/trixie.arm64v8 maps to debian-trixie.arm64v8-base
Both base stages are present in packaging/distros/debian/Dockerfile, so these targets will build with the new bases.
99-106
: Expect packaging CI to break on trixie until the sqlite pointer-type issue is fixed in core.The addition is fine, but Debian 13 builds currently fail due to an incompatible pointer type passed to sqlite3_last_insert_rowid in src/flb_blob_db.c (sqlite3* expected). Consider landing the core fix first, or be prepared for red builds when ok-package-test is triggered for these targets.
If you want to temporarily keep CI green while the core fix is prepared, I can draft options (e.g., temporarily skipping trixie in the packaging pipeline) — let me know.
# hadolint ignore=DL3008,DL3015 | ||
RUN apt-get -qq update && \ | ||
apt-get install -y curl ca-certificates build-essential \ | ||
make bash sudo wget unzip dh-make \ | ||
libsystemd-dev zlib1g-dev flex bison \ | ||
libssl3 libssl-dev libpq-dev postgresql-server-dev-all \ | ||
libsasl2-2 libsasl2-dev libyaml-dev libcurl4-openssl-dev pkg-config \ | ||
tar gzip && \ | ||
apt-get install -y --reinstall lsb-base lsb-release && \ | ||
mkdir -p "${CMAKE_HOME}" && \ | ||
cmake_download_url="${CMAKE_URL}/v${CMAKE_VERSION}/cmake-${CMAKE_VERSION}-linux-$(uname -m).tar.gz" && \ | ||
echo "Downloading CMake ${CMAKE_VERSION}: ${cmake_download_url} -> ${CMAKE_HOME}" && \ | ||
curl -jksSL "${cmake_download_url}" | tar -xzf - -C "${CMAKE_HOME}" --strip-components 1 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don’t disable TLS verification; remove curl -k and (ideally) verify the download.
Using -k skips certificate validation. At minimum, drop -k. Ideally, also verify the CMake tarball via checksum or signature.
Minimal hardening (remove -k):
- curl -jksSL "${cmake_download_url}" | tar -xzf - -C "${CMAKE_HOME}" --strip-components 1
+ curl -fsSL "${cmake_download_url}" | tar -xzf - -C "${CMAKE_HOME}" --strip-components 1
If you want, I can provide a follow-up patch to verify SHA-256 against the official cmake-${CMAKE_VERSION}-SHA-256.txt manifest.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
# hadolint ignore=DL3008,DL3015 | |
RUN apt-get -qq update && \ | |
apt-get install -y curl ca-certificates build-essential \ | |
make bash sudo wget unzip dh-make \ | |
libsystemd-dev zlib1g-dev flex bison \ | |
libssl3 libssl-dev libpq-dev postgresql-server-dev-all \ | |
libsasl2-2 libsasl2-dev libyaml-dev libcurl4-openssl-dev pkg-config \ | |
tar gzip && \ | |
apt-get install -y --reinstall lsb-base lsb-release && \ | |
mkdir -p "${CMAKE_HOME}" && \ | |
cmake_download_url="${CMAKE_URL}/v${CMAKE_VERSION}/cmake-${CMAKE_VERSION}-linux-$(uname -m).tar.gz" && \ | |
echo "Downloading CMake ${CMAKE_VERSION}: ${cmake_download_url} -> ${CMAKE_HOME}" && \ | |
curl -jksSL "${cmake_download_url}" | tar -xzf - -C "${CMAKE_HOME}" --strip-components 1 | |
# hadolint ignore=DL3008,DL3015 | |
RUN apt-get -qq update && \ | |
apt-get install -y curl ca-certificates build-essential \ | |
make bash sudo wget unzip dh-make \ | |
libsystemd-dev zlib1g-dev flex bison \ | |
libssl3 libssl-dev libpq-dev postgresql-server-dev-all \ | |
libsasl2-2 libsasl2-dev libyaml-dev libcurl4-openssl-dev pkg-config \ | |
tar gzip && \ | |
apt-get install -y --reinstall lsb-base lsb-release && \ | |
mkdir -p "${CMAKE_HOME}" && \ | |
cmake_download_url="${CMAKE_URL}/v${CMAKE_VERSION}/cmake-${CMAKE_VERSION}-linux-$(uname -m).tar.gz" && \ | |
echo "Downloading CMake ${CMAKE_VERSION}: ${cmake_download_url} -> ${CMAKE_HOME}" && \ | |
curl -fsSL "${cmake_download_url}" | tar -xzf - -C "${CMAKE_HOME}" --strip-components 1 |
🧰 Tools
🪛 Checkov (3.2.334)
[HIGH] 181-192: Ensure that certificate validation isn't disabled with curl
(CKV2_DOCKER_2)
# For staging upgrade we use the 'official-install' as the base | ||
ARG STAGING_BASE=docker.io/dokken/debian-13 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Staging “upgrade” flow doesn’t use the official-installed base; orphan stage and comment mismatch
- Line 1’s comment says “use the 'official-install' as the base”, but Line 27 uses an external base image via
${STAGING_BASE}
and Line 47 aliasesstaging-install
instead of upgrading fromofficial-install
. - The
staging-upgrade-prep
stage (Lines 23-25) removes the official apt source, but it’s never used as a base for the upgrade path.
Fix by basing staging-upgrade
on staging-upgrade-prep
, then re-adding the staging repo and installing/upgrade in that stage:
-FROM ${STAGING_BASE} as staging-install
+FROM ${STAGING_BASE} as staging-install
@@
-FROM staging-install as staging-upgrade
+FROM staging-upgrade-prep as staging-upgrade
+ARG STAGING_VERSION
+ARG STAGING_URL
+ENV FLUENT_BIT_PACKAGES_URL=${STAGING_URL}
+ENV FLUENT_BIT_PACKAGES_KEY=${FLUENT_BIT_PACKAGES_URL}/fluentbit.key
+SHELL ["/bin/bash", "-o", "pipefail", "-c"]
+# Prepare keyring and staging repo, then upgrade
+RUN apt-get update && apt-get install -y ca-certificates curl gnupg
+RUN install -m 0755 -d /etc/apt/keyrings \
+ && curl -fsSL "$FLUENT_BIT_PACKAGES_KEY" | gpg --dearmor -o /etc/apt/keyrings/fluent-bit-archive-keyring.gpg
+RUN . /etc/os-release && echo "deb [signed-by=/etc/apt/keyrings/fluent-bit-archive-keyring.gpg] $FLUENT_BIT_PACKAGES_URL/debian/$VERSION_CODENAME $VERSION_CODENAME main" > /etc/apt/sources.list.d/fluent-bit.list
+RUN apt-get update && apt-get install -y fluent-bit && rm -rf /var/lib/apt/lists/*
+RUN systemctl enable fluent-bit
+COPY ./test.sh /test.sh
+RUN chmod a+x /test.sh
This makes the upgrade stage actually exercise an upgrade from the official install to the staging repository on the same OS image.
Also applies to: 23-27, 47-48
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="packaging/testing/smoke/packages/Dockerfile.debian13"
if [[ ! -f "$file" ]]; then
echo "ERROR: File not found: $file" >&2
exit 2
fi
echo "=== File: $file ==="
awk '{printf "%4d: %s\n", NR, $0}' "$file"
echo
echo "=== Grep for relevant patterns ==="
grep -n -E 'staging-upgrade|staging-upgrade-prep|staging-install|STAGING_BASE|official-install|fluent-bit|staging-upgrade-prep' "$file" || true
Length of output: 2897
Staging upgrade stage doesn't use the prepared official-install base (staging-upgrade-prep)
The Dockerfile defines a prep stage that removes the official apt source but the staging-upgrade
stage is built FROM staging-install
(an external base), so the intended upgrade-from-official -> staging path is not exercised.
Affected locations:
- packaging/testing/smoke/packages/Dockerfile.debian13
- Line 1: misleading comment about using
official-install
as the base - Lines 23–25:
FROM official-install as staging-upgrade-prep
removes the official apt source - Line 27:
FROM ${STAGING_BASE} as staging-install
(external base) - Line 47:
FROM staging-install as staging-upgrade
— should usestaging-upgrade-prep
- Line 1: misleading comment about using
Suggested minimal fix (replace the final stage and re-add staging repo/key + install to actually perform the upgrade):
-FROM staging-install as staging-upgrade
+FROM staging-upgrade-prep as staging-upgrade
+ARG STAGING_VERSION
+ARG STAGING_URL
+ENV FLUENT_BIT_PACKAGES_URL=${STAGING_URL}
+ARG STAGING_KEY=${STAGING_URL}/fluentbit.key
+ENV FLUENT_BIT_PACKAGES_KEY=${STAGING_KEY}
+SHELL ["/bin/bash", "-o", "pipefail", "-c"]
+# re-add staging key/repo and upgrade
+RUN wget -qO - $FLUENT_BIT_PACKAGES_KEY | apt-key add - \
+ && echo "deb $FLUENT_BIT_PACKAGES_URL/debian/bookworm bookworm main" >> /etc/apt/sources.list \
+ && apt-get update && apt-get install -y fluent-bit \
+ && systemctl enable fluent-bit \
+ && rm -rf /var/lib/apt/lists/*
+COPY ./test.sh /test.sh
+RUN chmod a+x /test.sh
This ensures the upgrade stage starts from the image with the official install, then re-adds the staging repository and actually performs the upgrade.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packaging/testing/smoke/packages/Dockerfile.debian13 around lines 1 and
23–47, the Dockerfile's comment and final stage are wrong: the prep stage
removes the official apt source (FROM official-install as staging-upgrade-prep)
but the staging-upgrade stage starts FROM staging-install (external base), so
the upgrade-from-official path isn't exercised. Fix by changing the final stage
to start FROM staging-upgrade-prep instead of staging-install, and ensure that
within that final stage you re-add the staging repo and key and run the package
install/upgrade steps that were performed in staging-install so the image
actually performs the upgrade from the prepared official-install base.
RUN rm -f /etc/apt/sources.list.d/fluent-bit.list | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Keep apt source path consistent (.list.d vs sources.list)
- Line 24 removes /etc/apt/sources.list.d/fluent-bit.list, but Line 39 appends to /etc/apt/sources.list. Use the same .list.d path for staged installs/upgrades.
This is addressed in the dynamic-codename change; if you don’t adopt that, at least change Line 39 to:
-RUN echo "deb $FLUENT_BIT_PACKAGES_URL/debian/bookworm bookworm main" >> /etc/apt/sources.list
+RUN echo "deb $FLUENT_BIT_PACKAGES_URL/debian/trixie trixie main" > /etc/apt/sources.list.d/fluent-bit.list
Also applies to: 39-39
🤖 Prompt for AI Agents
In packaging/testing/smoke/packages/Dockerfile.debian13 around lines 24 to 39,
the Dockerfile removes /etc/apt/sources.list.d/fluent-bit.list at line 24 but
later appends a source to /etc/apt/sources.list at line 39; make the apt source
path consistent by changing the append at line 39 to write into
/etc/apt/sources.list.d/fluent-bit.list (and ensure the directory exists or
create it before writing), or alternatively change the removal to target
/etc/apt/sources.list if you prefer that layout — keep both operations using the
same .list.d path for staged installs/upgrades.
ARG STAGING_URL | ||
ENV FLUENT_BIT_PACKAGES_URL=${STAGING_URL} | ||
|
||
ARG STAGING_KEY=${STAGING_URL}/fluentbit.key | ||
ENV FLUENT_BIT_PACKAGES_KEY=${STAGING_KEY} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Fix ARG default referencing another ARG; derive key URL from the resolved repo URL
ARG STAGING_KEY=${STAGING_URL}/fluentbit.key
will not reliably expand ${STAGING_URL}
at parse/build time. Derive the key from the already-resolved FLUENT_BIT_PACKAGES_URL
instead.
ARG STAGING_URL
ENV FLUENT_BIT_PACKAGES_URL=${STAGING_URL}
-ARG STAGING_KEY=${STAGING_URL}/fluentbit.key
-ENV FLUENT_BIT_PACKAGES_KEY=${STAGING_KEY}
+ENV FLUENT_BIT_PACKAGES_KEY=${FLUENT_BIT_PACKAGES_URL}/fluentbit.key
If you need to override the key independently, consider adding an optional build arg and a RUN pre-step to override it safely:
ARG FLUENT_BIT_PACKAGES_KEY_OVERRIDE
RUN if [ -n "$FLUENT_BIT_PACKAGES_KEY_OVERRIDE" ]; then \
export FLUENT_BIT_PACKAGES_KEY="$FLUENT_BIT_PACKAGES_KEY_OVERRIDE"; \
fi
🌐 Web query:
Can an ARG default value reference a previously defined ARG in a Dockerfile? e.g. ARG A; ARG B=${A}/path
💡 Result:
No, you cannot directly reference a previously defined ARG as the default value of another ARG in a Dockerfile using syntax like ARG B=${A}/path
[9][5].
Key points and supporting details
-
ARG default values cannot reference other ARGs:
Docker's build system does not process ARG values as dynamic templates, so the default value of an ARG cannot be a function of another ARG already defined[9][5]. For example, the following construct will not work as intended:ARG A=/base ARG B=${A}/path # This does NOT substitute the value of A into B.
-
ARG resolution happens independently:
Each ARG's default value is evaluated at parse time and does not have access to the value of any other ARGs—even if they are defined earlier[9][5]. The only place dynamic ARG substitution works is in instructions after the ARGs themselves (like ENV, RUN, COPY, etc.) -
Common workaround: Use ENV for dynamic composition:
If you need a later variable to reference an earlier one, declare both ARGs and then set an ENV (environment variable) using both. For example:ARG A=/base ARG B=path ENV FINAL_PATH=${A}/${B} RUN echo $FINAL_PATH
-
Relevant documentation and references:
The Docker documentation and practical guides confirm that referencing ARGs within other ARG default values is not supported[9][1]. Some articles highlight the confusion and pitfalls of ARG/ENV variable separation in Dockerfiles[7][4]. -
Explicit values on the build command line:
You can work around this limitation at build time by passing a composite value directly with--build-arg
, e.g.docker build --build-arg A=/foo --build-arg B=/foo/path ...
—but this is handled externally, not by Dockerfile logic.
In summary:
Dockerfile ARG default value assignment is static—it cannot "see" or use the value of a previously defined ARG[9][5][7]. If you need such relationships, handle them via ENV instructions or explicit build-time logic outside the Dockerfile.
Citations:
- 1: https://vsupalov.com/docker-arg-env-variable-guide/
- 2: https://forums.docker.com/t/use-arg-in-from-in-multistage-build/124610
- 3: https://tech.serhatteker.com/post/2023-03/docker-default-arg-values/
- 4: ARG before FROM in Dockerfile doesn't behave as expected moby/moby#34129
- 5: https://www.geeksforgeeks.org/devops/docker-arg-instruction/
- 6: https://www.jeffgeerling.com/blog/2017/use-arg-dockerfile-dynamic-image-specification
- 7: https://qmacro.org/blog/posts/2024/05/13/using-arg-in-a-dockerfile-beware-the-gotcha/
- 8: https://docs.podman.io/en/v5.0.1/markdown/podman-run.1.html
- 9: https://kodekloud.com/blog/docker-build-args/
- 10: https://docs.docker.com/build/bake/reference/
Fix ARG default referencing another ARG; derive key URL from the resolved repo URL
Docker ARG defaults cannot expand other ARGs. Replace the ARG-based key with an ENV composed from the already-resolved FLUENT_BIT_PACKAGES_URL.
- File: packaging/testing/smoke/packages/Dockerfile.debian13 (lines ~31–36)
Proposed change:
ARG STAGING_URL
ENV FLUENT_BIT_PACKAGES_URL=${STAGING_URL}
-ARG STAGING_KEY=${STAGING_URL}/fluentbit.key
-ENV FLUENT_BIT_PACKAGES_KEY=${STAGING_KEY}
+ENV FLUENT_BIT_PACKAGES_KEY=${FLUENT_BIT_PACKAGES_URL}/fluentbit.key
If you need to override the key at build time, accept a build-arg and use it for the ENV (pass via --build-arg):
ARG STAGING_URL
ARG FLUENT_BIT_PACKAGES_KEY
ENV FLUENT_BIT_PACKAGES_URL=${STAGING_URL}
ENV FLUENT_BIT_PACKAGES_KEY=${FLUENT_BIT_PACKAGES_KEY}
# docker build --build-arg FLUENT_BIT_PACKAGES_KEY=https://.../fluentbit.key ...
🤖 Prompt for AI Agents
In packaging/testing/smoke/packages/Dockerfile.debian13 around lines 31–36, the
Dockerfile currently defines ARG STAGING_KEY using another ARG default (which is
invalid); replace that with an optional build-arg and derive the key ENV from
the already-resolved FLUENT_BIT_PACKAGES_URL: keep ARG STAGING_URL and set ENV
FLUENT_BIT_PACKAGES_URL=${STAGING_URL}, add ARG FLUENT_BIT_PACKAGES_KEY
(optional) and then set ENV FLUENT_BIT_PACKAGES_KEY so it uses the provided
build-arg when given or otherwise is composed from FLUENT_BIT_PACKAGES_URL (e.g.
set an ENV that appends /fluentbit.key to FLUENT_BIT_PACKAGES_URL); document
that the key can be overridden via --build-arg FLUENT_BIT_PACKAGES_KEY=...
RUN wget -qO - $FLUENT_BIT_PACKAGES_KEY | apt-key add - | ||
RUN echo "deb $FLUENT_BIT_PACKAGES_URL/debian/bookworm bookworm main" >> /etc/apt/sources.list | ||
# hadolint ignore=DL3015,DL3008,DL3009 | ||
RUN apt-get update && apt-get install -y fluent-bit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Use keyrings and dynamic codename; avoid apt-key and hard-coded bookworm
- apt-key is deprecated and removed in newer Debians; use a keyring under /etc/apt/keyrings with signed-by.
- The source list hard-codes bookworm. On Debian 13 (trixie), mixing releases is risky and unintended for this PR. Use the runtime codename from /etc/os-release.
- Write the entry to /etc/apt/sources.list.d/fluent-bit.list for consistency with removal in Line 24.
Apply:
-RUN wget -qO - $FLUENT_BIT_PACKAGES_KEY | apt-key add -
-RUN echo "deb $FLUENT_BIT_PACKAGES_URL/debian/bookworm bookworm main" >> /etc/apt/sources.list
-# hadolint ignore=DL3015,DL3008,DL3009
-RUN apt-get update && apt-get install -y fluent-bit
+RUN apt-get update && apt-get install -y ca-certificates curl gnupg
+RUN install -m 0755 -d /etc/apt/keyrings \
+ && curl -fsSL "$FLUENT_BIT_PACKAGES_KEY" | gpg --dearmor -o /etc/apt/keyrings/fluent-bit-archive-keyring.gpg
+RUN . /etc/os-release && echo "deb [signed-by=/etc/apt/keyrings/fluent-bit-archive-keyring.gpg] $FLUENT_BIT_PACKAGES_URL/debian/$VERSION_CODENAME $VERSION_CODENAME main" > /etc/apt/sources.list.d/fluent-bit.list
+# hadolint ignore=DL3015,DL3008,DL3009
+RUN apt-get update && apt-get install -y fluent-bit && rm -rf /var/lib/apt/lists/*
🤖 Prompt for AI Agents
In packaging/testing/smoke/packages/Dockerfile.debian13 around lines 38 to 41,
replace the deprecated apt-key usage and hard-coded "bookworm" by saving the
Fluent Bit GPG key to /etc/apt/keyrings (creating the dir if missing), then add
a sources list file under /etc/apt/sources.list.d/fluent-bit.list that uses
signed-by pointing to that keyring; construct the distro codename dynamically
(e.g., by sourcing /etc/os-release and using VERSION_CODENAME) instead of
hard-coding bookworm so the apt entry matches the runtime Debian codename, then
run apt-get update and install fluent-bit as before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this, it looks like all the right things from a packaging perspective - just need to sort out the compilation failure.
@nielsek can you sort the DCO failure out as that's a hard requirement to merge? |
done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packaging/distros/debian/Dockerfile (2)
181-192
: Don’t disable TLS verification; drop curl -k and (ideally) verify the CMake download.This reintroduces the insecure -k flag in the new trixie stage. At minimum, remove -k; ideally add checksum verification of the tarball before extraction.
Minimal hardening (remove -k):
- curl -jksSL "${cmake_download_url}" | tar -xzf - -C "${CMAKE_HOME}" --strip-components 1 + curl -fsSL "${cmake_download_url}" | tar -xzf - -C "${CMAKE_HOME}" --strip-components 1Stronger hardening (verify SHA-256 before extracting):
- cmake_download_url="${CMAKE_URL}/v${CMAKE_VERSION}/cmake-${CMAKE_VERSION}-linux-$(uname -m).tar.gz" && \ - echo "Downloading CMake ${CMAKE_VERSION}: ${cmake_download_url} -> ${CMAKE_HOME}" && \ - curl -jksSL "${cmake_download_url}" | tar -xzf - -C "${CMAKE_HOME}" --strip-components 1 + arch="$(uname -m)" && \ + base_url="${CMAKE_URL}/v${CMAKE_VERSION}" && \ + tarball="cmake-${CMAKE_VERSION}-linux-${arch}.tar.gz" && \ + echo "Downloading CMake ${CMAKE_VERSION}: ${base_url}/${tarball} -> ${CMAKE_HOME}" && \ + cd /tmp && \ + curl -fsSLO "${base_url}/${tarball}" && \ + curl -fsSLO "${base_url}/cmake-${CMAKE_VERSION}-SHA-256.txt" && \ + grep " ${tarball}\$" "cmake-${CMAKE_VERSION}-SHA-256.txt" | sha256sum -c - && \ + tar -xzf "/tmp/${tarball}" -C "${CMAKE_HOME}" --strip-components 1 && \ + rm -f "/tmp/${tarball}" "/tmp/cmake-${CMAKE_VERSION}-SHA-256.txt"
207-218
: Repeat for ARM64: remove curl -k and add checksum verification.Same security concern as the x86_64 trixie stage. Drop -k and, ideally, verify the tarball’s checksum prior to extraction.
Minimal hardening:
- curl -jksSL "${cmake_download_url}" | tar -xzf - -C "${CMAKE_HOME}" --strip-components 1 + curl -fsSL "${cmake_download_url}" | tar -xzf - -C "${CMAKE_HOME}" --strip-components 1Stronger hardening:
- cmake_download_url="${CMAKE_URL}/v${CMAKE_VERSION}/cmake-${CMAKE_VERSION}-linux-$(uname -m).tar.gz" && \ - echo "Downloading CMake ${CMAKE_VERSION}: ${cmake_download_url} -> ${CMAKE_HOME}" && \ - curl -jksSL "${cmake_download_url}" | tar -xzf - -C "${CMAKE_HOME}" --strip-components 1 + arch="$(uname -m)" && \ + base_url="${CMAKE_URL}/v${CMAKE_VERSION}" && \ + tarball="cmake-${CMAKE_VERSION}-linux-${arch}.tar.gz" && \ + echo "Downloading CMake ${CMAKE_VERSION}: ${base_url}/${tarball} -> ${CMAKE_HOME}" && \ + cd /tmp && \ + curl -fsSLO "${base_url}/${tarball}" && \ + curl -fsSLO "${base_url}/cmake-${CMAKE_VERSION}-SHA-256.txt" && \ + grep " ${tarball}\$" "cmake-${CMAKE_VERSION}-SHA-256.txt" | sha256sum -c - && \ + tar -xzf "/tmp/${tarball}" -C "${CMAKE_HOME}" --strip-components 1 && \ + rm -f "/tmp/${tarball}" "/tmp/cmake-${CMAKE_VERSION}-SHA-256.txt"
🧹 Nitpick comments (2)
packaging/distros/debian/Dockerfile (2)
181-189
: Reduce image size and improve reproducibility: use --no-install-recommends and clean apt caches.Optional but recommended for slimmer stages and fewer transient deps.
-RUN apt-get -qq update && \ - apt-get install -y curl ca-certificates build-essential \ +RUN apt-get -qq update && \ + apt-get install -y --no-install-recommends curl ca-certificates build-essential \ make bash sudo wget unzip dh-make \ libsystemd-dev zlib1g-dev flex bison \ libssl3 libssl-dev libpq-dev postgresql-server-dev-all \ libsasl2-2 libsasl2-dev libyaml-dev libcurl4-openssl-dev pkg-config \ - tar gzip && \ + tar gzip && \ apt-get install -y --reinstall lsb-base lsb-release && \ + rm -rf /var/lib/apt/lists/* && \ mkdir -p "${CMAKE_HOME}" && \Note: apply the same pattern to the ARM64 trixie stage for consistency.
172-195
: Heads-up: Debian 13 build currently fails in flb_blob_db.c (sqlite3 pointer type mismatch).Packaging additions themselves are fine, but building the trixie stage presently fails around 56% with sqlite3_last_insert_rowid expecting sqlite3* while receiving __internal_flb_sqldb*. This needs a source fix or a temporary packaging workaround; otherwise these stages won’t go green.
Suggested next steps:
- Prefer fixing the source (ensure sqlite3_last_insert_rowid is called with an sqlite3* handle; if wrapped, pass the underlying sqlite3*).
- If you need green packaging while the fix lands, either:
- Temporarily exclude debian/trixie targets from CI/build matrix, or
- Carry a minimal downstream patch in packaging for flb_blob_db.c, to be dropped once upstream is fixed.
I can draft the minimal code patch to adjust the call site in flb_blob_db.c. Want me to open a follow-up PR for that?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (6)
packaging/README.md
(1 hunks)packaging/build-config.json
(1 hunks)packaging/distros/debian/Dockerfile
(1 hunks)packaging/testing/smoke/packages/Dockerfile.debian13
(1 hunks)packaging/update-apt-repo.sh
(1 hunks)packaging/update-repos.sh
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- packaging/update-apt-repo.sh
- packaging/README.md
- packaging/testing/smoke/packages/Dockerfile.debian13
- packaging/update-repos.sh
- packaging/build-config.json
🧰 Additional context used
🪛 Checkov (3.2.334)
packaging/distros/debian/Dockerfile
[HIGH] 181-192: Ensure that certificate validation isn't disabled with curl
(CKV2_DOCKER_2)
[HIGH] 207-218: Ensure that certificate validation isn't disabled with curl
(CKV2_DOCKER_2)
Thanks for your work. |
@nielsek if you can rebase I think this should pick up the fixes? |
Signed-off-by: Niels Kristensen <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packaging/distros/debian/Dockerfile (2)
207-218
: Repeat: drop curl -k in ARM64 trixie stage.Same fix as x86_64 stage; consider adding checksum verification as a follow-up.
- curl -jksSL "${cmake_download_url}" | tar -xzf - -C "${CMAKE_HOME}" --strip-components 1 + curl -jsSLf "${cmake_download_url}" | tar -xzf - -C "${CMAKE_HOME}" --strip-components 1
181-192
: Security: don’t disable TLS verification in curl.Remove -k to avoid bypassing cert verification when fetching CMake.
- curl -jksSL "${cmake_download_url}" | tar -xzf - -C "${CMAKE_HOME}" --strip-components 1 + curl -jsSLf "${cmake_download_url}" | tar -xzf - -C "${CMAKE_HOME}" --strip-components 1
🧹 Nitpick comments (1)
packaging/distros/debian/Dockerfile (1)
172-221
: Nice addition; consider de-duping the per-distro CMake install RUN block.To reduce maintenance, factor the shared apt-get and CMake install snippet into a single ARG-driven build stage (or use a single RUN with minimal diffs gated by env).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
packaging/README.md
(1 hunks)packaging/build-config.json
(1 hunks)packaging/distros/debian/Dockerfile
(1 hunks)packaging/testing/smoke/packages/Dockerfile.debian13
(1 hunks)packaging/update-apt-repo.sh
(1 hunks)packaging/update-repos.sh
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packaging/update-apt-repo.sh
- packaging/testing/smoke/packages/Dockerfile.debian13
- packaging/build-config.json
- packaging/README.md
🧰 Additional context used
🪛 Checkov (3.2.334)
packaging/distros/debian/Dockerfile
[HIGH] 181-192: Ensure that certificate validation isn't disabled with curl
(CKV2_DOCKER_2)
[HIGH] 207-218: Ensure that certificate validation isn't disabled with curl
(CKV2_DOCKER_2)
🔇 Additional comments (1)
packaging/update-repos.sh (1)
50-57
: Add trixie looks correct; verify staging path and apt updater support.
- Confirm AWS has s3://$AWS_S3_BUCKET_STAGING/debian/trixie populated before running with AWS_SYNC=true.
- Ensure update-apt-repo.sh correctly handles DEB_REPO=debian/trixie (component names, dists index, signing).
Signed-off-by: Niels Kristensen <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (4)
packaging/testing/smoke/packages/Dockerfile.debian13 (4)
39-39
: Prefer dynamic codename over hard-coding “trixie”
Using VERSION_CODENAME from /etc/os-release avoids drift if STAGING_BASE changes.-RUN echo "deb $FLUENT_BIT_PACKAGES_URL/debian/trixie trixie main" >> /etc/apt/sources.list +RUN . /etc/os-release && echo "deb [signed-by=/etc/apt/keyrings/fluent-bit-archive-keyring.gpg] $FLUENT_BIT_PACKAGES_URL/debian/$VERSION_CODENAME $VERSION_CODENAME main" > /etc/apt/sources.list.d/fluent-bit.list
38-41
: Replace deprecated apt-key; use keyrings + signed-by and clean apt listsapt-key is deprecated/removed on newer Debian. Store the key under /etc/apt/keyrings and reference it via signed-by. Also clean apt lists to keep the layer slim.
-SHELL ["/bin/bash", "-o", "pipefail", "-c"] -RUN wget -qO - $FLUENT_BIT_PACKAGES_KEY | apt-key add - -RUN echo "deb $FLUENT_BIT_PACKAGES_URL/debian/trixie trixie main" >> /etc/apt/sources.list -# hadolint ignore=DL3015,DL3008,DL3009 -RUN apt-get update && apt-get install -y fluent-bit +SHELL ["/bin/bash", "-o", "pipefail", "-c"] +RUN apt-get update && apt-get install -y ca-certificates curl gnupg +RUN install -m 0755 -d /etc/apt/keyrings \ + && curl -fsSL "$FLUENT_BIT_PACKAGES_KEY" | gpg --dearmor -o /etc/apt/keyrings/fluent-bit-archive-keyring.gpg +RUN . /etc/os-release && echo "deb [signed-by=/etc/apt/keyrings/fluent-bit-archive-keyring.gpg] $FLUENT_BIT_PACKAGES_URL/debian/$VERSION_CODENAME $VERSION_CODENAME main" > /etc/apt/sources.list.d/fluent-bit.list +# hadolint ignore=DL3015,DL3008,DL3009 +RUN apt-get update && apt-get install -y fluent-bit && rm -rf /var/lib/apt/lists/*
24-24
: Keep APT source path consistent (.list.d) and avoid mixing files
Removing /etc/apt/sources.list.d/fluent-bit.list but appending to /etc/apt/sources.list is inconsistent; write your entry to the same .list.d path you removed.-RUN echo "deb $FLUENT_BIT_PACKAGES_URL/debian/trixie trixie main" >> /etc/apt/sources.list +RUN echo "deb [signed-by=/etc/apt/keyrings/fluent-bit-archive-keyring.gpg] $FLUENT_BIT_PACKAGES_URL/debian/trixie trixie main" > /etc/apt/sources.list.d/fluent-bit.listAlso applies to: 39-39
34-35
: ARG default cannot reference another ARG; derive key URL from resolved repo URL
Docker ARG defaults don’t expand other ARGs. Use ENV based on FLUENT_BIT_PACKAGES_URL or accept an override build-arg.-ARG STAGING_KEY=${STAGING_URL}/fluentbit.key -ENV FLUENT_BIT_PACKAGES_KEY=${STAGING_KEY} +ENV FLUENT_BIT_PACKAGES_KEY=${FLUENT_BIT_PACKAGES_URL}/fluentbit.key +# Optional override: +# ARG FLUENT_BIT_PACKAGES_KEY +# ENV FLUENT_BIT_PACKAGES_KEY=${FLUENT_BIT_PACKAGES_KEY:-${FLUENT_BIT_PACKAGES_URL}/fluentbit.key}
🧹 Nitpick comments (2)
packaging/testing/smoke/packages/Dockerfile.debian13 (2)
28-30
: STAGING_VERSION is unused; either drop it or use it to pin the package version
Currently it’s set but not used. If keeping, pin install to the provided version; otherwise remove the ARG/ENV.-RUN apt-get update && apt-get install -y fluent-bit +RUN apt-get update && if [ -n "${STAGING_VERSION:-}" ]; then apt-get install -y "fluent-bit=${STAGING_VERSION}"; else apt-get install -y fluent-bit; fiAlso applies to: 41-41
17-17
: curl | sh from master is brittle; consider pinning
Pin install.sh to a tag/commit to make smoke tests reproducible.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packaging/testing/smoke/packages/Dockerfile.debian13
(1 hunks)
🔇 Additional comments (2)
packaging/testing/smoke/packages/Dockerfile.debian13 (2)
10-15
: Confirm RELEASE_URL/KEY are honored by install.sh
If install.sh ignores these, the official-install stage won’t use the intended release repo/key. Verify and adjust if needed.
1-48
: Heads-up: core build failure on Debian 13 (sqlite3_last_insert_rowid pointer type) blocks packaging
Your smoke image won’t run until the core build issue on GCC 14+/Debian 13 is resolved (see PR discussion). Re-run packaging tests after that fix lands.
@@ -0,0 +1,47 @@ | |||
# For staging upgrade we use the 'official-install' as the base |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Staging upgrade path is not exercised; wire the final stage to the prepared official base
Comment says upgrade uses official-install, but staging-upgrade is FROM staging-install. Start staging-upgrade FROM staging-upgrade-prep and perform the repo/key setup + upgrade there so we actually test “official → staging” on the same OS image.
-FROM staging-install as staging-upgrade
+FROM staging-upgrade-prep as staging-upgrade
+ARG STAGING_VERSION
+ARG STAGING_URL
+ENV FLUENT_BIT_PACKAGES_URL=${STAGING_URL}
+ENV FLUENT_BIT_PACKAGES_KEY=${FLUENT_BIT_PACKAGES_URL}/fluentbit.key
+SHELL ["/bin/bash", "-o", "pipefail", "-c"]
+# Prepare keyring and staging repo, then upgrade
+RUN apt-get update && apt-get install -y ca-certificates curl gnupg
+RUN install -m 0755 -d /etc/apt/keyrings \
+ && curl -fsSL "$FLUENT_BIT_PACKAGES_KEY" | gpg --dearmor -o /etc/apt/keyrings/fluent-bit-archive-keyring.gpg
+RUN . /etc/os-release && echo "deb [signed-by=/etc/apt/keyrings/fluent-bit-archive-keyring.gpg] $FLUENT_BIT_PACKAGES_URL/debian/$VERSION_CODENAME $VERSION_CODENAME main" > /etc/apt/sources.list.d/fluent-bit.list
+# hadolint ignore=DL3015,DL3008,DL3009
+RUN apt-get update && if [ -n "${STAGING_VERSION:-}" ]; then apt-get install -y "fluent-bit=${STAGING_VERSION}"; else apt-get install -y fluent-bit; fi \
+ && rm -rf /var/lib/apt/lists/*
+RUN systemctl enable fluent-bit
+COPY ./test.sh /test.sh
+RUN chmod a+x /test.sh
Also applies to: 23-25, 47-48
🤖 Prompt for AI Agents
In packaging/testing/smoke/packages/Dockerfile.debian13 around lines 1 (and also
affecting lines 23-25 and 47-48), the staging-upgrade stage currently uses the
wrong base; change the staging-upgrade stage to start FROM the
staging-upgrade-prep stage instead of staging-install, and move the repo/key
setup and the actual upgrade commands into that staging-upgrade stage so the
upgrade runs on the prepared image; also ensure the final stage is still wired
to the prepared official-install base where required so the Dockerfile actually
exercises the official → staging upgrade on the same OS image.
@patrick-stephens rebased and tested build: ![]() install: ![]() service running: ![]() |
Added packaging for Debian 13 (trixie). No package dependency changes done from Debian 12 (bookworm).
This is not quite working yet, due to this compilation error when running
build.sh -d debian/trixie
:Compiling for Debian 12 works fine.
Fixes #10716
Enter
[N/A]
in the box, if an item is not applicable to your change.Testing
Before we can approve your change; please submit the following in a comment:
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-test
label to test for all targets (requires maintainer to do).Documentation
Backporting
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
Summary by CodeRabbit
New Features
Tests
Documentation
Chores