Default kubernetes.version if omitted (gardener#8198)

* Prefactor: Split defaulting and validation

Fixup: to prefactoring commit

* Default Kubernetes version

* Move defaulting to admission plugin

Defaulting logic that relies on the Kubernetes version must run after the Kubernetes version has been set. This code can't remain in the API defaulting area since this part if processed before the admission plugins.

* Remove duplicate code in admission tests

* Make version optional

* Refine checks for test results

* Consolidate defaulting

* Fix integration test

* Skip defaulting for shoots in deletion

Defaulting should generally not be performed when the shoot is in deletion because another validation potentially blocks this change, see https://github.com/gardener/gardener/blob/599e7c50b76cb9215864d88190122ace566f0f5c/pkg/apis/core/validation/shoot.go#L277.

* Change doc

* Fix some linter findings

docs/api-reference/core.md

@@ -6284,7 +6284,10 @@ string
 </em>
 </td>
 <td>
-<p>Version is the semantic Kubernetes version to use for the Shoot cluster.</p>
+<em>(Optional)</em>
+<p>Version is the semantic Kubernetes version to use for the Shoot cluster.
+Defaults to the highest supported minor and patch version given in the referenced cloud profile.
+The version can be omitted completely or partially specified, e.g. <code>&lt;major&gt;.&lt;minor&gt;</code>.</p>
 </td>
 </tr>
 <tr>

docs/concepts/apiserver_admission_plugins.md

@@ -150,7 +150,7 @@ _(enabled by default)_
 This admission controller reacts on `CREATE`, `UPDATE` and `DELETE` operations for `Shoot`s.
 It validates certain configurations in the specification against the referred `CloudProfile` (e.g., machine images, machine types, used Kubernetes version, ...).
 Generally, it performs validations that cannot be handled by the static API validation due to their dynamic nature (e.g., when something needs to be checked against referred resources).
-Additionally, it takes over certain defaulting tasks (e.g., default machine image for worker pools).
+Additionally, it takes over certain defaulting tasks (e.g., default machine image for worker pools, default Kubernetes version).
 
 ## `ShootManagedSeed`
 
@@ -182,4 +182,4 @@ _(disabled by default)_
 
 This admission controller reacts on `CREATE` operations for `Shoot`s.
 If enabled, it adds a set of common suffixes configured in its admission plugin configuration to the `Shoot` (`spec.systemComponents.coreDNS.rewriting.commonSuffixes`) (for more information, see [DNS Search Path Optimization](../usage/dns-search-path-optimization.md)).
-Already existing `Shoot`s will not be affected by this admission plugin.
+Already existing `Shoot`s will not be affected by this admission plugin.

docs/development/new-kubernetes-version.md

@@ -81,7 +81,11 @@ There is a CI/CD job that runs periodically and releases a new `hyperkube` image
 - Maintain copies of the `DaemonSet` controller's scheduling logic:
   - `gardener-resource-manager`'s [`Node` controller](../concepts/resource-manager.md#node-controllerpkgresourcemanagercontrollernode) uses a copy of parts of the `DaemonSet` controller's logic for determining whether a specific `Node` should run a daemon pod of a given `DaemonSet`: see [this file](https://github.com/gardener/gardener/blob/master/pkg/resourcemanager/controller/node/helper/daemon_controller.go).
   - Check the referenced upstream files for changes to the `DaemonSet` controller's logic and adapt our copies accordingly. This might include introducing version-specific checks in our codebase to handle different shoot cluster versions.
-- Bump the used Kubernetes version for local `Shoot` and local e2e test.
+- Maintain version specific defaulting logic in shoot admission plugin:
+  - Sometimes default values for shoots are intentionally changed with the introduction of a new Kubernetes version.
+  - The final Kubernetes version for a shoot is determined in the [Shoot Validator Admission Plugin](https://github.com/gardener/gardener/blob/17dfefaffed6c5e125e35b6614c8dcad801839f1/plugin/pkg/shoot/validator/admission.go).
+  - Any defaulting logic that depends on the version should be placed in this admission plugin ([example](https://github.com/gardener/gardener/blob/f754c071e6cf8e45f7ac7bc5924acaf81b96dc06/plugin/pkg/shoot/validator/admission.go#L782)).
+- Bump the used Kubernetes version for local e2e test.
   - See [this](https://github.com/gardener/gardener/pull/5255/commits/5707c4c7a4fd265b176387178b755cabeea89ffe) example commit.
 
 #### Filing the Pull Request

example/90-shoot.yaml

@@ -145,7 +145,7 @@ spec:
   #   sshAccess:
   #     enabled: false
   kubernetes:
-    version: 1.25.4
+  # version: 1.27.3
   # enableStaticTokenKubeconfig: true
   # allowPrivilegedContainers: true # 'true' means that all authenticated users can use the "gardener.privileged" PodSecurityPolicy, allowing full unrestricted access to Pod features.
   # kubeAPIServer:

example/provider-local/shoot-workerless.yaml

@@ -9,5 +9,3 @@ spec:
   region: local
   provider:
     type: local
-  kubernetes:
-    version: 1.27.1

example/provider-local/shoot.yaml

@@ -26,7 +26,6 @@ spec:
       maxSurge: 1
       maxUnavailable: 0
   kubernetes:
-    version: 1.27.1
     kubelet:
       seccompDefault: true
       serializeImagePulls: false

extensions/pkg/controller/worker/machines.go

@@ -166,7 +166,7 @@ func WorkerPoolHash(pool extensionsv1alpha1.WorkerPool, cluster *extensionscontr
 		}
 	}
 
-	// Do not consider the shoot annotations here to prevent unintended node roll outs.
+	// Do not consider the shoot annotations here to prevent unintended node roll-outs.
 	if helper.IsNodeLocalDNSEnabled(cluster.Shoot.Spec.SystemComponents, map[string]string{}) {
 		data = append(data, "node-local-dns")
 	}

pkg/apis/core/types_shoot.go

@@ -430,7 +430,13 @@ type Kubernetes struct {
 	KubeProxy *KubeProxyConfig
 	// Kubelet contains configuration settings for the kubelet.
 	Kubelet *KubeletConfig
+	// Note: Even though 'Version' is an optional field for users, we deliberately chose to not make it a pointer
+	// because the field is guaranteed to be not-empty after the admission plugin processed the shoot object.
+	// Thus, pointer handling for this field is not beneficial and would make things more cumbersome.
+
 	// Version is the semantic Kubernetes version to use for the Shoot cluster.
+	// Defaults to the highest supported minor and patch version given in the referenced cloud profile.
+	// The version can be omitted completely or partially specified, e.g. `<major>.<minor>`.
 	Version string
 	// VerticalPodAutoscaler contains the configuration flags for the Kubernetes vertical pod autoscaler.
 	VerticalPodAutoscaler *VerticalPodAutoscaler

pkg/apis/core/v1beta1/defaults.go

@@ -26,7 +26,6 @@ import (
 
 	v1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants"
 	"github.com/gardener/gardener/pkg/utils/timewindow"
-	versionutils "github.com/gardener/gardener/pkg/utils/version"
 )
 
 func addDefaultingFuncs(scheme *runtime.Scheme) error {
@@ -93,15 +92,6 @@ func SetDefaults_Shoot(obj *Shoot) {
 		obj.Spec.Kubernetes.KubeControllerManager = &KubeControllerManagerConfig{}
 	}
 
-	if obj.Spec.Kubernetes.EnableStaticTokenKubeconfig == nil {
-		// Error is ignored here because we cannot do anything meaningful with it - variable will default to "false".
-		if k8sLessThan126, _ := versionutils.CheckVersionMeetsConstraint(obj.Spec.Kubernetes.Version, "< 1.26"); k8sLessThan126 {
-			obj.Spec.Kubernetes.EnableStaticTokenKubeconfig = pointer.Bool(true)
-		} else {
-			obj.Spec.Kubernetes.EnableStaticTokenKubeconfig = pointer.Bool(false)
-		}
-	}
-
 	if obj.Spec.Purpose == nil {
 		p := ShootPurposeEvaluation
 		obj.Spec.Purpose = &p
@@ -156,12 +146,6 @@ func SetDefaults_Shoot(obj *Shoot) {
 
 	// these fields are relevant only for shoot with workers
 	if len(obj.Spec.Provider.Workers) > 0 {
-		// Errors are ignored here because we cannot do anything meaningful with them - variables will default to `false`.
-		k8sLess125, _ := versionutils.CheckVersionMeetsConstraint(obj.Spec.Kubernetes.Version, "< 1.25")
-		if obj.Spec.Kubernetes.AllowPrivilegedContainers == nil && k8sLess125 && !isPSPDisabled(obj) {
-			obj.Spec.Kubernetes.AllowPrivilegedContainers = pointer.Bool(true)
-		}
-
 		if obj.Spec.Kubernetes.KubeAPIServer.DefaultNotReadyTolerationSeconds == nil {
 			obj.Spec.Kubernetes.KubeAPIServer.DefaultNotReadyTolerationSeconds = pointer.Int64(300)
 		}
@@ -172,14 +156,6 @@ func SetDefaults_Shoot(obj *Shoot) {
 		if obj.Spec.Kubernetes.KubeControllerManager.NodeCIDRMaskSize == nil {
 			obj.Spec.Kubernetes.KubeControllerManager.NodeCIDRMaskSize = calculateDefaultNodeCIDRMaskSize(&obj.Spec)
 		}
-		k8sLess127, _ := versionutils.CheckVersionMeetsConstraint(obj.Spec.Kubernetes.Version, "< 1.27")
-		if obj.Spec.Kubernetes.KubeControllerManager.NodeMonitorGracePeriod == nil {
-			if k8sLess127 {
-				obj.Spec.Kubernetes.KubeControllerManager.NodeMonitorGracePeriod = &metav1.Duration{Duration: 2 * time.Minute}
-			} else {
-				obj.Spec.Kubernetes.KubeControllerManager.NodeMonitorGracePeriod = &metav1.Duration{Duration: 40 * time.Second}
-			}
-		}
 
 		if obj.Spec.Kubernetes.KubeScheduler == nil {
 			obj.Spec.Kubernetes.KubeScheduler = &KubeSchedulerConfig{}
@@ -310,7 +286,7 @@ func SetDefaults_KubeAPIServerConfig(obj *KubeAPIServerConfig) {
 }
 
 // SetDefaults_KubeControllerManagerConfig sets default values for KubeControllerManagerConfig objects.
-func SetDefaults_KubeControllerManagerConfig(obj *KubeControllerManagerConfig) {}
+func SetDefaults_KubeControllerManagerConfig(_ *KubeControllerManagerConfig) {}
 
 // SetDefaults_Networking sets default values for Networking objects.
 func SetDefaults_Networking(obj *Networking) {
@@ -460,14 +436,3 @@ func addTolerations(tolerations *[]Toleration, additionalTolerations ...Tolerati
 		*tolerations = append(*tolerations, toleration)
 	}
 }
-
-func isPSPDisabled(shoot *Shoot) bool {
-	if shoot.Spec.Kubernetes.KubeAPIServer != nil {
-		for _, plugin := range shoot.Spec.Kubernetes.KubeAPIServer.AdmissionPlugins {
-			if plugin.Name == "PodSecurityPolicy" && pointer.BoolDeref(plugin.Disabled, false) {
-				return true
-			}
-		}
-	}
-	return false
-}

pkg/apis/core/v1beta1/defaults_test.go

@@ -115,7 +115,7 @@ var _ = Describe("Defaults", func() {
 						Version: "1.22.1",
 					},
 					Provider: Provider{
-						Workers: []Worker{Worker{}},
+						Workers: []Worker{{}},
 					},
 				},
 			}
@@ -352,33 +352,6 @@ var _ = Describe("Defaults", func() {
 		})
 
 		Describe("kubeControllerManager settings", func() {
-			It("should not overwrite the kube-controller-manager's node monitor grace period", func() {
-				nodeMonitorGracePeriod := &metav1.Duration{Duration: time.Minute}
-				obj.Spec.Kubernetes.KubeControllerManager = &KubeControllerManagerConfig{NodeMonitorGracePeriod: nodeMonitorGracePeriod}
-
-				SetObjectDefaults_Shoot(obj)
-
-				Expect(obj.Spec.Kubernetes.KubeControllerManager.NodeMonitorGracePeriod).To(Equal(nodeMonitorGracePeriod))
-			})
-
-			It("should default the kube-controller-manager's node monitor grace period to 2 minutes for Shoot cluster with k8s version < 1.27", func() {
-				obj.Spec.Kubernetes.Version = "1.26.0"
-				obj.Spec.Kubernetes.KubeControllerManager = &KubeControllerManagerConfig{}
-
-				SetObjectDefaults_Shoot(obj)
-
-				Expect(obj.Spec.Kubernetes.KubeControllerManager.NodeMonitorGracePeriod).To(Equal(&metav1.Duration{Duration: 2 * time.Minute}))
-			})
-
-			It("should default the kube-controller-manager's node monitor grace period to 40 seconds for Shoot cluster with k8s version >= 1.27", func() {
-				obj.Spec.Kubernetes.Version = "1.27.0"
-				obj.Spec.Kubernetes.KubeControllerManager = &KubeControllerManagerConfig{}
-
-				SetObjectDefaults_Shoot(obj)
-
-				Expect(obj.Spec.Kubernetes.KubeControllerManager.NodeMonitorGracePeriod).To(Equal(&metav1.Duration{Duration: 40 * time.Second}))
-			})
-
 			Describe("nodeCIDRMaskSize", func() {
 				Context("IPv4", func() {
 					It("should make nodeCIDRMaskSize big enough for 2*maxPods", func() {
@@ -707,95 +680,6 @@ var _ = Describe("Defaults", func() {
 			})
 		})
 
-		Context("static token kubeconfig", func() {
-			It("should not default the enableStaticTokenKubeconfig field when it is set", func() {
-				obj.Spec.Kubernetes = Kubernetes{
-					Version:                     "1.24.0",
-					EnableStaticTokenKubeconfig: pointer.Bool(false),
-				}
-
-				SetObjectDefaults_Shoot(obj)
-
-				Expect(obj.Spec.Kubernetes.EnableStaticTokenKubeconfig).To(PointTo(BeFalse()))
-			})
-
-			It("should default the enableStaticTokenKubeconfig field to true for k8s version < 1.26", func() {
-				obj.Spec.Kubernetes = Kubernetes{Version: "1.25.0"}
-
-				SetObjectDefaults_Shoot(obj)
-
-				Expect(obj.Spec.Kubernetes.EnableStaticTokenKubeconfig).To(PointTo(BeTrue()))
-			})
-
-			It("should default the enableStaticTokenKubeconfig field to false for k8s version >= 1.26", func() {
-				obj.Spec.Kubernetes = Kubernetes{Version: "1.26.0"}
-
-				SetObjectDefaults_Shoot(obj)
-
-				Expect(obj.Spec.Kubernetes.EnableStaticTokenKubeconfig).To(PointTo(BeFalse()))
-			})
-		})
-
-		Context("k8s version < 1.25", func() {
-			BeforeEach(func() {
-				obj.Spec.Kubernetes = Kubernetes{
-					Version:       "1.24.0",
-					KubeAPIServer: &KubeAPIServerConfig{},
-				}
-				obj.Spec.Provider = Provider{
-					Workers: []Worker{Worker{}},
-				}
-			})
-
-			Context("allowPrivilegedContainers field is not set", func() {
-				It("should set the field to true if PodSecurityPolicy admission plugin is not disabled and shoot has workers", func() {
-					SetObjectDefaults_Shoot(obj)
-
-					Expect(obj.Spec.Kubernetes.AllowPrivilegedContainers).To(PointTo(BeTrue()))
-				})
-
-				It("should not set the field if the shoot is workerless", func() {
-					obj.Spec.Provider.Workers = nil
-					SetObjectDefaults_Shoot(obj)
-
-					Expect(obj.Spec.Kubernetes.AllowPrivilegedContainers).To(BeNil())
-				})
-
-				It("should not default the field if PodSecurityPolicy admission plugin is disabled in the shoot spec", func() {
-					obj.Spec.Kubernetes.KubeAPIServer = &KubeAPIServerConfig{
-						AdmissionPlugins: []AdmissionPlugin{
-							{
-								Name:     "PodSecurityPolicy",
-								Disabled: pointer.Bool(true),
-							},
-						},
-					}
-					SetObjectDefaults_Shoot(obj)
-
-					Expect(obj.Spec.Kubernetes.AllowPrivilegedContainers).To(BeNil())
-				})
-
-				It("should not default the field if the Shoot is workerless", func() {
-					obj.Spec.Provider.Workers = nil
-					SetObjectDefaults_Shoot(obj)
-
-					Expect(obj.Spec.Kubernetes.AllowPrivilegedContainers).To(BeNil())
-				})
-			})
-
-			Context("allowPrivilegedContainers field is set", func() {
-				BeforeEach(func() {
-					obj.Spec.Kubernetes.AllowPrivilegedContainers = pointer.Bool(false)
-				})
-
-				It("should not set the field", func() {
-					SetObjectDefaults_Shoot(obj)
-
-					Expect(obj.Spec.Kubernetes.AllowPrivilegedContainers).To(PointTo(BeFalse()))
-				})
-			})
-		})
-
 		Context("k8s version >= 1.25", func() {
 			BeforeEach(func() {
 				obj.Spec.Kubernetes.Version = "1.25.0"

pkg/apis/core/v1beta1/types_shoot.go

@@ -530,8 +530,15 @@ type Kubernetes struct {
 	// Kubelet contains configuration settings for the kubelet.
 	// +optional
 	Kubelet *KubeletConfig `json:"kubelet,omitempty" protobuf:"bytes,7,opt,name=kubelet"`
+	// Note: Even though 'Version' is an optional field for users, we deliberately chose to not make it a pointer
+	// because the field is guaranteed to be not-empty after the admission plugin processed the shoot object.
+	// Thus, pointer handling for this field is not beneficial and would make things more cumbersome.
+
 	// Version is the semantic Kubernetes version to use for the Shoot cluster.
-	Version string `json:"version" protobuf:"bytes,8,opt,name=version"`
+	// Defaults to the highest supported minor and patch version given in the referenced cloud profile.
+	// The version can be omitted completely or partially specified, e.g. `<major>.<minor>`.
+	// +optional
+	Version string `json:"version,omitempty" protobuf:"bytes,8,opt,name=version"`
 	// VerticalPodAutoscaler contains the configuration flags for the Kubernetes vertical pod autoscaler.
 	// +optional
 	VerticalPodAutoscaler *VerticalPodAutoscaler `json:"verticalPodAutoscaler,omitempty" protobuf:"bytes,9,opt,name=verticalPodAutoscaler"`