-
Notifications
You must be signed in to change notification settings - Fork 256
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
feat: record resources in codegen #746
feat: record resources in codegen #746
Conversation
resources/secrets/src/lib.rs
Outdated
let secrets = factory.get_secrets().await?; | ||
|
||
Ok(SecretStore { secrets }) | ||
} | ||
} | ||
|
||
/// Store that holds all the secrets available to a deployment | ||
#[derive(Deserialize, Serialize, Clone)] |
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.
Since this is also the Output
we'll end up storing the secret values in the DB. I don't know if we want to do that though 🤷♂️
Ok(build_data.clone()) | ||
} | ||
|
||
async fn output(self, factory: &mut dyn Factory) -> Result<Self::Output, crate::Error> { | ||
let secrets = factory.get_secrets().await?; |
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.
Maybe this can now read the secrets from the filesystem rather than getting them from factory? This means we can remove the secrets from the factory.
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.
But maybe we would still want to access historical secrets?
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 storing them on the filesystem rather than the database makes sense, that way they reflect the latest build and are easily deleted and replaced. But I'm not sure if that will be an issue when we allow setting secrets from the console, I feel like in that scenario having them in the DB would make more sense. 🤔
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.
Perhaps Secrets
shouldn't be a resource at all? Rather something we read from Secrets.toml
, store in the DB (or fs), and then set them in the environment on startup?
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 also thinking maybe they should not be a mutable resource
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.
LGTM, thanks!
Description of change
This updates the codegen to correctly record all types of resources being created... including secrets and static folders.
How Has This Been Tested (if applicable)?
Using the local runner.