packaging: add packages index generator and release CI#12150
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughAdds a package catalog generator that parses local or S3 artifacts into JSON and HTML indexes, tests both discovery modes, provides preview support, and integrates validation and staging publication into GitHub Actions. ChangesPackages catalog pipeline
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Listing as Local or S3 listing
participant Generator as Package index generator
participant Parser as Catalog parser
participant Storage as Catalog storage
Listing->>Generator: Provide artifact paths
Generator->>Parser: Parse unified object list
Parser-->>Generator: Return version catalog rows
Generator->>Storage: Write catalog files
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 665a78d50a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| function is_downloadable_linux(name) { | ||
| return name ~ /^fluent-bit-[0-9]+(\.[0-9]+)+-[0-9]+\.(x86_64|aarch64|arm64)\.rpm$/ \ | ||
| || name ~ /^fluent-bit_[0-9]+(\.[0-9]+)+_(amd64|arm64|aarch64)\.deb$/ |
There was a problem hiding this comment.
For Raspbian releases, the published target is raspbian/bookworm and the package build names DEBs from dpkg --print-architecture, which is armhf for the arm32v7 Raspbian image. This regex only accepts amd64, arm64, and aarch64, so every Raspbian package is ignored and the generated versions.json/linux_repos omits a supported Linux repository even after the release workflow publishes it.
Useful? React with 👍 / 👎.
| if (name == "fluent-bit-" v ".pkg") { | ||
| macos_pkg[v] = path | ||
| } else if (name == "fluent-bit-" v ".dmg") { | ||
| macos_dmg[v] = path |
There was a problem hiding this comment.
When the release bucket contains the packages produced by the existing macOS workflow, they are named with architecture suffixes such as fluent-bit-<version>-apple.pkg and fluent-bit-<version>-intel.pkg, but these comparisons only accept fluent-bit-<version>.pkg or .dmg. As a result the generator records the version but leaves artifacts.macos.pkg and artifacts.macos.dmg as null, so the new package index cannot expose macOS downloads.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
.github/workflows/staging-release.yaml (1)
374-375: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDisable persisted checkout credentials.
This job only needs a checkout to run the generator. Prevent the read token from remaining in Git configuration for subsequent steps.
Proposed fix
- name: Checkout code uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/staging-release.yaml around lines 374 - 375, Update the actions/checkout step in the staging release workflow to disable persisted credentials by setting its persist-credentials option to false, leaving the pinned action version and other checkout behavior unchanged.Source: Linters/SAST tools
packaging/build-catalog.awk (1)
107-124: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
idxis an implicit global innote_linux.
idxis assigned inside the function but isn't declared as a local (function parameter list only haskey, labelafter the real params). It becomes a global variable — harmless today since nothing else usesidx, but a latent AWK footgun if the script grows.-function note_linux(v, path, name, repo, key, label) { +function note_linux(v, path, name, repo, key, label, idx) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packaging/build-catalog.awk` around lines 107 - 124, Declare idx as a local variable in the note_linux function’s local-parameter list alongside key and label, keeping its existing assignment and use unchanged.packaging/generate-packages-index.sh (1)
211-212: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winHardcoded
+7dfooter stripping is coupled totree's exact HTML output format.
sed -e '/<hr>/,+7d'assumes the report/copyright footer after<hr>is always exactly 7 lines. This depends on the installedtreeversion's HTML template (CI installs it unpinned viaapt-get install -y jq tree); a differenttreeversion could leave stray footer text in the publicindex.html, or delete the closing</body></html>tags if the footer is shorter.♻️ Proposed fix: delete to end-of-file and re-append closing tags
tree --noreport --charset utf-8 --fromfile "$TREE_LIST" -H "$BASE_URL" | \ - sed -e '/<hr>/,+7d' > "$WORK_DIR/tree.html" + sed -e '/<hr>/,$d' > "$WORK_DIR/tree.html" +printf '</body>\n</html>\n' >> "$WORK_DIR/tree.html"Please confirm what
treeversion the CI runner installs and whether its HTML footer format matches this assumption.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packaging/generate-packages-index.sh` around lines 211 - 212, Replace the fixed `sed -e '/<hr>/,+7d'` footer removal in the tree HTML generation pipeline with logic that removes everything from `<hr>` through end-of-file, then re-appends the required closing `</body></html>` tags. Do not rely on the installed `tree` version or a fixed footer line count, and preserve the generated document’s valid closing structure.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/pr-packages-index.yaml:
- Around line 8-13: Update the path filter in the pr-packages-index workflow to
include packaging/testing/generate-preview-from-s3.sh, and add a job step that
validates this script offline using an AWS_S3_LISTING_FILE fixture, or at
minimum performs a syntax check. Keep the existing package-index validation flow
unchanged.
In `@packaging/build-catalog.awk`:
- Around line 18-38: Update extract_version so the version parser stops at the
macOS extension separator while still accepting dots within numeric version
components. Ensure filenames such as fluent-bit-VERSION.pkg and
fluent-bit-VERSION.dmg produce VERSION without a trailing dot, while preserving
existing parsing for Linux and Windows artifact names.
In `@packaging/generate-packages-index.sh`:
- Around line 233-237: Validate LATEST_VERSION against SORTED_VERSIONS before
assigning LATEST in the override branch. If it is not an exact match for a
discovered version, fail with a clear error instead of generating catalog
outputs that reference a nonexistent version; preserve the existing fallback to
the final sorted version when no override is provided.
In `@packaging/testing/test-packages-index.sh`:
- Around line 92-96: Remove the LATEST_VERSION override from both generator
invocations in the test script, while preserving the existing 4.2.7 assertions
so the tests exercise automatic highest-version selection. Keep any explicit
override coverage separate from these default-selection runs.
- Around line 90-150: The S3 test currently checks only partial output and lacks
invalid-input coverage. Extend the S3 assertions to compare `.latest`,
`.base_url`, and `.versions` against the local result while excluding
`generated_at`, then add expected-failure cases covering an empty `BASE_PATH`
and a nonexistent `AWS_S3_LISTING_FILE`, using the existing test helpers and
preserving clear failure validation.
---
Nitpick comments:
In @.github/workflows/staging-release.yaml:
- Around line 374-375: Update the actions/checkout step in the staging release
workflow to disable persisted credentials by setting its persist-credentials
option to false, leaving the pinned action version and other checkout behavior
unchanged.
In `@packaging/build-catalog.awk`:
- Around line 107-124: Declare idx as a local variable in the note_linux
function’s local-parameter list alongside key and label, keeping its existing
assignment and use unchanged.
In `@packaging/generate-packages-index.sh`:
- Around line 211-212: Replace the fixed `sed -e '/<hr>/,+7d'` footer removal in
the tree HTML generation pipeline with logic that removes everything from `<hr>`
through end-of-file, then re-appends the required closing `</body></html>` tags.
Do not rely on the installed `tree` version or a fixed footer line count, and
preserve the generated document’s valid closing structure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7bda1093-e44e-4261-aa4c-6a4c3a117df9
📒 Files selected for processing (7)
.github/workflows/pr-packages-index.yaml.github/workflows/staging-release.yamlpackaging/build-catalog.awkpackaging/generate-packages-index.shpackaging/testing/.gitignorepackaging/testing/generate-preview-from-s3.shpackaging/testing/test-packages-index.sh
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/pr-packages-index.yaml:
- Around line 63-65: Strengthen the preview validation steps in the workflow by
checking the contents of latest-version.txt and versions.json, not just their
existence. Assert that both fixtures identify 4.2.7 as the latest release, while
retaining the existing index.html existence check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d8d3513e-b2d4-4aab-81a7-f980854d36e6
📒 Files selected for processing (6)
.github/workflows/pr-packages-index.yaml.github/workflows/staging-release.yamlpackaging/build-catalog.awkpackaging/generate-packages-index.shpackaging/testing/generate-preview-from-s3.shpackaging/testing/test-packages-index.sh
🚧 Files skipped from review as they are similar to previous changes (4)
- packaging/testing/generate-preview-from-s3.sh
- .github/workflows/staging-release.yaml
- packaging/generate-packages-index.sh
- packaging/build-catalog.awk
|
note: before merging this once is ready, pls cleanup the commit history (cc: @lecaros) |
cosmo0920
left a comment
cosmo0920
left a comment
There was a problem hiding this comment.
The committed workflows' idea looks fine but there's lots of occurrences of fixed version numbers. Do we need to determine dynamically with reading CMakeLists.txt's definition or other part of this project?
Add a generator for packages.fluentbit.io that produces index.html, versions.json, and latest-version.txt from a local mirror or S3 listing. Wire the generator into the staging release workflow so the release bucket is updated after Linux, Windows, and macOS package syncs. Add a PR workflow that runs the offline regression test with jq and tree. Signed-off-by: lecaros <lecaros@chronosphere.io>
a56ed2b to
3e30b80
Compare
|
thanks @cosmo0920 for the review. I've also cleaned up the commit history |
3 participants
Add a generator for packages.fluentbit.io that produces index.html, versions.json, and latest-version.txt from a local mirror or S3 listing.
Wire the generator into the staging release workflow so the release bucket is updated after Linux, Windows, and macOS package syncs. Add a PR workflow that runs the offline regression test with jq and tree.
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