UI: Add KV view for wrap tool #29677

lane-wetmore · 2025-02-19T22:44:52Z

Description

This PR adds a KV view to the wrap tool based off the existing pattern when creating KV secrets. This resolves #28579. The default view is the still the JSON editor, but can be toggled between the two views while retaining input.

TODO only if you're a HashiCorp employee

Backport Labels: If this fix needs to be backported, use the appropriate backport/ label that matches the desired release branch. Note that in the CE repo, the latest release branch will look like backport/x.x.x, but older release branches will be backport/ent/x.x.x+ent.
- LTS: If this fixes a critical security vulnerability or severity 1 bug, it will also need to be backported to the current LTS versions of Vault. To ensure this, use all available enterprise labels.
ENT Breakage: If this PR either 1) removes a public function OR 2) changes the signature
of a public function, even if that change is in a CE file, double check that
applying the patch for this PR to the ENT repo and running tests doesn't
break any tests. Sometimes ENT only tests rely on public functions in CE
files.
Jira: If this change has an associated Jira, it's referenced either
in the PR description, commit message, or branch name.
RFC: If this change has an associated RFC, please link it in the description.
ENT PR: If this change has an associated ENT PR, please link it in the
description. Also, make sure the changelog is in this PR, not in your ENT PR.

github-actions · 2025-02-19T22:48:25Z

Build Results:
All builds succeeded! ✅

github-actions · 2025-02-19T23:06:55Z

CI Results:
All Go tests succeeded! ✅

hellobontempo

Great work! Excellent PR description, thanks for including screenshots. 📸

I think this is nearly there! There is a bug when there's a linting error and the user toggles to the key/value view. The wrap data button is disabled and the linting errors get a little wonky. We also want to be consistent with the type for this.wrapData. I offered a couple suggestions of either initializing it as null or an {}. Let me know if you want to discuss more at all or have any questions 😄

changelog/29677.txt

ui/app/components/tools/wrap.hbs

hellobontempo · 2025-02-20T23:05:34Z

ui/app/components/tools/wrap.js

  @tracked errorMessage = '';
+  @tracked showJson = true;
+  get jsonWrapStartValue() {


Getters are useful for computing return values. we're just returning a static string here so this can just be a variable.

On that note - If it's only used in the getter below, we probably don't need a variable and can just reference this string below.

Another option is to initialize this.wrapData = {}. This would remove the need for the ternary (I'm not worried about preserving the line break) and we could just consistently returned the stringified object. Since stringify is a helper, it can actually be called directly in the template if you want to go that route instead.

vault/ui/app/components/tools/unwrap.hbs

Line 22 in 4617af3

@value={{stringify this.unwrapData}}

hellobontempo · 2025-02-20T23:08:22Z

ui/tests/integration/components/tools/wrap-test.js

+
+    await this.renderComponent();
+    await codemirror().setValue(this.wrapData);
+    await click('[data-test-toggle-input="json"]');


After toggling, before submitting can we input one more row of data? I want to make sure the tracked property is updated properly. It may be useful to input a multi line string here as well! That way we're also making sure to test for the original request.

ui/app/components/tools/wrap.hbs

hellobontempo · 2025-02-21T00:40:24Z

ui/app/components/tools/wrap.hbs

+      {{#if this.showJson}}
+        <div class="field">
+          <div class="control">
+            <JsonEditor


Looking at the screenshot, it looks like we could use a bit of a margin between this and the toolbar. The JsonEditor has "splattributes" which means if you apply class= here that will apply to where ever ...attributes is spread in the component. (ember docs if you want to read up more)

Adding class="has-top-margin-s" should make this visually match the spacing we see in the KV secrets view (or -m like below if that looks better to you!)

Good point! I think going with -s for the json editor and -l for the KV object editor looks good and prevents the Data to wrap label from jumping around too much.

ui/app/components/tools/wrap.js

hellobontempo · 2025-02-21T01:11:10Z

ui/app/components/tools/wrap.js


  @action
  reset(clearData = true) {
    this.token = '';
    this.errorMessage = '';
    this.wrapTTL = null;
-    if (clearData) this.wrapData = '{\n}';
+    if (clearData) this.wrapData = '';


We should keep the type consistent for this.wrapData. Since we use it for a falsey check above - kinda thinking we should initialize this as null and reset it to that for consistency. That way if it does have a value, we know it will consistently be an object. It's a little confusing now if it should be a JSON object or a string

hellobontempo · 2025-02-21T01:21:14Z

ui/app/components/tools/wrap.js

@@ -46,13 +63,13 @@ export default class ToolsWrap extends Component {
    codemirror.performLint();
    const hasErrors = codemirror?.state.lint.marked?.length > 0;
    this.buttonDisabled = hasErrors;
-    if (!hasErrors) this.wrapData = val;
+    if (!hasErrors) this.wrapData = JSON.parse(val);


It looks like we get into a weird state if there are linting errors and the user toggles the JSON button (go to the key/value view and then back to the json editor). the JSON "resets", though it still shows there's a linting issue and the wrap button is still disabled. One way to address this is by resetting the linting errors when a user toggles.

add kv view for wrap tool

d9d2cb5

lane-wetmore added the ui label Feb 19, 2025

lane-wetmore self-assigned this Feb 19, 2025

lane-wetmore requested a review from a team as a code owner February 19, 2025 22:44

github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Feb 19, 2025

lane-wetmore added this to the 1.20.0-rc milestone Feb 20, 2025

lane-wetmore added 2 commits February 20, 2025 11:28

Merge branch 'main' into ui/VAULT-33463-add-kv-view-in-wrap

8d4c220

add changelog entry

2148968

lane-wetmore requested a review from a team as a code owner February 20, 2025 17:30

lane-wetmore requested a review from mladlow February 20, 2025 17:30

update toggle and tests

7a6320a

hellobontempo reviewed Feb 20, 2025

View reviewed changes

hellobontempo reviewed Feb 21, 2025

View reviewed changes

ui/app/components/tools/wrap.hbs Outdated Show resolved Hide resolved

hellobontempo reviewed Feb 21, 2025

View reviewed changes

ui/app/components/tools/wrap.js Show resolved Hide resolved

hellobontempo reviewed Feb 21, 2025

View reviewed changes

ui/app/components/tools/wrap.js Show resolved Hide resolved

hellobontempo reviewed Feb 21, 2025

View reviewed changes

lane-wetmore added 3 commits February 21, 2025 16:30

update changelog, style updates, fix linting error bug

54a97b9

update tests

cb93b73

update test to include multiline input

cca8f0d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UI: Add KV view for wrap tool #29677

UI: Add KV view for wrap tool #29677

lane-wetmore commented Feb 19, 2025 •

edited

Loading

github-actions bot commented Feb 19, 2025 •

edited

Loading

github-actions bot commented Feb 19, 2025 •

edited

Loading

hellobontempo left a comment •

edited

Loading

hellobontempo Feb 20, 2025 •

edited

Loading

hellobontempo Feb 20, 2025

hellobontempo Feb 21, 2025 •

edited

Loading

lane-wetmore Feb 21, 2025

hellobontempo Feb 21, 2025 •

edited

Loading

hellobontempo Feb 21, 2025 •

edited

Loading

UI: Add KV view for wrap tool #29677

Are you sure you want to change the base?

UI: Add KV view for wrap tool #29677

Conversation

lane-wetmore commented Feb 19, 2025 • edited Loading

Description

TODO only if you're a HashiCorp employee

github-actions bot commented Feb 19, 2025 • edited Loading

github-actions bot commented Feb 19, 2025 • edited Loading

hellobontempo left a comment • edited Loading

Choose a reason for hiding this comment

hellobontempo Feb 20, 2025 • edited Loading

Choose a reason for hiding this comment

hellobontempo Feb 20, 2025

Choose a reason for hiding this comment

hellobontempo Feb 21, 2025 • edited Loading

Choose a reason for hiding this comment

lane-wetmore Feb 21, 2025

Choose a reason for hiding this comment

hellobontempo Feb 21, 2025 • edited Loading

Choose a reason for hiding this comment

hellobontempo Feb 21, 2025 • edited Loading

Choose a reason for hiding this comment

lane-wetmore commented Feb 19, 2025 •

edited

Loading

github-actions bot commented Feb 19, 2025 •

edited

Loading

github-actions bot commented Feb 19, 2025 •

edited

Loading

hellobontempo left a comment •

edited

Loading

hellobontempo Feb 20, 2025 •

edited

Loading

hellobontempo Feb 21, 2025 •

edited

Loading

hellobontempo Feb 21, 2025 •

edited

Loading

hellobontempo Feb 21, 2025 •

edited

Loading