From c8114fdd72a227869881418fa99525544f148d30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Max=20Str=C3=BCbing?= <mxstrbng@gmail.com> Date: Thu, 3 Apr 2025 07:42:29 +0000 Subject: [PATCH 1/3] fix: apply http agent if needed --- src/config.ts | 3 +++ src/config_test.ts | 27 ++++++++++++++++++++++++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/config.ts b/src/config.ts index aad84dfd24..f1205dad51 100644 --- a/src/config.ts +++ b/src/config.ts @@ -1,5 +1,6 @@ import fs from 'node:fs'; import https from 'node:https'; +import http from 'node:http'; import yaml from 'js-yaml'; import net from 'node:net'; import path from 'node:path'; @@ -544,6 +545,8 @@ export class KubeConfig implements SecurityAuthentication { } else { throw new Error('Unsupported proxy type'); } + } else if (cluster && cluster.server && cluster.server.startsWith('http:')) { + agent = new http.Agent(agentOptions); } else { agent = new https.Agent(agentOptions); } diff --git a/src/config_test.ts b/src/config_test.ts index b5af672df6..b9ed9adf03 100644 --- a/src/config_test.ts +++ b/src/config_test.ts @@ -3,6 +3,7 @@ import { deepEqual, deepStrictEqual, notStrictEqual, rejects, strictEqual, throw import child_process from 'node:child_process'; import { readFileSync } from 'node:fs'; import https from 'node:https'; +import http from 'node:http'; import { Agent, RequestOptions } from 'node:https'; import path, { dirname, join } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -298,7 +299,7 @@ describe('KubeConfig', () => { }); }); - describe('applyHTTPSOptions', () => { + describe.only('applyHTTPSOptions', () => { it('should apply tls-server-name to https.RequestOptions', async () => { const kc = new KubeConfig(); kc.loadFromFile(kcTlsServerNameFileName); @@ -448,6 +449,30 @@ describe('KubeConfig', () => { message: 'Unsupported proxy type', }); }); + it('should apply http agent if cluster.server starts with http and no proxy-url is provided', async () => { + const kc = new KubeConfig(); + kc.loadFromFile(kcProxyUrl); + kc.setCurrentContext('contextE'); + + const testServerName = 'http://example.com'; + const rc = new RequestContext(testServerName, HttpMethod.GET); + + await kc.applySecurityAuthentication(rc); + + strictEqual(rc.getAgent() instanceof http.Agent, true); + }); + it('should apply https agent if cluster.server starts with https and no proxy-url is provided', async () => { + const kc = new KubeConfig(); + kc.loadFromFile(kcProxyUrl); + kc.setCurrentContext('contextF'); + + const testServerName = 'https://example.com'; + const rc = new RequestContext(testServerName, HttpMethod.GET); + + await kc.applySecurityAuthentication(rc); + + strictEqual(rc.getAgent() instanceof https.Agent, true); + }); }); describe('loadClusterConfigObjects', () => { From 692e4492d064c4fa317b12839057c0c7edd72e49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Max=20Str=C3=BCbing?= <mxstrbng@gmail.com> Date: Thu, 3 Apr 2025 13:15:31 +0000 Subject: [PATCH 2/3] chore: address review comments --- src/config.ts | 2 +- src/config_test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/config.ts b/src/config.ts index f1205dad51..80c14282f3 100644 --- a/src/config.ts +++ b/src/config.ts @@ -545,7 +545,7 @@ export class KubeConfig implements SecurityAuthentication { } else { throw new Error('Unsupported proxy type'); } - } else if (cluster && cluster.server && cluster.server.startsWith('http:')) { + } else if (cluster?.server?.startsWith('http:')) { agent = new http.Agent(agentOptions); } else { agent = new https.Agent(agentOptions); diff --git a/src/config_test.ts b/src/config_test.ts index b9ed9adf03..10a48cc573 100644 --- a/src/config_test.ts +++ b/src/config_test.ts @@ -299,7 +299,7 @@ describe('KubeConfig', () => { }); }); - describe.only('applyHTTPSOptions', () => { + describe('applyHTTPSOptions', () => { it('should apply tls-server-name to https.RequestOptions', async () => { const kc = new KubeConfig(); kc.loadFromFile(kcTlsServerNameFileName); From 37659920ecd4b1b40330b75bc5ddbd29eb90553e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Max=20Str=C3=BCbing?= <mxstrbng@gmail.com> Date: Sat, 5 Apr 2025 09:49:44 +0000 Subject: [PATCH 3/3] fix: check for `skipTlsVerify` before allowing http connection --- src/cache_test.ts | 2 +- src/config.ts | 4 +++- src/config_test.ts | 21 +++++++++++++++++++-- src/watch_test.ts | 2 +- testdata/kubeconfig-proxy-url.yaml | 25 +++++++++++++++++++++++++ 5 files changed, 49 insertions(+), 5 deletions(-) diff --git a/src/cache_test.ts b/src/cache_test.ts index 493a42ba72..315a4f55c5 100644 --- a/src/cache_test.ts +++ b/src/cache_test.ts @@ -11,7 +11,7 @@ import { ListPromise } from './informer.js'; import nock from 'nock'; import { Watch } from './watch.js'; -const server = 'http://foo.company.com'; +const server = 'https://foo.company.com'; const fakeConfig: { clusters: Cluster[]; diff --git a/src/config.ts b/src/config.ts index 80c14282f3..45a3f55677 100644 --- a/src/config.ts +++ b/src/config.ts @@ -545,8 +545,10 @@ export class KubeConfig implements SecurityAuthentication { } else { throw new Error('Unsupported proxy type'); } - } else if (cluster?.server?.startsWith('http:')) { + } else if (cluster?.server?.startsWith('http:') && cluster.skipTLSVerify) { agent = new http.Agent(agentOptions); + } else if (cluster?.server?.startsWith('http:') && !cluster.skipTLSVerify) { + throw new Error('HTTP protocol is not allowed when skipTLSVerify is not set or false'); } else { agent = new https.Agent(agentOptions); } diff --git a/src/config_test.ts b/src/config_test.ts index 10a48cc573..405b188d9f 100644 --- a/src/config_test.ts +++ b/src/config_test.ts @@ -1,5 +1,12 @@ import { after, before, beforeEach, describe, it, mock } from 'node:test'; -import { deepEqual, deepStrictEqual, notStrictEqual, rejects, strictEqual, throws } from 'node:assert'; +import assert, { + deepEqual, + deepStrictEqual, + notStrictEqual, + rejects, + strictEqual, + throws, +} from 'node:assert'; import child_process from 'node:child_process'; import { readFileSync } from 'node:fs'; import https from 'node:https'; @@ -461,11 +468,21 @@ describe('KubeConfig', () => { strictEqual(rc.getAgent() instanceof http.Agent, true); }); - it('should apply https agent if cluster.server starts with https and no proxy-url is provided', async () => { + it('should throw an error if cluster.server starts with http, no proxy-url is provided and insecure-skip-tls-verify is not set', async () => { const kc = new KubeConfig(); kc.loadFromFile(kcProxyUrl); kc.setCurrentContext('contextF'); + const testServerName = 'http://example.com'; + const rc = new RequestContext(testServerName, HttpMethod.GET); + + await assert.rejects(kc.applySecurityAuthentication(rc), Error); + }); + it('should apply https agent if cluster.server starts with https and no proxy-url is provided', async () => { + const kc = new KubeConfig(); + kc.loadFromFile(kcProxyUrl); + kc.setCurrentContext('contextG'); + const testServerName = 'https://example.com'; const rc = new RequestContext(testServerName, HttpMethod.GET); diff --git a/src/watch_test.ts b/src/watch_test.ts index a9da7881ec..0e9c65c0e0 100644 --- a/src/watch_test.ts +++ b/src/watch_test.ts @@ -7,7 +7,7 @@ import { Cluster, Context, User } from './config_types.js'; import { Watch } from './watch.js'; import { IncomingMessage } from 'node:http'; -const server = 'http://foo.company.com'; +const server = 'https://foo.company.com'; const fakeConfig: { clusters: Cluster[]; diff --git a/testdata/kubeconfig-proxy-url.yaml b/testdata/kubeconfig-proxy-url.yaml index 8d88117af9..03ea1a2cdf 100644 --- a/testdata/kubeconfig-proxy-url.yaml +++ b/testdata/kubeconfig-proxy-url.yaml @@ -20,6 +20,15 @@ clusters: server: htto://exampleerror.com proxy-url: http://example:8080 name: clusterD + - cluster: + certificate-authority-data: Q0FEQVRA + server: http://exampleerror.com + insecure-skip-tls-verify: true + name: clusterE + - cluster: + certificate-authority-data: Q0FEQVRA + server: http://exampleerror.com + name: clusterF contexts: - context: @@ -38,6 +47,14 @@ contexts: cluster: clusterD user: userD name: contextD + - context: + cluster: clusterE + user: userE + name: contextE + - context: + cluster: clusterF + user: userF + name: contextF current-context: contextA kind: Config @@ -59,3 +76,11 @@ users: user: client-certificate-data: XVNFUl9DQURBVEE= client-key-data: XVNFUl9DS0RBVEE= + - name: userE + user: + client-certificate-data: XVNFUl9DQURBVEE= + client-key-data: XVNFUl9DS0RBVEE= + - name: userF + user: + client-certificate-data: XVNFUl9DQURBVEE= + client-key-data: XVNFUl9DS0RBVEE=