From b6cf26f65d318aaf2f9e77f0614f1f3108a32afd Mon Sep 17 00:00:00 2001 From: Liuqing Yue Date: Sat, 28 Oct 2023 17:50:38 +0800 Subject: [PATCH 1/7] ci(services/azfile): add azfile integration test Signed-off-by: Liuqing Yue --- .github/workflows/service_test_azfile.yml | 68 +++++++++++++++++++++++ core/src/services/azfile/backend.rs | 3 +- core/src/services/azfile/core.rs | 13 ++++- core/src/types/operator/builder.rs | 2 + core/src/types/scheme.rs | 2 + 5 files changed, 84 insertions(+), 4 deletions(-) create mode 100644 .github/workflows/service_test_azfile.yml diff --git a/.github/workflows/service_test_azfile.yml b/.github/workflows/service_test_azfile.yml new file mode 100644 index 000000000000..e44b6b0154fa --- /dev/null +++ b/.github/workflows/service_test_azfile.yml @@ -0,0 +1,68 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +name: Service Test Azfile + +on: + push: + branches: + - main + pull_request: + branches: + - main + paths: + - "core/src/**" + - "core/tests/**" + - "!core/src/docs/**" + - "!core/src/services/**" + - "core/src/services/azfile/**" + - ".github/workflows/service_test_azfile.yml" + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }}-${{ github.event_name }} + cancel-in-progress: true + +jobs: + azure_azfile: + runs-on: ubuntu-latest + if: github.event_name == 'push' || !github.event.pull_request.head.repo.fork + steps: + - uses: actions/checkout@v4 + - name: Setup Rust toolchain + uses: ./.github/actions/setup + with: + need-nextest: true + + - name: Load secret + id: op-load-secret + uses: 1password/load-secrets-action@v1 + with: + export-env: true + env: + OP_SERVICE_ACCOUNT_TOKEN: ${{ secrets.OP_SERVICE_ACCOUNT_TOKEN }} + OPENDAL_AZFILE_ACCOUNT_NAME: op://services/azfile/account_name + OPENDAL_AZFILE_ENDPOINT: op://services/azfile/endpoint + OPENDAL_AZFILE_ACCOUNT_KEY: op://services/azfile/account_key + OPENDAL_AZFILE_SHARE_NAME: op://services/azfile/share_name + + - name: Test + shell: bash + working-directory: core + run: cargo nextest run behavior + env: + OPENDAL_TEST: azfile + OPENDAL_AZDLS_ROOT: test/ diff --git a/core/src/services/azfile/backend.rs b/core/src/services/azfile/backend.rs index c7d8ad41cb91..e77f09af56b7 100644 --- a/core/src/services/azfile/backend.rs +++ b/core/src/services/azfile/backend.rs @@ -195,6 +195,7 @@ impl Builder for AzfileBuilder { let config_loader = AzureStorageConfig { account_name: Some(account_name), + account_key: self.account_key.clone(), sas_token: self.sas_token.clone(), ..Default::default() }; @@ -380,7 +381,7 @@ impl Accessor for AzfileBackend { let status = resp.status(); match status { - StatusCode::ACCEPTED => { + StatusCode::ACCEPTED | StatusCode::NOT_FOUND => { resp.into_body().consume().await?; Ok(RpDelete::default()) } diff --git a/core/src/services/azfile/core.rs b/core/src/services/azfile/core.rs index ce9717ac5536..57a8cf8ffeb3 100644 --- a/core/src/services/azfile/core.rs +++ b/core/src/services/azfile/core.rs @@ -38,7 +38,7 @@ use crate::*; const X_MS_VERSION: &str = "x-ms-version"; const X_MS_WRITE: &str = "x-ms-write"; -const X_MS_RENAME_SOURCE: &str = "x-ms-rename-source"; +const X_MS_FILE_RENAME_SOURCE: &str = "x-ms-file-rename-source"; const X_MS_CONTENT_LENGTH: &str = "x-ms-content-length"; const X_MS_TYPE: &str = "x-ms-type"; @@ -269,7 +269,7 @@ impl AzfileCore { "{}/{}/{}?comp=rename", self.endpoint, self.share_name, - percent_encode_path(&p) + percent_encode_path(&new_p) ) }; @@ -277,7 +277,14 @@ impl AzfileCore { req = req.header(CONTENT_LENGTH, 0); - req = req.header(X_MS_RENAME_SOURCE, percent_encode_path(&new_p)); + let destination_url = format!( + "{}/{}/{}", + self.endpoint, + self.share_name, + percent_encode_path(&p) + ); + + req = req.header(X_MS_FILE_RENAME_SOURCE, &destination_url); let mut req = req .body(AsyncBody::Empty) diff --git a/core/src/types/operator/builder.rs b/core/src/types/operator/builder.rs index c7976e2c7b58..211444064581 100644 --- a/core/src/types/operator/builder.rs +++ b/core/src/types/operator/builder.rs @@ -159,6 +159,8 @@ impl Operator { Scheme::Azblob => Self::from_map::(map)?.finish(), #[cfg(feature = "services-azdls")] Scheme::Azdls => Self::from_map::(map)?.finish(), + #[cfg(feature = "services-azfile")] + Scheme::Azfile => Self::from_map::(map)?.finish(), #[cfg(feature = "services-cacache")] Scheme::Cacache => Self::from_map::(map)?.finish(), #[cfg(feature = "services-cos")] diff --git a/core/src/types/scheme.rs b/core/src/types/scheme.rs index 879dc6b9e19a..32b84c04c5a9 100644 --- a/core/src/types/scheme.rs +++ b/core/src/types/scheme.rs @@ -165,6 +165,8 @@ impl Scheme { Scheme::Azblob, #[cfg(feature = "services-azdls")] Scheme::Azdls, + #[cfg(feature = "services-azfile")] + Scheme::Azfile, #[cfg(feature = "services-cacache")] Scheme::Cacache, #[cfg(feature = "services-cos")] From c8ce2338a96072cdc25cc8be1dbd6e75bccfba6e Mon Sep 17 00:00:00 2001 From: Liuqing Yue Date: Sat, 28 Oct 2023 17:59:52 +0800 Subject: [PATCH 2/7] docs: add comment for special case --- core/src/services/azfile/backend.rs | 5 +++++ core/src/services/azfile/core.rs | 11 ++++++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/core/src/services/azfile/backend.rs b/core/src/services/azfile/backend.rs index e77f09af56b7..75e475699e97 100644 --- a/core/src/services/azfile/backend.rs +++ b/core/src/services/azfile/backend.rs @@ -290,6 +290,11 @@ impl Accessor for AzfileBackend { Ok(RpCreateDir::default()) } _ => { + // we cannot just check status code because 409 Conflict has two meaning: + // 1. If a directory by the same name is being deleted when Create Directory is called, the server returns status code 409 (Conflict) + // 2. If a directory or file with the same name already exists, the operation fails with status code 409 (Conflict). + // but we just need case 2 (already exists) + // ref: https://learn.microsoft.com/en-us/rest/api/storageservices/create-directory if resp .headers() .get("x-ms-error-code") diff --git a/core/src/services/azfile/core.rs b/core/src/services/azfile/core.rs index 57a8cf8ffeb3..8c86bb8347f2 100644 --- a/core/src/services/azfile/core.rs +++ b/core/src/services/azfile/core.rs @@ -262,7 +262,7 @@ impl AzfileCore { "{}/{}/{}?restype=directory&comp=rename", self.endpoint, self.share_name, - percent_encode_path(&p) + percent_encode_path(&new_p) ) } else { format!( @@ -277,14 +277,19 @@ impl AzfileCore { req = req.header(CONTENT_LENGTH, 0); - let destination_url = format!( + // x-ms-file-rename-source specifies the file or directory to be renamed. + // the value must be a URL style path + // the official document does not mention the URL style path + // find the solution from the community FAQ and implementation of the Java-SDK + // ref: https://learn.microsoft.com/en-us/answers/questions/799611/azure-file-service-rest-api(rename)?page=1 + let source_url = format!( "{}/{}/{}", self.endpoint, self.share_name, percent_encode_path(&p) ); - req = req.header(X_MS_FILE_RENAME_SOURCE, &destination_url); + req = req.header(X_MS_FILE_RENAME_SOURCE, &source_url); let mut req = req .body(AsyncBody::Empty) From c0a65ea3fdd4b5dee8e562b57a6e5a8b363d14b8 Mon Sep 17 00:00:00 2001 From: Liuqing Yue Date: Sat, 28 Oct 2023 18:08:01 +0800 Subject: [PATCH 3/7] fix: test_rename_overwrite should pass --- core/src/services/azfile/core.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/src/services/azfile/core.rs b/core/src/services/azfile/core.rs index 8c86bb8347f2..ef47e85499f4 100644 --- a/core/src/services/azfile/core.rs +++ b/core/src/services/azfile/core.rs @@ -41,6 +41,7 @@ const X_MS_WRITE: &str = "x-ms-write"; const X_MS_FILE_RENAME_SOURCE: &str = "x-ms-file-rename-source"; const X_MS_CONTENT_LENGTH: &str = "x-ms-content-length"; const X_MS_TYPE: &str = "x-ms-type"; +const X_MS_FILE_RENAME_REPLACE_IF_EXISTS: &str = "x-ms-file-rename-replace-if-exists"; pub struct AzfileCore { pub root: String, @@ -291,6 +292,8 @@ impl AzfileCore { req = req.header(X_MS_FILE_RENAME_SOURCE, &source_url); + req = req.header(X_MS_FILE_RENAME_REPLACE_IF_EXISTS, "true"); + let mut req = req .body(AsyncBody::Empty) .map_err(new_request_build_error)?; From 4311f19f0f62c11432b80c880a3e9300520107e2 Mon Sep 17 00:00:00 2001 From: Liuqing Yue Date: Sat, 28 Oct 2023 18:16:23 +0800 Subject: [PATCH 4/7] fix: bypass test_reader_tail because not support --- core/src/services/azfile/core.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/core/src/services/azfile/core.rs b/core/src/services/azfile/core.rs index ef47e85499f4..84fdc8a9faf3 100644 --- a/core/src/services/azfile/core.rs +++ b/core/src/services/azfile/core.rs @@ -112,7 +112,19 @@ impl AzfileCore { let mut req = Request::get(&url); - req = req.header(RANGE, range.to_header()); + if !range.is_full() { + // azfile doesn't support read with suffix range. + // + // ref: https://learn.microsoft.com/en-us/rest/api/storageservices/specifying-the-range-header-for-file-service-operations + if range.offset().is_none() && range.size().is_some() { + return Err(Error::new( + ErrorKind::Unsupported, + "azblob doesn't support read with suffix range", + )); + } + + req = req.header(RANGE, range.to_header()); + } let mut req = req .body(AsyncBody::Empty) From bce1eb35a7a791fb19219e00c6f58a7d9e089efc Mon Sep 17 00:00:00 2001 From: Liuqing Yue Date: Sat, 28 Oct 2023 20:21:30 +0800 Subject: [PATCH 5/7] refactor: migrate to test planner Signed-off-by: Liuqing Yue --- .github/services/azfile/azfile/action.yml | 40 +++++++++++++ .github/workflows/service_test_azfile.yml | 68 ----------------------- 2 files changed, 40 insertions(+), 68 deletions(-) create mode 100644 .github/services/azfile/azfile/action.yml delete mode 100644 .github/workflows/service_test_azfile.yml diff --git a/.github/services/azfile/azfile/action.yml b/.github/services/azfile/azfile/action.yml new file mode 100644 index 000000000000..b07006934a0e --- /dev/null +++ b/.github/services/azfile/azfile/action.yml @@ -0,0 +1,40 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +name: azfile +description: 'Behavior test for Azfile' + +runs: + using: "composite" + steps: + - name: Setup + uses: 1password/load-secrets-action@v1 + with: + export-env: true + env: + OP_SERVICE_ACCOUNT_TOKEN: ${{ secrets.OP_SERVICE_ACCOUNT_TOKEN }} + OPENDAL_AZFILE_ACCOUNT_NAME: op://services/azfile/account_name + OPENDAL_AZFILE_ENDPOINT: op://services/azfile/endpoint + OPENDAL_AZFILE_ACCOUNT_KEY: op://services/azfile/account_key + OPENDAL_AZFILE_SHARE_NAME: op://services/azfile/share_name + + - name: Add extra settings + shell: bash + run: | + cat << EOF >> $GITHUB_ENV + OPENDAL_AZFILE_ROOT=test/ + EOF diff --git a/.github/workflows/service_test_azfile.yml b/.github/workflows/service_test_azfile.yml deleted file mode 100644 index e44b6b0154fa..000000000000 --- a/.github/workflows/service_test_azfile.yml +++ /dev/null @@ -1,68 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. - -name: Service Test Azfile - -on: - push: - branches: - - main - pull_request: - branches: - - main - paths: - - "core/src/**" - - "core/tests/**" - - "!core/src/docs/**" - - "!core/src/services/**" - - "core/src/services/azfile/**" - - ".github/workflows/service_test_azfile.yml" - -concurrency: - group: ${{ github.workflow }}-${{ github.ref }}-${{ github.event_name }} - cancel-in-progress: true - -jobs: - azure_azfile: - runs-on: ubuntu-latest - if: github.event_name == 'push' || !github.event.pull_request.head.repo.fork - steps: - - uses: actions/checkout@v4 - - name: Setup Rust toolchain - uses: ./.github/actions/setup - with: - need-nextest: true - - - name: Load secret - id: op-load-secret - uses: 1password/load-secrets-action@v1 - with: - export-env: true - env: - OP_SERVICE_ACCOUNT_TOKEN: ${{ secrets.OP_SERVICE_ACCOUNT_TOKEN }} - OPENDAL_AZFILE_ACCOUNT_NAME: op://services/azfile/account_name - OPENDAL_AZFILE_ENDPOINT: op://services/azfile/endpoint - OPENDAL_AZFILE_ACCOUNT_KEY: op://services/azfile/account_key - OPENDAL_AZFILE_SHARE_NAME: op://services/azfile/share_name - - - name: Test - shell: bash - working-directory: core - run: cargo nextest run behavior - env: - OPENDAL_TEST: azfile - OPENDAL_AZDLS_ROOT: test/ From 0269878303f1d96b512a16fd7bb16ae25ceaf0b6 Mon Sep 17 00:00:00 2001 From: Liuqing Yue Date: Sat, 28 Oct 2023 20:27:26 +0800 Subject: [PATCH 6/7] fixup --- .github/services/azfile/azfile/action.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/services/azfile/azfile/action.yml b/.github/services/azfile/azfile/action.yml index b07006934a0e..d2dc23cad5d9 100644 --- a/.github/services/azfile/azfile/action.yml +++ b/.github/services/azfile/azfile/action.yml @@ -26,7 +26,6 @@ runs: with: export-env: true env: - OP_SERVICE_ACCOUNT_TOKEN: ${{ secrets.OP_SERVICE_ACCOUNT_TOKEN }} OPENDAL_AZFILE_ACCOUNT_NAME: op://services/azfile/account_name OPENDAL_AZFILE_ENDPOINT: op://services/azfile/endpoint OPENDAL_AZFILE_ACCOUNT_KEY: op://services/azfile/account_key From 3b5a8ec91d7e697e4ac141134ffbd67c864e2612 Mon Sep 17 00:00:00 2001 From: Liuqing Yue Date: Sat, 28 Oct 2023 20:35:58 +0800 Subject: [PATCH 7/7] fix: enable azfile for java binding --- bindings/java/Cargo.toml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bindings/java/Cargo.toml b/bindings/java/Cargo.toml index df1ca7e66dd2..c5e7707b96bf 100644 --- a/bindings/java/Cargo.toml +++ b/bindings/java/Cargo.toml @@ -82,6 +82,7 @@ services-all = [ "services-wasabi", "services-mongodb", "services-sqlite", + "services-azfile", ] # Default services provided by opendal. @@ -129,6 +130,7 @@ services-wasabi = ["opendal/services-wasabi"] services-mysql = ["opendal/services-mysql"] services-mongodb = ["opendal/services-mongodb"] services-sqlite = ["opendal/services-sqlite"] +services-azfile = ["opendal/services-azfile"] [dependencies] anyhow = "1.0.71"