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.

Sign up for GitHub

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

Jump to bottom

Create/update Tower project with scm_credential #14618

Merged
merged 1 commit into from
Apr 17, 2017
Merged

Create/update Tower project with scm_credential #14618

merged 1 commit into from
Apr 17, 2017

Conversation

jameswnl
Copy link
Contributor

@jameswnl jameswnl commented Apr 3, 2017 •
edited
Loading

If a configuration_script_source is to be associated with a credential, the create_in_provider and update_in_provider is being sent a VMDB's authentication_id. We need to convert it to credential => authentication.manager_ref (Tower's credential native ID) so that Tower can act on.

@miq-bot add_labels providers/ansible_tower

@jameswnl
Copy link
Contributor Author

jameswnl commented Apr 3, 2017

@miq-bot add_labels bug

@miq-bot miq-bot added the bug label Apr 3, 2017
@jameswnl
Copy link
Contributor Author

jameswnl commented Apr 6, 2017

@miq-bot add_label fine/yes

blomquisg
blomquisg reviewed Apr 6, 2017
View reviewed changes
...ls/manageiq/providers/ansible_tower/shared/automation_manager/configuration_script_source.rb Outdated
@@ -2,7 +2,14 @@ module ManageIQ::Providers::AnsibleTower::Shared::AutomationManager::Configurati
extend ActiveSupport::Concern

module ClassMethods
def provider_params(params)
authentication_id = params.delete('authentication_id') || params.delete(:authentication_id)
Copy link
Member

Choose a reason for hiding this comment

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

Really? We don't know if the params are strings or symbols?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From UI, they will be strings since they will go through http. Keeping the symbol form here is to maintain consistency with the ATTRIBUTES we defined (they are in symbol form).

So if backend is the caller, it can use whole suites of symbols.

So it's a preference thing. If you guys like to get rid of it, I am not against it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The string form is removed now since API team has merged the patch to always convert keys to symbol.

@miq-bot
Copy link
Member

miq-bot commented Apr 6, 2017

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

@jameswnl jameswnl force-pushed the cred-pro branch 2 times, most recently from d550c23 to 881a101 Compare April 17, 2017 15:40
@miq-bot
Copy link
Member

miq-bot commented Apr 17, 2017

Checked commit jameswnl@f067258 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks good. 🏆

@blomquisg blomquisg merged commit 56c062e into ManageIQ:master Apr 17, 2017
@blomquisg blomquisg added this to the Sprint 59 Ending Apr 24, 2017 milestone Apr 17, 2017
simaishi pushed a commit that referenced this pull request Apr 17, 2017
@blomquisg @simaishi
Merge pull request #14618 from jameswnl/cred-pro
15167c8 
Create/update Tower project with scm_credential
(cherry picked from commit 56c062e)
@simaishi
Copy link
Contributor

Fine backport details:

$ git log -1
commit 15167c817746c912b59f421e0024aa776026e861
Author: Greg Blomquist <[email protected]>
Date:   Mon Apr 17 14:31:34 2017 -0400

    Merge pull request #14618 from jameswnl/cred-pro
    
    Create/update Tower project with scm_credential
    (cherry picked from commit 56c062eba131b7b0bb1691e953ccb73f263f0daf)

@simaishi
Copy link
Contributor

BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1439145

@jameswnl jameswnl deleted the cred-pro branch April 19, 2017 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blomquisg blomquisg blomquisg left review comments

Assignees
No one assigned
Labels
bug fine/backported providers/automation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jameswnl @miq-bot @simaishi @blomquisg @Fryguy