Skip to content

Conversation

@daniosim
Copy link
Contributor

@daniosim daniosim commented Oct 12, 2020

  • address AWS018 and GEN003 tfsec warnings
  • made example more complete but was a bit difficult because requires networking configuration from other modules
  • fixed some input variable descriptions

Notes about addressing tfsec warnings:

  • Added descriptions for security group rules to address AWS018
  • The current module setup where the master db user's password is directly provided as an input variable (stored as plain-text in state) definitely seems like something we'd want to address. Added DEV-14410 to the backlog.

Unrelated comment/question: Why are we setting up a db parameter group resource for each db instance when it's not specifying any parameters to apply anyway?

}

resource "aws_security_group_rule" "tamr_vm" {
description = "Rule for ingress from Tamr VM to Postgres"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Idea for future improvement (out of scope for this ticket):
Probably don't need to be as prescriptive as we are in this module having a 1 vm + list of spark security group IDs.

Could probably just take a list that includes both.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parameter_group_name = "example-rds-postgres-pg"
identifier_prefix = "example-rds-"
username = "example-tamr-master"
password = "foo" #tfsec:ignore:GEN003
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good choice, since I think this is actually too short of a password to be accepted by RDS (maybe I'm thinking of something else though)

@souza-dan
Copy link
Contributor

Unrelated comment/question: Why are we setting up a db parameter group resource for each db instance when it's not specifying any parameters to apply anyway?

I think we're using the parameter group for setting the database type to postgres and the version. If there's a better way to specify those or there's more we could be doing with it, I'm all for it!

@daniosim
Copy link
Contributor Author

Unrelated comment/question: Why are we setting up a db parameter group resource for each db instance when it's not specifying any parameters to apply anyway?

I think we're using the parameter group for setting the database type to postgres and the version. If there's a better way to specify those or there's more we could be doing with it, I'm all for it!

Ah, yeah I was just curious. But it does look like the aws_db_instance resource supports engine and engine_version inputs.

@daniosim daniosim merged commit fda8e55 into master Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants