diff --git a/pkg/registry/service/rest.go b/pkg/registry/service/rest.go index 454b707ffa..b632ee3357 100644 --- a/pkg/registry/service/rest.go +++ b/pkg/registry/service/rest.go @@ -284,6 +284,26 @@ func (rs *REST) ResourceLocation(ctx api.Context, id string) (*url.URL, http.Rou return nil, nil, errors.NewBadRequest(fmt.Sprintf("invalid service request %q", id)) } + // If a port *number* was specified, find the corresponding service port name + if portNum, err := strconv.ParseInt(portStr, 10, 64); err == nil { + svc, err := rs.registry.GetService(ctx, svcName) + if err != nil { + return nil, nil, err + } + found := false + for _, svcPort := range svc.Spec.Ports { + if svcPort.Port == int(portNum) { + // use the declared port's name + portStr = svcPort.Name + found = true + break + } + } + if !found { + return nil, nil, errors.NewServiceUnavailable(fmt.Sprintf("no service port %d found for service %q", portNum, svcName)) + } + } + eps, err := rs.endpoints.GetEndpoints(ctx, svcName) if err != nil { return nil, nil, err @@ -308,15 +328,6 @@ func (rs *REST) ResourceLocation(ctx api.Context, id string) (*url.URL, http.Rou Scheme: svcScheme, Host: net.JoinHostPort(ip, strconv.Itoa(port)), }, rs.proxyTransport, nil - } else { - port, err := strconv.ParseInt(portStr, 10, 64) - if err == nil && int(port) == ss.Ports[i].Port { - ip := ss.Addresses[rand.Intn(len(ss.Addresses))].IP - return &url.URL{ - Scheme: svcScheme, - Host: net.JoinHostPort(ip, portStr), - }, rs.proxyTransport, nil - } } } } diff --git a/pkg/registry/service/rest_test.go b/pkg/registry/service/rest_test.go index f4de8f1391..0749fbe58f 100644 --- a/pkg/registry/service/rest_test.go +++ b/pkg/registry/service/rest_test.go @@ -462,6 +462,14 @@ func TestServiceRegistryResourceLocation(t *testing.T) { ObjectMeta: api.ObjectMeta{Name: "foo"}, Spec: api.ServiceSpec{ Selector: map[string]string{"bar": "baz"}, + Ports: []api.ServicePort{ + // Service port 9393 should route to endpoint port "p", which is port 93 + {Name: "p", Port: 9393, TargetPort: intstr.FromString("p")}, + + // Service port 93 should route to unnamed endpoint port, which is port 80 + // This is to test that the service port definition is used when determining resource location + {Name: "", Port: 93, TargetPort: intstr.FromInt(80)}, + }, }, }) redirector := rest.Redirector(storage) @@ -490,7 +498,7 @@ func TestServiceRegistryResourceLocation(t *testing.T) { t.Errorf("Expected %v, but got %v", e, a) } - // Test a name + port number. + // Test a name + port number (service port 93 -> target port 80) location, _, err = redirector.ResourceLocation(ctx, "foo:93") if err != nil { t.Errorf("Unexpected error: %v", err) @@ -498,6 +506,18 @@ func TestServiceRegistryResourceLocation(t *testing.T) { if location == nil { t.Errorf("Unexpected nil: %v", location) } + if e, a := "//1.2.3.4:80", location.String(); e != a { + t.Errorf("Expected %v, but got %v", e, a) + } + + // Test a name + port number (service port 9393 -> target port "p" -> endpoint port 93) + location, _, err = redirector.ResourceLocation(ctx, "foo:9393") + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if location == nil { + t.Errorf("Unexpected nil: %v", location) + } if e, a := "//1.2.3.4:93", location.String(); e != a { t.Errorf("Expected %v, but got %v", e, a) } diff --git a/test/e2e/proxy.go b/test/e2e/proxy.go index a1260a71b5..2ec8e0963f 100644 --- a/test/e2e/proxy.go +++ b/test/e2e/proxy.go @@ -145,13 +145,19 @@ func proxyContext(version string) { } expectations := map[string]string{ svcProxyURL("", "portname1") + "/": "foo", + svcProxyURL("", "80") + "/": "foo", svcProxyURL("", "portname2") + "/": "bar", + svcProxyURL("", "81") + "/": "bar", svcProxyURL("http", "portname1") + "/": "foo", + svcProxyURL("http", "80") + "/": "foo", svcProxyURL("http", "portname2") + "/": "bar", + svcProxyURL("http", "81") + "/": "bar", svcProxyURL("https", "tlsportname1") + "/": "tls baz", + svcProxyURL("https", "443") + "/": "tls baz", svcProxyURL("https", "tlsportname2") + "/": "tls qux", + svcProxyURL("https", "444") + "/": "tls qux", podProxyURL("", "80") + "/": `test`, podProxyURL("", "160") + "/": "foo", @@ -172,9 +178,8 @@ func proxyContext(version string) { subresourcePodProxyURL("https", "443") + "/": `test`, subresourcePodProxyURL("https", "460") + "/": "tls baz", subresourcePodProxyURL("https", "462") + "/": "tls qux", + // TODO: below entries don't work, but I believe we should make them work. - // svcPrefix + ":80": "foo", - // svcPrefix + ":81": "bar", // podPrefix + ":dest1": "foo", // podPrefix + ":dest2": "bar", } @@ -231,6 +236,8 @@ func doProxy(f *Framework, path string) (body []byte, statusCode int, d time.Dur d = time.Since(start) if len(body) > 0 { Logf("%v: %s (%v; %v)", path, truncate(body, maxDisplayBodyLen), statusCode, d) + } else { + Logf("%v: %s (%v; %v)", path, "no body", statusCode, d) } return }