Avoid requiring SA in Flux plugin in the UI (#5535)

### Description of the change This PR removed the `required=true` UI validation if using the Flux plugin for installing a package. ### Benefits UI consistent with the Flux backend. ### Possible drawbacks N/A ### Applicable issues - fixes #5522 ### Additional information ![image](https://user-images.githubusercontent.com/11535726/197036382-3aaf4196-4724-46ad-bfa9-68f5eb26b32d.png) Signed-off-by: Antonio Gamez Diaz <[email protected]>
vmware-tanzu · Oct 26, 2022 · 2a57dd4 · 2a57dd4
1 parent ddfb398
commit 2a57dd4
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 6 deletions.
diff --git a/dashboard/src/components/DeploymentForm/DeploymentForm.tsx b/dashboard/src/components/DeploymentForm/DeploymentForm.tsx
@@ -26,7 +26,7 @@ import { ThunkDispatch } from "redux-thunk";
 import { Kube } from "shared/Kube";
 import { FetchError, IStoreState } from "shared/types";
 import * as url from "shared/url";
-import { getPluginsRequiringSA, k8sObjectNameRegex } from "shared/utils";
+import { getPluginsAllowingSA, getPluginsRequiringSA, k8sObjectNameRegex } from "shared/utils";
 import DeploymentFormBody from "./DeploymentFormBody";
 interface IRouteParams {
   cluster: string;
@@ -99,7 +99,7 @@ export default function DeploymentForm() {
 
   useEffect(() => {
     // Populate the service account list if the plugin requires it
-    if (getPluginsRequiringSA().includes(pluginObj.name)) {
+    if (getPluginsAllowingSA().includes(pluginObj.name)) {
       // We assume the user has enough permissions to do that. Fallback to a simple input maybe?
       Kube.getServiceAccountNames(targetCluster, targetNamespace)
         .then(saList => setServiceAccountList(saList.serviceaccountNames))
@@ -232,14 +232,14 @@ export default function DeploymentForm() {
                 </CdsInput>
                 {
                   // TODO(agamez): let plugins define their own components instead of hardcoding the logic here
-                  getPluginsRequiringSA().includes(pluginObj.name) ? (
+                  getPluginsAllowingSA().includes(pluginObj.name) ? (
                     <>
                       <CdsSelect layout="horizontal" id="serviceaccount-selector">
                         <label>Service Account</label>
                         <select
                           value={reconciliationOptions.serviceAccountName}
                           onChange={onChangeSA}
-                          required={true}
+                          required={getPluginsRequiringSA().includes(pluginObj.name)}
                         >
                           <option key=""></option>
                           {serviceAccountList?.map(o => (

diff --git a/dashboard/src/shared/utils.test.ts b/dashboard/src/shared/utils.test.ts
@@ -17,6 +17,7 @@ import {
   getPluginIcon,
   getPluginName,
   getPluginPackageName,
+  getPluginsAllowingSA,
   getPluginsRequiringSA,
   getPluginsSupportingRollback,
   getSupportedPackageRepositoryAuthTypes,
@@ -145,7 +146,11 @@ it("getPluginByName", () => {
 });
 
 it("getPluginsRequiringSA", () => {
-  expect(getPluginsRequiringSA()).toStrictEqual([
+  expect(getPluginsRequiringSA()).toStrictEqual([PluginNames.PACKAGES_KAPP]);
+});
+
+it("getPluginsAllowingSA", () => {
+  expect(getPluginsAllowingSA()).toStrictEqual([
     PluginNames.PACKAGES_FLUX,
     PluginNames.PACKAGES_KAPP,
   ]);

diff --git a/dashboard/src/shared/utils.ts b/dashboard/src/shared/utils.ts
@@ -219,10 +219,15 @@ export function getPluginByName(pluginName: PluginNames | string) {
   }
 }
 
-export function getPluginsRequiringSA(): string[] {
+export function getPluginsAllowingSA(): string[] {
   return [PluginNames.PACKAGES_FLUX, PluginNames.PACKAGES_KAPP];
 }
 
+// getPluginsRequiringSA should return a subset of getPluginsAllowingSA
+export function getPluginsRequiringSA(): string[] {
+  return [PluginNames.PACKAGES_KAPP];
+}
+
 export function getPluginsSupportingRollback(): string[] {
   return [PluginNames.PACKAGES_HELM];
 }