Credential.manager_ref need to be an integer for Tower 3.3 #18155
Conversation
| let(:auth_one) { FactoryGirl.create(:authentication, :manager_ref => 6) } | ||
| let(:auth_two) { FactoryGirl.create(:authentication, :manager_ref => 8) } | ||
| let(:auth_one) { FactoryGirl.create(:embedded_ansible_credential, :manager_ref => 6) } | ||
| let(:auth_two) { FactoryGirl.create(:embedded_ansible_credential, :manager_ref => 8) } |
There was a problem hiding this comment.
The previous code was setting manager_ref as an integer, but it be a string to match the column type.
| let(:auth_three) { FactoryGirl.create(:authentication, :manager_ref => 14) } | ||
| let(:auth_one) { FactoryGirl.create(:embedded_ansible_credential, :manager_ref => 6) } | ||
| let(:auth_two) { FactoryGirl.create(:embedded_ansible_credential, :manager_ref => 10) } | ||
| let(:auth_three) { FactoryGirl.create(:embedded_ansible_credential, :manager_ref => 14) } |
There was a problem hiding this comment.
Same here, manager_ref always be set to a string. Then native_ref converts to the expected type based on the sub-class.
There was a problem hiding this comment.
This is actually funny: in inventory refresh code, we do assign the manager_ref with whatever Tower sends us, which is an integer. But it will be converted to string in db record somehow.
There was a problem hiding this comment.
We should actually update the parser to convert to match the Db column then. Standard graph refresh will do the casting but batch graph refresh will not
spec/factories/authentication.rb
Outdated
| :class => "ManageIQ::Providers::EmbeddedAnsible::AutomationManager::ScmCredential" | ||
|
|
||
| factory :embedded_ansible_credential, |
There was a problem hiding this comment.
@jameswnl I think all these Authentication factory changes look correct. Could you give it a quick review?
There was a problem hiding this comment.
I can't find where :embedded_ansible_credential is defined
There was a problem hiding this comment.
sorry, nvm, it's right here. (was expecting it defined ahead of being used)
There was a problem hiding this comment.
can you move this up before the beginning of other embedded_ansible*?
spec/factories/authentication.rb
Outdated
| :class => "ManageIQ::Providers::AnsibleTower::AutomationManager::NetworkCredential" | ||
|
|
||
| factory :ansible_scm_credential, | ||
| :parent => :automation_manager_authentication, | ||
| :parent => :ansible_credential, | ||
| :class => "ManageIQ::Providers::AnsibleTower::AutomationManager::ScmCredential" | ||
|
|
||
| factory :embedded_ansible_amazon_credential, | ||
| :parent => :automation_manager_authentication, |
lfu
force-pushed
the
ansible_cred_ref_1640533
branch
from
5a41087
to
54159c0
Compare
lfu
force-pushed
the
ansible_cred_ref_1640533
branch
from
54159c0
to
bf62a8c
Compare
|
Checked commits lfu/manageiq@026f4f1~...bf62a8c with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
|
LGTM |
Credential.manager_ref need to be an integer for Tower 3.3 (cherry picked from commit 9ac7cfa) https://bugzilla.redhat.com/show_bug.cgi?id=1640533
|
Hammer backport details: |
Follow up of ManageIQ/manageiq-providers-ansible_tower#134.
Change the callers of
credential.manager_refto usenative_ref.Includes ManageIQ/manageiq-providers-ansible_tower#138.
Fix https://bugzilla.redhat.com/show_bug.cgi?id=1640533
@miq-bot assign @gmcculloug
@miq-bot add_label blocker, bug, hammer/yes