Skip to content

Commit

Permalink
Fix migration of rules including passwords
Browse files Browse the repository at this point in the history
It was possible that `cmk-update-config` failed because of an
exception in `cmk --automation update-passwords-merged-file`.

This exception was raised, if multiple old rules (which have to
contain passwords) in multiple folders belonged to a migrated
rulesets. After saving every `rules.mk` file, the automation
command was called.

So, after saving migrated rules in the first file the hook
command would be called. Processing the non-migrated rules in
the other files triggered the command into raising an AttributeError.

This is now fixed by saving all `rules.mk` files first and
calling the hook command only once afterward.

CMK-21820

Change-Id: I3175473b2d8b73578de091e514bf675b05234f17
  • Loading branch information
crazyscientist committed Feb 28, 2025
1 parent 81af462 commit b503834
Show file tree
Hide file tree
Showing 2 changed files with 182 additions and 13 deletions.
36 changes: 23 additions & 13 deletions cmk/gui/watolib/rulesets.py
Original file line number Diff line number Diff line change
Expand Up @@ -435,27 +435,30 @@ def replace_folder_config(

self.replace_folder_ruleset_config(folder, ruleset_config, varname)

@staticmethod
def _update_password_file() -> None:
subprocess.check_call(
["cmk", "--automation", "update-passwords-merged-file"], stdout=subprocess.DEVNULL
)

@staticmethod
def _save_folder(
folder: Folder,
rulesets: Mapping[RulesetName, Ruleset],
unknown_rulesets: Mapping[str, Mapping[str, Sequence[RuleSpec[object]]]],
) -> None:
) -> bool:
RuleConfigFile(Path(folder.rules_file_path())).save_rulesets_and_unknown_rulesets(
rulesets, unknown_rulesets
)

# check if this contains a password. If so, update the password file
if any(
# check if this contains a password. If so, the password file must be updated
return any(
process_configuration_to_parameters(rule.value).found_secrets
for name, rules in rulesets.items()
if RuleGroup.is_active_checks_rule(name) or RuleGroup.is_special_agents_rule(name)
for rule in rules.get_folder_rules(folder)
if isinstance(rule.value, dict) # this is true for all _FormSpec_ SSC rules.
):
subprocess.check_call(
["cmk", "--automation", "update-passwords-merged-file"], stdout=subprocess.DEVNULL
)
)

def exists(self, name: RulesetName) -> bool:
return name in self._rulesets
Expand Down Expand Up @@ -522,16 +525,22 @@ def load_all_rulesets() -> AllRulesets:

def save(self) -> None:
"""Save all rulesets of all folders recursively"""
self._save_rulesets_recursively(folder_tree().root_folder())
if self._save_rulesets_recursively(folder_tree().root_folder()):
self._update_password_file()

def save_folder(self, folder: Folder) -> None:
self._save_folder(folder, self._rulesets, self._unknown_rulesets)
if self._save_folder(folder, self._rulesets, self._unknown_rulesets):
self._update_password_file()

def _save_rulesets_recursively(self, folder: Folder) -> None:
def _save_rulesets_recursively(self, folder: Folder) -> bool:
needs_password_file_updating = False
for subfolder in folder.subfolders():
self._save_rulesets_recursively(subfolder)
needs_password_file_updating |= self._save_rulesets_recursively(subfolder)

self._save_folder(folder, self._rulesets, self._unknown_rulesets)
needs_password_file_updating |= self._save_folder(
folder, self._rulesets, self._unknown_rulesets
)
return needs_password_file_updating


def visible_rulesets(rulesets: Mapping[RulesetName, Ruleset]) -> Mapping[RulesetName, Ruleset]:
Expand Down Expand Up @@ -615,7 +624,8 @@ def load_folder_rulesets(folder: Folder) -> FolderRulesets:
return self

def save_folder(self) -> None:
RulesetCollection._save_folder(self._folder, self._rulesets, self._unknown_rulesets)
if RulesetCollection._save_folder(self._folder, self._rulesets, self._unknown_rulesets):
RulesetCollection._update_password_file()


class Ruleset:
Expand Down
159 changes: 159 additions & 0 deletions tests/unit/cmk/gui/watolib/test_watolib_rulesets.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
#!/usr/bin/env python3
# Copyright (C) 2025 Checkmk GmbH - License: GNU General Public License v2
# This file is part of Checkmk (https://checkmk.com). It is subject to the terms and
# conditions defined in the file COPYING, which is part of this source code package.
import subprocess
from collections.abc import Callable
from typing import Any

import pytest

from cmk.gui.valuespec import Dictionary
from cmk.gui.wato.pages._password_store_valuespecs import (
IndividualOrStoredPassword,
)
from cmk.gui.watolib.hosts_and_folders import Folder, folder_tree
from cmk.gui.watolib.rulesets import (
AllRulesets,
Rule,
RuleConditions,
RuleConfigFile,
RuleOptions,
Ruleset,
)
from cmk.gui.watolib.rulespec_groups import RulespecGroupMonitoringConfigurationVarious
from cmk.gui.watolib.rulespecs import Rulespec


class MockCall:
def __init__(self, callback: Callable[..., Any] | None = None) -> None:
self._callback = callback
self.call_list: list[tuple[tuple[Any, ...], dict[str, Any]]] = []

def __call__(self, *args: Any, **kwargs: Any) -> Any:
self.call_list.append((args, kwargs))
if callable(self._callback):
return self._callback()
return None

@property
def call_count(self) -> int:
return len(self.call_list)


@pytest.fixture(name="allrulesets_with_rules_in_multiple_files")
def fixture_allrulesets_with_rules_in_multiple_files() -> AllRulesets:
rule_name = "special_agents:foo_rule"

ruleset = Ruleset(
name=rule_name,
tag_to_group_map={},
rulespec=Rulespec(
name="foo-rulespec",
group=RulespecGroupMonitoringConfigurationVarious,
title=lambda: "foo-rulespec",
valuespec=lambda: Dictionary(
elements=[("value", IndividualOrStoredPassword())], optional_keys=False
),
match_type="dict",
item_type=None,
item_spec=None,
item_name=None,
item_help=None,
is_optional=False,
is_deprecated=False,
is_cloud_and_managed_edition_only=False,
is_for_services=False,
is_binary_ruleset=False,
factory_default={
"value": (
"cmk_postprocessed",
"explicit_password",
("uuid992d5025-f689-43de-b9c4-09db3a345976", "password"),
)
},
help_func=None,
doc_references=None,
),
)
tree = folder_tree()
root_folder = tree.root_folder()
sub_folder = Folder(
tree=tree, # type: ignore[abstract]
name="Sub",
folder_id="sub",
folder_path="sub",
parent_folder=root_folder,
validators=root_folder.validators,
title="Sub",
attributes=root_folder.attributes,
locked=False,
locked_subfolders=False,
num_hosts=0,
hosts=None,
)
root_folder._subfolders["sub"] = sub_folder
tree.all_folders = lambda: {"": root_folder, "sub": sub_folder} # type: ignore[method-assign]
ruleset.append_rule(
folder=root_folder,
rule=Rule(
id_="id-1",
folder=root_folder,
ruleset=ruleset,
conditions=RuleConditions(host_folder=root_folder.name()),
options=RuleOptions(disabled=False, description="", comment="", docu_url=""),
value={
"value": (
"cmk_postprocessed",
"explicit_password",
("uuid992d5025-f689-43de-b9c4-09db3a345977", "root password"),
)
},
),
)
ruleset.append_rule(
folder=sub_folder,
rule=Rule(
id_="id-2",
folder=sub_folder,
ruleset=ruleset,
conditions=RuleConditions(host_folder=sub_folder.name()),
options=RuleOptions(disabled=False, description="", comment="", docu_url=""),
value={
"value": (
"cmk_postprocessed",
"explicit_password",
("uuid992d5025-f689-43de-b9c4-09db3a345978", "sub password"),
)
},
),
)
return AllRulesets(rulesets={rule_name: ruleset})


@pytest.mark.parametrize(
["callback"],
[
pytest.param(None, id="Successful hook call"),
pytest.param(lambda *args, **kwargs: 1 / 0, id="Exception in hook call"),
],
)
@pytest.mark.usefixtures("request_context", "allrulesets_with_rules_in_multiple_files")
def test_all_rulesets_save(
callback: Callable[..., Any] | None,
allrulesets_with_rules_in_multiple_files: AllRulesets,
monkeypatch: pytest.MonkeyPatch,
) -> None:
mock_check_call = MockCall(callback=callback)
mock_save_call = MockCall()
monkeypatch.setattr(subprocess, "check_call", mock_check_call)
monkeypatch.setattr(RuleConfigFile, "save_rulesets_and_unknown_rulesets", mock_save_call)

if callback is not None:
with pytest.raises(ZeroDivisionError):
allrulesets_with_rules_in_multiple_files.save()
else:
allrulesets_with_rules_in_multiple_files.save()

assert mock_save_call.call_count == 2
assert mock_check_call.call_count == 1

0 comments on commit b503834

Please sign in to comment.