-
Notifications
You must be signed in to change notification settings - Fork 791
yamllint: cover all changed files ending in yaml, yml or fmf #14601
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
base: master
Are you sure you want to change the base?
Changes from all commits
37ef4fb
975e2d6
a4e4e59
f4ee494
02bffa8
57094c2
5fa35ca
79b309d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,10 +3,10 @@ on: | |
| pull_request: | ||
| branches: [master, 'stabilization*'] | ||
| permissions: | ||
| contents: read | ||
| contents: read | ||
| jobs: | ||
| yamllint: | ||
| name: Yaml Lint on Changed Controls and Profiles Files | ||
| name: Yaml Lint on Changed yaml files | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Install Git | ||
|
|
@@ -27,37 +27,41 @@ jobs: | |
| url="repos/$repo/pulls/$pr_number/files" | ||
| response=$(gh api "$url" --paginate) | ||
| echo "$response" | jq -r '.[].filename' > filenames.txt | ||
| cat filenames.txt | ||
|
|
||
| if grep -q "controls/" filenames.txt; then | ||
| echo "CONTROLS_CHANGES=true" >> $GITHUB_ENV | ||
| else | ||
| echo "CONTROLS_CHANGES=false" >> $GITHUB_ENV | ||
| fi | ||
| if grep -q "\.profile" filenames.txt; then | ||
| echo "PROFILES_CHANGES=true" >> $GITHUB_ENV | ||
| else | ||
| echo "PROFILES_CHANGES=false" >> $GITHUB_ENV | ||
| fi | ||
| env: | ||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
|
|
||
| - name: Install yamllint | ||
| if: ${{ env.CONTROLS_CHANGES == 'true' || env.PROFILES_CHANGES == 'true' }} | ||
| run: pip install yamllint | ||
|
|
||
| - name: Run yamllint in Control Files Modified by PR | ||
| if: ${{ env.CONTROLS_CHANGES == 'true' }} | ||
| run: | | ||
| for control_file in $(cat filenames.txt | grep "controls/"); do | ||
| echo "Running yamllint on $control_file..." | ||
| yamllint "$control_file" | ||
| done | ||
|
|
||
| - name: Run yamllint in Profile Files Modified by PR | ||
| if: ${{ env.PROFILES_CHANGES == 'true' }} | ||
| - name: Run yamllint on files modified by the PR | ||
| run: | | ||
| for profile_file in $(cat filenames.txt | grep "\.profile"); do | ||
| echo "Running yamllint on $profile_file..." | ||
| yamllint "$profile_file" | ||
| exit_code=0 | ||
| for file in $(cat filenames.txt); do | ||
| if [[ ! -f "$file" ]]; then | ||
| continue | ||
| fi | ||
| echo "Running yamllint on $file..." | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will this work on YAML files that contain Jinja macros?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the last commit I added a script which strips Jinja and still allows us to lint files. |
||
| if grep -qP '\{\{[%{#]' "$file"; then | ||
| # File contains Jinja2 constructs — strip them before linting. | ||
| # yamllint -s exits: 0 = clean, 1 = errors, 2 = warnings only. | ||
| output=$(python3 utils/strip_jinja_for_yamllint.py "$file" \ | ||
| | yamllint -s -c .yamllint - 2>&1) | ||
| rc=$? | ||
| # Show all output (warnings and errors), replacing "stdin" | ||
| # with the actual filename since yamllint reads from a pipe. | ||
| if [ -n "$output" ]; then | ||
| echo "$output" | sed "s|^stdin|$file|" | ||
| fi | ||
| # Fail only on errors (exit code 1), not warnings (exit code 2). | ||
| if [ "$rc" -eq 1 ]; then | ||
| exit_code=1 | ||
| fi | ||
| else | ||
| yamllint -s -c .yamllint "$file" | ||
| rc=$? | ||
| if [ "$rc" -eq 1 ]; then | ||
| exit_code=1 | ||
| fi | ||
| fi | ||
| done | ||
| exit $exit_code | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,22 @@ | ||
| --- | ||
| extends: default | ||
| locale: en_US.UTF-8 | ||
| yaml-files: | ||
| - "*.yaml" | ||
| - "*.yml" | ||
| - "*.fmf" | ||
| - "*.profile" | ||
|
|
||
| # https://yamllint.readthedocs.io/en/stable/rules.html | ||
| rules: | ||
| comments: disable | ||
| comments-indentation: disable | ||
| document-start: disable | ||
| truthy: disable # do not check for strict true / false boolean values | ||
| comments: disable # disable syntax checking of comments | ||
| comments-indentation: disable # disable indentation checks for comments | ||
| document-start: disable # do not require the document start marker | ||
| empty-lines: | ||
| level: warning | ||
| level: warning # only warn about empty lines | ||
| indentation: | ||
| # pass if spaces are used for indentation and number of spaces is consistent through a file | ||
| spaces: consistent | ||
| line-length: disable | ||
| line-length: | ||
| max: 99 # allow lines up to 99 chars | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do all our YAML files meet the line lenghth limit of 99 lines?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They probably don't, but this should run only on newly modified files. At the same time, we impose this limit on our selves... so with this approach, it will be enforced gradually. Do you think it is a problem? |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| #!/usr/bin/env python3 | ||
| """Strip Jinja2 constructs from YAML files to make them yamllint-safe. | ||
|
|
||
| This project uses Jinja2 templating ({{% %}}, {{{ }}}, {{# #}}) inside YAML | ||
| files. yamllint cannot parse these constructs, so this script removes them | ||
| while preserving line numbers (replaced regions become blank lines) so that | ||
| yamllint error messages still point to the correct source lines. | ||
|
|
||
| Usage: | ||
| python3 utils/strip_jinja_for_yamllint.py FILE | ||
|
|
||
| The cleaned content is written to stdout. | ||
| """ | ||
|
|
||
| import re | ||
| import sys | ||
|
|
||
|
|
||
| def _replace_with_blanks(match): | ||
| """Replace a match with the same number of newlines to preserve line numbers.""" | ||
| return "\n" * match.group(0).count("\n") | ||
|
|
||
|
|
||
| def strip_jinja(content): | ||
| # 1. Remove whole-line Jinja block tags: {{% ... %}} on their own line(s). | ||
| # Match the entire line (including leading whitespace) to avoid leaving | ||
| # trailing spaces behind. | ||
| content = re.sub( | ||
| r"^[ \t]*\{\{%.*?%\}\}[ \t]*$", | ||
| _replace_with_blanks, | ||
| content, | ||
| flags=re.MULTILINE | re.DOTALL, | ||
| ) | ||
| # Remove any remaining inline block tags (rare). | ||
| content = re.sub(r"\{\{%.*?%\}\}", _replace_with_blanks, content, flags=re.DOTALL) | ||
|
|
||
| # 2. Remove whole-line Jinja comments: {{# ... #}} | ||
| content = re.sub( | ||
| r"^[ \t]*\{\{#.*?#\}\}[ \t]*$", | ||
| _replace_with_blanks, | ||
| content, | ||
| flags=re.MULTILINE | re.DOTALL, | ||
| ) | ||
| # Remove any remaining inline comments. | ||
| content = re.sub(r"\{\{#.*?#\}\}", "", content, flags=re.DOTALL) | ||
|
|
||
| # 3a. Standalone Jinja expressions occupying entire lines — these typically | ||
| # expand to top-level YAML keys (e.g. ocil/ocil_clause macros) or | ||
| # Ansible tasks, so replacing them with a placeholder string would | ||
| # produce invalid YAML. Replace with a YAML-safe comment placeholder | ||
| # to avoid trailing whitespace on otherwise blank lines. | ||
| content = re.sub( | ||
| r"^([ \t]*)\{\{\{.*?\}\}\}[ \t]*$", | ||
| lambda m: m.group(1) + "# jinja" + "\n" * (m.group(0).count("\n") - 1) | ||
| if m.group(0).count("\n") > 0 | ||
| else m.group(1) + "# jinja", | ||
| content, | ||
| flags=re.MULTILINE | re.DOTALL, | ||
| ) | ||
|
|
||
| # 3b. Inline Jinja expressions embedded inside a YAML value — replace | ||
| # with a short placeholder so the surrounding YAML stays valid. | ||
| content = re.sub(r"\{\{\{.*?\}\}\}", "JINJA_EXPRESSION", content) | ||
|
|
||
| return content | ||
|
|
||
|
|
||
| def main(): | ||
| if len(sys.argv) != 2: | ||
| print(f"Usage: {sys.argv[0]} FILE", file=sys.stderr) | ||
| sys.exit(2) | ||
|
|
||
| with open(sys.argv[1]) as f: | ||
| sys.stdout.write(strip_jinja(f.read())) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| main() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have removed this insertion of
PROFILES_CHANGESandCONTROL_CHANGESenvironment variables to GITHUB_ENV. But, the "Install yamllint" phase still uses these environment variables. I suspect it doesn't work.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the leftover conditional.