refactor(juno): fix typescript errors (k8s client) (#843)

* fix: Fix k8s-client watch error

* fix(juno): replace regular function class methods with arrow functions

* refactor(juno): fix typescript errors (k8s client)

* refactor(juno): create changeset

* refactor(juno): enhance k8s api error type definition

fixes #812

Author: mark-karnaukh-extern-sap

.changeset/healthy-needles-return.md

@@ -0,0 +1,5 @@
+---
+"@cloudoperators/juno-k8s-client": patch
+---
+
+fix typescript errors (k8s client)

packages/k8s-client/eslint.config.mjs

@@ -14,14 +14,7 @@ export default [
         project: ["./tsconfig.json"], // Ensure this points to your tsconfig.json
       },
     },
-    // TODO: We need to make all of this checks on again, step by step
-    rules: {
-      "@typescript-eslint/no-unsafe-assignment": "off",
-      "@typescript-eslint/no-unsafe-call": "off",
-      "@typescript-eslint/no-unsafe-argument": "off",
-      "@typescript-eslint/no-floating-promises": "off",
-      "@typescript-eslint/no-unsafe-member-access": "off",
-    },
+    rules: {},
     ignores: ["vitest.config.ts", "vite.config.ts"],
   },
 ]

packages/k8s-client/src/apiErrorHandler.ts

