[Pkg-freeipa-devel] [Git][freeipa-team/freeipa-healthcheck][upstream] 28 commits: man: Confusing typo about excludes

Timo Aaltonen (@tjaalton) gitlab at salsa.debian.org
Fri Aug 26 08:05:36 BST 2022



Timo Aaltonen pushed to branch upstream at FreeIPA packaging / freeipa-healthcheck


Commits:
97e91f10 by Stanislav Levin at 2022-02-15T12:55:00-05:00
man: Confusing typo about excludes

Fixes: https://github.com/freeipa/freeipa-healthcheck/issues/246
Signed-off-by: Stanislav Levin <slev at altlinux.org>

- - - - -
f03af6e6 by Rob Crittenden at 2022-02-17T11:54:09-05:00
Don't depend on IPA status when suppressing pki checks

The pki healthchecks are noisy if a CA is not configured. We
want to suppresse these in IPA so don't make the checks visible
if a CA is not configured.

So this means we need to be able to run in these conditions:

1. IPA is configured with a CA: the pki checks are run
2. IPA is configured without a CA: the pki checks are not run
3. IPA is not configured: the pki checks are run

Which basically equates to three states: True, False, None

This was done originally with the ca_configured variable set to
None. Using some inside knowledge the registries are loaded which
will set ca_configured to True or False in the IPA registry.
Using that we can determine if the pki checks should be available.
Unfortunately I changed the initialization to False so it always
assumes that IPA is installed. ca_configured will be False for the
case of IPA not installed instead of None so we can't handle that
last state.

So initialize ca_configured to None so we can satisfy all three
states.

https://github.com/freeipa/freeipa-healthcheck/issues/201

Signed-off-by: Rob Crittenden <rcritten at redhat.com>

- - - - -
5e8b7caa by Stanislav Levin at 2022-02-21T15:49:28-05:00
gha: Drop Fedora 33

- dropped Fedora 33 due to EOL (since Dec1 2021):
  https://docs.fedoraproject.org/en-US/releases/eol/

- added released Fedora 35
  https://fedorapeople.org/groups/schedule/f-35/f-35-key-tasks.html

- added Python3.10 for flake8/pep8 tasks

Fixes: https://github.com/freeipa/freeipa-healthcheck/issues/251
Signed-off-by: Stanislav Levin <slev at altlinux.org>

- - - - -
f1df5820 by Stanislav Levin at 2022-02-21T15:54:11-05:00
nss_checker: Make user/group names platform-dependent

- make use of ipaplatform for user/group names to avoid hardcoded values
- correct mocking in tests

There were several issues with these tests:
- mocking of builtins on global level is discouraged, mock on
  target module level instead
- wrong mocked group, `IPAGroupMemberCheck` checks if apache user
  has supplementary group `ipaapi`, while test mocked `apache` group
- wrong mocked `struct_group`, it should include user names having
  given supplementary group, not a primary one:
  ipaapi: (ipaapi, apache) => ipaapi: (apache,)

Fixes: https://github.com/freeipa/freeipa-healthcheck/issues/247
Signed-off-by: Stanislav Levin <slev at altlinux.org>

- - - - -
b4af69d2 by Stanislav Levin at 2022-02-21T15:54:11-05:00
files_checker: Make user/group names platform-dependent

- make use of ipaplatform for user/group names to avoid hardcoded values

Fixes: https://github.com/freeipa/freeipa-healthcheck/issues/247
Signed-off-by: Stanislav Levin <slev at altlinux.org>

- - - - -
a106b83b by Stanislav Levin at 2022-03-24T13:32:23-04:00
pylint: Skip redundant-u-string-prefix

Pylint 2.10 introduced new checker `redundant-u-string-prefix`:
> Used when we detect a string with a u prefix. These prefixes were
  necessary in Python 2 to indicate a string was Unicode, but since Python
  3.0 strings are Unicode by default.

There are ~40 emitted warnings right now. They can be fixed on
refactorings without any rush.

Fixes: https://github.com/freeipa/freeipa-healthcheck/issues/244
Signed-off-by: Stanislav Levin <slev at altlinux.org>

- - - - -
c3b5b3f3 by Stanislav Levin at 2022-03-24T13:32:23-04:00
pylint: Skip consider-using-f-string

- f-strings are not mandatory
- format can be more readable
- there are 140 spotted issues

Fixes: https://github.com/freeipa/freeipa-healthcheck/issues/244
Signed-off-by: Stanislav Levin <slev at altlinux.org>

- - - - -
2e0075b9 by Stanislav Levin at 2022-03-24T13:32:23-04:00
pylint: Skip use-dict-literal/use-list-literal

Pylint 2.10 introduced new checkers:
> Emitted when using dict() to create an empty dictionary instead of the
  literal {}. The literal is faster as it avoids an additional function
  call.

> Emitted when using list() to create an empty list instead of the
  literal []. The literal is faster as it avoids an additional function
  call.

Too many unessential changes.

Fixes: https://github.com/freeipa/freeipa-healthcheck/issues/244
Signed-off-by: Stanislav Levin <slev at altlinux.org>

- - - - -
938e40b5 by Stanislav Levin at 2022-03-24T13:32:23-04:00
pylint: Skip unspecified-encoding

Pylint 2.10 introduced new checker:
> It is better to specify an encoding when opening documents. Using the
  system default implicitly can create problems on other operating
  systems. See https://www.python.org/dev/peps/pep-0597/

According to that PEP:
> open(filename) isn't explicit about which encoding is expected:
  - If ASCII is assumed, this isn't a bug, but may result in decreased
    performance on Windows, particularly with non-Latin-1 locale
    encodings
  - If UTF-8 is assumed, this may be a bug or a platform-specific script
  - If the locale encoding is assumed, the behavior is as expected (but
    could change if future versions of Python modify the default)

IPA requires UTF-8 environments.

Fixes: https://github.com/freeipa/freeipa-healthcheck/issues/244
Signed-off-by: Stanislav Levin <slev at altlinux.org>

- - - - -
4684156d by Stanislav Levin at 2022-03-24T13:32:23-04:00
pylint: Enable useless-suppression

https://pylint.pycqa.org/en/latest/user_guide/message-control.html#detecting-useless-disables:

> As pylint gets better and false positives are removed, disables that
  became useless can accumulate and clutter the code. In order to clean
  them you can enable the useless-suppression warning.

This doesn't enforce useless-suppression warnings as errors. The idea is
cleanup of these warings on every Pylint's bump.

Fixes: https://github.com/freeipa/freeipa-healthcheck/issues/244
Signed-off-by: Stanislav Levin <slev at altlinux.org>

- - - - -
d58af596 by Stanislav Levin at 2022-03-24T13:32:23-04:00
pylint: Pin Pylint to compatible release

Either Fedora's or PyPI's release of Pylint can break
ipa-healthcheck's CI. With this change only maintainers of
ipa-healthcheck can decide which version of Pylint the project
is compatible with.

Fixes: https://github.com/freeipa/freeipa-healthcheck/issues/244
Signed-off-by: Stanislav Levin <slev at altlinux.org>

- - - - -
eaa8ffdb by Stanislav Levin at 2022-03-24T13:32:23-04:00
pylint: Skip use-implicit-booleaness-not-comparison

Pylint 2.12.0 introduced new checker:
> Used when Pylint detects that collection literal comparison is being
  used to check for emptiness; Use implicit booleaness insteadof a
  collection classes; empty collections are considered as false

Comparison of variable to equality to collection:
> Lexicographical comparison between built-in collections works as follows:
  For two collections to compare equal, they must be of the same type,
  have the same length, and each pair of corresponding elements must
  compare equal (for example, [1,2] == (1,2) is false because the type is
  not the same).
  Collections that support order comparison are ordered the same as their
  first unequal elements (for example, [1,2,x] <= [1,2,y] has the same
  value as x <= y). If a corresponding element does not exist, the shorter
  collection is ordered first (for example, [1,2] < [1,2,3] is true).

So, `assert value == {}` is not the same as `assert not value`.

Fixes: https://github.com/freeipa/freeipa-healthcheck/issues/244
Signed-off-by: Stanislav Levin <slev at altlinux.org>

- - - - -
02048641 by Stanislav Levin at 2022-03-24T13:32:23-04:00
pylint: Fix consider-using-dict-items

Pylint 2.9 introduced new check:
> New checker consider-using-dict-items. Emitted when iterating over
dictionary keys and then indexing the same dictionary with the key
within loop body.

Fixes: https://github.com/freeipa/freeipa-healthcheck/issues/244
Signed-off-by: Stanislav Levin <slev at altlinux.org>

- - - - -
075be732 by Stanislav Levin at 2022-03-24T13:32:23-04:00
pylint: Sync pylint plugin to FreeIPA

This is backport of
freeipa/freeipa at a1f0f2743d8accedd32385cf79d99c654313fd6f.

Fixes: https://github.com/freeipa/freeipa-healthcheck/issues/244
Signed-off-by: Stanislav Levin <slev at altlinux.org>

- - - - -
a720d10e by Rob Crittenden at 2022-03-29T12:50:40-04:00
Use the subject base from the IPA configuration, not REALM

The expected certificates were hardcoded with O={REALM} which
would return false-positives if the customer defined their
own certificate subject base.

Also add a search filter to only retrieve the certificate(s) we
want to examine rather than the entire contents.

Fixes: https://github.com/freeipa/freeipa-healthcheck/issues/253

Signed-off-by: Rob Crittenden <rcritten at redhat.com>

- - - - -
715bd0dd by Rob Crittenden at 2022-03-29T12:50:40-04:00
Remove debugging print statement in the tests

Notice this while ensuring I didn't have any leftovers during
development.

Related: https://github.com/freeipa/freeipa-healthcheck/issues/253

Signed-off-by: Rob Crittenden <rcritten at redhat.com>

- - - - -
621a32bb by Rob Crittenden at 2022-04-06T15:46:17-04:00
Allow multiple file modes in the FileChecker

There are some cases where a strict file mode is not
necessary. The kadmind.log is one.

It is owned root:root so there is no real difference
between 0600 and 0640. So allow both.

https://bugzilla.redhat.com/show_bug.cgi?id=2058239

Signed-off-by: Rob Crittenden <rcritten at redhat.com>

- - - - -
eb6cb8ec by Rob Crittenden at 2022-05-04T15:46:18-04:00
Validate that a known output-type has been selected

A user may pass an unknown value in via the configuration file.

https://bugzilla.redhat.com/show_bug.cgi?id=2079698

Signed-off-by: Rob Crittenden <rcritten at redhat.com>

- - - - -
58790073 by Rob Crittenden at 2022-05-04T15:46:18-04:00
Limit config file delimiters to =, catch empty values

ConfigParser allows both = and : as a delimiter. Limit to
just = to match the configuration file man page.

Don't allow empty values in options in the config file.

https://bugzilla.redhat.com/show_bug.cgi?id=2079739

Signed-off-by: Rob Crittenden <rcritten at redhat.com>

- - - - -
9967eb8a by Rob Crittenden at 2022-05-04T15:46:18-04:00
Relocate eval of debug/verbose in case they are set in config file

Since the configuration file allows options to be set we need
to evaluate them after the merge.

Leaving version handling pre-config load since it makes no sense
within the config file.

https://bugzilla.redhat.com/show_bug.cgi?id=2079861

Signed-off-by: Rob Crittenden <rcritten at redhat.com>

- - - - -
195adff6 by Rob Crittenden at 2022-05-04T15:46:18-04:00
Convert configuration option strings into native data types

When loading options from the configuration file they will all
be strings. This breaks existing boolean checks (if <something>)
and some assumptions about integer types (e.g. timeout, indent).

So try to detect the data type, defaulting to remain as a string.

Also hardcode some type validation for known keys to prevent
things like debug=foo (which would evaluate to True).

https://bugzilla.redhat.com/show_bug.cgi?id=2079861

Signed-off-by: Rob Crittenden <rcritten at redhat.com>

- - - - -
ecc0df42 by Rob Crittenden at 2022-05-04T15:46:43-04:00
Verify that the number of KDC workers matches the CPUs

Maximizing the number of workers improves scalability.

Particularly in VM environments the number can change over
time. The value is set at installation time but could change
later. Warn if they values don't match.

https://github.com/freeipa/freeipa-healthcheck/issues/258

Signed-off-by: Rob Crittenden <rcritten at redhat.com>

- - - - -
b42dc1ca by Michal Polovka at 2022-05-24T08:59:30-04:00
Tests: Debug option should be respected in cfg file

Test if the debug=True option is respected if provided
in the configuration file.

Related: https://bugzilla.redhat.com/show_bug.cgi?id=2079861

Signed-off-by: Michal Polovka <mpolovka at redhat.com>

- - - - -
7e8ef44a by Michal Polovka at 2022-05-24T08:59:30-04:00
Tests: User to be warned about incorrect output type

Test if the error message for the situation is descriptive and
user-friendly  when the output_type provided in the cfg file is incorrect.

Related: https://bugzilla.redhat.com/show_bug.cgi?id=2079698

Signed-off-by: Michal Polovka <mpolovka at redhat.com>

- - - - -
25fa4f35 by Michal Polovka at 2022-05-27T09:03:50-04:00
Tests: User to be warned about incorrect delimiter

Test if the error message is descriptive and informative when incorrect
delimiter is supplied in the configuration file.

Related: https://bugzilla.redhat.com/show_bug.cgi?id=2079739

Signed-off-by: Michal Polovka <mpolovka at redhat.com>

- - - - -
63599177 by Rob Crittenden at 2022-06-02T10:47:02-04:00
Read the IPA CA certificate to obtain the serial number

The dogtag connectivity test contains the command
ipa cert-show to verify that the CA basically works
and we are allowed to use it. It had a hardcoded 1 as
the CA certificate since pre-random serial numbers it was
a predictable value.

Instead read the CA cert and pluck the serial number from
it and use that value instead. /etc/ipa/ca.crt may contain
multiple certificates in the case of an external CA install
or additional certs added by ipa-cacert-manage so sift
through the list to find the expected IPA subject.

https://github.com/freeipa/freeipa-healthcheck/issues/260

Signed-off-by: Rob Crittenden <rcritten at redhat.com>

- - - - -
76133fe8 by Rob Crittenden at 2022-06-02T10:47:27-04:00
gha: Replace F34/35 with F35/36

- dropping Fedora 34 because it is due to EOL soon

Signed-off-by: Rob Crittenden <rcritten at redhat.com>

- - - - -
89d1736c by Rob Crittenden at 2022-06-02T11:11:31-04:00
Become 0.11

Signed-off-by: Rob Crittenden <rcritten at redhat.com>

- - - - -


26 changed files:

- .github/workflows/pipelines.yml
- man/man5/ipahealthcheck.conf.5
- pylint_plugins.py
- pylintrc
- setup.py
- src/ipahealthcheck/core/config.py
- src/ipahealthcheck/core/constants.py
- src/ipahealthcheck/core/core.py
- src/ipahealthcheck/core/files.py
- src/ipahealthcheck/core/output.py
- src/ipahealthcheck/dogtag/ca.py
- src/ipahealthcheck/ipa/certs.py
- src/ipahealthcheck/ipa/files.py
- src/ipahealthcheck/ipa/idns.py
- + src/ipahealthcheck/ipa/kdc.py
- src/ipahealthcheck/ipa/nss.py
- src/ipahealthcheck/system/filesystemspace.py
- tests/test_config.py
- tests/test_core_files.py
- tests/test_dogtag_connectivity.py
- tests/test_ipa_cert_match.py
- tests/test_ipa_dns.py
- + tests/test_ipa_kdc.py
- tests/test_ipa_nss.py
- tests/test_ipa_proxy.py
- tests/test_options.py


Changes:

=====================================
.github/workflows/pipelines.yml
=====================================
@@ -10,7 +10,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        python-version: [3.9]
+        python-version: ['3.9', '3.10']
 
     steps:
     - uses: actions/checkout at v2
@@ -35,7 +35,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        fedora-release: [33, 34]
+        fedora-release: [35, 36]
 
     steps:
     - uses: actions/checkout at v2
@@ -49,9 +49,10 @@ jobs:
                 freeipa-server \
                 freeipa-server-trust-ad \
                 tox \
-                python3-pylint \
+                python3-pip \
                 python3-pytest \
                 ; \
+            python3 -m pip install --user --ignore-installed 'pylint ~= 2.12.2' ; \
             cd /root/src; \
             tox -vv -elint; \
             "
@@ -65,7 +66,6 @@ jobs:
                 freeipa-server \
                 freeipa-server-trust-ad \
                 tox \
-                python3-pylint \
                 python3-pytest \
                 ; \
             cd /root/src; \


=====================================
man/man5/ipahealthcheck.conf.5
=====================================
@@ -32,7 +32,7 @@ Options must appear in the section named [default]. There are no other sections
 
 Options may be defined that are not used. Be careful of misspellings, they will not be rejected.
 .SH "EXCLUDES"
-There may be reasons that a user will want to suppress some results. One example is a customer certificate that is generating a warning because it is unknown to IPA. Excluding a result key does not prevent it from running, it is filtered from the reported results. Excluding by source or key will prevent it from running at all. Services will not be excluded because other checks may rely on them (ipahealthcheck.meta.services).
+There may be reasons that a user will want to suppress some results. One example is a customer certificate that is generating a warning because it is unknown to IPA. Excluding a result key does not prevent it from running, it is filtered from the reported results. Excluding by source or check will prevent it from running at all. Services will not be excluded because other checks may rely on them (ipahealthcheck.meta.services).
 
 Each excludes type may be listed multiple times. Invalid sources, checks and/or keys will not be flagged. These configuration options are only processed when found in the EXCLUDES section and are otherwise ignored.
 


=====================================
pylint_plugins.py
=====================================
@@ -406,6 +406,7 @@ AstroidBuilder(MANAGER).string_build(textwrap.dedent(
     api.env.container_automount = DN()
     api.env.container_ca = DN()
     api.env.container_ca_renewal = DN()
+    api.env.container_subids = DN()
     api.env.container_caacl = DN()
     api.env.container_certmap = DN()
     api.env.container_certmaprules = DN()
@@ -470,10 +471,6 @@ AstroidBuilder(MANAGER).string_build(textwrap.dedent(
     api.env.interactive = True
     api.env.ipalib = ''  # object
     api.env.kinit_lifetime = None
-    api.env.lite_pem = ''
-    api.env.lite_profiler = ''
-    api.env.lite_host = ''
-    api.env.lite_port = 0
     api.env.log = ''  # object
     api.env.logdir = ''  # object
     api.env.mode = ''
@@ -500,6 +497,32 @@ AstroidBuilder(MANAGER).string_build(textwrap.dedent(
     api.env.version = ''
     api.env.wait_for_dns = 0
     api.env.webui_prod = True
+
+    # defined in ipaclient/install/ipa_epn.py
+    api.env.smtp_server = ""
+    api.env.smtp_port = 0
+    api.env.smtp_user = None
+    api.env.smtp_password = None
+    api.env.smtp_client_cert = None
+    api.env.smtp_client_key = None
+    api.env.smtp_client_key_pass = None
+    api.env.smtp_timeout = 0
+    api.env.smtp_security = ""
+    api.env.smtp_admin = ""
+    api.env.smtp_delay = None
+    api.env.mail_from = None
+    api.env.notify_ttls = ""
+    api.env.msg_charset = ""
+    api.env.msg_subtype = ""
+    api.env.msg_subject = ""
+
+    # defined in contrib/lite-server.py
+    api.env.lite_pem = ''
+    api.env.lite_profiler = ''
+    api.env.lite_host = ''
+    api.env.lite_port = 0
+    api.env.lite_tracemalloc = False
+
     """
 ))
 


=====================================
pylintrc
=====================================
@@ -28,10 +28,18 @@ valid-metaclass-classmethod-first-arg=cls
 
 enable=
     all,
-    python3
+    python3,
+    useless-suppression,
 
 disable=
-    I,
+    bad-inline-option,
+    c-extension-no-member,
+    deprecated-pragma,
+    file-ignored,
+    locally-disabled,
+    raw-checker-failed,
+    suppressed-message,
+    use-symbolic-message-instead,
     duplicate-code,
     interface-not-implemented,
     no-self-use,
@@ -54,7 +62,6 @@ disable=
     bad-builtin,
     bad-indentation,
     broad-except,
-    consider-using-dict-items,
     dangerous-default-value,
     eval-used,
     exec-used,
@@ -109,6 +116,12 @@ disable=
     consider-using-with,  # pylint 2.8.0, contextmanager is not mandatory
     consider-using-max-builtin,  # pylint 2.8.0, can be more readable
     consider-using-min-builtin,  # pylint 2.8.0, can be more readable
+    redundant-u-string-prefix,  # pylint 2.10.0, too many unessential changes
+    consider-using-f-string,  # pylint 2.11.0, format can be more readable
+    use-dict-literal,  # pylint 2.10.0 dict vs {}
+    use-list-literal,  # pylint 2.10.0 list() vs []
+    unspecified-encoding,  # pylint 2.10.0, ASCII or UTF8 and platform-specific
+    use-implicit-booleaness-not-comparison,  # pylint 2.12.2, weak comparison
 
 [REPORTS]
 


=====================================
setup.py
=====================================
@@ -3,7 +3,7 @@ from setuptools import find_packages, setup
 
 setup(
     name='ipahealthcheck',
-    version='0.10',
+    version='0.11',
     namespace_packages=['ipahealthcheck', 'ipaclustercheck'],
     package_dir={'': 'src'},
     # packages=find_packages(where='src'),
@@ -43,6 +43,7 @@ setup(
             'ipadns = ipahealthcheck.ipa.idns',
             'ipafiles = ipahealthcheck.ipa.files',
             'ipahost = ipahealthcheck.ipa.host',
+            'ipakdc = ipahealthcheck.ipa.kdc',
             'ipaproxy = ipahealthcheck.ipa.proxy',
             'ipameta = ipahealthcheck.ipa.meta',
             'ipanss = ipahealthcheck.ipa.nss',


=====================================
src/ipahealthcheck/core/config.py
=====================================
@@ -93,6 +93,27 @@ class Config:
             self.__d[key] = d[key]
 
 
+def convert_string(value):
+    """
+    Reading options from the configuration file will leave them as
+    strings. This breaks boolean values so attempt to convert them.
+    """
+    if not isinstance(value, str):
+        return value
+
+    if value.lower() in (
+        "true",
+        "false",
+    ):
+        return value.lower() == 'true'
+    else:
+        try:
+            value = int(value)
+        except ValueError:
+            pass
+    return value
+
+
 def read_config(config_file):
     """
     Simple configuration file reader
@@ -109,7 +130,8 @@ def read_config(config_file):
         )
         return config
 
-    parser = ConfigParser(dict_type=DuplicateOrderedDict, strict=False)
+    parser = ConfigParser(dict_type=DuplicateOrderedDict, strict=False,
+                          delimiters='=')
     try:
         parser.read(config_file)
     except ParsingError as e:
@@ -125,7 +147,31 @@ def read_config(config_file):
 
     for (key, value) in items:
         if not key.startswith('excludes_'):
-            config[key] = value
+            if len(value) == 0 or value is None:
+                logging.error(
+                    "Empty value for %s in %s [%s]",
+                    key, config_file, CONFIG_SECTION
+                )
+                return None
+            else:
+                # Try to do some basic validation. This is unfortunately
+                # hardcoded.
+                if key in ('all', 'debug', 'failures_only', 'verbose'):
+                    if value.lower() not in ('true', 'false'):
+                        logging.error(
+                            "%s is not a valid boolean in %s [%s]",
+                            key, config_file, CONFIG_SECTION
+                        )
+                        return None
+                elif key in ('indent', 'timeout'):
+                    if not isinstance(convert_string(value), int):
+                        logging.error(
+                            "%s is not a valid integer in %s [%s]",
+                            key, config_file, CONFIG_SECTION
+                        )
+                        return None
+                # Some rough type translation from strings
+                config[key] = convert_string(value)
 
     if parser.has_section(EXCLUDE_SECTION):
         items = parser.items(EXCLUDE_SECTION)


=====================================
src/ipahealthcheck/core/constants.py
=====================================
@@ -2,8 +2,6 @@
 # Copyright (C) 2019 FreeIPA Contributors see COPYING for license
 #
 
-DEFAULT_OUTPUT = 'json'
-
 # Error reporting result
 SUCCESS = 0
 WARNING = 10


=====================================
src/ipahealthcheck/core/core.py
=====================================
@@ -320,7 +320,7 @@ class RunChecks:
     def run_healthcheck(self):
         framework = object()
         plugins = []
-        output = constants.DEFAULT_OUTPUT
+        output = None
 
         logger.setLevel(logging.WARNING)
 
@@ -346,12 +346,6 @@ class RunChecks:
         if rval is not None:
             return rval
 
-        if options.verbose:
-            logger.setLevel(logging.INFO)
-
-        if options.debug:
-            logger.setLevel(logging.DEBUG)
-
         if options.config is not None:
             config = read_config(options.config)
         else:
@@ -366,19 +360,35 @@ class RunChecks:
         self.options = config
         options = config
 
+        if options.verbose:
+            logger.setLevel(logging.INFO)
+
+        if options.debug:
+            logger.setLevel(logging.DEBUG)
+
         # pylint: disable=assignment-from-none
         rval = self.pre_check()
         # pylint: enable=assignment-from-none
         if rval is not None:
             return rval
 
+        # The pki checks are noisy if a CA is not configured so we
+        # want to suppress that for IPA.
+        #
+        # There are 3 possible states:
+        # 1. IPA is configured with a CA
+        # 2. IPA is configured without a CA
+        # 3. IPA is not configured
+        #
         # If we have IPA configured without a CA then we want to skip
-        # the pkihealthcheck plugins otherwise they will generated a
-        # lot of false positives. The IPA plugins are loaded first so
-        # which should set ca_configured in its registry to True or
-        # False. We will skip the pkihealthcheck plugins only if
-        # ca_configured is False which means that it was set by IPA.
-        ca_configured = False
+        # the pkihealthcheck plugins
+        #
+        # The IPA registry will set ca_configured in its registry to True
+        # or False. We will skip the pkihealthcheck plugins only if
+        # ca_configured is False which means that it was set by IPA. So
+        # we initialize ca_configured to None so that the pki checks
+        # will always be executed with pki-healthcheck.
+        ca_configured = None
         for name, registry in find_registries(self.entry_points).items():
             try:
                 registry.initialize(framework, config, options)
@@ -403,6 +413,9 @@ class RunChecks:
             if out.__name__.lower() == options.output_type:
                 output = out(options)
                 break
+        if output is None:
+            print(f"Unknown output-type '{options.output_type}'")
+            return 1
 
         if options.list_sources:
             return list_sources(plugins)
@@ -426,7 +439,7 @@ class RunChecks:
                                                      options.check)
             results.extend(run_plugins(plugins, available,
                                        options.source, options.check, config,
-                                       int(config.timeout)))
+                                       config.timeout))
 
         if options.source and len(results.results) == 0:
             for plugin in plugins:


=====================================
src/ipahealthcheck/core/files.py
=====================================
@@ -16,7 +16,7 @@ class FileCheck:
        files is a tuple of tuples. Each tuple consists of:
            (path, expected_perm, expected_owner, expected_group)
 
-       perm is in the form of a POSIX ACL: e.g. 0440, 0770.
+       perm is a POSIX ACL as either a string or tuple: e.g. 0440, (0770,).
        owner and group are names or a tuple of names, not uid/gid.
 
        If owner and/or group are tuples then all names are checked.
@@ -33,6 +33,8 @@ class FileCheck:
                 owner = tuple((owner,))
             if not isinstance(group, tuple):
                 group = tuple((group,))
+            if not isinstance(mode, tuple):
+                mode = tuple((mode,))
             if not os.path.exists(path):
                 for type in ('mode', 'owner', 'group'):
                     key = '%s_%s' % (path.replace('/', '_'), type)
@@ -43,19 +45,32 @@ class FileCheck:
             stat = os.stat(path)
             fmode = str(oct(stat.st_mode)[-4:])
             key = '%s_mode' % path.replace('/', '_')
-            if mode != fmode:
-                if mode < fmode:
+            if fmode not in mode:
+                if len(mode) == 1:
+                    modes = mode[0]
+                else:
+                    modes = 'one of {}'.format(','.join(mode))
+                if all(m < fmode for m in mode):
                     yield Result(self, constants.WARNING, key=key,
-                                 path=path, type='mode', expected=mode,
+                                 path=path, type='mode', expected=modes,
                                  got=fmode,
                                  msg='Permissions of %s are too permissive: '
-                                 '%s and should be %s' % (path, fmode, mode))
-                if mode > fmode:
+                                 '%s and should be %s' %
+                                 (path, fmode, modes))
+                elif all(m > fmode for m in mode):
                     yield Result(self, constants.ERROR, key=key,
-                                 path=path, type='mode', expected=mode,
+                                 path=path, type='mode', expected=modes,
                                  got=fmode,
                                  msg='Permissions of %s are too restrictive: '
-                                 '%s and should be %s' % (path, fmode, mode))
+                                 '%s and should be %s' %
+                                 (path, fmode, modes))
+                else:
+                    yield Result(self, constants.ERROR, key=key,
+                                 path=path, type='mode', expected=modes,
+                                 got=fmode,
+                                 msg='Permissions of %s are unexpected: '
+                                 '%s and should be %s' %
+                                 (path, fmode, modes))
             else:
                 yield Result(self, constants.SUCCESS, key=key,
                              type='mode', path=path)


=====================================
src/ipahealthcheck/core/output.py
=====================================
@@ -111,7 +111,7 @@ class JSON(Output):
 
     def __init__(self, options):
         super().__init__(options)
-        self.indent = int(options.indent)
+        self.indent = options.indent
 
     def generate(self, data):
         output = json.dumps(data, indent=self.indent)


=====================================
src/ipahealthcheck/dogtag/ca.py
=====================================
@@ -9,11 +9,13 @@ from ipahealthcheck.core.plugin import Result
 from ipahealthcheck.core.plugin import duration
 from ipahealthcheck.core import constants
 
-from ipalib import api, errors
+from ipalib import api, errors, x509
 from ipaplatform.paths import paths
 from ipaserver.install import certs
+from ipaserver.install import ca
 from ipaserver.install import krainstance
 from ipapython.directivesetter import get_directive
+from ipapython.dn import DN
 from cryptography.hazmat.primitives.serialization import Encoding
 
 logger = logging.getLogger()
@@ -102,22 +104,59 @@ class DogtagCertsConnectivityCheck(DogtagPlugin):
             logger.debug('CA is not configured, skipping connectivity check')
             return
 
+        config = api.Command.config_show()
+
+        subject_base = config['result']['ipacertificatesubjectbase'][0]
+        ipa_subject = ca.lookup_ca_subject(api, subject_base)
+        try:
+            certs = x509.load_certificate_list_from_file(paths.IPA_CA_CRT)
+        except Exception as e:
+            yield Result(self, constants.ERROR,
+                         key='ipa_ca_crt_file_missing',
+                         path=paths.IPA_CA_CRT,
+                         error=str(e),
+                         msg='The IPA CA cert file {path} could not be '
+                             'opened: {error}')
+            return
+
+        found = False
+        for cert in certs:
+            if DN(cert.subject) == ipa_subject:
+                found = True
+                break
+
+        if not found:
+            yield Result(self, constants.ERROR,
+                         key='ipa_ca_cert_not_found',
+                         subject=str(ipa_subject),
+                         path=paths.IPA_CA_CRT,
+                         msg='The CA certificate with subject {subject} '
+                             'was not found in {path}')
+            return
+        # Load the IPA CA certificate to obtain its serial number. This
+        # was traditionally 1 prior to random serial number support.
         # There is nothing special about cert 1. Even if there is no cert
         # serial number 1 but the connection is ok it is considered passing.
         try:
-            api.Command.cert_show(1, all=True)
+            api.Command.cert_show(cert.serial_number, all=True)
         except errors.CertificateOperationError as e:
-            if 'not found' not in str(e):
+            if 'not found' in str(e):
                 yield Result(self, constants.ERROR,
                              key='cert_show_1',
-                             msg='Request for certificate failed, %s' %
-                                 e)
+                             error=str(e),
+                             serial=str(cert.serial_number),
+                             msg='Serial number not found: {error}')
             else:
-                yield Result(self, constants.SUCCESS)
+                yield Result(self, constants.ERROR,
+                             key='cert_show_1',
+                             error=str(e),
+                             serial=str(cert.serial_number),
+                             msg='Request for certificate failed: {error}')
         except Exception as e:
             yield Result(self, constants.ERROR,
                          key='cert_show_1',
-                         msg='Request for certificate failed, %s' %
-                             e)
+                             error=str(e),
+                         serial=str(cert.serial_number),
+                         msg='Request for certificate failed: {error')
         else:
             yield Result(self, constants.SUCCESS)


