-
Notifications
You must be signed in to change notification settings - Fork 361
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
support putifabsent #8428
base: master
Are you sure you want to change the base?
support putifabsent #8428
Changes from 4 commits
d92b29d
f18a67c
81de1fc
7d2a511
768ef8c
b07a5ab
c8a508c
81c8a79
5fc97da
1dd06fd
a635bb4
717f224
d558f57
150776e
d7e22f4
4b1533b
1528c90
08cc251
2873d3a
7e42c88
b46deb3
126144f
4a56ace
c4387d9
51d8105
6f2431a
047bd48
55a2d72
c620816
69764b1
0b2e2b4
8b4d12f
67da789
005f174
278a345
94ed11f
e7508d5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -20,6 +20,7 @@ import ( | |||||
) | ||||||
|
||||||
const ( | ||||||
IfNoneMatchHeader = "If-None-Match" | ||||||
CopySourceHeader = "x-amz-copy-source" | ||||||
CopySourceRangeHeader = "x-amz-copy-source-range" | ||||||
QueryParamUploadID = "uploadId" | ||||||
|
@@ -30,7 +31,6 @@ type PutObject struct{} | |||||
|
||||||
func (controller *PutObject) RequiredPermissions(req *http.Request, repoID, _, destPath string) (permissions.Node, error) { | ||||||
copySource := req.Header.Get(CopySourceHeader) | ||||||
|
||||||
if len(copySource) == 0 { | ||||||
return permissions.Node{ | ||||||
Permission: permissions.Permission{ | ||||||
|
@@ -298,6 +298,11 @@ func handlePut(w http.ResponseWriter, req *http.Request, o *PathOperation) { | |||||
o.Incr("put_object", o.Principal, o.Repository.Name, o.Reference) | ||||||
storageClass := StorageClassFromHeader(req.Header) | ||||||
opts := block.PutOpts{StorageClass: storageClass} | ||||||
err := o.checkIfAbsent(req) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This intention is right but not the approach. Instead, our S3 Gateway should only forward the request to AWS and let them handle the precondition check.
Challenge with PutObject call:Our AWS SDK is outdated, so it does not contain the built-in option in the new version IfNoneMatch. We have 2 options:a. Upgrade the AWS SDK (preferable) after verifying there no breaking changes (separate PR)
NOTE: Same should apply for Multipart Uploads |
||||||
if err != nil { | ||||||
_ = o.EncodeError(w, req, err, gatewayErrors.Codes.ToAPIErr(gatewayErrors.ErrObjectExists)) | ||||||
return | ||||||
} | ||||||
address := o.PathProvider.NewPath() | ||||||
blob, err := upload.WriteBlob(req.Context(), o.BlockStore, o.Repository.StorageNamespace, address, req.Body, req.ContentLength, opts) | ||||||
if err != nil { | ||||||
|
@@ -325,3 +330,22 @@ func handlePut(w http.ResponseWriter, req *http.Request, o *PathOperation) { | |||||
o.SetHeader(w, "ETag", httputil.ETag(blob.Checksum)) | ||||||
w.WriteHeader(http.StatusOK) | ||||||
} | ||||||
|
||||||
func (o *PathOperation) checkIfAbsent(req *http.Request) error { | ||||||
Header := req.Header.Get(IfNoneMatchHeader) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
switch Header { | ||||||
case "": | ||||||
return nil | ||||||
case "*": | ||||||
_, err := o.Catalog.GetEntry(req.Context(), o.Repository.Name, o.Reference, o.Path, catalog.GetEntryParams{}) | ||||||
if err == nil { | ||||||
return gatewayErrors.ErrObjectExists | ||||||
} | ||||||
default: | ||||||
_, err := o.Catalog.GetEntry(req.Context(), o.Repository.Name, o.Reference, Header, catalog.GetEntryParams{}) | ||||||
if err == nil { | ||||||
return gatewayErrors.ErrObjectExists | ||||||
} | ||||||
} | ||||||
return nil | ||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The correct error is PreconditionFailed with status code 412 (StatusPreconditionFailed).
As for the description, the exact AWS message is:
let's stick to what AWS returns.