Skip to content

Conversation

milosgajdos
Copy link
Member

@milosgajdos milosgajdos commented Feb 28, 2025

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

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>
@milosgajdos milosgajdos requested a review from squizzi February 28, 2025 16:56
@milosgajdos
Copy link
Member Author

PTAL @thaJeztah

Copy link
Member

@thaJeztah thaJeztah left a 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()
Copy link
Member

@thaJeztah thaJeztah Mar 1, 2025

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?

Copy link
Member

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) 😆

Copy link
Member Author

Choose a reason for hiding this comment

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

Makese sense: 7884c71

Copy link
Member

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>
@milosgajdos milosgajdos merged commit 1b01625 into distribution:main Mar 1, 2025
19 checks passed
@milosgajdos milosgajdos deleted the use-md5-in-gcs-driver branch March 1, 2025 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable MD5 checksums on Google Cloud Storage
3 participants