Skip to content

Commit 03d8596

Browse files
committed
[collect] update for strict confinement for juju
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 grab the data to a different location, such as somewhere in the home directory. Using a directory such as ~/.cache/sos-collect-juju is being used to overcome this issue. Related: #3399 Signed-off-by: Arif Ali <arif.ali@canonical.com>
1 parent 045bb37 commit 03d8596

2 files changed

Lines changed: 92 additions & 4 deletions

File tree

sos/collector/transports/juju.py

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,12 @@
1010

1111

1212
import subprocess
13+
import os
14+
import shutil
1315

1416
from sos.collector.exceptions import JujuNotInstalledException
1517
from sos.collector.transports import RemoteTransport
16-
from sos.utilities import sos_get_command_output
18+
from sos.utilities import sos_get_command_output, parse_version
1719

1820

1921
class JujuSSH(RemoteTransport):
@@ -79,12 +81,44 @@ def _copy_file_to_remote(self, fname, dest):
7981
res = sos_get_command_output(cmd, timeout=15)
8082
return res["status"] == 0
8183

84+
def _get_juju_version(self):
85+
"""Grab the version of juju"""
86+
res = sos_get_command_output("juju version")
87+
return res['output'].split("-", maxsplit=1)[0]
88+
8289
def _retrieve_file(self, fname, dest):
8390
self._chmod(fname) # juju scp needs the archive to be world-readable
8491
model, unit = self.address.split(":")
8592
model_option = f"-m {model}" if model else ""
86-
cmd = f"juju scp {model_option} -- -r {unit}:{fname} {dest}"
87-
res = sos_get_command_output(cmd)
93+
94+
if parse_version(self._get_juju_version()) >= parse_version("3"):
95+
# From juju 3.0 onwards the juju client is a strictly confined
96+
# snap. Strict confinement prevents the snap from writing to
97+
# arbitrary paths on the host (such as sos' tmpdir under /tmp or
98+
# the snap's own private tmp namespace). It can, however, write
99+
# into the invoking user's $HOME thanks to the 'home' snap
100+
# interface. So we scp the file into a staging directory under
101+
# $HOME and then move it to the requested destination ourselves.
102+
#
103+
# This avoids reaching into the snap's private confinement dir
104+
# (/tmp/snap-private-tmp/...) and removes the previous requirement
105+
# of running sos collect as root/with sudo for juju.
106+
staging_dir = os.path.join(
107+
os.path.expanduser("~"), ".cache", "sos-collect-juju"
108+
)
109+
os.makedirs(staging_dir, exist_ok=True)
110+
staged_file = os.path.join(staging_dir, os.path.basename(fname))
111+
112+
cmd = (
113+
f"juju scp {model_option} -- -r "
114+
f"{unit}:{fname} {staging_dir}"
115+
)
116+
res = sos_get_command_output(cmd)
117+
if res["status"] == 0:
118+
shutil.move(staged_file, dest)
119+
else:
120+
cmd = f"juju scp {model_option} -- -r {unit}:{fname} {dest}"
121+
res = sos_get_command_output(cmd)
88122
return res["status"] == 0
89123

90124

tests/unittests/juju/juju_transports_test.py

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,14 @@ def setUp(self):
3333
address="model_abc:unit_abc",
3434
)
3535

36+
# pylint: disable=no-method-argument
37+
def get_juju_version():
38+
return "2.9.45"
39+
40+
# pylint: disable=no-method-argument
41+
def get_juju_version_3():
42+
return "3.1.0"
43+
3644
@patch("sos.collector.transports.juju.subprocess.check_output")
3745
def test_check_juju_installed_err(self, mock_subprocess_check_output):
3846
"""Raise error if juju is not installed."""
@@ -72,17 +80,63 @@ def test_remote_exec(self):
7280
self.juju_ssh.remote_exec == "juju ssh -m model_abc unit_abc"
7381
)
7482

83+
@patch(
84+
"sos.collector.transports.juju.JujuSSH._get_juju_version",
85+
side_effect=get_juju_version,
86+
)
7587
@patch(
7688
"sos.collector.transports.juju.sos_get_command_output",
7789
return_value={"status": 0},
7890
)
7991
@patch("sos.collector.transports.juju.JujuSSH._chmod", return_value=True)
8092
# pylint: disable=unused-argument
81-
def test_retrieve_file(self, mock_chmod, mock_sos_get_cmd_output):
93+
def test_retrieve_file(
94+
self,
95+
mock_chmod,
96+
mock_sos_get_cmd_output,
97+
mock_get_juju_version
98+
):
8299
self.juju_ssh._retrieve_file(fname="file_abc", dest="/tmp/sos-juju/")
83100
mock_sos_get_cmd_output.assert_called_with(
84101
"juju scp -m model_abc -- -r unit_abc:file_abc /tmp/sos-juju/"
85102
)
86103

104+
@patch("sos.collector.transports.juju.shutil.move")
105+
@patch("sos.collector.transports.juju.os.makedirs")
106+
@patch(
107+
"sos.collector.transports.juju.os.path.expanduser",
108+
return_value="/home/user_abc",
109+
)
110+
@patch(
111+
"sos.collector.transports.juju.JujuSSH._get_juju_version",
112+
side_effect=get_juju_version_3,
113+
)
114+
@patch(
115+
"sos.collector.transports.juju.sos_get_command_output",
116+
return_value={"status": 0},
117+
)
118+
@patch("sos.collector.transports.juju.JujuSSH._chmod", return_value=True)
119+
# pylint: disable=unused-argument
120+
def test_retrieve_file_juju_3(
121+
self,
122+
mock_chmod,
123+
mock_sos_get_cmd_output,
124+
mock_get_juju_version,
125+
mock_expanduser,
126+
mock_makedirs,
127+
mock_move,
128+
):
129+
"""For juju 3+ the file is staged under $HOME (confinement-safe) and
130+
then moved to the destination, without sudo or private-tmp hacks."""
131+
staging_dir = "/home/user_abc/.cache/sos-collect-juju"
132+
self.juju_ssh._retrieve_file(fname="file_abc", dest="/tmp/sos-juju/")
133+
mock_makedirs.assert_called_with(staging_dir, exist_ok=True)
134+
mock_sos_get_cmd_output.assert_called_with(
135+
f"juju scp -m model_abc -- -r unit_abc:file_abc {staging_dir}"
136+
)
137+
mock_move.assert_called_with(
138+
f"{staging_dir}/file_abc", "/tmp/sos-juju/"
139+
)
140+
87141

88142
# vim: set et ts=4 sw=4 :

0 commit comments

Comments
 (0)