UI add custom metadata to KV2 #12169

Monkeychip · 2021-07-26T16:07:55Z

This PR adds custom metadata to the KV2 secret engine. It also adds the config endpoint when you're mounting the KV2 secret engine.

Here is a movie walk-through of the changes. API changes have been made on main (see PR here).

Shows metadata on secret-engine setup
Shows creation of custom metadata
Shows view and edit page of custom metadata
I log in as a user bob with a policy that only lets me list metadata but not read update or create. I, therefore, don't have access to the metadata tab.

kv.mp4

TODOs that will happen in later PRs

Handle new view when only have secret data permissions (e.g. no metadata). Includes a view with a search box to find secret.
Add test coverage.
Conditionally hide config options when select version 1.

hashishaw

Preliminary comments because this review is gonna take a while 😅

hashishaw · 2021-08-25T17:47:58Z

ui/app/adapters/secret-engine.js

+      }))(data);
+      // remove extra params from data
+      /*eslint no-unused-vars: ["error", { "ignoreRestSiblings": true }]*/
+      let { max_versions, delete_version_after, cas_required, ...newData } = data;


If any of these are undefined this will blow up, so we just need to make sure the serializer always returns those values even if they're falsey

Maybe you could help me on this one. In my testing by trying to set all values to falsey I couldn't get it to blow up, but maybe I'm missing something.

If there's no way for those to be undefined, we should be good! Just wanted to call that out as a thing to test with default values for example

hashishaw · 2021-08-25T17:49:28Z

ui/app/adapters/secret-engine.js

