fix: print logs if any error appears#2725
Conversation
| args = append(args, | ||
| fmt.Sprintf(`&& bzip2 -c /tmp/scan/%s | base64`, resultFileName), | ||
| ) | ||
| return []string{"/bin/sh"}, append([]string{"-c"}, strings.Join(args, " ")) |
There was a problem hiding this comment.
TBH I'm not a huge fan of running a shell to capture commands as it can be used in a malicious way to inject something.
WDYT if we remove the reliance on arbitrary shell commands and capture the output (and the error codes) of Go library commands (such as compress logs, encode to base64, capture the output etc.?
There was a problem hiding this comment.
@simar7 sorry, it seems I misunderstood something
These commands are executed inside a separate k8s Job based on the Trivy image.
I didn’t fully understand your point about using Go code directly.
To be honest, I’m also not a big fan of reading the report and/or the logs as container logs, but this was done by design and I don’t have a better solution at the moment. tried using volumes, but they also come with their own overhead.
There was a problem hiding this comment.
I think we can take this opportunity to get rid of getCommandAndArgs completely and replace it with actual library calls. Since we build these args and eventually call trivy, we should really use trivy as a library rather than call trivy binary.
There was a problem hiding this comment.
Actually, we don’t call Trivy directly — instead, we create a Kubernetes Job based on the Trivy image. So this is a much more complex setup than it might appear at first glance.
Changing this approach would significantly impact the application’s architecture and needs to be carefully considered. For example, at the moment, Kubernetes handles Job scheduling automatically, but with such a change, image scanning workloads could start overloading a specific node, among other potential issues.
Therefore, I’d suggest discussing this idea separately before making any architectural decisions.
As for this PR, it simply restores log output in case of errors, which will greatly improve troubleshooting (this behavior was present before #2591).
There was a problem hiding this comment.
Fair enough. Can we open an issue to change this in a different PR? I think we should find a way to not rely on arbitrary shell commands within the operator.
|
i also got the same issue when we can expect this to get merge in main branch ? |
afdesk
changed the title
3 participants
Description
After changes introduced in #2591 , all console output was redirected to a file. As a result, error messages were silently ignored and not visible in the pod logs.
This PR addresses the issue by checking the exit code after execution. If an error occurs, the log file content is printed to the console, making debugging easier and preventing error messages from being overlooked.
Also there was updated a log message for unsupported OS/arch.
Before:
{"level":"error","ts":"2025-08-28T10:35:43Z","logger":"reconciler.scan job","msg":"Scan job container","job":"trivy-system/scan-vulnerabilityreport-5c6d5f497b","container":"trivy-operator","status.reason":"Error","status.message":"","stacktrace":"github.com/aquasecurity/trivy-operator/pkg/vulnerabilityreport/controller.(*ScanJobController).completedContainers\n\t/Users/amf/aqua/trivy-operator/pkg/vulnerabilityreport/controller/scanjob.go:441..."}After:
{"level":"info","ts":"2025-08-28T10:11:23Z","logger":"reconciler.scan job","msg":"Scan job container","job":"trivy-system/scan-vulnerabilityreport-68bb4cf948","container":"trivy-operator","status.reason":"Error","status.message":"By default, only Linux amd64 images are supported for scanning."}Related issues
Checklist