Skip to content

Commit

Permalink
fix: handle users/groups with no names in push_path (#1082)
Browse files Browse the repository at this point in the history
* Handle users/groups with no names.
  • Loading branch information
tonyandrewmeyer authored Nov 29, 2023
1 parent 681bce2 commit 18abc17
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 4 deletions.
14 changes: 12 additions & 2 deletions ops/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -2467,6 +2467,16 @@ def _build_fileinfo(path: Union[str, Path]) -> pebble.FileInfo:
import grp
import pwd
info = path.lstat()
try:
pw_name = pwd.getpwuid(info.st_uid).pw_name
except KeyError:
logger.warning("Could not get name for user %s", info.st_uid)
pw_name = None
try:
gr_name = grp.getgrgid(info.st_gid).gr_name
except KeyError:
logger.warning("Could not get name for group %s", info.st_gid)
gr_name = None
return pebble.FileInfo(
path=str(path),
name=path.name,
Expand All @@ -2475,9 +2485,9 @@ def _build_fileinfo(path: Union[str, Path]) -> pebble.FileInfo:
permissions=stat.S_IMODE(info.st_mode),
last_modified=datetime.datetime.fromtimestamp(info.st_mtime),
user_id=info.st_uid,
user=pwd.getpwuid(info.st_uid).pw_name,
user=pw_name,
group_id=info.st_gid,
group=grp.getgrgid(info.st_gid).gr_name)
group=gr_name)

@staticmethod
def _list_recursive(list_func: Callable[[Path], Iterable[pebble.FileInfo]],
Expand Down
23 changes: 22 additions & 1 deletion test/test_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
from collections import OrderedDict
from test.test_helpers import fake_script, fake_script_calls
from textwrap import dedent
from unittest.mock import patch
from unittest.mock import MagicMock, patch

import pytest

Expand Down Expand Up @@ -969,6 +969,27 @@ def test_run_error(self):
self.assertEqual(str(cm.exception), 'ERROR cannot get status\n')
self.assertEqual(cm.exception.args[0], 'ERROR cannot get status\n')

@patch("grp.getgrgid")
@patch("pwd.getpwuid")
def test_push_path_unnamed(self, getpwuid: MagicMock, getgrgid: MagicMock):
getpwuid.side_effect = KeyError
getgrgid.side_effect = KeyError
harness = ops.testing.Harness(ops.CharmBase, meta='''
name: test-app
containers:
foo:
resource: foo-image
''')
harness.begin()
harness.set_can_connect('foo', True)
container = harness.model.unit.containers['foo']

with tempfile.TemporaryDirectory() as push_src:
push_path = pathlib.Path(push_src) / 'src.txt'
push_path.write_text('hello')
container.push_path(push_path, "/")
assert container.exists("/src.txt"), 'push_path failed: file "src.txt" missing'


class PushPullCase:
"""Test case for table-driven tests."""
Expand Down
15 changes: 14 additions & 1 deletion test/test_testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
import typing
import unittest
import uuid
from unittest.mock import MagicMock
from unittest.mock import MagicMock, patch

import pytest
import yaml
Expand Down Expand Up @@ -4096,6 +4096,7 @@ class PebbleStorageAPIsTestMixin:

assertEqual = unittest.TestCase.assertEqual # noqa
assertIn = unittest.TestCase.assertIn # noqa
assertIs = unittest.TestCase.assertIs # noqa
assertIsInstance = unittest.TestCase.assertIsInstance # noqa
assertRaises = unittest.TestCase.assertRaises # noqa

Expand Down Expand Up @@ -4572,6 +4573,18 @@ def test_make_dir_with_ownership(self):
dir_ = client.list_files(f"{self.prefix}/dir{idx}", itself=True)[0]
self.assertEqual(dir_.path, f"{self.prefix}/dir{idx}")

@patch("grp.getgrgid")
@patch("pwd.getpwuid")
def test_list_files_unnamed(self, getpwuid: MagicMock, getgrgid: MagicMock):
getpwuid.side_effect = KeyError
getgrgid.side_effect = KeyError
data = 'data'
self.client.push(f"{self.prefix}/file", data)
files = self.client.list_files(f"{self.prefix}/")
self.assertEqual(len(files), 1)
self.assertIs(files[0].user, None)
self.assertIs(files[0].group, None)


class TestFilesystem(unittest.TestCase, _TestingPebbleClientMixin):
def setUp(self) -> None:
Expand Down

0 comments on commit 18abc17

Please sign in to comment.