Merge pull request #492 from digitalfishpond/label-key-validation

Label Key validation on Deploy page

src/app/frontend/deploy/deploylabel.html

@@ -20,6 +20,14 @@ limitations under the License.
            placeholder="{{labelCtrl.label.key}}" ng-disabled="!labelCtrl.label.editable">
     <ng-messages for="labelForm.key.$error" ng-if="labelForm.key.$invalid">
       <ng-message when="unique">{{labelCtrl.label.key}} is not unique.</ng-message>
+      <ng-message when="prefixPattern">
+        Prefix (before slash) is not a valid DNS subdomain prefix. Example: my-domain.com
+      </ng-message>
+      <ng-message when="namePattern">
+        Label key name should be lower or upper-case alphanumeric with '-', '_' and '.' between words only
+      </ng-message>
+      <ng-message when="prefixLength">Prefix should not exceed 253 characters</ng-message>
+      <ng-message when="nameLength">Label Key name should not exceed 63 characters</ng-message>
     </ng-messages>
   </md-input-container>
   <p flex="5"></p>

src/app/frontend/deploy/deploylabel_controller.js

@@ -88,22 +88,44 @@ export default class DeployLabelController {
   addNewLabel_() { this.labels.push(new DeployLabel()); }
 
   /**
-   * Validates label withing label form.
+   * Validates label within label form.
    * Current checks:
    *  - duplicated key
+   *  - key prefix pattern
+   *  - key name pattern
+   *  - key prefix length
+   *  - key name length
    * @param {!angular.FormController|undefined} labelForm
    * @private
    */
+  // TODO: @digitalfishpond Move these validations to directives
   validateKey_(labelForm) {
     if (angular.isDefined(labelForm)) {
       /** @type {!angular.NgModelController} */
       let elem = labelForm.key;
+      /** @type {!RegExp} */
+      let PrefixPattern = /^(.*\/.*)$/;
+      /** @type {boolean} */
+      let isPrefixed = PrefixPattern.test(this.label.key);
+      /** @type {number} */
+      let slashPosition = isPrefixed ? this.label.key.indexOf("/") : -1;
 
-      // TODO(floreks): Validate label key/value.
       /** @type {boolean} */
-      let isValid = !this.isDuplicated_();
+      let isUnique = !this.isKeyDuplicated_();
+      /** @type {boolean} */
+      let isKeyPrefixPatternOk = this.matchesKeyPrefixPattern_(isPrefixed, slashPosition);
+      /** @type {boolean} */
+      let isKeyNamePatternOk = this.matchesKeyNamePattern_(isPrefixed, slashPosition);
+      /** @type {boolean} */
+      let isKeyPrefixLengthOk = this.matchesKeyPrefixLength_(isPrefixed, slashPosition);
+      /** @type {boolean} */
+      let isKeyNameLengthOk = this.matchesKeyNameLength_(isPrefixed, slashPosition);
 
-      elem.$setValidity('unique', isValid);
+      elem.$setValidity('unique', isUnique);
+      elem.$setValidity('prefixPattern', isKeyPrefixPatternOk);
+      elem.$setValidity('namePattern', isKeyNamePatternOk);
+      elem.$setValidity('prefixLength', isKeyPrefixLengthOk);
+      elem.$setValidity('nameLength', isKeyNameLengthOk);
     }
   }
 
@@ -113,7 +135,7 @@ export default class DeployLabelController {
    * @return {boolean}
    * @private
    */
-  isDuplicated_() {
+  isKeyDuplicated_() {
     /** @type {number} */
     let duplications = 0;
 
@@ -126,6 +148,84 @@ export default class DeployLabelController {
     return duplications > 1;
   }
 
+  /**
+   * Returns true if the label key prefix (before the "/" if there is one) matches a lowercase
+   * alphanumeric character
+   * optionally followed by lowercase alphanumeric or '-' or '.' and ending with a lower case
+   * alphanumeric character,
+   * with '.' only permitted if surrounded by lowercase alphanumeric characters (eg:
+   * 'good.prefix-pattern',
+   * otherwise returns false.
+   * @return {boolean}
+   * @param {boolean} isPrefixed
+   * @param {number} slashPosition
+   * @private
+   */
+  matchesKeyPrefixPattern_(isPrefixed, slashPosition) {
+    /** @type {!RegExp} */
+    let labelKeyPrefixPattern = /^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$/;
+    /** @type {string} */
+    let labelPrefix = isPrefixed ? this.label.key.substring(0, slashPosition) : "valid-pattern";
+
+    return (labelKeyPrefixPattern.test(labelPrefix));
+  }
+
+  /**
+   * Returns true if the label key name (after the "/" if there is one) matches an alphanumeric
+   * character (upper
+   * or lower case) optionally followed by alphanumeric or -_. and ending with an alphanumeric
+   * character
+   * (upper or lower case), otherwise returns false.
+   * @return {boolean}
+   * @param {boolean} isPrefixed
+   * @param {number} slashPosition
+   * @private
+   */
+  matchesKeyNamePattern_(isPrefixed, slashPosition) {
+    /** @type {!RegExp} */
+    let labelKeyNamePattern = /^([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$/;
+    /** @type {string} */
+    let labelName = isPrefixed ? this.label.key.substring(slashPosition + 1) : this.label.key;
+
+    return (labelKeyNamePattern.test(labelName));
+  }
+
+  /**
+   * Returns true if the label key name (after the "/" if there is one) is equal or shorter than 253
+   * characters,
+   * otherwise returns false.
+   * @return {boolean}
+   * @param {boolean} isPrefixed
+   * @param {number} slashPosition
+   * @private
+   */
+  matchesKeyPrefixLength_(isPrefixed, slashPosition) {
+    /** @type {number} */
+    let maxLength = 253;
+    /** @type {string} */
+    let labelPrefix = isPrefixed ? this.label.key.substring(0, slashPosition) : '';
+
+    return (labelPrefix.length <= maxLength);
+  }
+
+  /**
+   * Returns true if the label key name (after the "/" if there is one) is equal or shorter than 63
+   * characters,
+   * otherwise returns false.
+   * @return {boolean}
+   * @param {boolean} isPrefixed
+   * @param {number} slashPosition
+   * @private
+   */
+  matchesKeyNameLength_(isPrefixed, slashPosition) {
+    /** @type {number} */
+    let maxLength = 63;
+    /** @type {string} */
+    let labelName = isPrefixed ? this.label.key.substring(slashPosition + 1) : this.label.key;
+
+    return (labelName.length <= maxLength);
+  }
+
   /**
    * Returns true if label key and value are not empty, false otherwise.
    * @param {!DeployLabel} label

src/test/frontend/deploy/deployfromsettings_controller_test.js

@@ -403,11 +403,11 @@ describe('DeployFromSettings controller', () => {
     };
 
     // then
-    for (let pattern in allPatterns) {
+    Object.keys(allPatterns).forEach((pattern) => {
       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();
-    }
+    });
   });
 
   /**

src/test/frontend/deploy/deploylabel_controller_test.js

@@ -151,4 +151,234 @@ describe('DeployLabel controller', () => {
     // then
     expect(labelForm.key.$valid).toBeTruthy();
   });
+
+  /**
+   * RegExp for prefix checks that whatever is before the slash (if anything) matches:
+   * beginning and ending with a lowercase letter or number, with single character
+   * '.' and/or single/multiple character '-' between words, not touching each other,
+   * seperated from key name with a single slash (no slashes in the prefix are
+   * currently permitted by the back end validation)
+   */
+  it('should set validity to false when key prefix does not conform to RegExp ' +
+         '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)* ',
+     () => {
+       // given
+       let failPrefixes = [
+         '.dotatbegining/key',
+         'dotatend./key',
+         'dot-next-.to-dash/key',
+         '-dash-at-beginning/key',
+         'dash-at-end-/key',
+         'CapitalLetter/key',
+         'more/than/one/slash/key',
+         'illegal_characters/key',
+         'space in prefix/key',
+       ];
+       failPrefixes.forEach((failPrefix) => {
+         ctrl.label = new DeployLabel(failPrefix);
+         ctrl.labels = [
+           ctrl.label,
+         ];
+
+         // when
+         ctrl.check(labelForm);
+
+         // then
+         expect(labelForm.key.$valid).toBeFalsy();
+       });
+     });
+
+  it('should set validity to true when key prefix conforms to RegExp ' +
+         '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)* ',
+     () => {
+       // given
+       let passPrefixes = [
+         'validdns.com/key',
+         'validdns.co.uk/key',
+         'valid-dns.com/key',
+         'validdns/key',
+         '01234/key',
+       ];
+       passPrefixes.forEach((passPrefix) => {
+         ctrl.label = new DeployLabel(passPrefix);
+         ctrl.labels = [
+           ctrl.label,
+         ];
+
+         // when
+         ctrl.check(labelForm);
+
+         // then
+         expect(labelForm.key.$valid).toBeTruthy();
+       });
+     });
+
+  /**
+   * RegExp for key name checks that whatever is after the slash (if there is one)
+   * or the whole word (if there is no slash) matches:
+   * beginning and ending with upper or lowercase alphanumeric characters
+   * separated by '.', '_', '-' only.
+   */
+  it('should set validity to false when key name (after slash) does not conform to RegExp ' +
+         '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9] ',
+     () => {
+       // given
+       let failKeyNames = [
+         'no-key-name-after-slash/',
+         '-dash-at-beginning',
+         'dash-at-end-',
+         '_underscore_at_beginning',
+         'underscore_at_end_',
+         '.dot.at.beginning',
+         'dot.at.end.',
+         'illegal$character',
+         'illegal@character',
+       ];
+
+       failKeyNames.forEach((failKeyName) => {
+         ctrl.label = new DeployLabel(failKeyName);
+         ctrl.labels = [
+           ctrl.label,
+         ];
+
+         // when
+         ctrl.check(labelForm);
+
+         // then
+         expect(labelForm.key.$valid).toBeFalsy();
+       });
+     });
+
+  it('should set validity to true when key name (after slash) conforms to RegExp ' +
+         '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9] ',
+     () => {
+       // given
+       let passKeyNames = [
+         'prefix/key',
+         'key',
+         'key.dot',
+         'key-dash',
+         'key_underscore',
+         'KeyCapital',
+       ];
+
+       passKeyNames.forEach((passKeyName) => {
+         ctrl.label = new DeployLabel(passKeyName);
+         ctrl.labels = [
+           ctrl.label,
+         ];
+
+         // when
+         ctrl.check(labelForm);
+
+         // then
+         expect(labelForm.key.$valid).toBeTruthy();
+       });
+     });
+
+  /**
+   * key prefix (before slash) should be no longer than 253 characters.
+   */
+  it('should set validity to false when key prefix (before slash) exceeds 253 characters ', () => {
+    // given
+    // failKey contains a 254 character prefix before slash
+    let stringLength = 254;
+    let failPrefix = new Array(stringLength + 1).join('x');
+    let failKey = `${failPrefix}/validkeyname`;
+
+    ctrl.label = new DeployLabel(failKey);
+    ctrl.labels = [
+      ctrl.label,
+    ];
+
+    // when
+    ctrl.check(labelForm);
+
+    // then
+    expect(labelForm.key.$valid).toBeFalsy();
+  });
+
+  it('should set validity to true when key prefix (before slash) is not longer than 253 characters ',
+     () => {
+       // given
+       // passKey contains a 253 character prefix before slash
+       let stringLength = 253;
+       let passPrefix = (new Array(stringLength + 1).join('x'));
+       let passKey = `${passPrefix}/validkeyname`;
+
+       ctrl.label = new DeployLabel(passKey);
+       ctrl.labels = [
+         ctrl.label,
+       ];
+
+       // when
+       ctrl.check(labelForm);
+
+       // then
+       expect(labelForm.key.$valid).toBeTruthy();
+     });
+
+  /**
+   * key name (after slash or whole string if no slash in string) should be no longer than 253
+   * characters.
+   */
+  it('should set validity to false when key name ' +
+         '(after slash, or whole string if no slash present) ' +
+         'exceeds 63 characters ',
+     () => {
+       // given
+       let stringLength = 64;
+       let failNameNoPrefix = (new Array(stringLength + 1).join('x'));
+       let failNameWithPrefix = `validprefix.com/${failNameNoPrefix}`;
+
+       let failKeyNames = [
+         failNameNoPrefix,
+         failNameWithPrefix,
+       ];
+
+       failKeyNames.forEach((failKeyName) => {
+         ctrl.label = new DeployLabel(failKeyName);
+         ctrl.labels = [
+           ctrl.label,
+         ];
+
+         // when
+         ctrl.check(labelForm);
+
+         // then
+         expect(labelForm.key.$valid).toBeFalsy();
+       });
+     });
+
+  /**
+   * key name (after slash or whole string if no slash in string) should be no longer than 63
+   * characters.
+   */
+  it('should set validity to true when key name ' +
+         '(after slash, or whole string if no slash present) ' +
+         'does not exceed 63 characters ',
+     () => {
+       // given
+       let stringLength = 63;
+       let passNameNoPrefix = (new Array(stringLength + 1).join('x'));
+       let passNameWithPrefix = `validprefix.com/${passNameNoPrefix}`;
+
+       let passKeyNames = [
+         passNameNoPrefix,
+         passNameWithPrefix,
+       ];
+
+       passKeyNames.forEach((passKeyName) => {
+         ctrl.label = new DeployLabel(passKeyName);
+         ctrl.labels = [
+           ctrl.label,
+         ];
+
+         // when
+         ctrl.check(labelForm);
+
+         // then
+         expect(labelForm.key.$valid).toBeTruthy();
+       });
+     });
 });