Prep work for creating one WIF configuration component #29345

Monkeychip · 2025-01-10T18:55:54Z

Description

Pulling out some changes from a larger PR #29323. These changes are more clean up 🧹 related and distract a little from the core changes to that pr.

Specifically:

Cleaned up AWS models to match the pattern of Azure and GCP. All the changes are naming changes. No functional changes.
Using my new helper string-array-to-camel I was able to greatly simplify the method expectedConfigKeys which I use heavily in testing WIF engines. Before, this method was handling all sorts of options ex: expectedConfigKeys("aws-root-create") or ``expectedConfigKeys("aws-root-create-wif"). To simplify this method and clarify the potential keys, I pulled all the key options out. Now each engine has three options: generic, account or wif. And these key selectors change depending on if I'm viewing details or create views, so I use my stringArrayToCamel()` helper to help here.

No user facing changes.

ent tests pass.

github-actions · 2025-01-10T19:18:26Z

CI Results:
All Go tests succeeded! ✅

Monkeychip · 2025-01-10T19:21:59Z

ui/app/models/aws/root-config.js

@@ -45,23 +45,33 @@ export default class AwsRootConfig extends Model {
  iamEndpoint;
  @attr('string', { label: 'STS endpoint' }) stsEndpoint;
  @attr('number', {
-    label: 'Maximum retries',


The api name for this is "Max retries". I'm going to opt out of overriding this because it makes it complicated for testing.

Monkeychip · 2025-01-10T19:22:59Z

ui/app/models/aws/root-config.js

@@ -45,23 +45,33 @@ export default class AwsRootConfig extends Model {
  iamEndpoint;
  @attr('string', { label: 'STS endpoint' }) stsEndpoint;
  @attr('number', {
-    label: 'Maximum retries',
    subText: 'Number of max retries the client should use for recoverable errors. Default is -1.',
  })
  maxRetries;



no functional changes, just restructuring to match azure and gcp models—see here.

Monkeychip · 2025-01-10T19:23:25Z

ui/app/models/ssh/ca-config.js

@@ -38,16 +38,15 @@ export default class SshCaConfig extends Model {
  @attr('string') backend; // dynamic path of secret -- set on response from value passed to queryRecord
  @attr('string', { sensitive: true }) privateKey; // obfuscated, never returned by API
  @attr('string') publicKey;


no functional changes, just restructuring to match azure and gcp models—see here.

Monkeychip · 2025-01-10T19:24:05Z

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

@@ -219,7 +219,6 @@ module('Acceptance | aws | configuration', function (hooks) {
    });

    test('it shows AWS mount configuration details', async function (assert) {
-      assert.expect(12);


assert.expect unnecessary because we'll get hit if one of the keys fails.

Monkeychip · 2025-01-10T19:27:54Z

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

@@ -231,6 +230,7 @@ module('Acceptance | aws | configuration', function (hooks) {
      createConfig(this.store, path, type); // create the aws root config in the store
      await click(SES.configTab);
      for (const key of expectedConfigKeys(type)) {


You'll see a couple of these exceptions added to these loops. This is because of a clean up work done on the secret-engine-helper.

In that helper, I changed the method expectedConfigKeys to create generic arrays of expected values. Before I had two arrays for each engine, one for details and one for create—I opted to remove the two arrays and just make one for each engine to clarify within the tests, as I do here, what key we should not see in this iteration because the value is not returned from the API.

Monkeychip · 2025-01-10T19:29:12Z

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

@@ -368,7 +368,7 @@ module('Acceptance | aws | configuration', function (hooks) {
        .doesNotExist('Access type section does not render for a community user');
      // check all the form fields are present
      await click(GENERAL.toggleGroup('Root config options'));
-      for (const key of expectedConfigKeys('aws-root-create')) {


the value passed into expectedConfigKeys is changed here because of the above mentioned changed: I cleaned up this method to create generic arrays of keys that could come in camelized or not camelized.

true as a second param indicates "camelized"

Monkeychip · 2025-01-10T19:30:03Z

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

@@ -68,6 +68,8 @@ module('Acceptance | GCP | configuration', function (hooks) {
        service_account_email: 'service-email',
        identity_token_audience: 'audience',
        identity_token_ttl: 720000,


I did not originally tests these values on edit, but I am now.

Monkeychip · 2025-01-10T19:31:05Z

ui/tests/helpers/secret-engine/secret-engine-helpers.js

+    await fillIn(GENERAL.ttl.input('Identity token TTL'), '7200');
+  }
+};
+


Everything above this line is just re-organization, no changes. Below is the clean up of the expectedConfigKeys using my new helper method and making the arrays more generic.

Monkeychip · 2025-01-10T19:38:37Z

ui/app/models/aws/lease-config.js

@@ -32,8 +32,15 @@ export default class AwsLeaseConfig extends Model {
  })
  lease;



no functional changes, just restructuring to match azure and gcp models—see here.

Monkeychip · 2025-01-10T19:39:56Z

ui/tests/helpers/secret-engine/secret-engine-helpers.js

@@ -20,6 +21,32 @@ export const createSecretsEngine = (store, type, path) => {
  });
  return store.peekRecord('secret-engine', path);
 };


All changes until the next comment are just moving methods around to make it easier to read

github-actions · 2025-01-10T20:25:45Z

Build Results:
All builds succeeded! ✅

emoncuso · 2025-01-10T20:34:05Z

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

@@ -231,6 +230,7 @@ module('Acceptance | aws | configuration', function (hooks) {
      createConfig(this.store, path, type); // create the aws root config in the store
      await click(SES.configTab);
      for (const key of expectedConfigKeys(type)) {
+        if (key === 'Secret key') return; // secret-key is not returned by the API


I think return will stop overall execution of the function.

Maybe continue?

Suggested change

if (key === 'Secret key') return; // secret-key is not returned by the API

if (key === 'Secret key') continue; // secret-key is not returned by the API

emoncuso · 2025-01-10T20:34:32Z

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

@@ -94,6 +94,7 @@ module('Acceptance | Azure | configuration', function (hooks) {
        });
        await enablePage.enable(this.type, path);
        for (const key of expectedConfigKeys('azure')) {
+          if (key === 'Client secret') return; // client-secret is not returned by the API


Suggested change

if (key === 'Client secret') return; // client-secret is not returned by the API

if (key === 'Client secret') continue; // client-secret is not returned by the API

emoncuso · 2025-01-10T20:34:45Z

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

@@ -102,6 +104,7 @@ module('Acceptance | GCP | configuration', function (hooks) {
      });
      await enablePage.enable(this.type, path);
      for (const key of expectedConfigKeys(this.type)) {
+        if (key === 'Credentials') return; // not returned by the API


Suggested change

if (key === 'Credentials') return; // not returned by the API

if (key === 'Credentials') continue; // not returned by the API

emoncuso

Looks good to me!

* initial things without helper changes * adjust test for clean up of secret-engine-helper * remove added line thats better in next pr * remove extra check * 🧹 * replace return with continue within loops

initial things without helper changes

7d700aa

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

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

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

adjust test for clean up of secret-engine-helper

eff5b97

Monkeychip commented Jan 10, 2025

View reviewed changes

remove added line thats better in next pr

de30c7e

Monkeychip commented Jan 10, 2025

View reviewed changes

Monkeychip added 2 commits January 10, 2025 12:41

remove extra check

0d97240

🧹

bb8cfa9

Monkeychip marked this pull request as ready for review January 10, 2025 20:21

Monkeychip requested a review from a team as a code owner January 10, 2025 20:21

emoncuso reviewed Jan 10, 2025

View reviewed changes

replace return with continue within loops

94a9345

emoncuso approved these changes Jan 10, 2025

View reviewed changes

Monkeychip enabled auto-merge (squash) January 10, 2025 21:37

beagins approved these changes Jan 10, 2025

View reviewed changes

Monkeychip merged commit a73a698 into main Jan 10, 2025
32 of 33 checks passed

Monkeychip deleted the ui/VAULT-33327/clean-up branch January 10, 2025 22:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prep work for creating one WIF configuration component #29345

Prep work for creating one WIF configuration component #29345

Monkeychip commented Jan 10, 2025 •

edited

Loading

github-actions bot commented Jan 10, 2025 •

edited

Loading

Monkeychip Jan 10, 2025

Monkeychip Jan 10, 2025 •

edited

Loading

Monkeychip Jan 10, 2025 •

edited

Loading

Monkeychip Jan 10, 2025

Monkeychip Jan 10, 2025

Monkeychip Jan 10, 2025 •

edited

Loading

Monkeychip Jan 10, 2025

Monkeychip Jan 10, 2025

Monkeychip Jan 10, 2025

Monkeychip Jan 10, 2025

github-actions bot commented Jan 10, 2025

emoncuso Jan 10, 2025

emoncuso Jan 10, 2025

emoncuso Jan 10, 2025

emoncuso left a comment

		@@ -32,8 +32,15 @@ export default class AwsLeaseConfig extends Model {
		})
		lease;

	if (key === 'Secret key') return; // secret-key is not returned by the API
	if (key === 'Secret key') continue; // secret-key is not returned by the API

	if (key === 'Client secret') return; // client-secret is not returned by the API
	if (key === 'Client secret') continue; // client-secret is not returned by the API

	if (key === 'Credentials') return; // not returned by the API
	if (key === 'Credentials') continue; // not returned by the API

Prep work for creating one WIF configuration component #29345

Prep work for creating one WIF configuration component #29345

Conversation

Monkeychip commented Jan 10, 2025 • edited Loading

Description

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

Choose a reason for hiding this comment

Monkeychip Jan 10, 2025 • edited Loading

Choose a reason for hiding this comment

Monkeychip Jan 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Monkeychip Jan 10, 2025 • edited Loading

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

github-actions bot commented Jan 10, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emoncuso left a comment

Choose a reason for hiding this comment

Monkeychip commented Jan 10, 2025 •

edited

Loading

github-actions bot commented Jan 10, 2025 •

edited

Loading

Monkeychip Jan 10, 2025 •

edited

Loading

Monkeychip Jan 10, 2025 •

edited

Loading

Monkeychip Jan 10, 2025 •

edited

Loading