Skip to content

Commit 5f08691

Browse files
authored
Merge pull request #5305 from aramase/aramase/d/kep_3331_ga_v1.34
KEP-3331: update kep for GA in v1.34
2 parents c1a01d2 + 737cae0 commit 5f08691

File tree

3 files changed

+69
-34
lines changed

3 files changed

+69
-34
lines changed

keps/prod-readiness/sig-auth/3331.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,5 @@ alpha:
33
approver: "deads2k"
44
beta:
55
approver: "jpbetz"
6+
stable:
7+
approver: "jpbetz"

keps/sig-auth/3331-structured-authentication-configuration/README.md

Lines changed: 64 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,10 @@
2222
- [Beta](#beta)
2323
- [Pre-GA follow-up](#pre-ga-follow-up)
2424
- [GA](#ga)
25+
- [Possible future work](#possible-future-work)
2526
- [Upgrade / Downgrade Strategy](#upgrade--downgrade-strategy)
2627
- [Version Skew Strategy](#version-skew-strategy)
27-
- [Open Questions](#open-questions)
28+
- [Questions](#questions)
2829
- [Production Readiness Review Questionnaire](#production-readiness-review-questionnaire)
2930
- [Feature Enablement and Rollback](#feature-enablement-and-rollback)
3031
- [Rollout, Upgrade and Rollback Planning](#rollout-upgrade-and-rollback-planning)
@@ -163,6 +164,7 @@ jwt:
163164
```
164165
165166
The minimum valid JWT payload must contain the following claims:
167+
166168
```yaml
167169
{
168170
"iss": "https://example.com", // must match the issuer.url
@@ -680,13 +682,24 @@ providers such as Okta, Azure AD, etc:
680682
- The Kubespray team is integrating Structured Authentication in this [PR](https://github.com/kubernetes-sigs/kubespray/pull/11841).
681683
- Confirm with [Gardener](https://gardener.cloud/) that the new functionality addresses their use case.
682684
- The Gardener team has integrated Structured Authentication in this [PR](https://github.com/gardener/gardener/pull/10244).
683-
- Confirm with [SPIFFE](https://spiffe.io/) that the new functionality addresses their use case.
684-
- The SPIFFE team recommends Structured Authentication in their [README](https://github.com/spiffe/k8s-spiffe-workload-jwt-exec-auth?tab=readme-ov-file#setup-the-kubernetes-cluster-auth).
685-
- Add a full documentation with examples for the most popular providers, e.g., Okta, Dex, Auth0
685+
- Confirm with [SPIFFE](https://spiffe.io/) that the new functionality addresses their use case.
686+
- The SPIFFE team recommends Structured Authentication in their [README](https://github.com/spiffe/k8s-spiffe-workload-jwt-exec-auth?tab=readme-ov-file#setup-the-kubernetes-cluster-auth).
686687
- Migration guide
687-
- e2e test with an external provider completed and enabled
688-
- Get distributed claims working with CEL
689-
- Decide if we want to support egress selection configuration and how to do so
688+
- This is already done in the [beta blog post](https://kubernetes.io/blog/2024/04/25/structured-authentication-moves-to-beta/#migration-from-command-line-arguments-to-configuration-file).
689+
690+
#### Possible future work
691+
692+
- These tasks are not required for GA, but they are important to consider for the future of the feature. They will be added behind feature gates and will be fully backward compatible.
693+
- Get distributed claims working with CEL
694+
- can we implement a CEL type resolver so that a cel expression `claims.foo` gets resolved via a distributed claim the first time it is used?
695+
- this seems likely and preferable so we only resolve the things we need (in case an early validation rule fails and short-circuits).
696+
- Egress selection configuration
697+
- Better caching to support distributed claims in a more performant way
698+
- Should the token expiration cache know about the `exp` field instead of hard coding `10` seconds?
699+
- Remove 64 jwt authenticators limit
700+
- implementation detail: we should probably parse the `iss` claim out once.
701+
- Currently the structure of authenticators is a list of authenticators, but we could change it to a map of authenticators with issuer as the key.
702+
- Audit annotations set on claim/user validation failure
690703
691704
### Upgrade / Downgrade Strategy
692705
@@ -707,49 +720,58 @@ unset the `--authentication-config` flag and restore the `--oidc-*` flags to con
707720
This is an API server only change and does not affect other components. If the API server is
708721
not the minimum required version (v1.29), the feature will not be available.
709722
710-
<<[UNRESOLVED open questions that don't clearly fit elsewhere ]>>
711-
## Open Questions
712-
713-
The following questions are still open and need to be addressed or rejected or deferred before the KEP is marked as GA.
723+
## Questions
714724
715725
- should we have any revocation mechanism?
716726
=> use revocation endpoint if it is in the discovery document?
717727
=> related issue https://github.com/kubernetes/kubernetes/issues/71151
718-
- should audit annotations be set on validation failure?
728+
729+
> We don't have any plans to add revocation at this time. Because of this the docs will be updated to make sure the tokens are short-lived as they are not revocable.
730+
719731
- decide what error should be returned if CEL eval fails at runtime
720732
`500 Internal Sever Error` seem appropriate but authentication can only do `401`
733+
734+
> We always return `401 Unathorized` and log the error message. This is consistent with the existing OIDC authenticator behavior.
735+
721736
- distributed claims with fancier resolution requirements (such as access tokens as input)
722737
- This will be considered for getting distributed claims working with CEL
723-
- implementation detail: we should probably parse the `iss` claim out once
738+
739+
> This is suggesting a scenario where your distributed claims require an access token, but that token isn’t embedded in the ID token. Instead, the cluster admin configuring the system would somehow provide the access token. For example, there could be a client credentials plugin at the API server that fetches or refreshes an access token and makes it available for distributed claim fetching. We are not going to support this approach.
740+
724741
- are `iat` and `nbf` required?
742+
743+
> We have already documented the minimum required JWT payload. The `iat` and `nbf` claims are not required.
744+
725745
- is `sub` required or is the requirement to just have some username field?
746+
747+
> We have already documented the minimum required JWT payload. The `sub` claim is not required, but the username claim must be present.
748+
726749
- confirm cel comprehensions/mapping is powerful enough to transform the input claims into a filtered / transformed `map[string][]string` output for extra
727750

728-
For distributed claims:
751+
> We don't need to this because we changed the structure of our configuration of `ExtraMapping` to be a list of key and expression. So we don't need one CEL comprehension to do this anymore.
729752

730-
```json
731-
claims = {
732-
"foo":"bar",
733-
"foo.bar": "...",
734-
"true": "...",
735-
"_claim_names": {
736-
"groups": "group_source"
737-
},
738-
"_claim_sources": {
739-
"group_source": {"endpoint": "https://example.com/claim_source"}
740-
}
741-
}
742-
```
753+
For distributed claims:
754+
755+
```json
756+
claims = {
757+
"foo":"bar",
758+
"foo.bar": "...",
759+
"true": "...",
760+
"_claim_names": {
761+
"groups": "group_source"
762+
},
763+
"_claim_sources": {
764+
"group_source": {"endpoint": "https://example.com/claim_source"}
765+
}
766+
}
767+
```
743768

744-
- can we implement a CEL type resolver so that a cel expression `claims.foo` gets resolved via a distributed claim the first time it is used?
745-
- this seems likely and preferable so we only resolve the things we need (in case an early validation rule fails and short-circuits).
746-
- Decide behavior of the token cache on config reload
747-
- Should the token expiration cache know about the `exp` field instead of hard coding `10` seconds?
748-
- this requires awareness of key rotation to implement safely
749769
- For CEL expressions, do we want to safe guard access to fields that might not exist or stop existing at any moment?
750770
- Using `has()` to guard access to fields.
751771
- Could we do some kind of defaulting for fields that don't exist?
752772
773+
> We are not planning to do this. Users can use `OptionalTypes` in CEL to check if a field exists or not and to provide default values if they don't.
774+
753775
## Production Readiness Review Questionnaire
754776

755777
### Feature Enablement and Rollback
@@ -781,6 +803,16 @@ FeatureSpec{
781803
}
782804
```
783805

806+
**Stable**
807+
808+
```go
809+
FeatureSpec{
810+
Default: true,
811+
LockToDefault: true,
812+
PreRelease: featuregate.GA,
813+
}
814+
```
815+
784816
###### Does enabling the feature change any default behavior?
785817

786818
No. `AuthenticationConfiguration`is new in the v1.29 release. Furthermore, even with the feature enabled by default, the user needs to

keps/sig-auth/3331-structured-authentication-configuration/kep.yaml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,12 @@ approvers:
1111
- "@liggitt"
1212
creation-date: "2022-06-02"
1313
status: implementable
14-
stage: beta
15-
latest-milestone: "v1.33"
14+
stage: stable
15+
latest-milestone: "v1.34"
1616
milestone:
1717
alpha: "v1.29"
1818
beta: "v1.30"
19+
stable: "v1.34"
1920
feature-gates:
2021
- name: StructuredAuthenticationConfiguration
2122
components:

0 commit comments

Comments
 (0)