UI (sidebranch) WIF Issuer field #28187

hashishaw · 2024-08-26T18:17:00Z

Description

This PR adds a WIF-only field issuer to the AWS configure form. This value is read and written to a global endpoint, not part of the AWS configuration models. On the configuration edit route we attempt to read this value if it's enterprise + AWS, and pass the currently configured value to the form component so a user can choose to update it.

The field has a placeholder when issuer is not set or can't be read:

When the issuer could not be read, but the user updates it the modal says:

Otherwise when the value is changed, the modal says:

Ent tests passed

hellobontempo · 2024-08-26T18:20:20Z

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

+  original = '';
+  @tracked issuer = '';
+  @tracked noRead = false;
+  init(initialIssuer: string | undefined) {


I was reading a little about init() vs constructor() is there a reason you opted to use init() here?

Since I need to pass in an arg, I wasn't sure if that was going to be available on constructor. I can try it though

Just a guess here, but because she calls the init method later. You initialize the class, and init gets called, but you can also then call init outside the class to isolate calling that specific method? Learning alongside you.

Oh no need to change it, I was just curious if there was a reason! I can read up more on the pros/cons of each one. I do know that you can still pass args using constructor, I used it in the kv patch form.

Yep, Angel from what I was reading init can be called when a class is partially instantiated? Whereas the constructor is before.

Feel free to share what you find! Since the constructor can be called with the arg I like that, but open to pros/cons

hellobontempo · 2024-08-26T18:22:02Z

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

+    }
+  }
+  // this is to make it work with FormField
+  @action set(_: string, val: string) {


Do you need @action here?

I believe so, but I can play around with it

commenting so I can learn here as well (or correct my understanding), but if I remember right the action decorator binds the function directly to the instance of the class it's called from. So by calling this in the action, you know that this refers to the class it's within. I believe this is a bigger deal when you're passing actions around, but please correct me if that's wrong.

Good memory Angel, and well said! I believe this is the case

Awesome! But this is a javascript class, right? I thought the @action decorator was specific to just ember components 🤔

Pretty sure components have evolved to be very thin wrappers around classes, and decorators are pretty class-agnostic. I can't find specific docs on this, but from here we see reference to the binding that Angel mentioned:

The action decorator also binds actions, so you can refer to them directly in templates without the {{action}} helper:

and here we see that an action is applied on a regular old class. Elsewhere in this post we see another reference to the point of @action, which is binding this correctly to the class:

Note that we’re using the @action decorator here to bind the context of the someObserver method, so it’s this always refers to the class instance.

This method specifically is being passed to <FormField /> and called from the class so the decorator is extraneous, but if and when we update this to use HDS components directly or otherwise calling this update from the template, we will need the @action decorator

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

github-actions · 2024-08-26T18:27:25Z

CI Results:
All Go tests succeeded! ✅

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

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

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

github-actions · 2024-08-27T16:09:20Z

Build Results:
All builds succeeded! ✅

Monkeychip · 2024-08-27T16:25:03Z

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

-      this.flashMessages.info('No changes detected.');
-      this.transition();
-    }
+  @action continueSubmitForm() {


This is a little easier to follow how the interruption happens. Nice mix of solutions without the extra tracked property.

Monkeychip · 2024-08-27T16:26:20Z

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

+    })
+  );
+
+  save = task(


I really like how easily this reads even thought there's a lot happening!

Monkeychip

Awesome work 🙌🏻

hellobontempo

Great work!

hellobontempo · 2024-08-27T16:12:35Z

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

+    </M.Body>
+    <M.Footer>
+      <Hds::ButtonSet>


No need to change, but just out of curiosity - is there a reason you didn't use the built-in footer here? For what it's worth, I think it allows you to call F.close which will call the onClose function above. I'm not sure of the difference stylistically

<M.Footer as |F|> <Hds::Button type="button" @text="Confirm" {{on "click" F.close}} /> </M.Footer>

I copied this from a different component, so that's the only reason

hellobontempo · 2024-08-27T16:14:32Z

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

+    }
+  }
+  // this is to make it work with FormField
+  @action set(_: string, val: string) {


Awesome! But this is a javascript class, right? I thought the @action decorator was specific to just ember components 🤔

hellobontempo · 2024-08-27T16:17:42Z

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

+        this.saveIssuerWarning = `You are updating the global issuer config. This will overwrite Vault's current issuer ${
+          this.issuerConfig.noRead ? 'if it exists ' : ''
+        }and may affect other configurations using this value. Continue?`;
+        // exit task until user confirms


great comments, thank you!

* manual cherry pick to deal with all the merge things * changelog * test fixes * Update 28148.txt * fix tests failures after main merge * fix test failures after main merge * Add Access Type and conditionally render WIF fields (#28149) * initial work. * remove access_type * better no model logic well kind of * rollback attrs * remove defaults * stopping point * wip changing back to sidebranch * hustling shuffling and serializing * some of the component test coverage * disable acces type if editing * test coverage * hide max retries that sneaky bugger * cleanup * cleanup * Update root-config.js * remove flash message check, locally passes great but on ci flaky * clean up * thank you chelsea * test clean up per enterprise vs community * address pr comments * welp a miss add * UI (sidebranch) WIF Issuer field (#28187) * Add type declaration files for aws config models * use updated task syntax for save method on configure-aws * fix types on edit route * fetch issuer on configure edit page if aws + enterprise * track issuer within configure-aws component * add placeholder support on form-field * Add warning if issuer changed from previous value or could not be read * cleanup * preliminary tests * dont use while loop so we can test the modal * tests * cleanup * fix tests * remove extra tracked value and duplicate changed attrs check * modal footer --------- Co-authored-by: Angel Garbarino <[email protected]> * Display issuer on Configuration details (#28209) * display issuer on configuration details * workflow complete, now on to testing * handle issuer things * fix all the broken tests things * add test coveragE: * cleanup * rename model/adapter * Update configure-aws.ts * Update aws-configuration-test.js * 90 percent there for pr comments * last one for tonight * a few more because why not * hasDirtyAttributes fixes * revert back to previous noRead->queryIssuerError --------- Co-authored-by: Chelsea Shaw <[email protected]>

hashishaw added 7 commits August 26, 2024 12:16

Add type declaration files for aws config models

fd87a02

use updated task syntax for save method on configure-aws

ec924aa

fix types on edit route

542bd9e

fetch issuer on configure edit page if aws + enterprise

d7f0aa6

track issuer within configure-aws component

e63c76d

add placeholder support on form-field

3ea4029

Add warning if issuer changed from previous value or could not be read

0bcc20d

hashishaw added ui pr/no-changelog pr/no-milestone labels Aug 26, 2024

github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Aug 26, 2024

hellobontempo reviewed Aug 26, 2024

View reviewed changes

ui/app/components/secret-engine/configure-aws.ts Outdated Show resolved Hide resolved

hellobontempo reviewed Aug 26, 2024

View reviewed changes

ui/app/components/secret-engine/configure-aws.ts Outdated Show resolved Hide resolved

hellobontempo reviewed Aug 26, 2024

View reviewed changes

ui/app/components/secret-engine/configure-aws.ts Outdated Show resolved Hide resolved

Monkeychip reviewed Aug 26, 2024

View reviewed changes

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

Monkeychip reviewed Aug 26, 2024

View reviewed changes

ui/app/components/secret-engine/configure-aws.ts Show resolved Hide resolved

Monkeychip reviewed Aug 26, 2024

View reviewed changes

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

hashishaw and others added 7 commits August 26, 2024 14:59

cleanup

a72e09c

preliminary tests

6186392

dont use while loop so we can test the modal

ec07a59

tests

b766c0e

cleanup

6263eb8

fix tests

623eddf

remove extra tracked value and duplicate changed attrs check

24d7323

hashishaw marked this pull request as ready for review August 27, 2024 16:05

hashishaw requested a review from a team as a code owner August 27, 2024 16:05

Monkeychip reviewed Aug 27, 2024

View reviewed changes

Monkeychip approved these changes Aug 27, 2024

View reviewed changes

hellobontempo approved these changes Aug 27, 2024

View reviewed changes

hashishaw merged commit e1c9f6d into ui/VAULT-27326/wif-aws-sidebranch-3 Aug 27, 2024
5 of 7 checks passed

hashishaw deleted the ui/VAULT-28471/wif-issuer-field branch August 27, 2024 19:25

modal footer

53afd37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UI (sidebranch) WIF Issuer field #28187

UI (sidebranch) WIF Issuer field #28187

hashishaw commented Aug 26, 2024 •

edited

Loading

hellobontempo Aug 26, 2024

hashishaw Aug 26, 2024

Monkeychip Aug 26, 2024

hellobontempo Aug 27, 2024 •

edited

Loading

hashishaw Aug 27, 2024

hellobontempo Aug 26, 2024

hashishaw Aug 26, 2024

Monkeychip Aug 26, 2024

hashishaw Aug 26, 2024

hellobontempo Aug 27, 2024

hashishaw Aug 27, 2024

hashishaw Aug 27, 2024

github-actions bot commented Aug 26, 2024

github-actions bot commented Aug 27, 2024

Monkeychip Aug 27, 2024

Monkeychip Aug 27, 2024

Monkeychip left a comment

hellobontempo left a comment

hellobontempo Aug 27, 2024

hashishaw Aug 27, 2024

hellobontempo Aug 27, 2024

hellobontempo Aug 27, 2024

UI (sidebranch) WIF Issuer field #28187

UI (sidebranch) WIF Issuer field #28187

Conversation

hashishaw commented Aug 26, 2024 • edited Loading

Description

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hellobontempo Aug 27, 2024 • 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

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 Aug 26, 2024

github-actions bot commented Aug 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Monkeychip left a 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

Choose a reason for hiding this comment

hashishaw commented Aug 26, 2024 •

edited

Loading

hellobontempo Aug 27, 2024 •

edited

Loading