From 79a419e033df6c01ffd62eb20feedb01170e1315 Mon Sep 17 00:00:00 2001 From: Miroslav Lisik Date: Thu, 25 Jan 2024 15:25:05 +0100 Subject: [PATCH] fix: do not put empty uid/gid options to an uidgid file * fixed command `pcs cluster uidgid add` Resolves #772 --- CHANGELOG.md | 10 ++ pcs/cluster.py | 9 +- pcs/utils.py | 32 +++++- pcs_test/tier1/legacy/test_cluster.py | 154 ++++++++++++++++++++++---- 4 files changed, 173 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ee5aab0b..62e7f230 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # Change Log +## [Unreleased] + +### Fixed +- Do not put empty uid/gid corosync configuration options to an uidgid file + when not specified in `pcs cluster uidgid add` command. Empty options cause + corosync start failure. ([ghissue#772]) + +[ghissue#772]: https://github.com/ClusterLabs/pcs/issues/772 + + ## [0.11.7] - 2024-01-11 ### Security diff --git a/pcs/cluster.py b/pcs/cluster.py index 01689684..070287c1 100644 --- a/pcs/cluster.py +++ b/pcs/cluster.py @@ -1176,8 +1176,8 @@ def cluster_uidgid( del lib modifiers.ensure_only_supported() if not argv: - found = False uid_gid_files = os.listdir(settings.corosync_uidgid_dir) + uid_gid_lines: list[str] = [] for ug_file in uid_gid_files: uid_gid_dict = utils.read_uid_gid_file(ug_file) if "uid" in uid_gid_dict or "gid" in uid_gid_dict: @@ -1188,9 +1188,10 @@ def cluster_uidgid( if "gid" in uid_gid_dict: line += uid_gid_dict["gid"] - print(line) - found = True - if not found and not silent_list: + uid_gid_lines.append(line) + if uid_gid_lines: + print("\n".join(sorted(uid_gid_lines))) + elif not silent_list: print_to_stderr("No uidgids configured") return diff --git a/pcs/utils.py b/pcs/utils.py index 44987be1..188f75bc 100644 --- a/pcs/utils.py +++ b/pcs/utils.py @@ -16,9 +16,11 @@ import xml.dom.minidom import xml.etree.ElementTree as ET from functools import lru_cache from io import BytesIO +from textwrap import dedent from typing import ( Any, Dict, + Optional, Tuple, ) from urllib.parse import urlencode @@ -221,6 +223,25 @@ def read_uid_gid_file(uidgid_filename): return uidgid +def get_uidgid_file_content( + uid: Optional[str] = None, gid: Optional[str] = None +) -> Optional[str]: + if not uid and not gid: + return None + uid_gid_lines = [] + if uid: + uid_gid_lines.append(f" uid: {uid}") + if gid: + uid_gid_lines.append(f" gid: {gid}") + return dedent( + """\ + uidgid {{ + {uid_gid_keys} + }} + """ + ).format(uid_gid_keys="\n".join(uid_gid_lines)) + + def write_uid_gid_file(uid, gid): """ Commandline options: no options @@ -237,11 +258,12 @@ def write_uid_gid_file(uid, gid): counter = counter + 1 uidgid_filename = orig_filename + "-" + str(counter) - data = "uidgid {\n uid: %s\ngid: %s\n}\n" % (uid, gid) - with open( - os.path.join(settings.corosync_uidgid_dir, uidgid_filename), "w" - ) as uidgid_file: - uidgid_file.write(data) + data = get_uidgid_file_content(uid, gid) + if data: + with open( + os.path.join(settings.corosync_uidgid_dir, uidgid_filename), "w" + ) as uidgid_file: + uidgid_file.write(data) def find_uid_gid_files(uid, gid): diff --git a/pcs_test/tier1/legacy/test_cluster.py b/pcs_test/tier1/legacy/test_cluster.py index 6e72fe80..03064000 100644 --- a/pcs_test/tier1/legacy/test_cluster.py +++ b/pcs_test/tier1/legacy/test_cluster.py @@ -1,3 +1,4 @@ +import os from functools import partial from unittest import TestCase @@ -6,6 +7,8 @@ from pcs_test.tools.misc import get_test_resource as rc from pcs_test.tools.misc import ( get_tmp_dir, get_tmp_file, + outdent, + read_test_resource, skip_unless_root, write_file_to_tmpfile, ) @@ -18,28 +21,40 @@ from pcs_test.tools.pcs_runner import ( class UidGidTest(TestCase): def setUp(self): self.uid_gid_dir = get_tmp_dir("tier1_cluster_uidgid") + self._pcs = partial( + pcs, + None, + mock_settings={"corosync_uidgid_dir": self.uid_gid_dir.name}, + ) def tearDown(self): self.uid_gid_dir.cleanup() + def assert_uidgid_file_content(self, filename, content): + self.assertEqual( + read_test_resource(os.path.join(self.uid_gid_dir.name, filename)), + content, + ) + + def assert_uidgid_file_removed(self, filename): + self.assertEqual( + os.path.isfile(os.path.join(self.uid_gid_dir.name, filename)), + False, + ) + def test_uidgid(self): # pylint: disable=too-many-statements - _pcs = partial( - pcs, - None, - mock_settings={"corosync_uidgid_dir": self.uid_gid_dir.name}, - ) - stdout, stderr, retval = _pcs("cluster uidgid".split()) + stdout, stderr, retval = self._pcs("cluster uidgid".split()) self.assertEqual(stdout, "") self.assertEqual(stderr, "No uidgids configured\n") self.assertEqual(retval, 0) - stdout, stderr, retval = _pcs("cluster uidgid add".split()) + stdout, stderr, retval = self._pcs("cluster uidgid add".split()) self.assertEqual(stdout, "") self.assertTrue(stderr.startswith("\nUsage:")) self.assertEqual(retval, 1) - stdout, stderr, retval = _pcs("cluster uidgid rm".split()) + stdout, stderr, retval = self._pcs("cluster uidgid rm".split()) self.assertEqual(stdout, "") self.assertTrue( stderr.startswith( @@ -49,19 +64,30 @@ class UidGidTest(TestCase): ) self.assertEqual(retval, 1) - stdout, stderr, retval = _pcs("cluster uidgid xx".split()) + stdout, stderr, retval = self._pcs("cluster uidgid xx".split()) self.assertEqual(stdout, "") self.assertTrue(stderr.startswith("\nUsage:")) self.assertEqual(retval, 1) - stdout, stderr, retval = _pcs( + stdout, stderr, retval = self._pcs( "cluster uidgid add uid=testuid gid=testgid".split() ) self.assertEqual(stdout, "") self.assertEqual(stderr, "") self.assertEqual(retval, 0) + self.assert_uidgid_file_content( + "pcs-uidgid-testuid-testgid", + outdent( + """\ + uidgid { + uid: testuid + gid: testgid + } + """ + ), + ) - stdout, stderr, retval = _pcs( + stdout, stderr, retval = self._pcs( "cluster uidgid add uid=testuid gid=testgid".split() ) self.assertEqual(stdout, "") @@ -72,7 +98,7 @@ class UidGidTest(TestCase): ) self.assertEqual(retval, 1) - stdout, stderr, retval = _pcs( + stdout, stderr, retval = self._pcs( "cluster uidgid delete uid=testuid2 gid=testgid2".split() ) self.assertEqual(stdout, "") @@ -82,7 +108,7 @@ class UidGidTest(TestCase): ) self.assertEqual(retval, 1) - stdout, stderr, retval = _pcs( + stdout, stderr, retval = self._pcs( "cluster uidgid remove uid=testuid gid=testgid2".split() ) self.assertEqual(stdout, "") @@ -92,7 +118,7 @@ class UidGidTest(TestCase): ) self.assertEqual(retval, 1) - stdout, stderr, retval = _pcs( + stdout, stderr, retval = self._pcs( "cluster uidgid rm uid=testuid2 gid=testgid".split() ) self.assertEqual(stdout, "") @@ -104,57 +130,139 @@ class UidGidTest(TestCase): ) self.assertEqual(retval, 1) - stdout, stderr, retval = _pcs("cluster uidgid".split()) + stdout, stderr, retval = self._pcs("cluster uidgid".split()) self.assertEqual(stdout, "UID/GID: uid=testuid gid=testgid\n") self.assertEqual(stderr, "") self.assertEqual(retval, 0) - stdout, stderr, retval = _pcs( + stdout, stderr, retval = self._pcs( "cluster uidgid delete uid=testuid gid=testgid".split() ) self.assertEqual(stdout, "") self.assertEqual(stderr, "") self.assertEqual(retval, 0) + self.assert_uidgid_file_removed("pcs-uidgid-testuid-testgid") - stdout, stderr, retval = _pcs( + stdout, stderr, retval = self._pcs( "cluster uidgid add uid=testuid gid=testgid".split() ) self.assertEqual(stdout, "") self.assertEqual(stderr, "") self.assertEqual(retval, 0) - stdout, stderr, retval = _pcs("cluster uidgid".split()) + stdout, stderr, retval = self._pcs("cluster uidgid".split()) self.assertEqual(stdout, "UID/GID: uid=testuid gid=testgid\n") self.assertEqual(stderr, "") self.assertEqual(retval, 0) - stdout, stderr, retval = _pcs( + stdout, stderr, retval = self._pcs( "cluster uidgid remove uid=testuid gid=testgid".split() ) self.assertEqual(stdout, "") self.assertEqual(stderr, "") self.assertEqual(retval, 0) - stdout, stderr, retval = _pcs( + stdout, stderr, retval = self._pcs( "cluster uidgid add uid=testuid gid=testgid".split() ) self.assertEqual(stdout, "") self.assertEqual(stderr, "") self.assertEqual(retval, 0) + self.assert_uidgid_file_content( + "pcs-uidgid-testuid-testgid", + outdent( + """\ + uidgid { + uid: testuid + gid: testgid + } + """ + ), + ) - stdout, stderr, retval = _pcs("cluster uidgid".split()) + stdout, stderr, retval = self._pcs("cluster uidgid".split()) self.assertEqual(stdout, "UID/GID: uid=testuid gid=testgid\n") self.assertEqual(stderr, "") self.assertEqual(retval, 0) - stdout, stderr, retval = _pcs( + stdout, stderr, retval = self._pcs( "cluster uidgid delete uid=testuid gid=testgid".split() ) self.assertEqual(stdout, "") self.assertEqual(stderr, "") self.assertEqual(retval, 0) + self.assert_uidgid_file_removed("pcs-uidgid-testuid-testgid") + + stdout, stderr, retval = self._pcs("cluster uidgid".split()) + self.assertEqual(stdout, "") + self.assertEqual(stderr, "No uidgids configured\n") + self.assertEqual(retval, 0) + + def test_missing_uid_gid(self): + stdout, stderr, retval = self._pcs( + "cluster uidgid add uid=1000".split() + ) + self.assertEqual(stdout, "") + self.assertEqual(stderr, "") + self.assertEqual(retval, 0) + self.assert_uidgid_file_content( + "pcs-uidgid-1000-", + outdent( + """\ + uidgid { + uid: 1000 + } + """ + ), + ) + + stdout, stderr, retval = self._pcs( + "cluster uidgid add gid=1001".split() + ) + self.assertEqual(stdout, "") + self.assertEqual(stderr, "") + self.assertEqual(retval, 0) + self.assert_uidgid_file_content( + "pcs-uidgid--1001", + outdent( + """\ + uidgid { + gid: 1001 + } + """ + ), + ) + + stdout, stderr, retval = self._pcs("cluster uidgid".split()) + self.assertEqual( + stdout, + outdent( + """\ + UID/GID: uid= gid=1001 + UID/GID: uid=1000 gid= + """ + ), + ) + self.assertEqual(stderr, "") + self.assertEqual(retval, 0) + + stdout, stderr, retval = self._pcs( + "cluster uidgid delete uid=1000".split() + ) + self.assertEqual(stdout, "") + self.assertEqual(stderr, "") + self.assertEqual(retval, 0) + self.assert_uidgid_file_removed("pcs-uidgid-1000-") + + stdout, stderr, retval = self._pcs( + "cluster uidgid delete gid=1001".split() + ) + self.assertEqual(stdout, "") + self.assertEqual(stderr, "") + self.assertEqual(retval, 0) + self.assert_uidgid_file_removed("pcs-uidgid--1001") - stdout, stderr, retval = _pcs("cluster uidgid".split()) + stdout, stderr, retval = self._pcs("cluster uidgid".split()) self.assertEqual(stdout, "") self.assertEqual(stderr, "No uidgids configured\n") self.assertEqual(retval, 0) -- 2.25.1