Prevent execution with incompatible yamllint configuration (#4139)

.github/workflows/tox.yml

@@ -73,7 +73,7 @@ jobs:
     env:
       # Number of expected test passes, safety measure for accidental skip of
       # tests. Update value if you add/remove tests.
-      PYTEST_REQPASS: 857
+      PYTEST_REQPASS: 858
     steps:
       - uses: actions/checkout@v4
         with:

.vscode/settings.json

@@ -25,7 +25,7 @@
     "python.testing.unittestEnabled": false,
     "mypy-type-checker.severity": {
         "error": "Warning",
-      },
+    },
     "sortLines.filterBlankLines": true,
     "yaml.completion": true,
     "yaml.customTags": [
@@ -39,6 +39,8 @@
         "editor.codeActionsOnSave": {
             "source.organizeImports": "explicit",
             "source.fixAll": "explicit"
-        }
+        },
+        "editor.defaultFormatter": "ms-python.black-formatter",
+        "editor.formatOnSave": true
     }
 }

.yamllint

@@ -1,16 +1,25 @@
 ---
 rules:
+  braces:
+    min-spaces-inside: 0
+    max-spaces-inside: 1
   comments:
     # prettier compatibility
     min-spaces-from-content: 1
+  comments-indentation: false
   document-start:
     present: true
+  key-duplicates:
+    forbid-duplicated-merge-keys: true
   indentation:
     level: error
     indent-sequences: consistent
   octal-values:
     forbid-implicit-octal: true
     forbid-explicit-octal: true
+  # quoted-strings:
+  #   quote-type: double
+  #   required: only-when-needed
 ignore: |
   .tox
   examples/playbooks/example.yml

examples/yamllint/incompatible-config/.yamllint

@@ -0,0 +1,14 @@
+# This config file is full of yamllint configuration settings that are
+# incompatible with ansible-lint. It used for testing their detection.
+rules:
+  comments:
+    min-spaces-from-content: 2
+  comments-indentation: false
+  braces:
+    min-spaces-inside: 1
+    max-spaces-inside: 2
+  key-duplicates:
+    forbid-duplicated-merge-keys: false
+  octal-values:
+    forbid-implicit-octal: false
+    forbid-explicit-octal: false

src/ansiblelint/data/.yamllint

@@ -0,0 +1,25 @@
+extends: default
+rules:
+  comments:
+    # https://github.com/prettier/prettier/issues/6780
+    min-spaces-from-content: 1
+  # https://github.com/adrienverge/yamllint/issues/384
+  comments-indentation: false
+  document-start: disable
+  # 160 chars was the default used by old E204 rule, but
+  # you can easily change it or disable in your .yamllint file.
+  line-length:
+    max: 160
+  # We are adding an extra space inside braces as that's how prettier does it
+  # and we are trying not to fight other linters.
+  braces:
+    min-spaces-inside: 0 # yamllint defaults to 0
+    max-spaces-inside: 1 # yamllint defaults to 0
+  # key-duplicates:
+  #   forbid-duplicated-merge-keys: true # not enabled by default
+  octal-values:
+    forbid-implicit-octal: true # yamllint defaults to false
+    forbid-explicit-octal: true # yamllint defaults to false
+  # quoted-strings:
+  #   quote-type: double
+  #   required: only-when-needed

src/ansiblelint/rules/yaml.md

@@ -1,24 +1,8 @@
 # yaml
 
