Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

App name validation fix #451

Merged
merged 1 commit into from
Feb 29, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/app/frontend/deploy/deployfromsettings.html
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@
<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>
<ng-message when="pattern">Application name should contain only lowercase letters, numbers, and '-' between words
<ng-message when="pattern">Application name should start with a lowercase letter
, and 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-message when="maxlength">Application name should have no more than 24 characters</ng-message>
</ng-messages>

<md-progress-linear class="kd-deploy-form-progress" md-mode="indeterminate" ng-show="ctrl.form.name.$pending">
Expand Down
4 changes: 2 additions & 2 deletions src/app/frontend/deploy/deployfromsettings_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,13 +127,13 @@ export default class DeployFromSettingsController {
* (leading and trailing spaces are ignored by default)
* @export {!RegExp}
*/
this.namePattern = new RegExp('^[a-z0-9]([-a-z0-9]*[a-z0-9])?$');
this.namePattern = new RegExp('^[a-z]([-a-z0-9]*[a-z0-9])?$');

/**
* Maximum length for Application name
* @export {string}
*/
this.nameMaxLength = '63';
this.nameMaxLength = '24';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test must be adapted as well

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cheld PTAL


/**
* Whether to run the container as privileged user.
Expand Down
25 changes: 13 additions & 12 deletions src/test/frontend/deploy/deployfromsettings_controller_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -362,18 +362,19 @@ describe('DeployFromSettings controller', () => {

/**
* 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.
* svc, label). Therefore, the app-name validation pattern is based on the servicePattern, but
* must conform with all validation patterns of all the created resources.
* Currently, the ui pattern that conforms with all patterns starts with a lowercase letter,
* is lowercase 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])?)*',
servicePattern: '[a-z]([-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])?',
rcPattern: '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*',
appNamePattern: namePattern,
};

Expand All @@ -388,7 +389,7 @@ describe('DeployFromSettings controller', () => {
/**
* 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
* Remark: The app-name pattern excludes some RC 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', () => {
Expand Down Expand Up @@ -422,20 +423,20 @@ describe('DeployFromSettings controller', () => {
* 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
* service names of more than 24 chars via this form, while it is possible to RC pod names
* of 253 chars manually. This is a restriction of the current design.
*
* ctrl.maxNameLength = 63
* ctrl.maxNameLength = 24
*/
it('should limit input that conforms to all created resources', () => {

// service names are max. 253 chars
expect(ctrl.maxNameLength <= 253);
// service names are max. 24 chars
expect(ctrl.maxNameLength <= 24);

// 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);
// RC name are max. 253 chars.
expect(ctrl.maxNameLength <= 253);
});
});