Always set service-account-jwks-uri to LB URL even with custom issuer

Signed-off-by: Shaza Aldawamneh <[email protected]>

Author: ShazaAldawamneh

pkg/operator/configobservation/auth/auth_serviceaccountissuer.go

@@ -128,16 +128,14 @@ func observedConfig(existingConfig map[string]interface{},
 	// If the issuer is not set in KAS, we rely on the config-overrides.yaml to set both
 	// the issuer and the api-audiences but configure the jwks-uri to point to
 	// the LB so that it does not default to KAS IP which is not included in the serving certs
-	if observedActiveIssuer == defaultServiceAccountIssuerValue {
-		infrastructureConfig, err := getInfrastructureConfig("cluster")
-		if err != nil {
-			return existingConfig, append(errs, err)
-		}
-		if apiServerExternalURL := infrastructureConfig.Status.APIServerURL; len(apiServerExternalURL) == 0 {
-			return existingConfig, append(errs, fmt.Errorf("APIServerURL missing from infrastructure/cluster"))
-		} else {
-			apiServerArguments["service-account-jwks-uri"] = []interface{}{apiServerExternalURL + "/openid/v1/jwks"}
-		}
+	infrastructureConfig, err := getInfrastructureConfig("cluster")
+	if err != nil {
+		return existingConfig, append(errs, err)
+	}
+	if apiServerExternalURL := infrastructureConfig.Status.APIServerURL; len(apiServerExternalURL) == 0 {
+		return existingConfig, append(errs, fmt.Errorf("APIServerURL missing from infrastructure/cluster"))
+	} else {
+		apiServerArguments["service-account-jwks-uri"] = []interface{}{apiServerExternalURL + "/openid/v1/jwks"}
 	}
 
 	return map[string]interface{}{"apiServerArguments": apiServerArguments}, errs

pkg/operator/configobservation/auth/auth_serviceaccountissuer_test.go

@@ -54,11 +54,12 @@ func TestObservedConfig(t *testing.T) {
 			expectedChange:     true,
 		},
 		{
-			name:           "issuer set, no previous issuer",
-			existingIssuer: "",
-			issuer:         "https://example.com",
-			expectedIssuer: "https://example.com",
-			expectedChange: true,
+			name:               "issuer set, no previous issuer",
+			existingIssuer:     "",
+			issuer:             "https://example.com",
+			expectedIssuer:     "https://example.com",
+			expectInternalJWKI: true,
+			expectedChange:     true,
 		},
 		{
 			name:                   "previous issuer was default, new is custom value",
@@ -67,14 +68,15 @@ func TestObservedConfig(t *testing.T) {
 			expectedIssuer:         "https://example.com",
 			trustedIssuers:         []string{defaultServiceAccountIssuerValue},
 			expectedTrustedIssuers: []string{defaultServiceAccountIssuerValue},
-			expectInternalJWKI:     false, // this proves we remove the internal api LB when custom value is set
+			expectInternalJWKI:     true, // now jwks-uri should always point to LB URL
 			expectedChange:         true,
 		},
 		{
-			name:           "issuer set, previous issuer same",
-			existingIssuer: "https://example.com",
-			issuer:         "https://example.com",
-			expectedIssuer: "https://example.com",
+			name:               "issuer set, previous issuer same",
+			existingIssuer:     "https://example.com",
+			issuer:             "https://example.com",
+			expectedIssuer:     "https://example.com",
+			expectInternalJWKI: true,
 		},
 		{
 			name:                   "issuer set, previous issuer and trusted issuers same",
@@ -83,20 +85,23 @@ func TestObservedConfig(t *testing.T) {
 			trustedIssuers:         []string{"https://trusted.example.com"},
 			expectedIssuer:         "https://example.com",
 			expectedTrustedIssuers: []string{"https://trusted.example.com"},
+			expectInternalJWKI:     true,
 		},
 		{
-			name:           "issuer set, previous issuer different",
-			existingIssuer: "https://example.com",
-			issuer:         "https://example2.com",
-			expectedIssuer: "https://example2.com",
-			expectedChange: true,
+			name:               "issuer set, previous issuer different",
+			existingIssuer:     "https://example.com",
+			issuer:             "https://example2.com",
+			expectedIssuer:     "https://example2.com",
+			expectInternalJWKI: true,
+			expectedChange:     true,
 		},
 		{
-			name:           "auth getter error",
-			existingIssuer: "https://example2.com",
-			issuer:         "https://example.com",
-			authError:      expectedErrAuth,
-			expectedIssuer: "https://example2.com",
+			name:               "auth getter error",
+			existingIssuer:     "https://example2.com",
+			issuer:             "https://example.com",
+			authError:          expectedErrAuth,
+			expectedIssuer:     "https://example2.com",
+			expectInternalJWKI: true,
 		},
 		{
 			name:               "infra getter error",
@@ -106,6 +111,14 @@ func TestObservedConfig(t *testing.T) {
 			expectedIssuer:     defaultServiceAccountIssuerValue,
 			expectInternalJWKI: true,
 		},
+		{
+			name:               "custom issuer, no previous issuer",
+			existingIssuer:     "",
+			issuer:             "https://custom.com",
+			expectedIssuer:     "https://custom.com",
+			expectInternalJWKI: true, // should always set jwks-uri
+			expectedChange:     true,
+		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			testRecorder := events.NewInMemoryRecorder("SAIssuerTest", clock.RealClock{})
@@ -199,11 +212,9 @@ func apiConfigForIssuer(issuer string, trustedIssuers []string) *kubecontrolplan
 		"service-account-issuer": append([]string{issuer}, trustedIssuers...),
 		"api-audiences":          append([]string{issuer}, trustedIssuers...),
 	}
-	if issuer == defaultServiceAccountIssuerValue {
-		//delete(args, "service-account-issuer")
-		//delete(args, "api-audiences")
-		args["service-account-jwks-uri"] = kubecontrolplanev1.Arguments{testLBURI}
-	}
+	//delete(args, "service-account-issuer")
+	//delete(args, "api-audiences")
+	args["service-account-jwks-uri"] = kubecontrolplanev1.Arguments{testLBURI}
 
 	return &kubecontrolplanev1.KubeAPIServerConfig{
 		TypeMeta: metav1.TypeMeta{