diff --git a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation.go b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation.go index 5e80e2dc69d..0b781e718dd 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation.go @@ -104,7 +104,7 @@ func validateJWTAuthenticator(authenticator api.JWTAuthenticator, fldPath *field compiler := authenticationcel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true)) state := &validationState{} - allErrs = append(allErrs, validateIssuer(authenticator.Issuer, disallowedIssuers, fldPath.Child("issuer"))...) + allErrs = append(allErrs, validateIssuer(authenticator.Issuer, disallowedIssuers, fldPath.Child("issuer"), structuredAuthnFeatureEnabled)...) allErrs = append(allErrs, validateClaimValidationRules(compiler, state, authenticator.ClaimValidationRules, fldPath.Child("claimValidationRules"), structuredAuthnFeatureEnabled)...) allErrs = append(allErrs, validateClaimMappings(compiler, state, authenticator.ClaimMappings, fldPath.Child("claimMappings"), structuredAuthnFeatureEnabled)...) allErrs = append(allErrs, validateUserValidationRules(compiler, state, authenticator.UserValidationRules, fldPath.Child("userValidationRules"), structuredAuthnFeatureEnabled)...) @@ -118,12 +118,12 @@ type validationState struct { usesEmailVerifiedClaim bool } -func validateIssuer(issuer api.Issuer, disallowedIssuers sets.Set[string], fldPath *field.Path) field.ErrorList { +func validateIssuer(issuer api.Issuer, disallowedIssuers sets.Set[string], fldPath *field.Path, structuredAuthnFeatureEnabled bool) field.ErrorList { var allErrs field.ErrorList allErrs = append(allErrs, validateIssuerURL(issuer.URL, disallowedIssuers, fldPath.Child("url"))...) - allErrs = append(allErrs, validateIssuerDiscoveryURL(issuer.URL, issuer.DiscoveryURL, fldPath.Child("discoveryURL"))...) - allErrs = append(allErrs, validateAudiences(issuer.Audiences, issuer.AudienceMatchPolicy, fldPath.Child("audiences"), fldPath.Child("audienceMatchPolicy"))...) + allErrs = append(allErrs, validateIssuerDiscoveryURL(issuer.URL, issuer.DiscoveryURL, fldPath.Child("discoveryURL"), structuredAuthnFeatureEnabled)...) + allErrs = append(allErrs, validateAudiences(issuer.Audiences, issuer.AudienceMatchPolicy, fldPath.Child("audiences"), fldPath.Child("audienceMatchPolicy"), structuredAuthnFeatureEnabled)...) allErrs = append(allErrs, validateCertificateAuthority(issuer.CertificateAuthority, fldPath.Child("certificateAuthority"))...) return allErrs @@ -137,13 +137,17 @@ func validateIssuerURL(issuerURL string, disallowedIssuers sets.Set[string], fld return validateURL(issuerURL, disallowedIssuers, fldPath) } -func validateIssuerDiscoveryURL(issuerURL, issuerDiscoveryURL string, fldPath *field.Path) field.ErrorList { +func validateIssuerDiscoveryURL(issuerURL, issuerDiscoveryURL string, fldPath *field.Path, structuredAuthnFeatureEnabled bool) field.ErrorList { var allErrs field.ErrorList if len(issuerDiscoveryURL) == 0 { return nil } + if !structuredAuthnFeatureEnabled { + allErrs = append(allErrs, field.Invalid(fldPath, issuerDiscoveryURL, "discoveryURL is not supported when StructuredAuthenticationConfiguration feature gate is disabled")) + } + if len(issuerURL) > 0 && strings.TrimRight(issuerURL, "/") == strings.TrimRight(issuerDiscoveryURL, "/") { allErrs = append(allErrs, field.Invalid(fldPath, issuerDiscoveryURL, "discoveryURL must be different from URL")) } @@ -181,7 +185,7 @@ func validateURL(issuerURL string, disallowedIssuers sets.Set[string], fldPath * return allErrs } -func validateAudiences(audiences []string, audienceMatchPolicy api.AudienceMatchPolicyType, fldPath, audienceMatchPolicyFldPath *field.Path) field.ErrorList { +func validateAudiences(audiences []string, audienceMatchPolicy api.AudienceMatchPolicyType, fldPath, audienceMatchPolicyFldPath *field.Path, structuredAuthnFeatureEnabled bool) field.ErrorList { var allErrs field.ErrorList if len(audiences) == 0 { @@ -189,6 +193,10 @@ func validateAudiences(audiences []string, audienceMatchPolicy api.AudienceMatch return allErrs } + if len(audiences) > 1 && !structuredAuthnFeatureEnabled { + allErrs = append(allErrs, field.Invalid(fldPath, audiences, "multiple audiences are not supported when StructuredAuthenticationConfiguration feature gate is disabled")) + } + seenAudiences := sets.NewString() for i, audience := range audiences { fldPath := fldPath.Index(i) diff --git a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation_test.go b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation_test.go index aa603b870d8..61778590bc8 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation_test.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation_test.go @@ -693,80 +693,98 @@ func TestValidateIssuerDiscoveryURL(t *testing.T) { fldPath := field.NewPath("issuer", "discoveryURL") testCases := []struct { - name string - in string - issuerURL string - want string + name string + in string + issuerURL string + want string + structuredAuthnFeatureEnabled bool }{ { - name: "url is empty", - in: "", - want: "", + name: "url is empty", + in: "", + want: "", + structuredAuthnFeatureEnabled: true, }, { - name: "url parse error", - in: "https://oidc.oidc-namespace.svc:invalid-port", - want: `issuer.discoveryURL: Invalid value: "https://oidc.oidc-namespace.svc:invalid-port": parse "https://oidc.oidc-namespace.svc:invalid-port": invalid port ":invalid-port" after host`, + name: "url parse error", + in: "https://oidc.oidc-namespace.svc:invalid-port", + want: `issuer.discoveryURL: Invalid value: "https://oidc.oidc-namespace.svc:invalid-port": parse "https://oidc.oidc-namespace.svc:invalid-port": invalid port ":invalid-port" after host`, + structuredAuthnFeatureEnabled: true, }, { - name: "url is not https", - in: "http://oidc.oidc-namespace.svc", - want: `issuer.discoveryURL: Invalid value: "http://oidc.oidc-namespace.svc": URL scheme must be https`, + name: "url is not https", + in: "http://oidc.oidc-namespace.svc", + want: `issuer.discoveryURL: Invalid value: "http://oidc.oidc-namespace.svc": URL scheme must be https`, + structuredAuthnFeatureEnabled: true, }, { - name: "url user info is not allowed", - in: "https://user:pass@oidc.oidc-namespace.svc", - want: `issuer.discoveryURL: Invalid value: "https://user:pass@oidc.oidc-namespace.svc": URL must not contain a username or password`, + name: "url user info is not allowed", + in: "https://user:pass@oidc.oidc-namespace.svc", + want: `issuer.discoveryURL: Invalid value: "https://user:pass@oidc.oidc-namespace.svc": URL must not contain a username or password`, + structuredAuthnFeatureEnabled: true, }, { - name: "url raw query is not allowed", - in: "https://oidc.oidc-namespace.svc?query", - want: `issuer.discoveryURL: Invalid value: "https://oidc.oidc-namespace.svc?query": URL must not contain a query`, + name: "url raw query is not allowed", + in: "https://oidc.oidc-namespace.svc?query", + want: `issuer.discoveryURL: Invalid value: "https://oidc.oidc-namespace.svc?query": URL must not contain a query`, + structuredAuthnFeatureEnabled: true, }, { - name: "url fragment is not allowed", - in: "https://oidc.oidc-namespace.svc#fragment", - want: `issuer.discoveryURL: Invalid value: "https://oidc.oidc-namespace.svc#fragment": URL must not contain a fragment`, + name: "url fragment is not allowed", + in: "https://oidc.oidc-namespace.svc#fragment", + want: `issuer.discoveryURL: Invalid value: "https://oidc.oidc-namespace.svc#fragment": URL must not contain a fragment`, + structuredAuthnFeatureEnabled: true, }, { - name: "valid url", + name: "valid url", + in: "https://oidc.oidc-namespace.svc", + want: "", + structuredAuthnFeatureEnabled: true, + }, + { + name: "valid url with path", + in: "https://oidc.oidc-namespace.svc/path", + want: "", + structuredAuthnFeatureEnabled: true, + }, + { + name: "discovery url same as issuer url", + issuerURL: "https://issuer-url", + in: "https://issuer-url", + want: `issuer.discoveryURL: Invalid value: "https://issuer-url": discoveryURL must be different from URL`, + structuredAuthnFeatureEnabled: true, + }, + { + name: "discovery url same as issuer url, with trailing slash", + issuerURL: "https://issuer-url", + in: "https://issuer-url/", + want: `issuer.discoveryURL: Invalid value: "https://issuer-url/": discoveryURL must be different from URL`, + structuredAuthnFeatureEnabled: true, + }, + { + name: "discovery url same as issuer url, with multiple trailing slashes", + issuerURL: "https://issuer-url", + in: "https://issuer-url///", + want: `issuer.discoveryURL: Invalid value: "https://issuer-url///": discoveryURL must be different from URL`, + structuredAuthnFeatureEnabled: true, + }, + { + name: "discovery url same as issuer url, issuer url with trailing slash", + issuerURL: "https://issuer-url/", + in: "https://issuer-url", + want: `issuer.discoveryURL: Invalid value: "https://issuer-url": discoveryURL must be different from URL`, + structuredAuthnFeatureEnabled: true, + }, + { + name: "discovery url set but structured authn feature disabled", in: "https://oidc.oidc-namespace.svc", - want: "", - }, - { - name: "valid url with path", - in: "https://oidc.oidc-namespace.svc/path", - want: "", - }, - { - name: "discovery url same as issuer url", - issuerURL: "https://issuer-url", - in: "https://issuer-url", - want: `issuer.discoveryURL: Invalid value: "https://issuer-url": discoveryURL must be different from URL`, - }, - { - name: "discovery url same as issuer url, with trailing slash", - issuerURL: "https://issuer-url", - in: "https://issuer-url/", - want: `issuer.discoveryURL: Invalid value: "https://issuer-url/": discoveryURL must be different from URL`, - }, - { - name: "discovery url same as issuer url, with multiple trailing slashes", - issuerURL: "https://issuer-url", - in: "https://issuer-url///", - want: `issuer.discoveryURL: Invalid value: "https://issuer-url///": discoveryURL must be different from URL`, - }, - { - name: "discovery url same as issuer url, issuer url with trailing slash", - issuerURL: "https://issuer-url/", - in: "https://issuer-url", - want: `issuer.discoveryURL: Invalid value: "https://issuer-url": discoveryURL must be different from URL`, + want: `issuer.discoveryURL: Invalid value: "https://oidc.oidc-namespace.svc": discoveryURL is not supported when StructuredAuthenticationConfiguration feature gate is disabled`, }, } for _, tt := range testCases { t.Run(tt.name, func(t *testing.T) { - got := validateIssuerDiscoveryURL(tt.issuerURL, tt.in, fldPath).ToAggregate() + got := validateIssuerDiscoveryURL(tt.issuerURL, tt.in, fldPath, tt.structuredAuthnFeatureEnabled).ToAggregate() if d := cmp.Diff(tt.want, errString(got)); d != "" { t.Fatalf("URL validation mismatch (-want +got):\n%s", d) } @@ -779,10 +797,11 @@ func TestValidateAudiences(t *testing.T) { audienceMatchPolicyFldPath := field.NewPath("issuer", "audienceMatchPolicy") testCases := []struct { - name string - in []string - matchPolicy string - want string + name string + in []string + matchPolicy string + want string + structuredAuthnFeatureEnabled bool }{ { name: "audiences is empty", @@ -812,27 +831,36 @@ func TestValidateAudiences(t *testing.T) { want: "", }, { - name: "duplicate audience", - in: []string{"audience", "audience"}, - matchPolicy: "MatchAny", - want: `issuer.audiences[1]: Duplicate value: "audience"`, + name: "duplicate audience", + in: []string{"audience", "audience"}, + matchPolicy: "MatchAny", + want: `issuer.audiences[1]: Duplicate value: "audience"`, + structuredAuthnFeatureEnabled: true, }, { - name: "match policy not set with multiple audiences", - in: []string{"audience1", "audience2"}, - want: `issuer.audienceMatchPolicy: Invalid value: "": audienceMatchPolicy must be MatchAny for multiple audiences`, + name: "match policy not set with multiple audiences", + in: []string{"audience1", "audience2"}, + want: `issuer.audienceMatchPolicy: Invalid value: "": audienceMatchPolicy must be MatchAny for multiple audiences`, + structuredAuthnFeatureEnabled: true, }, { - name: "valid multiple audiences", + name: "valid multiple audiences", + in: []string{"audience1", "audience2"}, + matchPolicy: "MatchAny", + want: "", + structuredAuthnFeatureEnabled: true, + }, + { + name: "multiple audiences set when structured authn feature is disabled", in: []string{"audience1", "audience2"}, matchPolicy: "MatchAny", - want: "", + want: `issuer.audiences: Invalid value: []string{"audience1", "audience2"}: multiple audiences are not supported when StructuredAuthenticationConfiguration feature gate is disabled`, }, } for _, tt := range testCases { t.Run(tt.name, func(t *testing.T) { - got := validateAudiences(tt.in, api.AudienceMatchPolicyType(tt.matchPolicy), fldPath, audienceMatchPolicyFldPath).ToAggregate() + got := validateAudiences(tt.in, api.AudienceMatchPolicyType(tt.matchPolicy), fldPath, audienceMatchPolicyFldPath, tt.structuredAuthnFeatureEnabled).ToAggregate() if d := cmp.Diff(tt.want, errString(got)); d != "" { t.Fatalf("Audiences validation mismatch (-want +got):\n%s", d) }