fix(secret): link serviceaccount (#950)

* fix(secrets): link unlink secrets to service accounts * fix(secrets): service account test
openshift · Jun 21, 2024 · f898233 · f898233
1 parent 668de24
commit f898233
Show file tree

Hide file tree

Showing 6 changed files with 172 additions and 10 deletions.
diff --git a/src/components/ImportForm/__tests__/GitImportForm.spec.tsx b/src/components/ImportForm/__tests__/GitImportForm.spec.tsx
@@ -65,6 +65,8 @@ describe('GitImportForm', () => {
   it('should call createResource on application submit', () => {
     routerRenderer(<GitImportForm applicationName={undefined} />);
     expect(screen.getByText('Create application')).toBeDisabled();
+    const componentButton = screen.getByText('Add a component');
+    fireEvent.click(componentButton);
     fireEvent.input(screen.getByPlaceholderText('Enter name'), { target: { value: 'test-app' } });
     expect(screen.getByText('Create application')).not.toBeDisabled();
     fireEvent.click(screen.getByText('Create application'));

diff --git a/src/components/Secrets/__tests___/secret-actions.spec.ts b/src/components/Secrets/__tests___/secret-actions.spec.ts
@@ -0,0 +1,40 @@
+import '@testing-library/jest-dom';
+import { renderHook } from '@testing-library/react-hooks';
+import { SecretType } from '../../../types';
+import { useAccessReviewForModel } from '../../../utils/rbac';
+import { useSecretActions } from '../secret-actions';
+
+jest.mock('../../../utils/rbac', () => ({
+  useAccessReviewForModel: jest.fn(() => [true, true]),
+}));
+
+jest.mock('../../modal/ModalProvider', () => ({
+  useModalLauncher: jest.fn(() => 'cta'),
+}));
+
+const useAccessReviewForModelMock = useAccessReviewForModel as jest.Mock;
+
+describe('useSecretActions', () => {
+  beforeEach(() => {
+    useAccessReviewForModelMock.mockReturnValue([true, true]);
+  });
+
+  it('should contain trigger actions', async () => {
+    const { result } = renderHook(() =>
+      useSecretActions({
+        metadata: { name: 'test-secret' },
+        type: SecretType.opaque,
+      } as any),
+    );
+    const actions = result.current;
+
+    expect(actions[0]).toEqual(
+      expect.objectContaining({
+        label: 'Delete',
+        disabledTooltip: "You don't have access to delete this secret",
+        id: 'delete-test-secret',
+        disabled: false,
+      }),
+    );
+  });
+});
diff --git a/src/components/Secrets/secret-actions.ts b/src/components/Secrets/secret-actions.ts
@@ -1,4 +1,4 @@
-import { RemoteSecretModel } from '../../models';
+import { SecretModel } from '../../models';
 import { Action } from '../../shared/components/action-menu/types';
 import { SecretKind } from '../../types';
 import { useAccessReviewForModel } from '../../utils/rbac';
@@ -7,7 +7,7 @@ import { secretDeleteModal } from './secret-modal';
 
 export const useSecretActions = (secret: SecretKind): Action[] => {
   const showModal = useModalLauncher();
-  const [canDelete] = useAccessReviewForModel(RemoteSecretModel, 'delete');
+  const [canDelete] = useAccessReviewForModel(SecretModel, 'delete');
   return [
     {
       cta: () => showModal(secretDeleteModal(secret)),

diff --git a/src/components/Secrets/utils/__tests__/service-account-utils.spec.ts b/src/components/Secrets/utils/__tests__/service-account-utils.spec.ts
@@ -58,6 +58,11 @@ describe('linkSecretToServiceAccount', () => {
             path: '/imagePullSecrets',
             value: [{ name: 'test-secret' }],
           },
+          {
+            op: 'replace',
+            path: '/secrets',
+            value: [{ name: 'test-secret' }],
+          },
         ],
       }),
     );
@@ -82,6 +87,37 @@ describe('linkSecretToServiceAccount', () => {
               { name: 'test-secret' },
             ],
           },
+          {
+            op: 'replace',
+            path: '/secrets',
+            value: [{ name: 'secret1' }, { name: 'secret2' }, { name: 'test-secret' }],
+          },
+        ],
+      }),
+    );
+  });
+
+  it('should create new array for empty list', async () => {
+    k8sGetResourceMock.mockReturnValue({
+      metadata: { name: 'test-cdq' },
+    });
+    k8sPatchResourceMock.mockClear();
+    await linkSecretToServiceAccount(imagePullSecret, 'test');
+    expect(k8sPatchResourceMock).toHaveBeenCalled();
+    expect(k8sPatchResourceMock).toHaveBeenCalledWith(
+      expect.objectContaining({
+        model: ServiceAccountModel,
+        patches: [
+          {
+            op: 'replace',
+            path: '/imagePullSecrets',
+            value: [{ name: 'test-secret' }],
+          },
+          {
+            op: 'replace',
+            path: '/secrets',
+            value: [{ name: 'test-secret' }],
+          },
         ],
       }),
     );
@@ -107,6 +143,51 @@ describe('UnLinkSecretFromServiceAccount', () => {
     expect(k8sPatchResourceMock).not.toHaveBeenCalled();
   });
 
+  it('should return early if no secrets array in service account ', async () => {
+    k8sGetResourceMock.mockReturnValue({
+      metadata: { name: 'test-cdq' },
+    });
+    k8sPatchResourceMock.mockClear();
+    await unLinkSecretFromServiceAccount(imagePullSecret, null);
+    expect(k8sPatchResourceMock).not.toHaveBeenCalled();
+  });
+
+  it('should return [] when no resources found ', async () => {
+    k8sGetResourceMock.mockReturnValue({
+      metadata: { name: 'test-cdq' },
+      imagePullSecrets: [{ name: 'random-secret' }],
+      secrets: [],
+    });
+    k8sPatchResourceMock.mockClear();
+    await unLinkSecretFromServiceAccount(
+      {
+        metadata: { name: 'ip-secret3' },
+        type: imagePullSecret.type,
+        kind: imagePullSecret.kind,
+        apiVersion: imagePullSecret.apiVersion,
+      },
+      'test',
+    );
+    expect(k8sPatchResourceMock).toHaveBeenCalled();
+    expect(k8sPatchResourceMock).toHaveBeenCalledWith(
+      expect.objectContaining({
+        model: ServiceAccountModel,
+        patches: [
+          {
+            op: 'replace',
+            path: '/imagePullSecrets',
+            value: [{ name: 'random-secret' }],
+          },
+          {
+            op: 'replace',
+            path: '/secrets',
+            value: [],
+          },
+        ],
+      }),
+    );
+  });
+
   it('should remove correct imagePull secrets list ', async () => {
     k8sPatchResourceMock.mockClear();
     await unLinkSecretFromServiceAccount(
@@ -128,6 +209,11 @@ describe('UnLinkSecretFromServiceAccount', () => {
             path: '/imagePullSecrets',
             value: [{ name: 'ip-secret1' }, { name: 'ip-secret2' }, { name: 'ip-secret4' }],
           },
+          {
+            op: 'replace',
+            path: '/secrets',
+            value: [{ name: 'secret1' }, { name: 'secret2' }],
+          },
         ],
       }),
     );

diff --git a/src/components/Secrets/utils/service-account-utils.ts b/src/components/Secrets/utils/service-account-utils.ts
@@ -11,10 +11,17 @@ export const linkSecretToServiceAccount = async (secret: SecretKind, namespace:
     model: ServiceAccountModel,
     queryOptions: { name: PIPELINE_SERVICE_ACCOUNT, ns: namespace },
   });
-  const existingSecrets = serviceAccount?.imagePullSecrets as SecretKind[];
+
+  const existingIPSecrets = serviceAccount?.imagePullSecrets as SecretKind[];
+  const imagePullSecretList = existingIPSecrets
+    ? [...existingIPSecrets, { name: secret.metadata.name }]
+    : [{ name: secret.metadata.name }];
+
+  const existingSecrets = serviceAccount?.secrets as SecretKind[];
   const secretList = existingSecrets
     ? [...existingSecrets, { name: secret.metadata.name }]
     : [{ name: secret.metadata.name }];
+
   k8sPatchResource({
     model: ServiceAccountModel,
     queryOptions: {
@@ -25,6 +32,11 @@ export const linkSecretToServiceAccount = async (secret: SecretKind, namespace:
       {
         op: 'replace',
         path: `/imagePullSecrets`,
+        value: imagePullSecretList,
+      },
+      {
+        op: 'replace',
+        path: `/secrets`,
         value: secretList,
       },
     ],
@@ -40,14 +52,31 @@ export const unLinkSecretFromServiceAccount = async (secret: SecretKind, namespa
     queryOptions: { name: PIPELINE_SERVICE_ACCOUNT, ns: namespace ?? secret.metadata?.namespace },
   });
 
-  const existingSecrets = serviceAccount?.imagePullSecrets;
-  if (!Array.isArray(existingSecrets) || existingSecrets.length === 0) {
+  const existingIPSecrets = serviceAccount?.imagePullSecrets;
+  const existingSecrets = serviceAccount?.secrets;
+
+  if (
+    Array.isArray(existingIPSecrets) &&
+    existingIPSecrets.length === 0 &&
+    Array.isArray(existingSecrets) &&
+    existingSecrets.length === 0
+  ) {
     return;
   }
 
-  const secretList = (existingSecrets as { [key: string]: string }[]).filter(
-    (s) => s.name !== secret.metadata?.name,
-  );
+  const imagePullSecretsList =
+    Array.isArray(existingIPSecrets) && existingIPSecrets.length >= 0
+      ? (existingIPSecrets as { [key: string]: string }[]).filter(
+          (s) => s.name !== secret.metadata?.name,
+        )
+      : [];
+
+  const secretsList =
+    Array.isArray(existingSecrets) && existingSecrets.length >= 0
+      ? (existingSecrets as { [key: string]: string }[]).filter(
+          (s) => s.name !== secret.metadata?.name,
+        )
+      : [];
 
   k8sPatchResource({
     model: ServiceAccountModel,
@@ -59,7 +88,12 @@ export const unLinkSecretFromServiceAccount = async (secret: SecretKind, namespa
       {
         op: 'replace',
         path: `/imagePullSecrets`,
-        value: secretList,
+        value: imagePullSecretsList,
+      },
+      {
+        op: 'replace',
+        path: `/secrets`,
+        value: secretsList,
       },
     ],
   });

diff --git a/src/types/secret.ts b/src/types/secret.ts
@@ -129,7 +129,7 @@ export enum SecretType {
 export type ServiceAccountKind = {
   automountServiceAccountToken?: boolean;
   imagePullSecrets?: SecretKind[] | { [key: string]: string }[];
-  secrets?: SecretKind[] | { [key: string]: string };
+  secrets?: SecretKind[] | { [key: string]: string }[];
 } & K8sResourceCommon;
 
 export enum SecretTypeAbstraction {