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

pomerium: Fix Enpoints port mapping #1102

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

BenoitKnecht
Copy link

Summary

When trying to determine which Endpoints port to use based on a named
Service port, we weren't finding the right one when the Service
spec.ports[].name didn't match the Service spec.ports[].targetPort
name.

This commit simplifies the mapping logic, and ensure the extended test
from the previous commit all succeed.

Here's an example of a Service and its associated Endpoints:

kind: Service
spec:
  ports:
  - name: grafana
    port: 80
    targetPort: grafana-http
kind: Endpoints
subsets:
- ports:
  - name: grafana
    port: 3000

An Ingress refers to the Service by name, and to a specific port either by
name or by number.

If by name (grafana in the example above), then it's always the same as
the port name on the Endpoints.

But if by number (80 in the example above), then we need to find the
correct Endpoints port by matching the Service targetPort.

Checklist

  • reference any related issues
  • updated docs
  • updated unit tests
  • updated UPGRADING.md
  • add appropriate tag (improvement / bug / etc)
  • ready for review

@BenoitKnecht BenoitKnecht requested a review from a team as a code owner February 10, 2025 13:52
@BenoitKnecht BenoitKnecht requested review from wasaga and removed request for a team February 10, 2025 13:52
Test more combinations of named and numbered ports, and in particular
avoid tests that happen to use the same name for the `port` and
`targetPort`, as they could pass despite the implementation being
broken.

Signed-off-by: Benoît Knecht <bknecht@protonmail.ch>
When trying to determine which Endpoints port to use based on a named
Service port, we weren't finding the right one when the Service
`spec.ports[].name` didn't match the Service `spec.ports[].targetPort`
name.

This commit simplifies the mapping logic, and ensure the extended test
from the previous commit all succeed.

Signed-off-by: Benoît Knecht <bknecht@protonmail.ch>
@BenoitKnecht BenoitKnecht force-pushed the service-to-endpoints-mapping branch from f0ff23e to beefbbc Compare February 11, 2025 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants