From 43a071e5d892dc3cc4c733f024ccfc15655a63e1 Mon Sep 17 00:00:00 2001 From: Jordan Reimer Date: Tue, 25 Jan 2022 13:13:06 -0700 Subject: [PATCH] Backport 1.7.x: Raft peer removal bug (#13214) * Raft peer removal bug (#13098) * fixes issue removing raft peer via cli not reflected in UI until refresh * adds changelog entry * removes faker * attempts to fix global error in circle ci run --- changelog/13098.txt | 3 + ui/app/adapters/server.js | 6 +- ui/app/components/file-to-array-buffer.js | 4 +- ui/app/routes/vault/cluster/storage.js | 5 +- ui/mirage/factories/configuration.js | 26 ++++++++ ui/mirage/factories/server.js | 9 +++ ui/tests/.eslintrc.js | 1 - ui/tests/acceptance/raft-storage-test.js | 73 +++++++++++++++++++++++ 8 files changed, 123 insertions(+), 4 deletions(-) create mode 100644 changelog/13098.txt create mode 100644 ui/mirage/factories/configuration.js create mode 100644 ui/mirage/factories/server.js create mode 100644 ui/tests/acceptance/raft-storage-test.js diff --git a/changelog/13098.txt b/changelog/13098.txt new file mode 100644 index 000000000000..84bbcbfee05b --- /dev/null +++ b/changelog/13098.txt @@ -0,0 +1,3 @@ +```release-note:bug +ui: Fixes issue removing raft storage peer via cli not reflected in UI until refresh +``` \ No newline at end of file diff --git a/ui/app/adapters/server.js b/ui/app/adapters/server.js index 437e7d4da283..b7c7cda60b33 100644 --- a/ui/app/adapters/server.js +++ b/ui/app/adapters/server.js @@ -1,8 +1,12 @@ import ApplicationAdapter from './application'; +const fetchUrl = '/v1/sys/storage/raft/configuration'; export default ApplicationAdapter.extend({ urlForFindAll() { - return '/v1/sys/storage/raft/configuration'; + return fetchUrl; + }, + urlForQuery() { + return fetchUrl; }, urlForDeleteRecord() { return '/v1/sys/storage/raft/remove-peer'; diff --git a/ui/app/components/file-to-array-buffer.js b/ui/app/components/file-to-array-buffer.js index 6c67df20de10..43c3e6ba42c2 100644 --- a/ui/app/components/file-to-array-buffer.js +++ b/ui/app/components/file-to-array-buffer.js @@ -29,7 +29,9 @@ export default Component.extend({ readFile(file) { const reader = new FileReader(); - reader.onload = () => this.send('onChange', reader.result, file); + if (!this.isDestroyed && !this.isDestroying) { + reader.onload = () => this.send('onChange', reader.result, file); + } reader.readAsArrayBuffer(file); }, diff --git a/ui/app/routes/vault/cluster/storage.js b/ui/app/routes/vault/cluster/storage.js index f111a000fbe4..c9808bc539ae 100644 --- a/ui/app/routes/vault/cluster/storage.js +++ b/ui/app/routes/vault/cluster/storage.js @@ -3,7 +3,10 @@ import ClusterRoute from 'vault/mixins/cluster-route'; export default Route.extend(ClusterRoute, { model() { - return this.store.findAll('server'); + // findAll method will return all records in store as well as response from server + // when removing a peer via the cli, stale records would continue to appear until refresh + // query method will only return records from response + return this.store.query('server', {}); }, actions: { diff --git a/ui/mirage/factories/configuration.js b/ui/mirage/factories/configuration.js new file mode 100644 index 000000000000..a64f830923ba --- /dev/null +++ b/ui/mirage/factories/configuration.js @@ -0,0 +1,26 @@ +import { Factory, trait } from 'ember-cli-mirage'; + +export default Factory.extend({ + auth: null, + data: null, // populated via traits + lease_duration: 0, + lease_id: '', + renewable: true, + request_id: '22068a49-a504-41ad-b5b0-1eac71659190', + warnings: null, + wrap_info: null, + + // add servers to test raft storage configuration + withRaft: trait({ + afterCreate(config, server) { + if (!config.data) { + config.data = { + config: { + index: 0, + servers: server.serializerOrRegistry.serialize(server.createList('server', 2)), + }, + }; + } + }, + }), +}); diff --git a/ui/mirage/factories/server.js b/ui/mirage/factories/server.js new file mode 100644 index 000000000000..2d9100b4d00e --- /dev/null +++ b/ui/mirage/factories/server.js @@ -0,0 +1,9 @@ +import { Factory } from 'ember-cli-mirage'; + +export default Factory.extend({ + address: '127.0.0.1', + node_id: (i) => `raft_node_${i}`, + protocol_version: '3', + voter: true, + leader: true, +}); diff --git a/ui/tests/.eslintrc.js b/ui/tests/.eslintrc.js index 182d28b6c208..dbd2a5460c1c 100644 --- a/ui/tests/.eslintrc.js +++ b/ui/tests/.eslintrc.js @@ -4,7 +4,6 @@ module.exports = { embertest: true, }, globals: { - faker: true, server: true, $: true, authLogout: false, diff --git a/ui/tests/acceptance/raft-storage-test.js b/ui/tests/acceptance/raft-storage-test.js new file mode 100644 index 000000000000..62ebc6e74eb9 --- /dev/null +++ b/ui/tests/acceptance/raft-storage-test.js @@ -0,0 +1,73 @@ +import { module, test } from 'qunit'; +import { setupApplicationTest } from 'ember-qunit'; +import { setupMirage } from 'ember-cli-mirage/test-support'; +import { click, visit } from '@ember/test-helpers'; +import authPage from 'vault/tests/pages/auth'; +import logout from 'vault/tests/pages/logout'; + +module('Acceptance | raft storage', function(hooks) { + setupApplicationTest(hooks); + setupMirage(hooks); + + hooks.beforeEach(async function() { + this.config = this.server.create('configuration', 'withRaft'); + this.server.get('/sys/internal/ui/resultant-acl', () => + this.server.create('configuration', { data: { root: true } }) + ); + this.server.get('/sys/license/features', () => ({})); + await authPage.login(); + }); + hooks.afterEach(function() { + return logout.visit(); + }); + + test('it should render correct number of raft peers', async function(assert) { + assert.expect(3); + + let didRemovePeer = false; + this.server.get('/sys/storage/raft/configuration', () => { + if (didRemovePeer) { + this.config.data.config.servers.pop(); + } else { + // consider peer removed by external means (cli) after initial request + didRemovePeer = true; + } + return this.config; + }); + + await visit('/vault/storage/raft'); + assert.dom('[data-raft-row]').exists({ count: 2 }, '2 raft peers render in table'); + // leave route and return to trigger config fetch + await visit('/vault/secrets'); + await visit('/vault/storage/raft'); + const store = this.owner.lookup('service:store'); + assert.equal( + store.peekAll('server').length, + 2, + 'Store contains 2 server records since remove peer was triggered externally' + ); + assert.dom('[data-raft-row]').exists({ count: 1 }, 'Only raft nodes from response are rendered'); + }); + + test('it should remove raft peer', async function(assert) { + assert.expect(3); + + this.server.get('/sys/storage/raft/configuration', () => this.config); + this.server.post('/sys/storage/raft/remove-peer', (schema, req) => { + const body = JSON.parse(req.requestBody); + assert.equal( + body.server_id, + this.config.data.config.servers[1].node_id, + 'Remove peer request made with node id' + ); + return {}; + }); + + await visit('/vault/storage/raft'); + assert.dom('[data-raft-row]').exists({ count: 2 }, '2 raft peers render in table'); + await click('[data-raft-row]:nth-child(2) [data-test-popup-menu-trigger]'); + await click('[data-test-confirm-action-trigger]'); + await click('[data-test-confirm-button'); + assert.dom('[data-raft-row]').exists({ count: 1 }, 'Raft peer successfully removed'); + }); +});