Skip to content

Add Resource Providers for HCP Vault Dedicated #162

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

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

Conversation

MouhsinElmajdouby
Copy link
Member

No description provided.

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Mar 20, 2025
@fmeheust fmeheust self-requested a review March 21, 2025 10:28
Copy link
Member

@fmeheust fmeheust left a comment

Choose a reason for hiding this comment

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

Review of HCP Vault Dedicated

@@ -534,7 +565,786 @@ For the JSON type of provider (HCP Vault Dedicated, HCP Vault Secrets, HTTP/HTTP
- method
- optional parameters (depends on the cloud provider).

## Caching configuration
## Dedicated Vault Username Provider
Copy link
Member

Choose a reason for hiding this comment

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

Missing a title for Resource Providers as there is for Config Providers

<td>
The field inside the JSON secret that contains the required value.
If the secret contains multiple keys, this parameter specifies which key to extract.
If the secret contains only one key, the value is automatically used.
Copy link
Member

Choose a reason for hiding this comment

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

Is that the case event if a fieldName with a different value is provided?

</table>

An example of a [connection properties file](https://docs.oracle.com/en/database/oracle/oracle-database/23/jajdb/oracle/jdbc/OracleConnection.html#CONNECTION_PROPERTY_CONFIG_FILE) that configures this provider can be found in [example-vault-secrets.properties](example-vault-secrets.properties).

Copy link
Member

Choose a reason for hiding this comment

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

We only support JSON values with multiple keys for dedicated vault and not for HCP? This question is valid for all resource providers

<tr>
<td><code>type</code></td>
<td>The wallet format.</td>
<td>SSO, PKCS12, PEM</td>
Copy link
Member

Choose a reason for hiding this comment

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

Format the types as on the provider below

<tr>
<td><code>fieldName</code> (Optional)</td>
<td>
The field inside the JSON secret that contains the required value.
Copy link
Member

Choose a reason for hiding this comment

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

Isn't the value supposed to be a wallet? Which JSON secret are we talking about here?

<tr>
<td><code>fieldName</code> (Optional)</td>
<td>
The field inside the JSON secret that contains the required value.
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this supposed to be tnsnames.ora file? Which JSON secret are we talking about here?

new ResourceParameter("authenticationMethod", AUTHENTICATION_METHOD,
"auto-detect",
HcpVaultDedicatedResourceProvider::parseAuthenticationMethod),
new ResourceParameter("vaultAddr", VAULT_ADDR),
Copy link
Member

Choose a reason for hiding this comment

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

Could it be useful to create constants or and Enum instead?

fileBytes = Base64.getDecoder().decode(secretBytes);
} else {
fileBytes = secretBytes;
}
Copy link
Member

Choose a reason for hiding this comment

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

I belive the code from line 85 to 93 is duplicated (several places), is it possible to create a method and call it instead?

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