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

feat: Add nested dictionary formating check #284

Merged
merged 1 commit into from
Jul 23, 2022
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: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ All notable changes in **salt-lint** are documented below.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]
### Added
- Rule 219 for catching missing over-indentation of nested dicts ([#284](https://github.com/warpnet/salt-lint/pull/284)).

## [0.8.0] (2021-11-09)
### Fixed
Expand Down
53 changes: 53 additions & 0 deletions docs/rules/formatting.md
Original file line number Diff line number Diff line change
Expand Up @@ -345,3 +345,56 @@ As described by the [official SaltStack documentation](https://docs.saltstack.co
> The initial implementation of top.sls and Include declaration followed the python import model where a slash is represented as a period. This means that a SLS file with a period in the name ( besides the suffix period) can not be referenced. For example, webserver_1.0.sls is not referenceable because webserver_1.0 would refer to the directory/file webserver_1/0.sls
>
> The same applies for any subdirectories, this is especially 'tricky' when git repos are created. Another command that typically can't render it's output is `state.show_sls` of a file in a path that contains a dot.

___

## 219

**Nested dictionaries should be over-indented**

As described by the [official SaltStack documentation](https://docs.saltproject.io/en/latest/topics/troubleshooting/yaml_idiosyncrasies.html#nested-dictionaries):

> When dictionaries are nested within other data structures (particularly lists), the indentation logic sometimes changes. Examples of where this might happen include context and default options

```yaml
/etc/http/conf/http.conf:
file:
- managed
- source: salt://apache/http.conf
- user: root
- group: root
- mode: 644
- template: jinja
- context:
custom_var: "override"
- defaults:
custom_var: "default value"
other_var: 123
```
> Notice that while the indentation is two spaces per level, for the values under the context and defaults options there is a four-space indent. If only two spaces are used to indent, then those keys will be considered part of the same dictionary that contains the context key, and so the data will not be loaded correctly.

### Problematic code
```yaml
/etc/http/conf/http.conf:
file.managed:
- source: salt://apache/http.conf
- template: jinja
- context:
custom_var: "override"
- defaults:
custom_var: "default value"
other_var: 123
```

### Correct code
```yaml
/etc/http/conf/http.conf:
file.managed:
- source: salt://apache/http.conf
- template: jinja
- context:
custom_var: "override"
- defaults:
custom_var: "default value"
other_var: 123
```
1 change: 1 addition & 0 deletions docs/rules/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ Rule | Description
[212](formatting/#212) | Most files should not contain irregular spaces
[213](formatting/#213) | SaltStack recommends using `cmd.run` together with `onchanges`, rather than `cmd.wait`
[214](formatting/#214) | SLS file with a period in the name (besides the suffix period) can not be referenced
[219](formatting/#219) | Nested dicts should be properly over-indented

___

Expand Down
45 changes: 45 additions & 0 deletions saltlint/rules/NestedDictRule.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# -*- coding: utf-8 -*-
# Copyright (c) 2020 Warpnet B.V.

import re
from saltlint.linter.rule import Rule
from saltlint.utils import get_rule_skips_from_text
from saltlint.utils import LANGUAGE_SLS


class NestedDictRule(Rule):
id = "219"
shortdesc = "Nested dictionaries (in context or defaults) should be over-indented"
description = "Nested dictionaries (in context or defaults) should be over-indented"

severity = "HIGH"
languages = [LANGUAGE_SLS]
tags = ["formatting"]
version_added = "develop"

regex = re.compile(
r"^(\s+)-\s+(context|defaults):[^{[\n]*\n^\1\s{0,3}[^-{[\s]*:\s.+",
re.MULTILINE,
)

def matchtext(self, file, text):
results = []

for match in re.finditer(self.regex, text):
# Get the location of the regex match
start = match.start()
end = match.end()

# Get the line number of the last character
lines = text[:end].splitlines()
line_no = len(lines)

# Skip result if noqa for this rule ID is found in section
section = text[start:end]
if self.id in get_rule_skips_from_text(section):
continue

# Append the match to the results
results.append((line_no, lines[-1], self.shortdesc))

return results
118 changes: 118 additions & 0 deletions tests/unit/TestNestedDictRule.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
# -*- coding: utf-8 -*-
# Copyright (c) 2020 Warpnet B.V.

import unittest

from saltlint.linter.collection import RulesCollection
from saltlint.rules.NestedDictRule import NestedDictRule
from tests import RunFromText


GOOD_NESTED_DICT_STATE = """
/etc/http/conf/http.conf:
file.managed:
- source: salt://apache/http.conf
- template: jinja
- context:
custom_var: "override"
- defaults:
custom_var: "default value"
other_var: 123

sblaisot marked this conversation as resolved.
Show resolved Hide resolved
/etc/http/conf/http.conf:
file.managed:
- source: salt://apache/http.conf
- mode: 644
- template: jinja
- context: { custom_var: "override" }
- defaults: {
custom_var: "default value",
other_var: 123
}


/etc/http/conf/http.conf:
file.managed:
- source: salt://apache/http.conf
- template: jinja
- context:
custom_var: "override"
- defaults:
custom_var: "default value"
other_var: 123

/etc/http/conf/http.conf:
file.managed:
- source: salt://apache/http.conf
- template: jinja
- context:
custom_var: "override" # noqa: 219
- defaults:
custom_var: "default value"
other_var: 123

/etc/http/conf/http.conf:
file.managed:
- source: salt://apache/http.conf
- template: jinja
- context: # noqa: 219
custom_var: "override"
- defaults:
custom_var: "default value"
other_var: 123
"""

BAD_NESTED_DICT_STATE = """
/etc/http/conf/http.conf:
file.managed:
- source: salt://apache/http.conf
- template: jinja
- context:
custom_var: "override"
- defaults:
custom_var: "default value"
other_var: 123

/etc/http/conf/http.conf:
file.managed:
- source: salt://apache/http.conf
- template: jinja
- context:
custom_var: "override"
- defaults:
custom_var: "default value"
other_var: 123

/etc/http/conf/http.conf:
file.managed:
- source: salt://apache/http.conf
- template: jinja
- context:
custom_var: "override"
- defaults:
custom_var: "default value"
other_var: 123
"""


class TestNestedDictRule(unittest.TestCase):
collection = RulesCollection()

def setUp(self):
self.collection.register(NestedDictRule())

def test_statement_positive(self):
runner = RunFromText(self.collection)
results = runner.run_state(GOOD_NESTED_DICT_STATE)
self.assertEqual(0, len(results))

def test_statement_negative(self):
runner = RunFromText(self.collection)
results = runner.run_state(BAD_NESTED_DICT_STATE)
self.assertEqual(4, len(results))

# Check line numbers of the results
self.assertEqual(7, results[0].linenumber)
self.assertEqual(19, results[1].linenumber)
self.assertEqual(27, results[2].linenumber)
self.assertEqual(29, results[3].linenumber)