SmartState Docker Creds for AWS #2525

jerryk55 · 2017-10-26T17:39:34Z

Add a new credentials tab for the Amazon provider that takes a userid and password
to be used when performing Smartstate Analysis. This tab is labeled "SmartState Docker".
These credentials will be passed to the AWS instance actually performing SSA to be used to log in to the Docker Registry. These credentials are NOT required, and no validation will be performed on them. They will simply be saved in the database to be passed to the SmartState Analysis agent.

This PR is dependent upon ManageIQ/manageiq-providers-amazon#330 which adds the smartstate_docker authtype to the Amazon provider.

Please note that while this PR is marked as WIP, it will need to be merged for the 4.6 build. It is functioning properly except for the following nits:

The SmartState Docker tab is marked with a red "!" which would lead one to believe that it is
not valid even after credentials are entered. This is related to the fact that we are not validating them.
Even though the SmartState Docker credentials are not required, the "Add" button for a new provider is not enabled until the new tab is clicked on. The credentials do not have to be entered.
If credentials on the SmartState Docker tab are entered and saved in the database, the next
time the Amazon provider is edited - the userid is not filled in to the appropriate field to be modified.
It needs to be entered again if one wishes to make changes to the password.

Finally - as a follow on we wish to enable or disable this tab based on a configuration setting. This will be added in a separate PR.

@roliveri @hsong-rh @Fryguy @AparnaKarve @h-kataria please review. We may wish to merge this as is if we cannot resolve the above nits so that the feature makes it into the build. Your mileage may vary.

Screenshots -

Old page to add Amazon Provider with Validation Required -

New Page to add Amazon Provider - Default Credentials Tab -

Old Page to add Amazon Provider - Credentials Validated -

New Page to add Amazon Provider - Smartstate Tab

Old page to Edit Amazon Provider - Validation Required

New page to edit Amazon Provider - Default Tab

Old page to edit Amazon Provider - Validated credentials

New page to edit Amazon Provider - Smartstate Tab

Links

Add smartstate_docker authtype manageiq-providers-amazon#330

Steps for Testing/QA

Add an Amazon Provider. Enter the default credentials. Click on the "SmartState Docker" tab.
enter the userid and password. Click "Add". Both sets of credentials will be stored in the VMDB.
These can be validated by inspecting the "authentications" table in the database.

jerryk55 · 2017-10-26T17:41:22Z

@miq-bot add_label wip

miq-bot · 2017-10-26T17:48:36Z

This pull request is not mergeable. Please rebase and repush.

Add a new credentials tab for the Amazon provider that takes a userid and password to be used when performing Smartstate Analysis. These credentials will be passed to the AWS instance actually performing SSA to be used to log in to the Docker Registry.

Fix warnings called out by Rubocop.

miq-bot · 2017-10-26T18:14:20Z

Checked commits jerryk55/manageiq-ui-classic@60d2725~...d6c88f6 with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0
4 files checked, 3 offenses detected

app/views/layouts/angular/_multi_auth_credentials.html.haml

⚠️ - Line 158 - Prefer to_s over string interpolation.
⚠️ - Line 159 - Prefer to_s over string interpolation.
⚠️ - Line 24 - Redundant curly braces around a hash parameter.

AparnaKarve · 2017-10-27T15:28:06Z

The tab implementation is not correct here, since

the error indicator on the SmartState docker tab never goes away
the Add button shows disabled on the Default tab but enabled for SmartState docker tab

The attached patch would resolve the above issue (works well for me)
However, @jerryk55 could not successfully confirm that.
ems_patch.txt

I suppose, we could resolve this issue in a follow-up and go ahead with the merge for now.

/cc @h-kataria

jerryk55 · 2017-10-27T15:39:10Z

Correct @AparnaKarve @h-kataria when I test the patch the error indicator on the Smartstate Tab still doesn't go away, and worse the Add button is never enabled so I cannot save the provider. If we get agreement I will turn off the "WIP" on this and we can address the issues with a follow-on PR.

