Add orchestration stacks to RBAC #18308

alexander-demicev · 2018-12-20T15:02:21Z

Add orchestration stacks to RBAC

https://bugzilla.redhat.com/show_bug.cgi?id=1597125
@miq-bot assign lpichler

kbrock

Thanks.

Hope this helps a little

kbrock · 2018-12-20T16:17:41Z

lib/rbac/filterer.rb

@@ -110,6 +110,7 @@ class Filterer
      'MiqRequest'             => :descendant_ids,
      'MiqRequestTask'         => nil, # tenant only
      'MiqTemplate'            => :ancestor_ids,
+      'OrchestrationStack'     => nil,


Seems templates / generic objects tend to be ancestor_ids,
and core objects tend to be descendant_ids.

I expected this one to be descendant_ids

ok, can we remove this?

a. I think this array is only for tenancy and not cloud tenancy
b. Having a nil in here will not do anything

Also, can you verify that we are not calling into accessible_tenant_ids_strategy for OrchestrationStack?
I'm pretty sure we don't need that

let me make clear those things...

a. I think this array is only for tenancy and not cloud tenancy

It works togethers. We need find ids from tenants and then find their equivalents in cloud tenants.

in filterer:

it goes to scope_to_cloud_tenant

then to the TenancyCommonMixin#tenant_id_clause

and then in TenancyCommonMixin#tenant_id_clause it calls tenant_id_clause_format but from CloudTenancyMixin

b. Having a nil in here will not do anything
so form previous answer (2) it is needed to have nil here - it means "current" strategy (it is considering just tenant from logged user - no ancestors or descendants)

Also, can you verify that we are not calling into accessible_tenant_ids_strategy for OrchestrationStack?
I'm pretty sure we don't need that

Yes I verified it when I was looking for previous answers. accessible_tenant_ids_strategy is called also from OrchestrationStack and it is called here

@kbrock does it make sense now ? If you other questions let me know!

thanks!

spec/lib/rbac/filterer_spec.rb

mansam · 2019-01-09T17:16:16Z

@alexander-demichev can you fix the failing tests?

Loicavenel · 2019-01-28T10:40:17Z

@lpichler can you review this one, thanks

kbrock · 2019-01-30T00:43:00Z

@Loicavenel can you verify that a user should/should not be able to see the orchestration stacks for a parent or a child?

This is the only model that defines the privileges as nil when asking (can you see this resource in parent or child tenants)

The other test seems to describe it differently. It is confusing what the intent is here. I'm thinking something is not right, but @lpichler understands this better than I

lpichler · 2019-01-30T16:42:02Z

I am reviewing ...

lpichler · 2019-01-30T16:54:49Z

spec/lib/rbac/filterer_spec.rb

    let(:c_t_flavor_other)      { FactoryBot.create(:cloud_tenant_flavor, :cloud_tenant => cloud_tenant_other, :flavor => flavor_other) }
-    let!(:all_objects)          { [project1_volume, project2_volume, volume_other, cloud_tenant_other, project1_c_t_flavor, project2_c_t_flavor, c_t_flavor_other] }
+    let!(:all_objects)          { [project1_volume, project2_volume, volume_other, cloud_tenant_other, project1_c_t_flavor, project2_c_t_flavor, c_t_flavor_other, project1_orchestration_stack, project2_orchestration_stack, orchestration_stack_other] }

    it "lists its own project's objects and other objects where tenant_mapping is not enabled" do


@alexander-demichev "....is enabled " ... ?

in that change, please also update this comment

@aufi @alexander-demichev can you change it please ?

lpichler · 2019-01-31T16:03:45Z

spec/lib/rbac/filterer_spec.rb

@@ -2540,6 +2552,15 @@ def get_rbac_results_for_and_expect_objects(klass, expected_objects)

      results = described_class.search(:class => Flavor, :user => other_user).first
      expect(results).to match_array [project1_flavor, project2_flavor, flavor_other]
+
+      results = described_class.search(:class => OrchestrationStack, :user => project1_user).first
+      expect(results).to match_array [project1_orchestration_stack, project2_orchestration_stack, orchestration_stack_other]


if we are querying all OrchestrationStacks so here can be
OrchestrationStack.all instead of [project1_orchestration_stack, project2_orchestration_stack, orchestration_stack_other]

I guess it is more visible that it is not affecting anything if OpenStack mapping is disabled

lpichler · 2019-01-31T16:42:08Z

openstack mapping is enabled?

My Company 
-> T1  (tied with user UT1 thru grp GT1 )
-> OSP Provider Tenant 
    -> OT1 (tied with user UOT1 thru grp GOT1, tied with orch. stack OS1 with cloud tenant 1 : OS_CL1 )
        -> OT2 (tied with user UOT2 thru grp GOT2, tied with orch. stack OS2 with cloud tenant 2 : OS_CL2 )
    -> OT3 (tied with user UOT3 thru grp GOT3,  tied with orch. stack OS3 with cloud tenant 3 : OS_CL3 )

strategy is nil, it means Orch.Stacks from current tenant.

Those case comes to my mind:

questions:

logged user UT1 should see which Orch.Stacks ?
??
logged user UOT1 should see which Orch.Stacks ?
just OS1 ?
logged user UOT2 should see which Orch.Stacks ?
just OS2 ?
logged user UOT3 should see which Orch.Stacks ?
just OS3 ?

@alexander-demichev
it will be needed to covered also in specs but let's clear out the questions + explation about other_XX

kbrock · 2019-03-06T20:02:13Z

I feel that when tenant mapping is disabled, we should still respect some form of tenancy.
I understand us using the web tenants that come from the provider.
I had thought that flag meant we are mapping the cloud tenants to our tenants.

But right now, the tests are saying that the flag means "skip all tenancy".
This seems very wrong, but it is not my area of expertise.

@agrare Who is the business person who can define cloud_ems and tenant_mapping?

agrare · 2019-03-07T21:28:08Z

@gtanzillo knows more about tenancy than I do, hopefully he can help 🤞

Loicavenel · 2019-03-07T22:39:39Z

We did implement tenancy in few steps, first it was Instances only and then network and storage.. Stack was most likely left over... This is when using Tenant Mapping between our tenancy and OpensTack tenancy, correct?

lpichler · 2019-03-08T06:51:53Z

@kbrock

But right now, the tests are saying that the flag means "skip all tenancy".
This seems very wrong, but it is not my area of expertise.

let me check, it but if tenant mapping is disabled we don't have a way how to do "tenancy" and basically tenant mapping is way how to do tenancy for "cloud" models(Stacks, CloudSubnet,... ).

.. This is when using Tenant Mapping between our tenancy and OpenStack tenancy, correct?

yes exactly, nowadays it is only for OpenStack tenancy

Loicavenel · 2019-03-08T07:59:56Z

@lpichler one day, we should list all Objects that can be Tenant related.

aufi · 2019-06-26T13:45:21Z

@lpichler Hi Libor, is there something needed to be updated before merging? (Alex is on PTO, so asking there, thanks)

aufi · 2019-07-02T11:49:16Z

openstack tenant mapping is enabled?

Yes, otherwise there is no filtering and CF user can see the same inventory as OpenStack admin user entered during adding OSP provider credentials.

Tenant mapping allows filtering inventory for Users synced from OSP to CF based on its project/tenant (in CF modelled as CloudTenant; that can be linked to a CF Tenant which is used for filtering)

My Company
-> T1 (tied with user UT1 thru grp GT1 )
-> OSP Provider Tenant
-> OT1 (tied with user UOT1 thru grp GOT1, tied with orch. stack OS1 with cloud tenant 1 : OS_CL1 )
-> OT2 (tied with user UOT2 thru grp GOT2, tied with orch. stack OS2 with cloud tenant 2 : OS_CL2 )
-> OT3 (tied with user UOT3 thru grp GOT3, tied with orch. stack OS3 with cloud tenant 3 : OS_CL3 )
strategy is nil, it means Orch.Stacks from current tenant.

