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

Data Transform Admin APIs #13293

Merged
merged 2 commits into from
Sep 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/v/redpanda/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ set(swags
cluster
debug
usage
shadow_indexing)
shadow_indexing
transform)

set(swag_files)
foreach(swag ${swags})
Expand Down
162 changes: 162 additions & 0 deletions src/v/redpanda/admin/api-doc/transform.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
{
"apiVersion": "0.0.1",
"swaggerVersion": "1.2",
Copy link
Member

Choose a reason for hiding this comment

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

The swagger on the admin API doesn't really work and isn't consistent, but I think we would generally have better luck with tooling when we do get to fixing it, if it was 2.0.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry is the suggestion here to make this one use the 2.0 format?

Copy link
Member

Choose a reason for hiding this comment

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

Fixing the swagger for the admin API is a huge project, I think it would be less burdensome if we use v2.0 for new stuff.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool, I will update the transform doc to be v2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seastar does not actually support the v2 format even if seastar2code.py says that it does. Here's the patch, but the code generator explodes. @BenPope do you think we should fix it now? To be honest I would love a better format than swagger for these APIs anyways.

diff --git a/src/v/redpanda/admin/api-doc/transform.json b/src/v/redpanda/admin/api-doc/transform.json
index 7b2c3b04f..0b50ff949 100644
--- a/src/v/redpanda/admin/api-doc/transform.json
+++ b/src/v/redpanda/admin/api-doc/transform.json
@@ -1,64 +1,89 @@
 {
-  "apiVersion": "0.0.1",
-  "swaggerVersion": "1.2",
-  "basePath": "/v1",
-  "resourcePath": "/transform",
-  "apis": [
-    {
-      "path": "/v1/transform/deploy",
-      "operations": [
-        {
-          "method": "POST",
-          "summary": "Deploy a transform based on the provided metadata and wasm binary in the request payload. The payload should be the transform metadata in JSON format immediately followed by the WebAssembly binary without any delimiters.",
-          "nickname": "deploy_transform",
-          "consumes": [
-            "application/json+wasm"
-          ],
-          "type": "void",
-          "produces": [
-            "application/json"
-          ]
+  "swagger": "2.0",
+  "info": {
+    "title": "Data Transforms API",
+    "description": "API for WebAssembly powered Data Transforms in Redpanda",
+    "version": "0.0.1"
+  },
+  "basePath": "/v1/transform",
+  "schemes": [
+    "http"
+  ],
+  "consumes": [],
+  "produces": [
+    "application/json"
+  ],
+  "paths": {
+    "/deploy": {
+      "post": {
+        "operationId": "deploy_transform",
+        "description": "Deploy a transform based on the provided metadata and wasm binary in the request payload. The payload should be the transform metadata in JSON format immediately followed by the WebAssembly binary without any delimiters.",
+        "consumes": [
+          "application/json+wasm"
+        ],
+        "parameters": [
+          {
+            "name": "request_metadata",
+            "in": "body",
+            "description": "the metadata for the following wasm binary",
+            "required": true,
+            "schema": {
+              "$ref": "#/definitions/deploy_transform_request"
+            }
+          }
+        ],
+        "responses": {
+          "200": {
+            "type": "void"
+          }
         }
-      ]
+      }
     },
-    {
-      "path": "/v1/transform/delete",
-      "operations": [
-        {
-          "method": "POST",
-          "summary": "Delete a transform based on the provided name in the request payload.",
-          "type": "void",
-          "nickname": "delete_transform",
-          "consumes": [
-            "application/json"
-          ],
-          "produces": [
-            "application/json"
-          ]
+    "/list": {
+      "get": {
+        "operationId": "list_transform",
+        "description": "Delete a transform based on the provided name in the request payload.",
+        "consumes": [
+          "application/json"
+        ],
+        "parameters": [],
+        "responses": {
+          "200": {
+            "type": "array",
+            "items": {
+              "$ref": "#/definitions/transform_metadata"
+            }
+          }
         }
-      ]
+      }
     },
-    {
-      "path": "/v1/transform/list",
-      "operations": [
-        {
-          "method": "GET",
-          "type": "array",
-          "items": {
-            "type": "transform_metadata"
-          },
-          "summary": "List all transforms, the status report contains a cluster wide aggregated view of the live transform execution state.",
-          "nickname": "list_transforms",
-          "produces": [
-            "application/json"
-          ]
+    "/delete": {
+      "post": {
+        "operationId": "delete_transform",
+        "consumes": [
+          "application/json"
+        ],
+        "parameters": [
+          {
+            "name": "request",
+            "in": "body",
+            "description": "the request for the delete operation",
+            "required": true,
+            "schema": {
+              "$ref": "#/definitions/delete_transform_request"
+            }
+          }
+        ],
+        "responses": {
+          "200": {
+            "type": "void"
+          }
         }
-      ]
+      }
     }
-  ],
-  "models": {
+  },
+  "definitions": {
     "deploy_transform_request": {
-      "id": "deploy_transform_request",
       "properties": {
         "name": {
           "type": "string"
@@ -75,14 +100,13 @@
         "environment": {
           "type": "array",
           "items": {
-            "type": "environment_variable"
+            "type": "#/definitions/environment_variable"
           },
           "description": "The environment variable configuration for a transform"
         }
       }
     },
     "delete_transform_request": {
-      "id": "delete_transform_request",
       "properties": {
         "name": {
           "type": "string"
@@ -90,7 +114,6 @@
       }
     },
     "transform_metadata": {
-      "id": "transform_metadata",
       "properties": {
         "name": {
           "type": "string"
@@ -113,14 +136,13 @@
         "environment": {
           "type": "array",
           "items": {
-            "type": "environment_variable"
+            "type": "#/definitions/environment_variable"
           },
           "description": "The environment variable configuration for a transform"
         }
       }
     },
     "partition_transform_status": {
-      "id": "partition_transform_status",
       "description": "The status of a single partition's transform",
       "properties": {
         "node_id": {

Copy link
Member

Choose a reason for hiding this comment

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

@BenPope do we have some information on how to make new API files? I think people are copy-pasting without knowing of a better way (i myself am clueless). Given the existence of a multiple (2, 3?) different styles, do we gain much from selecting one over the other if the transition ends up being mechanical?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ben and I had a conversation in DM about this (probably should have had this in a core channel) but there are no examples of adding models in the admin endpoints for swagger v2.

Honestly I like v2 much more, but would rather prefer a better codegen mechanism here instead of invalid json files :/

"basePath": "/v1",
"apis": [
{
"path": "/v1/transform/deploy",
"operations": [
{
"method": "POST",
"summary": "Deploy a transform based on the provided metadata and wasm binary in the request payload. The payload should be the transform metadata in JSON format immediately followed by the WebAssembly binary without any delimiters.",
"nickname": "deploy_transform",
"consumes": [
"application/json+wasm"
],
"type": "void",
"produces": [
"application/json"
]
}
]
},
{
"path": "/v1/transform/{name}",
"operations": [
{
"method": "DELETE",
rockwotj marked this conversation as resolved.
Show resolved Hide resolved
"summary": "Delete a transform.",
"type": "void",
"nickname": "delete_transform",
"consumes": [
"application/json"
],
"produces": [
"application/json"
],
"parameters": [
{
"name": "name",
"in": "path",
"required": true,
"type": "string"
}
]
}
]
},
{
"path": "/v1/transform",
"operations": [
{
"method": "GET",
"type": "array",
"items": {
"type": "transform_metadata"
},
"summary": "List all transforms, the status report contains a cluster wide aggregated view of the live transform execution state.",
"nickname": "list_transforms",
"produces": [
"application/json"
]
}
]
}
],
"models": {
"deploy_transform_request": {
"id": "deploy_transform_request",
"properties": {
"name": {
"type": "string"
},
"input_topic": {
"type": "string"
},
"output_topics": {
"type": "array",
"items": {
"type": "string"
}
},
"environment": {
"type": "array",
"items": {
"type": "environment_variable"
NyaliaLui marked this conversation as resolved.
Show resolved Hide resolved
},
"description": "The environment variable configuration for a transform"
}
}
},
"transform_metadata": {
NyaliaLui marked this conversation as resolved.
Show resolved Hide resolved
"id": "transform_metadata",
"properties": {
"name": {
"type": "string"
},
"input_topic": {
"type": "string"
},
"output_topics": {
"type": "array",
"items": {
"type": "string"
}
},
"status": {
"type": "array",
"items": {
"type": "partition_transform_status"
}
},
"environment": {
"type": "array",
"items": {
"type": "environment_variable"
},
"description": "The environment variable configuration for a transform"
}
}
},
"partition_transform_status": {
"id": "partition_transform_status",
"description": "The status of a single partition's transform",
"properties": {
"node_id": {
"type": "int",
"description": "id of a node"
},
"partition": {
"type": "int",
"description": "partition in the input topic"
},
"core": {
"type": "int",
"description": "id of a core on a given node"
},
"status": {
"type": "string",
"enum": [
"running",
"inactive",
"errored",
"unknown"
],
"description": "the status of a transform"
}
}
},
"environment_variable": {
"id": "environment_variable",
"description": "Single key value pair for an environment",
"properties": {
"key": {
"type": "string"
},
"value": {
"type": "string"
}
}
}
}
}
35 changes: 35 additions & 0 deletions src/v/redpanda/admin_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
#include "redpanda/admin/api-doc/shadow_indexing.json.hh"
#include "redpanda/admin/api-doc/status.json.hh"
#include "redpanda/admin/api-doc/transaction.json.hh"
#include "redpanda/admin/api-doc/transform.json.hh"
#include "redpanda/admin/api-doc/usage.json.hh"
#include "resource_mgmt/memory_sampling.h"
#include "rpc/errc.h"
Expand Down Expand Up @@ -323,6 +324,8 @@ void admin_server::configure_admin_routes() {
rb->register_api_file(_server._routes, "debug");
rb->register_function(_server._routes, insert_comma);
rb->register_api_file(_server._routes, "cluster");
rb->register_function(_server._routes, insert_comma);
rb->register_api_file(_server._routes, "transform");
register_config_routes();
register_cluster_config_routes();
register_raft_routes();
Expand All @@ -339,6 +342,7 @@ void admin_server::configure_admin_routes() {
register_self_test_routes();
register_cluster_routes();
register_shadow_indexing_routes();
register_wasm_transform_routes();
}

namespace {
Expand Down Expand Up @@ -5445,3 +5449,34 @@ admin_server::sampled_memory_profile_handler(
co_return co_await ss::make_ready_future<ss::json::json_return_type>(
std::move(resp));
}

void admin_server::register_wasm_transform_routes() {
register_route_raw_async<superuser>(
ss::httpd::transform_json::deploy_transform,
[this](
std::unique_ptr<ss::http::request> req,
std::unique_ptr<ss::http::reply> rep) {
return deploy_transform(std::move(req), std::move(rep));
});
register_route<user>(
ss::httpd::transform_json::list_transforms,
[this](auto req) { return list_transforms(std::move(req)); });
register_route<superuser>(
ss::httpd::transform_json::delete_transform,
[this](auto req) { return delete_transform(std::move(req)); });
}

ss::future<ss::json::json_return_type>
admin_server::delete_transform(std::unique_ptr<ss::http::request>) {
throw ss::httpd::bad_request_exception("data transforms not enabled");
}

ss::future<ss::json::json_return_type>
admin_server::list_transforms(std::unique_ptr<ss::http::request>) {
throw ss::httpd::bad_request_exception("data transforms not enabled");
}

ss::future<std::unique_ptr<ss::http::reply>> admin_server::deploy_transform(
std::unique_ptr<ss::http::request>, std::unique_ptr<ss::http::reply>) {
throw ss::httpd::bad_request_exception("data transforms not enabled");
}
9 changes: 9 additions & 0 deletions src/v/redpanda/admin_server.h
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,7 @@ class admin_server {
void register_self_test_routes();
void register_cluster_routes();
void register_shadow_indexing_routes();
void register_wasm_transform_routes();

ss::future<ss::json::json_return_type> patch_cluster_config_handler(
std::unique_ptr<ss::http::request>, const request_auth_result&);
Expand Down Expand Up @@ -484,6 +485,14 @@ class admin_server {
ss::future<ss::json::json_return_type>
sampled_memory_profile_handler(std::unique_ptr<ss::http::request>);

// Transform routes
ss::future<std::unique_ptr<ss::http::reply>> deploy_transform(
std::unique_ptr<ss::http::request>, std::unique_ptr<ss::http::reply>);
ss::future<ss::json::json_return_type>
list_transforms(std::unique_ptr<ss::http::request>);
ss::future<ss::json::json_return_type>
delete_transform(std::unique_ptr<ss::http::request>);

ss::future<> throw_on_error(
ss::http::request& req,
std::error_code ec,
Expand Down
Loading