fix: filemanager secret cache #928
Conversation
|
Sorry, not a Rust guy... perhaps @brainstorm should have a look a this? Also, how does this compare to solutions documented by AWS like: |
That one is using It's probably worth switching to that crate anyway as it already implements the locking. |
| @@ -28,6 +29,7 @@ async fn main() -> Result<(), Error> { | |||
| Arc::new(config), | |||
| Arc::new(s3::Client::with_defaults().await), | |||
| Arc::new(sqs::Client::with_defaults().await), | |||
| Arc::new(Mutex::new(secrets_manager::Client::with_defaults().await)), | |||
There was a problem hiding this comment.
Is this where the secret is fetched or the client is initialised?
This code is inside the main function. Does that mean it runs inside the handler?
If so, would simply putting this outside the handler function be easier / better?
Take advantage of execution environment reuse to improve the performance of your function. Initialize SDK clients and database connections outside of the function handler...
(from: https://docs.aws.amazon.com/lambda/latest/dg/rust-handler.html#rust-best-practices)
There was a problem hiding this comment.
Yes, that's where the client gets initialized. This is put into the AppState struct which is shared for all requests.
This code is inside the main function. Does that mean it runs inside the handler?
Everything inside the main function before run(app).await should only be initialized once when the Lambda execution loads (i.e. it's already outside of the handler). app in this context is the handler, and it's an axum Router, which is used to process requests. This state is cloned for each request, but that's okay because it still uses the same cache variable which is stored on heap inside a reference counting pointer - Arc.
The Mutex locks the client exclusively so that it's mutable. This is required to modify the state as it has to insert an entry when fetching a secret (or get an existing entry).
The aws_secretsmanager_caching crate does a similar thing except the lock is inside that crate and it's a RwLock. I think this should be better as it doesn't lock exclusively for reads - allowing multiple at the same time.
As an aside, this pattern (i.e. Arc<Mutex<T>> or Arc<RwLock<T>>) allows cloning/modifying shared state across threads from a regular & reference, rather than requiring &mut. E.g. noting the function that aws_secretsmanager_caching makes public has a &self rather than &mut self, even though that function does modify the state of self (through the RwLock). This normally wouldn't be allowed in Rust. I think Mutex/RwLock uses some unsafe code underneath to convince the compiler that this is okay.
Hopefully that makes sense, let me know!
Closes #925
Changes
get_secret_valuecalls that persists within a Lambda execution environment.