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

GEP: Client Certificate Validation for TLS terminating at the Gateway Listener #2080

Merged
merged 1 commit into from
Jul 20, 2023
Merged

GEP: Client Certificate Validation for TLS terminating at the Gateway Listener #2080

merged 1 commit into from
Jul 20, 2023

Conversation

arkodg
Copy link
Contributor

@arkodg arkodg commented May 31, 2023 •
edited
Loading

/kind gep

Fixes #91

@k8s-ci-robot k8s-ci-robot added kind/gep PRs related to Gateway Enhancement Proposal(GEP) do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels May 31, 2023
@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label May 31, 2023
@arkodg
Copy link
Contributor Author

arkodg commented May 31, 2023

will continue adding more content in the GEP once Ive received broad consensus on the goals & non goals

@robscott robscott mentioned this pull request Jun 12, 2023
Closed
frankbu
frankbu reviewed Jun 23, 2023
View reviewed changes
geps/gep-91.md
(Gateway Listener in this case) during a [TLS Handshake Protocol][], also commonly referred to as mutual TLS (mTLS).

## Goals
- Define an API field to specify the CA Certificate within the Gateway Listener configuration that can be used as a trusted anchor to validate the certificates presented by the client.
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't the existing certificateRefs field where the CA certificate would be specified? Is the idea that this GEP will add a field that can be used to specify something like a client validation cacert key in a cacertificateRefs secret, for example? Maybe adding an API section with a proposal would be helpful to start the discussion?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah you're right @frankbu , ive been advised to not define the HOW until the the maintainers have agreed on the goals here, so taking it one step at a time

Copy link
Contributor

Choose a reason for hiding this comment

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

fwiw, this goal of adding a way to config mTLS termination in a Gateway is valuable and very contained scope IMO.

Copy link
Member

Choose a reason for hiding this comment

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

Agree that feels like the inevitable placement here. Let's get this in and then we can work on the implementation details.

@robscott
Copy link
Member

Thanks @arkodg! Will give @shaneutt and @youngnick a bit more time to review this one, but don't think there's anything controversial here.

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: arkodg, robscott

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 Jun 23, 2023
@frankbu
Copy link
Contributor

frankbu commented Jul 19, 2023

@robscott @shaneutt @youngnick

Hey guys, since this is no more than a simple (I believe agreed to) problem statement that has been stagnant for a couple of months, can we either get this merged now or otherwise update it with an API proposal so we can start discussing the solution?

@argkodg do you have an API for this that you are waiting to propose?

@arkodg
Copy link
Contributor Author

arkodg commented Jul 19, 2023

yah @frankbu, I have an API in mind, that I plan on adding as a follow up PR

@robscott
Copy link
Member

Thanks for the ping on this one @frankbu! I don't think there's anything controversial in the GEP at this point. I've already approved, would like @shaneutt or @youngnick to sign off on this though.

/release-note-none

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jul 19, 2023
@youngnick
Copy link
Contributor

Yeah, this is pretty tightly scoped as written, so I'm supportive.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 20, 2023
@k8s-ci-robot k8s-ci-robot merged commit 88bb307 into kubernetes-sigs:main Jul 20, 2023
@arkodg arkodg deleted the gep-client-cert-validation-gateway branch July 20, 2023 05:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@frankbu frankbu frankbu left review comments

@robscott robscott Awaiting requested review from robscott

@youngnick youngnick Awaiting requested review from youngnick

Assignees

@youngnick youngnick

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/gep PRs related to Gateway Enhancement Proposal(GEP) lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

GEP: Client Certificate Verification for Gateway Listeners
5 participants
@arkodg @robscott @k8s-ci-robot @frankbu @youngnick