From 2aee9b26f77276dea2d5cd2c1501d5341e74184e Mon Sep 17 00:00:00 2001 From: Andrew Sy Kim Date: Mon, 7 Nov 2022 10:22:44 -0500 Subject: [PATCH] fix node address validation Signed-off-by: Andrew Sy Kim --- .../core/node/storage/storage_test.go | 171 +++++++++++++++++- pkg/registry/core/node/strategy.go | 8 +- 2 files changed, 174 insertions(+), 5 deletions(-) diff --git a/pkg/registry/core/node/storage/storage_test.go b/pkg/registry/core/node/storage/storage_test.go index af228b8a43a..89df2edf16b 100644 --- a/pkg/registry/core/node/storage/storage_test.go +++ b/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) + } + }) + } +} diff --git a/pkg/registry/core/node/strategy.go b/pkg/registry/core/node/strategy.go index 79298df78ed..562c6e2db3f 100644 --- a/pkg/registry/core/node/strategy.go +++ b/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 }