-
Notifications
You must be signed in to change notification settings - Fork 9
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
Cached FIM - Part 0 - Terraform and RDS Bastion Setup #599
Conversation
@@ -58,6 +58,7 @@ module "iam-roles" { | |||
prod_account_id = local.env.prod_account_id | |||
region = local.env.region | |||
nws_shared_account_s3_bucket = local.env.nws_shared_account_s3_bucket | |||
viz_proc_admin_rw_secret_arn = module.secrets-manager.secret_arns["viz-proc-admin-rw-user"] |
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.
@nickchadwick-noaa - I'd love your thoughts on how I did this / am wondering if it will work, given that I'm referencing a module that exists further down in the file.
Basically, I wanted to use the best practice approach of letting Redshift communicate directly with secrets manager to get the password it needs for the external data schema connection to the Viz RDS database, instead of running SQL with the text password included (we should probably switch to this IAM-based route on the RDS foreign data wrapper setups as well - I think it is supported).
Anyways, I figured it was good to specify that Redshift only has the ability to access this one secret, hence passing it in as variable here. Seems like it could get messy, but I figure we can just pass that entire secret_arns list if we want, and then reference specific secrets within the IAM policy json, as required... if we need to add more.
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 like this approach. It will technically work even if that secrets module is further down the file.
The IAM Roles section of the repo is the one piece I have not completely revamped due to it's complexity and how messy the IAM policies are. I think it would be a really good idea to eventually replace all RDS authentication with IAM Roles instead of username/passwords https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/UsingWithRDS.IAMDBAuth.html
I think this is good for now, and then eventually I'll try to just redo the IAM module entirely.
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.
Cool, thanks. That sounds good. I've wondered about doing IAM DB authentication across the board as well, and the only thing I'm concerned about with that is an implication on the number of connections (which are many, from our lambda functions), which they vaguely say may be affected. But it would be really nice to only have to worry about the IAM roles on the Lambdas. But at the least, we could go this route on the RDS Bastion, to keep the passwords out of the raw SQL sent to the dbs for the foreign data wrapper setups.
Of course, the other option on the lambdas is to just store the secret name in an env variable, and have the aws api lookup the password during run-time. That would be a little more secure than how we currently have it, but would also have some [I think negligible] performance cost. Not sure how that performance cost compares to the IAM role route, maybe it's a wash,
egis_db_user_secret_string = module.secrets-manager.secret_strings["egis-pg-rds-secret"] | ||
egis_portal_password = local.env.viz_ec2_hydrovis_egis_pass | ||
viz_redshift_host = module.redshift-viz.dns_name | ||
viz_redshift_db_name = local.env.viz_redshift_db_name |
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 really only added the viz_redshift variables here. (had to change indentation, so it highlighted everything)
viz_db_address = module.rds-viz.instance.address | ||
viz_db_port = module.rds-viz.instance.port | ||
viz_db_name = local.env.viz_db_name | ||
viz_redshift_master_secret_string = module.secrets-manager.secret_strings["viz_redshift_master"] |
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 really only added the viz_redshift variables here. (had to change indentation, so it highlighted everything)
This PR is the first step to implementing the new Cached FIM Workflow I've been developing (I'll have 1-3 follow-up PRs - I'm trying to split this all up into appropriately sized chunks).
This PR:
Testing / Deployment Considerations:
I've tested the IAM Role and Redshift setup in terraform, and the RDS Bastion steps manually on both Redshift and RDS... but I haven't tested it all together through the central terraform module yet - I'm hoping this should be pretty smooth, and I can help troubleshoot any deployment issues in real-time, Nick. I've also included a couple comments in the code below for your consideration.