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

ansible-lint should not report var-naming[no-role-prefix] in certain situations - block variables and task variables #3544

Closed
zeten30 opened this issue Jun 12, 2023 · 6 comments · Fixed by #3550
Assignees
Labels
Milestone

Comments

@zeten30
Copy link

zeten30 commented Jun 12, 2023

Summary

Ansible-lint (starting version 6.17) reports var-naming[no-role-prefix] issue for all role variables that are not starting with <role_name> prefix.

I don't think it's correct. Do we rally want' to prefix every single variable in the role like this. It's not throwing warning for variables created by loops (for example.)

Issue Type
  • Bug Report
OS / ENVIRONMENT
ansible-lint --version
ansible-lint 6.17.0 using ansible-core:2.15.0 ruamel-yaml:0.17.21 ruamel-yaml-clib:0.2.7
  • ansible installation method: pip
  • ansible-lint installation method: pip
STEPS TO REPRODUCE
ansible-lint roles/ playbook.yml

# (code attached)

WARNING  Listing 2 violation(s) that are fatal
var-naming[no-role-prefix]: Variables names from within roles should use role_name_ as a prefix. (vars: some_block_internal_var)
roles/lint_test/tasks/main.yml:8 Task/Handler: Block var test

var-naming[no-role-prefix]: Variables names from within roles should use role_name_ as a prefix. (vars: temp_var)
roles/lint_test/tasks/main.yml:20 Task/Handler: Task vars

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

                    Rule Violation Summary                     
 count tag                        profile rule associated tags 
     2 var-naming[no-role-prefix] basic   idiom  
Desired Behavior

I think that task vars:

- name: Task vars
  vars:
    temp_var: this_is_temp_var
  ansible.builtin.debug:
    var: temp_var

or block vars (technically task vars too)

- name: Block var test
  # I think ansible-lint should nore report var-naming[no-role-prefix] here
  vars:
    some_block_internal_var: "block var"
  block:
    - name: Debug inside block
      ansible.builtin.debug:
        var: some_block_internal_var

- name: Debug outside the block - inside the role code
  ansible.builtin.debug:
    var: some_block_internal_var

should not be reported as issues.

Actual Behavior

Getting linter issues for things that should probably be ignored/skipped.

@zeten30 zeten30 added bug new Triage required labels Jun 12, 2023
@zeten30
Copy link
Author

zeten30 commented Jun 12, 2023

Screenshot from 2023-06-12 09-15-59

@zeten30
Copy link
Author

zeten30 commented Jun 12, 2023

var_naming.zip

@zeten30
Copy link
Author

zeten30 commented Jun 12, 2023

The variables showed above (in the example role & playbook) are valid only inside task or block. I don't think they should be reported.

Bare variable exported using set_fact should be reported. It's accessible later / is valid since declaration to the end of the play.

The warning is not displayed for the same code, but from an included file. Such code has the same effect, variable is defined for the rest of the play.

- name: Include tasks
  ansible.builtin.include_tasks: include/main.yml

# include/main.yml
- name: Set fact in include
  ansible.builtin.set_fact:
    potentionally_bad_named_var: test
ansible-playbook -i inventory/ playbook.yml
...

TASK [Debug of another set_fact var - but not reported] ************************************************************************************************************************************************************************
ok: [localhost] => 
  potentionally_bad_named_var: test ## it's available

PLAY RECAP *********************************************************************************************************************************************************************************************************************
localhost                  : ok=11   changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0
ansible-lint roles/ playbook.yml
WARNING  Listing 2 violation(s) that are fatal
var-naming[no-role-prefix]: Variables names from within roles should use role_name_ as a prefix. (vars: some_block_internal_var)
roles/lint_test/tasks/main.yml:8 Task/Handler: Block var test

var-naming[no-role-prefix]: Variables names from within roles should use role_name_ as a prefix. (vars: temp_var)
roles/lint_test/tasks/main.yml:21 Task/Handler: Task vars

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

                    Rule Violation Summary                     
 count tag                        profile rule associated tags 
     2 var-naming[no-role-prefix] basic   idiom                

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

@zeten30
Copy link
Author

zeten30 commented Jun 12, 2023

var_naming2.zip

@loganmzz
Copy link

loganmzz commented Jun 12, 2023

I agree. I have an "additional" case (just specific block var-spec) which break the rule itself:

I have role A which call reusable role B

Then, role B expects b_params to contains its params:

  • roles/b/tasks/main.yml:
- debug:
    var: b_params
  • roles/a/tasks/main.yml:
- include_role:
    name: b
  var:
    b_params: "Hello World !"

@ssbarnea ssbarnea removed the new Triage required label Jun 12, 2023
@ssbarnea ssbarnea self-assigned this Jun 12, 2023
@ssbarnea ssbarnea added this to the 6.17.x milestone Jun 12, 2023
@zeten30
Copy link
Author

zeten30 commented Jun 14, 2023

@ssbarnea TYVM. 6.17.1 works for me.

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.

3 participants