*** mhen_ is now known as mhen | 01:22 | |
*** tosky is now known as Guest1012 | 12:14 | |
*** tosky__ is now known as tosky | 12:14 | |
*** tosky is now known as Guest1013 | 12:30 | |
*** tosky__ is now known as tosky | 12:30 | |
opendevreview | Ade Lee proposed openstack/castellan master: Add secret consumers https://review.opendev.org/c/openstack/castellan/+/858057 | 12:50 |
---|---|---|
dmendiza[m] | #startmeeting barbican | 13:00 |
opendevmeet | Meeting started Tue Sep 20 13:00:10 2022 UTC and is due to finish in 60 minutes. The chair is dmendiza[m]. Information about MeetBot at http://wiki.debian.org/MeetBot. | 13:00 |
opendevmeet | Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. | 13:00 |
opendevmeet | The meeting name has been set to 'barbican' | 13:00 |
dmendiza[m] | #topic Roll Call | 13:00 |
dmendiza[m] | Courtesy ping for ade_lee dave-mccowan d34dh0r53 hrybacki jamespage Luzi lxkong mhen rm_work tosky xek nearyo oleksandry | 13:00 |
tosky | hi | 13:01 |
dmendiza[m] | Hi tosky ! | 13:01 |
rajiv | hi | 13:02 |
ade_lee | o/ | 13:02 |
dmendiza[m] | Alright, let's get started | 13:03 |
dmendiza[m] | #topic Review Past Meeting Action Items | 13:03 |
dmendiza[m] | #link https://meetings.opendev.org/meetings/barbican/2022/barbican.2022-09-13-13.02.html | 13:03 |
dmendiza[m] | We didn't have any | 13:04 |
dmendiza[m] | #topic Liaison Updates | 13:04 |
dmendiza[m] | tosky: any updates this week? | 13:04 |
tosky | you mentioned an issue with grenade last week, but I didn't follow up, sorry: was it solved? | 13:06 |
tosky | apart from that, no updates | 13:06 |
dmendiza[m] | tosky: not yet. I did submit a patch to disable voting on grenade to unblock the gate | 13:07 |
dmendiza[m] | I looked into it briefly, and it looks like creating an encrypted volume is failing | 13:07 |
dmendiza[m] | because of a permissions issue | 13:07 |
dmendiza[m] | but I haven't had a chance to dig in to find the root cause | 13:07 |
dmendiza[m] | *barbican permission issue that is | 13:07 |
dmendiza[m] | seems we're rejecting an order request to generate the key | 13:08 |
dmendiza[m] | On the release side of things we still don't have an RC1 | 13:08 |
dmendiza[m] | We'd like to include xek's patch: | 13:08 |
dmendiza[m] | #link https://review.opendev.org/c/openstack/barbican/+/857047 | 13:10 |
dmendiza[m] | unfortunately it does not work for sqlite | 13:10 |
tosky | it's weird because it was working when the grenade pathc was merged | 13:10 |
dmendiza[m] | tosky: agreed ... real weird indeed. | 13:10 |
dmendiza[m] | tosky: now there's also apparently an issue with our .zuul.yaml file | 13:10 |
dmendiza[m] | looks like maybe zuul was updated and the yaml file is no longer valid | 13:11 |
dmendiza[m] | ... back to xek's patch ... I need to work on my SQLAlchemy chops to try to figure out a migration that will work for all DB backends | 13:12 |
dmendiza[m] | also, we don't test sqlite at the gate, even though we default to sqlite in the barbican conf | 13:12 |
ade_lee | dmendiza[m], yeah -we should consider defaulting to something else if we don't test it | 13:13 |
ade_lee | or test it | 13:14 |
dmendiza[m] | Well, it's easy to default to sqlite for dev | 13:14 |
dmendiza[m] | and it would not be an issue if we had not removed the auto-generate schema feature | 13:14 |
dmendiza[m] | so we're stuck having to apply migrations | 13:15 |
dmendiza[m] | which we apparently don't claim to support for sqlite: | 13:15 |
dmendiza[m] | #link https://opendev.org/openstack/barbican/src/branch/master/barbican/model/migration/commands.py#L47-L48 | 13:15 |
ade_lee | dmendiza[m], :/ - so if I started with a brand new db, would the db migrations have worked? | 13:16 |
ade_lee | for sqlite? | 13:16 |
ade_lee | coz right now, they break with xek patch | 13:17 |
dmendiza[m] | ade_lee: no, the SQL generated by xek's patch does not work on sqlite period | 13:17 |
dmendiza[m] | there may be a way to rewrite the migration | 13:17 |
dmendiza[m] | using "batch" migrations: | 13:17 |
dmendiza[m] | #link https://alembic.sqlalchemy.org/en/latest/batch.html | 13:17 |
ade_lee | yeah - agreed , but we should test somewhere in the gate to make sure something doesn't slip through | 13:17 |
ade_lee | ifwe're going to support slqite at all | 13:18 |
tosky | dmendiza[m]: zuul issue - it may be that queue statement, there was an announced change in zuul | 13:18 |
dmendiza[m] | tosky: ack, I'll throw up a patch after this meeting | 13:18 |
dmendiza[m] | ade_lee: to answer your question RE: brand new db, there's two ways of applying the current schema to the db. One way is to generate schema from the current models (we used to do this automatically), the second way is to apply migrations. | 13:19 |
dmendiza[m] | presumaby, doing the former would work on SQLite | 13:20 |
ade_lee | ok - and we no longer do the former .. | 13:20 |
dmendiza[m] | correct | 13:20 |
dmendiza[m] | Anyway, I'll continue to work on this patch, but if we can't get sqlite working by the end of the week, then we may need to release RC1 without it | 13:22 |
dmendiza[m] | OK, moving on | 13:23 |
dmendiza[m] | #topic Secret Consumers | 13:23 |
dmendiza[m] | As I mentioned, we'll try to get xek's patch merged | 13:23 |
dmendiza[m] | ade_lee: any updates on the client side of things? | 13:24 |
ade_lee | yeah - lots of progress | 13:24 |
ade_lee | we have some barbicanclient patches up and also a castellan patch that uses the stuff in the barbicanclient | 13:25 |
ade_lee | in general stuff seems to be working and we're adding tests (unit/functional etc) | 13:25 |
ade_lee | waiting on reviews mostly now -- nudge nudge | 13:25 |
dmendiza[m] | ack, will review as soon as possible | 13:26 |
dmendiza[m] | OK, moving on | 13:26 |
dmendiza[m] | #topic Secure RBAC | 13:26 |
dmendiza[m] | I think we're good for now | 13:26 |
dmendiza[m] | #topic Open Discussion | 13:27 |
dmendiza[m] | Any topics y'all want to talk about? | 13:27 |
rajiv | hi, I have 3 queries to ask | 13:27 |
dmendiza[m] | rajiv: go ahead | 13:28 |
rajiv | 1. could i know why only generic container secrets can be updated ? why not certificate containers : https://github.com/openstack/barbican/blob/stable/yoga/barbican/api/controllers/containers.py#L246-L247 | 13:28 |
rajiv | would there be any issues if we comment or remove these 2 code lines ? | 13:29 |
dmendiza[m] | rajiv: by design. Generic containers can be modified, but "certificate" containers are supposed to represent a cert + key + passphrase. For a typical x509, you would not "modify" they key, you would just issue a new cert, for example. | 13:29 |
rajiv | correct, but there is no option to update the new cert ? | 13:30 |
dmendiza[m] | The recommended workflow is to create a new container for the new cert + key + passphrase | 13:31 |
rajiv | okay, in general there is no harm to add certificate container to this list or remove the if condition ? | 13:31 |
dmendiza[m] | IIRC, the main concern was with having certificates changed under you. e.g. I expect a certain certificate, but someone updated it then the original certificate is lost | 13:32 |
dmendiza[m] | since "certificate" type containers only allow a single cert | 13:32 |
rajiv | okay, i have a custom role "keymanager_admin" who can only edit secrets not even admin, there are only 1-2 users per project with "keymanager_admin" role. hence i presume its safe to patch this code ? | 13:33 |
dmendiza[m] | rajiv: if your workflow requires you to modify the container, then why not use "generic" type? | 13:34 |
dmendiza[m] | I am not sure we would merge a change to the behavior of non-generic containers | 13:34 |
dmendiza[m] | ade_lee: what's your opinion? | 13:34 |
rajiv | the user wants only certificate type! i did try convince but had no luck | 13:35 |
rajiv | i see this bug also reports the same : https://storyboard.openstack.org/#!/story/2007629 | 13:35 |
* dmendiza[m] looks at bug report | 13:37 | |
ade_lee | sorry .. reading up .. | 13:37 |
dmendiza[m] | Yeah, that's an interesting use case ... although not quite the same as you're proposing. The bug report mentions the ability for a secret to be created, and then the payload uploaded. | 13:38 |
dmendiza[m] | but secret still cannot be modified | 13:38 |
rajiv | yes | 13:38 |
dmendiza[m] | it's a design decision we made when we started barbican to disallow changing secrets and focing a user to create a new one. | 13:38 |
dmendiza[m] | from my point of view your user has two options 1) use "generic" container and add/remove secrets whenever they want or 2) use "certificate" type containers and create a new one when the certiricate or key changes. | 13:39 |
ade_lee | dmendiza[m], I'm not fundamentally opposed to this change. | 13:40 |
rajiv | okay, would there be any progress on that bug ? or would there be anything else i need to check if i disable the if condition ? | 13:40 |
dmendiza[m] | rajiv: I'll have to test a few things for that bug report | 13:40 |
ade_lee | dmendiza[m], I can imagine for instance that certificates expire and need to be replaced. Is it an undue burden to require folks to find all the clients and update with a new cert? | 13:40 |
rajiv | i can patch my internal repo and not create a PR upstream :) | 13:41 |
rajiv | @ade_lee this is another complain i received from the user as well. | 13:41 |
ade_lee | dmendiza[m], after all its not some random person updating the cert | 13:41 |
dmendiza[m] | rajiv: it may make sense to allow for creating an empty "certificate" type container and then allow the secrets to be added later, but still not allow delete/update after that. | 13:42 |
dmendiza[m] | ade_lee: I am ooposed to the change because it would override and lose the old cert | 13:42 |
dmendiza[m] | ade_lee: which, even when expired, may be needed to decrypt prior traffic for example | 13:43 |
ade_lee | dmendiza[m], good point | 13:43 |
ade_lee | dmendiza[m], feels like we need a better way to track old certs | 13:44 |
ade_lee | rajiv, maybe this is a good candidate for a spec to be written -- at least then we can discuss all the requirements and design decisions | 13:44 |
rajiv | ade_lee: sure | 13:45 |
rajiv | 2. few weeks back, i requested for sorting secrets by name, i created a patch but later found https://github.com/openstack/barbican/blob/stable/yoga/barbican/model/repositories.py#L669-L670 , | 13:45 |
ade_lee | rajiv, we may ultimately decided not to change anything but at least we can specify why | 13:45 |
rajiv | as sql "like" supports % and _ operators support regex, would i still need a patch or is this fine to proceed ? does the docu need mentioning of these operators ? | 13:46 |
ade_lee | rajiv, so you're saying you can get sorting by passing in a regex/operator in the query string? | 13:48 |
dmendiza[m] | sorry, I'm not sure I understand how % and _ are used. Would that be part of the user query? Does it work currently? | 13:48 |
rajiv | we cannot filter names but only sort them, right ? | 13:49 |
rajiv | by using % and _ operators in the curl command, i am able to perform regex operations. I would like to know, if this is an acceptable approach or would i need to go ahead testing the patch : https://github.com/sapcc/barbican/commit/f6f50c90ad89982dca9b7c34a52c70dede09c750 | 13:50 |
dmendiza[m] | OH, I think the "like" operation will filter, not just sort | 13:50 |
rajiv | yes, | 13:50 |
rajiv | example : | 13:50 |
rajiv | curl -s 'https://keymanager-3.xxx.cloud.sap/v1/secrets?name=my%' -H "X-Auth-Token: ${OS_AUTH_TOKEN}" | jq -r | grep total "total": 3 | 13:51 |
rajiv | curl -s 'https://keymanager-3.xxx.cloud.xxx/v1/secrets?name=raj%' -H "X-Auth-Token: ${OS_AUTH_TOKEN}" | jq -r | grep total "total": 1 | 13:51 |
rajiv | all of the below options mentioned, 'a_%', 'a%o', here for example, worked : https://www.w3schools.com/sql/sql_like.asp | 13:53 |
dmendiza[m] | neat | 13:53 |
dmendiza[m] | rajiv: is your patch needed to get those curl commands to work? | 13:54 |
rajiv | dmendiza[m]: nope, while testing the patch i bumped into this "like" operation, later tested with % and _ operators | 13:55 |
rajiv | i guess i neednt go ahead developing a patch ? this also works for other parameters, alg, mode, etc | 13:55 |
dmendiza[m] | I see, yeah, it would be awesome if you could add that to the barbican docs | 13:57 |
rajiv | cool, | 13:58 |
dmendiza[m] | ... almost out of time ... | 13:58 |
rajiv | last q, i am starting my python dev skills, i need assistance pagination bug | 13:58 |
dmendiza[m] | rajiv: you had one more Q? | 13:58 |
rajiv | https://github.com/sapcc/barbican/pull/2/files this doesnt go as i thought, whom could i reach out to for help ? | 13:59 |
dmendiza[m] | You can definitely ask for help here | 14:00 |
rajiv | cool, its related to https://storyboard.openstack.org/#!/story/2009322 | 14:01 |
rajiv | thats it from me | 14:01 |
dmendiza[m] | Fridays is usually when I have time to do things upstream, so that would be a good day to ping me and have a look | 14:02 |
dmendiza[m] | That's it for meeting time, I gotta run to another meeting. | 14:02 |
dmendiza[m] | See y'all online! | 14:02 |
dmendiza[m] | #endmeeting | 14:02 |
opendevmeet | Meeting ended Tue Sep 20 14:02:31 2022 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4) | 14:02 |
opendevmeet | Minutes: https://meetings.opendev.org/meetings/barbican/2022/barbican.2022-09-20-13.00.html | 14:02 |
opendevmeet | Minutes (text): https://meetings.opendev.org/meetings/barbican/2022/barbican.2022-09-20-13.00.txt | 14:02 |
opendevmeet | Log: https://meetings.opendev.org/meetings/barbican/2022/barbican.2022-09-20-13.00.log.html | 14:02 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!