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

Bug Fix and Glimmerize secret-edit component #14941

Merged
merged 11 commits into from
Apr 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelog/14941.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
ui: Fix issue with KV not recomputing model when you changed versions.
```
2 changes: 0 additions & 2 deletions ui/app/components/secret-edit-toolbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
* @secretDataIsAdvanced={{secretDataIsAdvanced}}
* @showAdvancedMode={{showAdvancedMode}}
* @modelForData={{this.modelForData}}
* @navToNearestAncestor={{this.navToNearestAncestor}}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A mixin method we were not using. We passed it into the secret-delete-menu but didn't actually use it.

* @canUpdateSecretData={{canUpdateSecretData}}
* @codemirrorString={{codemirrorString}}
* @wrappedData={{wrappedData}}
Expand All @@ -30,7 +29,6 @@
* @param {boolean} secretDataIsAdvanced - used to determine if show JSON toggle
* @param {boolean} showAdvacnedMode - used for JSON toggle
* @param {object} modelForData - a modified version of the model with secret data
* @param {string} navToNearestAncestor - route to nav to if press cancel
* @param {boolean} canUpdateSecretData - permissions that show the create new version button or not.
* @param {string} codemirrorString - used to copy the JSON
* @param {object} wrappedData - when copy the data it's the token of the secret returned.
Expand Down
191 changes: 85 additions & 106 deletions ui/app/components/secret-edit.js
Original file line number Diff line number Diff line change
@@ -1,84 +1,63 @@
/* eslint ember/no-computed-properties-in-native-classes: 'warn' */
/**
* @module SecretEdit
* SecretEdit component manages the secret and model data, and displays either the create, update, empty state or show view of a KV secret.
*
* @example
* ```js
* <SecretEdit @model={{model}}/>
* <SecretEdit @model={{model}} @mode="create" @baseKey={{this.baseKey}} @key={{this.model}} @initialKey={{this.initialKey}} @onRefresh={{action "refresh"}} @onToggleAdvancedEdit={{action "toggleAdvancedEdit"}} @preferAdvancedEdit={{this.preferAdvancedEdit}}/>
* ```
/
* @param {object} model - Model returned from route secret-v2
/This component is initialized from the secret-edit-layout.hbs file
* @param {object} model - Model returned from secret-v2 which is generated in the secret-edit route
* @param {string} mode - Edit, create, etc.
* @param {string} baseKey - Provided for navigation.
* @param {object} key - Passed through, copy of the model.
* @param {string} initialKey - model's name.
* @param {function} onRefresh - action that refreshes the model
* @param {function} onToggleAdvancedEdit - changes the preferAdvancedEdit to true or false
* @param {boolean} preferAdvancedEdit - property set from the controller of show/edit/create route passed in through secret-edit-layout
*/

import { inject as service } from '@ember/service';
import Component from '@ember/component';
import { computed } from '@ember/object';
import { alias, or } from '@ember/object/computed';
import FocusOnInsertMixin from 'vault/mixins/focus-on-insert';
import WithNavToNearestAncestor from 'vault/mixins/with-nav-to-nearest-ancestor';
import Component from '@glimmer/component';
import { action } from '@ember/object';
import { tracked } from '@glimmer/tracking';
import KVObject from 'vault/lib/kv-object';
import { maybeQueryRecord } from 'vault/macros/maybe-query-record';
import { alias, or } from '@ember/object/computed';

export default Component.extend(FocusOnInsertMixin, WithNavToNearestAncestor, {
wizard: service(),
store: service(),

// a key model
key: null,
model: null,

// a value to pre-fill the key input - this is populated by the corresponding
// 'initialKey' queryParam
initialKey: null,

// set in the route's setupController hook
mode: null,

secretData: null,

// called with a bool indicating if there's been a change in the secretData and customMetadata
onDataChange() {},
onRefresh() {},
onToggleAdvancedEdit() {},

// did user request advanced mode
preferAdvancedEdit: false,

// use a named action here so we don't have to pass one in
// this will bubble to the route
toggleAdvancedEdit: 'toggleAdvancedEdit',

codemirrorString: null,
export default class SecretEdit extends Component {
@service wizard;
@service store;

isV2: false,
@tracked secretData = null;
@tracked isV2 = false;
@tracked codemirrorString = null;

init() {
this._super(...arguments);
let secrets = this.model.secretData;
if (!secrets && this.model.selectedVersion) {
this.set('isV2', true);
secrets = this.model.belongsTo('selectedVersion').value().secretData;
constructor() {
super(...arguments);
let secrets = this.args.model.secretData;
if (!secrets && this.args.model.selectedVersion) {
this.isV2 = true;
secrets = this.args.model.belongsTo('selectedVersion').value().secretData;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a bit of an interesting way of accessing this property. You should be able to do this.args.model.selectedVersion.secretData.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a copy over from how it was done before. I'm hesitant to change it in case I'm missing a reasoning for accessing the store again. maybe a refresh after choosing the version or something like that.

}
const data = KVObject.create({ content: [] }).fromJSON(secrets);
this.set('secretData', data);
this.set('codemirrorString', data.toJSONString());
if (data.isAdvanced()) {
this.set('preferAdvancedEdit', true);
}
Comment on lines -65 to -67
Copy link
Contributor

Choose a reason for hiding this comment

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

If preferAdvancedEdit is an arg is this check happening higher up now? I'm just wondering since the component was potentially overriding the original value if we will run into state issues.

if (this.wizard.featureState === 'details' && this.mode === 'create') {
let engine = this.model.backend.includes('kv') ? 'kv' : this.model.backend;
this.secretData = data;
this.codemirrorString = data.toJSONString();
if (this.wizard.featureState === 'details' && this.args.mode === 'create') {
let engine = this.args.model.backend.includes('kv') ? 'kv' : this.args.model.backend;
this.wizard.transitionFeatureMachine('details', 'CONTINUE', engine);
}
},
}

checkSecretCapabilities: maybeQueryRecord(
@maybeQueryRecord(
'capabilities',
(context) => {
if (!context.model || context.mode === 'create') {
if (!context.args.model || context.args.mode === 'create') {
return;
}
let backend = context.isV2 ? context.get('model.engine.id') : context.model.backend;
let id = context.model.id;
let backend = context.isV2 ? context.args.model.engine.id : context.args.model.backend;
let id = context.args.model.id;
let path = context.isV2 ? `${backend}/data/${id}` : `${backend}/${id}`;
return {
id: path,
Expand All @@ -88,17 +67,18 @@ export default Component.extend(FocusOnInsertMixin, WithNavToNearestAncestor, {
'model',
'model.id',
'mode'
),
canUpdateSecretData: alias('checkSecretCapabilities.canUpdate'),
canReadSecretData: alias('checkSecretCapabilities.canRead'),
)
checkSecretCapabilities;
@alias('checkSecretCapabilities.canUpdate') canUpdateSecretData;
@alias('checkSecretCapabilities.canRead') canReadSecretData;

checkMetadataCapabilities: maybeQueryRecord(
@maybeQueryRecord(
'capabilities',
(context) => {
if (!context.model || !context.isV2) {
if (!context.args.model || !context.isV2) {
return;
}
let backend = context.model.backend;
let backend = context.args.model.backend;
let path = `${backend}/metadata/`;
return {
id: path,
Expand All @@ -108,60 +88,59 @@ export default Component.extend(FocusOnInsertMixin, WithNavToNearestAncestor, {
'model',
'model.id',
'mode'
),
canDeleteSecretMetadata: alias('checkMetadataCapabilities.canDelete'),
canUpdateSecretMetadata: alias('checkMetadataCapabilities.canUpdate'),
canReadSecretMetadata: alias('checkMetadataCapabilities.canRead'),
)
checkMetadataCapabilities;
@alias('checkMetadataCapabilities.canDelete') canDeleteSecretMetadata;
@alias('checkMetadataCapabilities.canUpdate') canUpdateSecretMetadata;
@alias('checkMetadataCapabilities.canRead') canReadSecretMetadata;

requestInFlight: or('model.isLoading', 'model.isReloading', 'model.isSaving'),
@or('model.isLoading', 'model.isReloading', 'model.isSaving') requestInFlight;
@or('requestInFlight', 'model.isFolder', 'model.flagsIsInvalid') buttonDisabled;

buttonDisabled: or('requestInFlight', 'model.isFolder', 'model.flagsIsInvalid'),

modelForData: computed('isV2', 'model', function () {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

here will be the backport fix. I'll make this computed('isV2', 'model', 'selectedVersion', function ...

let { model } = this;
get modelForData() {
let { model } = this.args;
if (!model) return null;
return this.isV2 ? model.belongsTo('selectedVersion').value() : model;
}),
}

basicModeDisabled: computed('secretDataIsAdvanced', 'showAdvancedMode', function () {
get basicModeDisabled() {
return this.secretDataIsAdvanced || this.showAdvancedMode === false;
}),
}

secretDataAsJSON: computed('secretData', 'secretData.[]', function () {
get secretDataAsJSON() {
return this.secretData.toJSON();
}),
}

secretDataIsAdvanced: computed('secretData', 'secretData.[]', function () {
get secretDataIsAdvanced() {
return this.secretData.isAdvanced();
}),
}

showAdvancedMode: or('secretDataIsAdvanced', 'preferAdvancedEdit'),
get showAdvancedMode() {
return this.secretDataIsAdvanced || this.args.preferAdvancedEdit;
}
Comment on lines +118 to +120
Copy link
Contributor

Choose a reason for hiding this comment

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

Following up from our zoom conversation that this new getter will cover the scenario where preferAdvancedEdit was previously being set in the constructor based on the isAdvanced value.


isWriteWithoutRead: computed(
'model.failedServerRead',
'modelForData.failedServerRead',
'isV2',
function () {
if (!this.model) return;
// if the version couldn't be read from the server
if (this.isV2 && this.modelForData.failedServerRead) {
return true;
}
// if the model couldn't be read from the server
if (!this.isV2 && this.model.failedServerRead) {
return true;
}
get isWriteWithoutRead() {
if (!this.args.model) {
return false;
}
),

actions: {
refresh() {
this.onRefresh();
},

toggleAdvanced(bool) {
this.onToggleAdvancedEdit(bool);
},
},
});
// if the version couldn't be read from the server
if (this.isV2 && this.modelForData.failedServerRead) {
return true;
}
// if the model couldn't be read from the server
if (!this.isV2 && this.args.model.failedServerRead) {
return true;
}
return false;
}

@action
refresh() {
this.args.onRefresh();
}

@action
toggleAdvanced(bool) {
this.args.onToggleAdvancedEdit(bool);
}
}
1 change: 0 additions & 1 deletion ui/app/templates/components/secret-edit-toolbar.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
<SecretDeleteMenu
@modelForData={{@modelForData}}
@model={{@model}}
@navToNearestAncestor={{@navToNearestAncestor}}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again another mixin method we were passing through but never using.

@isV2={{@isV2}}
@refresh={{action @editActions.refresh}}
@canReadSecretMetadata={{@canReadSecretMetadata}}
Expand Down
33 changes: 16 additions & 17 deletions ui/app/templates/components/secret-edit.hbs
Original file line number Diff line number Diff line change
@@ -1,38 +1,38 @@
<PageHeader as |p|>
<p.top>
<KeyValueHeader
@baseKey={{this.baseKey}}
@baseKey={{@baseKey}}
@path="vault.cluster.secrets.backend.list"
@mode={{this.mode}}
@root={{this.root}}
@mode={{@mode}}
@root={{@root}}
@showCurrent={{true}}
/>
</p.top>
<p.levelLeft>
<h1 class="title is-3">
{{#if (eq this.mode "create")}}
{{#if (eq @mode "create")}}
Create secret
{{else if (and this.isV2 (eq this.mode "edit"))}}
{{else if (and this.isV2 (eq @mode "edit"))}}
Create new version
{{else if (eq this.mode "edit")}}
{{else if (eq @mode "edit")}}
Edit secret
{{else}}
{{this.key.id}}
{{@key.id}}
{{/if}}
</h1>
</p.levelLeft>
</PageHeader>
{{! tabs for show only }}
{{#if (eq this.mode "show")}}
{{#if (eq @mode "show")}}
<div class="tabs-container box is-bottomless is-marginless is-fullwidth is-paddingless">
<nav class="tabs">
<ul>
<LinkTo @route="vault.cluster.secrets.backend.show" @model={{this.key.id}} data-test-secret-tab>
<LinkTo @route="vault.cluster.secrets.backend.show" @model={{@key.id}} data-test-secret-tab>
Secret
</LinkTo>
{{! must have read access to /metadata see tab or update to update metadata}}
{{#if (or this.canReadSecretMetadata this.canUpdateSecretMetadata)}}
<LinkTo @route="vault.cluster.secrets.backend.metadata" @model={{this.key.id}} data-test-secret-metadata-tab>
<LinkTo @route="vault.cluster.secrets.backend.metadata" @model={{@key.id}} data-test-secret-metadata-tab>
Metadata
</LinkTo>
{{/if}}
Expand All @@ -42,24 +42,23 @@
{{/if}}

<SecretEditToolbar
@mode={{this.mode}}
@model={{this.model}}
@mode={{@mode}}
@model={{@model}}
@isV2={{this.isV2}}
@isWriteWithoutRead={{this.isWriteWithoutRead}}
@secretDataIsAdvanced={{this.secretDataIsAdvanced}}
@showAdvancedMode={{this.showAdvancedMode}}
@modelForData={{this.modelForData}}
@navToNearestAncestor={{this.navToNearestAncestor}}
@canUpdateSecretData={{this.canUpdateSecretData}}
@canReadSecretMetadata={{this.canReadSecretMetadata}}
@codemirrorString={{this.codemirrorString}}
@editActions={{hash toggleAdvanced=(action "toggleAdvanced") refresh=(action "refresh")}}
/>

{{#if (or (eq this.mode "create") (eq this.mode "edit"))}}
{{#if (or (eq @mode "create") (eq @mode "edit"))}}
<SecretCreateOrUpdate
@mode={{this.mode}}
@model={{this.model}}
@mode={{@mode}}
@model={{@model}}
@showAdvancedMode={{this.showAdvancedMode}}
@modelForData={{this.modelForData}}
@error={{this.error}}
Expand All @@ -70,7 +69,7 @@
@canReadSecretData={{this.canReadSecretData}}
@canReadSecretMetadata={{this.canReadSecretMetadata}}
/>
{{else if (eq this.mode "show")}}
{{else if (eq @mode "show")}}
<SecretFormShow
@isV2={{this.isV2}}
@modelForData={{this.modelForData}}
Expand Down