rust/lancedb
1
0
Fork 0
mirror of https://github.com/lancedb/lancedb.git synced 2026-11-13 22:26:53 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(secrets): address Secrets the way every other object is addressed

Browse Source 
Secret CRUD used plural collection routes with the identity in the body, which
is the one shape in the HTTP surface that does not match Table and Function
CRUD. #4166 moved Functions onto `<object>/{id}/<action>`; this does the same
for Secrets, so there is one route grammar rather than one per object type.

    POST /v1/secrets/create      ->  POST /v1/secret/{id}/create
    POST /v1/secrets/alter       ->  POST /v1/secret/{id}/alter
    POST /v1/secrets/describe    ->  POST /v1/secret/{id}/describe
    POST /v1/secrets/drop        ->  POST /v1/secret/{id}/drop
    POST /v1/secrets/list        ->  GET  /v1/namespace/{id}/secret/list

`{id}` is the namespace path and the name joined with the client's configured
delimiter, built by the same helper a table identifier is. The name and the
namespace therefore leave every request body: create and alter carry the value
alone, drop and describe carry nothing, and listing moves its `page_token` to a
query parameter. `add_namespace_path` goes with them -- a root Secret is
addressed by its bare name, so there is no longer an empty field to omit.

Identifiers are written into the path as-is rather than percent-encoded. The
charset a Secret name and a namespace segment admit already excludes everything
that would need escaping, and a route that reads like the ones beside it is
worth more than encoding against a charset that does not exist yet. If that
charset widens, this is the place that has to widen with it.

Listing takes a real namespace rather than rejecting a non-root one the way the
Function route does: Secrets are addressable inside nested namespaces, so the
namespace identifier is the whole point of the route.

`build_secret_identifier` is a wrapper over `build_table_identifier` rather than
a second implementation. The grammar belongs to the namespace spec rather than
to tables, and a call site should read as the thing it addresses.

Also removes `_DECLARED_SECRET`, which has been unreferenced since client-side
binding validation was dropped, and whose pattern was the environment variable
grammar rather than a Secret name's.

