chore(Coder plugin): update all Backstage code to use preview SDK (#131)

* chore: add vendored version of experimental Coder SDK

* chore: update CoderClient class to use new SDK

* chore: delete mock SDK

* fix: improve data hiding for CoderSdk

* docs: update typo

* wip: commit progress on updating Coder client

* wip: commit more progress on updating types

* chore: remove valibot type definitions from global constants file

* chore: rename mocks file

* fix: update type mismatches

* wip: commit more update progress

* wip: commit progress on updating client/SDK integration

* fix: get all tests passing for CoderClient

* fix: update UrlSync updates

* fix: get all tests passing

* chore: update all mock data to use Coder core entity mocks

* fix: add extra helpers to useCoderSdk

* fix: add additional properties to hide from SDK

* fix: shrink down the API of useCoderSdk

* update method name for clarity

* chore: removal vestigal endpoint properties

Author: Parkreiner

plugins/backstage-plugin-coder/src/api/CoderClient.test.ts

@@ -1,8 +1,4 @@
-import {
-  CODER_AUTH_HEADER_KEY,
-  CoderClient,
-  disabledClientError,
-} from './CoderClient';
+import { CODER_AUTH_HEADER_KEY, CoderClient } from './CoderClient';
 import type { IdentityApi } from '@backstage/core-plugin-api';
 import { UrlSync } from './UrlSync';
 import { rest } from 'msw';
@@ -12,8 +8,8 @@ import { delay } from '../utils/time';
 import {
   mockWorkspacesList,
   mockWorkspacesListForRepoSearch,
-} from '../testHelpers/mockCoderAppData';
-import type { Workspace, WorkspacesResponse } from '../typesConstants';
+} from '../testHelpers/mockCoderPluginData';
+import type { Workspace, WorkspacesResponse } from './vendoredSdk';
 import {
   getMockConfigApi,
   getMockDiscoveryApi,
@@ -100,50 +96,6 @@ describe(`${CoderClient.name}`, () => {
     });
   });
 
-  describe('cleanupClient functionality', () => {
-    it('Will prevent any new SDK requests from going through', async () => {
-      const client = new CoderClient({ apis: getConstructorApis() });
-      client.cleanupClient();
-
-      // Request should fail, even though token is valid
-      await expect(() => {
-        return client.syncToken(mockCoderAuthToken);
-      }).rejects.toThrow(disabledClientError);
-
-      await expect(() => {
-        return client.sdk.getWorkspaces({
-          q: 'owner:me',
-          limit: 0,
-        });
-      }).rejects.toThrow(disabledClientError);
-    });
-
-    it('Will abort any pending requests', async () => {
-      const client = new CoderClient({
-        initialToken: mockCoderAuthToken,
-        apis: getConstructorApis(),
-      });
-
-      // Sanity check to ensure that request can still go through normally
-      const workspacesPromise1 = client.sdk.getWorkspaces({
-        q: 'owner:me',
-        limit: 0,
-      });
-
-      await expect(workspacesPromise1).resolves.toEqual<WorkspacesResponse>({
-        workspaces: mockWorkspacesList,
-        count: mockWorkspacesList.length,
-      });
-
-      const workspacesPromise2 = client.sdk.getWorkspaces({
-        q: 'owner:me',
-        limit: 0,
-      });
-      client.cleanupClient();
-      await expect(() => workspacesPromise2).rejects.toThrow();
-    });
-  });
-
   // Eventually the Coder SDK is going to get too big to test every single
   // function. Focus tests on the functionality specifically being patched in
   // for Backstage
@@ -180,10 +132,10 @@ describe(`${CoderClient.name}`, () => {
       });
 
       const { urlSync } = apis;
-      const apiEndpoint = await urlSync.getApiEndpoint();
+      const assetsEndpoint = await urlSync.getAssetsEndpoint();
 
