Add belongsto filter for other network models #16151

lpichler · 2017-10-09T09:49:38Z

same thing as was done in #16063
but for other models in Network menu

belongsto filter is H & C tab in the group setting as is shown on the picture:

Description of bug:
We have selected network manager but for some models this filter is not applied:
and we have 2 network manager one is openstack and second azure

before:
example for CloudSubnet, you can see also CloudSubnet for openstack network manager

we need to see only Azure's CloudSubnets
so I added this rule other models as we did in #16063
after:

Links

EUWE BZ:https://bugzilla.redhat.com/show_bug.cgi?id=1465093
FINE BZ:https://bugzilla.redhat.com/show_bug.cgi?id=1463422

@miq-bot add_label blocker, rbac, bug, fine, euwe
@miq-bot assign @gtanzillo

when we will filter these network models by RBAC we will need apply filter entered in belongsto filter (H & C tab in group setting) - this array will help to handle this situation.

miq-bot · 2017-10-09T09:50:24Z

@lpichler Cannot apply the following labels because they are not recognized: fine, euwe

bdunne · 2017-10-09T14:57:06Z

spec/lib/rbac/filterer_spec.rb

          end

-          it 'lists no cloud networks' do
+          it 'doesn\'t list cloud networks' do


I think most specs use " to avoid these issues.

👍 changed to ", in the separated commit, thanks

It looks like this is another one. The last commit fixes a different escaped single quote.

yes I missed one context

lpichler · 2017-10-09T16:34:50Z

@jrafanie I added parentheses https://github.com/ManageIQ/manageiq/pull/16151/files#diff-8d948055de14ced0e63abf9637a9a788R590,
thanks for the review!

jrafanie · 2017-10-09T17:49:09Z

lib/rbac/filterer.rb

-        if [ExtManagementSystem, Host].any? { |x| vcmeta.kind_of?(x) } && klass <= VmOrTemplate ||
-           vcmeta.kind_of?(ManageIQ::Providers::NetworkManager)        && klass <= CloudNetwork
+        if ([ExtManagementSystem, Host].any? { |x| vcmeta.kind_of?(x) } && klass <= VmOrTemplate) ||
+           (vcmeta.kind_of?(ManageIQ::Providers::NetworkManager)        && NETWORK_MODELS_FOR_BELONGSTO_FILTER.any? { |association_class| klass <= association_class.safe_constantize })


As discussed, as a followup for master, we'll give this conditional a method name that hopefully makes more sense.

jrafanie · 2017-10-09T17:50:13Z

lib/rbac/filterer.rb

+      SecurityGroup
+      FloatingIp
+      NetworkPort
+      LoadBalancer


For the future, we should find a way to not hardcode this list because we'll need to update this list the moment we add something new.

Can this be sorted alphabetically?

@jrafanie 👍 absolutely agree, @bdunne 👍

jrafanie · 2017-10-09T18:03:00Z

spec/lib/rbac/filterer_spec.rb

+          NetworkRouter
+          SecurityGroup
+          ManageIQ::Providers::NetworkManager
+        ).freeze


Can you use the rbac filterer's constant instead of having another hardcoded list?

jrafanie · 2017-10-09T19:15:55Z

spec/lib/rbac/filterer_spec.rb

+              FactoryGirl.create(network_model.underscore,  :ext_management_system => network_manager_1)
+            end
+
+            context "when records match belognsto filter" do


I know you moved this but it's typo'd: belongsto

jrafanie · 2017-10-09T19:16:11Z

spec/lib/rbac/filterer_spec.rb

+              end
+            end
+
+            context "when records don't match belognsto filter" do


and put it on the same level as context 'when records match belognsto filter' context 'when records don\'t match belognsto filter'

exception is network manager - it is already created in upper block

we need to add together cases with are using cloud_network and cloud_network_1 models

lpichler · 2017-10-09T20:15:53Z

@jrafanie @bdunne added your suggestions, typos fixed

jrafanie · 2017-10-09T20:36:00Z

spec/lib/rbac/filterer_spec.rb

+          NetworkRouter,
+          SecurityGroup,
+          ManageIQ::Providers::NetworkManager
+        ].freeze


Sorry @lpichler, I wasn't clear. Can you do described_class::NETWORK_MODELS_FOR_BELONGSTO_FILTER + ManageIQ::Providers::NetworkManager so we're not duplicating the hardcoded list here and the code?

👍 oh yes sure, I read it badly

miq-bot · 2017-10-09T21:03:03Z

Checked commits lpichler/manageiq@f9fc44f~...3fd9feb with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks fine. 🍰

jrafanie

LGTM once green

lpichler · 2017-10-10T19:37:05Z

@miq-bot add_label euwe/yes, fine/yes, @simaishi

miq-bot · 2017-10-10T19:38:35Z

@lpichler Cannot apply the following label because they are not recognized: @simaishi

…work_model Add belongsto filter for other network models (cherry picked from commit cf2f4e7) https://bugzilla.redhat.com/show_bug.cgi?id=1465093

simaishi · 2017-10-23T14:17:16Z

Euwe backport details:

$ git log -1
commit a5e250fb7326597c7ae19155cd329c1d58ac8f40
Author: Brandon Dunne <[email protected]>
Date:   Tue Oct 10 09:53:29 2017 -0400

    Merge pull request #16151 from lpichler/add_belongsto_filter_for_newtwork_model
    
    Add belongsto filter for other network models
    (cherry picked from commit cf2f4e7b0096844e55f9c87f14085775860deea7)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1465093