@@ -3,12 +3,21 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-interface ApiErrorType extends Error {
+export interface K8sApiError extends Error {
+  apiVersion?: string
+  kind?: string
+  message: string
+  metadata?: Record<string, any>
+  reason?: string
+  status?: string
+  code?: number
   response?: {
     data?: { message?: string }
+    status?: number
   }
 }
-const apiErrorHandler = async (apiError: ApiErrorType): Promise<never> => {
+
+const apiErrorHandler = async (apiError: K8sApiError): Promise<K8sApiError> => {
   const error = apiError.response?.data ? new Error(apiError.response.data.message ?? apiError.message) : apiError
 
   return Promise.reject(error)

packages/k8s-client/src/client.ts

@@ -6,7 +6,7 @@
 import request from "./request"
 import { buildUrl } from "./urlHelpers"
 import { Watch, ADDED, MODIFIED, DELETED, ERROR } from "./watch"
-import handleApiError from "./apiErrorHandler"
+import handleApiError, { K8sApiError } from "./apiErrorHandler"
 
 interface ClientOptions {
   apiEndpoint: string
@@ -17,7 +17,7 @@ interface ClientOptions {
 interface RequestOptions {
   params?: Record<string, any>
   headers?: Record<string, string>
-  body?: any
+  body?: Object | null
   signal?: AbortSignal
   mode?: RequestMode
   cache?: RequestCache
@@ -51,30 +51,30 @@ function createClient(options: ClientOptions) {
     }
   }
 
-  function head(path: string, options: RequestOptions = {}): Promise<Response> {
+  function head(path: string, options: RequestOptions = {}): Promise<Response | K8sApiError> {
     return request("HEAD", buildUrl(apiEndpoint, path), extendOptions(options)).catch(handleApiError)
   }
 
-  function get(path: string, options: RequestOptions = {}): Promise<any> {
+  function get(path: string, options: RequestOptions = {}): Promise<unknown> {
     return request("GET", buildUrl(apiEndpoint, path), extendOptions(options))
       .then((res) => res.json())
       .catch(handleApiError)
   }
 
-  function post(path: string, data: {}, options: RequestOptions = {}): Promise<any> {
+  function post(path: string, data: {}, options: RequestOptions = {}): Promise<unknown> {
     const result = request("POST", buildUrl(apiEndpoint, path), extendOptions(options, { body: data }))
       .then((res) => res.json())
       .catch(handleApiError)
     return result
   }
 
-  function put(path: string, data: {}, options: RequestOptions = {}): Promise<any> {
+  function put(path: string, data: {}, options: RequestOptions = {}): Promise<unknown> {
     return request("PUT", buildUrl(apiEndpoint, path), extendOptions(options, { body: data }))
       .then((res) => res.json())
       .catch(handleApiError)
   }
 
-  function patch(path: string, data: {}, options: RequestOptions = {}): Promise<any> {
+  function patch(path: string, data: {}, options: RequestOptions = {}): Promise<unknown> {
     return request(
       "PATCH",
       buildUrl(apiEndpoint, path),
@@ -87,7 +87,7 @@ function createClient(options: ClientOptions) {
       .catch(handleApiError)
   }
 
-  function del(path: string, data?: {} | null, options: RequestOptions = {}): Promise<any> {
+  function del(path: string, data?: {} | null, options: RequestOptions = {}): Promise<unknown> {
     return request("DELETE", buildUrl(apiEndpoint, path), extendOptions(options, { body: data }))
       .then((res) => res.json())
       .catch(handleApiError)

packages/k8s-client/src/request.ts

@@ -5,16 +5,18 @@
 
 import { buildUrl } from "./urlHelpers"
 import * as logger from "./logger"
+import { K8sApiError } from "./apiErrorHandler"
 
 // Define the shape of the options parameter
 interface RequestOptions {
   params?: Record<string, any>
   signal?: AbortSignal
-  headers?: HeadersInit
-  body?: any
+  headers?: HeadersInit | null
+  body?: Object | null
   mode?: RequestMode
   cache?: RequestCache
   credentials?: RequestCredentials
+  [key: string]: any
 }
 
 // Check response status
@@ -23,7 +25,7 @@ const checkStatus = (response: Response): Response => {
     return response
   } else {
     const error = new Error(response.statusText || `${response.status}`)
-    ;(error as any).response = response // Type assertion to attach the response to the error
+    ;(error as K8sApiError).response = response // Type assertion to attach the response to the error
     throw error
   }
 }
@@ -41,14 +43,17 @@ function request(method: string, url: string, options: RequestOptions = {}): Pro
   if (options.params) url = buildUrl(url, options.params)
 
   // add allowed options to fetch
-  const fetchOptions: RequestInit = ["signal", "headers", "body", "mode", "cache", "credentials"].reduce(
+  const requestFields = ["signal", "headers", "body", "mode", "cache", "credentials"] as const
+
+  const fetchOptions: RequestInit = requestFields.reduce(
     (map, key) => {
-      if (options[key as keyof RequestOptions]) {
-        map[key as keyof RequestInit] = options[key as keyof RequestOptions]
+      if (options[key]) {
+        return { ...map, [key]: options[key] }
       }
+
       return map
     },
-    { credentials: "same-origin", method } as RequestInit
+    { credentials: "same-origin", method }
   )
 
   // stringify body if it's an object

packages/k8s-client/src/watch.ts

@@ -5,20 +5,38 @@
 
 import request from "./request"
 import * as logger from "./logger"
+import { K8sApiError } from "./apiErrorHandler"
 
 const ADDED = "ADDED"
 const MODIFIED = "MODIFIED"
 const DELETED = "DELETED"
 const ERROR = "ERROR"
 const BOOKMARK = "BOOKMARK"
 
-type Listener = (_items: any) => void
+type Listener = (_items: unknown) => void
+
+interface WatchEvent {
+  type: string
+  object?: {
+    metadata?: {
+      resourceVersion?: string
+    }
+    code?: number
+  }
+}
+
+interface ApiResponseData {
+  items: unknown
+  metadata: {
+    resourceVersion: string | null
+  }
+}
 
 interface WatchOptions {
-  params?: Record<string, any>
+  params?: Record<string, string>
   headers?: Record<string, string>
   signal?: AbortSignal
-  body?: any
+  body?: Object | null
   mode?: RequestMode
   cache?: RequestCache
   credentials?: RequestCredentials
@@ -69,7 +87,7 @@ class Watch {
     this.listeners[type].push(listener)
   }
 
-  private informListeners = (type: string, items: any) => {
+  private informListeners = (type: string, items: unknown) => {
     const listeners = this.listeners[type]
     if (listeners) {
       listeners.forEach((listener) => {
@@ -83,7 +101,7 @@ class Watch {
     }
   }
 
-  private handleEvents = (events: any) => {
+  private handleEvents = (events: WatchEvent | WatchEvent[]) => {
     if (!Array.isArray(events)) events = [events]
 
     // Pass correct "this" context to be referenced in asynchronous callbacks
@@ -92,9 +110,9 @@ class Watch {
     setTimeout(() => {
       const eventsByType: Record<string, any[]> = {}
 
-      events.forEach((event: any) => {
+      events.forEach((event: WatchEvent) => {
         if (event.type === BOOKMARK) {
-          that.resourceVersion = event.object.metadata.resourceVersion
+          that.resourceVersion = event?.object?.metadata?.resourceVersion ?? null
         } else {
           if (!eventsByType[event.type]) {
             eventsByType[event.type] = []
@@ -111,7 +129,10 @@ class Watch {
     if (this.resourceVersion) return this.resourceVersion
 
     logger.debug(this.PREFIX, "get resource version from API")
-    const { metadata, items } = await request("GET", this.url, this.options).then((response) => response.json())
+    const { metadata, items } = (await request("GET", this.url, this.options).then((response) =>
+      // 'Body.json()' type definition expects 'any' as a return value
+      response.json()
+    )) as ApiResponseData
 
     this.resourceVersion = metadata.resourceVersion
     if (items) this.informListeners(ADDED, items)
@@ -155,10 +176,11 @@ class Watch {
           const events = data.split(/\n|\r|\r\n/)
           data = events.pop() ?? ""
 
-          const parsedEvents: any[] = []
+          const parsedEvents: WatchEvent[] = []
           events.forEach((e) => {
-            const parsedEvent = JSON.parse(e)
-            if (parsedEvent.type === ERROR && parsedEvent.object.code === 410) {
+            // 'JSON.parse' type definition expects 'any' as a return value
+            const parsedEvent = JSON.parse(e) as WatchEvent
+            if (parsedEvent.type === ERROR && parsedEvent?.object?.code === 410) {
               that.cancel()
               setTimeout(() => {
                 logger.debug(that.PREFIX, "resource is gone 410", "recreate watch request!")
@@ -173,7 +195,7 @@ class Watch {
           that.handleEvents(parsedEvents)
         }
       })
-      .catch((e) => {
+      .catch((e: K8sApiError) => {
         if (e.name === "AbortError") return
         const status = e.code || e?.response?.status
 
@@ -184,7 +206,7 @@ class Watch {
           return
         }
 
-        if ([404].includes(status)) {
+        if (status === 404) {
           that.handleEvents({ type: ERROR, object: e })
           that.cancel()
           return

packages/k8s-client/test/client.test.ts

@@ -22,9 +22,10 @@ describe("k8sClient", () => {
   describe("createClient", () => {
     describe("all required options are provided", () => {
       const options = { apiEndpoint: "test", token: "test" }
+      const result = expect(() => createClient(options))
 
       test("should not throw an error", () => {
-        expect(() => createClient(options)).not.toThrow()
+        result.not.toThrow()
       })
 
       test("should return a client object", () => {
@@ -59,16 +60,18 @@ describe("Client", () => {
       expect(client.get).toBeDefined()
     })
 
-    test("call request", () => {
-      client.get("/api/v1", { params: { key1: "value1", key2: "value2" } })
+    test("call request", async () => {
+      await client.get("/api/v1", { params: { key1: "value1", key2: "value2" } })
       expect(request).toHaveBeenLastCalledWith("GET", "https://test.com/api/v1", {
+        // linter complains of the asymmetric matcher "expect.anything()" - unsafe 'any' value assignment
+        // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
         headers: expect.anything(),
         params: { key1: "value1", key2: "value2" },
       })
     })
 
-    test("allow to override default options", () => {
-      client.get("/api/v1", {
+    test("allow to override default options", async () => {
+      await client.get("/api/v1", {
         headers: { Authorization: "test", "Content-Type": "text" },
       })
       expect(request).toHaveBeenLastCalledWith("GET", "https://test.com/api/v1", {
@@ -82,9 +85,11 @@ describe("Client", () => {
       expect(client.post).toBeDefined()
     })
 
-    test("call request", () => {
-      client.post("/api/v1", { key1: "value1" }, { params: { key1: "value1", key2: "value2" } })
+    test("call request", async () => {
+      await client.post("/api/v1", { key1: "value1" }, { params: { key1: "value1", key2: "value2" } })
       expect(request).toHaveBeenLastCalledWith("POST", "https://test.com/api/v1", {
+        // linter complains of the asymmetric matcher "expect.anything()" - unsafe 'any' value assignment
+        // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
         headers: expect.anything(),
         params: { key1: "value1", key2: "value2" },
         body: { key1: "value1" },
@@ -97,8 +102,8 @@ describe("Client", () => {
       expect(client.put).toBeDefined()
     })
 
-    test("call request", () => {
-      client.put("/api/v1", { key1: "value1" }, { params: { key1: "value1", key2: "value2" } })
+    test("call request", async () => {
+      await client.put("/api/v1", { key1: "value1" }, { params: { key1: "value1", key2: "value2" } })
       expect(request).toHaveBeenLastCalledWith("PUT", "https://test.com/api/v1", {
         headers: {
           Authorization: "Bearer test",
@@ -115,8 +120,8 @@ describe("Client", () => {
       expect(client.patch).toBeDefined()
     })
 
-    test("call request", () => {
-      client.patch(
+    test("call request", async () => {
+      await client.patch(
         "/api/v1",
         { key1: "value1" },
         {
@@ -139,11 +144,13 @@ describe("Client", () => {
       expect(client.delete).toBeDefined()
     })
 
-    test("call request", () => {
-      client.delete("/api/v1", null, {
+    test("call request", async () => {
+      await client.delete("/api/v1", null, {
         params: { key1: "value1", key2: "value2" },
       })
       expect(request).toHaveBeenLastCalledWith("DELETE", "https://test.com/api/v1", {
+        // linter complains of the asymmetric matcher "expect.anything()" - unsafe 'any' value assignment
+        // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
         headers: expect.anything(),
         body: null,
         params: { key1: "value1", key2: "value2" },
@@ -156,9 +163,11 @@ describe("Client", () => {
       expect(client.head).toBeDefined()
     })
 
-    test("call request", () => {
-      client.head("/api/v1", { params: { key1: "value1", key2: "value2" } })
+    test("call request", async () => {
+      await client.head("/api/v1", { params: { key1: "value1", key2: "value2" } })
       expect(request).toHaveBeenLastCalledWith("HEAD", "https://test.com/api/v1", {
+        // linter complains of the asymmetric matcher "expect.anything()" - unsafe 'any' value assignment
+        // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
         headers: expect.anything(),
         params: { key1: "value1", key2: "value2" },
       })