Those case comes to my mind:

questions:

1. logged user UT1  should see which Orch.Stacks ?
   ??

If UT1 is not created by Tenant Mapping feature, there is no change/no filtering. Otherwise it depends on CloudTenant which is linked to UT1 and User used to login to CF.

logged user UOT1 should see which Orch.Stacks ?
just OS1 ?

logged user UOT2 should see which Orch.Stacks ?
just OS2 ?

logged user UOT3 should see which Orch.Stacks ?
just OS3 ?

All yes - see only objects within own tenant. An access inheritance is off by default in OpenStack (it can be set, but that would bring quite lot of complexity IMO)

@alexander-demichev
it will be needed to covered also in specs but let's clear out the questions + explation about other_XX

Feel free correct me!

kbrock · 2019-07-03T15:42:56Z

Does ServiceTemplate even need TenancyMixin?

I'm having trouble having a model that supports (non cloud) tenancy with an option that says ignore tenancy.

lpichler · 2019-07-11T14:43:43Z

@aufi Thank you for your response and reviewed it again it looks good to me, I am considering that all cases are covered as I mentioned in comment

@kbrock Did you mean other model than ServiceTemplate (there is nothing about ServiceTemplate) or I am not sure how ServiceTemplate is related.

I'm having trouble having a model that supports (non cloud) tenancy with an option that says ignore tenancy.

I am not sure I understand this concern correctly - it is not about option with saying "ignore tenancy" Those changes are adding tenancy control for cloud object: OrchestrationStack.

We started this effort in #14036 and this PR just extends it about OrchestrationStack.

Anyway as this is related to RBAC any concern is very welcomed :)

lpichler

👍

lpichler · 2019-07-11T15:04:41Z

@miq-bot assign @gtanzillo or @kbrock :)

miq-bot · 2019-07-11T15:05:31Z

@lpichler 'gtanzillo or kbrock :)' is an invalid assignee, ignoring...

kbrock · 2019-07-12T16:54:33Z

@lpichler ugh. yea, does OrchestrationStack even need TenancyMixin?

lpichler · 2019-07-12T17:55:28Z

