Improve Role Required Indicators/Validation

- RBAC Role type improvements - Remove rule 'Non-Resource URL' field. RBAC role type is namespaced, non-resource url's are not. - Apply additional validation ( - rule resource AND api group required, rather than default rule resource, non-resource url or api group required - name is required - Show Required indicator for all fields (reflecting validation) - Other Role type improvements - Show Required indicator for rule Verbs field - This does mean that when the page initially loads we show an empty rule row with required fields... but user can ignore this and successfully save the role anyway. The alternative is to not show an empty rule to begin with... but this forces the user to `Add Resource`. I feel the later is more of an issue than the former. - name is required
rancher · Apr 21, 2021 · 92c6610 · 92c6610
1 parent d8ef0cb
commit 92c6610
Show file tree

Hide file tree

Showing 6 changed files with 50 additions and 32 deletions.
diff --git a/assets/translations/en-us.yaml b/assets/translations/en-us.yaml
@@ -3139,7 +3139,9 @@ validation:
   roleTemplate:
     roleTemplateRules: 
       missingVerb: You must specify at least one verb for each resource grant
-      missingResource: You must specify at least one Resource, Non-Resource URL or API Group for each resource grant
+      missingResource: You must specify a Resource for each resource grant
+      missingApiGroup: You must specify an API Group for each resource grant
+      missingOneResource: You must specify at least one Resource, Non-Resource URL or API Group for each resource grant
   service:
     externalName:
       none: External Name is required on an ExternalName Service.

