feat: add support for yaml.unmarshal builtin #100
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This uses the "yaml" npm package to provide support for unmarshalling YAML strings within rego via the yaml.unmarshal() function. Test functionality has been included to verify parsing and handling of syntax, metadata, reference and warnings to match the current behavior of the opa tool. Signed-off-by: Aron Carroll <aron.carroll@snyk.io>
aron
force-pushed
the
feat/yaml-builtin
branch
from
04e1e5b
to
21ee8f4
Compare
srenatus
approved these changes
Dec 10, 2021
|
@srenatus thanks!
Are there any docs on how to do this? I'd be happy to take a look as this functionality is something we're keen to start using. |
|
Here's the gist: You'd add the |
This was referenced Dec 10, 2021
aron
added a commit
to snyk/cli
that referenced
this pull request
Jan 4, 2022
#### What does this PR do? This PR bumps @open-policy-agent/opa-wasm to [1.6.0](https://github.com/open-policy-agent/npm-opa-wasm/releases/tag/1.6.0) to include support for the `yaml.unmarshal()` function in Rego and support for Node 10. This allows the `snyk iac` command to support additional policies that unmarshall YAML content within Terraform configuration. #### Where should the reviewer start? package.json is the primary change here but the implementation of `yaml.unmarshal()` is here: open-policy-agent/npm-opa-wasm#100 This also removes the use of the `mock-fs` package in the file-scanner.spec.ts test as the opa-wasm package will use `console.error` to write out a warning to stderr when loading the wasm fixture and this somehow causes `mock-fs` to throw the following error bringing the test suite down: ``` ENOENT: no such file or directory, lstat '/Users/Aron/Code/snyk/node_modules/@jest/console/node_modules' ``` Rather than debug this weird behavior we've decided to just mock out the function that loads the fixtures directly in the test suite. #### How should this be manually tested? [rules.zip](https://github.com/snyk/snyk/files/7719060/rules.zip) Download the above zip file containing a custom rules bundle and unzip: With the `snyk-iac-rules` tool build a custom bundle: ``` % snyk-iac-rules build ``` Then test the fixture with the latest snyk cli: ``` % snyk-dev iac test --rules=bundle.tar.gz rules/HELLO/fixtures/sg.tf ``` You should see 15 issues found vs. 14 when run with current snyk. #### What are the relevant tickets? [CFG-1271]
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This uses the "yaml" npm package to provide support for unmarshalling YAML strings within rego via the
yaml.unmarshal()function. The implementation itself is straightforward. I've added the src/builtins/yaml.js file as per the strings.js example and exposed theyaml.parse()function under the"yaml.unmarshal"key.There's a bit of awkwardness to silence the default warnings emitted by the "yaml" library which required setting a
YAML_SILENCE_WARNINGSglobal and then resetting it to avoid conflicts.Choices for YAML parser in the JS ecosystem seem to fall between yaml and js-yaml. Both are actively maintained. I went with
yamlbecause it has zero dependencies and in our testing at Snyk on a wide variety of YAML fixtures it has the closest alignment with the ghodss/yaml YAML library used by OPA.Test functionality has been included in the test/opa-yaml-support.test.js file. This verifies parsing of a generic YAML structure. Handling of syntax, metadata, reference and warnings has also been exercised using fixtures from the yaml test suite, these are all ignored.
I've run the
fmtandlintcommands on the source as well asopa fmton the rego fixtures.