[services] Update AWS Rust SDK
ClosedPublic
Actions

Authored by bartek on Jul 25 2024, 12:03 AM.

Details

Reviewers

varun
will

Commits

rCOMMaa2d0a93a94b: [services] Update AWS Rust SDK

Summary

Updated AWS SDK for Rust. Most notable changes:

All builders now return Result - they fail only on programmer error - when required items are not set, so used expect()
Needed to upgrade siwe crate to resolve dependency conflict

Other changes described in inline comments

Depends on D12878

Test Plan

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

bartek held this revision as a draft.

Rebase

bartek published this revision for review.Jul 25 2024, 12:46 AM

bartek added inline comments.

Cargo.toml
87–88 ↗	(On Diff #42759)	Needed to update `siwe` to resolve dependency version conflict with AWS crates Needed to add `time` which should be re-exported from `siwe` but it's not. It's required to specify time format in SIWE message/signature verification.
services/backup/src/database/mod.rs
345 ↗	(On Diff #42759)	We could use `unwrap()` as well, errors here are rather obvious when looking at the docs (or even source code)
services/blob/src/service.rs
120–123 ↗	(On Diff #42759)	`content_length()` now returns `Option<i64>` instead of `Result<u64-convertible, SomeError>`. Should I create a follow-up to check all places where we do `as u64` conversions?
services/identity/src/database.rs
613–614 ↗	(On Diff #42759)	`response.items()` now always returns a string slice instead of `Option<Vec>`. It uses `unwrap_or_default()` under the hood.
613–614 ↗	(On Diff #42759)	Should've given the opposite name
services/identity/src/ddb_utils.rs
269 ↗	(On Diff #42759)	`item` is now `AttributeMap`, it was `Option<_>` previously
services/identity/src/siwe.rs
39–46 ↗	(On Diff #42759)	`verify()` is now `async`. first arg (`signature`) is now `&[u8]` so no longer need to `try_into()`. last three args are now moved to `VerificationOpts` `timestamp` unfortunately requires `time::OffsetDateTime` instead of `chrono::Utc`
shared/comm-lib/src/database.rs
565 ↗	(On Diff #42759)	`keys` is now `Vec<AttributeMap>` directly, no longer wrapped in `Option<_>`

varun added inline comments.

services/blob/src/service.rs
120–123 ↗	(On Diff #42759)	how about something like this to avoid overflow? and yeah a follow-up task would be great, thanks!
services/identity/src/database.rs
613–614 ↗	(On Diff #42759)	prefer "available" to "free"
services/identity/src/siwe.rs
39–46 ↗	(On Diff #42759)	looked into why this became async -- it's to support smart contract wallets https://linear.app/comm/issue/ENG-4363/update-login-wallet-user-rpc-to-verify-signed-message-validity this is on our roadmap, so were always going to have to increase the version of the `siwe` crate

This revision is now accepted and ready to land.Jul 25 2024, 12:55 PM

services/blob/src/service.rs
120–123 ↗	(On Diff #42759)	Good idea. By the way I checked other places with `as u64` and they look safe. It's usually a `usize` -> `u64` conversion.
services/identity/src/siwe.rs
39–46 ↗	(On Diff #42759)	Good to know that