Tests: lancedb secret unit tests 7, `first_class_function_slice1` 9,
`first_class_function_slice2` 3, 93 Python tests across both slices.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UfmeJ533rQDnPBkMtjerV6
This commit is contained in:
Jonathan M Hsieh
2026-09-15 19:29:09 +00:00
co-authored by Claude Opus 5
parent dddc94474f
commit 6b77670e5c
3 changed files with 123 additions and 109 deletions
Download Patch File Download Diff File Expand all files Collapse all files
python/python/lancedb/functions.py
-1
View File
@@ -559,7 +559,6 @@ class RefreshColumnResult(_RemoteValue):
_FUNCTION_NAME = re.compile(r"^[A-Za-z_][A-Za-z0-9_.-]*$")
_DECLARED_SECRET = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$")
_FUNCTION_BLOB_V2_TYPE = "blob_v2"
_ARROW_EXTENSION_NAME_KEY = "ARROW:extension:name"
python/python/tests/test_first_class_function_slice2.py
+32 -20
View File
@@ -1528,6 +1528,11 @@ def _mock_remote_function_catalog():
if len(parts) == 4 and parts[0] == "v1" and parts[1] == "function"
else (None, None)
)
secret_action = (
(urllib.parse.unquote(parts[2]), parts[3])
if len(parts) == 4 and parts[0] == "v1" and parts[1] == "secret"
else (None, None)
)
if function_action[1] == "create":
state["version"] = {
"name": function_action[0],
@@ -1559,20 +1564,14 @@ def _mock_remote_function_catalog():
elif function_action[1] == "drop":
assert body == {"version": "fv_exact"}
response = {"dropped": True}
elif self.path in ("/v1/secrets/create", "/v1/secrets/alter"):
assert set(body) == {"name", "value"}
elif secret_action[1] in ("create", "alter"):
# The Secret is the path identifier, so the body is the value.
assert set(body) == {"value"}
assert secret_action[0] == "openai-prod"
response = {}
elif self.path == "/v1/secrets/list":
if "page_token" not in body:
response = {
"secrets": [{"name": "openai-prod"}],
"page_token": "next",
}
else:
assert body["page_token"] == "next"
response = {"secrets": [{"name": "hf-prod"}]}
elif self.path == "/v1/secrets/drop":
assert body == {"name": "openai-prod"}
elif secret_action[1] == "drop":
assert secret_action[0] == "openai-prod"
assert body == {}
response = {}
else:
status = 404
@@ -1586,6 +1585,16 @@ def _mock_remote_function_catalog():
for key, values in urllib.parse.parse_qs(url.query).items()
}
state["requests"].append((url.path, query))
if url.path == "/v1/namespace/$/secret/list":
if "page_token" not in query:
self._write_response(
200,
{"secrets": [{"name": "openai-prod"}], "page_token": "next"},
)
else:
assert query["page_token"] == "next"
self._write_response(200, {"secrets": [{"name": "hf-prod"}]})
return
if url.path != "/v1/namespace/$/function/list":
self._write_response(404, {"error": "not found"})
return
@@ -1698,15 +1707,18 @@ def test_remote_secret_verbs_round_trip():
routes = [path for path, _ in state["requests"]]
assert routes == [
"/v1/secrets/create",
"/v1/secrets/alter",
"/v1/secrets/list",
"/v1/secrets/list",
"/v1/secrets/drop",
"/v1/secret/openai-prod/create",
"/v1/secret/openai-prod/alter",
"/v1/namespace/$/secret/list",
"/v1/namespace/$/secret/list",
"/v1/secret/openai-prod/drop",
]
assert state["requests"][0][1] == {"name": "openai-prod", "value": "sk-live-0001"}
# The listing returns names, and the client has no way to ask for more.
# The Secret is the path identifier, so the body is the value alone.
assert state["requests"][0][1] == {"value": "sk-live-0001"}
# Listing is a GET: the first page asks for nothing, the second resumes on
# the token the server handed back, and neither carries a body.
assert state["requests"][2][1] == {}
assert state["requests"][3][1] == {"page_token": "next"}
def test_building_a_binding_contacts_no_server():
rust/lancedb/src/remote/db.rs
+91 -88
View File
@@ -282,22 +282,6 @@ pub struct RemoteHostOverrides {
pub sql: Option<String>,
}
/// Attach a namespace path to a Secret request body.
///
/// A root path is omitted rather than sent empty, so a root request is byte
/// identical to one from a client that predates namespace addressing.
fn add_namespace_path(body: &mut serde_json::Value, namespace_path: &[String]) {
if namespace_path.is_empty() {
return;
}
body["namespace_path"] = serde_json::Value::Array(
namespace_path
.iter()
.map(|segment| serde_json::Value::String(segment.clone()))
.collect(),
);
}
impl RemoteDatabase {
pub(crate) fn try_new(
uri: &str,
@@ -381,6 +365,17 @@ impl<S: HttpSend> RemoteDatabase<S> {
///
/// The value is a request field and never a path segment or query
/// parameter, which keeps it out of access logs and proxy traces.
/// The path segment addressing one Secret.
///
/// Written into the path as-is, the way a table identifier is. The charset
/// a Secret name and a namespace segment admit excludes everything that
/// would need escaping, and a `/v1/secret/prod$openai/drop` that reads like
/// the table and Function routes beside it is worth more than encoding
/// against a charset that does not exist yet.
fn secret_id(&self, name: &str, namespace_path: &[String]) -> String {
build_secret_identifier(name, namespace_path, &self.client.id_delimiter)
}
async fn post_secret_write<T: serde::Serialize>(&self, route: &str, body: &T) -> Result<()> {
let req = self.client.post(route).json(body);
// This call is what says the body is a credential. Nothing downstream
@@ -538,6 +533,16 @@ fn build_table_identifier(name: &str, namespace: &[String], delimiter: &str) ->
}
}
/// A Secret's path identifier: its namespace path and name joined the way every
/// other object's is.
///
/// Secrets are not tables, but the identifier grammar belongs to the namespace
/// spec rather than to any one object type, and having two would mean a caller
/// had to know which kind of name it was holding.
fn build_secret_identifier(name: &str, namespace: &[String], delimiter: &str) -> String {
build_table_identifier(name, namespace, delimiter)
}
fn build_namespace_identifier(namespace: &[String], delimiter: &str) -> String {
if namespace.is_empty() {
// According to the namespace spec, use delimiter to represent root namespace
@@ -634,36 +639,17 @@ struct RemoteCreateFunctionRequest<'a> {
/// today: they are different operations to the service -- one refuses an
/// existing name, the other requires it -- and either may grow a field the
/// other has no meaning for.
///
/// The name and its namespace are the path identifier, so neither appears here.
#[derive(serde::Serialize)]
struct RemoteCreateSecretRequest<'a> {
name: &'a str,
value: &'a str,
/// Omitted at the root, so a request from a client that predates namespaces
/// is byte-identical to one that does not use them.
#[serde(skip_serializing_if = "<[String]>::is_empty")]
namespace_path: &'a [String],
}
/// Replace the credential behind a Secret the database already holds.
#[derive(serde::Serialize)]
struct RemoteAlterSecretRequest<'a> {
name: &'a str,
value: &'a str,
#[serde(skip_serializing_if = "<[String]>::is_empty")]
namespace_path: &'a [String],
}
/// One page of a Secret listing. A struct rather than an inline object so the
/// request and the response are declared the same way -- a reader of one finds
/// the other.
#[derive(serde::Serialize)]
struct RemoteListSecretsRequest<'a> {
#[serde(skip_serializing_if = "Option::is_none")]
page_token: Option<String>,
/// Omitted at the root, so a listing from a client that predates namespaces
/// is byte-identical to one that does not use them.
#[serde(skip_serializing_if = "<[String]>::is_empty")]
namespace_path: &'a [String],
}
#[derive(serde::Deserialize)]
@@ -922,39 +908,34 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
value: &str,
namespace_path: &[String],
) -> Result<()> {
let secret_id = self.secret_id(name, namespace_path);
self.post_secret_write(
"/v1/secrets/create",
&RemoteCreateSecretRequest {
name,
value,
namespace_path,
},
&format!("/v1/secret/{secret_id}/create"),
&RemoteCreateSecretRequest { value },
)
.await
}
async fn alter_secret(&self, name: &str, value: &str, namespace_path: &[String]) -> Result<()> {
let secret_id = self.secret_id(name, namespace_path);
self.post_secret_write(
"/v1/secrets/alter",
&RemoteAlterSecretRequest {
name,
value,
namespace_path,
},
&format!("/v1/secret/{secret_id}/alter"),
&RemoteAlterSecretRequest { value },
)
.await
}
async fn list_secrets(&self, namespace_path: &[String]) -> Result<Vec<String>> {
let namespace_id = build_namespace_identifier(namespace_path, &self.client.id_delimiter);
let path = format!("/v1/namespace/{namespace_id}/secret/list");
let mut names = Vec::new();
let mut page_token: Option<String> = None;
let mut seen_page_tokens = HashSet::new();
loop {
let body = RemoteListSecretsRequest {
page_token: page_token.clone(),
namespace_path,
};
let req = self.client.post("/v1/secrets/list").json(&body);
let mut req = self.client.get(&path);
if let Some(token) = &page_token {
req = req.query(&[("page_token", token)]);
}
let (request_id, response) = self.client.send(req).await?;
let response = self.client.check_response(&request_id, response).await?;
let status = response.status();
@@ -978,18 +959,16 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
}
async fn drop_secret(&self, name: &str, namespace_path: &[String]) -> Result<()> {
let mut body = serde_json::json!({ "name": name });
add_namespace_path(&mut body, namespace_path);
let req = self.client.post("/v1/secrets/drop").json(&body);
let secret_id = self.secret_id(name, namespace_path);
let req = self.client.post(&format!("/v1/secret/{secret_id}/drop"));
let (request_id, response) = self.client.send(req).await?;
self.client.check_response(&request_id, response).await?;
Ok(())
}
async fn describe_secret(&self, name: &str, namespace_path: &[String]) -> Result<SecretInfo> {
let mut body = serde_json::json!({ "name": name });
add_namespace_path(&mut body, namespace_path);
let req = self.client.post("/v1/secrets/describe").json(&body);
let secret_id = self.secret_id(name, namespace_path);
let req = self.client.post(&format!("/v1/secret/{secret_id}/describe"));
let (request_id, response) = self.client.send(req).await?;
let response = self.client.check_response(&request_id, response).await?;
response.json().await.err_to_http(request_id)
@@ -3246,7 +3225,10 @@ mod tests {
#[tokio::test]
async fn test_create_and_alter_secret_send_the_value_in_the_request_body() {
for (route, call) in [("/v1/secrets/create", true), ("/v1/secrets/alter", false)] {
for (route, call) in [
("/v1/secret/openai-prod/create", true),
("/v1/secret/openai-prod/alter", false),
] {
let conn = Connection::new_with_handler(move |request| {
assert_eq!(request.method(), &reqwest::Method::POST);
assert_eq!(request.url().path(), route);
@@ -3255,8 +3237,8 @@ mod tests {
assert!(request.url().query().is_none(), "{:?}", request.url());
let body: serde_json::Value =
serde_json::from_slice(request.body().unwrap().as_bytes().unwrap()).unwrap();
assert_eq!(body["name"], "openai-prod");
assert_eq!(body["value"], "sk-live-0001");
// The name is the path identifier, so the body is the value alone.
assert_eq!(body, serde_json::json!({ "value": "sk-live-0001" }));
http::Response::builder().status(200).body("{}").unwrap()
});
if call {
@@ -3274,11 +3256,14 @@ mod tests {
#[tokio::test]
async fn test_list_secrets_walks_pages_and_returns_names_only() {
let conn = Connection::new_with_handler(|request| {
assert_eq!(request.url().path(), "/v1/secrets/list");
let body: serde_json::Value =
serde_json::from_slice(request.body().unwrap().as_bytes().unwrap()).unwrap();
let page = body.get("page_token").and_then(|token| token.as_str());
let body = match page {
assert_eq!(request.method(), &reqwest::Method::GET);
assert_eq!(request.url().path(), "/v1/namespace/$/secret/list");
let page = request
.url()
.query_pairs()
.find(|(key, _)| key == "page_token")
.map(|(_, value)| value.into_owned());
let body = match page.as_deref() {
None => r#"{"secrets":[{"name":"openai-prod"}],"page_token":"p2"}"#,
Some("p2") => r#"{"secrets":[{"name":"hf-prod"}]}"#,
Some(other) => panic!("unexpected page token: {other}"),
@@ -3309,32 +3294,37 @@ mod tests {
}
#[tokio::test]
async fn test_drop_secret_posts_the_name_alone() {
async fn test_drop_and_describe_address_the_secret_in_the_path() {
let conn = Connection::new_with_handler(|request| {
assert_eq!(request.url().path(), "/v1/secrets/drop");
let body: serde_json::Value =
serde_json::from_slice(request.body().unwrap().as_bytes().unwrap()).unwrap();
assert_eq!(body, serde_json::json!({"name": "openai-prod"}));
assert_eq!(request.url().path(), "/v1/secret/openai-prod/drop");
// Nothing is left to say once the path names the Secret.
assert!(request.body().is_none(), "{:?}", request.body());
http::Response::builder().status(200).body("{}").unwrap()
});
conn.drop_secret("openai-prod", &[]).await.unwrap();
let conn = Connection::new_with_handler(|request| {
assert_eq!(request.url().path(), "/v1/secret/openai-prod/describe");
assert!(request.body().is_none(), "{:?}", request.body());
http::Response::builder()
.status(200)
.body(r#"{"name":"openai-prod","created_at_millis":1,"updated_at_millis":2}"#)
.unwrap()
});
let info = conn.describe_secret("openai-prod", &[]).await.unwrap();
assert_eq!(info.name, "openai-prod");
}
/// A namespace path is sent when there is one and omitted when there is
/// not, so a root request stays byte identical to one from a client that
/// predates namespace addressing -- which is what lets the parameter ship
/// before every server implements it.
/// The namespace is part of the identifier the path addresses, joined with
/// the client's configured delimiter the way every other object's is. A
/// root Secret is therefore addressed by its bare name, and a namespaced
/// one by the joined path -- there is no body field either way.
#[tokio::test]
async fn test_a_namespace_path_is_sent_only_when_it_is_not_root() {
async fn test_a_namespace_path_is_addressed_in_the_path() {
let conn = Connection::new_with_handler(|request| {
let body: serde_json::Value =
serde_json::from_slice(request.body().unwrap().as_bytes().unwrap()).unwrap();
assert_eq!(
body,
serde_json::json!({
"name": "openai-prod",
"namespace_path": ["prod", "vision"],
})
request.url().path(),
"/v1/secret/prod$vision$openai-prod/drop"
);
http::Response::builder().status(200).body("{}").unwrap()
});
@@ -3343,12 +3333,25 @@ mod tests {
.unwrap();
let conn = Connection::new_with_handler(|request| {
let body: serde_json::Value =
serde_json::from_slice(request.body().unwrap().as_bytes().unwrap()).unwrap();
assert!(body.get("namespace_path").is_none(), "{body}");
assert_eq!(request.url().path(), "/v1/secret/openai-prod/drop");
http::Response::builder().status(200).body("{}").unwrap()
});
conn.drop_secret("openai-prod", &[]).await.unwrap();
// Listing is namespace-scoped, so the namespace is the whole identifier.
let conn = Connection::new_with_handler(|request| {
assert_eq!(
request.url().path(),
"/v1/namespace/prod$vision/secret/list"
);
http::Response::builder()
.status(200)
.body(r#"{"secrets":[]}"#)
.unwrap()
});
conn.list_secrets(&["prod".to_string(), "vision".to_string()])
.await
.unwrap();
}
#[tokio::test]