Skip to content

Enable CGO to support plugins requiring it - #461

Open
simontesar wants to merge 2 commits into
ironcore-dev:mainfrom
simontesar:enable-cgo
Open

Enable CGO to support plugins requiring it#461
simontesar wants to merge 2 commits into
ironcore-dev:mainfrom
simontesar:enable-cgo

Conversation

@simontesar

@simontesar simontesar commented Aug 3, 2026

Copy link
Copy Markdown

This PR is to test if the pipeline has issues with CGO. Would fix #460 .

Summary by CodeRabbit

  • Bug Fixes
    • Improved cross-platform build support.
    • Resolved runtime compatibility issues by including the required system library in the final image.

Signed-off-by: simontesar <simon@simontesar.com>
Signed-off-by: simontesar <simon@simontesar.com>
@simontesar
simontesar marked this pull request as ready for review August 4, 2026 11:20
@simontesar
simontesar requested a review from a team as a code owner August 4, 2026 11:20
@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The Dockerfile now uses tonistiigi/xx, Clang, and LLD for cross-platform builds. It installs target-architecture packages, builds with CGO enabled, verifies the binary, and adds libgcc_s.so.1 to the runtime image.

Changes

Cross-platform CGO build

Layer / File(s) Summary
Cross-platform build and runtime packaging
Dockerfile
The builder uses xx tooling, installs target packages, builds with xx-go and CGO enabled, verifies the binary, and includes libgcc_s.so.1 in the runtime image.

Estimated code review effort: 3 (Moderate) | ~15–30 minutes

Suggested reviewers: adracus

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description states the CGO purpose and references issue #460, but it omits the required Proposed Changes section and uses an incomplete issue-closing format. Add a Proposed Changes section with the implementation details and replace “Would fix #460” with “Fixes #460”.},{
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly states that the pull request enables CGO to support plugins that require it.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

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 comments (1)
Dockerfile (1)

31-34: 🩺 Stability & Availability | 🔵 Trivial | 🏗️ Heavy lift

Verify the final image with the CGO plugin path.

xx-verify fedhcp checks the target binary format. It does not prove that all dynamic CGO dependencies are present in the final distroless image. The new libgcc_s.so.1 copy addresses one dependency, but the build can still pass while /fedhcp fails when the range plugin opens SQLite.

Add a smoke test for both linux/amd64 and linux/arm64 that starts the final image with the SQLite-backed range configuration. Alternatively, compare the final binary’s DT_NEEDED entries with the libraries packaged in the runtime image. The xx documentation describes xx-verify as a target-file verification step, not a runtime dependency check. (github.com)

🤖 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 `@Dockerfile` around lines 31 - 34, Extend the Docker build validation after
xx-verify to exercise the final distroless image for both linux/amd64 and
linux/arm64, starting fedhcp with the SQLite-backed range configuration and
confirming the range plugin loads successfully. Ensure the check validates
runtime CGO dependencies, not only binary format, and fails the build when
either architecture cannot start with that configuration.

Source: MCP tools

🤖 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.

Nitpick comments:
In `@Dockerfile`:
- Around line 31-34: Extend the Docker build validation after xx-verify to
exercise the final distroless image for both linux/amd64 and linux/arm64,
starting fedhcp with the SQLite-backed range configuration and confirming the
range plugin loads successfully. Ensure the check validates runtime CGO
dependencies, not only binary format, and fails the build when either
architecture cannot start with that configuration.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 78b752e8-159c-4fdd-bfb4-ae63e841b864

📥 Commits

Reviewing files that changed from the base of the PR and between 629c656 and eb6f137.

📒 Files selected for processing (1)
  • Dockerfile

@simontesar

Copy link
Copy Markdown
Author

I don't think there is a reasonable followup to that coderabbit comment. I'd love to have some kind of testing for this but it feels out of scope for this PR.

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.

Disabled CGO potentially breaks some upstream coredhcp plugins

1 participant