Merge pull request #2513 from ssbarnea/fix/delinting

Refactored molecule linting

Reviewed-by: https://github.com/apps/ansible-zuul

docs/ci.rst

@@ -298,8 +298,6 @@ conflict.
       name: galaxy
     driver:
       name: docker
-    lint:
-      name: yamllint
     platforms:
       - name: instance1-$TOX_ENVNAME
         image: mariadb
@@ -309,12 +307,8 @@ conflict.
         command: /usr/sbin/init
     provisioner:
       name: ansible
-      lint:
-        name: ansible-lint
     verifier:
       name: testinfra
-      lint:
-        name: flake8
 
 .. _`GitHub Actions`: https://github.com/features/actions
 .. _`Factors`: http://tox.readthedocs.io/en/latest/config.html#factors-and-factor-conditional-settings

docs/configuration.rst

@@ -83,18 +83,49 @@ Podman
     :undoc-members:
 
 
-.. _linters:
+.. _lint:
 
 Lint
 ----
 
-Molecule handles project linting by invoking configurable linters.
+Starting with v3, Molecule handles project linting by invoking and external
+lint commands as exemplified below.
 
-Yaml Lint
-^^^^^^^^^
+The decision to remove the complex linting support was not easily taken as we
+do find it very useful. The issue was that molecule runs on scenarios and
+linting is usually performed at repository level.
+
+It makes little sense to perform linting in more than one place per project.
+Molecule was able to use up to three linters and while it was aimed to flexible
+about them, it ended up creating more confusions to the users. We decided to
+maximize flexibility by just calling an external shell command.
+
+.. code-block:: yaml
+
+    lint: |
+        yamllint .
+        ansible-lint
+        flake8
+
+The older format is no longer supported and you have to update the
+``molecule.yml`` when you upgrade. If you don't want to do any linting,
+it will be enough to remove all lint related sections from the file.
+
+.. code-block:: yaml
+
+    # old v2 format, no longer supported
+    lint:
+      name: yamllint
+      enabled: true
+    provisioner:
+      lint:
+        name: ansible-lint
+      options: ...
+      env: ...
+    verifier:
+      lint:
+        name: flake8
 
-.. autoclass:: molecule.lint.yamllint.Yamllint()
-   :undoc-members:
 
 .. _platforms:
 
@@ -117,14 +148,6 @@ Ansible
 .. autoclass:: molecule.provisioner.ansible.Ansible()
    :undoc-members:
 
-Lint
-....
-
-Molecule handles provisioner linting by invoking configurable linters.
-
-.. autoclass:: molecule.provisioner.lint.ansible_lint.AnsibleLint()
-   :undoc-members:
-
 .. _root_scenario:
 
 Scenario
@@ -157,25 +180,9 @@ Ansible
 .. autoclass:: molecule.verifier.ansible.Ansible()
    :undoc-members:
 
-Lint
-....
-
-.. autoclass:: molecule.verifier.lint.ansible_lint.AnsibleLint()
-   :undoc-members:
 
 Testinfra
 ^^^^^^^^^
 
 .. autoclass:: molecule.verifier.testinfra.Testinfra()
    :undoc-members:
-
-.. _root_lint:
-
-Lint
-....
-
-.. autoclass:: molecule.verifier.lint.flake8.Flake8()
-   :undoc-members:
-
-.. autoclass:: molecule.verifier.lint.precommit.PreCommit()
-   :undoc-members:

docs/examples.rst

@@ -281,8 +281,6 @@ lives in a shared location and ``molecule.yml`` is points to the shared tests.
     verifier:
     name: testinfra
     directory: ../resources/tests/
-    lint:
-      name: flake8
 
 .. _parallel-usage-example:
 

docs/getting-started.rst

@@ -103,9 +103,8 @@ keys represent the high level components that Molecule provides. These are:
 * The :ref:`driver` provider. Molecule uses `Docker`_ by default. Molecule uses
   the driver to delegate the task of creating instances.
 
