fix(k8s): fix paths in requests to kubernetes api (#5476)

* fix(k8s): fix paths in requests to kubernetes api * chore: more verbose comment
garden-io · Nov 23, 2023 · 783cc66 · 783cc66
1 parent 456fa59
commit 783cc66
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 2 deletions.
diff --git a/core/src/plugins/kubernetes/api.ts b/core/src/plugins/kubernetes/api.ts
@@ -428,7 +428,17 @@ export class KubeApi {
     retryOpts?: RetryOpts
   }): Promise<Response> {
     const baseUrl = this.config.getCurrentCluster()!.server
-    const url = new URL(path, baseUrl)
+
+    // 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.
+    // 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.
+    // See: https://developer.mozilla.org/en-US/docs/Learn/Common_questions/Web_mechanics/What_is_a_URL#absolute_urls_vs._relative_urls
+    // 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.
+    // That's why we need to remove the leading `/` from the path and ensure that the base path ends with a `/`.
+
+    const relativePath = path.replace(/^\//, "")
+    const absoluteBaseUrl = baseUrl.endsWith("/") ? baseUrl : baseUrl + "/"
+
+    const url = new URL(relativePath, absoluteBaseUrl)
 
     for (const [key, value] of Object.entries(opts.query ?? {})) {
       url.searchParams.set(key, value)

diff --git a/core/test/integ/src/plugins/kubernetes/api.ts b/core/test/integ/src/plugins/kubernetes/api.ts
@@ -299,14 +299,15 @@ describe("KubeApi", () => {
     let server: Server<typeof IncomingMessage, typeof ServerResponse>
     let wasRetried: boolean
     let reqCount: number
+    let requestUrl: string | undefined
     let statusCodeHandler: () => number
 
     before(async () => {
       class TestKubeConfig extends KubeConfig {
         override getCurrentCluster() {
           return {
             name: "test-cluster",
-            server: `http://${hostname}:${port}/`,
+            server: `http://${hostname}:${port}/clusters/test`,
             skipTLSVerify: true,
           }
         }
@@ -315,6 +316,7 @@ describe("KubeApi", () => {
       api = new KubeApi(log, "test-context", config)
 
       server = createServer((req, res) => {
+        requestUrl = req.url
         let bodyRaw = ""
         reqCount++
         wasRetried = reqCount > 1
@@ -335,6 +337,7 @@ describe("KubeApi", () => {
     beforeEach(() => {
       reqCount = 0
       wasRetried = false
+      requestUrl = ""
       statusCodeHandler = () => {
         throw "implement in test case"
       }
@@ -344,6 +347,28 @@ describe("KubeApi", () => {
       server.close()
     })
 
+    it("should correctly merge the paths together when absolute paths are used", async () => {
+      statusCodeHandler = () => 200
+      await api.request({
+        log,
+        path: "version",
+        opts: { method: "GET" },
+      })
+
+      expect(requestUrl).to.eql(`/clusters/test/version`)
+    })
+
+    it("should correctly merge the paths together when relative paths are used", async () => {
+      statusCodeHandler = () => 200
+      await api.request({
+        log,
+        path: "/version",
+        opts: { method: "GET" },
+      })
+
+      expect(requestUrl).to.eql(`/clusters/test/version`)
+    })
+
     it("should do a basic request without failure", async () => {
       statusCodeHandler = () => 200
       const res = await api.request({