Try to fix CC rollout issue

Signed-off-by: Stefan Büringer [email protected]

bootstrap/kubeadm/api/v1beta1/kubeadmconfigtemplate_webhook.go

@@ -17,26 +17,51 @@ limitations under the License.
 package v1beta1
 
 import (
+	"context"
+	"fmt"
+
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	runtime "k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )
 
 func (r *KubeadmConfigTemplate) SetupWebhookWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewWebhookManagedBy(mgr).
 		For(r).
+		WithDefaulter(&KubeadmConfigTemplateWebhook{}).
 		Complete()
 }
 
+// KubeadmConfigTemplateWebhook implements a custom defaulting webhook for KubeadmConfigTemplate.
+// +kubebuilder:object:generate=false
+type KubeadmConfigTemplateWebhook struct{}
+
 // +kubebuilder:webhook:verbs=create;update,path=/mutate-bootstrap-cluster-x-k8s-io-v1beta1-kubeadmconfigtemplate,mutating=true,failurePolicy=fail,groups=bootstrap.cluster.x-k8s.io,resources=kubeadmconfigtemplates,versions=v1beta1,name=default.kubeadmconfigtemplate.bootstrap.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1
 
-var _ webhook.Defaulter = &KubeadmConfigTemplate{}
+var _ webhook.CustomDefaulter = &KubeadmConfigTemplateWebhook{}
 
 // Default implements webhook.Defaulter so a webhook will be registered for the type.
-func (r *KubeadmConfigTemplate) Default() {
-	DefaultKubeadmConfigSpec(&r.Spec.Template.Spec)
+func (k KubeadmConfigTemplateWebhook) Default(ctx context.Context, raw runtime.Object) error {
+	obj, ok := raw.(*KubeadmConfigTemplate)
+	if !ok {
+		return apierrors.NewBadRequest(fmt.Sprintf("expected a KubeadmConfigTemplate but got a %T", obj))
+	}
+
+	req, err := admission.RequestFromContext(ctx)
+	if err != nil {
+		return apierrors.NewBadRequest(fmt.Sprintf("expected a admission.Request inside context: %v", err))
+	}
+
+	//
+	//if !topology.ShouldSkipDefaulting(req, obj) {
+	if req.UserInfo.Username != "kubernetes-admin" {
+		DefaultKubeadmConfigSpec(&obj.Spec.Template.Spec)
+	}
+
+	return nil
 }
 
 // +kubebuilder:webhook:verbs=create;update,path=/validate-bootstrap-cluster-x-k8s-io-v1beta1-kubeadmconfigtemplate,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=bootstrap.cluster.x-k8s.io,resources=kubeadmconfigtemplates,versions=v1beta1,name=validation.kubeadmconfigtemplate.bootstrap.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1

internal/controllers/topology/cluster/structuredmerge/dryrun.go

@@ -19,8 +19,10 @@ package structuredmerge
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 
 	jsonpatch "github.com/evanphx/json-patch/v5"
+	"github.com/google/go-cmp/cmp"
 	"github.com/pkg/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -51,18 +53,56 @@ func dryRunSSAPatch(ctx context.Context, dryRunCtx *dryRunSSAPatchInput) (bool,
 		ignorePaths:  dryRunCtx.helperOptions.ignorePaths,
 	}
 
+	// FIXME(sbueringer) TBD
+	// * we can skip defaulting here, but what if defaulting was only run on one of:
+	//   * KubeadmConfigTemplate referenced in the ClusterClass
+	//   * KubeadmConfigTemplate used in an existing MachineDeployment
+
 	// Add TopologyDryRunAnnotation to notify validation webhooks to skip immutability checks.
-	if err := unstructured.SetNestedField(dryRunCtx.dryRunUnstructured.Object, "", "metadata", "annotations", clusterv1.TopologyDryRunAnnotation); err != nil {
+	if err := unstructured.SetNestedField(dryRunCtx.dryRunUnstructured.Object, "", "metadata", "annotations", clusterv1.TopologyDryRunAnnotation); err != nil { // FIXME: need this too?
+		return false, false, errors.Wrap(err, "failed to add topology dry-run annotation")
+	}
+	if err := unstructured.SetNestedField(dryRunCtx.originalUnstructured.Object, "", "metadata", "annotations", clusterv1.TopologyDryRunAnnotation); err != nil { // FIXME: need this too?
 		return false, false, errors.Wrap(err, "failed to add topology dry-run annotation")
 	}
 
-	// Do a server-side apply dry-run request to get the updated object.
+	beforeDryRunUnstructured := dryRunCtx.dryRunUnstructured.DeepCopy()
+
+	// Do a server-side apply dry-run request for dryRunUnstructured to get the updated object.
 	err := dryRunCtx.client.Patch(ctx, dryRunCtx.dryRunUnstructured, client.Apply, client.DryRunAll, client.FieldOwner(TopologyManagerName), client.ForceOwnership)
 	if err != nil {
 		// This catches errors like metadata.uid changes.
 		return false, false, errors.Wrap(err, "failed to request dry-run server side apply")
 	}
 
+	// Set managed fields to nil, otherwise we get:
+	// failed to request dry-run server side apply: metadata.managedFields must be nil
+	//dryRunCtx.originalUnstructured.SetManagedFields(nil)
+
+	beforeOriginalUnstructured := dryRunCtx.originalUnstructured.DeepCopy()
+	testString := cmp.Diff(beforeOriginalUnstructured, beforeDryRunUnstructured)
+	fmt.Print(testString)
+
+	// Do a server-side apply dry-run for originalUnstructured to get the updated object.
+	// Note: This is done to ensure that the latest defaulting logic is also applied to originalUnstructured.
+	// This is not always the case for all objects in the APIserver after a Cluster API upgrade.
+	// Note: It's fine to ignore differences introduced by new defaulting as based on API conventions
+	// these changes are not allowed to lead to a change in behavior (at least not without an API version bump).
+	err = dryRunCtx.client.Patch(ctx, dryRunCtx.originalUnstructured, client.Apply, client.DryRunAll, client.FieldOwner(TopologyManagerName), client.ForceOwnership)
+	if err != nil {
+		// This catches errors like metadata.uid changes.
+		return false, false, errors.Wrap(err, "failed to request dry-run server side apply")
+	}
+
+	testString2 := cmp.Diff(dryRunCtx.originalUnstructured, dryRunCtx.dryRunUnstructured)
+	fmt.Print(testString2)
+
+	testString3 := cmp.Diff(beforeOriginalUnstructured, dryRunCtx.originalUnstructured)
+	fmt.Print(testString3)
+
+	testString4 := cmp.Diff(beforeDryRunUnstructured, dryRunCtx.dryRunUnstructured)
+	fmt.Print(testString4)
+
 	// Cleanup the dryRunUnstructured object to remove the added TopologyDryRunAnnotation
 	// and remove the affected managedFields for `manager=capi-topology` which would
 	// otherwise show the additional field ownership for the annotation we added and

internal/controllers/topology/cluster/structuredmerge/serversidepathhelper.go

@@ -70,6 +70,7 @@ func NewServerSidePatchHelper(ctx context.Context, original, modified client.Obj
 
 	// Filter the modifiedUnstructured object to only contain changes intendet to be done.
 	// The originalUnstructured object will be filtered in dryRunSSAPatch using other options.
+	filterObject(originalUnstructured, helperOptions)
 	filterObject(modifiedUnstructured, helperOptions)
 
 	// Carry over uid to match the intent to:
@@ -95,7 +96,7 @@ func NewServerSidePatchHelper(ctx context.Context, original, modified client.Obj
 		var err error
 		hasChanges, hasSpecChanges, err = dryRunSSAPatch(ctx, &dryRunSSAPatchInput{
 			client:               c,
-			originalUnstructured: originalUnstructured,
+			originalUnstructured: originalUnstructured.DeepCopy(),
 			dryRunUnstructured:   modifiedUnstructured.DeepCopy(),
 			helperOptions:        helperOptions,
 		})

util/topology/topology.go

@@ -29,6 +29,17 @@ import (
 // This ensures that the immutability check is skipped only when dry-running and when the operations has been invoked by the topology controller.
 // Instead, kubectl dry-run behavior remains consistent with the one user gets when doing kubectl apply (immutability is enforced).
 func ShouldSkipImmutabilityChecks(req admission.Request, obj metav1.Object) bool {
+	return isDryRun(req, obj)
+
+}
+
+// ShouldSkipDefaulting returns true if it is a dry-run request and the object
+// FIXME(sbueringer): also consider other func name
+func ShouldSkipDefaulting(req admission.Request, obj metav1.Object) bool {
+	return isDryRun(req, obj)
+}
+
+func isDryRun(req admission.Request, obj metav1.Object) bool {
 	// Check if the request is a dry-run
 	if req.DryRun == nil || !*req.DryRun {
 		return false