| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-03-31 | |||
| 21:51:18 | bnemec | There is. You explicitly set the new rule in your policy file and the deprecation warning goes away. | |
| 21:51:30 | dansmith | bnemec: but we've spent years telling people NOT to do that | |
| 21:51:39 | gmann | bnemec: but that is not expected | |
| 21:51:47 | gmann | yeah what dansmith mentioned | |
| 21:51:48 | melwitt | yeah ... that seems backwards. shouldn't default mean an empty policy file? | |
| 21:51:54 | dansmith | yes | |
| 21:51:55 | bnemec | It's essentially the operator saying, yes, we've looked at this change and it's fine. | |
| 21:52:04 | melwitt | that's what we made a big deal about policy in code back when that was added | |
| 21:52:11 | gmann | true | |
| 21:52:13 | dansmith | copying defaults into the policy file to shut it the eff up moves us backwards many years | |
| 21:52:39 | melwitt | not sure I'd want to confuse users by saying "ok now go set every single rule in your policy file" that seems counterintuitive | |
| 21:53:08 | melwitt | I would imagine their takeaway from that is we've removed policy-in-code and we're back to the old way | |
| 21:53:51 | dansmith | right, because they won't go to the trouble of understanding why, | |
| 21:54:02 | dansmith | they will just hear that is the solution to make it stop warning | |
| 21:54:03 | bnemec | I will say I don't think the policy deprecation mechanism was designed for deprecating the entire policy file at once, like we're doing now. | |
| 21:54:42 | melwitt | yes, I agree this is not the usual thing | |
| 21:54:50 | gmann | yeah, in this case where all defaults are changing warning is too much. | |
| 21:55:00 | bnemec | Also, the redundant rule tool was designed to let them clean up their policy files once the deprecation process is over. | |
| 21:55:00 | gmann | for one or two policy change and name etc then it is fine | |
| 21:55:13 | bnemec | It will tell them that they have rules in their policy file that don't need to be there anymore. | |
| 21:55:36 | gmann | yup that wanring make sense. | |
| 21:56:12 | bnemec | I wonder if we should make that a runtime check instead of a separate tool. | |
| 21:56:13 | gmann | if rule name changing | |
| 21:57:14 | bnemec | He knows way more about why things work the way they do than I do. | |
| 22:04:09 | melwitt | taking some steps back, I feel like we have changed the defaults for a reason right, that we think they are better defaults. when I say new defaults become "active" I mean when they become the only default, when we're no longer OR'ing them. I feel like maybe we don't need to sound an alarm about it, but rather let the notice in the release notes explain the upcoming change and how to dump the new defaults to review. that is our normal | |
| 22:04:09 | melwitt | way to communicate upcoming changes to our operators | |
| 22:22:09 | openstackgerrit | melanie witt proposed openstack/nova master: Add info about affinity requests to the troubleshooting doc https://review.opendev.org/715092 | |
| 22:30:59 | sean-k-mooney | dansmith: if you are about can you read the question i left in https://review.opendev.org/#/c/715326/4. basicaily im wondering if i should be binding the arqs in teh conductor during evacuate or on the destinaiton node. and if the conducrot are we ok with the rpc change that will required like we did in build_and_run_instance | |
| 22:31:30 | sean-k-mooney | dansmith: im done for the day so no rush but ill try and rework that tomorrow | |
| 23:18:19 | gmann | melwitt: dansmith lgtm. I will propose the disable warnings when I cut the releasenotes and doc for new defautls. | |
| 23:18:59 | gmann | i think gibi planning this in ussuri highlights also which also good enough signals | |
| 23:26:05 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Pass the actual target in limits policy https://review.opendev.org/715761 | |
| #openstack-nova - 2020-04-01 | |||
| 00:14:32 | openstackgerrit | Merged openstack/nova master: Add test coverage of existing hypervisors policies https://review.opendev.org/715029 | |
| 00:18:58 | brinzhang_ | gibi, dansmith: I have reviewed your talking, and saw comment in the latest patch, I will update the update_volume_attachment_v285 to a new schema as raw from gmann suggestion | |
| 00:24:49 | brinzhang_ | dansmith: I will update the comments left by gmann to the latest patch. | |
| 00:25:35 | brinzhang_ | dansmith: I will add you to the core-author, thanks | |
| 00:27:10 | lbragstad_ | we decided to OR deprecated policies with the new defaults so that we could allow for a smoother migration to the new policy enforcement concept | |
| 00:28:41 | lbragstad | we felt compelled to add deprecation warnings so that operators would know if/when a user's permissions adhered to the old policy concepts, and not the new way | |
| 00:30:14 | lbragstad | ideally, the operator would take that as an action to either adjust the users permissions to fit the new concept (oh, they're an operator they need the 'admin' role on the system) or evaluate if that user really needed that authorization | |
| 00:32:24 | lbragstad | but, i can totally empathize with warning overload and i can understand how that might teach users the wrong thing by being overly noisy | |
| 00:38:35 | lbragstad | maybe we could introduce a simpler way of opting into the new concept, instead of telling people to write policies back into a file for warnings to go away (only to remove them later if they're fine with the new defaults) | |
| 00:44:03 | gmann | lbragstad: yeah, i am thinking to add warning disable flag per rule in DocumentedRuleDefault so that we can 1. skip the common default change policy to log warning 2. but at same time rule name change keep adding warning. | |
| 00:44:42 | gmann | i mean instead of global flag, support per rule flag and policy implementor decide which rule they want to add warning. | |
| 00:45:30 | gmann | because if rule name is changed and rule is in policy file we should add warning. | |
| 00:45:45 | gmann | that is case in nova case when granularity is added to adopt the new defaults | |
| 01:01:14 | openstackgerrit | Brin Zhang proposed openstack/nova master: Allow PUT volume attachments API to modify delete_on_termination https://review.opendev.org/693828 | |
| 01:01:52 | brinzhang_ | dansmith: gmann: https://review.opendev.org/#/c/693828/ addressed your comments | |
| 01:06:09 | lbragstad | gmann warnings for policy removal/renames make sense | |
| 01:07:02 | lbragstad | that functionality could also be incorporated into another oslo.policy cli tool (if it isn't already?) | |
| 01:14:19 | gmann | lbragstad: or we can suppress warning based on global flags (new flag) but policy renaming always log warning . keeping this one always and rest all based on flag - https://github.com/openstack/oslo.policy/blob/c483dee1f306b698448fbee9169038159209e916/oslo_policy/policy.py#L670 | |
| 01:14:55 | gmann | brinzhang_: thanks i will check | |
| 01:15:26 | lbragstad | gmann yeah - that makes sense | |
| 01:15:41 | brinzhang_ | gmann: what do you thinf of this change? https://review.opendev.org/#/c/693828/23/nova/api/openstack/compute/volumes.py@495 | |
| 01:15:55 | lbragstad | gmann how much policy work does nova have left? | |
| 01:16:18 | lbragstad | the reviews have been on my list, but i clearly haven't gotten around to them | |
| 01:17:24 | brinzhang_ | update an attachments not swap, we will update the attachment firstly, then check the attachment_id !=volumeId, then do swap, that will all admin_or_owner firstly, and then do admin policy | |
| 01:17:25 | gmann | lbragstad: i think ~18 policy are left | |
| 01:17:34 | lbragstad | total?! | |
| 01:17:45 | gmann | counting :) | |
| 01:18:06 | lbragstad | wow - nice work, gmann | |
| 01:18:31 | gmann | lbragstad: around 50 | |
| 01:18:50 | brinzhang_ | dansmith, gmann: this looks so strange, is it? | |
| 01:19:33 | gmann | lbragstad: let me propose warning thing in this week and you can check if that make sense especially from other project point of view | |
| 01:20:10 | lbragstad | gmann ok - do we want to think about a different way for people to opt into the new concept, though? | |
| 01:20:49 | gmann | lbragstad: did not get ? | |
| 01:21:50 | lbragstad | today, if i upgrade to Ussuri and nova has all these new defaults, i need to override all of them to opt into the new way of doing things | |
| 01:21:57 | gmann | brinzhang_: you mean we are going to 403 late if non-admin try to update_swap ? | |
| 01:22:30 | lbragstad | gmann do we want to make that easier by not having operators write policies into a file | |
| 01:22:37 | brinzhang_ | gmann: 403? | |
| 01:22:50 | lbragstad | but still allow them to opt into the new system | |
| 01:23:12 | lbragstad | like what dansmith and melwitt were saying earlier | |
| 01:23:32 | gmann | lbragstad: ohk. they do not need to write policy. if they enable scope and add new defaults in their system then they adopt the new policy | |
| 01:23:55 | gmann | or you are saying new things only and no old things supported ? | |
| 01:24:37 | lbragstad | let's say i want to adopt this right away and i don't want policies OR'd | |
| 01:24:53 | gmann | brinzhang_: i mean policy unauthorize error for non-admin doing update + swap. | |
| 01:25:08 | lbragstad | that requires me (as the operator) to go and update my nova policy file to override all the policies with their new defaults, right? | |
| 01:25:09 | brinzhang_ | gmann: yes | |
| 01:25:36 | gmann | yeah if they want to stop old tokens/role then yes they need to override | |
| 01:25:40 | brinzhang_ | gmann: maybe we should move L494 to L497, what do you think? | |
| 01:25:49 | gmann | but in nova case it is easy as we are controlling those via common base rules | |
| 01:26:46 | gmann | lbragstad: we are testing that simulation also- basically operator needs to do this - https://github.com/openstack/nova/blob/c31de903dd73de1c0a41bcfe4c867daa6eb10f85/nova/tests/unit/policies/base.py#L103 | |
| 01:28:03 | gmann | brinzhang_: we cannot do that becasue swap operation is async and might not be completed before we update deleet flag. | |
| 01:28:40 | gmann | brinzhang_: we can move the both policy checks in starting. i mean check id !=volume_id then check swap policy also. | |
| 01:30:22 | lbragstad | gmann i wonder if we could come up with a way to opt in without having to write to the policy file (after reading the scrollback, i can see how that might be misleading to operators if we've told them not to do that in the past) | |
| 01:30:24 | gmann | brinzhang_: anyways that part we can do in https://review.opendev.org/#/c/711194/10 | |
| 01:31:42 | gmann | lbragstad: i see your point. how about doing it via enforce_scope ? if it is true along with scope enable we remove the ORing old rules ? | |
| 01:32:10 | lbragstad | gmann yeah - i think that would work | |
| 01:32:34 | gmann | basically enforce_scope flag is new policy though we kept new defaults roles separate but still someone want to adopt both at same time | |
| 01:33:15 | brinzhang_ | gmann: I have an doubt, if the swaped volume is a new volume, maybe we will get bdm raised exception.VolumeBDMNotFound, that we dont do _update_volume_regular(), and cannot completed the swap operation, right? | |
| 01:33:22 | lbragstad | we originally intended that to be an all-or-nothing option | |
| 01:33:36 | gmann | lbragstad: for existing project who have already exposed reno or doc which is keystone only :). does that flag scope change is bad for user ? | |
| 01:33:54 | lbragstad | so - operators would only set it once they 1.) updated their policies (which might not be needed anymore) and 2.) audited their users to make sure the ones that need system scope have it | |
| 01:34:07 | gmann | yeah. | |
| 01:34:32 | lbragstad | gmann i need to think through it a bit more | |
| 01:35:37 | lbragstad | i doubt anyone is running enforce_scope in production, yet? | |
| 01:36:59 | gmann | ok, in that way it make sense to make it all-or-nothing. | |
| 01:38:19 | gmann | because new flag controlling scope and defaults roles separately does not make sense. | |
| 01:38:40 | lbragstad | yeah - you mean using the old policies and setting enforce scope to True? | |
| 01:38:41 | gmann | and overahead for us also to maintain the old deprecated things | |
| 01:38:59 | gmann | yeah. | |