Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2021-11-30
18:08:53 gmann dansmith: ok, checking
18:08:54 dansmith I wonder if we're not really testing fake policy like we think in functional?
18:09:47 gmann dansmith: which is good :). I wanted to remove those fake policy from unit as well as from functional tests completely but that might need more tests modification but something we should do.
18:10:29 dansmith don't disagree that it would be better, I just don't know why this is required right now,
18:10:40 dansmith since I would expect us to at most be testing with old defaults OR'd in
18:11:13 gmann sure, I will check it where we are missing things.
18:11:46 gmann dansmith: did you see my comment https://review.opendev.org/c/openstack/nova/+/816206/9/nova/policies/base.py
18:12:12 gmann dansmith: if CONTEXT_ADMIN if more readable then it is fine otherwise we can ad ADMIN ?
18:12:27 dansmith gmann: oh yeah I did, I just got distracted by the functional failures
18:12:59 gmann I started converting SYSTEM_READER to SYSTEM_ADMIN on top of your patch so doing it in base patch will avoid rebase or so
18:13:00 gmann sure
18:57:52 gmann dansmith: this is reason for functional test failure https://review.opendev.org/c/openstack/nova/+/816206/comment/510a59e0_5ffbc2f5/
18:58:12 gmann dansmith: functional test using the real policy helped us to capture it.
19:00:04 dansmith gmann: ahh, I was probably conflating that rule with the admin_or_owner below it when thinking that we'd still have the old default
19:01:40 gmann dansmith: yeah, and those role hierarchy fix in 819907 made test passing because of 'admin' being used as user-id for functional test https://review.opendev.org/c/openstack/nova/+/819907/comment/eba56840_d585da0a/
19:02:17 dansmith gmann: you mean that's why they worked before I switched the rule...
19:03:14 dansmith gmann: should I use admin_or_owner for those flavor-extra-spec rules, or add the DEPRECATED_ADMIN_OR_OWNER to context_admin/
19:05:22 gmann dansmith: I think DEPRECATED_ADMIN_POLICY as CONTEXT_ADMIN is going to replace SYSTEM_ADMIN only . I was trying to do it this way https://review.opendev.org/c/openstack/nova/+/819389
19:06:07 gmann and PROJECT_ADMIN going to be with DEPRECATED_ADMIN_OR_OWNER which has project_id in that
19:08:22 gmann dansmith: let me update my patch and then you can use the new ADMIN rule for place of role:admin
19:08:56 dansmith gmann: ack
19:09:06 dansmith gmann: we still want my patch to pass proper roles from the fixture though right?
19:09:32 dansmith presumably we need to also let you get a fixture with no (or foo) roles for testing that member is enforced
19:14:38 gmann dansmith: for is_admin L1092 yeah it is ok but else part make reader also give member authority https://review.opendev.org/c/openstack/nova/+/819907/1/nova/tests/fixtures/nova.py#1092
19:15:22 gmann if we remove the else part and let real role being tested what test use then it should be ok
19:15:37 dansmith gmann: well, it just assumes member if not admin, which all the current users of that fixture are going to expect.. I'm saying it needs to be extended to allow you to tell it what roles you want
19:15:43 gmann and later we can add user_id as 'member', 'reader' etc and add role accodringl;y
19:15:48 dansmith gmann: otherwise non-admin stuff will all fail too because roles=[] can't do aything
19:16:02 gmann yeah that is true
19:16:36 dansmith if we fix the deprecated rule bit, then this doesn't matter just yet, but before we can test with deprecated rules disabled, we'll have to fix this I think
19:17:51 gmann dansmith: right for No legacy tests we need that. and we can add we can add user_id == 'member' and add only member role and else part as add reader
19:18:05 dansmith sure, or just pass roles= to the fixture
19:18:27 dansmith right now, the user that is used is something other than "member" so we probably need a bunch of changes for that first
19:18:57 gmann +1, much better
19:30:41 opendevreview Dan Smith proposed openstack/nova master: Make API fixture pass roles https://review.opendev.org/c/openstack/nova/+/819907
19:30:47 dansmith gmann: like this-ish ^
19:33:45 gmann dansmith: +1 yeah, and you want default to reader only (lower level of access) ? here https://review.opendev.org/c/openstack/nova/+/819907/2/nova/tests/functional/api/client.py#140
19:34:30 dansmith gmann: that defaults to member, not reader.. since that's what everything else would expect currently, it seemed like that would be the best default
19:34:54 dansmith looks like maybe it's only used in one other place though,
19:35:04 dansmith so I guess I could default it even lower and fix that one too
19:35:32 dansmith default should be roles=[] right? since that's what you get if you're not explicitly granted reader/member/admin on a project
19:35:40 gmann dansmith: yeah. but if it need mroe test fix then we can leave as of now and later we can do while moving func tests also to reader/mem,ber
19:35:43 dansmith oh nm, it's just used here
19:37:41 opendevreview Dan Smith proposed openstack/nova master: Make API fixture pass roles https://review.opendev.org/c/openstack/nova/+/819907
19:37:51 dansmith I thought client was used a few other places directly, but it's not, so we should be good to default to roles=[] ^
19:39:04 gmann +1. yeah its self.api and self.admin_api
19:39:06 gmann as of now
19:39:19 dansmith yup
19:39:27 sean-k-mooney well default to roles=None
19:39:39 sean-k-mooney rather then []
19:39:40 opendevreview Ghanshyam proposed openstack/nova master: Introduce 'admin' policy base rule https://review.opendev.org/c/openstack/nova/+/819389
19:39:49 sean-k-mooney sicne you should not use mutable defaults but same effect
19:39:56 gmann dansmith: ^^ this is for 'admin' basically renaming 'context_is_admin' to 'admin'
19:40:02 dansmith sean-k-mooney: did you look at the patch? :)
19:40:11 dansmith gmann: cool thanks
19:40:24 sean-k-mooney nope just saw the converstaion scroll by
19:40:43 dansmith sean-k-mooney: trying not to take offense that you think I don't know not to use mutable defaults :)
19:41:50 sean-k-mooney :)
19:42:58 sean-k-mooney i still see it in code often enough but ya you already did it the right way https://review.opendev.org/c/openstack/nova/+/819907/3/nova/tests/functional/api/client.py#140
19:49:52 gmann I think we have hacking rule for that.
20:13:37 dansmith gmann: I don't think your admin patch works for me by itself,
20:13:44 dansmith since it's still requiring role:admin, which isn't what we have today
20:13:56 dansmith it needs to be is_admin in order to work as-is right?
20:24:52 gmann dansmith: context set is_admin based on this rule itself https://github.com/openstack/nova/blob/d630615a02469442fb50ed4aa7e092206a28166a/nova/context.py#L138
20:24:58 gmann https://review.opendev.org/c/openstack/nova/+/819389/4/nova/policy.py
20:29:16 gmann but as it is two level deprecated rule combined I hope each one is logical ORed by oslo policy. but is it failing on your patch? if so then we can avoid to rename it for now. and add DEPRECATED_ADMIN_POLICY in is_context_admin rule only
20:39:49 gmann dansmith: yeah, i ran your patch with my change and it does not work. oslo policy add only one level of deprecated rule in logical OR.
20:40:56 gmann dansmith: I will abandon my patch and let's rename CONTEXT_ADMIN to ADMIN in your patch but keeping rule:is_context_admin. commented in https://review.opendev.org/c/openstack/nova/+/816206/comment/543216d1_15fecfb1/
21:43:47 dansmith gmann: okay, but that's not enough, it needs to be "role:admin or is_admin:True"
21:44:21 dansmith so do you want me to do that on context_is_admin with a deprecated_rule= or add it to admin_api?
21:44:51 gmann dansmith: yeah that will add ORed both
21:45:59 gmann dansmith: yeah deprecated_rule=DEPRECATED_ADMIN_POLICY .
21:46:13 dansmith okay that's not enough either, I need this: https://termbin.com/bnqh
21:46:15 dansmith you okay with that?
21:46:27 dansmith oh wait
21:46:37 dansmith it's enough to use it as-is for today, you're right
21:46:45 dansmith I'm thinking this needs to work for both old and new, but not as of this patch
21:46:58 dansmith so yeah, I will just add the deprecated_admin_policy there
21:47:12 gmann if you do this then it should ADMIN = 'rule:context_is_admin'
21:47:58 dansmith yep, hadn't saved that before paste :)
21:48:31 dansmith running all the tests now, but expect this to work now
21:48:40 gmann dansmith: so with that you do not need this right? 'check_str='is_admin:True or role:admin','
21:48:52 dansmith correct, for just this patch, I don't
21:48:59 gmann yeah.
21:50:40 dansmith there are just too many balls in the air with this stuff for my tiny brain :D
23:06:26 artom dansmith, for the record, I'm painfully aware of the door that your last line opened, but am steadfastly refusing to step through it
23:06:54 dansmith artom: lol, I *literally* almost pinged you, asking if you wanted to do the mriedem thing in his stead :D
23:07:48 artom I'm all grown up now
23:08:33 dansmith riiight
23:09:09 artom No really, I'm organizing expensive calls and everything :P
23:37:08 opendevreview Dan Smith proposed openstack/nova master: Revert project-specific APIs for servers https://review.opendev.org/c/openstack/nova/+/816206
23:37:08 opendevreview Dan Smith proposed openstack/nova master: Allow per-context rule in error messages https://review.opendev.org/c/openstack/nova/+/816865
23:42:42 opendevreview Ghanshyam proposed openstack/nova master: Convert SYSTEM_ADMIN|READER to Admin and system scope https://review.opendev.org/c/openstack/nova/+/819390
23:44:28 opendevreview Ghanshyam proposed openstack/nova master: Convert SYSTEM_ADMIN|READER to Admin and system scope https://review.opendev.org/c/openstack/nova/+/819390
23:44:33 opendevreview Ghanshyam proposed openstack/nova master: Convert SYSTEM_ADMIN|READER to Admin and system scope https://review.opendev.org/c/openstack/nova/+/819390
#openstack-nova - 2021-12-01
02:29:47 opendevreview Ghanshyam proposed openstack/nova master: Convert SYSTEM_ADMIN|READER to Admin and system scope https://review.opendev.org/c/openstack/nova/+/819390
08:42:49 luk4s Good morning all
08:44:07 luk4s We enabled tenant isolation with placement https://docs.openstack.org/nova/wallaby/admin/aggregates.html#tenant-isolation-with-placement and have two aggregates: one for private tenants and one for generic use.
08:45:55 luk4s For private tenants we want the allocations to happened first on the private aggregate and when that is full we want to move on to the generic one once the private one is full.
08:46:14 luk4s Is this possible?

Earlier   Later