From 24332c27ef1eaa2ffa30d78208adf308069d870a Mon Sep 17 00:00:00 2001 From: claire bontempo Date: Wed, 25 Sep 2024 17:58:50 -0700 Subject: [PATCH] remove explicit defaults and types so we retrieve from backend, decouple enabling auto tidy from duration, move params to auto settings section --- ui/app/models/pki/tidy.js | 44 +++---- ui/lib/pki/addon/components/pki-tidy-form.hbs | 41 +++--- ui/lib/pki/addon/components/pki-tidy-form.ts | 6 - ui/tests/acceptance/pki/pki-tidy-test.js | 4 +- ui/tests/helpers/pki/pki-selectors.ts | 1 - .../pki/page/pki-tidy-auto-settings-test.js | 5 +- .../components/pki/pki-tidy-form-test.js | 117 +++++++++++------- 7 files changed, 121 insertions(+), 97 deletions(-) diff --git a/ui/app/models/pki/tidy.js b/ui/app/models/pki/tidy.js index f339448a4108..7b40ad1c7f58 100644 --- a/ui/app/models/pki/tidy.js +++ b/ui/app/models/pki/tidy.js @@ -16,7 +16,7 @@ export default class PkiTidyModel extends Model { label: 'Tidy ACME enabled', labelDisabled: 'Tidy ACME disabled', mapToBoolean: 'tidyAcme', - helperTextDisabled: 'Tidying of ACME accounts, orders and authorizations is disabled', + helperTextDisabled: 'Tidying of ACME accounts, orders and authorizations is disabled.', helperTextEnabled: 'The amount of time that must pass after creation that an account with no orders is marked revoked, and the amount of time after being marked revoked or deactivated.', detailsLabel: 'ACME account safety buffer', @@ -31,47 +31,45 @@ export default class PkiTidyModel extends Model { }) tidyAcme; + // * auto tidy only fields @attr('boolean', { label: 'Automatic tidy enabled', - defaultValue: false, + labelDisabled: 'Automatic tidy disabled', + helperTextDisabled: 'Automatic tidy operations will not run.', }) - enabled; // auto-tidy only + enabled; // renders outside FormField loop as a toggle, auto tidy fields only render if enabled @attr({ - label: 'Automatic tidy enabled', - labelDisabled: 'Automatic tidy disabled', - mapToBoolean: 'enabled', + editType: 'ttl', helperTextEnabled: 'Sets the interval_duration between automatic tidy operations; note that this is from the end of one operation to the start of the next.', - helperTextDisabled: 'Automatic tidy operations will not run.', - detailsLabel: 'Automatic tidy duration', + hideToggle: true, formatTtl: true, }) - intervalDuration; // auto-tidy only + intervalDuration; - @attr('string', { + @attr({ label: 'Minimum startup backoff duration', - defaultValue: '5m', editType: 'ttl', helperTextEnabled: - 'Sets the min_startup_backoff_duration field which forces the minimum delay after Vault startup auto-tidy can run', + 'Sets the min_startup_backoff_duration field which forces the minimum delay after Vault startup auto-tidy can run.', hideToggle: true, formatTtl: true, }) - minStartupBackoffDuration; // auto-tidy only + minStartupBackoffDuration; - @attr('string', { + @attr({ label: 'Maximum startup backoff duration', - defaultValue: '15m', editType: 'ttl', helperTextEnabled: - 'Sets the max_startup_backoff_duration field which forces the maximum delay after Vault startup auto-tidy can run', + 'Sets the max_startup_backoff_duration field which forces the maximum delay after Vault startup auto-tidy can run.', hideToggle: true, formatTtl: true, }) - maxStartupBackoffDuration; // auto-tidy only + maxStartupBackoffDuration; + // * end of auto-tidy only fields - @attr('string', { + @attr({ editType: 'ttl', helperTextEnabled: 'Specifies a duration that issuers should be kept for, past their NotAfter validity period. Defaults to 365 days (8760 hours).', @@ -98,7 +96,7 @@ export default class PkiTidyModel extends Model { }) revocationQueueSafetyBuffer; // enterprise only - @attr('string', { + @attr({ editType: 'ttl', helperTextEnabled: 'For a certificate to be expunged, the time must be after the expiration time of the certificate (according to the local clock) plus the safety buffer. Defaults to 72 hours.', @@ -153,16 +151,14 @@ export default class PkiTidyModel extends Model { tidyRevokedCerts; get allGroups() { - const groups = [ - { autoTidy: ['enabled', 'intervalDuration', ...this.autoTidyConfigFields] }, - ...this.sharedFields, - ]; + const groups = [{ autoTidy: ['enabled', ...this.autoTidyConfigFields] }, ...this.sharedFields]; return this._expandGroups(groups); } // fields that are specific to auto-tidy get autoTidyConfigFields() { - return ['minStartupBackoffDuration', 'maxStartupBackoffDuration']; + // 'enabled' is not included here because it is responsible for hiding/showing these params and renders separately in the form + return ['intervalDuration', 'minStartupBackoffDuration', 'maxStartupBackoffDuration']; } // shared between auto and manual tidy operations diff --git a/ui/lib/pki/addon/components/pki-tidy-form.hbs b/ui/lib/pki/addon/components/pki-tidy-form.hbs index 5edc547eeb1e..31d6b1f7cf15 100644 --- a/ui/lib/pki/addon/components/pki-tidy-form.hbs +++ b/ui/lib/pki/addon/components/pki-tidy-form.hbs @@ -14,28 +14,33 @@
{{#if (eq @tidyType "auto")}} - {{#let this.intervalDurationAttr as |attr|}} - + {{#let (get @tidy.allByKey "enabled") as |enabledAttr|}} +
+ + + + {{if @tidy.enabled enabledAttr.options.label enabledAttr.options.labelDisabled}} + + {{#unless @tidy.enabled}} +

{{enabledAttr.options.helperTextDisabled}}

+ {{/unless}} + + +
{{/let}} - {{#if @tidy.enabled}} +

+ Automatic tidy settings +

{{#each @tidy.autoTidyConfigFields as |field|}} {{/each}} {{/if}} {{/if}} - {{#each @tidy.formFieldGroups as |fieldGroup|}} - {{#each-in fieldGroup as |group fields|}} - {{#if (or (eq @tidyType "manual") @tidy.enabled)}} + + {{#if (or (eq @tidyType "manual") @tidy.enabled)}} + {{#each @tidy.formFieldGroups as |fieldGroup|}} + {{#each-in fieldGroup as |group fields|}}

{{group}}

@@ -58,9 +63,9 @@ {{/if}} {{/if}} {{/each}} - {{/if}} - {{/each-in}} - {{/each}} + {{/each-in}} + {{/each}} + {{/if}}
diff --git a/ui/lib/pki/addon/components/pki-tidy-form.ts b/ui/lib/pki/addon/components/pki-tidy-form.ts index a5a290ad821e..1b388b77eade 100644 --- a/ui/lib/pki/addon/components/pki-tidy-form.ts +++ b/ui/lib/pki/addon/components/pki-tidy-form.ts @@ -23,11 +23,9 @@ interface Args { } interface PkiTidyTtls { - intervalDuration: string; acmeAccountSafetyBuffer: string; } interface PkiTidyBooleans { - enabled: boolean; tidyAcme: boolean; } @@ -37,10 +35,6 @@ export default class PkiTidyForm extends Component { @tracked errorBanner = ''; @tracked invalidFormAlert = ''; - get intervalDurationAttr() { - return this.args.tidy?.allByKey.intervalDuration; - } - @task @waitFor *save(event: Event) { diff --git a/ui/tests/acceptance/pki/pki-tidy-test.js b/ui/tests/acceptance/pki/pki-tidy-test.js index 76b75aed7bf9..0c6d93605d72 100644 --- a/ui/tests/acceptance/pki/pki-tidy-test.js +++ b/ui/tests/acceptance/pki/pki-tidy-test.js @@ -129,7 +129,7 @@ module('Acceptance | pki tidy', function (hooks) { await click(PKI_TIDY.tidyEmptyStateConfigure); await click(PKI_TIDY.tidyConfigureModal.tidyModalAutoButton); assert.dom(PKI_TIDY_FORM.tidyFormName('auto')).exists('Auto tidy form exists'); - await click(PKI_TIDY_FORM.toggleLabel('Automatic tidy disabled')); + await click(GENERAL.ttl.toggle('enabled')); assert .dom(PKI_TIDY_FORM.tidySectionHeader('ACME operations')) .exists('Auto tidy form enabled shows ACME operations field'); @@ -192,7 +192,7 @@ module('Acceptance | pki tidy', function (hooks) { await click(PKI_TIDY.tidyConfigureModal.tidyOptionsModal); assert.dom(PKI_TIDY.tidyConfigureModal.configureTidyModal).exists('Configure tidy modal exists'); await click(PKI_TIDY.tidyConfigureModal.tidyModalAutoButton); - await click(PKI_TIDY_FORM.toggleLabel('Automatic tidy disabled')); + await click(GENERAL.ttl.toggle('enabled')); await click(PKI_TIDY_FORM.inputByAttr('tidyCertStore')); await click(PKI_TIDY_FORM.inputByAttr('tidyRevokedCerts')); await click(PKI_TIDY_FORM.tidySave); diff --git a/ui/tests/helpers/pki/pki-selectors.ts b/ui/tests/helpers/pki/pki-selectors.ts index 2bd7bd320e99..4b9839d21f40 100644 --- a/ui/tests/helpers/pki/pki-selectors.ts +++ b/ui/tests/helpers/pki/pki-selectors.ts @@ -195,7 +195,6 @@ export const PKI_TIDY_FORM = { tidyFormName: (attr: string) => `[data-test-tidy-form="${attr}"]`, inputByAttr: (attr: string) => `[data-test-input="${attr}"]`, toggleInput: (attr: string) => `[data-test-input="${attr}"] input`, - intervalDuration: '[data-test-ttl-value="Automatic tidy enabled"]', acmeAccountSafetyBuffer: '[data-test-ttl-value="Tidy ACME enabled"]', toggleLabel: (label: string) => `[data-test-toggle-label="${label}"]`, tidySectionHeader: (header: string) => `[data-test-tidy-header="${header}"]`, diff --git a/ui/tests/integration/components/pki/page/pki-tidy-auto-settings-test.js b/ui/tests/integration/components/pki/page/pki-tidy-auto-settings-test.js index 3968dd9ad24b..658a91573889 100644 --- a/ui/tests/integration/components/pki/page/pki-tidy-auto-settings-test.js +++ b/ui/tests/integration/components/pki/page/pki-tidy-auto-settings-test.js @@ -51,12 +51,14 @@ module('Integration | Component | page/pki-tidy-auto-settings', function (hooks) .hasText('Edit auto-tidy', 'toolbar edit link has correct text'); assert.dom('[data-test-row="enabled"] [data-test-label-div]').hasText('Automatic tidy enabled'); - assert.dom('[data-test-row="intervalDuration"] [data-test-label-div]').hasText('Automatic tidy duration'); + assert.dom('[data-test-value-div="Automatic tidy enabled"]').hasText('No'); + assert.dom('[data-test-value-div="Interval duration"]').hasText('2 days'); // Universal operations assert.dom('[data-test-group-title="Universal operations"]').hasText('Universal operations'); assert .dom('[data-test-value-div="Tidy the certificate store"]') .exists('Renders universal field when value exists'); + assert.dom('[data-test-value-div="Tidy the certificate store"]').hasText('No'); assert .dom('[data-test-value-div="Tidy revoked certificates"]') .doesNotExist('Does not render universal field when value null'); @@ -65,6 +67,7 @@ module('Integration | Component | page/pki-tidy-auto-settings', function (hooks) assert .dom('[data-test-value-div="Tidy expired issuers"]') .exists('Renders issuer op field when value exists'); + assert.dom('[data-test-value-div="Tidy expired issuers"]').hasText('Yes'); assert .dom('[data-test-value-div="Tidy legacy CA bundle"]') .doesNotExist('Does not render issuer op field when value null'); diff --git a/ui/tests/integration/components/pki/pki-tidy-form-test.js b/ui/tests/integration/components/pki/pki-tidy-form-test.js index 1861781a4e51..9f6c824eaaa1 100644 --- a/ui/tests/integration/components/pki/pki-tidy-form-test.js +++ b/ui/tests/integration/components/pki/pki-tidy-form-test.js @@ -10,6 +10,8 @@ import { hbs } from 'ember-cli-htmlbars'; import { setupEngine } from 'ember-engines/test-support'; import { setupMirage } from 'ember-cli-mirage/test-support'; import { PKI_TIDY_FORM } from 'vault/tests/helpers/pki/pki-selectors'; +import { GENERAL } from 'vault/tests/helpers/general-selectors'; +import { convertToSeconds } from 'core/utils/duration-utils'; module('Integration | Component | pki tidy form', function (hooks) { setupRenderingTest(hooks); @@ -24,21 +26,36 @@ module('Integration | Component | pki tidy form', function (hooks) { this.onSave = () => {}; this.onCancel = () => {}; this.manualTidy = this.store.createRecord('pki/tidy', { backend: 'pki-manual-tidy' }); + this.autoTidyServerDefaults = { + enabled: false, + interval_duration: '12h', + safety_buffer: '3d', + issuer_safety_buffer: '365d', + min_startup_backoff_duration: '5m', + max_startup_backoff_duration: '15m', + }; this.store.pushPayload('pki/tidy', { modelName: 'pki/tidy', id: 'pki-auto-tidy', + // setting defaults here to simulate how this form works in the app. + // on init, we retrieve these from the server and pre-populate form (instead of explicitly set on the model) + ...this.autoTidyServerDefaults, }); this.autoTidy = this.store.peekRecord('pki/tidy', 'pki-auto-tidy'); + this.numTidyAttrs = Object.keys(this.autoTidy.allByKey).length; }); test('it hides or shows fields depending on auto-tidy toggle', async function (assert) { - assert.expect(47); const sectionHeaders = [ + 'Automatic tidy settings', 'Universal operations', 'ACME operations', 'Issuer operations', 'Cross-cluster operations', ]; + const loopAssertCount = this.numTidyAttrs * 2 - 3; // loop skips 3 params + const headerAssertCount = sectionHeaders.length * 2; + assert.expect(loopAssertCount + headerAssertCount + 4); await render( hbs` @@ -51,11 +68,13 @@ module('Integration | Component | pki tidy form', function (hooks) { `, { owner: this.engine } ); - assert.dom(PKI_TIDY_FORM.toggleInput('intervalDuration')).isNotChecked('Automatic tidy is disabled'); - assert.dom(`[data-test-ttl-form-label="Automatic tidy disabled"]`).exists('renders disabled label text'); + assert.dom(GENERAL.toggleInput('enabled')).isNotChecked(); + assert + .dom(GENERAL.ttl.toggle('enabled')) + .hasText('Automatic tidy disabled Automatic tidy operations will not run.'); this.autoTidy.eachAttribute((attr) => { - if (attr === 'enabled' || attr === 'intervalDuration') return; + if (attr === 'enabled') return; assert .dom(PKI_TIDY_FORM.inputByAttr(attr)) .doesNotExist(`does not render ${attr} when auto tidy disabled`); @@ -66,12 +85,12 @@ module('Integration | Component | pki tidy form', function (hooks) { }); // ENABLE AUTO TIDY - await click(PKI_TIDY_FORM.toggleInput('intervalDuration')); - assert.dom(PKI_TIDY_FORM.toggleInput('intervalDuration')).isChecked('Automatic tidy is enabled'); - assert.dom(`[data-test-ttl-form-label="Automatic tidy enabled"]`).exists('renders enabled text'); + await click(GENERAL.toggleInput('enabled')); + assert.dom(GENERAL.toggleInput('enabled')).isChecked(); + assert.dom(GENERAL.ttl.toggle('enabled')).hasText('Automatic tidy enabled'); this.autoTidy.eachAttribute((attr) => { - const skipFields = ['enabled', 'tidyAcme', 'intervalDuration']; + const skipFields = ['enabled', 'tidyAcme']; if (skipFields.includes(attr)) return; // combined with duration ttl or asserted elsewhere assert.dom(PKI_TIDY_FORM.inputByAttr(attr)).exists(`renders ${attr} when auto tidy enabled`); }); @@ -82,10 +101,9 @@ module('Integration | Component | pki tidy form', function (hooks) { }); test('it renders all attribute fields, including enterprise', async function (assert) { - assert.expect(33); + assert.expect(35); this.autoTidy.enabled = true; - const skipFields = ['enabled', 'tidyAcme', 'intervalDuration']; // combined with duration ttl or asserted separately - const fieldsNotPartOfManualTidy = ['minStartupBackoffDuration', 'maxStartupBackoffDuration']; // fields we shouldn't see on the manual tidy screen + const skipFields = ['enabled', 'tidyAcme']; // combined with duration ttl or asserted separately await render( hbs` { if (skipFields.includes(attr)) return; - if (fieldsNotPartOfManualTidy.includes(attr)) { + // auto tidy fields we shouldn't see in the manual tidy form + if (this.manualTidy.autoTidyConfigFields.includes(attr)) { assert .dom(PKI_TIDY_FORM.inputByAttr(attr)) .doesNotExist(`${attr} should not appear on manual tidyType`); @@ -183,19 +203,35 @@ module('Integration | Component | pki tidy form', function (hooks) { test('it should change the attributes on the model', async function (assert) { assert.expect(12); + // ttl picker defaults to seconds, unless unit is set by default value (set in beforeEach hook) + // on submit, any user inputted values should be converted to seconds for the payload + const fillInValues = { + acmeAccountSafetyBuffer: { time: 680, unit: 'h' }, + intervalDuration: { time: 10, unit: 'h' }, + issuerSafetyBuffer: { time: 20, unit: 'd' }, + maxStartupBackoffDuration: { time: 30, unit: 'm' }, + minStartupBackoffDuration: { time: 10, unit: 'm' }, + pauseDuration: { time: 30, unit: 's' }, + revocationQueueSafetyBuffer: { time: 40, unit: 's' }, + safetyBuffer: { time: 50, unit: 'd' }, + }; + const calcValue = (param) => { + const { time, unit } = fillInValues[param]; + return `${convertToSeconds(time, unit)}s`; + }; this.server.post('/pki-auto-tidy/config/auto-tidy', (schema, req) => { assert.propEqual( JSON.parse(req.requestBody), { - acme_account_safety_buffer: '72h', + acme_account_safety_buffer: '48h', enabled: true, - min_startup_backoff_duration: '5m', - max_startup_backoff_duration: '15m', - interval_duration: '10s', - issuer_safety_buffer: '20s', - pause_duration: '30s', - revocation_queue_safety_buffer: '40s', - safety_buffer: '50s', + min_startup_backoff_duration: calcValue('minStartupBackoffDuration'), + max_startup_backoff_duration: calcValue('maxStartupBackoffDuration'), + interval_duration: calcValue('intervalDuration'), + issuer_safety_buffer: calcValue('issuerSafetyBuffer'), + pause_duration: calcValue('pauseDuration'), + revocation_queue_safety_buffer: calcValue('revocationQueueSafetyBuffer'), + safety_buffer: calcValue('safetyBuffer'), tidy_acme: true, tidy_cert_metadata: true, tidy_cert_store: true, @@ -222,16 +258,14 @@ module('Integration | Component | pki tidy form', function (hooks) { { owner: this.engine } ); - assert.dom(PKI_TIDY_FORM.toggleInput('intervalDuration')).isNotChecked('Automatic tidy is disabled'); - assert.dom(PKI_TIDY_FORM.toggleLabel('Automatic tidy disabled')).exists('auto tidy has disabled label'); + assert.dom(GENERAL.toggleInput('enabled')).isNotChecked(); + assert.dom(GENERAL.ttl.toggle('enabled')).hasTextContaining('Automatic tidy disabled'); assert.false(this.autoTidy.enabled, 'enabled is false on model'); // enable auto-tidy - await click(PKI_TIDY_FORM.toggleInput('intervalDuration')); - await fillIn(PKI_TIDY_FORM.intervalDuration, 10); - - assert.dom(PKI_TIDY_FORM.toggleInput('intervalDuration')).isChecked('toggle enabled auto tidy'); - assert.dom(PKI_TIDY_FORM.toggleLabel('Automatic tidy enabled')).exists('auto tidy has enabled label'); + await click(GENERAL.toggleInput('enabled')); + assert.dom(GENERAL.toggleInput('enabled')).isChecked(); + assert.dom(GENERAL.ttl.toggle('enabled')).hasText('Automatic tidy enabled'); assert.dom(PKI_TIDY_FORM.toggleInput('acmeAccountSafetyBuffer')).isNotChecked('ACME tidy is disabled'); assert @@ -240,30 +274,23 @@ module('Integration | Component | pki tidy form', function (hooks) { assert.false(this.autoTidy.tidyAcme, 'tidyAcme is false on model'); await click(PKI_TIDY_FORM.toggleInput('acmeAccountSafetyBuffer')); - await fillIn(PKI_TIDY_FORM.acmeAccountSafetyBuffer, 3); // units are days based on defaultValue + await fillIn(PKI_TIDY_FORM.acmeAccountSafetyBuffer, 2); // units are days based on defaultValue + assert.dom(PKI_TIDY_FORM.toggleInput('acmeAccountSafetyBuffer')).isChecked('ACME tidy is enabled'); + assert.dom(PKI_TIDY_FORM.toggleLabel('Tidy ACME enabled')).exists('ACME label has correct enabled text'); assert.true(this.autoTidy.tidyAcme, 'tidyAcme toggles to true'); - const fillInValues = { - issuerSafetyBuffer: 20, - pauseDuration: 30, - revocationQueueSafetyBuffer: 40, - safetyBuffer: 50, - minStartupBackoffDuration: 300, - maxStartupBackoffDuration: 900, - }; this.autoTidy.eachAttribute(async (attr, { type }) => { - const skipFields = ['enabled', 'tidyAcme', 'intervalDuration', 'acmeAccountSafetyBuffer']; // combined with duration ttl or asserted separately + const skipFields = ['enabled', 'tidyAcme', 'acmeAccountSafetyBuffer']; // combined with duration ttl or asserted separately if (skipFields.includes(attr)) return; + // all params right now are either a boolean or TTL, this if/else will need to be updated if that changes if (type === 'boolean') { await click(PKI_TIDY_FORM.inputByAttr(attr)); - } - if (type === 'string') { - await fillIn(PKI_TIDY_FORM.toggleInput(attr), `${fillInValues[attr]}`); + } else { + const { time } = fillInValues[attr]; + await fillIn(PKI_TIDY_FORM.toggleInput(attr), `${time}`); } }); - assert.dom(PKI_TIDY_FORM.toggleInput('acmeAccountSafetyBuffer')).isChecked('ACME tidy is enabled'); - assert.dom(PKI_TIDY_FORM.toggleLabel('Tidy ACME enabled')).exists('ACME label has correct enabled text'); await click(PKI_TIDY_FORM.tidySave); }); @@ -274,11 +301,11 @@ module('Integration | Component | pki tidy form', function (hooks) { assert.propEqual( JSON.parse(req.requestBody), { + ...this.autoTidyServerDefaults, acme_account_safety_buffer: '720h', - enabled: false, tidy_acme: false, }, - 'response contains auto-tidy params' + 'response contains default auto-tidy params' ); }); this.onSave = () => assert.ok(true, 'onSave callback fires on save success');