diff --git a/components/auth/RoleDetailEdit.vue b/components/auth/RoleDetailEdit.vue
@@ -156,6 +156,9 @@ export default {
     doneLocationOverride() {
       return this.value.listLocation;
     },
+    ruleClass() {
+      return `col ${ this.isNamespaced ? 'span-4' : 'span-3' }`;
+    },
     // Detail View
     rules() {
       return this.createRules(this.value);
@@ -374,23 +377,29 @@ export default {
           >
             <template #column-headers>
               <div class="column-headers row">
-                <div class="col span-3">
-                  <label class="text-label">{{ t('rbac.roletemplate.tabs.grantResources.tableHeaders.verbs') }}</label>
+                <div :class="ruleClass">
+                  <label class="text-label">{{ t('rbac.roletemplate.tabs.grantResources.tableHeaders.verbs') }}
+                    <span class="required">*</span>
+                  </label>
                 </div>
-                <div class="col span-3">
-                  <label class="text-label">{{ t('rbac.roletemplate.tabs.grantResources.tableHeaders.resources') }}</label>
+                <div :class="ruleClass">
+                  <label class="text-label">{{ t('rbac.roletemplate.tabs.grantResources.tableHeaders.resources') }}
+                    <span v-if="isNamespaced" class="required">*</span>
+                  </label>
                 </div>
-                <div class="col span-3">
+                <div v-if="!isNamespaced" :class="ruleClass">
                   <label class="text-label">{{ t('rbac.roletemplate.tabs.grantResources.tableHeaders.nonResourceUrls') }}</label>
                 </div>
-                <div class="col span-3">
-                  <label class="text-label">{{ t('rbac.roletemplate.tabs.grantResources.tableHeaders.apiGroups') }}</label>
+                <div :class="ruleClass">
+                  <label class="text-label">{{ t('rbac.roletemplate.tabs.grantResources.tableHeaders.apiGroups') }}
+                    <span v-if="isNamespaced" class="required">*</span>
+                  </label>
                 </div>
               </div>
             </template>
             <template #columns="props">
               <div class="columns row">
-                <div class="col span-3">
+                <div :class="ruleClass">
                   <Select
                     :value="props.row.value.verbs"
                     class="lg"
@@ -402,7 +411,7 @@ export default {
                     @input="updateSelectValue(props.row.value, 'verbs', $event)"
                   />
                 </div>
-                <div class="col span-3">
+                <div :class="ruleClass">
                   <Select
                     :value="getRule('resources', props.row.value)"
                     :options="resourceOptions"
@@ -412,14 +421,14 @@ export default {
                     @input="setRule('resources', props.row.value, $event)"
                   />
                 </div>
-                <div class="col span-3">
+                <div v-if="!isNamespaced" :class="ruleClass">
                   <LabeledInput
                     :value="getRule('nonResourceURLs', props.row.value)"
                     :mode="mode"
                     @input="setRule('nonResourceURLs', props.row.value, $event)"
                   />
                 </div>
-                <div class="col span-3">
+                <div :class="ruleClass">
                   <LabeledInput
                     :value="getRule('apiGroups', props.row.value)"
                     :mode="mode"
@@ -466,6 +475,10 @@ export default {
 </template>
 
 <style lang="scss" scoped>
+  .required {
+    color: var(--error);
+  }
+
   ::v-deep {
     .column-headers {
       margin-right: 75px;

diff --git a/models/management.cattle.io.globalrole.js b/models/management.cattle.io.globalrole.js
@@ -4,6 +4,7 @@ import { CATTLE_API_GROUP, SUBTYPE_MAPPING } from '@/models/management.cattle.io
 import { uniq } from '@/utils/array';
 import Vue from 'vue';
 import { get } from '@/utils/object';
+import Role from './rbac.authorization.k8s.io.role';
 
 const BASE = 'user-base';
 const USER = 'user';
@@ -14,14 +15,7 @@ const GLOBAL = SUBTYPE_MAPPING.GLOBAL.key;
 
 export default {
   customValidationRules() {
-    return [
-      {
-        path:           'rules',
-        validators:     ['roleTemplateRules'],
-        nullable:       false,
-        type:           'array',
-      },
-    ];
+    return Role.customValidationRules();
   },
 
   details() {

diff --git a/models/management.cattle.io.roletemplate.js b/models/management.cattle.io.roletemplate.js
@@ -3,6 +3,7 @@ import { uniq } from '@/utils/array';
 import Vue from 'vue';
 import { get } from '@/utils/object';
 import { DESCRIPTION } from '@/config/labels-annotations';
+import Role from './rbac.authorization.k8s.io.role';
 
 export const CATTLE_API_GROUP = '.cattle.io';
 
@@ -56,14 +57,7 @@ export const VERBS = [
 
 export default {
   customValidationRules() {
-    return [
-      {
-        path:           'rules',
-        validators:     ['roleTemplateRules'],
-        nullable:       false,
-        type:           'array',
-      },
-    ];
+    return Role.customValidationRules();
   },
 
   details() {

diff --git a/models/rbac.authorization.k8s.io.role.js b/models/rbac.authorization.k8s.io.role.js
@@ -5,9 +5,16 @@ import { uniq } from '@/utils/array';
 export default {
   customValidationRules() {
     return [
+      {
+        path:           'name',
+        translationKey: 'nameNsDescription.name.label',
+        required:       true,
+        nullable:       false,
+        type:           'string',
+      },
       {
         path:           'rules',
-        validators:     ['roleTemplateRules'],
+        validators:     [`roleTemplateRules:${ this.type }`],
         nullable:       false,
         type:           'array',
       },

diff --git a/utils/validators/role-template.js b/utils/validators/role-template.js
@@ -1,11 +1,19 @@
+import { RBAC } from '@/config/types';
 import isEmpty from 'lodash/isEmpty';
 
-export function roleTemplateRules(rules = [], getters, errors, validatorArgs) {
+export function roleTemplateRules(rules = [], getters, errors, validatorArgs = []) {
   if (rules.some(rule => isEmpty(rule.verbs))) {
     errors.push(getters['i18n/t']('validation.roleTemplate.roleTemplateRules.missingVerb'));
   }
 
-  if (rules.some(rule => isEmpty(rule.resources) && isEmpty(rule.nonResourceURLs) && isEmpty(rule.apiGroups))) {
-    errors.push(getters['i18n/t']('validation.roleTemplate.roleTemplateRules.missingResource'));
+  if (validatorArgs[0] === RBAC.ROLE) {
+    if (rules.some(rule => isEmpty(rule.resources))) {
+      errors.push(getters['i18n/t']('validation.roleTemplate.roleTemplateRules.missingResource'));
+    }
+    if (rules.some(rule => isEmpty(rule.apiGroups))) {
+      errors.push(getters['i18n/t']('validation.roleTemplate.roleTemplateRules.missingApiGroup'));
+    }
+  } else if (rules.some(rule => isEmpty(rule.resources) && isEmpty(rule.nonResourceURLs) && isEmpty(rule.apiGroups))) {
+    errors.push(getters['i18n/t']('validation.roleTemplate.roleTemplateRules.missingOneResource'));
   }
 }