Skip to content

fix: false positive in DL3022#900

Merged
lorenzo merged 4 commits intohadolint:masterfrom
swarnimarun:patch-1
Nov 16, 2022
Merged

fix: false positive in DL3022#900
lorenzo merged 4 commits intohadolint:masterfrom
swarnimarun:patch-1

Conversation

@swarnimarun
Copy link
Copy Markdown
Contributor

What I did

Simple patch to fix bug with hadolint raising false positives if the stages are mentioned by index rather than names.
Such as,

FROM X as stage1
RUN # do some work, likely, build something to copy

FROM Y as stage2
COPY --from=0 bar .

How I did it

I use decimal conversion over string to get the index and compare it to the count of the stages in source, not sure if there was a better way to write the code I just wrote it because I needed it to be fixed.

How to verify it

Added a couple tests to DL3022Spec.hs

Note: Let me know if there are any mistakes I will fix em. Though as it's a weekend I might not always be available.

@swarnimarun
Copy link
Copy Markdown
Contributor Author

swarnimarun commented Nov 12, 2022

sorry had to add another test as I remembered this case was broken, also fixed it :P

fixed it with a bit of hack (names aliases can't be plain numbered indexes, so it should be perfectly fine, but it's a hack nonetheless), would have had to add more stuff to state otherwise.

@lorenzo
Copy link
Copy Markdown
Member

lorenzo commented Nov 14, 2022

I would change the state to be able to store numbers as well. It feels messy having to parse a number each time you want to check the state.

Do you think you can make that change?

@swarnimarun
Copy link
Copy Markdown
Contributor Author

@lorenzo let me know if it's fine now, not that good at Haskell. But tried to follow project conventions.

I couldn't come up with anything more beautiful so feel free to make edits. :P

@swarnimarun
Copy link
Copy Markdown
Contributor Author

Also sorry if I don't follow up immediately a bit loaded on the work side of life.

@lorenzo
Copy link
Copy Markdown
Member

lorenzo commented Nov 16, 2022

looks great, thanks!

@lorenzo lorenzo merged commit a39f857 into hadolint:master Nov 16, 2022
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.

2 participants