Backport 1.9.x: Raft Peer Removal Bug (#13212)

* 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

@@ -0,0 +1,3 @@
+```release-note:bug
+ui: Fixes issue removing raft storage peer via cli not reflected in UI until refresh
+```

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';

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);
   },
 

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: {

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)),
+          },
+        };
+      }
+    },
+  }),
+});

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,
+});

ui/tests/.eslintrc.js

@@ -4,7 +4,6 @@ module.exports = {
     embertest: true,
   },
   globals: {
-    faker: true,
     server: true,
     $: true,
     authLogout: false,

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');
+  });
+});