diff --git a/src/app/frontend/deploy/deploylabel.html b/src/app/frontend/deploy/deploylabel.html index c52a23fe61cd92c4332abba3a78d3e0e7f99dd3d..a774a13a2192083ccaabe713c5b95c449aa2e23a 100644 --- a/src/app/frontend/deploy/deploylabel.html +++ b/src/app/frontend/deploy/deploylabel.html @@ -20,6 +20,14 @@ limitations under the License. placeholder="{{labelCtrl.label.key}}" ng-disabled="!labelCtrl.label.editable"> {{labelCtrl.label.key}} is not unique. + + Prefix (before slash) is not a valid DNS subdomain prefix. Example: my-domain.com + + + Label key name should be lower or upper-case alphanumeric with '-', '_' and '.' between words only + + Prefix should not exceed 253 characters + Label Key name should not exceed 63 characters

diff --git a/src/app/frontend/deploy/deploylabel_controller.js b/src/app/frontend/deploy/deploylabel_controller.js index 0168ba62e00fc855b9f7a034207de55f746730c3..9d0c3981e97eca0e8ee0649316c05a9a84addd3b 100644 --- a/src/app/frontend/deploy/deploylabel_controller.js +++ b/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 diff --git a/src/test/frontend/deploy/deployfromsettings_controller_test.js b/src/test/frontend/deploy/deployfromsettings_controller_test.js index 361a9a5030a6e49a07ceeaab0319395214dd16f7..7737581db7dfe8ec1956d74ab1949d342a003cac 100644 --- a/src/test/frontend/deploy/deployfromsettings_controller_test.js +++ b/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(); - } + }); }); /** diff --git a/src/test/frontend/deploy/deploylabel_controller_test.js b/src/test/frontend/deploy/deploylabel_controller_test.js index 727cfde4e7137046bd02bded7027e6c46d43c278..3a997dbcb19922042f4bda06afc6f5ec422b0135 100644 --- a/src/test/frontend/deploy/deploylabel_controller_test.js +++ b/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(); + }); + }); });