| *** redrobot4 is now known as redrobot | 04:52 | |
| *** zbr is now known as Guest164 | 05:03 | |
| *** rpittau|afk is now known as rpittau | 06:59 | |
| *** slaweq_ is now known as slaweq | 11:42 | |
| *** rpittau is now known as rpittau|afk | 12:45 | |
| abhishekk | #startmeeting glance | 14:00 |
|---|---|---|
| opendevmeet | Meeting 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 http://wiki.debian.org/MeetBot. | 14:00 |
| opendevmeet | Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. | 14:00 |
| opendevmeet | The meeting name has been set to 'glance' | 14:00 |
| abhishekk | #topic roll call | 14:00 |
| abhishekk | #link https://etherpad.openstack.org/p/glance-team-meeting-agenda | 14:00 |
| abhishekk | o/ | 14:00 |
| dansmith | o/ | 14:00 |
| rosmaita | o/ | 14:00 |
| croelandt | o/ | 14:00 |
| abhishekk | lets wait couple of minutes for jokke_ | 14:01 |
| jokke_ | o/ | 14:01 |
| abhishekk | cool | 14:01 |
| abhishekk | #topic release/periodic jobs update | 14:01 |
| abhishekk | M2 is next week, so is our spec freeze | 14:01 |
| abhishekk | I am planning to Tag M2 on 13th July | 14:02 |
| abhishekk | Before that I/we need to verify config refresh patch and releasenotes are inline or not | 14:03 |
| abhishekk | Periodic jobs all green \o/ | 14:03 |
| jokke_ | nice | 14:03 |
| abhishekk | M2 Target progress check | 14:03 |
| rosmaita | we may need to review those jobs and make sure they are still testing something :) | 14:04 |
| abhishekk | Quotas, everything is done from glance side | 14:04 |
| abhishekk | :P | 14:04 |
| jokke_ | rosmaita: :P | 14:04 |
| abhishekk | I think credit goes to dansmith for figuring out timeout issue, which was harassing us mostly during milestone releases | 14:05 |
| abhishekk | Thank you all for your reviews | 14:05 |
| rosmaita | hooray for dansmith ! | 14:05 |
| * dansmith blushes | 14:05 | |
| abhishekk | We need to followup with tempest patches | 14:05 |
| abhishekk | dansmith, ^^^ | 14:05 |
| dansmith | quota ones you mean | 14:05 |
| abhishekk | yeah | 14:06 |
| dansmith | this week has been crazy busy for me with other stuff, | 14:06 |
| dansmith | and I don't normally harp on the tempest people until the patches are actually merge-able | 14:06 |
| dansmith | but yeah I'll start working on that again tomorrow or next week | 14:06 |
| abhishekk | ack, I can followup with them as well | 14:06 |
| dansmith | mergeable meaning.. the feature has landed | 14:06 |
| dansmith | I know the tempest test needs at least a unit test for the limits client I added | 14:07 |
| abhishekk | right | 14:07 |
| abhishekk | Cache API | 14:07 |
| abhishekk | I think implementation is up, waiting on additional tests and doc changes | 14:08 |
| abhishekk | If those are up till Monday EOD then we will consider it for M2 else we will shift it to M3 | 14:08 |
| dansmith | link? | 14:08 |
| jokke_ | yup, working on those and the client calls | 14:08 |
| abhishekk | #link https://review.opendev.org/c/openstack/glance/+/792022 | 14:09 |
| abhishekk | Client changes will be done in M3 timeframe, need to merge client spec before next week | 14:09 |
| dansmith | abhishekk: thanks | 14:09 |
| abhishekk | client spec link #https://review.opendev.org/c/openstack/glance-specs/+/794709 | 14:10 |
| abhishekk | moving to next topic | 14:10 |
| abhishekk | #topic Policy refactoring | 14: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 |
| abhishekk | jokke_, ack and agree | 14:12 |
| abhishekk | Spec freeze is approaching and we have almost settled everything, just pending thing is nested check vs backward compatibility | 14:12 |
| abhishekk | AFAIK, considering our default policies I am more than sure we are not breaking any backward compatibility by avoiding nested policy checks | 14:13 |
| abhishekk | also there is no need to major version bump to avoid nested policy checks | 14:14 |
| dansmith | also 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 behavior | 14:14 |
| abhishekk | Fact 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 proposal | 14:15 |
| croelandt | do we really change the API though? | 14:16 |
| dansmith | please specify what is changing exactly | 14:16 |
| croelandt | like, we are not removing get_foo or what get_foo means | 14:16 |
| dansmith | croelandt: exactly | 14:16 |
| croelandt | I was slightly annoyed with this but dansmith's comment seemed reasonable to me | 14:16 |
| abhishekk | correct | 14:16 |
| croelandt | the 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 anymore | 14:17 |
| dansmith | croelandt: 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 |
| dansmith | whereas if each policy needs to be secure, it doesn't matter | 14:17 |
| abhishekk | jokke_, could you please explore more on this? | 14:18 |
| dansmith | jokke_: can you explain that in more detail? because I don't understand that | 14:18 |
| croelandt | Do you mean you can be allowed to modify_image but not get_image | 14:18 |
| croelandt | and therefore you might change an image but not print it to check it worked? | 14:18 |
| abhishekk | Because I can see no use of blocking get call and giving access to update | 14:18 |
| dansmith | ah, 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 review | 14:20 |
| dansmith | that'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 written | 14:20 |
| croelandt | dansmith: but usually you have "r" if you have "w" | 14:20 |
| croelandt | so it's more of a config issue on the user side | 14:20 |
| dansmith | croelandt: not in a dropbox situation | 14:21 |
| croelandt | hm | 14:21 |
| dansmith | croelandt: but yes, by default and in most cases an operator would give people r and w at the same time | 14:21 |
| rosmaita | but the new default will allow both GET and UPDATE, right? | 14:21 |
| croelandt | ok | 14:21 |
| croelandt | not sure I've ever done the dropbox thingy | 14:21 |
| dansmith | croelandt: 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 spec | 14:21 |
| dansmith | rosmaita: yes | 14:21 |
| abhishekk | rosmaita, not only the new defualt but the current one as well | 14:21 |
| rosmaita | so the difference is that now an operator could do a weird policy config that they couldn't do before | 14:21 |
| dansmith | croelandt: maybe you went to college too recently.. in my day, we submitted assignments on the department unix system in a dropbox directory :) | 14:22 |
| dansmith | rosmaita: yeah, and I explained several legit reasons they might want to do that | 14:22 |
| jokke_ | dansmith: but we do not follow the unix permission model in a first place and we never have | 14:22 |
| croelandt | dansmith: oh yeah yeah | 14:22 |
| dansmith | rosmaita: 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 |
| croelandt | I've had students do taht! | 14:22 |
| dansmith | rosmaita: which we already have some tests that do that :) | 14:22 |
| rosmaita | dansmith: i am actually agreeing with you (i think) | 14:23 |
| dansmith | jokke_: of course not, it's just a classic example of "writer-only" access patterns | 14:23 |
| dansmith | rosmaita: yep | 14:23 |
| abhishekk | I am totally failing to understand why someone will block get call if he gives aceess to delete,download,upload,update calls | 14:24 |
| rosmaita | i guess my question is, there are probably weird behaviors configurable in policy now that don't make sense, too | 14:24 |
| dansmith | abhishekk: my examples in the spec discussion earlier? | 14:24 |
| rosmaita | so i'm not convinced this should be considered an API change | 14:25 |
| abhishekk | dansmith, yep | 14:25 |
| jokke_ | abhishekk: well dansmith had couple of examples of usecases where that might be handy in the previous revision comments | 14:25 |
| dansmith | rosmaita: 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 reality | 14:25 |
| abhishekk | that is what our next topic is | 14:26 |
| abhishekk | #link https://review.opendev.org/c/openstack/glance/+/798266 | 14:26 |
| abhishekk | Policy checks Vs Read Only checks | 14:26 |
| abhishekk | This is the case with almost all metadef policies as well | 14:26 |
| abhishekk | It passes the policy check but gets rejected at DB layer on visibility check | 14:27 |
| dansmith | rosmaita: almost all of the existing policies resolve to this: https://github.com/openstack/glance/blob/master/glance/policies/base.py#L69 | 14:27 |
| dansmith | rosmaita: which means anyone can do anything | 14:27 |
| dansmith | but of course, they can't because we have hard-coded behaviors that you can't override, calcified in the DB layers | 14:27 |
| rosmaita | yeah, but with policy-in-code, "default" is never used anymore | 14:28 |
| dansmith | rosmaita: eh? it's always used | 14:28 |
| rosmaita | where? | 14:28 |
| abhishekk | for each policy we are using rule:default | 14:29 |
| abhishekk | s/each/most | 14:29 |
| dansmith | https://github.com/openstack/glance/blob/master/glance/policies/image.py#L34 | 14:29 |
| dansmith | every one of those is using rule:default unless you have experimental RBAC turned on | 14:29 |
| dansmith | but today, everyone has rule:default for most everything | 14:29 |
| dansmith | abhishekk and I know this, because our tests are wildly wrong on most things :) | 14:30 |
| abhishekk | yeah | 14:30 |
| * dansmith wonders if rosmaita's head just exploded | 14:31 | |
| rosmaita | i will shut up, but oslo policy used to (and maybe still does) recognize a special rule named 'default' | 14:31 |
| abhishekk | So we can have/enforce get check at API level but frankly speaking it will be of no use | 14:31 |
| dansmith | rosmaita: our default rule is defined with a check string of "" | 14:31 |
| rosmaita | yeah, so i don't like that on 2 levels ... anyway, i will shut up as not being hip to glance policy changes since train | 14:32 |
| abhishekk | Ok | 14:32 |
| dansmith | hope that wasm | 14:33 |
| dansmith | wasn't a rage quit | 14: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 mess | 14:34 |
| abhishekk | hopefully | 14:34 |
| jokke_ | oh, he dropped | 14:34 |
| dansmith | yeah I dunno about any behavior of what happens if you don't specify a default rule, | 14:35 |
| dansmith | but we do, and I'm sure it's getting honored | 14:35 |
| jokke_ | yup | 14:35 |
| abhishekk | I think this is the right time to set everything in correct order | 14:35 |
| dansmith | yeah, because this is confusing and hairy and so getting things straight while we all have context is a good plan | 14:36 |
| abhishekk | agree | 14:36 |
| rosmaita | sorry about that | 14:37 |
| abhishekk | he is back :D | 14:37 |
| dansmith | rosmaita: gave us a scare :) | 14:37 |
| jokke_ | rosmaita: wb, we were wondering if you had mic drop moment | 14:38 |
| rosmaita | i blame bluejeans (because i am multitasking) | 14:38 |
| abhishekk | aha | 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 mess | 14:38 |
| abhishekk | and we need multitasking badly :D | 14:38 |
| jokke_ | rosmaita: ^^ just replay of what I typed when you dropped | 14:38 |
| rosmaita | yeah, i was talikng from old knowledge | 14:38 |
| rosmaita | anyway, maybe it's too late now, but i would not advise having a rule named "default" | 14:39 |
| dansmith | anyway, 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 enforced | 14:39 |
| dansmith | if we have to to move forward, then we can, but I think it's detrimental to actually making this understandable from the outside | 14:39 |
| rosmaita | i think we probably mostly agree on that? | 14:39 |
| dansmith | you and I do | 14:40 |
| abhishekk | agree on not having default? | 14:40 |
| dansmith | I thought he meant "agree on not tying get and update together" | 14:40 |
| abhishekk | I am onboard to that from beginning | 14: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 now | 14:41 |
| dansmith | again, please specify the exact behavior that is brekaing | 14:41 |
| abhishekk | with context to default policies today we have | 14:42 |
| jokke_ | dansmith: I've done it multiple times, most recently 22min ago | 14:42 |
| rosmaita | ok, i am now in this meeting 100% | 14:42 |
| croelandt | rosmaita: ... and it's over! | 14:43 |
| rosmaita | i wish | 14:43 |
| rosmaita | so jokke_, i don't see that there is an API guarantee about being able to use every call in the API | 14:43 |
| abhishekk | jokke_, according to me we are not breaking anything if we consider our default policies as of today | 14:43 |
| jokke_ | abhishekk: defaults have nothing to do with it. | 14:44 |
| abhishekk | Then ? | 14:44 |
| rosmaita | jokke_: 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 production | 14:45 |
| rosmaita | i think if i read your comment in context i will grasp your point better | 14:45 |
| jokke_ | #link https://review.opendev.org/c/openstack/glance-specs/+/796753/3/specs/xena/approved/glance/glance-policy-refactoring.rst | 14:46 |
| rosmaita | ty | 14:46 |
| dansmith | our 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 so | 14:47 |
| jokke_ | like they have for past years | 14:47 |
| croelandt | dansmith: 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 :D | 14:48 |
| dansmith | well, 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 PUT | 14:48 |
| jokke_ | croelandt: I do that even before entering into rounabout | 14:48 |
| jokke_ | roundabout | 14:48 |
| abhishekk | SO even if the road is one way ?? | 14:49 |
| dansmith | croelandt: 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 |
| croelandt | dansmith: yeah :) | 14:49 |
| dansmith | and 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 |
| croelandt | I mean, worst case scenario, users will complain and the admin will give them read permission on their own images, right? | 14:49 |
| rosmaita | jokke_: i still don't see this as a change in api behavior | 14:50 |
| abhishekk | exactly | 14:50 |
| dansmith | jokke_: I'm perfectly fine not synthesizing behavioral promises that don't exist :) | 14:50 |
| rosmaita | it's a change in what can be configured via policy | 14:50 |
| dansmith | croelandt: 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 described | 14:51 |
| dansmith | rosmaita: 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 |
| abhishekk | Even if we consider update situation that we should enforce get check, what is the need for similar check on delete/download/upload calls | 14:51 |
| dansmith | a 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 account | 14:52 |
| dansmith | like 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 delete | 14:53 |
| dansmith | otherwise that bot's account becomes a huge target because it's so powerul | 14:53 |
| dansmith | *powerful | 14:53 |
| abhishekk | Last 7 minutes | 14:53 |
| abhishekk | I would suggest everyone should review new revision and vote accordingly | 14:53 |
| jokke_ | dansmith: that bot example is very much the reason why we have property protections | 14:54 |
| jokke_ | so you don't expose your sensitive information through the metadata | 14:54 |
| dansmith | jokke_: when PP was added, was that considered a breaking API change? | 14:55 |
| abhishekk | I am also considering this for FFE as I don't want to miss this opportunity to correct the things | 14:55 |
| jokke_ | abhishekk: :P | 14:55 |
| abhishekk | As I said if it doesn't happen now, it will never happen again | 14:55 |
| dansmith | yep, | 14:55 |
| dansmith | and we're really already very short on time and have a LONG road ahead | 14:56 |
| abhishekk | yes | 14:56 |
| rosmaita | ok, i have not reviewed the latest patch set on the sepc, will do so right now | 14:56 |
| abhishekk | ack, thank you | 14:56 |
| abhishekk | That's it from me today | 14:56 |
| abhishekk | Moving to open discussion | 14:56 |
| abhishekk | #topic Open discussion | 14:56 |
| abhishekk | Nothing from me | 14:57 |
| rosmaita | ok, i definitely have jokke_'s concerns in mind and they are clearly stated in gerrit, so i will think carefully about them while reviewing | 14:57 |
| abhishekk | ack | 14:57 |
| jokke_ | I have nothing for open, will keep grinding the cache api stuff and the swift driver bugfixes | 14:58 |
| abhishekk | I am exploring metadef | 14:58 |
| abhishekk | Sounds like our command line need more helpful/meaningful messages | 14:59 |
| abhishekk | Time to wrap up | 14:59 |
| abhishekk | thank you all | 14:59 |
| jokke_ | TY | 14:59 |
| abhishekk | have a nice weekend o/~ | 14:59 |
| rosmaita | thanks! | 15:00 |
| abhishekk | #endmeeting | 15:00 |
| opendevmeet | Meeting ended Thu Jul 8 15:00:02 2021 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4) | 15:00 |
| opendevmeet | Minutes: https://meetings.opendev.org/meetings/glance/2021/glance.2021-07-08-14.00.html | 15:00 |
| opendevmeet | Minutes (text): https://meetings.opendev.org/meetings/glance/2021/glance.2021-07-08-14.00.txt | 15:00 |
| opendevmeet | Log: https://meetings.opendev.org/meetings/glance/2021/glance.2021-07-08-14.00.log.html | 15:00 |
| *** frickler is now known as frickler_pto | 15:03 | |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!