UI/fix kv data cache (#14489) (#14533)

* KV fetches recent version on every page, no longer disallow new version without metadata access * Don't flash no read permissions warning * Send noMetadataVersion on destroy if version is undefined * test coverage * add changelog, fix tests * Fix failing test
hashicorp · Mar 16, 2022 · e634c5f · e634c5f
1 parent fee1524
commit e634c5f
Show file tree

Hide file tree

Showing 8 changed files with 94 additions and 100 deletions.
diff --git a/changelog/14489.txt b/changelog/14489.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+ui: Fixes caching issue on kv new version create
+```
diff --git a/ui/app/adapters/secret-v2-version.js b/ui/app/adapters/secret-v2-version.js
@@ -26,7 +26,7 @@ export default ApplicationAdapter.extend({
   },
 
   findRecord() {
-    return this._super(...arguments).catch(errorOrModel => {
+    return this._super(...arguments).catch((errorOrModel) => {
       // if the response is a real 404 or if the secret is gated by a control group this will be an error,
       // otherwise the response will be the body of a deleted / destroyed version
       if (errorOrModel instanceof AdapterError) {
@@ -44,7 +44,7 @@ export default ApplicationAdapter.extend({
   },
 
   queryRecord(id, options) {
-    return this.ajax(this.urlForQueryRecord(id), 'GET', options).then(resp => {
+    return this.ajax(this.urlForQueryRecord(id), 'GET', options).then((resp) => {
       if (options.wrapTTL) {
         return resp;
       }
@@ -63,7 +63,7 @@ export default ApplicationAdapter.extend({
   createRecord(store, modelName, snapshot) {
     let backend = snapshot.belongsTo('secret').belongsTo('engine').id;
     let path = snapshot.attr('path');
-    return this._super(...arguments).then(resp => {
+    return this._super(...arguments).then((resp) => {
       resp.id = JSON.stringify([backend, path, resp.version]);
       return resp;
     });
@@ -139,6 +139,7 @@ export default ApplicationAdapter.extend({
     } else if (deleteType === 'soft-delete') {
       return this.softDelete(backend, path, version);
     } else {
+      version = version || currentVersionForNoReadMetadata;
       return this.deleteByDeleteType(backend, path, deleteType, version);
     }
   },

diff --git a/ui/app/routes/vault/cluster/secrets/backend/secret-edit.js b/ui/app/routes/vault/cluster/secrets/backend/secret-edit.js
@@ -248,20 +248,7 @@ export default Route.extend(UnloadModelRoute, {
     if (modelType === 'secret-v2') {
       // after the the base model fetch, kv-v2 has a second associated
       // version model that contains the secret data
-
-      // if no read access to metadata, return current Version from secret data.
-      if (!secretModel.currentVersion) {
-        let adapter = this.store.adapterFor('secret-v2-version');
-        try {
-          secretModel.currentVersion = await adapter.getSecretDataVersion(backend, secret);
-        } catch {
-          // will get error if you have deleted the secret
-          // if this is the case do nothing
-        }
-        secretModel = await this.fetchV2Models(capabilities, secretModel, params);
-      } else {
-        secretModel = await this.fetchV2Models(capabilities, secretModel, params);
-      }
+      secretModel = await this.fetchV2Models(capabilities, secretModel, params);
     }
     return {
       secret: secretModel,
@@ -283,10 +270,7 @@ export default Route.extend(UnloadModelRoute, {
       capabilities: model.capabilities,
       baseKey: { id: secret },
       // mode will be 'show', 'edit', 'create'
-      mode: this.routeName
-        .split('.')
-        .pop()
-        .replace('-root', ''),
+      mode: this.routeName.split('.').pop().replace('-root', ''),
       backend,
       preferAdvancedEdit,
       backendType,
@@ -322,15 +306,15 @@ export default Route.extend(UnloadModelRoute, {
       // when you don't have read access on metadata we add currentVersion to the model
       // this makes it look like you have unsaved changes and prompts a browser warning
       // here we are specifically ignoring it.
-      if (mode === 'edit' && (changedKeys.length && changedKeys[0] === 'currentVersion')) {
+      if (mode === 'edit' && changedKeys.length && changedKeys[0] === 'currentVersion') {
         version && version.rollbackAttributes();
         return true;
       }
       // until we have time to move `backend` on a v1 model to a relationship,
       // it's going to dirty the model state, so we need to look for it
       // and explicity ignore it here
       if (
-        (mode !== 'show' && (changedKeys.length && changedKeys[0] !== 'backend')) ||
+        (mode !== 'show' && changedKeys.length && changedKeys[0] !== 'backend') ||
         (mode !== 'show' && version && Object.keys(version.changedAttributes()).length)
       ) {
         if (

diff --git a/ui/app/templates/components/secret-create-or-update.hbs b/ui/app/templates/components/secret-create-or-update.hbs
@@ -150,13 +150,13 @@
   <form onsubmit={{action "createOrUpdateKey" "edit"}}>
     <div class="box is-sideless is-fullwidth is-marginless padding-top">
       <MessageError @model={{@modelForData}} @errorMessage={{this.error}} />
-      {{#unless @canReadSecretData}}
+      {{#if (eq @canReadSecretData false)}}
         <AlertBanner
           @type="warning"
           @message="You do not have read permissions. If a secret exists here creating a new secret will overwrite it."
           data-test-warning-no-read-permissions
         />
-      {{/unless}}
+      {{/if}}
       <NamespaceReminder @mode="edit" @noun="secret" />
       {{#if this.isCreateNewVersionFromOldVersion}}
         <div class="form-section">

diff --git a/ui/app/templates/components/secret-edit-toolbar.hbs b/ui/app/templates/components/secret-edit-toolbar.hbs
@@ -97,12 +97,15 @@
       {{#let (concat 'vault.cluster.secrets.backend.' (if (eq @mode 'show') 'edit' 'show')) as |targetRoute|}}
         {{#if @isV2}}
           <ToolbarLink
-            @params={{array targetRoute @model.id (query-params version=@modelForData.version)}}
+            {{! Always create new version from latest if no metadata read access }}
+            @params={{array
+              targetRoute
+              @model.id
+              (query-params version=(if @model.canReadMetadata @modelForData.version ""))
+            }}
             @data-test-secret-edit="true"
             @replace={{true}}
             @type="add"
-            @disabled={{@model.failedServerRead}}
-            @disabledTooltip="Metadata read access is required to create new version"
           >
             Create new version
           </ToolbarLink>

diff --git a/ui/tests/acceptance/access/identity/_shared-alias-tests.js b/ui/tests/acceptance/access/identity/_shared-alias-tests.js
@@ -5,7 +5,7 @@ import aliasShowPage from 'vault/tests/pages/access/identity/aliases/show';
 import createItemPage from 'vault/tests/pages/access/identity/create';
 import showItemPage from 'vault/tests/pages/access/identity/show';
 
-export const testAliasCRUD = async function(name, itemType, assert) {
+export const testAliasCRUD = async function (name, itemType, assert) {
   let itemID;
   let aliasID;
   let idRow;
@@ -59,7 +59,7 @@ export const testAliasCRUD = async function(name, itemType, assert) {
   assert.equal(aliasIndexPage.items.filterBy('id', aliasID).length, 0, `${itemType}: the row is deleted`);
 };
 
-export const testAliasDeleteFromForm = async function(name, itemType, assert) {
+export const testAliasDeleteFromForm = async function (name, itemType, assert) {
   let itemID;
   let aliasID;
   let idRow;
@@ -86,14 +86,13 @@ export const testAliasDeleteFromForm = async function(name, itemType, assert) {
     `${itemType}: navigates to edit on create`
   );
   await page.editForm.delete();
-  await settled();
+  await page.editForm.waitForConfirm();
   await page.editForm.confirmDelete();
   await settled();
   assert.ok(
     aliasIndexPage.flashMessage.latestMessage.startsWith('Successfully deleted'),
     `${itemType}: shows flash message`
   );
-
   assert.equal(
     currentRouteName(),
     'vault.cluster.access.identity.aliases.index',