Skip to content

Fix parameters passed for GCS URL generation#6940

Merged
ggainey merged 1 commit into
pulp:mainfrom
balasankarc:fix-gcs-support
Sep 17, 2025
Merged

Fix parameters passed for GCS URL generation#6940
ggainey merged 1 commit into
pulp:mainfrom
balasankarc:fix-gcs-support

Conversation

@balasankarc

@balasankarc balasankarc commented Sep 13, 2025

Copy link
Copy Markdown

When GCP support was originally added in #3481, the parameters were response_disposition and content_type. When it was refactored in #4251, the headers for GCS were mistakenly changed and expanded.

These parameters get used by generate_signed_url method defined in https://github.com/googleapis/python-storage/blob/a8109e0/google/cloud/storage/blob.py#L463, through
https://github.com/jschneier/django-storages/blob/758ad6f/storages/backends/gcloud.py#L350

This PR ensures only the supported parameters are passed to that function call.

closes #6917

@balasankarc

Copy link
Copy Markdown
Author

@bmbouter I confirmed this changes fixes the issue mentioned in #6917. Can you please help me find a reviewer for the PR? Thanks.

When GCP support was originally added, the parameters were
`response_disposition` and `content_type`. When it was refactored in
pulp#4251, the headers were GCS were
mistakenly changed and expanded.

These parameters get used by `generate_signed_url` method defined in
https://github.com/googleapis/python-storage/blob/a8109e0/google/cloud/storage/blob.py#L463,
through
https://github.com/jschneier/django-storages/blob/758ad6f/storages/backends/gcloud.py#L350

This PR ensures only the supported parameters are passed to that
function call.

closes pulp#6917

Signed-off-by: Balasankar 'Balu' C <balu@dravidam.net>

@ggainey ggainey left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor nitpick

Comment thread pulpcore/constants.py
"Content-Encoding": "content_encoding",
}
# https://gcloud.readthedocs.io/en/latest/storage-blobs.html
# https://gcloud.readthedocs.io/en/latest/storage-blobs.html#google.cloud.storage.blob.Blob.generate_signed_url

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor nit - can we get this on its own line? :)

@mdellweg mdellweg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I cannot tell whether this is the right thing to do, and we neither support nor test GCS.
(Some time ago we failed setting up the corresponding test runner in a deterministic way and abandoned the idea.)
So I am going to approve this change with the disclaimer that this does not change our stance on the supported storage backends as stated here:
https://pulpproject.org/pulpcore/docs/admin/reference/settings/#storages

Thank you for your effort.

@ggainey ggainey left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm going to withdraw the change-request - doesn't affect anythiing other than "aesthetics", and not worth holding up the PR. Approving, while seconding @mdellweg 's commentary.

@ggainey ggainey merged commit fc99a60 into pulp:main Sep 17, 2025
14 checks passed
@balasankarc

Copy link
Copy Markdown
Author

@ggainey @bmbouter Is a minor version release (3.90.0) planned soon? If not, would it be possible to have a patch version for 3.89.x with this fix released?

@patchback

patchback Bot commented Sep 18, 2025

Copy link
Copy Markdown

Backport to 3.89: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.89/fc99a60a59398909ebab38ffe9e3f2196f9c9855/pr-6940

Backported as #6956

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

@ggainey

ggainey commented Sep 18, 2025

Copy link
Copy Markdown
Contributor

@ggainey @bmbouter Is a minor version release (3.90.0) planned soon? If not, would it be possible to have a patch version for 3.89.x with this fix released?

I'll backport to 3.89 and get a z-release out shortly - that way we don't have to wait for a new Y

@ggainey

ggainey commented Sep 18, 2025

Copy link
Copy Markdown
Contributor

@ggainey @bmbouter Is a minor version release (3.90.0) planned soon? If not, would it be possible to have a patch version for 3.89.x with this fix released?

I'll backport to 3.89 and get a z-release out shortly - that way we don't have to wait for a new Y

https://pypi.org/project/pulpcore/3.89.1/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Using GCS as storage backend fails when URL signing is enabled

3 participants