Terraform Refactor Stage I Guide
This guide will describe with examples the major changes to be made to the Terraform code base during the first stage of the Terraform refactoring for the upgrade from 0.11 to 0.12.
Lists of security group CIDR blocks variables
As is, any CIDR blocks needed for security group rules have been passed into the module seperately and the CIDR block lists built within the module and then used by a security group rule Terraform resource or passed into another module which will then create a security group rule resource. Just looking at these variables being passed into the module at the calling service level doesn’t give much indication of what they may be being used for behind the scenes in the module. Where this occurs they should be replaced by complete list variables (although currently lists as a string seperated by “,”) with descriptive names as to their usage such as
1variable "ec2_ssh_22_allowed_cidr_blocks" {
2 description = "List of CIDR blocks in string format seperated by ',' allowed SSH access to the Artifactory ec2 instances on port 22"
3}
4
5variable "elb_https_443_allowed_cidr_blocks" {
6 description = "List of CIDR blocks in string format seperated by ',' allowed https access to the Artifactory ELB on port 443"
7}
The variable names are in the format: type of resource the CIDR blocks will have access to, the protocol the traffic will use, and the port the traffic is allowed over. This allows the module to have less variables, some of which may even have not been used once passed into the module.
Inside the module these variables can then be used as variables for security group resources or modules
1module "bastion_inbound_22" {
2 source = "../../../core/aws-security-groups/rules/cidr"
3 type = "ingress"
4 from_port = 22
5 to_port = 22
6 protocol = "tcp"
7 cidr_blocks = var.ec2_ssh_22_allowed_cidr_blocks
8 security_group_id = aws_security_group.instance_security_group.id
9}
10
11module "artifactory_nat_inbound_443" {
12 source = "../../../core/aws-security-groups/rules/cidr"
13 type = "ingress"
14 from_port = 443
15 to_port = 443
16 protocol = "tcp"
17 cidr_blocks = var.elb_https_443_allowed_cidr_blocks
18 security_group_id = aws_security_group.elb_security_group.id
19}
As for building the values for these variables, if it is required, this should now be done as local variables in the calling service, so as to keep the module call tidy and clear.
A complete list should be built and then one final join used to convert the list into a “,” seperated string.
1locals {
2 elb_https_443_allowed_cidr_blocks = join(",",
3 formatlist("%s/32",
4 concat(
5 split(",", data.terraform_remote_state.access.outputs.nat_gateway_ip_address),
6 split(",", data.terraform_remote_state.tools.outputs.nat_gateway_ip_address)
7 )
8 )
9 )
10 asg_subnet_ids = split(",", data.terraform_remote_state.tools.outputs.private_subnet_ids)
11 availability_zones = split(",", data.terraform_remote_state.tools.outputs.availability_zones)
12 elb_subnet_ids = split(",", data.terraform_remote_state.tools.outputs.public_subnet_ids)
13 public_subnet_ids = split(",", data.terraform_remote_state.tools.outputs.private_subnet_ids)
14}
15
16module "artifactory" {
17 source = "../../../modules/support/service/artifactory"
18
19 # AWS Data Source Variables
20 artifactory_ami = data.aws_ami.ubuntu_16_04.id
21 artifactory_ssl_certificate_arn = data.terraform_remote_state.global.outputs.acm_certificate_arn
22 asg_subnet_ids = local.asg_subnet_ids
23 availability_zones = local.availability_zones
24 aws_region = var.aws_region
25 db_master_password = data.vault_generic_secret.artifactory.data["db_master_password"]
26 db_master_username = data.vault_generic_secret.artifactory.data["db_master_username"]
27 ec2_ssh_22_allowed_cidr_blocks = data.terraform_remote_state.management.outputs.vpc_cidr_block
28 elb_https_443_allowed_cidr_blocks = local.elb_https_443_allowed_cidr_blocks
29 elb_subnet_ids = local.elb_subnet_ids
30 global_acm_certificate_arn = data.terraform_remote_state.global.outputs.acm_certificate_arn
31 private_subnet_ids = data.terraform_remote_state.tools.outputs.private_subnet_ids
32 private_local_route53_zone_id = data.terraform_remote_state.management.outputs.private_local_route53_zone_id
33 public_route53_zone_id = data.terraform_remote_state.global.outputs.digital_nbrown_co_uk_zone_id
34 public_subnet_ids = local.public_subnet_ids
35 vpc_id = data.terraform_remote_state.tools.outputs.vpc_id
36 vpc_cidr_block = data.terraform_remote_state.management.outputs.vpc_cidr_block
37
38 # Standalone Module Variables
39 artifactory_asg_desired = "1"
40 artifactory_asg_max = "1"
41 artifactory_asg_min = "1"
42 artifactory_ec2_keypair_name = "tools-artifactory-linux-key-pair"
43 artifactory_instance_name = "artifactory"
44 artifactory_instance_type = "t2.large"
45 environment = "tools"
46 puppet_role = "artifactory"
47 service_name = "artifactory"
48}
Submodule consolodation
Further to the above consolodation of variables, where there is multiple module calls for different cidr block variables, if those variables have been consolodated to one list of subnets to be allowed access to a certain service, only one module should be called to add those security group rules to a security group.
For example
1module "forwarders_to_elb_1_security_group_rule" {
2 source = "../../../local/aws-security-groups/rules/cidr"
3
4 security_group_id = module.forwarders_to_elb_1_security_group.security_group_id
5 type = "ingress"
6 protocol = "tcp"
7 from_port = "5000"
8 to_port = "5000"
9 cidr_blocks = "${var.reserved_forwarders_cidr_block_1},${var.reserved_forwarders_cidr_block_2},${var.reserved_forwarders_cidr_block_3},${var.reserved_forwarders_cidr_block_4},${var.reserved_forwarders_cidr_block_5}"
10}
11
12module "sg_rules_set_allow_5044_to_5044" {
13
14source = "../../../../modules/local/elb-to-instance-sg-ruleset"
15
16lb_port_range_from = 5000
17lb_port_range_to = 5000
18instance_port_range_to = 5000
19cidr_blocks = "10.229.0.0/16,${data.terraform_remote_state.tools.outputs.vpc_cidr_block},${var.this_vpc_cidr_block},${module.nbrown-network-ips.nbrown_griffin_house},${var.softlayer_amsterdam_forwarders_cidr_block},${var.softlayer_london_forwarders_cidr_block},${var.griffin_house_websphere_forwarders_cidr_block}"
20elbs_security_group_id = aws_security_group.elb_security_group.id
21instances_security_group_id = aws_security_group.instance_security_group.id
22}
can be condensed into
1module "sg_rules_set_allow_5000_to_5000" {
2 source = "../../../../modules/local/elb-to-instance-sg-ruleset"
3
4 lb_port_range_from = 5000
5 lb_port_range_to = 5000
6 instance_port_range_from = 5000
7 instance_port_range_to = 5000
8 cidr_blocks = var.elb_logstash_5000_allowed_cidr_blocks
9 elbs_security_group_id = aws_security_group.elb_security_group.id
10 instances_security_group_id = aws_security_group.instance_security_group.id
11}
In this instance this also cuts down the number of security groups to be attached to the ELB from two to one.
Hard coded sensitive values
If any hard coded usernames or password for database access etc are found in the codebase these should be added the the vault and vault lookup performed on their vault keys to obtain these values.
1provider "vault" {
2 address = "https://vault.tools.digital.nbrown.co.uk"
3}
4
5db_master_password = data.vault_generic_secret.artifactory.data["db_master_password"]
6db_master_username = data.vault_generic_secret.artifactory.data["db_master_username"]
Module usage of aws_iam_account_alias
aws_iam_account_alias should be defined within the service modules and not passed in as variables, this reduces the amount of variables for the module and
1data "aws_iam_account_alias" "current" {}General data sources
With the exception of aws_iam_account_alias mentioned above, data sources should be used in the calling service and the values obtained from them passed into the module where needed to keep the module generic and reusable.
Calling service variables
In most cases calling services should only have one variable aws_region usually with a defualt value of eu-west-1. All other module variables should be hard coded into the module call.
Module calling style
All variables populated by a data source, local variable or variable should be seperated from variables with hard coded values
1module "artifactory" {
2 source = "../../../modules/support/service/artifactory"
3
4 # AWS Data Source Variables
5 artifactory_ami = data.aws_ami.ubuntu_16_04.id
6 artifactory_ssl_certificate_arn = data.terraform_remote_state.global.outputs.acm_certificate_arn
7 asg_subnet_ids = local.asg_subnet_ids
8 availability_zones = local.availability_zones
9 aws_region = var.aws_region
10 db_master_password = data.vault_generic_secret.artifactory.data["db_master_password"]
11 db_master_username = data.vault_generic_secret.artifactory.data["db_master_username"]
12 ec2_ssh_22_allowed_cidr_blocks = data.terraform_remote_state.management.outputs.vpc_cidr_block
13 elb_https_443_allowed_cidr_blocks = local.elb_https_443_allowed_cidr_blocks
14 elb_subnet_ids = local.elb_subnet_ids
15 global_acm_certificate_arn = data.terraform_remote_state.global.outputs.acm_certificate_arn
16 private_subnet_ids = data.terraform_remote_state.tools.outputs.private_subnet_ids
17 private_local_route53_zone_id = data.terraform_remote_state.management.outputs.private_local_route53_zone_id
18 public_route53_zone_id = data.terraform_remote_state.global.outputs.digital_nbrown_co_uk_zone_id
19 public_subnet_ids = local.public_subnet_ids
20 vpc_id = data.terraform_remote_state.tools.outputs.vpc_id
21 vpc_cidr_block = data.terraform_remote_state.management.outputs.vpc_cidr_block
22
23 # Standalone Module Variables
24 artifactory_asg_desired = "1"
25 artifactory_asg_max = "1"
26 artifactory_asg_min = "1"
27 artifactory_ec2_keypair_name = "tools-artifactory-linux-key-pair"
28 artifactory_instance_name = "artifactory"
29 artifactory_instance_type = "t2.large"
30 environment = "tools"
31 puppet_role = "artifactory"
32 service_name = "artifactory"
33}
Comment formating and location
Comments should be defined with a # and placed above the entire resource/module block with extra context as to which variable the comment refers to to ensure consistent positioning formatting within the resource/module declaration.
1# TODO - Use templatefile inline method instead of separate template_file data source for user_data
2# TODO - Use native list for subnet_id
3resource "aws_instance" "influxdb_a" {
4 ami = var.influxdb_ami
5 user_data = data.template_file.bootstrap_a.rendered
6 iam_instance_profile = aws_iam_instance_profile.instance_profile.name
7 disable_api_termination = false
8 instance_type = var.influxdb_instance_type
9 key_name = var.influxdb_ec2_keypair_name
10 monitoring = true
11 subnet_id = element(var.private_subnet_ids, 0)
12 vpc_security_group_ids = [aws_security_group.instance_security_group.id]
13
14 root_block_device {
15 volume_type = "gp2"
16 volume_size = 1024
17 }
18
19 tags = {
20 Name = "${var.environment}-influxdb-a"
21 Type = "EC2 Instance"
22 Environment = var.environment
23 }
24}
25
Vault provider location
Any vault provider should be defined in providers.tf rather than vault.tf.
1provider "vault" {
2 address = "https://vault.tools.digital.nbrown.co.uk"
3}
Use of AMI data sources to use the latest AMI images
Hard coded AMIs should be replaced with data sources filtered to the latest version of the AMI, whether they are public AMIs from major vendors such as Ubuntu or Amazon Linux, or internally built AMIs from NBrown.
The exception being where a fixed AMI is needed to ensure a service works as a breaking change may have been introduced in an updated software package/configuration in a later AMI version. The reason for using a fixed AMI ID should be detailed in a comment where it is used.
Below is an example of using a data source to obtain the latest Ubuntu 16.04 AMI ID using an account owner field to ensure it is coming from Ubuntu and a regular expression to ensure you get a x86-64 Ubuntu 16.04 AMI ID.
1# This is the latest official 16.04 AMI from Ubuntu
2data "aws_ami" "ubuntu_16_04" {
3 most_recent = true
4 owners = ["099720109477"]
5 name_regex = "^ubuntu/images/hvm-ssd/ubuntu-xenial-16.04-amd64-server-\\d{8}$"
6}
This ID can then be used as follows
1artifactory_ami_id = data.aws_ami.ubuntu_16_04.idUpdate tfstate key paths
Key paths where Terraform state is defined should be changed from terraform to terraform-012, both in remote state data sources and terraform configuration blocks.
For example
1data "terraform_remote_state" "access" {
2 backend = "s3"
3
4 config = {
5 bucket = "nbrown-terraform-remote"
6 key = "dpm/terraform/access/infrastructure/terraform.tfstate"
7 region = "eu-west-1"
8 }
9}
should be changed to
1data "terraform_remote_state" "access" {
2 backend = "s3"
3
4 config = {
5 bucket = "nbrown-terraform-remote"
6 key = "dpm/terraform-012/access/infrastructure/terraform.tfstate"
7 region = "eu-west-1"
8 }
9}
And
1terraform {
2 backend "s3" {
3 bucket = "nbrown-terraform-remote"
4 key = "dpm/terraform/access/service/openvpn/terraform.tfstate"
5 region = "eu-west-1"
6 acl = "bucket-owner-full-control"
7 }
8}
should be changed to
1terraform {
2 backend "s3" {
3 bucket = "nbrown-terraform-remote"
4 key = "dpm/terraform-012/access/service/openvpn/terraform.tfstate"
5 region = "eu-west-1"
6 acl = "bucket-owner-full-control"
7 }
8}
List value formatting
In lists with multiple values each value should be on their own line in alphabetical order
1enabled_metrics = [
2 "GroupDesiredCapacity",
3 "GroupInServiceInstances",
4 "GroupMaxSize",
5 "GroupMinSize",
6 "GroupPendingInstances",
7 "GroupStandbyInstances",
8 "GroupTerminatingInstances",
9 "GroupTotalInstances"
10]
In lists with a single value the whole statement should be on one line
1target_group_arns = [aws_alb_target_group.alb_target_group.arn]Variable naming
Variables should be as descriptive as possible, in the case of what value you are passing in. For example
artifactory_ssl_certificate_arn should be used in place of artifactory_ssl_certificate
artifactory_ami_id should be used in place of artifactory_ami
Newline consistency
All files should end with a single newline in line with POSIX standards.
Comments