Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not cache the service account token in the devWorkspace client #1283

Merged
merged 1 commit into from
Jan 2, 2025
Merged

Do not cache the service account token in the devWorkspace client #1283

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/dashboard-backend/src/__tests__/app.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const mockProcessExit = jest.fn();
});

jest.mock('../routes/api/helpers/getDevWorkspaceClient.ts');
jest.mock('../routes/api/helpers/getServiceAccountToken.ts');
describe('App', () => {
afterEach(() => {
jest.clearAllMocks();
Expand Down
7 changes: 4 additions & 3 deletions packages/dashboard-backend/src/app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ import { registerEditorsRoutes } from '@/routes/api/editors';
import { registerEventsRoutes } from '@/routes/api/events';
import { registerGettingStartedSamplesRoutes } from '@/routes/api/gettingStartedSample';
import { registerGitConfigRoutes } from '@/routes/api/gitConfig';
import { getDevWorkspaceSingletonClient } from '@/routes/api/helpers/getDevWorkspaceClient';
import { getDevWorkspaceClient } from '@/routes/api/helpers/getDevWorkspaceClient';
import { getServiceAccountToken } from '@/routes/api/helpers/getServiceAccountToken';
import { registerKubeConfigRoute } from '@/routes/api/kubeConfig';
import { registerPersonalAccessTokenRoutes } from '@/routes/api/personalAccessToken';
import { registerPodmanLoginRoute } from '@/routes/api/podmanLogin';
Expand Down Expand Up @@ -71,8 +72,8 @@ export default async function buildApp(server: FastifyInstance): Promise<unknown
},
);

const { devWorkspaceClusterServiceApi } = getDevWorkspaceSingletonClient();
await devWorkspaceClusterServiceApi.watchInAllNamespaces();
const devWorkspaceClient = getDevWorkspaceClient(getServiceAccountToken());
await devWorkspaceClient.devWorkspaceClusterApi.watchInAllNamespaces();

server.register(import('@fastify/rate-limit'));

Expand Down
31 changes: 3 additions & 28 deletions packages/dashboard-backend/src/devworkspaceClient/__tests__/index.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import * as mockClientNode from '@kubernetes/client-node';
import { KubeConfig } from '@kubernetes/client-node';

import { DevWorkspaceClient } from '@/devworkspaceClient';
import { DevWorkspaceApiService } from '@/devworkspaceClient/services/devWorkspaceApi';
import { DevWorkspaceClusterApiService } from '@/devworkspaceClient/services/devWorkspaceClusterApiService';
import { DevWorkspaceTemplateApiService } from '@/devworkspaceClient/services/devWorkspaceTemplateApi';
Expand All @@ -27,9 +28,7 @@ import { ServerConfigApiService } from '@/devworkspaceClient/services/serverConf
import { SshKeysService } from '@/devworkspaceClient/services/sshKeysApi';
import { UserProfileApiService } from '@/devworkspaceClient/services/userProfileApi';

import { DevWorkspaceClient, DevWorkspaceSingletonClient } from '..';

jest.mock('../services/devWorkspaceApi.ts');
jest.mock('@/devworkspaceClient/services/devWorkspaceApi.ts');

describe('DevWorkspace client', () => {
let config: KubeConfig;
Expand All @@ -50,6 +49,7 @@ describe('DevWorkspace client', () => {
expect(client.devworkspaceApi).toBeInstanceOf(DevWorkspaceApiService);
expect(client.dockerConfigApi).toBeInstanceOf(DockerConfigApiService);
expect(client.eventApi).toBeInstanceOf(EventApiService);
expect(client.devWorkspaceClusterApi).toBeInstanceOf(DevWorkspaceClusterApiService);
expect(client.kubeConfigApi).toBeInstanceOf(KubeConfigApiService);
expect(client.logsApi).toBeInstanceOf(LogsApiService);
expect(client.podApi).toBeInstanceOf(PodApiService);
Expand All @@ -60,28 +60,3 @@ describe('DevWorkspace client', () => {
expect(client.sshKeysApi).toBeInstanceOf(SshKeysService);
});
});

describe('DevWorkspace singleton client', () => {
let config: KubeConfig;
beforeEach(() => {
const { KubeConfig } = mockClientNode;
config = new KubeConfig();
config.makeApiClient = jest.fn().mockImplementation(() => ({}));
});

afterEach(() => {
jest.clearAllMocks();
});

test('client', () => {
const client = DevWorkspaceSingletonClient.getInstance(config);

expect(client.devWorkspaceClusterServiceApi).toBeInstanceOf(DevWorkspaceClusterApiService);
});

test('return same instance', () => {
const client = DevWorkspaceSingletonClient.getInstance(config);

expect(client.devWorkspaceClusterServiceApi).toEqual(client.devWorkspaceClusterServiceApi);
});
});
24 changes: 4 additions & 20 deletions packages/dashboard-backend/src/devworkspaceClient/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import {
IDevWorkspaceApi,
IDevWorkspaceClient,
IDevWorkspaceClusterApi,
IDevWorkspaceSingletonClient,
IDevWorkspaceTemplateApi,
IDockerConfigApi,
IEditorsApi,
Expand Down Expand Up @@ -110,6 +109,10 @@ export class DevWorkspaceClient implements IDevWorkspaceClient {
return new GitConfigApiService(this.kubeConfig);
}

get devWorkspaceClusterApi(): IDevWorkspaceClusterApi {
return new DevWorkspaceClusterApiService(this.kubeConfig);
}

get gettingStartedSampleApi(): IGettingStartedSampleApi {
return new GettingStartedSamplesApiService(this.kubeConfig);
}
Expand All @@ -130,22 +133,3 @@ export class DevWorkspaceClient implements IDevWorkspaceClient {
return new WorkspacePreferencesApiService(this.kubeConfig);
}
}

let devWorkspaceSingletonClient: DevWorkspaceSingletonClient;

/**
* Singleton client for devworkspace services.
*/
export class DevWorkspaceSingletonClient implements IDevWorkspaceSingletonClient {
static getInstance(kc: k8s.KubeConfig): DevWorkspaceSingletonClient {
if (!devWorkspaceSingletonClient) {
devWorkspaceSingletonClient = new DevWorkspaceSingletonClient(kc);
}
return devWorkspaceSingletonClient;
}

public readonly devWorkspaceClusterServiceApi: IDevWorkspaceClusterApi;
private constructor(kc: k8s.KubeConfig) {
this.devWorkspaceClusterServiceApi = new DevWorkspaceClusterApiService(kc);
}
}
1 change: 1 addition & 0 deletions packages/dashboard-backend/src/routes/__tests__/factoryAcceptanceRedirect.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { FastifyInstance } from 'fastify';
import { setup, teardown } from '@/utils/appBuilder';

jest.mock('../api/helpers/getDevWorkspaceClient.ts');
jest.mock('../api/helpers/getServiceAccountToken.ts');
describe('Factory Acceptance Redirect', () => {
let app: FastifyInstance;

Expand Down
1 change: 1 addition & 0 deletions packages/dashboard-backend/src/routes/api/__tests__/clusterInfo.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { baseApiPath } from '@/constants/config';
import { setup, teardown } from '@/utils/appBuilder';

jest.mock('../helpers/getDevWorkspaceClient.ts');
jest.mock('../helpers/getServiceAccountToken.ts');
describe('Cluster Info Route', () => {
let app: FastifyInstance;
const clusterConsoleUrl = 'cluster-console-url';
Expand Down
1 change: 1 addition & 0 deletions packages/dashboard-backend/src/routes/api/__tests__/dataResolver.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { setup, teardown } from '@/utils/appBuilder';

jest.mock('@/routes/api/helpers/getCertificateAuthority');
jest.mock('../helpers/getDevWorkspaceClient.ts');
jest.mock('../helpers/getServiceAccountToken.ts');
const axiosInstanceMock = jest.fn();
(axiosInstance.get as jest.Mock).mockImplementation(axiosInstanceMock);
const defaultAxiosInstanceMock = jest.fn();
Expand Down
1 change: 1 addition & 0 deletions packages/dashboard-backend/src/routes/api/__tests__/devworkspaceTemplates.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
import { setup, teardown } from '@/utils/appBuilder';

jest.mock('../helpers/getDevWorkspaceClient.ts');
jest.mock('../helpers/getServiceAccountToken.ts');
jest.mock('../helpers/getToken.ts');

describe('DevWorkspaceTemplates Routes', () => {
Expand Down
1 change: 1 addition & 0 deletions packages/dashboard-backend/src/routes/api/__tests__/devworkspaces.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { setup, teardown } from '@/utils/appBuilder';

jest.mock('../helpers/getDevWorkspaceClient.ts');
jest.mock('../helpers/getToken.ts');
jest.mock('../helpers/getServiceAccountToken.ts');

describe('DevWorkspaces Routes', () => {
let app: FastifyInstance;
Expand Down
1 change: 1 addition & 0 deletions packages/dashboard-backend/src/routes/api/__tests__/dockerConfig.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { setup, teardown } from '@/utils/appBuilder';

jest.mock('../helpers/getDevWorkspaceClient.ts');
jest.mock('../helpers/getToken.ts');
jest.mock('../helpers/getServiceAccountToken.ts');

describe('Docker Config Routes', () => {
let app: FastifyInstance;
Expand Down
1 change: 1 addition & 0 deletions packages/dashboard-backend/src/routes/api/__tests__/events.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { setup, teardown } from '@/utils/appBuilder';

jest.mock('../helpers/getToken.ts');
jest.mock('../helpers/getDevWorkspaceClient.ts');
jest.mock('../helpers/getServiceAccountToken.ts');

describe('Events Route', () => {
let app: FastifyInstance;
Expand Down
5 changes: 2 additions & 3 deletions packages/dashboard-backend/src/routes/api/__tests__/gitConfig.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,12 @@ jest.mock('../helpers/getDevWorkspaceClient.ts', () => ({
read: mockRead,
patch: mockPatch,
},
}),
getDevWorkspaceSingletonClient: () => ({
devWorkspaceClusterServiceApi: {
devWorkspaceClusterApi: {
watchInAllNamespaces: jest.fn().mockResolvedValueOnce(true),
},
}),
}));
jest.mock('../helpers/getServiceAccountToken.ts');
jest.mock('../helpers/getToken.ts');

describe('Gitconfig Routes', () => {
Expand Down
1 change: 1 addition & 0 deletions packages/dashboard-backend/src/routes/api/__tests__/kubeConfig.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { setup, teardown } from '@/utils/appBuilder';

jest.mock('../helpers/getDevWorkspaceClient.ts');
jest.mock('../helpers/getToken.ts');
jest.mock('../helpers/getServiceAccountToken.ts');

describe('Kube Config Route', () => {
let app: FastifyInstance;
Expand Down
1 change: 1 addition & 0 deletions packages/dashboard-backend/src/routes/api/__tests__/personalAccessToken.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { setup, teardown } from '@/utils/appBuilder';

jest.mock('../helpers/getToken.ts');
jest.mock('../helpers/getDevWorkspaceClient.ts');
jest.mock('../helpers/getServiceAccountToken.ts');

describe('Personal Access Token Route', () => {
let app: FastifyInstance;
Expand Down
1 change: 1 addition & 0 deletions packages/dashboard-backend/src/routes/api/__tests__/pods.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { setup, teardown } from '@/utils/appBuilder';

jest.mock('../helpers/getToken.ts');
jest.mock('../helpers/getDevWorkspaceClient.ts');
jest.mock('../helpers/getServiceAccountToken.ts');

describe('Pods Route', () => {
let app: FastifyInstance;
Expand Down
1 change: 1 addition & 0 deletions packages/dashboard-backend/src/routes/api/__tests__/sshKeys.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { setup, teardown } from '@/utils/appBuilder';

jest.mock('../helpers/getToken.ts');
jest.mock('../helpers/getDevWorkspaceClient.ts');
jest.mock('../helpers/getServiceAccountToken.ts');

describe('SSH Keys Route', () => {
let app: FastifyInstance;
Expand Down
1 change: 1 addition & 0 deletions packages/dashboard-backend/src/routes/api/__tests__/workspacePreferences.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { setup, teardown } from '@/utils/appBuilder';

jest.mock('../helpers/getToken.ts');
jest.mock('../helpers/getDevWorkspaceClient.ts');
jest.mock('../helpers/getServiceAccountToken.ts');

describe('Workspace Preferences', () => {
let app: FastifyInstance;
Expand Down
7 changes: 4 additions & 3 deletions packages/dashboard-backend/src/routes/api/devworkspaceCluster.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
import { FastifyInstance } from 'fastify';

import { baseApiPath } from '@/constants/config';
import { getDevWorkspaceSingletonClient } from '@/routes/api/helpers/getDevWorkspaceClient';
import { getDevWorkspaceClient } from '@/routes/api/helpers/getDevWorkspaceClient';
import { getServiceAccountToken } from '@/routes/api/helpers/getServiceAccountToken';
import { getSchema } from '@/services/helpers';

const tags = ['Devworkspace Cluster'];
Expand All @@ -24,8 +25,8 @@ export function registerDevWorkspaceClusterRoutes(instance: FastifyInstance) {
`${baseApiPath}/devworkspace/running-workspaces-cluster-limit-exceeded`,
getSchema({ tags }),
async function () {
const { devWorkspaceClusterServiceApi } = getDevWorkspaceSingletonClient();
return await devWorkspaceClusterServiceApi.isRunningWorkspacesClusterLimitExceeded();
const devWorkspaceClient = getDevWorkspaceClient(getServiceAccountToken());
return await devWorkspaceClient.devWorkspaceClusterApi.isRunningWorkspacesClusterLimitExceeded();
},
);
});
Expand Down
22 changes: 7 additions & 15 deletions packages/dashboard-backend/src/routes/api/helpers/__mocks__/getDevWorkspaceClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import { IncomingHttpHeaders } from 'http';

import {
DevWorkspaceClient,
DevWorkspaceSingletonClient,
IDevWorkspaceApi,
IDevWorkspaceClusterApi,
IDevWorkspaceTemplateApi,
Expand Down Expand Up @@ -244,6 +243,13 @@ export const getDevWorkspaceClient = jest.fn(
read: _namespace => Promise.resolve({} as api.IGitConfig),
patch: (_namespace, _gitconfig) => Promise.resolve({} as api.IGitConfig),
} as IGitConfigApi,
devWorkspaceClusterApi: {
isRunningWorkspacesClusterLimitExceeded: () =>
Promise.resolve(stubIsRunningWorkspaceClusterLimitExceeded),
watchInAllNamespaces(): Promise<void> {
return Promise.resolve();
},
} as IDevWorkspaceClusterApi,
sshKeysApi: {
add: (_namespace, _sshKey) => Promise.resolve({} as api.SshKey),
delete: (_namespace, _name) => Promise.resolve(),
Expand All @@ -258,17 +264,3 @@ export const getDevWorkspaceClient = jest.fn(
} as DevWorkspaceClient;
},
);

export const getDevWorkspaceSingletonClient = jest.fn(
(..._args: Parameters<typeof helper>): DevWorkspaceSingletonClient => {
return {
devWorkspaceClusterServiceApi: {
isRunningWorkspacesClusterLimitExceeded: () =>
Promise.resolve(stubIsRunningWorkspaceClusterLimitExceeded),
watchInAllNamespaces(): Promise<void> {
return Promise.resolve();
},
} as IDevWorkspaceClusterApi,
};
},
);
8 changes: 1 addition & 7 deletions packages/dashboard-backend/src/routes/api/helpers/getDevWorkspaceClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,8 @@
* Red Hat, Inc. - initial API and implementation
*/

import { DevWorkspaceClient, DevWorkspaceSingletonClient } from '@/devworkspaceClient';
import { DevWorkspaceClient } from '@/devworkspaceClient';
import { DwClientProvider } from '@/services/kubeclient/dwClientProvider';
import { DwSingletonClientProvider } from '@/services/kubeclient/dwSingletonClientProvider';

/**
* Creates DevWorkspace Client depending on the context for the specified request.
Expand All @@ -21,8 +20,3 @@ export function getDevWorkspaceClient(token: string): DevWorkspaceClient {
const dwClientProvider = new DwClientProvider();
return dwClientProvider.getDWClient(token);
}

export function getDevWorkspaceSingletonClient(): DevWorkspaceSingletonClient {
const dwSingletonClientProvider = DwSingletonClientProvider.getInstance();
return dwSingletonClientProvider.getDWSingletonClient();
}
31 changes: 0 additions & 31 deletions ...ges/dashboard-backend/src/services/kubeclient/__tests__/dwSingletonClientProvider.spec.ts
View file
Open in desktop

This file was deleted.

44 changes: 0 additions & 44 deletions packages/dashboard-backend/src/services/kubeclient/dwSingletonClientProvider.ts
View file
Open in desktop

This file was deleted.

Loading