ansible / ansible-lint-action

❗️Replaced by https://github.com/marketplace/actions/run-ansible-lint
https://github.com/marketplace/actions/run-ansible-lint
MIT License
254 stars 132 forks source link

Leading space on first label of arguments #87

Closed nleiva closed 2 years ago

nleiva commented 2 years ago

Hi, thank you for 6.0.1, it addressed all the issues I've hit in the past, great job!

I believe there might be a small issue when parsing the arguments. I was trying to either just warn or skip unpredictability rules, however -w unpredictability wouldn't work.

unpredictability:  # Warn about code that might not work in a predictable way
  - ignore-errors
  - partial-become
  - risky-file-permissions

I added security in case you needed a list.

- name: Lint Ansible Playbook
  uses: ansible/ansible-lint-action@v6.0.1
  with:  
    args: "-w unpredictability,security"

Which in the logs translates to warn_list=[' unpredictability,security']. That leading space was not letting ansible-lint ignore these rules:

Warning: ignore-errors Use failed_when and specify error conditions instead of using ignore_errors.
Error: risky-file-permissions File permissions unset or incorrect.

I inverted the order, and problem solved.

- name: Lint Ansible Playbook
  uses: ansible/ansible-lint-action@v6.0.1
  with:  
    args: "-w security,unpredictability"

Logs show warn_list=[' security,unpredictability'] and no more errors. This is the commit: https://github.com/nleiva/ansible-web-server/commit/7428cb0fbab2d03d84bc58feb4f04d10fdf067e9

ssbarnea commented 2 years ago

Can you please check if you get the same issue when calling ansible-lint 6.0.2 from the command line? I tried and apparently from command line it does recognize the command separated list correctly, without the extra space.

nleiva commented 2 years ago

Yeah, from the CLI I don't experience any issues.

officel commented 2 years ago

I am maybe encountering this same type of event.

my wf is

    steps:
      - uses: actions/checkout@master

      - name: Lint Ansible Playbook
        uses: ansible/ansible-lint-action@v6.0.2
        with:
          args: "-c ansible/.ansible-lint"

result is

image

tigattack commented 2 years ago

Same issue here 👍

markdorison commented 2 years ago

I am experiencing the same issue:

      - name: Run ansible-lint
        uses: ansible/ansible-lint-action@v6
        with:
          args: "-c ansible/.config/ansible-lint.yml ansible/"
CleanShot 2022-03-29 at 17 35 35@2x
ssbarnea commented 2 years ago

A PR would be really welcomed here!

markdorison commented 2 years ago

Updated ansible-lint-action version after upstream changes. ChromaticHQ/devops#1620

I would love to create one. I was searching the source to see where args might be manipulated but I am not seeing much. Does anyone have any insight as to where this might be occurring?

ssbarnea commented 2 years ago

I bet that if you use code below it will work:

with:
  args:
    - -c
    - ansible/.config/ansible-lint.yml
    - ansible/

Still, even if that works, the bug is still valid as we should split the args using shlex when it is a string instead of a list or arguments.

officel commented 2 years ago

Unfortunately no. Because only strings are allowed as input (args) for actions. Arrays are not allowed.

ssbarnea commented 2 years ago

I did a bit of research and it seems that current implementation of GHA does not allow us to pass arbitrary number of arguments to an action.

As I do not want to go to the slope of having to maintain a plethora of options for this action, I think I will replace args with path and mention that you can only pass one value.

Users will have to make use of ansible-lint config file for any arguments or fork the action and modify its action.yml file.

In fact even the path argument could be avoided because you could still edit the linter config to make it look only at certain paths, and avoid the need to pass it any arguments.

Sounds ok as a workaround? I am open to suggestions.

officel commented 2 years ago

Isn't renaming necessary? Forcing other users to make changes is not a good idea.

I haven't found where it bugs me yet, but I did find a workaround for the -c option. Also, on another matter, the --project-dir option doesn't seem to work. It seems to work around that too.

    steps:
      - uses: actions/checkout@master

      - name: copy .ansible-lint
        shell: bash
        run: |
          cp ansible/.ansible-lint .

      - name: Lint Ansible Playbook
        uses: ansible/ansible-lint-action@v6
        with:
          args: ansible

it's my usecase, I have an ansible directory in my repository. The reason is that there are other directories, such as the terraform directory.

I am in the ansible directory when I work with ansible. So .ansible-lint is also in the ansible directory

The error with -c option can be avoided by copying the .ansible-lint in my workflow.

By passing the working directory at the end of the args (path you changed) option, I was able to make it almost the same as running locally.

I would like to see the changes to path revert for those who want to use the other options that are normal in ansible-lint.

Thanks for reading my bad English. 😄