fix node address validation

Signed-off-by: Andrew Sy Kim <andrewsy@google.com>

pkg/registry/core/node/storage/storage_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package storage
 
 import (
+	"fmt"
 	"testing"
 
 	"k8s.io/apimachinery/pkg/api/resource"
@@ -24,11 +25,14 @@ import (
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
+	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
 	"k8s.io/apiserver/pkg/registry/generic"
 	genericregistrytest "k8s.io/apiserver/pkg/registry/generic/testing"
+	"k8s.io/apiserver/pkg/registry/rest"
 	etcd3testing "k8s.io/apiserver/pkg/storage/etcd3/testing"
 	api "k8s.io/kubernetes/pkg/apis/core"
 	kubeletclient "k8s.io/kubernetes/pkg/kubelet/client"
+	proxyutil "k8s.io/kubernetes/pkg/proxy/util"
 	"k8s.io/kubernetes/pkg/registry/registrytest"
 )
 
@@ -40,7 +44,10 @@ func newStorage(t *testing.T) (*REST, *etcd3testing.EtcdTestServer) {
 		DeleteCollectionWorkers: 1,
 		ResourcePrefix:          "nodes",
 	}
-	storage, err := NewStorage(restOptions, kubeletclient.KubeletClientConfig{}, nil)
+	storage, err := NewStorage(restOptions, kubeletclient.KubeletClientConfig{
+		Port:                  10250,
+		PreferredAddressTypes: []string{string(api.NodeInternalIP)},
+	}, nil)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -156,3 +163,165 @@ func TestShortNames(t *testing.T) {
 	expected := []string{"no"}
 	registrytest.AssertShortNames(t, storage, expected)
 }
+
+func TestResourceLocation(t *testing.T) {
+	testCases := []struct {
+		name     string
+		node     api.Node
+		query    string
+		location string
+		err      error
+	}{
+		{
+			name: "proxyable hostname with default port",
+			node: api.Node{
+				ObjectMeta: metav1.ObjectMeta{Name: "node0"},
+				Status: api.NodeStatus{
+					Addresses: []api.NodeAddress{
+						{
+							Type:    api.NodeInternalIP,
+							Address: "10.0.0.1",
+						},
+					},
+				},
+			},
+			query:    "node0",
+			location: "10.0.0.1:10250",
+			err:      nil,
+		},
+		{
+			name: "proxyable hostname with kubelet port in query",
+			node: api.Node{
+				ObjectMeta: metav1.ObjectMeta{Name: "node0"},
+				Status: api.NodeStatus{
+					Addresses: []api.NodeAddress{
+						{
+							Type:    api.NodeInternalIP,
+							Address: "10.0.0.1",
+						},
+					},
+				},
+			},
+			query:    "node0:5000",
+			location: "10.0.0.1:5000",
+			err:      nil,
+		},
+		{
+			name: "proxyable hostname with kubelet port in status",
+			node: api.Node{
+				ObjectMeta: metav1.ObjectMeta{Name: "node0"},
+				Status: api.NodeStatus{
+					Addresses: []api.NodeAddress{
+						{
+							Type:    api.NodeInternalIP,
+							Address: "10.0.0.1",
+						},
+					},
+					DaemonEndpoints: api.NodeDaemonEndpoints{
+						KubeletEndpoint: api.DaemonEndpoint{
+							Port: 5000,
+						},
+					},
+				},
+			},
+			query:    "node0",
+			location: "10.0.0.1:5000",
+			err:      nil,
+		},
+		{
+			name: "non-proxyable hostname with default port",
+			node: api.Node{
+				ObjectMeta: metav1.ObjectMeta{Name: "node0"},
+				Status: api.NodeStatus{
+					Addresses: []api.NodeAddress{
+						{
+							Type:    api.NodeInternalIP,
+							Address: "127.0.0.1",
+						},
+					},
+				},
+			},
+			query:    "node0",
+			location: "",
+			err:      proxyutil.ErrAddressNotAllowed,
+		},
+		{
+			name: "non-proxyable hostname with kubelet port in query",
+			node: api.Node{
+				ObjectMeta: metav1.ObjectMeta{Name: "node0"},
+				Status: api.NodeStatus{
+					Addresses: []api.NodeAddress{
+						{
+							Type:    api.NodeInternalIP,
+							Address: "127.0.0.1",
+						},
+					},
+				},
+			},
+			query:    "node0:5000",
+			location: "",
+			err:      proxyutil.ErrAddressNotAllowed,
+		},
+		{
+			name: "non-proxyable hostname with kubelet port in status",
+			node: api.Node{
+				ObjectMeta: metav1.ObjectMeta{Name: "node0"},
+				Status: api.NodeStatus{
+					Addresses: []api.NodeAddress{
+						{
+							Type:    api.NodeInternalIP,
+							Address: "127.0.0.1",
+						},
+					},
+					DaemonEndpoints: api.NodeDaemonEndpoints{
+						KubeletEndpoint: api.DaemonEndpoint{
+							Port: 443,
+						},
+					},
+				},
+			},
+			query:    "node0",
+			location: "",
+			err:      proxyutil.ErrAddressNotAllowed,
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			storage, server := newStorage(t)
+			defer server.Terminate(t)
+			defer storage.Store.DestroyFunc()
+
+			ctx := genericapirequest.WithNamespace(genericapirequest.NewDefaultContext(), fmt.Sprintf("namespace-%s", testCase.name))
+			key, _ := storage.KeyFunc(ctx, testCase.node.Name)
+			if err := storage.Storage.Create(ctx, key, &testCase.node, nil, 0, false); err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+
+			redirector := rest.Redirector(storage)
+			location, _, err := redirector.ResourceLocation(ctx, testCase.query)
+
+			if err != nil && testCase.err != nil {
+				if err.Error() != testCase.err.Error() {
+					t.Fatalf("Unexpected error: %v, expected: %v", err, testCase.err)
+				}
+
+				return
+			}
+
+			if err != nil && testCase.err == nil {
+				t.Fatalf("Unexpected error: %v, expected: %v", err, testCase.err)
+			} else if err == nil && testCase.err != nil {
+				t.Fatalf("Expected error but got none, err: %v", testCase.err)
+			}
+
+			if location == nil {
+				t.Errorf("Unexpected nil resource location: %v", location)
+			}
+
+			if location.Host != testCase.location {
+				t.Errorf("Unexpected host: expected %v, but got %v", testCase.location, location.Host)
+			}
+		})
+	}
+}

pkg/registry/core/node/strategy.go

@@ -247,6 +247,10 @@ func ResourceLocation(getter ResourceGetter, connection client.ConnectionInfoGet
 		return nil, nil, err
 	}
 
+	if err := proxyutil.IsProxyableHostname(ctx, &net.Resolver{}, info.Hostname); err != nil {
+		return nil, nil, errors.NewBadRequest(err.Error())
+	}
+
 	// We check if we want to get a default Kubelet's transport. It happens if either:
 	// - no port is specified in request (Kubelet's port is default)
 	// - the requested port matches the kubelet port for this node
@@ -259,10 +263,6 @@ func ResourceLocation(getter ResourceGetter, connection client.ConnectionInfoGet
 			nil
 	}
 
-	if err := proxyutil.IsProxyableHostname(ctx, &net.Resolver{}, info.Hostname); err != nil {
-		return nil, nil, errors.NewBadRequest(err.Error())
-	}
-
 	// Otherwise, return the requested scheme and port, and the proxy transport
 	return &url.URL{Scheme: schemeReq, Host: net.JoinHostPort(info.Hostname, portReq)}, proxyTransport, nil
 }