@kbrock No,OrchestrationStack just need CloudTenancyMixin which implements tenancy logic (let's say "Cloud tenancy") thru CloudTenants and it uses TenancyCommonMixin.
Classic TenancyMixin also uses TenancyCommonMixin where is logic which pulled out from original TenancyMixin - it is done here.

kbrock · 2019-07-12T18:37:06Z

Aah, you are right:

OrchestrationStack.respond_to?(:scope_by_tenant?)
# => false

kbrock · 2019-07-12T19:17:43Z

I'm good to merge after those 2 proposed changes

miq-bot · 2019-07-16T07:45:53Z

Checked commit alexander-demicev@0719af3 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
3 files checked, 0 offenses detected
Everything looks fine. 🏆

lpichler · 2019-07-16T08:45:04Z

I'm good to merge after those 2 proposed changes

so change here is done and this was explained.
@kbrock are you ok with merging now ?

thanks!

aufi · 2019-07-18T08:15:35Z

Thanks 👍

kbrock · 2019-07-18T20:36:13Z

@aufi thank you. sorry for all the red tape

miq-bot assigned lpichler Dec 20, 2018

kbrock added lifecycle/orchestration core/rbac labels Dec 20, 2018

kbrock reviewed Dec 20, 2018

View reviewed changes

alexander-demicev force-pushed the add-stacks-to-rbac branch from 8564952 to c5b3fc1 Compare December 21, 2018 09:57

alexander-demicev force-pushed the add-stacks-to-rbac branch from c5b3fc1 to 8c9fed4 Compare January 10, 2019 13:18

alexander-demicev force-pushed the add-stacks-to-rbac branch from 8c9fed4 to ac0bae4 Compare January 24, 2019 14:39

lpichler reviewed Jan 30, 2019

View reviewed changes

lpichler reviewed Jan 31, 2019

View reviewed changes

alexander-demicev force-pushed the add-stacks-to-rbac branch from ac0bae4 to bebf685 Compare February 11, 2019 13:05

lpichler approved these changes Jul 11, 2019

View reviewed changes

Add orchestration stacks to RBAC

0719af3

alexander-demicev force-pushed the add-stacks-to-rbac branch from bebf685 to 0719af3 Compare July 16, 2019 07:40

kbrock added the hammer/no label Jul 17, 2019

kbrock merged commit a3f822a into ManageIQ:master Jul 17, 2019

kbrock added this to the Sprint 116 Ending Jul 22, 2019 milestone Jul 17, 2019

kbrock assigned kbrock and unassigned lpichler Jul 17, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add orchestration stacks to RBAC #18308

Add orchestration stacks to RBAC #18308

alexander-demicev commented Dec 20, 2018

kbrock left a comment

kbrock Dec 20, 2018

kbrock Jul 12, 2019

lpichler Jul 15, 2019

mansam commented Jan 9, 2019

Loicavenel commented Jan 28, 2019

kbrock commented Jan 30, 2019 •

edited

Loading

lpichler commented Jan 30, 2019

lpichler Jan 30, 2019

kbrock Jul 12, 2019

lpichler Jul 15, 2019

lpichler Jan 31, 2019 •

edited

Loading

lpichler commented Jan 31, 2019

kbrock commented Mar 6, 2019

agrare commented Mar 7, 2019

Loicavenel commented Mar 7, 2019

lpichler commented Mar 8, 2019

Loicavenel commented Mar 8, 2019

aufi commented Jun 26, 2019

aufi commented Jul 2, 2019

kbrock commented Jul 3, 2019

lpichler commented Jul 11, 2019

lpichler left a comment

lpichler commented Jul 11, 2019

miq-bot commented Jul 11, 2019

kbrock commented Jul 12, 2019

lpichler commented Jul 12, 2019

kbrock commented Jul 12, 2019

kbrock commented Jul 12, 2019

miq-bot commented Jul 16, 2019

lpichler commented Jul 16, 2019

aufi commented Jul 18, 2019

kbrock commented Jul 18, 2019

Add orchestration stacks to RBAC #18308

Add orchestration stacks to RBAC #18308

Conversation

alexander-demicev commented Dec 20, 2018

kbrock left a comment

Choose a reason for hiding this comment

kbrock Dec 20, 2018

Choose a reason for hiding this comment

kbrock Jul 12, 2019

Choose a reason for hiding this comment

lpichler Jul 15, 2019

Choose a reason for hiding this comment

mansam commented Jan 9, 2019

Loicavenel commented Jan 28, 2019

kbrock commented Jan 30, 2019 • edited Loading

lpichler commented Jan 30, 2019

lpichler Jan 30, 2019

Choose a reason for hiding this comment

kbrock Jul 12, 2019

Choose a reason for hiding this comment

lpichler Jul 15, 2019

Choose a reason for hiding this comment

lpichler Jan 31, 2019 • edited Loading

Choose a reason for hiding this comment

lpichler commented Jan 31, 2019

kbrock commented Mar 6, 2019

agrare commented Mar 7, 2019

Loicavenel commented Mar 7, 2019

lpichler commented Mar 8, 2019

Loicavenel commented Mar 8, 2019

aufi commented Jun 26, 2019

aufi commented Jul 2, 2019

kbrock commented Jul 3, 2019

lpichler commented Jul 11, 2019

lpichler left a comment

Choose a reason for hiding this comment

lpichler commented Jul 11, 2019

miq-bot commented Jul 11, 2019

kbrock commented Jul 12, 2019

lpichler commented Jul 12, 2019

kbrock commented Jul 12, 2019

kbrock commented Jul 12, 2019

miq-bot commented Jul 16, 2019

lpichler commented Jul 16, 2019

aufi commented Jul 18, 2019

kbrock commented Jul 18, 2019

kbrock commented Jan 30, 2019 •

edited

Loading

lpichler Jan 31, 2019 •

edited

Loading