CLI: rewrite in rust #1577

svix-onelson · 2024-12-19T00:57:32Z

To date, the Go CLI has lived in its own repo and has received infrequent updates, failing to track our APIs as they change.

In an effort to improve on this, this diff adds new Rust version of the CLI here in the webhooks repo. The idea is to release the CLI along with the libs as a part of the same process (https://github.com/svix/monorepo-private/issues/9620).

Modules in the source tree under cli_types/ and cmds/api/ (excluding the mod.rs) are largely produced via our new codegen tooling (h/t @svix-jplatte).

Other modules under cmds/ are written by hand, the most involved being listen which depends on greenfield code under relay/.

Differences

Comparing the Go and Rust CLIs, there are some differences. The list of features added in the Rust version that are lacking in Go's is long thanks to the new codegen.

It'll be more useful to focus on what isn't here.

`--data-*` flags

In the Go CLI we exposed flags named --data-foo where foo would be a field in the expected JSON body sent with the request. These flags have not been carried forward. Instead users will be required to write out the full JSON body as a command-line argument.

Serde will give some feedback if the body fails to parse given the expected schema even before the request is made to the server.

Future work is planned to include an example body for each place we accept JSON in the long help.

Import/Export

svix import and svix export have been left behind. For import, users are encouraged to leverage svix event-type import-openapi which should fill the same need.

For export, I think the best alternative is using svix event-type list and handling the responses as needed.

Server URL no longer mandatory

The GO CLI required users to set the URL for the API server they wanted to interact with.
In the new Rust CLI, we will default to the issuer of the auth token. Typically this is going to be the safest default since manually specifying a Server URL should only be necessary when doing local development or talking to a non-production-SaaS server. The impacted group for this change would be enterprise customers and Svix employees and the fix is to do what they'd have needed to do all along: set the URL explicitly.

Verification

Reviewers will note that there's a dramatic lack of unit tests 😇
To this, I assure you there's technically more coverage on this CLI than there is on the Go one 🤣.

The majority of the verification for this has been manual testing. If you have specific concerns, pitch a test and I can run through the scenario for you (or you can try it yourself!)

OSS Reviews

There are several new-to-us crates included in this diff. I'll call each out in the comments below, looking for 👍🏻 from the security team.

This is the hackathon yield. Some initial work was done by hand to implement API calls for application, endpoint, and event type. These were used as the basis for new codegen templates which then were used to fill in many more resource types. More resources are yet to be implemented via codegen, pending additional work. Some basic coloring support was added for printing JSON output. Additional color support for other messages can be added at a future time. Signature verification is supported, with the caveat that some upstream fixes are needed in the rust client lib to get around the "timestamp too old" enforcement. We also have a new command for generating a signature given a body, timestamp, message id, and secret! The old Go cli didn't support this. Shell completions are here. Future work could implement a nicer display of available completions. More revs to follow to add more features, including the notable ommission: the `listen` command which proxies webhooks to a local HTTP server using a websocket over on the Play servers.

The `listen` command in the CLI allows you to use Svix Play as a public webhook receiver, then via websocket, tunnel the webhooks through to your local machine where they are forwarded to a local HTTP server. Responses from the local server are shuttled back to Play via websocket where they will show up in the UI. This is a non-trivial change compared to the rest of the core CLI. The translation from Go to Rust was not so straightforward, and not just because of the difference in design of libs. Moving from a GC'd runtime to a borrow-checked compliler means the original shape of the code is not so easily reproducable. Lots of manual testing indicates this _seems to work_ as expected. The Go version has some color output support for the logging of messages incoming and outgoing which are not here, but could be added later. This is rough. My aim was to get something together that "works" so we can move things forward. No doubt there are things that can be shifted around to help organize this more idiomatically -- soon.

When `SVIX_SERVER_URL` is unset or an empty string, look at the `SVIX_DEBUG_URL` variable. Some tweaks were made to `Config` to better support this. A new getter fn has been added and the `pub` visibility removed for both the `server_url` and `debug_url` fields. N.b. a plain serde alias doesn't work for this since if the `server_url` field is set on disk and you set `SVIX_DEBUG_URL`, the config will fail to load with "multiple values for server_url." Having a 2nd field for `debug_url` more closely matches what happened in Go.

This tweaks the colors for int, float, bool, null so that there are fewer "white" items in the output. White now appears in colon separators for object keys, braces, and square brackets. Everything else should be something else.

The Go CLI treated the server url as a mandatory config value. Instead, we're going to leave this unset when we instantiate a Svix client so that it will use the server url of the token issuer. The prompts for `svix login` now only ask for the auth token which should cut down on the chance for a mismatch. For the most part, server url is only something that should be customized if targeting a non-production SaaS environment (dev, test, staging and so on). This can still be configured by manually adding a `server_url` entry to the `config.toml` or at invocation time via the `SVIX_SERVER_URL`/`SVIX_DEBUG_URL` environment variables.

The Svix SDK uses the system tls anyway so vendoring for the ws client doesn't give a big help.

Note that some SDK sources are not tracked by git so you may need to re-run the top-level `regen_openapi.sh` script before the CLI will build.

The relay server uses a client-generated token to identify client sessions, and these are exclusive (i.e. only one client can connect using a given token). Any time we run `svix listen` we write the token text to the user's config file to allow _subsequent_ invocations to maintain the same token. This is nice but it also means if you invoke `svix listen` in 2 shells at the same time, the same token is loaded from config, and the 2nd invocation fails -- the server rejects the connection. This diff is a bit hacky, bit essentially when we see this specific control frame from the server, we can generate a new token _on the fly_, never peristing it to config, allowing `listen` to run normally. The newly generated token is given in the welcome message as per usual.

Feels sort of silly to add a dep to the manifest for this, but `open-rs` looks over-the-top well maintained, and also the Go CLI used a dep for this so why not?

This is needed by the CLI which is less useful if it can only verify signatures using timestamps that are within the 5 minute tolerance per the system's clock. The Go lib has this, I think specifically to support the old Go CLI.

This means the CLI can work with signatures that were produced outside of a 5 minute window! Much more useful.

While looking at the SDK source, I noticed there's a `with_token` method on the Svix client. This means we no longer have to pass the CLI's config value down into the authentication command's `exec()` method. Instead, use the already constructed client and build a new one from it using the supplied token argument.

Instead of importing from file on disk, either json or csv, use the openapi import under `event-type`. For export, I guess just save the response from `event-type list`.

svix-jplatte · 2024-12-23T11:01:38Z

svix-cli/Cargo.lock

+name = "idna_adapter"
+version = "1.2.0"


Maybe we should pin this the same way we do in svix-server, to reduce dependencies / build times a bit.

svix-jplatte · 2024-12-23T11:06:27Z

svix-cli/src/cli_types/mod.rs

+// FIXME: in the new codegen, we can look to add a `cli` feature that conditionally adds these
+//   attributes or trait impls to the wrapped types.
+//   At that point we can use the SDK types directly and remove this module.


You want to keep this idea alive?

svix-jplatte · 2024-12-23T11:07:04Z

svix-cli/src/cli_types/mod.rs

+#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, ValueEnum)]
+pub enum Ordering {
+    Ascending,
+    Descending,
+}
+
+impl From<Ordering> for api::Ordering {
+    fn from(value: Ordering) -> Self {
+        match value {
+            Ordering::Ascending => api::Ordering::Ascending,
+            Ordering::Descending => api::Ordering::Descending,
+        }
+    }
+}


Is this still needed with svix::api::Ordering implementing FromStr now?

svix-jplatte · 2024-12-23T11:09:08Z

svix-cli/src/cmds/open.rs

+/// Quickly open Svix pages in your browser
+#[derive(Subcommand)]
+pub enum OpenCommands {
+    /// Open the Svix API documentation


Maybe this would make it easier to tell the difference between "api" and "docs"?

Suggested change

/// Open the Svix API documentation

/// Open the Svix API reference

svix-jplatte · 2024-12-23T11:09:50Z

svix-cli/src/cmds/signature.rs

+                let webhook = svix::webhooks::Webhook::new(&secret)?;
+                let mut headers = http::HeaderMap::with_capacity(3);
+                headers.insert("svix-id", msg_id.parse()?);
+                headers.insert("svix-timestamp", format!("{}", timestamp).parse()?);


Suggested change

headers.insert("svix-timestamp", format!("{}", timestamp).parse()?);

headers.insert("svix-timestamp", timestamp.to_string().parse()?);

svix-jplatte · 2024-12-23T11:14:41Z

svix-cli/src/relay/mod.rs

+            println!(
+                r#"
+Webhook Relay is now listening at:
+{}
+
+All requests on this endpoint will be forwarded to your local URL:
+{}
+"#,
+                receive_url(&start_response.token),
+                self.local_url,
+            );


Could use printdoc! here and below.

svix-jplatte · 2024-12-23T11:15:50Z

svix-cli/src/relay/mod.rs

+        let request = websocket_url.to_string().into_client_request()?;
+        let (stream, _resp) = connect_async(request)
+            .await
+            .with_context(|| "failed to connect to websocket server")?;


Suggested change

.with_context(|| "failed to connect to websocket server")?;

.context("failed to connect to websocket server")?;

svix-jplatte · 2024-12-23T11:17:20Z

svix-cli/src/relay/mod.rs

+            tx.send(Message::Binary(
+                serde_json::to_vec(&msg)
+                    .expect("trivial serialization")
+                    .into(),
+            )),


Why not Message::Text / serde_json::to_string?

svix-jplatte · 2024-12-23T11:18:02Z

svix-cli/src/relay/mod.rs

+    const SKIP_USER_AGENT: HeaderValue = HeaderValue::from_static("Go-http-client/1.1");
+    for (k, v) in headers {
+        // FIXME: the Go cli used to unset the useragent if it matched the Go http client. Do we need something similar here? idk why this is needed.
+        if k == http::header::USER_AGENT && v == SKIP_USER_AGENT {


Haha, I hope we don't actually need this?

svix-jplatte · 2024-12-23T11:18:12Z

svix-cli/src/relay/mod.rs

+    Ok(out)
+}
+
+//


Empty comment?

svix-onelson force-pushed the onelson/rust-cli branch 3 times, most recently from 40c8d76 to 14f9685 Compare December 19, 2024 22:50

svix-onelson added 16 commits December 20, 2024 11:15

CLI: rename command, svix-signature -> signature

6390b27

CLI: add trailing newline to json output

2362f7d

CLI: customize the JSON color styler to have less "white"

a4db712

This tweaks the colors for int, float, bool, null so that there are fewer "white" items in the output. White now appears in colon separators for object keys, braces, and square brackets. Everything else should be something else.

CLI: cargo update, remove unused deps in manifest

ae3697e

CLI: switch to native-tls (non-vendored) for tokio-tungstenite

1275d13

The Svix SDK uses the system tls anyway so vendoring for the ws client doesn't give a big help.

CLI: use path dep for rust SDK

a9e5adf

Note that some SDK sources are not tracked by git so you may need to re-run the top-level `regen_openapi.sh` script before the CLI will build.

CLI: implement open command

ae677c8

Feels sort of silly to add a dep to the manifest for this, but `open-rs` looks over-the-top well maintained, and also the Go CLI used a dep for this so why not?

Libs(Rust): add verify_ignoring_timestamp akin to Go lib

51aa6a9

This is needed by the CLI which is less useful if it can only verify signatures using timestamps that are within the 5 minute tolerance per the system's clock. The Go lib has this, I think specifically to support the old Go CLI.

CLI: make signature verify use verify_ignoring_timestamp

8f65f81

This means the CLI can work with signatures that were produced outside of a 5 minute window! Much more useful.

CLI: mods then consts

4e4da58

svix-onelson force-pushed the onelson/rust-cli branch 2 times, most recently from 9e25750 to 0be5bcd Compare December 20, 2024 21:37

CLI: update to latest codegen, add message-attempt support (and more)

f7804d6

svix-onelson force-pushed the onelson/rust-cli branch from 0be5bcd to f7804d6 Compare December 20, 2024 21:42

CLI: remove import/export

25fff17

Instead of importing from file on disk, either json or csv, use the openapi import under `event-type`. For export, I guess just save the response from `event-type list`.

svix-onelson added the open source review label Dec 20, 2024

svix-onelson marked this pull request as ready for review December 20, 2024 22:16

svix-onelson requested a review from a team as a code owner December 20, 2024 22:16

svix-jplatte reviewed Dec 23, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLI: rewrite in rust #1577

CLI: rewrite in rust #1577

svix-onelson commented Dec 19, 2024 •

edited

Loading

svix-jplatte Dec 23, 2024

svix-jplatte Dec 23, 2024

svix-jplatte Dec 23, 2024

svix-jplatte Dec 23, 2024

svix-jplatte Dec 23, 2024

svix-jplatte Dec 23, 2024

svix-jplatte Dec 23, 2024

svix-jplatte Dec 23, 2024

svix-jplatte Dec 23, 2024

svix-jplatte Dec 23, 2024

	/// Open the Svix API documentation
	/// Open the Svix API reference

	headers.insert("svix-timestamp", format!("{}", timestamp).parse()?);
	headers.insert("svix-timestamp", timestamp.to_string().parse()?);

	.with_context(\|\| "failed to connect to websocket server")?;
	.context("failed to connect to websocket server")?;

+                  Ok(out)
+              }
+              //

CLI: rewrite in rust #1577

Are you sure you want to change the base?

CLI: rewrite in rust #1577

Conversation

svix-onelson commented Dec 19, 2024 • edited Loading

Differences

--data-* flags

Import/Export

Server URL no longer mandatory

Verification

OSS Reviews

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

svix-onelson commented Dec 19, 2024 •

edited

Loading

`--data-*` flags