[07:05] morning [07:42] PR snapd#10034 closed: tests/main/uc20-create-partitions: fix tests cleanup [08:00] morning [08:01] pstolowski: hey [08:15] good morning pstolowski and mborzecki [08:22] mvo: hey! [08:22] thanks for landing 10034 [08:22] PR snapd#10036 opened: interfaces/apparmor: allow reading /proc/sys/kernel/random/entropy_avail [08:32] my pleasure [09:54] mborzecki: hi, I re-reviewed #10006 [09:54] Bug #10006: python libglade program crashes on load [09:54] PR #10006: cmd/snap-bootstrap: refactor handling of ubuntu-save, do not use secboot's implicit fallback [09:59] pstolowski: hi, I have a question/consideration in #9959, does it make sense? [09:59] PR #9959: o/snapstate: update validation sets assertions with auto-refresh

[09:59] looking [10:01] pedronis: ah yes, makes sense, thank you [10:07] mborzecki: #10022 needs a 2nd review [10:07] Bug #10022: don't build-depend on mingw32 if possible [10:07] PR #10022: boot, o/devicestate: split makeBootable20 into two parts [10:07] pedronis: i'll take a look, and thanks for a review in 10022 [10:48] pedronis: i've updated 10006 [10:52] mborzecki: can you tell me more about what happens with the locking of keys in that test? you just removed the mocking now [10:54] mborzecki: TestInitramfsMountsRecoverModeDegradedEncryptedDataUnencryptedSaveHappy, this is also happy because Save is ignored again? [10:55] pedronis: yeah, i was confused by the test code previously, testRecoveryHappymode() installs a separate mock which does the right check, but i forgot to remove the one in the test before committing and pushing [10:56] and it does indeed check that the lock code is called always [10:56] ah, I see that now, thx [10:59] pedronis: as for the test cases i actually consider them happy in the sense that s-b does not fail, though the one with unencrypted data & encrypted save is rather artificial at this point [10:59] mborzecki: I +1ed but it needs a thorough review from Ian, also left a couple final comments [10:59] thanks! [11:12] mborzecki, hi [11:12] cachio: hi, what's up? [11:12] I see some tests show [11:12] tests.invariant: it seems snap-confine has crashed [11:12] I reproduced that and I see the directory /tmp/snap.rootfs_YwlgKW created is empty [11:13] any idea in case the dir is empty it failed? [11:13] because we are making a test fail but there are not any logs, so no way to understand what happened [11:14] dmesg and journal logs are not showing any useful info [11:16] cachio: yeah, saw that too, but i don't any ideas at this time the directory is supposed to be removed by snap-confine, and it will abort if it cannot do so, hope that #10033 will help tracking it down [11:16] Bug #10033: bittornado: new changes from Debian require merging [11:16] PR #10033: tests: run the reset.sh helper and check test invariants while the test is restored [11:17] pedronis: wrt refresh control, is there a good way of knowing upfront if reboot will be required? i'm looking at boot participant but it modifies the system (requests reboot already) [11:18] (sorry, not requests the reboot but prepares for it, to be precise0` [11:18] ) [11:25] pstolowski, hey, could you plase take a quick look to 10026? [11:26] cachio: sure [11:26] tx [11:31] +1 [11:34] pedronis: I added a bunch more tests to 9981, hopefully captures what we discussed last week [11:34] pedronis: (not urgent) [11:36] pstolowski: not in general, I think we need to be conservative for a while [11:37] mvo: thx, it's in my queue [11:38] pedronis: i.e. just assume kernel/gadget triggers reboot? [11:38] pstolowski: kernel always does, gadget sometimes, but yes [11:39] pstolowski: we can refine this later [11:39] if possible [11:40] pedronis: ok [12:08] PR snapd#10026 closed: tests: use retry tool instead a loops [12:14] pedronis: do you want to take a look at https://github.com/snapcore/snapd/pull/10036 before I land it? [12:14] PR #10036: interfaces/apparmor: allow reading /proc/sys/kernel/random/entropy_avail [12:15] mborzecki: lgtm [12:15] ack [12:18] PR snapd#10036 closed: interfaces/apparmor: allow reading /proc/sys/kernel/random/entropy_avail [13:18] PR snapd#10037 opened: boot: export helper for clearing tried system state, add tests [13:33] PR snapd#10038 opened: tests: replace while commands with the retry tool [14:09] * pstolowski bbl [15:17] mvo, https://github.com/snapcore/spread/pull/82 [15:17] PR spread#82: Introducing tags for the test which allows to filter executions [15:18] this is the pr to add tags toe tests in spread [15:18] perhaps I could add a way to mix tags [15:18] so we run tests wich match with more than 1 tag [15:19] any feedback is appreciated [15:19] pedronis, your feedback is also appreciated [15:21] cachio_: I have a look [15:24] mvo, tx [15:36] re [15:58] * cachio_ lunch [16:25] ijohnson: I reviewed (finally) #9965, the code looks good but the flag/comments/usage seem confusing to me, let me know whether my comments make sense or not [16:25] PR #9965: sysconfig, o/configstate/configcore: first step to image build pi-config application [16:25] pedronis: thanks let me take a look at your review [16:26] and yes I fully expected the names of things to need some work on that pr [16:28] pedronis: I see your points about the some of the flags not making sense or not being clear enough, those all seem reasonable, I will try to address that today [16:29] ijohnson: my other main point is that the new function needs to be pointed at something quite different recovery/seed vs rootfs [16:29] if I understand correctly [16:30] pedronis: yes the rootDir is indeed a recovery system for the preinstall vs a rootfs for the early / initramfs fs only config [16:30] ok [16:31] at least I understood things, so I think my suggestions are in the right direction [16:32] yes I agree they are [16:54] mvo: pedronis: could I trouble one of you for a sudo merge of #9924? the store is on fire right now, and the tests that failed there are all unrelated to the pr [16:54] PR #9924: interfaces/docker-support: add autobind unix rules to docker-support [16:55] ijohnson: sure, let me do that [16:55] thanks [16:55] ijohnson: and you can trouble me anytime [16:55] careful what you wish for! [16:56] mvo: while you are sudo merging, you could also merge #10022, it is mostly green except one unrelated failure [16:56] Bug #10022: don't build-depend on mingw32 if possible [16:56] PR #10022: boot, o/devicestate: split makeBootable20 into two parts [16:56] ijohnson: consider it done [16:56] oh no, pr numbers in snapd have gotten so high that mup can't tell if we mean lp bugs or snapd prs :-O [16:57] ijohnson: I think we need to ask gustavo to bump it to 20000 [16:57] * ogra got curious when we'll see the mingw version of snapd [16:57] mvo: yeah probably [16:58] ogra: we will see mingw snapd when win32 is implemented by wsl [16:58] haha [16:59] PR snapd#9924 closed: interfaces/docker-support: add autobind unix rules to docker-support [16:59] PR snapd#10022 closed: boot, o/devicestate: split makeBootable20 into two parts [17:01] mvo: I looked again at #9981, asked some questions there [17:01] PR #9981: gadget: policy for gadget/kernel refreshes [17:03] mvo: I looked at the new commits [17:03] mostly [17:04] pedronis: in #10005, you are limiting yourself to snap-revision timestamps, but xnox made a point on the associated forum topic about using also the account-key which signed the model assertion, as that timestamp will (in the worst case) be the newest thing available, is there a reason we can't use any assertion that is valid to move time forward ? [17:04] Bug #10005: network properties revert after reboot [17:04] PR #10005: seed: ReadSystemEssentialAndBetterEarliestTime [17:04] pedronis: thanks, will reply later or in my morning! I need to think about if we need to check for "f the kernel has assets and update: true the gadget does use at least one of the kernel assets" - the helper may be wrong I will double check, maybe silly me [17:04] ijohnson: I use the model timestamp too [17:04] itself [17:04] oh sorry didn't actually finish reading the function I see that now [17:05] ijohnson: no we can't use any assertion, they could be signed by neither the brand nor the store [17:06] and relevant account-key themselves must be older than the things signed with them [17:07] mvo: I think the helper does the other direction [17:07] which is just failing early [17:08] pedronis: interesting. Yes, we need to only trust things that are chained to the root of trust. [17:08] you guys are probably aware, but the snapstore is down [17:08] pedronis: can you detect when assertions are signed by the store though? i.e. if they chain to the root of trust, trust their timestamps? [17:09] (well i guess we can't trust the timestamp that one signed locally and pushed to the store) [17:09] sigh [17:09] our ci is showing https://paste.ubuntu.com/p/b7w2RdjjbC/ [17:10] users and getting "504 Gateway Time-out" in their browsers [17:11] https://status.snapcraft.io/ [17:12] pedronis: thx [17:14] pedronis: so LoadAssertions() will load assertions into the DB that might not be signed by an unknown account-key ? [17:14] pedronis: ah I see the attack where a legitimate user creates an account-key which is signed by the store, then puts it into the image along with another assertion they signed themselves with that key but this account-key would have a root of trust that doesn't involve the brand account-key [17:14] do we currently validate that snap-revision assertions are signed by the store account-key and only the store account-key? [17:14] ijohnson: yes we do [17:15] pedronis: ok, then the code you have is fine I think [17:15] well I should rephrase, I am clear on the intent of the code and it makes sense enough that I can continue with my review [17:19] ijohnson: if you want to see how, see (snaprev *SnapRevision) checkConsistency in snap_asserts.go [17:24] pedronis: got it thanks for that, what do you think about expanding to also allow snap-declaration assertions given that we have a very similar check for snap-declaration as we do snap-revision assertions? === ijohnson is now known as ijohnson|lunch [17:27] ijohnson|lunch: please add a comment about that, we can (it's rare but can happen that the snap-declaration has been revised an is newer than the snap-revision) [17:28] ack [17:44] PR snapd#10039 opened: daemon: switch preexisting daemon_test tests to apiBaseSuite and .req [18:10] PR snapcraft#3471 closed: Add flutter-stable and -beta extension variants === probono9 is now known as probono === kali___ is now known as kaliy [22:39] this snap prometheus-alertmanager seems to load its config from /var/snap/prometheus-alertmanager/11/alertmanager.yml -- can I change that config? [22:39] its not in the current/common directory, so I'm unsure [23:27] micah, look at "ls -lh /var/snap/prometheus-alertmanagercurrent" .... and you'll find its just a symlink 😉 [23:28] ogra: yeah, but when it gets upgraded (presumably to 12) then my config file in 11 wont be used I guess [23:30] when 12 gets installed the content of 11 gets copied to 12 first ... and after the upgrade the symlink points to 12 [23:31] ah ok! I didn't know about copying to the next one [23:31] and in case you decide t run "snap revert " the symlink will point back to 11 and the binary from 11 will be executed [23:31] that way the matching config will always run with the corect binary [23:40] thanks ogra