KEP-3331: update for beta in v1.30 #4461

aramase · 2024-01-31T00:57:40Z

Update KEP for beta in v1.30
Issue link: Structured Authentication Config #3331

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

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

aramase · 2024-02-05T17:44:51Z

/uncc kikisdeliveryservice
/cc liggitt
/assign enj deads2k

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

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

jpbetz · 2024-02-06T02:58:09Z

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

-
-[supported limits]: https://git.k8s.io/community//sig-scalability/configs-and-limits/thresholds.md
-->
+No. There would be very minimal addition to the memory used by the API Server and


Note the "Maybe" response to this question in #4456. Does the same logic apply here?

The "Maybe" response in #4456 is for Will enabling / using this feature result in increasing time taken by any operations covered by existing SLIs/SLOs?. Are you referring to that response for this KEP?

My mistake, I was comparing answers between these KEPs because they have similar behavior, but got mis-aligned here.

Signed-off-by: Anish Ramasekar <[email protected]>

aramase · 2024-02-06T00:04:48Z

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

-* Benchmarks are required to see how different CEL expressions affects authentication time.
-  * There will be a upper bound of 5s for the CEL expression evaluation.


This has been moved down to beta graduation criteria.

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

aramase · 2024-02-06T04:04:38Z

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

-
-[supported limits]: https://git.k8s.io/community//sig-scalability/configs-and-limits/thresholds.md
-->
+No. There would be very minimal addition to the memory used by the API Server and


The "Maybe" response in #4456 is for Will enabling / using this feature result in increasing time taken by any operations covered by existing SLIs/SLOs?. Are you referring to that response for this KEP?

stlaz

I believe there are way too many TODOs: in the "Design details" section for this to be moved to beta. The enhancement feels unfinished.

If you don't have answers for these items, group them in an "Open questions" subsection and remove the TODOs: from the text.

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

enj · 2024-02-06T17:04:15Z

I believe there are way too many TODOs: in the "Design details" section for this to be moved to beta. The enhancement feels unfinished.

If you don't have answers for these items, group them in an "Open questions" subsection and remove the TODOs: from the text.

While I agree that the TODOs can be organized in a better way, they are focused around pushing this feature to the limits of what is possible with the new config/CEL/reload/etc. Even if we never address those items, we would still make progress over what is possible today.

The actual community needs are:

Support multiple ClientIDs
Identify user by more JWT claims than just a single one
Allow any claim from an ID token to be mapped to user info extra attributes
Fail authentication unless user is a member of a specific set of groups
Required claims does not support arrays
Forward audiences to authorization via user info extra attributes

All of these are easily addressed by the KEP as-is, and the vast majority of code is shared with the existing OIDC flags with good integration test coverage.

Signed-off-by: Anish Ramasekar <[email protected]>

aramase · 2024-02-06T18:55:47Z

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

