Skip to content

Commit c2e5d73

Browse files
committed
targetconfigcontroller: make sure extension-apiserver-authentication has necessary annotations
configmap kube-system/extension-apiserver-authentication is created by kube-apiserver, but it doesn't have ownership metadata. This commit updates target config controller to set necessary metadata (ownership and description)
1 parent 0bec046 commit c2e5d73

File tree

3 files changed

+229
-0
lines changed

3 files changed

+229
-0
lines changed
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
# NOTE: This asset defines the required annotations for the live
2+
# kube-system/extension-apiserver-authentication ConfigMap. It is not
3+
# applied directly; the operator reads these annotations and reconciles
4+
# them on the existing ConfigMap created by kube-apiserver.
5+
apiVersion: v1
6+
kind: ConfigMap
7+
metadata:
8+
name: extension-apiserver-authentication
9+
namespace: kube-system
10+
annotations:
11+
"openshift.io/owning-component": "kube-apiserver"
12+
"openshift.io/description": "CA holding the root certificate bundle used to verify client certificates on incoming requests"

pkg/operator/targetconfigcontroller/targetconfigcontroller.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,11 @@ func createTargetConfig(ctx context.Context, c TargetConfigController, recorder
237237
errors = append(errors, fmt.Errorf("%q: %v", "configmap/trusted-ca-bundle", err))
238238
}
239239

