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

Adding AWS S3 Configuration Provider and AWS Secrets Manager Provider #135

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

ting-lan-wang
Copy link
Member

Combining the work by @psilberk into this branch.

This branch adds the following Configuration Providers for AWS Services:

  • AwsS3ConfigurationProvider: this provider retrieves the configuration in JSON Payload format from AWS S3.
  • AwsSecretsManagerConfigurationProvider: which retrieves the configuration in JSON Payload format from AWS Secrets Manager.
  • AwsJsonSecretsManagerProvider: which retrieves the JSON configuration from a AWS Secrets Manager.

psilberk and others added 7 commits September 18, 2024 16:40
 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
@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Dec 6, 2024

// Sample default URL if non present
if (args.length == 0) {
url = "jdbc:oracle:thin:@config-awss3://{bucket-name}/{key-name}";
Copy link
Member

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:

Copy link
Member

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>
Copy link
Member

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}
Copy link
Member

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 @@
################################################################################
Copy link
Member

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>
Copy link
Member

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?

Copy link
Member

@psilberk psilberk left a 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants