Skip to content

Conversation

@cnsky1103
Copy link

@cnsky1103 cnsky1103 commented Dec 15, 2025

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

#21596

Made webhook validation optional by defaulting webhook metadata even when VALIDATE is omitted and skipping signature checks when no expression is provided.

dev=> create table wbhtable (
  data JSONB
) WITH (
  connector = 'webhook');
CREATE_TABLE
dev=> show create table wbhtable;
      Name       |                           Create Sql
-----------------+-----------------------------------------------------------------
 public.wbhtable | CREATE TABLE wbhtable (data JSONB) WITH (connector = 'webhook')
(1 row)
curl -v -X POST \
  http://localhost:4560/webhook/dev/public/wbhtable \
  -H 'Content-Type: application/json' \
  -d '{"data":"abc"}'

< HTTP/1.1 200 OK
< Content-Length: 0
< Access-Control-Allow-Origin: *
< Connection: keep-alive
< Date: Mon, 15 Dec 2025 02:19:04 GMT
< Keep-Alive: timeout=4
< Proxy-Connection: keep-alive
< Vary: origin, access-control-request-method, access-control-request-headers
<
* Connection #0 to host 127.0.0.1 left intact

Checklist

  • I have written necessary rustdoc comments.
  • I have added necessary unit tests and integration tests.
  • I have added test labels as necessary.
  • I have added fuzzing tests or opened an issue to track them.
  • My PR contains breaking changes.
  • My PR changes performance-critical code, so I will run (micro) benchmarks and present the results.
  • I have checked the Release Timeline and Currently Supported Versions to determine which release branches I need to cherry-pick this PR into.

Documentation

  • My PR needs documentation updates.
Release note


Some(expr.to_expr_proto())
} else {
None
Copy link
Member

Choose a reason for hiding this comment

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

How about suggesting to users (via notice_to_user) that a VALIDATE clause is strongly recommended for safety or production usages?

Copy link
Collaborator

Choose a reason for hiding this comment

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

+1

Copy link
Contributor

Choose a reason for hiding this comment

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

+2

Copy link
Author

Choose a reason for hiding this comment

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

message added

Copy link
Collaborator

@hzxa21 hzxa21 left a comment

Choose a reason for hiding this comment

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

LGTM.

@Chiro11 PTAL as well.

@Chiro11
Copy link
Contributor

Chiro11 commented Dec 15, 2025

LGTM.

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.

5 participants