-
Notifications
You must be signed in to change notification settings - Fork 541
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[collect] update for strict confinement for juju #3422
[collect] update for strict confinement for juju #3422
Conversation
Congratulations! One of the builds has completed. 🍾 You can install the built RPMs by following these steps:
Please note that the RPMs should be used only in a testing environment. |
With juju versions 3 and above, when collecting the tarballs from machines it will grab them into a strictly confined area. This means that we need to be able to access this area via sudo. In order for this now to be fully supported, we need sudo on the host that is running juju, otherwise sos collect on a juju environment will not work. Related: sosreport#3399 Signed-off-by: Arif Ali <arif.ali@canonical.com>
e68903a
to
841becd
Compare
This has been tested by me and my colleague, and works as expected, based on the snap created through the CI |
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.
I have some concerns noted below. This mainly comes from me not knowing juju and I could use some insight into what's actually happening here.
sos_get_command_output(f"sudo mkdir {juju_tmpdir}") | ||
sos_get_command_output(f"sudo chmod o+rwx {juju_tmpdir}") | ||
cmd = f"juju scp {model_option} -- -r {unit}:{fname} {self.tmpdir}" | ||
res = sos_get_command_output(cmd) | ||
cmd2 = f"sudo cp {juju_tmpdir}/{fname.split('/')[-1]} {dest}" | ||
sos_get_command_output(cmd2) | ||
sos_get_command_output(f"sudo chmod 644 {dest}") |
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.
I am really not a fan of embedding sudo
into a flow here, or shelling out to mess with the permissions and directory structure.
Let's take a step back and review. What does "strict confinement" mean for juju? As a regular user running, what causes the juju
commands I run to pull data to write as root? If I, as a regular user, can leverage juju
commands to execute within a machine, why do I need to do special steps to access certain data within that machine? I.E. why does "juju-root" matter when pulling the data but not when executing the commands on behalf of the local user?
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.
I can totally understand, and this is nothing to do with juju
itself, but how juju is installed. juju
is installed as a snap
, and snaps
are either classic confinement or strict confinement.
juju 2.9 and below were classic confinement, and hence it had the capability of writing to the classic /tmp
. As soon as an application is strictly confined (and is the case for juju >=3), and you specify /tmp
, this will automatically expand to /tmp/snap-private-tmp/snap.<snap-name>/tmp
. So, when we do a juju scp
and bring it onto the collector machine, it will copy it to /tmp/snap-private-tmp/snap.<snap-name>/tmp/sos.XXXXX
.
If, however, we were using $HOME and some directory there to copy the file to, we should be able to, due to the following connections we have. The /tmp
is a special area, that security confinement does not allow it to access files from other snaps and applications
❱❱❱ snap connections juju | grep home
home juju:home :home -
Below is a link to a similar question on this
https://askubuntu.com/questions/1227248/how-to-make-snaps-see-the-real-tmp
I hope that makes sense
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.
@TurboTurtle thoughts on this, I am keen for this to land for 4.7.0 if possible. Unless, you can advise on a different way to handle this?
@dnegreira anything you would like to chime in on?
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.
@arif-ali I'm still not keen on embedding sudo
and the like.
If we're writing to a private /tmp
location due to it being packaged as a snap, would it suffice to have the snap packaging set sos' entire tmpdir to that private location? As in, in sos.conf
overriding --tmp-dir
to /tmp/snap-private-tmp/snap.<snap-name>/
with the snap packaging, leaving deb packaging as the "normal" /tmp
?
Alternatively, is juju
non-root capable? If not, would it be acceptable to say sos collect
now requires root for juju
collections?
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 is not a snap issue of the sos
and hence changing the default for the snap does not make sense here. This issue would exist on any machine that even has the deb or rpm version of sos
, and is trying to access a juju
cluster using v3
This is ultimately an issue with how juju
is packaged and not sos
. As mentioned above, there are 2 ways to package a snap, strict and classic. strict is the recommended way, and hence juju is like this from 3 onwards. The reason why sos
is not is due to the nature that it is a debugging tool, and it doesn't make sense for it to be strictly confined.
The folder /tmp/snap-private-tmp
is only written by strictly confined snaps, in this case this is the juju
snap that is doing this and not sos
. When we scp files from the remote host using juju scp< machine-id>:<source> /tmp/sos.xxxx/
. That command is actually copying the file into /tmp/snap-private-tmp/snap.juju/tmp/sos.xxxx
and not into /tmp/sos.xxxx
. This is the essence of strictly confined snaps. This folder is only accessible to root
users unfortunately, and hence the need to use sudo
.
I appreciate this is not ideal and goes against in what this is happening here, but unfortunately this would be the only way we can easily resolve this.
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.
The only way I see, instead of running sudo, which I am also not very keen on doing, is forcing the collect to run as root, as we already do for report
?
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.
@dnegreira that will defeat the purpose, as juju may not be configured for the root user, and hence will not be able to actually do sudo juju ssh
or sudo juju scp
right?
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.
By default no, but one could point the env variable JUJU_DATA
or directly point to the ssh key under ~/.local/share/juju/ and it -should- work.
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.
The only way I see, instead of running sudo, which I am also not very keen on doing, is forcing the collect to run as root, as we already do for
report
?
To be clear, we'd make this root required for the juju
cluster profile/transport specifically within collect
, not for the entirety of collect
. Unfortunately I think this is the only path forward, as I simply cannot get myself over the embedding of sudo
here.
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.
To be clear, we'd make this root required for the
juju
cluster profile/transport specifically withincollect
, not for the entirety ofcollect
. Unfortunately I think this is the only path forward, as I simply cannot get myself over the embedding ofsudo
here.
Cool, I'll have to re-think how we do this then, leave it with me; thanks for the inputs
With juju versions 3 and above, when collecting the tarballs from machines it will grab them into a strictly confined area. This means that we need to be able to access this area via sudo.
In order for this now to be fully supported, we need sudo on the host that is running juju, otherwise sos collect on a juju environment will not work.
Related: #3399
Please place an 'X' inside each '[]' to confirm you adhere to our Contributor Guidelines