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.

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

fix: allow grpcs and tls for client_certificate #9270

Closed

Conversation

shaneutt
Copy link
Contributor

Summary

A discussion in kong/kubernetes-ingress-controller wherein a user was trying to use mTLS with gRPC indicated a bug in the Kong Gateway where grpcs wasn't allowed to have a client_certificate, this PR attempts to fix that (and also add tls while we're at it for similar reasons).

Full changelog

  • Fixed a bug where grpcs and tls could not be used with client_certificate in services

Issue reference

Fix for Kong/kubernetes-ingress-controller#2831

@shaneutt
Copy link
Contributor Author

I could use some help in verifying this solution according to the problem that was seen in Kong/kubernetes-ingress-controller#2831.

@shaneutt shaneutt marked this pull request as ready for review September 7, 2022 14:00
@shaneutt shaneutt requested a review from a team as a code owner September 7, 2022 14:00
@shaneutt shaneutt marked this pull request as draft September 7, 2022 14:00
@shaneutt shaneutt marked this pull request as ready for review September 27, 2022 12:41
@hbagdi hbagdi requested a review from flrgh October 25, 2022 22:17
@flrgh flrgh force-pushed the shaneutt/grpc-client-cert-blocked branch from 1187be1 to e84ed94 Compare October 27, 2022 17:53
Copy link
Contributor

@flrgh flrgh left a comment

Choose a reason for hiding this comment

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

The schema change looks correct to me 👍 @shaneutt can you please rebase and add a tests for the grpc and tls use cases? These types of connections/requests are handled very differently from regular https requests within Kong (and nginx for that matter), so I'm not 100% certain they will work without requiring any other code changes.

@shaneutt
Copy link
Contributor Author

shaneutt commented Nov 9, 2022

Thanks @flrgh 👍

I'm going to temporarily close this, as in the time since I originally made this PR I've changed teams. The intention right now is that someone from my previous team will be picking this back up in the future.

@shaneutt shaneutt closed this Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants