Expose an iPXEWith to provide a cert bundle

[grahamc]

Summary by CodeRabbit

• New Features

  • iPXE can be built with an optional certificate bundle to embed trust information.
  • Default EFI boot artifact now includes the configured certificate bundle when present.

• Chores

  • iPXE construction centralized into a reusable factory to ensure consistent handling of certificate configuration and to make EFI defaults derive from that factory.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Introduced a new exported factory lib.mkiPXE that centralizes iPXE overrideAttrs (patch merging and optional TRUST makeFlags), refactored iPXE construction to use the factory, and updated ipxeX8664Efi to be built via the factory with an optional certBundle.

Changes

Cohort / File(s)

Summary

iPXE factory and usage flake.nix

Added public lib.mkiPXE factory { system, certBundle ? null } that calls nixpkgsFor.${system}.ipxe.overrideAttrs, merges iPxePatches into patches, and conditionally appends ''TRUST=${certBundle}'' to makeFlags. Replaced direct ipxe.overrideAttrs with iPXE = self.lib.mkiPXE { inherit system; }. Updated ipxeX8664Efi default to (self.lib.mkiPXE { system = "x86_64-linux"; certBundle = cfg.httpBootRootCertificate; }) + "/ipxe.efi".

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Single-file change with a consistent refactor.
• Pay attention to:
  • Correct optional handling of certBundle and use of lib.optional semantics.
  • Preservation and ordering of oldAttrs.patches when merging iPxePatches.
  • Correct propagation of system when replacing prior pkgsX8664Linux-based references.

Poem

🐇
I hop through flake and stitch each patch,
A tiny factory clears the batch,
TRUST tucked in, neat as a seed,
One path now grows from many weed,
Hooray — builds sprout fast, no scratch!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title mentions 'iPXEWith' but the actual change introduces 'mkiPXE' function; the title is somewhat misleading about the specific implementation detail.	Revise the title to accurately reflect the change, such as 'Extract iPXE configuration into mkiPXE factory function' or 'Introduce mkiPXE factory for certificate bundle support'.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
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
• Post copyable unit tests in a comment
• Commit unit tests in branch push-rlkxykkkyoyt

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3094c6f and 5cb2cbd.

📒 Files selected for processing (1)

• flake.nix (3 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• flake.nix

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: DeterminateCI / inventory

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

flake.nix (1)

60-66: Code is functionally correct; TRUST syntax is valid.

The implementation properly uses the iPXE TRUST= makefile parameter with the cert bundle path. While using pkgs.lib.optional is functionally equivalent to pkgsX8664Linux.lib.optional (since lib is system-independent in nixpkgs), using pkgsX8664Linux.lib would be a minor stylistic improvement for consistency with the package source being overridden.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4410f81 and 8168f04.

📒 Files selected for processing (1)

• flake.nix (2 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: DeterminateCI / inventory

🔇 Additional comments (2)

flake.nix (2)

58-58: LGTM: Clean delegation to the new factory function.

The refactoring correctly delegates to iPXEWith with no cert bundle, preserving the original behavior while enabling reuse.

---

154-156: LGTM: Clean usage of the new factory function.

The refactoring correctly uses iPXEWith with the certificate bundle from the configuration option. The hardcoded x86_64-linux system reference is appropriate for producing an x86_64 EFI binary.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

flake.nix (1)

51-60: TRUST makeFlag format is correct; function naming is a matter of style preference.

The implementation properly uses iPXE's documented TRUST parameter format. The code ''TRUST=${certBundle}'' correctly expands to a file path (e.g., TRUST=/nix/store/...cert.pem) that iPXE's build system expects. The conditional injection via nixpkgs.lib.optional is well-structured for optional certificate bundles.

Renaming mkiPXE to mkIPXE is optional—both naming conventions exist in Nix ecosystems and this is a stylistic preference rather than a technical concern.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8168f04 and 397d577.

📒 Files selected for processing (1)

• flake.nix (2 hunks)

🔇 Additional comments (2)

flake.nix (2)

68-68: LGTM! Good refactoring to use the factory.

The refactoring eliminates duplication and centralizes the iPXE construction logic. The use of self.lib.mkiPXE makes it clear that this is using the standardized factory function.

---

156-159: LGTM! Excellent simplification.

The refactoring successfully consolidates the cert bundle handling into the factory function. The certBundle parameter is properly passed from cfg.httpBootRootCertificate, making the cert injection mechanism clear and maintainable.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

flake.nix (1)

51-60: Nice iPXE factory extraction; consider future‑proofing makeFlags

Centralizing patch + TRUST handling in lib.mkiPXE is a good cleanup and makes call sites simpler. To be a bit more robust against changes in the upstream ipxe derivation, you could avoid assuming makeFlags is always present:

-        makeFlags = oldAttrs.makeFlags ++ (nixpkgs.lib.optional (certBundle != null)
-          ''TRUST=${certBundle}'');
+        makeFlags = (oldAttrs.makeFlags or []) ++
+          (nixpkgs.lib.optional (certBundle != null) ''TRUST=${certBundle}'');

This keeps behavior identical today while avoiding an attr‑missing failure if makeFlags ever disappears upstream.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 397d577 and 3094c6f.

📒 Files selected for processing (1)

• flake.nix (2 hunks)

🔇 Additional comments (2)

flake.nix (2)

68-68: iPXE is now per‑system instead of hard‑wired to x86_64; double‑check this matches your matrix

Using self.lib.mkiPXE { inherit system; } means packages.${system}.iPXE now builds iPXE for each entry in supportedSystems, rather than always using an x86_64 nixpkgs set. This is probably what you want, but it does change the behavior for e.g. aarch64-linux/aarch64-darwin consumers that might previously have been getting an x86_64 build via pkgsX8664Linux.

If the intent was “always build the x86_64 iPXE image regardless of host system”, you may want to keep system = "x86_64-linux" here instead.

---

156-159: ipxeX8664Efi default correctly reuses mkiPXE and wires in the cert bundle

Refactoring the ipxeX8664Efi default to:

• always build from mkiPXE { system = "x86_64-linux"; … }, and
• pass certBundle = cfg.httpBootRootCertificate

keeps the previous behavior (embed TRUST only when a bundle is configured, still targeting x86_64) while avoiding the duplicate overrideAttrs logic. This looks correct and aligns the module default with the new factory.