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.

Sign up for GitHub

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

feat(source): use the default_provider for AWS-related connectors #17933

Merged
merged 4 commits into from
Aug 12, 2024
Merged

feat(source): use the default_provider for AWS-related connectors #17933

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fc7abfc
solution for issue 17849, adding default fallback to aws default cred…
psheets Jul 29, 2024
c3c7cfa
Merge branch 'main' into 17849-support-aws-creds-kinesis
psheets Jul 29, 2024
916cf3e
Merge branch 'main' into 17849-support-aws-creds-kinesis
psheets Aug 3, 2024
a9f2abc
add an env var to control it
fuyufjh Aug 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions src/connector/src/connector_common/common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ pub const PRIVATE_LINK_TARGETS_KEY: &str = "privatelink.targets";

const AWS_MSK_IAM_AUTH: &str = "AWS_MSK_IAM";

/// The environment variable to disable using default credential from environment.
/// It's recommended to set this variable to `false` in cloud hosting environment.
const DISABLE_DEFAULT_CREDENTIAL: &str = "DISABLE_DEFAULT_CREDENTIAL";
Copy link
Contributor

Choose a reason for hiding this comment

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

Shall it affect services from cloud vendors besides AWS as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

If the idea looks good to you, I'll add the support of Azure/GCP in future PRs. This PR was supposed to complete #17851

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. Thanks!


#[derive(Debug, Clone, Deserialize)]
pub struct AwsPrivateLinkItem {
pub az_id: Option<String>,
Expand All @@ -57,6 +61,7 @@ use aws_config::sts::AssumeRoleProvider;
use aws_credential_types::provider::SharedCredentialsProvider;
use aws_types::region::Region;
use aws_types::SdkConfig;
use risingwave_common::util::env_var::env_var_is_true;

/// A flatten config map for aws auth.
#[derive(Deserialize, Debug, Clone, WithOptions)]
Expand Down Expand Up @@ -104,7 +109,7 @@ impl AwsAuthProps {
}
}

fn build_credential_provider(&self) -> ConnectorResult<SharedCredentialsProvider> {
async fn build_credential_provider(&self) -> ConnectorResult<SharedCredentialsProvider> {
if self.access_key.is_some() && self.secret_key.is_some() {
Ok(SharedCredentialsProvider::new(
aws_credential_types::Credentials::from_keys(
Expand All @@ -113,6 +118,10 @@ impl AwsAuthProps {
self.session_token.clone(),
),
))
} else if !env_var_is_true(DISABLE_DEFAULT_CREDENTIAL) {
Ok(SharedCredentialsProvider::new(
aws_config::default_provider::credentials::default_provider().await,
))
} else {
bail!("Both \"access_key\" and \"secret_key\" are required.")
}
Expand Down Expand Up @@ -140,7 +149,7 @@ impl AwsAuthProps {
pub async fn build_config(&self) -> ConnectorResult<SdkConfig> {
let region = self.build_region().await?;
let credentials_provider = self
.with_role_provider(self.build_credential_provider()?)
.with_role_provider(self.build_credential_provider().await?)
.await?;
let mut config_loader = aws_config::from_env()
.region(region)
Expand Down
Loading