Merge pull request #296 from digitalfishpond/validation-on-deploy

Validation and layout on deploy page

Author: bryk

src/app/frontend/deploy/deploy.html

@@ -31,10 +31,11 @@ <h3 class="md-headline kd-deploy-form-title">Deploy a Containerized App</h3>
       </kd-help-section>
 
       <div ng-switch="ctrl.selection">
-        <deploy-from-settings ng-switch-when="Settings" name="ctrl.name"
+        <deploy-from-settings ng-switch-when="Settings" class="md-body-1" name="ctrl.name"
             namespaces="ctrl.namespaces" detail="ctrl.detail" form="ctrl.deployForm" protocols="ctrl.protocols">
         </deploy-from-settings>
-        <deploy-from-file ng-switch-when="File" name="ctrl.name" detail="ctrl.detail" form="ctrl.deployForm">
+        <deploy-from-file ng-switch-when="File" class="md-body-1" name="ctrl.name" detail="ctrl.detail"
+            form="ctrl.deployForm">
         </deploy-from-file>
       </div>
 

src/app/frontend/deploy/deployfromsettings.html

@@ -17,18 +17,22 @@
 <kd-help-section>
   <md-input-container class="md-block" md-is-error="ctrl.isNameError()">
     <label>App name</label>
-    <input ng-model="ctrl.name" name="name" namespace="ctrl.namespace" required kd-unique-name
-        ng-model-options="{ updateOn: 'default blur', debounce: { 'default': 500, 'blur': 0 } }">
-    <md-progress-linear class="kd-deploy-form-progress" md-mode="indeterminate"
-        ng-show="ctrl.form.name.$pending">
+    <input ng-model="ctrl.name" name="name" namespace="ctrl.namespace" required ng-pattern="ctrl.namePattern"
+           ng-model-options="{ updateOn: 'default blur', debounce: { 'default': 500, 'blur': 0 } }" kd-unique-name
+           ng-maxlength="ctrl.nameMaxLength">
+    <md-progress-linear class="kd-deploy-form-progress" md-mode="indeterminate" ng-show="ctrl.form.name.$pending">
     </md-progress-linear>
     <ng-messages for="ctrl.form.name.$error" role="alert" multiple>
       <ng-message when="required">Application name is required.</ng-message>
-      <ng-message when="uniqueName">
-        Application with this name already exists within namespace <i>{{ctrl.namespace}}</i>.
+      <ng-message when="uniqueName">Application with this name
+        already exists within namespace <i>{{ctrl.namespace}}</i>.</ng-message>
+      <ng-message when="pattern">Application name should contain only lowercase letters, numbers, and '-' between words
       </ng-message>
+      <ng-message when="maxlength">Application name should have no more than 63 characters</ng-message>
     </ng-messages>
+
   </md-input-container>
+
   <kd-user-help>
     An 'app' label with this value will be added to the Replication Controller and Service that get deployed.
     <a href="http://kubernetes.io/v1.1/docs/user-guide/labels.html" target="_blank">Learn more</a>
@@ -55,8 +59,9 @@
     <label>Number of pods</label>
     <input ng-model="ctrl.replicas" type="number" required min="1" name="replicas">
     <ng-messages for="ctrl.form.replicas.$error" role="alert" multiple>
+      <ng-message when="required">Number of pods is required.</ng-message>
       <ng-message when="number">Number of pods must be a positive integer.</ng-message>
-      <ng-message when="min">Number of pods must be positive.</ng-message>
+      <ng-message when="min">Number of pods must be at least 1.</ng-message>
     </ng-messages>
   </md-input-container>
   <kd-user-help>
@@ -81,11 +86,12 @@
 
 <kd-help-section>
   <md-checkbox ng-model="ctrl.isExternal" class="md-primary"
-             ng-model-options="{ debounce: { 'default': 500, 'blur': 0 } }">
+               ng-model-options="{ debounce: { 'default': 500, 'blur': 0 } }">
     Expose service externally
   </md-checkbox>
 </kd-help-section>
 
+<!-- advanced options -->
 <div ng-show="ctrl.isMoreOptionsEnabled()">
   <kd-help-section>
     <md-input-container>
@@ -99,18 +105,18 @@
   </kd-help-section>
 
   <kd-help-section>
-    <div>
-      <div>Labels</div>
-      <div layout="column">
-        <div layout="row">
-          <p flex>Key</p>
-          <p flex>Value</p>
-        </div>
-        <div ng-repeat="label in ctrl.labels">
-          <kd-deploy-label layout="row" flex label="label" labels="ctrl.labels"></kd-deploy-label>
-        </div>
+    <div layout="column">
+      <div class="kd-label-title md-body-2">Labels</div>
+      <div layout="row" class="kd-label-header-row">
+        <div flex="45">Key</div>
+        <div flex="5"></div>
+        <div flex="40">Value</div>
+      </div>
+      <div ng-repeat="label in ctrl.labels">
+         <kd-deploy-label layout="row" flex label="label" labels="ctrl.labels"></kd-deploy-label>
       </div>
     </div>
+
     <kd-user-help>
       The specified labels will be applied to the created Replication Controller, Service (if any) and Pods.
       Common labels include release, environment, tier, partition and track.
@@ -165,6 +171,7 @@
           <ng-message when="min">CPU requirement must be given as a positive number.</ng-message>
         </ng-messages>
       </md-input-container>
+      <div flex="5"></div>
       <md-input-container flex>
         <label>Memory requirement (MiB)</label>
         <input ng-model="ctrl.memoryRequirement" type="number" name="memoryRequirement" min="0">
@@ -200,9 +207,11 @@
   </kd-help-section>
 
   <kd-help-section>
-    <md-switch ng-model="ctrl.runAsPrivileged" class="md-primary">
-      Run as privileged
-    </md-switch>
+    <div class="md-block">
+      <md-checkbox ng-model="ctrl.runAsPrivileged" class="md-primary">
+        Run as privileged
+      </md-checkbox>
+    </div>
     <kd-user-help>
       Processes in privileged containers are equivalent to processes running as root on the host.
     </kd-user-help>
@@ -219,14 +228,14 @@
   </kd-help-section>
 </div>
 
+<!-- show/hide advanced options toggle -->
 <md-button class="md-primary kd-deploy-moreoptions-button" type="button"
            ng-click="ctrl.switchMoreOptions()"
            ng-hide="ctrl.isMoreOptionsEnabled()">
-  <md-icon>arrow_drop_down</md-icon>More options
+  <md-icon>arrow_drop_down</md-icon>show advanced options
 </md-button>
-
 <md-button class="md-primary kd-deploy-moreoptions-button" type="button"
            ng-click="ctrl.switchMoreOptions()"
            ng-show="ctrl.isMoreOptionsEnabled()">
-  <md-icon>arrow_drop_up</md-icon>Less options
+  <md-icon>arrow_drop_up</md-icon>hide advanced options
 </md-button>

src/app/frontend/deploy/deployfromsettings.scss

@@ -13,11 +13,10 @@
 // limitations under the License.
 
 @import '../variables';
+$bar-height: 2px;
 
 md-progress-linear {
   &.kd-deploy-form-progress {
-    $bar-height: 2px;
-
     clear: left;
     height: $bar-height;
     margin-bottom: -$bar-height;

src/app/frontend/deploy/deployfromsettings_controller.js

@@ -121,6 +121,20 @@ export default class DeployFromSettingsController {
      */
     this.name = '';
 
+    /**
+     * Checks that a name begins and ends with a lowercase letter
+     * and contains nothing but lowercase letters and hyphens ("-")
+     * (leading and trailing spaces are ignored by default)
+     * @export {RegExp}
+     */
+    this.namePattern = new RegExp('^[a-z0-9]([-a-z0-9]*[a-z0-9])?$');
+
+    /**
+     * Maximum length for Application name
+     * @export {string}
+     */
+    this.nameMaxLength = '63';
+
     /**
      * Whether to run the container as privileged user.
      * @export {boolean}

src/app/frontend/deploy/environmentvariables.html

@@ -14,7 +14,7 @@
 limitations under the License.
 -->
 
-<div class="kd-environment-variables-title">Environment variables</div>
+<div class="kd-environment-variables-title md-body-2">Environment variables</div>
 <div ng-repeat="variable in ctrl.variables">
   <ng-form name="variablesForm" layout="row">
     <md-input-container flex class="kd-deploy-input-row">
@@ -25,6 +25,7 @@
         <ng-message when="pattern">Variable name must be a valid C identifier.</ng-message>
       </ng-messages>
     </md-input-container>
+    <div flex="5"></div>
     <md-input-container flex class="kd-deploy-input-row">
       <label>Value</label>
       <input ng-model="variable.value" name="value">

src/app/frontend/deploy/environmentvariables.scss

@@ -15,5 +15,5 @@
 @import '../variables';
 
 .kd-environment-variables-title {
-  margin: $baseline-grid 0;
+  margin: $baseline-grid * 2 0;
 }

src/app/frontend/deploy/portmappings.html

@@ -26,6 +26,7 @@
         <ng-message when="max">Port must less than 65536.</ng-message>
       </ng-messages>
     </md-input-container>
+    <div flex="5"></div>
     <md-input-container flex class="kd-deploy-input-row">
       <label>Target port</label>
       <input ng-model="portMapping.targetPort" ng-change="ctrl.addProtocolIfNeeed()"
@@ -36,6 +37,7 @@
         <ng-message when="max">Target port must less than 65536.</ng-message>
       </ng-messages>
     </md-input-container>
+    <div flex="5"></div>
     <md-input-container flex="none" class="kd-deploy-input-row">
       <label>Protocol</label>
       <md-select ng-model="portMapping.protocol" name="protocol" is-external="ctrl.isExternal"

src/test/frontend/deploy/deployfromsettings_controller_test.js

@@ -359,4 +359,83 @@ describe('DeployFromSettings controller', () => {
       expect(ctrl.imagePullSecret).toEqual('');
     });
   });
+
+  /**
+   * The value entered for ‘App Name” is used implicitly as the name for several resources (pod, rc,
+   * svc, label). Therefore, the app-name validation pattern is based on the RC-pattern, but must
+   * conform with all validation patterns of all the created resources.
+   * Currently, the ui pattern that conforms with all patterns is alpha-numeric with dashes between.
+   */
+  it('should allow strings that conform to the patterns of all created resources', () => {
+    // given the pattern used by the App Name field in the UI
+    let namePattern = ctrl.namePattern;
+    // given the patterns of all the names that are implicitly created
+    let allPatterns = {
+      servicePattern: '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*',
+      labelPattern: '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?',
+      rcPattern: '[a-z0-9]([-a-z0-9]*[a-z0-9])?',
+      appNamePattern: namePattern,
+    };
+
+    // then
+    for (let pattern in allPatterns) {
+      expect('mylowercasename'.match(allPatterns[pattern])).toBeDefined();
+      expect('my-name-with-dashes-between'.match(allPatterns[pattern])).toBeDefined();
+      expect('my-n4m3-with-numb3r5'.match(allPatterns[pattern])).toBeDefined();
+    }
+  });
+
+  /**
+   * The value entered for 'App Name' is used implicitly as the name for several resources (pod, rc,
+   * svc, label).
+   * Remark: The app-name pattern excludes some service names and label values, which could be
+   * created manually. This is a restriction of the current design.
+   */
+  it('should reject names that fail to conform to appNamePattern', () => {
+
+    // given
+    let appNamePattern = ctrl.namePattern;
+
+    // then the following valid service names will be rejected
+    expect('validservice.com'.match(appNamePattern)).toBeNull();
+    expect('validservice/path'.match(appNamePattern)).toBeNull();
+
+    // then the following valid label names will be rejected
+    expect('valid_label'.match(appNamePattern)).toBeNull();
+    expect('ValidLabel'.match(appNamePattern)).toBeNull();
+    expect('validLabel'.match(appNamePattern)).toBeNull();
+
+    // then these input values will be rejected
+    expect('@myname-with-illegal-char-prefix'.match(appNamePattern)).toBeNull();
+    expect('myname-with-illegal-char-suffix#'.match(appNamePattern)).toBeNull();
+    expect('myname-with-illegal_char-in-middle'.match(appNamePattern)).toBeNull();
+    expect('-myname-with-hyphen-prefix'.match(appNamePattern)).toBeNull();
+    expect('myname-with-hyphen-suffix-'.match(appNamePattern)).toBeNull();
+    expect('Myname-With-Capital-Letters'.match(appNamePattern)).toBeNull();
+    expect('myname-with-german-umlaut-äö'.match(appNamePattern)).toBeNull();
+    expect('my name with spaces'.match(appNamePattern)).toBeNull();
+    expect('  '.match(appNamePattern)).toBeNull();
+  });
+
+  /**
+   * The data from the App Name field of the deploy form is used implicitly for the creation of
+   * Service Name, Label Name and RC name. Pod names are truncated by the api server and therefore
+   * ignored.
+   * Remark: The maximum characters number should match all three, thereby excluding
+   * service names of more than 63 chars via this form, while it is possible to create service names
+   * of 253 chars manually. This is a restriction of the current design.
+   *
+   * ctrl.maxNameLength = 63
+   */
+  it('should limit input that conforms to all created resources', () => {
+
+    // service names are max. 253 chars
+    expect(ctrl.maxNameLength <= 253);
+
+    //  label are max. 63 chars. the 256 prefix cannot be entered
+    expect(ctrl.maxNameLength <= 63);
+
+    // RC name are max. 63 chars.
+    expect(ctrl.maxNameLength <= 63);
+  });
 });