Checkmk git commits September 2018

checkmk-commits@lists.checkmk.com

12 participants
371 discussions

6477 FIX Fixed links in the snaping "About Check_MK"

by Marcel Arentz

Module: check_mk Branch: master Commit: 807ae4b57db7fe01afc289edd7f42b6c3a9ddc5d URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=807ae4b57db7fe… Author: Marcel Arentz <ma(a)mathias-kettner.de> Date: Fri Sep 7 12:52:25 2018 +0200 6477 FIX Fixed links in the snaping "About Check_MK" The links now lead to the correct pages of the new website again. Change-Id: If6921b589442551feac990afaec3730b7577f746 --- .werks/6477 | 10 ++++++++++ cmk/gui/plugins/sidebar/about.py | 7 +++---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/.werks/6477 b/.werks/6477 new file mode 100644 index 0000000..dc7297d --- /dev/null +++ b/.werks/6477 @@ -0,0 +1,10 @@ +Title: Fixed links in the snaping "About Check_MK" +Level: 1 +Component: multisite +Compatible: compat +Edition: cre +Version: 1.6.0i1 +Date: 1536317465 +Class: fix + +The links now lead to the correct pages of the new website again. diff --git a/cmk/gui/plugins/sidebar/about.py b/cmk/gui/plugins/sidebar/about.py index 001f5e1..8b02051 100644 --- a/cmk/gui/plugins/sidebar/about.py +++ b/cmk/gui/plugins/sidebar/about.py @@ -51,10 +51,9 @@ class About(SidebarSnapin): def show(self): html.write(_("Version: ") + cmk.__version__) html.open_ul() - bulletlink(_("Homepage"), "https://mathias-kettner.com/check_mk.html") - bulletlink(_("Documentation"), "https://mathias-kettner.com/checkmk.html") - bulletlink(_("Download"), "https://mathias-kettner.com/check_mk_download.html") - bulletlink("Mathias Kettner", "https://mathias-kettner.com") + bulletlink(_("Homepage"), "https://mathias-kettner.com/index.html") + bulletlink(_("Documentation"), "https://mathias-kettner.com/cms.html") + bulletlink(_("Download"), "https://mathias-kettner.com/download.php") html.close_ul()

6 years

6552 FIX Role changes now create change entries on central site

by Lars Michelsen

Module: check_mk Branch: master Commit: a181ae6987961709bb3118810d2d3a28eff0cba3 URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=a181ae69879617… Author: Lars Michelsen <lm(a)mathias-kettner.de> Date: Fri Sep 7 13:56:16 2018 +0200 6552 FIX Role changes now create change entries on central site Changes to the roles in WATO triggered change entries for remote sites where the user login is allowed but were missing to add change entries for the central site. This is relevant for the "discard changes" feature. In case such a role change is made by one user and a another, less privileged user, tries to discards his changes it would be possible to discard the changes of the first user in case no change entry is created. Change-Id: I5ce62497b00158b6e55459c0fd267528f10ecfba --- .werks/6552 | 16 ++++++++++++++++ cmk/gui/config.py | 7 ++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/.werks/6552 b/.werks/6552 new file mode 100644 index 0000000..793214e --- /dev/null +++ b/.werks/6552 @@ -0,0 +1,16 @@ +Title: Role changes now create change entries on central site +Level: 1 +Component: wato +Class: fix +Compatible: compat +Edition: cre +State: unknown +Version: 1.6.0i1 +Date: 1536239687 + +Changes to the roles in WATO triggered change entries for remote sites where the user login +is allowed but were missing to add change entries for the central site. + +This is relevant for the "discard changes" feature. In case such a role change is made by one user +and a another, less privileged user, tries to discards his changes it would be possible to discard +the changes of the first user in case no change entry is created. diff --git a/cmk/gui/config.py b/cmk/gui/config.py index 7acf278..b109980 100644 --- a/cmk/gui/config.py +++ b/cmk/gui/config.py @@ -1008,9 +1008,14 @@ def _has_distributed_wato_file(): and os.stat(cmk.paths.check_mk_config_dir + "/distributed_wato.mk").st_size != 0 +def get_login_sites(): + """Returns the WATO slave sites a user may login and the local site""" + return get_login_slave_sites() + [ omd_site() ] + + # TODO: All site listing functions should return the same data structure, e.g. a list of # pairs (site_id, site) -def get_login_sites(): +def get_login_slave_sites(): """Returns a list of site ids which are WATO slave sites and users can login""" sites = [] for site_id, site in wato_slave_sites():

6 years

6551 FIX Fixed missing permission checking during "Discard changes"

by Lars Michelsen

Module: check_mk Branch: master Commit: d83e2ed04df3bc37cca7b84f02ccb59b386fb241 URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=d83e2ed04df3bc… Author: Lars Michelsen <lm(a)mathias-kettner.de> Date: Fri Sep 7 13:55:05 2018 +0200 6551 FIX Fixed missing permission checking during "Discard changes" Users with reduced privileges that have access to WATO e.g. for managing their own hosts in specific folders have access to the "Activate changes" page for activating their config changes. On that page there is a button "discard changes" which can be used to undo the configuration changes that have been made since the last activation. A limitation of this feature is that it can only revert the whole configuration at all, not only the configuration made by this user. A permitted user would discard all changes, even the ones made by others in the moment he clicks on discard changes. The activation mechanism is aware of the situation where others have made changes. Users that are not permitted to activate foreign changes are not able to activate their changes in this situation and get a warning message about this. However, the discard changes action was missing such a check which allowed those users to discard the changes of others while they should not be allowed to do so. The discard changes action is now using the same logic as the activate changes action for checking whether or not a user is allowed to do this. Change-Id: I09433119f02bc3b4149f431430d7271f542da610 --- .werks/6551 | 32 ++++++++++++++++++++++++++++++++ cmk/gui/wato/__init__.py | 24 ++++++++++++++++++++++-- cmk/gui/watolib.py | 2 +- 3 files changed, 55 insertions(+), 3 deletions(-) diff --git a/.werks/6551 b/.werks/6551 new file mode 100644 index 0000000..5c54c02 --- /dev/null +++ b/.werks/6551 @@ -0,0 +1,32 @@ +Title: Fixed missing permission checking during "Discard changes" +Level: 2 +Component: wato +Class: fix +Compatible: compat +Edition: cre +State: unknown +Version: 1.6.0i1 +Date: 1536237858 + +Users with reduced privileges that have access to WATO e.g. for managing their +own hosts in specific folders have access to the "Activate changes" page for +activating their config changes. On that page there is a button "discard +changes" which can be used to undo the configuration changes that have been +made since the last activation. + +A limitation of this feature is that it can only revert the whole configuration +at all, not only the configuration made by this user. A permitted user would +discard all changes, even the ones made by others in the moment he clicks on +discard changes. + +The activation mechanism is aware of the situation where others have made +changes. Users that are not permitted to activate foreign changes are not +able to activate their changes in this situation and get a warning message +about this. + +However, the discard changes action was missing such a check which allowed +those users to discard the changes of others while they should not be allowed +to do so. + +The discard changes action is now using the same logic as the activate changes +action for checking whether or not a user is allowed to do this. diff --git a/cmk/gui/wato/__init__.py b/cmk/gui/wato/__init__.py index 6a222f1..0d57e58 100644 --- a/cmk/gui/wato/__init__.py +++ b/cmk/gui/wato/__init__.py @@ -5475,7 +5475,7 @@ class ModeActivateChanges(WatoMode, watolib.ActivateChanges): home_button() # TODO: Remove once new changes mechanism has been implemented - if config.user.may("wato.activate") and self.has_changes() and self._get_last_wato_snapshot_file(): + if self._may_discard_changes(): html.context_button(_("Discard Changes!"), html.makeactionuri([("_action", "discard")]), "discard", id="discard_changes_button") @@ -5487,6 +5487,23 @@ class ModeActivateChanges(WatoMode, watolib.ActivateChanges): html.context_button(_("Audit Log"), watolib.folder_preserving_link([("mode", "auditlog")]), "auditlog") + + def _may_discard_changes(self): + if not config.user.may("wato.activate"): + return False + + if not self.has_changes(): + return False + + if not config.user.may("wato.activateforeign") and self._has_foreign_changes_on_any_site(): + return False + + if not self._get_last_wato_snapshot_file(): + return False + + return True + + def action(self): if html.var("_action") != "discard": return @@ -5494,6 +5511,9 @@ class ModeActivateChanges(WatoMode, watolib.ActivateChanges): if not html.check_transaction(): return + if not self._may_discard_changes(): + return + # TODO: Remove once new changes mechanism has been implemented # Now remove all currently pending changes by simply restoring the last automatically # taken snapshot. Then activate the configuration. This should revert all pending changes. @@ -5593,7 +5613,7 @@ class ModeActivateChanges(WatoMode, watolib.ActivateChanges): return if not config.user.may("wato.activateforeign") \ - and self._has_foreign_changes_on_all_sites(): + and self._has_foreign_changes_on_any_site(): html.show_warning(_("Sorry, you are not allowed to activate changes of other users.")) return diff --git a/cmk/gui/watolib.py b/cmk/gui/watolib.py index 689d0f1..9022916 100644 --- a/cmk/gui/watolib.py +++ b/cmk/gui/watolib.py @@ -5080,7 +5080,7 @@ class ActivateChanges(object): if self._is_foreign(change)) - def _has_foreign_changes_on_all_sites(self): + def _has_foreign_changes_on_any_site(self): return any(change for _change_id, change in self._changes if self._is_foreign(change) and self._affects_all_sites(change))

6 years

6464 FIX Linux Agent: Fix excluded filesystems in df section

by Konstantin Büttner

