From 750f53381509416439b60ffb2ec25601b495e4e0 Mon Sep 17 00:00:00 2001 From: Luis Sanchez Date: Wed, 10 Sep 2025 10:28:06 -0400 Subject: [PATCH] WIP: use server-side-apply to update client-ca bundle --- .../targetconfigcontroller.go | 40 +++++++++++++------ .../targetconfigcontroller_test.go | 15 +++++-- 2 files changed, 40 insertions(+), 15 deletions(-) diff --git a/pkg/operator/targetconfigcontroller/targetconfigcontroller.go b/pkg/operator/targetconfigcontroller/targetconfigcontroller.go index 7a4406f670..b8e6c74f52 100644 --- a/pkg/operator/targetconfigcontroller/targetconfigcontroller.go +++ b/pkg/operator/targetconfigcontroller/targetconfigcontroller.go @@ -35,6 +35,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/util/sets" + applycorev1 "k8s.io/client-go/applyconfigurations/core/v1" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" coreclientv1 "k8s.io/client-go/kubernetes/typed/core/v1" @@ -402,22 +403,37 @@ func ManageClientCABundle(ctx context.Context, lister corev1listers.ConfigMapLis return nil, false, err } - if creationRequired { - caBundleConfigMap, err = client.ConfigMaps(operatorclient.TargetNamespace).Create(ctx, requiredConfigMap, metav1.CreateOptions{}) - resourcehelper.ReportCreateEvent(recorder, caBundleConfigMap, err) - if err != nil { - return nil, false, err + patch := applycorev1.ConfigMap(caBundleConfigMapName, operatorclient.TargetNamespace) + patch.WithData(requiredConfigMap.Data) + tmp := &metav1.ObjectMeta{} + additionalAnnotations.EnsureTLSMetadataUpdate(tmp) + patch.WithAnnotations(tmp.Annotations) + + if creationRequired || updateRequired { + + result, err := client.ConfigMaps(operatorclient.TargetNamespace).Apply(ctx, patch, metav1.ApplyOptions{Force: true, FieldManager: "cluster-kube-apiserver-operator"}) + + // emit events + if (result != nil && result.ResourceVersion == "1") || creationRequired { + // either the configmap was created or we expected it to be created and it failed + resourcehelper.ReportCreateEvent(recorder, caBundleConfigMap, err) + } else { + // an existing configmap was updated, even if we expected it to be created + resourcehelper.ReportUpdateEvent(recorder, caBundleConfigMap, err) } - klog.V(2).Infof("Created client CA bundle configmap %s/%s", caBundleConfigMap.Namespace, caBundleConfigMap.Name) - return caBundleConfigMap, true, nil - } else if updateRequired { - caBundleConfigMap, err = client.ConfigMaps(operatorclient.TargetNamespace).Update(ctx, requiredConfigMap, metav1.UpdateOptions{}) - resourcehelper.ReportUpdateEvent(recorder, caBundleConfigMap, err) + + // handle the err from the Apply call if err != nil { return nil, false, err } - klog.V(2).Infof("Updated client CA bundle configmap %s/%s", caBundleConfigMap.Namespace, caBundleConfigMap.Name) - return caBundleConfigMap, true, nil + + if result.ResourceVersion == "1" { + klog.V(2).Infof("Created client CA bundle configmap %s/%s", result.Namespace, result.Name) + } else { + klog.V(2).Infof("Updated client CA bundle configmap %s/%s", result.Namespace, result.Name) + } + + return result, true, nil } return caBundleConfigMap, false, nil diff --git a/pkg/operator/targetconfigcontroller/targetconfigcontroller_test.go b/pkg/operator/targetconfigcontroller/targetconfigcontroller_test.go index cf0744efe1..2fdb6fb337 100644 --- a/pkg/operator/targetconfigcontroller/targetconfigcontroller_test.go +++ b/pkg/operator/targetconfigcontroller/targetconfigcontroller_test.go @@ -634,6 +634,7 @@ func TestManageClientCABundle(t *testing.T) { name: "create new client-ca configmap when none exists", existingConfigMaps: []*corev1.ConfigMap{}, expectedConfigMap: &corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{Kind: "ConfigMap", APIVersion: "v1"}, ObjectMeta: metav1.ObjectMeta{ Name: "client-ca", Namespace: operatorclient.TargetNamespace, @@ -661,6 +662,7 @@ func TestManageClientCABundle(t *testing.T) { }, }, expectedConfigMap: &corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{Kind: "ConfigMap", APIVersion: "v1"}, ObjectMeta: metav1.ObjectMeta{ Name: "client-ca", Namespace: operatorclient.TargetNamespace, @@ -698,6 +700,7 @@ func TestManageClientCABundle(t *testing.T) { }, }, expectedConfigMap: &corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{Kind: "ConfigMap", APIVersion: "v1"}, ObjectMeta: metav1.ObjectMeta{ Name: "client-ca", Namespace: operatorclient.TargetNamespace, @@ -715,6 +718,7 @@ func TestManageClientCABundle(t *testing.T) { name: "annotations update", existingConfigMaps: []*corev1.ConfigMap{ { + TypeMeta: metav1.TypeMeta{Kind: "ConfigMap", APIVersion: "v1"}, ObjectMeta: metav1.ObjectMeta{ Name: "client-ca", Namespace: operatorclient.TargetNamespace, @@ -737,6 +741,7 @@ func TestManageClientCABundle(t *testing.T) { }, }, expectedConfigMap: &corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{Kind: "ConfigMap", APIVersion: "v1"}, ObjectMeta: metav1.ObjectMeta{ Name: "client-ca", Namespace: operatorclient.TargetNamespace, @@ -787,6 +792,7 @@ func TestManageClientCABundle(t *testing.T) { }, }, expectedConfigMap: &corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{Kind: "ConfigMap", APIVersion: "v1"}, ObjectMeta: metav1.ObjectMeta{ Name: "client-ca", Namespace: operatorclient.TargetNamespace, @@ -826,6 +832,7 @@ func TestManageClientCABundle(t *testing.T) { }, }, expectedConfigMap: &corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{Kind: "ConfigMap", APIVersion: "v1"}, ObjectMeta: metav1.ObjectMeta{ Name: "client-ca", Namespace: operatorclient.TargetNamespace, @@ -843,7 +850,7 @@ func TestManageClientCABundle(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - client := fake.NewSimpleClientset() + client := fake.NewClientset() // Create existing configmaps for _, cm := range test.existingConfigMaps { @@ -867,7 +874,8 @@ func TestManageClientCABundle(t *testing.T) { // Assert change expectations require.Equal(t, test.expectedChanged, changed, "Expected changed=%v, got changed=%v", test.expectedChanged, changed) - // Compare with expected configmap + // Compare with expected configmap, ignoring managed fields + resultConfigMap.ManagedFields = nil require.Equal(t, test.expectedConfigMap, resultConfigMap) // Verify the configmap exists in the cluster @@ -875,7 +883,8 @@ func TestManageClientCABundle(t *testing.T) { require.NoError(t, err) require.NotNil(t, storedConfigMap) - // Ensure the returned configmap matches what's stored in the cluster + // Ensure the returned configmap matches what's stored in the cluster, ignoring managed fields + storedConfigMap.ManagedFields = nil require.Equal(t, storedConfigMap, resultConfigMap, "returned configmap should match stored configmap") // Verify events were recorded if changes were made