Rusoto: 0.33 breaks STS with EC2

Created on 20 Aug 2018  路  5Comments  路  Source: rusoto/rusoto

Consider the following code, which used to work (modulo new_with vs new and such) in 0.32:

extern crate rusoto_core;
extern crate rusoto_ec2;
extern crate rusoto_sts;

use rusoto_core::request::HttpClient;
use rusoto_core::Region;
use rusoto_ec2::Ec2Client;
use rusoto_sts::{StsAssumeRoleSessionCredentialsProvider, StsClient};

fn main() {
    let sts = StsClient::new(Region::UsEast1);
    let provider = StsAssumeRoleSessionCredentialsProvider::new(
        sts,
        "arn:aws:sts::1122334455:role/myrole".to_owned(),
        "session-name".to_owned(),
        None,
        None,
        None,
        None,
    );

    Ec2Client::new_with(HttpClient::new().unwrap(), provider, Region::default());
}

This no longer compiles since rusoto_sts::custom::credential::StsSessionCredentialsClient is neither Send nor Sync, which Ec2Client::new_with now requires.

Easy bug help wanted

Most helpful comment

Oops! That should be a relatively easy fix though.

Would be great to also add at least a doc test for this so that we prevent regress again.

All 5 comments

Oops! That should be a relatively easy fix though.

Would be great to also add at least a doc test for this so that we prevent regress again.

I think this basically breaks all use of STS, so probably something that should be relatively high priority. I'd be happy to give it a shot if you could give some guidance on what would need to be changed.

That鈥榙 be fantastic! I鈥榣l try to provide some guidance later today.

Okay, so here we go for initial mentoring instructions!

  • The relevant source code can be found in rusoto/services/sts/src/custom/credential.rs
  • This file includes multiple different credential provider implementations (such as StsAssumeRoleSessionCredentialsProvider), that all wrap an StsClient in the form a boxed trait object (grep for Box<StsSessionCredentialsClient>)
  • In order to make all these providers Send + Sync, we need to change the trait object to also include the Send and Sync markers, in other words, change the box to be Box<StsSessionCredentialsClient + Send + Sync>
  • I don't see any direct knock-on effects for this change, but it could be that changing it results in a bunch of cascading type errors that need to be fixed up (likely by adding Send and Sync bounds as well)
  • After this is done, it would be great to ensure that we don't regress, by adding assertions to check that all the credential providers implement Sync and Send. You can see an example of how this is done here: https://github.com/rusoto/rusoto/blob/master/rusoto/core/src/future.rs#L121

Let me know if this works for you @jonhoo, or whether you need more detail for any of the steps!

@srijs fixed in #1110.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

matthewkmayer picture matthewkmayer  路  5Comments

clee picture clee  路  5Comments

kaushalyap picture kaushalyap  路  4Comments

Mythra picture Mythra  路  6Comments

matthewkmayer picture matthewkmayer  路  4Comments