One WIF configuration component #29367

Monkeychip · 2025-01-17T20:56:15Z

Description

GCP is the next WIF engine to allow create/edit configuration. While creating it, I noticed it was a near identical copy/paste of the configure-azure component. Thus, I decided to create a shared create/edit configure component called configure-wif for all WIF configurable engines. Short-term this includes: AWS, Azure and soon GCP. Long-term this may include more 🤷‍♀️.

All of these WIF configurations include shared workflows:

Access Type options
Issuer modal and network requests
Community vs. Enterprise views
Error flow if either the modal.save() fails and/or issuer.save() fails.

Notes

Before this change, AWS allowed a user to save the lease config if the root config failed. This is no longer the case. The user must save the root config if they want to save the lease config. Originally, I had this loophole in there in the rare case a user had the ability to save the lease but not the root config. However, accounting for this case in a shared workflow creates more overhead than the benefit of potentially preventing something that is very unlikely.
I do not touch SSH. SSH is also one of the 4 configurable engines (AWS, Azure, GCP and SSH). However, it's configuration workflow is significantly different because it's not WIF configurable and it does some odd generate key things.
Combines the component tests but not the acceptance configuration tests for these engines.
there are no user facing changes. GCP will be added in a subsequent PR.

…the two components and associated files the new component replaces

github-actions · 2025-01-17T21:18:18Z

CI Results:
All Go tests succeeded! ✅

…ngine helper

Monkeychip · 2025-01-21T18:06:20Z

ui/app/controllers/vault/cluster/secrets/backend/configuration/edit.js

@@ -3,54 +3,14 @@
 * SPDX-License-Identifier: BUSL-1.1
 */

-import { isPresent } from '@ember/utils';


definitely forget to remove this after the original aws/ssh configuration work. Double checked and because the save actions are now happening for all configurable secret engines within components, this controller and it's methods were not being hit. However, now I'm using it to add two attributes I want to use in the route template and pass down to the components.

github-actions · 2025-01-21T18:16:01Z

Build Results:
All builds succeeded! ✅

hellobontempo

Great work making this component more flexible. Mostly cleanup suggestions that are non-blocking. The only blocking comment is about the errorMessage as I'm not sure the error handling is working as expected.

Also it'd be great if we can remove assert.ok usage. Let me know if you want me to DM you the team pattern discussion I remembered that from! 😄

ui/app/components/secret-engine/configure-wif.hbs

ui/tests/integration/components/secret-engine/configure-wif-test.js

Monkeychip · 2025-01-22T18:17:47Z

ui/app/components/secret-engine/configure-wif.ts

+ * Copyright (c) HashiCorp, Inc.
+ * SPDX-License-Identifier: BUSL-1.1
+ */
+


interesting.. gh now shows this as a new file instead of changes on configure-azure.ts —that's why the file count jump in this last push.

ui/app/components/secret-engine/configure-wif.hbs

hellobontempo · 2025-01-22T21:23:36Z

ui/app/components/secret-engine/configure-wif.hbs

-    {{#if @secondModel}}
-      <h2 class="title is-5 has-bottom-padding-s has-top-margin-l" data-test-second-model-title>
+    {{! additionalConfigModel fields show regardless of the vault version or what access type is selected }}
+    {{#if @additionalConfigModel}}


This rename already makes things much clearer! Thanks for the update.

hellobontempo · 2025-01-22T21:27:25Z

ui/app/routes/vault/cluster/secrets/backend/configuration/edit.ts

-      // create a key that corresponds with the configs model order
-      // ex: adapterPath = ssh/ca-config, convert to: first-model so that you can pass to component @model={{this.model.first-model}}
-      const standardizedKey = this.standardizedModelName(type, adapterPath);
+    for (const modelName of MOUNT_CONFIG_MODEL_NAMES[type] as string[]) {


Thanks for the rename, this is easier to parse (but also grrr Ember data, I will not miss you when you're gone)

hellobontempo · 2025-01-22T21:28:12Z

ui/tests/acceptance/secrets/backend/aws/aws-configuration-test.js

-        assert.false(
-          true,
-          'post request was made to config/lease when no data was changed. test should fail.'
+        throw new Error(


👏 This is much easier for me to follow what's happening here! Thanks for updating

hellobontempo · 2025-01-22T21:28:52Z

ui/tests/acceptance/secrets/backend/aws/aws-configuration-test.js

@@ -196,37 +194,12 @@ module('Acceptance | aws | configuration', function (hooks) {
      await runCmd(`delete sys/mounts/${path}`);
    });

-    test('it does not save lease AWS configuration if root configuration errored on save', async function (assert) {


Is this the behavior that changed that you mentioned?

ui/tests/acceptance/secrets/backend/aws/aws-configuration-test.js

hellobontempo

Great work! I think it may be worth checking with design about the error message display, I don't feel qualified to decide that 😂

ui/tests/integration/components/secret-engine/configure-wif-test.js

hellobontempo · 2025-01-24T18:02:39Z

ui/tests/integration/components/secret-engine/configure-wif-test.js

+      });
+
+      module('Azure specific', function (hooks) {
+        hooks.beforeEach(function () {


hellobontempo · 2025-01-24T18:03:56Z

ui/tests/integration/components/secret-engine/configure-wif-test.js

+
+          assert
+            .dom(SES.configureNote('azure'))
+            .doesNotExist('Note specific to AWS does not show for Azure secret engine when configuring.');


Thanks for adding this assertion!

hellobontempo · 2025-01-24T18:04:16Z

ui/tests/integration/components/secret-engine/configure-wif-test.js

@@ -144,7 +144,7 @@ module('Integration | Component | SecretEngine::ConfigureWif', function (hooks)
          assert.true(this.flashDangerSpy.notCalled, 'No danger flash messages called.');
          assert.true(this.flashSuccessSpy.notCalled, 'No success flash messages called.');

-          assert.ok(
+          assert.true(


hellobontempo

Great work! Thanks for addressing all of those cleanup items! ✨

make one component and make one test file for that component. remove …

4228154

…the two components and associated files the new component replaces

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

Monkeychip added 2 commits January 17, 2025 13:30

make access type subtext dynamic based on model type

eda5a70

clean up

06ae272

Monkeychip added ui pr/no-changelog labels Jan 17, 2025

Monkeychip added this to the 1.19.0-rc milestone Jan 17, 2025

Monkeychip added 4 commits January 17, 2025 14:15

clean up

7cd35d7

remove model attr for display purposes

0c82bba

split out lease to another second config model type and make is-wif-e…

c6e856a

…ngine helper

welp missed the old controller

8559a65

Monkeychip commented Jan 21, 2025

View reviewed changes

Monkeychip and others added 2 commits January 21, 2025 11:07

small removal of overkill comment

a63f0e9

Merge branch 'main' into ui/VAULT-33327/one-config-component-2

e9ba4c9

Monkeychip marked this pull request as ready for review January 21, 2025 18:12

Monkeychip requested a review from a team as a code owner January 21, 2025 18:12

vercel bot deployed to Preview January 21, 2025 18:16 View deployment

hellobontempo requested changes Jan 22, 2025

View reviewed changes

pr feedback

3a6e79d

Monkeychip commented Jan 22, 2025

View reviewed changes

Monkeychip added 2 commits January 22, 2025 12:07

save lease config if only thing changed

ff0689d

error handling in acceptance test

4257c9f

Monkeychip commented Jan 22, 2025

View reviewed changes

ui/app/components/secret-engine/configure-wif.hbs Show resolved Hide resolved

hellobontempo reviewed Jan 22, 2025

View reviewed changes

ui/tests/acceptance/secrets/backend/aws/aws-configuration-test.js Outdated Show resolved Hide resolved

hellobontempo previously approved these changes Jan 22, 2025

View reviewed changes

test fix

c5529b4

Monkeychip dismissed hellobontempo’s stale review via c5529b4 January 23, 2025 16:59

Monkeychip added 2 commits January 23, 2025 10:08

replace notOk with throw

d0c05c0

move back error message

0cec8e4

hellobontempo reviewed Jan 23, 2025

View reviewed changes

ui/tests/integration/components/secret-engine/configure-wif-test.js Outdated Show resolved Hide resolved

Monkeychip added 2 commits January 24, 2025 09:52

clean up focused largely on wif component test

81e3c15

replace ok with true

f7f1cce

hellobontempo reviewed Jan 24, 2025

View reviewed changes

hellobontempo approved these changes Jan 24, 2025

View reviewed changes

Monkeychip merged commit 088bb4b into main Jan 24, 2025
32 of 33 checks passed

Monkeychip deleted the ui/VAULT-33327/one-config-component-2 branch January 24, 2025 18:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

One WIF configuration component #29367

One WIF configuration component #29367

Monkeychip commented Jan 17, 2025 •

edited

Loading

github-actions bot commented Jan 17, 2025 •

edited

Loading

Monkeychip Jan 21, 2025 •

edited

Loading

github-actions bot commented Jan 21, 2025

hellobontempo left a comment

Monkeychip Jan 22, 2025

hellobontempo Jan 22, 2025

hellobontempo Jan 22, 2025

Monkeychip Jan 23, 2025

hellobontempo Jan 22, 2025

hellobontempo Jan 22, 2025

Monkeychip Jan 24, 2025

hellobontempo left a comment

hellobontempo Jan 24, 2025

hellobontempo Jan 24, 2025

hellobontempo Jan 24, 2025

hellobontempo left a comment

One WIF configuration component #29367

One WIF configuration component #29367

Conversation

Monkeychip commented Jan 17, 2025 • edited Loading

Description

github-actions bot commented Jan 17, 2025 • edited Loading

Monkeychip Jan 21, 2025 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Jan 21, 2025

hellobontempo 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

hellobontempo 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

hellobontempo left a comment

Choose a reason for hiding this comment

Monkeychip commented Jan 17, 2025 •

edited

Loading

github-actions bot commented Jan 17, 2025 •

edited

Loading

Monkeychip Jan 21, 2025 •

edited

Loading