Skip to content

Commit

Permalink
[bugfix, ui] Allow running jobs from a namespace-limited token (#13659)
Browse files Browse the repository at this point in the history
* Allow running jobs from a namespace-limited token

* qpNamespace cleanup

* Looks like parse can deal with a * namespace

* A little diff cleanup

* Defensive destructuring

* Removing accidental friendly-fire on can-scale

* Testfix: Job run buttons from jobs index

* Testfix: activeRegion job adapter string

* Testfix: unit tests for job abilities correctly reflect the any-namespace rule

* Testfix: job editor test looks for requests with namespace applied on plan
  • Loading branch information
philrenaud authored and lgfa29 committed Jul 11, 2022
1 parent bacb23e commit 61d8dd8
Show file tree
Hide file tree
Showing 7 changed files with 70 additions and 13 deletions.
29 changes: 26 additions & 3 deletions ui/app/abilities/job.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import AbstractAbility from './abstract';
import { computed } from '@ember/object';
import { computed, get } from '@ember/object';
import { or } from '@ember/object/computed';

export default class Job extends AbstractAbility {
Expand All @@ -9,7 +9,7 @@ export default class Job extends AbstractAbility {
@or(
'bypassAuthorization',
'selfTokenIsManagement',
'policiesSupportRunning',
'specificNamespaceSupportsRunning',
'policiesSupportScaling'
)
canScale;
Expand All @@ -23,8 +23,31 @@ export default class Job extends AbstractAbility {
@or('bypassAuthorization', 'selfTokenIsManagement', 'policiesSupportDispatching')
canDispatch;

@computed('rulesForNamespace.@each.capabilities')
policyNamespacesIncludePermissions(policies = [], permissions = []) {
// For each policy record, extract all policies of all namespaces
const allNamespacePolicies = policies
.toArray()
.map(policy => get(policy, 'rulesJSON.Namespaces'))
.flat()
.map((namespace = {}) => {
return namespace.Capabilities;
})
.flat()
.compact();

// Check for requested permissions
return allNamespacePolicies.some(policy => {
return permissions.includes(policy);
});
}

@computed('token.selfTokenPolicies.[]')
get policiesSupportRunning() {
return this.policyNamespacesIncludePermissions(this.token.selfTokenPolicies, ['submit-job']);
}

@computed('rulesForNamespace.@each.capabilities')
get specificNamespaceSupportsRunning() {
return this.namespaceIncludesCapability('submit-job');
}

Expand Down
2 changes: 1 addition & 1 deletion ui/app/adapters/job.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export default class JobAdapter extends WatchableNamespaceIDs {
}

parse(spec) {
const url = addToPath(this.urlForFindAll('job'), '/parse');
const url = addToPath(this.urlForFindAll('job'), '/parse?namespace=*');
return this.ajax(url, 'POST', {
data: {
JobHCL: spec,
Expand Down
4 changes: 2 additions & 2 deletions ui/app/components/job-editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export default class JobEditor extends Component {
try {
yield this.job.parse();
} catch (err) {
const error = messageFromAdapterError(err) || 'Could not parse input';
const error = messageFromAdapterError(err, 'parse jobs') || 'Could not parse input';
this.set('parseError', error);
this.scrollToError();
return;
Expand All @@ -61,7 +61,7 @@ export default class JobEditor extends Component {
const plan = yield this.job.plan();
this.set('planOutput', plan);
} catch (err) {
const error = messageFromAdapterError(err) || 'Could not plan job';
const error = messageFromAdapterError(err, 'plan jobs') || 'Could not plan job';
this.set('planError', error);
this.scrollToError();
}
Expand Down
31 changes: 31 additions & 0 deletions ui/tests/acceptance/jobs-list-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,37 @@ module('Acceptance | jobs list', function(hooks) {
await JobsList.visit({ namespace: READ_AND_WRITE_NAMESPACE });
assert.notOk(JobsList.runJobButton.isDisabled);

await JobsList.visit({ namespace: READ_ONLY_NAMESPACE });
assert.notOk(JobsList.runJobButton.isDisabled);
});

test('when the user has no client tokens that allow them to run a job', async function(assert) {
const READ_AND_WRITE_NAMESPACE = 'read-and-write-namespace';
const READ_ONLY_NAMESPACE = 'read-only-namespace';

server.create('namespace', { id: READ_ONLY_NAMESPACE });

const policy = server.create('policy', {
id: 'something',
name: 'something',
rulesJSON: {
Namespaces: [
{
Name: READ_ONLY_NAMESPACE,
Capabilities: ['list-job'],
},
],
},
});

clientToken.policyIds = [policy.id];
clientToken.save();

window.localStorage.nomadTokenSecret = clientToken.secretId;

await JobsList.visit({ namespace: READ_AND_WRITE_NAMESPACE });
assert.ok(JobsList.runJobButton.isDisabled);

await JobsList.visit({ namespace: READ_ONLY_NAMESPACE });
assert.ok(JobsList.runJobButton.isDisabled);
});
Expand Down
2 changes: 1 addition & 1 deletion ui/tests/integration/components/job-editor-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ module('Integration | Component | job-editor', function(hooks) {
await renderNewJob(this, job);
await planJob(spec);
const requests = this.server.pretender.handledRequests.mapBy('url');
assert.ok(requests.includes('/v1/jobs/parse'), 'HCL job spec is parsed first');
assert.ok(requests.includes('/v1/jobs/parse?namespace=*'), 'HCL job spec is parsed first');
assert.ok(requests.includes(`/v1/job/${newJobName}/plan`), 'HCL job spec is planned');
assert.ok(
requests.indexOf('/v1/jobs/parse') < requests.indexOf(`/v1/job/${newJobName}/plan`),
Expand Down
13 changes: 8 additions & 5 deletions ui/tests/unit/abilities/job-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -236,14 +236,17 @@ module('Unit | Ability | job', function(hooks) {
this.owner.register('service:system', mockSystem);
this.owner.register('service:token', mockToken);

assert.ok(this.can.cannot('run job', null, { namespace: 'production-web' }));
assert.ok(
this.can.can(
'run job',
null,
{ namespace: 'production-web' },
'The existence of a single namespace where a job can be run means that can run is enabled'
)
);
assert.ok(this.can.can('run job', null, { namespace: 'production-api' }));
assert.ok(this.can.can('run job', null, { namespace: 'production-other' }));
assert.ok(this.can.can('run job', null, { namespace: 'something-suffixed' }));
assert.ok(
this.can.cannot('run job', null, { namespace: 'something-more-suffixed' }),
'expected the namespace with the greatest number of matched characters to be chosen'
);
assert.ok(
this.can.can('run job', null, { namespace: '000-abc-999' }),
'expected to be able to match against more than one wildcard'
Expand Down
2 changes: 1 addition & 1 deletion ui/tests/unit/adapters/job-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,7 @@ module('Unit | Adapter | Job', function(hooks) {
await this.subject().parse('job "name-goes-here" {');

const request = this.server.pretender.handledRequests[0];
assert.equal(request.url, `/v1/jobs/parse?region=${region}`);
assert.equal(request.url, `/v1/jobs/parse?namespace=*&region=${region}`);
assert.equal(request.method, 'POST');
assert.deepEqual(JSON.parse(request.requestBody), {
JobHCL: 'job "name-goes-here" {',
Expand Down

0 comments on commit 61d8dd8

Please sign in to comment.