-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
S3 ingestion can assume role #10995
S3 ingestion can assume role #10995
Conversation
"properties": { | ||
"accessKeyId": "KLJ78979SDFdS2", | ||
"secretAccessKey": "KLS89s98sKJHKJKJH8721lljkd", | ||
"assumeRoleArn": "arn:aws:iam::2981002874992:role/role-s3" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe above are some dummy IDs and roles and not some actual ones.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. All fake.
@JsonProperty | ||
private PasswordProvider accessKeyId; | ||
@JsonProperty | ||
private PasswordProvider secretAccessKey; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
accessKeyId and SecretAccessKey are also nullable, Can we add annotation these as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added.
Fixed all except coverage. The code is integration with AWS API, writing mocks for it will effectively replace about 90% of code, which is unreasonable for testing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 , LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@@ -115,8 +115,8 @@ | |||
<dependency> | |||
<groupId>com.amazonaws</groupId> | |||
<artifactId>aws-java-sdk-sts</artifactId> | |||
<scope>provided</scope> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aws-common
doesn't contain it, surprisingly Kinesis extension contains it, when Kinesis ext. is removed then the S3 ext. fails.
Description
This feature allows S3 ingestion to use the AssumeRole capability of AWS that helps with cross-account access.
It introduces 2 additional fields in S3InputSourceConfig (those are optional).
This PR has: