From 0f978d969ba4ceb643ccce134d5ffb5ffcadf12c Mon Sep 17 00:00:00 2001
From: Dimitri Savineau <dsavinea@redhat.com>
Date: Fri, 11 Oct 2019 11:42:36 -0400
Subject: [PATCH] Remove validate action and notario dependency

The current ceph-validate role is using both validate action and fail
module tasks to validate the ceph configuration.
The validate action is based on the notario python library. When one of
the notario validation fails then a python stack trace is reported to the
ansible task. This output isn't understandable by users.

This patch removes the validate action and the notario depencendy. The
validation is now done with only fail ansible module.

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

Signed-off-by: Dimitri Savineau <dsavinea@redhat.com>
---
 ceph-ansible.spec.in               |   2 -
 plugins/actions/validate.py        | 318 -----------------------------
 requirements.txt                   |   1 -
 roles/ceph-validate/tasks/main.yml |  88 +++++++-
 tests/requirements.txt             |   1 -
 5 files changed, 85 insertions(+), 325 deletions(-)
 delete mode 100644 plugins/actions/validate.py

diff --git a/ceph-ansible.spec.in b/ceph-ansible.spec.in
index fa436226e..e2ceb2ead 100644
--- a/ceph-ansible.spec.in
+++ b/ceph-ansible.spec.in
@@ -22,11 +22,9 @@ Requires: ansible >= 2.8
 %if 0%{?rhel} == 7
 BuildRequires: python2-devel
 Requires: python2-netaddr
-Requires: python2-notario >=  0.0.13
 %else
 BuildRequires: python3-devel
 Requires: python3-netaddr
-Requires: python3-notario >=  0.0.13
 %endif
 
 %description
diff --git a/plugins/actions/validate.py b/plugins/actions/validate.py
deleted file mode 100644
index 5debffe20..000000000
--- a/plugins/actions/validate.py
+++ /dev/null
@@ -1,318 +0,0 @@
-
-from ansible.plugins.action import ActionBase
-from distutils.version import LooseVersion
-from ansible.module_utils.six import string_types
-from ansible.errors import AnsibleUndefinedVariable
-
-try:
-    from __main__ import display
-except ImportError:
-    from ansible.utils.display import Display
-    display = Display()
-
-try:
-    import notario
-except ImportError:
-    msg = "The python-notario library is missing. Please install it on the node you are running ceph-ansible to continue."  # noqa E501
-    display.error(msg)
-    raise SystemExit(msg)
-
-if LooseVersion(notario.__version__) < LooseVersion("0.0.13"):
-    msg = "The python-notario libary has an incompatible version. Version >= 0.0.13 is needed, current version: %s" % notario.__version__
-    display.error(msg)
-    raise SystemExit(msg)
-
-from notario.exceptions import Invalid
-from notario.validators import types, chainable, iterables
-from notario.decorators import optional
-from notario.store import store as notario_store
-
-
-CEPH_RELEASES = ['jewel', 'kraken', 'luminous', 'mimic', 'nautilus', 'octopus']
-
-
-class ActionModule(ActionBase):
-
-    def run(self, tmp=None, task_vars=None):
-        # we must use vars, since task_vars will have un-processed variables
-        host_vars = self.expand_all_jinja2_templates(task_vars['vars'])
-        host = host_vars['ansible_hostname']
-        mode = self._task.args.get('mode', 'permissive')
-
-        self._supports_check_mode = False  # XXX ?
-        self._supports_async = True
-
-        result = {}
-        result['_ansible_verbose_always'] = True
-
-        try:
-            notario_store["groups"] = host_vars["groups"]
-            notario_store["ansible_distribution"] = host_vars["ansible_distribution"]
-            notario_store["containerized_deployment"] = host_vars["containerized_deployment"]  # noqa E501
-            notario.validate(host_vars, install_options, defined_keys=True)
-
-            if host_vars["ceph_origin"] == "repository" and not host_vars["containerized_deployment"]:
-                notario.validate(
-                    host_vars, ceph_origin_repository, defined_keys=True)
-
-                if host_vars["ceph_repository"] == "community":
-                    notario.validate(
-                        host_vars, ceph_repository_community, defined_keys=True)  # noqa E501
-
-                if host_vars["ceph_repository"] == "rhcs":
-                    notario.validate(
-                        host_vars, ceph_repository_rhcs, defined_keys=True)
-
-                if host_vars["ceph_repository"] == "dev":
-                    notario.validate(
-                        host_vars, ceph_repository_dev, defined_keys=True)
-
-                if host_vars["ceph_repository"] == "obs":
-                    notario.validate(
-                        host_vars, ceph_repository_obs, defined_keys=True)
-
-                if host_vars["ceph_repository"] == "custom":
-                    notario.validate(host_vars, ceph_repository_custom, defined_keys=True)
-
-            # store these values because one must be defined
-            # and the validation method
-            # will need access to all three through the store
-            notario_store["monitor_address"] = host_vars.get(
-                "monitor_address", None)
-            notario_store["monitor_address_block"] = host_vars.get(
-                "monitor_address_block", None)
-            notario_store["monitor_interface"] = host_vars.get(
-                "monitor_interface", None)
-
-            if host_vars["mon_group_name"] in host_vars["group_names"]:
-                notario.validate(host_vars, monitor_options, defined_keys=True)
-
-            notario_store["radosgw_address"] = host_vars.get(
-                "radosgw_address", None)
-            notario_store["radosgw_address_block"] = host_vars.get(
-                "radosgw_address_block", None)
-            notario_store["radosgw_interface"] = host_vars.get(
-                "radosgw_interface", None)
-
-            if host_vars["rgw_group_name"] in host_vars["group_names"]:
-                notario.validate(host_vars, rados_options, defined_keys=True)
-
-            # validate osd scenario setup
-            if host_vars["osd_group_name"] in host_vars["group_names"]:
-                notario.validate(host_vars, osd_options, defined_keys=True)
-                notario_store['osd_objectstore'] = host_vars["osd_objectstore"]
-                if not host_vars.get('osd_auto_discovery'):
-                    if host_vars.get("devices"):
-                        notario.validate(
-                            host_vars, lvm_batch_scenario, defined_keys=True)
-                    elif notario_store['osd_objectstore'] == 'filestore':
-                        notario.validate(
-                            host_vars, lvm_filestore_scenario, defined_keys=True)  # noqa E501
-                    elif notario_store['osd_objectstore'] == 'bluestore':
-                        notario.validate(
-                            host_vars, lvm_bluestore_scenario, defined_keys=True)  # noqa E501
-
-        except Invalid as error:
-            display.vvvv("Notario Failure: %s" % str(error))
-            msg = "[{}] Validation failed for variable: {}".format(
-                host, error.path[0])
-            display.error(msg)
-            reason = "[{}] Reason: {}".format(host, error.reason)
-            try:
-                if "schema is missing" not in str(error):
-                    for i in range(0, len(error.path)):
-                        if i == 0:
-                            given = "[{}] Given value for {}".format(
-                                    host, error.path[0])
-                        else:
-                            given = given + ": {}".format(error.path[i])
-                    if given:
-                        display.error(given)
-                else:
-                    given = ""
-                    reason = "[{}] Reason: {}".format(host, error.message)
-            except KeyError:
-                given = ""
-            display.error(reason)
-            result['failed'] = mode == 'strict'
-            result['msg'] = "\n".join([s for s in (msg, reason, given) if len(s) > 0])
-            result['stderr_lines'] = result['msg'].split('\n')
-
-
-        return result
-
-    def expand_all_jinja2_templates(self, variables):
-        for k, v in variables.items():
-            try:
-                if self._templar.is_template(v):
-                    variables[k] = self.expand_jinja2_template(v)
-            except AnsibleUndefinedVariable as e:
-                variables[k] = u"VARIABLE IS NOT DEFINED!"
-
-        return variables
-
-    def expand_jinja2_template(self, var):
-        expanded_var = self._templar.template(var, convert_bare=True,
-                                              fail_on_undefined=True)
-        if expanded_var == var:
-            if not isinstance(expanded_var, string_types):
-                raise AnsibleUndefinedVariable
-            expanded_var = self._templar.template("{{%s}}" % expanded_var,
-                                                  convert_bare=True,
-                                                  fail_on_undefined=True)
-        return expanded_var
-
-# Schemas
-
-
-def osd_objectstore_choices(value):
-    assert value in [
-        'bluestore', 'filestore'], "osd_objectstore must be either 'bluestore' or 'filestore'"  # noqa E501
-
-
-def ceph_origin_choices(value):
-    if not notario_store["containerized_deployment"]:
-        assert value in ['repository', 'distro',
-                         'local'], "ceph_origin must be either 'repository', 'distro' or 'local'"  # noqa E501
-
-
-def ceph_repository_choices(value):
-    msg = "ceph_repository must be either 'community', 'rhcs', 'dev', 'custom', 'uca' or 'obs'"
-    assert value in ['community', 'rhcs', 'dev', 'custom', 'uca', 'obs'], msg
-
-
-def ceph_repository_type_choices(value):
-    assert value in [
-        'cdn', 'iso'], "ceph_repository_type must be either 'cdn' or 'iso'"
-
-
-def validate_monitor_options(value):
-    """
-    Either monitor_address, monitor_address_block or monitor_interface must
-    be defined.
-    """
-    monitor_address_given = notario_store["monitor_address"] != "0.0.0.0"
-    monitor_address_block_given = notario_store["monitor_address_block"] != "subnet"  # noqa E501
-    monitor_interface_given = notario_store["monitor_interface"] != "interface"
-
-    msg = "Either monitor_address, monitor_address_block or monitor_interface must be provided"  # noqa E501
-
-    assert any([monitor_address_given, monitor_address_block_given,
-                monitor_interface_given]), msg
-
-
-def validate_dmcrypt_bool_value(value):
-    assert value in ["true", True, "false",
-                     False], "dmcrypt can be set to true/True or false/False (default)"
-
-
-def validate_osd_auto_discovery_bool_value(value):
-    assert value in ["true", True, "false",
-                     False], "osd_auto_discovery can be set to true/True or false/False (default)"
-
-
-def validate_objectstore(value):
-    assert value in [
-        "filestore", "bluestore"], "objectstore must be set to 'filestore' or 'bluestore'"  # noqa E501
-
-
-def validate_ceph_stable_release(value):
-    assert value in CEPH_RELEASES, "ceph_stable_release must be set to one of the following: %s" % ", ".join(  # noqa E501
-        CEPH_RELEASES)
-
-
-def validate_rados_options(value):
-    """
-    Either radosgw_interface, radosgw_address or radosgw_address_block must
-    be defined.
-    """
-    radosgw_address_given = notario_store["radosgw_address"] != "0.0.0.0"
-    radosgw_address_block_given = notario_store["radosgw_address_block"] != "subnet"
-    radosgw_interface_given = notario_store["radosgw_interface"] != "interface"
-
-    msg = "Either radosgw_address, radosgw_address_block or radosgw_interface must be provided"  # noqa E501
-
-    assert any([radosgw_address_given, radosgw_address_block_given,
-                radosgw_interface_given]), msg
-
-
-install_options = (
-    ("ceph_origin", ceph_origin_choices),
-    ("containerized_deployment", types.boolean),
-    ('osd_objectstore', osd_objectstore_choices),
-)
-
-ceph_origin_repository = ("ceph_repository", ceph_repository_choices)
-
-ceph_repository_community = (
-    ("ceph_mirror", types.string),
-    ("ceph_stable_key", types.string),
-    ("ceph_stable_release", validate_ceph_stable_release),
-    ("ceph_stable_repo", types.string),
-)
-
-ceph_repository_rhcs = (
-    ("ceph_repository_type", ceph_repository_type_choices),
-    ("ceph_rhcs_version", chainable.AnyIn(types.string, types.integer)),
-)
-
-ceph_repository_dev = (
-    ("ceph_dev_branch", types.string),
-    ("ceph_dev_sha1", types.string),
-)
-
-ceph_repository_obs = (
-    ("ansible_distribution", "openSUSE Leap"),
-    ("ceph_obs_repo", types.string),
-)
-
-ceph_repository_custom = (
-    ("ceph_custom_key", types.string),
-    ("ceph_custom_repo", types.string),
-)
-
-ceph_repository_uca = (
-    ("ceph_stable_openstack_release_uca", types.string),
-    ("ceph_stable_release_uca", types.string),
-    ("ceph_stable_repo_uca", types.string),
-)
-
-monitor_options = (
-    ("cluster_network", types.string),
-    ("fsid", types.string),
-    ("monitor_address", validate_monitor_options),
-    ("monitor_address_block", validate_monitor_options),
-    ("monitor_interface", validate_monitor_options),
-    ("public_network", types.string),
-)
-
-rados_options = (
-    ("radosgw_address", validate_rados_options),
-    ("radosgw_address_block", validate_rados_options),
-    ("radosgw_interface", validate_rados_options),
-)
-
-osd_options = (
-    (optional("dmcrypt"), validate_dmcrypt_bool_value),
-    (optional("osd_auto_discovery"), types.boolean),
-)
-
-lvm_batch_scenario = ("devices", iterables.AllItems(types.string))
-
-lvm_filestore_scenario = ("lvm_volumes", iterables.AllItems((
-    (optional('crush_device_class'), types.string),
-    ('data', types.string),
-    (optional('data_vg'), types.string),
-    ('journal', types.string),
-    (optional('journal_vg'), types.string),
-)))
-
-lvm_bluestore_scenario = ("lvm_volumes", iterables.AllItems((
-    (optional('crush_device_class'), types.string),
-    ('data', types.string),
-    (optional('data_vg'), types.string),
-    (optional('db'), types.string),
-    (optional('db_vg'), types.string),
-    (optional('wal'), types.string),
-    (optional('wal_vg'), types.string),
-)))
diff --git a/requirements.txt b/requirements.txt
index 60fa2b3b1..be1faebc3 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,4 +1,3 @@
 # These are Python requirements needed to run ceph-ansible master
-notario>=0.0.13
 ansible>=2.8,<2.9
 netaddr
diff --git a/roles/ceph-validate/tasks/main.yml b/roles/ceph-validate/tasks/main.yml
index 2a3b7f466..58620e803 100644
--- a/roles/ceph-validate/tasks/main.yml
+++ b/roles/ceph-validate/tasks/main.yml
@@ -1,7 +1,89 @@
 ---
-- name: validate provided configuration
-  validate:
-    mode: strict
+- name: validate ceph_origin
+  fail:
+    msg: "ceph_origin must be either 'repository', 'distro' or 'local'"
+  when:
+    - not containerized_deployment | bool
+    - ceph_origin not in ['repository', 'distro', 'local']
+
+- name: validate ceph_repository
+  fail:
+    msg: "ceph_repository must be either 'community', 'rhcs', 'dev', 'custom' or 'uca'"
+  when:
+    - ceph_origin == 'repository'
+    - ceph_repository not in ['community', 'rhcs', 'dev', 'custom', 'uca']
+
+- name: validate ceph_repository_community
+  fail:
+    msg: "ceph_stable_release must be either 'nautilus' or 'octopus'"
+  when:
+    - ceph_origin == 'repository'
+    - ceph_repository == 'community'
+    - ceph_stable_release not in ['nautilus', 'octopus']
+
+- name: validate ceph_repository_type
+  fail:
+    msg: "ceph_repository_type must be either 'cdn' or 'iso'"
+  when:
+    - ceph_origin == 'repository'
+    - ceph_repository == 'rhcs'
+    - ceph_repository_type not in ['cdn', 'iso']
+
+- name: validate osd_objectstore
+  fail:
+    msg: "osd_objectstore must be either 'bluestore' or 'filestore'"
+  when: osd_objectstore not in ['bluestore', 'filestore']
+
+- name: validate monitor network configuration
+  fail:
+    msg: "Either monitor_address, monitor_address_block or monitor_interface must be provided"
+  when:
+    - mon_group_name in group_names
+    - monitor_address == '0.0.0.0'
+    - monitor_address_block == 'subnet'
+    - monitor_interface == 'interface'
+
+- name: validate radosgw network configuration
+  fail:
+    msg: "Either radosgw_address, radosgw_address_block or radosgw_interface must be provided"
+  when:
+    - rgw_group_name in group_names
+    - radosgw_address == '0.0.0.0'
+    - radosgw_address_block == 'subnet'
+    - radosgw_interface == 'interface'
+
+- name: validate osd nodes
+  when: osd_group_name in group_names
+  block:
+    - name: validate lvm osd scenario
+      fail:
+        msg: 'devices or lvm_volumes must be defined for lvm osd scenario'
+      when:
+        - not osd_auto_discovery | default(false) | bool
+        - devices is undefined
+        - lvm_volumes is undefined
+
+    - name: validate filestore lvm osd scenario
+      fail:
+        msg: 'data and journal keys must be defined in lvm_volumes'
+      when:
+        - osd_objectstore == 'filestore'
+        - not osd_auto_discovery | default(false) | bool
+        - lvm_volumes is defined
+        - lvm_volumes | length > 0
+        - item.data is undefined or item.journal is undefined
+      with_items: '{{ lvm_volumes }}'
+
+    - name: validate bluestore lvm osd scenario
+      fail:
+        msg: 'data key must be defined in lvm_volumes'
+      when:
+        - osd_objectstore == 'bluestore'
+        - not osd_auto_discovery | default(false) | bool
+        - lvm_volumes is defined
+        - lvm_volumes | length > 0
+        - item.data is undefined
+      with_items: '{{ lvm_volumes }}'
 
 - name: warning deprecation for fqdn configuration
   fail:
diff --git a/tests/requirements.txt b/tests/requirements.txt
index ade745990..ba3842b42 100644
--- a/tests/requirements.txt
+++ b/tests/requirements.txt
@@ -3,7 +3,6 @@ six==1.10.0
 testinfra>=3.0,<3.1
 pytest-xdist==1.28.0
 pytest>=4.4,<4.5
-notario>=0.0.13
 ansible>=2.8,<2.9
 Jinja2>=2.10
 netaddr