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

Fixes Regex to match only container files more strictly #4867

Merged
merged 7 commits into from
Nov 15, 2023

Conversation

Chocrates
Copy link
Contributor

@Chocrates Chocrates commented Nov 13, 2023

Fixes #4846

Proposed Changes

We check for files with the name (case insensitive):

  • Dockerfile
  • Containerfile
  • .Dockerfile
  • .Containerfile

This appears to protect us against catching files with a name like Dockerfile.md

I tested the regex like so:

Screenshot from 2023-11-13 16-07-07

Readiness Checklist

Author/Contributor

  • If documentation is needed for this change, has that been included in this pull request

Reviewing Maintainer

  • Label as breaking if this is a large fundamental change
  • Label as either automation, bug, documentation, enhancement, infrastructure, or performance

Sorry, something went wrong.

Verified

This commit was signed with the committer’s verified signature.
bigdaz Daz DeBoer
…ontainer Dockerfile but that are not actually dockerfiles
@zkoppert zkoppert self-assigned this Nov 13, 2023
@zkoppert zkoppert added bug Something isn't working enhancement New feature or request and removed enhancement New feature or request labels Nov 13, 2023
zkoppert
zkoppert previously approved these changes Nov 13, 2023
Copy link
Member

@zkoppert zkoppert left a comment

Choose a reason for hiding this comment

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

Great! Thank you! Appreciate the testing!

@Chocrates
Copy link
Contributor Author

I haven't run an end to end test on this sha yet, I need to read up on running it locally. Do you know what the testing errors are about? I haven't dug in to them deeply but looks like it was expecting ansible tests that weren't found?

https://github.com/super-linter/super-linter/actions/runs/6856308690/job/18644478272?pr=4867#step:9:1936

@zkoppert
Copy link
Member

I'll take a look. 👀

@zkoppert
Copy link
Member

@ferrarimarco
Copy link
Collaborator

I think there's an extra space before the regex. :)

@ferrarimarco
Copy link
Collaborator

Also, I wonder if we can add some test cases for this regex...

Copy link
Member

@zkoppert zkoppert left a comment

Choose a reason for hiding this comment

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

Think we got this one dialed in now.

@zkoppert zkoppert added this pull request to the merge queue Nov 15, 2023
Merged via the queue into super-linter:main with commit 6c7dc51 Nov 15, 2023
@Chocrates Chocrates deleted the fix-file-list branch November 15, 2023 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hadolint catching non dockerfiles
3 participants