Add support for authenticating access to S3 buckets using temporary credentials #3086
Conversation
…tion header and the list of signed headers; applied them consistently, eliminating the hardwired constants; added the {session_token} to the list of headers so authentication with temporary credentials can succeed
…t VFD initialization call
aivazis
requested review from
lrknox,
derobins,
byrnHDF,
fortnern,
jhendersonHDF,
qkoziol,
vchoi-hdfgroup,
bmribler,
glennsong09,
mattjala and
brtnfld
as code owners
|
Maybe worthwhile to take a look at PR #3030 - ROS3: (feature) Temporary security credentials and HDF forum page https://forum.hdfgroup.org/t/ros3-temporary-security-credentials/11135. Both of them is exactly about the same thing you want to achieve but taking into account not breaking the API. |
|
i did. as it happens I'm on a webex with Aleksandar as i write this. the code here evolves the existing |
derobins
added
Priority - 1. High 🔼
|
|
||
| typedef struct H5FD_ros3_fapl_t { | ||
| int32_t version; | ||
| hbool_t authenticate; | ||
| char aws_region[H5FD_ROS3_MAX_REGION_LEN + 1]; | ||
| char secret_id[H5FD_ROS3_MAX_SECRET_ID_LEN + 1]; | ||
| char secret_key[H5FD_ROS3_MAX_SECRET_KEY_LEN + 1]; | ||
| char session_token[H5FD_ROS3_MAX_SESSION_TOKEN_LEN + 1]; |
There was a problem hiding this comment.
Changing a struct in a public header breaks binary compatibility so this change could not be merged downstream to 1.14, etc.
There was a problem hiding this comment.
the change comes with a bump in H5FD_CURR_ROS3_FAPL_T_VERSION, so it is easy to detect clients that have not been recompiled against the updated headers/library. we should be able to restore binary compatibility. shall we walk through the use cases, or are you guys happy to say this is a 1.15 fix?
derobins
added
the
Merge - Develop Only
|
Hi @aivazis, this looks fairly similar to @jwsblokland's PR mentioned previously, where we decided to add a new H5P routine so that the previous API doesn't change and the PR could be merged rather than waiting for a major release of HDF5. Does this PR include functionality that is different from that PR or is it the same session token functionality but added as part of the fapl structure? |
|
well, i'm not particularly strongly invested in exactly how this capability hole is plugged. my users consider support for temporary credentials as critical, so i took what to me looked like the natural path: the deficient data structure has a built-in versioning mechanism, designed to handle the evolution of its schema, which is precisely what this is about. i added logic so that clients that have not been recompiled with the new header and present a version 1 structure to the api are supported by never dereferencing the missing field. this looks like "binary compatibility" to me. i've already successfully patched 1.10, 1.12, and 1.14, built the libraries and tools, and tested clients that had been compiled against libraries without the patch without noticing any bad behavior. but maybe i'm missing something. i've added initializers for the new field to all the tools that touch it, and repaired all the test drivers, except for in addition to support for the missing I'm happy to help any which way you think is the right way to move forward. |
|
PR #3030 has been merged to develop. We can consider any parts of this PR that go beyond 3030 in a separate PR to develop. |
This PR modifies the ROS3 VFD to provide support for presenting a
session_tokento AWS.There is also some minor restructuring of the DEBUG macros in both
H5FDros3.candH5FDs3comms.cto help with API tracing and access pattern tracking.