Enable changing bound service account token issuer from default

pkg/operator/configobservation/auth/auth_serviceaccountissuer.go

@@ -0,0 +1,57 @@
+package auth
+
+import (
+	"fmt"
+	"net/url"
+
+	"k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/klog"
+
+	"github.com/openshift/cluster-kube-apiserver-operator/pkg/operator/configobservation"
+	"github.com/openshift/library-go/pkg/operator/configobserver"
+	"github.com/openshift/library-go/pkg/operator/events"
+)
+
+// ObserveServiceAccountIssuer changes apiServerArguments.service-account-issuer from the default
+// value if Authentication.Spec.ServiceAccountIssuer specifies a valid non-empty value.
+func ObserveServiceAccountIssuer(genericListers configobserver.Listers, recorder events.Recorder, existingConfig map[string]interface{}) (map[string]interface{}, []error) {
+	listers := genericListers.(configobservation.Listers)
+	errs := []error{}
+	prevObservedConfig := map[string]interface{}{}
+
+	topLevelMetadataFilePath := []string{"apiServerArguments", "service-account-issuer"}
+	currentIssuer, _, err := unstructured.NestedString(existingConfig, topLevelMetadataFilePath...)
+	if err != nil {
+		errs = append(errs, err)
+	}
+	if len(currentIssuer) > 0 {
+		if err := unstructured.SetNestedField(prevObservedConfig, currentIssuer, topLevelMetadataFilePath...); err != nil {
+			errs = append(errs, err)
+		}
+	}
+
+	observedConfig := map[string]interface{}{}
+	authConfig, err := listers.AuthConfigLister.Get("cluster")
+	if errors.IsNotFound(err) {
+		klog.Warningf("authentications.config.openshift.io/cluster: not found")
+		return observedConfig, errs
+	}
+	if err != nil {
+		errs = append(errs, err)
+		return prevObservedConfig, errs
+	}
+
+	newIssuer := authConfig.Spec.ServiceAccountIssuer
+	if len(newIssuer) > 0 {
+		if _, err := url.Parse(newIssuer); err != nil {
+			errs = append(errs, fmt.Errorf("spec.serviceAccountIssuer contained a ':' but was not a valid URL: %v", err))
+			return prevObservedConfig, errs
+		}
+	}
+	if err := unstructured.SetNestedField(observedConfig, newIssuer, topLevelMetadataFilePath...); err != nil {
+		errs = append(errs, err)
+	}
+
+	return observedConfig, errs
+}

pkg/operator/configobservation/configobservercontroller/observe_config_controller.go

@@ -93,6 +93,7 @@ func NewConfigObserver(
 			apiserver.ObserveAdditionalCORSAllowedOrigins,
 			libgoapiserver.ObserveTLSSecurityProfile,
 			auth.ObserveAuthMetadata,
+			auth.ObserveServiceAccountIssuer,
 			encryption.NewEncryptionConfigObserver(
 				operatorclient.TargetNamespace,
 				// static path at which we expect to find the encryption config secret

test/e2e/bound_sa_token_test.go

@@ -1,6 +1,8 @@
 package e2e
 
 import (
+	"bytes"
+	"encoding/json"
 	"reflect"
 	"testing"
 	"time"
@@ -12,6 +14,7 @@ import (
 	v1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/kubernetes"
 	clientcorev1 "k8s.io/client-go/kubernetes/typed/core/v1"
@@ -214,3 +217,82 @@ func TestTokenRequestAndReview(t *testing.T) {
 	require.Empty(t, trev.Status.Error)
 	require.True(t, trev.Status.Authenticated)
 }
+
+// TestChangeServiceAccountIssuer checks that the operator considers
+// the value set for Authentication.ServiceAccountIssuer when setting
+// the configuration configmap for the apiserver pods.
+func TestChangeServiceAccountIssuer(t *testing.T) {
+	kubeConfig, err := testlibrary.NewClientConfigForTest()
+	require.NoError(t, err)
+	kubeClient, err := kubernetes.NewForConfig(kubeConfig)
+	require.NoError(t, err)
+	coreClient := kubeClient.CoreV1()
+	configClient, err := configclient.NewForConfig(kubeConfig)
+	require.NoError(t, err)
+
+	// Wait for operator readiness
+	test.WaitForKubeAPIServerClusterOperatorAvailableNotProgressingNotDegraded(t, configClient)
+
+	defaultIssuer := "auth.openshift.io"
+
+	// Check that the default issuer is set in the operand config
+	require.NoError(t, pollForOperandIssuer(t, coreClient, defaultIssuer))
+
+	// Update the issuer to an invalid value (contains : but is not a url)
+	invalidIssuer := "foo:bar"
+	setServiceAccountIssuer(t, configClient, invalidIssuer)
+	// Check that the default issuer is not changed in the config
+	err = pollForOperandIssuer(t, coreClient, invalidIssuer)
+	require.NotNil(t, err)
+
+	nonDefaultIssuer := "https://my-custom-issuer.com"
+	// Update the issuer to a valid value
+	setServiceAccountIssuer(t, configClient, nonDefaultIssuer)
+	// Check that the issuer has changed to the non-default value
+	require.NoError(t, pollForOperandIssuer(t, coreClient, nonDefaultIssuer))
+
+	// Clear the issuer
+	setServiceAccountIssuer(t, configClient, "")
+	// Check that the issuer has changed back to the default
+	require.NoError(t, pollForOperandIssuer(t, coreClient, defaultIssuer))
+}
+
+func pollForOperandIssuer(t *testing.T, client clientcorev1.CoreV1Interface, expectedIssuer string) error {
+	return wait.PollImmediate(interval, regularTimeout, func() (done bool, err error) {
+		configMap, err := client.ConfigMaps(operatorclient.TargetNamespace).Get("config", metav1.GetOptions{})
+		if err != nil {
+			t.Errorf("failed to retrieve apiserver config configmap: %v", err)
+			return false, nil
+		}
+		// key has a .yaml extension but actual format is json
+		rawConfig := configMap.Data["config.yaml"]
+		if len(rawConfig) == 0 {
+			t.Logf("config.yaml is empty in apiserver config configmap")
+			return false, nil
+		}
+		config := map[string]interface{}{}
+		if err := json.NewDecoder(bytes.NewBuffer([]byte(rawConfig))).Decode(&config); err != nil {
+			t.Errorf("error parsing config, %v", err)
+			return false, nil
+		}
+		issuers, found, err := unstructured.NestedStringSlice(config, "apiServerArguments", "service-account-issuer")
+		if !found {
+			t.Log("apiServerArguments.service-account-issuer not found in config")
+			return false, nil
+		}
+		issuer := issuers[0]
+		if !found || expectedIssuer != issuer {
+			t.Logf("expected service account issuer to be %q, got %q", expectedIssuer, issuer)
+			return false, nil
+		}
+		return true, nil
+	})
+}
+
+func setServiceAccountIssuer(t *testing.T, client configclient.ConfigV1Interface, issuer string) {
+	auth, err := client.Authentications().Get("cluster", metav1.GetOptions{})
+	require.NoError(t, err)
+	auth.Spec.ServiceAccountIssuer = issuer
+	_, err = client.Authentications().Update(auth)
+	require.NoError(t, err)
+}