Remove more defaulting from KubeadmConfig/KubeadmConfigTemplate/KCP/KCPTemplate (#12495)

Author: sbueringer

api/bootstrap/kubeadm/v1beta2/kubeadm_types.go

@@ -375,7 +375,7 @@ type NodeRegistrationOptions struct {
 	// imagePullPolicy specifies the policy for image pulling
 	// during kubeadm "init" and "join" operations. The value of
 	// this field must be one of "Always", "IfNotPresent" or
-	// "Never". Defaults to "IfNotPresent".
+	// "Never". Defaults to "IfNotPresent" if not set.
 	// +kubebuilder:validation:Enum=Always;IfNotPresent;Never
 	// +optional
 	ImagePullPolicy corev1.PullPolicy `json:"imagePullPolicy,omitempty"`

api/bootstrap/kubeadm/v1beta2/kubeadmconfig_types.go

@@ -120,7 +120,8 @@ type KubeadmConfigSpec struct {
 	// +optional
 	NTP *NTP `json:"ntp,omitempty"`
 
-	// format specifies the output format of the bootstrap data
+	// format specifies the output format of the bootstrap data.
+	// Defaults to cloud-config if not set.
 	// +optional
 	Format Format `json:"format,omitempty"`
 
@@ -134,28 +135,6 @@ type KubeadmConfigSpec struct {
 	Ignition *IgnitionSpec `json:"ignition,omitempty"`
 }
 
-// Default defaults a KubeadmConfigSpec.
-func (c *KubeadmConfigSpec) Default() {
-	if c.Format == "" {
-		c.Format = CloudConfig
-	}
-	if c.InitConfiguration != nil && c.InitConfiguration.NodeRegistration.ImagePullPolicy == "" {
-		c.InitConfiguration.NodeRegistration.ImagePullPolicy = "IfNotPresent"
-	}
-	if c.JoinConfiguration != nil && c.JoinConfiguration.NodeRegistration.ImagePullPolicy == "" {
-		c.JoinConfiguration.NodeRegistration.ImagePullPolicy = "IfNotPresent"
-	}
-	if c.JoinConfiguration != nil && c.JoinConfiguration.Discovery.File != nil {
-		if kfg := c.JoinConfiguration.Discovery.File.KubeConfig; kfg != nil {
-			if kfg.User.Exec != nil {
-				if kfg.User.Exec.APIVersion == "" {
-					kfg.User.Exec.APIVersion = "client.authentication.k8s.io/v1"
-				}
-			}
-		}
-	}
-}
-
 // Validate ensures the KubeadmConfigSpec is valid.
 func (c *KubeadmConfigSpec) Validate(pathPrefix *field.Path) field.ErrorList {
 	var allErrs field.ErrorList

bootstrap/kubeadm/config/crd/bases/bootstrap.cluster.x-k8s.io_kubeadmconfigs.yaml

@@ -4,27 +4,6 @@ kind: MutatingWebhookConfiguration
 metadata:
   name: mutating-webhook-configuration
 webhooks:
-- admissionReviewVersions:
-  - v1
-  - v1beta1
-  clientConfig:
-    service:
-      name: webhook-service
-      namespace: system
-      path: /mutate-bootstrap-cluster-x-k8s-io-v1beta2-kubeadmconfig
-  failurePolicy: Fail
-  name: default.kubeadmconfig.bootstrap.cluster.x-k8s.io
-  rules:
-  - apiGroups:
-    - bootstrap.cluster.x-k8s.io
-    apiVersions:
-    - v1beta2
-    operations:
-    - CREATE
-    - UPDATE
-    resources:
-    - kubeadmconfigs
-  sideEffects: None
 - admissionReviewVersions:
   - v1
   - v1beta1

bootstrap/kubeadm/config/crd/bases/bootstrap.cluster.x-k8s.io_kubeadmconfigtemplates.yaml

@@ -0,0 +1,44 @@
+/*
+Copyright 2021 The Kubernetes Authors.
+
+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 defaulting contains defaulting code that is used in CABPK and KCP.
+package defaulting
+
+import bootstrapv1 "sigs.k8s.io/cluster-api/api/bootstrap/kubeadm/v1beta2"
+
+// ApplyPreviousKubeadmConfigDefaults defaults a KubeadmConfig with default values we used in the past.
+// This is done in multiple places (webhooks and KCP controller) to ensure no rollouts are triggered now that
+// we removed this defaulting from webhooks.
+func ApplyPreviousKubeadmConfigDefaults(c *bootstrapv1.KubeadmConfigSpec) {
+	if c.Format == "" {
+		c.Format = bootstrapv1.CloudConfig
+	}
+	if c.InitConfiguration != nil && c.InitConfiguration.NodeRegistration.ImagePullPolicy == "" {
+		c.InitConfiguration.NodeRegistration.ImagePullPolicy = "IfNotPresent"
+	}
+	if c.JoinConfiguration != nil && c.JoinConfiguration.NodeRegistration.ImagePullPolicy == "" {
+		c.JoinConfiguration.NodeRegistration.ImagePullPolicy = "IfNotPresent"
+	}
+	if c.JoinConfiguration != nil && c.JoinConfiguration.Discovery.File != nil {
+		if kfg := c.JoinConfiguration.Discovery.File.KubeConfig; kfg != nil {
+			if kfg.User.Exec != nil {
+				if kfg.User.Exec.APIVersion == "" {
+					kfg.User.Exec.APIVersion = "client.authentication.k8s.io/v1"
+				}
+			}
+		}
+	}
+}

bootstrap/kubeadm/config/webhook/manifests.yaml

@@ -1053,10 +1053,14 @@ func (r *KubeadmConfigReconciler) resolveDiscoveryKubeConfig(cfg *bootstrapv1.Fi
 		}
 	}
 	if cfg.KubeConfig.User.Exec != nil {
+		apiVersion := cfg.KubeConfig.User.Exec.APIVersion
+		if apiVersion == "" {
+			apiVersion = "client.authentication.k8s.io/v1"
+		}
 		user.Exec = &clientcmdv1.ExecConfig{
 			Command:            cfg.KubeConfig.User.Exec.Command,
 			Args:               cfg.KubeConfig.User.Exec.Args,
-			APIVersion:         cfg.KubeConfig.User.Exec.APIVersion,
+			APIVersion:         apiVersion,
 			ProvideClusterInfo: ptr.Deref(cfg.KubeConfig.User.Exec.ProvideClusterInfo, false),
 			InteractiveMode:    "Never",
 		}
@@ -1356,6 +1360,11 @@ func (r *KubeadmConfigReconciler) computeClusterConfigurationAndAdditionalData(c
 func (r *KubeadmConfigReconciler) storeBootstrapData(ctx context.Context, scope *Scope, data []byte) error {
 	log := ctrl.LoggerFrom(ctx)
 
+	format := scope.Config.Spec.Format
+	if format == "" {
+		format = bootstrapv1.CloudConfig
+	}
+
 	secret := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      scope.Config.Name,
@@ -1375,7 +1384,7 @@ func (r *KubeadmConfigReconciler) storeBootstrapData(ctx context.Context, scope
 		},
 		Data: map[string][]byte{
 			"value":  data,
-			"format": []byte(scope.Config.Spec.Format),
+			"format": []byte(format),
 		},
 		Type: clusterv1.ClusterSecretType,
 	}

bootstrap/kubeadm/defaulting/defaulting.go

@@ -827,7 +827,8 @@ func TestBootstrapDataFormat(t *testing.T) {
 			clusterInitialized: true,
 		},
 		{
-			name: "Empty format field",
+			name:   "Empty format field",
+			format: bootstrapv1.CloudConfig,
 		},
 	}
 
@@ -2315,8 +2316,7 @@ func TestKubeadmConfigReconciler_ResolveDiscoveryFileKubeConfig(t *testing.T) {
 								KubeConfig: &bootstrapv1.FileDiscoveryKubeConfig{
 									User: bootstrapv1.KubeConfigUser{
 										Exec: &bootstrapv1.KubeConfigAuthExec{
-											APIVersion: "client.authentication.k8s.io/v1",
-											Command:    "/usr/bin/bootstrap",
+											Command: "/usr/bin/bootstrap",
 											Env: []bootstrapv1.KubeConfigAuthExecEnv{
 												{Name: "ENV_TEST", Value: "value"},
 											},

bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller.go

@@ -33,31 +33,16 @@ import (
 func (webhook *KubeadmConfig) SetupWebhookWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewWebhookManagedBy(mgr).
 		For(&bootstrapv1.KubeadmConfig{}).
-		WithDefaulter(webhook).
 		WithValidator(webhook).
 		Complete()
 }
 
-// +kubebuilder:webhook:verbs=create;update,path=/mutate-bootstrap-cluster-x-k8s-io-v1beta2-kubeadmconfig,mutating=true,failurePolicy=fail,groups=bootstrap.cluster.x-k8s.io,resources=kubeadmconfigs,versions=v1beta2,name=default.kubeadmconfig.bootstrap.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1
 // +kubebuilder:webhook:verbs=create;update,path=/validate-bootstrap-cluster-x-k8s-io-v1beta2-kubeadmconfig,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=bootstrap.cluster.x-k8s.io,resources=kubeadmconfigs,versions=v1beta2,name=validation.kubeadmconfig.bootstrap.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1
 
 // KubeadmConfig implements a validation and defaulting webhook for KubeadmConfig.
 type KubeadmConfig struct{}
 
 var _ webhook.CustomValidator = &KubeadmConfig{}
-var _ webhook.CustomDefaulter = &KubeadmConfig{}
-
-// Default implements webhook.Defaulter so a webhook will be registered for the type.
-func (webhook *KubeadmConfig) Default(_ context.Context, obj runtime.Object) error {
-	c, ok := obj.(*bootstrapv1.KubeadmConfig)
-	if !ok {
-		return apierrors.NewBadRequest(fmt.Sprintf("expected a KubeadmConfig but got a %T", obj))
-	}
-
-	c.Spec.Default()
-
-	return nil
-}
 
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
 func (webhook *KubeadmConfig) ValidateCreate(_ context.Context, obj runtime.Object) (admission.Warnings, error) {

bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller_test.go

@@ -27,43 +27,10 @@ import (
 
 	bootstrapv1 "sigs.k8s.io/cluster-api/api/bootstrap/kubeadm/v1beta2"
 	"sigs.k8s.io/cluster-api/feature"
-	"sigs.k8s.io/cluster-api/internal/webhooks/util"
 )
 
 var ctx = ctrl.SetupSignalHandler()
 
-func TestKubeadmConfigDefault(t *testing.T) {
-	utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.ClusterTopology, true)
-
-	g := NewWithT(t)
-
-	kubeadmConfig := &bootstrapv1.KubeadmConfig{
-		ObjectMeta: metav1.ObjectMeta{
-			Namespace: "foo",
-		},
-		Spec: bootstrapv1.KubeadmConfigSpec{},
-	}
-	updateDefaultingKubeadmConfig := kubeadmConfig.DeepCopy()
-	updateDefaultingKubeadmConfig.Spec.Verbosity = ptr.To[int32](4)
-	webhook := &KubeadmConfig{}
-	t.Run("for KubeadmConfig", util.CustomDefaultValidateTest(ctx, updateDefaultingKubeadmConfig, webhook))
-
-	g.Expect(webhook.Default(ctx, kubeadmConfig)).To(Succeed())
-
-	g.Expect(kubeadmConfig.Spec.Format).To(Equal(bootstrapv1.CloudConfig))
-
-	ignitionKubeadmConfig := &bootstrapv1.KubeadmConfig{
-		ObjectMeta: metav1.ObjectMeta{
-			Namespace: "foo",
-		},
-		Spec: bootstrapv1.KubeadmConfigSpec{
-			Format: bootstrapv1.Ignition,
-		},
-	}
-	g.Expect(webhook.Default(ctx, ignitionKubeadmConfig)).To(Succeed())
-	g.Expect(ignitionKubeadmConfig.Spec.Format).To(Equal(bootstrapv1.Ignition))
-}
-
 func TestKubeadmConfigValidate(t *testing.T) {
 	cases := map[string]struct {
 		in                    *bootstrapv1.KubeadmConfig