[05:11] morning [06:26] good morning [06:30] PR snapd#9198 closed: features: add HiddenSnapFolder feature flag [06:36] zyga: hey [06:36] hey :) [06:36] finally some cold air, eh? [06:36] yeah, much cooler (and nicer) [06:37] later summer/early autumn, cold mornings/evening, warm during the day [06:37] s/later/late/ [06:39] errand, back ~11 [06:39] mvo: hey [06:39] irc, eh,... [06:39] good morning mborzecki and zyga [06:40] need to run an errand, back ~11 hopefully [06:40] good morning mvo [06:40] mborzecki o/ [06:40] * zyga waits for Lucy to wake up [06:41] * zyga goes to squash merge https://github.com/snapcore/snapd/pull/7825 [06:41] PR #7825: many: use transient scope for tracking apps and hooks

[06:41] just need to think of a proper commit message [06:43] is anyone else using "git reflog" like the recent call lists on pre-smartphones? [06:45] PR snapd#9204 opened: sandbox: track applications unconditionally [06:46] ^ this is not yet ready for review [06:46] I want to see how it affects our tests [06:57] I'll go do some reviews [06:57] and then break to handle lucy being awake [06:57] and should then return for 1:1 and remaining work [07:00] PR snapd#7825 closed: many: use transient scope for tracking apps and hooks

[07:03] morning [07:03] good morning Pawel [07:41] good morning pstolowski [07:42] o/ [07:48] just fyi (all) 8982 needs reviews, samuele is happy with it on a high level but did not do a full review (not urgent though) [08:12] ack [08:36] mborzecki: hey, i've updated selinux profile in #9084, can you take a look (last commit)? [08:36] PR #9084: o/snapstate: check disk space before creating automatic snapshot on remove (3/N) [08:53] In the office [09:08] mborzecki hey [09:08] mborzecki I've proposed a draft that attempts systemd-based app tracking by default [09:08] and I got a few denials for selinux [09:08] 1) type=AVC msg=audit(08/24/20 07:27:34.527:12644) : avc: denied { getattr } for pid=78926 comm=snap path=/run/user/0/bus dev="tmpfs" ino=22956 scontext=system_u:system_r:snappy_cli_t:s0 tcontext=unconfined_u:object_r:user_tmp_t:s0 tclass=sock_file permissive=1 [09:09] 2) type=AVC msg=audit(08/24/20 07:27:34.528:12645) : avc: denied { write } for pid=78926 comm=snap name=bus dev="tmpfs" ino=22956 scontext=system_u:system_r:snappy_cli_t:s0 tcontext=unconfined_u:object_r:user_tmp_t:s0 tclass=sock_file permissive=1 [09:09] 3) type=AVC msg=audit(08/24/20 07:27:34.529:12646) : avc: denied { connectto } for pid=78926 comm=snap path=/run/user/0/bus scontext=system_u:system_r:snappy_cli_t:s0 tcontext=unconfined_u:unconfined_r:unconfined_dbusd_t:s0-s0:c0.c1023 tclass=unix_stream_socket permissive=1 [09:09] I would appreciate some ideas on how to tackle that [09:10] those are from fedora 32 [09:12] mborzecki I've added a comment on https://github.com/snapcore/snapd/pull/9204 with the same information [09:12] PR #9204: sandbox: track applications unconditionally [09:23] re [09:27] mborzecki: hi, I did a pass on #9201 [09:27] PR #9201: [RFC] boot: observe update & rollback of trusted assets [09:31] pedronis: thanks, would have some time for a chat later? [09:31] mborzecki: before the standup [09:31] pedronis: sounds good [09:31] let me add something [09:35] zyga: looks like this should be covered by dbus chat interfaces [09:35] zyga: let me see if there's something for chattign with systemd specifically [09:35] thank you! [09:40] zyga: there's a bunch of systemd_dbus_chat in refpolicy but those seem to cover logind, timedated, machined, resolved [09:40] do we need a new interface or is there a better way out? [09:43] zyga: hm but it's user_tmp_t, can you try adding userdom_write_user_tmp_sockets(snappy_cli_t) to the policy, reload and see what happens? [09:45] mborzecki sure, I'll try [09:49] mborzecki: heh, my selinux fix worked everywhere except for centos 8. although there i see "type=AVC msg=audit(1598261418.464:4691): avc: denied { getattr } for pid=133098 comm="snapd" path="/var/snap/lxd/common/ns/shmounts" [09:50] pstolowski: why is snapd looking there? [09:50] mborzecki: probably because "du" [09:50] to estimate size of the data [09:50] yes exactly [09:50] in a way we need one-file-system that really means ignore-magic-filesystems [09:53] pstolowski: zyga: there's --one-file-system switch to du [09:53] but we don't really want that [09:53] we want to allow people to have other file systems mounted on /var/snap/blargh [09:53] what we want is to filter out nsfs [09:54] or procfs [09:54] zyga: should those other filesystems count towards snapshot size? [09:54] that's interesting. i wonder what happens when we do actual snapshot [09:55] it probably archives an empty file [09:55] it's a permissive profile [09:56] it's a bind mount of /proc/PID/ns/mnt to an empty file [09:56] so the archiver will just see the empty file [10:03] yeah but what about the rest of proc\ [10:03] pstolowski is all of proc mounted in SNAP_DATA? [10:03] zyga: i don't know, re-running to see what happens (it's a lxd snap_ [10:03] ) [10:03] pstolowski I think you will only find nsfs [10:04] the rest of the filesystems are mounted inside that thing [10:04] it's like a chest for more mounts [10:07] mborzecki I have one deny left [10:07] mborzecki I have one deny left [10:08] type=AVC msg=audit(08/24/20 10:05:41.032:928) : avc: denied { connectto } for pid=29450 comm=snap path=/run/user/0/bus scontext=system_u:system_r:snappy_cli_t:s0 tcontext=unconfined_u:unconfined_r:unconfined_dbusd_t:s0-s0:c0.c1023 tclass=unix_stream_socket permissive=1 [10:09] brb [10:09] zyga: ok, so you probably need unconfined_dbus_chat(snappy_cli_t) now [10:09] morning folks [10:09] hey ijohnson :) [10:09] mborzecki trying [10:09] good morning ijohnson [10:09] o/ [10:09] mmm have we not had a new snapd edge since 8-22 ? [10:09] zyga: and maybe unconfined_dbus_connect(snappy_clit_t) too [10:09] ijohnson: hey [10:10] or 22-8 for y'all europeans :-) [10:10] hi ijohnson ! [10:10] hey pstolowski [10:10] ijohnson looking [10:10] yeah [10:10] it seems so [10:11] smells odd [10:11] ijohnson snapd had a long weekend? :D [10:11] haha maybe [10:11] either stuck in moderation or the publish pipeline got blocked somewhere [10:15] ijohnson: probably no changes in edge since 22 ? [10:16] mvo: ah actually you're right there were a couple commits this morning but nothing over the weekend [10:16] I'm just so used to seeing the snapd snap update every day [10:22] re [10:24] ha, does it mean we had a long weekend instead? [10:24] as in two days without patches [10:25] ijohnson https://listed.zygoon.pl/17659/introduction-to-bashunit-unit-testing-for-bash-scripts :) [10:39] do we hijack the post-refresh hook for the core* snaps as well as the configure hook ? [10:42] zyga: very cool stuff! that's really nice to see it all come together like that, one thing I wonder though is that in the failure output you have for bashunit you don't see what the actual output of `hello_world` function is before the grep fails, so it's a bit difficult to debug [10:43] zyga: that's probably intristic to using bash however so probably not worth looking into, but I wonder if it would be helpful to have some kind of test util command you can use in the unit test file that saves the output inside pipes only to be displayed on test failure [10:43] zyga: something like `hello_world | echo_on_fail | grep -qFx 'Hello world'` [10:43] just a thought [10:44] I've wanted something like that _so_ many times when looking at spread failures [10:48] * zyga-x240 switches devices [10:51] mvo: 2.46 branches are red, do you want to merge master or cherry pick all the fixes back> [10:51] zyga-x240: will merge master to it today [10:52] ok [10:52] do 2.46 jamie branches make sense to review then? [10:56] zyga-x240: no, please not [10:56] zyga-x240: in a meeting now [10:56] ok [10:56] zyga-x240: we can close them [11:16] mvo: ack, doing that now [11:16] ta [11:21] PR # closed: snapd#9192, snapd#9193, snapd#9194, snapd#9195 [11:25] real simple uc20 PR: https://github.com/snapcore/snapd/pull/9205 [11:25] PR #9205: boot/initramfs_test.go: reset boot vars on the bootloader for each iteration [11:26] PR snapd#9205 opened: boot/initramfs_test.go: reset boot vars on the bootloader for each iteration [11:26] is shell surprising? https://paste.ubuntu.com/p/FGYyvVXmm8/ [11:28] our || true pattern is dangerous [11:28] wow just wow [11:28] how can such fundamental things in bash be so broken [11:28] ijohnson: it's documented :) [11:28] it's a feature [11:28] just ill-designed IMO [11:28] I think we can do something like "not" [11:29] didn't we have a NOT in spread ? [11:29] we have "not" in snapd [11:29] it's really exactly the same feature in bash that requires it [11:30] yes. it's surprising :-) (not what I expect with `set -e`) [11:30] diddledan: it's just another case of https://listed.zygoon.pl/17629/broken-composition-or-the-tale-of-bash-and-set-e [11:30] I guess you need to add set -e inside the function? [11:31] diddledan: but just seeing the code behave this way is shocking [11:31] diddledan: no :) [11:31] diddledan: try that [11:31] it's ignored [11:31] gah [11:31] the link I referenced explains why [11:31] * diddledan reads it [11:31] * zyga-x240 reviews nested.sh [11:32] it's the same with /bin/sh (which in Ubuntu 20.04 is dash, right?) [11:32] yes [11:32] it's a very old feature [11:33] the `do-something && do-something-on-success || do-something-on-fail` is a common pattern that I've seen all over the interweb [11:35] diddledan: it all depends on what those are [11:35] diddledan: also, remove the echo that says "surprising" and it's well, also documented but more surprising [11:37] gosh, there's a lot of strange interactions you've highlighted [11:42] mvo: shellcheck issue (reported in early 2019) https://github.com/koalaman/shellcheck/issues/1484 [11:44] cachio: how much work would it take to make nested.sh a non-sourced script? [11:44] morning cachio [11:45] zyga-x240, hi, I already did something like that a time ago [11:45] ijohnson, hi [11:45] cachio: oh? where? [11:45] good morning [11:45] zyga-x240, I closed that a time ago [11:45] why? [11:45] I need to open it again [11:46] because there were too many changes [11:46] I'm reading https://github.com/snapcore/snapd/pull/9098 and I'm very worried about bugs [11:46] PR #9098: tests: new organization for nested tests [11:46] well, we can start small [11:46] Iand I decided to create a new change to address that [11:46] but I think we must go that way [11:47] I could make it again [11:47] please start small [11:47] it could be a tool [11:47] prepare/restore + execute [11:47] yeah, I think sourcing is a no-go [11:48] port a single test (keep nested.sh as-is) [11:48] over time it will replace the other [11:49] zyga-x240, ok, makes sense [11:49] cachio: why is execute remote using "$*"? [11:49] https://github.com/snapcore/snapd/pull/9098/files#diff-3af5dfa44ec70d885e9485dbb117f52fR844 [11:49] PR #9098: tests: new organization for nested tests [11:51] zyga-x240, it is very old [11:51] zyga-x240, I think federico created that [11:51] I think that's wrong [11:51] and nobody updated that [11:51] and I suspect it's broken with regards to quoting [11:52] cachio: I did a quick pass over https://github.com/snapcore/snapd/pull/9098#pullrequestreview-473374133 [11:52] PR #9098: tests: new organization for nested tests [11:52] zyga-x240, nice, thanks [11:52] I'll take a look [11:53] cachio: let me know if anything I said there is unreasonable please [11:54] cachio: please look through all the functions, they should not create new globals unless that's exactly desired [11:54] e.g. start_nested_classic_vm defines a dozen or so new globals [11:55] zyga-x240, ok, np, I'll update that [11:55] cachio: wait_for_ssh defines retry as a global [11:55] please go through the entire file [11:56] zyga-x240, ok, I'ldd [11:56] I'll do [11:57] thanks, [11:57] the more we learn about shell the more we need to be careful [11:58] zyga-x240, yes hehehe [12:25] mborzecki: pondering what to do about that second denial on centos8 only; is "allow snappy_t unconfined_service_t:file getattr;" too terrible? [12:25] pstolowski: wouldn't that only mask the issue? [12:27] mborzecki: fwtw these files appear empty, and are included in actual snapshot [12:30] pstolowski: they are empty [12:31] pstolowski: they are mount points to contain nsfs objects [12:32] PR snapcraft#3260 opened: tools: update setuptools in environment-setup [12:34] * zyga-x240 -> lunch [12:46] PR snapd#9160 closed: boot, o/devicestate: observe existing recovery bootloader trusted boot assets [12:50] degville: hi, welcome back! it seems you made a nice shot of milky way but unfortunately for some reason i see very pixelated/blurry pictures, something wrong with google photos here i suspect [12:52] zyga-x240: what's your take on passing function names to a bash function? I want to inject any function to be called during the execution of another function, but only sometimes, something like passing a func() as a param in Go [12:52] zyga-x240: does my idea seem terrible given your current experiences ? [12:53] I could just add all the functions I need inside the main function and just use a silly named option/switch to the function when I call it [12:55] ijohnson: re [12:55] ijohnson: let me read backlog [12:55] zyga-x240: sure no rush [12:55] ijohnson: no, not really [12:55] ijohnson: there's even a way to do "pointers" in bash if you need to [12:56] ijohnson: the more important detail is exactly how is the function called [12:56] ijohnson: I think that disabling set -e, calling the function, recording $? and re-enabling set -e is probably correct, to the best of my understanding [12:56] ijohnson: some simpler functions are also correct in more generic cases, those that involve a single command [12:57] zyga-x240: the function I need to call is very simple, but it needs to be called at a very specific point in time [12:58] ijohnson: I'm happy to review and suggest improvements [12:58] but actually I think I can get away with just a single implementation, thinking about it I don't know that I need multiple different functions to be called so I think I'll just use a special optional argument to the original function I need to call [13:15] pstolowski: thanks for letting me know (and the welcome back!) - I'll check, but it's pretty low quality anyway as it was just a long exposure shot taken with my phone. I do have a RAW version, so I may try and play with it in Darktable. [13:23] PR snapcraft#3261 opened: requirements: pin setuptools devel requirement [13:29] zyga-x240: there's no way to set the 'script' interpreter in spread, or is there? [13:29] mborzecki: no and in addition, spread joins many separate task together [13:29] IIRC [13:32] let's all switch to julia instead of bash [13:32] alternative to julia would be the good ol lisp [13:34] ijohnson: I think js is the most realistic alternative [13:34] * ijohnson really doesn't wanna write js tho [13:34] it's not great but has arguably the best tooling [13:34] I see your clojures in js and raise you an offer of erlang [13:35] I think it's not something we should pick in a rush [13:35] maybe figuring out how to transition to anything is more relevant [13:35] then we can consider alternatives [13:37] zyga-x240, I see this error https://github.com/snapcore/snapd/runs/1021534236#step:5:1744 [13:38] todya the image has been updated [13:38] did you already see that? [13:40] cachio strange, is that image changing from efi to legacy boot? [13:42] no [13:42] zyga, it shouldn't [13:42] I am trying to reproduce [13:44] ok [14:11] PR snapd#9206 opened: boot: complain about reused asset name during initial install [14:31] snap wait --help panics [14:32] pstolowski snap save on f32 says "broken: invalid snapshot" [14:33] https://pastebin.ubuntu.com/p/cxHqJ9MZQn/ [14:34] zyga: backtrace goes to go-flags [14:34] maybe go-flags is old [14:35] zyga: maybe a particular version is broken [14:35] the beauty of maintaining packages [14:36] zyga: ? is this my PR? [14:36] nope [14:36] mborzecki it creshes on [14:36] descPadding := strings.Repeat(" ", descStart-len(argPrefix)) [14:36] afk [14:37] dog needs to go out [14:41] we run snapshot-basic spread test everywhere, are you sure it's not some local issue on your f32? [14:48] pstolowski: I did a pass on #9199 [14:48] PR #9199: snapstate: installSizeInfo helper that calculates total size of snaps and their prerequisites [14:49] pedronis: just saw it, thanks [14:51] mborzecki: does the selinux workaround in #9084 look fine to you? it passed now (except for unrelated test failure on centos8 and suse) [14:51] PR #9084: o/snapstate: check disk space before creating automatic snapshot on remove (3/N) [14:55] pstolowski: yeah, i don't think we have any other options at this point [14:56] mborzecki: ok thanks, i'll land this [15:01] PR snapd#9084 closed: o/snapstate: check disk space before creating automatic snapshot on remove (3/N) [15:03] pstolowski: still, it'd be nice to maybe ask the lxd team what that ns/shmounts contains, my rough guess it's some bit of /proc/self/ns/.. of the lxd process, hence the unconfined_t label (and nsfs fstype) === davdunc_ is now known as davdunc === bluesabre_ is now known as bluesabre [15:07] pedronis: interesting, so the idea is to essentially exclude snaps that could have been installed in the meantime when computing total? [15:07] * cachio lunch === mborzeck1 is now known as mborzecki [15:23] pstolowski: yes, as I said, it doesn't cover all cases, but it get us closer to the kind of code that we would need for that [15:26] pedronis: right, got it, thanks [15:34] * zyga-x240 goes for PT [15:38] pstolowski: I updated 8982 [15:38] pstolowski: thanks for your review, the misgging BadRequest test highlighted a bad error message [15:38] mvo: yes, i looked briefly, thanks for the changes [15:39] 👍 [15:53] +1 [16:15] yay now 8982 just needs a second review [16:17] PR snapd#9207 opened: boot/bootstate20: reboot to rollback to previous kernel [18:01] re [18:02] today's exercise was *intense* [18:02] I'm genuinely tired [18:02] * zyga-x240 needs water [18:18] * zyga-x240 is rehydrated [18:18] let's write some ocd [18:18] *code [18:32] * zyga-x240 goes upstairs [18:53] PR core20#82 opened: [RFC] hooks: mv docker user/group definition to extrausers [19:22] PR snapd#9208 opened: tests/nested/core20/kernel-failover: add test for failed refresh of uc20 kernel [19:25] * ijohnson EODs