h-kataria · 2017-10-27T15:55:09Z

@jerryk55 can you please add before/after screenshots, it is easy to test/verify changes.

jerryk55 · 2017-10-27T16:19:24Z

@h-kataria will do.

h-kataria · 2017-10-27T16:21:49Z

@jerryk55 verified patch from @AparnaKarve , seems to be working as expected, please apply that to your PR, then i can test/verify again.

jerryk55 · 2017-10-27T16:44:06Z

@h-kataria the patch did not work for me but I will try again. This could be some flakey cache issue on my appliance.

chessbyte · 2017-10-27T17:04:45Z

@jerryk55 also, don't forget to add before/after screenshots so people can look at the PR and quickly understand how it affects the UI

h-kataria · 2017-10-27T18:28:51Z

@jerryk55 i think i will merge this PR as is since patch from @AparnaKarve does not seem to work for you, please make a follow up PR with those changes. Please mark this PR as non-WIP once it is ready to merge.

jerryk55 · 2017-10-27T19:14:31Z

@chessbyte in the process...

jerryk55 · 2017-10-27T20:36:33Z

@h-kataria @chessbyte before and after screenshots added for adding and editing an Amazon provider with one or both sets of credentials.

jerryk55 · 2017-10-27T20:37:51Z

@miq-bot remove_label wip

chessbyte · 2017-10-27T20:40:29Z

@jerryk55 @Fryguy should we call it SmartState Container, instead of SmartState Docker?

jerryk55 · 2017-10-27T20:43:52Z

@chessbyte I will leave it up to those with stronger opinions. We already changed the name once before based on consensus from @roliveri @Fryguy and @bdunne. Originally it was named "RHSM Credentials" because those were the initial set of credentials required. Then @bdunne stated that the creds were for the Docker Registry, and that upstream and downstream they could be different.

chessbyte · 2017-10-27T20:49:27Z

@jerryk55 thanks for the clarification.
@h-kataria please review/merge. We can always change label name in a future PR

@Fryguy

The visibility of the Amazon Docker Creds tab will be based on a true Settings value. The setting used is Settings.ems.ems_amazon.agent_coordinator.docker_login_required. If the value is false no tab is presented. This feature is implemented based on offline discussions around the original PR implementing the tab - ManageIQ#2525 with @Fryguy.

@Fryguy

Re-adds commit b5ea01f after cleaning up the branch which was pushed with commits from another PR inadvertantly. The visibility of the Amazon Docker Creds tab will be based on a true Settings value. The setting used is Settings.ems.ems_amazon.agent_coordinator.docker_login_required. If the value is false no tab is presented. This feature is implemented based on offline discussions around the original PR implementing the tab - ManageIQ#2525 with @Fryguy.

@Fryguy

The visibility of the Amazon Docker Creds tab will be based on a true Settings value. The setting used is Settings.ems.ems_amazon.agent_coordinator.docker_login_required. If the value is false no tab is presented. This feature is implemented based on offline discussions around the original PR implementing the tab - ManageIQ#2525 with @Fryguy.

@Fryguy

The visibility of the Amazon Docker Creds tab will be based on a true Settings value. The setting used is Settings.ems.ems_amazon.agent_coordinator.docker_login_required. If the value is false no tab is presented. This feature is implemented based on offline discussions around the original PR implementing the tab - ManageIQ#2525 with @Fryguy. This is the original desired functionality but it was not implemented in time for the 4.6 cutoff.

miq-bot added the wip label Oct 26, 2017

jerryk55 mentioned this pull request Oct 26, 2017

Add smartstate_docker authtype ManageIQ/manageiq-providers-amazon#330

Merged

miq-bot added the unmergeable label Oct 26, 2017

SmartState Docker Creds for AWS

60d2725

Add a new credentials tab for the Amazon provider that takes a userid and password to be used when performing Smartstate Analysis. These credentials will be passed to the AWS instance actually performing SSA to be used to log in to the Docker Registry.

jerryk55 force-pushed the amazon_rhsm_credentials branch from f19d6a1 to 60d2725 Compare October 26, 2017 17:58

miq-bot removed the unmergeable label Oct 26, 2017

Rubocop Warnings

d6c88f6

Fix warnings called out by Rubocop.

chessbyte added compute/cloud enhancement labels Oct 27, 2017

chessbyte mentioned this pull request Oct 27, 2017

Various changes for SSA support ManageIQ/manageiq-providers-amazon#329

Merged

jerryk55 changed the title ~~[WIP] SmartState Docker Creds for AWS~~ SmartState Docker Creds for AWS Oct 27, 2017

miq-bot removed the wip label Oct 27, 2017

h-kataria self-assigned this Oct 27, 2017

h-kataria added the fine/no label Oct 27, 2017

h-kataria added this to the Sprint 72 Ending Oct 30, 2017 milestone Oct 27, 2017

h-kataria removed this from the Sprint 72 Ending Oct 30, 2017 milestone Oct 27, 2017

h-kataria added this to the Sprint 72 Ending Oct 30, 2017 milestone Oct 27, 2017

h-kataria merged commit 4780722 into ManageIQ:master Oct 27, 2017

jerryk55 mentioned this pull request Nov 7, 2017

Amazon Docker Creds Visibility Based on Setting #2657

Closed

jerryk55 mentioned this pull request Nov 8, 2017

Amazon Docker Creds Visibility Based on Setting #2666

Merged

agrare mentioned this pull request Oct 15, 2020

AWS SmartState Docker endpoint tab is missing on jansa #7425

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SmartState Docker Creds for AWS #2525

SmartState Docker Creds for AWS #2525

jerryk55 commented Oct 26, 2017 •

edited

Loading

jerryk55 commented Oct 26, 2017

miq-bot commented Oct 26, 2017

miq-bot commented Oct 26, 2017

AparnaKarve commented Oct 27, 2017 •

edited

Loading

jerryk55 commented Oct 27, 2017

h-kataria commented Oct 27, 2017

jerryk55 commented Oct 27, 2017

h-kataria commented Oct 27, 2017

jerryk55 commented Oct 27, 2017

chessbyte commented Oct 27, 2017

h-kataria commented Oct 27, 2017

jerryk55 commented Oct 27, 2017

jerryk55 commented Oct 27, 2017

jerryk55 commented Oct 27, 2017

chessbyte commented Oct 27, 2017

jerryk55 commented Oct 27, 2017 •

edited

Loading

chessbyte commented Oct 27, 2017

SmartState Docker Creds for AWS #2525

SmartState Docker Creds for AWS #2525

Conversation

jerryk55 commented Oct 26, 2017 • edited Loading

Links

Steps for Testing/QA

jerryk55 commented Oct 26, 2017

miq-bot commented Oct 26, 2017

miq-bot commented Oct 26, 2017

AparnaKarve commented Oct 27, 2017 • edited Loading

jerryk55 commented Oct 27, 2017

h-kataria commented Oct 27, 2017

jerryk55 commented Oct 27, 2017

h-kataria commented Oct 27, 2017

jerryk55 commented Oct 27, 2017

chessbyte commented Oct 27, 2017

h-kataria commented Oct 27, 2017

jerryk55 commented Oct 27, 2017

jerryk55 commented Oct 27, 2017

jerryk55 commented Oct 27, 2017

chessbyte commented Oct 27, 2017

jerryk55 commented Oct 27, 2017 • edited Loading

chessbyte commented Oct 27, 2017

jerryk55 commented Oct 26, 2017 •

edited

Loading

AparnaKarve commented Oct 27, 2017 •

edited

Loading

jerryk55 commented Oct 27, 2017 •

edited

Loading