Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KV automatic delete state issue in UI #13166

Merged
merged 4 commits into from
Nov 23, 2021
Merged

KV automatic delete state issue in UI #13166

merged 4 commits into from
Nov 23, 2021

Conversation

zofskeez
Copy link
Contributor

When an automatic deletion time for a secret passes during a UI session the state was not properly updating to reflect the change. Instead of displaying the placeholder to indicate that the version has been deleted the secret is displayed with missing data.
image

The secret-edit route is fetching the version data but since the deletionTime does not change the deleted computed property on the secret-v2-version model was returning the cached value. After converting the model to a native class and using a getter the deleted value is calculated each time it is accessed which fixes the state issue.

image

To test:

  • in a terminal run vault kv put secret/testversions key="value" && vault kv metadata put -delete-version-after="30s" secret/testversions
  • before 30 seconds view the secret in the UI
  • after 30 seconds go back to the secrets list (do not refresh) and then back to the secret again
  • the version deleted placeholder copy should display.

I started working on an acceptance test but since we are dealing with time elapsing I became concerned that it might end up flaky. If we think it's important to cover this case I'm happy to work on it more.

@vercel vercel bot temporarily deployed to Preview – vault November 16, 2021 18:56 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook November 16, 2021 18:56 Inactive
…version action when users cannot read metadata
Copy link
Contributor

@Monkeychip Monkeychip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done 👏 Thanks for taking all the extra steps here too! Oh, don't forget the labels too. UI and not sure if this is getting backported or not. Sounds like something that should be backported to 1.9 which is where the KV work was done. If you backport farther than that you might hit a lot of changes that make this merge difficult.

@zofskeez zofskeez added ui bug Used to indicate a potential bug backport labels Nov 23, 2021
@zofskeez zofskeez added this to the 1.7.7 milestone Nov 23, 2021
@zofskeez zofskeez merged commit 91407e1 into main Nov 23, 2021
@zofskeez zofskeez deleted the ui/kv-delete-bug branch November 23, 2021 21:17
zofskeez added a commit that referenced this pull request Nov 23, 2021
* converts secret-v2-version model to native class -- fixes issues with cached values for deleted prop

* adds changelog entry

* adds disabled state to ToolbarLink component and disables create new version action when users cannot read metadata

* updates secret-edit acceptance test
@zofskeez zofskeez modified the milestones: 1.7.7, 1.9.1 Nov 23, 2021
zofskeez added a commit that referenced this pull request Nov 23, 2021
* converts secret-v2-version model to native class -- fixes issues with cached values for deleted prop

* adds changelog entry

* adds disabled state to ToolbarLink component and disables create new version action when users cannot read metadata

* updates secret-edit acceptance test
qk4l pushed a commit to qk4l/vault that referenced this pull request Feb 4, 2022
* converts secret-v2-version model to native class -- fixes issues with cached values for deleted prop

* adds changelog entry

* adds disabled state to ToolbarLink component and disables create new version action when users cannot read metadata

* updates secret-edit acceptance test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport bug Used to indicate a potential bug ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants