From 47024a27d503a5c627f240fffc7100a4551b9608 Mon Sep 17 00:00:00 2001
From: Jordan Reimer <zofskeez@gmail.com>
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');
+ });
+});
--
GitLab