[UI] textbox to select KSA when creating runs/jobs (#3651)

* Add service account field to run and job api objects * Update description * Fix field casing * Use service account from api object * Fix bug and add unit test * [UI] Allow choosing Kubernetes service account * fix unit tests * fix format * Also clone service account * service account UI features * Add unit test for cloning service account * Fix frontend integration tests
kubeflow · May 7, 2020 · e5bd2df · e5bd2df
1 parent 9c16e12
commit e5bd2df
Show file tree

Hide file tree

Showing 6 changed files with 353 additions and 1 deletion.
diff --git a/frontend/src/apis/job/api.ts b/frontend/src/apis/job/api.ts
@@ -138,11 +138,17 @@ export interface ApiJob {
    */
   pipeline_spec?: ApiPipelineSpec;
   /**
-   * Optional input field. Specify which resource this run belongs to.
+   * Optional input field. Specify which resource this job belongs to.
    * @type {Array<ApiResourceReference>}
    * @memberof ApiJob
    */
   resource_references?: Array<ApiResourceReference>;
+  /**
+   * Optional input field. Specify which Kubernetes service account this job uses.
+   * @type {string}
+   * @memberof ApiJob
+   */
+  service_account?: string;
   /**
    *
    * @type {string}

diff --git a/frontend/src/apis/run/api.ts b/frontend/src/apis/run/api.ts
@@ -346,6 +346,12 @@ export interface ApiRun {
    * @memberof ApiRun
    */
   resource_references?: Array<ApiResourceReference>;
+  /**
+   * Optional input field. Specify which Kubernetes service account this run uses.
+   * @type {string}
+   * @memberof ApiRun
+   */
+  service_account?: string;
   /**
    * Output. The time that the run created.
    * @type {Date}

diff --git a/frontend/src/pages/NewRun.test.tsx b/frontend/src/pages/NewRun.test.tsx
@@ -130,6 +130,7 @@ describe('NewRun', () => {
       run: {
         id: 'some-mock-run-id',
         name: 'some mock run name',
+        service_account: 'pipeline-runner',
         pipeline_spec: {
           pipeline_id: 'original-run-pipeline-id',
           workflow_manifest: '{}',
@@ -785,6 +786,25 @@ describe('NewRun', () => {
       expect(tree.state('runName')).toBe('Clone (2) of some run');
     });
 
+    it('uses service account in the original run', async () => {
+      const defaultRunDetail = newMockRunDetail();
+      const runDetail = {
+        ...defaultRunDetail,
+        run: {
+          ...defaultRunDetail.run,
+          service_account: 'sa1',
+        },
+      };
+      const props = generateProps();
+      props.location.search = `?${QUERY_PARAMS.cloneFromRun}=${runDetail.run!.id}`;
+      getRunSpy.mockImplementation(() => runDetail);
+
+      tree = shallow(<TestNewRun {...props} />);
+      await TestUtils.flushPromises();
+
+      expect(tree.state('serviceAccount')).toBe('sa1');
+    });
+
     it('uses the query param experiment ID over the one in the original run if an ID is present in both', async () => {
       const experiment = newMockExperiment();
       const runDetail = newMockRunDetail();
@@ -1189,6 +1209,9 @@ describe('NewRun', () => {
       (tree.instance() as TestNewRun).handleChange('description')({
         target: { value: 'test run description' },
       });
+      (tree.instance() as TestNewRun).handleChange('serviceAccount')({
+        target: { value: 'service-account-name' },
+      });
       await TestUtils.flushPromises();
 
       tree
@@ -1205,6 +1228,7 @@ describe('NewRun', () => {
         pipeline_spec: {
           parameters: MOCK_PIPELINE.parameters,
         },
+        service_account: 'service-account-name',
         resource_references: [
           {
             key: {
@@ -1259,6 +1283,7 @@ describe('NewRun', () => {
         pipeline_spec: {
           parameters: [{ name: 'testName', value: '{\n  "test2": "value2"\n}' }],
         },
+        service_account: '',
         resource_references: [
           {
             key: {
@@ -1351,6 +1376,7 @@ describe('NewRun', () => {
           pipeline_id: undefined,
           workflow_manifest: '{"metadata":{"name":"embedded"},"parameters":[]}',
         },
+        service_account: 'pipeline-runner',
         resource_references: [],
       });
       // TODO: verify route change happens
@@ -1655,6 +1681,7 @@ describe('NewRun', () => {
 
       instance.handleChange('runName')({ target: { value: 'test run name' } });
       instance.handleChange('description')({ target: { value: 'test run description' } });
+      instance.handleChange('serviceAccount')({ target: { value: 'service-account-name' } });
       await TestUtils.flushPromises();
 
       tree
@@ -1675,6 +1702,7 @@ describe('NewRun', () => {
         pipeline_spec: {
           parameters: MOCK_PIPELINE.parameters,
         },
+        service_account: 'service-account-name',
         resource_references: [
           {
             key: {

diff --git a/frontend/src/pages/NewRun.tsx b/frontend/src/pages/NewRun.tsx
@@ -57,12 +57,15 @@ import { Description } from '../components/Description';
 import { NamespaceContext } from '../lib/KubeflowClient';
 import { NameWithTooltip } from '../components/CustomTableNameColumn';
 import { PredicateOp, ApiFilter } from '../apis/filter';
+import { HelpButton } from 'src/atoms/HelpButton';
+import { ExternalLink } from 'src/atoms/ExternalLink';
 
 interface NewRunState {
   description: string;
   errorMessage: string;
   experiment?: ApiExperiment;
   experimentName: string;
+  serviceAccount: string;
   experimentSelectorOpen: boolean;
   isBeingStarted: boolean;
   isClone: boolean;
@@ -116,6 +119,7 @@ export class NewRun extends Page<{ namespace?: string }, NewRunState> {
     description: '',
     errorMessage: '',
     experimentName: '',
+    serviceAccount: '',
     experimentSelectorOpen: false,
     isBeingStarted: false,
     isClone: false,
@@ -176,6 +180,7 @@ export class NewRun extends Page<{ namespace?: string }, NewRunState> {
       description,
       errorMessage,
       experimentName,
+      serviceAccount,
       experimentSelectorOpen,
       isClone,
       isFirstRunInExperiment,
@@ -499,6 +504,27 @@ export class NewRun extends Page<{ namespace?: string }, NewRunState> {
             }}
           />
 
+          <div>
+            This run will use the following Kubernetes service account.{' '}
+            <HelpButton
+              helpText={
+                <div>
+                  Note, the service account needs{' '}
+                  <ExternalLink href='https://github.com/argoproj/argo/blob/v2.3.0/docs/workflow-rbac.md'>
+                    minimum permissions required by argo workflows
+                  </ExternalLink>{' '}
+                  and extra permissions the specific task requires.
+                </div>
+              }
+            />
+          </div>
+          <Input
+            value={serviceAccount}
+            onChange={this.handleChange('serviceAccount')}
+            label='Service Account (Optional)'
+            variant='outlined'
+          />
+
           {/* One-off/Recurring Run Type */}
           <div className={commonCss.header}>Run Type</div>
           {isClone && <span>{isRecurringRun ? 'Recurring' : 'One-off'}</span>}
@@ -910,6 +936,7 @@ export class NewRun extends Page<{ namespace?: string }, NewRunState> {
     let usePipelineFromRunLabel = '';
     let name = '';
     let pipelineVersionName = '';
+    const serviceAccount = originalRun.service_account || '';
 
     // Case 1: a legacy run refers to a pipeline without specifying version.
     const referencePipelineId = RunUtils.getPipelineId(originalRun);
@@ -984,6 +1011,7 @@ export class NewRun extends Page<{ namespace?: string }, NewRunState> {
       usePipelineFromRunLabel,
       useWorkflowFromRun,
       workflowFromRun,
+      serviceAccount,
     });
 
     this._validate();
@@ -1039,6 +1067,7 @@ export class NewRun extends Page<{ namespace?: string }, NewRunState> {
           : undefined,
       },
       resource_references: references,
+      service_account: this.state.serviceAccount,
     };
     if (this.state.isRecurringRun) {
       newRun = Object.assign(newRun, {