Expose Secret Client Wrapper #173
Conversation
lib/build-plugin.js
Outdated
| @@ -32,6 +34,10 @@ function buildPlugin(Client, pluginOpts) { | |||
| const client = new Client(opts.clientOptions) | |||
| const concurrency = opts.concurrency || DEFAULT_GET_CONCURRENCY | |||
|
|
|||
| // Register client wrapper | |||
| fastify.decorate('secretClient', new ClientWrapper(Client, fastify, opts.clientOptions)) | |||
There was a problem hiding this comment.
Not too sure on this method of exposing, at minimum given there is support for namespacing this would clash with other auth plugins (very bad). Alternatively could think about prefixing with namespace, or giving it a home inside the secrets object.
There was a problem hiding this comment.
then why don't we support namespacing for the client wrapper too?
lib/build-plugin.js
Outdated
| @@ -32,6 +34,10 @@ function buildPlugin(Client, pluginOpts) { | |||
| const client = new Client(opts.clientOptions) | |||
| const concurrency = opts.concurrency || DEFAULT_GET_CONCURRENCY | |||
|
|
|||
| // Register client wrapper | |||
| fastify.decorate('secretClient', new ClientWrapper(Client, fastify, opts.clientOptions)) | |||
There was a problem hiding this comment.
ok let's review this design decision. I would not encapsulate all the existing logic inside the new ClientWrapper class. let's just add a decorator which is a function to refresh secrets (or a specific secret) using the existing client. we may not need to create a new class in the first place
|
is this ready to be reviewed? |
|
Should be good, I still had to check for a close method being passed in to avoid |
README.md
Outdated
|
|
||
| console.log(fastify.secrets.auth.TOKEN) // Initial secret value | ||
|
|
||
| await fastify.secretsClient.refresh("TOKEN") |
There was a problem hiding this comment.
provide a way to specify a name for the refresh method, which defaults to refresh but may be changed in case the user has a "refresh" secret
| await fastify.secretsClient.refresh("TOKEN") | |
| await fastify.secrets.auth.refresh("TOKEN") |
|
@rp4rk any progress? |
|
Just firing through the tests now, should be straightforward enough 👍 |
README.md
Outdated
|
|
||
| await fastify.ready() | ||
|
|
||
| const refreshedSecrets = await fastify.secrets.refresh() // { 'TOKEN': 'refreshed value' } |
README.md
Outdated
| @@ -114,6 +114,57 @@ await fastify.ready() | |||
| console.log(fastify.secrets.db.PG_PASS) | |||
| ``` | |||
|
|
|||
| #### Refreshing Secrets | |||
|
|
|||
| In the event secrets need to be dynamically refreshed, a refresh method is exposed to allow for the refreshing of single, sets, or all secrets scoped to the provided namespace. The signature of the refresh method is as follows, | |||
There was a problem hiding this comment.
| In the event secrets need to be dynamically refreshed, a refresh method is exposed to allow for the refreshing of single, sets, or all secrets scoped to the provided namespace. The signature of the refresh method is as follows, | |
| In the event secrets need to be dynamically refreshed, a refresh method is exposed to allow for the refreshing of single, sets, or all secrets scoped to the provided namespace. The signature of the refresh method is as follows: |
There was a problem hiding this comment.
also, let's make extra sure that this all works with both the array mode and object mode definition of secrets please!
There was a problem hiding this comment.
Added an explicit test to cover this scenario here.
lib/build-plugin.js
Outdated
| if (namespace) { | ||
| decorateWithSecrets(fastify, namespace, { | ||
| ...existingSecrets, | ||
| ...refreshedSecrets | ||
| }) | ||
| } else { | ||
| decorateWithSecrets(fastify, namespace, { ...existingSecrets, ...refreshedSecrets }) | ||
| } |
There was a problem hiding this comment.
it seems to me that the two branches here are doing exactly the same thing
lib/build-plugin.js
Outdated
| @@ -66,4 +102,4 @@ function buildPlugin(Client, pluginOpts) { | |||
| return plugin | |||
| } | |||
|
|
|||
| module.exports = buildPlugin | |||
| module.exports = { buildPlugin } | |||
There was a problem hiding this comment.
Originally thought I may need to export some of the factored out functions to meet coverage requirements, turned out to be a non-issue
Closes #170
Implements a wrapper for the provided secrets client and exposes a method that allows for refreshing of singular secrets (for now).