Skip to content

OCPBUGS-46086: Always set service-account-jwks-uri to LB URL even with custom issuer #1919

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-bot merged 1 commit into from
Mar 16, 2026
Merged

OCPBUGS-46086: Always set service-account-jwks-uri to LB URL even with custom issuer #1919

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
67 changes: 60 additions & 7 deletions pkg/operator/configobservation/auth/auth_serviceaccountissuer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,21 +125,74 @@ func observedConfig(existingConfig map[string]interface{},
"api-audiences": apiServerArgumentValue,
}

// 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 the issuer is not set in KAS, we rely on config-overrides.yaml to provide both
// the service-account-issuer and api-audiences. In that case, we still configure
// service-account-jwks-uri to point at the API server load balancer endpoint so we
// don’t fall back to the KAS IP, which is not included in the serving cert SANs.
//
// The service-account-jwks-uri is configured based on the issuer type:
//
// Default issuer:
// We set service-account-jwks-uri to the API server load balancer endpoint
// (<apiServerURL>/openid/v1/jwks) so clients don’t fall back to the KAS IP,
// which is not covered by the serving certificate SANs.
//
// Custom issuer:
// A custom issuer is expected to be an OIDC issuer (a URL). It should fall into
Comment thread
ShazaAldawamneh marked this conversation as resolved.
// one of two categories:
//
// 1) Issuer is the external cluster URL. In this case, the API server already
// serves both the discovery document and public keys at:
// - <issuer>/.well-known/openid-configuration
// - <issuer>/openid/v1/jwks
// This is effectively the same behavior as the default issuer case.
//
// 2) Issuer is any other URL. In this case, we expect the user to host both the
// OIDC discovery document and JWKS outside of the cluster at:
// - <issuer>/.well-known/openid-configuration
// - <issuer>/openid/v1/jwks
//
// Any other JWKS location would require a future enhancement (RFE) and is not
// currently supported.

if observedActiveIssuer == defaultServiceAccountIssuerValue {
// Default issuer → use API LB
infrastructureConfig, err := getInfrastructureConfig("cluster")
if err != nil {
return existingConfig, append(errs, err)
}
if apiServerExternalURL := infrastructureConfig.Status.APIServerURL; len(apiServerExternalURL) == 0 {

apiServerExternalURL := infrastructureConfig.Status.APIServerURL
if 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"}
}
}

apiServerArguments["service-account-jwks-uri"] = []interface{}{
apiServerExternalURL + "/openid/v1/jwks",
}
} else {
// Custom issuer → only set jwks-uri if the issuer is a valid HTTPS URL,
// as required by the OpenID Discovery 1.0 spec. If it's not, we skip setting
// the flag and log a warning. Per the kube-apiserver docs, a non-HTTPS issuer
// will also cause ServiceAccountIssuerDiscovery to be disabled.
parsed, parseErr := url.Parse(observedActiveIssuer)
switch {
case parseErr != nil:
klog.Warningf("custom service account issuer %q could not be parsed as a URL: %v; "+
"ServiceAccountIssuerDiscovery will be disabled and service-account-jwks-uri will not be configured. "+
"It is highly recommended that the issuer be a valid HTTPS URL per the OpenID Discovery spec.",
observedActiveIssuer, parseErr)
case parsed.Scheme != "https":
klog.Warningf("custom service account issuer %q has scheme %q instead of \"https\"; "+
"ServiceAccountIssuerDiscovery will be disabled and service-account-jwks-uri will not be configured. "+
"It is highly recommended that the issuer be a valid HTTPS URL per the OpenID Discovery spec.",
observedActiveIssuer, parsed.Scheme)
default:
apiServerArguments["service-account-jwks-uri"] = []interface{}{
observedActiveIssuer + "/openid/v1/jwks",
}
}
}
return map[string]interface{}{"apiServerArguments": apiServerArguments}, errs

}
Expand Down
152 changes: 91 additions & 61 deletions pkg/operator/configobservation/auth/auth_serviceaccountissuer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,87 +31,128 @@ func TestObservedConfig(t *testing.T) {
issuer string
trustedIssuers []string
existingIssuer string
existingJWKSURI string
authError error
infraError error
expectedIssuer string
expectedTrustedIssuers []string
expectedChange bool
expectInternalJWKI bool
expectedJWKSURI string
}{
{
name: "no issuer, no previous issuer means we default",
existingIssuer: "",
issuer: defaultServiceAccountIssuerValue,
expectedIssuer: defaultServiceAccountIssuerValue,
expectInternalJWKI: true,
name: "no issuer, no previous issuer means we default",
existingIssuer: "",
existingJWKSURI: "",
issuer: defaultServiceAccountIssuerValue,
expectedIssuer: defaultServiceAccountIssuerValue,
expectedJWKSURI: testLBURI,
},
{
name: "no issuer, previous issuer",
existingIssuer: "https://example.com",
issuer: defaultServiceAccountIssuerValue,
expectedIssuer: defaultServiceAccountIssuerValue,
expectInternalJWKI: true,
expectedChange: true,
name: "no issuer, previous issuer",
existingIssuer: "https://example.com",
existingJWKSURI: "",
issuer: defaultServiceAccountIssuerValue,
expectedIssuer: defaultServiceAccountIssuerValue,
expectedJWKSURI: testLBURI,
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: "",
existingJWKSURI: "",
issuer: "https://example.com",
expectedIssuer: "https://example.com",
expectedJWKSURI: "https://example.com/openid/v1/jwks",
expectedChange: true,
},
{
name: "previous issuer was default, new is custom value",
existingIssuer: defaultServiceAccountIssuerValue,
existingJWKSURI: testLBURI,
issuer: "https://example.com",
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
Comment thread
ShazaAldawamneh marked this conversation as resolved.
expectedJWKSURI: "https://example.com/openid/v1/jwks",
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",
existingJWKSURI: "https://example.com/openid/v1/jwks",
issuer: "https://example.com",
expectedIssuer: "https://example.com",
expectedJWKSURI: "https://example.com/openid/v1/jwks",
},
{
name: "issuer set, previous issuer and trusted issuers same",
existingIssuer: "https://example.com",
existingJWKSURI: "https://example.com/openid/v1/jwks",
issuer: "https://example.com",
trustedIssuers: []string{"https://trusted.example.com"},
expectedIssuer: "https://example.com",
expectedTrustedIssuers: []string{"https://trusted.example.com"},
expectedJWKSURI: "https://example.com/openid/v1/jwks",
},
{
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",
existingJWKSURI: "https://example.com/openid/v1/jwks",
issuer: "https://example2.com",
expectedIssuer: "https://example2.com",
expectedJWKSURI: "https://example2.com/openid/v1/jwks",
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",
existingJWKSURI: "https://example2.com/openid/v1/jwks",
issuer: "https://example.com",
authError: expectedErrAuth,
expectedIssuer: "https://example2.com",
expectedJWKSURI: "https://example2.com/openid/v1/jwks", // preserve existing
},
{
name: "infra getter error",
existingIssuer: defaultServiceAccountIssuerValue,
issuer: defaultServiceAccountIssuerValue,
infraError: expectedErrInfra,
expectedIssuer: defaultServiceAccountIssuerValue,
expectInternalJWKI: true,
name: "infra getter error",
existingIssuer: defaultServiceAccountIssuerValue,
existingJWKSURI: testLBURI,
issuer: defaultServiceAccountIssuerValue,
infraError: expectedErrInfra,
expectedIssuer: defaultServiceAccountIssuerValue,
expectedJWKSURI: testLBURI, // no previous + infra error -> do NOT set JWKS
},
{
name: "default issuer, no previous issuer, infra getter error",
existingIssuer: "",
existingJWKSURI: "",
issuer: defaultServiceAccountIssuerValue,
expectedIssuer: "",
infraError: expectedErrInfra,
expectedJWKSURI: "",
},
Comment thread
ShazaAldawamneh marked this conversation as resolved.
{
name: "custom issuer is not a URL, jwks-uri should be skipped",
existingIssuer: "",
existingJWKSURI: "",
issuer: "my-legacy-issuer",
expectedIssuer: "my-legacy-issuer",
expectedJWKSURI: "", // flag should be omitted, not set to "my-legacy-issuer/openid/v1/jwks"
expectedChange: true,
},
{
name: "custom issuer is http not https, jwks-uri should be skipped",
existingIssuer: "",
existingJWKSURI: "",
issuer: "http://example.com",
expectedIssuer: "http://example.com",
expectedJWKSURI: "", // kube-apiserver rejects non-https, so we skip the flag
expectedChange: true,
},
} {
t.Run(tc.name, func(t *testing.T) {
testRecorder := events.NewInMemoryRecorder("SAIssuerTest", clock.RealClock{})

newConfig, errs := observedConfig(
unstructuredAPIConfigForIssuer(t, tc.existingIssuer, tc.trustedIssuers),
unstructuredAPIConfigForIssuer(t, tc.existingIssuer, tc.trustedIssuers, tc.existingJWKSURI),
func(_ string) (*operatorv1.KubeAPIServer, error) {
return kasStatusForIssuer(tc.issuer, tc.trustedIssuers...), tc.authError
},
Expand All @@ -129,7 +170,7 @@ func TestObservedConfig(t *testing.T) {
if tc.authError == nil && tc.infraError == nil {
require.Len(t, errs, 0)
}
expectedConfig = apiConfigForIssuer(tc.expectedIssuer, tc.expectedTrustedIssuers)
expectedConfig = apiConfigForIssuer(tc.expectedIssuer, tc.expectedTrustedIssuers, tc.expectedJWKSURI)

// Check that errors are passed through
if tc.authError != nil {
Expand All @@ -151,19 +192,8 @@ func TestObservedConfig(t *testing.T) {
Kind: "KubeAPIServerConfig",
},
}

require.NoError(t, json.Unmarshal(jsonConfig, unmarshalledConfig))
uri, ok := unmarshalledConfig.APIServerArguments["service-account-jwks-uri"]
if tc.expectInternalJWKI {
if !ok {
t.Errorf("expected service-account-jwks-uri to be set, it is not")
} else {
require.Equal(t, uri, kubecontrolplanev1.Arguments{testLBURI})
}
}
if !tc.expectInternalJWKI && ok {
t.Errorf("expected no service-account-jwks-uri to be set, it is %+v", uri.String())
}
// Deep comparison of the entire configuration handles the JWKS URI check automatically.
require.Equal(t, expectedConfig, unmarshalledConfig, cmp.Diff(expectedConfig, unmarshalledConfig))
require.True(t, tc.expectedChange == (len(testRecorder.Events()) > 0))
})
Expand Down Expand Up @@ -194,17 +224,17 @@ func kasStatusForIssuer(active string, trustedIssuers ...string) *operatorv1.Kub
}
}

func apiConfigForIssuer(issuer string, trustedIssuers []string) *kubecontrolplanev1.KubeAPIServerConfig {
func apiConfigForIssuer(issuer string, trustedIssuers []string, jwksURI string) *kubecontrolplanev1.KubeAPIServerConfig {
args := map[string]kubecontrolplanev1.Arguments{
"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}
// Only include the JWKS URI if it is non-empty. This ensures that the
// service-account-jwks-uri flag is omitted from the configuration
// when not required, rather than being passed as an empty string.
if jwksURI != "" {
args["service-account-jwks-uri"] = kubecontrolplanev1.Arguments{jwksURI}
}

return &kubecontrolplanev1.KubeAPIServerConfig{
TypeMeta: metav1.TypeMeta{
Kind: "KubeAPIServerConfig",
Expand All @@ -216,8 +246,8 @@ func apiConfigForIssuer(issuer string, trustedIssuers []string) *kubecontrolplan
// unstructuredAPIConfigForIssuer round-trips through the golang type
// to ensure the input to the function under test will match what will
// be received at runtime.
func unstructuredAPIConfigForIssuer(t *testing.T, issuer string, trustedIssuers []string) map[string]interface{} {
config := apiConfigForIssuer(issuer, trustedIssuers)
func unstructuredAPIConfigForIssuer(t *testing.T, issuer string, trustedIssuers []string, jwksURI string) map[string]interface{} {
config := apiConfigForIssuer(issuer, trustedIssuers, jwksURI)
// Unmarshaling to unstructured requires explicitly setting kind
config.TypeMeta = metav1.TypeMeta{
Kind: "KubeAPIServerConfig",
Expand Down