+    // for kv2 we make two network requests
+    if (data.type === 'kv' && data.options.version !== 1) {
+      // data has both data for sys mount and the config, we need to separate them
+      let configData = (({ max_versions, delete_version_after, cas_required }) => ({


I wonder if we can pull out the splitting logic to a helper method so that we can reuse it in other areas (like update) and it will be easier to catch errors and unit test there

Good idea. Let me work on that today.

Okay, created a new helper called split-object

ui/app/adapters/secret-engine.js

hashishaw

Great work on this beast so far! 👏

hashishaw · 2021-08-26T20:26:25Z

ui/app/components/secret-create-or-update.js

+    };
+    // for validation, return array of path names already assigned
+    if (Ember.testing) {
+      this.secretPaths = ['beep', 'bop', 'boop'];


very curious why we need to hardcode this for testing 🤔

In validation, we test for secret paths that already exist, and throw a validation if they do. This ensures the paths already exist. It also prevents us from hitting the adapter and getting an error in testing for making an adapter call that returns a 404.

hashishaw · 2021-08-26T20:29:19Z

ui/app/components/secret-create-or-update.js

+          if (isV2) {
+            secret.set('id', key);
+          }
+          if (isV2 && Object.keys(secret.changedAttributes()).length) {


I've run into unexpected behavior before by treating .length as a boolean. Granted, usually it had to do with JSX but it may be safer to check > 0 if that's what you're aiming for

hashishaw · 2021-08-26T20:33:52Z

ui/app/components/secret-edit-metadata.js

+    try {
+      await model.save();
+    } catch (err) {
+      // error


We don't want to do anything for error?

Good call, I added handling of the error.

hashishaw · 2021-08-26T20:35:12Z

ui/app/components/secret-edit-metadata.js

+  @action
+  onSaveChanges(event) {
+    event.preventDefault();
+    this.save();


Since we're not awaiting this async method, it's possible that the method returns before save finishes. Also, since save ends with a transition I wonder if we even need the return after this line. Maybe return this.save() instead?

Good call. Amended.

ui/app/helpers/split-object.js

ui/app/routes/vault/cluster/secrets/backend/metadata.js

hashishaw · 2021-08-26T21:06:50Z

ui/app/templates/components/kv-object-editor.hbs

      </InfoTooltip>
    {{/if}}
  </label>
+  {{#if true}}


do we need the if true?

Good catch. Amended.

hashishaw · 2021-08-26T21:08:24Z

ui/app/templates/components/secret-create-or-update.hbs

+    <div class="box is-sideless is-fullwidth is-marginless is-paddingless">
+      <MessageError @model={{@modelForData}} @errorMessage={{this.error}} />
+      <NamespaceReminder @mode="edit" @noun="secret" />
+      {{#if (and (not @model.failedServerRead) (not @model.selectedVersion.failedServerRead) (not-eq @model.selectedVersion.version @model.currentVersion))}}


This is... so much template logic 🤯 Maybe we could pull that logic into the component js file?

Good idea. This was a carry-over of previous templating, but it looks a lot better inside the js file. Amended.

hashishaw · 2021-08-26T21:12:51Z

ui/app/templates/components/secret-list/item.hbs

@@ -45,7 +45,7 @@
                      </button>
                    </li>
                  {{else}}
-                    {{#if @item.canRead}}
+                    {{#if (or @item.canReadSecretData @item.canRead)}}


Oh, so to confirm canReadSecretData is for kvv2, canRead is for kvv1? (and so on with edit and delete)

hashishaw · 2021-08-26T21:14:27Z

ui/tests/acceptance/secrets/backend/kv/secret-test.js

-    await click('[data-test-secret-edit="true"]');
+    await editPage.save();
+    await settled();
+    await editPage.metadataTab();
    await settled();
    // convert to number for IE11 browserstack test


Honestly we should drop that test, we're not supporting IE11 anymore. But that's out of scope for this PR 😄

Good call. I'll make a note in my test ticket.

hashishaw

🚀

* initial setup * form field editType kv is very helpful * setting up things * setup two routes for metadata * routing * clean up routing * meh router changes not my favorite but its working * show metadata * add controller for backendCrumb mixin * setting up edit metadata and trimming SecretEditMetadata component * add edit metadata save functionality * create new version work * setup model and formfieldgroups for added config data. * add config network request to secret-engine * fix validations on config * add config rows * breaking up secret edit * add validation for metadata on create * stuff, but broken now on metadata tab * fix metadata route error * permissions * saving small text changes * permissions * cleanup * some test fixes and convert secret create or update to glimmer * all these changes fix secret create kv test * remove alert banners per design request * fix error for array instead of object in jsonEditor * add changelog * styling * turn into glimmer component * cleanup * test failure fix * add delete or * clean up * remove all hardcoded for api integration * add helper and fix create mode on create new version * address chelseas pr comments * add jsdocs to helper * fix test

Monkeychip added 2 commits July 23, 2021 15:31

initial setup

7e39250

form field editType kv is very helpful

975f45a

vercel bot deployed to Preview – vault-storybook July 26, 2021 19:25 View deployment

vercel bot temporarily deployed to Preview – vault July 26, 2021 19:25 Inactive

setting up things

f311c18

vercel bot temporarily deployed to Preview – vault July 27, 2021 22:09 Inactive

vercel bot deployed to Preview – vault-storybook July 27, 2021 22:09 View deployment

Monkeychip changed the base branch from main to ui/kv-custom-metadata July 27, 2021 22:11

setup two routes for metadata

e74a728

vercel bot temporarily deployed to Preview – vault July 28, 2021 18:54 Inactive

vercel bot deployed to Preview – vault-storybook July 28, 2021 18:54 View deployment

routing

83322b1

vercel bot deployed to Preview – vault-storybook July 28, 2021 21:26 View deployment

vercel bot temporarily deployed to Preview – vault July 28, 2021 21:26 Inactive

clean up routing

fed0f56

vercel bot temporarily deployed to Preview – vault July 29, 2021 16:19 Inactive

vercel bot deployed to Preview – vault-storybook July 29, 2021 16:19 View deployment

meh router changes not my favorite but its working

5f26562

vercel bot deployed to Preview – vault-storybook July 29, 2021 17:01 View deployment

vercel bot temporarily deployed to Preview – vault July 29, 2021 17:01 Inactive

Monkeychip added 3 commits July 29, 2021 11:03

merge in main

5a0cb94

show metadata

640f8eb

add controller for backendCrumb mixin

d5e5ed5

vercel bot temporarily deployed to Preview – vault July 30, 2021 16:48 Inactive

vercel bot deployed to Preview – vault-storybook July 30, 2021 16:48 View deployment

setting up edit metadata and trimming SecretEditMetadata component

dee6798

vercel bot deployed to Preview – vault-storybook July 30, 2021 17:59 View deployment

vercel bot temporarily deployed to Preview – vault July 30, 2021 17:59 Inactive

add edit metadata save functionality

8ccea40

vercel bot deployed to Preview – vault-storybook August 2, 2021 17:00 View deployment

vercel bot deployed to Preview – vault-storybook August 24, 2021 20:18 View deployment

vercel bot temporarily deployed to Preview – vault August 24, 2021 20:18 Inactive

Monkeychip added the ui label Aug 24, 2021

Monkeychip added this to the 1.9 milestone Aug 24, 2021

Monkeychip marked this pull request as ready for review August 25, 2021 17:08

remove all hardcoded for api integration

cd534e3

vercel bot deployed to Preview – vault-storybook August 25, 2021 17:09 View deployment

vercel bot temporarily deployed to Preview – vault August 25, 2021 17:09 Inactive

Monkeychip changed the title ~~initial setup~~ UI add custom metadata to KV2 Aug 25, 2021

hashishaw reviewed Aug 25, 2021

View reviewed changes

add helper and fix create mode on create new version

408000d

vercel bot deployed to Preview – vault-storybook August 26, 2021 20:02 View deployment

vercel bot temporarily deployed to Preview – vault August 26, 2021 20:02 Inactive

hashishaw reviewed Aug 26, 2021

View reviewed changes

ui/app/adapters/secret-engine.js Show resolved Hide resolved

hashishaw requested changes Aug 26, 2021

View reviewed changes

address chelseas pr comments

643faed

vercel bot deployed to Preview – vault-storybook August 30, 2021 19:33 View deployment

vercel bot temporarily deployed to Preview – vault August 30, 2021 19:33 Inactive

add jsdocs to helper

abff87d

vercel bot temporarily deployed to Preview – vault August 30, 2021 19:41 Inactive

vercel bot deployed to Preview – vault-storybook August 30, 2021 19:41 View deployment

fix test

89bc486

vercel bot deployed to Preview – vault-storybook August 30, 2021 20:03 View deployment

vercel bot temporarily deployed to Preview – vault August 30, 2021 20:03 Inactive

hashishaw approved these changes Aug 30, 2021

View reviewed changes

Monkeychip merged commit 9e4a095 into main Aug 31, 2021

Monkeychip deleted the ui/kv-customer-custom-metadata branch August 31, 2021 15:41

Monkeychip mentioned this pull request May 1, 2023

Policy behavior are reflected differently via CLI and web UI #7571

Closed

cwchristerw mentioned this pull request Jul 12, 2023

Key-value custom_metadata is not visible in the UI #21773

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UI add custom metadata to KV2 #12169

UI add custom metadata to KV2 #12169

Monkeychip commented Jul 26, 2021 •

edited

hashishaw left a comment

hashishaw Aug 25, 2021

Monkeychip Aug 26, 2021

hashishaw Aug 26, 2021

hashishaw Aug 25, 2021

Monkeychip Aug 26, 2021

Monkeychip Aug 26, 2021

hashishaw left a comment

hashishaw Aug 26, 2021

Monkeychip Aug 30, 2021

hashishaw Aug 26, 2021

Monkeychip Aug 30, 2021

hashishaw Aug 26, 2021

Monkeychip Aug 30, 2021

hashishaw Aug 26, 2021

Monkeychip Aug 30, 2021

hashishaw Aug 26, 2021

Monkeychip Aug 30, 2021

hashishaw Aug 26, 2021

Monkeychip Aug 30, 2021

hashishaw Aug 26, 2021

hashishaw Aug 26, 2021

Monkeychip Aug 30, 2021

hashishaw left a comment

UI add custom metadata to KV2 #12169

UI add custom metadata to KV2 #12169

Conversation

Monkeychip commented Jul 26, 2021 • edited

hashishaw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hashishaw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hashishaw left a comment

Choose a reason for hiding this comment

Monkeychip commented Jul 26, 2021 •

edited