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

feat: multiple file scan support #106

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

feat: multiple file scan support #106

wants to merge 6 commits into from

Conversation

06kellyjac
Copy link
Member

@06kellyjac 06kellyjac commented Sep 10, 2020

Can now provide several files to kubesec scan to be validated.

Resolves #103

@06kellyjac 06kellyjac marked this pull request as draft September 10, 2020 17:22
@06kellyjac
Copy link
Member Author

Back to draft as there's some big ol race condition issues

The change to have multi-file scanning done async was causing bad race
conditions.
It was crashing with the following: `fatal error: concurrent map read and map write`
And a lot of goroutine stack traces.
After some investigation the old versions can have a goroutine crash
if you pretty much DOS it but with the previous change it would crash
very quickly.
I tried go's `--race` feature but it couldn't find the issue.
Adding additional debug logging didn't really help either.

Synchronous is good enough here as this is just for the local scan.
We can investigate this more indepth another time.
The changes made meant that it could return null rather than an error.

The original behavior isn't ideal as a not particularly fatal issue of
having a single empty file in multiple provided causes the whole thing
to stop and return an error.

It'll probably take a breaking change to return the results + an error
with a log. Or just looking at some existing error output structures
from kubeval and making it similar.
@06kellyjac 06kellyjac marked this pull request as ready for review September 17, 2020 15:29
@06kellyjac 06kellyjac self-assigned this Jan 7, 2021
@06kellyjac 06kellyjac marked this pull request as draft January 7, 2021 16:36
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.

How to recursively scan all *.yaml files
1 participant