Skip to content
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.

Sign up for GitHub

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 for GDAL related assets #218

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+27 −15
Open

Support for GDAL related assets #218

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0d1ff28
Support for GDAL related assets
ahuarte47 Sep 21, 2022
8cd9f32
Do sign_asset_href more generic
ahuarte47 Sep 23, 2022
e9862df
Do not override asset.href, otherwise new invocations will use first …
ahuarte47 Sep 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/qgis_stac/gui/assets_dialog.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,18 @@ def sign_asset_href(self, asset_href):
signed_href = pc.sign(asset_href)
return signed_href

if asset_href.startswith("s3://"):
Copy link
Collaborator

Choose a reason for hiding this comment

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

@ahuarte47 can you put the code for with the new changes into its own function the sign_asset_href method is for signing planetary computer SAS based items hrefs with a SAS token.

Copy link
Author

@ahuarte47 ahuarte47 Sep 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Is not the generation of signed urls the purpose of sign_asset_href? IMHO this function can manage all cases without modifying the current behavior or the existence of new future sign methods. Do you agree? Just chaning the description of the method?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Lets make another function for the new changes.

Copy link

Choose a reason for hiding this comment

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

@Samweli can't really understand. The plugin and the sign method are not named as microsoft or planetarycomputer dependent => because the plugin is a Common, have to be and remain general.

sign_asset_href is a enough general name that can refer to the signing action that is common to any cloud resource not only that from microsoft services.

In resume, what is the rationale to move a sign feature for only GC or AWS to a different method respect signing of microsoft cloud?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@ahuarte47 @luipir I prefer to have another function for the new changes, at some point I ll also rename the current sign_asset_href to a more specific PC related name.

signed_href = gdal.GetSignedURL(f"/vsis3/{asset_href[5:]}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Couldn't find documentation for the GetSignedURL https://gdal.org/api/python/osgeo.gdal.html?highlight=getsigned#osgeo.gdal.GetSignedURL, can you explain what it tries to achieve?

Copy link
Author

Choose a reason for hiding this comment

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

gdal.GetSignedURL tranforms a GDAL virtual file system path to one https signed url. Depending on the virtual file system type, GDAL reads its specific authentication settings from environment variables. This is an example:
https://github.com/OSGeo/gdal/blob/master/autotest/gcore/vsioss.py#L185

Copy link

Choose a reason for hiding this comment

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

"A signed URL is a URL that provides limited permission and time to make a request. Signed URLs contain authentication information in their query string, allowing users without credentials to perform specific actions on a resource" from https://cloud.google.com/storage/docs/access-control/signed-urls

Copy link
Author

Choose a reason for hiding this comment

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

For example, GDAL reads several environment variables (AWS_NO_SIGN_REQUEST, AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, AWS_SESSION_TOKEN...) to create the signed url. It is very useful custom way to auhenticate, apart from using the UI of authentication that your plugin provides.

Copy link
Author

@ahuarte47 ahuarte47 Sep 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@Samweli you can use the Collection Digital Earth Africa - s2l2a in the predefined Catalog. Before, You have to define this environment variable AWS_NO_SIGN_REQUEST=YES. Using gdal 3.5.1 it works.

Copy link
Collaborator

Choose a reason for hiding this comment

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

So these new changes will not work without user having to define the environment variable AWS_NO_SIGN_REQUEST?

Copy link
Author

@ahuarte47 ahuarte47 Oct 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Yes. For other use cases, credentials will be configured defining AWS_PROFILE or AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY. It depends on how the STAC Catalog is deployed in the cloud-provider.

return signed_href

if asset_href.startswith("gs://"):
signed_href = gdal.GetSignedURL(f"/vsigs/{asset_href[5:]}")
return signed_href

if asset_href.startswith("/vsi"):
signed_href = gdal.GetSignedURL(asset_href)
return signed_href

return asset_href

def download_progress(self, value):
Expand Down