Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug #49274: Added check for spamassassin rules #11

Open
wants to merge 1 commit into
base: 4.4-0
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
univention-management-console-module-diagnostic (5.0.1-9) unstable; urgency=medium

* Bug #49274: Added check for spamassassin rules

-- Christian Castens <[email protected]> Wed, 10 Apr 2019 11:17:36 +0200

univention-management-console-module-diagnostic (5.0.1-8) unstable; urgency=medium

* Bug #46643: Use new option --mask-msad-differences for ntacl sysvolcheck
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
#!/usr/bin/python2.7
# coding: utf-8
#
# Univention Management Console module:
# System Diagnosis UMC module
#
# Copyright 2016-2019 Univention GmbH
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only 2019-2020

#
# http://www.univention.de/
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://

#
# All rights reserved.
#
# The source code of this program is made available
# under the terms of the GNU Affero General Public License version 3
# (GNU AGPL V3) as published by the Free Software Foundation.
#
# Binary versions of this program provided by Univention to you as
# well as other copyrighted, protected or trademarked materials like
# Logos, graphics, fonts, specific documentations and configurations,
# cryptographic keys etc. are subject to a license agreement between
# you and Univention and not subject to the GNU AGPL V3.
#
# In the case you use this program under the terms of the GNU AGPL V3,
# the program is provided in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU Affero General Public License for more details.
#
# You should have received a copy of the GNU Affero General Public
# License with the Debian GNU/Linux or Univention distribution in file
# /usr/share/common-licenses/AGPL-3; if not, see
# <http://www.gnu.org/licenses/>.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://


import os
import ldap
import socket
Comment on lines +35 to +36
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unneeded

import subprocess

import univention
import univention.uldap
import univention.lib.misc
import univention.admin.uldap
import univention.admin.modules as udm_modules
import univention.config_registry
from univention.config_registry import handler_set as ucr_set
from univention.config_registry import handler_unset as ucr_unset
Comment on lines +39 to +46
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably also unneeded.

from univention.management.console.modules.diagnostic import Critical, ProblemFixed, MODULE, Success

from univention.lib.i18n import Translation
_ = Translation('univention-management-console-module-diagnostic').translate
run_descr = ["Trying to authenticate with machine password against LDAP Similar to running: univention-ldapsearch -LLLs base dn"]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a copy-pasted-text from the check used as a template for this new check. This must be updated.

title = _('Check if spamassassin rules exist')
description = _('Spamassassin rules exist')
links = [{
}]

def update_and_restart(umc_instance):
subprocess.check_call(["sa-update"])
subprocess.check_call(["systemctl", "restart", "amavis.service"])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Use systemctl try-reload-or-restart … as the service might not exists or currently is not running
  2. spamassass.service should be added (to the same call as systemctl can restart multiple services with one call).

run(umc_instance, retest=True)

actions = {
'update_and_restart': update_and_restart,
}


def run(_umc_instance, retest=False):
buttons = []
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unused here as it is redefined later on.

try:
subprocess.check_call(["dpkg", "-s", "spamassassin"])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use dpkg-query -W spamassassin here please.

except subprocess.CalledProcessError:
raise Success('Spamassassin is not installed')

sa_version = subprocess.check_output(["spamassassin", "-V"])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multiple blanks here → pep8
Also use
perl -M'Mail::SpamAssassin' -e 'print($Mail::SpamAssassin::VERSION);'
to get the version in the right format, so you can remove the following 3 lines to parse and format the version number.

sa_version = sa_version.split()[2]
sa_version = sa_version.split('.')
folder_name = '%s.%03d%03d' % (int(sa_version[0]), int(sa_version[1]), int(sa_version[2]))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe build the complete path here once, e.g.
folder_path = "/var/lib/spamassassin/%s.%03d%03d" % (…)
and use it below both times,

if not os.path.exists("/var/lib/spamassassin/"+folder_name):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concatenating strings with + is considered bad style in Python. See comment above.

buttons = [{
'action': 'update_and_restart',
'name': 'update_and_restart',
'label': 'update spamassassin rules'
}]
raise Critical('spamassassin rules could not be found', buttons=buttons)
if not os.listdir('/var/lib/spamassassin/'+folder_name):
buttons = [{
'action': 'update_and_restart',
'name': 'update_and_restart',
'label': 'update spamassassin rules'
}]
raise Critical('spamassassin rules could not be found', buttons=buttons)
Comment on lines +85 to +91
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is the same block as above, combine them as
if not (exists(folder_path) and listdir(folder_path)):

if retest: raise ProblemFixed()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line-break please

raise Success()

if __name__ == '__main__':
from univention.management.console.modules.diagnostic import main
main()