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.

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

Extend allowed_cloud_network for providers that don't support allowed_ci #41

Closed
wants to merge 1 commit into from

Conversation

d-m-u
Copy link
Contributor

@d-m-u d-m-u commented Jan 16, 2018

PR ManageIQ/manageiq#16811 adds RBAC filtering to allowed_cloud_network base class but two of the providers don't support the allowed_ci method so for them we need to overwrite the allowed_cloud_networks, to just return allowed_cloud_networks. Which this does! (Per ManageIQ/manageiq#16824 (comment))

Fixes issue caused by fix of https://bugzilla.redhat.com/show_bug.cgi?id=1533277
(Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1535189)

Related to:

ManageIQ/manageiq-providers-openstack#197

@d-m-u
Copy link
Contributor Author

d-m-u commented Jan 16, 2018

@miq-bot assign @gmcculloug

@d-m-u d-m-u changed the title Extend allowed_cloud_network for providers that don't support allowed_ci [WIP] Extend allowed_cloud_network for providers that don't support allowed_ci Jan 16, 2018
@d-m-u
Copy link
Contributor Author

d-m-u commented Jan 16, 2018

@Ladas @syncrou can you review this please?

@d-m-u
Copy link
Contributor Author

d-m-u commented Jan 16, 2018

@miq-bot add_label bug

@d-m-u d-m-u changed the title [WIP] Extend allowed_cloud_network for providers that don't support allowed_ci Extend allowed_cloud_network for providers that don't support allowed_ci Jan 16, 2018
@d-m-u
Copy link
Contributor Author

d-m-u commented Jan 16, 2018

@miq-bot add_label gaprindashvili/yes

def allowed_cloud_networks(_options = {})
return {} unless (src_obj = provider_or_tenant_object)

src_obj.all_cloud_networks.each_with_object({}) do |cn, hash|
Copy link

Choose a reason for hiding this comment

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

Per @Ladas' comment This makes sense to me. Though adding a test per his comment would help prove the point.

@d-m-u d-m-u force-pushed the fixing_allowed_cloud_networks branch from cd4a662 to 6e810a5 Compare January 16, 2018 20:57
let(:template) { FactoryGirl.create(:template_google, :name => "template", :ext_management_system => ems) }
let(:workflow) do
stub_dialog
allow_any_instance_of(User).to receive(:get_timezone).and_return(Time.zone)
Copy link

Choose a reason for hiding this comment

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

@d-m-u - You should be able to swap out line 9 with allow(User).to receive_messages(:server_timezone => "UTC") to get rid of one of the Rubocop warnings about allow_any_instance_of

@d-m-u d-m-u force-pushed the fixing_allowed_cloud_networks branch 3 times, most recently from 488fedd to 1d8775b Compare January 17, 2018 14:38
@d-m-u d-m-u force-pushed the fixing_allowed_cloud_networks branch from 1d8775b to e284168 Compare January 17, 2018 14:49
@miq-bot
Copy link
Member

miq-bot commented Jan 17, 2018

Checked commit d-m-u@e284168 with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🏆

Copy link

@syncrou syncrou left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@d-m-u d-m-u closed this Jan 18, 2018
@d-m-u d-m-u deleted the fixing_allowed_cloud_networks branch January 29, 2018 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants