Fix IPXEBootConfig URLs for digest-based image references

[xkonni]

Summary

• The old strings.Split(image, ":") code split a digest reference like registry/org/image@sha256:<hash> at the colon, producing imageName=...@sha256&version=<hash> in the stored IPXEBootConfig URL, causing HTTP 400 errors during iPXE boot
• Switch URL construction to url.Values.Encode() so query parameters are properly percent-encoded
• Add V(1) log lines for specImage, imageName, imageVersion, and the three built URLs to aid future debugging

Fixes #328

Test plan

• go test ./internal/controller/ -run TestImageURLFromSpecImage — verifies all three layer URLs have correct imageName, version (with sha256: prefix), and layerDigest parameters for a long-hostname digest reference

Summary by CodeRabbit

• New Features

  • Enhanced image proxy URL construction with improved parameter handling and logging.

• Bug Fixes

  • Improved handling of container image references with digest information to ensure proper URL formatting.

• Tests

  • Added regression test for image URL builder to validate query parameter preservation and formatting.

[coderabbitai]

Warning

Review limit reached

@xkonni, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 49 minutes and 33 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 289d412e-9498-4134-9ece-3b37efb8267f

📥 Commits

Reviewing files that changed from the base of the PR and between 563ee22 and c3b168b.

📒 Files selected for processing (1)

• server/imageproxyserver.go

📝 Walkthrough

Walkthrough

The PR consolidates image URL generation for iPXE boot configs. A new buildImageURL helper uses url.Values.Encode() to safely percent-encode query parameters containing digest colons, replacing inline fmt.Sprintf formatting. The PXE controller's getImageDetailsFromConfig is refactored to call this helper for kernel, initrd, and squashfs layer URLs, and now logs the parsed image reference and computed URLs.

Changes

Digest URL encoding consolidation

Layer / File(s)	Summary
URL encoding helper and validation internal/controller/serverbootconfig_helpers.go, internal/controller/serverbootconfig_helpers_test.go	The buildImageURL function uses url.Values to percent-encode imageName, version, and layerDigest query parameters, safely handling colons in digest strings. TestImageURLFromSpecImage validates that digest-suffixed image names are properly separated and the sha256: prefix is preserved in the version parameter.
PXE controller URL generation refactoring internal/controller/serverbootconfiguration_pxe_controller.go	getImageDetailsFromConfig now accepts a logger parameter and constructs kernel, initrd, and squashfs image URLs by calling buildImageURL instead of inline fmt.Sprintf, while emitting logs for parsed image references and computed URLs.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested labels

area/metal-automation, ok-to-publish, ok-to-image, size/S

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main fix: correcting IPXEBootConfig URLs for digest-based image references, which aligns with the core changeset.
Description check	✅ Passed	The description adequately explains the root cause, the fix using url.Values.Encode(), added logging, and references issue #328 with a test plan.
Linked Issues check	✅ Passed	The PR fully addresses issue #328 by switching to url.Values.Encode() for proper percent-encoding of query parameters and adding diagnostic logging.
Out of Scope Changes check	✅ Passed	All changes directly support fixing digest-based image reference URLs and adding test coverage; no unrelated modifications are present.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/url-encode-image-query-params

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

internal/controller/serverbootconfig_helpers.go (1)

68-77: Query parameter keys match the server parser (imageName, layerDigest, version)

buildImageURL sets imageName, version, and layerDigest, and parseImageURL uses imageKey="imageName", versionKey="version", and layerDigestKey="layerDigest" to read them; the helper test also asserts these exact query keys. The only mismatch is the error string ('image'/'layer'/'version') which is misleading—consider aligning it with the actual parameter names.

🤖 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 `@internal/controller/serverbootconfig_helpers.go` around lines 68 - 77, The
error messages in parseImageURL use generic labels ('image'/'layer'/'version')
that don't match the actual query parameter keys set by buildImageURL
(imageName, version, layerDigest) and the helper test; update the error strings
in parseImageURL (and any related test expectations) to reference the exact
parameter names imageName, layerDigest, and version so they align with
buildImageURL, the parser keys (imageKey, versionKey, layerDigestKey), and the
helper test assertions.

🤖 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 `@internal/controller/serverbootconfig_helpers.go`:
- Around line 68-77: The error messages in parseImageURL use generic labels
('image'/'layer'/'version') that don't match the actual query parameter keys set
by buildImageURL (imageName, version, layerDigest) and the helper test; update
the error strings in parseImageURL (and any related test expectations) to
reference the exact parameter names imageName, layerDigest, and version so they
align with buildImageURL, the parser keys (imageKey, versionKey,
layerDigestKey), and the helper test assertions.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 46d51669-83c0-452e-95d9-a4ac537c134b

📥 Commits

Reviewing files that changed from the base of the PR and between 5d3a022 and 563ee22.

📒 Files selected for processing (3)

• internal/controller/serverbootconfig_helpers.go
• internal/controller/serverbootconfig_helpers_test.go
• internal/controller/serverbootconfiguration_pxe_controller.go