@@ -454,7 +379,6 @@ type JWTAuthenticator struct {
        // Claim must be a string or string array claim.
        // Expression must produce a string or string array value.
        // "", [], missing, and null values are treated as having no groups.
-        // TODO: investigate if you could make a single expression to construct groups from multiple claims. If not, maybe []PrefixedClaimOrExpression?


We have a unit test for this: https://github.com/kubernetes/kubernetes/blob/b693f09d544b17c82af4437bd93ea0b165e2622e/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc_test.go#L2268

aramase · 2024-02-06T19:23:46Z

/assign jpbetz

(for PRR)

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

Signed-off-by: Anish Ramasekar <[email protected]>

jpbetz · 2024-02-07T00:19:28Z

/approve
For PRR

stlaz · 2024-02-07T11:41:42Z

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

+<<[UNRESOLVED open questions that don't clearly fit elsewhere ]>>
+## Open Questions
+
+The following questions are still open and need to be addressed or rejected or deferred before the KEP is marked as GA.


This is much better, thank you 👍

enj · 2024-02-07T16:43:00Z

/lgtm
/approve

(Jordan is too busy to do a review pass, and IMO this is more than ready)

k8s-ci-robot · 2024-02-07T16:43:10Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aramase, enj, jpbetz

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~keps/prod-readiness/OWNERS~~ [jpbetz]
~~keps/sig-auth/OWNERS~~ [enj]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory labels Jan 31, 2024

k8s-ci-robot requested review from enj and kikisdeliveryservice January 31, 2024 00:57

k8s-ci-robot added the sig/auth Categorizes an issue or PR as relevant to SIG Auth. label Jan 31, 2024

aramase changed the title ~~KEP-3331: update for beta in v1.30~~ [WIP] KEP-3331: update for beta in v1.30 Jan 31, 2024

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Jan 31, 2024

aramase force-pushed the aramase/c/kep_3331_beta_update branch from a770bad to 6366dd0 Compare February 1, 2024 01:57

aramase changed the title ~~[WIP] KEP-3331: update for beta in v1.30~~ KEP-3331: update for beta in v1.30 Feb 1, 2024

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 1, 2024

enj reviewed Feb 1, 2024

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 2, 2024

aramase force-pushed the aramase/c/kep_3331_beta_update branch from 6366dd0 to 65d5f4e Compare February 5, 2024 08:56

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 5, 2024

aramase force-pushed the aramase/c/kep_3331_beta_update branch from 65d5f4e to 86bdeb8 Compare February 5, 2024 17:02

k8s-ci-robot assigned deads2k and enj Feb 5, 2024

k8s-ci-robot requested review from liggitt and removed request for kikisdeliveryservice February 5, 2024 17:44

aramase force-pushed the aramase/c/kep_3331_beta_update branch 3 times, most recently from 5e0a514 to 85700d9 Compare February 6, 2024 00:06

aramase requested a review from enj February 6, 2024 00:07

jpbetz mentioned this pull request Feb 6, 2024

Structured Authentication Config #3331

Open

51 tasks

jpbetz reviewed Feb 6, 2024

View reviewed changes

KEP-3331: update for beta in v1.30

151cfff

Signed-off-by: Anish Ramasekar <[email protected]>

aramase force-pushed the aramase/c/kep_3331_beta_update branch from 85700d9 to 151cfff Compare February 6, 2024 04:01

aramase commented Feb 6, 2024

View reviewed changes

stlaz suggested changes Feb 6, 2024

View reviewed changes

sgaist reviewed Feb 6, 2024

View reviewed changes

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

consolidate TODOs into open questions and add pre-ga follow-up

ab0b761

Signed-off-by: Anish Ramasekar <[email protected]>

aramase commented Feb 6, 2024

View reviewed changes

k8s-ci-robot assigned jpbetz Feb 6, 2024

enj reviewed Feb 6, 2024

View reviewed changes

aramase requested a review from enj February 6, 2024 21:15

aramase force-pushed the aramase/c/kep_3331_beta_update branch from e0d1604 to ef7d879 Compare February 6, 2024 21:24

review feedback

1e2e95b

Signed-off-by: Anish Ramasekar <[email protected]>

aramase force-pushed the aramase/c/kep_3331_beta_update branch from ef7d879 to 1e2e95b Compare February 6, 2024 22:11

AnaMMedina21 mentioned this pull request Feb 7, 2024

Structured Authorization Configuration #3221

Closed

29 tasks

stlaz reviewed Feb 7, 2024

View reviewed changes

enj added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Feb 7, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 7, 2024

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 7, 2024

k8s-ci-robot merged commit cd86e44 into kubernetes:master Feb 7, 2024

k8s-ci-robot added this to the v1.30 milestone Feb 7, 2024

aramase deleted the aramase/c/kep_3331_beta_update branch February 7, 2024 16:59

		* Benchmarks are required to see how different CEL expressions affects authentication time.
		* There will be a upper bound of 5s for the CEL expression evaluation.

KEP-3331: update for beta in v1.30 #4461

KEP-3331: update for beta in v1.30 #4461

Uh oh!

Conversation

aramase commented Jan 31, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aramase commented Feb 5, 2024

Uh oh!

Uh oh!

Uh oh!

jpbetz Feb 6, 2024

Choose a reason for hiding this comment

Uh oh!

aramase Feb 6, 2024

Choose a reason for hiding this comment

Uh oh!

jpbetz Feb 6, 2024

Choose a reason for hiding this comment

Uh oh!

aramase Feb 6, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aramase Feb 6, 2024

Choose a reason for hiding this comment

Uh oh!

stlaz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

enj commented Feb 6, 2024

Uh oh!

aramase Feb 6, 2024

Choose a reason for hiding this comment

Uh oh!

aramase commented Feb 6, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jpbetz commented Feb 7, 2024

Uh oh!

stlaz Feb 7, 2024

Choose a reason for hiding this comment

Uh oh!

enj commented Feb 7, 2024

Uh oh!

k8s-ci-robot commented Feb 7, 2024

Uh oh!

Uh oh!