Extract the registry deployment into component. Add missing unit tests (

gardener#36)

* Require at least one cache to be specified

* Add unit tests for the shoot validator

* Fix `addDefaultingFuncs` for the config API

* Remove test suite for package that does not define any tests

* Extract the registry deployment into component. Add unit tests for the new component

A new component is introduced in `./pkg/component/registrycaches`. The component deploys registry caches. It implements the well-known `component.DeployWaiter` interface.

This commit also moves the existing controller from `./pkg/controller` to dedicated pkg `./pkg/controller/extension`. The controller name is changed from `registry-cache` to `extension-controller`.
This commit also introduces a new pkg `./pkg/constants`. It exports constants that are used from 2 or more packages.

* Address review comments

cmd/gardener-extension-registry-cache-admission/app/app.go

@@ -30,7 +30,7 @@ import (
 
 	admissioncmd "github.com/gardener/gardener-extension-registry-cache/pkg/admission/cmd"
 	registryinstall "github.com/gardener/gardener-extension-registry-cache/pkg/apis/registry/install"
-	"github.com/gardener/gardener-extension-registry-cache/pkg/controller"
+	"github.com/gardener/gardener-extension-registry-cache/pkg/constants"
 )
 
 var log = logf.Log.WithName("gardener-extension-registry-cache-admission")
@@ -56,7 +56,7 @@ func NewAdmissionCommand(ctx context.Context) *cobra.Command {
 	)
 
 	cmd := &cobra.Command{
-		Use: fmt.Sprintf("gardener-extension-%s-admission", controller.Type),
+		Use: fmt.Sprintf("gardener-extension-%s-admission", constants.ExtensionType),
 
 		RunE: func(cmd *cobra.Command, args []string) error {
 			verflag.PrintAndExitIfRequested()

cmd/gardener-extension-registry-cache/app/app.go

@@ -19,7 +19,7 @@ import (
 	"fmt"
 
 	extensionscontroller "github.com/gardener/gardener/extensions/pkg/controller"
-	"github.com/gardener/gardener/extensions/pkg/controller/heartbeat"
+	heartbeatcontroller "github.com/gardener/gardener/extensions/pkg/controller/heartbeat"
 	"github.com/gardener/gardener/extensions/pkg/util"
 	gardenerhealthz "github.com/gardener/gardener/pkg/healthz"
 	"github.com/spf13/cobra"
@@ -30,7 +30,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 
 	registryinstall "github.com/gardener/gardener-extension-registry-cache/pkg/apis/registry/install"
-	"github.com/gardener/gardener-extension-registry-cache/pkg/controller"
+	extensioncontroller "github.com/gardener/gardener-extension-registry-cache/pkg/controller/extension"
 )
 
 // NewServiceControllerCommand creates a new command that is used to start the registry service controller.
@@ -88,10 +88,10 @@ func (o *Options) run(ctx context.Context) error {
 	}
 
 	ctrlConfig := o.registryOptions.Completed()
-	ctrlConfig.Apply(&controller.DefaultAddOptions.Config)
-	o.controllerOptions.Completed().Apply(&controller.DefaultAddOptions.ControllerOptions)
-	o.reconcileOptions.Completed().Apply(&controller.DefaultAddOptions.IgnoreOperationAnnotation)
-	o.heartbeatOptions.Completed().Apply(&heartbeat.DefaultAddOptions)
+	ctrlConfig.Apply(&extensioncontroller.DefaultAddOptions.Config)
+	o.controllerOptions.Completed().Apply(&extensioncontroller.DefaultAddOptions.ControllerOptions)
+	o.reconcileOptions.Completed().Apply(&extensioncontroller.DefaultAddOptions.IgnoreOperationAnnotation)
+	o.heartbeatOptions.Completed().Apply(&heartbeatcontroller.DefaultAddOptions)
 
 	if err := o.controllerSwitches.Completed().AddToManager(ctx, mgr); err != nil {
 		return fmt.Errorf("could not add controllers to manager: %w", err)

pkg/admission/validator/shoot.go

@@ -24,8 +24,9 @@ import (
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
+	api "github.com/gardener/gardener-extension-registry-cache/pkg/apis/registry"
 	"github.com/gardener/gardener-extension-registry-cache/pkg/apis/registry/validation"
-	"github.com/gardener/gardener-extension-registry-cache/pkg/controller"
+	"github.com/gardener/gardener-extension-registry-cache/pkg/constants"
 )
 
 // shoot validates shoots
@@ -50,7 +51,7 @@ func (s *shoot) Validate(_ context.Context, new, _ client.Object) error {
 	var ext *core.Extension
 	var fldPath *field.Path
 	for i, ex := range shoot.Spec.Extensions {
-		if ex.Type == controller.Type {
+		if ex.Type == constants.ExtensionType {
 			ext = ex.DeepCopy()
 			fldPath = field.NewPath("spec", "extensions").Index(i)
 			break
@@ -62,17 +63,19 @@ func (s *shoot) Validate(_ context.Context, new, _ client.Object) error {
 
 	for _, worker := range shoot.Spec.Provider.Workers {
 		if worker.CRI.Name != "containerd" {
-			return fmt.Errorf("containerruntime needs to be containerd when container registry cache is used")
+			return fmt.Errorf("container runtime needs to be containerd when the registry-cache extension is enabled")
 		}
 	}
 
 	providerConfigPath := fldPath.Child("providerConfig")
+	if ext.ProviderConfig == nil {
+		return field.Required(providerConfigPath, "providerConfig is required for the registry-cache extension")
+	}
 
-	registryConfig, err := decodeRegistryConfig(s.decoder, ext.ProviderConfig, providerConfigPath)
-	if err != nil {
-		return err
+	registryConfig := &api.RegistryConfig{}
+	if err := runtime.DecodeInto(s.decoder, ext.ProviderConfig.Raw, registryConfig); err != nil {
+		return fmt.Errorf("failed to decode providerConfig: %w", err)
 	}
 
 	return validation.ValidateRegistryConfig(registryConfig, providerConfigPath).ToAggregate()
-
 }

pkg/admission/validator/shoot_test.go

@@ -0,0 +1,165 @@
+// Copyright (c) 2023 SAP SE or an SAP affiliate company. All rights reserved. This file is licensed under the Apache Software License, v. 2 except as noted otherwise in the LICENSE file
+//
+// 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 validator_test
+
+import (
+	"context"
+	"encoding/json"
+
+	extensionswebhook "github.com/gardener/gardener/extensions/pkg/webhook"
+	"github.com/gardener/gardener/pkg/apis/core"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	. "github.com/onsi/gomega/gstruct"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/serializer"
+	"k8s.io/apimachinery/pkg/util/validation/field"
+
+	"github.com/gardener/gardener-extension-registry-cache/pkg/admission/validator"
+	api "github.com/gardener/gardener-extension-registry-cache/pkg/apis/registry"
+	"github.com/gardener/gardener-extension-registry-cache/pkg/apis/registry/v1alpha1"
+)
+
+var _ = Describe("Shoot validator", func() {
+
+	Describe("#Validate", func() {
+		var (
+			ctx  = context.Background()
+			size = resource.MustParse("20Gi")
+
+			shootValidator extensionswebhook.Validator
+
+			shoot *core.Shoot
+		)
+
+		BeforeEach(func() {
+			scheme := runtime.NewScheme()
+			Expect(api.AddToScheme(scheme)).To(Succeed())
+			Expect(v1alpha1.AddToScheme(scheme)).To(Succeed())
+
+			decoder := serializer.NewCodecFactory(scheme, serializer.EnableStrict).UniversalDecoder()
+			shootValidator = validator.NewShootValidator(decoder)
+
+			shoot = &core.Shoot{
+				Spec: core.ShootSpec{
+					Extensions: []core.Extension{
+						{
+							Type: "registry-cache",
+							ProviderConfig: &runtime.RawExtension{
+								Raw: encode(&v1alpha1.RegistryConfig{
+									TypeMeta: metav1.TypeMeta{
+										APIVersion: v1alpha1.SchemeGroupVersion.String(),
+										Kind:       "RegistryConfig",
+									},
+									Caches: []v1alpha1.RegistryCache{
+										{
+											Upstream: "docker.io",
+											Size:     &size,
+										},
+									},
+								}),
+							},
+						},
+					},
+					Provider: core.Provider{
+						Workers: []core.Worker{
+							{
+								CRI: &core.CRI{Name: "containerd"},
+							},
+						},
+					},
+				},
+			}
+		})
+
+		It("should return err when new is not a Shoot", func() {
+			err := shootValidator.Validate(ctx, &corev1.Pod{}, nil)
+			Expect(err).To(MatchError("wrong object type *v1.Pod"))
+		})
+
+		It("should do nothing when the Shoot does no specify a registry-cache extension", func() {
+			shoot.Spec.Extensions[0].Type = "foo"
+
+			Expect(shootValidator.Validate(ctx, shoot, nil)).To(Succeed())
+		})
+
+		It("should return err when there is contrainer runtime that is not containerd", func() {
+			worker := core.Worker{
+				CRI: &core.CRI{
+					Name: "docker",
+				},
+			}
+			shoot.Spec.Provider.Workers = append(shoot.Spec.Provider.Workers, worker)
+
+			err := shootValidator.Validate(ctx, shoot, nil)
+			Expect(err).To(MatchError("container runtime needs to be containerd when the registry-cache extension is enabled"))
+		})
+
+		It("should return err when registry-cache's providerConfig is nil", func() {
+			shoot.Spec.Extensions[0].ProviderConfig = nil
+
+			err := shootValidator.Validate(ctx, shoot, nil)
+			Expect(err).To(PointTo(MatchFields(IgnoreExtras, Fields{
+				"Type":   Equal(field.ErrorTypeRequired),
+				"Field":  Equal("spec.extensions[0].providerConfig"),
+				"Detail": Equal("providerConfig is required for the registry-cache extension"),
+			})))
+		})
+
+		It("should return err when registry-cache's providerConfig cannot be decoded", func() {
+			shoot.Spec.Extensions[0].ProviderConfig = &runtime.RawExtension{
+				Raw: []byte(`{"bar": "baz"}`),
+			}
+
+			err := shootValidator.Validate(ctx, shoot, nil)
+			Expect(err).To(MatchError(ContainSubstring("failed to decode providerConfig")))
+		})
+
+		It("should return err when registry-cache's providerConfig is invalid", func() {
+			shoot.Spec.Extensions[0].ProviderConfig = &runtime.RawExtension{
+				Raw: encode(&v1alpha1.RegistryConfig{
+					TypeMeta: metav1.TypeMeta{
+						APIVersion: v1alpha1.SchemeGroupVersion.String(),
+						Kind:       "RegistryConfig",
+					},
+					Caches: []v1alpha1.RegistryCache{
+						{
+							Upstream: "https://registry.example.com",
+						},
+					},
+				}),
+			}
+
+			err := shootValidator.Validate(ctx, shoot, nil)
+			Expect(err).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{
+				"Type":   Equal(field.ErrorTypeInvalid),
+				"Field":  Equal("spec.extensions[0].providerConfig.caches[0].upstream"),
+				"Detail": ContainSubstring("upstream must not include a scheme"),
+			}))))
+		})
+
+		It("should succeed for valid Shoot", func() {
+			Expect(shootValidator.Validate(ctx, shoot, nil)).To(Succeed())
+		})
+	})
+})
+
+func encode(obj runtime.Object) []byte {
+	data, _ := json.Marshal(obj)
+	return data
+}

pkg/admission/validator/webhook.go

@@ -21,7 +21,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 
-	"github.com/gardener/gardener-extension-registry-cache/pkg/controller"
+	"github.com/gardener/gardener-extension-registry-cache/pkg/constants"
 )
 
 const (
@@ -38,7 +38,7 @@ func New(mgr manager.Manager) (*extensionswebhook.Webhook, error) {
 	logger.Info("Setting up webhook", "name", Name)
 
 	return extensionswebhook.New(mgr, extensionswebhook.Args{
-		Provider: controller.Type,
+		Provider: constants.ExtensionType,
 		Name:     Name,
 		Path:     "/webhooks/validate",
 		Validators: map[extensionswebhook.Validator][]extensionswebhook.Type{

pkg/apis/config/v1alpha1/defaults.go

@@ -18,7 +18,6 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 )
 
-func addDefaultingFuncs(_ *runtime.Scheme) error {
-	return nil
-	// return RegisterDefaults(scheme)
+func addDefaultingFuncs(scheme *runtime.Scheme) error {
+	return RegisterDefaults(scheme)
 }

pkg/apis/registry/v1alpha1/types.go

@@ -19,9 +19,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-// RegistryResourceName is the name for registry resources in the shoot.
-const RegistryResourceName = "extension-registry-cache"
-
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 
 // RegistryConfig contains information about registry caches to deploy.

pkg/apis/registry/validation/validation.go

@@ -17,6 +17,7 @@ package validation
 import (
 	"strings"
 
+	"k8s.io/apimachinery/pkg/api/resource"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 
 	"github.com/gardener/gardener-extension-registry-cache/pkg/apis/registry"
@@ -26,6 +27,10 @@ import (
 func ValidateRegistryConfig(config *registry.RegistryConfig, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 
+	if len(config.Caches) == 0 {
+		allErrs = append(allErrs, field.Required(fldPath.Child("caches"), "at least one cache must be provided"))
+	}
+
 	for i, cache := range config.Caches {
 		allErrs = append(allErrs, validateRegistryCache(cache, fldPath.Child("caches").Index(i))...)
 	}
@@ -37,8 +42,8 @@ func validateRegistryCache(cache registry.RegistryCache, fldPath *field.Path) fi
 	var allErrs field.ErrorList
 
 	allErrs = append(allErrs, validateUpstream(fldPath.Child("upstream"), cache.Upstream)...)
-	if size := cache.Size; size != nil && size.Sign() != 1 {
-		allErrs = append(allErrs, field.Invalid(fldPath.Child("size"), size, "size must be a quantity greater than zero"))
+	if cache.Size != nil {
+		allErrs = append(allErrs, validatePositiveQuantity(*cache.Size, fldPath.Child("size"))...)
 	}
 
 	return allErrs
@@ -58,3 +63,12 @@ func validateUpstream(fldPath *field.Path, upstream string) field.ErrorList {
 
 	return allErrors
 }
+
+// validatePositiveQuantity validates that a Quantity is positive.
+func validatePositiveQuantity(value resource.Quantity, fldPath *field.Path) field.ErrorList {
+	allErrs := field.ErrorList{}
+	if value.Cmp(resource.Quantity{}) <= 0 {
+		allErrs = append(allErrs, field.Invalid(fldPath, value.String(), "must be greater than 0"))
+	}
+	return allErrs
+}