Make .spec.networking.nodes in Shoots mutable (gardener#7368)

* Make Shoot.Spec.Networking.Nodes mutable. * Add immutability check to managedseed shoot admission. * Add featuregate. * Update docs/deployment/feature_gates.md Co-authored-by: Rafael Franzke <rafael.franzke@sap.com> * Add additional owners to feature gate. * Enable feature gate for local deployment. --------- Co-authored-by: Rafael Franzke <rafael.franzke@sap.com>
nickytd · Feb 6, 2023 · 8c73d03 · 8c73d03
1 parent bf73f29
commit 8c73d03
Show file tree

Hide file tree

Showing 18 changed files with 116 additions and 29 deletions.
diff --git a/docs/api-reference/core.md b/docs/api-reference/core.md
@@ -6783,7 +6783,8 @@ string
 </td>
 <td>
 <em>(Optional)</em>
-<p>Nodes is the CIDR of the entire node network. This field is immutable.</p>
+<p>Nodes is the CIDR of the entire node network.
+This field is immutable if the feature gate MutableShootSpecNetworkingNodes is disabled.</p>
 </td>
 </tr>
 <tr>

diff --git a/docs/deployment/feature_gates.md b/docs/deployment/feature_gates.md
@@ -37,6 +37,7 @@ The following tables are a summary of the feature gates that you can set on diff
 | DefaultSeccompProfile                      | `false` | `Alpha` | `1.54` |        |
 | CoreDNSQueryRewriting                      | `false` | `Alpha` | `1.55` |        |
 | IPv6SingleStack                            | `false` | `Alpha` | `1.63` |        |
+| MutableShootSpecNetworkingNodes            | `false` | `Alpha` | `1.64` |        |
 
 ## Feature Gates for Graduated or Deprecated Features
 
@@ -162,3 +163,4 @@ A *General Availability* (GA) feature is also referred to as a *stable* feature.
 | DefaultSeccompProfile                      | `gardenlet`, `gardener-operator` | Enables the defaulting of the seccomp profile for Gardener managed workload in the garden or seed to `RuntimeDefault`.                                                                                                                                                                                                                                                                                                                                                                                           |
 | CoreDNSQueryRewriting                      | `gardenlet`                      | Enables automatic DNS query rewriting in shoot cluster's CoreDNS to shortcut name resolution of fully qualified in-cluster and out-of-cluster names, which follow a user-defined pattern. Details can be found in [DNS Search Path Optimization](../usage/dns-search-path-optimization.md).                                                                                                                                                                                                                      |
 | IPv6SingleStack                            | `gardener-apiserver`             | Allows creating shoot clusters with [IPv6 single-stack networking](../usage/ipv6.md) ([GEP-21](../proposals/21-ipv6-singlestack-local.md)).                                                                                                                                                                                                                                                                                                                                                                      |
+| MutableShootSpecNetworkingNodes            | `gardener-apiserver`             | Allows updating the field `spec.networking.nodes`. The validity of the values has to be checked in the provider extensions. Only enable this feature gate when your system runs provider extensions which have implemented the validation.                                                                                                                                                                                                                                                                                                                                                  |
diff --git a/example/gardener-local/controlplane/values.yaml b/example/gardener-local/controlplane/values.yaml
@@ -161,6 +161,7 @@ global:
       SeedChange: true
       HAControlPlanes: true
       IPv6SingleStack: true
+      MutableShootSpecNetworkingNodes: true
     resources: {}
 
   # Gardener admission controller configuration values

diff --git a/hack/local-development/start-apiserver b/hack/local-development/start-apiserver
@@ -57,7 +57,7 @@ apiserver_flags="
   --secure-port=8443 \
   --tls-cert-file $TLS_CERT_FILE \
   --tls-private-key-file $TLS_KEY_FILE \
-  --feature-gates HAControlPlanes=true,SeedChange=true,IPv6SingleStack=true \
+  --feature-gates HAControlPlanes=true,SeedChange=true,IPv6SingleStack=true,MutableShootSpecNetworkingNodes=true \
   --shoot-admin-kubeconfig-max-expiration=24h \
   --enable-admission-plugins=ShootVPAEnabledByDefault \
   --v 2"

diff --git a/pkg/apis/core/types_shoot.go b/pkg/apis/core/types_shoot.go
@@ -932,7 +932,8 @@ type Networking struct {
 	ProviderConfig *runtime.RawExtension
 	// Pods is the CIDR of the pod network. This field is immutable.
 	Pods *string
-	// Nodes is the CIDR of the entire node network. This field is immutable.
+	// Nodes is the CIDR of the entire node network.
+	// This field is immutable if the feature gate MutableShootSpecNetworkingNodes is disabled.
 	Nodes *string
 	// Services is the CIDR of the service network. This field is immutable.
 	Services *string

diff --git a/pkg/apis/core/v1alpha1/generated.proto b/pkg/apis/core/v1alpha1/generated.proto
diff --git a/pkg/apis/core/v1alpha1/types_shoot.go b/pkg/apis/core/v1alpha1/types_shoot.go
@@ -1186,7 +1186,8 @@ type Networking struct {
 	// Pods is the CIDR of the pod network. This field is immutable.
 	// +optional
 	Pods *string `json:"pods,omitempty" protobuf:"bytes,3,opt,name=pods"`
-	// Nodes is the CIDR of the entire node network. This field is immutable.
+	// Nodes is the CIDR of the entire node network.
+	// This field is immutable if the feature gate MutableShootSpecNetworkingNodes is disabled.
 	// +optional
 	Nodes *string `json:"nodes,omitempty" protobuf:"bytes,4,opt,name=nodes"`
 	// Services is the CIDR of the service network. This field is immutable.

diff --git a/pkg/apis/core/v1beta1/generated.proto b/pkg/apis/core/v1beta1/generated.proto
diff --git a/pkg/apis/core/v1beta1/types_shoot.go b/pkg/apis/core/v1beta1/types_shoot.go
@@ -1198,7 +1198,8 @@ type Networking struct {
 	// Pods is the CIDR of the pod network. This field is immutable.
 	// +optional
 	Pods *string `json:"pods,omitempty" protobuf:"bytes,3,opt,name=pods"`
-	// Nodes is the CIDR of the entire node network. This field is immutable.
+	// Nodes is the CIDR of the entire node network.
+	// This field is immutable if the feature gate MutableShootSpecNetworkingNodes is disabled.
 	// +optional
 	Nodes *string `json:"nodes,omitempty" protobuf:"bytes,4,opt,name=nodes"`
 	// Services is the CIDR of the service network. This field is immutable.

diff --git a/pkg/apis/core/validation/shoot.go b/pkg/apis/core/validation/shoot.go
@@ -36,12 +36,14 @@ import (
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/apimachinery/pkg/util/validation/field"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/utils/pointer"
 	"k8s.io/utils/strings/slices"
 
 	"github.com/gardener/gardener/pkg/apis/core"
 	"github.com/gardener/gardener/pkg/apis/core/helper"
 	v1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants"
+	"github.com/gardener/gardener/pkg/features"
 	"github.com/gardener/gardener/pkg/utils"
 	gardenerutils "github.com/gardener/gardener/pkg/utils/gardener"
 	"github.com/gardener/gardener/pkg/utils/timewindow"
@@ -169,6 +171,10 @@ func ValidateShootTemplateUpdate(newShootTemplate, oldShootTemplate *core.ShootT
 
 	allErrs = append(allErrs, ValidateShootSpecUpdate(&newShootTemplate.Spec, &oldShootTemplate.Spec, newShootTemplate.ObjectMeta, fldPath.Child("spec"))...)
 
+	if oldShootTemplate.Spec.Networking.Nodes != nil {
+		allErrs = append(allErrs, apivalidation.ValidateImmutableField(newShootTemplate.Spec.Networking.Nodes, oldShootTemplate.Spec.Networking.Nodes, fldPath.Child("spec", "networking", "nodes"))...)
+	}
+
 	return allErrs
 }
 
@@ -598,7 +604,7 @@ func validateNetworkingUpdate(newNetworking, oldNetworking core.Networking, fldP
 	if oldNetworking.Services != nil {
 		allErrs = append(allErrs, apivalidation.ValidateImmutableField(newNetworking.Services, oldNetworking.Services, fldPath.Child("services"))...)
 	}
-	if oldNetworking.Nodes != nil {
+	if !utilfeature.DefaultFeatureGate.Enabled(features.MutableShootSpecNetworkingNodes) && oldNetworking.Nodes != nil {
 		allErrs = append(allErrs, apivalidation.ValidateImmutableField(newNetworking.Nodes, oldNetworking.Nodes, fldPath.Child("nodes"))...)
 	}
 

diff --git a/pkg/apis/core/validation/shoot_test.go b/pkg/apis/core/validation/shoot_test.go
@@ -2671,6 +2671,30 @@ var _ = Describe("Shoot Validation Tests", func() {
 				}))))
 			})
 
+			It("should forbid changing the networking nodes range", func() {
+				shoot.Spec.Networking.Nodes = pointer.String("10.181.0.0/18")
+				newShoot := prepareShootForUpdate(shoot)
+				newShoot.Spec.Networking.Nodes = pointer.String("10.181.0.0/16")
+
+				errorList := ValidateShootUpdate(newShoot, shoot)
+
+				Expect(errorList).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{
+					"Type":  Equal(field.ErrorTypeInvalid),
+					"Field": Equal("spec.networking.nodes"),
+				}))))
+			})
+
+			It("should allow increasing the networking nodes range if feature gate is enabled", func() {
+				DeferCleanup(test.WithFeatureGate(utilfeature.DefaultMutableFeatureGate, features.MutableShootSpecNetworkingNodes, true))
+				shoot.Spec.Networking.Nodes = pointer.String("10.181.0.0/18")
+				newShoot := prepareShootForUpdate(shoot)
+				newShoot.Spec.Networking.Nodes = pointer.String("10.181.0.0/16")
+
+				errorList := ValidateShootUpdate(newShoot, shoot)
+
+				Expect(errorList).To(HaveLen(0))
+			})
+
 			It("should forbid specifying unsupported IP family", func() {
 				shoot.Spec.Networking.IPFamilies = []core.IPFamily{"IPv5"}
 

diff --git a/pkg/apis/seedmanagement/validation/managedseedset_test.go b/pkg/apis/seedmanagement/validation/managedseedset_test.go
@@ -22,12 +22,15 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/util/validation/field"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/utils/pointer"
 
 	"github.com/gardener/gardener/pkg/apis/core"
 	gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1"
 	"github.com/gardener/gardener/pkg/apis/seedmanagement"
 	. "github.com/gardener/gardener/pkg/apis/seedmanagement/validation"
+	"github.com/gardener/gardener/pkg/features"
+	"github.com/gardener/gardener/pkg/utils/test"
 )
 
 var _ = Describe("ManagedSeedSet Validation Tests", func() {
@@ -54,7 +57,8 @@ var _ = Describe("ManagedSeedSet Validation Tests", func() {
 					Version: "1.18.14",
 				},
 				Networking: core.Networking{
-					Type: "foo",
+					Type:  "foo",
+					Nodes: pointer.String("10.181.0.0/18"),
 				},
 				Provider: core.Provider{
 					Type: "foo",
@@ -378,8 +382,10 @@ var _ = Describe("ManagedSeedSet Validation Tests", func() {
 		})
 
 		It("should forbid changes to immutable fields in shootTemplate", func() {
+			DeferCleanup(test.WithFeatureGate(utilfeature.DefaultMutableFeatureGate, features.MutableShootSpecNetworkingNodes, true))
 			shootCopy := shoot.DeepCopy()
 			shootCopy.Spec.Region = "other-region"
+			shootCopy.Spec.Networking.Nodes = pointer.String("10.181.0.0/16")
 			newManagedSeedSet.Spec.ShootTemplate.Spec = shootCopy.Spec
 
 			errorList := ValidateManagedSeedSetUpdate(newManagedSeedSet, managedSeedSet)
@@ -390,6 +396,11 @@ var _ = Describe("ManagedSeedSet Validation Tests", func() {
 					"Field":  Equal("spec.shootTemplate.spec.region"),
 					"Detail": Equal("field is immutable"),
 				})),
+				PointTo(MatchFields(IgnoreExtras, Fields{
+					"Type":   Equal(field.ErrorTypeInvalid),
+					"Field":  Equal("spec.shootTemplate.spec.networking.nodes"),
+					"Detail": Equal("field is immutable"),
+				})),
 			))
 		})
 	})

