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.

Sign up for GitHub

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

Jump to bottom

transform: validate wasm binaries at deploy time #16290

Merged
merged 11 commits into from
Feb 6, 2024
Merged

transform: validate wasm binaries at deploy time #16290

merged 11 commits into from
Feb 6, 2024

Conversation

rockwotj
Copy link
Contributor

As part of the multiple output topics project, we will be supporting multiple ABI versions.
To help reduce user confusion when the case of when the SDK is newer than Redpanda, we will
validate the versions we support at deploy time. As we currently will provide backwards compatibility,
but not forwards compatibility.

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v23.3.x
  • v23.2.x
  • v23.1.x

Release Notes

Improvements

  • Validate transform code at deploy time to ensure the correct SDK is used.

@rockwotj rockwotj requested review from gousteris and removed request for a team January 25, 2024 17:12
@rockwotj
Copy link
Contributor Author

This is blocked on the corresponding vtools changes

@rockwotj rockwotj self-assigned this Jan 25, 2024
@rockwotj
cmake: add wat feature to wasmtime
0c830a5 
For testing, it will be nice to be able to define these tests using the
WAT format. Enable the feature.

Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>
@rockwotj
cmake: upgrade wasmtime
c2f6741 
Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>
@rockwotj
third_party: update rust crates
73ff2a1 
We upgraded wasmtime and the features we use, so update the crates too

Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>
@github-actions github-actions bot added the area/wasm WASM Data Transforms label Jan 25, 2024
@vbotbuildovich
Copy link
Collaborator

vbotbuildovich commented Jan 26, 2024 •
edited

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/44321#018d4303-0ddf-4ae5-a558-2dc0d50feb7a

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/44321#018d4303-0dd6-4903-927c-d89505bef9df

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/44321#018d4304-9021-405d-9b26-482da3599a38

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/44321#018d4304-9026-4618-bbb7-efa1bf0859a2

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/44327#018d5052-783e-4981-aa70-51ddbd41f3df

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/44718#018d7b90-c748-47ea-997b-d46c66cc8f66

@rockwotj
Copy link
Contributor Author

Force push: push a v1 vs v2 wasm spec difference

@redpanda-data redpanda-data deleted a comment from vbotbuildovich Jan 26, 2024
@rockwotj
Copy link
Contributor Author

Our friend resource_mgmt_rpunit strikes again.

@oleiman
Copy link
Member

oleiman commented Jan 26, 2024

Our friend resource_mgmt_rpunit strikes again.

Pretty sure this is timing out on every CI run, currently.

oleiman
oleiman reviewed Feb 5, 2024
View reviewed changes
Copy link
Member

@oleiman oleiman left a comment

Choose a reason for hiding this comment

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

Handful of non-pressing questions, and I feel like the plumbing might benefit from a DT test. Otherwise looks great to me.

