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(cli): add new flag --max-file-size to control the max file size #6670

Merged
merged 29 commits into from
Dec 20, 2023

Conversation

tomk-orca
Copy link
Contributor

@tomk-orca tomk-orca commented Aug 27, 2023

Closes #6306
Closes #5989

Proposed Changes
-implement a flag, control the size of the files scanned
-the flag should have as default value 5MB

I submit this contribution under the Apache-2.0 license.

@github-actions github-actions bot added community Community contribution feature request Community: new feature request labels Aug 27, 2023
@tomk-orca tomk-orca changed the title feat(CLI): add new flag --max-file-size to controle the max file size… update(CLI): add new flag --max-file-size to controle the max file size Aug 27, 2023
@tomk-orca tomk-orca changed the title update(CLI): add new flag --max-file-size to controle the max file size feat(CLI): add new flag --max-file-size to controle the max file size Aug 27, 2023
@github-actions github-actions bot added feature request Community: new feature request and removed feature request Community: new feature request labels Aug 27, 2023
@tomk-orca tomk-orca changed the title feat(CLI): add new flag --max-file-size to controle the max file size feat(cli): add new flag --max-file-size to control the max file size Aug 27, 2023
@tomk-orca tomk-orca force-pushed the 6306 branch 3 times, most recently from ae2b50f to d099129 Compare August 27, 2023 13:50
Copy link
Contributor

@pereiramarco011 pereiramarco011 left a comment

Choose a reason for hiding this comment

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

Left a couple of comments, other than that, and Lior's comment LGTM 😄

Keep up the good work 👍

internal/console/assets/scan-flags.json Outdated Show resolved Hide resolved
e2e/fixtures/assets/scan_help Outdated Show resolved Hide resolved
Copy link
Contributor

@JoaoAtGit JoaoAtGit left a comment

Choose a reason for hiding this comment

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

Hi @tomk-orca,
how are you?
I was checking your changes and to me, they look good.
I just have a request, on the file


you can add a line more or less like this
log.Info().Msgf("Max file size: %d Mb", flags.GetIntFlag(flags.MaxFileSizeFlag)) and please write an e2e test.

@JoaoAtGit
Copy link
Contributor

Hi @tomk-orca, any update on this pull request?

JoaoAtGit
JoaoAtGit previously approved these changes Dec 12, 2023
Copy link
Contributor

@gabriel-cx gabriel-cx left a comment

Choose a reason for hiding this comment

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

@JoaoCxMartins kindly add this new command in this file, so the users can check it in our doc webpage.

docs/commands.md Outdated Show resolved Hide resolved
e2e/fixtures/assets/scan_help Outdated Show resolved Hide resolved
internal/console/assets/scan-flags.json Outdated Show resolved Hide resolved
gabriel-cx
gabriel-cx previously approved these changes Dec 14, 2023
JoaoAtGit
JoaoAtGit previously approved these changes Dec 14, 2023
@asofsilva
Copy link
Contributor

Hi @tomk-orca,

could you please add an E2E test where you can scan a file larger than 5MB to ensure that the file is not scanned? You can also ensure that, when using --max-file-size flag, the file is scanned.

Thank you :)

@kaplanlior
Copy link
Contributor

Hi @tomk-orca,

could you please add an E2E test where you can scan a file larger than 5MB to ensure that the file is not scanned? You can also ensure that, when using --max-file-size flag, the file is scanned.

Thank you :)

We should add them for now, as he is probably unenviable for the E2E tests.

Please don't block due to this, we can even add a task to complete those as a separate issue.

@pereiramarco011 pereiramarco011 dismissed stale reviews from gabriel-cx, JoaoAtGit, and themself via 8730faf December 19, 2023 15:50
cxMiguelSilva
cxMiguelSilva previously approved these changes Dec 19, 2023
@pereiramarco011 pereiramarco011 merged commit d6a8be2 into Checkmarx:master Dec 20, 2023
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Community contribution feature request Community: new feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add an option to override the max size limit file Kubernetes scan skips Pods due to file size
8 participants