diff --git a/pkg/apis/seedmanagement/validation/validation_suite_test.go b/pkg/apis/seedmanagement/validation/validation_suite_test.go
@@ -19,9 +19,12 @@ import (
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+
+	"github.com/gardener/gardener/pkg/apiserver/features"
 )
 
 func TestValidation(t *testing.T) {
 	RegisterFailHandler(Fail)
+	features.RegisterFeatureGates()
 	RunSpecs(t, "APIs SeedManagement Validation Suite")
 }
diff --git a/pkg/apiserver/features/features.go b/pkg/apiserver/features/features.go
@@ -27,5 +27,6 @@ func RegisterFeatureGates() {
 		features.HAControlPlanes,
 		features.SeedChange,
 		features.IPv6SingleStack,
+		features.MutableShootSpecNetworkingNodes,
 	)))
 }
diff --git a/pkg/features/features.go b/pkg/features/features.go
@@ -104,6 +104,11 @@ const (
 	// owner: @timebertt
 	// alpha: v1.63.0
 	IPv6SingleStack featuregate.Feature = "IPv6SingleStack"
+
+	// MutableShootSpecNetworkingNodes allows updating the field `spec.networking.nodes`.
+	// owner: @axel7born @ScheererJ @DockToFuture @kon-angelo
+	// alpha: v1.64.0
+	MutableShootSpecNetworkingNodes featuregate.Feature = "MutableShootSpecNetworkingNodes"
 )
 
 var allFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{
@@ -114,11 +119,12 @@ var allFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{
 	SeedChange:         {Default: true, PreRelease: featuregate.Beta},
 	ReversedVPN:        {Default: true, PreRelease: featuregate.GA, LockToDefault: true},
 	CopyEtcdBackupsDuringControlPlaneMigration: {Default: true, PreRelease: featuregate.Beta},
-	ForceRestore:          {Default: false, PreRelease: featuregate.Alpha},
-	HAControlPlanes:       {Default: false, PreRelease: featuregate.Alpha},
-	DefaultSeccompProfile: {Default: false, PreRelease: featuregate.Alpha},
-	CoreDNSQueryRewriting: {Default: false, PreRelease: featuregate.Alpha},
-	IPv6SingleStack:       {Default: false, PreRelease: featuregate.Alpha},
+	ForceRestore:                    {Default: false, PreRelease: featuregate.Alpha},
+	HAControlPlanes:                 {Default: false, PreRelease: featuregate.Alpha},
+	DefaultSeccompProfile:           {Default: false, PreRelease: featuregate.Alpha},
+	CoreDNSQueryRewriting:           {Default: false, PreRelease: featuregate.Alpha},
+	IPv6SingleStack:                 {Default: false, PreRelease: featuregate.Alpha},
+	MutableShootSpecNetworkingNodes: {Default: false, PreRelease: featuregate.Alpha},
 }
 
 // GetFeatures returns a feature gate map with the respective specifications. Non-existing feature gates are ignored.

diff --git a/pkg/openapi/openapi_generated.go b/pkg/openapi/openapi_generated.go
diff --git a/plugin/pkg/shoot/managedseed/admission.go b/plugin/pkg/shoot/managedseed/admission.go
@@ -152,6 +152,11 @@ func (v *ManagedSeed) validateUpdate(ctx context.Context, a admission.Attributes
 		return apierrors.NewInternalError(errors.New("could not convert resource into Shoot object"))
 	}
 
+	oldShoot, ok := a.GetOldObject().(*core.Shoot)
+	if !ok {
+		return apierrors.NewInternalError(errors.New("could not convert resource into Shoot object"))
+	}
+
 	var allErrs field.ErrorList
 	if nginxIngressEnabled := gardencorehelper.NginxIngressEnabled(shoot.Spec.Addons); nginxIngressEnabled {
 		allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "addons", "nginxIngress", "enabled"), nginxIngressEnabled, "shoot ingress addon is not supported for managed seeds - use the managed seed ingress controller"))
@@ -160,6 +165,10 @@ func (v *ManagedSeed) validateUpdate(ctx context.Context, a admission.Attributes
 		allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "kubernetes", "verticalPodAutoscaler", "enabled"), vpaEnabled, "shoot VPA has to be enabled for managed seeds"))
 	}
 
+	if oldShoot.Spec.Networking.Nodes != nil && *oldShoot.Spec.Networking.Nodes != *shoot.Spec.Networking.Nodes {
+		allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "networking", "nodes"), shoot.Spec.Networking.Nodes, "field is immutable for managed seeds"))
+	}
+
 	if len(allErrs) > 0 {
 		return apierrors.NewInvalid(a.GetKind().GroupKind(), shoot.Name, allErrs)
 	}