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(rego): accept fs.FS for data #1191

Merged
merged 5 commits into from
Feb 21, 2023
Merged

Conversation

knqyf263
Copy link
Contributor

@knqyf263 knqyf263 commented Feb 19, 2023

Add a new option to pass the data file system so that the file system can be abstract.
aquasecurity/trivy#3578

@knqyf263 knqyf263 self-assigned this Feb 19, 2023
@knqyf263 knqyf263 changed the title feat(rego): add a new option to pass data feat(rego): add a new scanner option to pass data Feb 19, 2023
@chen-keinan
Copy link
Contributor

chen-keinan commented Feb 19, 2023

@chen-keinan What if passing data directly rather than creating a temp file? I've created a PoC. Would you take a look?

since trivy already support accepting rego data, maybe it better to enhance rego data to accept in memory FS rather than real fs and use the current logic exist in trivy , probably will be much simpler, wdyt ?

@knqyf263
Copy link
Contributor Author

memfs is supported for input, but not for data. As far as I understand, OPA cannot mix real files and Go structs.
https://github.com/open-policy-agent/opa/blob/c0360f1af571c1087be574a361d06b8961b1fb93/loader/loader.go#L121-L127

We need to load files, create a memory filesystem, and add the Go struct there, then we can pass it to FileLoader. Passing map[string]any to loader.Result looks simpler to me. But I might be wrong as I'm not following Rego updates recently.

Also, I think data is likely passed as a Go struct in the OPA world. It doesn't have to be a file.
https://github.com/open-policy-agent/opa/blob/c0360f1af571c1087be574a361d06b8961b1fb93/rego/example_test.go#L324-L335

@knqyf263
Copy link
Contributor Author

I've updated PR to accept fs.FS for data according to Chen's suggestion.
c2f5040

@knqyf263 knqyf263 changed the title feat(rego): add a new scanner option to pass data feat(rego): accept fs.FS for data Feb 20, 2023
pkg/scanners/helm/scanner.go Outdated Show resolved Hide resolved
pkg/rego/load.go Outdated Show resolved Hide resolved
@chen-keinan
Copy link
Contributor

lgtm

chen-keinan
chen-keinan previously approved these changes Feb 20, 2023
@knqyf263
Copy link
Contributor Author

@simar7 @giorod3 Could you take a look? We just want to load the OPA data from fs.FS.

@simar7
Copy link
Member

simar7 commented Feb 20, 2023

@knqyf263 the idea seems fine to me but how will we expose this to the callers? More specifically, would any trivy flags would need to be changed OR internally instead of creating temp files, the caller would pass along something that implements fs.FS?

@knqyf263
Copy link
Contributor Author

knqyf263 commented Feb 21, 2023

the caller would pass along something that implements fs.FS?

Yes, but it would be simple. If the caller just wants to use defsec as before, they can use os.DirFS, which is a wrapper of a real file system.
https://pkg.go.dev/os#DirFS

In Trivy, we create fs.FS for data like we already do for policy.
https://github.com/aquasecurity/trivy/blob/641e75af6a27144794c1d62f0d0e1df5c00c2f4d/pkg/misconf/scanner.go#L229-L252

Copy link
Member

@simar7 simar7 left a comment

Choose a reason for hiding this comment

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

lgtm

@simar7 simar7 merged commit 9b3d8bd into aquasecurity:master Feb 21, 2023
@knqyf263 knqyf263 deleted the feat/pass_data branch February 22, 2023 09:27
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.

3 participants