Resolves: rhbz#2163953 rhbz#2216434 rhbz#2217850 rhbz#2219407

- Added BuildRequires: debugedit - for generating MiniDebugInfo - triggered by removing find-debuginfo.sh from rpm
- Make use of filters when extracting tarballs to enhance security if provided by Python (pcs config restore command)
- Exporting constraints with rules in form of pcs commands now escapes # and fixes spaces in dates to make the commands valid
- Constraints containing options unsupported by pcs are not exported and a warning is printed instead
- Using spaces in dates in location constraint rules is deprecated
This commit is contained in:
Michal Pospisil 2023-07-12 16:54:23 +02:00
parent ddf1de3c5d
commit a8b1b4c1d8
5 changed files with 1784 additions and 68 deletions

File diff suppressed because it is too large Load Diff

View File

@ -1,64 +0,0 @@
From 835939b80f3e75616eb585bb1e17dec0a6083a88 Mon Sep 17 00:00:00 2001
From: Tomas Jelinek <tojeline@redhat.com>
Date: Tue, 30 May 2023 13:07:47 +0200
Subject: [PATCH] fix resource move regression
---
CHANGELOG.md | 3 +++
pcs/lib/pacemaker/live.py | 5 ++++-
pcs_test/tools/custom_mock.py | 2 +-
3 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index c7661929..0c56758c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -27,6 +27,8 @@
Pacemaker-2.1.5-rc1 ([rhbz#2177996])
- Make `pcs resource disable --simulate --brief` documentation clearer
([rhbz#2109852])
+- Fixed a regression causing crash in `pcs resource move` command (broken since
+ pcs-0.11.5) ([rhbz#2210855])
### Changed
- Commands for displaying cluster configuration have been slightly updated:
@@ -50,6 +52,7 @@
[rhbz#2175881]: https://bugzilla.redhat.com/show_bug.cgi?id=2175881
[rhbz#2177996]: https://bugzilla.redhat.com/show_bug.cgi?id=2177996
[rhbz#2179388]: https://bugzilla.redhat.com/show_bug.cgi?id=2179388
+[rhbz#2210855]: https://bugzilla.redhat.com/show_bug.cgi?id=2210855
## [0.11.5] - 2023-03-01
diff --git a/pcs/lib/pacemaker/live.py b/pcs/lib/pacemaker/live.py
index c2ba5d60..5a4bfec2 100644
--- a/pcs/lib/pacemaker/live.py
+++ b/pcs/lib/pacemaker/live.py
@@ -384,7 +384,10 @@ def simulate_cib_xml(runner, cib_xml):
string cib_xml -- CIB XML to simulate
"""
try:
- with tools.get_tmp_file() as new_cib_file, tools.get_tmp_file() as transitions_file:
+ with (
+ tools.get_tmp_file(None) as new_cib_file,
+ tools.get_tmp_file(None) as transitions_file,
+ ):
cmd = [
__exec("crm_simulate"),
"--simulate",
diff --git a/pcs_test/tools/custom_mock.py b/pcs_test/tools/custom_mock.py
index 9380afce..90f12abd 100644
--- a/pcs_test/tools/custom_mock.py
+++ b/pcs_test/tools/custom_mock.py
@@ -101,7 +101,7 @@ class TmpFileMock:
except StopIteration:
pass
- def _mock_side_effect(self, data=None, binary=False):
+ def _mock_side_effect(self, data, binary=False):
def _seek_callback(offset):
if offset != 0:
raise AssertionError(
--
2.40.1

View File

@ -0,0 +1,302 @@
From 24a8e84e3f81fc846a8d60dc636c9d42fc7a0cd8 Mon Sep 17 00:00:00 2001
From: Miroslav Lisik <mlisik@redhat.com>
Date: Tue, 4 Jul 2023 21:43:38 +0200
Subject: [PATCH 2/3] fix displaying duplicate records in property commands
---
pcs/cli/cluster_property/output.py | 65 +++++++++----------
.../cli/cluster_property/test_command.py | 15 +++++
.../tier0/cli/cluster_property/test_output.py | 31 ++++++---
.../lib/commands/test_cluster_property.py | 28 ++++++++
4 files changed, 93 insertions(+), 46 deletions(-)
diff --git a/pcs/cli/cluster_property/output.py b/pcs/cli/cluster_property/output.py
index c538c5c1..c9c46d1c 100644
--- a/pcs/cli/cluster_property/output.py
+++ b/pcs/cli/cluster_property/output.py
@@ -31,21 +31,15 @@ class PropertyConfigurationFacade:
readonly_properties: StringCollection,
) -> None:
self._properties = properties
+ self._first_nvpair_set = (
+ self._properties[0].nvpairs if self._properties else []
+ )
self._properties_metadata = properties_metadata
self._readonly_properties = readonly_properties
- self._defaults_map = {
- metadata.name: metadata.default
- for metadata in self._properties_metadata
- if metadata.default is not None
+ self._defaults_map = self.get_defaults(include_advanced=True)
+ self._name_nvpair_dto_map = {
+ nvpair_dto.name: nvpair_dto for nvpair_dto in self._first_nvpair_set
}
- self._name_nvpair_dto_map = (
- {
- nvpair_dto.name: nvpair_dto
- for nvpair_dto in self._properties[0].nvpairs
- }
- if self._properties
- else {}
- )
@classmethod
def from_properties_dtos(
@@ -105,17 +99,6 @@ class PropertyConfigurationFacade:
return value
return self._defaults_map.get(property_name, custom_default)
- @staticmethod
- def _filter_names_advanced(
- metadata: ResourceAgentParameterDto,
- property_names: Optional[StringSequence] = None,
- include_advanced: bool = False,
- ) -> bool:
- return bool(
- (not property_names and (include_advanced or not metadata.advanced))
- or (property_names and metadata.name in property_names)
- )
-
def get_defaults(
self,
property_names: Optional[StringSequence] = None,
@@ -123,11 +106,10 @@ class PropertyConfigurationFacade:
) -> dict[str, str]:
return {
metadata.name: metadata.default
- for metadata in self._properties_metadata
- if metadata.default is not None
- and self._filter_names_advanced(
- metadata, property_names, include_advanced
+ for metadata in self.get_properties_metadata(
+ property_names, include_advanced
)
+ if metadata.default is not None
}
def get_properties_metadata(
@@ -135,23 +117,34 @@ class PropertyConfigurationFacade:
property_names: Optional[StringSequence] = None,
include_advanced: bool = False,
) -> Sequence[ResourceAgentParameterDto]:
- return [
- metadata
- for metadata in self._properties_metadata
- if self._filter_names_advanced(
- metadata, property_names, include_advanced
- )
- ]
+ if property_names:
+ filtered_metadata = [
+ metadata
+ for metadata in self._properties_metadata
+ if metadata.name in property_names
+ ]
+ else:
+ filtered_metadata = [
+ metadata
+ for metadata in self._properties_metadata
+ if include_advanced or not metadata.advanced
+ ]
+ deduplicated_metadata = {
+ metadata.name: metadata for metadata in filtered_metadata
+ }
+ return list(deduplicated_metadata.values())
def get_name_value_default_list(self) -> list[tuple[str, str, bool]]:
name_value_default_list = [
(nvpair_dto.name, nvpair_dto.value, False)
- for nvpair_dto in self._name_nvpair_dto_map.values()
+ for nvpair_dto in self._first_nvpair_set
]
name_value_default_list.extend(
[
(metadata_dto.name, metadata_dto.default, True)
- for metadata_dto in self._properties_metadata
+ for metadata_dto in self.get_properties_metadata(
+ include_advanced=True
+ )
if metadata_dto.name not in self._name_nvpair_dto_map
and metadata_dto.default is not None
]
diff --git a/pcs_test/tier0/cli/cluster_property/test_command.py b/pcs_test/tier0/cli/cluster_property/test_command.py
index b54d0e58..f8cc2afa 100644
--- a/pcs_test/tier0/cli/cluster_property/test_command.py
+++ b/pcs_test/tier0/cli/cluster_property/test_command.py
@@ -21,6 +21,21 @@ from pcs_test.tools.misc import dict_to_modifiers
FIXTURE_PROPERTY_METADATA = ClusterPropertyMetadataDto(
properties_metadata=[
+ ResourceAgentParameterDto(
+ name="property_name",
+ shortdesc="Duplicate property",
+ longdesc=None,
+ type="string",
+ default="duplicate_default",
+ enum_values=None,
+ required=False,
+ advanced=False,
+ deprecated=False,
+ deprecated_by=[],
+ deprecated_desc=None,
+ unique_group=None,
+ reloadable=False,
+ ),
ResourceAgentParameterDto(
name="property_name",
shortdesc=None,
diff --git a/pcs_test/tier0/cli/cluster_property/test_output.py b/pcs_test/tier0/cli/cluster_property/test_output.py
index 0ce8f6a8..59d33466 100644
--- a/pcs_test/tier0/cli/cluster_property/test_output.py
+++ b/pcs_test/tier0/cli/cluster_property/test_output.py
@@ -21,6 +21,7 @@ FIXTURE_TWO_PROPERTY_SETS = [
CibNvpairDto(id="", name="readonly2", value="ro_val2"),
CibNvpairDto(id="", name="property2", value="val2"),
CibNvpairDto(id="", name="property1", value="val1"),
+ CibNvpairDto(id="", name="property1", value="duplicate_val1"),
],
),
CibNvsetDto(
@@ -39,6 +40,7 @@ FIXTURE_READONLY_PROPERTIES_LIST = ["readonly1", "readonly2"]
FIXTURE_TEXT_OUTPUT_FIRST_SET = dedent(
"""\
Cluster Properties: id1 score=150
+ property1=duplicate_val1
property1=val1
property2=val2
readonly1=ro_val1
@@ -75,6 +77,7 @@ def fixture_property_metadata(
FIXTURE_PROPERTY_METADATA_LIST = [
+ fixture_property_metadata(name="property1", default="duplicate_default1"),
fixture_property_metadata(name="property1", default="default1"),
fixture_property_metadata(name="property2", default="default2"),
fixture_property_metadata(
@@ -136,7 +139,7 @@ class TestPropertyConfigurationFacadeGetPropertyValue(TestCase):
)
def test_property_value_from_first_set(self):
- self.assertEqual(self.facade.get_property_value("property1"), "val1")
+ self.assertEqual(self.facade.get_property_value("property2"), "val2")
def test_property_value_from_second_set(self):
self.assertEqual(self.facade.get_property_value("property3"), None)
@@ -152,6 +155,11 @@ class TestPropertyConfigurationFacadeGetPropertyValue(TestCase):
"custom",
)
+ def test_property_with_multiple_values(self):
+ self.assertEqual(
+ self.facade.get_property_value("property1"), "duplicate_val1"
+ )
+
class TestPropertyConfigurationFacadeGetPropertyValueOrDefault(TestCase):
def setUp(self):
@@ -163,7 +171,7 @@ class TestPropertyConfigurationFacadeGetPropertyValueOrDefault(TestCase):
def test_property_value_from_first_set(self):
self.assertEqual(
- self.facade.get_property_value_or_default("property1"), "val1"
+ self.facade.get_property_value_or_default("property2"), "val2"
)
def test_property_value_not_in_set(self):
@@ -239,21 +247,22 @@ class TestPropertyConfigurationFacadeGetPropertiesMetadata(TestCase):
)
def test_metadata_without_advanced(self):
- metadata = FIXTURE_PROPERTY_METADATA_LIST[0:2]
- self.assertEqual(self.facade.get_properties_metadata(), metadata)
+ metadata = FIXTURE_PROPERTY_METADATA_LIST[1:3]
+ self.assertCountEqual(self.facade.get_properties_metadata(), metadata)
def test_metadata_with_advanced(self):
- metadata = FIXTURE_PROPERTY_METADATA_LIST
- self.assertEqual(
- self.facade.get_properties_metadata(include_advanced=True), metadata
+ metadata = FIXTURE_PROPERTY_METADATA_LIST[1:]
+ self.assertCountEqual(
+ self.facade.get_properties_metadata(include_advanced=True),
+ metadata,
)
def test_metadata_specified(self):
metadata = (
- FIXTURE_PROPERTY_METADATA_LIST[0:1]
+ FIXTURE_PROPERTY_METADATA_LIST[1:2]
+ FIXTURE_PROPERTY_METADATA_LIST[-1:]
)
- self.assertEqual(
+ self.assertCountEqual(
self.facade.get_properties_metadata(
property_names=["property4", "property1"]
),
@@ -275,6 +284,7 @@ class TestPropertyConfigurationFacadeGetNameValueDefaultList(TestCase):
("readonly2", "ro_val2", False),
("property2", "val2", False),
("property1", "val1", False),
+ ("property1", "duplicate_val1", False),
("property3", "default3", True),
("property4", "default4", True),
]
@@ -503,7 +513,8 @@ class TestPropertiesToCmd(TestCase):
"""\
pcs property set --force -- \\
property2=val2 \\
- property1=val1
+ property1=val1 \\
+ property1=duplicate_val1
"""
)
self.assert_lines(facade, output)
diff --git a/pcs_test/tier0/lib/commands/test_cluster_property.py b/pcs_test/tier0/lib/commands/test_cluster_property.py
index c7cb7ae5..c02761a0 100644
--- a/pcs_test/tier0/lib/commands/test_cluster_property.py
+++ b/pcs_test/tier0/lib/commands/test_cluster_property.py
@@ -911,6 +911,10 @@ class TestGetProperties(TestCase):
)
self.env_assist.assert_reports([])
+ @mock.patch(
+ "pcs.lib.cib.rule.in_effect.has_rule_in_effect_status_tool",
+ lambda: True,
+ )
def test_evaluate_expired_but_no_set_rule(self):
self.config.runner.cib.load(
crm_config=fixture_crm_config_properties([("set_id", {})])
@@ -924,6 +928,30 @@ class TestGetProperties(TestCase):
),
)
+ @mock.patch(
+ "pcs.lib.cib.rule.in_effect.has_rule_in_effect_status_tool",
+ lambda: False,
+ )
+ def test_evaluate_expired_no_status_tool(self):
+ self.config.runner.cib.load(
+ crm_config=fixture_crm_config_properties([("set_id", {})])
+ )
+ self.assertEqual(
+ self.command(evaluate_expired=True),
+ ListCibNvsetDto(
+ nvsets=[
+ CibNvsetDto(id="set_id", options={}, rule=None, nvpairs=[])
+ ]
+ ),
+ )
+ self.env_assist.assert_reports(
+ [
+ fixture.warn(
+ reports.codes.RULE_IN_EFFECT_STATUS_DETECTION_NOT_SUPPORTED,
+ )
+ ]
+ )
+
class TestGetPropertiesMetadata(MetadataErrorMixin, TestCase):
_load_cib_when_metadata_error = False
--
2.41.0

View File

@ -0,0 +1,55 @@
From e47799cbdd588649872efd24d6bcfa78acb23ecb Mon Sep 17 00:00:00 2001
From: Tomas Jelinek <tojeline@redhat.com>
Date: Tue, 11 Jul 2023 14:09:17 +0200
Subject: [PATCH 3/3] use a filter when extracting a config backup tarball
---
pcs/config.py | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/pcs/config.py b/pcs/config.py
index 56c49aae..d750f52f 100644
--- a/pcs/config.py
+++ b/pcs/config.py
@@ -488,14 +488,36 @@ def config_restore_local(infile_name, infile_obj):
if "rename" in extract_info and extract_info["rename"]:
if tmp_dir is None:
tmp_dir = tempfile.mkdtemp()
- tarball.extractall(tmp_dir, [tar_member_info])
+ if hasattr(tarfile, "data_filter"):
+ # Safe way of extraction is available since Python 3.12,
+ # hasattr above checks if it's available.
+ # It's also backported to 3.11.4, 3.10.12, 3.9.17.
+ # It may be backported to older versions in downstream.
+ tarball.extractall(
+ tmp_dir, [tar_member_info], filter="data"
+ )
+ else:
+ # Unsafe way of extraction
+ # Remove once we don't support Python 3.8 and older
+ tarball.extractall(tmp_dir, [tar_member_info])
path_full = extract_info["path"]
shutil.move(
os.path.join(tmp_dir, tar_member_info.name), path_full
)
else:
dir_path = os.path.dirname(extract_info["path"])
- tarball.extractall(dir_path, [tar_member_info])
+ if hasattr(tarfile, "data_filter"):
+ # Safe way of extraction is available since Python 3.12,
+ # hasattr above checks if it's available.
+ # It's also backported to 3.11.4, 3.10.12, 3.9.17.
+ # It may be backported to older versions in downstream.
+ tarball.extractall(
+ dir_path, [tar_member_info], filter="data"
+ )
+ else:
+ # Unsafe way of extracting
+ # Remove once we don't support Python 3.8 and older
+ tarball.extractall(dir_path, [tar_member_info])
path_full = os.path.join(dir_path, tar_member_info.name)
file_attrs = extract_info["attrs"]
os.chmod(path_full, file_attrs["mode"])
--
2.41.0

View File

@ -1,6 +1,6 @@
Name: pcs
Version: 0.11.6
Release: 1%{?dist}
Release: 2%{?dist}
# https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/
# https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#Good_Licenses
# GPL-2.0-only: pcs
@ -101,6 +101,9 @@ Source101: https://github.com/ClusterLabs/pcs-web-ui/releases/download/%{ui_comm
# pcs patches: <= 200
# Patch0: bzNUMBER-01-name.patch
Patch0: do-not-support-cluster-setup-with-udp-u-transport.patch
Patch1: bz2163953-01-constraint-fixes.patch
Patch2: bz2217850-01-fix-exporting-location-constraints-with-rules.patch
Patch3: bz2219407-01-use-a-filter-when-extracting-a-config-backup-tarball.patch
# ui patches: >200
# Patch201: bzNUMBER-01-name.patch
@ -158,6 +161,8 @@ BuildRequires: resource-agents
BuildRequires: sbd
# for working with qdevice certificates (certutil) - used in configure.ac
BuildRequires: nss-tools
# for generating MiniDebugInfo with find-debuginfo
BuildRequires: debugedit
# python and libraries for pcs, setuptools for pcs entrypoint
Requires: python3 >= 3.9
@ -291,6 +296,9 @@ update_times_patch(){
%autopatch -p1 -M 200
# update_times_patch %%{PATCH0}
update_times_patch %{PATCH0}
update_times_patch %{PATCH1}
update_times_patch %{PATCH2}
update_times_patch %{PATCH3}
# generate .tarball-version if building from an untagged commit, not a released version
# autogen uses git-version-gen which uses .tarball-version for generating version number
@ -379,10 +387,10 @@ cp %{pcs_bundled_dir}/src/dacite-*/README.md dacite_README.md
# We are not building debug package for pcs but we need to add MiniDebuginfo
# to the bundled shared libraries from rubygem extensions in order to satisfy
# rpmdiff's binary stripping checker.
# Therefore we call find-debuginfo.sh script manually in order to strip
# Therefore we call find-debuginfo from debugedit manually in order to strip
# binaries and add MiniDebugInfo with .gnu_debugdata section
/usr/lib/rpm/find-debuginfo.sh -j2 -m -i -S debugsourcefiles.list
# find-debuginfo.sh generated some files into /usr/lib/debug and
find-debuginfo -j2 -m -i -S debugsourcefiles.list
# find-debuginfo generated some files into /usr/lib/debug and
# /usr/src/debug/ that we don't want in the package
rm -rf $RPM_BUILD_ROOT%{_libdir}/debug
rm -rf $RPM_BUILD_ROOT/usr/lib/debug
@ -527,6 +535,14 @@ run_all_tests
%license pyagentx_LICENSE.txt
%changelog
* Mon Jul 10 2023 Michal Pospisil <mpospisi@redhat.com> - 0.11.6-2
- Added BuildRequires: debugedit - for generating MiniDebugInfo - triggered by removing find-debuginfo.sh from rpm
- Make use of filters when extracting tarballs to enhance security if provided by Python (pcs config restore command)
- Exporting constraints with rules in form of pcs commands now escapes # and fixes spaces in dates to make the commands valid
- Constraints containing options unsupported by pcs are not exported and a warning is printed instead
- Using spaces in dates in location constraint rules is deprecated
- Resolves: rhbz#2163953 rhbz#2216434 rhbz#2217850 rhbz#2219407
* Tue Jun 20 2023 Michal Pospisil <mpospisi@redhat.com> - 0.11.6-1
- Rebased to the latest upstream sources (see CHANGELOG.md)
- Updated bundled rubygems: puma, tilt