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

GitHub Actions output doesn't reflect warn list settings. #1670

Closed
jorhett opened this issue Jul 16, 2021 · 5 comments
Closed

GitHub Actions output doesn't reflect warn list settings. #1670

jorhett opened this issue Jul 16, 2021 · 5 comments
Assignees
Labels
Milestone

Comments

@jorhett
Copy link

jorhett commented Jul 16, 2021

Summary

When run under GitHub actions, ansible-lint's output correctly identifies how many warnings and errors are found according to the Ansible-lint file. However, the output it provides for GitHub Actions doesn't take this into account... causing Errors which are marked down to Warnings to appear in red.

Issue Type
  • Bug Report
Ansible and Ansible Lint details
$ ansible --version
ansible [core 2.11.2]
  
$ ansible-lint --version
ansible-lint 5.0.12 using ansible 2.11.2
  • ansible installation method: pip
  • ansible-lint installation method: pip
OS / ENVIRONMENT

GitHub Actions, python3 environment (example below)

STEPS TO REPRODUCE
---
warn_list:
  - no-changed-when
  - unnamed-task
lint:
    runs-on: ubuntu-20.04
    steps:
     - name: Set up Python 3.9
        uses: actions/setup-python@v2
        with:
          python-version: 3.9
          
    - name: Install ansible, ansible-lint, yamllint
        run: pip3 install yamllint ansible ansible-lint

      - name: ansible-lint changed roles
        continue-on-error: true
        run: ansible-lint -test.yaml

Contents of test.yaml:

- hosts: localhost
  tasks:
    - command: whoami
Desired Behaviour

The output should colorize and label these two problems as warnings (yellow)

Actual Behaviour

Text output indicates the correct count of warnings/errors based on the configuration file.
GitHub PR annotations are labeled according to the default configuration. Things that are warnings are labeled with the word Failure and colored red.

See screenshots

# Github output
Error: no-changed-when Commands should not change things if nothing needs doing
Error: unnamed-task All tasks should be named
Error: deprecated-command-syntax rm used in place of argument state=absent to file module
Warning: ignore-errors Use failed_when and specify error conditions instead of using ignore_errors
Error: no-changed-when Commands should not change things if nothing needs doing
Error: unnamed-task All tasks should be named
Warning: var-spacing Variables should have spaces before and after: {{tubi_rc_redis_dir}}/*.aof
Error: deprecated-command-syntax rm used in place of argument state=absent to file module
Warning: ignore-errors Use failed_when and specify error conditions instead of using ignore_errors
Error: no-changed-when Commands should not change things if nothing needs doing
Error: unnamed-task All tasks should be named
Warning: var-spacing Variables should have spaces before and after: {{tubi_rc_redis_dir}}/*.conf
Error: deprecated-command-syntax rm used in place of argument state=absent to file module
Warning: ignore-errors Use failed_when and specify error conditions instead of using ignore_errors
Error: no-changed-when Commands should not change things if nothing needs doing
Error: unnamed-task All tasks should be named
Warning: var-spacing Variables should have spaces before and after: {{tubi_rc_redis_dir}}/*.rdb
Error: no-changed-when Commands should not change things if nothing needs doing
Error: risky-shell-pipe Shells that use pipes should set the pipefail option

# Text output
Finished with 6 failure(s), 13 warning(s) on 1 files.
@jorhett jorhett added bug new Triage required labels Jul 16, 2021
@jorhett
Copy link
Author

jorhett commented Jul 16, 2021

Screen Shot 2021-07-15 at 7 16 23 PM

@ssbarnea ssbarnea self-assigned this Jul 16, 2021
@ssbarnea ssbarnea removed the new Triage required label Jul 16, 2021
@ssbarnea
Copy link
Member

Indeed this is because current implementation only uses hardcoded/static severity configured by each rule and not really considering the warn_list when computing the level.

Fixing this bug may require little bit of refactoring, mainly we should modify the found Matches to record inside them if they were downgraded to warning level by warn_list, so they could also be displayed correctly.

In fact I am inclined to think that probably the new implementation will disregard original severity field and only report matches as errors or warnings.

I am not even sure what was the original reason for introducing the severity field and if someone relies on it. AFAIK, a linter should only report errors and warnings, first one causing a failure and second one being only a message. I am not sure what would count as an "info" level.

@webknjaz @tadeboro @felixfontein DYIT?

@djdanielsson
Copy link

djdanielsson commented Oct 5, 2022

Is this being worked on?

@ssbarnea
Copy link
Member

ssbarnea commented Oct 5, 2022

I checked and I am unable to reproduce the bug with latest beta version, all tasks that were marked as warning in Summary report was also using github warning annotations instead of errors.

@ssbarnea ssbarnea closed this as completed Oct 5, 2022
@djdanielsson
Copy link

This is still an issue

@ssbarnea ssbarnea reopened this Oct 6, 2022
@ssbarnea ssbarnea added this to the 6.8.x milestone Oct 6, 2022
ssbarnea added a commit to ssbarnea/ansible-lint that referenced this issue Oct 6, 2022
ssbarnea added a commit to ssbarnea/ansible-lint that referenced this issue Oct 6, 2022
ssbarnea added a commit to ssbarnea/ansible-lint that referenced this issue Oct 6, 2022
- Display warnings different than errors on all output formats
- Document YAML specific errors
- Display warning using yellow AND adding (warning) suffix
- Corrected cases where warning was determined incorrectly
- Display only the detailed rule tag on matches (reduce clutter)
- Capitalize YAML error messages

Fixes: ansible#1670
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants