Add spread tests for mount namespace layout #169
Conversation
This patch adds a simple test that looks at /proc/self/mountinfo within confined applications. The test discards some information that is too variable to be useful but retains the most essential facts. This test will be changed by the upcoming /media sharing patches. It is intended to be useful for before/after comparison. Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a really nice test. I didn't spend too much time looking at the mountinfo contents and focused mostly on what the test was verifying at a higher level. I only had two comments/questions that I'd like cleared up before giving my ack.
import json | ||
import re | ||
|
||
_boring_fs = set(['cgroup', 'fusectl', 'debugfs', 'pstore', 'securityfs', 'mqueue', 'hugetlbfs']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we ignoring these filesystems?
At least cgroup, debugfs, and securityfs are very security sensitive. At least a comment on the definition of a boring filesystem would be helpful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ha, because I wanted to make it shorter (because there was an instability that I joust found and fixed). I'll amend this test to ignore nothing.
"fs_type": "ext4", | ||
"mount_opts": "rw,noatime", | ||
"mount_point": "/etc", | ||
"mount_src": "/dev/sda", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't the /dev/sda
portion going to be brittle? For example, if testing in QEMU using a virtio, I suspect this will be /dev/vda
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch, I'll change it to /dev/BLOCK
or something like this
So, the remapping script needed some love. Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
I fixed both comments and more (the processing script was lousy so I wrote a real one) |
Various parts of the system can come up with nondeterministic ordering. This results in, e.g. cgroups being ordered randomly from one boot to another. The goal of the script is to discard the randomness so it resorted to sorting but the sort criteria contains random names from various bits of snap-confine setup. This patch splits the sorting so that we first discard randomness from mount_point alone (giving us something that we can sort) and then sorts the list by a few more factors, after which the remainder of the de-randomization is performed. Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
FYI: I finally fixed the non-determinism. It appears that there are differences between the core and ubuntu-core snaps. This is now reflected in the test. |
This patch adds a simple test that looks at /proc/self/mountinfo within
confined applications. The test discards some information that is too
variable to be useful but retains the most essential facts.
This test will be changed by the upcoming /media sharing patches. It is
intended to be useful for before/after comparison.
Signed-off-by: Zygmunt Krynicki zygmunt.krynicki@canonical.com