-This rule checks YAML syntax by using [yamllint] but with few minor default
-configuration changes.
-
-!!! warning
-
-    [Auto-fix](../autofix.md) functionality will change **inline comment indentation to one
-    character instead of two**, which is the default of [yamllint]. The reason
-    for this decision is for keeping reformatting compatibility
-    with [prettier], which is the most popular reformatter.
-
-    ```yaml title=".yamllint"
-    rules:
-      comments:
-        min-spaces-from-content: 1 # prettier compatibility
-    ```
-
-    There is no need to create this yamllint config file, but if you also
-    run yamllint yourself, you might want to create it to make it behave
-    the same way as ansible-lint.
+This rule checks YAML syntax by using [yamllint] library but with a
+[specific default configuration](#yamllint-configuration), one that is
+compatible with both, our internal reformatter (`--fix`) and also [prettier].
 
 You can disable YAML syntax violations by adding `yaml` to the `skip_list` in
 your Ansible-lint configuration as follows:
@@ -93,8 +77,10 @@ precedence over our defaults.
 
 ## Additional Information for Multiline Strings
 
-Adhering to yaml[line-length] rule, for writing multiline strings we recommend using Block Style Indicator: literal style indicated by a pipe (|) or folded style indicated by a right angle bracket (>), instead of escaping the newlines with backslashes.
-Reference [guide] for writing multiple line strings in yaml.
+Adhering to yaml[line-length] rule, for writing multiline strings we recommend
+using Block Style Indicator: literal style indicated by a pipe (|) or folded
+style indicated by a right angle bracket (>), instead of escaping the newlines
+with backslashes. Reference [guide] for writing multiple line strings in yaml.
 
 ## Problematic code
 
@@ -115,10 +101,53 @@ foo2: "0o777" # <-- Explicitly quoting octal is less risky.
 bar: ... # Correct comment indentation.
 ```
 
+## Yamllint configuration
+
+If you decide to add a custom yamllint config to your project, ansible-lint
+might refuse to run if it detects that some of your options are incompatible and
+ask you to correct them. When this happens, you will see a message like the one
+below:
+
+```
+CRITICAL Found incompatible custom yamllint configuration (.yamllint), please either remove the file or edit it to comply with:
+  - comments.min-spaces-from-content must be 1
+  - braces.min-spaces-inside must be 0
+  - braces.max-spaces-inside must be 1
+  - octal-values.forbid-implicit-octal must be true
+  - octal-values.forbid-explicit-octal must be true
+
+Read https://ansible.readthedocs.io/projects/lint/rules/yaml/ for more details regarding why we have these requirements.
+```
+
+!!! warning
+
+    [Auto-fix](../autofix.md) functionality will change **inline comment indentation to one
+    character instead of two**, which is the default of [yamllint]. The reason
+    for this decision was to keep reformatting compatibility
+    with [prettier], which is the most popular reformatter.
+
+    ```yaml title=".yamllint"
+    rules:
+      comments:
+        min-spaces-from-content: 1 # prettier compatibility
+    ```
+
+    There is no need to create this yamllint config file, but if you also
+    run yamllint yourself, you might want to create it to make it behave
+    the same way as ansible-lint.
+
+Below you can find the default yamllint configuration that our linter will use
+when there is no custom file present.
+
+```yaml
+{!../src/ansiblelint/data/.yamllint!}
+```
+
 [1.1]: https://yaml.org/spec/1.1/
 [1.2.0]: https://yaml.org/spec/1.2.0/
 [1.2.2]: https://yaml.org/spec/1.2.2/
 [yaml specification]: https://yaml.org/
-[guide]: https://docs.ansible.com/ansible/latest/reference_appendices/YAMLSyntax.html#yaml-basics
+[guide]:
+  https://docs.ansible.com/ansible/latest/reference_appendices/YAMLSyntax.html#yaml-basics
 [prettier]: https://prettier.io/
 [yamllint]: https://yamllint.readthedocs.io/en/stable/

src/ansiblelint/yaml_utils.py

@@ -7,6 +7,7 @@
 import logging
 import os
 import re
+import sys
 from collections.abc import Callable, Iterator, Sequence
 from io import StringIO
 from pathlib import Path
@@ -30,6 +31,7 @@
     ANNOTATION_KEYS,
     NESTED_TASK_KEYS,
     PLAYBOOK_TASK_KEYWORDS,
+    RC,
 )
 from ansiblelint.utils import Task
 
@@ -46,30 +48,6 @@
 _logger = logging.getLogger(__name__)
 
 
-YAMLLINT_CONFIG = """
-extends: default
-rules:
-  comments:
-    # https://github.com/prettier/prettier/issues/6780
-    min-spaces-from-content: 1
-  # https://github.com/adrienverge/yamllint/issues/384
-  comments-indentation: false
-  document-start: disable
-  # 160 chars was the default used by old E204 rule, but
-  # you can easily change it or disable in your .yamllint file.
-  line-length:
-    max: 160
-  # We are adding an extra space inside braces as that's how prettier does it
-  # and we are trying not to fight other linters.
-  braces:
-    min-spaces-inside: 0  # yamllint defaults to 0
-    max-spaces-inside: 1  # yamllint defaults to 0
-  octal-values:
-    forbid-implicit-octal: true  # yamllint defaults to false
-    forbid-explicit-octal: true  # yamllint defaults to false
-"""
-
-
 def deannotate(data: Any) -> Any:
     """Remove our annotations like __file__ and __line__ and return a JSON serializable object."""
     if isinstance(data, dict):
@@ -85,10 +63,9 @@ def deannotate(data: Any) -> Any:
     return data
 
 
-@functools.lru_cache(maxsize=1)
 def load_yamllint_config() -> YamlLintConfig:
     """Load our default yamllint config and any customized override file."""
-    config = YamlLintConfig(content=YAMLLINT_CONFIG)
+    config = YamlLintConfig(file=Path(__file__).parent / "data" / ".yamllint")
     # if we detect local yamllint config we use it but raise a warning
     # as this is likely to get out of sync with our internal config.
     for path in [
@@ -105,10 +82,66 @@ def load_yamllint_config() -> YamlLintConfig:
                 "internal yamllint config.",
                 file,
             )
-            config_override = YamlLintConfig(file=str(file))
-            config_override.extend(config)
-            config = config_override
+            custom_config = YamlLintConfig(file=str(file))
+            custom_config.extend(config)
+            config = custom_config
             break
+
+    # Look for settings incompatible with our reformatting
+    checks: list[tuple[str, str | int | bool]] = [
+        (
+            "comments.min-spaces-from-content",
+            1,
+        ),
+        (
+            "comments-indentation",
+            False,
+        ),
+        (
+            "braces.min-spaces-inside",
+            0,
+        ),
+        (
+            "braces.max-spaces-inside",
+            1,
+        ),
+        (
+            "octal-values.forbid-implicit-octal",
+            True,
+        ),
+        (
+            "octal-values.forbid-explicit-octal",
+            True,
+        ),
+        # (
+        #     "key-duplicates.forbid-duplicated-merge-keys", # v1.34.0+
+        #     True,
+        # ),
+        # (
+        #   "quoted-strings.quote-type", "double",
+        # ),
+        # (
+        #   "quoted-strings.required", "only-when-needed",
+        # ),
+    ]
+    errors = []
+    for setting, expected_value in checks:
+        v = config.rules
+        for key in setting.split("."):
+            if not isinstance(v, dict):  # pragma: no cover
+                break
+            if key not in v:  # pragma: no cover
+                break
+            v = v[key]
+        if v != expected_value:
+            msg = f"{setting} must be {str(expected_value).lower()}"
+            errors.append(msg)
+    if errors:
+        nl = "\n"
+        msg = f"Found incompatible custom yamllint configuration ({file}), please either remove the file or edit it to comply with:{nl}  - {nl + '  - '.join(errors)}.{nl}{nl}Read https://ansible.readthedocs.io/projects/lint/rules/yaml/ for more details regarding why we have these requirements."
+        logging.fatal(msg)
+        sys.exit(RC.INVALID_CONFIG)
+
     _logger.debug("Effective yamllint rules used: %s", config.rules)
     return config
 

test/test_yaml_utils.py

@@ -1,5 +1,6 @@
 """Tests for yaml-related utility functions."""
 
+# pylint: disable=too-many-lines
 from __future__ import annotations
 
 from io import StringIO
@@ -11,7 +12,8 @@
 from yamllint.linter import run as run_yamllint
 
 import ansiblelint.yaml_utils
-from ansiblelint.file_utils import Lintable
+from ansiblelint.constants import RC
+from ansiblelint.file_utils import Lintable, cwd
 from ansiblelint.utils import task_in_list
 
 if TYPE_CHECKING:
@@ -989,3 +991,12 @@ def test_deannotate(
 ) -> None:
     """Ensure deannotate works as intended."""
     assert ansiblelint.yaml_utils.deannotate(before) == after
+
+
+def test_yamllint_incompatible_config() -> None:
+    """Ensure we can detect incompatible yamllint settings."""
+    with (
+        cwd(Path("examples/yamllint/incompatible-config")),
+        pytest.raises(SystemExit, match=f"^{RC.INVALID_CONFIG}$"),
+    ):
+        ansiblelint.yaml_utils.load_yamllint_config()