| === KindTwo is now known as KindOne | ||
| === KindTwo is now known as KindOne | ||
| === KindTwo is now known as KindOne | ||
| mborzecki | morning | 05:12 |
|---|---|---|
| mborzecki | mvo: hi, there's a typo in https://github.com/snapcore/snapd/pull/8822 that static checks choke on | 06:03 |
| mup | PR #8822: release: 2.45.1 <Created by mvo5> <https://github.com/snapcore/snapd/pull/8822> | 06:03 |
| mvo | mborzecki: meh, ok | 06:05 |
| mvo | mborzecki: thank you, will fix | 06:05 |
| zyga | hey guys | 06:12 |
| mborzecki | zyga: hey | 06:29 |
| zyga | hey | 06:29 |
| mborzecki | zyga: how is your back? | 06:29 |
| zyga | thinking about how to say it | 06:29 |
| zyga | f*** uP? | 06:29 |
| zyga | not good for sure | 06:30 |
| mborzecki | zyga: sad to hear that | 06:31 |
| mvo | hey zyga - good morning | 06:39 |
| zyga | hey mvo | 06:39 |
| pstolowski | morning | 07:03 |
| mvo | hey pstolowski - good morning | 07:09 |
| mup | PR snapd#8804 closed: tests: port xdg-settings test to tests.session <Test Robustness> <Created by zyga> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8804> | 07:23 |
| mup | PR snapd#8815 closed: tests: port snap-handle-link test to tests.session <Test Robustness> <Created by zyga> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8815> | 07:23 |
| zyga | thanks mvo! | 07:32 |
| mvo | zyga: thank *you* | 07:32 |
| zyga | mvo: MRI on Wednesday at 15:00 | 07:40 |
| zyga | so before then I'm working from bed | 07:40 |
| mvo | zyga: ok | 07:41 |
| mup | PR snapd#8825 opened: tests: move a few more tests to snapstate_install_test <Test Robustness> <Created by stolowski> <https://github.com/snapcore/snapd/pull/8825> | 07:43 |
| zyga | https://github.com/snapcore/snapd/pull/8826 | 07:44 |
| mup | PR #8826: tests: assorted small patches <Simple 😃> <Created by zyga> <https://github.com/snapcore/snapd/pull/8826> | 07:44 |
| zyga | pstolowski: +1 | 07:47 |
| zyga | heh | 07:48 |
| mup | PR snapd#8826 opened: tests: assorted small patches <Simple 😃> <Created by zyga> <https://github.com/snapcore/snapd/pull/8826> | 07:48 |
| zyga | I've seen so many tasks that never get a ping back from travis | 07:48 |
| zyga | mvo: perhaps it is time to drop travis from the "required" list? | 07:48 |
| zyga | the only thing left is the CLA check | 07:49 |
| pstolowski | zyga: ty! | 07:50 |
| mvo | zyga: I guess we could remove it totally, yes? | 07:50 |
| zyga | mvo: I would keep it for a while because IIRC mborzecki wanted to do some changes to the CLA checker | 07:51 |
| mborzecki | anyone remember whether time-control only works if you use timedatectl? | 07:51 |
| zyga | and having a 2nd guess would be better | 07:51 |
| zyga | mborzecki: as in the interface? | 07:51 |
| zyga | mborzecki: let me look | 07:51 |
| zyga | mborzecki: it's literally defined as such but perhaps that's a bit too tight | 07:52 |
| zyga | mborzecki: what kind of extra permissions do you think it should grant? | 07:53 |
| mborzecki | zyga: there's https://forum.snapcraft.io/t/access-to-bin-date-in-strict-confinement/18041 but the reporter gets EPERM (so likely not caused by AA) | 07:54 |
| zyga | mmm | 07:55 |
| zyga | how does date set time | 07:55 |
| zyga | clock_settime or something like that? | 07:55 |
| mborzecki | zyga: strace tells met it's clock_settime | 07:55 |
| zyga | so a seccomp filter is probably what needs changing | 07:56 |
| zyga | IMO it's sensible | 07:56 |
| mborzecki | yup | 07:56 |
| mborzecki | ok, let me open a PR and mark it for security review | 07:57 |
| zyga | thanks | 07:58 |
| zyga | maybe tweak the spread test in the PR as well | 07:58 |
| zyga | I think we have one | 07:58 |
| zyga | mvo: there was a question about https://bugs.launchpad.net/ubuntu/+source/snapd/+bug/1881350 and release to groovy | 08:20 |
| mup | Bug #1881350: snap seeding fails with libc6-lse <snapd:Fix Committed by zyga> <glibc (Ubuntu):New> <snapd (Ubuntu):New> <https://launchpad.net/bugs/1881350> | 08:20 |
| zyga | do you think it's better to do 2.45.1 or just a distro patch? | 08:20 |
| pedronis | pstolowski: mborzecki: hi, when you have time, could you re-review #8702 (it follows my suggestions, I think, now) | 08:23 |
| mup | PR #8702: overlord/configstate: add system.kernel.printk.console-loglevel option <Created by EthanHsieh> <https://github.com/snapcore/snapd/pull/8702> | 08:23 |
| mborzecki | pedronis: sure, will do | 08:23 |
| pstolowski | pedronis: sure | 08:23 |
| pedronis | thx | 08:23 |
| mvo | zyga: I uploaded to groovy already | 08:23 |
| zyga | ah, fantastic | 08:24 |
| mvo | zyga: and it's there already (i.e. autopkgtest passed) | 08:24 |
| zyga | mvo: my daughter doesn't recognize me :) | 09:10 |
| zyga | mvo: she's scared when she sees me now | 09:10 |
| mvo | zyga: haha - oh boy | 09:11 |
| mvo | zyga: but you *do look different' | 09:11 |
| mborzecki | zyga: shaved your beard? | 09:12 |
| zyga | yep | 09:12 |
| mborzecki | so we no longer poke jamie, but rather set 'needs security review' tag? | 09:19 |
| mborzecki | zyga: https://github.com/snapcore/snapd/pull/8827 | 09:20 |
| mup | PR #8827: interfaces/builtin/time-control: allow POSIX clock API <Needs security review> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/8827> | 09:20 |
| zyga | mborzecki: yes | 09:20 |
| zyga | done | 09:21 |
| zyga | mborzecki: any weird things happen when you change the time? | 09:21 |
| zyga | perhaps that test should be invoked via a time namespace? | 09:21 |
| zyga | just to make sure it affects the system less | 09:22 |
| mup | PR snapd#8827 opened: interfaces/builtin/time-control: allow POSIX clock API <Needs security review> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/8827> | 09:24 |
| mborzecki | zyga: it gets restored in the test, the ntp should slowly update the clock | 09:24 |
| mborzecki | zyga: btw. do any of the systems have a kernel that supports time ns already? | 09:24 |
| zyga | mborzecki: yeah, I think 20.04+ should be fine | 09:24 |
| mborzecki | iirc it's in 5.6 | 09:24 |
| zyga | it was a added a while ago | 09:24 |
| zyga | or maybe I read the patch a while ago :D | 09:24 |
| mborzecki | it's automatically a part of new uts ns right? | 09:25 |
| pedronis | mborzecki: mmh, that PR is not using the file I thought it should use | 09:26 |
| zyga | mborzecki: no, I don’t think it is | 09:27 |
| mborzecki | pedronis: 99-snapd.conf? | 09:27 |
| zyga | Uts is a separate ns | 09:27 |
| pedronis | mborzecki: yes, actually the code is confusing | 09:27 |
| pedronis | why is it mentioning 10-console-messages.conf | 09:27 |
| pedronis | I see it has both | 09:27 |
| pedronis | I asked a question there now | 09:28 |
| mborzecki | zyga: https://kernelnewbies.org/Linux_5.6#Time_Namespaces hmm my version if unshare is unaware of CLONE_NEWTIME | 09:28 |
| pedronis | mborzecki: I'm not sure why it bothers with 10-console-messages.conf at al | 09:29 |
| pedronis | it should just write an empty 99-snapd.conf, no? | 09:29 |
| pedronis | or remove it | 09:29 |
| mborzecki | pedronis: aiui it reloads the old/default setting | 09:29 |
| mborzecki | so it's applied now, rather than on the next reboot | 09:29 |
| pedronis | I see, feels fragile | 09:32 |
| pedronis | maybe we should just sysctl --system all the time | 09:32 |
| pedronis | if there's a change | 09:33 |
| pedronis | mborzecki: I commented about that | 09:34 |
| mborzecki | pedronis: hm yes, that's an option, i think one could argue if you tweak something at runtime, it could get overwritten | 09:34 |
| pedronis | mvo: what's the state of this: https://bugs.launchpad.net/snapd/+bug/1867415 ? | 09:36 |
| mup | Bug #1867415: Hardlinking snaps wastes 400 MB tmpfs RAM in live CDs <rls-ff-incoming> <snapd:New for mvo> <snapd (Ubuntu):Fix Released> <https://launchpad.net/bugs/1867415> | 09:36 |
| mvo | pedronis: this is fixed, let me update the bug | 09:39 |
| mvo | pedronis: oh, let me see, someone claims it's not | 09:40 |
| pedronis | mvo: yes, exactly, that's why I'm asking | 09:40 |
| mvo | pedronis: I will try to reproduce now | 09:40 |
| * mvo is downloading | 09:41 | |
| mup | PR snapd#8828 opened: tests: clean up up the use of configcoreSuite in the configcore tests <Test Robustness> <Created by stolowski> <https://github.com/snapcore/snapd/pull/8828> | 09:54 |
| abeato | tianon, hey, are all interfaces used by the docker snap actually needed? I was looking into a way to restrict what the snap can do | 09:56 |
| zyga | is the store under load? | 09:56 |
| zyga | error: cannot get snap sections: cannot sections: got unexpected HTTP status code 403 via GET to "https://api.snapcraft.io/api/v1/snaps/sections" | 09:56 |
| zyga | pedronis, mborzecki: tracking broken out as a separate PR https://github.com/snapcore/snapd/pull/8829 | 09:57 |
| mup | PR #8829: sandbox/cgroup: add tracking helpers <Created by zyga> <https://github.com/snapcore/snapd/pull/8829> | 09:57 |
| zyga | I chose to bundle ConfirmSystemdServiceTracking in the same PR as that function itself is tiny and completes the picture as to how this is meant to be used | 09:58 |
| zyga | this is half of the large PR now and contains really just the logic to create a transient scope, along with unit tests | 09:58 |
| zyga | I didn't add the spread tests as those would pull in virtually everything from the original | 09:58 |
| mup | PR snapd#8829 opened: sandbox/cgroup: add tracking helpers <Created by zyga> <https://github.com/snapcore/snapd/pull/8829> | 09:59 |
| zyga | the design was reviewed by jamie and he gave a +1 on the original PR | 10:01 |
| pedronis | zyga: mvo: this has a lot of back and forth by you but no priority or status != new: https://bugs.launchpad.net/ubuntu/+source/snapd/+bug/1873004 | 10:03 |
| mup | Bug #1873004: lxd interaction blocked until snapd was restarted <lxd (Ubuntu):New> <snapd (Ubuntu):New> <https://launchpad.net/bugs/1873004> | 10:03 |
| zyga | pedronis: yes, I think this is a duplicate of an existing issue that was investigated but not fixed earlier | 10:04 |
| zyga | pedronis: I will look at launchpad to find the other report | 10:04 |
| zyga | pedronis: it _looks_ like snapd starts before we mount snaps, and if core is mounted after snapd, this will happen | 10:04 |
| zyga | (no interfaces) | 10:04 |
| zyga | ah wait | 10:05 |
| zyga | wrong bug | 10:05 |
| mvo | oh, is that a dupe of the one andy had on friday ? | 10:05 |
| zyga | I looked at the other tap | 10:05 |
| zyga | *tab | 10:05 |
| zyga | no no, sorry let me read the correct link | 10:05 |
| zyga | hmm | 10:06 |
| zyga | hmm | 10:10 |
| zyga | mvo: I don't have any more ideas for that bug | 10:11 |
| zyga | the frequency seems to suggest it is related to core reloading | 10:12 |
| zyga | er, refreshing | 10:12 |
| pedronis | mborzecki: bit confused by the rename to UpdateBootScript, not sure how it helps with confusion with InstallBootConfig | 10:18 |
| * zyga prepares the next segment | 10:29 | |
| * zyga takes a break from writing and reads https://github.com/snapcore/snapd/pull/8675/files | 10:38 | |
| mup | PR #8675: osutil: add disks pkg for associating mountpoints with disks/partitions <UC20> <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/8675> | 10:38 |
| mborzecki | pedronis: i'm thinking about https://github.com/snapcore/snapd/pull/8775#discussion_r434505499 bot end up named grub.cfg, so maybe grub.cfg and recovery/grub.cfg? | 10:44 |
| mup | PR #8775: [RFC] bootloader, boot: boot scripts, edition <Needs Samuele review> <UC20> <⛔ Blocked> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/8775> | 10:44 |
| pedronis | mborzecki: should we have a chat before the standup about the grub scripts? | 10:45 |
| mborzecki | pedronis: now or later? | 10:45 |
| pedronis | mborzecki: can do in 5 min? | 10:46 |
| mborzecki | pedronis: sounds fine | 10:46 |
| pedronis | mborzecki: going to the standup | 10:53 |
| * mvo hugs pstolowski for 8828 | 10:54 | |
| pstolowski | mvo: thanks for the review & suggestions! | 11:14 |
| * zyga needs a break | 11:21 | |
| zyga | man, you have kids and they are at home | 11:21 |
| zyga | but when *everyone* just comes to the same room | 11:21 |
| zyga | and the dog too | 11:21 |
| zyga | and then everyone starts moving | 11:21 |
| zyga | it's a bit too much | 11:21 |
| pedronis | zyga: I commented/asked some question in #8573 | 11:34 |
| mup | PR #8573: overlord/snapstate: inhibit startup while unlinked <Created by zyga> <https://github.com/snapcore/snapd/pull/8573> | 11:34 |
| mup | PR snapd#8830 opened: bootloader: introduce bootloarder assets, import grub.cfg with an edition marker <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/8830> | 11:39 |
| zyga | Thank you! | 11:43 |
| mup | PR snapd#8831 opened: tests: use configcoreSuite in journalSuite and remove some duplicated code <Test Robustness> <Created by stolowski> <https://github.com/snapcore/snapd/pull/8831> | 11:44 |
| pedronis | pstolowski: I did a pass on #8812 | 11:53 |
| mup | PR #8812: o/snapstate: service-control task handler (4/N) <Needs Samuele review> <Services ⚙️> <Created by stolowski> <https://github.com/snapcore/snapd/pull/8812> | 11:53 |
| pstolowski | pedronis: great, thank you! | 11:53 |
| pedronis | some high-level questions/comments | 11:53 |
| mup | PR snapd#8722 closed: tests: check that host settings like hostname are settable on core <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8722> | 11:54 |
| pedronis | zyga: does #8829 need a security review? | 11:57 |
| mup | PR #8829: sandbox/cgroup: add tracking helpers <Created by zyga> <https://github.com/snapcore/snapd/pull/8829> | 11:57 |
| zyga | pedronis: I think jamie could indicate if he needs to review it again but my understanding it he gave this a +1 already | 11:57 |
| zyga | if he has the time for a quick look to decide if a deeper look is necessary | 11:57 |
| zyga | that would be okay | 11:57 |
| zyga | jdstrand: ^ | 11:57 |
| mup | PR snapd#8832 opened: travis.yml: removed, all our checks run in GH actions now <Simple 😃> <Skip spread> <Created by mvo5> <https://github.com/snapcore/snapd/pull/8832> | 11:59 |
| clmsy | is it possible to try core20 out without updating kernel or its a no go | 12:11 |
| clmsy | i set the gadget to use 20 and left the kernel snap 4.4.x (core16 kernel) i guess its normal that i cant use to build an image this way | 12:12 |
| pedronis | clmsy: core20 needs a new model assertion syntax and new initramfs, because of the latter an old kernel will not work as is | 12:22 |
| === mbriza2 is now known as mbriza | ||
| clmsy | thx pedronis! | 12:27 |
| ogra | popey, gitea just exploded in my face ... seems it sets a versioned SNAP_USER_DATA everywhere in its app.ini instead of using the /current link so after an update it cant access its data anymore ... | 12:34 |
| ogra | (fixing app.ini manually to use the symlink everywhere foxes the app and it starts again) | 12:37 |
| ogra | *fixes | 12:37 |
| zyga | uh | 12:43 |
| pedronis | zyga: dbusutil/dbustest/stug.bo needs a blankline before package atm the copyright is the doc comment of the package | 12:53 |
| zyga | ah, thanks | 12:53 |
| zyga | I didn't check that | 12:53 |
| pedronis | I have a check, but it's a bit hacky, maybe we should add to run-checks anyway | 12:54 |
| pedronis | mborzecki: I did a pass on #8830, mostly naming | 12:54 |
| mup | PR #8830: bootloader: introduce bootloarder assets, import grub.cfg with an edition marker <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/8830> | 12:54 |
| mborzecki | pedronis: thanks! | 12:54 |
| popey | ogra i am on vacation, do feel free to provide feedback upstream (which publishes the snap) | 13:22 |
| mup | PR snapd#8833 opened: sandbox/cgroup: remove redundant pathOfProcPidCgroup <Simple 😃> <Created by zyga> <https://github.com/snapcore/snapd/pull/8833> | 13:25 |
| ogra | popey, will do, thanks ... enjoy your vac. | 13:27 |
| mup | PR snapd#8834 opened: dbusutil/dbustest: add separate license from package <Simple 😃> <Skip spread> <Created by zyga> <https://github.com/snapcore/snapd/pull/8834> | 13:30 |
| cachio | mvo, I created a PR for skip | 13:40 |
| cachio | #8835 | 13:40 |
| mup | PR #8835: POC: skip binary to stip tests in an easy way <Proof of concept> <⛔ Blocked> <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/8835> | 13:40 |
| mvo | cachio: thank you! looking now | 13:41 |
| cachio | mvo, this is to address this https://github.com/snapcore/spread/pull/72 | 13:41 |
| mup | PR spread#72: Run condition <Blocked> <Reviewed> <Created by sergiocazzolato> <https://github.com/snapcore/spread/pull/72> | 13:41 |
| mborzecki | pedronis: cmatsuoka: i can take a look at making some of the exported gadget code private tomorrow morning | 13:43 |
| mborzecki | the updaters, device lookup, filesystem image cuold all be private | 13:44 |
| mup | PR snapd#8835 opened: POC: skip binary to stip tests in an easy way <Proof of concept> <⛔ Blocked> <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/8835> | 13:45 |
| mborzecki | we did not agree on what to do with the code what was introduced for the tool that could build images, or did we? | 13:45 |
| cmatsuoka | mborzecki: ack, thanks! | 13:45 |
| mup | PR snapcraft#3164 closed: cli: remove enable-ci command <Created by sergiusens> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/3164> | 13:46 |
| mup | PR snapd#8836 opened: sandbox/cgroup: add tests for ParsePids <Simple 😃> <Created by zyga> <https://github.com/snapcore/snapd/pull/8836> | 14:00 |
| zyga | hmmm | 14:19 |
| zyga | not that many spread tests are in flight | 14:19 |
| zyga | mvo: I wonder if it would help if we also ran unit tests in self-hosted runners | 14:19 |
| zyga | after all, the worker is really mostly idle apart from waiting for network IO from spread | 14:20 |
| mvo | zyga: works for me | 14:20 |
| zyga | mvo: I somewhat worry about RAM usage though, not sure how much our test suite takes | 14:20 |
| * zyga checks | 14:20 | |
| zyga | mvo: I'll experiment with some PRs later | 14:20 |
| zyga | mvo: I think now we're waiting for unit tests to pass to run spread and have more spread capacity than unit test capacity | 14:21 |
| mvo | zyga: yeah, makes sense | 14:21 |
| zyga | hmm, actually, my view was stale 32 spread jobs in flight | 14:22 |
| zyga | so perhaps that would not win much | 14:22 |
| zyga | exactly at capacity | 14:22 |
| * zyga breaks for lunch | 14:41 | |
| pedronis | mborzecki: I commented on the name of the assets get function again | 14:44 |
| mborzecki | nice, i like assets.Internal | 14:49 |
| mup | PR snapd#8398 closed: usersession/userd: add "slack" to the white list of URL schemes handled by xdg-open <Created by troyready> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8398> | 14:50 |
| mup | PR snapd#8800 closed: usersession/userd: add "slack" to the white list of URL schemes handled by xdg-open - 2.45 <Created by jdstrand> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/8800> | 14:50 |
| zyga | mborzecki: what is the word "edition" used for, is it like "version"? | 14:56 |
| mborzecki | zyga: yeah, in the same feel as edition in gadget.yaml | 14:57 |
| zyga | ah | 14:57 |
| zyga | I forgot we have that | 14:57 |
| ijohnson | zyga: what versions of opensuse would you say are supported by snapd? | 15:07 |
| zyga | 15.1 and tumbleweed | 15:07 |
| zyga | but .45 is not out there yet as I had issues with auth | 15:07 |
| zyga | (the package is ready for a while, I just need to return to it) | 15:08 |
| jdstrand | zyga: with PR 8829, is that broken out from PR 7825 unmodified? if so and you didn't change anything (significant, ie, algorithm or implementation-wise) in this part of the code, then I don't need to re-review | 15:09 |
| mup | PR #8829: sandbox/cgroup: add tracking helpers <Created by zyga> <https://github.com/snapcore/snapd/pull/8829> | 15:09 |
| mup | PR #7825: many: use transient scope for tracking apps and hooks <Needs security review> <Security-High> <Created by zyga> <https://github.com/snapcore/snapd/pull/7825> | 15:09 |
| zyga | jdstrand: there are way more tests | 15:09 |
| jdstrand | tests are great :) | 15:09 |
| jdstrand | others can review those | 15:09 |
| jdstrand | variable renames, equivalent/simple refactoring, also not needed in this go code | 15:10 |
| zyga | jdstrand: and there's a new helper that checks for the cgroup of a service (using an existing helper but technically the function is new) - it is on line https://github.com/snapcore/snapd/pull/8829/files#diff-4706d268ffa8aa3999d09cac05b3ced2R110 | 15:10 |
| mup | PR #8829: sandbox/cgroup: add tracking helpers <Created by zyga> <https://github.com/snapcore/snapd/pull/8829> | 15:10 |
| jdstrand | s/not needed/don't need a security review/ | 15:10 |
| zyga | you may want to eye that if you want, I included it here because in all cases we call either the new CreateTransientScope or the new ConfirmSystemdServiceTracking | 15:11 |
| jdstrand | that function is fine and using a reviewed-already api | 15:11 |
| jdstrand | zyga: well, if you feel that my cursory glance missed something and you want me to review, please add the needs security review tag | 15:11 |
| zyga | jdstrand: I think this is okay, I really don't think there's anything security related here | 15:12 |
| zyga | and there's no new design | 15:12 |
| zyga | just broken out code | 15:12 |
| zyga | tests | 15:12 |
| zyga | and some moving around to different packages to fit the structure better | 15:12 |
| jdstrand | yeah, that is what it looked like | 15:12 |
| jdstrand | I mean, yes, we need to have the design and code for this feature reviewed, but we did that extensively in PR 7825 | 15:13 |
| mup | PR #7825: many: use transient scope for tracking apps and hooks <Needs security review> <Security-High> <Created by zyga> <https://github.com/snapcore/snapd/pull/7825> | 15:13 |
| zyga | yes, this is my understanding as well | 15:13 |
| zyga | mborzecki: https://github.com/snapcore/snapd/pull/8830#pullrequestreview-426362895 | 15:35 |
| mup | PR #8830: bootloader: introduce bootloarder assets, import grub.cfg with an edition marker <UC20> <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/8830> | 15:35 |
| zyga | man I was up for like 30 seconds | 15:40 |
| zyga | and it hurts for a few minutes now | 15:40 |
| roadmr | ouch zyga :( | 15:41 |
| zyga | yeah, this is not a good time | 15:41 |
| mup | PR snapcraft#3162 closed: tests: remove scenario usage from lifecycle order <maintenance> <Created by sergiusens> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/3162> | 15:41 |
| mup | PR snapcraft#3163 closed: cli: remove the hidden inspect command <maintenance> <Created by sergiusens> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/3163> | 15:41 |
| * cachio lunch | 15:45 | |
| === KindTwo is now known as KindOne | ||
| pedronis | zyga: I wrote a quick mail about /usr/lib/snapd | 16:05 |
| zyga | pedronis: checking | 16:05 |
| zyga | pedronis: I like this a lot | 16:08 |
| zyga | let me think a little about it and draft something for tomorrow | 16:08 |
| * zyga goes to rest for a while, putting the laptop away | 16:21 | |
| mup | PR snapd#8827 closed: interfaces/builtin/time-control: allow POSIX clock API <Needs security review> <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/8827> | 16:30 |
| Saviq | anyone else seeing snaps unstable on travis today? I'm getting a lot of failed lxd snap installs… | 16:35 |
| tianon | abeato: I guess that depends on what you want to "lock down" -- "privileged", "home", and "removable-media" are definitely optional, but the others are all related to core functionality of Docker you'd have to go out of your way to not use IIRC | 16:42 |
| zyga | Saviq: we actually moved away from travis *today* | 16:43 |
| abeato | tianon, I actually tried removing "privileged", but get into some problems when starting dockerd | 16:43 |
| abeato | it was some permission for sysfs | 16:44 |
| Saviq | zyga: lucky | 16:44 |
| zyga | Saviq: maybe store load? | 16:44 |
| Saviq | could be, there's very little info on what went wrong, suggesting to look at the journal… | 16:45 |
| tianon | abeato: ahh, I can reproduce -- that's definitely related to the mitigations in 19.03.11 for CVE-2020-13401 (open /proc/sys/net/ipv6/conf/docker0/accept_ra: permission denied) | 16:45 |
| abeato | tianon, yeah, it was that | 16:46 |
| tianon | abeato: that one's technically something that has to be updated in the snapd "docker-support" profiles, IIRC (not something we can fix in the Docker snap directly, AFAIK) | 16:46 |
| abeato | tianon, I see, is it something that will be fixed eventually? | 16:47 |
| ijohnson | tianon: o/ | 16:50 |
| ijohnson | tianon: I am just now running into the same issue for `/proc/sys/net/ipv6/conf/docker0/accept_ra` | 16:50 |
| tianon | abeato: I *think* it'll require a PR to https://github.com/snapcore/snapd/blob/269ced7c4f31bbd912f73cba822f522244da16d5/interfaces/builtin/docker_support.go, since the denial is likely apparmor | 16:50 |
| ijohnson | tianon: is the right fix to just allow that in the policy? | 16:50 |
| tianon | :D | 16:50 |
| tianon | yeah | 16:51 |
| ijohnson | ok, let me see maybe the docker snap should just be using network-control instead? | 16:51 |
| tianon | it has that already, doesn't it? | 16:51 |
| ijohnson | I don't remember if network-control provides for that access or not | 16:51 |
| tianon | ah, it has network and network-bind but not network-control | 16:51 |
| ijohnson | tianon: yeah I think network-control should give access to that sysfs thing | 16:52 |
| tianon | https://github.com/snapcore/snapd/blob/269ced7c4f31bbd912f73cba822f522244da16d5/interfaces/builtin/network_control.go#L84-L93 it does appear so :) | 16:52 |
| ijohnson | tianon: you will need to add network-control to the plugs for dockerd, then we will probably need to get the assertion for the docker snap updated to allow auto-connection of network-control | 16:52 |
| ijohnson | tianon: can you start a new forum topic on forum.snapcraft.io about granting network-control to the docker snap and ping jdstrand ? | 16:53 |
| tianon | sure thing :) | 16:53 |
| ijohnson | thanks tianon | 16:53 |
| abeato | tianon, ijohnson thanks | 16:54 |
| mup | PR snapd#8834 closed: dbusutil/dbustest: separate license from package <Simple 😃> <Skip spread> <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/8834> | 16:55 |
| ijohnson | tianon: fyi I just finished testing adding the network-control interface to the docker snap locally and it fixed that issue so that should be all that's needed, we shouldn't need interface changes to docker-support I think | 16:56 |
| ijohnson | tianon: but let's see how the forum topic goes | 16:56 |
| tianon | nice :D | 16:56 |
| tianon | https://forum.snapcraft.io/t/auto-connect-docker-to-network-control/18054?u=tianon :) | 16:58 |
| ijohnson | tianon: great, and just to be clear the docker snap does allow capability net_raw in the apparmor policy, so I think it is theoretically possible for that CVE to affect the snap | 17:02 |
| ijohnson | tianon: though I'm not sure how well the docker snap supports specifying different capability sets for the container | 17:02 |
| * ijohnson has never tried it | 17:02 | |
| tianon | ijohnson: yeah, the snap is absolutely affected by that CVE -- not sure if it goes all the way to the host level, but certainly cross-container (and likely *is* able to send RAs to the host) | 17:03 |
| ijohnson | tianon: ok, yeah so we should try to get this fixed asap then, also how does the mitigation work, as I tried to reproduce the issue where the docker snap couldn't write to that sysfs file in a VM but it worked fine, presumably because the networking in the VM was different? | 17:04 |
| ijohnson | err the networking setup in a VM is different from a real machine is what I meant | 17:05 |
| jdstrand | roadmr: hey, can you pull 20200608-1642UTC ? | 17:05 |
| ijohnson | because I see it on a real machine but not on VM's | 17:05 |
| tianon | ijohnson: the mitigation disables accepting IPv6 RA (by writing to that sysfs value) on brdige devices Docker creates (which wouldn't be changed in a VM vs on a physical host); your VM might have had IPv6 completely disabled, in which case the mitigation wouldn't be necessary (that sysfs file wouldn't exist at all) | 17:07 |
| tianon | ijohnson: https://github.com/moby/moby/compare/v19.03.10...v19.03.11 is pretty small (just the mitigation patch) | 17:08 |
| tianon | ijohnson: reading that patch, your VM might have something like "failed to read ipv6 net.ipv6.conf.<bridge>.accept_ra" as a warning in the dockerd logs | 17:08 |
| ijohnson | hmm it seems that my VM has ipv6 enabled, but still the docker snap is able to run | 17:14 |
| ijohnson | perhaps that accept_ra was already disabled in the VM | 17:14 |
| tianon | hmm | 17:15 |
| tianon | maybe | 17:15 |
| tianon | but it writes it whether it's disabled or not | 17:15 |
| ijohnson | yes, I wonder if there's not a default bridge set somehow? | 17:16 |
| ijohnson | hmm so on a fresh focal VM, I have /proc/sys/net/ipv6/conf/default/accept_ra set as "1" | 17:16 |
| === KindTwo is now known as KindOne | ||
| tianon | ijohnson: given the current positive direction of that thread, I should upload an update with network-control to edge right? :) | 18:24 |
| ijohnson | tianon: yes please | 18:24 |
| ijohnson | tianon: I assume when jdstrand or some other store admin gets a chance they will process the auto-connection | 18:24 |
| ijohnson | tianon: but also iirc you should be able to just push a new version with network-control to edge, and that will be installable, but folks won't have it magically work without running manually `snap connect docker:network-control` but after the auto-connection is issued then that will magically work | 18:25 |
| tianon | ijohnson: right :) | 18:26 |
| ijohnson | tianon: while not really relevant anymore since you got auto-connect, I debugged a bit more in the VM and I think the bug is only when docker snap is upgraded and the old version of that bridge didn't have the ipv6 disabled, I think that when this version of the docker snap is installed fresh it just creates the bridge with that disabled | 18:27 |
| ijohnson | but maybe I'm missing something else | 18:27 |
| ijohnson | s/ipv6 disabled/ipv6 RA packet things/ | 18:27 |
| tianon | interesting | 18:27 |
| tianon | well, FWIW, the commit is pushed; as soon as launchpad gets around to it, it should be in edge :) | 18:28 |
| ijohnson | great thanks! | 18:29 |
| jdstrand | tianon: we have the votes now but normally we wait 7 days for reviewers to comment. is this an emergency? | 18:47 |
| tianon | jdstrand: I'd defer that to ijohnson -- I don't think it's an emergency, but it is preventing folks from using the "docker" snap without privileged enabled | 18:48 |
| jdstrand | tianon: the current docker snap or one in edge/etc? | 18:49 |
| jdstrand | and by current, I mean 'stable' | 18:49 |
| ijohnson | jdstrand: tianon: I don't know that I'd call it an emergency, but I do think it is urgent since the current docker snap on stable is somewhat broken for existing installations and I don't think it's reasonable to revert to the previous one, as the previous docker snap suffers from that critical CVE | 18:49 |
| tianon | stable | 18:49 |
| ijohnson | jdstrand: but folks can "un-break" their installs by refreshing to edge and then manually connecting network-control | 18:50 |
| jdstrand | ijohnson: so, stable already has network-control? | 18:50 |
| ijohnson | jdstrand: the critical CVE being https://nvd.nist.gov/vuln/detail/CVE-2020-13401 | 18:50 |
| ijohnson | jdstrand: no, only edge has network-control | 18:50 |
| ijohnson | as per tianon's commit today | 18:50 |
| jdstrand | ijohnson: but stable and edge have the fix, and you want to promate something with network-control to stable? | 18:50 |
| ijohnson | well actually edge may not have it yet, I have not looked at the lp builders status to see if they built it or not | 18:50 |
| jdstrand | promote* | 18:51 |
| jdstrand | and by fix, I mean the cve fix | 18:51 |
| ijohnson | ah | 18:51 |
| ijohnson | so that CVE is currently fixed on all channels of the docker snap | 18:51 |
| jdstrand | ok, but it regressed in certain ituations | 18:51 |
| ijohnson | yes | 18:52 |
| jdstrand | gotcha. ok, I'll fast track it | 18:52 |
| ijohnson | thanks jdstrand | 18:52 |
| jdstrand | ijohnson, tianon: done | 18:57 |
| ijohnson | nice thank you | 18:57 |
| tianon | thank you! | 18:57 |
| tianon | now if I could just figure out why the snap has intermittent build failures on launchpad that I can't reproduce locally... /o\ | 18:58 |
| roadmr | jdstrand: hi! certainly - will put it in the queue. | 19:27 |
| mup | PR snapd#8826 closed: tests: assorted small patches <Simple 😃> <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/8826> | 19:41 |
| zyga | jdstrand: not urgent but small: mvo requested a 3rd review of a test you wrote that I modify: https://github.com/snapcore/snapd/pull/8803 | 19:48 |
| mup | PR #8803: tests: port interfaces-many-core-provided to tests.session <Test Robustness> <Created by zyga> <https://github.com/snapcore/snapd/pull/8803> | 19:48 |
| roadmr | jdstrand: fun, looks like those 2.45 base declaration changes broke a few store tests :) | 20:49 |
| jdstrand | roadmr: oh? was network-status one of the special ones that was used in the testsuite? | 21:01 |
| roadmr | jdstrand: yep, looks like it | 21:01 |
| roadmr | jdstrand: f.ex I had it in an EXPECTED_DENIED_CONNECTION_INTERFACES list but it's not in that category anymore | 21:02 |
| roadmr | (because its deny-connection: true got removed) | 21:02 |
| jdstrand | roadmr: it looks like you could use mir instead | 21:03 |
| roadmr | jdstrand: thanks! I'll for sure replace with that if needed | 21:03 |
| jdstrand | roadmr: yes, network-status is no longer denied, for sure | 21:03 |
| roadmr | jdstrand: some of the tests just check the interface categories generically so should cope with me moving network-status out of the EXPECTED_DENIED list | 21:04 |
| * jdstrand nods | 21:04 | |
| roadmr | jdstrand: it's also no longer in EXPECTED_APP_PROVIDED_SLOT_INTERFACES as it's now provided by core | 21:06 |
| roadmr | jdstrand: so all the changes seem to make sense; I'll probably tag you in the MP for a sanity check | 21:06 |
| roadmr | none of this looks controversial in any case | 21:06 |
| jdstrand | happy to look at it, but your assessment is correct | 21:07 |
| jdstrand | afaics | 21:07 |
| jdstrand | (from way over here :) | 21:07 |
| roadmr | jdstrand: osram-dp2i-avahi is indeed in a brand store O_o they should file a support case | 21:08 |
| jdstrand | roadmr: ok, I responded to say to file a support case | 21:09 |
| roadmr | makes sense | 21:09 |
| roadmr | thanks! | 21:10 |
| mup | PR snapd#8825 closed: tests: move a few more tests to snapstate_install_test <Test Robustness> <Created by stolowski> <Merged by cmatsuoka> <https://github.com/snapcore/snapd/pull/8825> | 21:22 |
| roadmr | jdstrand: https://code.launchpad.net/~roadmr/software-center-agent/+git/software-center-agent/+merge/385315 for when you have a min - no rush, I'm out of runway today but will check back tomorrow :) | 21:33 |
| mup | PR snapd#8831 closed: tests: use configcoreSuite in journalSuite and remove some duplicated code <Test Robustness> <Created by stolowski> <Merged by cmatsuoka> <https://github.com/snapcore/snapd/pull/8831> | 21:57 |
| mup | PR snapcraft#3165 opened: Update cmake plugin to use more modern flags <Created by GamePad64> <https://github.com/snapcore/snapcraft/pull/3165> | 22:22 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!