-
Notifications
You must be signed in to change notification settings - Fork 566
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
4.x: Adds OciSecretsConfigSourceProvider.java #7391
Conversation
Signed-off-by: Laird Nelson <[email protected]>
Signed-off-by: Laird Nelson <[email protected]>
…ptions Signed-off-by: Laird Nelson <[email protected]>
…g out into its own method Signed-off-by: Laird Nelson <[email protected]>
…s for more clarity Signed-off-by: Laird Nelson <[email protected]>
…ition Signed-off-by: Laird Nelson <[email protected]>
…etc. Signed-off-by: Laird Nelson <[email protected]>
...in/java/io/helidon/integrations/oci/secrets/configsource/OciSecretsConfigSourceProvider.java
Show resolved
Hide resolved
...in/java/io/helidon/integrations/oci/secrets/configsource/OciSecretsConfigSourceProvider.java
Outdated
Show resolved
Hide resolved
...in/java/io/helidon/integrations/oci/secrets/configsource/OciSecretsConfigSourceProvider.java
Outdated
Show resolved
Hide resolved
...in/java/io/helidon/integrations/oci/secrets/configsource/OciSecretsConfigSourceProvider.java
Outdated
Show resolved
Hide resolved
...in/java/io/helidon/integrations/oci/secrets/configsource/OciSecretsConfigSourceProvider.java
Outdated
Show resolved
Hide resolved
...-config-source/src/test/java/io/helidon/integrations/oci/secrets/configsource/UsageTest.java
Outdated
Show resolved
Hide resolved
...in/java/io/helidon/integrations/oci/secrets/configsource/OciSecretsConfigSourceProvider.java
Outdated
Show resolved
Hide resolved
...in/java/io/helidon/integrations/oci/secrets/configsource/OciSecretsConfigSourceProvider.java
Outdated
Show resolved
Hide resolved
...in/java/io/helidon/integrations/oci/secrets/configsource/OciSecretsConfigSourceProvider.java
Outdated
Show resolved
Hide resolved
...in/java/io/helidon/integrations/oci/secrets/configsource/OciSecretsConfigSourceProvider.java
Outdated
Show resolved
Hide resolved
...in/java/io/helidon/integrations/oci/secrets/configsource/OciSecretsConfigSourceProvider.java
Show resolved
Hide resolved
...in/java/io/helidon/integrations/oci/secrets/configsource/OciSecretsConfigSourceProvider.java
Show resolved
Hide resolved
...in/java/io/helidon/integrations/oci/secrets/configsource/OciSecretsConfigSourceProvider.java
Outdated
Show resolved
Hide resolved
...in/java/io/helidon/integrations/oci/secrets/configsource/OciSecretsConfigSourceProvider.java
Outdated
Show resolved
Hide resolved
...-config-source/src/test/java/io/helidon/integrations/oci/secrets/configsource/UsageTest.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Laird Nelson <[email protected]>
Signed-off-by: Laird Nelson <[email protected]>
…ght annotation per instruction Signed-off-by: Laird Nelson <[email protected]>
…om.xml from other OCI projects Signed-off-by: Laird Nelson <[email protected]>
Signed-off-by: Laird Nelson <[email protected]>
} | ||
|
||
@Test | ||
void testUsage() { |
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'm still not seeing the name change or other pure unit test coverage here.
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.
Could you give me an idea of what you would like to see tested? The reason I ask is because everything else about this is part of Helidon Config, so all of those mechanisms are tested. NodeConfigSources
return NodeContent
and the Helidon Config framework takes it from there. Everything else in this class is related to connecting to OCI and actually getting a secret. So I'm genuinely not sure what to unit test.
If you have a suggestion for the name of whatever test you're thinking of I am happy to use it.
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.
There is some 400+ lines in OciSecretsConfigSourceProvider
that should be tested since as it is right now there is nothing currently test covered in the normal case where the pipeline or developer typically does not have an $HOME/.oci/config present in their environment. I am suggesting we have some unit-test coverage on this code for the typical case. Perhaps even consider making more methods package private to be able to test them in isolation, eg. static boolean isModified(Instant pollInstant, Instant closestSecretExpirationInstant)
, static void completeAllSecretsTasks(Collection<? extends Callable<Void>> tasks, AutoCloseable secrets)
, Optional<NodeContent> load(Supplier<? extends Vaults> vaultsSupplier, Supplier<? extends Secrets> secretsSupplier, ListSecretsRequest listSecretsRequest)
with some fakes or mocks, etc.
Signed-off-by: Laird Nelson <[email protected]>
Signed-off-by: Laird Nelson <[email protected]>
...ig-source/src/test/java/io/helidon/integrations/oci/secrets/configsource/IsModifiedTest.java
Outdated
Show resolved
Hide resolved
...-config-source/src/test/java/io/helidon/integrations/oci/secrets/configsource/UsageTest.java
Outdated
Show resolved
Hide resolved
...fig-source/src/test/java/io/helidon/integrations/oci/secrets/configsource/ValueNodeTest.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Laird Nelson <[email protected]>
Signed-off-by: Laird Nelson <[email protected]>
Signed-off-by: Laird Nelson <[email protected]>
Addresses issue #4238.
Description:
OciSecretsConfigSourceProvider
is a Helidon SEConfigSourceProvider
that creates aConfigSource
that sources its information from an OCI Vault using the OCI Secrets Retrieval and Vaults APIs. TheConfigSource
so created is, as I've been directed, aNodeConfigSource
and aPollableSource
. Its "have I been modified?" logic is based on metadata also sourced from the vault.This PR supersedes other PRs that I've made in this general area as it now follows the most recent instructions and directions in this area that I've received. It should be easily back-portable to the
helidon-3.x
branch. If I understand properly, it should be usable as-is by Helidon's MicroProfile Config implementation via already-present Helidon mechanisms for includingConfigSourceProvider
implementations, though I'm not personally familiar yet with how to do this.I will be closing and re-pointing other PRs here.
Once this PR is approved (if), I will backport it to
helidon-3.x
.Documentation impact: There will need to be a couple of examples showing how to configure this
ConfigSourceProvider
in ameta-config.yaml
. (Examples already exist showing how to configure otherConfigSourceProvider
s; this one is not very different.) Javadoc already exists in this PR to show usage and a sample configuration.