yamllint: cover all changed files ending in yaml, yml or fmf#14601
yamllint: cover all changed files ending in yaml, yml or fmf#14601vojtapolasek wants to merge 8 commits intoComplianceAsCode:masterfrom
Conversation
.github/workflows/ci_lint.yml
Outdated
| if: ${{ env.CONTROLS_CHANGES == 'true' }} | ||
| run: | | ||
| for control_file in $(cat filenames.txt | grep "controls/"); do | ||
| for control_file in $(cat filenames.txt | grep "controls/.*\.yml"); do |
There was a problem hiding this comment.
There is a similar regex on line 32. Should that regex be changed as well?
|
Are you sure you don't want to configure |
vojtapolasek
force-pushed
the
make_yamllint_filepaths_stricter
branch
from
8ca69d6 to
d48dd28
Compare
Filter out files inside the .yamllint configuration
also enforce line length of 99
vojtapolasek
force-pushed
the
make_yamllint_filepaths_stricter
branch
from
d48dd28 to
975e2d6
Compare
vojtapolasek
force-pushed
the
make_yamllint_filepaths_stricter
branch
from
b803433 to
02bffa8
Compare
vojtapolasek
changed the title
jan-cerny
left a comment
jan-cerny
left a comment
There was a problem hiding this comment.
@vojtapolasek Do I understand it well that now the .profile files won't be checked?
|
@jan-cerny oops, correct, I have to fix this. |
|
/retest |
.github/workflows/ci_lint.yml
Outdated
| yamllint "$profile_file" | ||
| for file in $(cat filenames.txt); do | ||
| echo "Running yamllint on $file..." | ||
| yamllint -c .yamllint "$file" |
There was a problem hiding this comment.
I think this will run yamllint on all files modified by the PR, think OVALs, Python files, ... It can slow the job down.
There was a problem hiding this comment.
I think it will not slow things down, the .yamllint config file explicitly specifies globs which will be actually scanned.
| if grep -q "\.profile" filenames.txt; then | ||
| echo "PROFILES_CHANGES=true" >> $GITHUB_ENV | ||
| else | ||
| echo "PROFILES_CHANGES=false" >> $GITHUB_ENV |
There was a problem hiding this comment.
You have removed this insertion of PROFILES_CHANGES and CONTROL_CHANGES environment 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.
I removed the leftover conditional.
| echo "Running yamllint on $profile_file..." | ||
| yamllint "$profile_file" | ||
| for file in $(cat filenames.txt); do | ||
| echo "Running yamllint on $file..." |
There was a problem hiding this comment.
Will this work on YAML files that contain Jinja macros?
There was a problem hiding this comment.
In the last commit I added a script which strips Jinja and still allows us to lint files.
| spaces: consistent | ||
| line-length: disable | ||
| line-length: | ||
| max: 99 # allow lines up to 99 chars |
There was a problem hiding this comment.
Do all our YAML files meet the line lenghth limit of 99 lines?
There was a problem hiding this comment.
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?
Many YAML files in this project contain Jinja2 templating ({{% %}},
{{{ }}}, {{# #}}) which causes yamllint to report false syntax errors.
Add a helper script (utils/strip_jinja_for_yamllint.py) that removes
Jinja constructs while preserving line numbers, and update the CI
workflow to use it for files that contain Jinja. Files without Jinja
are linted directly as before. Warnings are reported but do not fail
the job.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
3 participants
Description:
Rationale:
grep "controls/"pattern matched any file path containingcontrols/, which could include non-YAML files that yamllint cannot process. Restricting tocontrols/.*\.ymlensures only relevant YAML files are linted, avoiding spurious CI failures.Review Hints:
.github/workflows/ci_lint.yml(line 52).