-
Notifications
You must be signed in to change notification settings - Fork 5
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
Added a provider for GCP #89
Conversation
ojdbc-provider-gcp/README.md
Outdated
A sign-in screen appears. After you sign in, your credentials are stored in the local credential file used by ADC. | ||
|
||
## GCP Object Storage Config Provider | ||
The Oracle DataSource uses a new prefix `jdbc:oracle:thin:@config-gcp-object:` to be able to identify that the configuration parameters should be loaded using GCP Object Storage. Users only need to indicate the project, bucket and object containing the JSON payload, with the following syntax: |
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 it should be without the '-' (like config-ociobject), and config-gcpstorage (being the other one config-gcpsecret)
* A provider for JSON payload which contains configuration from GCP Object | ||
* Storage. See {@link #getJson(String)} for the spec of the JSON payload. | ||
*/ | ||
public class GcpObjectStorageConfigurationProvider extends OracleConfigurationJsonProvider { |
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.
Same observation regarding the naming (in the readme) this should be GcpCloudStorage....
* Manager. | ||
* See {@link #getJson(String)} for the spec of the JSON payload. | ||
**/ | ||
public class GcpVaultSecretConfigurationProvider extends OracleConfigurationJsonProvider { |
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.
As in the readme, I think this one should be GcpSecretManagerConfiguratioProvider
* A standalone example that configures Oracle JDBC to be provided with the | ||
* connection properties retrieved from OCI Object Storage. | ||
*/ | ||
public class SimpleObjectStorageExample { |
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.
Same observation with the name (CloudStorage), but the demo is perfect, thanks
* A standalone example that configures Oracle JDBC to be provided with the | ||
* connection properties retrieved from GCP Secret Manager. | ||
*/ | ||
public class SimpleVaultJsonExample { |
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.
SimpleSecretManagerExample?
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 looks very very very good, thanks!
I added just some recommendation for classes and provider names.
I think we won't be able to merge it until we get the approval for the GCP Java SDK.
...er-gcp/src/main/java/oracle/jdbc/provider/gcp/resource/GcpSecretManagerPasswordProvider.java
Outdated
Show resolved
Hide resolved
ojdbc-provider-gcp/src/main/java/oracle/jdbc/provider/gcp/secrets/GcpSecretManagerFactory.java
Outdated
Show resolved
Hide resolved
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.
Adding GCP support is really great! Thanks!
.../main/java/oracle/jdbc/provider/gcp/configuration/GcpSecretManagerConfigurationProvider.java
Outdated
Show resolved
Hide resolved
README.md
Outdated
contain providers for a particular cloud platform: | ||
<dl> | ||
<dt><a href="ojdbc-provider-oci/README.md">Oracle JDBC OCI Providers</a></dt> | ||
<dd>Providers for integration with Oracle Cloud Infrastructure (OCI)</dd> | ||
<dt><a href="ojdbc-provider-azure/README.md">Oracle JDBC Azure Providers</a></dt> | ||
<dd>Providers for integration with Microsoft Azure</dd> | ||
<dt><a href="ojdbc-provider-gdp/README.md">Oracle JDBC GCP Providers</a></dt> |
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.
gdp should be gcp
This project add the following providers for Google Cloud Platform:
This pull request also adds samples on how to use these providers.