From 7c7d626420a16b3d343faf0367554f595c6cc7e6 Mon Sep 17 00:00:00 2001 From: Jordan Reimer Date: Tue, 9 Nov 2021 15:05:25 -0700 Subject: [PATCH] Raft peer removal bug (#13098) * fixes issue removing raft peer via cli not reflected in UI until refresh * adds changelog entry --- changelog/13098.txt | 3 + ui/app/adapters/server.js | 6 +- ui/app/routes/vault/cluster/storage.js | 5 +- ui/mirage/factories/configuration.js | 27 +++++++++ ui/mirage/factories/server.js | 10 ++++ ui/tests/acceptance/raft-storage-test.js | 73 ++++++++++++++++++++++++ 6 files changed, 122 insertions(+), 2 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 000000000..84bbcbfee --- /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 437e7d4da..b7c7cda60 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/routes/vault/cluster/storage.js b/ui/app/routes/vault/cluster/storage.js index f111a000f..c9808bc53 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 000000000..3adea1e6c --- /dev/null +++ b/ui/mirage/factories/configuration.js @@ -0,0 +1,27 @@ +import { Factory, trait } from 'ember-cli-mirage'; +import faker from 'faker'; + +export default Factory.extend({ + auth: null, + data: null, // populated via traits + lease_duration: 0, + lease_id: '', + renewable: () => faker.datatype.boolean(), + request_id: () => faker.datatype.uuid(), + 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 000000000..96ef5f53e --- /dev/null +++ b/ui/mirage/factories/server.js @@ -0,0 +1,10 @@ +import { Factory } from 'ember-cli-mirage'; +import faker from 'faker'; + +export default Factory.extend({ + address: () => faker.internet.ip(), + node_id: i => `raft_node_${i}`, + protocol_version: '3', + voter: () => faker.datatype.boolean(), + leader: () => faker.datatype.boolean(), +}); diff --git a/ui/tests/acceptance/raft-storage-test.js b/ui/tests/acceptance/raft-storage-test.js new file mode 100644 index 000000000..62ebc6e74 --- /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'); + }); +});