From fcf31ffeaf77859f9e57c10ce40a3465652bb9bb Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Thu, 28 Jan 2021 14:29:08 -0800 Subject: [PATCH 1/2] Always include the region param in server monitor requests The region will naturally be appended to URLs via token.authorizedRequest but agent members includes all servers across all regions so relying on the application-level region isn't good enough. --- ui/app/components/agent-monitor.js | 8 +++++++- .../integration/components/agent-monitor-test.js | 14 ++++++++------ 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/ui/app/components/agent-monitor.js b/ui/app/components/agent-monitor.js index 8cff2581c81..79188ed0924 100644 --- a/ui/app/components/agent-monitor.js +++ b/ui/app/components/agent-monitor.js @@ -33,10 +33,16 @@ export default class AgentMonitor extends Component { const type = this.server ? 'server_id' : 'client_id'; const id = this.server ? this.server.id : this.client && this.client.id; - return { + const params = { log_level: this.level, [type]: id, }; + + if (this.server) { + params.region = this.server.region; + } + + return params; } didInsertElement() { diff --git a/ui/tests/integration/components/agent-monitor-test.js b/ui/tests/integration/components/agent-monitor-test.js index 5a9728f990b..587967d7df6 100644 --- a/ui/tests/integration/components/agent-monitor-test.js +++ b/ui/tests/integration/components/agent-monitor-test.js @@ -34,10 +34,10 @@ module('Integration | Component | agent-monitor', function(hooks) { const commonTemplate = hbs` + @level={{this.level}} + @client={{this.client}} + @server={{this.server}} + @onLevelChange={{this.onLevelChange}} /> `; test('basic appearance', async function(assert) { @@ -61,7 +61,7 @@ module('Integration | Component | agent-monitor', function(hooks) { test('when provided with a client, AgentMonitor streams logs for the client', async function(assert) { this.setProperties({ level: 'info', - client: { id: 'client1' }, + client: { id: 'client1', region: 'us-west-1' }, }); run.later(run, run.cancelTimers, INTERVAL); @@ -73,12 +73,13 @@ module('Integration | Component | agent-monitor', function(hooks) { assert.ok(logRequest.url.includes('client_id=client1')); assert.ok(logRequest.url.includes('log_level=info')); assert.notOk(logRequest.url.includes('server_id')); + assert.notOk(logRequest.url.includes('region=')); }); test('when provided with a server, AgentMonitor streams logs for the server', async function(assert) { this.setProperties({ level: 'warn', - server: { id: 'server1' }, + server: { id: 'server1', region: 'us-west-1' }, }); run.later(run, run.cancelTimers, INTERVAL); @@ -89,6 +90,7 @@ module('Integration | Component | agent-monitor', function(hooks) { assert.ok(logRequest.url.startsWith('/v1/agent/monitor')); assert.ok(logRequest.url.includes('server_id=server1')); assert.ok(logRequest.url.includes('log_level=warn')); + assert.ok(logRequest.url.includes('region=us-west-1')); assert.notOk(logRequest.url.includes('client_id')); }); From 2c13731d95ab84d39c13625594cca27b45439736 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Thu, 28 Jan 2021 14:30:20 -0800 Subject: [PATCH 2/2] Don't include the region param in authorizedRequest if it's already in the URL --- ui/app/services/token.js | 2 +- ui/tests/unit/services/token-test.js | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/ui/app/services/token.js b/ui/app/services/token.js index 78d43feb1a7..d98ac234648 100644 --- a/ui/app/services/token.js +++ b/ui/app/services/token.js @@ -94,7 +94,7 @@ export default class TokenService extends Service { authorizedRequest(url, options) { if (this.get('system.shouldIncludeRegion')) { const region = this.get('system.activeRegion'); - if (region) { + if (region && url.indexOf('region=') === -1) { url = addParams(url, { region }); } } diff --git a/ui/tests/unit/services/token-test.js b/ui/tests/unit/services/token-test.js index 5bc72437ae9..edf1bc26662 100644 --- a/ui/tests/unit/services/token-test.js +++ b/ui/tests/unit/services/token-test.js @@ -50,6 +50,17 @@ module('Unit | Service | Token', function(hooks) { ); }); + test('authorizedRequest does not include the region param when the region param is already in the URL', function(assert) { + const token = this.subject(); + + token.authorizedRequest('/path?query=param®ion=already-here'); + assert.equal( + this.server.handledRequests.pop().url, + '/path?query=param®ion=already-here', + 'The region param that is already in the URL takes precedence over the region in the service' + ); + }); + test('authorizedRawRequest bypasses adding the region param', function(assert) { const token = this.subject();