Skip to content

lib/backup/s3remote: disable enforcement of integrity check#8630

Closed
zekker6 wants to merge 1 commit intomasterfrom
gh-8622
Closed

lib/backup/s3remote: disable enforcement of integrity check#8630
zekker6 wants to merge 1 commit intomasterfrom
gh-8622

Conversation

@zekker6
Copy link
Copy Markdown
Member

@zekker6 zekker6 commented Apr 1, 2025

Integrity check enforcement was enabled in aws-sdk-go-v2 by default (see aws/aws-sdk-go-v2#2960). This leads to inability to use vmbackup, vmrestore and vmbackupmanager with 3rd party providers which do not support this option.

Currently, it is possible to use AWS_REQUEST_CHECKSUM_CALCULATION=when_required and AWS_RESPONSE_CHECKSUM_VALIDATION=when_required environment variables to restore previous behaviour but it requires a manual intervention from users' side.

This change disables this check by default to restore previous behaviour of aws-sdk-go-v2.

See: #8622

Integrity check enforcement was enabled in aws-sdk-go-v2 by default (see aws/aws-sdk-go-v2#2960).
This leads to inability to use vmbackup, vmrestore and vmbackupmanager with 3rd party providers which do not support this option.

Currently, it is possible to use `AWS_REQUEST_CHECKSUM_CALCULATION=when_required` and `AWS_RESPONSE_CHECKSUM_VALIDATION=when_required` environment variables to restore previous behaviour but it requires a manual intervention from users' side.

This change disables this check by default to restore previous behaviour of aws-sdk-go-v2.

Signed-off-by: Zakhar Bessarab <z.bessarab@victoriametrics.com>
Copy link
Copy Markdown
Contributor

@f41gh7 f41gh7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@wwojdanowski
Copy link
Copy Markdown

@zekker6 @f41gh7 In my opinion the proposed changes will not resolve the issue. The upload logic decides if there is a need to use a "multipart" mode
https://github.com/aws/aws-sdk-go-v2/blob/2fc9c3b79153b74b01f39d4b1895501b1e18b328/feature/s3/manager/upload.go#L346
I tried running the backup with the aforementioned variables and the upload still failed but during the multipart upload.

@zekker6
Copy link
Copy Markdown
Member Author

zekker6 commented Apr 1, 2025

@wwojdanowski Did you check the variables which are used in the latest comment here? (note that these have different value compared to earlier reply)

Also, this config should be propagated to uploader from S3 client as uploader is created with client base here.

Here is a vmbackup image build based on this PR, you could try that to verify if this changes works for you: victoriametrics/vmbackup:heads-gh-8622-0-g52aed046a9
For binary package you can build vmbackup from sources by using these docs.

@wwojdanowski
Copy link
Copy Markdown

@zekker6 ok I'll try later with this image, thanks

@wwojdanowski
Copy link
Copy Markdown

Hi @zekker6 unfortunately it fails just like before, on a multipart upload

./vmbackup-prod -version
vmbackup-20250401-130113-heads-gh-8622-0-g52aed046a9

2025-04-02T10:45:26.240Z fatal VictoriaMetrics/app/vmbackup/main.go:104 cannot create backup: cannot upload part{path: "data/big/2025_04/1832724CC3EAF86D/index.bin", file_size: 47823450, offset: 0, size: 47823450} to S3{bucket: "backup-bucket-123", dir: "vmstorage-0/latest/"}: cannot upload data to "data/big/2025_04/1832724CC3EAF86D/index.bin" at S3{bucket: "backup-bucket-123", dir: "vmstorage-0/latest/"} (remote path "vmstorage-0/latest/data/big/2025_04/1832724CC3EAF86D/index.bin/0000000002D9BA5A_0000000000000000_0000000002D9BA5A"): upload multipart failed, upload id: 2~4ecQt16s3tpLr8PG-ATxkU9Y4PB3vKD, cause: operation error S3: UploadPart, https response error StatusCode: 400, RequestID: tx00000401c8b482387632b-0067ed1545-cc03b2aa-default, HostID: cc03b2aa-default-default, api error XAmzContentSHA256Mismatch: UnknownError

@zekker6
Copy link
Copy Markdown
Member Author

zekker6 commented Apr 2, 2025

@wwojdanowski Thank you for testing, I'll try to replicate this behaviour by using some larger chunks of data against Linode S3.

@zekker6
Copy link
Copy Markdown
Member Author

zekker6 commented Apr 3, 2025

Closing in favor of #8642 as disabling integrity checks does not resolve issues with some of S3-compatible storage providers. Even with integrity checks disabled AWS SDK will still try to verify SHA256 checksums which will fail with some S3-compatible storage providers.

@zekker6 zekker6 closed this Apr 3, 2025
zekker6 added a commit that referenced this pull request Apr 3, 2025
Pin AWS libraries to version before 2025-01-15 (see
https://github.com/aws/aws-sdk-go-v2/releases/tag/release-2025-01-15).

This version enabled request and response checksum verification by
default which breaks compatibility with non-AWS S3-compatible storage
providers.

See: #8622

Supersedes #8630

---------

Signed-off-by: Zakhar Bessarab <z.bessarab@victoriametrics.com>
zekker6 added a commit that referenced this pull request Apr 3, 2025
Pin AWS libraries to version before 2025-01-15 (see
https://github.com/aws/aws-sdk-go-v2/releases/tag/release-2025-01-15).

This version enabled request and response checksum verification by
default which breaks compatibility with non-AWS S3-compatible storage
providers.

See: #8622

Supersedes #8630

---------

Signed-off-by: Zakhar Bessarab <z.bessarab@victoriametrics.com>
@valyala valyala deleted the gh-8622 branch April 25, 2025 20:50
truepele pushed a commit to truepele/VictoriaMetrics that referenced this pull request Jun 21, 2025
Pin AWS libraries to version before 2025-01-15 (see
https://github.com/aws/aws-sdk-go-v2/releases/tag/release-2025-01-15).

This version enabled request and response checksum verification by
default which breaks compatibility with non-AWS S3-compatible storage
providers.

See: VictoriaMetrics#8622

Supersedes VictoriaMetrics#8630

---------

Signed-off-by: Zakhar Bessarab <z.bessarab@victoriametrics.com>
truepele pushed a commit to truepele/VictoriaMetrics that referenced this pull request Jun 21, 2025
Pin AWS libraries to version before 2025-01-15 (see
https://github.com/aws/aws-sdk-go-v2/releases/tag/release-2025-01-15).

This version enabled request and response checksum verification by
default which breaks compatibility with non-AWS S3-compatible storage
providers.

See: VictoriaMetrics#8622

Supersedes VictoriaMetrics#8630

---------

Signed-off-by: Zakhar Bessarab <z.bessarab@victoriametrics.com>
zekker6 added a commit that referenced this pull request Oct 10, 2025
…9844)

Updates:
- unpin AWS dependencies and run `make vendor-update`
- add config options to enable checksums only if required by storage in
order to preserve backwards compatibility

Related issues:
- #9748
- #8622

Tested with: AWS S3, self-hosted MinIO, Linode object storage as it was
failing previously with multi-part uploads (reported here -
#8630 (comment)).
An updated library allows (PR with the
fix - aws/aws-sdk-go-v2#3151) overriding
multi-part upload configurations so that compatibility can be preserved.

Signed-off-by: Zakhar Bessarab <z.bessarab@victoriametrics.com>
zekker6 added a commit that referenced this pull request Oct 10, 2025
…9844)

Updates:
- unpin AWS dependencies and run `make vendor-update`
- add config options to enable checksums only if required by storage in
order to preserve backwards compatibility

Related issues:
- #9748
- #8622

Tested with: AWS S3, self-hosted MinIO, Linode object storage as it was
failing previously with multi-part uploads (reported here -
#8630 (comment)).
An updated library allows (PR with the
fix - aws/aws-sdk-go-v2#3151) overriding
multi-part upload configurations so that compatibility can be preserved.

Signed-off-by: Zakhar Bessarab <z.bessarab@victoriametrics.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants