fix(write): prevent writes to soft-deleted databases - #26722
Conversation
Soft-deleted databases have been accepting write operations during their deletion grace period. Users typically have no reason to write data to a database scheduled for deletion. This change adds validation in WriteValidator::initialize() to check if a database is marked as deleted and rejects write attempts with a DatabaseDeleted error. Querying deleted databases remains allowed for data recovery purposes. * fixes #26721
3fa1e81 to
834ed5f
Compare
|
example of error it should ideally be a 400 series error. Other client errors also return a 500: for example, db name is too long
And another with 400
|
This commit makes the error for attempting to write to a soft deleted db a bad request instead of the default error mapping which is a server error.
|
I added a commit which makes it a 400 BAD REQUEST error in the mapping now. I also suppressed the influxdb/influxdb3_server/src/http.rs Lines 283 to 287 in a0c400d |
| #[derive(Debug, Serialize)] | ||
| struct ErrorMessage<T: Serialize> { | ||
| error: String, | ||
| #[serde(skip_serializing_if = "Option::is_none")] |
There was a problem hiding this comment.
This impacts how all ErrorMessage's are serialized to json for all errors that map through it.
There was a problem hiding this comment.
I tend to agree with this change, but it looks like this caused a test failure or two that will need to be fixed for CI to go ✅
There was a problem hiding this comment.
Yes! I saw the failure yesterday too but didn't get to it until today.
I believe i fixed it; CI will tell me.
| let serialized = serde_json::to_string(&err).unwrap(); | ||
| let body = bytes_to_response_body(serialized); | ||
| ResponseBuilder::new() | ||
| .status(StatusCode::BAD_REQUEST) |
There was a problem hiding this comment.
400 bad request is better than 500 internal server error imo. I also debated 403: forbidden but i think 400 is better.
There was a problem hiding this comment.
Agreed. AFAICT we've reserved 403 strictly for permissions related errors.
|
on merge hold for next release. |
* fix(write): prevent writes to soft-deleted databases Soft-deleted databases have been accepting write operations during their deletion grace period. Users typically have no reason to write data to a database scheduled for deletion. This change adds validation in WriteValidator::initialize() to check if a database is marked as deleted and rejects write attempts with a DatabaseDeleted error. Querying deleted databases remains allowed for data recovery purposes. * fixes #26721 * chore: cannot write to soft deleted db is a 400 error This commit makes the error for attempting to write to a soft deleted db a bad request instead of the default error mapping which is a server error. * chore: fix test expectations because of the json output change
* fix(write): prevent writes to soft-deleted databases (#26722) * fix(write): prevent writes to soft-deleted databases Soft-deleted databases have been accepting write operations during their deletion grace period. Users typically have no reason to write data to a database scheduled for deletion. This change adds validation in WriteValidator::initialize() to check if a database is marked as deleted and rejects write attempts with a DatabaseDeleted error. Querying deleted databases remains allowed for data recovery purposes. * fixes #26721 * chore: cannot write to soft deleted db is a 400 error This commit makes the error for attempting to write to a soft deleted db a bad request instead of the default error mapping which is a server error. * chore: fix test expectations because of the json output change * chore: Port changes from Enterprise (#26755) * fix: Ensure catalog UUID is preserved during migration --------- Co-authored-by: Phil Bracikowski <13472206+philjb@users.noreply.github.com> Co-authored-by: Stuart Carnie <stuart.carnie@gmail.com>
2 participants
Soft-deleted databases have been accepting write operations during their deletion grace period. Users typically have no reason to write data to a database scheduled for deletion.
This change adds validation in WriteValidator::initialize() to check if a database is marked as deleted and rejects write attempts with a DatabaseDeleted error. Querying deleted databases remains allowed for data recovery purposes.