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

Building a WASM Bundle with multiple entrypoints specified via metadata annotation will result in a panic #6661

Closed
dxh9845 opened this issue Apr 2, 2024 · 5 comments · Fixed by #6667
Labels

Comments

@dxh9845
Copy link
Contributor

dxh9845 commented Apr 2, 2024

Short description

Building an OPA Bundle with WASM as the targer results in a nil panic. I can replicate this consistently in both Linux and Mac Darwin Arm64 variants of OPA. It appears that this behavior was introduced in version v0.59.0

[panic: runtime error: index out of range [6] with length 6

goroutine 1 [running]:
github.com/open-policy-agent/opa/compile.(*Compiler).compilePlan(0x140000c8140, {0x1036c90c0?, 0x302030203030303?})
	github.com/open-policy-agent/opa/compile/compile.go:597 +0xb50
github.com/open-policy-agent/opa/compile.(*Compiler).compileWasm(0x140000c8140, {0x103e5c438, 0x1047b9f60})
	github.com/open-policy-agent/opa/compile/compile.go:663 +0x130
github.com/open-policy-agent/opa/compile.(*Compiler).Build(0x140000c8140, {0x103e5c438, 0x1047b9f60})
	github.com/open-policy-agent/opa/compile/compile.go:335 +0x12c
github.com/open-policy-agent/opa/cmd.dobuild({0x1400000eac8, 0x14000532330, 0x1, 0x1, 0x0, {{0x140000be080, 0x6, 0x8}, 0x1}, {0x16d2b28b4, ...}, ...}, ...)
	github.com/open-policy-agent/opa/cmd/build.go:327 +0x7e8
github.com/open-policy-agent/opa/cmd.init.1.func2(0x140000ce700?, {0x140000fa480?, 0x4?, 0x103686844?})
	github.com/open-policy-agent/opa/cmd/build.go:226 +0x50
github.com/spf13/cobra.(*Command).execute(0x140000ee608, {0x140000fa300, 0x17, 0x18})
	github.com/spf13/cobra@v1.8.0/command.go:987 +0x828
github.com/spf13/cobra.(*Command).ExecuteC(0x104738860)
	github.com/spf13/cobra@v1.8.0/command.go:1115 +0x344
github.com/spf13/cobra.(*Command).Execute(0x104624008?)
	github.com/spf13/cobra@v1.8.0/command.go:1039 +0x1c
main.main()
	github.com/open-policy-agent/opa/main.go:14 +0x24
make: *** [bundle] Error 2](https://github.com/dxh9845/opa-broken-wasm-build-poc)

Examples:

  • For server and CLI, the flags/configuration that you provided to OPA:
opa build  --prune-unused -b bundle -t wasm

Steps To Reproduce

  1. Clone test repo: https://github.com/dxh9845/opa-broken-wasm-build-poc
  2. Build the OPA bundle with the following command:
make build
  1. See a panic from an index out of bounds:
opa build  --prune-unused -b bundle -t wasm
panic: runtime error: index out of range [2] with length 2

goroutine 1 [running]:
github.com/open-policy-agent/opa/compile.(*Compiler).compilePlan(0x140003dd680, {0x1039f90c0?, 0x3b09?})
	github.com/open-policy-agent/opa/compile/compile.go:597 +0xb50
github.com/open-policy-agent/opa/compile.(*Compiler).compileWasm(0x140003dd680, {0x10418c438, 0x104ae9f60})
	github.com/open-policy-agent/opa/compile/compile.go:663 +0x130
github.com/open-policy-agent/opa/compile.(*Compiler).Build(0x140003dd680, {0x10418c438, 0x104ae9f60})
	github.com/open-policy-agent/opa/compile/compile.go:335 +0x12c
github.com/open-policy-agent/opa/cmd.dobuild({0x140003e98d8, 0x1400050e6f0, 0x1, 0x1, 0x0, {{0x0, 0x0, 0x0}, 0x0}, {0x1039c093c, ...}, ...}, ...)
	github.com/open-policy-agent/opa/cmd/build.go:327 +0x7e8
github.com/open-policy-agent/opa/cmd.init.1.func2(0x140001ad600?, {0x140003cab40?, 0x4?, 0x1039b6844?})
	github.com/open-policy-agent/opa/cmd/build.go:226 +0x50
github.com/spf13/cobra.(*Command).execute(0x140003bac08, {0x140003caaf0, 0x5, 0x5})
	github.com/spf13/cobra@v1.8.0/command.go:987 +0x828
github.com/spf13/cobra.(*Command).ExecuteC(0x104a68860)
	github.com/spf13/cobra@v1.8.0/command.go:1115 +0x344
github.com/spf13/cobra.(*Command).Execute(0x104954008?)
	github.com/spf13/cobra@v1.8.0/command.go:1039 +0x1c
main.main()
	github.com/open-policy-agent/opa/main.go:14 +0x24
make: *** [build] Error 2```

## Expected behavior
The bundle should be built and output. 

## Additional context
I use the "--prune-unused" flag to build the WASM bundle. I was able to recreate this with any version above v0.58.0, including the latest (v0.63.0 at time of writing)
@dxh9845 dxh9845 added the bug label Apr 2, 2024
@dxh9845
Copy link
Contributor Author

dxh9845 commented Apr 2, 2024

I'm working on recreating a minimal example - will post when that's complete

@dxh9845 dxh9845 changed the title Building WASM bundle in versions greather than 0.59.0 results in a index out of bounds panic Building WASM bundle in versions greater than 0.59.0 results in a index out of bounds panic Apr 2, 2024
@philipaconrad
Copy link
Contributor

philipaconrad commented Apr 3, 2024

Using just the panic backtrace, I have good reason to believe that in compile/compile.go, the c.entrypoints and c.entrypointrefs slices are ending up being different lengths while iterating over the elements of the larger slice, which is how the index out of bounds panic is happening.

I'll keep poking around to see if there's any obvious spots where c.entrypointrefs is getting appended-to when c.entrypoints isn't.

@philipaconrad
Copy link
Contributor

So, after some back-and-forth with @dxh9845 in the OPA Slack, I have good reason to believe the issue may actually be in how we handle entrypoint metadata annotations, specifically, it looks like we may be messing up cases where the entrypoint is seen both on the CLI and via an entrypoint annotation.

@dxh9845 dxh9845 changed the title Building WASM bundle in versions greater than 0.59.0 results in a index out of bounds panic Building a WASM Bundle with multiple entrypoints specified via metadata annotation will result in a panic Apr 3, 2024
@dxh9845
Copy link
Contributor Author

dxh9845 commented Apr 3, 2024

Was able to recreate a minimal POC after narrowing down the problem. https://github.com/dxh9845/opa-broken-wasm-build-poc

@philipaconrad
Copy link
Contributor

I now have a minimal testcase for repro in the compile package for the overlapping CLI + entrypoint annotation case that panics. I'm checking one other possible case that needs testing while I'm here, and then I'll start work on fixing the breakage(s).

philipaconrad added a commit to philipaconrad/opa that referenced this issue Apr 3, 2024
This commit fixes a panic that could occur when `opa build` was provided
an entrypoint from both a CLI flag, and via entrypoint metadata
annotation.

The fix is simple: deduplicate the slice of entrypoint refs that the
compiler uses, before compiling WASM or Plan targets.

Fixes: open-policy-agent#6661

Signed-off-by: Philip Conrad <philipaconrad@gmail.com>
philipaconrad added a commit to philipaconrad/opa that referenced this issue Apr 3, 2024
This commit fixes a panic that could occur when `opa build` was provided
an entrypoint from both a CLI flag, and via entrypoint metadata
annotation.

The fix is simple: deduplicate the slice of entrypoint refs that the
compiler uses, before compiling WASM or Plan targets.

Fixes: open-policy-agent#6661

Signed-off-by: Philip Conrad <philipaconrad@gmail.com>
philipaconrad added a commit to philipaconrad/opa that referenced this issue Apr 3, 2024
This commit fixes a panic that could occur when `opa build` was provided
an entrypoint from both a CLI flag, and via entrypoint metadata
annotation.

The fix is simple: deduplicate the slice of entrypoint refs that the
compiler uses, before compiling WASM or Plan targets.

Fixes: open-policy-agent#6661

Signed-off-by: Philip Conrad <philipaconrad@gmail.com>
philipaconrad added a commit to philipaconrad/opa that referenced this issue Apr 3, 2024
This commit fixes a panic that could occur when `opa build` was provided
an entrypoint from both a CLI flag, and via entrypoint metadata
annotation.

The fix is simple: deduplicate the slice of entrypoint refs that the
compiler uses, before compiling WASM or Plan targets.

Fixes: open-policy-agent#6661

Signed-off-by: Philip Conrad <philipaconrad@gmail.com>
philipaconrad added a commit to philipaconrad/opa that referenced this issue Apr 3, 2024
This commit fixes a panic that could occur when `opa build` was provided
an entrypoint from both a CLI flag, and via entrypoint metadata
annotation.

The fix is simple: deduplicate the slice of entrypoint refs that the
compiler uses, before compiling WASM or Plan targets.

Fixes: open-policy-agent#6661

Signed-off-by: Philip Conrad <philipaconrad@gmail.com>
philipaconrad added a commit to philipaconrad/opa that referenced this issue Apr 3, 2024
This commit fixes a panic that could occur when `opa build` was provided
an entrypoint from both a CLI flag, and via entrypoint metadata
annotation.

The fix is simple: deduplicate the slice of entrypoint refs that the
compiler uses, before compiling WASM or Plan targets.

Fixes: open-policy-agent#6661

Co-authored-by: Daniel Herzig <danielherzig96@gmail.com>
Signed-off-by: Philip Conrad <philipaconrad@gmail.com>
philipaconrad added a commit to philipaconrad/opa that referenced this issue Apr 3, 2024
This commit fixes a panic that could occur when `opa build` was provided
an entrypoint from both a CLI flag, and via entrypoint metadata
annotation.

The fix is simple: deduplicate the slice of entrypoint refs that the
compiler uses, before compiling WASM or Plan targets.

Fixes: open-policy-agent#6661

Co-authored-by: Daniel Herzig <danielherzig96@gmail.com>
Signed-off-by: Philip Conrad <philipaconrad@gmail.com>
philipaconrad added a commit to philipaconrad/opa that referenced this issue Apr 3, 2024
This commit fixes a panic that could occur when `opa build` was provided
an entrypoint from both a CLI flag, and via entrypoint metadata
annotation.

The fix is simple: deduplicate the slice of entrypoint refs that the
compiler uses, before compiling WASM or Plan targets.

Fixes: open-policy-agent#6661

Co-authored-by: Daniel Herzig <danielherzig96@gmail.com>
Signed-off-by: Philip Conrad <philipaconrad@gmail.com>
ashutosh-narkar pushed a commit that referenced this issue Apr 3, 2024
This commit fixes a panic that could occur when `opa build` was provided
an entrypoint from both a CLI flag, and via entrypoint metadata
annotation.

The fix is simple: deduplicate the slice of entrypoint refs that the
compiler uses, before compiling WASM or Plan targets.

Fixes: #6661

Co-authored-by: Daniel Herzig <danielherzig96@gmail.com>
Signed-off-by: Philip Conrad <philipaconrad@gmail.com>
tsidebottom pushed a commit to tsidebottom/opa- that referenced this issue Apr 17, 2024
This commit fixes a panic that could occur when `opa build` was provided
an entrypoint from both a CLI flag, and via entrypoint metadata
annotation.

The fix is simple: deduplicate the slice of entrypoint refs that the
compiler uses, before compiling WASM or Plan targets.

Fixes: open-policy-agent#6661

Co-authored-by: Daniel Herzig <danielherzig96@gmail.com>
Signed-off-by: Philip Conrad <philipaconrad@gmail.com>
Signed-off-by: Thomas Sidebottom <thomas.sidebottom@va.gov>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants