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

UI: PKI Generate Root Form #18712

Merged
merged 14 commits into from
Jan 18, 2023
Merged

UI: PKI Generate Root Form #18712

merged 14 commits into from
Jan 18, 2023

Conversation

hashishaw
Copy link
Collaborator

@hashishaw hashishaw commented Jan 13, 2023

This PR adds a new component in the PKI engine, PkiGenerateRoot. It also renames the model/adapter/serializer pki/config to pki/action to better represent that the actions it encompasses can happen in places other than initial PKI configuration.

Form is rendered on config if Generate Root is selected
Screen Shot 2023-01-13 at 4 04 39 PM
Screen Shot 2023-01-13 at 4 04 47 PM

Form is rendered from issuers > generate root selection
Screen Shot 2023-01-13 at 4 05 13 PM
Screen Shot 2023-01-13 at 4 05 15 PM

Follow-on tasks:

  • Add URL section and action (after save, post to new endpoint) to form
  • Fill out tests for pki/action serializer
  • Write tests for pki-key-parameters

@hashishaw hashishaw enabled auto-merge (squash) January 17, 2023 16:34
Copy link
Contributor

@hellobontempo hellobontempo left a comment

Choose a reason for hiding this comment

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

Nice work on this! This form is a doozy. Holding off approving until tests pass, and we've confirmed the expected key parameters functionality

ui/app/adapters/pki/action.js Outdated Show resolved Hide resolved
ui/app/decorators/model-form-fields.js Show resolved Hide resolved
ui/app/models/pki/action.js Outdated Show resolved Hide resolved
ui/app/models/pki/action.js Show resolved Hide resolved
ui/app/models/pki/action.js Show resolved Hide resolved
ui/lib/pki/addon/components/pki-generate-root.js Outdated Show resolved Hide resolved
ui/lib/pki/addon/components/pki-key-parameters.js Outdated Show resolved Hide resolved
ui/lib/pki/addon/routes/issuers/generate-root.js Outdated Show resolved Hide resolved
ui/lib/pki/addon/templates/issuers/generate-root.hbs Outdated Show resolved Hide resolved
@model={{this.model}}
@onCancel={{transition-to "vault.cluster.secrets.backend.pki.issuers.index"}}
@onSave={{transition-to "vault.cluster.secrets.backend.pki.issuers.index"}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Would saving take us to the generated root cert?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I used to have a note about parsing the issuer from the save response and passing that for the onSave transition. I plan on looking into that with the generate-csr work -- for now, I figured this is a reasonable if not ideal transition

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeesh - the pki workflow flow 🤯. Yeah, sounds more than reasonable!

Copy link
Contributor

@hellobontempo hellobontempo left a comment

Choose a reason for hiding this comment

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

really great work on this 🥇

I was wrong about your changes to the pki-key-paremeters component (yay - tests worked!?) It didn't affect the functionality of the form to generate a key 💯.

Sorry about the mis-comment!

this.showGroup = isOpen ? group : null;
}

get defaultFields() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Having a component getter makes this so much clearer. Instead of passing a huge array of objects with arrays of attrs as groupPropertyNames to the model decorator (like we did in pki/roles)

@hashishaw hashishaw merged commit 2273c7b into main Jan 18, 2023
@hashishaw hashishaw deleted the ui/VAULT-9362/generate-root branch January 18, 2023 18:20
dhuckins pushed a commit that referenced this pull request Jan 19, 2023
dhuckins pushed a commit that referenced this pull request Jan 19, 2023
jayant07-yb pushed a commit to jayant07-yb/hashicorp-vault-integrations that referenced this pull request Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants