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

enhancement: ansible-lint - Improved activation by checking for .ansible-lint config file #3697

Merged
merged 5 commits into from
Jun 30, 2024

Conversation

TommyE123
Copy link
Contributor

@TommyE123 TommyE123 commented Jun 26, 2024

ansible-lint - Improved activation by checking for .ansible-lint config file

Fixes #1252 and #2132 and #3424 (all 3 already closed)

Context:

While testing a previous issue for ansible-lint I noticed that even using ANSIBLE_DIRECTORY: . I couldn’t get it to activate properly on Gitlab or Azure

Looking at the suggested directory layout I don’t think it makes sense to use the files_sub_directory: ansible as a default as this folder isn’t mentioned. All this appears to be currently doing is acting as a method to keep ansible-lint switched off unless the .mega-linter.yml is adjusted as a workaround to activate.

I’ve avoid checking for the Rules folder as currently there wasn’t a method in megalinter I could see to easily check for this folder. Also setting it to a sub directory could mean false detections and possibly only scanning that one folder. I’m open to suggestions if this sounds like an issue.

Reading others comments I personally think a better approach would be to harness the existing megalinter functionality to check if a file is present. In this case the .ansible-lint.

Changes:

• Removed unrequired default files_sub_directory: ansible property in ansible.megalinter-descriptor.yml
• Added:
active_only_if_file_found:
- ".ansible-lint"
• Added a default .ansible-lint file in templates.
• Simplified the tests.

Testing:

I've done a mixture of tests through different pipelines.

AZURE

Scenario 1: – Not Activated

No .ansible-lint File
image

Scenario 2: – Activated

Root .ansible-lint
image

Scenario 3: – Activated

LINTER_RULES_PATH: "/_Pipelines/LinterRules"
image

GITHUB

Scenario 1: – Activated

ANSIBLE_ANSIBLE_LINT_RULES_PATH: config
image

GITLAB

Scenario 1: – Not Activated

No .ansible-lint File
image

Scenario 2: – Activated

Root .ansible-lint
image

Scenario 3: – Activated

ANSIBLE_ANSIBLE_LINT_RULES_PATH: "/_Pipelines/LinterRules"
image

Scenario 4: – Activated (note missing . at front of filename)

ANSIBLE_ANSIBLE_LINT_RULES_PATH: "/_Pipelines/LinterRules"
ANSIBLE_ANSIBLE_LINT_CONFIG_FILE: ansible-lint
image

Scenario 5: – Activated

ANSIBLE_ANSIBLE_LINT_RULES_PATH: config
ANSIBLE_ANSIBLE_LINT_CONFIG_FILE: ansible-lint
image

I'm happy to do more test scenarios on request!

If there are any issues or concerns, please let me know, and I will address them promptly.
Thank you for reviewing this PR and I look forward to your feedback.
Tom

@TommyE123 TommyE123 marked this pull request as ready for review June 28, 2024 15:18
@echoix
Copy link
Collaborator

echoix commented Jun 28, 2024

The reasoning seems good, but I still don't understand completely what are the implications, as ansible is not an ecosystem I understand well. I will try to reread all the issues again later or in a couple days.

Maybe @nvuillam knows better?

@TommyE123
Copy link
Contributor Author

Hi @alexanderbazhenoff,

I hope you don't mind me reaching out to you directly. Given your expertise with Ansible, I wanted to see if I could get your thoughts on this PR I've been working on.

Would you be willing to take a look and share any input?
Any insights would be greatly appreciated.

Thank you for your time,
Tom

@nvuillam
Copy link
Member

Looks good to me but let's wait for a real expert's opinion: @alexanderbazhenoff :)

Copy link
Member

@nvuillam nvuillam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks ok to me :)

@nvuillam nvuillam merged commit 632d60c into oxsecurity:main Jun 30, 2024
6 checks passed
@alexanderbazhenoff
Copy link
Contributor

Hi @TommyE123 @nvuillam Sorry for late reply.

Overall the tests look good.
Improvement of activation by checking for .ansible-lint config file is a good news.

@TommyE123 TommyE123 deleted the ansible_improvements branch July 4, 2024 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ansible Linter is not enabled
4 participants