Merge pull request #7769 from karthikjeeyar/fix-image-creds

Bug 1914209: Associate image secret name to pipeline serviceaccount imagePullSecrets

frontend/packages/pipelines-plugin/src/components/pipelines/modals/common/PipelineSecretSection.tsx

@@ -51,7 +51,11 @@ const PipelineSecretSection: React.FC<PipelineSecretSectionProps> = ({ namespace
       .then((resp) => {
         actions.setSubmitting(false);
         setFieldValue(secretOpenField.name, false);
-        associateServiceAccountToSecret(resp, namespace);
+        associateServiceAccountToSecret(
+          resp,
+          namespace,
+          values.annotations.key === SecretAnnotationId.Image,
+        );
       })
       .catch((err) => {
         actions.setSubmitting(false);

frontend/packages/pipelines-plugin/src/utils/__tests__/pipeline-serviceaccount-test-data.ts

@@ -0,0 +1,19 @@
+export const mockPipelineServiceAccount = {
+  kind: 'ServiceAccount',
+  apiVersion: 'v1',
+  metadata: {
+    name: 'pipeline',
+    namespace: 'karthik',
+    uid: 'fc2e60f5-d42f-4ba7-82ee-e03be2cbe2a8',
+  },
+  secrets: [
+    {
+      name: 'pipeline-dockercfg-p2jwc',
+    },
+  ],
+  imagePullSecrets: [
+    {
+      name: 'pipeline-dockercfg-p2jwc',
+    },
+  ],
+};

frontend/packages/pipelines-plugin/src/utils/__tests__/pipeline-utils.spec.ts

@@ -5,6 +5,7 @@ import {
   LOG_SOURCE_RUNNING,
   LOG_SOURCE_TERMINATED,
 } from '@console/internal/components/utils';
+import * as k8s from '@console/internal/module/k8s';
 import { ContainerStatus } from '@console/internal/module/k8s';
 import { SecretAnnotationId } from '../../components/pipelines/const';
 import { runStatus } from '../pipeline-augment';
@@ -18,12 +19,18 @@ import {
   calculateRelativeTime,
   hasInlineTaskSpec,
   LatestPipelineRunStatus,
+  updateServiceAccount,
 } from '../pipeline-utils';
 import {
   constructPipelineData,
   mockPipelinesJSON,
   mockRunDurationTest,
 } from './pipeline-test-data';
+import { mockPipelineServiceAccount } from './pipeline-serviceaccount-test-data';
+
+beforeAll(() => {
+  jest.spyOn(k8s, 'k8sUpdate').mockImplementation((model, data) => data);
+});
 
 describe('pipeline-utils ', () => {
   it('For first pipeline there should be 1 stages of length 2', () => {
@@ -167,4 +174,30 @@ describe('pipeline-utils ', () => {
     const hasSpec = hasInlineTaskSpec(mockPipelinesJSON[0].spec.tasks);
     expect(hasSpec).toBe(false);
   });
+
+  it('expect service account to have secret name available only in secrets property', async () => {
+    const gitSecretName = 'git-secret';
+    const serviceAccount = await updateServiceAccount(
+      gitSecretName,
+      mockPipelineServiceAccount,
+      false,
+    );
+    expect(serviceAccount.secrets.find((s) => s.name === gitSecretName)).toBeDefined();
+    expect(serviceAccount.imagePullSecrets.find((s) => s.name === gitSecretName)).toBeUndefined();
+    expect(serviceAccount.secrets).toHaveLength(2);
+    expect(serviceAccount.imagePullSecrets).toHaveLength(1);
+  });
+
+  it('expect service account to have secret name available in secrets and imagePullSecrets properties', async () => {
+    const imageSecretName = 'image-secret';
+    const serviceAccount = await updateServiceAccount(
+      imageSecretName,
+      mockPipelineServiceAccount,
+      true,
+    );
+    expect(serviceAccount.secrets.find((s) => s.name === imageSecretName)).toBeDefined();
+    expect(serviceAccount.imagePullSecrets.find((s) => s.name === imageSecretName)).toBeDefined();
+    expect(serviceAccount.secrets).toHaveLength(2);
+    expect(serviceAccount.imagePullSecrets).toHaveLength(2);
+  });
 });

frontend/packages/pipelines-plugin/src/utils/pipeline-utils.ts

@@ -55,8 +55,14 @@ interface Resource {
   resource?: string;
   from?: string[];
 }
+
+interface ServiceAccountSecretNames {
+  [name: string]: string;
+}
+
 export type ServiceAccountType = {
-  secrets: { [name: string]: string }[];
+  secrets: ServiceAccountSecretNames[];
+  imagePullSecrets: ServiceAccountSecretNames[];
 } & K8sResourceCommon;
 
 export interface PipelineVisualizationTaskItem {
@@ -381,17 +387,28 @@ export const pipelineRunDuration = (run: PipelineRun | TaskRunKind): string => {
 export const updateServiceAccount = (
   secretName: string,
   originalServiceAccount: ServiceAccountType,
+  updateImagePullSecrets: boolean,
 ): Promise<ServiceAccountType> => {
   const updatedServiceAccount = _.cloneDeep(originalServiceAccount);
   updatedServiceAccount.secrets = [...updatedServiceAccount.secrets, { name: secretName }];
+  if (updateImagePullSecrets) {
+    updatedServiceAccount.imagePullSecrets = [
+      ...updatedServiceAccount.imagePullSecrets,
+      { name: secretName },
+    ];
+  }
   return k8sUpdate(ServiceAccountModel, updatedServiceAccount);
 };
 
-export const associateServiceAccountToSecret = (secret: SecretKind, namespace: string) => {
+export const associateServiceAccountToSecret = (
+  secret: SecretKind,
+  namespace: string,
+  isImageSecret: boolean,
+) => {
   k8sGet(ServiceAccountModel, PIPELINE_SERVICE_ACCOUNT, namespace)
     .then((serviceAccount) => {
       if (_.find(serviceAccount.secrets, (s) => s.name === secret.metadata.name) === undefined) {
-        updateServiceAccount(secret.metadata.name, serviceAccount);
+        updateServiceAccount(secret.metadata.name, serviceAccount, isImageSecret);
       }
     })
     .catch((err) => {