Skip to content

Fix UploadPartCopy source handling #127442

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

DaveCTurner
Copy link
Contributor

The leading / on the x-amz-copy-source header is optional; SDKv1
includes it but SDKv2 does not. Today we handle its omission when
handling a CopyObject request but we must do the same for the
UploadPartCopy path.

Closes #127436

The leading `/` on the `x-amz-copy-source` header is optional; SDKv1
includes it but SDKv2 does not. Today we handle its omission when
handling a `CopyObject` request but we must do the same for the
`UploadPartCopy` path.

Closes elastic#127436
@DaveCTurner DaveCTurner added >test Issues or PRs that are addressing/adding tests :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs v8.19.0 v9.1.0 labels Apr 27, 2025
@DaveCTurner DaveCTurner requested a review from nicktindall April 27, 2025 19:20
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination)

@elasticsearchmachine elasticsearchmachine added the Team:Distributed Coordination Meta label for Distributed Coordination team label Apr 27, 2025
Copy link
Contributor

@nicktindall nicktindall left a comment

Choose a reason for hiding this comment

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

LGTM

@nicktindall nicktindall merged commit 9772b5e into elastic:main Apr 28, 2025
17 checks passed
@nicktindall nicktindall added auto-backport Automatically create backport pull requests when merged and removed auto-backport Automatically create backport pull requests when merged labels Apr 28, 2025
@nicktindall
Copy link
Contributor

I notice the 8.19 label on this PR, but S3HttpHandler doesn't seem to handle x-amz-copy-source currently. Also S3RepositoryAnalysisIT doesn't appear to be muted on that branch.

I'll leave it as is for now, but if there's something you'd like me to backport I'm happy to have a go.

benchaplin pushed a commit to benchaplin/elasticsearch that referenced this pull request Apr 28, 2025
The leading `/` on the `x-amz-copy-source` header is optional; SDKv1
includes it but SDKv2 does not. Today we handle its omission when
handling a `CopyObject` request but we must do the same for the
`UploadPartCopy` path.

Closes elastic#127436
@DaveCTurner DaveCTurner deleted the 2025/04/27/127436-S3RepositoryAnalysisRestIT-testRepositoryAnalysis branch May 6, 2025 06:48
@DaveCTurner
Copy link
Contributor Author

Ah yeah good point we don't copy objects in 8.19.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs Team:Distributed Coordination Meta label for Distributed Coordination team >test Issues or PRs that are addressing/adding tests v8.19.0 v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] S3RepositoryAnalysisRestIT testRepositoryAnalysis failing
3 participants