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

Create secret management input component #2192

Draft
wants to merge 25 commits into
base: main
Choose a base branch
from

Conversation

daphnemaeve
Copy link
Contributor

Adds button to open secret management wizard for credential types without kind of 'external'.

Before:
Screenshot 2024-04-26 at 11 56 50 AM

After:
Screenshot 2024-04-26 at 11 56 28 AM

@github-actions github-actions bot added the AWX Label to indicate changes relevant to AWX label Apr 26, 2024
@daphnemaeve daphnemaeve force-pushed the create-secret-mangement-input-component branch from 352031c to d881510 Compare April 30, 2024 18:24
@github-actions github-actions bot added the AFW Ansible Framework label May 1, 2024
@nixocio nixocio requested a review from kialam May 2, 2024 15:20
@daphnemaeve daphnemaeve force-pushed the create-secret-mangement-input-component branch from 5114ac2 to 8e14583 Compare May 7, 2024 18:29
requiredFields={requiredFields}
/>
);
} else if (credentialType.kind === 'ssh' && field.id === 'become_method') {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should not be removed...it is an edge case and we should keep this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, I'll put that back in. Does this case also have the potential to be an encrypted field?

Copy link
Contributor

Choose a reason for hiding this comment

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

@daphnemaeve it will have to manual test that one.

@daphnemaeve daphnemaeve force-pushed the create-secret-mangement-input-component branch from f7a863c to d821117 Compare May 13, 2024 23:56
@kialam
Copy link
Member

kialam commented May 15, 2024

Encrypted fields should only show up for fields that have values in them. See old UI for reference:
Screenshot 2024-05-15 at 10 20 05 AM
New UI:
Screenshot 2024-05-15 at 10 20 02 AM
Fields like Private Key Passphrase, etc. should not show up as encrypted.

@kialam
Copy link
Member

kialam commented May 15, 2024

A few more things:

  1. For the multiline input, the key icon is disabled in the edit form, and the undo/revert button should go to the left of the field:

Screenshot 2024-05-15 at 10 20 02 AM

  1. Clicking revert and then undo to put back the previous value sends an empty string to the API instead of the previously stored value.
  2. We should rename SecretManagementInputField since these fields are not tied to the Credential Plugins work.
  3. I see some commits that were also in a previous PR that was merged: https://github.com/ansible/ansible-ui/pull/2228/commits. To avoid any potential conflicts or unexpected behavior I would either rebase against main and if the commits still show up I would delete them. I think squashing a few of the other commit messages would also clean this branch up and mark relevant work more clearly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AFW Ansible Framework AWX Label to indicate changes relevant to AWX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants