-
Notifications
You must be signed in to change notification settings - Fork 7
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
Adding AWS S3 Configuration Provider and AWS Secrets Manager Provider #135
base: main
Are you sure you want to change the base?
Conversation
Changes to be committed: modified: ojdbc-provider-aws/pom.xml new file: ojdbc-provider-aws/src/main/java/oracle/jdbc/provider/aws/authentication/AwsAuthenticationMethod.java new file: ojdbc-provider-aws/src/main/java/oracle/jdbc/provider/aws/authentication/AwsBasicCredentialsFactory.java new file: ojdbc-provider-aws/src/main/java/oracle/jdbc/provider/aws/configuration/AWSAppConfigProvider.java new file: ojdbc-provider-aws/src/main/java/oracle/jdbc/provider/aws/configuration/AWSAppConfigurationURLParser.java new file: ojdbc-provider-aws/src/main/java/oracle/jdbc/provider/aws/configuration/AWSConfigurationParameters.java modified: ojdbc-provider-aws/src/main/resources/META-INF/services/oracle.jdbc.spi.OracleConfigurationProvider new file: ojdbc-provider-samples/src/main/java/oracle/jdbc/provider/aws/configuration/SimpleAWSAppConfigExample.java
|
||
// Sample default URL if non present | ||
if (args.length == 0) { | ||
url = "jdbc:oracle:thin:@config-awss3://{bucket-name}/{key-name}"; |
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.
It's true the AWS calls the file "key" but would it be easier to understand {file-name} instead of key?
And actually it could be
url = "jdbc:oracle:thin:@config-awss3://{S3-URI}";
maybe with a comment
url = "jdbc:oracle:thin:@config-aws{S3-URI}"; is valid too:
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 think with a sample (like "mybucket/myconfigfile.json") could be easier to follow
<dependency> | ||
<groupId>com.oracle.database.jdbc</groupId> | ||
<artifactId>ojdbc-provider-aws</artifactId> | ||
<version>1.2.0</version> |
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.
1.0.1, right?
The Oracle DataSource uses a new prefix `jdbc:oracle:thin:@config-awss3:` to be able to identify that the configuration parameters should be loaded using AWS S3. Users only need to indicate the S3 URI of the object that contains the JSON payload, with the following syntax: | ||
|
||
<pre> | ||
jdbc:oracle:thin:@config-awass3://{s3-uri} |
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.
typo on awa
maybe we can also point out the optional aws{s3 uri}
@@ -0,0 +1,88 @@ | |||
################################################################################ |
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 think this is the THIRD_PARTY_LICENSES.txt
</dependency> | ||
<dependency> | ||
<groupId>software.amazon.awssdk</groupId> | ||
<artifactId>appconfig</artifactId> |
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.
Do we still need this?
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.
Looks good except for some minor comments I just left. Please let me know when addressed.
Combining the work by @psilberk into this branch.
This branch adds the following Configuration Providers for AWS Services: