Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# NOTE: This asset defines the required annotations for the live
# kube-system/extension-apiserver-authentication ConfigMap. It is not
# applied directly; the operator reads these annotations and reconciles
# them on the existing ConfigMap created by kube-apiserver.
apiVersion: v1
kind: ConfigMap
metadata:
name: extension-apiserver-authentication
namespace: kube-system
annotations:
"openshift.io/owning-component": "kube-apiserver"
"openshift.io/description": "CA holding the root certificate bundle used to verify client certificates on incoming requests"
42 changes: 42 additions & 0 deletions pkg/operator/targetconfigcontroller/targetconfigcontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,11 @@ func createTargetConfig(ctx context.Context, c TargetConfigController, recorder
errors = append(errors, fmt.Errorf("%q: %v", "configmap/trusted-ca-bundle", err))
}

err = ensureKubeAPIServerExtensionAuthenticationCA(ctx, c.kubeClient.CoreV1(), recorder)
Copy link
Contributor

@wangke19 wangke19 Sep 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new logic is not covered by unit tests. Adding a unit test for ensureKubeAPIServerExtensionAuthenticationCA would be beneficial to verify its behavior under different conditions
Example Test Cases to Add:

  1. ConfigMap does not exist: The function should do nothing and return no error.
  2. ConfigMap exists, no annotations: The function should add the required annotations.
  3. ConfigMap exists, partial/unrelated annotations: The function should add the required annotations without removing existing ones.
  4. ConfigMap exists, incorrect annotation value: The function should correct the value of the annotation.
  5. ConfigMap exists, all annotations correct: The function should do nothing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. This already exists - configmap not found (Get error)
  2. Done - configmap exists but missing annotations, update succeeds
  3. Added unrelated annotations are not removed
  4. There is one - configmap exists but missing annotations, update succeeds - which is almost matching, but added configmap exists with incorrect OpenShiftComponent annotation, update succeeds just to be sure
  5. This exists already - configmap exists with correct annotations, no update needed

if err != nil {
errors = append(errors, fmt.Errorf("%q: %v", "configmap/extension-apiserver-authentication", err))
}

err = ensureLocalhostRecoverySAToken(ctx, c.kubeClient.CoreV1(), recorder)
if err != nil {
errors = append(errors, fmt.Errorf("%q: %v", "serviceaccount/localhost-recovery-client", err))
Expand Down Expand Up @@ -507,6 +512,43 @@ func ensureKubeAPIServerTrustedCA(ctx context.Context, client coreclientv1.CoreV
return err
}

func ensureKubeAPIServerExtensionAuthenticationCA(ctx context.Context, client coreclientv1.CoreV1Interface, recorder events.Recorder) error {
required := resourceread.ReadConfigMapV1OrDie(bindata.MustAsset("assets/kube-apiserver/extension-apiserver-authentication-cm.yaml"))
cmClient := client.ConfigMaps(metav1.NamespaceSystem)

cm, err := cmClient.Get(ctx, "extension-apiserver-authentication", metav1.GetOptions{})
if err != nil {
// kube-apiserver creates this CM; don't degrade while waiting.
if apierrors.IsNotFound(err) {
return nil
}
return err
}

// Ensure that the config map is updated with the required annotations
modified := false
if cm.Annotations == nil {
cm.Annotations = make(map[string]string)
}

for key, expected := range required.Annotations {
if actual, ok := cm.Annotations[key]; !ok || actual != expected {
cm.Annotations[key] = expected
modified = true
}
}

if modified {
if _, err := cmClient.Update(ctx, cm, metav1.UpdateOptions{}); err != nil {
recorder.Warningf("AnnotationUpdateFailed", "Failed to update annotations on configmap kube-system/extension-apiserver-authentication: %v", err)
return err
}
return nil
}

return nil
}

func ensureLocalhostRecoverySAToken(ctx context.Context, client coreclientv1.CoreV1Interface, recorder events.Recorder) error {
requiredSA := resourceread.ReadServiceAccountV1OrDie(bindata.MustAsset("assets/kube-apiserver/localhost-recovery-sa.yaml"))
requiredToken := resourceread.ReadSecretV1OrDie(bindata.MustAsset("assets/kube-apiserver/localhost-recovery-token.yaml"))
Expand Down
178 changes: 178 additions & 0 deletions pkg/operator/targetconfigcontroller/targetconfigcontroller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,17 @@ import (
"k8s.io/utils/clock"

"github.com/ghodss/yaml"
"github.com/google/go-cmp/cmp"
"github.com/openshift/api/annotations"
kubecontrolplanev1 "github.com/openshift/api/kubecontrolplane/v1"
operatorv1 "github.com/openshift/api/operator/v1"
"github.com/openshift/cluster-kube-apiserver-operator/bindata"
"github.com/openshift/cluster-kube-apiserver-operator/pkg/operator/operatorclient"
"github.com/openshift/library-go/pkg/operator/events"
"github.com/openshift/library-go/pkg/operator/resource/resourcemerge"
"github.com/openshift/library-go/pkg/operator/resource/resourceread"
"github.com/stretchr/testify/require"
clientgotesting "k8s.io/client-go/testing"
)

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

return certPEM, nil
}

// TestEnsureKubeAPIServerExtensionAuthenticationCA tests the behavior of ensureKubeAPIServerExtensionAuthenticationCA
func TestEnsureKubeAPIServerExtensionAuthenticationCA(t *testing.T) {
ctx := context.Background()
recorder := events.NewInMemoryRecorder("test", clock.RealClock{})

t.Run("configmap not found (Get error)", func(t *testing.T) {
// Create a fake client with no configmap in kube-system
client := fake.NewSimpleClientset()
err := ensureKubeAPIServerExtensionAuthenticationCA(ctx, client.CoreV1(), recorder)
if err != nil {
t.Fatalf("expected nil error when configmap is missing, got: %v", err)
}
})

t.Run("get failure (non-NotFound) returns error", func(t *testing.T) {
client := fake.NewSimpleClientset()
client.Fake.PrependReactor("get", "configmaps", func(action clientgotesting.Action) (bool, runtime.Object, error) {
return true, nil, fmt.Errorf("conflict")
})
err := ensureKubeAPIServerExtensionAuthenticationCA(ctx, client.CoreV1(), recorder)
if err == nil || !strings.Contains(err.Error(), "conflict") {
t.Fatalf("expected non-NotFound get error to propagate, got: %v", err)
}
})

t.Run("configmap exists but missing annotations, update succeeds", func(t *testing.T) {
// Create a configmap without annotations
cm := &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "extension-apiserver-authentication",
Namespace: "kube-system",
},
}
client := fake.NewSimpleClientset(cm)
err := ensureKubeAPIServerExtensionAuthenticationCA(ctx, client.CoreV1(), recorder)
if err != nil {
t.Fatalf("expected nil error after update, got: %v", err)
}
updatedCM, err := client.CoreV1().ConfigMaps("kube-system").Get(ctx, "extension-apiserver-authentication", metav1.GetOptions{})
if err != nil {
t.Fatalf("failed to get updated configmap: %v", err)
}
if updatedCM.Annotations == nil || updatedCM.Annotations[annotations.OpenShiftComponent] != "kube-apiserver" {
t.Fatalf("expected annotation not set, got: %v", updatedCM.Annotations)
}
})

t.Run("configmap exists with correct annotations, no update needed", func(t *testing.T) {
required := resourceread.ReadConfigMapV1OrDie(bindata.MustAsset("assets/kube-apiserver/extension-apiserver-authentication-cm.yaml"))

// Create a configmap with the expected annotation already present
cm := &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "extension-apiserver-authentication",
Namespace: "kube-system",
Annotations: required.Annotations,
},
}
client := fake.NewSimpleClientset(cm)
err := ensureKubeAPIServerExtensionAuthenticationCA(ctx, client.CoreV1(), recorder)
if err != nil {
t.Fatalf("expected nil error when annotations are already correct, got: %v", err)
}

