diff --git a/internal/controller/serverbootconfig_helpers.go b/internal/controller/serverbootconfig_helpers.go index 6a0b00a..5a82dde 100644 --- a/internal/controller/serverbootconfig_helpers.go +++ b/internal/controller/serverbootconfig_helpers.go @@ -19,6 +19,7 @@ package controller import ( "context" "fmt" + "net/url" "strings" "github.com/distribution/reference" @@ -64,6 +65,17 @@ func BuildImageReference(imageName, imageVersion string) string { return fmt.Sprintf("%s:%s", imageName, imageVersion) } +// buildImageURL constructs a percent-encoded image proxy URL. +// url.Values.Encode ensures the colon in digest versions (e.g. sha256:) is encoded +// as %3A so it is not misinterpreted as a delimiter by HTTP clients or kube-apiserver. +func buildImageURL(serviceURL, imageName, imageVersion, layerDigest string) string { + params := url.Values{} + params.Set("imageName", imageName) + params.Set("version", imageVersion) + params.Set("layerDigest", layerDigest) + return serviceURL + "/image?" + params.Encode() +} + // ExtractServerNetworkIDs extracts IP addresses (and optionally MAC addresses) from a Server's network interfaces. // Returns a slice of IP addresses as strings. If includeMACAddresses is true, MAC addresses are also included. func ExtractServerNetworkIDs(server *metalv1alpha1.Server, includeMACAddresses bool) []string { diff --git a/internal/controller/serverbootconfig_helpers_test.go b/internal/controller/serverbootconfig_helpers_test.go index 9806d2e..81b3cba 100644 --- a/internal/controller/serverbootconfig_helpers_test.go +++ b/internal/controller/serverbootconfig_helpers_test.go @@ -5,6 +5,7 @@ package controller import ( "errors" + "net/url" "testing" metalv1alpha1 "github.com/ironcore-dev/metal-operator/api/v1alpha1" @@ -72,6 +73,46 @@ func TestBuildImageReference(t *testing.T) { } } +func TestImageURLFromSpecImage(t *testing.T) { + // Regression test for the URL construction step. + // The old strings.Split(image, ":") code split + // "registry.../gardenlinux@sha256:a5f8b641..." + // into ("registry.../gardenlinux@sha256", "a5f8b641..."), + // producing imageName=...@sha256&version=a5f8b641... in the stored URL. + // ParseImageReference correctly splits into ("registry.../gardenlinux", "sha256:a5f8b641..."). + // This test verifies that buildImageURL stores those values without mangling. + const ( + serviceURL = "https://boot-operator.example.svc.cluster.local:8080" + imageName = "registry.global.example.com/ccloud-ghcr-io-mirror/gardenlinux/gardenlinux" + imageVersion = "sha256:a5f8b641e52e34b230f6335663fe85c94db89d7d34c184478ec0faaf6747703d" + kernelDigest = "sha256:f1b8b8dfd3b9f810662becdbcf508357fb71ad5c0c709a97350522d71e0592ad" + initrdDigest = "sha256:44d8ed8c6f3ca903cc52c3b281011869c2d5ebccfc662409dc559d6e2890234f" + squashDigest = "sha256:4b505f664719aa635a91cd1543026374ee6a09849edb29aca6096a256f51185d" + ) + + for _, tc := range []struct{ label, digest string }{ + {"kernel", kernelDigest}, + {"initrd", initrdDigest}, + {"squashfs", squashDigest}, + } { + rawURL := buildImageURL(serviceURL, imageName, imageVersion, tc.digest) + parsed, err := url.Parse(rawURL) + if err != nil { + t.Fatalf("%s: url.Parse(%q) error: %v", tc.label, rawURL, err) + } + q := parsed.Query() + if got := q.Get("imageName"); got != imageName { + t.Errorf("%s: imageName = %q, want %q (must not contain @sha256 suffix)", tc.label, got, imageName) + } + if got := q.Get("version"); got != imageVersion { + t.Errorf("%s: version = %q, want %q (sha256: prefix must be preserved)", tc.label, got, imageVersion) + } + if got := q.Get("layerDigest"); got != tc.digest { + t.Errorf("%s: layerDigest = %q, want %q", tc.label, got, tc.digest) + } + } +} + var _ = Describe("PatchServerBootConfigWithError", func() { var ns *corev1.Namespace diff --git a/internal/controller/serverbootconfiguration_pxe_controller.go b/internal/controller/serverbootconfiguration_pxe_controller.go index 5bebc00..32db81b 100644 --- a/internal/controller/serverbootconfiguration_pxe_controller.go +++ b/internal/controller/serverbootconfiguration_pxe_controller.go @@ -105,7 +105,7 @@ func (r *ServerBootConfigurationPXEReconciler) reconcile(ctx context.Context, lo } log.V(1).Info("Got system IP from BootConfig", "systemIPs", systemIPs) - kernelURL, initrdURL, squashFSURL, err := r.getImageDetailsFromConfig(ctx, bootConfig) + kernelURL, initrdURL, squashFSURL, err := r.getImageDetailsFromConfig(ctx, log, bootConfig) if err != nil { if patchErr := PatchServerBootConfigWithError(ctx, r.Client, types.NamespacedName{Name: bootConfig.Name, Namespace: bootConfig.Namespace}, err); patchErr != nil { @@ -200,20 +200,22 @@ func (r *ServerBootConfigurationPXEReconciler) getSystemIPFromBootConfig(ctx con return ExtractServerNetworkIDs(server, false), nil } -func (r *ServerBootConfigurationPXEReconciler) getImageDetailsFromConfig(ctx context.Context, config *metalv1alpha1.ServerBootConfiguration) (string, string, string, error) { +func (r *ServerBootConfigurationPXEReconciler) getImageDetailsFromConfig(ctx context.Context, log logr.Logger, config *metalv1alpha1.ServerBootConfiguration) (string, string, string, error) { imageName, imageVersion, err := ParseImageReference(config.Spec.Image) if err != nil { return "", "", "", err } + log.V(1).Info("Parsed image reference", "specImage", config.Spec.Image, "imageName", imageName, "imageVersion", imageVersion) kernelDigest, initrdDigest, squashFSDigest, err := r.getLayerDigestsFromNestedManifest(ctx, imageName, imageVersion) if err != nil { return "", "", "", fmt.Errorf("failed to fetch layer digests: %w", err) } - kernelURL := fmt.Sprintf("%s/image?imageName=%s&version=%s&layerDigest=%s", r.IPXEServiceURL, imageName, imageVersion, kernelDigest) - initrdURL := fmt.Sprintf("%s/image?imageName=%s&version=%s&layerDigest=%s", r.IPXEServiceURL, imageName, imageVersion, initrdDigest) - squashFSURL := fmt.Sprintf("%s/image?imageName=%s&version=%s&layerDigest=%s", r.IPXEServiceURL, imageName, imageVersion, squashFSDigest) + kernelURL := buildImageURL(r.IPXEServiceURL, imageName, imageVersion, kernelDigest) + initrdURL := buildImageURL(r.IPXEServiceURL, imageName, imageVersion, initrdDigest) + squashFSURL := buildImageURL(r.IPXEServiceURL, imageName, imageVersion, squashFSDigest) + log.V(1).Info("Built image URLs", "kernelURL", kernelURL, "initrdURL", initrdURL, "squashfsURL", squashFSURL) return kernelURL, initrdURL, squashFSURL, nil } diff --git a/server/imageproxyserver.go b/server/imageproxyserver.go index 92d10b8..617fbc4 100644 --- a/server/imageproxyserver.go +++ b/server/imageproxyserver.go @@ -524,7 +524,7 @@ func parseImageURL(queries url.Values) (imageDetails ImageDetails, err error) { version := queries.Get(versionKey) if ociImageName == "" || layerDigest == "" || version == "" { - return ImageDetails{}, fmt.Errorf("missing required query parameters 'image' or 'layer' or 'version'") + return ImageDetails{}, fmt.Errorf("missing required query parameters 'imageName', 'layerDigest', or 'version'") } ociImageName = strings.TrimSuffix(ociImageName, ".efi")