Use standard create/update/delete features for Authentications

[agrare]

Move the /api/authentications endpoint towards using standard create/update/delete supports features rather than checking for specific methods.

The goal is to use the standard create_ems_resource and ems_resource actions here

Depends on:

• Update authentications supports to reflect update and delete manageiq#22894
• Add supports :update/:delete to AWX Credential manageiq-providers-awx#24

[kbrock]

Darn, this is very close to the api_resource model.

almost like we want to define:

def resource_search(id, type)
  ::Authentication.descendant_get(type)
end

def create_resource(type, _id, data)
  api_resource(type, nil, "Creating", :supports => :create) do |klass|
    manager_resource, attrs = validate_auth_attrs(data)
    {:task_id => klass.create_in_provider_queue(manager_resource.id, attrs.deep_symbolize_keys)}
  end
end

But may need to fall back to api_action.

Now that I think about it, I had refactored this method a number of times.
Never got it quite right.

---

This is a definite improvement.

[kbrock]

ooh, right

I kept splitting apart the validate_auth_attrs:

def create_resource(_type, _id, data)
  type, id = data_resource(data)
  # maybe attrs from data_resource ?
  api_resource(type, id, "Creating", :supports => :create) do |auth, klass|
    {:task_id => klass.create_in_provider_queue(auth.id, attrs.deep_symbolize_keys)}
  end
end

def data_resource(data)
  href = Href.new(data['manager_resource']['href'])
  raise 'invalid manager_resource href specified' unless href.subject && href.subject_id
  [href.subject, href.subject_id]
end

nvr mind. there is a reason I never created a PR for this.

[kbrock]

the tests turned out very nicely.

I think getting to api_action or api_resource is a great start.

I think I have refactored this a number of times but got derailed with so many changes across the other controllers. Lets do as much as you want here but get it in.

The changes to the tests really show that you are on the right track.

[miq-bot]

This pull request has been automatically marked as stale because it has not been updated for at least 3 months.

If these changes are still valid, please remove the stale label, make any changes requested by reviewers (if any), and ensure that this issue is being looked at by the assigned/reviewer(s)

Thank you for all your contributions! More information about the ManageIQ triage process can be found in the triage process documentation.

[kbrock]

@agrare un-WIP if you ready to merge this one.

There was only one suggestion for api_resource, but I'm ok merging without that change.

[agrare]

Hey thanks for the nudge @kbrock let me rebase this and see if it still passes specs

[miq-bot]

Checked commit agrare@a6bb13d with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint
4 files checked, 4 offenses detected

spec/requests/authentications_spec.rb

• ❗ - Line 200, Col 107 - Layout/SpaceInsideHashLiteralBraces - Space inside } detected.
• ❗ - Line 200, Col 12 - Layout/SpaceInsideHashLiteralBraces - Space inside { detected.

spec/requests/configuration_script_payloads_spec.rb

• ❗ - Line 232, Col 107 - Layout/SpaceInsideHashLiteralBraces - Space inside } detected.
• ❗ - Line 232, Col 12 - Layout/SpaceInsideHashLiteralBraces - Space inside { detected.

[kbrock]

kicking. I want this in

[Fryguy]

Not all authentications are tied to a provider, so will this change prevent any operations on those? I'm asking because I thought this one was "special" for exactly this reason.

[agrare]

Not all authentications are tied to a provider, so will this change prevent any operations on those? I'm asking because I thought this one was "special" for exactly this reason.

This still doesn't require authentications to be tied to a provider, previously we used only the presence of a create_in_provider_queue method existing on the class, this just changes that to use supports :create/:update instead.

[agrare]

@kbrock @Fryguy I still need to update the providers to ensure they have the correct supports set, realizing now that is why I had it WIP before #1220 (comment)

[kbrock]

I still need to update the providers to ensure they have the correct supports set

can you restate this?
What supports (I see :delete) and where do those need to go?

[agrare]

What supports (I see :delete) and where do those need to go?

We have :create, :update, and :delete here. We used to check for respond_to? on create_in_provider_queue,, update_in_provider_queue, and delete_in_provider_queue.

We need to find all authentication classes that define these methods but not corresponding supports and add the supports feature in otherwise they will no longer work after this change. Just the first example I looked for, https://github.com/ManageIQ/manageiq-providers-awx/blob/master/app/models/manageiq/providers/awx/automation_manager/credential.rb#L12 has only supports :create but it responds to update/delete_in_provider_queue

[miq-bot]

This pull request has been automatically marked as stale because it has not been updated for at least 3 months.

If these changes are still valid, please remove the stale label, make any changes requested by reviewers (if any), and ensure that this issue is being looked at by the assigned/reviewer(s).

[kbrock]

re: WIP

I think ManageIQ/manageiq#22894 is the only change needed. It does not require a change in the class listed. (though I did add a caveat that adding to each of the child leaf nodes is an option)

[agrare]

@kbrock I think this is the list

>> Authentication.leaf_subclasses.select { |a| a.new.respond_to?(:update_in_provider_queue) }.reject { |a| a.supports?(:update) }.map(&:name)
=> 
["ManageIQ::Providers::EmbeddedAnsible::AutomationManager::AmazonCredential",  
 "ManageIQ::Providers::EmbeddedAnsible::AutomationManager::AzureCredential",   
 "ManageIQ::Providers::EmbeddedAnsible::AutomationManager::GoogleCredential",  
 "ManageIQ::Providers::EmbeddedAnsible::AutomationManager::OpenstackCredential",
 "ManageIQ::Providers::EmbeddedAnsible::AutomationManager::RhvCredential",     
 "ManageIQ::Providers::EmbeddedAnsible::AutomationManager::VmwareCredential",  
 "ManageIQ::Providers::EmbeddedAnsible::AutomationManager::MachineCredential", 
 "ManageIQ::Providers::EmbeddedAnsible::AutomationManager::NetworkCredential", 
 "ManageIQ::Providers::EmbeddedAnsible::AutomationManager::ScmCredential",     
 "ManageIQ::Providers::EmbeddedAnsible::AutomationManager::VaultCredential",   
 "ManageIQ::Providers::Workflows::AutomationManager::ScmCredential",           
 "ManageIQ::Providers::Workflows::AutomationManager::WorkflowCredential",      
 "ManageIQ::Providers::AnsibleTower::AutomationManager::AmazonCredential",     
 "ManageIQ::Providers::AnsibleTower::AutomationManager::AzureCredential",      
 "ManageIQ::Providers::AnsibleTower::AutomationManager::GoogleCredential",
 "ManageIQ::Providers::AnsibleTower::AutomationManager::OpenstackCredential",
 "ManageIQ::Providers::AnsibleTower::AutomationManager::RhvCredential",
 "ManageIQ::Providers::AnsibleTower::AutomationManager::Satellite6Credential",
 "ManageIQ::Providers::AnsibleTower::AutomationManager::VmwareCredential",
 "ManageIQ::Providers::AnsibleTower::AutomationManager::MachineCredential",
 "ManageIQ::Providers::AnsibleTower::AutomationManager::NetworkCredential",
 "ManageIQ::Providers::AnsibleTower::AutomationManager::ScmCredential",
 "ManageIQ::Providers::AnsibleTower::AutomationManager::VaultCredential",
 "ManageIQ::Providers::Awx::AutomationManager::AmazonCredential",
 "ManageIQ::Providers::Awx::AutomationManager::AzureCredential",
 "ManageIQ::Providers::Awx::AutomationManager::GoogleCredential",
 "ManageIQ::Providers::Awx::AutomationManager::OpenstackCredential",
 "ManageIQ::Providers::Awx::AutomationManager::RhvCredential",
 "ManageIQ::Providers::Awx::AutomationManager::Satellite6Credential",
 "ManageIQ::Providers::Awx::AutomationManager::VmwareCredential",
 "ManageIQ::Providers::Awx::AutomationManager::MachineCredential",
 "ManageIQ::Providers::Awx::AutomationManager::NetworkCredential",
 "ManageIQ::Providers::Awx::AutomationManager::ScmCredential",
 "ManageIQ::Providers::Awx::AutomationManager::VaultCredential"]

So your PR ManageIQ/manageiq#22894 covers the EmbeddedAnsible ones, so we just need ManageIQ/manageiq-providers-awx#24 and one for workflows