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

sanity[cannot-ignore] issues inconsistent with sanity check #3951

Closed
andy-maier opened this issue Dec 17, 2023 · 2 comments
Closed

sanity[cannot-ignore] issues inconsistent with sanity check #3951

andy-maier opened this issue Dec 17, 2023 · 2 comments
Assignees
Labels

Comments

@andy-maier
Copy link

andy-maier commented Dec 17, 2023

Summary

In our collection, we need to ignore certain issues the ansible sanity check otherwise would fail on. ansible-lint claims for a subset of these ignores that they would not be permitted:

sanity[cannot-ignore]: Ignore file contains ..... at line ....., which is not a permitted ignore.

However, the sanity check fails when these ignores are removed from the ignore files.

It seems that ansible-lint has a different idea about these ignores than the ansible sanity check. That makes life difficult for users, particularly since the Ansible AutomationHub runs ansible-lint when uploading a collection (Ansible Galaxy does not).

I have categorized the "sanity[cannot-ignore]" issues reported by ansibe-lint for our ibm.ibm_zhmc collection, and did not find a way to avoid the need for these ignores:

Ignored sanity issue                   Ansible core    Notes
--------------------------------------------------------------------------------------------------------
pylint:raise-missing-from              2.9, 2.10       "raise from" requires Python 3
validate-modules:return-syntax-error   all             Missing type on generic {property}
validate-modules:no-log-needed         2.11 - 2.15     zhmc_lpar: os_ipl_token is not a secret
pylint:forgotten-debug-statement       2.14 - 2.15     Intentional pdb.set_trace() call in test code
pylint!skip                            2.9             Unreliable duplicate-code issues (bug in sanity check in that Ansible version?)
rstcheck!skip                          2.9             Expecting property name enclosed in double quotes (bug in sanity check in that Ansible version?)

See also zhmcclient/zhmc-ansible-modules#784 where we try to work this problem on our end.

Issue Type
  • Bug Report
OS / ENVIRONMENT

Environment when reproducing it locally:

  • OS: macOS
  • Python: 3.12
  • ansible installation method: pip
  • ansible-lint installation method: pip
  • ansible version:
ansible-lint --version
ansible-lint 6.22.1 using ansible-core:2.16.2 ansible-compat:4.1.10 ruamel-yaml:0.18.5 ruamel-yaml-clib:0.2.8
STEPS TO REPRODUCE

Steps to reproduce how the AutomationHub runs ansible-lint on the ibm.ibm_zhmc collection:

git clone https://github.com/zhmcclient/zhmc-ansible-modules.git
cd zhmc-ansible-modules
make dist
mkdir dist/tmp
cd dist/tmp
tar -xf ../ibm-ibm_zhmc-1.7.1.tar.gz
ansible-lint --profile production -f pep8

Just in case of interest, this runs the ansible sanity check:

make sanity
Desired Behavior

ansible-lint should be consistent with the ansible sanity check and thus should not flag ignores accepted by the sanity check.

Actual Behavior

See above.

@alisonlhart
Copy link
Contributor

alisonlhart commented Dec 20, 2023

Hello @andy-maier ! This rule is behaving as intended, as it is designed to help limit the number of sanity ignores utilized in certified collections. This check only applies on Automation Hub for that reason. It is contained within the "production" profile, which is specified as the certified and validated collection enforcement profile.

The implementation is designed to limit sanity ignores for issues that can be resolved in the majority of cases in certified collections.

The rule details are enforced by Ansible Partner Engineering, similar to #3950. Please reach out to us at ansiblepartners@redhat.com with any questions about the details, and we can provide advice on how to resolve some of the underlying causes for these ignores or how to clear the majority of ansible-lint sanity errors.

@ssbarnea ssbarnea closed this as not planned Won't fix, can't repro, duplicate, stale Dec 20, 2023
@andy-maier
Copy link
Author

andy-maier commented Jan 9, 2024

@alisonlhart @ssbarnea
We want to support Ansible back to 2.9 on a best-can-do basis.

Since there is no official support for older ansible-core versions (currently below 2.14), we don't need to run the ansible sanity check when we test against these older ansible versions and therefore can delete the corresponding older ignore files, and get rid of most of the ignore issues that way.

That leaves the following two ignore issues to be addressed:

Ignored sanity issue                   Ansible core    Notes
--------------------------------------------------------------------------------------------------------
validate-modules:return-syntax-error   2.14 - 2.16     Generic return property specified without type
validate-modules:no-log-needed         2.14 - 2.16     os_ipl_token module parameter is not a secret

On validate-modules:return-syntax-error:

In some of our modules, we have a large list of return properties that changes with different versions of the HMC device our collection operates on. We therefore refer to the documentation for that device and use a generic property name {property} with no type in the RETURN documentation to document these properties, e.g. for the zhmc_lpar module:

RETURN = """
changed:
  description: Indicates if any change has been made by the module.
    For C(state=facts), always will be false.
  returned: always
  type: bool
msg:
  description: An error message that describes the failure.
  returned: failure
  type: str
lpar:
  description:
    - "The resource properties of the LPAR, after any specified updates have
       been applied, for C(state=active), C(state=loaded) and C(state=set).
       For any other values of C(state), the dictionary will be empty."
    - "Note that the returned properties may show different values than the ones
       that were specified as input for the update. For example, memory
       properties may be rounded up, hexadecimal strings may be shown with a
       different representation format, and other properties may change as a
       result of updating some properties. For details, see the data model of
       the 'Logical Partition' object in the :term:`HMC API` book."
  returned: success
  type: dict
  contains:
    name:
      description: "LPAR name"
      type: str
    "{property}":
      description: "Additional properties of the LPAR, as described in
        the data model of the 'Logical Partition' object in the
        :term:`HMC API` book.
        The property names have hyphens (-) as described in that book."
   sample:
     . . .

On validate-modules:no-log-needed:

We have only one occurrence of this, on the module input parameter named "os_ipl_token" of the zhmc_lpar module.
I suppose that ansible-lint has a heuristic that parameters that have the word "token" in their name are secrets. That is not the case for this parameter (it only has a correlation purpose), and therefore we ignore the no-log-needed issue.

Any suggestions on what to do about these two issues?

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

No branches or pull requests

4 participants