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.

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

Add experimental AWS_PROFILE support (#2178) #2891

Merged
merged 3 commits into from
Oct 25, 2022

Conversation

tustvold
Copy link
Contributor

@tustvold tustvold commented Oct 18, 2022

Which issue does this PR close?

Closes #2178

Rationale for this change

Support for the more esoteric AWS auth options has been requested in various places, rather than forcing downstreams to workaround this by using an SDK such as aws-sdk-rust, lets just provide an option to use aws-sdk-rust to get credentials from a profile. This lets people explicitly opt-in to the full suite of credential providers, with the accompanying dependency explosion, whilst still using the same code for actual communication with object storage. I still would argue aws-vault is a superior solution for credential management than relying on inconsistent SDK implementations which store credentials in plain text on disk, but we should meet users where they currently are.

What changes are included in this PR?

Adds an optional aws_profile feature which uses aws-config to provide the full suite of AWS credential functionality, including SSO, external credential providers, role chaining, etc...

Are there any user-facing changes?

No

@timvw
Copy link

timvw commented Oct 18, 2022

I agree with aws-vault being a better solution to store credentials. (With sso profiles there are no credentials stored in plain text files though).

Having to type aws-vault exec --profile xxx application is just not very ergonomic in the case of short-lived/cli apps.

@tustvold
Copy link
Contributor Author

With sso profiles there are no credentials stored in plain text files though

It's been a while since I used it, but if memory serves it caches both the final credentials, and the returned SAML assertion. The former typically has a TTL of 1 hour, and the latter 24 hours. Certainly better than non-ephemeral credentials, but still not great 😅

Having to type aws-vault exec --profile xxx application

FWIW you can type aws-vault exec --profile xxx to get a shell with credentials, a bit like pyenv

@tustvold tustvold marked this pull request as ready for review October 22, 2022 22:06
@tustvold
Copy link
Contributor Author

tustvold commented Oct 22, 2022

I have tested this locally and it at least appears to work for me, I therefore think it is ready for review. Perhaps @wjones127, @timvw and/or @roeap might be able to test this out / help review this?

@timvw
Copy link

timvw commented Oct 23, 2022

Works for me (tested on commit (2079a67)). Thank you!

cargo test --package object_store --features aws_profile --lib aws::tests::s3_with_sso_profile -- --exact
  #[tokio::test]
  async fn s3_with_sso_profile() -> Result<()>{

      env::set_var("AWS_PROFILE", "XXX");

      let s3 = AmazonS3Builder::from_env()
          .with_bucket_name("YYY")
          .build()?;

      let path = Path::parse("ZZZ")?;
      let files =
      s3.list(Some(&path))
          .await?
          .try_collect::<Vec<ObjectMeta>>()
          .await?;

      assert!(files.len() > 0);

      Ok(())
  }
`

@timvw
Copy link

timvw commented Oct 23, 2022

Also verified that when AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY and AWS_PROFILE are set that the first two get preference over AWS_PROFILE (similar to how aws cli behaves).

@crepererum
Copy link
Contributor

crepererum commented Oct 25, 2022

Do we really need the full SDK for this? Skimming through https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-profiles.html it seems this is only an ini file at a fixed (OS-specific) location where the section is the profile name and the key-value dict in that section are the env. variables we're looking for.

Ah, I see:

Adds an optional aws_profile feature which uses aws-config to provide the full suite of AWS credential functionality, including SSO, external credential providers, role chaining, etc...

Copy link
Contributor

@crepererum crepererum left a comment

Choose a reason for hiding this comment

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

Looks OK, even though I kinda dislike the massive dependency-weight that this feature carries.

@tustvold
Copy link
Contributor Author

Yeah, its a bit of a hack. Hopefully over time we can remove the need for it

@tustvold tustvold merged commit 9c315ce into apache:master Oct 25, 2022
@ursabot
Copy link

ursabot commented Oct 25, 2022

Benchmark runs are scheduled for baseline = 4620abf and contender = 9c315ce. 9c315ce is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Skipped ⚠️ Benchmarking of arrow-rs-commits is not supported on ec2-t3-xlarge-us-east-2] ec2-t3-xlarge-us-east-2
[Skipped ⚠️ Benchmarking of arrow-rs-commits is not supported on test-mac-arm] test-mac-arm
[Skipped ⚠️ Benchmarking of arrow-rs-commits is not supported on ursa-i9-9960x] ursa-i9-9960x
[Skipped ⚠️ Benchmarking of arrow-rs-commits is not supported on ursa-thinkcentre-m75q] ursa-thinkcentre-m75q
Buildkite builds:
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

@timvw
Copy link

timvw commented Nov 4, 2022

Are there plans to release a newer version of the object_store crate with these changes?

@tustvold
Copy link
Contributor Author

tustvold commented Nov 7, 2022

I was waiting for a critical mass of features to warrant a release. I'll probably look to cut a release regardless in the next week or two.

@alamb
Copy link
Contributor

alamb commented Dec 1, 2022

#3229

fvaleye added a commit to delta-io/delta-rs that referenced this pull request Dec 9, 2022
# Description
- Add the support of `AWS_PROFILE` environment variable for AWS S3
- Bump version of `object_store` to `0.5.2` 

# Related Issue(s)
- relate to apache/arrow-rs#3229
- relate to apache/arrow-rs#2891
- closes #855
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
object-store Object Store Interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

object_store: Support assuming roles directly when using AWS S3
5 participants