Skip to content
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

Jump to bottom

Add docs for Structured Authn beta #45108

Merged
merged 1 commit into from
Mar 24, 2024
Merged

Add docs for Structured Authn beta #45108

merged 1 commit into from
Mar 24, 2024

Conversation

aramase
Copy link
Member

@aramase aramase commented Feb 12, 2024 •
edited
Loading

@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 12, 2024
@netlify Netlify
Copy link

netlify bot commented Feb 12, 2024 •
edited
Loading

👷 Deploy Preview for kubernetes-io-vnext-staging processing.

Name Link
🔨 Latest commit b35e434
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-vnext-staging/deploys/65f9fadfcfb6ef0008bcf7f0

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Feb 12, 2024
@Shubham82
Copy link
Contributor

/sig auth

@k8s-ci-robot k8s-ci-robot added the sig/auth Categorizes an issue or PR as relevant to SIG Auth. label Feb 13, 2024
@reylejano
Copy link
Member

/milestone 1.30

@k8s-ci-robot k8s-ci-robot added this to the 1.30 milestone Feb 16, 2024
@chanieljdan
Copy link
Contributor

Hi @aramase 👋 just a reminder to take a look at Documenting for a release - PR Ready for Review to get your PR ready for review before Tuesday March 12th 2024 18:00 PST.

Thank you!

@enj
Copy link
Member

enj commented Mar 11, 2024

Lets make sure this PR addresses #45140

@enj enj mentioned this pull request Mar 11, 2024
Open
41 tasks
@enj
Copy link
Member

enj commented Mar 11, 2024 •
edited
Loading

Also lets make sure to document any limitations (the main one that comes to mind is that distributed claims do not work via CEL expressions).

Edit: also, the lack of egress selection support.

@aramase aramase force-pushed the aramase/d/kep_3331_beta_docs branch from 1a9c88d to 34cdc2b Compare March 12, 2024 22:59
@aramase aramase marked this pull request as ready for review March 12, 2024 22:59
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 12, 2024
@k8s-ci-robot k8s-ci-robot requested review from deads2k and liggitt March 12, 2024 22:59
@aramase aramase force-pushed the aramase/d/kep_3331_beta_docs branch 2 times, most recently from 12063b1 to 734a7df Compare March 12, 2024 23:01
@aramase
Copy link
Member Author

aramase commented Mar 12, 2024

/assign enj

@aramase aramase force-pushed the aramase/d/kep_3331_beta_docs branch from 734a7df to 05e5342 Compare March 13, 2024 07:21
@aramase
Copy link
Member Author

aramase commented Mar 13, 2024

/assign liggitt

liggitt
liggitt reviewed Mar 14, 2024
View reviewed changes
Copy link
Member

@liggitt liggitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did a quick sweep, would like lgtm from @enj

@aramase aramase force-pushed the aramase/d/kep_3331_beta_docs branch from 05e5342 to bc73c18 Compare March 14, 2024 18:02
enj
enj suggested changes Mar 15, 2024
View reviewed changes
Copy link
Member

@enj enj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments.

sftim
sftim reviewed Mar 18, 2024
View reviewed changes
Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

On line 314 there is a heading, #### Configuring the API Server.

For beta, please hoist that up to be a 3rd level heading; I might change it to “Cluster-level authentication configuration”. You might need to do some light rewording; it's OK to do the minimum needed to get this PR to merge, and to file an issue about any follow-up tidying that you'd like to recommend.

Using a 3rd level heading allows the heading to show up in in the page navigation; deeper headings don't show up there.

@drewhagen
Copy link
Member

Hello @aramase 👋, it looks like you've already engaged feedback in review of this PR. Great job! Keep this up. 🚀

@liggitt When you have a moment, can you please give a review for technical accuracy? A friendly reminder that this PR needs doc review complete by Doc Freeze on March 26th 18:00 PT to get this into the release.

Thanks!

@liggitt
Copy link
Member

liggitt commented Mar 18, 2024

@liggitt When you have a moment, can you please give a review for technical accuracy?

I deferred to @enj in #45108 (review), looks like he has some review comments outstanding from #45108 (review)

@aramase aramase force-pushed the aramase/d/kep_3331_beta_docs branch 3 times, most recently from 5e45136 to 11191e9 Compare March 19, 2024 19:08
@aramase
Add docs for Structured Authn beta
b35e434 
Signed-off-by: Anish Ramasekar <anish.ramasekar@gmail.com>
@aramase aramase force-pushed the aramase/d/kep_3331_beta_docs branch from 11191e9 to b35e434 Compare March 19, 2024 20:51
@aramase
Copy link
Member Author

aramase commented Mar 19, 2024 •
edited
Loading

For beta, please hoist that up to be a 3rd level heading; I might change it to “Cluster-level authentication configuration”. You might need to do some light rewording; it's OK to do the minimum needed to get this PR to merge, and to file an issue about any follow-up tidying that you'd like to recommend.

### OpenID Connect Tokens is a 3rd level heading and currently Authentication Configuration only supports configuring JWT authenticators (authenticators of this type). If we hoist it up to 3rd level heading, it'll become its own Authentication Strategy which is not what we intend?

@aramase aramase requested review from sftim and enj March 19, 2024 20:55
@enj
Copy link
Member

enj commented Mar 21, 2024

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 21, 2024
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 2cd6fb7c8a9a602bda4b89420b79b2da35fed348

@chanieljdan
Copy link
Contributor

Hi @kubernetes/sig-docs-en-owners 👋. Technical review for this PR is complete, just need an approval from a member of sig-docs-en-owner, does this look ready for approval? Thanks in advance! 😄

natalisucks
natalisucks reviewed Mar 24, 2024
View reviewed changes
content/en/docs/reference/access-authn-authz/authentication.md

{{< note >}}
When the feature is enabled, setting both `--authentication-config` and any of the `--oidc-*` flags will result in an error. If you want to use the feature, you have to remove the `--oidc-*` flags and use the configuration file instead.
If you specify `--authentication-config` along with any of the `--oidc-*` command line arguments, this is
a misconfiguration. In this situation, the API server reports an errors and then immediately exits.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: error should be singular here

natalisucks
natalisucks approved these changes Mar 24, 2024
View reviewed changes
Copy link
Contributor

@natalisucks natalisucks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve

I agree with @sftim that any additional changes can be made in a follow-up PR so that we can ensure this makes the Docs Freeze deadline. Thanks @aramase, @enj, and @liggitt.

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: enj, natalisucks

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:

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 k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 24, 2024
@k8s-ci-robot k8s-ci-robot merged commit 864ac8b into kubernetes:dev-1.30 Mar 24, 2024
6 checks passed
@aramase aramase deleted the aramase/d/kep_3331_beta_docs branch March 24, 2024 16:15
This was referenced Mar 24, 2024
Closed
Merged
Closed
@enj enj mentioned this pull request Apr 8, 2024
Closed
5 tasks
@drewhagen
Copy link
Member

/milestone 1.30

@k8s-ci-robot k8s-ci-robot added this to the 1.30 milestone Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liggitt liggitt

@natalisucks natalisucks

@deads2k deads2k

@sftim sftim

@enj enj

Assignees

@liggitt liggitt

@enj enj

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
SIG Auth
Archived in project
Milestone
1.30
Development

Successfully merging this pull request may close these issues.
10 participants
@aramase @Shubham82 @reylejano @chanieljdan @enj @drewhagen @liggitt @k8s-ci-robot @natalisucks @sftim