Influxdata
/
influxdb
mirror of https://github.com/influxdata/influxdb.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

feat(idpe-17887): enable `/` in db name for v1 write. (#8235) 

* test case for proposed new behavior in v1 write endpoint.
* autogen and default are equivalent reserved words for rp
* have write endpoint match query endpoint, in that db and rp are always concated
pull/24376/head
wiedld 2023-07-18 09:36:25 -07:00 committed by GitHub
parent 1723aa46a6
commit efae0f108a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 53 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

67
router/src/server/http/write/single_tenant/mod.rs
View File

@ -64,9 +64,7 @@ impl From<&SingleTenantExtractError> for hyper::StatusCode {
SingleTenantExtractError::NoBucketSpecified => Self::BAD_REQUEST,
SingleTenantExtractError::InvalidNamespace(_) => Self::BAD_REQUEST,
SingleTenantExtractError::ParseV1Request(
V1WriteParseError::NoQueryParams
| V1WriteParseError::DecodeFail(_)
| V1WriteParseError::ContainsRpSeparator,
V1WriteParseError::NoQueryParams | V1WriteParseError::DecodeFail(_),
) => Self::BAD_REQUEST,
SingleTenantExtractError::ParseV2Request(
V2WriteParseError::NoQueryParams | V2WriteParseError::DecodeFail(_),
@ -125,10 +123,6 @@ async fn parse_v1(
// Extract the write parameters.
let write_params = WriteParamsV1::try_from(req)?;
// Extracting the write parameters validates the db field never contains the
// '/' separator to avoid ambiguity with the "namespace/rp" construction.
debug_assert!(!write_params.db.contains(V1_NAMESPACE_RP_SEPARATOR));
// Extract or construct the namespace name string from the write parameters
let namespace = NamespaceName::new(match write_params.rp {
RetentionPolicy::Unspecified | RetentionPolicy::Autogen => write_params.db,
@ -316,22 +310,65 @@ mod tests {
}
);
// Prevent ambiguity by denying the `/` character in the DB
// Permit `/` character in the DB
test_parse_v1!(
no_rp_db_with_rp_separator,
query_string = "?db=bananas/are/great",
want = Err(Error::SingleTenantError(
SingleTenantExtractError::ParseV1Request(V1WriteParseError::ContainsRpSeparator)
))
want = Ok(WriteParams{ namespace, precision }) => {
assert_eq!(namespace.as_str(), "bananas/are/great");
assert_matches!(precision, Precision::Nanoseconds);
}
);
// Prevent ambiguity by denying the `/` character in the RP
// Permit the `/` character in the RP
test_parse_v1!(
rp_with_rp_separator,
query_string = "?db=bananas&rp=are/great",
want = Err(Error::SingleTenantError(
SingleTenantExtractError::ParseV1Request(V1WriteParseError::ContainsRpSeparator)
))
want = Ok(WriteParams{ namespace, precision }) => {
assert_eq!(namespace.as_str(), "bananas/are/great");
assert_matches!(precision, Precision::Nanoseconds);
}
);
// `/` character is allowed in the DB, if a named RP is specified
test_parse_v1!(
db_with_rp_separator_and_rp,
query_string = "?db=foo/bar&rp=my_rp",
want = Ok(WriteParams{ namespace, precision }) => {
assert_eq!(namespace.as_str(), "foo/bar/my_rp");
assert_matches!(precision, Precision::Nanoseconds);
}
);
// Always concat, even if this results in duplication rp within the namespace.
// ** this matches the query API behavior **
test_parse_v1!(
db_with_rp_separator_and_duplicate_rp,
query_string = "?db=foo/my_rp&rp=my_rp",
want = Ok(WriteParams{ namespace, precision }) => {
assert_eq!(namespace.as_str(), "foo/my_rp/my_rp");
assert_matches!(precision, Precision::Nanoseconds);
}
);
// `/` character is allowed in the DB, if an autogen RP is specified
test_parse_v1!(
db_with_rp_separator_and_rp_autogen,
query_string = "?db=foo/bar&rp=autogen",
want = Ok(WriteParams{ namespace, precision }) => {
assert_eq!(namespace.as_str(), "foo/bar");
assert_matches!(precision, Precision::Nanoseconds);
}
);
// `/` character is allowed in the DB, if a default RP is specified
test_parse_v1!(
db_with_rp_separator_and_rp_default,
query_string = "?db=foo/bar&rp=default",
want = Ok(WriteParams{ namespace, precision }) => {
assert_eq!(namespace.as_str(), "foo/bar");
assert_matches!(precision, Precision::Nanoseconds);
}
);
test_parse_v1!(

22
router/src/server/http/write/v1.rs
View File

@ -29,12 +29,6 @@ pub enum V1WriteParseError {
/// The request contains invalid parameters.
#[error("failed to deserialize db/rp/precision in request: {0}")]
DecodeFail(#[from] serde::de::value::Error),
/// The provided "db" or "rp" value contains the reserved `/` character.
///
/// See [`V1_NAMESPACE_RP_SEPARATOR`].
#[error("db cannot contain the reserved character '/'")]
ContainsRpSeparator,
}
/// May be empty string, explicit rp name, or `autogen`. As provided at the
@ -61,7 +55,7 @@ impl<'de> Deserialize<'de> for RetentionPolicy {
Ok(match s.as_str() {
"" => RetentionPolicy::Unspecified,
"''" => RetentionPolicy::Unspecified,
"autogen" => RetentionPolicy::Autogen,
"autogen" | "default" => RetentionPolicy::Autogen,
_ => RetentionPolicy::Named(s),
})
}
@ -90,20 +84,6 @@ impl<T> TryFrom<&Request<T>> for WriteParamsV1 {
let query = req.uri().query().ok_or(V1WriteParseError::NoQueryParams)?;
let params: WriteParamsV1 = serde_urlencoded::from_str(query)?;
// No namespace (db) is ever allowed to contain a `/` to prevent
// ambiguity with the namespace/rp NamespaceName construction.
if params.db.contains(V1_NAMESPACE_RP_SEPARATOR) {
return Err(V1WriteParseError::ContainsRpSeparator);
}
// Likewise the "rp" field itself cannot contain the `/` character if
// specified.
if let RetentionPolicy::Named(s) = &params.rp {
if s.contains(V1_NAMESPACE_RP_SEPARATOR) {
return Err(V1WriteParseError::ContainsRpSeparator);
}
}
Ok(params)
}
}