Merged
Conversation
tishin-endou
force-pushed
the
feature/s3compatsigv4
branch
2 times, most recently
from
aecc4db to
f07a35c
Compare
tishin-endou
force-pushed
the
feature/s3compatsigv4
branch
from
6b0e5a5 to
dedff1f
Compare
…name When renaming a folder, the old folder prefix object (e.g. 'oldname/') was not deleted because _delete_folder skipped objects without a VersionId. This fix includes unversioned objects in the bulk delete batch and explicitly deletes the folder prefix when the versioning API is unsupported. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…lder After deleting all child objects in batches, the folder prefix object (e.g. 'foldername/') may not be included in list_object_versions and thus remains orphaned. Explicitly check and delete it after the batch deletion loop. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ename When _delete_folder raises NotFoundError (because list_object_versions does not return the folder prefix object), BaseProvider.move swallows the 404 and the prefix object remains. Override move in S3CompatSigV4 to explicitly check and delete the folder prefix after super().move(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
_folder_prefix_exists checks CommonPrefixes, which does not include empty folder prefix objects with no children. Replace the existence check with unconditional try/except deletion since S3 DELETE on a non-existent key safely returns 204. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…2 fallback When get_full_revision fails with MetadataError (e.g., MinIO without versioning), fall back to ListObjectsV2 with pagination, EncodingType url, unquote_plus decoding, and strip_whitespace=False to correctly handle Japanese filenames and large folders. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…igation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… key deletion - Remove EncodingType: 'url' from ListObjectsV2 params so S3 returns raw key names instead of URL-encoded ones - Remove parse.unquote_plus() from XML response parsing (no longer needed) - Change Quiet: True to Quiet: False and add error checking/logging for delete_objects responses Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…te_folder MinIO without versioning returns VersionId='null' (string literal) in ListObjectVersions responses. Passing this to delete_objects does not actually delete the objects. Filter out 'null' version IDs and fall back to direct delete_object calls when no real versions exist. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…n MinIO boto3 automatically adds encoding-type=url to presigned URLs for list_object_versions. This causes MinIO to return URL-encoded keys (e.g. Japanese characters become percent-encoded). The encoded keys were passed directly to delete_objects, which silently reported success but did not actually delete the objects since the keys didn't match. Also reverts the VersionId='null' filtering from the previous commit, as local testing with MinIO confirmed that VersionId='null' works correctly for deletion — the real issue was the URL-encoded keys.
Split test_delete into two tests: - test_delete_null_version: verifies direct DELETE when VersionId='null' (MinIO) - test_delete_with_real_versions: verifies batch delete_objects with real version IDs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Revert test_delete to expect delete_objects with VersionId='null' (VersionId='null' filtering was reverted in 52c9e54) - Add _folder_prefix_exists mock (list_objects_v2 GET) to test_delete_confirm_delete, test_delete_folder_with_versions, and test_delete_folder_truncated_response Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove logger.info calls added in b16401f that log object keys (file paths) at INFO level, which could leak sensitive information. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- XML parse: except Exception -> except ExpatError - Multipart upload: except Exception -> except UploadError - Multipart abort retry: except Exception -> except (UploadError, ExpatError) - Folder prefix cleanup (move, _delete_folder): except Exception -> except DeleteError with warning log - Fix inverted abort condition (if aborted -> if not aborted) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1. Remove XML response body from parse error/warning logs 2. Add debug log for MetadataError fallback in download pre-flight 3. Add comment for KeyError: pass on ETag header check 4. Change delete path logging from INFO to DEBUG 5. Remove Key/VersionId from delete error logs, log only count and codes 6. Add debug log for MetadataError fallback in delete 7. Sanitize _delete_folder fallback error logging 8. Sanitize _delete_folder error logging 9. Remove full XML response debug logging in get_full_revision 10. Remove presigned URL from ListObjectVersions error log Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tion The _chunked_upload and _abort_chunked_upload methods intentionally catch all exceptions to ensure upload cleanup (abort) runs regardless of the error type. This matches the upstream s3 provider behavior. Also revert 'if not aborted' back to 'if aborted' to match s3 provider. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Use NextContinuationToken instead of NextMarker for ListObjectsV2 pagination - Decode URL-encoded fields after XML parsing instead of before to prevent invalid XML with special characters (e.g. & in key names) - Raise DeleteError on partial deletion failure in _delete_folder fallback to match main deletion path behavior
tishin-endou
force-pushed
the
feature/s3compatsigv4
branch
from
720e631 to
c9c8ab1
Compare
tishin-endou
force-pushed
the
feature/s3compatsigv4
branch
from
1b51506 to
25b8f9c
Compare
When the frontend sends next_token= (empty string) as a query parameter, the empty string was passed to S3 as ContinuationToken, causing InvalidArgument error. Change the check from "is not None" to truthy evaluation so empty strings are treated the same as None.
tishin-endou
force-pushed
the
feature/s3compatsigv4
branch
from
25b8f9c to
68b4940
Compare
next_token_stringをS3CompatSigV4FolderMetadataでラップして返していたため、
クライアント側でKeyError('Prefix')が発生していた。
s3compatプロバイダーと同様に文字列そのものをappendするよう修正。
テストのアサーションも合わせて更新。
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
tishin-endou
force-pushed
the
feature/s3compatsigv4
branch
from
c0b6256 to
4be94dd
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ticket
https://redmine.devops.rcos.nii.ac.jp/issues/56471
テスト
https://redmine.devops.rcos.nii.ac.jp/issues/58400
Purpose
AWS Signature Version 4 認証に対応した新しいストレージアドオン「S3 Compatible Storage (SigV4)」を追加します。
Changes
RDM-osf.io
新規 s3compatsigv4 アドオンの実装
モデル、ビュー、シリアライザー、ルーティング
ユーザー設定・ノード設定テンプレート
機関ストレージ用管理画面の対応
ユニットテスト
日本語翻訳ファイルの追加
Dockerfile およびアドオン設定の更新
RDM-waterbutler
ファイル操作用の s3compatsigv4 プロバイダーを追加
背景
一部のS3互換ストレージサービス（新しいバージョンのMinIOなど）では、Signature Version 4 による認証が必須となっています。既存の s3compat アドオンでは対応できないため、新規アドオンとして実装しました。
テスト
Side effects
QA Notes
ユニットテスト実装
MinIO, Sakura, Wasabi のSigV4対応ストレージで動作確認済み
E2Eテスト
RCOSDP/RDM-e2e-test-nb#20
https://redmine.devops.rcos.nii.ac.jp/issues/57800
Deployment Notes
同時にリリースが必要なプルリクエスト
RCOSDP/RDM-osf.io#673