240+
err = ensureKubeAPIServerExtensionAuthenticationCA(ctx, c.kubeClient.CoreV1(), recorder)
241+
if err != nil {
242+
errors = append(errors, fmt.Errorf("%q: %v", "configmap/extension-apiserver-authentication", err))
243+
}
244+
240245
err = ensureLocalhostRecoverySAToken(ctx, c.kubeClient.CoreV1(), recorder)
241246
if err != nil {
242247
errors = append(errors, fmt.Errorf("%q: %v", "serviceaccount/localhost-recovery-client", err))
@@ -507,6 +512,40 @@ func ensureKubeAPIServerTrustedCA(ctx context.Context, client coreclientv1.CoreV
507512
return err
508513
}
509514

515+
func ensureKubeAPIServerExtensionAuthenticationCA(ctx context.Context, client coreclientv1.CoreV1Interface, recorder events.Recorder) error {
516+
required := resourceread.ReadConfigMapV1OrDie(bindata.MustAsset("assets/kube-apiserver/extension-apiserver-authentication-cm.yaml"))
517+
cmClient := client.ConfigMaps("kube-system")
518+
519+
cm, err := cmClient.Get(ctx, "extension-apiserver-authentication", metav1.GetOptions{})
520+
if err != nil {
521+
// kube-apiserver creates this CM; don't degrade while waiting.
522+
return nil
523+
}
524+
525+
// Ensure that the config map is updated with the required annotations
526+
modified := false
527+
if cm.Annotations == nil {
528+
cm.Annotations = make(map[string]string)
529+
}
530+
531+
for key, expected := range required.Annotations {
532+
if actual, ok := cm.Annotations[key]; !ok || actual != expected {
533+
cm.Annotations[key] = expected
534+
modified = true
535+
}
536+
}
537+
538+
if modified {
539+
if _, err := cmClient.Update(ctx, cm, metav1.UpdateOptions{}); err != nil {
540+
recorder.Warningf("AnnotationUpdateFailed", "Failed to update annotations on configmap kube-system/extension-apiserver-authentication: %v", err)
541+
return err
542+
}
543+
return nil
544+
}
545+
546+
return nil
547+
}
548+
510549
func ensureLocalhostRecoverySAToken(ctx context.Context, client coreclientv1.CoreV1Interface, recorder events.Recorder) error {
511550
requiredSA := resourceread.ReadServiceAccountV1OrDie(bindata.MustAsset("assets/kube-apiserver/localhost-recovery-sa.yaml"))
512551
requiredToken := resourceread.ReadSecretV1OrDie(bindata.MustAsset("assets/kube-apiserver/localhost-recovery-token.yaml"))

pkg/operator/targetconfigcontroller/targetconfigcontroller_test.go

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,17 @@ import (
2626
"k8s.io/utils/clock"
2727

2828
"github.com/ghodss/yaml"
29+
"github.com/google/go-cmp/cmp"
2930
"github.com/openshift/api/annotations"
3031
kubecontrolplanev1 "github.com/openshift/api/kubecontrolplane/v1"
3132
operatorv1 "github.com/openshift/api/operator/v1"
33+
"github.com/openshift/cluster-kube-apiserver-operator/bindata"
3234
"github.com/openshift/cluster-kube-apiserver-operator/pkg/operator/operatorclient"
3335
"github.com/openshift/library-go/pkg/operator/events"
3436
"github.com/openshift/library-go/pkg/operator/resource/resourcemerge"
37+
"github.com/openshift/library-go/pkg/operator/resource/resourceread"
3538
"github.com/stretchr/testify/require"
39+
clientgotesting "k8s.io/client-go/testing"
3640
)
3741

3842
var codec = scheme.Codecs.LegacyCodec(scheme.Scheme.PrioritizedVersionsAllGroups()...)
@@ -1217,3 +1221,177 @@ func generateTemporaryCertificate() (certPEM []byte, err error) {
12171221

12181222
return certPEM, nil
12191223
}
1224+
1225+
// TestEnsureKubeAPIServerExtensionAuthenticationCA tests the behavior of ensureKubeAPIServerExtensionAuthenticationCA
1226+
func TestEnsureKubeAPIServerExtensionAuthenticationCA(t *testing.T) {
1227+
ctx := context.Background()
1228+
recorder := events.NewInMemoryRecorder("test", clock.RealClock{})
1229+
1230+
t.Run("configmap not found (Get error)", func(t *testing.T) {
1231+
// Create a fake client with no configmap in kube-system
1232+
client := fake.NewSimpleClientset()
1233+
err := ensureKubeAPIServerExtensionAuthenticationCA(ctx, client.CoreV1(), recorder)
1234+
if err != nil {
1235+
t.Fatalf("expected nil error when configmap is missing, got: %v", err)
1236+
}
1237+
})
1238+
1239+
t.Run("get failure (non-NotFound) returns error", func(t *testing.T) {
1240+
client := fake.NewSimpleClientset()
1241+
client.Fake.PrependReactor("get", "configmaps", func(action clientgotesting.Action) (bool, runtime.Object, error) {
1242+
return true, nil, fmt.Errorf("conflict")
1243+
})
1244+
err := ensureKubeAPIServerExtensionAuthenticationCA(ctx, client.CoreV1(), recorder)
1245+
if err == nil || !strings.Contains(err.Error(), "conflict") {
1246+
t.Fatalf("expected non-NotFound get error to propagate, got: %v", err)
1247+
}
1248+
})
1249+
1250+
t.Run("configmap exists but missing annotations, update succeeds", func(t *testing.T) {
1251+
// Create a configmap without annotations
1252+
cm := &corev1.ConfigMap{
1253+
ObjectMeta: metav1.ObjectMeta{
1254+
Name: "extension-apiserver-authentication",
1255+
Namespace: "kube-system",
1256+
},
1257+
}
1258+
client := fake.NewSimpleClientset(cm)
1259+
err := ensureKubeAPIServerExtensionAuthenticationCA(ctx, client.CoreV1(), recorder)
1260+
if err != nil {
1261+
t.Fatalf("expected nil error after update, got: %v", err)
1262+
}
1263+
updatedCM, err := client.CoreV1().ConfigMaps("kube-system").Get(ctx, "extension-apiserver-authentication", metav1.GetOptions{})
1264+
if err != nil {
1265+
t.Fatalf("failed to get updated configmap: %v", err)
1266+
}
1267+
if updatedCM.Annotations == nil || updatedCM.Annotations[annotations.OpenShiftComponent] != "kube-apiserver" {
1268+
t.Fatalf("expected annotation not set, got: %v", updatedCM.Annotations)
1269+
}
1270+
})
1271+
1272+
t.Run("configmap exists with correct annotations, no update needed", func(t *testing.T) {
1273+
required := resourceread.ReadConfigMapV1OrDie(bindata.MustAsset("assets/kube-apiserver/extension-apiserver-authentication-cm.yaml"))
1274+
1275+
// Create a configmap with the expected annotation already present
1276+
cm := &corev1.ConfigMap{
1277+
ObjectMeta: metav1.ObjectMeta{
1278+
Name: "extension-apiserver-authentication",
1279+
Namespace: "kube-system",
1280+
Annotations: required.Annotations,
1281+
},
1282+
}
1283+
client := fake.NewSimpleClientset(cm)
1284+
err := ensureKubeAPIServerExtensionAuthenticationCA(ctx, client.CoreV1(), recorder)
1285+
if err != nil {
1286+
t.Fatalf("expected nil error when annotations are already correct, got: %v", err)
1287+
}
1288+
1289+
// Check that client only did one action
1290+
if len(client.Actions()) != 1 {
1291+
t.Fatalf("expected one action, got: %v", client.Actions())
1292+
}
1293+
action := client.Actions()[0]
1294+
if action.GetVerb() != "get" {
1295+
t.Fatalf("expected get action, got: %v", action)
1296+
}
1297+
getAction := action.(clientgotesting.GetAction)
1298+
if getAction.GetName() != "extension-apiserver-authentication" {
1299+
t.Fatalf("expected get action for configmap 'extension-apiserver-authentication', got: %v", getAction)
1300+
}
1301+
if getAction.GetNamespace() != "kube-system" {
1302+
t.Fatalf("expected get action for namespace 'kube-system', got: %v", getAction)
1303+
}
1304+
})
1305+
1306+
t.Run("update failure propagates error", func(t *testing.T) {
1307+
// Create a configmap without annotations
1308+
cm := &corev1.ConfigMap{
1309+
ObjectMeta: metav1.ObjectMeta{
1310+
Name: "extension-apiserver-authentication",
1311+
Namespace: "kube-system",
1312+
},
1313+
}
1314+
client := fake.NewSimpleClientset(cm)
1315+
1316+
// Inject reactor to simulate update failure
1317+
client.Fake.PrependReactor("update", "configmaps", func(action clientgotesting.Action) (bool, runtime.Object, error) {
1318+
return true, nil, fmt.Errorf("simulated update failure")
1319+
})
1320+
1321+
err := ensureKubeAPIServerExtensionAuthenticationCA(ctx, client.CoreV1(), recorder)
1322+
if err == nil || !strings.Contains(err.Error(), "simulated update failure") {
1323+
t.Fatalf("expected update failure error, got: %v", err)
1324+
}
1325+
})
1326+
1327+
t.Run("unrelated annotations are not removed", func(t *testing.T) {
1328+
unrelatedAnnotations := map[string]string{
1329+
"unrelated.annotation/key1": "value1",
1330+
"unrelated.annotation/key2": "value2",
1331+
}
1332+
1333+
// Create a configmap with unrelated annotations
1334+
cm := &corev1.ConfigMap{
1335+
ObjectMeta: metav1.ObjectMeta{
1336+
Name: "extension-apiserver-authentication",
1337+
Namespace: "kube-system",
1338+
Annotations: unrelatedAnnotations,
1339+
},
1340+
}
1341+
client := fake.NewSimpleClientset(cm)
1342+
1343+
err := ensureKubeAPIServerExtensionAuthenticationCA(ctx, client.CoreV1(), recorder)
1344+
if err != nil {
1345+
t.Fatalf("expected nil error, got: %v", err)
1346+
}
1347+
1348+
updatedCM, err := client.CoreV1().ConfigMaps("kube-system").Get(ctx, "extension-apiserver-authentication", metav1.GetOptions{})
1349+
if err != nil {
1350+
t.Fatalf("failed to get updated configmap: %v", err)
1351+
}
1352+
1353+
required := resourceread.ReadConfigMapV1OrDie(bindata.MustAsset("assets/kube-apiserver/extension-apiserver-authentication-cm.yaml"))
1354+
1355+
expectedAnnotations := map[string]string{}
1356+
for k, v := range required.Annotations {
1357+
expectedAnnotations[k] = v
1358+
}
1359+
for k, v := range unrelatedAnnotations {
1360+
expectedAnnotations[k] = v
1361+
}
1362+
1363+
diff := cmp.Diff(expectedAnnotations, updatedCM.Annotations)
1364+
if diff != "" {
1365+
t.Fatalf("expected annotations to match, but got diff:\n%s", diff)
1366+
}
1367+
})
1368+
1369+
t.Run("configmap exists with incorrect OpenShiftComponent annotation, update succeeds", func(t *testing.T) {
1370+
// Create a configmap with an incorrect OpenShiftComponent annotation
1371+
cm := &corev1.ConfigMap{
1372+
ObjectMeta: metav1.ObjectMeta{
1373+
Name: "extension-apiserver-authentication",
1374+
Namespace: "kube-system",
1375+
Annotations: map[string]string{
1376+
annotations.OpenShiftComponent: "incorrect-value",
1377+
},
1378+
},
1379+
}
1380+
client := fake.NewSimpleClientset(cm)
1381+
1382+
err := ensureKubeAPIServerExtensionAuthenticationCA(ctx, client.CoreV1(), recorder)
1383+
if err != nil {
1384+
t.Fatalf("expected nil error, got: %v", err)
1385+
}
1386+
1387+
updatedCM, err := client.CoreV1().ConfigMaps("kube-system").Get(ctx, "extension-apiserver-authentication", metav1.GetOptions{})
1388+
if err != nil {
1389+
t.Fatalf("failed to get updated configmap: %v", err)
1390+
}
1391+
1392+
// Verify the OpenShiftComponent annotation is updated to the correct value
1393+
if updatedCM.Annotations == nil || updatedCM.Annotations[annotations.OpenShiftComponent] != "kube-apiserver" {
1394+
t.Errorf("expected annotation %s=kube-apiserver, got: %v", annotations.OpenShiftComponent, updatedCM.Annotations)
1395+
}
1396+
})
1397+
}

0 commit comments

Comments
 (0)