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: Add option to silent the patch output by using --silent flag #699

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

shishir-11
Copy link
Contributor

Add option to silent the patch output by using --silent flag.

copa patch -i $IMAGE --silent

does not produce output except some end INFO.

Closes #643

Copy link

codecov bot commented Jul 11, 2024

Codecov Report

Attention: Patch coverage is 4.34783% with 22 lines in your changes missing coverage. Please review.

Project coverage is 34.02%. Comparing base (9da1246) to head (deb5e30).

Files Patch % Lines
pkg/patch/patch.go 0.00% 21 Missing ⚠️
pkg/patch/cmd.go 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #699      +/-   ##
==========================================
- Coverage   34.22%   34.02%   -0.20%     
==========================================
  Files          18       18              
  Lines        1578     1590      +12     
==========================================
+ Hits          540      541       +1     
- Misses       1007     1018      +11     
  Partials       31       31              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Shishir Kushwaha <[email protected]>
@shishir-11 shishir-11 force-pushed the silent-option-patch branch 2 times, most recently from d563d3a to 9e4308b Compare July 11, 2024 05:03
@shishir-11
Copy link
Contributor Author

the lint check is failing on a piece of code that is unrelated to me ? can someone help me fix that or is it fine eitherways

@ashnamehrotra
Copy link
Contributor

@shishir-11 that lint error was fixed in #682, a rebase should fix that

@shishir-11
Copy link
Contributor Author

@ashnamehrotra can you please check if this is fine or something more needs to be done, thank you.

pkg/patch/cmd.go Outdated
@@ -66,6 +68,7 @@ func NewPatchCmd() *cobra.Command {
flags.DurationVar(&ua.timeout, "timeout", 5*time.Minute, "Timeout for the operation, defaults to '5m'")
flags.StringVarP(&ua.scanner, "scanner", "s", "trivy", "Scanner used to generate the report, defaults to 'trivy'")
flags.BoolVar(&ua.ignoreError, "ignore-errors", false, "Ignore errors and continue patching")
flags.BoolVar(&ua.silent, "silent", false, "silences output while processing")
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we specify that it only silences buildkit output?

@ashnamehrotra
Copy link
Contributor

@shishir-11 this looks good to me! I can do another review once the test pushed

Signed-off-by: Shishir Kushwaha <[email protected]>
Signed-off-by: Shishir Kushwaha <[email protected]>
@shishir-11 shishir-11 force-pushed the silent-option-patch branch from b845b24 to fdb32cf Compare July 19, 2024 19:49
Signed-off-by: Shishir Kushwaha <[email protected]>
@shishir-11
Copy link
Contributor Author

shishir-11 commented Jul 19, 2024

@ashnamehrotra can you please take a look at the failing unit test , do you know why that might be happening , is it possible it lacks permissions to execute apk update

},
{
name: "Silent flag used",
args: []string{"-t", "3.7-alpine-patched", "-i", "alpine:3.14", "--silent"},
Copy link
Contributor

Choose a reason for hiding this comment

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

we should use a different tag here, or omit it and use default

@ashnamehrotra
Copy link
Contributor

@ashnamehrotra can you please take a look at the failing unit test , do you know why that might be happening , is it possible it lacks permissions to execute apk update

It shouldn't need permissions to execute apk update, and copa is able to patch the image locally and run cmd_test.go locally. Could we try running with --debug to see the full error output?

Signed-off-by: Shishir Kushwaha <[email protected]>
Signed-off-by: Shishir Kushwaha <[email protected]>
Signed-off-by: Shishir Kushwaha <[email protected]>
@shishir-11
Copy link
Contributor Author

How do i run debug ? I'm unable to run it .

@ashnamehrotra
Copy link
Contributor

How do i run debug ? I'm unable to run it .

Debug is part of newRootCmd rather than NewPatchCmd since it is a root level flag for copa, which is why it is failing here. I think it would make sense for silent to be the same, can we move it to the newRootCmd() function? We wouldn't be able to add test for this case in cmd_test.go, but it would make more sense to have it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🆕 New
Development

Successfully merging this pull request may close these issues.

[REQ] Add option to "silent" the output
2 participants