246 lines
7.2 KiB
Diff
246 lines
7.2 KiB
Diff
From 78d552efe620879599509ff0f9b04b8764e1878f Mon Sep 17 00:00:00 2001
|
|
From: Andrew Sy Kim <andrewsy@google.com>
|
|
Date: Mon, 7 Nov 2022 10:22:44 -0500
|
|
Subject: [PATCH] fix node address validation
|
|
|
|
Signed-off-by: Andrew Sy Kim <andrewsy@google.com>
|
|
---
|
|
.../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 bf6b6a57708..f879f327279 100644
|
|
--- a/pkg/registry/core/node/strategy.go
|
|
+++ b/pkg/registry/core/node/strategy.go
|
|
@@ -249,6 +249,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
|
|
@@ -261,10 +265,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
|
|
}
|
|
--
|
|
2.25.1
|
|
|