// Check that client only did one action
if len(client.Actions()) != 1 {
t.Fatalf("expected one action, got: %v", client.Actions())
}
action := client.Actions()[0]
if action.GetVerb() != "get" {
t.Fatalf("expected get action, got: %v", action)
}
getAction := action.(clientgotesting.GetAction)
if getAction.GetName() != "extension-apiserver-authentication" {
t.Fatalf("expected get action for configmap 'extension-apiserver-authentication', got: %v", getAction)
}
if getAction.GetNamespace() != "kube-system" {
t.Fatalf("expected get action for namespace 'kube-system', got: %v", getAction)
}
})

t.Run("update failure propagates error", func(t *testing.T) {
// Create a configmap without annotations
cm := &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "extension-apiserver-authentication",
Namespace: "kube-system",
},
}
client := fake.NewSimpleClientset(cm)

// Inject reactor to simulate update failure
client.Fake.PrependReactor("update", "configmaps", func(action clientgotesting.Action) (bool, runtime.Object, error) {
return true, nil, fmt.Errorf("simulated update failure")
})

err := ensureKubeAPIServerExtensionAuthenticationCA(ctx, client.CoreV1(), recorder)
if err == nil || !strings.Contains(err.Error(), "simulated update failure") {
t.Fatalf("expected update failure error, got: %v", err)
}
})

t.Run("unrelated annotations are not removed", func(t *testing.T) {
unrelatedAnnotations := map[string]string{
"unrelated.annotation/key1": "value1",
"unrelated.annotation/key2": "value2",
}

// Create a configmap with unrelated annotations
cm := &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "extension-apiserver-authentication",
Namespace: "kube-system",
Annotations: unrelatedAnnotations,
},
}
client := fake.NewSimpleClientset(cm)

err := ensureKubeAPIServerExtensionAuthenticationCA(ctx, client.CoreV1(), recorder)
if err != nil {
t.Fatalf("expected nil error, got: %v", err)
}

updatedCM, err := client.CoreV1().ConfigMaps("kube-system").Get(ctx, "extension-apiserver-authentication", metav1.GetOptions{})
if err != nil {
t.Fatalf("failed to get updated configmap: %v", err)
}

required := resourceread.ReadConfigMapV1OrDie(bindata.MustAsset("assets/kube-apiserver/extension-apiserver-authentication-cm.yaml"))

expectedAnnotations := map[string]string{}
for k, v := range required.Annotations {
expectedAnnotations[k] = v
}
for k, v := range unrelatedAnnotations {
expectedAnnotations[k] = v
}

diff := cmp.Diff(expectedAnnotations, updatedCM.Annotations)
if diff != "" {
t.Fatalf("expected annotations to match, but got diff:\n%s", diff)
}
})

t.Run("configmap exists with incorrect OpenShiftComponent annotation, update succeeds", func(t *testing.T) {
// Create a configmap with an incorrect OpenShiftComponent annotation
cm := &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "extension-apiserver-authentication",
Namespace: "kube-system",
Annotations: map[string]string{
annotations.OpenShiftComponent: "incorrect-value",
},
},
}
client := fake.NewSimpleClientset(cm)

err := ensureKubeAPIServerExtensionAuthenticationCA(ctx, client.CoreV1(), recorder)
if err != nil {
t.Fatalf("expected nil error, got: %v", err)
}

updatedCM, err := client.CoreV1().ConfigMaps("kube-system").Get(ctx, "extension-apiserver-authentication", metav1.GetOptions{})
if err != nil {
t.Fatalf("failed to get updated configmap: %v", err)
}

// Verify the OpenShiftComponent annotation is updated to the correct value
if updatedCM.Annotations == nil || updatedCM.Annotations[annotations.OpenShiftComponent] != "kube-apiserver" {
t.Errorf("expected annotation %s=kube-apiserver, got: %v", annotations.OpenShiftComponent, updatedCM.Annotations)
}
})
}