s3: return NoSuchKey/NoSuchBucket for a missing CopyObject source (#10332)

* s3: CopyObject returns NoSuchKey for a missing copy source

* s3: CopyObject returns NoSuchBucket for a missing source bucket
This commit is contained in:
Chris Lu
2026-07-14 11:53:53 -07:00
committed by GitHub
parent 311bc3a6df
commit f3e4a73696
2 changed files with 18 additions and 20 deletions
@@ -24,14 +24,14 @@ func TestClassifyCopySourceError(t *testing.T) {
want s3err.ErrorCode
}{
{"regular entry", &filer_pb.Entry{Name: "o"}, nil, s3err.ErrNone},
{"nil entry", nil, nil, s3err.ErrInvalidCopySource},
{"directory entry", &filer_pb.Entry{IsDirectory: true}, nil, s3err.ErrInvalidCopySource},
{"nil entry", nil, nil, s3err.ErrNoSuchKey},
{"directory entry", &filer_pb.Entry{IsDirectory: true}, nil, s3err.ErrNoSuchKey},
{"delete marker entry", deleteMarker, nil, s3err.ErrNoSuchKey},
{"not found sentinel", nil, filer_pb.ErrNotFound, s3err.ErrInvalidCopySource},
{"wrapped not found", nil, fmt.Errorf("read %s: %w", "o", filer_pb.ErrNotFound), s3err.ErrInvalidCopySource},
{"grpc not found", nil, status.Error(codes.NotFound, "no entry"), s3err.ErrInvalidCopySource},
{"invalid version id", nil, errInvalidVersionID, s3err.ErrInvalidCopySource},
{"delete marker error", nil, ErrDeleteMarker, s3err.ErrInvalidCopySource},
{"not found sentinel", nil, filer_pb.ErrNotFound, s3err.ErrNoSuchKey},
{"wrapped not found", nil, fmt.Errorf("read %s: %w", "o", filer_pb.ErrNotFound), s3err.ErrNoSuchKey},
{"grpc not found", nil, status.Error(codes.NotFound, "no entry"), s3err.ErrNoSuchKey},
{"invalid version id", nil, errInvalidVersionID, s3err.ErrNoSuchKey},
{"delete marker error", nil, ErrDeleteMarker, s3err.ErrNoSuchKey},
{"transient unavailable", nil, status.Error(codes.Unavailable, "filer down"), s3err.ErrServiceUnavailable},
{"store error", nil, errors.New("connection reset"), s3err.ErrInternalError},
// A message merely mentioning the sentinel text must not downgrade a store error.
+11 -13
View File
@@ -69,16 +69,14 @@ func hasPrefixFold(s, prefix string) bool {
return len(s) >= len(prefix) && strings.EqualFold(s[:len(prefix)], prefix)
}
// classifyCopySourceError maps a copy-source lookup to an S3 error: a missing,
// invalid, or directory source stays a client error, but a transient store
// error becomes a retryable 5xx so a resumable copy/commit survives a blip.
// classifyCopySourceError maps a copy-source lookup to an S3 error: a missing
// or directory source is NoSuchKey like AWS, but a transient store error
// becomes a retryable 5xx so a resumable copy/commit survives a blip.
func classifyCopySourceError(entry *filer_pb.Entry, err error) s3err.ErrorCode {
if err == nil {
if entry == nil || entry.IsDirectory {
return s3err.ErrInvalidCopySource
return s3err.ErrNoSuchKey
}
// The latest-version pointer tracks delete markers too; copying one
// must be NoSuchKey like every other handler, not a copy of the stub.
if deleteMarker, ok := entry.Extended[s3_constants.ExtDeleteMarkerKey]; ok && string(deleteMarker) == "true" {
return s3err.ErrNoSuchKey
}
@@ -86,7 +84,7 @@ func classifyCopySourceError(entry *filer_pb.Entry, err error) s3err.ErrorCode {
}
if errors.Is(err, filer_pb.ErrNotFound) || status.Code(err) == codes.NotFound ||
errors.Is(err, errInvalidVersionID) || errors.Is(err, ErrDeleteMarker) {
return s3err.ErrInvalidCopySource
return s3err.ErrNoSuchKey
}
if isTransientFilerError(err) {
return s3err.ErrServiceUnavailable
@@ -171,10 +169,10 @@ func (s3a *S3ApiServer) CopyObjectHandler(w http.ResponseWriter, r *http.Request
srcVersioningState, err := s3a.getVersioningState(srcBucket)
if err != nil {
glog.Errorf("Error checking versioning state for source bucket %s: %v", srcBucket, err)
// Only a missing bucket is the client's fault; a store error must stay
// retryable, matching the destination-bucket lookup.
// A missing source bucket is NoSuchBucket like AWS; a store error must
// stay retryable, matching the destination-bucket lookup.
if errors.Is(err, filer_pb.ErrNotFound) {
s3err.WriteErrorResponse(w, r, s3err.ErrInvalidCopySource)
s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchBucket)
return
}
s3err.WriteErrorResponse(w, r, s3err.ErrInternalError)
@@ -870,10 +868,10 @@ func (s3a *S3ApiServer) CopyObjectPartHandler(w http.ResponseWriter, r *http.Req
srcVersioningState, err := s3a.getVersioningState(srcBucket)
if err != nil {
glog.Errorf("Error checking versioning state for source bucket %s: %v", srcBucket, err)
// Only a missing bucket is the client's fault; a store error must stay
// retryable, matching the destination-bucket lookup.
// A missing source bucket is NoSuchBucket like AWS; a store error must
// stay retryable, matching the destination-bucket lookup.
if errors.Is(err, filer_pb.ErrNotFound) {
s3err.WriteErrorResponse(w, r, s3err.ErrInvalidCopySource)
s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchBucket)
return
}
s3err.WriteErrorResponse(w, r, s3err.ErrInternalError)