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

fix: create S3 client with from_env() instead of new() to allow eks and fargate container builds #25829

Merged
merged 1 commit into from
Jan 13, 2025

Conversation

cpinflux
Copy link
Contributor

@cpinflux cpinflux commented Jan 13, 2025

Creates AmazonS3 object using all environmental variables for additional cases where command line parameters are not appropriate.

This makes step 4 here possible - https://docs.aws.amazon.com/sdk-for-rust/latest/dg/credproviders.html

Alternative approach may have been to add a similar command line option for AWS_CONTAINER_CREDENTIALS_RELATIVE_URI but this makes no sense to provide on a CLI given it is only to be set automatically on Fargate and EKS containers.

As from_env() looks up all relevant environmental variables, it no longer makes sense to look for them as part of the CLI option parsing, so those relevant env options are removed.

Closes #25828

Describe your proposed changes here.

  • I've read the contributing section of the project README.
  • Signed CLA (if not already signed).

…nd fargate container builds

Creates AmazonS3 object using all environmental variables for additional cases where command line parameters are not appropriate.

This makes step 4 here possible - https://docs.aws.amazon.com/sdk-for-rust/latest/dg/credproviders.html

Alternative approach may have been to add a similar command line option for AWS_CONTAINER_CREDENTIALS_RELATIVE_URI but this makes no sense to provide on a CLI given it is only to be set automatically on Fargate and EKS containers.

As from_env() looks up all relevant environmental variables, it no longer makes sense to look for them as part of the CLI option parsing, so those relevant env options are removed.

Fixes influxdata#25828
@hiltontj hiltontj requested a review from pauldix January 13, 2025 21:54
@pauldix pauldix merged commit 26bf912 into influxdata:main Jan 13, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

S3 credentials not correctly obtained for Fargate / EKS
3 participants