[03:28] PR snapcraft#2158 opened: rust plugin: fix cargo builds and run tests [05:05] morning [05:14] hey hey :) [05:15] zyga: hey [05:29] PR snapd#5273 closed: testutil: add test support for Fstatfs [05:30] woot, thank you! [05:38] so are we doing the `snap list --format=..` thing or not? [05:40] I ... don't know [05:40] I think not [05:42] mborzecki: 5266 is simple and green [05:45] mborzecki: it looks like no, lets see if the bugreport is happy about the snap list|awk|tail +2 solution [05:53] it was a nice feature :( [05:53] mborzecki: yeah, I have grown to like it as well [05:58] there was a panic from spread in #5271 https://paste.ubuntu.com/p/XXqr7TY9KM/ [05:58] PR #5271: [WIP] cmd: attempt to start the document portal if running with a session bus [05:58] niemeyer: ^^ [06:15] mborzecki: I think we saw a few panics yesterday and gustavo is aware of them [06:15] but I'm not sure if those are the same panics or if spread was changed but some panics remained [06:15] hey mvo, good morning [06:17] PR snapd#5266 closed: interfaces/builtin/docker: use commonInterface over specific struct [06:17] https://github.com/snapcore/snapd/pull/5230 needs a 2nd review [06:17] PR #5230: interfaces/udisks2: also implement implicit classic slot [06:18] zyga: good morning to you as well! [06:18] mvo: what's the state of https://github.com/snapcore/snapd/pull/5250 [06:18] PR #5250: interfaces/udev,misc: only trigger udev events on input subsystem as needed [06:18] there's a conflict but I see that you took care of the feedback [06:18] are you working on this or is the conflict recent [06:18] er [06:18] sorry [06:18] bad PR [06:18] https://github.com/snapcore/snapd/pull/5226 [06:18] PR #5226: data: add systemd user environment generator [06:18] this is what I meant [06:24] zyga: the user envirronment generator? iirc it had some packaging issues I need to look at them [06:24] ack, [06:24] I asked some follow up packaging questions there [06:24] ta [06:24] +1 on everything if it works :) [06:24] ta² [06:24] unicode :D [06:26] https://forum.snapcraft.io/t/organize-another-dir-causes-the-items-under-host-root-directory-to-be-copied-in-another-dir/5806 [06:58] PR snapd#5259 closed: devicestate: support seeding from a base snap instead of core === pstolowski|afk is now known as pstolowski [07:04] morning o/ [07:06] pstolowski: hey [07:06] hey pstolowski [07:26] hey pawel [07:45] * zyga -> walk [07:46] or maybe in 15 minutes [07:53] PR snapd#5275 opened: cmd/snap: use snaptest.MockSnapCurrent in `snap run` tests [07:53] trivial pr ^^ [08:08] PR snapd#5276 opened: devicestate: support seeding from a base snap instead of core [08:18] mborzecki: https://github.com/snapcore/snapd/pull/5277 :) [08:18] PR #5277: cmd/snap-update-ns: add helper for checking for read-only filesystems [08:18] I'll take your PR for snap run [08:18] PR snapd#5277 opened: cmd/snap-update-ns: add helper for checking for read-only filesystems [08:18] haha ;) trade PRs [08:18] spread still panics [08:18] can we revert spread somehow? [08:18] i'm looking into spread right now [08:19] +1 on the PR [08:19] and I need to go or my dog will hate me [08:20] zyga: it won't hate you, it'll just piss on the floor :P [08:20] and *then* he will hate me ;) [08:20] no, he's too humble for that I'm not that evil [08:20] * zyga -> walk [08:21] the spread crash is not happening all the time, right? I saw at least one green PR from me this morning [08:22] mborzecki: can I restart your failed MockSnapCurrent PR? or do you want to keep it for the error ? [08:22] mvo: go ahead [08:26] mvo: some comments on #5274 [08:26] PR #5274: configstate: deny configuration of base snaps and for the "snapd" snap [08:28] niemeyer: https://github.com/snapcore/spread/pull/59 [08:28] PR spread#59: spread: do not panic if error message from google backend is empty [08:28] pedronis: great, thank you! [08:35] mvo: I don't understand the changer about errDoNothing in 5276 [08:40] mvo: I probably read the new code wrong but it's a bit unclear what's the intention of it [08:41] pedronis: let me double check [08:41] just avoid calling trivialSeeding twice? [08:41] I mean from two places [08:42] but then the comment in import Assertion is misleading [08:43] pedronis: thanks, let me rework this [08:45] mvo: I mean we will not use the model returned by importAssertionsFromSeed in that case [08:48] also it doesn't get set in state anyway so it wouldn't help later code that needs a model in all cases [08:48] Hi! Does anybody knows something the vim snap? https://forum.snapcraft.io/t/vim-snap/5573 [08:53] pedronis: yeah, I think this was a misconception my part, these bits can be undone [08:53] mvo: I suppose maybe because at some point you were passing model in trivialSeeding ? [08:53] pedronis: exactly [08:54] pedronis: using "core" for the config makes all of this much simpler [08:54] I see, anyway as I said return a model that way from importAssertionsFromSeed breaks its invariant so we would needed to do something else anyway [08:54] * pedronis errands [08:54] pedronis: there will be some small complications in the followups because some code checks for that the snap is installed when getting config [08:55] pedronis: thank you, I will update the PRs [09:06] re [09:06] pachulo: hey [09:07] pachulo: since snap crafters are the publisher I would suggest asking popey about it [09:16] mborzecki: my PR is green :) [09:16] no better chance to review it than now :) [09:16] oh I see it's reviewed already [09:17] * zyga will forever ponder what is the condition under GitHub updates the page without a reload [09:17] mvo: ^ quick 2nd review on 5277 [09:17] heh, github ui is terrible [09:17] I _like_ the way it looks but _hate_ the way it is stale without apparent reason [09:17] the funniest is when you comment on a hunk that gets changed in another patch, and you forgot to switch to 'show all patches' [09:19] thank you for the review on 5272 pawel [09:23] thanks zyga ! [09:23] pachulo: note that you also got a response on the foruom [09:23] *forum [09:24] jamie there is right that there should be a repository under snapcrafters on github [09:26] mvo: thank you mvo [09:26] mborzecki: to the point about GitHub UX being not too great, I got a comment (approval) from mvo and it was shown as a comment but the comment / approval count below was stale [09:28] PR snapd#5277 closed: cmd/snap-update-ns: add helper for checking for read-only filesystems [09:31] pedronis: thanks again for the review(s), I update the PRs, I added a testcase that loads a snap-setup without a type. interesstingly this does not crash, it only crashes if there is an empty (or invalid) "type":"" in the json [09:31] pedronis: but maybe you have something different in midn? [09:31] mind [09:48] PR snapd#5275 closed: cmd/snap: use snaptest.MockSnapCurrent in `snap run` tests [10:16] pachulo: wassup? [10:17] popey: is the vim snap owned by snapcrafters/ [10:17] popey: and if so, where is the snapcraft.yaml [10:17] sergio initially created the snap, sergiusens ^ [10:20] mborzecki: one more helper before the main meat of the validation logic [10:20] mborzecki: #5278 [10:20] PR #5278: cmd/snap-update-ns: add IsTrustedTmpfs and tests [10:20] mvo: ah see, no, nothing different, just forgot that behavior of the json umarshaller [10:20] PR snapd#5278 opened: cmd/snap-update-ns: add IsTrustedTmpfs and tests [10:24] pedronis: thank you! I feel much better having a test for this :) [10:26] mvo: I will re-review after lunch [10:26] Hi all. A notice and reminder that I'm off work today and tomorrow. [10:29] Chipaca: ack [10:31] pedronis: ta [10:31] Chipaca: enjoy! we miss you already [10:32] mvo: :-) [10:37] * zyga considers lunch for a change [10:45] Chipaca, after implementing the /v2/snaps?select=...&snaps=... API in snapd-glib, can you think of any reason to ever have a client use the /v2/snaps/[name] API? I'm thinking of deprecating those old methods. [10:46] I guess we continue to use it with POST, but not GET [10:49] robert_ancell: I'd say if there is it's because of a bug [10:49] Chipaca, so be clear, /v2/snaps/[name] is really redundant now? [10:50] robert_ancell: well, it's going to be faster but probably not an issue in practice [10:50] robert_ancell: but, yeah [10:51] and it might not even be measurably faster -- i'd have to try to measure [10:58] zyga ping [11:00] Go [11:02] ah, there is a bit of a difference /v2/snaps?snaps=blah returns 200 and /v2/snaps/blah returns 404 [11:03] zyga do we treat kernel snap differently when it comes to interfaces used by hooks? [11:04] Not that I know of [11:04] Kernel should not have interfaces or hooks AFAIR [11:08] hmm snapshots separate elements using _ in snaphot file names, will need some extra care for parallel installs [11:11] zyga I think joc spotted my error here [11:11] zyga indeed it's a bit special case I have here, but let's see :) [11:11] What is the use case? [11:12] zyga pi3 kernel update :) [11:12] Oj [11:12] Tell me more please [11:12] zyga let me proof tested first :) [11:12] Ok [11:18] zyga: I updated 5263 based on your feedback (thanks for this btw). also thanks to mborzecki [11:18] Ack [11:18] I will check after lunch [11:24] zyga, popey: that was my first classic snap ever, we didn't even have a build service yet iirc [11:34] one huge renames patch coming up, 97 files changed, 875 insertions(+), 702 deletions(-) [11:35] What is that [11:36] Switching to under_score ;-)) [11:36] I’m almost done with lunch [11:36] Will be home soon [11:42] mborzecki: why renaming the dir ones ? [11:43] me is probably missing something [11:44] pedronis: because some paths end up using StoreName() instead of InstanceName(), hence *Dir() were changed to make me go though each place and update it accordingly [11:45] mborzecki: why would a path use StoreName ? [11:45] genuine question [11:46] pedronis: eg. inside snap-exec after remount, apparmor profiles which refer to the paths inside the mount ns [11:47] mborzecki: but there is no store variant of them, you then do snap.MountDir(snapName, rev) and similar ? [11:48] pedronis: yes [11:48] and the Instance*Dir() use the same snap.*Dir() helpers [11:48] mborzecki: I suppose we could mount files at some point, not sure the it easy though [11:48] sorry [11:48] I mean share mount files [11:49] it'd be nice [11:49] right now each instance will get own copy [11:49] that's an easier starting point [11:49] there are fun questions also about conflicts [11:50] given that they can be on different revisions that's also a bit easier to handle [11:50] mborzecki: anyway afaiu snap-confine and snap-exec are the only things that need to deal with dirs without instance-key, right? [11:51] pedronis: and apparmor backend so far [11:52] mborzecki: I see an StoreName indeed, why ? [11:53] to repelace SNAP_NAME, but unclear SNAP_NAME can still be used in profiles? [11:54] ah, it's because inside vs outside? [11:57] mborzecki: ok, I see, we could really sort of keep the old names, but maybe the new names are clearer [11:58] because of this inside vs outside issue [11:58] pedronis: yup [11:58] pedronis: we can roll back the *Dir() renames later on when the store vs instance name thing is sorted out [12:01] mborzecki: anyway it's not tragically big, just a bit, anyway will need a span of quite time to go over it [12:03] mborzecki: did you see test that needed doubling, or did you still not look into that? [12:07] pedronis: i doubled only few tests, mostly in snap [12:09] mborzecki: most places that need StoreName probably need one, places using InstanceName it depends I suppose [12:10] re === pstolowski is now known as pstolowski|lunch [12:27] zyga: what comment should I add to 5263 [12:27] that there's no locking done by specific functions [12:27] (so that we don't forget this) [12:29] zyga: aha, ok [12:29] hmm [12:29] spread keeps crashing [12:29] zyga: will do [12:29] yeah, spread is not happy today [12:30] which is odd because some PRs are just ireland-grass-green [12:30] while others fail after 1st minute [12:31] mborzecki: I haven't really been following the conversation (I was conducting and interview-- note to channel: there are open positions on the Ubuntu Security team!) [12:32] mborzecki: but otoh the things to think about are file paths (both where snaps live, things are mounted, but also apparmor, seccomp, udev, dbus policy files) [12:33] mborzecki: then the security label ('profile foo' in apparmor policy) and the udev tag (which annoyingly must use underscores) [12:34] jdstrand: do the underscores have any special meaning there? [12:34] mborzecki: yes! :) they are the delimiter instead of '.'. udev tags can't have periods [12:35] mborzecki: eg: SUBSYSTEM=="drm", KERNEL=="card[0-9]*", TAG+="snap_0ad_0ad" [12:35] mborzecki: that's ok though. we are very strict that it is 'snap_name_cmd' [12:35] mborzecki: so to date, there will only ever be exactly two underscores [12:36] jdstrand: right, but then if you have snap_0ad_local_0ad it's fine too right? [12:36] that would be difficult [12:36] snap_0ad_0ad_local would be better [12:36] well [12:36] you could do it [12:37] i mean, the snap is named 0ad_local in this case [12:37] if count(underscores) == 3, then name is 0ad_local [12:37] it does mean we'll need to adjust that parsing everywhere though [12:41] and to finish my PSA for security team position: https://boards.greenhouse.io/canonical/jobs/1158266. it's a great team to work for! :) [12:42] jdstrand: hmm snap-device-helper seems to do some funny stuff with those names [12:43] jdstrand: I would totally apply if I hadn't a great team already [12:43] hehe [12:43] I'm not trying to poach snapd team members :) [12:43] there are others in the channel who may see it ;) [12:43] jdstrand: I know :) [12:44] jdstrand: still you guys have a great team [12:44] mvo: that said, you would be a wonderful addition, as would any of the snapd devs (but I'm not poaching-- I just love working with all of you. good thing I get to continue doing so :) [12:44] mvo: thanks! it's true. the security team is awesome [12:44] heh :) [12:44] * mvo blushes [12:46] mborzecki: right, it needs to convert back to '.' for the device cgroup name in /sys/fs/cgroup/devices [12:46] mborzecki: eg: /sys/fs/cgroup/devices/snap.firefox.firefox [12:46] mborzecki: so it would have to learn to do snap_firefox_local_firefox -> snap.firefox_local.firefox [12:47] jdstrand: right, so another small update there :/ [12:47] mborzecki: btw do you have that huge rename PR ready? [12:47] mborzecki: we try to use '.' as the delimiter everywhere we can for consistency (I tried to use '_' in the early days, but '.' was deemed prettier (is is)) [12:47] it* is [12:48] zyga: yes, #5253 [12:48] PR #5253: snap: introduce new fields for parallel snap installation [12:48] I'll review it after standupo [12:48] jdstrand: use # and watch as the world burns :P [12:48] hehe [12:50] zyga: I think you made me write locking code in the errtracker "db". its just too embarassing to write a comment that there is none [12:50] zyga: smart move ;) [12:50] hahaha [12:51] * zyga mutters "success" ;) [12:55] mborzecki: note that in the apparmor profile itself, the security label is set in various apparmor variables at the top: [12:55] @{SNAP_NAME}="0ad" [12:55] @{SNAP_REVISION}="18" [12:55] @{PROFILE_DBUS}="snap_2e0ad_2e0ad" [12:55] @{INSTALL_DIR}="/{,var/lib/snapd/}snap" [12:55] profile "snap.0ad.0ad" (attach_disconnected,mediate_deleted) { [12:55] mborzecki: you probably saw that, but I mention it specifically for PROFILE_DBUS [12:56] this is such an old concept it will take a while to change everywhere [12:57] mborzecki: that will probably just work, but do make sure you get snap_2e0ad_2elocal_2e0ad when have snap.0ad.0ad [12:57] mborzecki: sorry [12:57] jdstrand: SNAP_NAME=0ad_local, then PROFILE_DBUS will be snap_2e0ad_local_2e0ad [12:57] mborzecki: that will probably just work, but do make sure you get snap_2e0ad_2elocal_2e0ad when have snap.0ad_local.0ad [12:57] jdstrand: and profile "snap.0ad_local.0ad" [12:58] jdstrand: snap_2e0ad_2elocal_2e0ad instead of snap_2e0ad_local_2e0ad then? [12:58] snap.0ad_local.0ad translated to dbus is snap_2e0ad_2elocal_2e0ad [12:58] oh no [12:59] _2e is '.' [12:59] gimme a sec [12:59] mborzecki: seems SNAP_NAME is sometimes used in places that really need the instance name in the templates [13:00] what you use depends on how the mounts are setup [13:00] not all uses are for directories in the namespace [13:00] standup time [13:00] right [13:00] I don't know what is happening at the filesystem level [13:01] but you'll the security label to include _local for IPC, etc [13:01] you'll want [13:02] mborzecki: it would be easiest if the file accesses looked like /snap/foo_local/... instead of /snap/foo/... [13:03] jdstrand: hm well, it was requested that we bind mount to /snap/foo [13:04] mborzecki: but if that isn't possible/desirable, you'll need to introduce another apparmor variable: SNAP_NAME_FILE (or something) [13:04] eg: [13:04] @{SNAP_NAME}="0ad_local" [13:04] @{SNAP_NAME_FILE}="0ad" [13:04] with file accesses using @{SNAP_NAME_FILE} and everything else @{SNAP_NAME} [13:06] mborzecki: that is tricky though. $SNAP is easy enough, but SNAP_DATA, SNAP_COMMON and especially SNAP_USER_DATA and SNAP_USER_COMMON are going to be harder, since you'll have to manage all those addition bind mounts [13:06] it is possible, just more complexity [13:07] it is only possible because of the recent user mount work btw since a root processes will be mucking around in the user's home for SNAP_USER_DATA and SNAP_USER_COMMON [13:09] mvo: https://github.com/snapcore/snapd/pull/5263 is green and has +2 [13:09] we are setting ourselves up for if there is a bug, then 0ad_local might be able to write to 0ad's user data. from a complexity/secure design perspective, I recommend 0ad_local everywhere [13:09] PR #5263: errtracker: do not send duplicated reports [13:10] mborzecki: ^. that doesn't mean I would block the design. but I think that there needs to be an active decision that the benefit outweighs the complexity/risk [13:12] mborzecki: I would like to be involved in the PR reviews as they pertain to security [13:12] jdstrand: sure [13:12] * jdstrand adds a card to trello so ratliff is aware [13:12] ratliff: this'll be a blue item [13:13] * ratliff reads back to understand the priority [13:15] mborzecki: hmm, I'm not sure how parallel installs are going to work with IPC. eg, two network-managers or two dockers installed [13:16] mborzecki: they necessarily need to have their service in the global namespace, so will conflict with each other... [13:18] jdstrand: i suppose common sense will have to prevail here, unless the other docker instance is configured to listen on different path things will break [13:19] jdstrand: maybe postgres usecase is simpler, local snaps on different ports (docker mangles some kernel state so it's probably not the best thing to be installed mulitple times) [13:19] mborzecki: yes... perhaps we can start be saying "if you implement a slot, you can't do this" [13:20] mborzecki: that said, gnome apps all need to slot a dbus interface for the well-known name [13:20] mborzecki: so they are going to break if parallel installed [13:21] (and it isn't just gnome) [13:21] jdstrand: this, probably snaps that have socket activation too [13:21] yeah [13:22] at least using abstract socket paths [13:22] mborzecki: can you describe the feature of parallel installs? is it documented somewhere? [13:22] jdstrand: https://forum.snapcraft.io/t/parallel-snap-installs/5763 [13:23] mborzecki: well, any path where the client expects to find the server somewhere specific. it could be a dbus well-known name, abstract socket, named socket, pipe, ... all kinds of stuff [13:24] jdstrand, mborzecki: Can we have a quick sync up call shortly? (after standup, on going) [13:24] niemeyer: ok [13:25] niemeyer: maybe after that we can discuss update-alternatives and interface docs [13:25] jdstrand: Sounds good! [13:27] ratliff: I created a preliminary card in trello === pstolowski|lunch is now known as pstolowski [13:39] uhhh [13:39] my coffee machine erupted :/ [13:39] niemeyer: pstolowski: this can be re-reviewed now: https://github.com/snapcore/snapd/pull/5221 [13:39] PR #5221: snap: parse connect instructions in gadget.yaml [13:39] ok [13:39] it punctured the capsule incorrectly and all the coffee went outside the wrong way [13:40] zyga: Perhaps it's one of those recalled Keurigs [13:40] no, I'm not familiar with those [13:40] zyga: just get a moka pot ;) [13:40] I'll get a mop and a pot ;) [13:54] guys, if you see econnreset test failure again please let me know, i lost yesterday's log when I re-loaded the tab with travis log today [13:56] pstolowski: should we add some logging to the retry logic to see the error when we don't retry? [13:57] pedronis: yep, that may help [14:00] niemeyer, mborzecki: I tried to summarize much of the above in https://forum.snapcraft.io/t/parallel-snap-installs/5763/3 [14:03] jdstrand: thanks! [14:08] zyga: a second review for 5274 would be great, you did a first pass already afaict [14:10] ACLU [14:10] Ack [14:10] I wonder what spellchecker contains ACLU [14:10] niemeyer, mborzecki: I'm ready whenever you are [14:11] jdstrand: Just finishing a pre-scheduled meeting I had until the half hour.. should be off in 20 or so [14:11] ack [14:18] PR snapd#5279 opened: interfaces/builtin: create socketcan interface [14:30] fyi, I've added a review of PR 5279 to my list. I have a number of questions. others might want to wait until I do my first review [14:30] PR #5279: interfaces/builtin: create socketcan interface [14:31] joc: fyi ^ (and thanks for the PR. I'm stepping into a meeting so it'll be a bit) [14:31] np, thanks for looking at it jdstrand [14:35] jdstrand, mborzecki: https://meet.google.com/dnp-muwd-mng [14:36] PR snapd#5280 opened: httputil: extra debug if an error is not retried [14:41] who can I poke to get a human reviewer for https://launchpad.net/~gerboland/+snap/chromium-mir-kiosk/+build/241717 [14:41] PR snapcraft#2128 closed: project_loader: stop setting LD_LIBRARY_PATH [14:45] ah, there's a button. ignore me [15:01] jdstrand: hey, around [15:05] didrocks: hey, so we were thinking we were going to have the update-alternatives meeting now, but it'll be in an hour or so [15:06] didrocks: your attendance isn't stricly required, so if you can't attend that's ok, but if you can that would be great. is that an ok time? [15:06] jdstrand: hum, I can't be around at that time, I have some family duties [15:06] jdstrand: I don't think indeed that I'm required, you know update-alternatives as well as I do if not better :) [15:07] didrocks: that's fine. I understand the problem and even the specifics of the access you desire [15:07]