Thursday, 2021-07-08

*** redrobot4 is now known as redrobot04:52
*** zbr is now known as Guest16405:03
*** rpittau|afk is now known as rpittau06:59
*** slaweq_ is now known as slaweq11:42
*** rpittau is now known as rpittau|afk12:45
abhishekk#startmeeting glance14:00
opendevmeetMeeting started Thu Jul  8 14:00:13 2021 UTC and is due to finish in 60 minutes.  The chair is abhishekk. Information about MeetBot at
opendevmeetUseful Commands: #action #agreed #help #info #idea #link #topic #startvote.14:00
opendevmeetThe meeting name has been set to 'glance'14:00
abhishekk#topic roll call14:00
abhishekklets wait couple of minutes for jokke_ 14:01
abhishekk#topic release/periodic jobs update14:01
abhishekkM2 is next week, so is our spec freeze 14:01
abhishekkI am planning to Tag M2 on 13th July14:02
abhishekkBefore that I/we need to verify config refresh patch and releasenotes are inline or not14:03
abhishekkPeriodic jobs all green \o/14:03
abhishekkM2 Target progress check14:03
rosmaitawe may need to review those jobs and make sure they are still testing something :)14:04
abhishekkQuotas, everything is done from glance side14:04
jokke_rosmaita: :P14:04
abhishekkI think credit goes to dansmith for figuring out timeout issue, which was harassing us mostly during milestone releases14:05
abhishekkThank you all for your reviews14:05
rosmaitahooray for dansmith !14:05
* dansmith blushes14:05
abhishekkWe need to followup with tempest patches14:05
abhishekkdansmith, ^^^14:05
dansmithquota ones you mean14:05
dansmiththis week has been crazy busy for me with other stuff,14:06
dansmithand I don't normally harp on the tempest people until the patches are actually merge-able14:06
dansmithbut yeah I'll start working on that again tomorrow or next week14:06
abhishekkack, I can followup with them as well14:06
dansmithmergeable meaning.. the feature has landed14:06
dansmithI know the tempest test needs at least a unit test for the limits client I added14:07
abhishekkCache API14:07
abhishekkI think implementation is up, waiting on additional tests and doc changes14:08
abhishekkIf those are up till Monday EOD then we will consider it for M2 else we will shift it to M314:08
jokke_yup, working on those and the client calls 14:08
abhishekkClient changes will be done in M3 timeframe, need to merge client spec before next week14:09
dansmithabhishekk: thanks14:09
abhishekkclient spec link #
abhishekkmoving to next topic 14:10
abhishekk#topic Policy refactoring14:10
jokke_abhishekk: I don't care when the client patch gets merged, but makes it easier for me to work on the docs and tests when I have the workflow sorted out and manually testable ;)14:11
abhishekkjokke_, ack and agree14:12
abhishekkSpec freeze is approaching and we have almost settled everything, just pending thing is nested check vs backward compatibility14:12
abhishekkAFAIK, considering our default policies I am more than sure we are not breaking any backward compatibility by avoiding nested policy checks14:13
abhishekkalso there is no need to major version bump to avoid nested policy checks14:14
dansmithalso because the default policy that comes as a result of the refactor (i.e. replacing our existing "" with an actual rule) will keep the same default behavior14:14
abhishekkFact is our most of the policies are broken 14:14
jokke_I'm more worried about the fundamental behavioural change than what ever our default behaviour happens to be at any given time.14:14
jokke_something that started as "We should refactor where our policies gets enforced" is turning proper hot mess changing the API behaviour is my problem with that proposal14:15
croelandtdo we really change the API though?14:16
dansmithplease specify what is changing exactly14:16
croelandtlike, we are not removing get_foo or what get_foo means14:16
dansmithcroelandt: exactly14:16
croelandtI was slightly annoyed with this but dansmith's comment seemed reasonable to me14:16
croelandtthe changes seem to be "implementation details" rather than "contract with the API user"14:16
jokke_we do. If you're able to change any part of the image as of now you can always verify that your change took effect. After the proposed change that is not the case anymore14:17
dansmithcroelandt: also, the policy-writing admin knowing that our internals check get before update is definitely requiring them to understand on and rely on internal implementation details,14:17
dansmithwhereas if each policy needs to be secure, it doesn't matter14:17
abhishekkjokke_, could you please explore more on this?14:18
dansmithjokke_: can you explain that in more detail? because I don't understand that14:18
croelandtDo you mean you can be allowed to modify_image but not get_image14:18
croelandtand therefore you might change an image but not print it to check it worked?14:18
abhishekkBecause I can see no use of blocking get call and giving access to update 14:18
dansmithah, okay, but if we return 200 then the change was made, if you have to get it later to check, we're breaking our RESTfulness :)14:19
jokke_as of now, to be able to do a chage, say in the image metadata, you need to have ability to get that image object as the get_ is prerequisite to do any writes. As proposed that would not be the case anymore leading to situations where people can blindly change things without being able to fetch that data to confirm it. I did walk through this already in the review14:20
dansmiththat's like saying that UNIX shouldn't support dropbox directories of -wx because you can't ls the directory to make sure the file was written14:20
croelandtdansmith: but usually you have "r" if you have "w"14:20
croelandtso it's more of a config issue on the user side14:20
dansmithcroelandt: not in a dropbox situation14:21
dansmithcroelandt: but yes, by default and in most cases an operator would give people r and w at the same time14:21
rosmaitabut the new default will allow both GET and UPDATE, right?14:21
croelandtnot sure I've ever done the dropbox thingy14:21
dansmithcroelandt: I'm just saying we don't need to force them to do that, like for the bot account cases I detailed earlier in the spec14:21
dansmithrosmaita: yes14:21
abhishekkrosmaita, not only the new defualt but the current one as well14:21
rosmaitaso the difference is that now an operator could do a weird policy config that they couldn't do before14:21
dansmithcroelandt: maybe you went to college too recently.. in my day, we submitted assignments on the department unix system in a dropbox directory :)14:22
dansmithrosmaita: yeah, and I explained several legit reasons they might want to do that14:22
jokke_dansmith: but we do not follow the unix permission model in a first place and we never have14:22
croelandtdansmith: oh yeah yeah14:22
dansmithrosmaita: the important thing I think, is that they also won't be able to configure wildly insecure rules for update that rely in "can see the image" as a gateway,14:22
croelandtI've had students do taht!14:22
dansmithrosmaita: which we already have some tests that do that :)14:22
rosmaitadansmith: i am actually agreeing with you (i think)14:23
dansmithjokke_: of course not, it's just a classic example of "writer-only" access patterns14:23
dansmithrosmaita: yep14:23
abhishekkI am totally failing to understand why someone will block get call if he gives aceess to delete,download,upload,update calls14:24
rosmaitai guess my question is, there are probably weird behaviors configurable in policy now that don't make sense, too14:24
dansmithabhishekk: my examples in the spec discussion earlier?14:24
rosmaitaso i'm not convinced this should be considered an API change14:25
abhishekkdansmith, yep14:25
jokke_abhishekk: well dansmith had couple of examples of usecases where that might be handy in the previous revision comments14:25
dansmithrosmaita: for sure, but also most of the enforcement is hard-coded in python, which means people think they can actually control things in policy, but can't.. like most of the rules are technically "open to anyone" and just rely on hard-coded python in reality14:25
abhishekkthat is what our next topic is14:26
abhishekkPolicy checks Vs Read Only checks14:26
abhishekkThis is the case with almost all metadef policies as well14:26
abhishekkIt passes the policy check but gets rejected at DB layer on visibility check14:27
dansmithrosmaita: almost all of the existing policies resolve to this:
dansmithrosmaita: which means anyone can do anything14:27
dansmithbut of course, they can't because we have hard-coded behaviors that you can't override, calcified in the DB layers14:27
rosmaitayeah, but with policy-in-code, "default" is never used anymore14:28
dansmithrosmaita: eh? it's always used14:28
abhishekkfor each policy we are using rule:default14:29
dansmithevery one of those is using rule:default unless you have experimental RBAC turned on14:29
dansmithbut today, everyone has rule:default for most everything14:29
dansmithabhishekk and I know this, because our tests are wildly wrong on most things :)14:30
* dansmith wonders if rosmaita's head just exploded14:31
rosmaitai will shut up, but oslo policy used to (and maybe still does) recognize a special rule named 'default'14:31
abhishekkSo we can have/enforce get check at API level but frankly speaking it will be of no use14:31
dansmithrosmaita: our default rule is defined with a check string of ""14:31
rosmaitayeah, so i don't like that on 2 levels ... anyway, i will shut up as not being hip to glance policy changes since train14:32
dansmithhope that wasm14:33
dansmithwasn't a rage quit14:34
jokke_rosmaita: IIUC the 'default' policy came from oslo_policy as long as it wasn't specified. Which was obviously noticeable, say if you did not have policy file. Since the policy in code, we always overwrite it with empty checkstring, and that might not be so obvious, like everything else on this policy in code mess14:34
jokke_oh, he dropped14:34
dansmithyeah I dunno about any behavior of what happens if you don't specify a default rule,14:35
dansmithbut we do, and I'm sure it's getting honored14:35
abhishekkI think this is the right time to set everything in correct order14:35
dansmithyeah, because this is confusing and hairy and so getting things straight while we all have context is a good plan14:36
rosmaitasorry about that14:37
abhishekkhe is back :D14:37
dansmithrosmaita: gave us a scare :)14:37
jokke_rosmaita: wb, we were wondering if you had mic drop moment14:38
rosmaitai blame bluejeans (because i am multitasking)14:38
jokke_rosmaita: IIUC the 'default' policy came from oslo_policy as long as it wasn't specified. Which was obviously noticeable, say if you did not have policy file. Since the policy in code, we always overwrite it with empty checkstring, and that might not be so obvious, like everything else on this policy in code mess14:38
abhishekkand we need multitasking badly :D14:38
jokke_rosmaita: ^^ just replay of what I typed when you dropped14:38
rosmaitayeah, i was talikng from old knowledge14:38
rosmaitaanyway, maybe it's too late now, but i would not advise having a rule named "default"14:39
dansmithanyway, I do not thinking tying update and get policies together at the lower layers will do operators any favors. at best they limit what they can do, and at worst they encourage them to write insecure update policies banking on the get policy being enforced14:39
dansmithif we have to to move forward, then we can, but I think it's detrimental to actually making this understandable from the outside14:39
rosmaitai think we probably mostly agree on that?14:39
dansmithyou and I do14:40
abhishekkagree on not having default?14:40
dansmithI thought he meant "agree on not tying get and update together"14:40
abhishekkI am onboard to that from beginning14:41
jokke_yeah, if we were designing a new API I would be way easier convinced that it's reasonable approach to move forward. Not when it's breaking behaviour that has been established and enforced for, what, 7-8 years now14:41
dansmithagain, please specify the exact behavior that is brekaing14:41
abhishekkwith context to default policies today we have14:42
jokke_dansmith: I've done it multiple times, most recently 22min ago14:42
rosmaitaok, i am now in this meeting 100%14:42
croelandtrosmaita: ... and it's over!14:43
rosmaitai wish14:43
rosmaitaso jokke_, i don't see that there is an API guarantee about being able to use every call in the API14:43
abhishekkjokke_, according to me we are not breaking anything if we consider our default policies as of today14:43
jokke_abhishekk: defaults have nothing to do with it.14:44
abhishekkThen ?14:44
rosmaitajokke_: apologies for not having this already, but can you paste the link to the review where you already mentioned this?14:45
jokke_abhishekk: how the API behaves under _different_ config situation and what the user can expect from it. I think we can all agree that our _default_ policies are not something anyone would run in production14:45
rosmaitai think if i read your comment in context i will grasp your point better14:45
dansmithour API is super broken if everyone that does an image update has to get the image right after and verify that something took effect when we reported 200...14:46
jokke_dansmith: our api is super broken also if anyone doing such change cannot do so14:47
jokke_like they have for past years14:47
croelandtdansmith: I'm the kind of guy who checks both ways when crossing a one-way street, and I must say I like to check everything like that :D14:48
dansmithwell, the HTTP contract with a 200 says "we did that thing".. there's no actual contract in HTTP says that you must be able to GET things you PUT14:48
jokke_croelandt: I do that even before entering into rounabout14:48
abhishekkSO even if the road is one way ??14:49
dansmithcroelandt: sure, but telling the prof you want to be able to make sure other students' homework is correct too is not justification for having r-- access :)14:49
croelandtdansmith: yeah :)14:49
dansmithand in the case of security policy here, "rule of least privilege" IS the safer thing :)14:49
jokke_dansmith: so you're perfectly fine us to change our API behaviour how ever we like as long as it doesn't break HTTP protocol?14:49
croelandtI mean, worst case scenario, users will complain and the admin will give them read permission on their own images, right?14:49
rosmaitajokke_: i still don't see this as a change in api behavior14:50
dansmithjokke_: I'm perfectly fine not synthesizing behavioral promises that don't exist :)14:50
rosmaitait's a change in what can be configured via policy14:50
dansmithcroelandt: it's very unlikely this would even be a thing for owned images, and definitely not for actual human users.. this whole argument is rather contrived in when or how it would ever play out except for specific circumstances like I described14:51
dansmithrosmaita: right and if anything, it *appears* like we've allowed you to configure this for years, we just don't actually honor the policy in any meaningfulway. So it's really a bugfix :)14:51
abhishekkEven if we consider update situation that we should enforce get check, what is the need for similar check on delete/download/upload calls14:51
dansmitha garbage-collecting bot able to delete images but not see the license keys in their metadata is a clear example of why I should be able to grant only delete to a special service account14:52
dansmithlike a bot that deletes resources from an employee that has left should not need global permissions on the entire cloud to read anything, it only needs to delete things it is told to delete14:53
dansmithotherwise that bot's account becomes a huge target because it's so powerul14:53
abhishekkLast 7 minutes14:53
abhishekkI would suggest everyone should review new revision and vote accordingly14:53
jokke_dansmith: that bot example is very much the reason why we have property protections14:54
jokke_so you don't expose your sensitive information through the metadata14:54
dansmithjokke_: when PP was added, was that considered a breaking API change?14:55
abhishekkI am also considering this for FFE as I don't want to miss this opportunity to correct the things14:55
jokke_abhishekk: :P14:55
abhishekkAs I said if it doesn't happen now, it will never happen again14:55
dansmithand we're really already very short on time and have a LONG road ahead14:56
rosmaitaok, i have not reviewed the latest patch set on the sepc, will do so right now14:56
abhishekkack, thank you14:56
abhishekkThat's it from me today14:56
abhishekkMoving to open discussion14:56
abhishekk#topic Open discussion14:56
abhishekkNothing from me14:57
rosmaitaok, i definitely have jokke_'s concerns in mind and they are clearly stated in gerrit, so i will think carefully about them while reviewing14:57
jokke_I have nothing for open, will keep grinding the cache api stuff and the swift driver bugfixes 14:58
abhishekkI am exploring metadef 14:58
abhishekkSounds like our command line need more helpful/meaningful messages14:59
abhishekkTime to wrap up14:59
abhishekkthank you all14:59
abhishekkhave a nice weekend o/~14:59
opendevmeetMeeting ended Thu Jul  8 15:00:02 2021 UTC.  Information about MeetBot at . (v 0.1.4)15:00
opendevmeetMinutes (text):
*** frickler is now known as frickler_pto15:03

Generated by 2.17.2 by Marius Gedminas - find it at!