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(gateway-api): verify TLS/TCPRoute is accepted before pushing to Gateway #3226

Merged
merged 7 commits into from
Dec 8, 2022

Conversation

czeslavo
Copy link
Contributor

@czeslavo czeslavo commented Dec 7, 2022

What this PR does / why we need it:

Fixes lack of verifying if TLS/TCPRoutes are accepted by the Gateway before proceeding with provisioning. It effectively enables GEP-957 port matching for them -- without that even when a Route doesn't match any Listener, it will be provisioned in Kong Gateway, which is incorrect.

It also improves populating ListenerStatus.SupportedKinds by deriving kinds from listener's protocol or AllowedRoutes.Kinds.

Which issue this PR fixes:

Part of #2709.

Special notes for your reviewer:

UDPRoute is left with no modifications due to a lack of possibility to specify two separate publish services for TCP and UDP (see #2544), which is blocking passing the existing integration tests. The default "publish service" (ingress-controller-kong-proxy LoadBalancer) exposes all its ports as TCP ones (including the 9999 which is meant to be UDP). Due to that, it falls into this case where Listener is marked as not ready due to a missing UDP port.

We may decide to:

PR Readiness Checklist:

Complete these before marking the PR as ready to review:

  • the CHANGELOG.md release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR

@czeslavo czeslavo self-assigned this Dec 7, 2022
@czeslavo czeslavo added area/gateway-api Relating to upstream Kubernetes SIG Networking Gateway API bug Something isn't working and removed size/L labels Dec 7, 2022
@czeslavo czeslavo added this to the KIC v2.8.0 milestone Dec 7, 2022
@czeslavo czeslavo temporarily deployed to Configure ci December 8, 2022 00:13 — with GitHub Actions Inactive
@czeslavo czeslavo temporarily deployed to Configure ci December 8, 2022 00:30 — with GitHub Actions Inactive
@czeslavo czeslavo temporarily deployed to Configure ci December 8, 2022 00:43 — with GitHub Actions Inactive
@czeslavo czeslavo temporarily deployed to Configure ci December 8, 2022 01:12 — with GitHub Actions Inactive
@czeslavo czeslavo temporarily deployed to Configure ci December 8, 2022 01:12 — with GitHub Actions Inactive
@czeslavo czeslavo temporarily deployed to Configure ci December 8, 2022 01:13 — with GitHub Actions Inactive
@czeslavo czeslavo temporarily deployed to Configure ci December 8, 2022 01:13 — with GitHub Actions Inactive
@czeslavo czeslavo temporarily deployed to Configure ci December 8, 2022 01:18 — with GitHub Actions Inactive
@czeslavo czeslavo temporarily deployed to Configure ci December 8, 2022 01:19 — with GitHub Actions Inactive
@czeslavo czeslavo marked this pull request as ready for review December 8, 2022 01:20
@czeslavo czeslavo requested a review from a team as a code owner December 8, 2022 01:20
@czeslavo czeslavo temporarily deployed to Configure ci December 8, 2022 01:24 — with GitHub Actions Inactive
@czeslavo czeslavo temporarily deployed to Configure ci December 8, 2022 01:55 — with GitHub Actions Inactive
@czeslavo czeslavo temporarily deployed to Configure ci December 8, 2022 01:55 — with GitHub Actions Inactive
@czeslavo czeslavo temporarily deployed to Configure ci December 8, 2022 02:20 — with GitHub Actions Inactive
@czeslavo czeslavo temporarily deployed to Configure ci December 8, 2022 02:50 — with GitHub Actions Inactive
@czeslavo czeslavo temporarily deployed to Configure ci December 8, 2022 02:50 — with GitHub Actions Inactive
@pmalek
Copy link
Member

pmalek commented Dec 8, 2022

As for the UDP issue: I'm personally in favor of postponing this (unless we have a strong desire to include this in 2.8). Reason being that the fix might take some time and be more involved (although I can be convinced otherwise).

@czeslavo
Copy link
Contributor Author

czeslavo commented Dec 8, 2022

As for the UDP issue: I'm personally in favor of postponing this (unless we have a strong desire to include this in 2.8). Reason being that the fix might take some time and be more involved (although I can be convinced otherwise).

Yup, I also think that will be the most reasonable approach. Adding the UDP publish service, adapting helm charts for that and testing properly might take some time.

@czeslavo czeslavo merged commit b0bad1f into main Dec 8, 2022
@czeslavo czeslavo deleted the fix/tcp-tls-routes-port-matching branch December 8, 2022 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/gateway-api Relating to upstream Kubernetes SIG Networking Gateway API bug Something isn't working size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants