-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Enable MD5 check on GCS driver #4586
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
Enable MD5 check on GCS driver #4586
Conversation
Apparently you can upload 0-size content wihtout GCS reportin any errors back to you. This is something a lot of our users experienced and reported. See here for at least one example: github.com/distribution/issues/3018 This sets tbe MD5 sum on the uploaded content which should rectify things according to the docs: https://pkg.go.dev/cloud.google.com/go/storage#ObjectAttrs Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
PTAL @thaJeztah |
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.
LGTM
left a minor suggestion
@@ -425,6 +426,10 @@ func (d *driver) putContent(ctx context.Context, obj *storage.ObjectHandle, cont | |||
if _, err := bytes.NewReader(content).WriteTo(wc); err != nil { | |||
return err | |||
} | |||
h := md5.New() |
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.
Would it be worth adding a comment here (to save future visitors from having to go through git history?
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.
LOL autocorrect; it's really git
I meant, not hot
(edited) 😆
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.
Makese sense: 7884c71
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.
Thanks!
Adding a code comment that explains setting MD5 Sum field. Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
Apparently, you can upload 0-size content without GCS reporting any errors back to you. Registry operations fail in these situations
This is something a lot of our users experienced and reported. See here for at least one example:
#3018
This PR sets the MD5 sum on the uploaded content which should rectify things according to the docs:
https://pkg.go.dev/cloud.google.com/go/storage#ObjectAttrs
The alternative would be to set
CRC32C
but that's not been verified unlike the MD5 which is used by GitLab in their registry.Closes: #3018