From 9ec3f8cc3c09329761f711e35475b6272b6257ed Mon Sep 17 00:00:00 2001 From: Baptiste Jonglez Date: Sat, 12 Apr 2025 23:18:50 +0200 Subject: [PATCH 01/17] metadata: Create compact LMDB snapshots See #1006 LMDB files never shrink, so we can end up with a large database that contains a smaller amount of actual data. Compacting the snapshots is an easy win: it will write faster to disk, take less space, and if needed you can reimport an already-compacted snapshot as the main database. --- src/db/lmdb_adapter.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/db/lmdb_adapter.rs b/src/db/lmdb_adapter.rs index 40f1c867..259aa566 100644 --- a/src/db/lmdb_adapter.rs +++ b/src/db/lmdb_adapter.rs @@ -109,7 +109,7 @@ impl IDb for LmdbDb { let mut path = to.clone(); path.push("data.mdb"); self.db - .copy_to_path(path, heed::CompactionOption::Disabled)?; + .copy_to_path(path, heed::CompactionOption::Enabled)?; Ok(()) } From 02498a93d0d5fee5540420345f87a7c4e44635b9 Mon Sep 17 00:00:00 2001 From: Zoob Date: Sat, 19 Apr 2025 18:46:36 +0000 Subject: [PATCH 02/17] doc: fix Docker run volume mappings --- doc/book/quick-start/_index.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/book/quick-start/_index.md b/doc/book/quick-start/_index.md index 41867b19..2db4211b 100644 --- a/doc/book/quick-start/_index.md +++ b/doc/book/quick-start/_index.md @@ -129,9 +129,9 @@ docker run \ -d \ --name garaged \ -p 3900:3900 -p 3901:3901 -p 3902:3902 -p 3903:3903 \ - -v /etc/garage.toml:/path/to/garage.toml \ - -v /var/lib/garage/meta:/path/to/garage/meta \ - -v /var/lib/garage/data:/path/to/garage/data \ + -v /path/to/garage.toml:/etc/garage.toml \ + -v /path/to/garage/meta:/var/lib/garage/meta \ + -v /path/to/garage/data:/var/lib/garage/data \ dxflrs/garage:v1.1.0 ``` From 9b38cba6f318192eefa8db871fc1367b197cfe6d Mon Sep 17 00:00:00 2001 From: babykart Date: Sat, 22 Mar 2025 20:44:45 +0100 Subject: [PATCH 03/17] helm-chart: Add livenessProbe & readinessProbe Signed-off-by: babykart --- script/helm/garage/README.md | 4 +++- script/helm/garage/templates/workload.yaml | 17 ++++++++--------- script/helm/garage/values.yaml | 15 +++++++++++++++ 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/script/helm/garage/README.md b/script/helm/garage/README.md index c2eb086f..c9b54acd 100644 --- a/script/helm/garage/README.md +++ b/script/helm/garage/README.md @@ -1,6 +1,6 @@ # garage -![Version: 0.6.0](https://img.shields.io/badge/Version-0.6.0-informational?style=flat-square) ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square) ![AppVersion: v1.0.1](https://img.shields.io/badge/AppVersion-v1.0.1-informational?style=flat-square) +![Version: 0.7.0](https://img.shields.io/badge/Version-0.7.0-informational?style=flat-square) ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square) ![AppVersion: v1.1.0](https://img.shields.io/badge/AppVersion-v1.1.0-informational?style=flat-square) S3-compatible object store for small self-hosted geo-distributed deployments @@ -49,6 +49,7 @@ S3-compatible object store for small self-hosted geo-distributed deployments | initImage.pullPolicy | string | `"IfNotPresent"` | | | initImage.repository | string | `"busybox"` | | | initImage.tag | string | `"stable"` | | +| livenessProbe | object | `{}` | Specifies a livenessProbe | | monitoring.metrics.enabled | bool | `false` | If true, a service for monitoring is created with a prometheus.io/scrape annotation | | monitoring.metrics.serviceMonitor.enabled | bool | `false` | If true, a ServiceMonitor CRD is created for a prometheus operator https://github.com/coreos/prometheus-operator | | monitoring.metrics.serviceMonitor.interval | string | `"15s"` | | @@ -71,6 +72,7 @@ S3-compatible object store for small self-hosted geo-distributed deployments | podSecurityContext.runAsGroup | int | `1000` | | | podSecurityContext.runAsNonRoot | bool | `true` | | | podSecurityContext.runAsUser | int | `1000` | | +| readinessProbe | object | `{}` | Specifies a readinessProbe | | resources | object | `{}` | | | securityContext.capabilities | object | `{"drop":["ALL"]}` | The default security context is heavily restricted, feel free to tune it to your requirements | | securityContext.readOnlyRootFilesystem | bool | `true` | | diff --git a/script/helm/garage/templates/workload.yaml b/script/helm/garage/templates/workload.yaml index cb9e76a2..d144cb41 100644 --- a/script/helm/garage/templates/workload.yaml +++ b/script/helm/garage/templates/workload.yaml @@ -78,15 +78,14 @@ spec: {{- with .Values.extraVolumeMounts }} {{- toYaml . | nindent 12 }} {{- end }} - # TODO - # livenessProbe: - # httpGet: - # path: / - # port: 3900 - # readinessProbe: - # httpGet: - # path: / - # port: 3900 + {{- with .Values.livenessProbe }} + livenessProbe: + {{- toYaml . | nindent 12 }} + {{- end }} + {{- with .Values.readinessProbe }} + readinessProbe: + {{- toYaml . | nindent 12 }} + {{- end }} resources: {{- toYaml .Values.resources | nindent 12 }} volumes: diff --git a/script/helm/garage/values.yaml b/script/helm/garage/values.yaml index 38715e38..0a6a45c5 100644 --- a/script/helm/garage/values.yaml +++ b/script/helm/garage/values.yaml @@ -191,6 +191,21 @@ resources: {} # cpu: 100m # memory: 512Mi +# -- Specifies a livenessProbe +livenessProbe: {} + #httpGet: + # path: /health + # port: 3903 + #initialDelaySeconds: 5 + #periodSeconds: 30 +# -- Specifies a readinessProbe +readinessProbe: {} + #httpGet: + # path: /health + # port: 3903 + #initialDelaySeconds: 5 + #periodSeconds: 30 + nodeSelector: {} tolerations: [] From e6e4e051a1a6b005e9baa4875e1a65b9d4b04dcb Mon Sep 17 00:00:00 2001 From: babykart Date: Sat, 22 Mar 2025 20:49:48 +0100 Subject: [PATCH 04/17] helm-chart: Add metadata_auto_snapshot_interval Signed-off-by: babykart --- script/helm/garage/README.md | 1 + script/helm/garage/templates/configmap.yaml | 4 ++++ script/helm/garage/values.yaml | 4 ++++ 3 files changed, 9 insertions(+) diff --git a/script/helm/garage/README.md b/script/helm/garage/README.md index c9b54acd..1a187c84 100644 --- a/script/helm/garage/README.md +++ b/script/helm/garage/README.md @@ -23,6 +23,7 @@ S3-compatible object store for small self-hosted geo-distributed deployments | garage.existingConfigMap | string | `""` | if not empty string, allow using an existing ConfigMap for the garage.toml, if set, ignores garage.toml | | garage.garageTomlString | string | `""` | String Template for the garage configuration if set, ignores above values. Values can be templated, see https://garagehq.deuxfleurs.fr/documentation/reference-manual/configuration/ | | garage.kubernetesSkipCrd | bool | `false` | Set to true if you want to use k8s discovery but install the CRDs manually outside of the helm chart, for example if you operate at namespace level without cluster ressources | +| garage.metadataAutoSnapshotInterval | string | `""` | If this value is set, Garage will automatically take a snapshot of the metadata DB file at a regular interval and save it in the metadata directory. https://garagehq.deuxfleurs.fr/documentation/reference-manual/configuration/#metadata_auto_snapshot_interval | | garage.replicationMode | string | `"3"` | Default to 3 replicas, see the replication_mode section at https://garagehq.deuxfleurs.fr/documentation/reference-manual/configuration/#replication-mode | | garage.rpcBindAddr | string | `"[::]:3901"` | | | garage.rpcSecret | string | `""` | If not given, a random secret will be generated and stored in a Secret object | diff --git a/script/helm/garage/templates/configmap.yaml b/script/helm/garage/templates/configmap.yaml index 81ca205e..ab5b84db 100644 --- a/script/helm/garage/templates/configmap.yaml +++ b/script/helm/garage/templates/configmap.yaml @@ -19,6 +19,10 @@ data: compression_level = {{ .Values.garage.compressionLevel }} + {{- if .Values.garage.metadataAutoSnapshotInterval }} + metadata_auto_snapshot_interval = {{ .Values.garage.metadataAutoSnapshotInterval | quote }} + {{- end }} + rpc_bind_addr = "{{ .Values.garage.rpcBindAddr }}" # rpc_secret will be populated by the init container from a k8s secret object rpc_secret = "__RPC_SECRET_REPLACE__" diff --git a/script/helm/garage/values.yaml b/script/helm/garage/values.yaml index 0a6a45c5..bbb60db2 100644 --- a/script/helm/garage/values.yaml +++ b/script/helm/garage/values.yaml @@ -21,6 +21,10 @@ garage: # https://garagehq.deuxfleurs.fr/documentation/reference-manual/configuration/#compression-level compressionLevel: "1" + # -- If this value is set, Garage will automatically take a snapshot of the metadata DB file at a regular interval and save it in the metadata directory. + # https://garagehq.deuxfleurs.fr/documentation/reference-manual/configuration/#metadata_auto_snapshot_interval + metadataAutoSnapshotInterval: "" + rpcBindAddr: "[::]:3901" # -- If not given, a random secret will be generated and stored in a Secret object rpcSecret: "" From 3c20984a08528f1a6672c8afc83d2306a0361e40 Mon Sep 17 00:00:00 2001 From: babykart Date: Sat, 22 Mar 2025 20:52:47 +0100 Subject: [PATCH 05/17] helm-chart: Cosmetic changes Signed-off-by: babykart --- script/helm/garage/Chart.yaml | 30 ++++++++++++------------------ script/helm/garage/README.md | 6 ++++++ 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/script/helm/garage/Chart.yaml b/script/helm/garage/Chart.yaml index 1a3e27e0..7a89409e 100644 --- a/script/helm/garage/Chart.yaml +++ b/script/helm/garage/Chart.yaml @@ -1,24 +1,18 @@ apiVersion: v2 name: garage description: S3-compatible object store for small self-hosted geo-distributed deployments - -# A chart can be either an 'application' or a 'library' chart. -# -# Application charts are a collection of templates that can be packaged into versioned archives -# to be deployed. -# -# Library charts provide useful utilities or functions for the chart developer. They're included as -# a dependency of application charts to inject those utilities and functions into the rendering -# pipeline. Library charts do not define any templates and therefore cannot be deployed. type: application - -# This is the chart version. This version number should be incremented each time you make changes -# to the chart and its templates, including the app version. -# Versions are expected to follow Semantic Versioning (https://semver.org/) version: 0.7.0 - -# This is the version number of the application being deployed. This version number should be -# incremented each time you make changes to the application. Versions are not expected to -# follow Semantic Versioning. They should reflect the version the application is using. -# It is recommended to use it with quotes. appVersion: "v1.1.0" +home: https://garagehq.deuxfleurs.fr/ +icon: https://garagehq.deuxfleurs.fr/images/garage-logo.svg + +keywords: +- geo-distributed +- read-after-write-consistency +- s3-compatible + +sources: +- https://git.deuxfleurs.fr/Deuxfleurs/garage.git + +maintainers: [] \ No newline at end of file diff --git a/script/helm/garage/README.md b/script/helm/garage/README.md index 1a187c84..fcf988ca 100644 --- a/script/helm/garage/README.md +++ b/script/helm/garage/README.md @@ -4,6 +4,12 @@ S3-compatible object store for small self-hosted geo-distributed deployments +**Homepage:** + +## Source Code + +* + ## Values | Key | Type | Default | Description | From ad151cb1dc2c657db4c969a306349bc077ed648a Mon Sep 17 00:00:00 2001 From: "Maximilien R." Date: Wed, 23 Apr 2025 23:30:16 +0200 Subject: [PATCH 06/17] Fix #1007: hint that region can be changed depending on cluster config --- doc/book/connect/apps/index.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/book/connect/apps/index.md b/doc/book/connect/apps/index.md index 14868373..5ec9686c 100644 --- a/doc/book/connect/apps/index.md +++ b/doc/book/connect/apps/index.md @@ -69,7 +69,7 @@ $CONFIG = array( 'hostname' => '127.0.0.1', // Can also be a domain name, eg. garage.example.com 'port' => 3900, // Put your reverse proxy port or your S3 API port 'use_ssl' => false, // Set it to true if you have a TLS enabled reverse proxy - 'region' => 'garage', // Garage has only one region named "garage" + 'region' => 'garage', // Garage default region is named "garage", edit according to your cluster config 'use_path_style' => true // Garage supports only path style, must be set to true ], ], @@ -135,7 +135,7 @@ bucket but doesn't also know the secret encryption key. *Click on the picture to zoom* Add a new external storage. Put what you want in "folder name" (eg. "shared"). Select "Amazon S3". Keep "Access Key" for the Authentication field. -In Configuration, put your bucket name (eg. nextcloud), the host (eg. 127.0.0.1), the port (eg. 3900 or 443), the region (garage). Tick the SSL box if you have put an HTTPS proxy in front of garage. You must tick the "Path access" box and you must leave the "Legacy authentication (v2)" box empty. Put your Key ID (eg. GK...) and your Secret Key in the last two input boxes. Finally click on the tick symbol on the right of your screen. +In Configuration, put your bucket name (eg. nextcloud), the host (eg. 127.0.0.1), the port (eg. 3900 or 443), the region ("garage" if you use the default, or the one your configured in your `garage.toml`). Tick the SSL box if you have put an HTTPS proxy in front of garage. You must tick the "Path access" box and you must leave the "Legacy authentication (v2)" box empty. Put your Key ID (eg. GK...) and your Secret Key in the last two input boxes. Finally click on the tick symbol on the right of your screen. Now go to your "Files" app and a new "linked folder" has appeared with the name you chose earlier (eg. "shared"). @@ -238,7 +238,7 @@ object_storage: # Put localhost only if you have a garage instance running on that node endpoint: 'http://localhost:3900' # or "garage.example.com" if you have TLS on port 443 - # Garage supports only one region for now, named garage + # Garage default region is named "garage", edit according to your config region: 'garage' credentials: @@ -441,7 +441,7 @@ media_storage_providers: store_synchronous: True # do we want to wait that the file has been written before returning? config: bucket: matrix # the name of our bucket, we chose matrix earlier - region_name: garage # only "garage" is supported for the region field + region_name: garage # "garage" by default, edit according to your cluster config endpoint_url: http://localhost:3900 # the path to the S3 endpoint access_key_id: "GKxxx" # your Key ID secret_access_key: "xxxx" # your Secret Key From 14274bc13c2bc39ad54c3a36f5c6473897762009 Mon Sep 17 00:00:00 2001 From: Baptiste Jonglez Date: Thu, 8 May 2025 10:27:53 +0200 Subject: [PATCH 07/17] doc: Add systemd example to increase file descriptors limit --- doc/book/cookbook/systemd.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/book/cookbook/systemd.md b/doc/book/cookbook/systemd.md index c0ed7d1f..ebff8c15 100644 --- a/doc/book/cookbook/systemd.md +++ b/doc/book/cookbook/systemd.md @@ -28,6 +28,7 @@ StateDirectory=garage DynamicUser=true ProtectHome=true NoNewPrivileges=true +LimitNOFILE=42000 [Install] WantedBy=multi-user.target From 539af12d21567a39a074d3a73c893d98275c70d4 Mon Sep 17 00:00:00 2001 From: trinity-1686a Date: Mon, 19 May 2025 18:07:04 +0200 Subject: [PATCH 08/17] allow punnycode in bucket name --- src/api/admin/bucket.rs | 4 ++-- src/api/s3/bucket.rs | 2 +- src/garage/admin/bucket.rs | 2 +- src/model/bucket_alias_table.rs | 18 ++++++++---------- src/model/helper/locked.rs | 7 +++---- src/util/config.rs | 4 ++++ 6 files changed, 19 insertions(+), 18 deletions(-) diff --git a/src/api/admin/bucket.rs b/src/api/admin/bucket.rs index 2537bfc9..6cc21938 100644 --- a/src/api/admin/bucket.rs +++ b/src/api/admin/bucket.rs @@ -277,7 +277,7 @@ pub async fn handle_create_bucket( let helper = garage.locked_helper().await; if let Some(ga) = &req.global_alias { - if !is_valid_bucket_name(ga) { + if !is_valid_bucket_name(ga, garage.config.allow_punnycode) { return Err(Error::bad_request(format!( "{}: {}", ga, INVALID_BUCKET_NAME_MESSAGE @@ -292,7 +292,7 @@ pub async fn handle_create_bucket( } if let Some(la) = &req.local_alias { - if !is_valid_bucket_name(&la.alias) { + if !is_valid_bucket_name(&la.alias, garage.config.allow_punnycode) { return Err(Error::bad_request(format!( "{}: {}", la.alias, INVALID_BUCKET_NAME_MESSAGE diff --git a/src/api/s3/bucket.rs b/src/api/s3/bucket.rs index 3a09e769..d2a36c18 100644 --- a/src/api/s3/bucket.rs +++ b/src/api/s3/bucket.rs @@ -172,7 +172,7 @@ pub async fn handle_create_bucket( } // Create the bucket! - if !is_valid_bucket_name(&bucket_name) { + if !is_valid_bucket_name(&bucket_name, garage.config.allow_punnycode) { return Err(Error::bad_request(format!( "{}: {}", bucket_name, INVALID_BUCKET_NAME_MESSAGE diff --git a/src/garage/admin/bucket.rs b/src/garage/admin/bucket.rs index 1bdc6086..1ed0ebd8 100644 --- a/src/garage/admin/bucket.rs +++ b/src/garage/admin/bucket.rs @@ -126,7 +126,7 @@ impl AdminRpcHandler { #[allow(clippy::ptr_arg)] async fn handle_create_bucket(&self, name: &String) -> Result { - if !is_valid_bucket_name(name) { + if !is_valid_bucket_name(name, self.garage.config.allow_punnycode) { return Err(Error::BadRequest(format!( "{}: {}", name, INVALID_BUCKET_NAME_MESSAGE diff --git a/src/model/bucket_alias_table.rs b/src/model/bucket_alias_table.rs index 8bbe4118..04d808e8 100644 --- a/src/model/bucket_alias_table.rs +++ b/src/model/bucket_alias_table.rs @@ -22,14 +22,10 @@ mod v08 { pub use v08::*; impl BucketAlias { - pub fn new(name: String, ts: u64, bucket_id: Option) -> Option { - if !is_valid_bucket_name(&name) { - None - } else { - Some(BucketAlias { - name, - state: crdt::Lww::raw(ts, bucket_id), - }) + pub fn new(name: String, ts: u64, bucket_id: Option) -> Self { + BucketAlias { + name, + state: crdt::Lww::raw(ts, bucket_id), } } @@ -80,7 +76,7 @@ impl TableSchema for BucketAliasTable { /// In the case of Garage, bucket names must not be hex-encoded /// 32 byte string, which is excluded thanks to the /// maximum length of 63 bytes given in the spec. -pub fn is_valid_bucket_name(n: &str) -> bool { +pub fn is_valid_bucket_name(n: &str, punny: bool) -> bool { // Bucket names must be between 3 and 63 characters n.len() >= 3 && n.len() <= 63 // Bucket names must be composed of lowercase letters, numbers, @@ -92,7 +88,9 @@ pub fn is_valid_bucket_name(n: &str) -> bool { // Bucket names must not be formatted as an IP address && n.parse::().is_err() // Bucket names must not start with "xn--" - && !n.starts_with("xn--") + && (!n.starts_with("xn--") || punny) + // We are a bit stricter, to properly restrict punnycode in all labels + && (!n.contains(".xn--") || punny) // Bucket names must not end with "-s3alias" && !n.ends_with("-s3alias") } diff --git a/src/model/helper/locked.rs b/src/model/helper/locked.rs index 482e91b0..16b0bafc 100644 --- a/src/model/helper/locked.rs +++ b/src/model/helper/locked.rs @@ -57,7 +57,7 @@ impl<'a> LockedHelper<'a> { bucket_id: Uuid, alias_name: &String, ) -> Result<(), Error> { - if !is_valid_bucket_name(alias_name) { + if !is_valid_bucket_name(alias_name, self.0.config.allow_punnycode) { return Err(Error::InvalidBucketName(alias_name.to_string())); } @@ -88,8 +88,7 @@ impl<'a> LockedHelper<'a> { // writes are now done and all writes use timestamp alias_ts let alias = match alias { - None => BucketAlias::new(alias_name.clone(), alias_ts, Some(bucket_id)) - .ok_or_else(|| Error::InvalidBucketName(alias_name.clone()))?, + None => BucketAlias::new(alias_name.clone(), alias_ts, Some(bucket_id)), Some(mut a) => { a.state = Lww::raw(alias_ts, Some(bucket_id)); a @@ -218,7 +217,7 @@ impl<'a> LockedHelper<'a> { ) -> Result<(), Error> { let key_helper = KeyHelper(self.0); - if !is_valid_bucket_name(alias_name) { + if !is_valid_bucket_name(alias_name, self.0.config.allow_punnycode) { return Err(Error::InvalidBucketName(alias_name.to_string())); } diff --git a/src/util/config.rs b/src/util/config.rs index 73fc4ff4..f128177b 100644 --- a/src/util/config.rs +++ b/src/util/config.rs @@ -135,6 +135,10 @@ pub struct Config { /// Configuration for the admin API endpoint #[serde(default = "Default::default")] pub admin: AdminConfig, + + /// Allow punnycode in bucket names + #[serde(default)] + pub allow_punnycode: bool, } /// Value for data_dir: either a single directory or a list of dirs with attributes From a605a8080659b73939f6b3ff60bc0847ed0fb3c5 Mon Sep 17 00:00:00 2001 From: trinity-1686a Date: Mon, 19 May 2025 18:11:55 +0200 Subject: [PATCH 09/17] support punnycode in api/web endpoint --- Cargo.lock | 43 +-------------------------------------- Cargo.toml | 1 - src/api/common/Cargo.toml | 1 - src/api/common/helpers.rs | 3 +-- 4 files changed, 2 insertions(+), 46 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bd5a1f9f..e65778cc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1300,7 +1300,6 @@ dependencies = [ "http-body-util", "hyper 1.6.0", "hyper-util", - "idna 0.5.0", "md-5", "nom", "opentelemetry", @@ -2170,16 +2169,6 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b9e0384b61958566e926dc50660321d12159025e767c18e043daf26b70104c39" -[[package]] -name = "idna" -version = "0.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "634d9b1461af396cad843f47fdba5597a4f9e6ddd4bfb6ff5d85028c25cb12f6" -dependencies = [ - "unicode-bidi", - "unicode-normalization", -] - [[package]] name = "idna" version = "1.0.3" @@ -4252,21 +4241,6 @@ dependencies = [ "zerovec", ] -[[package]] -name = "tinyvec" -version = "1.9.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "09b3661f17e86524eccd4371ab0429194e0d7c008abb45f7a7495b1719463c71" -dependencies = [ - "tinyvec_macros", -] - -[[package]] -name = "tinyvec_macros" -version = "0.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20" - [[package]] name = "tokio" version = "1.44.1" @@ -4587,27 +4561,12 @@ version = "0.1.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2896d95c02a80c6d6a5d6e953d479f5ddf2dfdb6a244441010e373ac0fb88971" -[[package]] -name = "unicode-bidi" -version = "0.3.18" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5c1cb5db39152898a79168971543b1cb5020dff7fe43c8dc468b0885f5e29df5" - [[package]] name = "unicode-ident" version = "1.0.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5a5f39404a5da50712a4c1eecf25e90dd62b613502b7e925fd4e4d19b5c96512" -[[package]] -name = "unicode-normalization" -version = "0.1.24" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5033c97c4262335cded6d6fc3e5c18ab755e1a3dc96376350f3d8e9f009ad956" -dependencies = [ - "tinyvec", -] - [[package]] name = "unicode-segmentation" version = "1.12.0" @@ -4655,7 +4614,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32f8b686cadd1473f4bd0117a5d28d36b1ade384ea9b5069a1c40aefed7fda60" dependencies = [ "form_urlencoded", - "idna 1.0.3", + "idna", "percent-encoding", ] diff --git a/Cargo.toml b/Cargo.toml index 732f6f05..400c1840 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -58,7 +58,6 @@ git-version = "0.3.4" hex = "0.4" hexdump = "0.1" hmac = "0.12" -idna = "0.5" itertools = "0.12" ipnet = "2.9.0" lazy_static = "1.4" diff --git a/src/api/common/Cargo.toml b/src/api/common/Cargo.toml index 6d906423..b1a8b47a 100644 --- a/src/api/common/Cargo.toml +++ b/src/api/common/Cargo.toml @@ -28,7 +28,6 @@ err-derive.workspace = true hex.workspace = true hmac.workspace = true md-5.workspace = true -idna.workspace = true tracing.workspace = true nom.workspace = true pin-project.workspace = true diff --git a/src/api/common/helpers.rs b/src/api/common/helpers.rs index c8586de4..6fc4aa13 100644 --- a/src/api/common/helpers.rs +++ b/src/api/common/helpers.rs @@ -8,7 +8,6 @@ use hyper::{ body::{Body, Bytes}, Request, Response, }; -use idna::domain_to_unicode; use serde::{Deserialize, Serialize}; use garage_model::bucket_table::BucketParams; @@ -97,7 +96,7 @@ pub fn authority_to_host(authority: &str) -> Result { authority ))), }; - authority.map(|h| domain_to_unicode(h).0) + authority.map(|h| h.to_ascii_lowercase()) } /// Extract the bucket name and the key name from an HTTP path and possibly a bucket provided in From bba9202f310b257ed52d1a82052f05532495c62e Mon Sep 17 00:00:00 2001 From: trinity-1686a Date: Mon, 19 May 2025 20:36:03 +0200 Subject: [PATCH 10/17] add test for punycode --- src/api/admin/bucket.rs | 4 +- src/api/s3/bucket.rs | 2 +- src/garage/admin/bucket.rs | 2 +- src/garage/tests/common/garage.rs | 2 + src/garage/tests/s3/website.rs | 73 +++++++++++++++++++++++++++++++ src/model/bucket_alias_table.rs | 8 ++-- src/model/helper/locked.rs | 4 +- src/util/config.rs | 4 +- 8 files changed, 87 insertions(+), 12 deletions(-) diff --git a/src/api/admin/bucket.rs b/src/api/admin/bucket.rs index 6cc21938..f8bd1eb5 100644 --- a/src/api/admin/bucket.rs +++ b/src/api/admin/bucket.rs @@ -277,7 +277,7 @@ pub async fn handle_create_bucket( let helper = garage.locked_helper().await; if let Some(ga) = &req.global_alias { - if !is_valid_bucket_name(ga, garage.config.allow_punnycode) { + if !is_valid_bucket_name(ga, garage.config.allow_punycode) { return Err(Error::bad_request(format!( "{}: {}", ga, INVALID_BUCKET_NAME_MESSAGE @@ -292,7 +292,7 @@ pub async fn handle_create_bucket( } if let Some(la) = &req.local_alias { - if !is_valid_bucket_name(&la.alias, garage.config.allow_punnycode) { + if !is_valid_bucket_name(&la.alias, garage.config.allow_punycode) { return Err(Error::bad_request(format!( "{}: {}", la.alias, INVALID_BUCKET_NAME_MESSAGE diff --git a/src/api/s3/bucket.rs b/src/api/s3/bucket.rs index d2a36c18..23cceb84 100644 --- a/src/api/s3/bucket.rs +++ b/src/api/s3/bucket.rs @@ -172,7 +172,7 @@ pub async fn handle_create_bucket( } // Create the bucket! - if !is_valid_bucket_name(&bucket_name, garage.config.allow_punnycode) { + if !is_valid_bucket_name(&bucket_name, garage.config.allow_punycode) { return Err(Error::bad_request(format!( "{}: {}", bucket_name, INVALID_BUCKET_NAME_MESSAGE diff --git a/src/garage/admin/bucket.rs b/src/garage/admin/bucket.rs index 1ed0ebd8..073329c1 100644 --- a/src/garage/admin/bucket.rs +++ b/src/garage/admin/bucket.rs @@ -126,7 +126,7 @@ impl AdminRpcHandler { #[allow(clippy::ptr_arg)] async fn handle_create_bucket(&self, name: &String) -> Result { - if !is_valid_bucket_name(name, self.garage.config.allow_punnycode) { + if !is_valid_bucket_name(name, self.garage.config.allow_punycode) { return Err(Error::BadRequest(format!( "{}: {}", name, INVALID_BUCKET_NAME_MESSAGE diff --git a/src/garage/tests/common/garage.rs b/src/garage/tests/common/garage.rs index 8d71504f..2b0a381c 100644 --- a/src/garage/tests/common/garage.rs +++ b/src/garage/tests/common/garage.rs @@ -63,6 +63,8 @@ rpc_bind_addr = "127.0.0.1:{rpc_port}" rpc_public_addr = "127.0.0.1:{rpc_port}" rpc_secret = "{secret}" +allow_punycode = true + [s3_api] s3_region = "{region}" api_bind_addr = "127.0.0.1:{s3_port}" diff --git a/src/garage/tests/s3/website.rs b/src/garage/tests/s3/website.rs index 9a9e29f2..6d37eee8 100644 --- a/src/garage/tests/s3/website.rs +++ b/src/garage/tests/s3/website.rs @@ -533,3 +533,76 @@ async fn test_website_check_domain() { }) ); } + +#[tokio::test] +async fn test_website_puny() { + const BCKT_NAME: &str = "xn--pda.eu"; + let ctx = common::context(); + let bucket = ctx.create_bucket(BCKT_NAME); + + let data = ByteStream::from_static(BODY); + + ctx.client + .put_object() + .bucket(&bucket) + .key("index.html") + .body(data) + .send() + .await + .unwrap(); + + let client = Client::builder(TokioExecutor::new()).build_http(); + + let req = |suffix| { + Request::builder() + .method("GET") + .uri(format!("http://127.0.0.1:{}/", ctx.garage.web_port)) + .header("Host", format!("{}{}", BCKT_NAME, suffix)) + .body(Body::new(Bytes::new())) + .unwrap() + }; + + ctx.garage + .command() + .args(["bucket", "website", "--allow", BCKT_NAME]) + .quiet() + .expect_success_status("Could not allow website on bucket"); + + let mut resp = client.request(req("")).await.unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + assert_eq!( + resp.into_body().collect().await.unwrap().to_bytes(), + BODY.as_ref() + ); + + resp = client.request(req(".web.garage")).await.unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + assert_eq!( + resp.into_body().collect().await.unwrap().to_bytes(), + BODY.as_ref() + ); + + for bname in [ + BCKT_NAME.to_string(), + format!("{BCKT_NAME}.web.garage"), + format!("{BCKT_NAME}.s3.garage"), + ] { + let admin_req = || { + Request::builder() + .method("GET") + .uri(format!( + "http://127.0.0.1:{0}/check?domain={1}", + ctx.garage.admin_port, bname + )) + .body(Body::new(Bytes::new())) + .unwrap() + }; + + let admin_resp = client.request(admin_req()).await.unwrap(); + assert_eq!(admin_resp.status(), StatusCode::OK); + assert_eq!( + admin_resp.into_body().collect().await.unwrap().to_bytes(), + format!("Domain '{bname}' is managed by Garage").as_bytes() + ); + } +} diff --git a/src/model/bucket_alias_table.rs b/src/model/bucket_alias_table.rs index 04d808e8..276d0d1c 100644 --- a/src/model/bucket_alias_table.rs +++ b/src/model/bucket_alias_table.rs @@ -76,7 +76,7 @@ impl TableSchema for BucketAliasTable { /// In the case of Garage, bucket names must not be hex-encoded /// 32 byte string, which is excluded thanks to the /// maximum length of 63 bytes given in the spec. -pub fn is_valid_bucket_name(n: &str, punny: bool) -> bool { +pub fn is_valid_bucket_name(n: &str, puny: bool) -> bool { // Bucket names must be between 3 and 63 characters n.len() >= 3 && n.len() <= 63 // Bucket names must be composed of lowercase letters, numbers, @@ -88,9 +88,9 @@ pub fn is_valid_bucket_name(n: &str, punny: bool) -> bool { // Bucket names must not be formatted as an IP address && n.parse::().is_err() // Bucket names must not start with "xn--" - && (!n.starts_with("xn--") || punny) - // We are a bit stricter, to properly restrict punnycode in all labels - && (!n.contains(".xn--") || punny) + && (!n.starts_with("xn--") || puny) + // We are a bit stricter, to properly restrict punycode in all labels + && (!n.contains(".xn--") || puny) // Bucket names must not end with "-s3alias" && !n.ends_with("-s3alias") } diff --git a/src/model/helper/locked.rs b/src/model/helper/locked.rs index 16b0bafc..a5821f77 100644 --- a/src/model/helper/locked.rs +++ b/src/model/helper/locked.rs @@ -57,7 +57,7 @@ impl<'a> LockedHelper<'a> { bucket_id: Uuid, alias_name: &String, ) -> Result<(), Error> { - if !is_valid_bucket_name(alias_name, self.0.config.allow_punnycode) { + if !is_valid_bucket_name(alias_name, self.0.config.allow_punycode) { return Err(Error::InvalidBucketName(alias_name.to_string())); } @@ -217,7 +217,7 @@ impl<'a> LockedHelper<'a> { ) -> Result<(), Error> { let key_helper = KeyHelper(self.0); - if !is_valid_bucket_name(alias_name, self.0.config.allow_punnycode) { + if !is_valid_bucket_name(alias_name, self.0.config.allow_punycode) { return Err(Error::InvalidBucketName(alias_name.to_string())); } diff --git a/src/util/config.rs b/src/util/config.rs index f128177b..c74029e7 100644 --- a/src/util/config.rs +++ b/src/util/config.rs @@ -136,9 +136,9 @@ pub struct Config { #[serde(default = "Default::default")] pub admin: AdminConfig, - /// Allow punnycode in bucket names + /// Allow punycode in bucket names #[serde(default)] - pub allow_punnycode: bool, + pub allow_punycode: bool, } /// Value for data_dir: either a single directory or a list of dirs with attributes From c6bc3f229b5cba9625b240cf60117ba5dc3fba50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arma=C3=ABl=20Gu=C3=A9neau?= Date: Thu, 15 May 2025 23:30:00 +0200 Subject: [PATCH 11/17] Fix behavior of CopyObject wrt x-amz-website-redirect-location --- src/api/s3/copy.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/api/s3/copy.rs b/src/api/s3/copy.rs index a5b2d706..edda7e0f 100644 --- a/src/api/s3/copy.rs +++ b/src/api/s3/copy.rs @@ -29,6 +29,7 @@ use crate::error::*; use crate::get::{full_object_byte_stream, PreconditionHeaders}; use crate::multipart; use crate::put::{extract_metadata_headers, save_stream, ChecksumMode, SaveStreamResult}; +use crate::website::X_AMZ_WEBSITE_REDIRECT_LOCATION; use crate::xml::{self as s3_xml, xmlns_tag}; pub const X_AMZ_COPY_SOURCE_IF_MATCH: HeaderName = @@ -84,7 +85,18 @@ pub async fn handle_copy( Some(v) if v == hyper::header::HeaderValue::from_static("REPLACE") => { extract_metadata_headers(req.headers())? } - _ => source_object_meta_inner.into_owned().headers, + _ => { + // The x-amz-website-redirect-location header is not copied, instead + // it is replaced by the value from the request (or removed if no + // value was specified) + let is_redirect = + |(key, _): &(String, String)| key == X_AMZ_WEBSITE_REDIRECT_LOCATION.as_str(); + let mut headers: Vec<_> = source_object_meta_inner.headers.clone(); + headers.retain(|h| !is_redirect(h)); + let new_headers = extract_metadata_headers(req.headers())?; + headers.extend(new_headers.into_iter().filter(is_redirect)); + headers + } }, checksum: source_checksum, }; From 2dc3a6dbbe88a3498a9fc39c50aeb94124c39781 Mon Sep 17 00:00:00 2001 From: trinity-1686a Date: Thu, 22 May 2025 14:08:06 +0200 Subject: [PATCH 12/17] document allow_punycode configuration option --- doc/book/reference-manual/configuration.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/doc/book/reference-manual/configuration.md b/doc/book/reference-manual/configuration.md index e0fc17bc..09ce8d24 100644 --- a/doc/book/reference-manual/configuration.md +++ b/doc/book/reference-manual/configuration.md @@ -46,6 +46,7 @@ bootstrap_peers = [ "212fd62eeaca72c122b45a7f4fa0f55e012aa5e24ac384a72a3016413fa724ff@[fc00:F::1]:3901", ] +allow_punycode = false [consul_discovery] api = "catalog" @@ -115,6 +116,7 @@ Top-level configuration options: [`rpc_public_addr`](#rpc_public_addr), [`rpc_public_addr_subnet`](#rpc_public_addr_subnet) [`rpc_secret`/`rpc_secret_file`](#rpc_secret). +[`allow_punycode`](#allow_punycode). The `[consul_discovery]` section: [`api`](#consul_api), @@ -604,7 +606,7 @@ be obtained by running `garage node id` and then included directly in the key will be returned by `garage node id` and you will have to add the IP yourself. -### `allow_world_readable_secrets` or `GARAGE_ALLOW_WORLD_READABLE_SECRETS` (env) {#allow_world_readable_secrets} +#### `allow_world_readable_secrets` or `GARAGE_ALLOW_WORLD_READABLE_SECRETS` (env) {#allow_world_readable_secrets} Garage checks the permissions of your secret files to make sure they're not world-readable. In some cases, the check might fail and consider your files as @@ -616,6 +618,13 @@ permission verification. Alternatively, you can set the `GARAGE_ALLOW_WORLD_READABLE_SECRETS` environment variable to `true` to bypass the permissions check. +#### `allow_punycode` {#allow_punycode} + +Allow creating buckets with names containing punycode. When used for buckets served +as websites, this allows using almost any unicode character in the domain name. + +Default to `false`. + ### The `[consul_discovery]` section Garage supports discovering other nodes of the cluster using Consul. For this From ae3f7ee76cf3b45348ba97864313c8f6ddde6e7f Mon Sep 17 00:00:00 2001 From: Renjaya Raga Zenta Date: Tue, 20 May 2025 18:47:50 +0700 Subject: [PATCH 13/17] api: lifecycle: 404 if missing lifecycle config --- src/api/s3/lifecycle.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/api/s3/lifecycle.rs b/src/api/s3/lifecycle.rs index c140494e..ccda6cfd 100644 --- a/src/api/s3/lifecycle.rs +++ b/src/api/s3/lifecycle.rs @@ -27,7 +27,7 @@ pub async fn handle_get_lifecycle(ctx: ReqCtx) -> Result, Erro .body(string_body(xml))?) } else { Ok(Response::builder() - .status(StatusCode::NO_CONTENT) + .status(StatusCode::NOT_FOUND) .body(empty_body())?) } } From 0fd1b7342ba5626c97f832726567ba73b72aec0b Mon Sep 17 00:00:00 2001 From: babykart Date: Sat, 22 Mar 2025 21:03:52 +0100 Subject: [PATCH 14/17] Add Kubernetes CRD and the related kustomization Signed-off-by: babykart --- script/k8s/crd/garagenodes.deuxfleurs.fr.yaml | 43 +++++++++++++++++++ script/k8s/crd/kustomization.yaml | 5 +++ 2 files changed, 48 insertions(+) create mode 100644 script/k8s/crd/garagenodes.deuxfleurs.fr.yaml create mode 100644 script/k8s/crd/kustomization.yaml diff --git a/script/k8s/crd/garagenodes.deuxfleurs.fr.yaml b/script/k8s/crd/garagenodes.deuxfleurs.fr.yaml new file mode 100644 index 00000000..cd0fb166 --- /dev/null +++ b/script/k8s/crd/garagenodes.deuxfleurs.fr.yaml @@ -0,0 +1,43 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: garagenodes.deuxfleurs.fr +spec: + conversion: + strategy: None + group: deuxfleurs.fr + names: + kind: GarageNode + listKind: GarageNodeList + plural: garagenodes + singular: garagenode + scope: Namespaced + versions: + - name: v1 + schema: + openAPIV3Schema: + description: Auto-generated derived type for Node via `CustomResource` + properties: + spec: + properties: + address: + format: ip + type: string + hostname: + type: string + port: + format: uint16 + minimum: 0 + type: integer + required: + - address + - hostname + - port + type: object + required: + - spec + title: GarageNode + type: object + served: true + storage: true + subresources: {} \ No newline at end of file diff --git a/script/k8s/crd/kustomization.yaml b/script/k8s/crd/kustomization.yaml new file mode 100644 index 00000000..9f20eccf --- /dev/null +++ b/script/k8s/crd/kustomization.yaml @@ -0,0 +1,5 @@ +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization + +resources: +- garagenodes.deuxfleurs.fr.yaml \ No newline at end of file From b15e2cbb6ccc044b868cd8c56306c0b3a34610fa Mon Sep 17 00:00:00 2001 From: babykart Date: Sat, 22 Mar 2025 23:44:55 +0100 Subject: [PATCH 15/17] Update Kubernetes cookbook Signed-off-by: babykart --- doc/book/cookbook/kubernetes.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/doc/book/cookbook/kubernetes.md b/doc/book/cookbook/kubernetes.md index af04e94d..1e7674d7 100644 --- a/doc/book/cookbook/kubernetes.md +++ b/doc/book/cookbook/kubernetes.md @@ -26,6 +26,13 @@ Or deploy with custom values: helm install --create-namespace --namespace garage garage ./garage -f values.override.yaml ``` +If you want to manage the CustomRessourceDefinition used by garage for its `kubernetes_discovery` outside of the helm chart, add `garage.kubernetesSkipCrd: true` to your custom values and use the kustomization before deploying the helm chart: + +```bash +kubectl apply -k ../k8s/crd +helm install --create-namespace --namespace garage garage ./garage -f values.override.yaml +``` + After deploying, cluster layout must be configured manually as described in [Creating a cluster layout](@/documentation/quick-start/_index.md#creating-a-cluster-layout). Use the following command to access garage CLI: ```bash From 2ade8c86f62f0e9eafc2b6515b48f1d45722fb5a Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Tue, 18 Mar 2025 11:35:55 +0100 Subject: [PATCH 16/17] more resilience to inconsistent alias states --- src/api/admin/bucket.rs | 2 +- src/api/s3/bucket.rs | 4 +- src/model/helper/locked.rs | 118 +++++++++++++++++++++++++------------ 3 files changed, 84 insertions(+), 40 deletions(-) diff --git a/src/api/admin/bucket.rs b/src/api/admin/bucket.rs index f8bd1eb5..207693b6 100644 --- a/src/api/admin/bucket.rs +++ b/src/api/admin/bucket.rs @@ -382,7 +382,7 @@ pub async fn handle_delete_bucket( for ((key_id, alias), _, active) in state.local_aliases.items().iter() { if *active { helper - .unset_local_bucket_alias(bucket.id, key_id, alias) + .purge_local_bucket_alias(bucket.id, key_id, alias) .await?; } } diff --git a/src/api/s3/bucket.rs b/src/api/s3/bucket.rs index 23cceb84..26e2fc49 100644 --- a/src/api/s3/bucket.rs +++ b/src/api/s3/bucket.rs @@ -241,11 +241,11 @@ pub async fn handle_delete_bucket(ctx: ReqCtx) -> Result, Erro // 1. delete bucket alias if is_local_alias { helper - .unset_local_bucket_alias(*bucket_id, &api_key.key_id, bucket_name) + .purge_local_bucket_alias(*bucket_id, &api_key.key_id, bucket_name) .await?; } else { helper - .unset_global_bucket_alias(*bucket_id, bucket_name) + .purge_global_bucket_alias(*bucket_id, bucket_name) .await?; } diff --git a/src/model/helper/locked.rs b/src/model/helper/locked.rs index a5821f77..ecb24854 100644 --- a/src/model/helper/locked.rs +++ b/src/model/helper/locked.rs @@ -47,6 +47,10 @@ impl<'a> LockedHelper<'a> { KeyHelper(self.0) } + // ================================================ + // global bucket aliases + // ================================================ + /// Sets a new alias for a bucket in global namespace. /// This function fails if: /// - alias name is not valid according to S3 spec @@ -179,13 +183,14 @@ impl<'a> LockedHelper<'a> { .ok_or_else(|| Error::NoSuchBucket(alias_name.to_string()))?; // Checks ok, remove alias - let alias_ts = match bucket.state.as_option() { - Some(bucket_state) => increment_logical_clock_2( - alias.state.timestamp(), - bucket_state.aliases.get_timestamp(alias_name), - ), - None => increment_logical_clock(alias.state.timestamp()), - }; + let alias_ts = increment_logical_clock_2( + alias.state.timestamp(), + bucket + .state + .as_option() + .map(|p| p.aliases.get_timestamp(alias_name)) + .unwrap_or(0), + ); // ---- timestamp-ensured causality barrier ---- // writes are now done and all writes use timestamp alias_ts @@ -203,6 +208,10 @@ impl<'a> LockedHelper<'a> { Ok(()) } + // ================================================ + // local bucket aliases + // ================================================ + /// Sets a new alias for a bucket in the local namespace of a key. /// This function fails if: /// - alias name is not valid according to S3 spec @@ -215,14 +224,12 @@ impl<'a> LockedHelper<'a> { key_id: &String, alias_name: &String, ) -> Result<(), Error> { - let key_helper = KeyHelper(self.0); - if !is_valid_bucket_name(alias_name, self.0.config.allow_punycode) { return Err(Error::InvalidBucketName(alias_name.to_string())); } let mut bucket = self.bucket().get_existing_bucket(bucket_id).await?; - let mut key = key_helper.get_existing_key(key_id).await?; + let mut key = self.key().get_existing_key(key_id).await?; let key_param = key.state.as_option_mut().unwrap(); @@ -271,23 +278,13 @@ impl<'a> LockedHelper<'a> { key_id: &String, alias_name: &String, ) -> Result<(), Error> { - let key_helper = KeyHelper(self.0); - let mut bucket = self.bucket().get_existing_bucket(bucket_id).await?; - let mut key = key_helper.get_existing_key(key_id).await?; + let mut key = self.key().get_existing_key(key_id).await?; + let key_p = key.state.as_option().unwrap(); let bucket_p = bucket.state.as_option_mut().unwrap(); - if key - .state - .as_option() - .unwrap() - .local_aliases - .get(alias_name) - .cloned() - .flatten() - != Some(bucket_id) - { + if key_p.local_aliases.get(alias_name).cloned().flatten() != Some(bucket_id) { return Err(GarageError::Message(format!( "Bucket {:?} does not have alias {} in namespace of key {}", bucket_id, alias_name, key_id @@ -304,17 +301,17 @@ impl<'a> LockedHelper<'a> { .local_aliases .items() .iter() - .any(|((k, n), _, active)| *k == key.key_id && n == alias_name && *active); + .any(|((k, n), _, active)| (*k != key.key_id || n != alias_name) && *active); + if !has_other_global_aliases && !has_other_local_aliases { return Err(Error::BadRequest(format!("Bucket {} doesn't have other aliases, please delete it instead of just unaliasing.", alias_name))); } // Checks ok, remove alias - let key_param = key.state.as_option_mut().unwrap(); let bucket_p_local_alias_key = (key.key_id.clone(), alias_name.clone()); let alias_ts = increment_logical_clock_2( - key_param.local_aliases.get_timestamp(alias_name), + key_p.local_aliases.get_timestamp(alias_name), bucket_p .local_aliases .get_timestamp(&bucket_p_local_alias_key), @@ -323,7 +320,8 @@ impl<'a> LockedHelper<'a> { // ---- timestamp-ensured causality barrier ---- // writes are now done and all writes use timestamp alias_ts - key_param.local_aliases = LwwMap::raw_item(alias_name.clone(), alias_ts, None); + key.state.as_option_mut().unwrap().local_aliases = + LwwMap::raw_item(alias_name.clone(), alias_ts, None); self.0.key_table.insert(&key).await?; bucket_p.local_aliases = LwwMap::raw_item(bucket_p_local_alias_key, alias_ts, false); @@ -332,21 +330,68 @@ impl<'a> LockedHelper<'a> { Ok(()) } + /// Ensures a bucket does not have a certain local alias. + /// Contrarily to unset_local_bucket_alias, this does not + /// fail on any condition other than: + /// - bucket cannot be found (its fine if it is in deleted state) + /// - key cannot be found (its fine if alias in key points to nothing + /// or to another bucket) + pub async fn purge_local_bucket_alias( + &self, + bucket_id: Uuid, + key_id: &String, + alias_name: &String, + ) -> Result<(), Error> { + let mut bucket = self.bucket().get_internal_bucket(bucket_id).await?; + let mut key = self.key().get_internal_key(key_id).await?; + + let bucket_p_local_alias_key = (key.key_id.clone(), alias_name.clone()); + + let alias_ts = increment_logical_clock_2( + key.state + .as_option() + .map(|p| p.local_aliases.get_timestamp(alias_name)) + .unwrap_or(0), + bucket + .state + .as_option() + .map(|p| p.local_aliases.get_timestamp(&bucket_p_local_alias_key)) + .unwrap_or(0), + ); + + // ---- timestamp-ensured causality barrier ---- + // writes are now done and all writes use timestamp alias_ts + + if let Some(kp) = key.state.as_option_mut() { + kp.local_aliases = LwwMap::raw_item(alias_name.clone(), alias_ts, None); + self.0.key_table.insert(&key).await?; + } + + if let Some(bp) = bucket.state.as_option_mut() { + bp.local_aliases = LwwMap::raw_item(bucket_p_local_alias_key, alias_ts, false); + self.0.bucket_table.insert(&bucket).await?; + } + + Ok(()) + } + + // ================================================ + // permissions + // ================================================ + /// Sets permissions for a key on a bucket. /// This function fails if: /// - bucket or key cannot be found at all (its ok if they are in deleted state) - /// - bucket or key is in deleted state and we are trying to set permissions other than "deny - /// all" + /// - bucket or key is in deleted state and we are trying to set + /// permissions other than "deny all" pub async fn set_bucket_key_permissions( &self, bucket_id: Uuid, key_id: &String, mut perm: BucketKeyPerm, ) -> Result<(), Error> { - let key_helper = KeyHelper(self.0); - let mut bucket = self.bucket().get_internal_bucket(bucket_id).await?; - let mut key = key_helper.get_internal_key(key_id).await?; + let mut key = self.key().get_internal_key(key_id).await?; if let Some(bstate) = bucket.state.as_option() { if let Some(kp) = bstate.authorized_keys.get(key_id) { @@ -383,21 +428,20 @@ impl<'a> LockedHelper<'a> { Ok(()) } - // ---- + // ================================================ + // keys + // ================================================ /// Deletes an API access key pub async fn delete_key(&self, key: &mut Key) -> Result<(), Error> { let state = key.state.as_option_mut().unwrap(); // --- done checking, now commit --- - // (the step at unset_local_bucket_alias will fail if a bucket - // does not have another alias, the deletion will be - // interrupted in the middle if that happens) // 1. Delete local aliases for (alias, _, to) in state.local_aliases.items().iter() { if let Some(bucket_id) = to { - self.unset_local_bucket_alias(*bucket_id, &key.key_id, alias) + self.purge_local_bucket_alias(*bucket_id, &key.key_id, alias) .await?; } } From 8654eb19bf8a59f8ece8ad70ac8096c799858876 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Wed, 19 Mar 2025 12:39:32 +0100 Subject: [PATCH 17/17] implement repair procedure to fix inconsistent bucket aliases --- src/garage/cli/structs.rs | 3 + src/garage/repair/online.rs | 4 + src/model/helper/locked.rs | 193 ++++++++++++++++++++++++++++++++++++ 3 files changed, 200 insertions(+) diff --git a/src/garage/cli/structs.rs b/src/garage/cli/structs.rs index 4ec35e68..3652ef6b 100644 --- a/src/garage/cli/structs.rs +++ b/src/garage/cli/structs.rs @@ -478,6 +478,9 @@ pub enum RepairWhat { /// Recalculate block reference counters #[structopt(name = "block-rc", version = garage_version())] BlockRc, + /// Fix inconsistency in bucket aliases (WARNING: EXPERIMENTAL) + #[structopt(name = "aliases", version = garage_version())] + Aliases, /// Verify integrity of all blocks on disc #[structopt(name = "scrub", version = garage_version())] Scrub { diff --git a/src/garage/repair/online.rs b/src/garage/repair/online.rs index 47883f97..950cd5f7 100644 --- a/src/garage/repair/online.rs +++ b/src/garage/repair/online.rs @@ -88,6 +88,10 @@ pub async fn launch_online_repair( garage.block_manager.clone(), )); } + RepairWhat::Aliases => { + info!("Repairing bucket aliases (foreground)"); + garage.locked_helper().await.repair_aliases().await?; + } } Ok(()) } diff --git a/src/model/helper/locked.rs b/src/model/helper/locked.rs index ecb24854..98344b63 100644 --- a/src/model/helper/locked.rs +++ b/src/model/helper/locked.rs @@ -1,3 +1,7 @@ +use std::collections::{HashMap, HashSet}; + +use garage_db as db; + use garage_util::crdt::*; use garage_util::data::*; use garage_util::error::{Error as GarageError, OkOrMessage}; @@ -458,4 +462,193 @@ impl<'a> LockedHelper<'a> { Ok(()) } + + // ================================================ + // repair procedure + // ================================================ + + pub async fn repair_aliases(&self) -> Result<(), GarageError> { + self.0.db.transaction(|tx| { + info!("--- begin repair_aliases transaction ----"); + + // 1. List all non-deleted buckets, so that we can fix bad aliases + let mut all_buckets: HashSet = HashSet::new(); + + for item in tx.range::<&[u8], _>(&self.0.bucket_table.data.store, ..)? { + let bucket = self + .0 + .bucket_table + .data + .decode_entry(&(item?.1)) + .map_err(db::TxError::Abort)?; + if !bucket.is_deleted() { + all_buckets.insert(bucket.id); + } + } + + info!("number of buckets: {}", all_buckets.len()); + + // 2. List all aliases declared in bucket_alias_table and key_table + // Take note of aliases that point to non-existing buckets + let mut global_aliases: HashMap = HashMap::new(); + + { + let mut delete_global = vec![]; + for item in tx.range::<&[u8], _>(&self.0.bucket_alias_table.data.store, ..)? { + let mut alias = self + .0 + .bucket_alias_table + .data + .decode_entry(&(item?.1)) + .map_err(db::TxError::Abort)?; + if let Some(id) = alias.state.get() { + if all_buckets.contains(id) { + // keep aliases + global_aliases.insert(alias.name().to_string(), *id); + } else { + // delete alias + warn!( + "global alias: remove {} -> {:?} (bucket is deleted)", + alias.name(), + id + ); + alias.state.update(None); + delete_global.push(alias); + } + } + } + + info!("number of global aliases: {}", global_aliases.len()); + + info!("global alias table: {} entries fixed", delete_global.len()); + for ga in delete_global { + debug!("Enqueue update to global alias table: {:?}", ga); + self.0.bucket_alias_table.queue_insert(tx, &ga)?; + } + } + + let mut local_aliases: HashMap<(String, String), Uuid> = HashMap::new(); + + { + let mut delete_local = vec![]; + + for item in tx.range::<&[u8], _>(&self.0.key_table.data.store, ..)? { + let mut key = self + .0 + .key_table + .data + .decode_entry(&(item?.1)) + .map_err(db::TxError::Abort)?; + let Some(p) = key.state.as_option_mut() else { + continue; + }; + let mut has_changes = false; + for (name, _, to) in p.local_aliases.items().to_vec() { + if let Some(id) = to { + if all_buckets.contains(&id) { + local_aliases.insert((key.key_id.clone(), name), id); + } else { + warn!( + "local alias: remove ({}, {}) -> {:?} (bucket is deleted)", + key.key_id, name, id + ); + p.local_aliases.update_in_place(name, None); + has_changes = true; + } + } + } + if has_changes { + delete_local.push(key); + } + } + + info!("number of local aliases: {}", local_aliases.len()); + + info!("key table: {} entries fixed", delete_local.len()); + for la in delete_local { + debug!("Enqueue update to key table: {:?}", la); + self.0.key_table.queue_insert(tx, &la)?; + } + } + + // 4. Reverse the alias maps to determine the aliases per-bucket + let mut bucket_global: HashMap> = HashMap::new(); + let mut bucket_local: HashMap> = HashMap::new(); + + for (name, bucket) in global_aliases { + bucket_global.entry(bucket).or_default().push(name); + } + for ((key, name), bucket) in local_aliases { + bucket_local.entry(bucket).or_default().push((key, name)); + } + + // 5. Fix the bucket table to ensure consistency + let mut bucket_updates = vec![]; + + for item in tx.range::<&[u8], _>(&self.0.bucket_table.data.store, ..)? { + let bucket = self + .0 + .bucket_table + .data + .decode_entry(&(item?.1)) + .map_err(db::TxError::Abort)?; + let mut bucket2 = bucket.clone(); + let Some(param) = bucket2.state.as_option_mut() else { + continue; + }; + + // fix global aliases + { + let ga = bucket_global.remove(&bucket.id).unwrap_or_default(); + for (name, _, active) in param.aliases.items().to_vec() { + if active && !ga.contains(&name) { + warn!("bucket {:?}: remove global alias {}", bucket.id, name); + param.aliases.update_in_place(name, false); + } + } + for name in ga { + if param.aliases.get(&name).copied() != Some(true) { + warn!("bucket {:?}: add global alias {}", bucket.id, name); + param.aliases.update_in_place(name, true); + } + } + } + + // fix local aliases + { + let la = bucket_local.remove(&bucket.id).unwrap_or_default(); + for (pair, _, active) in param.local_aliases.items().to_vec() { + if active && !la.contains(&pair) { + warn!("bucket {:?}: remove local alias {:?}", bucket.id, pair); + param.local_aliases.update_in_place(pair, false); + } + } + for pair in la { + if param.local_aliases.get(&pair).copied() != Some(true) { + warn!("bucket {:?}: add local alias {:?}", bucket.id, pair); + param.local_aliases.update_in_place(pair, true); + } + } + } + + if bucket2 != bucket { + bucket_updates.push(bucket2); + } + } + + info!("bucket table: {} entries fixed", bucket_updates.len()); + for b in bucket_updates { + debug!("Enqueue update to bucket table: {:?}", b); + self.0.bucket_table.queue_insert(tx, &b)?; + } + + info!("--- end repair_aliases transaction ----"); + + Ok(()) + })?; + + info!("repair_aliases is done"); + + Ok(()) + } }