…work_model Add belongsto filter for other network models (cherry picked from commit cf2f4e7) https://bugzilla.redhat.com/show_bug.cgi?id=1463422

simaishi · 2017-11-13T21:25:06Z

Fine backport details:

$ git log -1
commit 88c03d39af0ffd49e3de9dce8e837218dd99e511
Author: Brandon Dunne <[email protected]>
Date:   Tue Oct 10 09:53:29 2017 -0400

    Merge pull request #16151 from lpichler/add_belongsto_filter_for_newtwork_model
    
    Add belongsto filter for other network models
    (cherry picked from commit cf2f4e7b0096844e55f9c87f14085775860deea7)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1463422

…for_newtwork_model Add belongsto filter for other network models (cherry picked from commit cf2f4e7) https://bugzilla.redhat.com/show_bug.cgi?id=1463422

Add array for network classes for belongsto filtering

f9fc44f

when we will filter these network models by RBAC we will need apply filter entered in belongsto filter (H & C tab in group setting) - this array will help to handle this situation.

lpichler changed the title ~~Add belongsto filter for other newtwork models~~ Add belongsto filter for other network models Oct 9, 2017

miq-bot added blocker bug core/rbac labels Oct 9, 2017

miq-bot assigned gtanzillo Oct 9, 2017

bdunne requested changes Oct 9, 2017

View reviewed changes

Update condition to work with multiple models

313da9c

lpichler force-pushed the add_belongsto_filter_for_newtwork_model branch from 6df1b54 to daf00cd Compare October 9, 2017 16:31

jrafanie reviewed Oct 9, 2017

View reviewed changes

lpichler force-pushed the add_belongsto_filter_for_newtwork_model branch from daf00cd to d1205f2 Compare October 9, 2017 20:11

lpichler added 9 commits October 9, 2017 22:14

Add rest of classes for belongsto filtering

c34f8e5

Move out context 'network manager is tagged'

ac34136

and put it on the same level as context 'when records match belognsto filter' context 'when records don\'t match belognsto filter'

Add describe block

0e0f1ac

Add intentded 'its' to cycle thru network models

630b727

Remove cases for network models - we have all models in loop now

7946041

Move factories to related context

0d6463c

Add dynamic factories

1445f59

exception is network manager - it is already created in upper block

Restructurize cases

9942c23

we need to add together cases with are using cloud_network and cloud_network_1 models

use double quotas, fix typos

e29a425

lpichler force-pushed the add_belongsto_filter_for_newtwork_model branch from d1205f2 to 881f347 Compare October 9, 2017 20:14

jrafanie reviewed Oct 9, 2017

View reviewed changes

Use models from class Filterer

3fd9feb

lpichler force-pushed the add_belongsto_filter_for_newtwork_model branch from 881f347 to 3fd9feb Compare October 9, 2017 20:59

jrafanie approved these changes Oct 9, 2017

View reviewed changes

bdunne approved these changes Oct 10, 2017

View reviewed changes

bdunne merged commit cf2f4e7 into ManageIQ:master Oct 10, 2017

bdunne assigned bdunne and unassigned gtanzillo Oct 10, 2017

bdunne added this to the Sprint 71 Ending Oct 16, 2017 milestone Oct 10, 2017

lpichler deleted the add_belongsto_filter_for_newtwork_model branch October 10, 2017 14:02

miq-bot added euwe/yes labels Oct 10, 2017

simaishi added euwe/backported and removed euwe/yes labels Oct 23, 2017

simaishi added fine/backported and removed fine/yes labels Nov 13, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add belongsto filter for other network models #16151

Add belongsto filter for other network models #16151

lpichler commented Oct 9, 2017 •

edited

Loading

miq-bot commented Oct 9, 2017

bdunne Oct 9, 2017

lpichler Oct 9, 2017

jrafanie Oct 9, 2017

lpichler Oct 9, 2017

lpichler commented Oct 9, 2017 •

edited

Loading

jrafanie Oct 9, 2017

lpichler Oct 9, 2017

jrafanie Oct 9, 2017

bdunne Oct 9, 2017

lpichler Oct 9, 2017

jrafanie Oct 9, 2017

lpichler Oct 9, 2017

jrafanie Oct 9, 2017

jrafanie Oct 9, 2017

lpichler commented Oct 9, 2017

jrafanie Oct 9, 2017

lpichler Oct 9, 2017

miq-bot commented Oct 9, 2017

jrafanie left a comment

lpichler commented Oct 10, 2017

miq-bot commented Oct 10, 2017

simaishi commented Oct 23, 2017

simaishi commented Nov 13, 2017

Add belongsto filter for other network models #16151

Add belongsto filter for other network models #16151

Conversation

lpichler commented Oct 9, 2017 • edited Loading

Links

miq-bot commented Oct 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lpichler commented Oct 9, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lpichler commented Oct 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miq-bot commented Oct 9, 2017

jrafanie left a comment

Choose a reason for hiding this comment

lpichler commented Oct 10, 2017

miq-bot commented Oct 10, 2017

simaishi commented Oct 23, 2017

simaishi commented Nov 13, 2017

lpichler commented Oct 9, 2017 •

edited

Loading

lpichler commented Oct 9, 2017 •

edited

Loading