-* The :ref:`linters` provider. Molecule uses :std:doc:`Yamllint
-  <yamllint:index>` by default to ensure that best practices are encouraged
-  when writing YAML.
+* The :ref:`lint` command. Molecule can call external commands to ensure
+  that best practices are encouraged.
 
 * The :ref:`platforms` definitions. Molecule relies on this to know which
   instances to create, name and to which group each instance belongs. If you

molecule/command/init/role.py

@@ -152,7 +152,5 @@ def role(
         'verifier_name': verifier_name,
     }
 
-    command_args['verifier_lint_name'] = api.verifiers()[verifier_name].default_linter
-
     r = Role(command_args)
     r.execute()

molecule/command/init/scenario.py

@@ -227,8 +227,6 @@ def scenario(
         'verifier_name': verifier_name,
     }
 
-    command_args['verifier_lint_name'] = api.verifiers()[verifier_name].default_linter
-
     driver_template = driver_template or os.environ.get(
         'MOLECULE_SCENARIO_DRIVER_TEMPLATE', None
     )

molecule/command/lint.py

@@ -23,13 +23,19 @@
 
 from molecule import logger
 from molecule.command import base
+from molecule import util
+
 
 LOG = logger.get_logger(__name__)
 
 
 class Lint(base.Base):
     """
-    Lint Command Class.
+    Lint command executes external linters.
+
+    You need to remember to install those linters. For convenience, there is a
+    package extra that installs the most common ones, use it like
+    ``pip install "molecule[lint]"``.
 
     .. program:: molecule lint
 
@@ -71,18 +77,25 @@ def execute(self):
         :return: None
         """
         self.print_info()
-        linters = [
-            l
-            for l in [
-                self._config.lint,
-                self._config.verifier.lint,
-                self._config.provisioner.lint,
-            ]
-            if l
-        ]
-
-        for l in linters:
-            l.execute()
+
+        # v3 migration code:
+        cmd = self._config.lint
+        if not cmd:
+            LOG.info("Lint is disabled.")
+            return
+
+        if cmd == 'yamllint':
+            msg = (
+                "Deprecated linter config found, migrate to v3 schema. "
+                "See https://github.com/ansible-community/molecule/issues/2293"
+            )
+            util.sysexit_with_message(msg)
+
+        try:
+            LOG.info("Executing: %s" % cmd)
+            util.run(cmd, shell=True, universal_newlines=True, check=True)
+        except Exception as e:
+            util.sysexit_with_message("Lint failed: %s: %s" % (e, e))
 
 
 @base.click_command_ex()

molecule/command/list.py

@@ -35,11 +35,7 @@
 
 class List(base.Base):
     """
-    Lint command executes external linters.
-
-    You need to remember to install those linters. For concenience, there is a
-    package extra that installs the most common ones, use it like
-    ``pip install "molecule[extra]"``.
+    List command shows information about current scenarios.
 
     .. program:: molecule list
 

molecule/config.py

@@ -35,8 +35,7 @@
 from molecule.dependency import ansible_galaxy
 from molecule.dependency import gilt
 from molecule.dependency import shell
-from molecule.lint import yamllint
-from molecule.model import schema_v2
+from molecule.model import schema_v3
 from molecule.provisioner import ansible
 
 
@@ -71,7 +70,7 @@ class Config(object):
     directory.  Molecule performs most functions within this directory.
 
     The :class:`.Config` object instantiates Dependency_, Driver_,
-    :ref:`root_lint`, Platforms_, Provisioner_, Verifier_,
+    :ref:`lint`, Platforms_, Provisioner_, Verifier_,
     :ref:`root_scenario`, and State_ references.
     """
 
@@ -181,21 +180,17 @@ def env(self):
             'MOLECULE_INSTANCE_CONFIG': self.driver.instance_config,
             'MOLECULE_DEPENDENCY_NAME': self.dependency.name,
             'MOLECULE_DRIVER_NAME': self.driver.name,
-            'MOLECULE_LINT_NAME': self.lint.name,
             'MOLECULE_PROVISIONER_NAME': self.provisioner.name,
-            'MOLECULE_PROVISIONER_LINT_NAME': self.provisioner.lint.name,
             'MOLECULE_SCENARIO_NAME': self.scenario.name,
             'MOLECULE_VERIFIER_NAME': self.verifier.name,
