Skip to content

Conversation

@kbrock
Copy link
Member

@kbrock kbrock commented Jan 19, 2024

This adds tenant filtering to Switches

depends upon:

see also:

aside

Unfortunately, switches do not have a tenant_id
so we needed to add a little extra code to find the tenant through the ems (which is through the host)

Switches were not filtering on tenant
Unfortunately, switches do not have a tenant_id
so we needed to add a little extra code to find the tenant through the ems
(which is through the host... sorry)
@miq-bot
Copy link
Member

miq-bot commented Jan 19, 2024

Checked commit kbrock@497aa9c with ruby 2.7.8, rubocop 1.56.3, haml-lint 0.51.0, and yamllint
3 files checked, 0 offenses detected
Everything looks fine. 🍰

Copy link
Member

@Fryguy Fryguy left a comment

Choose a reason for hiding this comment

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

LGTM, but since switches are also scoped to hosts when coming from the other direction, I'm not sure if this handles both directions, since the tests only test with a VM. It should, since the tenancy scoping is on the EMS which is shared.

Would like @agrare also to review.

@Fryguy
Copy link
Member

Fryguy commented Jan 23, 2024

This change looks good for something we'd backport to morohy, but moving forward we should make it work the way the other TenancyMixin'd models work, which is by adding a tenant_id column, and then just using TenancyMixin. This also eliminates the changes to rbac filterer.

@Fryguy Fryguy self-assigned this Jan 23, 2024
@Fryguy Fryguy merged commit 758d157 into ManageIQ:master Jan 23, 2024
@Fryguy
Copy link
Member

Fryguy commented Jan 23, 2024

@kbrock A conflict occurred during the backport of this pull request to morphy.

If this pull request is based on another pull request that has not been marked for backport, add the appropriate labels to the other pull request. Otherwise, please create a new pull request direct to the morphy branch in order to resolve this.

Conflict details:

diff --cc lib/rbac/filterer.rb
index 7720299ea7,7f98250ee4..0000000000
--- a/lib/rbac/filterer.rb
+++ b/lib/rbac/filterer.rb
@@@ -139,6 -152,10 +139,13 @@@ module Rba
        'Service'                => :descendant_ids,
        'ServiceTemplate'        => :ancestor_ids,
        'ServiceTemplateCatalog' => :ancestor_ids,
++<<<<<<< HEAD
++=======
+       'SecurityGroup'          => :descendant_ids,
+       'SecurityPolicy'         => :descendant_ids,
+       'SecurityPolicyRule'     => :descendant_ids,
+       'Switch'                 => :descendant_ids,
++>>>>>>> 758d157711 (Merge pull request #22843 from kbrock/rbac_switches)
        'Tenant'                 => :descendant_ids,
        'User'                   => :descendant_ids,
        'Vm'                     => :descendant_ids
diff --cc spec/lib/rbac/filterer_spec.rb
index 68af23b544,cf9b908103..0000000000
--- a/spec/lib/rbac/filterer_spec.rb
+++ b/spec/lib/rbac/filterer_spec.rb
@@@ -832,11 -836,12 +835,18 @@@ RSpec.describe Rbac::Filterer d
          end
        end
  
++<<<<<<< HEAD
 +      context "with accessible_tenant_ids filtering (strategy = :parent_ids)" do
 +        it "can see parent tenant's EMS" do
 +          ems = FactoryBot.create(:ems_vmware, :tenant => owner_tenant)
++=======
+       context "with accessible_tenant_ids filtering (strategy = :ancestor_ids)" do
+         it "can see parent and own tenant's EMS" do
+           owned_ems
+           child_ems
++>>>>>>> 758d157711 (Merge pull request #22843 from kbrock/rbac_switches)
            results = described_class.search(:class => "ExtManagementSystem", :miq_group => child_group).first
-           expect(results).to match_array [ems]
+           expect(results).to match_array [owned_ems, child_ems]
          end
  
          it "can't see descendant tenant's EMS" do

@Fryguy
Copy link
Member

Fryguy commented Jan 24, 2024

Manual backport tp morphy via #22851

@Fryguy
Copy link
Member

Fryguy commented Jan 31, 2024

Backported to quinteros in commit 6caf162.

commit 6caf162974107fc38bb0616e1be4593a1572d093
Author: Jason Frey <[email protected]>
Date:   Tue Jan 23 14:00:49 2024 -0500

    Merge pull request #22843 from kbrock/rbac_switches
    
    Add tenancy scoping to Switches
    
    (cherry picked from commit 758d1577112c22057df1808f1b34e3fe7fe52a22)

Fryguy added a commit that referenced this pull request Jan 31, 2024
Add tenancy scoping to Switches

(cherry picked from commit 758d157)
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.

4 participants