Module: check_mk Branch: master Commit: 4122c176cbe9f80bee939c21d3f087ebd6d6b166 URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=4122c176cbe9f8… Author: Konstantin Büttner <kb(a)mathias-kettner.de> Date: Fri Sep 7 12:02:41 2018 +0200 6464 FIX Linux Agent: Fix excluded filesystems in df section Change-Id: Ib13b37d1ed8fc4a9415e34675b5513d26fd693b8 --- .werks/6464 | 11 +++++++++++ agents/check_mk_agent.linux | 4 ++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/.werks/6464 b/.werks/6464 new file mode 100644 index 0000000..fea0e2e --- /dev/null +++ b/.werks/6464 @@ -0,0 +1,11 @@ +Title: Linux Agent: Fix excluded filesystems in df section +Level: 1 +Component: checks +Class: fix +Compatible: compat +Edition: cre +State: unknown +Version: 1.6.0i1 +Date: 1536314453 + + diff --git a/agents/check_mk_agent.linux b/agents/check_mk_agent.linux index 5180426..99177ac 100755 --- a/agents/check_mk_agent.linux +++ b/agents/check_mk_agent.linux @@ -169,12 +169,12 @@ function section_df() # The exclusion list is getting a bit of a problem. -l should hide any remote FS but seems # to be all but working. local excludefs="-x smbfs -x cifs -x iso9660 -x udf -x nfsv4 -x nfs -x mvfs -x zfs -x prl_fs" - df -PTlk "$excludefs" | sed 1d + df -PTlk $excludefs | sed 1d # df inodes information echo '<<<df>>>' echo '[df_inodes_start]' - df -PTli "$excludefs" | sed 1d + df -PTli $excludefs | sed 1d echo '[df_inodes_end]' }

6 years

Centralized snapin permission name computation

by Lars Michelsen

Module: check_mk Branch: master Commit: 8131da5e87db953cda04cf7cfe4afdac549fcaf8 URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=8131da5e87db95… Author: Lars Michelsen <lm(a)mathias-kettner.de> Date: Thu Aug 23 14:30:15 2018 +0200 Centralized snapin permission name computation CMK-855 Change-Id: I50ac7825c0686db83d5cff44a16eb71174c4a713 --- cmk/gui/plugins/sidebar/utils.py | 11 +++++++++++ cmk/gui/sidebar.py | 23 +++++++++++++---------- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/cmk/gui/plugins/sidebar/utils.py b/cmk/gui/plugins/sidebar/utils.py index ed9d618..2150483 100644 --- a/cmk/gui/plugins/sidebar/utils.py +++ b/cmk/gui/plugins/sidebar/utils.py @@ -48,6 +48,12 @@ class SidebarSnapin(object): @classmethod @abc.abstractmethod + def type_name(cls): + raise NotImplementedError() + + + @classmethod + @abc.abstractmethod def title(cls): raise NotImplementedError() @@ -74,6 +80,11 @@ class SidebarSnapin(object): @classmethod + def permission_name(cls): + return "sidesnap.%s" % cls.type_name() + + + @classmethod def allowed_roles(cls): return [ "admin", "user", "guest" ] diff --git a/cmk/gui/sidebar.py b/cmk/gui/sidebar.py index 3945402..d4f19f4 100644 --- a/cmk/gui/sidebar.py +++ b/cmk/gui/sidebar.py @@ -356,13 +356,17 @@ class UserSidebarConfig(object): user_config["snapins"] = self._transform_legacy_tuples(user_config["snapins"]) user_config["snapins"] = self._transform_legacy_off_state(user_config["snapins"]) - # Remove entries the user is not allowed for and silently skip - # configured but not existing snapins + # Remove not existing (e.g. legacy) snapins user_config["snapins"] = [ e for e in user_config["snapins"] - if e["snapin_type_id"] in snapin_registry - and self._user.may("sidesnap." + e["snapin_type_id"])] + if e["snapin_type_id"] in snapin_registry ] - return self._from_config(user_config) + user_config = self._from_config(user_config) + + # Remove entries the user is not allowed for + user_config["snapins"] = [ e for e in user_config["snapins"] + if config.user.may(e.snapin_type.permission_name()) ] + + return user_config def _transform_legacy_list_config(self, user_config): @@ -478,8 +482,7 @@ def page_side(): html.open_div(class_="scroll" if config.sidebar_show_scrollbar else None, id_="side_content") for snapin in user_config.snapins: name = snapin.snapin_type.type_name() - if not name in snapin_registry or not config.user.may("sidesnap." + name): - continue + # Performs the initial rendering and might return an optional refresh url, # when the snapin contents are refreshed from an external source refresh_url = render_snapin(name, snapin.visible) @@ -602,9 +605,9 @@ def ajax_snapin(): snapin_code = [] for snapname in snapnames: - if not config.user.may("sidesnap." + snapname): - continue snapin_class = snapin_registry.get(snapname) + if not config.user.may(snapin_class.permission_name()): + continue snapin = snapin_class() # When restart snapins are about to be refreshed, only render @@ -710,7 +713,7 @@ def page_add_snapin(): snapin = snapin_class() if name in used_snapins: continue - if not config.user.may("sidesnap." + name): + if not config.user.may(snapin_class.permission_name()): continue # not allowed for this user description = snapin.description()

6 years

Sidebar rendering now works with UserSidebarSnapin objects

by Lars Michelsen

Module: check_mk Branch: master Commit: 43502d30358110843011aaa8ed8c18704caef479 URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=43502d30358110… Author: Lars Michelsen <lm(a)mathias-kettner.de> Date: Thu Aug 23 14:45:38 2018 +0200 Sidebar rendering now works with UserSidebarSnapin objects CMK-855 Change-Id: Ie63fd54f951e7b200a7820166a9578c9279ec5c5 --- cmk/gui/sidebar.py | 64 ++++++++++++++++++++++++++---------------------------- 1 file changed, 31 insertions(+), 33 deletions(-) diff --git a/cmk/gui/sidebar.py b/cmk/gui/sidebar.py index d4f19f4..6ab217a 100644 --- a/cmk/gui/sidebar.py +++ b/cmk/gui/sidebar.py @@ -247,7 +247,7 @@ def ajax_message_read(): html.write("ERROR") -def sidebar_foot(): +def sidebar_foot(user_config): html.open_div(id_="side_footer") if config.user.may("general.configure_sidebar"): html.icon_button("sidebar_add_snapin.py", _("Add snapin to the sidebar"), "sidebar_addsnapin", @@ -272,7 +272,7 @@ def sidebar_foot(): html.close_div() html.close_div() - if UserSidebarConfig(config.user, config.sidebar).folded: + if user_config.folded: html.final_javascript("fold_sidebar();") @@ -485,18 +485,17 @@ def page_side(): # Performs the initial rendering and might return an optional refresh url, # when the snapin contents are refreshed from an external source - refresh_url = render_snapin(name, snapin.visible) + refresh_url = render_snapin(snapin) - snapin_class = snapin_registry.get(name) - if snapin_class.refresh_regularly(): + if snapin.snapin_type.refresh_regularly(): refresh_snapins.append([name, refresh_url]) - elif snapin_class.refresh_on_restart(): + elif snapin.snapin_type.refresh_on_restart(): refresh_snapins.append([name, refresh_url]) restart_snapins.append(name) html.close_div() - sidebar_foot() + sidebar_foot(user_config) html.close_div() html.write("<script language=\"javascript\">\n") @@ -514,21 +513,24 @@ def page_side(): html.body_end() -def render_snapin_styles(snapin): - styles = snapin.styles() +def render_snapin_styles(snapin_instance): + # type: (cmk.gui.plugins.sidebar.SidebarSnapin) -> None + styles = snapin_instance.styles() if styles: html.open_style() html.write(styles) html.close_style() -def render_snapin(name, visibility): - snapin_class = snapin_registry.get(name) - snapin = snapin_class() +def render_snapin(snapin): + # type: (UserSidebarSnapin) -> bytes + snapin_class = snapin.snapin_type + name = snapin_class.type_name() + snapin_instance = snapin_class() html.open_div(id_="snapin_container_%s" % name, class_="snapin") - render_snapin_styles(snapin) + render_snapin_styles(snapin_instance) # When not permitted to open/close snapins, the snapins are always opened - if visibility == SnapinVisibility.OPEN or not config.user.may("general.configure_sidebar"): + if snapin.visibility == SnapinVisibility.OPEN or not config.user.may("general.configure_sidebar"): style = None minimaxi = "mini" else: @@ -545,7 +547,7 @@ def render_snapin(name, visibility): "onmousedown" : "snapinStartDrag(event)", "onmouseup" : "snapinStopDrag(event)"} - html.open_div(class_=["head", visibility.value], **head_actions) + html.open_div(class_=["head", snapin.visibility.value], **head_actions) if config.user.may("general.configure_sidebar"): # Icon for mini/maximizing @@ -567,7 +569,7 @@ def render_snapin(name, visibility): toggle_actions = {"onclick" : "toggle_sidebar_snapin(this,'%s')" % toggle_url, "onmouseover": "this.style.cursor='pointer'", "onmouseout" : "this.style.cursor='auto'"} - html.b(HTML(snapin.title()), class_=["heading"], **toggle_actions) + html.b(HTML(snapin_class.title()), class_=["heading"], **toggle_actions) # End of header html.close_div() @@ -578,7 +580,7 @@ def render_snapin(name, visibility): try: # TODO: Refactor this confusing special case. Add deddicated method or something # to let the snapins make the sidebar know that there is a URL to fetch. - url = snapin.show() + url = snapin_instance.show() if not url is None: # Fetch the contents from an external URL. Don't render it on our own. refresh_url = url @@ -597,23 +599,21 @@ def ajax_snapin(): # Update online state of the user (if enabled) userdb.update_user_access_time(config.user.id) - snapname = html.var("name") - if snapname: - snapnames = [ snapname ] - else: - snapnames = html.var('names', '').split(',') + user_config = UserSidebarConfig(config.user, config.sidebar) + + snapin_id = html.var("name") + snapin_ids = [ snapin_id ] if snapin_id else html.var("names", "").split(",") snapin_code = [] - for snapname in snapnames: - snapin_class = snapin_registry.get(snapname) - if not config.user.may(snapin_class.permission_name()): + for snapin_id in snapin_ids: + snapin_instance = user_config.get_snapin(snapin_id).snapin_type() + if not config.user.may(snapin_instance.permission_name()): continue - snapin = snapin_class() # When restart snapins are about to be refreshed, only render # them, when the core has been restarted after their initial # rendering - if not snapin.refresh_regularly() and snapin.refresh_on_restart(): + if not snapin_instance.refresh_regularly() and snapin_instance.refresh_on_restart(): since = float(html.var('since', 0)) newest = since for site in sites.states().values(): @@ -627,10 +627,10 @@ def ajax_snapin(): with html.plugged(): try: - snapin.show() + snapin_instance.show() except Exception, e: write_snapin_exception(e) - e_message = _("Exception during snapin refresh (snapin \'%s\')") % snapname + e_message = _("Exception during snapin refresh (snapin \'%s\')") % snapin_instance.type_name() logger.error("%s %s: %s" % (html.request.requested_url, e_message, traceback.format_exc())) finally: snapin_code.append(html.drain()) @@ -710,13 +710,11 @@ def page_add_snapin(): html.open_div(class_=["add_snapin"]) for name, snapin_class in sorted(snapin_registry.items()): - snapin = snapin_class() if name in used_snapins: continue if not config.user.may(snapin_class.permission_name()): continue # not allowed for this user - description = snapin.description() transid = html.transaction_manager.get() url = 'sidebar_add_snapin.py?name=%s&_transid=%s&pos=top' % (name, transid) html.open_div(class_="snapinadder", @@ -725,9 +723,9 @@ def page_add_snapin(): html.open_div(class_=["snapin_preview"]) html.div('', class_=["clickshield"]) - render_snapin(name, SnapinVisibility.OPEN) + render_snapin(UserSidebarSnapin.from_snapin_type_id(name)) html.close_div() - html.div(description, class_=["description"]) + html.div(snapin_class.description(), class_=["description"]) html.close_div() html.close_div()

6 years

html.dropdown() and CascadingDropdown now support labels

by Lars Michelsen

Module: check_mk Branch: master Commit: 784031475a86b36a5ff44f0614891e2a6f32e70a URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=784031475a86b3… Author: Lars Michelsen <lm(a)mathias-kettner.de> Date: Fri Aug 24 18:11:12 2018 +0200 html.dropdown() and CascadingDropdown now support labels CMK-856 Change-Id: Ie723b109cff83167932502049379ea2251ac5d00 --- cmk/gui/htmllib.py | 3 +++ cmk/gui/valuespec.py | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/cmk/gui/htmllib.py b/cmk/gui/htmllib.py index f07702f..a3c585f 100644 --- a/cmk/gui/htmllib.py +++ b/cmk/gui/htmllib.py @@ -2417,6 +2417,9 @@ class html(HTMLGenerator): attrs["disabled"] = "disabled" self.hidden_field(varname, current, add_var=False) + if attrs.get("label"): + self.label(attrs["label"], for_=varname) + self.open_select(name=varname, id_=varname, **attrs) for value, text in chs: # if both the default in choices and current was '' then selected depended on the order in choices diff --git a/cmk/gui/valuespec.py b/cmk/gui/valuespec.py index d4a4c7d..4402d90 100644 --- a/cmk/gui/valuespec.py +++ b/cmk/gui/valuespec.py @@ -1767,6 +1767,7 @@ class CascadingDropdown(ValueSpec): else: self._choices = kwargs["choices"] # function, store for later + self._label = kwargs.get("label") self._separator = kwargs.get("separator", ", ") self._sorted = kwargs.get("sorted", True) self._orientation = kwargs.get("orientation", "vertical") # or horizontal @@ -1851,7 +1852,7 @@ class CascadingDropdown(ValueSpec): vp = varprefix + "_sel" onchange="valuespec_cascading_change(this, '%s', %d);" % (varprefix, len(choices)) - html.dropdown(vp, options, deflt=def_val, onchange=onchange, sorted=self._sorted) + html.dropdown(vp, options, deflt=def_val, onchange=onchange, sorted=self._sorted, label=self._label) # make sure, that the visibility is done correctly, in both # cases:

6 years

Simplify add_snapin(), move_snapin_before(), remove_snapin()

by Lars Michelsen

Module: check_mk Branch: master Commit: 32a060b65792d9dbdd4d6ec7edb00fe6cec5e5af URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=32a060b65792d9… Author: Lars Michelsen <lm(a)mathias-kettner.de> Date: Thu Aug 23 14:08:39 2018 +0200 Simplify add_snapin(), move_snapin_before(), remove_snapin() These methods now directly work with UserSidebarSnapin instances instead of all dealing with the Snapin IDs. Instances are now fetched using get_snapin(). CMK-855 Change-Id: I26664c4afeb3726a5556a8343055ed59426a2764 --- cmk/gui/sidebar.py | 92 +++++++++++++++++++----------------------- tests/unit/web/test_sidebar.py | 46 ++++++++++++++++----- 2 files changed, 78 insertions(+), 60 deletions(-) diff --git a/cmk/gui/sidebar.py b/cmk/gui/sidebar.py index c15b82a..3945402 100644 --- a/cmk/gui/sidebar.py +++ b/cmk/gui/sidebar.py @@ -301,64 +301,36 @@ class UserSidebarConfig(object): self.snapins.append(snapin) - def move_snapin_before(self, snapin_id, other_id): - """Move the snapin with the given ID before the other given snapin - The other_id may be None. In this case the snapin is moved to the end. + def move_snapin_before(self, snapin, other): + # type: (UserSidebarSnapin, Union[UserSidebarSnapin,None]) -> None + """Move the given snapin before the other given snapin. + The other may be None. In this case the snapin is moved to the end. """ - # Get current state of snaping being moved (open, closed) - snap_to_move = None - for snapin in self.snapins: - if snapin.snapin_type.type_name() == snapin_id: - snap_to_move = snapin - break - - if not snap_to_move: - raise MKUserError(None, "Snapin being moved is not configured") - - # Build new config by removing snaping at current position - # and add before "other_id" or as last if other_id is not set - new_snapins = [] - for snapin in self.snapins: - if snapin.snapin_type.type_name() == snapin_id: - continue # remove at this position - - elif snapin.snapin_type.type_name() == other_id: - new_snapins.append(snap_to_move) + self.snapins.remove(snapin) - new_snapins.append(snapin) + try: + other_index = self.snapins.index(other) + self.snapins.insert(other_index, snapin) + except ValueError: + self.snapins.append(snapin) - if not other_id: # insert as last - new_snapins.append(snap_to_move) - del self.snapins[:] - self.snapins.extend(new_snapins) - - - def remove_snapin(self, snapin_id): + def remove_snapin(self, snapin): + # type: (UserSidebarSnapin) -> None """Remove the given snapin from the users sidebar""" - new_snapins = [] - for snapin in self.snapins: - if snapin.snapin_type.type_name() == snapin_id: - continue - - new_snapins.append(snapin) - del self.snapins[:] - self.snapins.extend(new_snapins) + self.snapins.remove(snapin) - def set_snapin_visibility(self, snapin_id, state): - """This toggles a snapin between it's visibility states - - Possible states are: open, closed - """ + def get_snapin(self, snapin_id): for snapin in self.snapins: if snapin.snapin_type.type_name() == snapin_id: - snapin.visible = state - break + return snapin + raise KeyError("Snapin %r does not exist" % snapin_id) @property def snapins(self): + # type: () -> List[UserSidebarSnapin] return self._config["snapins"] @@ -470,6 +442,9 @@ class UserSidebarSnapin(object): def __eq__(self, other): + if not isinstance(other, UserSidebarSnapin): + return NotImplemented + return ( self.snapin_type == other.snapin_type and self.visible == other.visible @@ -675,10 +650,17 @@ def ajax_openclose(): raise MKUserError("state", "Invalid state: %s" % state) user_config = UserSidebarConfig(config.user, config.sidebar) + + try: + snapin = user_config.get_snapin(snapin_id) + except KeyError: + return + if state == "off": - user_config.remove_snapin(snapin_id) + user_config.remove_snapin(snapin) else: - user_config.set_snapin_visibility(snapin_id, SnapinVisibility(state)) + snapin.visible = SnapinVisibility(state) + user_config.save() @@ -687,14 +669,22 @@ def move_snapin(): if not config.user.may("general.configure_sidebar"): return - snapin_name = html.var("name") - before_name = html.var("before") + snapin_id = html.var("name") + before_id = html.var("before") user_config = UserSidebarConfig(config.user, config.sidebar) + try: - user_config.move_snapin_before(snapin_name, before_name) - except MKUserError: + snapin = user_config.get_snapin(snapin_id) + except KeyError: return + + try: + before_snapin = user_config.get_snapin(before_id) + except KeyError: + before_snapin = None + + user_config.move_snapin_before(snapin, before_snapin) user_config.save() diff --git a/tests/unit/web/test_sidebar.py b/tests/unit/web/test_sidebar.py index ddf9d64..98f0bfb 100644 --- a/tests/unit/web/test_sidebar.py +++ b/tests/unit/web/test_sidebar.py @@ -42,7 +42,26 @@ def test_user_config_add_snapin(): assert user_config.snapins == [snapin] -(a)pytest.mark.parametrize("move,before,result", [ +def test_user_config_get_snapin(): + user_config = sidebar.UserSidebarConfig(config.user, config.sidebar) + del user_config.snapins[:] + snapin = UserSidebarSnapin.from_snapin_type_id("tactical_overview") + user_config.add_snapin(snapin) + + assert user_config.get_snapin("tactical_overview") == snapin + + +def test_user_config_get_not_existing_snapin(): + user_config = sidebar.UserSidebarConfig(config.user, config.sidebar) + del user_config.snapins[:] + + with pytest.raises(KeyError) as e: + user_config.get_snapin("tactical_overview") + msg = "%s" % e + assert "does not exist" in msg + + +(a)pytest.mark.parametrize("move_id,before_id,result", [ ("tactical_overview", "views", [ UserSidebarSnapin.from_snapin_type_id("admin"), UserSidebarSnapin.from_snapin_type_id("tactical_overview"), @@ -66,7 +85,7 @@ def test_user_config_add_snapin(): UserSidebarSnapin.from_snapin_type_id("admin"), ]), ]) -def test_user_config_move_snapin_before(mocker, move, before, result): +def test_user_config_move_snapin_before(mocker, move_id, before_id, result): user_config = sidebar.UserSidebarConfig(config.user, config.sidebar) del user_config.snapins[:] user_config.snapins.extend([ @@ -75,13 +94,22 @@ def test_user_config_move_snapin_before(mocker, move, before, result): UserSidebarSnapin.from_snapin_type_id("tactical_overview"), ]) - if result is None: - with pytest.raises(MKUserError) as e: - user_config.move_snapin_before(move, before) - assert "Snapin being moved is not configured" in "%s" % e - else: - user_config.move_snapin_before(move, before) - assert user_config.snapins == result + try: + move = user_config.get_snapin(move_id) + except KeyError, e: + if result is None: + assert "does not exist" in "%s" % e + return + else: + raise + + try: + before = user_config.get_snapin(before_id) + except KeyError: + before = None + + user_config.move_snapin_before(move, before) + assert user_config.snapins == result def test_load_default_config(monkeypatch):

6 years

Add unit tests for sidebar config loading and saving

by Lars Michelsen

Module: check_mk Branch: master Commit: 93384160069341ae4aeff9acdacf9e8e8ca5b52b URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=93384160069341… Author: Lars Michelsen <lm(a)mathias-kettner.de> Date: Tue Aug 21 19:38:39 2018 +0200 Add unit tests for sidebar config loading and saving CMK-854 Change-Id: Ia9a9916f3460b636eef724983719dbecb936796c --- tests/unit/web/test_sidebar.py | 187 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 187 insertions(+) diff --git a/tests/unit/web/test_sidebar.py b/tests/unit/web/test_sidebar.py new file mode 100644 index 0000000..0f9cf0c --- /dev/null +++ b/tests/unit/web/test_sidebar.py @@ -0,0 +1,187 @@ +import pytest + +import cmk.gui.sidebar as sidebar +import cmk.gui.config as config +import cmk.gui.pages +import cmk.store as store +from cmk.gui.globals import html + + +(a)pytest.fixture(scope="module", autouse=True) +def gui_config(): + config._initialize_with_default_config() + + +# TODO: Can be removed once all snapins have been refactored +# to class based snapins +(a)pytest.fixture(scope="module", autouse=True) +def load_plugins(): + sidebar.load_plugins(True) + + +(a)pytest.fixture(scope="function", autouse=True) +def user(monkeypatch): + monkeypatch.setattr(config.user, "confdir", "") + monkeypatch.setattr(config.user, "may", lambda x: True) + + +def test_load_default_config(monkeypatch): + # TODO: Drop this once load_user_config uses + # config.user.load_file(...). Then patch this method + monkeypatch.setattr(store, "load_data_from_file", + lambda x: None) + + cfg = sidebar.load_user_config() + assert isinstance(cfg, dict) + assert isinstance(cfg["snapins"], list) + assert cfg["fold"] == False + assert cfg["snapins"] == [ + ('tactical_overview', 'open'), + ('search', 'open'), + ('views', 'open'), + ('reports', 'closed'), + ('bookmarks', 'open'), + ('admin', 'open'), + ('master_control', 'closed') + ] + + +def test_load_legacy_list_user_config(monkeypatch): + monkeypatch.setattr(store, "load_data_from_file", + lambda x: [("tactical_overview", "open"), + ("views", "closed")]) + + cfg = sidebar.load_user_config() + assert cfg["fold"] == False + assert cfg["snapins"] == [ + ('tactical_overview', 'open'), + ('views', 'closed'), + ] + + +def test_load_legacy_off_user_config(monkeypatch): + monkeypatch.setattr(store, "load_data_from_file", + lambda x: [("search", "off"), + ("views", "closed")]) + + cfg = sidebar.load_user_config() + assert cfg["fold"] == False + assert cfg["snapins"] == [ + ('views', 'closed'), + ] + + +def test_load_skip_not_existing(monkeypatch): + monkeypatch.setattr(store, "load_data_from_file", + lambda x: [("bla", "closed"), ("views", "closed")]) + + cfg = sidebar.load_user_config() + assert cfg["fold"] == False + assert cfg["snapins"] == [ + ('views', 'closed'), + ] + + +def test_load_skip_not_permitted(monkeypatch): + monkeypatch.setattr(store, "load_data_from_file", + lambda x: [("tactical_overview", "closed"), ("views", "closed")]) + monkeypatch.setattr(config.user, "may", lambda x: x != "sidesnap.tactical_overview") + + cfg = sidebar.load_user_config() + assert cfg["fold"] == False + assert cfg["snapins"] == [ + ('views', 'closed'), + ] + + +def test_load_user_config(monkeypatch): + monkeypatch.setattr(store, "load_data_from_file", lambda x: { + "fold": True, + "snapins": [ + ("search", "closed"), + ("views", "open"), + ] + }) + + cfg = sidebar.load_user_config() + assert cfg["fold"] == True + assert cfg["snapins"] == [ + ('search', 'closed'), + ('views', 'open'), + ] + + +def test_save_user_config_denied(mocker, monkeypatch): + monkeypatch.setattr(config.user, "may", lambda x: x != "general.configure_sidebar") + save_user_file_mock = mocker.patch.object(config.user, "save_file") + sidebar.save_user_config({}) + save_user_file_mock.assert_not_called() + + +def test_save_user_config_allowed(mocker, monkeypatch): + monkeypatch.setattr(config.user, "may", lambda x: x == "general.configure_sidebar") + save_user_file_mock = mocker.patch.object(config.user, "save_file") + sidebar.save_user_config({}) + save_user_file_mock.assert_called_once_with("sidebar", {}) + + +def test_ajax_fold_page(): + assert cmk.gui.pages.get_page_handler("sidebar_fold") == sidebar.ajax_fold + + +(a)pytest.mark.parametrize("origin_state,fold_var,set_state", [ + (False, "yes", True), + (True, "", False), +]) +def test_ajax_fold(mocker, origin_state, fold_var, set_state): + class MockHtml(object): + def var(self, varname): + if varname == "fold": + return fold_var + + m_load = mocker.patch.object(sidebar, "load_user_config", return_value={"fold": origin_state}) + m_save = mocker.patch.object(sidebar, "save_user_config") + html.set_current(MockHtml()) + + sidebar.ajax_fold() + + m_load.assert_called_once() + m_save.assert_called_once_with({"fold": set_state}) + + +def test_ajax_openclose_page(): + assert cmk.gui.pages.get_page_handler("sidebar_openclose") == sidebar.ajax_openclose + + +(a)pytest.mark.parametrize("origin_state,set_state", [ + ("open", "closed"), + ("closed", "open"), + ("closed", "closed"), + ("open", "open"), +]) +def test_ajax_openclose_close(mocker, origin_state, set_state): + class MockHtml(object): + def var(self, varname): + if varname == "name": + return "tactical_overview" + elif varname == "state": + return set_state + + m1 = mocker.patch.object(sidebar, "load_user_config", return_value={"snapins": [ + ("tactical_overview", origin_state), + ("views", "open"), + ]}) + html.set_current(MockHtml()) + m3 = mocker.patch.object(sidebar, "save_user_config") + + sidebar.ajax_openclose() + + m1.assert_called_once() + m3.assert_called_once_with({"snapins": [ + ("tactical_overview", set_state), + ("views", "open"), + ]}) + + +def test_move_snapin_page(): + assert cmk.gui.pages.get_page_handler("sidebar_move_snapin") == sidebar.move_snapin

6 years

Sidebar loading: Moved legacy transforms to dedicated helpers

by Lars Michelsen

Module: check_mk Branch: master Commit: 74bb44a176cd73edc2b8dc70ca9b9e5e15418ce4 URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=74bb44a176cd73… Author: Lars Michelsen <lm(a)mathias-kettner.de> Date: Thu Aug 23 11:02:09 2018 +0200 Sidebar loading: Moved legacy transforms to dedicated helpers CMK-855 Change-Id: Icd7a1ff9878599aed8fd878175803ac42be3e331 --- cmk/gui/sidebar.py | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/cmk/gui/sidebar.py b/cmk/gui/sidebar.py index 35a392a..4f26ea1 100644 --- a/cmk/gui/sidebar.py +++ b/cmk/gui/sidebar.py @@ -365,23 +365,35 @@ class UserSidebarConfig(object): (dictionary) on the fly""" user_config = self._user_config() - if type(user_config) == list: - user_config = { - "snapins" : user_config, - "fold": False, - } - - # Remove entries the user is not allowed for or which have state "off" (from legacy version) - # silently skip configured but not existant snapins + user_config = self._transform_legacy_list_config(user_config) + user_config = self._transform_legacy_off_state(user_config) + + # Remove entries the user is not allowed for and silently skip + # configured but not existing snapins user_config["snapins"] = [ entry for entry in user_config["snapins"] if entry[0] in snapin_registry - and entry[1] != "off" and self._user.may("sidesnap." + entry[0])] return user_config + def _transform_legacy_list_config(self, user_config): + if not isinstance(user_config, list): + return user_config + + return { + "snapins" : user_config, + "fold": False, + } + + + def _transform_legacy_off_state(self, user_config): + user_config = user_config.copy() + user_config["snapins"] = [ e for e in user_config["snapins"] if e[1] != "off" ] + return user_config + + def save(self): if self._user.may("general.configure_sidebar"): self._user.save_file("sidebar", self._config)

6 years

← Newer
1
...
27
28
29
30
31
32
33
...
38
Older →

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

Checkmk git commits September 2018