=====================================
src/ipahealthcheck/ipa/certs.py
=====================================
@@ -739,13 +739,15 @@ class IPADogtagCertsMatchCheck(IPAPlugin):
 
         def match_ldap_nss_certs_by_subject(plugin, ldap, db, dn,
                                             expected_nicks_subjects):
-            entries = ldap.get_entries(dn)
             all_ok = True
             for nick, subject in expected_nicks_subjects.items():
+                entries = ldap.get_entries(
+                    dn,
+                    filter=f'subjectname={subject}'
+                )
                 cert = db.get_cert_from_db(nick)
                 ok = any(
                     cert in entry["userCertificate"]
-                    and subject == entry["subjectName"][0]
                     for entry in entries
                     if "userCertificate" in entry
                 )
@@ -774,26 +776,28 @@ class IPADogtagCertsMatchCheck(IPAPlugin):
                                                       db, dn, 'CACertificate',
                                                       casigning_nick)
 
+        config = api.Command.config_show()
+        subject_base = config['result']['ipacertificatesubjectbase'][0]
         expected_nicks_subjects = {
             'ocspSigningCert cert-pki-ca':
-                'CN=OCSP Subsystem,O=%s' % api.env.realm,
+                f'CN=OCSP Subsystem,{subject_base}',
             'subsystemCert cert-pki-ca':
-                'CN=CA Subsystem,O=%s' % api.env.realm,
+                f'CN=CA Subsystem,{subject_base}',
             'auditSigningCert cert-pki-ca':
-                'CN=CA Audit,O=%s' % api.env.realm,
+                f'CN=CA Audit,{subject_base}',
             'Server-Cert cert-pki-ca':
-                'CN=%s,O=%s' % (api.env.host, api.env.realm),
+                f'CN={api.env.host},{subject_base}',
         }
 
         kra = krainstance.KRAInstance(api.env.realm)
         if kra.is_installed():
             kra_expected_nicks_subjects = {
                 'transportCert cert-pki-kra':
-                    'CN=KRA Transport Certificate,O=%s' % api.env.realm,
+                    f'CN=KRA Transport Certificate,{subject_base}',
                 'storageCert cert-pki-kra':
-                    'CN=KRA Storage Certificate,O=%s' % api.env.realm,
+                    f'CN=KRA Storage Certificate,{subject_base}',
                 'auditSigningCert cert-pki-kra':
-                    'CN=KRA Audit,O=%s' % api.env.realm,
+                    f'CN=KRA Audit,{subject_base}',
             }
             expected_nicks_subjects.update(kra_expected_nicks_subjects)
 


=====================================
src/ipahealthcheck/ipa/files.py
=====================================
@@ -35,20 +35,20 @@ class IPAFileNSSDBCheck(IPAPlugin, FileCheck):
         self.files = []
 
         self.collect_files(dsinstance.config_dirname(self.serverid),
-                           NSS_SQL_FILES, 'dirsrv', 'root', '0640')
+                           NSS_SQL_FILES, constants.DS_USER, 'root', '0640')
 
         # There always has to be a special one. pkcs11.txt has a different
         # group so pop off the auto-generated one and add a replacement.
         old = (os.path.join(dsinstance.config_dirname(self.serverid),
-               'pkcs11.txt'), 'dirsrv', 'root', '0640')
+               'pkcs11.txt'), constants.DS_USER, 'root', '0640')
         self.files.remove(old)
         new = (os.path.join(dsinstance.config_dirname(self.serverid),
-               'pkcs11.txt'), 'dirsrv', 'dirsrv', '0640')
+               'pkcs11.txt'), constants.DS_USER, constants.DS_GROUP, '0640')
         self.files.append(new)
 
         if self.ca.is_configured():
             self.collect_files(paths.PKI_TOMCAT_ALIAS_DIR, NSS_SQL_FILES,
-                               'pkiuser', 'pkiuser', '0600')
+                               constants.PKI_USER, constants.PKI_GROUP, '0600')
 
         return FileCheck.check(self)
 
@@ -71,8 +71,12 @@ class IPAFileCheck(IPAPlugin, FileCheck):
         self.files = []
 
         if self.ca.is_configured():
-            self.files.append((paths.RA_AGENT_PEM, 'root', 'ipaapi', '0440'))
-            self.files.append((paths.RA_AGENT_KEY, 'root', 'ipaapi', '0440'))
+            self.files.append(
+                (paths.RA_AGENT_PEM, 'root', constants.IPAAPI_GROUP, '0440')
+            )
+            self.files.append(
+                (paths.RA_AGENT_KEY, 'root', constants.IPAAPI_GROUP, '0440')
+            )
 
         if krbinstance.is_pkinit_enabled():
             self.files.append((paths.KDC_CERT, 'root', 'root', '0644'))
@@ -119,43 +123,52 @@ class IPAFileCheck(IPAPlugin, FileCheck):
         self.files.append((paths.IPA_CUSTODIA_AUDIT_LOG,
                           'root', 'root', '0644'))
 
-        self.files.append((paths.KADMIND_LOG, 'root', 'root', '0600'))
+        self.files.append((paths.KADMIND_LOG, 'root', 'root',
+                          ('0600', '0640')))
         self.files.append((paths.KRB5KDC_LOG, 'root', 'root', '0640'))
 
         inst = api.env.realm.replace('.', '-')
         self.files.append((paths.SLAPD_INSTANCE_ACCESS_LOG_TEMPLATE % inst,
-                           'dirsrv', 'dirsrv', '0600'))
+                           constants.DS_USER, constants.DS_GROUP, '0600'))
         self.files.append((paths.SLAPD_INSTANCE_ERROR_LOG_TEMPLATE % inst,
-                           'dirsrv', 'dirsrv', '0600'))
+                           constants.DS_USER, constants.DS_GROUP, '0600'))
 
         self.files.append((paths.VAR_LOG_HTTPD_ERROR, 'root', 'root', '0644'))
 
         for globpath in glob.glob("%s/debug*.log" % paths.TOMCAT_CA_DIR):
-            self.files.append((globpath, "pkiuser", "pkiuser", "0644"))
+            self.files.append(
+                (globpath, constants.PKI_USER, constants.PKI_GROUP, "0644")
+            )
 
         for globpath in glob.glob(
             "%s/ca_audit*" % paths.TOMCAT_SIGNEDAUDIT_DIR
         ):
-            self.files.append((globpath, 'pkiuser', 'pkiuser', '0640'))
+            self.files.append(
+                (globpath, constants.PKI_USER, constants.PKI_GROUP, '0640')
+            )
 
         for filename in ('selftests.log', 'system', 'transactions'):
             self.files.append((
                 os.path.join(paths.TOMCAT_CA_DIR, filename),
-                'pkiuser', 'pkiuser', '0640'
+                constants.PKI_USER, constants.PKI_GROUP, '0640'
             ))
 
         for globpath in glob.glob("%s/debug*.log" % paths.TOMCAT_KRA_DIR):
-            self.files.append((globpath, "pkiuser", "pkiuser", "0644"))
+            self.files.append(
+                (globpath, constants.PKI_USER, constants.PKI_GROUP, "0644")
+            )
 
         for globpath in glob.glob(
             "%s/ca_audit*" % paths.TOMCAT_KRA_SIGNEDAUDIT_DIR
         ):
-            self.files.append((globpath, 'pkiuser', 'pkiuser', '0640'))
+            self.files.append(
+                (globpath, constants.PKI_USER, constants.PKI_GROUP, '0640')
+            )
 
         for filename in ('selftests.log', 'system', 'transactions'):
             self.files.append((
                 os.path.join(paths.TOMCAT_KRA_DIR, filename),
-                'pkiuser', 'pkiuser', '0640'
+                constants.PKI_USER, constants.PKI_GROUP, '0640'
             ))
 
         return FileCheck.check(self)


=====================================
src/ipahealthcheck/ipa/idns.py
=====================================
@@ -98,14 +98,13 @@ class IPADNSSystemRecordsCheck(IPAPlugin):
         # For each SRV record that IPA thinks it should have, do a DNS
         # lookup of it and ensure that DNS has the same set of values
         # that IPA thinks it should.
-        for srv in srv_rec:
+        for srv, hosts in srv_rec.items():
             logger.debug("Search DNS for SRV record of %s", srv)
             try:
                 answers = query_srv(srv)
             except DNSException as e:
                 logger.debug("DNS record not found: %s", e.__class__.__name__)
                 answers = []
-            hosts = srv_rec[srv]
             for answer in answers:
                 logger.debug("DNS record found: %s", answer)
                 try:
@@ -124,10 +123,9 @@ class IPADNSSystemRecordsCheck(IPAPlugin):
                     msg='Expected SRV record missing',
                     key=self.srv_to_name(srv, host))
 
-        for uri in uri_rec:
+        for uri, hosts in uri_rec.items():
             logger.debug("Search DNS for URI record of %s", uri)
             answers = query_uri(uri)
-            hosts = uri_rec[uri]
             for answer in answers:
                 logger.debug("DNS record found: %s", answer)
                 try:
@@ -153,7 +151,7 @@ class IPADNSSystemRecordsCheck(IPAPlugin):
                     key=self.uri_to_name(uri, host)
                 )
 
-        for txt in txt_rec:
+        for txt, realms in txt_rec.items():
             logger.debug("Search DNS for TXT record of %s", txt)
             try:
                 answers = resolve(txt, rdatatype.TXT)
@@ -161,7 +159,6 @@ class IPADNSSystemRecordsCheck(IPAPlugin):
                 logger.debug("DNS record not found: %s", e.__class__.__name__)
                 answers = []
 
-            realms = txt_rec[txt]
             for answer in answers:
                 logger.debug("DNS record found: %s", answer)
                 realm = answer.to_text()


=====================================
src/ipahealthcheck/ipa/kdc.py
=====================================
@@ -0,0 +1,72 @@
+
+# Copyright (C) 2022 FreeIPA Contributors see COPYING for license
+#
+
+import logging
+import os
+
+from ipahealthcheck.ipa.plugin import IPAPlugin, registry
+from ipahealthcheck.core.plugin import Result, duration
+from ipahealthcheck.core import constants
+
+
+logger = logging.getLogger()
+SYSCONFIG = '/etc/sysconfig/krb5kdc'
+
+
+def get_contents(file):
+    with open(SYSCONFIG, "r") as fd:
+        lines = fd.readlines()
+    return lines
+
+
+ at registry
+class KDCWorkersCheck(IPAPlugin):
+    """Verify that the number of workers matches the number of cores"""
+
+    @duration
+    def check(self):
+        key = 'workers'
+        cpus = os.sysconf('SC_NPROCESSORS_ONLN')
+        logging.debug('Detected %s CPUs', cpus)
+
+        lines = get_contents(SYSCONFIG)
+
+        args_read = False
+        for line in lines:
+            sline = line.strip()
+            workers = 0
+            if sline.startswith('KRB5KDC_ARGS'):
+                args_read = True
+                sline = sline.split('=', maxsplit=1)[1]
+                if sline.find("-w") == -1:
+                    yield Result(self, constants.WARNING, key=key,
+                                 sysconfig=SYSCONFIG,
+                                 msg='No KDC workers defined in '
+                                 '{sysconfig}')
+                    return
+
+                # Making an assumption that this line is not misconfigured
+                # otherwise the KDC wouldn't start at all.
+                sline = sline.replace("'", "")
+                sline = sline.replace('"', "")
+                sline = sline.split()
+                for i in range(len(sline)):
+                    if sline[i] == '-w':
+                        workers = int(sline[i+1])
+                        break
+                if cpus == workers:
+                    yield Result(self, constants.SUCCESS, key=key)
+                else:
+                    yield Result(self, constants.WARNING, key=key,
+                                 cpus=cpus, workers=workers,
+                                 sysconfig=SYSCONFIG,
+                                 msg='The number of CPUs {cpus} does not '
+                                     'match the number of workers '
+                                     '{workers} in {sysconfig}')
+                break
+        if not args_read:
+            yield Result(self, constants.WARNING, key=key,
+                         sysconfig=SYSCONFIG,
+                         msg='KRB5KDC_ARGS is not set in '
+                         '{sysconfig}')


=====================================
src/ipahealthcheck/ipa/nss.py
=====================================
@@ -5,6 +5,8 @@
 import grp
 import logging
 
+from ipaplatform.constants import constants as platform_constants
+
 from ipahealthcheck.ipa.plugin import IPAPlugin, registry
 from ipahealthcheck.core.plugin import Result
 from ipahealthcheck.core.plugin import duration
@@ -22,7 +24,7 @@ logger = logging.getLogger()
 #
 # (group_name, (members,))
 GROUP_MEMBERS = (
-    ('ipaapi', ('apache',)),
+    (platform_constants.IPAAPI_GROUP, (platform_constants.HTTPD_USER,)),
 )
 
 


=====================================
src/ipahealthcheck/system/filesystemspace.py
=====================================
@@ -61,7 +61,7 @@ class FileSystemSpaceCheck(SystemPlugin):
 
     @duration
     def check(self):
-        for store in self._pathchecks:
+        for store, threshold in self._pathchecks.items():
             try:
                 percent_free = self.get_fs_free_space_percentage(store)
             except FileNotFoundError:
@@ -95,7 +95,6 @@ class FileSystemSpaceCheck(SystemPlugin):
                     threshold=self.min_free_percent
                 )
             free_space = self.get_fs_free_space(store)
-            threshold = self._pathchecks[store]
             if free_space < threshold:
                 yield Result(
                     self, constants.ERROR,


=====================================
tests/test_config.py
=====================================
@@ -6,7 +6,7 @@ import tempfile
 
 import pytest
 
-from ipahealthcheck.core.config import read_config
+from ipahealthcheck.core.config import read_config, convert_string
 
 
 def test_config_no_section():
@@ -59,3 +59,17 @@ def test_config_recursion():
         config._Config__d['_Config__d']
     except KeyError:
         pass
+
+
+def test_convert_string():
+    for value in ("s", "string", "BiggerString"):
+        assert convert_string(value) == value
+
+    for value in ("True", "true", True):
+        assert convert_string(value) is True
+
+    for value in ("False", "false", False):
+        assert convert_string(value) is False
+
+    for value in ("10", "99999", 807):
+        assert convert_string(value) == int(value)


=====================================
tests/test_core_files.py
=====================================
@@ -17,7 +17,8 @@ nobody = pwd.getpwnam('nobody')
 files = (('foo', 'root', 'root', '0660'),
          ('bar', 'nobody', 'nobody', '0664'),
          ('baz', ('root', 'nobody'), ('root', 'nobody'), '0664'),
-         ('fiz', ('root', 'bin'), ('root', 'bin'), '0664'),)
+         ('fiz', ('root', 'bin'), ('root', 'bin'), '0664'),
+         ('zap', ('root', 'bin'), ('root', 'bin'), ('0664', '0640'),))
 
 
 def make_stat(mode=33200, uid=0, gid=0):
@@ -27,6 +28,13 @@ def make_stat(mode=33200, uid=0, gid=0):
             mode = 0660
             owner = root
             group = root
+
+       Cheat sheet equivalents:
+           0600 = 33152
+           0640 = 33184
+           0644 = 33188
+           0660 = 33200
+           0666 = 33206
     """
     # (mode, ino, dev, nlink, uid, gid, size, atime, mtime, ctime)
     return posix.stat_result((mode, 1, 42, 1, uid, gid, 0, 1, 1, 1,))
@@ -81,6 +89,11 @@ def test_files_owner(mock_stat):
     assert my_results.results[3].kw.get('msg') == \
         'Ownership of fiz is nobody and should be one of root,bin'
 
+    assert my_results.results[4].result == constants.WARNING
+    assert my_results.results[4].kw.get('got') == 'nobody'
+    assert my_results.results[4].kw.get('expected') == 'root,bin'
+    assert my_results.results[4].kw.get('type') == 'owner'
+
 
 @patch('os.stat')
 def test_files_group(mock_stat):
@@ -119,6 +132,11 @@ def test_files_group(mock_stat):
     assert my_results.results[3].kw.get('msg') == \
         'Group of fiz is nobody and should be one of root,bin'
 
+    assert my_results.results[4].result == constants.WARNING
+    assert my_results.results[4].kw.get('got') == 'nobody'
+    assert my_results.results[4].kw.get('expected') == 'root,bin'
+    assert my_results.results[4].kw.get('type') == 'group'
+
 
 @patch('os.stat')
 def test_files_mode(mock_stat):
@@ -133,17 +151,35 @@ def test_files_mode(mock_stat):
     assert my_results.results[0].result == constants.SUCCESS
     assert my_results.results[1].result == constants.ERROR
 
-    mock_stat.return_value = make_stat(mode=33152)
+    # Too restrictive
+    mock_stat.return_value = make_stat(mode=33152)  # 0600
     results = capture_results(f)
     my_results = get_results(results, 'mode')
     assert my_results.results[0].result == constants.ERROR
     assert my_results.results[1].result == constants.ERROR
+    assert my_results.results[2].result == constants.ERROR
+    assert my_results.results[3].result == constants.ERROR
+    assert my_results.results[4].result == constants.ERROR
 
-    mock_stat.return_value = make_stat(mode=33206)
+    # Too permissive
+    mock_stat.return_value = make_stat(mode=33206)  # 0666
     results = capture_results(f)
     my_results = get_results(results, 'mode')
     assert my_results.results[0].result == constants.WARNING
     assert my_results.results[1].result == constants.WARNING
+    assert my_results.results[2].result == constants.WARNING
+    assert my_results.results[3].result == constants.WARNING
+    assert my_results.results[4].result == constants.WARNING
+
+    # Too restrictive with allowed multi-mode value
+    mock_stat.return_value = make_stat(mode=33184)  # 0640
+    results = capture_results(f)
+    my_results = get_results(results, 'mode')
+    assert my_results.results[0].result == constants.ERROR
+    assert my_results.results[1].result == constants.ERROR
+    assert my_results.results[2].result == constants.ERROR
+    assert my_results.results[3].result == constants.ERROR
+    assert my_results.results[4].result == constants.SUCCESS
 
 
 @patch('os.path.exists')
@@ -157,7 +193,7 @@ def test_files_not_found(mock_exists):
 
     for type in ('mode', 'group', 'owner'):
         my_results = get_results(results, type)
-        assert len(my_results.results) == 4
+        assert len(my_results.results) == len(f.files)
         for result in my_results.results:
             assert result.result == constants.SUCCESS
             assert result.kw.get('msg') == 'File does not exist'


=====================================
tests/test_dogtag_connectivity.py
=====================================
@@ -2,14 +2,46 @@
 # Copyright (C) 2019 FreeIPA Contributors see COPYING for license
 #
 
+from unittest.mock import Mock, patch
 from util import capture_results, CAInstance
 from util import m_api
+
 from base import BaseTest
 from ipahealthcheck.core import constants, config
 from ipahealthcheck.dogtag.plugin import registry
 from ipahealthcheck.dogtag.ca import DogtagCertsConnectivityCheck
-from unittest.mock import Mock
+
 from ipalib.errors import CertificateOperationError
+from ipaplatform.paths import paths
+from ipapython.dn import DN
+
+
+class IPACertificate:
+    def __init__(self, serial_number=1,
+                 subject='CN=Certificate Authority, O=%s' % m_api.env.realm):
+        self.serial_number = serial_number
+        self.subject = subject
+
+    def __eq__(self, other):
+        return self.serial_number == other.serial_number
+
+    def __hash__(self):
+        return hash(self.serial_number)
+
+
+subject_base = [{
+    'result':
+        {
+            'ipacertificatesubjectbase': [f'O={m_api.env.realm}'],
+        },
+}]
+
+bad_subject_base = [{
+    'result':
+        {
+            'ipacertificatesubjectbase': ['O=BAD'],
+        },
+}]
 
 
 class TestCAConnectivity(BaseTest):
@@ -18,12 +50,18 @@ class TestCAConnectivity(BaseTest):
         Mock(return_value=CAInstance()),
     }
 
-    def test_ca_connection_ok(self):
+    @patch('ipaserver.install.ca.lookup_ca_subject')
+    @patch('ipalib.x509.load_certificate_list_from_file')
+    def test_ca_connection_ok(self, mock_load_cert, mock_ca_subject):
         """CA connectivity check when cert_show returns a valid value"""
         m_api.Command.cert_show.side_effect = None
+        m_api.Command.config_show.side_effect = subject_base
         m_api.Command.cert_show.return_value = {
             u'result': {u'revoked': False}
         }
+        mock_load_cert.return_value = [IPACertificate(12345)]
+        mock_ca_subject.return_value = DN(('cn', 'Certificate Authority'),
+                                          f'O={m_api.env.realm}')
 
         framework = object()
         registry.initialize(framework, config.Config)
@@ -38,13 +76,20 @@ class TestCAConnectivity(BaseTest):
         assert result.source == 'ipahealthcheck.dogtag.ca'
         assert result.check == 'DogtagCertsConnectivityCheck'
 
-    def test_ca_connection_cert_not_found(self):
+    @patch('ipaserver.install.ca.lookup_ca_subject')
+    @patch('ipalib.x509.load_certificate_list_from_file')
+    def test_ca_connection_cert_not_found(self, mock_load_cert,
+                                          mock_ca_subject):
         """CA connectivity check for a cert that doesn't exist"""
         m_api.Command.cert_show.reset_mock()
+        m_api.Command.config_show.side_effect = subject_base
         m_api.Command.cert_show.side_effect = CertificateOperationError(
             message='Certificate operation cannot be completed: '
                     'EXCEPTION (Certificate serial number 0x0 not found)'
         )
+        mock_load_cert.return_value = [IPACertificate()]
+        mock_ca_subject.return_value = DN(('cn', 'Certificate Authority'),
+                                          f'O={m_api.env.realm}')
 
         framework = object()
         registry.initialize(framework, config.Config)
@@ -55,16 +100,82 @@ class TestCAConnectivity(BaseTest):
         assert len(self.results) == 1
 
         result = self.results.results[0]
-        assert result.result == constants.SUCCESS
+        assert result.result == constants.ERROR
+        assert result.source == 'ipahealthcheck.dogtag.ca'
+        assert result.check == 'DogtagCertsConnectivityCheck'
+        assert result.kw.get('key') == 'cert_show_1'
+        assert result.kw.get('serial') == '1'
+        assert result.kw.get('msg') == 'Serial number not found: {error}'
+
+    @patch('ipaserver.install.ca.lookup_ca_subject')
+    @patch('ipalib.x509.load_certificate_list_from_file')
+    def test_ca_connection_cert_file_not_found(self, mock_load_cert,
+                                               mock_ca_subject):
+        """CA connectivity check for a cert that doesn't exist"""
+        m_api.Command.cert_show.reset_mock()
+        m_api.Command.config_show.side_effect = subject_base
+        mock_load_cert.side_effect = FileNotFoundError()
+        mock_ca_subject.return_value = DN(('cn', 'Certificate Authority'),
+                                          f'O={m_api.env.realm}')
+
+        framework = object()
+        registry.initialize(framework, config.Config)
+        f = DogtagCertsConnectivityCheck(registry)
+
+        self.results = capture_results(f)
+
+        assert len(self.results) == 1
+
+        result = self.results.results[0]
+        assert result.result == constants.ERROR
         assert result.source == 'ipahealthcheck.dogtag.ca'
         assert result.check == 'DogtagCertsConnectivityCheck'
+        assert result.kw.get('key') == 'ipa_ca_crt_file_missing'
+        assert result.kw.get('path') == paths.IPA_CA_CRT
+
+    @patch('ipaserver.install.ca.lookup_ca_subject')
+    @patch('ipalib.x509.load_certificate_list_from_file')
+    def test_ca_connection_cert_not_in_file_list(self, mock_load_cert,
+                                                 mock_ca_subject):
+        """CA connectivity check for a cert that isn't in IPA_CA_CRT"""
+        m_api.Command.cert_show.reset_mock()
+        m_api.Command.config_show.side_effect = bad_subject_base
+        mock_load_cert.return_value = [IPACertificate()]
+        mock_ca_subject.return_value = DN(('cn', 'Certificate Authority'),
+                                          'O=BAD')
 
-    def test_ca_connection_down(self):
+        framework = object()
+        registry.initialize(framework, config.Config)
+        f = DogtagCertsConnectivityCheck(registry)
+
+        self.results = capture_results(f)
+
+        assert len(self.results) == 1
+
+        result = self.results.results[0]
+        assert result.result == constants.ERROR
+        assert result.source == 'ipahealthcheck.dogtag.ca'
+        assert result.check == 'DogtagCertsConnectivityCheck'
+        bad = bad_subject_base[0]['result']['ipacertificatesubjectbase'][0]
+        bad_subject = DN(f'CN=Certificate Authority,{bad}')
+        assert DN(result.kw['subject']) == bad_subject
+        assert result.kw['path'] == paths.IPA_CA_CRT
+        assert result.kw['msg'] == (
+            'The CA certificate with subject {subject} was not found in {path}'
+        )
+
+    @patch('ipaserver.install.ca.lookup_ca_subject')
+    @patch('ipalib.x509.load_certificate_list_from_file')
+    def test_ca_connection_down(self, mock_load_cert, mock_ca_subject):
         """CA connectivity check with the CA down"""
         m_api.Command.cert_show.side_effect = CertificateOperationError(
             message='Certificate operation cannot be completed: '
                     'Unable to communicate with CMS (503)'
         )
+        m_api.Command.config_show.side_effect = subject_base
+        mock_load_cert.return_value = [IPACertificate()]
+        mock_ca_subject.return_value = DN(('cn', 'Certificate Authority'),
+                                          f'O={m_api.env.realm}')
 
         framework = object()
         registry.initialize(framework, config.Config)
@@ -78,4 +189,93 @@ class TestCAConnectivity(BaseTest):
         assert result.result == constants.ERROR
         assert result.source == 'ipahealthcheck.dogtag.ca'
         assert result.check == 'DogtagCertsConnectivityCheck'
-        assert 'Unable to communicate' in result.kw.get('msg')
+        assert result.kw.get('msg') == (
+            'Request for certificate failed: {error}'
+        )
+
+    @patch('ipaserver.install.ca.lookup_ca_subject')
+    @patch('ipalib.x509.load_certificate_list_from_file')
+    def test_ca_connection_multiple_ok(self, mock_load_cert, mock_ca_subject):
+        """CA connectivity check when cert_show returns a valid value"""
+        m_api.Command.cert_show.side_effect = None
+        m_api.Command.config_show.side_effect = subject_base
+        m_api.Command.cert_show.return_value = {
+            u'result': {u'revoked': False}
+        }
+        mock_load_cert.return_value = [
+            IPACertificate(1, 'CN=something'),
+            IPACertificate(12345),
+        ]
+        mock_ca_subject.return_value = DN(('cn', 'Certificate Authority'),
+                                          f'O={m_api.env.realm}')
+
+        framework = object()
+        registry.initialize(framework, config.Config)
+        f = DogtagCertsConnectivityCheck(registry)
+
+        self.results = capture_results(f)
+
+        assert len(self.results) == 1
+
+        result = self.results.results[0]
+        assert result.result == constants.SUCCESS
+        assert result.source == 'ipahealthcheck.dogtag.ca'
+
+    @patch('ipaserver.install.ca.lookup_ca_subject')
+    @patch('ipalib.x509.load_certificate_list_from_file')
+    def test_ca_connection_multiple_ok_reverse(self, mock_load_cert,
+                                               mock_ca_subject):
+        """CA connectivity check when cert_show returns a valid value"""
+        m_api.Command.cert_show.side_effect = None
+        m_api.Command.config_show.side_effect = subject_base
+        m_api.Command.cert_show.return_value = {
+            u'result': {u'revoked': False}
+        }
+        mock_load_cert.return_value = [
+            IPACertificate(12345),
+            IPACertificate(1, 'CN=something'),
+        ]
+        mock_ca_subject.return_value = DN(('cn', 'Certificate Authority'),
+                                          f'O={m_api.env.realm}')
+
+        framework = object()
+        registry.initialize(framework, config.Config)
+        f = DogtagCertsConnectivityCheck(registry)
+
+        self.results = capture_results(f)
+
+        assert len(self.results) == 1
+
+        result = self.results.results[0]
+        assert result.result == constants.SUCCESS
+        assert result.source == 'ipahealthcheck.dogtag.ca'
+
+    @patch('ipaserver.install.ca.lookup_ca_subject')
+    @patch('ipalib.x509.load_certificate_list_from_file')
+    def test_ca_connection_not_found(self, mock_load_cert, mock_ca_subject):
+        """CA connectivity check when cert_show returns a valid value"""
+        m_api.Command.cert_show.side_effect = None
+        m_api.Command.config_show.side_effect = subject_base
+        m_api.Command.cert_show.return_value = {
+            u'result': {u'revoked': False}
+        }
+        mock_load_cert.return_value = [
+            IPACertificate(1, 'CN=something'),
+        ]
+        mock_ca_subject.return_value = DN(('cn', 'Certificate Authority'),
+                                          f'O={m_api.env.realm}')
+
+        framework = object()
+        registry.initialize(framework, config.Config)
+        f = DogtagCertsConnectivityCheck(registry)
+
+        self.results = capture_results(f)
+
+        assert len(self.results) == 1
+
+        result = self.results.results[0]
+        assert result.result == constants.ERROR
+        assert result.source == 'ipahealthcheck.dogtag.ca'
+        assert result.kw['msg'] == (
+            'The CA certificate with subject {subject} was not found in {path}'
+        )


=====================================
tests/test_ipa_cert_match.py
=====================================
@@ -44,8 +44,15 @@ class mock_ldap:
     def get_entries(self, base_dn, scope=SCOPE_SUBTREE, filter=None,
                     attrs_list=None, get_effective_rights=False, **kwargs):
         if self.results is None:
-            raise errors.NotFound(reason='test')
-        return self.results.values()
+            raise errors.NotFound(reason='None')
+        if filter:
+            (attr, value) = filter.split('=', maxsplit=1)
+            for result in self.results.values():
+                if result.get(attr)[0] == value:
+                    return [result]
+            raise errors.NotFound(reason='Not found %s' % filter)
+
+        return self.results
 
 
 class mock_ldap_conn:
@@ -82,6 +89,10 @@ class TestIPACertMatch(BaseTest):
         Mock(return_value=mock_ldap_conn())
     }
 
+    trust = {
+        ('%s IPA CA' % m_api.env.realm): 'u,u,u'
+    }
+
     @patch('ipalib.x509.load_certificate_list_from_file')
     @patch('ipaserver.install.certs.CertDB')
     def test_certs_match_ok(self, mock_certdb, mock_load_cert):
@@ -92,11 +103,8 @@ class TestIPACertMatch(BaseTest):
                                    'cn=certificates,cn=ipa,cn=etc',
                                     m_api.env.basedn),
                                 CACertificate=[IPACertificate()])
-        trust = {
-            ('%s IPA CA' % m_api.env.realm): 'u,u,u'
-        }
 
-        mock_certdb.return_value = mock_CertDB(trust)
+        mock_certdb.return_value = mock_CertDB(self.trust)
         mock_load_cert.return_value = [IPACertificate()]
 
         framework = object()
@@ -121,11 +129,8 @@ class TestIPACertMatch(BaseTest):
                                    'cn=certificates,cn=ipa,cn=etc',
                                     m_api.env.basedn),
                                 CACertificate=[IPACertificate()])
-        trust = {
-            ('%s IPA CA' % m_api.env.realm): 'u,u,u'
-        }
 
-        mock_certdb.return_value = mock_CertDB(trust)
+        mock_certdb.return_value = mock_CertDB(self.trust)
         mock_load_cert.return_value = [IPACertificate(serial_number=2)]
 
         framework = object()
@@ -155,15 +160,54 @@ class TestIPACertMatch(BaseTest):
         assert len(self.results) == 0
 
 
+default_subject_base = [{
+    'result':
+        {
+            'ipacertificatesubjectbase': [f'O={m_api.env.realm}'],
+        },
+}]
+
+custom_subject_base = [{
+    'result':
+        {
+            'ipacertificatesubjectbase': ['OU=Eng,O=ACME'],
+        },
+}]
+
+
 class TestIPADogtagCertMatch(BaseTest):
     patches = {
         'ipaserver.install.krainstance.KRAInstance':
         Mock(return_value=KRAInstance()),
     }
+    trust = {
+        'ocspSigningCert cert-pki-ca': 'u,u,u',
+        'caSigningCert cert-pki-ca': 'u,u,u',
+        'subsystemCert cert-pki-ca': 'u,u,u',
+        'auditSigningCert cert-pki-ca': 'u,u,Pu',
+        'Server-Cert cert-pki-ca': 'u,u,u',
+        'transportCert cert-pki-kra': 'u,u,u',
+        'storageCert cert-pki-kra': 'u,u,u',
+        'auditSigningCert cert-pki-kra': 'u,u,Pu',
+    }
+
+    def get_dogtag_subjects(self, hostname, base):
+        subject_base = base[0]['result']['ipacertificatesubjectbase'][0]
+        return (
+            f'CN=OCSP Subsystem,{subject_base}',
+            f'CN=CA Subsystem,{subject_base}',
+            f'CN=CA Audit,{subject_base}',
+            f'CN=%s,{subject_base}',
+            f'CN=KRA Transport Certificate,{subject_base}',
+            f'CN=KRA Storage Certificate,{subject_base}',
+            f'CN=KRA Audit,{subject_base}',
+            f'CN={hostname},{subject_base}',
+        )
 
     @patch('ipaserver.install.certs.CertDB')
     def test_certs_match_ok(self, mock_certdb):
         """ Ensure match check is ok"""
+        m_api.Command.config_show.side_effect = default_subject_base
         fake_conn = LDAPClient('ldap://localhost', no_schema=True)
         pkidbentry = LDAPEntry(fake_conn,
                                DN('uid=pkidbuser,ou=people,o=ipaca'),
@@ -177,25 +221,9 @@ class TestIPADogtagCertMatch(BaseTest):
                                 userCertificate=[IPACertificate()],
                                 subjectName=['test'])
         ldap_entries = [pkidbentry, casignentry]
-        trust = {
-            'ocspSigningCert cert-pki-ca': 'u,u,u',
-            'caSigningCert cert-pki-ca': 'u,u,u',
-            'subsystemCert cert-pki-ca': 'u,u,u',
-            'auditSigningCert cert-pki-ca': 'u,u,Pu',
-            'Server-Cert cert-pki-ca': 'u,u,u',
-            'transportCert cert-pki-kra': 'u,u,u',
-            'storageCert cert-pki-kra': 'u,u,u',
-            'auditSigningCert cert-pki-kra': 'u,u,Pu',
-        }
-
-        dogtag_entries_subjects = (
-            'CN=OCSP Subsystem,O=%s' % m_api.env.realm,
-            'CN=CA Subsystem,O=%s' % m_api.env.realm,
-            'CN=CA Audit,O=%s' % m_api.env.realm,
-            'CN=%s,O=%s' % (m_api.env.host, m_api.env.realm),
-            'CN=KRA Transport Certificate,O=%s' % m_api.env.realm,
-            'CN=KRA Storage Certificate,O=%s' % m_api.env.realm,
-            'CN=KRA Audit,O=%s' % m_api.env.realm,
+
+        dogtag_entries_subjects = self.get_dogtag_subjects(
+            m_api.env.host, default_subject_base
         )
 
         for i, subject in enumerate(dogtag_entries_subjects):
@@ -206,7 +234,7 @@ class TestIPADogtagCertMatch(BaseTest):
                               subjectName=[subject])
             ldap_entries.append(entry)
 
-        mock_certdb.return_value = mock_CertDB(trust)
+        mock_certdb.return_value = mock_CertDB(self.trust)
 
         framework = object()
         registry.initialize(framework, config.Config())
@@ -223,6 +251,7 @@ class TestIPADogtagCertMatch(BaseTest):
     @patch('ipaserver.install.certs.CertDB')
     def test_certs_mismatch(self, mock_certdb):
         """ Ensure mismatches are detected"""
+        m_api.Command.config_show.side_effect = default_subject_base
         fake_conn = LDAPClient('ldap://localhost', no_schema=True)
         pkidbentry = LDAPEntry(fake_conn,
                                DN('uid=pkidbuser,ou=people,o=ipaca'),
@@ -238,25 +267,9 @@ class TestIPADogtagCertMatch(BaseTest):
                                 userCertificate=[IPACertificate()],
                                 subjectName=['test'])
         ldap_entries = [pkidbentry, casignentry]
-        trust = {
-            'ocspSigningCert cert-pki-ca': 'u,u,u',
-            'caSigningCert cert-pki-ca': 'u,u,u',
-            'subsystemCert cert-pki-ca': 'u,u,u',
-            'auditSigningCert cert-pki-ca': 'u,u,Pu',
-            'Server-Cert cert-pki-ca': 'u,u,u',
-            'transportCert cert-pki-kra': 'u,u,u',
-            'storageCert cert-pki-kra': 'u,u,u',
-            'auditSigningCert cert-pki-kra': 'u,u,Pu',
-        }
-
-        dogtag_entries_subjects = (
-            'CN=OCSP Subsystem,O=%s' % m_api.env.realm,
-            'CN=CA Subsystem,O=%s' % m_api.env.realm,
-            'CN=CA Audit,O=%s' % m_api.env.realm,
-            'CN=%s,O=%s' % (m_api.env.host, m_api.env.realm),
-            'CN=KRA Transport Certificate,O=%s' % m_api.env.realm,
-            'CN=KRA Storage Certificate,O=%s' % m_api.env.realm,
-            'CN=KRA Audit,O=%s' % m_api.env.realm,
+
+        dogtag_entries_subjects = self.get_dogtag_subjects(
+            m_api.env.host, default_subject_base
         )
 
         for i, subject in enumerate(dogtag_entries_subjects):
@@ -267,7 +280,7 @@ class TestIPADogtagCertMatch(BaseTest):
                               subjectName=[subject])
             ldap_entries.append(entry)
 
-        mock_certdb.return_value = mock_CertDB(trust)
+        mock_certdb.return_value = mock_CertDB(self.trust)
 
         framework = object()
         registry.initialize(framework, config.Config())
@@ -280,3 +293,92 @@ class TestIPADogtagCertMatch(BaseTest):
         assert result.result == constants.ERROR
         assert result.source == 'ipahealthcheck.ipa.certs'
         assert result.check == 'IPADogtagCertsMatchCheck'
+
+    @patch('ipaserver.install.certs.CertDB')
+    def test_certs_match_ok_subject(self, mock_certdb):
+        """ Ensure match check is ok"""
+        m_api.Command.config_show.side_effect = custom_subject_base
+        fake_conn = LDAPClient('ldap://localhost', no_schema=True)
+        pkidbentry = LDAPEntry(fake_conn,
+                               DN('uid=pkidbuser,ou=people,o=ipaca'),
+                               userCertificate=[IPACertificate()],
+                               subjectName=['test'])
+        casignentry = LDAPEntry(fake_conn,
+                                DN('cn=%s IPA CA' % m_api.env.realm,
+                                   'cn=certificates,cn=ipa,cn=etc',
+                                    m_api.env.basedn),
+                                CACertificate=[IPACertificate()],
+                                userCertificate=[IPACertificate()],
+                                subjectName=['test'])
+        ldap_entries = [pkidbentry, casignentry]
+
+        dogtag_entries_subjects = self.get_dogtag_subjects(
+            m_api.env.host, custom_subject_base
+        )
+
+        for i, subject in enumerate(dogtag_entries_subjects):
+            entry = LDAPEntry(fake_conn,
+                              DN('cn=%i,ou=certificateRepository' % i,
+                                 'ou=ca,o=ipaca'),
+                              userCertificate=[IPACertificate()],
+                              subjectName=[subject])
+            ldap_entries.append(entry)
+
+        mock_certdb.return_value = mock_CertDB(self.trust)
+
+        framework = object()
+        registry.initialize(framework, config.Config())
+        f = IPADogtagCertsMatchCheck(registry)
+        f.conn = mock_ldap(ldap_entries)
+        self.results = capture_results(f)
+
+        assert len(self.results) == 3
+        for result in self.results.results:
+            assert result.result == constants.SUCCESS
+            assert result.source == 'ipahealthcheck.ipa.certs'
+            assert result.check == 'IPADogtagCertsMatchCheck'
+
+    @patch('ipaserver.install.certs.CertDB')
+    def test_certs_mismatch_subject(self, mock_certdb):
+        """ Ensure mismatches are detected"""
+        m_api.Command.config_show.side_effect = custom_subject_base
+        fake_conn = LDAPClient('ldap://localhost', no_schema=True)
+        pkidbentry = LDAPEntry(fake_conn,
+                               DN('uid=pkidbuser,ou=people,o=ipaca'),
+                               userCertificate=[IPACertificate(
+                                   serial_number=2
+                               )],
+                               subjectName=['test'])
+        casignentry = LDAPEntry(fake_conn,
+                                DN('cn=%s IPA CA' % m_api.env.realm,
+                                   'cn=certificates,cn=ipa,cn=etc',
+                                    m_api.env.basedn),
+                                CACertificate=[IPACertificate()],
+                                userCertificate=[IPACertificate()],
+                                subjectName=['test'])
+        ldap_entries = [pkidbentry, casignentry]
+
+        dogtag_entries_subjects = self.get_dogtag_subjects(
+            m_api.env.host, custom_subject_base
+        )
+
+        for i, subject in enumerate(dogtag_entries_subjects):
+            entry = LDAPEntry(fake_conn,
+                              DN('cn=%i,ou=certificateRepository' % i,
+                                 'ou=ca,o=ipaca'),
+                              userCertificate=[IPACertificate()],
+                              subjectName=[subject])
+            ldap_entries.append(entry)
+
+        mock_certdb.return_value = mock_CertDB(self.trust)
+
+        framework = object()
+        registry.initialize(framework, config.Config())
+        f = IPADogtagCertsMatchCheck(registry)
+        f.conn = mock_ldap(ldap_entries)
+        self.results = capture_results(f)
+
+        assert len(self.results) == 3
+        result = self.results.results[0]
+        assert result.result == constants.ERROR
+        assert result.source == 'ipahealthcheck.ipa.certs'


=====================================
tests/test_ipa_dns.py
=====================================
@@ -96,8 +96,8 @@ def query_uri(hosts):
     if version.MAJOR < 2 or (version.MAJOR == 2 and version.MINOR == 0):
         m = message.Message()
     elif version.MAJOR == 2 and version.MINOR > 0:
-        m = message.QueryMessage()  # pylint: disable=E1101
-        m = message.make_response(m)  # pylint: disable=E1101
+        m = message.QueryMessage()
+        m = message.make_response(m)
 
     rdtype = rdatatype.URI
     for name in ('_kerberos.', '_kpasswd.'):
@@ -158,8 +158,8 @@ def fake_query(qname, rdtype=rdatatype.A, rdclass=rdataclass.IN, count=1,
     if version.MAJOR < 2 or (version.MAJOR == 2 and version.MINOR == 0):
         m = message.Message()
     elif version.MAJOR == 2 and version.MINOR > 0:
-        m = message.QueryMessage()  # pylint: disable=E1101
-        m = message.make_response(m)  # pylint: disable=E1101
+        m = message.QueryMessage()
+        m = message.make_response(m)
     if rdtype in (rdatatype.A, rdatatype.AAAA):
         fqdn = DNSName(qname)
         fqdn = fqdn.make_absolute()


=====================================
tests/test_ipa_kdc.py
=====================================
@@ -0,0 +1,101 @@
+#
+# Copyright (C) 2022 FreeIPA Contributors see COPYING for license
+#
+
+from base import BaseTest
+from unittest.mock import patch
+from util import capture_results
+
+from ipahealthcheck.core import config, constants
+from ipahealthcheck.ipa.plugin import registry
+from ipahealthcheck.ipa.kdc import KDCWorkersCheck
+
+
+class TestKDCWorkers(BaseTest):
+    @patch('ipahealthcheck.ipa.kdc.get_contents')
+    @patch('os.sysconf')
+    def test_no_workers(self, mock_sysconf, mock_sysconfig):
+        mock_sysconf.return_value = 1
+        mock_sysconfig.return_value = ""
+        framework = object()
+        registry.initialize(framework, config.Config)
+        f = KDCWorkersCheck(registry)
+
+        self.results = capture_results(f)
+
+        assert len(self.results) == 1
+
+        result = self.results.results[0]
+        assert result.result == constants.WARNING
+        assert result.source == 'ipahealthcheck.ipa.kdc'
+        assert result.check == 'KDCWorkersCheck'
+        assert result.kw.get('key') == 'workers'
+        assert result.kw.get('sysconfig') == '/etc/sysconfig/krb5kdc'
+        assert result.kw.get('msg') == 'KRB5KDC_ARGS is not set in {sysconfig}'
+
+    @patch('ipahealthcheck.ipa.kdc.get_contents')
+    @patch('os.sysconf')
+    def test_workers_match_single(self, mock_sysconf, mock_sysconfig):
+        mock_sysconf.return_value = 1
+        mock_sysconfig.return_value = (
+            ("KRB5KDC_ARGS='-w 1'", "KRB5REALM=EXAMPLE.TEST")
+        )
+        framework = object()
+        registry.initialize(framework, config.Config)
+        f = KDCWorkersCheck(registry)
+
+        self.results = capture_results(f)
+
+        assert len(self.results) == 1
+
+        result = self.results.results[0]
+        assert result.result == constants.SUCCESS
+        assert result.source == 'ipahealthcheck.ipa.kdc'
+        assert result.check == 'KDCWorkersCheck'
+        assert result.kw.get('key') == 'workers'
+
+    @patch('ipahealthcheck.ipa.kdc.get_contents')
+    @patch('os.sysconf')
+    def test_workers_match_double(self, mock_sysconf, mock_sysconfig):
+        mock_sysconf.return_value = 1
+        mock_sysconfig.return_value = (
+            ('KRB5KDC_ARGS="-w 1"', "KRB5REALM=EXAMPLE.TEST")
+        )
+        framework = object()
+        registry.initialize(framework, config.Config)
+        f = KDCWorkersCheck(registry)
+
+        self.results = capture_results(f)
+
+        assert len(self.results) == 1
+
+        result = self.results.results[0]
+        assert result.result == constants.SUCCESS
+        assert result.source == 'ipahealthcheck.ipa.kdc'
+        assert result.check == 'KDCWorkersCheck'
+
+    @patch('ipahealthcheck.ipa.kdc.get_contents')
+    @patch('os.sysconf')
+    def test_workers_mismatch(self, mock_sysconf, mock_sysconfig):
+        mock_sysconf.return_value = 2
+        mock_sysconfig.return_value = (
+            ("KRB5KDC_ARGS='-w 1'", "KRB5REALM=EXAMPLE.TEST")
+        )
+        framework = object()
+        registry.initialize(framework, config.Config)
+        f = KDCWorkersCheck(registry)
+
+        self.results = capture_results(f)
+
+        assert len(self.results) == 1
+
+        result = self.results.results[0]
+        assert result.result == constants.WARNING
+        assert result.source == 'ipahealthcheck.ipa.kdc'
+        assert result.check == 'KDCWorkersCheck'
+        assert result.kw.get('key') == 'workers'
+        assert result.kw.get('cpus') == 2
+        assert result.kw.get('workers') == 1
+        assert result.kw.get("msg") == "The number of CPUs {cpus} " \
+                                       "does not match the number of " \
+                                       "workers {workers} in {sysconfig}"


=====================================
tests/test_ipa_nss.py
=====================================
@@ -7,6 +7,8 @@ from collections import namedtuple
 from unittest.mock import patch
 from util import capture_results
 
+from ipaplatform.constants import constants as platform_constants
+
 from ipahealthcheck.core import config, constants
 from ipahealthcheck.ipa.plugin import registry
 from ipahealthcheck.ipa.nss import IPAGroupMemberCheck
@@ -22,10 +24,11 @@ def make_group(name, members):
 
 
 class TestGroupMember(BaseTest):
-    @patch('grp.getgrnam')
+    @patch('ipahealthcheck.ipa.nss.grp.getgrnam')
     def test_ipaapi_group_ok(self, mock_grp):
-        mock_grp.return_value = make_group('apache', ('apache', 'ipaapi',))
-
+        mock_grp.return_value = make_group(
+            platform_constants.IPAAPI_GROUP, (platform_constants.HTTPD_USER,),
+        )
         framework = object()
         registry.initialize(framework, config.Config)
         registry.trust_agent = True
@@ -38,9 +41,11 @@ class TestGroupMember(BaseTest):
         result = self.results.results[0]
         assert result.result == constants.SUCCESS
 
-    @patch('grp.getgrnam')
+    @patch('ipahealthcheck.ipa.nss.grp.getgrnam')
     def test_ipaapi_bad_group(self, mock_grp):
-        mock_grp.side_effect = KeyError("name not found: 'ipaapi'")
+        mock_grp.side_effect = KeyError(
+            f"name not found: '{platform_constants.IPAAPI_GROUP}'"
+        )
 
         framework = object()
         registry.initialize(framework, config.Config)
@@ -53,12 +58,14 @@ class TestGroupMember(BaseTest):
 
         result = self.results.results[0]
         assert result.result == constants.ERROR
-        assert result.kw.get('key') == 'ipaapi'
+        assert result.kw.get('key') == platform_constants.IPAAPI_GROUP
         assert result.kw.get('msg') == 'group {key} does not exist'
 
-    @patch('grp.getgrnam')
+    @patch('ipahealthcheck.ipa.nss.grp.getgrnam')
     def test_ipaapi_missing_member(self, mock_grp):
-        mock_grp.return_value = make_group('apache', ('foo',))
+        mock_grp.return_value = make_group(
+            platform_constants.IPAAPI_GROUP, ('foo',)
+        )
 
         framework = object()
         registry.initialize(framework, config.Config)
@@ -71,6 +78,6 @@ class TestGroupMember(BaseTest):
 
         result = self.results.results[0]
         assert result.result == constants.ERROR
-        assert result.kw.get('key') == 'ipaapi'
+        assert result.kw.get('key') == platform_constants.IPAAPI_GROUP
         assert result.kw.get('msg') == \
             '{member} is not a member of group {key}'


=====================================
tests/test_ipa_proxy.py
=====================================
@@ -214,7 +214,6 @@ class TestIPAProxySecretCheck(BaseTest):
 
         self.results = capture_results(f)
 
-        print(self.results.results)
         assert len(self.results) == 2
 
         result = self.results.results[0]


=====================================
tests/test_options.py
=====================================
@@ -3,8 +3,11 @@
 #
 
 import argparse
+import io
+import logging
 import os
 import tempfile
+from contextlib import redirect_stdout
 from unittest.mock import patch
 
 from ipahealthcheck.core.core import RunChecks
@@ -40,6 +43,97 @@ def test_options_merge(mock_parse, mock_run, mock_service):
 
         # verify two valus that have defaults with our overriden values
         assert run.options.output_type == 'human'
-        assert run.options.indent == '5'
+        assert run.options.indent == 5
+    finally:
+        os.remove(config_path)
+
+
+ at patch('ipahealthcheck.core.core.run_service_plugins')
+ at patch('ipahealthcheck.core.core.run_plugins')
+ at patch('ipahealthcheck.core.core.parse_options')
+def test_cfg_file_debug_option(mock_parse, mock_run, mock_service):
+    """
+    Test if the debug option is respected in the configuration file
+
+    Related: https://bugzilla.redhat.com/show_bug.cgi?id=2079861
+    """
+    mock_service.return_value = (Results(), [])
+    mock_run.return_value = Results()
+    mock_parse.return_value = options
+    fd, config_path = tempfile.mkstemp()
+    os.close(fd)
+    with open(config_path, "w") as fd:
+        fd.write('[default]\n')
+        fd.write('debug=True\n')
+
+    try:
+        run = RunChecks([], config_path)
+        run.run_healthcheck()
+
+        assert run.options.debug
+    finally:
+        os.remove(config_path)
+
+
+ at patch('ipahealthcheck.core.core.run_service_plugins')
+ at patch('ipahealthcheck.core.core.run_plugins')
+ at patch('ipahealthcheck.core.core.parse_options')
+def test_incorrect_output_type_cfg_file(mock_parse, mock_run, mock_service):
+    """
+    Test the error message is user-friendly if the incorrect output type is
+    provided in cfg file
+
+    Related: https://bugzilla.redhat.com/show_bug.cgi?id=2079698
+    """
+    mock_service.return_value = (Results(), [])
+    mock_run.return_value = Results()
+    mock_parse.return_value = options
+    fd, config_path = tempfile.mkstemp()
+    os.close(fd)
+    with open(config_path, "w") as fd:
+        fd.write('[default]\n')
+        fd.write('output_type=42\n')
+
+    try:
+        run = RunChecks([], config_path)
+
+        f = io.StringIO()
+        with redirect_stdout(f):
+            run.run_healthcheck()
+
+        assert "Unknown output-type" in f.getvalue()
+
+    finally:
+        os.remove(config_path)
+
+
+ at patch('ipahealthcheck.core.core.run_service_plugins')
+ at patch('ipahealthcheck.core.core.run_plugins')
+ at patch('ipahealthcheck.core.core.parse_options')
+def test_incorrect_delimiter_cfg_file(mock_parse, mock_run, mock_service,
+                                      caplog):
+    """
+    Test the error message is user-friendly if the incorrect delimiter is
+    used in cfg file
+
+    Related: https://bugzilla.redhat.com/show_bug.cgi?id=2079739
+    """
+    mock_service.return_value = (Results(), [])
+    mock_run.return_value = Results()
+    mock_parse.return_value = options
+    fd, config_path = tempfile.mkstemp()
+    os.close(fd)
+    with open(config_path, "w") as fd:
+        fd.write('[default]\n')
+        fd.write('output_type;human\n')
+
+    try:
+        run = RunChecks([], config_path)
+
+        with caplog.at_level(logging.ERROR):
+            run.run_healthcheck()
+
+        assert "contains parsing errors" in caplog.text
+
     finally:
         os.remove(config_path)



View it on GitLab: https://salsa.debian.org/freeipa-team/freeipa-healthcheck/-/compare/d6035fe2c490c801a1b0c2774f93df7f0dbfd136...89d1736c39efc3f25845f6ee42cfec65e97ba10a

-- 
View it on GitLab: https://salsa.debian.org/freeipa-team/freeipa-healthcheck/-/compare/d6035fe2c490c801a1b0c2774f93df7f0dbfd136...89d1736c39efc3f25845f6ee42cfec65e97ba10a
You're receiving this email because of your account on salsa.debian.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://alioth-lists.debian.net/pipermail/pkg-freeipa-devel/attachments/20220826/afa531a8/attachment-0001.htm>


More information about the Pkg-freeipa-devel mailing list