Skip to content

Commit 783cc66

Browse files
authored
fix(k8s): fix paths in requests to kubernetes api (#5476)
* fix(k8s): fix paths in requests to kubernetes api * chore: more verbose comment
1 parent 456fa59 commit 783cc66

File tree

2 files changed

+37
-2
lines changed
  • core
    • src/plugins/kubernetes
    • test/integ/src/plugins/kubernetes

2 files changed

+37
-2
lines changed

core/src/plugins/kubernetes/api.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -428,7 +428,17 @@ export class KubeApi {
428428
retryOpts?: RetryOpts
429429
}): Promise<Response> {
430430
const baseUrl = this.config.getCurrentCluster()!.server
431-
const url = new URL(path, baseUrl)
431+
432+
// When using URL with a base path, the merging of the paths doesn't work as you are used to from most node request libraries.
433+
// It uses the semantics of browser URLs where a path starting with `/` is seen as absolute and thus it does not get merged with the base path.
434+
// See: https://developer.mozilla.org/en-US/docs/Learn/Common_questions/Web_mechanics/What_is_a_URL#absolute_urls_vs._relative_urls
435+
// Similarly, when the base path does not ends with a `/`, the last path segment is seen as a resource and also removed from the joined path.
436+
// That's why we need to remove the leading `/` from the path and ensure that the base path ends with a `/`.
437+
438+
const relativePath = path.replace(/^\//, "")
439+
const absoluteBaseUrl = baseUrl.endsWith("/") ? baseUrl : baseUrl + "/"
440+
441+
const url = new URL(relativePath, absoluteBaseUrl)
432442

433443
for (const [key, value] of Object.entries(opts.query ?? {})) {
434444
url.searchParams.set(key, value)

core/test/integ/src/plugins/kubernetes/api.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,14 +299,15 @@ describe("KubeApi", () => {
299299
let server: Server<typeof IncomingMessage, typeof ServerResponse>
300300
let wasRetried: boolean
301301
let reqCount: number
302+
let requestUrl: string | undefined
302303
let statusCodeHandler: () => number
303304

304305
before(async () => {
305306
class TestKubeConfig extends KubeConfig {
306307
override getCurrentCluster() {
307308
return {
308309
name: "test-cluster",
309-
server: `http://${hostname}:${port}/`,
310+
server: `http://${hostname}:${port}/clusters/test`,
310311
skipTLSVerify: true,
311312
}
312313
}
@@ -315,6 +316,7 @@ describe("KubeApi", () => {
315316
api = new KubeApi(log, "test-context", config)
316317

317318
server = createServer((req, res) => {
319+
requestUrl = req.url
318320
let bodyRaw = ""
319321
reqCount++
320322
wasRetried = reqCount > 1
@@ -335,6 +337,7 @@ describe("KubeApi", () => {
335337
beforeEach(() => {
336338
reqCount = 0
337339
wasRetried = false
340+
requestUrl = ""
338341
statusCodeHandler = () => {
339342
throw "implement in test case"
340343
}
@@ -344,6 +347,28 @@ describe("KubeApi", () => {
344347
server.close()
345348
})
346349

350+
it("should correctly merge the paths together when absolute paths are used", async () => {
351+
statusCodeHandler = () => 200
352+
await api.request({
353+
log,
354+
path: "version",
355+
opts: { method: "GET" },
356+
})
357+
358+
expect(requestUrl).to.eql(`/clusters/test/version`)
359+
})
360+
361+
it("should correctly merge the paths together when relative paths are used", async () => {
362+
statusCodeHandler = () => 200
363+
await api.request({
364+
log,
365+
path: "/version",
366+
opts: { method: "GET" },
367+
})
368+
369+
expect(requestUrl).to.eql(`/clusters/test/version`)
370+
})
371+
347372
it("should do a basic request without failure", async () => {
348373
statusCodeHandler = () => 200
349374
const res = await api.request({

0 commit comments

Comments
 (0)