Pluggable credential types #22995
Pluggable credential types #22995
Conversation
| def self.credential_type | ||
| "embedded_ansible_credential_types" | ||
| end | ||
|
|
There was a problem hiding this comment.
NOTE I think once we get away from separate "embedded_workflows" and "embedded_ansible" controllers we can drop these "groups" of credential types, because you would just call OPTIONS and pass in a type like we do with ext_management_systems. So you would as for the credential types for the Workflows AutomationManager or the EmbeddedAnsible AutomationManager.
But for now this keeps compatibility without having to hard code all of the types 👍
There was a problem hiding this comment.
I don't thin OPTIONS supports the collection_class, but it could.
There was a problem hiding this comment.
@Fryguy we pass in type=class_name to OPTIONS, that is how we get DDF params for a particular EMS once you've selected it in the dropdown
There was a problem hiding this comment.
oh interesting - we probably should be passing in collection_class. The type filtering doesn't work with the "middle" classes, whereas collection_class does.
There was a problem hiding this comment.
Sorry, clarification - type is fine for the purpose listed which is an EMS is selected and then we filter on that EMS. collection_class would be for other purposes.
| @@ -1,6 +1,10 @@ | |||
| class ManageIQ::Providers::EmbeddedAnsible::AutomationManager::Credential < ManageIQ::Providers::EmbeddedAutomationManager::Authentication | |||
| FRIENDLY_NAME = "Embedded Ansible Credential".freeze | |||
|
|
|||
| def self.credential_type | |||
| "embedded_ansible_credential_types" | |||
There was a problem hiding this comment.
I'm wondering if we even need this method. It acts as a key into a hash that's presented over the API, but
a) maybe we don't need the key at all
b) if we do need some sort of key, just use the class name
c) if we do need the key and it can't be the class name, then just derive the key with something like "#{klass.name.gsub("::", "_").underscore}_credential_types
There was a problem hiding this comment.
Note I'm fine merging as is, because it keeps the interface identical to how it was previously, but makes it pluggable, which is progress.
There was a problem hiding this comment.
I think https://github.com/ManageIQ/manageiq/pull/22995/files#r1567779476 answers this, we need it for now because of how the UI splits external / internal automation controllers up ignoring the actual EMS object and assuming there is only one of the internal types.
nasark
force-pushed
the
pluggable_credential_types
branch
from
3e4c291
to
e036c57
Compare
|
Checked commit nasark@e036c57 with ruby 2.7.8, rubocop 1.56.3, haml-lint 0.51.0, and yamllint |
Originally credential types were hard coded in the
Authenticationclass. With this change we can define credential types in the respective classes and then pull the info for the OPTIONS call from the newAuthentication.credential_typesmethodDepends on:
@miq-bot assign @agrare
@miq-bot add_reviewer @agrare
@miq-bot add_labels enhancement