Skip to content

Commit

Permalink
Fix endpoint matching if service uses target named ports (pomerium#400)
Browse files Browse the repository at this point in the history
If a Service definition uses strings as targetPort, the function
returned by getEndpointPortMatcher does not match and fallback to use
domain.

This coomit modifies that function to try to match by port or by name
fixing that situation.
  • Loading branch information
adrianlzt authored Oct 25, 2022
1 parent 0fe0183 commit a46df09
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 12 deletions.
10 changes: 7 additions & 3 deletions pomerium/ingress_to_route.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
corev1 "k8s.io/api/core/v1"
networkingv1 "k8s.io/api/networking/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/intstr"
"sigs.k8s.io/controller-runtime/pkg/log"

pb "github.com/pomerium/pomerium/pkg/grpc/config"
Expand Down Expand Up @@ -301,14 +302,17 @@ func getEndpointsURLs(ingressServicePort networkingv1.ServiceBackendPort, servic

func getEndpointPortMatcher(ingressServicePort networkingv1.ServiceBackendPort, servicePorts []corev1.ServicePort) func(port corev1.EndpointPort) bool {
if ingressServicePort.Name != "" {
ports := make(map[int32]bool)
ports := make(map[intstr.IntOrString]bool)
for _, sp := range servicePorts {
if sp.Name == ingressServicePort.Name {
ports[sp.TargetPort.IntVal] = true
ports[sp.TargetPort] = true
}
}
return func(port corev1.EndpointPort) bool {
return port.Name == ingressServicePort.Name && ports[port.Port]
pName := intstr.FromString(port.Name)
pNumber := intstr.FromInt(int(port.Port))

return port.Name == ingressServicePort.Name && (ports[pName] || ports[pNumber])
}
}

Expand Down
44 changes: 35 additions & 9 deletions pomerium/routes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,8 @@ func TestUpsertIngress(t *testing.T) {
corev1.TLSCertKey: []byte("A"),
},
Type: corev1.SecretTypeTLS,
}},
},
},
Services: map[types.NamespacedName]*corev1.Service{
{Name: "service", Namespace: "default"}: {
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -228,7 +229,8 @@ func TestSecureUpstream(t *testing.T) {
Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}},
Ports: []corev1.EndpointPort{{Name: "https", Port: 443}},
}},
}},
},
},
Services: map[types.NamespacedName]*corev1.Service{
{Name: "service", Namespace: "default"}: {
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -313,7 +315,8 @@ func TestCustomSecrets(t *testing.T) {
Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}},
Ports: []corev1.EndpointPort{{Name: "http", Port: 80}},
}},
}},
},
},
Services: map[types.NamespacedName]*corev1.Service{
{Name: "service", Namespace: "default"}: {
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -418,7 +421,8 @@ func TestKubernetesToken(t *testing.T) {
Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}},
Ports: []corev1.EndpointPort{{Name: "http", Port: 80}},
}},
}},
},
},
Services: map[types.NamespacedName]*corev1.Service{
{Name: "service", Namespace: "default"}: {
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -491,7 +495,8 @@ func TestTCPUpstream(t *testing.T) {
Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}},
Ports: []corev1.EndpointPort{{Name: "app", Port: 12345}},
}},
}},
},
},
Services: map[types.NamespacedName]*corev1.Service{
{Name: "service", Namespace: "default"}: {
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -731,7 +736,8 @@ func TestDefaultBackendService(t *testing.T) {
Backend: *ic.Spec.DefaultBackend,
}},
},
}}}
},
}}
cfg := new(pb.Config)
require.NoError(t, upsertRoutes(context.Background(), cfg, ic))
sort.Sort(routeList(cfg.Routes))
Expand Down Expand Up @@ -861,7 +867,8 @@ func TestUseServiceProxy(t *testing.T) {
Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}},
Ports: []corev1.EndpointPort{{Port: 80}},
}},
}},
},
},
Services: map[types.NamespacedName]*corev1.Service{
{Name: "service", Namespace: "default"}: {
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -997,6 +1004,23 @@ func TestServicePortsAndEndpoints(t *testing.T) {
},
false,
},
{
"named port and named target port",
networkingv1.ServiceBackendPort{Name: "http"},
[]corev1.ServicePort{{
Name: "http",
Port: 8000,
TargetPort: intstr.IntOrString{StrVal: "http", Type: intstr.String},
}},
[]corev1.EndpointSubset{{
Addresses: []corev1.EndpointAddress{{IP: "1.2.3.4"}},
Ports: []corev1.EndpointPort{{Name: "http", Port: 80}},
}},
[]string{
"http://1.2.3.4:80",
},
false,
},
{
"multiple IPs",
networkingv1.ServiceBackendPort{Name: "http"},
Expand Down Expand Up @@ -1081,7 +1105,8 @@ func TestServicePortsAndEndpoints(t *testing.T) {
Namespace: "default",
},
Subsets: tc.endpointSubsets,
}},
},
},
Services: map[types.NamespacedName]*corev1.Service{
{Name: "service", Namespace: "default"}: {
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -1231,7 +1256,8 @@ func TestEndpointsHTTPS(t *testing.T) {
Namespace: "default",
},
Subsets: tc.endpointSubsets,
}},
},
},
Services: map[types.NamespacedName]*corev1.Service{
{Name: "service", Namespace: "default"}: {
ObjectMeta: metav1.ObjectMeta{
Expand Down

0 comments on commit a46df09

Please sign in to comment.