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

ui: Warning and better error handling for invalid-named variables and job templates #19989

Merged
merged 4 commits into from
Feb 15, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
10 changes: 10 additions & 0 deletions ui/app/adapters/variable.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
// @ts-check
import ApplicationAdapter from './application';
import AdapterError from '@ember-data/adapter/error';
import InvalidError from '@ember-data/adapter/error';
import { pluralize } from 'ember-inflector';
import classic from 'ember-classic-decorator';
import { ConflictError } from '@ember-data/adapter/error';
Expand Down Expand Up @@ -141,6 +142,15 @@ export default class VariableAdapter extends ApplicationAdapter {
{ detail: _normalizeConflictErrorObject(payload), status: 409 },
]);
}
if (status === 400) {
return new InvalidError([
{
detail:
'Invalid name. Name must contain only alphanumeric or "-", "_", "~", or "/" characters, and be fewer than 128 characters in length.',
status: 400,
},
]);
}
return super.handleResponse(...arguments);
}
}
Expand Down
5 changes: 5 additions & 0 deletions ui/app/components/variable-form.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,11 @@
.
</F.Error>
{{/if}}
{{#if this.hasInvalidPath}}
<F.Error data-test-invalid-path-error>
Path must contain only alphanumeric or "-", "_", "~", or "/" characters, and be fewer than 128 characters in length.
</F.Error>
{{/if}}
{{#if this.isJobTemplateVariable}}
<F.HelperText data-test-job-template-hint>
Use this variable to generate job templates with <Hds::Copy::Snippet @textToCopy="nomad job init -template={{this.jobTemplateName}}" />
Expand Down
24 changes: 19 additions & 5 deletions ui/app/components/variable-form.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,11 @@ export default class VariableFormComponent extends Component {
}
}

get hasInvalidPath() {
let pathNameRegex = new RegExp('^[a-zA-Z0-9-_~/]{1,128}$');
return !pathNameRegex.test(trimPath([this.path]));
}

@action
validateKey(entry, e) {
const value = e.target.value;
Expand Down Expand Up @@ -269,18 +274,27 @@ export default class VariableFormComponent extends Component {

this.removeExitHandler();
this.router.transitionTo('variables.variable', this.args.model.id);
} catch (error) {
notifyConflict(this)(error);
} catch (e) {
notifyConflict(this)(e);
if (!this.hasConflict) {
let errorMessage = e;
if (e.errors && e.errors.length > 0) {
const nameInvalidError = e.errors.find((err) => err.status === 400);
if (nameInvalidError) {
errorMessage = nameInvalidError.detail;
}
}

console.log('caught an error', e);
this.notifications.add({
title: `Error saving ${this.path}`,
message: error,
message: errorMessage,
color: 'critical',
sticky: true,
});
} else {
if (error.errors[0]?.detail) {
set(this, 'conflictingVariable', error.errors[0].detail);
if (e.errors[0]?.detail) {
set(this, 'conflictingVariable', e.errors[0].detail);
}
window.scrollTo(0, 0); // because the k/v list may be long, ensure the user is snapped to top to read error
}
Expand Down
17 changes: 16 additions & 1 deletion ui/app/controllers/jobs/run/templates/new.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ export default class JobsRunTemplatesNewController extends Controller {
);
}

get hasInvalidName() {
let pathNameRegex = new RegExp('^[a-zA-Z0-9-_~/]{1,128}$');
return !pathNameRegex.test(this.templateName);
}

@action
updateKeyValue(key, value) {
if (this.model.keyValues.find((kv) => kv.key === key)) {
Expand Down Expand Up @@ -74,9 +79,19 @@ export default class JobsRunTemplatesNewController extends Controller {

this.router.transitionTo('jobs.run.templates');
} catch (e) {
let errorMessage =
'An unexpected error occurred when saving your Job template.';
console.log('caught', e);
if (e.errors && e.errors.length > 0) {
const nameInvalidError = e.errors.find((err) => err.status === 400);
if (nameInvalidError) {
errorMessage = nameInvalidError.detail;
}
}

this.notifications.add({
title: 'Job template cannot be saved.',
message: e,
message: errorMessage,
color: 'critical',
});
}
Expand Down
5 changes: 5 additions & 0 deletions ui/app/templates/jobs/run/templates/new.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@
There is already a templated named {{this.templateName}}.
</p>
{{/if}}
{{#if this.hasInvalidName}}
<p class="help is-danger" data-test-invalid-name-error>
Template name must contain only alphanumeric or "-", "_", "~", or "/" characters, and be fewer than 128 characters in length.
</p>
{{/if}}
</label>
{{#if this.system.shouldShowNamespaces}}
<label>
Expand Down
Loading