diff --git a/pkg/admission/validator/helper.go b/pkg/admission/validator/helper.go new file mode 100644 index 00000000..ac175a8e --- /dev/null +++ b/pkg/admission/validator/helper.go @@ -0,0 +1,35 @@ +// 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 + +import ( + "github.com/gardener/gardener/pkg/apis/core" + + "github.com/gardener/gardener-extension-registry-cache/pkg/constants" +) + +// FindRegistryCacheExtension finds the registry-cache extension. +// The first return argument is whether the extension was found. +// The second return argument is index of the extension in the list. -1 is returned if the extension is not found. +// The third return arguement is the extension itself. An empty extension is returned if the extension is not found. +func FindRegistryCacheExtension(extensions []core.Extension) (bool, int, core.Extension) { + for i, ext := range extensions { + if ext.Type == constants.ExtensionType { + return true, i, ext + } + } + + return false, -1, core.Extension{} +} diff --git a/pkg/admission/validator/helper_test.go b/pkg/admission/validator/helper_test.go new file mode 100644 index 00000000..4b13a7a1 --- /dev/null +++ b/pkg/admission/validator/helper_test.go @@ -0,0 +1,62 @@ +// 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 ( + "github.com/gardener/gardener/pkg/apis/core" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/runtime" + + "github.com/gardener/gardener-extension-registry-cache/pkg/admission/validator" +) + +var _ = Describe("Helpers", func() { + + DescribeTable("#FindRegistryCacheExtension", + func(extensions []core.Extension, expectedOk bool, expectedI int, expectedExt core.Extension) { + ok, i, ext := validator.FindRegistryCacheExtension(extensions) + Expect(ok).To(Equal(expectedOk)) + Expect(i).To(Equal(expectedI)) + Expect(ext).To(Equal(expectedExt)) + }, + + Entry("extensions is nil", + nil, + false, -1, core.Extension{}, + ), + Entry("extensions is empty", + []core.Extension{}, + false, -1, core.Extension{}, + ), + Entry("no registry-cache extension", + []core.Extension{ + {Type: "foo"}, + {Type: "bar"}, + {Type: "baz"}, + }, + false, -1, core.Extension{}, + ), + Entry("with registry-cache extension", + []core.Extension{ + {Type: "foo"}, + {Type: "bar"}, + {Type: "registry-cache", ProviderConfig: &runtime.RawExtension{Raw: []byte(`{"one": "two"}`)}}, + {Type: "baz"}, + }, + true, 2, core.Extension{Type: "registry-cache", ProviderConfig: &runtime.RawExtension{Raw: []byte(`{"one": "two"}`)}}, + ), + ) +}) diff --git a/pkg/admission/validator/shoot.go b/pkg/admission/validator/shoot.go index 0a8ab8b7..1f6d9fc7 100644 --- a/pkg/admission/validator/shoot.go +++ b/pkg/admission/validator/shoot.go @@ -26,7 +26,6 @@ import ( 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/constants" ) // shoot validates shoots @@ -42,22 +41,14 @@ func NewShootValidator(decoder runtime.Decoder) extensionswebhook.Validator { } // Validate validates the given shoot object -func (s *shoot) Validate(_ context.Context, new, _ client.Object) error { +func (s *shoot) Validate(_ context.Context, new, old client.Object) error { shoot, ok := new.(*core.Shoot) if !ok { return fmt.Errorf("wrong object type %T", new) } - var ext *core.Extension - var fldPath *field.Path - for i, ex := range shoot.Spec.Extensions { - if ex.Type == constants.ExtensionType { - ext = ex.DeepCopy() - fldPath = field.NewPath("spec", "extensions").Index(i) - break - } - } - if ext == nil { + ok, i, ext := FindRegistryCacheExtension(shoot.Spec.Extensions) + if !ok { return nil } @@ -67,7 +58,7 @@ func (s *shoot) Validate(_ context.Context, new, _ client.Object) error { } } - providerConfigPath := fldPath.Child("providerConfig") + providerConfigPath := field.NewPath("spec", "extensions").Index(i).Child("providerConfig") if ext.ProviderConfig == nil { return field.Required(providerConfigPath, "providerConfig is required for the registry-cache extension") } @@ -77,5 +68,30 @@ func (s *shoot) Validate(_ context.Context, new, _ client.Object) error { return fmt.Errorf("failed to decode providerConfig: %w", err) } - return validation.ValidateRegistryConfig(registryConfig, providerConfigPath).ToAggregate() + allErrs := field.ErrorList{} + + if old != nil { + oldShoot, ok := old.(*core.Shoot) + if !ok { + return fmt.Errorf("wrong object type %T for old object", old) + } + + oldOk, _, oldExt := FindRegistryCacheExtension(oldShoot.Spec.Extensions) + if oldOk { + if oldExt.ProviderConfig == nil { + return fmt.Errorf("providerConfig is not available on old Shoot") + } + + oldRegistryConfig := &api.RegistryConfig{} + if err := runtime.DecodeInto(s.decoder, oldExt.ProviderConfig.Raw, oldRegistryConfig); err != nil { + return fmt.Errorf("failed to decode providerConfig: %w", err) + } + + allErrs = append(allErrs, validation.ValidateRegistryConfigUpdate(oldRegistryConfig, registryConfig, providerConfigPath)...) + } + } + + allErrs = append(allErrs, validation.ValidateRegistryConfig(registryConfig, providerConfigPath)...) + + return allErrs.ToAggregate() } diff --git a/pkg/admission/validator/shoot_test.go b/pkg/admission/validator/shoot_test.go index a4351c2e..a2a41d7a 100644 --- a/pkg/admission/validator/shoot_test.go +++ b/pkg/admission/validator/shoot_test.go @@ -87,74 +87,130 @@ var _ = Describe("Shoot validator", func() { } }) - 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", + Context("Shoot creation (old is nil)", func() { + 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", }, - Caches: []v1alpha1.RegistryCache{ - { - Upstream: "https://registry.example.com", + } + 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 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 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 providerConfig is invalid", func() { + shoot.Spec.Extensions[0].ProviderConfig = &runtime.RawExtension{ + Raw: encode(&v1alpha1.RegistryConfig{ + TypeMeta: metav1.TypeMeta{ + APIVersion: v1alpha1.SchemeGroupVersion.String(), + Kind: "RegistryConfig", }, - }, - }), - } - - 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"), - })))) + 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()) + }) }) - It("should succeed for valid Shoot", func() { - Expect(shootValidator.Validate(ctx, shoot, nil)).To(Succeed()) + Context("Shoot update (old is set)", func() { + var oldShoot *core.Shoot + + BeforeEach(func() { + oldShoot = shoot.DeepCopy() + }) + + It("should return err when old is not a Shoot", func() { + err := shootValidator.Validate(ctx, shoot, &corev1.Pod{}) + Expect(err).To(MatchError("wrong object type *v1.Pod for old object")) + }) + + It("should return err when old Shoot registry-cache providerConfig is nil", func() { + oldShoot.Spec.Extensions[0].ProviderConfig = nil + + err := shootValidator.Validate(ctx, shoot, oldShoot) + Expect(err).To(MatchError(ContainSubstring("providerConfig is not available on old Shoot"))) + }) + + It("should return err when old Shoot registry-cache providerConfig cannot be decoded", func() { + oldShoot.Spec.Extensions[0].ProviderConfig = &runtime.RawExtension{ + Raw: []byte(`{"bar": "baz"}`), + } + + err := shootValidator.Validate(ctx, shoot, oldShoot) + Expect(err).To(MatchError(ContainSubstring("failed to decode providerConfig"))) + }) + + It("should return err when registry-cache providerConfig update is invalid", func() { + newSize := resource.MustParse("42Gi") + shoot.Spec.Extensions[0].ProviderConfig = &runtime.RawExtension{ + Raw: encode(&v1alpha1.RegistryConfig{ + TypeMeta: metav1.TypeMeta{ + APIVersion: v1alpha1.SchemeGroupVersion.String(), + Kind: "RegistryConfig", + }, + Caches: []v1alpha1.RegistryCache{ + { + Upstream: "docker.io", + Size: &newSize, + }, + }, + }), + } + + err := shootValidator.Validate(ctx, shoot, oldShoot) + Expect(err).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(field.ErrorTypeInvalid), + "Field": Equal("spec.extensions[0].providerConfig.caches[0].size"), + "Detail": Equal("field is immutable"), + })))) + }) }) }) }) diff --git a/pkg/apis/registry/helper/helper.go b/pkg/apis/registry/helper/helper.go new file mode 100644 index 00000000..ced87e4c --- /dev/null +++ b/pkg/apis/registry/helper/helper.go @@ -0,0 +1,32 @@ +// 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 helper + +import ( + "github.com/gardener/gardener-extension-registry-cache/pkg/apis/registry" +) + +// FindCacheByUpstream finds a cache by upstream. +// The first return argument is whether the extension was found. +// The third return arguement is the cache itself. An empty cache is returned if the cache is not found. +func FindCacheByUpstream(caches []registry.RegistryCache, upstream string) (bool, registry.RegistryCache) { + for _, cache := range caches { + if cache.Upstream == upstream { + return true, cache + } + } + + return false, registry.RegistryCache{} +} diff --git a/pkg/apis/registry/helper/helper_test.go b/pkg/apis/registry/helper/helper_test.go new file mode 100644 index 00000000..eb4457b3 --- /dev/null +++ b/pkg/apis/registry/helper/helper_test.go @@ -0,0 +1,56 @@ +// 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 helper_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/api/resource" + + "github.com/gardener/gardener-extension-registry-cache/pkg/apis/registry" + "github.com/gardener/gardener-extension-registry-cache/pkg/apis/registry/helper" +) + +var _ = Describe("Helpers", func() { + size := resource.MustParse("5Gi") + + DescribeTable("#FindRegistryCacheExtension", + func(caches []registry.RegistryCache, upstream string, expectedOk bool, expectedCache registry.RegistryCache) { + ok, cache := helper.FindCacheByUpstream(caches, upstream) + Expect(ok).To(Equal(expectedOk)) + Expect(cache).To(Equal(expectedCache)) + }, + Entry("caches is nil", + nil, + "docker.io", + false, registry.RegistryCache{}, + ), + Entry("caches is empty", + []registry.RegistryCache{}, + "docker.io", + false, registry.RegistryCache{}, + ), + Entry("no cache with the given upsteam", + []registry.RegistryCache{{Upstream: "gcr.io"}, {Upstream: "quay.io"}, {Upstream: "registry.k8s.io"}}, + "docker.io", + false, registry.RegistryCache{}, + ), + Entry("with cache with the given upsteam", + []registry.RegistryCache{{Upstream: "gcr.io"}, {Upstream: "quay.io"}, {Upstream: "docker.io", Size: &size}, {Upstream: "registry.k8s.io"}}, + "docker.io", + false, registry.RegistryCache{}, + ), + ) +}) diff --git a/pkg/apis/registry/validation/validation.go b/pkg/apis/registry/validation/validation.go index c9e8732a..54bea64f 100644 --- a/pkg/apis/registry/validation/validation.go +++ b/pkg/apis/registry/validation/validation.go @@ -17,10 +17,12 @@ package validation import ( "strings" + apiequality "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/util/validation/field" "github.com/gardener/gardener-extension-registry-cache/pkg/apis/registry" + "github.com/gardener/gardener-extension-registry-cache/pkg/apis/registry/helper" ) // ValidateRegistryConfig validates the passed configuration instance. @@ -38,6 +40,21 @@ func ValidateRegistryConfig(config *registry.RegistryConfig, fldPath *field.Path return allErrs } +// ValidateRegistryConfigUpdate validates the passed configuration update. +func ValidateRegistryConfigUpdate(oldConfig, newConfig *registry.RegistryConfig, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + for i, newCache := range newConfig.Caches { + if ok, oldCache := helper.FindCacheByUpstream(oldConfig.Caches, newCache.Upstream); ok { + if !apiequality.Semantic.DeepEqual(oldCache.Size, newCache.Size) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("caches").Index(i).Child("size"), newCache.Size.String(), "field is immutable")) + } + } + } + + return allErrs +} + func validateRegistryCache(cache registry.RegistryCache, fldPath *field.Path) field.ErrorList { var allErrs field.ErrorList diff --git a/pkg/apis/registry/validation/validation_test.go b/pkg/apis/registry/validation/validation_test.go index b4db8323..8d7f9b16 100644 --- a/pkg/apis/registry/validation/validation_test.go +++ b/pkg/apis/registry/validation/validation_test.go @@ -121,4 +121,37 @@ var _ = Describe("Validation", func() { )) }) }) + + Describe("#ValidateRegistryConfigUpdate", func() { + var oldRegistryConfig *api.RegistryConfig + + BeforeEach(func() { + oldRegistryConfig = registryConfig.DeepCopy() + }) + + It("should allow valid configuration update", func() { + size := resource.MustParse("5Gi") + newCache := api.RegistryCache{ + Upstream: "docker.io", + Size: &size, + GarbageCollectionEnabled: pointer.Bool(true), + } + registryConfig.Caches = append(registryConfig.Caches, newCache) + + Expect(ValidateRegistryConfigUpdate(oldRegistryConfig, registryConfig, fldPath)).To(BeEmpty()) + }) + + It("should deny cache size update", func() { + newSize := resource.MustParse("16Gi") + registryConfig.Caches[0].Size = &newSize + + Expect(ValidateRegistryConfigUpdate(oldRegistryConfig, registryConfig, fldPath)).To(ConsistOf( + PointTo(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(field.ErrorTypeInvalid), + "Field": Equal("providerConfig.caches[0].size"), + "Detail": Equal("field is immutable"), + })), + )) + }) + }) })