src/v/wasm/parser/tests/leb128_test.cc Show resolved Hide resolved
src/v/wasm/parser/include/wasm/parser/parser.h Show resolved Hide resolved
src/v/wasm/parser/parser.cc Show resolved Hide resolved
src/v/wasm/parser/parser.cc Show resolved Hide resolved
src/v/wasm/parser/parser.cc Outdated Show resolved Hide resolved
src/v/wasm/parser/parser.cc Show resolved Hide resolved
src/v/wasm/parser/README.md Outdated Show resolved Hide resolved
src/v/redpanda/admin/transform.cc Show resolved Hide resolved
src/v/redpanda/admin/server.cc Show resolved Hide resolved
@rockwotj
wasm/parser: add leb128 encoding/decoding
1c7baea 
Parsing wasm modules uses leb128 for numbers, so introduce a decoder for
this with good tests.

Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>
@rockwotj
wasm/parser: create a parser for wasm modules
3b1a131 
This will allow us to extract imports and exports, so we can validate
our ABI during deploys.

Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>
@rockwotj
wasm/parser: add module documentation
8077547 
Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>
@rockwotj
wasm: support deploy time validation
c6273ab 
Support validating a wasm module without the expensive of compiling and
running it. The validation is mostly a smoke test of the different
things that could go wrong. It's still possible to have a module that
does something "wrong" from an ABI prospective. I think this is good
enough because at some point you have to run the module to make sure it
does the right thing.

Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>
@rockwotj
transform/deploy: validate modules
438b339 
This does valiation of the WebAssembly binary during deploys.

Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>
@rockwotj
transform: use std::error_code consistently
6784b5e 
In the case of deploys errors could come from multiple different
subsystems, so we will be consistent and always return std::error_code
for all public methods in the transform subsystem.

Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>
@rockwotj
wasm: better error messages
fccae9c 
Have more understandable error messages for users when they deploy
functions, this will also help ducktape distinguish between them.

Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>
@rockwotj
ducktape/transforms: add tests for wasm validation
e714c01 
Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>
@rockwotj rockwotj requested a review from oleiman February 5, 2024 22:01
oleiman
oleiman approved these changes Feb 5, 2024
View reviewed changes
Copy link
Member

@oleiman oleiman left a comment

Choose a reason for hiding this comment

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

Nice, LGTM

@rockwotj
Copy link
Contributor Author

rockwotj commented Feb 6, 2024

/ci-repeat 4
skip-redpanda-build
skip-unit
dt-repeat=128
tests/rptest/tests/data_transforms_test.py

@rockwotj rockwotj merged commit 5650f5d into redpanda-data:dev Feb 6, 2024
18 checks passed
@rockwotj rockwotj deleted the validate branch February 6, 2024 08:24
@vbotbuildovich
Copy link
Collaborator

/backport v23.3.x

@vbotbuildovich
Copy link
Collaborator

Failed to create a backport PR to v23.3.x branch. I tried:

git remote add upstream https://github.com/redpanda-data/redpanda.git
git fetch --all
git checkout -b backport-pr-16290-v23.3.x-649 remotes/upstream/v23.3.x
git cherry-pick -x 0c830a5a60d258ee502211c8cf89d2d7ea53725e c2f6741c3ea597ce8cd560d70bc0ef7d688f591e 73ff2a12546d08aed02eac4c482a3d0e34a39344 1c7baea8856a213187a482a04a5b1cdc85c58472 3b1a131a2c15bbea6d9d7334dce57ef6f6d09c41 8077547d4b78f1dcce65a7868268d45c30f11d9a c6273ab49163bdb344957496f1d1fb7ecbc7f08c 438b3397af5dafe0a78501cad940b2545fdce5f5 6784b5eb30abda51c469af1c9396e2b709ddeea3 fccae9c8d37ce5130dcf5e5166ebdfc84bf248de e714c01d7ea095f151368615d32f851f1afe25e0

Workflow run logs.

@vbotbuildovich vbotbuildovich mentioned this pull request Feb 6, 2024
Closed
@rockwotj rockwotj mentioned this pull request Feb 6, 2024
Merged
dotnwat
dotnwat reviewed Feb 7, 2024
View reviewed changes
Copy link
Member

@dotnwat dotnwat left a comment

Choose a reason for hiding this comment

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

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dotnwat dotnwat dotnwat left review comments

@oleiman oleiman oleiman approved these changes

@BenPope BenPope Awaiting requested review from BenPope BenPope is a code owner

@emaxerrno emaxerrno Awaiting requested review from emaxerrno emaxerrno is a code owner

@dswang dswang Awaiting requested review from dswang

@gousteris gousteris Awaiting requested review from gousteris gousteris is a code owner automatically assigned from redpanda-data/devprod

@michael-redpanda michael-redpanda Awaiting requested review from michael-redpanda

Assignees

@rockwotj rockwotj

Labels
area/build area/redpanda area/wasm WASM Data Transforms
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@rockwotj @vbotbuildovich @oleiman @dotnwat