-            'MOLECULE_VERIFIER_LINT_NAME': self.verifier.lint.name,
             'MOLECULE_VERIFIER_TEST_DIRECTORY': self.verifier.directory,
         }
 
     @property
     @util.lru_cache()
     def lint(self):
-        lint_name = self.config['lint']['name']
-        if lint_name == 'yamllint':
-            return yamllint.Yamllint(self)
+        lint_name = self.config.get('lint', None)
+        return lint_name
 
     @property
     @util.lru_cache()
@@ -340,7 +335,6 @@ def _get_defaults(self):
                 'ssh_connection_options': [],
                 'safe_files': [],
             },
-            'lint': {'name': 'yamllint', 'enabled': True, 'options': {}, 'env': {}},
             'platforms': [],
             'provisioner': {
                 'name': 'ansible',
@@ -413,14 +407,13 @@ def _get_defaults(self):
                 'options': {},
                 'env': {},
                 'additional_files_or_dirs': [],
-                'lint': {'name': 'flake8', 'enabled': True, 'options': {}, 'env': {}},
             },
         }
 
     def _preflight(self, data):
         env = os.environ.copy()
         env = set_env_from_file(env, self.env_file)
-        errors, data = schema_v2.pre_validate(data, env, MOLECULE_KEEP_STRING)
+        errors, data = schema_v3.pre_validate(data, env, MOLECULE_KEEP_STRING)
         if errors:
             msg = "Failed to pre-validate.\n\n{}".format(errors)
             util.sysexit_with_message(msg, detail=data)
@@ -429,7 +422,7 @@ def _validate(self):
         msg = 'Validating schema {}.'.format(self.molecule_file)
         LOG.info(msg)
 
-        errors = schema_v2.validate(self.config)
+        errors = schema_v3.validate(self.config)
         if errors:
             msg = "Failed to validate.\n\n{}".format(errors)
             util.sysexit_with_message(msg)

molecule/cookiecutter/molecule/cookiecutter.json

@@ -2,11 +2,8 @@
     "molecule_directory": "molecule",
     "dependency_name": "OVERRIDDEN",
     "driver_name": "OVERRIDDEN",
-    "lint_name": "OVERRIDDEN",
     "provisioner_name": "OVERRIDDEN",
-    "provisioner_lint_name": "ansible-lint",
     "scenario_name": "OVERRIDDEN",
     "role_name": "OVERRIDDEN",
-    "verifier_name": "OVERRIDDEN",
-    "verifier_lint_name": "flake8"
+    "verifier_name": "OVERRIDDEN"
 }

molecule/cookiecutter/molecule/{{cookiecutter.role_name}}/{{cookiecutter.molecule_directory}}/{{cookiecutter.scenario_name}}/molecule.yml

@@ -3,8 +3,6 @@ dependency:
   name: {{ cookiecutter.dependency_name }}
 driver:
   name: {{ cookiecutter.driver_name }}
-lint:
-  name: {{ cookiecutter.lint_name }}
 platforms:
 {%- if cookiecutter.driver_name == 'docker' %}
   - name: instance
@@ -19,10 +17,5 @@ platforms:
 {%- endif %}
 provisioner:
   name: {{ cookiecutter.provisioner_name }}
-  lint:
-    name: {{ cookiecutter.provisioner_lint_name }}
-    enabled: false
 verifier:
   name: {{ cookiecutter.verifier_name }}
-  lint:
-    name: {{ cookiecutter.verifier_lint_name }}

molecule/lint/base.py

@@ -72,18 +72,16 @@ def name(self):
 
         :returns: str
         """
-        return self._config.config['lint']['name']
+        return self._config.config['lint']
 
     @property
     def enabled(self):
-        return self._config.config['lint']['enabled']
+        return self._config.config['lint']
 
     @property
     def options(self):
-        return util.merge_dicts(
-            self.default_options, self._config.config['lint']['options']
-        )
+        return self.default_options
 
     @property
     def env(self):
-        return util.merge_dicts(self.default_env, self._config.config['lint']['env'])
+        return util.merge_dicts(self.default_env)