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

Output "Passed with <> profile" is broken #2735

Closed
dbk-rabel opened this issue Nov 24, 2022 · 4 comments · Fixed by #3545
Closed

Output "Passed with <> profile" is broken #2735

dbk-rabel opened this issue Nov 24, 2022 · 4 comments · Fixed by #3545
Assignees
Labels

Comments

@dbk-rabel
Copy link

dbk-rabel commented Nov 24, 2022

Summary

ansible-lint shows a message "Passed with ... profile". But the given profile seems to never be the one, that we used to test.

Issue Type
  • Bug Report
Ansible and Ansible Lint details
$ ansible-lint --version
ansible-lint 6.8.7 using ansible 2.14.0
  • ansible-lint installation method: pip
OS / ENVIRONMENT

RHEL 8

STEPS TO REPRODUCE
$ ansible-lint --offline bootstrap.yml --profile=basic

WARNING  Overriding detected file kind 'yaml' with 'playbook' for given positional argument: ./bootstrap.yml

Passed with production profile: 0 failure(s), 0 warning(s) on 1 files.
Desired Behavior

Show the correct profile:

$ ansible-lint --offline bootstrap.yml --profile=basic

WARNING  Overriding detected file kind 'yaml' with 'playbook' for given positional argument: ./bootstrap.yml

Passed with basic profile: 0 failure(s), 0 warning(s) on 1 files.
Actual Behavior

Show the wrong profile:

$ ansible-lint --offline bootstrap.yml --profile=basic

WARNING  Overriding detected file kind 'yaml' with 'playbook' for given positional argument: ./bootstrap.yml

Passed with production profile: 0 failure(s), 0 warning(s) on 1 files.
@dbk-rabel dbk-rabel added bug new Triage required labels Nov 24, 2022
@ssbarnea ssbarnea self-assigned this Nov 24, 2022
@ssbarnea ssbarnea removed the new Triage required label Nov 24, 2022
@ericstengard
Copy link

ericstengard commented Feb 22, 2023

From what I gather the issue resides at https://github.com/ansible/ansible-lint/blob/main/src/ansiblelint/app.py#L293

I'd argue that there are issues both when we pass and fail. It is more confusing that other profiles are mentioned, I'd argue that when specifying profile x we don't care if we fail on a subset of profile x but rather if we actually fail on profile x. Since the profile y is a subset of x it is more confusing to mention profile y instead of just outputting "Failed with profile x". You're still presented with the faults and the profile they belong to which would be enough to signify to the user which subset of the chosen profile that failed.

The problem is that if you provide say production profile and fail on a rule specified in the min profile your output will be that you fail with min profile instead. The desired output would be the profile specified, since for example the production profile includes the min profile.

For the case of a successful run the output will always be production profile, it does not matter what the user sets --profile to. To verify one has to enable debug logs to verify that the specified profile is actually loaded. This is super intuitive and leads to a lot of confusion.

Since @dbk-rabel already showed steps to reproduce the second case described I'll add the first case I described above.

Ansible and Ansible Lint details

% ansible-lint --version
ansible-lint 6.13.1 using ansible 2.14.2
  • ansible-lint installation method: pip

OS / ENVIRONMENT

Ubuntu 20.04.5 LTS x86_64

STEPS TO REPRODUCE / ACTUAL BEHAVIOR

% ansible-lint --offline --profile=min

Passed with production profile: 0 failure(s), 0 warning(s) on 1 files.
% ansible-lint --offline --profile=production
WARNING  Listing 2 violation(s) that are fatal
yaml[trailing-spaces]: Trailing spaces
bootstrap.yml:3

fqcn[action-core]: Use FQCN for builtin module actions (debug).
bootstrap.yml:8 Use `ansible.builtin.debug` or `ansible.legacy.debug` instead.

Read documentation for instructions on how to ignore specific rule violations.

                   Rule Violation Summary
 count tag                   profile    rule associated tags
     1 yaml[trailing-spaces] basic      formatting, yaml
     1 fqcn[action-core]     production formatting

Failed after min profile: 2 failure(s), 0 warning(s) on 1 files.
% ansible-lint --offline --profile=safety
WARNING  Listing 1 violation(s) that are fatal
yaml[trailing-spaces]: Trailing spaces
bootstrap.yml:3

Read documentation for instructions on how to ignore specific rule violations.

                  Rule Violation Summary
 count tag                   profile rule associated tags
     1 yaml[trailing-spaces] basic   formatting, yaml

Failed after min profile: 1 failure(s), 0 warning(s) on 1 files.

DESIRED BEHAVIOR

% ansible-lint --offline --profile=safety
WARNING  Listing 1 violation(s) that are fatal
yaml[trailing-spaces]: Trailing spaces
bootstrap.yml:3

Read documentation for instructions on how to ignore specific rule violations.

                  Rule Violation Summary
 count tag                   profile rule associated tags
     1 yaml[trailing-spaces] basic   formatting, yaml

Failed with safety profile: 1 failure(s), 0 warning(s) on 1 files.
% ansible-lint --offline --profile=production
WARNING  Listing 2 violation(s) that are fatal
yaml[trailing-spaces]: Trailing spaces
bootstrap.yml:3

fqcn[action-core]: Use FQCN for builtin module actions (debug).
bootstrap.yml:8 Use `ansible.builtin.debug` or `ansible.legacy.debug` instead.

Read documentation for instructions on how to ignore specific rule violations.

                   Rule Violation Summary
 count tag                   profile    rule associated tags
     1 yaml[trailing-spaces] basic      formatting, yaml
     1 fqcn[action-core]     production formatting

Failed with production profile: 2 failure(s), 0 warning(s) on 1 files.

@ericstengard
Copy link

I might be able to allocate time to this if this isn't prioritized. Please confirm that I'm not the only one who believes that both success/fail messages are an issue.

@dbk-rabel
Copy link
Author

Thanks for the explaination!

At least in my opinion you are right and the messages should be changed. But I might be biased. ;)

@cidrblock
Copy link
Contributor

cidrblock commented Jun 7, 2023

I tend to agree, it should simply reflect the profile tested against.

We could also add a "last profile passed" if anyone thinks that would be valuable

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

Successfully merging a pull request may close this issue.

4 participants