Skip to content
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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

arif-ali
Copy link
Member

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

  • Is the commit message split over multiple lines and hard-wrapped at 72 characters?
  • Is the subject and message clear and concise?
  • Does the subject start with [plugin_name] if submitting a plugin patch or a [section_name] if part of the core sosreport code?
  • Does the commit contain a Signed-off-by: First Lastname email@example.com?
  • Are any related Issues or existing PRs properly referenced via a Closes (Issue) or Resolved (PR) line?

Copy link

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

  • sudo yum install -y dnf-plugins-core on RHEL 8
  • sudo dnf install -y dnf-plugins-core on Fedora
  • dnf copr enable packit/sosreport-sos-3422
  • And now you can install the packages.

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>
@arif-ali arif-ali force-pushed the sos-arif-collect-juju-snap-strict branch from e68903a to 841becd Compare November 24, 2023 22:26
@arif-ali
Copy link
Member Author

arif-ali commented Nov 28, 2023

This has been tested by me and my colleague, and works as expected, based on the snap created through the CI

https://pastebin.ubuntu.com/p/gpKYKzwxvp/

Copy link
Member

@TurboTurtle TurboTurtle left a 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.

Comment on lines +88 to +94
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}")
Copy link
Member

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?

Copy link
Member Author

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

Copy link
Member Author

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?

Copy link
Member

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?

Copy link
Member Author

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.

Copy link
Contributor

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 ?

Copy link
Member Author

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?

Copy link
Contributor

@dnegreira dnegreira Feb 3, 2024

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.

Copy link
Member

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.

Copy link
Member Author

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 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.

Cool, I'll have to re-think how we do this then, leave it with me; thanks for the inputs

@arif-ali arif-ali added this to the 4.8.0 milestone Feb 3, 2024
@arif-ali arif-ali marked this pull request as draft February 3, 2024 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants