UI: Always add the active region as a query param to API requests

CHANGELOG.md

@@ -21,6 +21,7 @@ BUG FIXES:
  * ui: Fixed missing namespace query param after changing acl tokens [[GH-8413](https://github.com/hashicorp/nomad/issues/8413)]
  * ui: Fixed exec to derive group and task when possible from allocation [[GH-8463](https://github.com/hashicorp/nomad/pull/8463)]
  * ui: Fixed runtime error when clicking "Run Job" while a prefix filter is set [[GH-8412](https://github.com/hashicorp/nomad/issues/8412)]
+ * ui: Fixed the absence of the region query parameter on various actions, such as job stop, allocation restart, node drain. [[GH-8477](https://github.com/hashicorp/nomad/issues/8477)]
  * vault: Fixed a bug where vault identity policies not considered in permissions check [[GH-7732](https://github.com/hashicorp/nomad/issues/7732)]
 
 ## 0.12.0 (July 9, 2020)

ui/app/adapters/application.js

@@ -49,15 +49,18 @@ export default class ApplicationAdapter extends RESTAdapter {
     });
   }
 
-  ajaxOptions(url, type, options = {}) {
+  ajaxOptions(url, verb, options = {}) {
     options.data || (options.data = {});
     if (this.get('system.shouldIncludeRegion')) {
+      // Region should only ever be a query param. The default ajaxOptions
+      // behavior is to include data attributes in the requestBody for PUT
+      // and POST requests. This works around that.
       const region = this.get('system.activeRegion');
       if (region) {
-        options.data.region = region;
+        url = associateRegion(url, region);
       }
     }
-    return super.ajaxOptions(url, type, options);
+    return super.ajaxOptions(url, verb, options);
   }
 
   // In order to remove stale records from the store, findHasMany has to unload
@@ -119,3 +122,7 @@ export default class ApplicationAdapter extends RESTAdapter {
     return this.urlForFindRecord(...arguments);
   }
 }
+
+function associateRegion(url, region) {
+  return url.indexOf('?') !== -1 ? `${url}&region=${region}` : `${url}?region=${region}`;
+}

ui/tests/unit/adapters/allocation-test.js

@@ -9,69 +9,108 @@ module('Unit | Adapter | Allocation', function(hooks) {
     this.store = this.owner.lookup('service:store');
     this.subject = () => this.store.adapterFor('allocation');
 
+    window.localStorage.clear();
+
     this.server = startMirage();
 
-    this.server.create('namespace');
-    this.server.create('node');
-    this.server.create('job', { createAllocations: false });
-    this.server.create('allocation', { id: 'alloc-1' });
+    this.initialize = async (allocationId, { region } = {}) => {
+      if (region) window.localStorage.nomadActiveRegion = region;
+
+      this.server.create('namespace');
+      this.server.create('region', { id: 'region-1' });
+      this.server.create('region', { id: 'region-2' });
+
+      this.server.create('node');
+      this.server.create('job', { createAllocations: false });
+      this.server.create('allocation', { id: 'alloc-1' });
+      this.system = this.owner.lookup('service:system');
+      await this.system.get('namespaces');
+      this.system.get('shouldIncludeRegion');
+      await this.system.get('defaultRegion');
+
+      const allocation = await this.store.findRecord('allocation', allocationId);
+      this.server.pretender.handledRequests.length = 0;
+
+      return allocation;
+    };
   });
 
   hooks.afterEach(function() {
     this.server.shutdown();
   });
 
-  test('`stop` makes the correct API call', async function(assert) {
-    const { pretender } = this.server;
-    const allocationId = 'alloc-1';
+  const testCases = [
+    {
+      variation: '',
+      id: 'alloc-1',
+      task: 'task-name',
+      region: null,
+      path: 'some/path',
+      ls: `GET /v1/client/fs/ls/alloc-1?path=${encodeURIComponent('some/path')}`,
+      stat: `GET /v1/client/fs/stat/alloc-1?path=${encodeURIComponent('some/path')}`,
+      stop: 'POST /v1/allocation/alloc-1/stop',
+      restart: 'PUT /v1/client/allocation/alloc-1/restart',
+    },
+    {
+      variation: 'with non-default region',
+      id: 'alloc-1',
+      task: 'task-name',
+      region: 'region-2',
+      path: 'some/path',
+      ls: `GET /v1/client/fs/ls/alloc-1?path=${encodeURIComponent('some/path')}&region=region-2`,
+      stat: `GET /v1/client/fs/stat/alloc-1?path=${encodeURIComponent(
+        'some/path'
+      )}&region=region-2`,
+      stop: 'POST /v1/allocation/alloc-1/stop?region=region-2',
+      restart: 'PUT /v1/client/allocation/alloc-1/restart?region=region-2',
+    },
+  ];
 
-    const allocation = await this.store.findRecord('allocation', allocationId);
-    pretender.handledRequests.length = 0;
+  testCases.forEach(testCase => {
+    test(`ls makes the correct API call ${testCase.variation}`, async function(assert) {
+      const { pretender } = this.server;
+      const allocation = await this.initialize(testCase.id, { region: testCase.region });
 
-    await this.subject().stop(allocation);
-    const req = pretender.handledRequests[0];
-    assert.equal(
-      `${req.method} ${req.url}`,
-      `POST /v1/allocation/${allocationId}/stop`,
-      `POST /v1/allocation/${allocationId}/stop`
-    );
-  });
+      await this.subject().ls(allocation, testCase.path);
+      const req = pretender.handledRequests[0];
+      assert.equal(`${req.method} ${req.url}`, testCase.ls);
+    });
 
-  test('`restart` makes the correct API call', async function(assert) {
-    const { pretender } = this.server;
-    const allocationId = 'alloc-1';
+    test(`stat makes the correct API call ${testCase.variation}`, async function(assert) {
+      const { pretender } = this.server;
+      const allocation = await this.initialize(testCase.id, { region: testCase.region });
 
-    const allocation = await this.store.findRecord('allocation', allocationId);
-    pretender.handledRequests.length = 0;
+      await this.subject().stat(allocation, testCase.path);
+      const req = pretender.handledRequests[0];
+      assert.equal(`${req.method} ${req.url}`, testCase.stat);
+    });
 
-    await this.subject().restart(allocation);
-    const req = pretender.handledRequests[0];
-    assert.equal(
-      `${req.method} ${req.url}`,
-      `PUT /v1/client/allocation/${allocationId}/restart`,
-      `PUT /v1/client/allocation/${allocationId}/restart`
-    );
-  });
+    test(`stop makes the correct API call ${testCase.variation}`, async function(assert) {
+      const { pretender } = this.server;
+      const allocation = await this.initialize(testCase.id, { region: testCase.region });
+
+      await this.subject().stop(allocation);
+      const req = pretender.handledRequests[0];
+      assert.equal(`${req.method} ${req.url}`, testCase.stop);
+    });
+
+    test(`restart makes the correct API call ${testCase.variation}`, async function(assert) {
+      const { pretender } = this.server;
+      const allocation = await this.initialize(testCase.id, { region: testCase.region });
+
+      await this.subject().restart(allocation);
+      const req = pretender.handledRequests[0];
+      assert.equal(`${req.method} ${req.url}`, testCase.restart);
+    });
+
+    test(`restart with optional task name makes the correct API call ${testCase.variation}`, async function(assert) {
+      const { pretender } = this.server;
+      const allocation = await this.initialize(testCase.id, { region: testCase.region });
 
-  test('`restart` takes an optional task name and makes the correct API call', async function(assert) {
-    const { pretender } = this.server;
-    const allocationId = 'alloc-1';
-    const taskName = 'task-name';
-
-    const allocation = await this.store.findRecord('allocation', allocationId);
-    pretender.handledRequests.length = 0;
-
-    await this.subject().restart(allocation, taskName);
-    const req = pretender.handledRequests[0];
-    assert.equal(
-      `${req.method} ${req.url}`,
-      `PUT /v1/client/allocation/${allocationId}/restart`,
-      `PUT /v1/client/allocation/${allocationId}/restart`
-    );
-    assert.deepEqual(
-      JSON.parse(req.requestBody),
-      { TaskName: taskName },
-      'Request body is correct'
-    );
+      await this.subject().restart(allocation, testCase.task);
+      const req = pretender.handledRequests[0];
+      assert.equal(`${req.method} ${req.url}`, testCase.restart);
+      assert.deepEqual(JSON.parse(req.requestBody), { TaskName: testCase.task });
+    });
   });
 });

ui/tests/unit/adapters/deployment-test.js

@@ -0,0 +1,68 @@
+import { module, test } from 'qunit';
+import { setupTest } from 'ember-qunit';
+import { startMirage } from 'nomad-ui/initializers/ember-cli-mirage';
+
+module('Unit | Adapter | Deployment', function(hooks) {
+  setupTest(hooks);
+
+  hooks.beforeEach(async function() {
+    this.store = this.owner.lookup('service:store');
+    this.system = this.owner.lookup('service:system');
+    this.subject = () => this.store.adapterFor('deployment');
+
+    window.localStorage.clear();
+
+    this.server = startMirage();
+
+    this.initialize = async ({ region } = {}) => {
+      if (region) window.localStorage.nomadActiveRegion = region;
+
+      this.server.create('region', { id: 'region-1' });
+      this.server.create('region', { id: 'region-2' });
+
+      this.server.create('node');
+      const job = this.server.create('job', { createAllocations: false });
+      const deploymentRecord = server.schema.deployments.where({ jobId: job.id }).models[0];
+
+      this.system.get('shouldIncludeRegion');
+      await this.system.get('defaultRegion');
+
+      const deployment = await this.store.findRecord('deployment', deploymentRecord.id);
+      this.server.pretender.handledRequests.length = 0;
+
+      return deployment;
+    };
+  });
+
+  hooks.afterEach(function() {
+    this.server.shutdown();
+  });
+
+  const testCases = [
+    {
+      variation: '',
+      region: null,
+      promote: id => `POST /v1/deployment/promote/${id}`,
+    },
+    {
+      variation: 'with non-default region',
+      region: 'region-2',
+      promote: id => `POST /v1/deployment/promote/${id}?region=region-2`,
+    },
+  ];
+
+  testCases.forEach(testCase => {
+    test(`promote makes the correct API call ${testCase.variation}`, async function(assert) {
+      const deployment = await this.initialize({ region: testCase.region });
+      await this.subject().promote(deployment);
+
+      const request = this.server.pretender.handledRequests[0];
+
+      assert.equal(`${request.method} ${request.url}`, testCase.promote(deployment.id));
+      assert.deepEqual(JSON.parse(request.requestBody), {
+        DeploymentId: deployment.id,
+        All: true,
+      });
+    });
+  });
+});