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

Client Exec Proxy #2693

Merged
merged 1 commit into from
Oct 7, 2022
Merged

Client Exec Proxy #2693

merged 1 commit into from
Oct 7, 2022

Conversation

nckturner
Copy link
Contributor

A KEP to propose a client-side proxy for a number of client authentication use cases.

@k8s-ci-robot 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 May 7, 2021
@k8s-ci-robot k8s-ci-robot added sig/auth Categorizes an issue or PR as relevant to SIG Auth. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels May 7, 2021
@nckturner
Copy link
Contributor Author

/cc @micahhausler

enj
enj requested changes May 10, 2021
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.

Please fill out the other sections.

We probably need to schedule some time to hash out the finer details of this API.

keps/sig-auth/NNNN-20201209-client-exec-proxy/kep.yaml Outdated Show resolved Hide resolved
keps/sig-auth/NNNN-20201209-client-exec-proxy/kep.yaml Outdated Show resolved Hide resolved
keps/sig-auth/NNNN-20201209-client-exec-proxy/kep.yaml Outdated Show resolved Hide resolved
keps/sig-auth/NNNN-20201209-client-exec-proxy/kep.yaml Outdated Show resolved Hide resolved
keps/sig-auth/NNNN-20201209-client-exec-proxy/kep.yaml Outdated Show resolved Hide resolved
keps/sig-auth/NNNN-20201209-client-exec-proxy/README.md Outdated Show resolved Hide resolved
keps/sig-auth/NNNN-20201209-client-exec-proxy/README.md Outdated Show resolved Hide resolved
keps/sig-auth/NNNN-20201209-client-exec-proxy/README.md Outdated Show resolved Hide resolved
keps/sig-auth/NNNN-20201209-client-exec-proxy/README.md Outdated Show resolved Hide resolved
keps/sig-auth/NNNN-20201209-client-exec-proxy/README.md Outdated Show resolved Hide resolved
@nckturner nckturner mentioned this pull request May 11, 2021
Closed
4 tasks
@enj enj self-assigned this May 17, 2021
@nckturner nckturner force-pushed the request-signing branch 7 times, most recently from 42a702b to 904f46f Compare July 22, 2021 23:20
@mikedanese
Copy link
Member

Thanks. The problem/motivation make sense to me and the approach is reasonable. Comments are mostly seeking clarification and additional specification in some areas.

@nckturner
Copy link
Contributor Author

/remove-lifecycle rotten

@k8s-ci-robot k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Jun 22, 2022
@nckturner
Copy link
Contributor Author

@enj Updated with your feedback.

enj
enj reviewed Sep 28, 2022
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.

Still need to review some of the later parts of the KEP doc.

keps/sig-auth/2718-20210511-client-exec-proxy/kep.yaml Outdated Show resolved Hide resolved
keps/sig-auth/2718-20210511-client-exec-proxy/README.md Outdated Show resolved Hide resolved
keps/sig-auth/2718-20210511-client-exec-proxy/README.md Outdated Show resolved Hide resolved
keps/sig-auth/2718-20210511-client-exec-proxy/README.md Show resolved Hide resolved
keps/sig-auth/2718-20210511-client-exec-proxy/README.md Outdated Show resolved Hide resolved
keps/sig-auth/2718-20210511-client-exec-proxy/README.md Show resolved Hide resolved
keps/sig-auth/2718-20210511-client-exec-proxy/README.md Outdated Show resolved Hide resolved
keps/sig-auth/2718-20210511-client-exec-proxy/README.md Outdated Show resolved Hide resolved
keps/sig-auth/2718-20210511-client-exec-proxy/README.md Outdated
Comment on lines 333 to 335
The client will authenticate to its proxy using mTLS. On exec, the proxy will
pass back client key and certificate authority data for the client to use for
authentication when establishing a connection.
Copy link
Member

Choose a reason for hiding this comment

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

Some details on the implementation may be interesting here. For example, I think we could us Go's reverse proxy code to implement this.

keps/sig-auth/2718-20210511-client-exec-proxy/README.md Outdated Show resolved Hide resolved
mikedanese
mikedanese reviewed Oct 3, 2022
View reviewed changes
keps/sig-auth/2718-20210511-client-exec-proxy/README.md Outdated Show resolved Hide resolved
keps/sig-auth/2718-20210511-client-exec-proxy/README.md Show resolved Hide resolved
keps/sig-auth/2718-20210511-client-exec-proxy/README.md Show resolved Hide resolved
keps/sig-auth/2718-20210511-client-exec-proxy/README.md Show resolved Hide resolved
keps/sig-auth/2718-20210511-client-exec-proxy/README.md Outdated Show resolved Hide resolved
keps/sig-auth/2718-20210511-client-exec-proxy/README.md Outdated Show resolved Hide resolved
keps/sig-auth/2718-20210511-client-exec-proxy/README.md Show resolved Hide resolved
keps/sig-auth/2718-20210511-client-exec-proxy/README.md Outdated Show resolved Hide resolved
keps/sig-auth/2718-20210511-client-exec-proxy/README.md Outdated Show resolved Hide resolved
johnbelamaric
johnbelamaric reviewed Oct 4, 2022
View reviewed changes
Copy link
Member

@johnbelamaric johnbelamaric 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 but looks pretty good from a PRR perspective.

keps/prod-readiness/sig-auth/2718.yaml Outdated Show resolved Hide resolved
keps/sig-auth/2718-20210511-client-exec-proxy/README.md Outdated Show resolved Hide resolved
keps/sig-auth/2718-20210511-client-exec-proxy/README.md Outdated Show resolved Hide resolved
@johnbelamaric
Copy link
Member

Ok, well, I am just waiting for SIG approval - PRR is fine. Deadline is close...

johnbelamaric
johnbelamaric reviewed Oct 6, 2022
View reviewed changes
keps/sig-auth/2718-20210511-client-exec-proxy/README.md
- Metric name:
- [Optional] Aggregation method:
- Components exposing the metric:
- Metric name: `rest_client_exec_plugin_with_proxy_call_total`
Copy link
Member

Choose a reason for hiding this comment

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

Is that just a count? can we differentiate errors? (non-blocking question)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we will be able to differentiate errors, the implementation follow the example of: https://github.com/kubernetes/client-go/blob/d3b97581d2e22eef0f02a6667e19dfc193df7a3e/plugin/pkg/client/auth/exec/metrics.go

@nckturner
Client Exec Proxy (kubernetes#2718)
b08382a 
A KEP to propose a client-side proxy for a number of client
authentication use cases.
@enj
Copy link
Member

enj commented Oct 6, 2022

/lgtm
/approve

This is close enough for us to figure out the final details during the implementation.

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 6, 2022
@mikedanese
Copy link
Member

/lgtm
/approve

@nckturner
Copy link
Contributor Author

/assign @johnbelamaric

@johnbelamaric
Copy link
Member

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: enj, johnbelamaric, mikedanese, nckturner

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 Oct 7, 2022
@k8s-ci-robot k8s-ci-robot merged commit e90cb8a into kubernetes:master Oct 7, 2022
@k8s-ci-robot k8s-ci-robot added this to the v1.26 milestone Oct 7, 2022
@nckturner nckturner mentioned this pull request Feb 1, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mikedanese mikedanese mikedanese left review comments

@rhockenbury rhockenbury rhockenbury left review comments

@enj enj enj requested changes

@deads2k deads2k deads2k left review comments

@zshihang zshihang zshihang left review comments

@johnbelamaric johnbelamaric johnbelamaric left review comments

@liggitt liggitt Awaiting requested review from liggitt

@micahhausler micahhausler Awaiting requested review from micahhausler

Assignees

@mikedanese mikedanese

@enj enj

@johnbelamaric johnbelamaric

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. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory 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. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
SIG Auth
Archived in project
Milestone
v1.26
Development

Successfully merging this pull request may close these issues.
9 participants
@nckturner @mikedanese @k8s-triage-robot @enj @johnbelamaric @k8s-ci-robot @rhockenbury @deads2k @zshihang