diff --git a/Dockerfile b/Dockerfile index 3babcf03..7338d43e 100644 --- a/Dockerfile +++ b/Dockerfile @@ -15,6 +15,7 @@ RUN go mod download COPY cmd/main.go cmd/main.go COPY api/ api/ COPY internal/controller/ internal/controller/ +COPY internal/registry/ internal/registry/ COPY server/ server/ COPY templates/ templates/ diff --git a/cmd/main.go b/cmd/main.go index 1446e718..ab05ab09 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -34,6 +34,7 @@ import ( bootv1alpha1 "github.com/ironcore-dev/boot-operator/api/v1alpha1" "github.com/ironcore-dev/boot-operator/internal/controller" + "github.com/ironcore-dev/boot-operator/internal/registry" bootserver "github.com/ironcore-dev/boot-operator/server" //+kubebuilder:scaffold:imports ) @@ -79,6 +80,7 @@ func main() { var ipxeServicePort int var imageServerURL string var architecture string + var allowedRegistries string flag.StringVar(&architecture, "architecture", "amd64", "Target system architecture (e.g., amd64, arm64)") flag.IntVar(&ipxeServicePort, "ipxe-service-port", 5000, "IPXE Service port to listen on.") @@ -98,6 +100,7 @@ func main() { flag.BoolVar(&secureMetrics, "metrics-secure", true, "If set the metrics endpoint is served securely") flag.BoolVar(&enableHTTP2, "enable-http2", false, "If set, HTTP/2 will be enabled for the metrics and webhook servers") + flag.StringVar(&allowedRegistries, "allowed-registries", "", "Comma-separated list of allowed OCI registries. Defaults to ghcr.io if not set.") controllers := switches.New( // core controllers @@ -227,6 +230,14 @@ func main() { os.Exit(1) } + // Initialize registry validator for OCI image validation + registryValidator := registry.NewValidator(allowedRegistries) + if allowedRegistries == "" { + setupLog.Info("Initialized registry validator", "allowedRegistries", "ghcr.io (default)") + } else { + setupLog.Info("Initialized registry validator", "allowedRegistries", allowedRegistries) + } + if controllers.Enabled(ipxeBootConfigController) { if err = (&controller.IPXEBootConfigReconciler{ Client: mgr.GetClient(), @@ -239,10 +250,11 @@ func main() { if controllers.Enabled(serverBootConfigControllerPxe) { if err = (&controller.ServerBootConfigurationPXEReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - IPXEServiceURL: ipxeServiceURL, - Architecture: architecture, + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + IPXEServiceURL: ipxeServiceURL, + Architecture: architecture, + RegistryValidator: registryValidator, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "ServerBootConfigPxe") os.Exit(1) @@ -251,10 +263,11 @@ func main() { if controllers.Enabled(serverBootConfigControllerHttp) { if err = (&controller.ServerBootConfigurationHTTPReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - ImageServerURL: imageServerURL, - Architecture: architecture, + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + ImageServerURL: imageServerURL, + Architecture: architecture, + RegistryValidator: registryValidator, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "ServerBootConfigHttp") os.Exit(1) @@ -311,7 +324,7 @@ func main() { }() setupLog.Info("starting image-proxy-server") - go bootserver.RunImageProxyServer(imageProxyServerAddr, mgr.GetClient(), serverLog.WithName("imageproxyserver")) + go bootserver.RunImageProxyServer(imageProxyServerAddr, mgr.GetClient(), registryValidator, serverLog.WithName("imageproxyserver")) setupLog.Info("starting manager") if err := mgr.Start(ctx); err != nil { diff --git a/cmdutils/suite_test.go b/cmdutils/suite_test.go index 75bd8f74..38f619ad 100644 --- a/cmdutils/suite_test.go +++ b/cmdutils/suite_test.go @@ -59,7 +59,7 @@ var _ = BeforeSuite(func() { // Note that you must have the required binaries setup under the bin directory to perform // the tests directly. When we run make test it will be setup and used automatically. BinaryAssetsDirectory: filepath.Join("..", "bin", "k8s", - fmt.Sprintf("1.34.0-%s-%s", runtime.GOOS, runtime.GOARCH)), + fmt.Sprintf("1.35.0-%s-%s", runtime.GOOS, runtime.GOARCH)), } sourceCfg, err := sourceEnv.Start() @@ -89,7 +89,7 @@ var _ = BeforeSuite(func() { // Note that you must have the required binaries setup under the bin directory to perform // the tests directly. When we run make test it will be setup and used automatically. BinaryAssetsDirectory: filepath.Join("..", "bin", "k8s", - fmt.Sprintf("1.34.0-%s-%s", runtime.GOOS, runtime.GOARCH)), + fmt.Sprintf("1.35.0-%s-%s", runtime.GOOS, runtime.GOARCH)), } // cfg is defined in this file globally. diff --git a/docs/README.md b/docs/README.md index 88b589fa..d7190162 100644 --- a/docs/README.md +++ b/docs/README.md @@ -19,16 +19,20 @@ Boot Operator includes the following key components: - Responds with an iPXE script, which the bare metal server uses to download the necessary OS components - This endpoint is typically called directly by the server during boot and is commonly used in PXE boot scenarios + - **HTTP Boot Server** - Handles `/httpboot` requests - Returns a JSON response containing the location of the UKI (Unified Kernel Image) that the server should download - The DHCP server extension typically handles the response and sends the UKI image location to the server - Common in modern cloud-native bare metal setups, especially for containers and minimal OS images + - **Image Proxy Server** - Handles `/image` requests - - Extracts layers from public OCI (Open Container Initiative) images, with current support for GHCR (GitHub Container Registry) only - - Downloads specific layers based on the requested URI and image specifications + - Extracts layers from OCI (Open Container Initiative) images, with support for multiple registries (e.g., GHCR, Docker Hub, and any OCI-compliant registry) + - Downloads specific layers based on the requested URI and image specifications + - Registry access is controlled via the `--allowed-registries` CLI flag (comma-separated list) + - By default (when not specified), only **ghcr.io** is allowed - Example: - `wget http://SERVER_ADDRESS:30007/image?imageName=ghcr.io/ironcore-dev/os-images/gardenlinux&version=1443.10&layerName=application/vnd.ironcore.image.squashfs.v1alpha1.squashfs` @@ -36,4 +40,34 @@ Boot Operator includes the following key components: - Handles `/ignition` requests - Responds with Ignition configuration content tailored to the client machine, identified by its UUID in the request URL. -These servers leverage Kubernetes controllers and API objects to manage the boot process and serve requests from bare metal machines. The architecture and specifics of the controllers and API objects are described in the architecture section of the documentation. \ No newline at end of file +These servers leverage Kubernetes controllers and API objects to manage the boot process and serve requests from bare metal machines. The architecture and specifics of the controllers and API objects are described in the architecture section of the documentation. + +## Registry Validation + +Boot Operator enforces OCI registry restrictions at two levels: + +1. **Controller level (early validation):** The PXE and HTTP boot controllers validate image references against the registry allow list during reconciliation. This means misconfigured or disallowed registries are rejected immediately when a `ServerBootConfiguration` is created, providing fast feedback before any machine attempts to boot. + +2. **Image Proxy Server level (runtime enforcement):** The image proxy server also validates registry domains before proxying layer downloads, acting as a second line of defense. + +Registry restrictions are configured via the `--allowed-registries` CLI flag on the manager binary. + +### Default Behavior + +By default (when `--allowed-registries` is not set), Boot Operator allows only **ghcr.io** registry. This provides a secure-by-default configuration with zero configuration needed for the common case. + +### Custom Configuration + +To allow additional registries or replace the default, use the `--allowed-registries` flag with a comma-separated list: + +```bash +--allowed-registries=ghcr.io,registry.example.com,quay.io +``` + +**Important:** When you set `--allowed-registries`, it completely replaces the default. If you want to use ghcr.io along with other registries, you must explicitly include `ghcr.io` in your list. + +### Registry Matching + +- Docker Hub variants (`docker.io`, `index.docker.io`, `registry-1.docker.io`) are normalized to `docker.io` for consistent matching. +- All registry domain matching is case-insensitive. +- Registries not in the allow list are denied. diff --git a/go.mod b/go.mod index 63e8c4d2..15c4b695 100644 --- a/go.mod +++ b/go.mod @@ -5,12 +5,14 @@ go 1.25.6 require ( github.com/containerd/containerd v1.7.30 github.com/coreos/butane v0.27.0 + github.com/distribution/reference v0.6.0 github.com/go-logr/logr v1.4.3 github.com/ironcore-dev/controller-utils v0.11.0 github.com/ironcore-dev/metal v0.0.0-20240624131301-18385f342755 github.com/ironcore-dev/metal-operator v0.3.0 github.com/onsi/ginkgo/v2 v2.28.1 github.com/onsi/gomega v1.39.1 + github.com/opencontainers/go-digest v1.0.0 github.com/opencontainers/image-spec v1.1.1 github.com/spf13/cobra v1.10.2 k8s.io/api v0.35.0 @@ -76,7 +78,6 @@ require ( github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.3-0.20250322232337-35a7c28c31ee // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect - github.com/opencontainers/go-digest v1.0.0 // indirect github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/prometheus/client_golang v1.23.2 // indirect diff --git a/go.sum b/go.sum index 3422208b..72485934 100644 --- a/go.sum +++ b/go.sum @@ -51,6 +51,8 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/distribution/reference v0.6.0 h1:0IXCQ5g4/QMHHkarYzh5l+u8T3t73zM5QvfrDyIgxBk= +github.com/distribution/reference v0.6.0/go.mod h1:BbU0aIcezP1/5jX/8MP0YiH4SdvB5Y4f/wlDRiLyi3E= github.com/emicklei/go-restful/v3 v3.13.0 h1:C4Bl2xDndpU6nJ4bc1jXd+uTmYPVUwkD6bFY/oTyCes= github.com/emicklei/go-restful/v3 v3.13.0/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc= github.com/evanphx/json-patch v0.5.2 h1:xVCHIVMUu1wtM/VkR9jVZ45N3FhZfYMMYGorLCR8P3k= diff --git a/internal/controller/serverbootconfig_helpers.go b/internal/controller/serverbootconfig_helpers.go new file mode 100644 index 00000000..af57c608 --- /dev/null +++ b/internal/controller/serverbootconfig_helpers.go @@ -0,0 +1,247 @@ +/* +Copyright 2024. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "encoding/json" + "fmt" + "io" + "strings" + + "github.com/containerd/containerd/remotes" + "github.com/distribution/reference" + metalv1alpha1 "github.com/ironcore-dev/metal-operator/api/v1alpha1" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" + corev1 "k8s.io/api/core/v1" + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +// ParseImageReference parses an OCI image reference and returns the image name and version. +// It handles tagged references, digest references, and untagged references (defaulting to "latest"). +func ParseImageReference(image string) (imageName, imageVersion string, err error) { + named, err := reference.ParseNormalizedNamed(image) + if err != nil { + return "", "", fmt.Errorf("invalid image reference: %w", err) + } + + if tagged, ok := named.(reference.Tagged); ok { + imageName = reference.FamiliarName(named) + imageVersion = tagged.Tag() + } else if canonical, ok := named.(reference.Canonical); ok { + imageName = reference.FamiliarName(named) + imageVersion = canonical.Digest().String() + } else { + // No tag or digest, use "latest" as default + imageName = reference.FamiliarName(named) + imageVersion = "latest" + } + + return imageName, imageVersion, nil +} + +// BuildImageReference constructs a properly formatted OCI image reference from name and version. +// Uses @ separator for digest-based references (sha256:..., sha512:...) and : for tags. +func BuildImageReference(imageName, imageVersion string) string { + if strings.HasPrefix(imageVersion, "sha256:") || strings.HasPrefix(imageVersion, "sha512:") { + return fmt.Sprintf("%s@%s", imageName, imageVersion) + } + return fmt.Sprintf("%s:%s", imageName, imageVersion) +} + +// FindManifestByArchitecture navigates an OCI image index to find the manifest for a specific architecture. +// If enableCNAMECompat is true, it first tries to find manifests using the legacy CNAME annotation approach. +// Returns the architecture-specific manifest, or an error if not found. +func FindManifestByArchitecture(ctx context.Context, resolver remotes.Resolver, name string, desc ocispec.Descriptor, architecture string, enableCNAMECompat bool) (ocispec.Manifest, error) { + manifestData, err := fetchContent(ctx, resolver, name, desc) + if err != nil { + return ocispec.Manifest{}, fmt.Errorf("failed to fetch manifest data: %w", err) + } + + var manifest ocispec.Manifest + if err := json.Unmarshal(manifestData, &manifest); err != nil { + return ocispec.Manifest{}, fmt.Errorf("failed to unmarshal manifest: %w", err) + } + + // If not an index, return the manifest directly + if desc.MediaType != ocispec.MediaTypeImageIndex { + return manifest, nil + } + + // Parse as index and find architecture-specific manifest + var indexManifest ocispec.Index + if err := json.Unmarshal(manifestData, &indexManifest); err != nil { + return ocispec.Manifest{}, fmt.Errorf("failed to unmarshal index manifest: %w", err) + } + + var targetManifestDesc ocispec.Descriptor + + // Backward compatibility for CNAME prefix based OCI (PXE only) + if enableCNAMECompat { + for _, m := range indexManifest.Manifests { + if strings.HasPrefix(m.Annotations["cname"], CNAMEPrefixMetalPXE) { + if m.Annotations["architecture"] == architecture { + targetManifestDesc = m + break + } + } + } + } + + // Standard platform-based architecture lookup + if targetManifestDesc.Digest == "" { + for _, m := range indexManifest.Manifests { + if m.Platform != nil && m.Platform.Architecture == architecture { + targetManifestDesc = m + break + } + } + } + + if targetManifestDesc.Digest == "" { + return ocispec.Manifest{}, fmt.Errorf("failed to find target manifest with architecture %s", architecture) + } + + // Fetch the nested manifest + nestedData, err := fetchContent(ctx, resolver, name, targetManifestDesc) + if err != nil { + return ocispec.Manifest{}, fmt.Errorf("failed to fetch nested manifest: %w", err) + } + + var nestedManifest ocispec.Manifest + if err := json.Unmarshal(nestedData, &nestedManifest); err != nil { + return ocispec.Manifest{}, fmt.Errorf("failed to unmarshal nested manifest: %w", err) + } + + return nestedManifest, nil +} + +// 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 { + ids := make([]string, 0, len(server.Status.NetworkInterfaces)) + + for _, nic := range server.Status.NetworkInterfaces { + // Add IPs + if len(nic.IPs) > 0 { + for _, ip := range nic.IPs { + ids = append(ids, ip.String()) + } + } else if nic.IP != nil && !nic.IP.IsZero() { + ids = append(ids, nic.IP.String()) + } + + // Add MAC address if requested + if includeMACAddresses && nic.MACAddress != "" { + ids = append(ids, nic.MACAddress) + } + } + + return ids +} + +// EnqueueServerBootConfigsReferencingSecret finds all ServerBootConfigurations in the same namespace +// that reference the given Secret via IgnitionSecretRef and returns reconcile requests for them. +func EnqueueServerBootConfigsReferencingSecret(ctx context.Context, c client.Client, secret client.Object) []reconcile.Request { + log := ctrl.LoggerFrom(ctx) + secretObj, ok := secret.(*corev1.Secret) + if !ok { + log.Error(nil, "Failed to decode object into Secret", "object", secret) + return nil + } + + bootConfigList := &metalv1alpha1.ServerBootConfigurationList{} + if err := c.List(ctx, bootConfigList, client.InNamespace(secretObj.Namespace)); err != nil { + log.Error(err, "Failed to list ServerBootConfiguration for Secret", "Secret", client.ObjectKeyFromObject(secretObj)) + return nil + } + + var requests []reconcile.Request + for _, bootConfig := range bootConfigList.Items { + if bootConfig.Spec.IgnitionSecretRef != nil && bootConfig.Spec.IgnitionSecretRef.Name == secretObj.Name { + requests = append(requests, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: bootConfig.Name, + Namespace: bootConfig.Namespace, + }, + }) + } + } + return requests +} + +// fetchContent fetches the content of an OCI descriptor using the provided resolver. +// It validates the content size matches the descriptor and returns the raw bytes. +func fetchContent(ctx context.Context, resolver remotes.Resolver, ref string, desc ocispec.Descriptor) ([]byte, error) { + fetcher, err := resolver.Fetcher(ctx, ref) + if err != nil { + return nil, fmt.Errorf("failed to get fetcher: %w", err) + } + + reader, err := fetcher.Fetch(ctx, desc) + if err != nil { + return nil, fmt.Errorf("failed to fetch content: %w", err) + } + + defer func() { + if cerr := reader.Close(); cerr != nil { + fmt.Printf("failed to close reader: %v\n", cerr) + } + }() + + data, err := io.ReadAll(reader) + if err != nil { + return nil, fmt.Errorf("failed to read content: %w", err) + } + + if int64(len(data)) != desc.Size { + return nil, fmt.Errorf("size mismatch: expected %d, got %d", desc.Size, len(data)) + } + + return data, nil +} + +// PatchServerBootConfigWithError updates the ServerBootConfiguration state to Error +// and sets an ImageValidation condition with the error details. +func PatchServerBootConfigWithError( + ctx context.Context, + c client.Client, + namespacedName types.NamespacedName, + err error, +) error { + var cur metalv1alpha1.ServerBootConfiguration + if fetchErr := c.Get(ctx, namespacedName, &cur); fetchErr != nil { + return fmt.Errorf("failed to fetch ServerBootConfiguration: %w", fetchErr) + } + base := cur.DeepCopy() + + cur.Status.State = metalv1alpha1.ServerBootConfigurationStateError + apimeta.SetStatusCondition(&cur.Status.Conditions, metav1.Condition{ + Type: "ImageValidation", + Status: metav1.ConditionFalse, + Reason: "ValidationFailed", + Message: err.Error(), + ObservedGeneration: cur.Generation, + }) + + return c.Status().Patch(ctx, &cur, client.MergeFrom(base)) +} diff --git a/internal/controller/serverbootconfig_helpers_test.go b/internal/controller/serverbootconfig_helpers_test.go new file mode 100644 index 00000000..9806d2eb --- /dev/null +++ b/internal/controller/serverbootconfig_helpers_test.go @@ -0,0 +1,178 @@ +// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and IronCore contributors +// SPDX-License-Identifier: Apache-2.0 + +package controller + +import ( + "errors" + "testing" + + metalv1alpha1 "github.com/ironcore-dev/metal-operator/api/v1alpha1" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + . "sigs.k8s.io/controller-runtime/pkg/envtest/komega" +) + +func TestBuildImageReference(t *testing.T) { + tests := []struct { + name string + imageName string + imageVersion string + want string + }{ + { + name: "tagged reference with simple tag", + imageName: "ghcr.io/ironcore-dev/gardenlinux", + imageVersion: "v1.0.0", + want: "ghcr.io/ironcore-dev/gardenlinux:v1.0.0", + }, + { + name: "tagged reference with latest", + imageName: "docker.io/library/ubuntu", + imageVersion: "latest", + want: "docker.io/library/ubuntu:latest", + }, + { + name: "digest reference with sha256", + imageName: "ghcr.io/ironcore-dev/gardenlinux", + imageVersion: "sha256:1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef", + want: "ghcr.io/ironcore-dev/gardenlinux@sha256:1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef", + }, + { + name: "digest reference with sha512", + imageName: "registry.example.com/myimage", + imageVersion: "sha512:abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890", + want: "registry.example.com/myimage@sha512:abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890", + }, + { + name: "tagged reference with numeric tag", + imageName: "localhost:5000/testimage", + imageVersion: "1.2.3", + want: "localhost:5000/testimage:1.2.3", + }, + { + name: "tagged reference with complex tag", + imageName: "registry.example.com/ironcore/gardenlinux-iso", + imageVersion: "arm64-v1.0.0-alpha", + want: "registry.example.com/ironcore/gardenlinux-iso:arm64-v1.0.0-alpha", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := BuildImageReference(tt.imageName, tt.imageVersion) + if got != tt.want { + t.Errorf("BuildImageReference(%q, %q) = %q, want %q", tt.imageName, tt.imageVersion, got, tt.want) + } + }) + } +} + +var _ = Describe("PatchServerBootConfigWithError", func() { + var ns *corev1.Namespace + + BeforeEach(func(ctx SpecContext) { + By("creating a test namespace") + ns = &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "test-", + }, + } + Expect(k8sClient.Create(ctx, ns)).To(Succeed()) + DeferCleanup(k8sClient.Delete, ns) + }) + + It("should patch ServerBootConfiguration with error state and condition", func(ctx SpecContext) { + By("creating a ServerBootConfiguration") + config := &metalv1alpha1.ServerBootConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-config", + Namespace: ns.Name, + }, + Spec: metalv1alpha1.ServerBootConfigurationSpec{ + ServerRef: corev1.LocalObjectReference{Name: "test-server"}, + Image: "test-image:latest", + }, + } + Expect(k8sClient.Create(ctx, config)).To(Succeed()) + + By("patching with error") + testErr := errors.New("registry validation failed: registry docker.io is not in the allowed list") + err := PatchServerBootConfigWithError(ctx, k8sClient, + types.NamespacedName{Name: config.Name, Namespace: config.Namespace}, testErr) + Expect(err).NotTo(HaveOccurred()) + + By("verifying the state is set to Error") + Eventually(Object(config)).Should(SatisfyAll( + HaveField("Status.State", metalv1alpha1.ServerBootConfigurationStateError), + )) + + By("verifying the ImageValidation condition is set") + var updated metalv1alpha1.ServerBootConfiguration + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: config.Name, Namespace: config.Namespace}, &updated)).To(Succeed()) + + condition := apimeta.FindStatusCondition(updated.Status.Conditions, "ImageValidation") + Expect(condition).NotTo(BeNil()) + Expect(condition.Type).To(Equal("ImageValidation")) + Expect(condition.Status).To(Equal(metav1.ConditionFalse)) + Expect(condition.Reason).To(Equal("ValidationFailed")) + Expect(condition.Message).To(Equal(testErr.Error())) + Expect(condition.ObservedGeneration).To(Equal(updated.Generation)) + }) + + It("should return error when ServerBootConfiguration does not exist", func(ctx SpecContext) { + By("attempting to patch non-existent config") + testErr := errors.New("some error") + err := PatchServerBootConfigWithError(ctx, k8sClient, + types.NamespacedName{Name: "non-existent", Namespace: ns.Name}, testErr) + + By("verifying error is returned") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failed to fetch ServerBootConfiguration")) + }) + + It("should update existing condition if called multiple times", func(ctx SpecContext) { + By("creating a ServerBootConfiguration") + config := &metalv1alpha1.ServerBootConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-config-update", + Namespace: ns.Name, + }, + Spec: metalv1alpha1.ServerBootConfigurationSpec{ + ServerRef: corev1.LocalObjectReference{Name: "test-server"}, + Image: "test-image:latest", + }, + } + Expect(k8sClient.Create(ctx, config)).To(Succeed()) + + By("patching with first error") + firstErr := errors.New("first error message") + err := PatchServerBootConfigWithError(ctx, k8sClient, + types.NamespacedName{Name: config.Name, Namespace: config.Namespace}, firstErr) + Expect(err).NotTo(HaveOccurred()) + + By("patching with second error") + secondErr := errors.New("second error message") + err = PatchServerBootConfigWithError(ctx, k8sClient, + types.NamespacedName{Name: config.Name, Namespace: config.Namespace}, secondErr) + Expect(err).NotTo(HaveOccurred()) + + By("verifying only one condition exists with latest message") + var updated metalv1alpha1.ServerBootConfiguration + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: config.Name, Namespace: config.Namespace}, &updated)).To(Succeed()) + + conditions := updated.Status.Conditions + imageValidationConditions := 0 + for _, c := range conditions { + if c.Type == "ImageValidation" { + imageValidationConditions++ + Expect(c.Message).To(Equal(secondErr.Error())) + } + } + Expect(imageValidationConditions).To(Equal(1)) + }) +}) diff --git a/internal/controller/serverbootconfiguration_http_controller.go b/internal/controller/serverbootconfiguration_http_controller.go index b0fb2c4e..7f80112b 100644 --- a/internal/controller/serverbootconfiguration_http_controller.go +++ b/internal/controller/serverbootconfiguration_http_controller.go @@ -12,7 +12,7 @@ import ( apimeta "k8s.io/apimachinery/pkg/api/meta" "github.com/containerd/containerd/remotes/docker" - ocispec "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/ironcore-dev/boot-operator/internal/registry" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" @@ -36,9 +36,10 @@ const ( type ServerBootConfigurationHTTPReconciler struct { client.Client - Scheme *runtime.Scheme - ImageServerURL string - Architecture string + Scheme *runtime.Scheme + ImageServerURL string + Architecture string + RegistryValidator *registry.Validator } //+kubebuilder:rbac:groups=metal.ironcore.dev,resources=serverbootconfigurations,verbs=get;list;watch @@ -87,7 +88,12 @@ func (r *ServerBootConfigurationHTTPReconciler) reconcile(ctx context.Context, l ukiURL, err := r.constructUKIURL(ctx, config.Spec.Image) if err != nil { - return ctrl.Result{}, fmt.Errorf("failed to construct UKI URL: %w", err) + log.Error(err, "Failed to construct UKI URL") + if patchErr := PatchServerBootConfigWithError(ctx, r.Client, + types.NamespacedName{Name: config.Name, Namespace: config.Namespace}, err); patchErr != nil { + return ctrl.Result{}, fmt.Errorf("failed to patch state to error: %w (original error: %w)", patchErr, err) + } + return ctrl.Result{}, err } log.V(1).Info("Extracted UKI URL for boot") @@ -148,6 +154,8 @@ func (r *ServerBootConfigurationHTTPReconciler) patchConfigStateFromHTTPState(ct switch httpBootConfig.Status.State { case bootv1alpha1.HTTPBootConfigStateReady: cur.Status.State = metalv1alpha1.ServerBootConfigurationStateReady + // Remove ImageValidation condition when transitioning to Ready + apimeta.RemoveStatusCondition(&cur.Status.Conditions, "ImageValidation") case bootv1alpha1.HTTPBootConfigStateError: cur.Status.State = metalv1alpha1.ServerBootConfigurationStateError } @@ -168,88 +176,47 @@ func (r *ServerBootConfigurationHTTPReconciler) getSystemUUIDFromServer(ctx cont return server.Spec.UUID, nil } -// getSystemNetworkIDsFromServer fetches the IPs from the network interfaces of the referenced Server object. +// getSystemNetworkIDsFromServer fetches the IPs and MAC addresses from the network interfaces of the referenced Server object. func (r *ServerBootConfigurationHTTPReconciler) getSystemNetworkIDsFromServer(ctx context.Context, config *metalv1alpha1.ServerBootConfiguration) ([]string, error) { server := &metalv1alpha1.Server{} if err := r.Get(ctx, client.ObjectKey{Name: config.Spec.ServerRef.Name}, server); err != nil { return nil, fmt.Errorf("failed to get Server: %w", err) } - nIDs := make([]string, 0, 2*len(server.Status.NetworkInterfaces)) - - for _, nic := range server.Status.NetworkInterfaces { - if len(nic.IPs) > 0 { - for _, ip := range nic.IPs { - nIDs = append(nIDs, ip.String()) - } - } else if nic.IP != nil && !nic.IP.IsZero() { - nIDs = append(nIDs, nic.IP.String()) - } - nIDs = append(nIDs, nic.MACAddress) - } - return nIDs, nil + return ExtractServerNetworkIDs(server, true), nil } func (r *ServerBootConfigurationHTTPReconciler) constructUKIURL(ctx context.Context, image string) (string, error) { - imageDetails := strings.Split(image, ":") - if len(imageDetails) != 2 { - return "", fmt.Errorf("invalid image format") + imageName, imageVersion, err := ParseImageReference(image) + if err != nil { + return "", err } - ukiDigest, err := r.getUKIDigestFromNestedManifest(ctx, imageDetails[0], imageDetails[1]) + ukiDigest, err := r.getUKIDigestFromNestedManifest(ctx, imageName, imageVersion) if err != nil { return "", fmt.Errorf("failed to fetch UKI layer digest: %w", err) } ukiDigest = strings.TrimPrefix(ukiDigest, "sha256:") - ukiURL := fmt.Sprintf("%s/%s/sha256-%s.efi", r.ImageServerURL, imageDetails[0], ukiDigest) + ukiURL := fmt.Sprintf("%s/%s/sha256-%s.efi", r.ImageServerURL, imageName, ukiDigest) return ukiURL, nil } func (r *ServerBootConfigurationHTTPReconciler) getUKIDigestFromNestedManifest(ctx context.Context, imageName, imageVersion string) (string, error) { + imageRef := BuildImageReference(imageName, imageVersion) + if err := r.RegistryValidator.ValidateImageRegistry(imageRef); err != nil { + return "", fmt.Errorf("registry validation failed: %w", err) + } + resolver := docker.NewResolver(docker.ResolverOptions{}) - imageRef := fmt.Sprintf("%s:%s", imageName, imageVersion) name, desc, err := resolver.Resolve(ctx, imageRef) if err != nil { return "", fmt.Errorf("failed to resolve image reference: %w", err) } - targetManifestDesc := desc - manifestData, err := fetchContent(ctx, resolver, name, desc) + manifest, err := FindManifestByArchitecture(ctx, resolver, name, desc, r.Architecture, false) if err != nil { - return "", fmt.Errorf("failed to fetch manifest data: %w", err) - } - - var manifest ocispec.Manifest - if desc.MediaType == ocispec.MediaTypeImageIndex { - var indexManifest ocispec.Index - if err := json.Unmarshal(manifestData, &indexManifest); err != nil { - return "", fmt.Errorf("failed to unmarshal index manifest: %w", err) - } - - for _, manifest := range indexManifest.Manifests { - platform := manifest.Platform - if manifest.Platform != nil && platform.Architecture == r.Architecture { - targetManifestDesc = manifest - break - } - } - if targetManifestDesc.Digest == "" { - return "", fmt.Errorf("failed to find target manifest with architecture %s", r.Architecture) - } - - nestedData, err := fetchContent(ctx, resolver, name, targetManifestDesc) - if err != nil { - return "", fmt.Errorf("failed to fetch nested manifest: %w", err) - } - - if err := json.Unmarshal(nestedData, &manifest); err != nil { - return "", fmt.Errorf("failed to unmarshal nested manifest: %w", err) - } - } else { - if err := json.Unmarshal(manifestData, &manifest); err != nil { - return "", fmt.Errorf("failed to unmarshal manifest: %w", err) - } + return "", err } for _, layer := range manifest.Layers { @@ -262,31 +229,7 @@ func (r *ServerBootConfigurationHTTPReconciler) getUKIDigestFromNestedManifest(c } func (r *ServerBootConfigurationHTTPReconciler) enqueueServerBootConfigReferencingIgnitionSecret(ctx context.Context, secret client.Object) []reconcile.Request { - log := ctrl.LoggerFrom(ctx) - secretObj, ok := secret.(*corev1.Secret) - if !ok { - log.Error(nil, "can't decode object into Secret", secret) - return nil - } - - bootConfigList := &metalv1alpha1.ServerBootConfigurationList{} - if err := r.List(ctx, bootConfigList, client.InNamespace(secretObj.Namespace)); err != nil { - log.Error(err, "failed to list ServerBootConfiguration for secret", secret) - return nil - } - - var requests []reconcile.Request - for _, bootConfig := range bootConfigList.Items { - if bootConfig.Spec.IgnitionSecretRef != nil && bootConfig.Spec.IgnitionSecretRef.Name == secretObj.Name { - requests = append(requests, reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: bootConfig.Name, - Namespace: bootConfig.Namespace, - }, - }) - } - } - return requests + return EnqueueServerBootConfigsReferencingSecret(ctx, r.Client, secret) } // SetupWithManager sets up the controller with the Manager. diff --git a/internal/controller/serverbootconfiguration_http_controller_test.go b/internal/controller/serverbootconfiguration_http_controller_test.go index 1f80c47c..64049023 100644 --- a/internal/controller/serverbootconfiguration_http_controller_test.go +++ b/internal/controller/serverbootconfiguration_http_controller_test.go @@ -65,7 +65,7 @@ var _ = Describe("ServerBootConfiguration Controller", func() { ServerRef: corev1.LocalObjectReference{ Name: server.Name, }, - Image: "ghcr.io/ironcore-dev/os-images/test-image:100.1", + Image: MockImageRef("ironcore-dev/os-images/test-image", "100.1"), IgnitionSecretRef: &corev1.LocalObjectReference{Name: "foo"}, }, } diff --git a/internal/controller/serverbootconfiguration_pxe_controller.go b/internal/controller/serverbootconfiguration_pxe_controller.go index 8b7220c4..30ef100b 100644 --- a/internal/controller/serverbootconfiguration_pxe_controller.go +++ b/internal/controller/serverbootconfiguration_pxe_controller.go @@ -20,8 +20,6 @@ import ( "context" "encoding/json" "fmt" - "io" - "strings" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" @@ -29,6 +27,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/ironcore-dev/boot-operator/api/v1alpha1" + "github.com/ironcore-dev/boot-operator/internal/registry" apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -39,9 +38,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" - "github.com/containerd/containerd/remotes" "github.com/containerd/containerd/remotes/docker" - ocispec "github.com/opencontainers/image-spec/specs-go/v1" ) const ( @@ -56,9 +53,10 @@ const ( type ServerBootConfigurationPXEReconciler struct { client.Client - Scheme *runtime.Scheme - IPXEServiceURL string - Architecture string + Scheme *runtime.Scheme + IPXEServiceURL string + Architecture string + RegistryValidator *registry.Validator } //+kubebuilder:rbac:groups=metal.ironcore.dev,resources=serverbootconfigurations,verbs=get;list;watch @@ -108,10 +106,11 @@ func (r *ServerBootConfigurationPXEReconciler) reconcile(ctx context.Context, lo kernelURL, initrdURL, squashFSURL, err := r.getImageDetailsFromConfig(ctx, bootConfig) if err != nil { - if err := r.patchState(ctx, bootConfig, metalv1alpha1.ServerBootConfigurationStateError); err != nil { - return ctrl.Result{}, fmt.Errorf("failed to patch server boot config state to %s: %w", metalv1alpha1.ServerBootConfigurationStateError, err) + if patchErr := PatchServerBootConfigWithError(ctx, r.Client, + types.NamespacedName{Name: bootConfig.Name, Namespace: bootConfig.Namespace}, err); patchErr != nil { + return ctrl.Result{}, fmt.Errorf("failed to patch server boot config state: %w (original error: %w)", patchErr, err) } - return ctrl.Result{}, fmt.Errorf("failed to get image details from BootConfig: %w", err) + return ctrl.Result{}, err } log.V(1).Info("Extracted OS image layer details") @@ -169,6 +168,8 @@ func (r *ServerBootConfigurationPXEReconciler) patchConfigStateFromIPXEState(ctx switch config.Status.State { case v1alpha1.IPXEBootConfigStateReady: bootConfig.Status.State = metalv1alpha1.ServerBootConfigurationStateReady + // Remove ImageValidation condition when transitioning to Ready + apimeta.RemoveStatusCondition(&bootConfig.Status.Conditions, "ImageValidation") case v1alpha1.IPXEBootConfigStateError: bootConfig.Status.State = metalv1alpha1.ServerBootConfigurationStateError } @@ -180,15 +181,6 @@ func (r *ServerBootConfigurationPXEReconciler) patchConfigStateFromIPXEState(ctx return r.Status().Patch(ctx, bootConfig, client.MergeFrom(bootConfigBase)) } -func (r *ServerBootConfigurationPXEReconciler) patchState(ctx context.Context, config *metalv1alpha1.ServerBootConfiguration, state metalv1alpha1.ServerBootConfigurationState) error { - configBase := config.DeepCopy() - config.Status.State = state - if err := r.Status().Patch(ctx, config, client.MergeFrom(configBase)); err != nil { - return err - } - return nil -} - func (r *ServerBootConfigurationPXEReconciler) getSystemUUIDFromBootConfig(ctx context.Context, config *metalv1alpha1.ServerBootConfiguration) (string, error) { server := &metalv1alpha1.Server{} if err := r.Get(ctx, client.ObjectKey{Name: config.Spec.ServerRef.Name}, server); err != nil { @@ -204,102 +196,42 @@ func (r *ServerBootConfigurationPXEReconciler) getSystemIPFromBootConfig(ctx con return nil, err } - systemIPs := make([]string, 0, len(server.Status.NetworkInterfaces)) - for _, nic := range server.Status.NetworkInterfaces { - if len(nic.IPs) > 0 { - for _, ip := range nic.IPs { - systemIPs = append(systemIPs, ip.String()) - } - continue - } - if nic.IP != nil && !nic.IP.IsZero() { - systemIPs = append(systemIPs, nic.IP.String()) - } - } - - return systemIPs, nil + return ExtractServerNetworkIDs(server, false), nil } func (r *ServerBootConfigurationPXEReconciler) getImageDetailsFromConfig(ctx context.Context, config *metalv1alpha1.ServerBootConfiguration) (string, string, string, error) { - imageDetails := strings.Split(config.Spec.Image, ":") - if len(imageDetails) != 2 { - return "", "", "", fmt.Errorf("invalid image format") + imageName, imageVersion, err := ParseImageReference(config.Spec.Image) + if err != nil { + return "", "", "", err } - kernelDigest, initrdDigest, squashFSDigest, err := r.getLayerDigestsFromNestedManifest(ctx, imageDetails[0], imageDetails[1]) + 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, imageDetails[0], imageDetails[1], kernelDigest) - initrdURL := fmt.Sprintf("%s/image?imageName=%s&version=%s&layerDigest=%s", r.IPXEServiceURL, imageDetails[0], imageDetails[1], initrdDigest) - squashFSURL := fmt.Sprintf("%s/image?imageName=%s&version=%s&layerDigest=%s", r.IPXEServiceURL, imageDetails[0], imageDetails[1], squashFSDigest) + 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) return kernelURL, initrdURL, squashFSURL, nil } func (r *ServerBootConfigurationPXEReconciler) getLayerDigestsFromNestedManifest(ctx context.Context, imageName, imageVersion string) (string, string, string, error) { + imageRef := BuildImageReference(imageName, imageVersion) + if err := r.RegistryValidator.ValidateImageRegistry(imageRef); err != nil { + return "", "", "", fmt.Errorf("registry validation failed: %w", err) + } + resolver := docker.NewResolver(docker.ResolverOptions{}) - imageRef := fmt.Sprintf("%s:%s", imageName, imageVersion) name, desc, err := resolver.Resolve(ctx, imageRef) if err != nil { return "", "", "", fmt.Errorf("failed to resolve image reference: %w", err) } - manifestData, err := fetchContent(ctx, resolver, name, desc) + manifest, err := FindManifestByArchitecture(ctx, resolver, name, desc, r.Architecture, true) if err != nil { - return "", "", "", fmt.Errorf("failed to fetch manifest data: %w", err) - } - - var manifest ocispec.Manifest - if err := json.Unmarshal(manifestData, &manifest); err != nil { - return "", "", "", fmt.Errorf("failed to unmarshal index manifest: %w", err) - } - - if desc.MediaType == ocispec.MediaTypeImageIndex { - var targetManifestDesc ocispec.Descriptor - var indexManifest ocispec.Index - if err := json.Unmarshal(manifestData, &indexManifest); err != nil { - return "", "", "", fmt.Errorf("failed to unmarshal index manifest: %w", err) - } - - // Backward compatibility for CNAME prefix based OCI - // TODO: To be removed later - for _, manifest := range indexManifest.Manifests { - if strings.HasPrefix(manifest.Annotations["cname"], CNAMEPrefixMetalPXE) { - if manifest.Annotations["architecture"] == r.Architecture { - targetManifestDesc = manifest - break - } - } - } - - if targetManifestDesc.Digest == "" { - for _, manifest := range indexManifest.Manifests { - platform := manifest.Platform - if manifest.Platform != nil { - if platform.Architecture == r.Architecture { - targetManifestDesc = manifest - break - } - } - } - } - - if targetManifestDesc.Digest == "" { - return "", "", "", fmt.Errorf("failed to find target manifest with architecture %s", r.Architecture) - } - - nestedData, err := fetchContent(ctx, resolver, name, targetManifestDesc) - if err != nil { - return "", "", "", fmt.Errorf("failed to fetch nested manifest: %w", err) - } - - var nestedManifest ocispec.Manifest - if err := json.Unmarshal(nestedData, &nestedManifest); err != nil { - return "", "", "", fmt.Errorf("failed to unmarshal nested manifest: %w", err) - } - manifest = nestedManifest + return "", "", "", err } var kernelDigest, initrdDigest, squashFSDigest string @@ -327,61 +259,8 @@ func (r *ServerBootConfigurationPXEReconciler) getLayerDigestsFromNestedManifest return kernelDigest, initrdDigest, squashFSDigest, nil } -func fetchContent(ctx context.Context, resolver remotes.Resolver, ref string, desc ocispec.Descriptor) ([]byte, error) { - fetcher, err := resolver.Fetcher(ctx, ref) - if err != nil { - return nil, fmt.Errorf("failed to get fetcher: %w", err) - } - - reader, err := fetcher.Fetch(ctx, desc) - if err != nil { - return nil, fmt.Errorf("failed to fetch content: %w", err) - } - - defer func() { - if cerr := reader.Close(); cerr != nil { - fmt.Printf("failed to close reader: %v\n", cerr) - } - }() - - data, err := io.ReadAll(reader) - if err != nil { - return nil, fmt.Errorf("failed to read content: %w", err) - } - - if int64(len(data)) != desc.Size { - return nil, fmt.Errorf("size mismatch: expected %d, got %d", desc.Size, len(data)) - } - - return data, nil -} - func (r *ServerBootConfigurationPXEReconciler) enqueueServerBootConfigFromIgnitionSecret(ctx context.Context, secret client.Object) []reconcile.Request { - log := ctrl.LoggerFrom(ctx) - secretObj, ok := secret.(*corev1.Secret) - if !ok { - log.Error(nil, "can't decode object into Secret", secret) - return nil - } - - bootConfigList := &metalv1alpha1.ServerBootConfigurationList{} - if err := r.List(ctx, bootConfigList, client.InNamespace(secretObj.Namespace)); err != nil { - log.Error(err, "failed to list ServerBootConfiguration for Secret", "Secret", client.ObjectKeyFromObject(secretObj)) - return nil - } - - var requests []reconcile.Request - for _, bootConfig := range bootConfigList.Items { - if bootConfig.Spec.IgnitionSecretRef != nil && bootConfig.Spec.IgnitionSecretRef.Name == secretObj.Name { - requests = append(requests, reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: bootConfig.Name, - Namespace: bootConfig.Namespace, - }, - }) - } - } - return requests + return EnqueueServerBootConfigsReferencingSecret(ctx, r.Client, secret) } // SetupWithManager sets up the controller with the Manager. diff --git a/internal/controller/serverbootconfiguration_pxe_controller_test.go b/internal/controller/serverbootconfiguration_pxe_controller_test.go index 604862cd..59daf307 100644 --- a/internal/controller/serverbootconfiguration_pxe_controller_test.go +++ b/internal/controller/serverbootconfiguration_pxe_controller_test.go @@ -64,7 +64,7 @@ var _ = Describe("ServerBootConfiguration Controller", func() { ServerRef: corev1.LocalObjectReference{ Name: server.Name, }, - Image: "ghcr.io/ironcore-dev/os-images/gardenlinux:1877.0", + Image: MockImageRef("ironcore-dev/os-images/gardenlinux", "1877.0"), IgnitionSecretRef: &corev1.LocalObjectReference{Name: "foo"}, }, } @@ -125,7 +125,7 @@ var _ = Describe("ServerBootConfiguration Controller", func() { ServerRef: corev1.LocalObjectReference{ Name: server.Name, }, - Image: "ghcr.io/gardenlinux/gardenlinux:1772.0", + Image: MockImageRef("gardenlinux/gardenlinux", "1772.0"), IgnitionSecretRef: &corev1.LocalObjectReference{Name: "foo"}, }, } diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 3bd6d233..e89ba00c 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -13,6 +13,7 @@ import ( "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/config" + metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" "github.com/ironcore-dev/controller-utils/modutils" metalv1alpha1 "github.com/ironcore-dev/metal-operator/api/v1alpha1" @@ -33,6 +34,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log/zap" bootv1alpha1 "github.com/ironcore-dev/boot-operator/api/v1alpha1" + "github.com/ironcore-dev/boot-operator/internal/registry" + testregistry "github.com/ironcore-dev/boot-operator/test/registry" //+kubebuilder:scaffold:imports ) @@ -43,9 +46,11 @@ const ( ) var ( - cfg *rest.Config - k8sClient client.Client - testEnv *envtest.Environment + cfg *rest.Config + k8sClient client.Client + testEnv *envtest.Environment + mockRegistry *testregistry.MockRegistry + allowedRegistries string ) func TestControllers(t *testing.T) { @@ -61,6 +66,18 @@ func TestControllers(t *testing.T) { var _ = BeforeSuite(func() { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) + By("starting mock OCI registry") + mockRegistry = testregistry.NewMockRegistry() + DeferCleanup(mockRegistry.Close) + + // Push test images to mock registry (using simple paths without localhost prefix) + Expect(mockRegistry.PushPXEImage("ironcore-dev/os-images/gardenlinux", "1877.0", runtime.GOARCH)).To(Succeed()) + Expect(mockRegistry.PushPXEImageOldFormat("gardenlinux/gardenlinux", "1772.0", runtime.GOARCH)).To(Succeed()) + Expect(mockRegistry.PushHTTPImage("ironcore-dev/os-images/test-image", "100.1")).To(Succeed()) + + // Set allowed registries to use mock registry + allowedRegistries = mockRegistry.RegistryAddress() + By("bootstrapping test environment") testEnv = &envtest.Environment{ CRDDirectoryPaths: []string{ @@ -75,7 +92,7 @@ var _ = BeforeSuite(func() { // Note that you must have the required binaries setup under the bin directory to perform // the tests directly. When we run make test it will be setup and used automatically. BinaryAssetsDirectory: filepath.Join("..", "..", "bin", "k8s", - fmt.Sprintf("1.34.0-%s-%s", runtime.GOOS, runtime.GOARCH)), + fmt.Sprintf("1.35.0-%s-%s", runtime.GOOS, runtime.GOARCH)), } var err error @@ -120,6 +137,9 @@ func SetupTest() *corev1.Namespace { k8sManager, err := ctrl.NewManager(cfg, ctrl.Options{ Scheme: scheme.Scheme, + Metrics: metricsserver.Options{ + BindAddress: "0", // Disable metrics server to avoid port 8080 conflicts with Tilt + }, Controller: config.Controller{ // need to skip unique controller name validation // since all tests need a dedicated controller @@ -128,17 +148,22 @@ func SetupTest() *corev1.Namespace { }) Expect(err).ToNot(HaveOccurred()) + registryValidator := registry.NewValidator(allowedRegistries) + Expect((&ServerBootConfigurationPXEReconciler{ - Client: k8sManager.GetClient(), - Scheme: k8sManager.GetScheme(), - IPXEServiceURL: "http://localhost:5000", - Architecture: "arm64", + Client: k8sManager.GetClient(), + Scheme: k8sManager.GetScheme(), + IPXEServiceURL: "http://localhost:5000", + Architecture: runtime.GOARCH, + RegistryValidator: registryValidator, }).SetupWithManager(k8sManager)).To(Succeed()) Expect((&ServerBootConfigurationHTTPReconciler{ - Client: k8sManager.GetClient(), - Scheme: k8sManager.GetScheme(), - ImageServerURL: "http://localhost:5000/httpboot", + Client: k8sManager.GetClient(), + Scheme: k8sManager.GetScheme(), + ImageServerURL: "http://localhost:5000/httpboot", + Architecture: runtime.GOARCH, + RegistryValidator: registryValidator, }).SetupWithManager(k8sManager)).To(Succeed()) go func() { @@ -149,3 +174,8 @@ func SetupTest() *corev1.Namespace { return ns } + +// MockImageRef returns a fully qualified image reference for the mock registry +func MockImageRef(name, tag string) string { + return fmt.Sprintf("%s/%s:%s", mockRegistry.RegistryAddress(), name, tag) +} diff --git a/internal/registry/validation.go b/internal/registry/validation.go new file mode 100644 index 00000000..ca185d60 --- /dev/null +++ b/internal/registry/validation.go @@ -0,0 +1,110 @@ +// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and IronCore contributors +// SPDX-License-Identifier: Apache-2.0 + +package registry + +import ( + "fmt" + "strings" + + "github.com/distribution/reference" +) + +const ( + // DefaultRegistry is the default Docker Hub registry domain + DefaultRegistry = "registry-1.docker.io" + // DockerHubDomain is the canonical short domain for Docker Hub + DockerHubDomain = "docker.io" + // DefaultAllowedRegistry is the default registry allowed when no configuration is provided + DefaultAllowedRegistry = "ghcr.io" +) + +// Validator provides registry validation with configurable allow list. +// If no allowed registries are configured, it defaults to allowing ghcr.io only. +type Validator struct { + AllowedRegistries string +} + +// NewValidator creates a new Validator with the given allowed registry list. +// The list is a comma-separated string of registry domains. +// If empty, defaults to allowing ghcr.io only. +func NewValidator(allowedRegistries string) *Validator { + return &Validator{ + AllowedRegistries: allowedRegistries, + } +} + +// ExtractRegistryDomain extracts the registry domain from an OCI image reference +// using the canonical Docker reference parser from github.com/distribution/reference. +// Returns an error if the image reference is malformed. +func ExtractRegistryDomain(imageRef string) (string, error) { + named, err := reference.ParseNormalizedNamed(imageRef) + if err != nil { + return "", fmt.Errorf("invalid image reference: %w", err) + } + domain := reference.Domain(named) + // The reference library normalizes Docker Hub to "docker.io", + // but we use "registry-1.docker.io" as our canonical constant. + if domain == DockerHubDomain { + return DefaultRegistry, nil + } + return domain, nil +} + +// normalizeDockerHubDomain normalizes Docker Hub domain variants to canonical form. +// All registry domains are converted to lowercase for case-insensitive comparison, +// as DNS/domain names are case-insensitive by specification. +func normalizeDockerHubDomain(domain string) string { + lowerDomain := strings.ToLower(domain) + switch lowerDomain { + case DockerHubDomain, "index.docker.io", DefaultRegistry: + return DockerHubDomain + default: + return lowerDomain + } +} + +// isInList checks if a value is in a comma-separated list (exact match only). +func isInList(registry string, list string) bool { + if list == "" { + return false + } + + // Normalize the registry domain for comparison + normalizedRegistry := normalizeDockerHubDomain(registry) + + items := strings.Split(list, ",") + for _, item := range items { + normalizedItem := normalizeDockerHubDomain(strings.TrimSpace(item)) + if normalizedItem == normalizedRegistry { + return true + } + } + return false +} + +// IsRegistryAllowed checks if a registry is allowed based on the allow list. +// If no allowed registries are configured, it defaults to allowing ghcr.io only. +func (v *Validator) IsRegistryAllowed(registry string) bool { + if v.AllowedRegistries != "" { + return isInList(registry, v.AllowedRegistries) + } + + // Default to allowing ghcr.io when no configuration is provided + return normalizeDockerHubDomain(registry) == DefaultAllowedRegistry +} + +// ValidateImageRegistry validates that an image reference uses an allowed registry. +func (v *Validator) ValidateImageRegistry(imageRef string) error { + registry, err := ExtractRegistryDomain(imageRef) + if err != nil { + return err + } + if !v.IsRegistryAllowed(registry) { + if v.AllowedRegistries != "" { + return fmt.Errorf("registry not allowed: %s (allowed registries: %s)", registry, v.AllowedRegistries) + } + return fmt.Errorf("registry not allowed: %s (only %s is allowed by default, use --allowed-registries to configure)", registry, DefaultAllowedRegistry) + } + return nil +} diff --git a/internal/registry/validation_test.go b/internal/registry/validation_test.go new file mode 100644 index 00000000..40fccfbd --- /dev/null +++ b/internal/registry/validation_test.go @@ -0,0 +1,373 @@ +// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and IronCore contributors +// SPDX-License-Identifier: Apache-2.0 + +package registry + +import ( + "testing" +) + +func TestExtractRegistryDomain(t *testing.T) { + tests := []struct { + name string + imageRef string + want string + wantErr bool + }{ + { + name: "ghcr.io with tag", + imageRef: "ghcr.io/ironcore-dev/os-images/gardenlinux:1877.0", + want: "ghcr.io", + }, + { + name: "custom registry with tag", + imageRef: "registry.example.com/ironcore/gardenlinux-iso:arm64", + want: "registry.example.com", + }, + { + name: "docker hub explicit", + imageRef: "docker.io/library/ubuntu:latest", + want: "registry-1.docker.io", + }, + { + name: "docker hub implicit with namespace", + imageRef: "library/ubuntu:latest", + want: "registry-1.docker.io", + }, + { + name: "docker hub implicit without namespace", + imageRef: "ubuntu:latest", + want: "registry-1.docker.io", + }, + { + name: "localhost with port", + imageRef: "localhost:5000/test-image:latest", + want: "localhost:5000", + }, + { + name: "registry with port", + imageRef: "registry.example.com:8080/namespace/image:tag", + want: "registry.example.com:8080", + }, + { + name: "no tag no digest", + imageRef: "ghcr.io/namespace/image", + want: "ghcr.io", + }, + { + name: "malformed reference returns error", + imageRef: "not a valid@image:reference", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := ExtractRegistryDomain(tt.imageRef) + if (err != nil) != tt.wantErr { + t.Errorf("ExtractRegistryDomain(%q) error = %v, wantErr %v", tt.imageRef, err, tt.wantErr) + return + } + if !tt.wantErr && got != tt.want { + t.Errorf("ExtractRegistryDomain(%q) = %q, want %q", tt.imageRef, got, tt.want) + } + }) + } +} + +func TestNormalizeDockerHubDomain(t *testing.T) { + tests := []struct { + name string + domain string + want string + }{ + { + name: "lowercase docker.io", + domain: "docker.io", + want: "docker.io", + }, + { + name: "uppercase DOCKER.IO", + domain: "DOCKER.IO", + want: "docker.io", + }, + { + name: "mixed case Docker.Io", + domain: "Docker.Io", + want: "docker.io", + }, + { + name: "lowercase index.docker.io", + domain: "index.docker.io", + want: "docker.io", + }, + { + name: "uppercase INDEX.DOCKER.IO", + domain: "INDEX.DOCKER.IO", + want: "docker.io", + }, + { + name: "mixed case Index.Docker.IO", + domain: "Index.Docker.IO", + want: "docker.io", + }, + { + name: "lowercase registry-1.docker.io", + domain: "registry-1.docker.io", + want: "docker.io", + }, + { + name: "uppercase REGISTRY-1.DOCKER.IO", + domain: "REGISTRY-1.DOCKER.IO", + want: "docker.io", + }, + { + name: "non-Docker Hub - ghcr.io preserved", + domain: "ghcr.io", + want: "ghcr.io", + }, + { + name: "non-Docker Hub - GHCR.IO normalized to lowercase", + domain: "GHCR.IO", + want: "ghcr.io", + }, + { + name: "non-Docker Hub - custom registry preserved", + domain: "registry.example.com", + want: "registry.example.com", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := normalizeDockerHubDomain(tt.domain) + if got != tt.want { + t.Errorf("normalizeDockerHubDomain(%q) = %q, want %q", tt.domain, got, tt.want) + } + }) + } +} + +func TestIsRegistryAllowed(t *testing.T) { + tests := []struct { + name string + registry string + allowList string + want bool + description string + }{ + { + name: "allowed registry - ghcr.io", + registry: "ghcr.io", + allowList: "ghcr.io,registry.example.com", + want: true, + description: "ghcr.io is in allow list", + }, + { + name: "allowed registry - custom", + registry: "registry.example.com", + allowList: "ghcr.io,registry.example.com", + want: true, + description: "registry.example.com is in allow list", + }, + { + name: "blocked registry - docker.io with allow list", + registry: "docker.io", + allowList: "ghcr.io,registry.example.com", + want: false, + description: "docker.io is NOT in allow list", + }, + { + name: "default - ghcr.io allowed when no config", + registry: "ghcr.io", + allowList: "", + want: true, + description: "ghcr.io is allowed by default when no configuration", + }, + { + name: "default - docker.io blocked when no config", + registry: "docker.io", + allowList: "", + want: false, + description: "docker.io is blocked by default when no configuration", + }, + { + name: "default - other registry blocked when no config", + registry: "registry.example.com", + allowList: "", + want: false, + description: "other registries blocked by default when no configuration", + }, + { + name: "whitespace handling", + registry: "ghcr.io", + allowList: " ghcr.io , registry.example.com ", + want: true, + description: "handles whitespace in allow list", + }, + { + name: "case-insensitive non-Docker registry matching", + registry: "GHCR.IO", + allowList: "ghcr.io", + want: true, + description: "all registries are case-insensitive (GHCR.IO matches ghcr.io)", + }, + { + name: "case-insensitive default - uppercase GHCR.IO", + registry: "GHCR.IO", + allowList: "", + want: true, + description: "uppercase GHCR.IO matches default ghcr.io", + }, + { + name: "case-insensitive default - mixed case Ghcr.Io", + registry: "Ghcr.Io", + allowList: "", + want: true, + description: "mixed case Ghcr.Io matches default ghcr.io", + }, + { + name: "custom allow list replaces default", + registry: "ghcr.io", + allowList: "docker.io,registry.example.com", + want: false, + description: "ghcr.io blocked when custom allow list doesn't include it", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + v := NewValidator(tt.allowList) + got := v.IsRegistryAllowed(tt.registry) + if got != tt.want { + t.Errorf("IsRegistryAllowed(%q) = %v, want %v (%s)", tt.registry, got, tt.want, tt.description) + } + }) + } +} + +func TestValidateImageRegistry(t *testing.T) { + tests := []struct { + name string + imageRef string + allowList string + wantErr bool + errContains string + description string + }{ + { + name: "allowed - ghcr.io image", + imageRef: "ghcr.io/ironcore-dev/os-images/gardenlinux:1877.0", + allowList: "ghcr.io,registry.example.com", + wantErr: false, + description: "ghcr.io image should be allowed", + }, + { + name: "allowed - custom registry image", + imageRef: "registry.example.com/ironcore/gardenlinux-iso:arm64", + allowList: "ghcr.io,registry.example.com", + wantErr: false, + description: "registry.example.com image should be allowed", + }, + { + name: "blocked - docker.io with allow list", + imageRef: "docker.io/library/ubuntu:latest", + allowList: "ghcr.io,registry.example.com", + wantErr: true, + errContains: "registry not allowed: registry-1.docker.io", + description: "docker.io should be blocked when not in allow list", + }, + { + name: "blocked - implicit docker hub", + imageRef: "ubuntu:latest", + allowList: "ghcr.io,registry.example.com", + wantErr: true, + errContains: "registry not allowed: registry-1.docker.io", + description: "implicit docker hub should be blocked", + }, + { + name: "error shows allowed registries", + imageRef: "docker.io/library/alpine:latest", + allowList: "ghcr.io,registry.example.com", + wantErr: true, + errContains: "allowed registries: ghcr.io,registry.example.com", + description: "error message should show the allowed registries", + }, + { + name: "default allows ghcr.io", + imageRef: "ghcr.io/test/image:latest", + allowList: "", + wantErr: false, + description: "ghcr.io should be allowed by default", + }, + { + name: "default blocks docker.io", + imageRef: "docker.io/library/nginx:latest", + allowList: "", + wantErr: true, + errContains: "only ghcr.io is allowed by default", + description: "docker.io should be blocked by default", + }, + { + name: "default blocks other registries", + imageRef: "registry.example.com/test/image:latest", + allowList: "", + wantErr: true, + errContains: "only ghcr.io is allowed by default", + description: "other registries should be blocked by default", + }, + { + name: "custom list replaces default - ghcr.io blocked", + imageRef: "ghcr.io/test/image:latest", + allowList: "docker.io,registry.example.com", + wantErr: true, + errContains: "registry not allowed: ghcr.io", + description: "ghcr.io should be blocked when custom list doesn't include it", + }, + { + name: "custom list replaces default - docker.io allowed", + imageRef: "docker.io/library/redis:latest", + allowList: "docker.io,registry.example.com", + wantErr: false, + description: "docker.io should be allowed when in custom list", + }, + { + name: "malformed image reference rejected", + imageRef: "not a valid@image:reference", + allowList: "ghcr.io,docker.io", + wantErr: true, + errContains: "invalid image reference", + description: "malformed image references should be rejected during parsing", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + v := NewValidator(tt.allowList) + err := v.ValidateImageRegistry(tt.imageRef) + if (err != nil) != tt.wantErr { + t.Errorf("ValidateImageRegistry(%q) error = %v, wantErr %v (%s)", tt.imageRef, err, tt.wantErr, tt.description) + return + } + if tt.wantErr && tt.errContains != "" { + if err == nil || !containsString(err.Error(), tt.errContains) { + t.Errorf("ValidateImageRegistry(%q) error = %v, should contain %q (%s)", tt.imageRef, err, tt.errContains, tt.description) + } + } + }) + } +} + +// Helper function to check if a string contains a substring +func containsString(s, substr string) bool { + return len(s) >= len(substr) && (s == substr || len(s) > len(substr) && containsSubstring(s, substr)) +} + +func containsSubstring(s, substr string) bool { + for i := 0; i <= len(s)-len(substr); i++ { + if s[i:i+len(substr)] == substr { + return true + } + } + return false +} diff --git a/server/imageproxyserver.go b/server/imageproxyserver.go index ca8f62e4..92d10b88 100644 --- a/server/imageproxyserver.go +++ b/server/imageproxyserver.go @@ -11,32 +11,262 @@ import ( "net/http/httputil" "net/url" "strings" + "sync" + "time" + "github.com/distribution/reference" "github.com/go-logr/logr" + "github.com/ironcore-dev/boot-operator/internal/registry" "sigs.k8s.io/controller-runtime/pkg/client" ) const ( - ghcrIOKey = "ghcr.io/" - keppelKey = "keppel.global.cloud.sap/" imageKey = "imageName" layerDigestKey = "layerDigest" versionKey = "version" MediaTypeUKI = "application/vnd.ironcore.image.uki" ) +type AuthMethod int + +const ( + AuthNone AuthMethod = iota // Anonymous access + AuthBearer // Bearer token via /token endpoint +) + +type RegistryInfo struct { + Domain string + AuthMethod AuthMethod + TokenURL string // For bearer token auth +} + +// TokenResponse represents the JSON response from an OCI registry token endpoint. +// Supports both Docker registry format (token) and OAuth2 format (access_token). type TokenResponse struct { - Token string `json:"token"` + Token string `json:"token"` // Docker registry format + AccessToken string `json:"access_token"` // OAuth2 format (takes precedence) } type ImageDetails struct { OCIImageName string + RegistryDomain string RepositoryName string LayerDigest string Version string } -func RunImageProxyServer(imageProxyServerAddr string, k8sClient client.Client, log logr.Logger) { +// registryCacheEntry holds registry info with expiration timestamp +type registryCacheEntry struct { + info *RegistryInfo + expiresAt time.Time +} + +// Cache registry info to avoid repeated probes +var registryCache = make(map[string]*registryCacheEntry) +var registryCacheMutex sync.RWMutex + +const ( + // registryCacheTTL defines how long registry auth info is cached + // After this duration, auth detection will be re-run to catch policy changes + registryCacheTTL = 15 * time.Minute + + // maxErrorResponseSize limits error response body reads to prevent memory exhaustion + maxErrorResponseSize = 4 * 1024 // 4KB - enough for error details + + // maxTokenResponseSize limits token response body reads to prevent memory exhaustion + maxTokenResponseSize = 64 * 1024 // 64KB - token responses are typically a few hundred bytes +) + +// Shared HTTP client for all registry operations to enable connection reuse. +// No Timeout at client level - allows unlimited body streaming for large image layers. +var httpClient = &http.Client{ + Transport: func() *http.Transport { + // Clone http.DefaultTransport to inherit proxy settings from environment + transport := http.DefaultTransport.(*http.Transport).Clone() + // Override specific fields for registry operations + transport.MaxIdleConnsPerHost = 10 // Connection pool per host + transport.IdleConnTimeout = 90 * time.Second // Keep-alive duration + transport.TLSHandshakeTimeout = 10 * time.Second // Security timeout + transport.ExpectContinueTimeout = 1 * time.Second // Reduce latency + transport.ResponseHeaderTimeout = 30 * time.Second // Timeout for response headers only + return transport + }(), +} + +// Parse WWW-Authenticate parameter value +func extractParam(header, param string) string { + start := strings.Index(header, param+"=\"") + if start == -1 { + return "" + } + start += len(param) + 2 + end := strings.Index(header[start:], "\"") + if end == -1 { + return "" + } + return header[start : start+end] +} + +// Parse Bearer token URL from WWW-Authenticate header +func extractTokenURL(authHeader, repository string) string { + realm := extractParam(authHeader, "realm") + service := extractParam(authHeader, "service") + + if realm == "" { + return "" + } + + // Build token URL with repository scope + scope := fmt.Sprintf("repository:%s:pull", repository) + if service != "" { + return fmt.Sprintf("%s?service=%s&scope=%s", realm, service, scope) + } + return fmt.Sprintf("%s?scope=%s", realm, scope) +} + +// Probe registry to determine auth requirements +func detectRegistryAuth(registryDomain, repository string) (*RegistryInfo, error) { + // Try GET /v2/ - standard registry probe endpoint + targetURL := fmt.Sprintf("https://%s/v2/", registryDomain) + resp, err := httpClient.Get(targetURL) + if err != nil { + return nil, fmt.Errorf("registry unreachable: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + info := &RegistryInfo{Domain: registryDomain} + + switch resp.StatusCode { + case http.StatusOK: + // Anonymous access allowed + info.AuthMethod = AuthNone + return info, nil + + case http.StatusUnauthorized: + // Parse WWW-Authenticate header + authHeader := resp.Header.Get("WWW-Authenticate") + if authHeader == "" { + return nil, fmt.Errorf("401 without WWW-Authenticate header") + } + + // HTTP auth scheme matching is case-insensitive per RFC 7235 + if len(authHeader) > 7 && strings.EqualFold(authHeader[:7], "bearer ") { + info.AuthMethod = AuthBearer + info.TokenURL = extractTokenURL(authHeader, repository) + return info, nil + } + + return nil, fmt.Errorf("unsupported auth: %s", authHeader) + + default: + return nil, fmt.Errorf("unexpected status: %d", resp.StatusCode) + } +} + +// Get or detect registry info with caching and TTL-based expiration +func getOrDetectRegistry(registry, repository string) (*RegistryInfo, error) { + // Cache key includes repository for per-repository auth granularity + cacheKey := fmt.Sprintf("%s/%s", registry, repository) + + registryCacheMutex.RLock() + if entry, exists := registryCache[cacheKey]; exists { + // Check if entry has expired + if time.Now().Before(entry.expiresAt) { + registryCacheMutex.RUnlock() + return entry.info, nil + } + } + registryCacheMutex.RUnlock() + + // Detect and cache with TTL + info, err := detectRegistryAuth(registry, repository) + if err != nil { + return nil, err + } + + registryCacheMutex.Lock() + registryCache[cacheKey] = ®istryCacheEntry{ + info: info, + expiresAt: time.Now().Add(registryCacheTTL), + } + registryCacheMutex.Unlock() + + return info, nil +} + +// Get bearer token from token URL +func getBearerToken(tokenURL string) (string, error) { + resp, err := httpClient.Get(tokenURL) + if err != nil { + return "", err + } + defer func() { _ = resp.Body.Close() }() + + // Check HTTP status before attempting to parse JSON + if resp.StatusCode != http.StatusOK { + // Limit error response body read to prevent memory exhaustion + limitedReader := io.LimitReader(resp.Body, maxErrorResponseSize) + body, _ := io.ReadAll(limitedReader) + return "", fmt.Errorf("token request failed with status %d: %s", resp.StatusCode, string(body)) + } + + // Limit token response body read to prevent memory exhaustion + limitedReader := io.LimitReader(resp.Body, maxTokenResponseSize) + body, err := io.ReadAll(limitedReader) + if err != nil { + return "", err + } + + var tokenResponse TokenResponse + if err := json.Unmarshal(body, &tokenResponse); err != nil { + return "", fmt.Errorf("failed to parse token response: %w", err) + } + + // Prefer access_token (OAuth2 standard) over token (Docker registry format) + if tokenResponse.AccessToken != "" { + return tokenResponse.AccessToken, nil + } + if tokenResponse.Token != "" { + return tokenResponse.Token, nil + } + + return "", fmt.Errorf("token response missing both 'token' and 'access_token' fields") +} + +// cleanupExpiredCacheEntries periodically removes expired entries from the registry cache +// to prevent unbounded memory growth. Runs every 5 minutes. +func cleanupExpiredCacheEntries(log logr.Logger) { + ticker := time.NewTicker(5 * time.Minute) + defer ticker.Stop() + + for range ticker.C { + now := time.Now() + registryCacheMutex.Lock() + + expiredKeys := make([]string, 0) + for key, entry := range registryCache { + if now.After(entry.expiresAt) { + expiredKeys = append(expiredKeys, key) + } + } + + for _, key := range expiredKeys { + delete(registryCache, key) + } + + remainingCount := len(registryCache) + registryCacheMutex.Unlock() + + if len(expiredKeys) > 0 { + log.V(1).Info("Cleaned up expired cache entries", "count", len(expiredKeys), "remainingEntries", remainingCount) + } + } +} + +func RunImageProxyServer(imageProxyServerAddr string, k8sClient client.Client, validator *registry.Validator, log logr.Logger) { + // Start background cleanup of expired cache entries + go cleanupExpiredCacheEntries(log) + http.HandleFunc("/image", func(w http.ResponseWriter, r *http.Request) { imageDetails, err := parseImageURL(r.URL.Query()) if err != nil { @@ -45,14 +275,7 @@ func RunImageProxyServer(imageProxyServerAddr string, k8sClient client.Client, l return } - if strings.HasPrefix(imageDetails.OCIImageName, ghcrIOKey) { - handleGHCR(w, r, &imageDetails, log) - } else if strings.HasPrefix(imageDetails.OCIImageName, keppelKey) { - handleKeppel(w, r, &imageDetails, log) - } else { - http.Error(w, "Bad Request", http.StatusBadRequest) - log.Info("Unsupported registry") - } + handleDockerRegistry(w, r, &imageDetails, validator, log) }) http.HandleFunc("/httpboot/", func(w http.ResponseWriter, r *http.Request) { @@ -65,14 +288,7 @@ func RunImageProxyServer(imageProxyServerAddr string, k8sClient client.Client, l return } - if strings.HasPrefix(imageDetails.OCIImageName, ghcrIOKey) { - handleGHCR(w, r, &imageDetails, log) - } else if strings.HasPrefix(imageDetails.OCIImageName, keppelKey) { - handleKeppel(w, r, &imageDetails, log) - } else { - http.Error(w, "Bad Request", http.StatusBadRequest) - log.Info("Unsupported registry") - } + handleDockerRegistry(w, r, &imageDetails, validator, log) }) log.Info("Starting image proxy server", "address", imageProxyServerAddr) @@ -92,14 +308,16 @@ func parseHttpBootImagePath(path string) (ImageDetails, error) { imageName := strings.Join(segments[:len(segments)-1], "/") digestSegment := segments[len(segments)-1] - var repositoryName string - if strings.HasPrefix(imageName, ghcrIOKey) { - repositoryName = strings.TrimPrefix(imageName, ghcrIOKey) - } else if strings.HasPrefix(imageName, keppelKey) { - repositoryName = strings.TrimPrefix(imageName, keppelKey) - } else { - return ImageDetails{}, fmt.Errorf("unsupported registry key") + // Extract registry domain and repository using distribution/reference + registryDomain, err := registry.ExtractRegistryDomain(imageName) + if err != nil { + return ImageDetails{}, err } + named, err := reference.ParseNormalizedNamed(imageName) + if err != nil { + return ImageDetails{}, fmt.Errorf("invalid image reference: %w", err) + } + repositoryName := reference.Path(named) digestSegment = strings.TrimSuffix(digestSegment, ".efi") @@ -110,96 +328,112 @@ func parseHttpBootImagePath(path string) (ImageDetails, error) { return ImageDetails{ OCIImageName: imageName, - LayerDigest: layerDigest, + RegistryDomain: registryDomain, RepositoryName: repositoryName, + LayerDigest: layerDigest, }, nil } -func handleGHCR(w http.ResponseWriter, r *http.Request, imageDetails *ImageDetails, log logr.Logger) { - log.Info("Processing Image Proxy request", "method", r.Method, "path", r.URL.Path, "clientIP", r.RemoteAddr) +func handleDockerRegistry(w http.ResponseWriter, r *http.Request, imageDetails *ImageDetails, validator *registry.Validator, log logr.Logger) { + registryDomain := imageDetails.RegistryDomain + repository := imageDetails.RepositoryName - bearerToken, err := imageDetails.getBearerToken() - if err != nil { - http.Error(w, "Internal Server Error", http.StatusInternalServerError) - log.Info("Error: Failed to obtain the bearer token", "error", err) + log.V(1).Info("Processing registry request", "registry", registryDomain, "repository", repository, "digest", imageDetails.LayerDigest) + + if !validator.IsRegistryAllowed(registryDomain) { + http.Error(w, "Forbidden: Registry not allowed", http.StatusForbidden) + log.Info("Registry blocked", "registry", registryDomain, "allowList", validator.AllowedRegistries) return } - digest := imageDetails.LayerDigest - targetURL := fmt.Sprintf("https://ghcr.io/v2/%s/blobs/%s", imageDetails.RepositoryName, digest) - proxyURL, _ := url.Parse(targetURL) - - proxy := &httputil.ReverseProxy{ - Director: imageDetails.modifyDirector(proxyURL, bearerToken, digest), - ModifyResponse: modifyProxyResponse(bearerToken), + // Auto-detect auth method (with caching) + registryInfo, err := getOrDetectRegistry(registryDomain, repository) + if err != nil { + http.Error(w, "Registry detection failed", http.StatusBadGateway) + log.Error(err, "Failed to detect registry", "registry", registryDomain) + return } - r.URL.Host = proxyURL.Host - r.URL.Scheme = proxyURL.Scheme - r.Host = proxyURL.Host - - proxy.ServeHTTP(w, r) -} - -func handleKeppel(w http.ResponseWriter, r *http.Request, imageDetails *ImageDetails, log logr.Logger) { - log.Info("Processing Image Proxy request for Keppel", "method", r.Method, "path", r.URL.Path, "clientIP", r.RemoteAddr) + // Get auth token if needed + var authToken string + switch registryInfo.AuthMethod { + case AuthBearer: + authToken, err = getBearerToken(registryInfo.TokenURL) + if err != nil { + http.Error(w, "Authentication failed", http.StatusUnauthorized) + log.Error(err, "Failed to get bearer token", "tokenURL", registryInfo.TokenURL) + return + } + log.V(1).Info("Obtained bearer token", "registry", registryDomain) + case AuthNone: + log.V(1).Info("Registry allows anonymous access", "registry", registryDomain) + } + // Proxy the blob request digest := imageDetails.LayerDigest - targetURL := fmt.Sprintf("https://%sv2/%s/blobs/%s", keppelKey, imageDetails.RepositoryName, digest) - proxyURL, _ := url.Parse(targetURL) + proxyURL := &url.URL{ + Scheme: "https", + Host: registryDomain, + Path: fmt.Sprintf("/v2/%s/blobs/%s", repository, digest), + } proxy := &httputil.ReverseProxy{ - Director: imageDetails.modifyDirector(proxyURL, "", digest), + Director: buildDirector(proxyURL, authToken, repository, digest), + ModifyResponse: buildModifyResponse(), } r.URL.Host = proxyURL.Host r.URL.Scheme = proxyURL.Scheme r.Host = proxyURL.Host + log.Info("Proxying registry request", "targetURL", proxyURL.String(), "authMethod", registryInfo.AuthMethod) proxy.ServeHTTP(w, r) } -func (imageDetails ImageDetails) getBearerToken() (string, error) { - url := fmt.Sprintf("https://ghcr.io/token?scope=repository:%s:pull", imageDetails.RepositoryName) - resp, err := http.Get(url) - if err != nil { - return "", err - } - defer func() { - _ = resp.Body.Close() - }() - - body, err := io.ReadAll(resp.Body) - if err != nil { - return "", err - } - - var tokenResponse TokenResponse - if err := json.Unmarshal(body, &tokenResponse); err != nil { - return "", err +func buildDirector(proxyURL *url.URL, bearerToken string, repository string, digest string) func(*http.Request) { + return func(req *http.Request) { + req.URL.Scheme = proxyURL.Scheme + req.URL.Host = proxyURL.Host + req.URL.Path = fmt.Sprintf("/v2/%s/blobs/%s", repository, digest) + if bearerToken != "" { + req.Header.Set("Authorization", "Bearer "+bearerToken) + } } - - return tokenResponse.Token, nil } -func modifyProxyResponse(bearerToken string) func(*http.Response) error { +func buildModifyResponse() func(*http.Response) error { return func(resp *http.Response) error { - resp.Header.Set("Authorization", "Bearer "+bearerToken) - - if resp.StatusCode == http.StatusTemporaryRedirect { + // Handle redirects (307, 308, 301, 302, 303) + if resp.StatusCode == http.StatusTemporaryRedirect || + resp.StatusCode == http.StatusPermanentRedirect || + resp.StatusCode == http.StatusMovedPermanently || + resp.StatusCode == http.StatusFound || + resp.StatusCode == http.StatusSeeOther { location, err := resp.Location() if err != nil { return err } - client := &http.Client{} - redirectReq, err := http.NewRequest("GET", location.String(), nil) + // Propagate original request context to enable cancellation on client disconnect + redirectReq, err := http.NewRequestWithContext(resp.Request.Context(), "GET", location.String(), nil) if err != nil { return err } - copyHeaders(resp.Request.Header, redirectReq.Header) - redirectResp, err := client.Do(redirectReq) + // Security: Strip sensitive headers on cross-host redirects to prevent + // leaking credentials (e.g., bearer tokens) to third-party CDN/mirrors. + // Also strip on HTTPS→HTTP downgrades even if same host. + isSchemeDowngrade := resp.Request.URL.Scheme == "https" && location.Scheme == "http" + if isSameHost(resp.Request.URL, location) && !isSchemeDowngrade { + // Same-host redirect without scheme downgrade: preserve all headers including Authorization + copyHeaders(resp.Request.Header, redirectReq.Header) + } else { + // Cross-host redirect or HTTPS→HTTP downgrade: exclude sensitive auth headers + copyHeadersExcluding(resp.Request.Header, redirectReq.Header, + []string{"Authorization", "Cookie", "Proxy-Authorization"}) + } + + redirectResp, err := httpClient.Do(redirectReq) if err != nil { return err } @@ -233,14 +467,55 @@ func copyHeaders(source http.Header, destination http.Header) { } } -func replaceResponse(originalResp, redirectResp *http.Response) { - for name, values := range redirectResp.Header { - for _, value := range values { - originalResp.Header.Set(name, value) +// copyHeadersExcluding copies headers from source to destination, excluding specified headers. +// Header name comparison is case-insensitive per HTTP specification. +func copyHeadersExcluding(source http.Header, destination http.Header, excludeHeaders []string) { + // Normalize excluded headers to lowercase for case-insensitive comparison + excludeMap := make(map[string]bool, len(excludeHeaders)) + for _, header := range excludeHeaders { + excludeMap[strings.ToLower(header)] = true + } + + for name, values := range source { + if !excludeMap[strings.ToLower(name)] { + for _, value := range values { + destination.Add(name, value) + } } } - originalResp.Body = redirectResp.Body +} + +// isSameHost compares two URLs to determine if they point to the same host. +// Comparison includes both hostname and port to prevent credential leakage across ports. +func isSameHost(url1, url2 *url.URL) bool { + if url1 == nil || url2 == nil { + return false + } + // URL.Host includes port if specified, e.g., "registry.io:443" + return strings.EqualFold(url1.Host, url2.Host) +} + +func replaceResponse(originalResp, redirectResp *http.Response) { + // Close the original body to prevent connection leaks + if originalResp.Body != nil { + _ = originalResp.Body.Close() + } + + // Replace all response metadata from redirectResp + originalResp.Status = redirectResp.Status originalResp.StatusCode = redirectResp.StatusCode + originalResp.Proto = redirectResp.Proto + originalResp.ProtoMajor = redirectResp.ProtoMajor + originalResp.ProtoMinor = redirectResp.ProtoMinor + originalResp.Header = redirectResp.Header.Clone() + originalResp.Body = redirectResp.Body + originalResp.ContentLength = redirectResp.ContentLength + originalResp.TransferEncoding = redirectResp.TransferEncoding + originalResp.Close = redirectResp.Close + originalResp.Uncompressed = redirectResp.Uncompressed + originalResp.Trailer = redirectResp.Trailer + originalResp.TLS = redirectResp.TLS + // Preserve originalResp.Request - it must point to the original request } func parseImageURL(queries url.Values) (imageDetails ImageDetails, err error) { @@ -253,30 +528,23 @@ func parseImageURL(queries url.Values) (imageDetails ImageDetails, err error) { } ociImageName = strings.TrimSuffix(ociImageName, ".efi") - var repositoryName string - if strings.HasPrefix(ociImageName, ghcrIOKey) { - repositoryName = strings.TrimPrefix(ociImageName, ghcrIOKey) - } else if strings.HasPrefix(ociImageName, keppelKey) { - repositoryName = strings.TrimPrefix(ociImageName, keppelKey) - } else { - return ImageDetails{}, fmt.Errorf("unsupported registry key") + + // Extract registry domain and repository using distribution/reference + registryDomain, err := registry.ExtractRegistryDomain(ociImageName) + if err != nil { + return ImageDetails{}, err + } + named, err := reference.ParseNormalizedNamed(ociImageName) + if err != nil { + return ImageDetails{}, fmt.Errorf("invalid image reference: %w", err) } + repositoryName := reference.Path(named) return ImageDetails{ OCIImageName: ociImageName, + RegistryDomain: registryDomain, RepositoryName: repositoryName, LayerDigest: layerDigest, Version: version, }, nil } - -func (ImageDetails ImageDetails) modifyDirector(proxyURL *url.URL, bearerToken string, digest string) func(*http.Request) { - return func(req *http.Request) { - req.URL.Scheme = proxyURL.Scheme - req.URL.Host = proxyURL.Host - req.URL.Path = fmt.Sprintf("/v2/%s/blobs/%s", ImageDetails.RepositoryName, digest) - if bearerToken != "" { - req.Header.Set("Authorization", "Bearer "+bearerToken) - } - } -} diff --git a/test/registry/registry.go b/test/registry/registry.go new file mode 100644 index 00000000..68c4e7d4 --- /dev/null +++ b/test/registry/registry.go @@ -0,0 +1,249 @@ +// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and IronCore contributors +// SPDX-License-Identifier: Apache-2.0 + +package registry + +import ( + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "sync" + + "github.com/opencontainers/go-digest" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" +) + +// OCI media types for boot components +const ( + // MediaTypeKernel represents the Linux kernel image media type + MediaTypeKernel = "application/vnd.ironcore.image.kernel" + // MediaTypeInitrd represents the initial ramdisk media type + MediaTypeInitrd = "application/vnd.ironcore.image.initramfs" + // MediaTypeSquashfs represents the SquashFS filesystem media type + MediaTypeSquashfs = "application/vnd.ironcore.image.squashfs" + // MediaTypeUKI represents the unified kernel image media type + MediaTypeUKI = "application/vnd.ironcore.image.uki" + // MediaTypeKernelOld represents the legacy kernel media type + MediaTypeKernelOld = "application/io.gardenlinux.kernel" + // MediaTypeInitrdOld represents the legacy initrd media type + MediaTypeInitrdOld = "application/io.gardenlinux.initrd" +) + +// MockRegistry provides an in-memory OCI registry for testing +type MockRegistry struct { + mu sync.RWMutex + manifests map[string]ocispec.Manifest // Key: "name:tag" or "name@digest" + manifestsByDigest map[digest.Digest]ocispec.Manifest // For digest lookups + indexes map[string]ocispec.Index + blobs map[digest.Digest][]byte + server *httptest.Server +} + +// NewMockRegistry creates a new mock OCI registry server +func NewMockRegistry() *MockRegistry { + r := &MockRegistry{ + manifests: make(map[string]ocispec.Manifest), + manifestsByDigest: make(map[digest.Digest]ocispec.Manifest), + indexes: make(map[string]ocispec.Index), + blobs: make(map[digest.Digest][]byte), + } + + mux := http.NewServeMux() + + // OCI Distribution API endpoints + mux.HandleFunc("/v2/", func(w http.ResponseWriter, req *http.Request) { + if req.URL.Path == "/v2/" { + // Version check endpoint + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(map[string]string{"version": "2.0"}) + return + } + + // Manifest endpoint + if strings.Contains(req.URL.Path, "/manifests/") { + r.handleManifest(w, req) + return + } + + http.NotFound(w, req) + }) + + r.server = httptest.NewServer(mux) + return r +} + +// Close shuts down the mock registry server +func (r *MockRegistry) Close() { + r.server.Close() +} + +// URL returns the base URL of the mock registry +func (r *MockRegistry) URL() string { + return r.server.URL +} + +// RegistryAddress returns the registry address without http:// prefix +func (r *MockRegistry) RegistryAddress() string { + return strings.TrimPrefix(r.URL(), "http://") +} + +// pushPXEManifest is a helper to store PXE manifests with given media types +func (r *MockRegistry) pushPXEManifest(name, tag string, kernelMedia, initrdMedia string) { + kernelDigest := digest.FromString(fmt.Sprintf("kernel-%s-%s", name, tag)) + initrdDigest := digest.FromString(fmt.Sprintf("initrd-%s-%s", name, tag)) + squashfsDigest := digest.FromString(fmt.Sprintf("squashfs-%s-%s", name, tag)) + configDigest := digest.FromString(fmt.Sprintf("config-%s-%s", name, tag)) + + manifest := ocispec.Manifest{ + MediaType: ocispec.MediaTypeImageManifest, + Config: ocispec.Descriptor{ + MediaType: "application/vnd.oci.image.config.v1+json", + Digest: configDigest, + Size: 2, + }, + Layers: []ocispec.Descriptor{ + { + MediaType: kernelMedia, + Digest: kernelDigest, + Size: 1024, + }, + { + MediaType: initrdMedia, + Digest: initrdDigest, + Size: 2048, + }, + { + MediaType: MediaTypeSquashfs, + Digest: squashfsDigest, + Size: 4096, + }, + }, + } + + ref := fmt.Sprintf("%s:%s", name, tag) + r.manifests[ref] = manifest + + // Calculate and store manifest digest + manifestBytes, _ := json.Marshal(manifest) + manifestDigest := digest.FromBytes(manifestBytes) + r.manifestsByDigest[manifestDigest] = manifest + + r.blobs[manifest.Config.Digest] = []byte("{}") + r.blobs[kernelDigest] = []byte("kernel-data") + r.blobs[initrdDigest] = []byte("initrd-data") + r.blobs[squashfsDigest] = []byte("squashfs-data") +} + +// PushPXEImage adds a PXE boot image with kernel, initrd, and squashfs layers +func (r *MockRegistry) PushPXEImage(name, tag, architecture string) error { + r.mu.Lock() + defer r.mu.Unlock() + r.pushPXEManifest(name, tag, MediaTypeKernel, MediaTypeInitrd) + return nil +} + +// PushPXEImageOldFormat adds a PXE boot image using old Gardenlinux media types +func (r *MockRegistry) PushPXEImageOldFormat(name, tag, architecture string) error { + r.mu.Lock() + defer r.mu.Unlock() + r.pushPXEManifest(name, tag, MediaTypeKernelOld, MediaTypeInitrdOld) + return nil +} + +// PushHTTPImage adds an HTTP boot image with UKI layer +func (r *MockRegistry) PushHTTPImage(name, tag string) error { + r.mu.Lock() + defer r.mu.Unlock() + + ukiDigest := digest.FromString(fmt.Sprintf("uki-%s-%s", name, tag)) + configDigest := digest.FromString(fmt.Sprintf("config-%s-%s", name, tag)) + + manifest := ocispec.Manifest{ + MediaType: ocispec.MediaTypeImageManifest, + Config: ocispec.Descriptor{ + MediaType: "application/vnd.oci.image.config.v1+json", + Digest: configDigest, + Size: 2, + }, + Layers: []ocispec.Descriptor{ + { + MediaType: MediaTypeUKI, + Digest: ukiDigest, + Size: 8192, + }, + }, + } + + ref := fmt.Sprintf("%s:%s", name, tag) + r.manifests[ref] = manifest + + // Calculate and store manifest digest + manifestBytes, _ := json.Marshal(manifest) + manifestDigest := digest.FromBytes(manifestBytes) + r.manifestsByDigest[manifestDigest] = manifest + + r.blobs[manifest.Config.Digest] = []byte("{}") + r.blobs[ukiDigest] = []byte("uki-data") + + return nil +} + +func (r *MockRegistry) handleManifest(w http.ResponseWriter, req *http.Request) { + // Match pattern: /v2/{name}/manifests/{reference} + parts := strings.Split(strings.TrimPrefix(req.URL.Path, "/v2/"), "/manifests/") + if len(parts) != 2 { + http.NotFound(w, req) + return + } + + name := parts[0] + reference := parts[1] + + r.mu.RLock() + defer r.mu.RUnlock() + + var manifest ocispec.Manifest + var exists bool + var manifestDigest string + + // Check if reference is a digest (sha256:...) + if strings.HasPrefix(reference, "sha256:") { + // Look up by digest + dgst, err := digest.Parse(reference) + if err != nil { + http.Error(w, "invalid digest", http.StatusBadRequest) + return + } + manifest, exists = r.manifestsByDigest[dgst] + manifestDigest = reference + } else { + // Look up by tag + imageRef := fmt.Sprintf("%s:%s", name, reference) + manifest, exists = r.manifests[imageRef] + // Calculate digest for Content-Digest header + manifestBytes, _ := json.Marshal(manifest) + dgst := digest.FromBytes(manifestBytes) + manifestDigest = dgst.String() + } + + if !exists { + http.Error(w, "manifest not found", http.StatusNotFound) + return + } + + if req.Method == http.MethodHead { + w.Header().Set("Content-Type", ocispec.MediaTypeImageManifest) + w.Header().Set("Docker-Content-Digest", manifestDigest) + w.WriteHeader(http.StatusOK) + return + } + + manifestData, _ := json.Marshal(manifest) + w.Header().Set("Content-Type", ocispec.MediaTypeImageManifest) + w.Header().Set("Docker-Content-Digest", manifestDigest) + w.WriteHeader(http.StatusOK) + _, _ = w.Write(manifestData) +}