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

why CombinedOutput instead of cmd.Output? #79

Open
fgiloux opened this issue Sep 21, 2021 · 5 comments
Open

why CombinedOutput instead of cmd.Output? #79

fgiloux opened this issue Sep 21, 2021 · 5 comments

Comments

@fgiloux
Copy link
Contributor

fgiloux commented Sep 21, 2021

In pkg/helpers.go the RunCommand function uses cmd.CombinedOutput. I am wondering why this was chosen instead of cmd.Output.
An issue with it is that when the container engines sends warnings they are added to the result sent to the standard output and this for instance cannot be unmarshaled from json in: if err := json.Unmarshal(output, &dockerInspect); err != nil { of the same file.

This resulted in the following error message:
unable to inspect the index image: invalid character 'i' in literal true (expecting 'r')

@camilamacedo86
Copy link
Collaborator

Hi @fgiloux,

That was used to get the result combined. Note that CombinedOutput runs a command and returns combined stdout and stderr. However, please feel free to propose changes to it as you see fits.

@camilamacedo86
Copy link
Collaborator

Hi @fgiloux,

Is this issue still required or we can close this one?

@fgiloux
Copy link
Contributor Author

fgiloux commented Feb 1, 2022

Hi @camilamacedo86. I don't think the issue disappeared. As soon as the container engine generates warnings things get broken

@camilamacedo86
Copy link
Collaborator

@fgiloux so, what exactly brokes?

If > if err := json.Unmarshal(output, &dockerInspect); err != nil {
fails then the error would be added to audit.errors and we have this data in the report

@fgiloux
Copy link
Contributor Author

fgiloux commented Apr 5, 2022

@camilamacedo86 the thing is that Unmarshal will not only fail when you have an error but also when the container engine outputs whatever message, like a warning that may not prevent the processing but now does because it is no proper json

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

No branches or pull requests

2 participants