-
-
Notifications
You must be signed in to change notification settings - Fork 364
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
Add support for VPC security group to RDS resource #937
Conversation
} | ||
return { attr_to_kwarg[attr] : attrs[attr] for attr in attrs.keys() } | ||
|
||
def _compare_instance_id(self, instance_id): | ||
# take care when comparing instance ids, as aws lowercases and converts to unicode | ||
return unicode(self.rds_dbinstance_id).lower() == unicode(instance_id).lower() | ||
|
||
def fetch_security_group_resources(self, config): | ||
def fetch_security_group_resources(self, config, type, idSelector): |
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.
nitpick: idSelector -> id_selector
vpcSecurityGroups = [ resources.ec2SecurityGroups.test-rds-sg ]; | ||
}; | ||
|
||
resources.ec2SecurityGroups.test-rds-sg = |
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 guess this should work on new AWS accounts that use default vpc but for EC2 classic accounts the sg won't be a vpc one ?
I think you could add a vpc as resource here and pass it to vpcId attribute of the security group ?
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.
@AmineChikhaoui this turns out to be trickier than that. Running from EC2-VPC account if I introduce a new VPC and assign a security group to that VPC then I also have to ensure that an RDS instance is created in the same VPC. To be able to do that we need to introduce DB Subnet Group resource (which currently is not available with NixOps) and extend RDS resource to support DB Subnet Group assignment. It is a lot of work but in a real production setup we need this functionality anyway, otherwise we will be locked to a default VPC.
With that said, do you think this work needs to be done in this or separate PR?
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.
@alexeiskachykhin I think for this PR you could start by adding an option for dbSubnetGroup that takes a string first so that we can create an rds dbinstance in a VPC != default. Later a separate resource can be implemented.
@alexeiskachykhin i have vpc-security-groups for rds-instances (including db-subnet-groups) implemented on my fork. if you're interested https://github.com/MapCaseGmbH/nixops/tree/pr-rds-vpc-securitygroup |
Hello! Thank you for this PR. In the past several months, some major changes have taken place in
This is all accumulating in to what I hope will be a NixOps 2.0 My hope is that by adding types and more thorough automated testing, However, because of the major changes, it has become likely that this If you would like to see this merge, please bring it up to date with Thank you again for the work you've done here, I am sorry to be Graham |
This pull request introduces an ability to specify VPC security groups assigned to an RDS instance.