Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Prevent execution with incompatible yamllint configuration #4139

Merged
merged 1 commit into from
May 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/tox.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
6 changes: 4 additions & 2 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
"python.testing.unittestEnabled": false,
"mypy-type-checker.severity": {
"error": "Warning",
},
},
"sortLines.filterBlankLines": true,
"yaml.completion": true,
"yaml.customTags": [
Expand All @@ -39,6 +39,8 @@
"editor.codeActionsOnSave": {
"source.organizeImports": "explicit",
"source.fixAll": "explicit"
}
},
"editor.defaultFormatter": "ms-python.black-formatter",
"editor.formatOnSave": true
}
}
9 changes: 9 additions & 0 deletions .yamllint
Original file line number Diff line number Diff line change
@@ -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
Expand Down
14 changes: 14 additions & 0 deletions examples/yamllint/incompatible-config/.yamllint
Original file line number Diff line number Diff line change
@@ -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
25 changes: 25 additions & 0 deletions src/ansiblelint/data/.yamllint
Original file line number Diff line number Diff line change
@@ -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
73 changes: 51 additions & 22 deletions src/ansiblelint/rules/yaml.md
Original file line number Diff line number Diff line change
@@ -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:
Expand Down Expand Up @@ -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

Expand All @@ -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/
91 changes: 62 additions & 29 deletions src/ansiblelint/yaml_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -30,6 +31,7 @@
ANNOTATION_KEYS,
NESTED_TASK_KEYS,
PLAYBOOK_TASK_KEYWORDS,
RC,
)
from ansiblelint.utils import Task

Expand All @@ -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):
Expand All @@ -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 [
Expand All @@ -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

Expand Down
13 changes: 12 additions & 1 deletion test/test_yaml_utils.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""Tests for yaml-related utility functions."""

# pylint: disable=too-many-lines
from __future__ import annotations

from io import StringIO
Expand All @@ -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:
Expand Down Expand Up @@ -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()
Loading