Merging this first version as a baseline. For future work: write a security.md document to explain how to report security vulnerabilities, and split off the release process in a separate releasing.md document
Reviewed-on: https://git.deuxfleurs.fr/Deuxfleurs/garage/pulls/1406
Otherwise the rustls dependency might be built with both aws-lc and ring backends,
leading to the following error in the k2v_client tests when
consul-discovery feature is enabled (including the reqwest dependency):
```
Could not automatically determine the process-level CryptoProvider from Rustls crate features.
Call CryptoProvider::install_default() before this point to select a provider manually, or make sure exactly one of the 'aws-lc-rs' and 'ring' features is enabled.
See the documentation of the CryptoProvider type for more information.
```
Co-authored-by: Yureka <yuka@yuka.dev>
Reviewed-on: https://git.deuxfleurs.fr/Deuxfleurs/garage/pulls/1412
Reviewed-by: Alex <lx@deuxfleurs.fr>
known_addrs in PeerInfoInternal is append-only — addresses accumulate
via add_addr() and PeerList gossip but are never removed. In dynamic
environments (k8s pod restarts, DHCP, NAT traversal), this list grows
unboundedly with stale addresses.
Combined with sequential iteration in try_connect() and no TCP connect
timeout in netapp.rs, each unreachable address blocks reconnection for
the kernel's TCP SYN timeout (75-130s on Linux). With 10+ stale
addresses, worst-case reconnection exceeds 750s — a full outage for
replication_factor=3 clusters.
This commit contains the two following changes:
1. Address failure tracking and pruning (peering.rs): Track consecutive
connection failures per address in PeerInfoInternal. After 3 failures,
prune from known_addrs. Reset count when address is re-advertised via
gossip or incoming connection. Prevents unbounded list growth.
2. Shuffle before connecting (peering.rs): Randomize address order in
try_connect() so the valid address (often appended last) gets a fair
chance instead of always trying stale addresses first.
known_addrs in PeerInfoInternal is append-only — addresses accumulate
via add_addr() and PeerList gossip but are never removed. In dynamic
environments (k8s pod restarts, DHCP, NAT traversal), this list grows
unboundedly with stale addresses.
Combined with sequential iteration in try_connect() and no TCP connect
timeout in netapp.rs, each unreachable address blocks reconnection for
the kernel's TCP SYN timeout (75-130s on Linux). With 10+ stale
addresses, worst-case reconnection exceeds 750s — a full outage for
replication_factor=3 clusters.
This patches includes a first change to fix this issue:
1. TCP connect timeout (netapp.rs): Wrap TcpStream::connect() in
tokio::time::timeout(10s). Caps per-address attempt from 75-130s
to 10s, reducing worst-case 10-addr reconnection from ~750s to ~100s.
## Summary
This PR ensures that the `LifecycleWorker` yields at least once to the Tokio scheduler in between each batch of 100 objects.
## Problem being solved
I'm administrating a Garage cluster which has been experiencing timeouts on all endpoints while the lifecycle worker is running at midnight UTC : `Ping timeout` error messages and even requests eventually failing due to `Could not reach quorum ...`.
I have found that this happens while the lifecycle worker is working on a big bucket (containing millions of objects) with a lifecycle rule that applies to very few objects.
The `process_object()` function does not hit any `await`:
- `last_bucket` is always the same, so the `bucket_table` is not read asynchronously
- no transaction is made on the `object_table` because my lifecycle rule (almost) never applies to any object
The first commit in this PR adds an executable which reproduces the problem that I've been experiencing in a self-contained way : the lifecycle worker starves the Tokio scheduler so much that no other task is able to run (or very rarely).
To run it : `cargo run -p garage_model --bin lifecycle-starvation-test`.
This commit can be dropped post-review, as it's only useful to demonstrate the starvation.
The error messages completely stopped after adding the extra yield to the nodes of my cluster.
The duration of the lifecycle worker task does not appear to have changed at all from what I can see (looking at the timestamps produced either by the self-contained binary or by each of my nodes with the `Lifecycle worker finished` message).
## Note
An other potential fix would have been to force the `WorkerProcessor` to yield before re-enqueuing a busy task, but this would have affected all Garage workers even though it's only the `LifecycleWorker` being uncooperative.
Reviewed-on: https://git.deuxfleurs.fr/Deuxfleurs/garage/pulls/1396
Reviewed-by: Alex <lx@deuxfleurs.fr>
Co-authored-by: Gauthier Zirnhelt <gauthier.zirnhelt@insimo.fr>
Co-committed-by: Gauthier Zirnhelt <gauthier.zirnhelt@insimo.fr>
This fixes a regression wrt garage-v1, likely caused by the version upgrade of quick_xml.
Currently, garage-v2 will emit empty ErrorDocument/IndexDocument/RedirectAllRequestsTo attributes in the response of GetBucketWebsite if there are no corresponding values.
This is somewhat wrong; at least, the S3 documentation for RedirectAllRequestsTo (https://docs.aws.amazon.com/AmazonS3/latest/API/API_RedirectAllRequestsTo.html) writes that it has a required HostName field. So emitting an empty RedirectAllRequestsTo is invalid.
This PR skips emitting XML attributes for these parameters if they contain no value.
Co-authored-by: Armaël Guéneau <armael.gueneau@ens-lyon.org>
Reviewed-on: https://git.deuxfleurs.fr/Deuxfleurs/garage/pulls/1391
Co-authored-by: Armael <armael@noreply.localhost>
Co-committed-by: Armael <armael@noreply.localhost>
## Summary
This PR fixes S3 `DeleteObjects` XML parsing when the request body is pretty-printed (contains indentation/newlines as whitespace text nodes).
Although PR #1324 already tried to address this, parsing could still fail with:
`InvalidRequest: Bad request: Invalid delete XML query`
because non-element nodes were validated but not actually skipped in the parsing loop.
## What changed
- In `src/api/s3/delete.rs`:
- Properly skip non-element whitespace text nodes while iterating over `<Delete>` children.
- Keep rejecting non-whitespace stray text content.
- Parse the root `<Delete>` element more robustly by selecting the first element child.
## Tests added
New unit tests in `src/api/s3/delete.rs`:
- `parse_delete_objects_xml_with_formatting`
- pretty-printed valid XML is accepted.
- `parse_delete_objects_xml_accepts_compact_valid_xml`
- compact valid XML is accepted.
- `parse_delete_objects_xml_rejects_non_whitespace_text_node`
- compact XML with stray text is rejected.
- `parse_delete_objects_xml_rejects_pretty_print_with_stray_text`
- pretty-printed XML with stray text is rejected.
## Validation
Executed:
```bash
cargo test -p garage_api_s3 parse_delete_objects_xml -- --nocapture
```
Result: all parser tests pass.
Reviewed-on: https://git.deuxfleurs.fr/Deuxfleurs/garage/pulls/1374
Co-authored-by: milouz1985 <francois.hoyez@gmail.com>
Co-committed-by: milouz1985 <francois.hoyez@gmail.com>
maximilien