diff --git a/pkg/kubelet/client/kubelet_client_test.go b/pkg/kubelet/client/kubelet_client_test.go index 8dc921b3af5..0c9a07f3db3 100644 --- a/pkg/kubelet/client/kubelet_client_test.go +++ b/pkg/kubelet/client/kubelet_client_test.go @@ -17,6 +17,8 @@ limitations under the License. package client import ( + "context" + "errors" "net" "net/http" "net/http/httptest" @@ -24,18 +26,52 @@ import ( "net/url" "strconv" "testing" + + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + utilnet "k8s.io/apimachinery/pkg/util/net" + "k8s.io/apiserver/pkg/server/egressselector" ) +func kubeletTestCertHelper(valid bool) KubeletTLSConfig { + if valid { + return KubeletTLSConfig{ + CertFile: "testdata/mycertvalid.cer", + KeyFile: "testdata/mycertvalid.key", + CAFile: "testdata/myCA.cer", + } + } + return KubeletTLSConfig{ + CertFile: "testdata/mycertinvalid.cer", + KeyFile: "testdata/mycertinvalid.key", + CAFile: "testdata/myCA.cer", + } +} + +func kubeletTestRoundTripHelper(t *testing.T, rt http.RoundTripper, addr string) { + req, err := http.NewRequest(http.MethodGet, addr, nil) + if err != nil { + t.Fatal(err) + } + resp, err := rt.RoundTrip(req) + if err != nil { + t.Fatal(err) + } + if resp.StatusCode != http.StatusOK { + dump, err := httputil.DumpResponse(resp, true) + if err != nil { + t.Fatal(err) + } + t.Fatal(string(dump)) + } +} + func TestMakeTransportInvalid(t *testing.T) { config := &KubeletClientConfig{ // Invalid certificate and key path - TLSClientConfig: KubeletTLSConfig{ - CertFile: "../../client/testdata/mycertinvalid.cer", - KeyFile: "../../client/testdata/mycertinvalid.key", - CAFile: "../../client/testdata/myCA.cer", - }, + TLSClientConfig: kubeletTestCertHelper(false), } - rt, err := MakeTransport(config) if err == nil { t.Errorf("Expected an error") @@ -47,14 +83,8 @@ func TestMakeTransportInvalid(t *testing.T) { func TestMakeTransportValid(t *testing.T) { config := &KubeletClientConfig{ - Port: 1234, - TLSClientConfig: KubeletTLSConfig{ - CertFile: "../../client/testdata/mycertvalid.cer", - // TLS Configuration - KeyFile: "../../client/testdata/mycertvalid.key", - // TLS Configuration - CAFile: "../../client/testdata/myCA.cer", - }, + Port: 1234, + TLSClientConfig: kubeletTestCertHelper(true), } rt, err := MakeTransport(config) @@ -66,6 +96,60 @@ func TestMakeTransportValid(t *testing.T) { } } +func TestMakeTransportWithLookUp(t *testing.T) { + dialFunc := func(ctx context.Context, network, addr string) (net.Conn, error) { + return net.Dial(network, addr) + } + tests := []struct { + name string + config *KubeletClientConfig + expectError bool + }{ + { + "test makeTransport with Lookup closure initialized", + &KubeletClientConfig{ + Port: 1234, + TLSClientConfig: kubeletTestCertHelper(true), + Lookup: func(_ egressselector.NetworkContext) (utilnet.DialFunc, error) { + return dialFunc, nil + }, + }, + false, + }, + { + "test makeTransport with Lookup closure returning error", + &KubeletClientConfig{ + Port: 1234, + TLSClientConfig: kubeletTestCertHelper(true), + Lookup: func(_ egressselector.NetworkContext) (utilnet.DialFunc, error) { + return nil, errors.New("mock error") + }, + }, + true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + rt, err := MakeInsecureTransport(tt.config) + if tt.expectError { + if err == nil { + t.Fatalf("expected error but got none: Lookup func is invalid") + } + return + } + if rt == nil { + t.Fatalf("rt should not be nil") + } + testServer := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer testServer.Close() + kubeletTestRoundTripHelper(t, rt, testServer.URL) + }) + } +} + func TestMakeInsecureTransport(t *testing.T) { testServer := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { w.WriteHeader(http.StatusOK) @@ -86,14 +170,8 @@ func TestMakeInsecureTransport(t *testing.T) { } config := &KubeletClientConfig{ - Port: uint(port), - TLSClientConfig: KubeletTLSConfig{ - CertFile: "../../client/testdata/mycertvalid.cer", - // TLS Configuration - KeyFile: "../../client/testdata/mycertvalid.key", - // TLS Configuration - CAFile: "../../client/testdata/myCA.cer", - }, + Port: uint(port), + TLSClientConfig: kubeletTestCertHelper(true), } rt, err := MakeInsecureTransport(config) @@ -103,20 +181,251 @@ func TestMakeInsecureTransport(t *testing.T) { if rt == nil { t.Error("rt should not be nil") } + kubeletTestRoundTripHelper(t, rt, testServer.URL) +} - req, err := http.NewRequest(http.MethodGet, testServer.URL, nil) - if err != nil { - t.Fatal(err) +// Mock NodeGetter for testing +type mockNodeGetter struct { + node *v1.Node + err error +} + +func (m *mockNodeGetter) Get(ctx context.Context, name string, options metav1.GetOptions) (*v1.Node, error) { + return m.node, m.err +} + +func TestNewNodeConnectionInfoGetter(t *testing.T) { + tests := []struct { + name string + nodes NodeGetter + config KubeletClientConfig + expectError bool + }{ + { + name: "valid config", + nodes: &mockNodeGetter{}, + config: KubeletClientConfig{ + Port: 10250, + PreferredAddressTypes: []string{"InternalIP", "ExternalIP"}, + TLSClientConfig: kubeletTestCertHelper(true), + }, + expectError: false, + }, + { + name: "invalid cert file", + nodes: &mockNodeGetter{}, + config: KubeletClientConfig{ + Port: 10250, + PreferredAddressTypes: []string{"InternalIP"}, + TLSClientConfig: kubeletTestCertHelper(false), + }, + expectError: true, + }, + { + name: "empty preferred address types", + nodes: &mockNodeGetter{}, + config: KubeletClientConfig{ + Port: 10250, + PreferredAddressTypes: []string{}, + TLSClientConfig: kubeletTestCertHelper(true), + }, + expectError: false, + }, } - response, err := rt.RoundTrip(req) - if err != nil { - t.Fatal(err) - } - if response.StatusCode != http.StatusOK { - dump, err := httputil.DumpResponse(response, true) - if err != nil { - t.Fatal(err) - } - t.Fatal(string(dump)) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + getter, err := NewNodeConnectionInfoGetter(tt.nodes, tt.config) + + if tt.expectError { + if err == nil { + t.Fatalf("expected error but got none") + } + return + } + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if getter == nil { + t.Fatalf("expected non-nil getter") + } + + // Verify the created getter has correct properties + ncig, ok := getter.(*NodeConnectionInfoGetter) + if !ok { + t.Fatalf("expected *NodeConnectionInfoGetter, got %T", getter) + } + + if ncig.nodes != tt.nodes { + t.Errorf("expected nodes to be set correctly") + } + + if ncig.scheme != "https" { + t.Errorf("expected scheme to be 'https', got %s", ncig.scheme) + } + + if ncig.defaultPort != int(tt.config.Port) { + t.Errorf("expected defaultPort to be %d, got %d", tt.config.Port, ncig.defaultPort) + } + + if len(ncig.preferredAddressTypes) != len(tt.config.PreferredAddressTypes) { + t.Errorf("expected %d preferred address types, got %d", + len(tt.config.PreferredAddressTypes), len(ncig.preferredAddressTypes)) + } + }) + } +} + +func TestGetConnectionInfo(t *testing.T) { + tests := []struct { + name string + nodeGetter NodeGetter + nodeName types.NodeName + expectedHost string + expectedPort string + expectError bool + }{ + { + name: "valid node with kubelet endpoint port", + nodeGetter: &mockNodeGetter{ + node: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{Name: "test-node"}, + Status: v1.NodeStatus{ + Addresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "192.168.1.10"}, + {Type: v1.NodeExternalIP, Address: "203.0.113.10"}, + }, + DaemonEndpoints: v1.NodeDaemonEndpoints{ + KubeletEndpoint: v1.DaemonEndpoint{Port: 10250}, + }, + }, + }, + }, + nodeName: "test-node", + expectedHost: "203.0.113.10", + expectedPort: "10250", + expectError: false, + }, + { + name: "valid node without kubelet endpoint port (uses default)", + nodeGetter: &mockNodeGetter{ + node: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{Name: "test-node"}, + Status: v1.NodeStatus{ + Addresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "192.168.1.10"}, + }, + DaemonEndpoints: v1.NodeDaemonEndpoints{ + KubeletEndpoint: v1.DaemonEndpoint{Port: 0}, + }, + }, + }, + }, + nodeName: "test-node", + expectedHost: "192.168.1.10", + expectedPort: "10250", // default port + expectError: false, + }, + { + name: "node with external IP preferred", + nodeGetter: &mockNodeGetter{ + node: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{Name: "test-node"}, + Status: v1.NodeStatus{ + Addresses: []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "192.168.1.10"}, + {Type: v1.NodeExternalIP, Address: "203.0.113.10"}, + }, + DaemonEndpoints: v1.NodeDaemonEndpoints{ + KubeletEndpoint: v1.DaemonEndpoint{Port: 10250}, + }, + }, + }, + }, + nodeName: "test-node", + expectedHost: "203.0.113.10", // External IP should be preferred when available + expectedPort: "10250", + expectError: false, + }, + { + name: "node not found", + nodeGetter: &mockNodeGetter{ + err: errors.New("node not found"), + }, + nodeName: "nonexistent-node", + expectError: true, + }, + { + name: "node with no addresses", + nodeGetter: &mockNodeGetter{ + node: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{Name: "test-node"}, + Status: v1.NodeStatus{ + Addresses: []v1.NodeAddress{}, + DaemonEndpoints: v1.NodeDaemonEndpoints{ + KubeletEndpoint: v1.DaemonEndpoint{Port: 10250}, + }, + }, + }, + }, + nodeName: "test-node", + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + config := KubeletClientConfig{ + Port: 10250, + PreferredAddressTypes: []string{"ExternalIP", "InternalIP"}, + TLSClientConfig: kubeletTestCertHelper(true), + } + getter, err := NewNodeConnectionInfoGetter(tt.nodeGetter, config) + if err != nil { + t.Fatalf("failed to create NodeConnectionInfoGetter: %v", err) + } + + tCtx := context.Background() + connInfo, err := getter.GetConnectionInfo(tCtx, tt.nodeName) + + if tt.expectError { + if err == nil { + t.Fatalf("expected error but got none") + } + return + } + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if connInfo == nil { + t.Fatalf("expected non-nil connection info") + } + + if connInfo.Scheme != "https" { + t.Errorf("expected scheme 'https', got %s", connInfo.Scheme) + } + + if connInfo.Hostname != tt.expectedHost { + t.Errorf("expected hostname %s, got %s", tt.expectedHost, connInfo.Hostname) + } + + if connInfo.Port != tt.expectedPort { + t.Errorf("expected port %s, got %s", tt.expectedPort, connInfo.Port) + } + + if connInfo.Transport == nil { + t.Errorf("expected non-nil Transport") + } + + if connInfo.InsecureSkipTLSVerifyTransport == nil { + t.Errorf("expected non-nil InsecureSkipTLSVerifyTransport") + } + }) } } diff --git a/pkg/client/testdata/OWNERS b/pkg/kubelet/client/testdata/OWNERS similarity index 100% rename from pkg/client/testdata/OWNERS rename to pkg/kubelet/client/testdata/OWNERS diff --git a/pkg/client/testdata/README.md b/pkg/kubelet/client/testdata/README.md similarity index 100% rename from pkg/client/testdata/README.md rename to pkg/kubelet/client/testdata/README.md diff --git a/pkg/client/testdata/myCA.cer b/pkg/kubelet/client/testdata/myCA.cer similarity index 100% rename from pkg/client/testdata/myCA.cer rename to pkg/kubelet/client/testdata/myCA.cer diff --git a/pkg/client/testdata/myCA.key b/pkg/kubelet/client/testdata/myCA.key similarity index 100% rename from pkg/client/testdata/myCA.key rename to pkg/kubelet/client/testdata/myCA.key diff --git a/pkg/client/testdata/mycertvalid.cer b/pkg/kubelet/client/testdata/mycertvalid.cer similarity index 100% rename from pkg/client/testdata/mycertvalid.cer rename to pkg/kubelet/client/testdata/mycertvalid.cer diff --git a/pkg/client/testdata/mycertvalid.key b/pkg/kubelet/client/testdata/mycertvalid.key similarity index 100% rename from pkg/client/testdata/mycertvalid.key rename to pkg/kubelet/client/testdata/mycertvalid.key diff --git a/pkg/client/testdata/mycertvalid.req b/pkg/kubelet/client/testdata/mycertvalid.req similarity index 100% rename from pkg/client/testdata/mycertvalid.req rename to pkg/kubelet/client/testdata/mycertvalid.req