-      const allWorkspacesAreRemapped = !workspaces.some(ws =>
-        ws.template_icon.startsWith(apiEndpoint),
+      const allWorkspacesAreRemapped = workspaces.every(ws =>
+        ws.template_icon.startsWith(assetsEndpoint),
       );
 
       expect(allWorkspacesAreRemapped).toBe(true);

plugins/backstage-plugin-coder/src/api/CoderClient.ts

@@ -1,19 +1,19 @@
-import globalAxios, {
+import {
   AxiosError,
-  type AxiosInstance,
   type InternalAxiosRequestConfig as RequestConfig,
 } from 'axios';
 import { type IdentityApi, createApiRef } from '@backstage/core-plugin-api';
-import {
-  type Workspace,
-  CODER_API_REF_ID_PREFIX,
-  WorkspacesRequest,
-  WorkspacesResponse,
-  User,
-} from '../typesConstants';
+import { CODER_API_REF_ID_PREFIX } from '../typesConstants';
 import type { UrlSync } from './UrlSync';
 import type { CoderWorkspacesConfig } from '../hooks/useCoderWorkspacesConfig';
-import { CoderSdk } from './MockCoderSdk';
+import {
+  type CoderSdk,
+  type User,
+  type Workspace,
+  type WorkspacesRequest,
+  type WorkspacesResponse,
+  makeCoderSdk,
+} from './vendoredSdk';
 
 export const CODER_AUTH_HEADER_KEY = 'Coder-Session-Token';
 const DEFAULT_REQUEST_TIMEOUT_MS = 20_000;
@@ -39,11 +39,6 @@ type CoderClientApi = Readonly<{
    * Return value indicates whether the token is valid.
    */
   syncToken: (newToken: string) => Promise<boolean>;
-
-  /**
-   * Cleans up a client instance, removing its links to all external systems.
-   */
-  cleanupClient: () => void;
 }>;
 
 const sharedCleanupAbortReason = new DOMException(
@@ -59,19 +54,30 @@ export const disabledClientError = new Error(
 );
 
 type ConstructorInputs = Readonly<{
+  /**
+   * initialToken is strictly for testing, and is basically limited to making it
+   * easier to test API logic.
+   *
+   * If trying to test UI logic that depends on CoderClient, it's probably
+   * better to interact with CoderClient indirectly through the auth components,
+   * so that React state is aware of everything.
+   */
   initialToken?: string;
-  requestTimeoutMs?: number;
 
+  requestTimeoutMs?: number;
   apis: Readonly<{
     urlSync: UrlSync;
     identityApi: IdentityApi;
   }>;
 }>;
 
+type RequestInterceptor = (
+  config: RequestConfig,
+) => RequestConfig | Promise<RequestConfig>;
+
 export class CoderClient implements CoderClientApi {
   private readonly urlSync: UrlSync;
   private readonly identityApi: IdentityApi;
-  private readonly axios: AxiosInstance;
 
   private readonly requestTimeoutMs: number;
   private readonly cleanupController: AbortController;
@@ -82,33 +88,28 @@ export class CoderClient implements CoderClientApi {
 
   constructor(inputs: ConstructorInputs) {
     const {
-      apis,
       initialToken,
+      apis: { urlSync, identityApi },
       requestTimeoutMs = DEFAULT_REQUEST_TIMEOUT_MS,
     } = inputs;
-    const { urlSync, identityApi } = apis;
 
     this.urlSync = urlSync;
     this.identityApi = identityApi;
-    this.axios = globalAxios.create();
-
     this.loadedSessionToken = initialToken;
     this.requestTimeoutMs = requestTimeoutMs;
-
     this.cleanupController = new AbortController();
     this.trackedEjectionIds = new Set();
 
-    this.sdk = this.getBackstageCoderSdk(this.axios);
+    this.sdk = this.createBackstageCoderSdk();
     this.addBaseRequestInterceptors();
   }
 
   private addRequestInterceptor(
-    requestInterceptor: (
-      config: RequestConfig,
-    ) => RequestConfig | Promise<RequestConfig>,
+    requestInterceptor: RequestInterceptor,
     errorInterceptor?: (error: unknown) => unknown,
   ): number {
-    const ejectionId = this.axios.interceptors.request.use(
+    const axios = this.sdk.getAxiosInstance();
+    const ejectionId = axios.interceptors.request.use(
       requestInterceptor,
       errorInterceptor,
     );
@@ -120,7 +121,8 @@ export class CoderClient implements CoderClientApi {
   private removeRequestInterceptorById(ejectionId: number): boolean {
     // Even if we somehow pass in an ID that hasn't been associated with the
     // Axios instance, that's a noop. No harm in calling method no matter what
-    this.axios.interceptors.request.eject(ejectionId);
+    const axios = this.sdk.getAxiosInstance();
+    axios.interceptors.request.eject(ejectionId);
 
     if (!this.trackedEjectionIds.has(ejectionId)) {
       return false;
@@ -179,10 +181,8 @@ export class CoderClient implements CoderClientApi {
     this.addRequestInterceptor(baseRequestInterceptor, baseErrorInterceptor);
   }
 
-  private getBackstageCoderSdk(
-    axiosInstance: AxiosInstance,
-  ): BackstageCoderSdk {
-    const baseSdk = new CoderSdk(axiosInstance);
+  private createBackstageCoderSdk(): BackstageCoderSdk {
+    const baseSdk = makeCoderSdk();
 
     const getWorkspaces: (typeof baseSdk)['getWorkspaces'] = async request => {
       const workspacesRes = await baseSdk.getWorkspaces(request);
@@ -335,23 +335,6 @@ export class CoderClient implements CoderClientApi {
       this.removeRequestInterceptorById(validationId);
     }
   };
-
-  cleanupClient = (): void => {
-    this.trackedEjectionIds.forEach(id => {
-      this.axios.interceptors.request.eject(id);
-    });
-
-    this.trackedEjectionIds.clear();
-    this.cleanupController.abort(sharedCleanupAbortReason);
-    this.loadedSessionToken = undefined;
-
-    // Not using this.addRequestInterceptor, because we don't want to track this
-    // interceptor at all. It should never be ejected once the client has been
-    // disabled
-    this.axios.interceptors.request.use(() => {
-      throw disabledClientError;
-    });
-  };
 }
 
 function appendParamToQuery(

plugins/backstage-plugin-coder/src/api/MockCoderSdk.ts

@@ -4,8 +4,8 @@ import {
   getMockConfigApi,
   getMockDiscoveryApi,
   mockBackstageAssetsEndpoint,
-  mockBackstageApiEndpoint,
   mockBackstageUrlRoot,
+  mockBackstageApiEndpointWithoutSdkPath,
 } from '../testHelpers/mockBackstageData';
 
 // Tests have to assume that DiscoveryApi and ConfigApi will always be in sync,
@@ -23,7 +23,7 @@ describe(`${UrlSync.name}`, () => {
     const cachedUrls = urlSync.getCachedUrls();
     expect(cachedUrls).toEqual<UrlSyncSnapshot>({
       baseUrl: mockBackstageUrlRoot,
-      apiRoute: mockBackstageApiEndpoint,
+      apiRoute: mockBackstageApiEndpointWithoutSdkPath,
       assetsRoute: mockBackstageAssetsEndpoint,
     });
   });
@@ -50,7 +50,7 @@ describe(`${UrlSync.name}`, () => {
 
     expect(newSnapshot).toEqual<UrlSyncSnapshot>({
       baseUrl: 'blah',
-      apiRoute: 'blah/coder/api/v2',
+      apiRoute: 'blah/coder',
       assetsRoute: 'blah/coder',
     });
   });
@@ -76,7 +76,7 @@ describe(`${UrlSync.name}`, () => {
 
     expect(onChange).toHaveBeenCalledWith({
       baseUrl: 'blah',
-      apiRoute: 'blah/coder/api/v2',
+      apiRoute: 'blah/coder',
       assetsRoute: 'blah/coder',
     } satisfies UrlSyncSnapshot);
 

plugins/backstage-plugin-coder/src/api/UrlSync.test.ts

@@ -42,14 +42,10 @@ const PROXY_URL_KEY_FOR_DISCOVERY_API = 'proxy';
 
 type UrlPrefixes = Readonly<{
   proxyPrefix: string;
-  apiRoutePrefix: string;
-  assetsRoutePrefix: string;
 }>;
 
 export const defaultUrlPrefixes = {
   proxyPrefix: `/api/proxy`,
-  apiRoutePrefix: '/api/v2',
-  assetsRoutePrefix: '', // Deliberately left as empty string
 } as const satisfies UrlPrefixes;
 
 export type UrlSyncSnapshot = Readonly<{
@@ -104,12 +100,10 @@ export class UrlSync implements UrlSyncApi {
   }
 
   private prepareNewSnapshot(newProxyUrl: string): UrlSyncSnapshot {
-    const { assetsRoutePrefix, apiRoutePrefix } = this.urlPrefixes;
-
     return {
       baseUrl: newProxyUrl.replace(proxyRouteReplacer, ''),
-      assetsRoute: `${newProxyUrl}${CODER_PROXY_PREFIX}${assetsRoutePrefix}`,
-      apiRoute: `${newProxyUrl}${CODER_PROXY_PREFIX}${apiRoutePrefix}`,
+      assetsRoute: `${newProxyUrl}${CODER_PROXY_PREFIX}`,
+      apiRoute: `${newProxyUrl}${CODER_PROXY_PREFIX}`,
     };
   }