From 47ee8fff900b921a11aa220b6a2c81f8727947af Mon Sep 17 00:00:00 2001 From: Shaza Aldawamneh Date: Thu, 11 Sep 2025 15:10:18 +0200 Subject: [PATCH] Always set service-account-jwks-uri to LB URL even with custom issuer Signed-off-by: Shaza Aldawamneh --- .../auth/auth_serviceaccountissuer.go | 18 +++--- .../auth/auth_serviceaccountissuer_test.go | 61 +++++++++++-------- 2 files changed, 44 insertions(+), 35 deletions(-) diff --git a/pkg/operator/configobservation/auth/auth_serviceaccountissuer.go b/pkg/operator/configobservation/auth/auth_serviceaccountissuer.go index 79ac4afe9a..639bcbc03b 100644 --- a/pkg/operator/configobservation/auth/auth_serviceaccountissuer.go +++ b/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 diff --git a/pkg/operator/configobservation/auth/auth_serviceaccountissuer_test.go b/pkg/operator/configobservation/auth/auth_serviceaccountissuer_test.go index f82271090a..8e153754a1 100644 --- a/pkg/operator/configobservation/auth/auth_serviceaccountissuer_test.go +++ b/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{