Skip to content

Commit 1c4112b

Browse files
sebastianczechmichalbil
sebastianczech
and
michalbil
authored
feat(MAJOR): Refactor module vpc, remove module subnet_set (#68)
* remove subnet_set module * extend vpc module to create subnets and route tables * simplify subnets creation (not using complex locals from subnet_set module) * adjust the variables in variables.tf and outputs inoutputs.tf in order that makes sense * refactor variables e.g. create new variable internet_gateway, which defines object with attributes for IGW creation/usage * adjust variables description to standard from Major refactor for modules and examples #53 * adjust all examples to new approach with vpc module * adjust CloudNGFW examples to changes delivered for other examples in fix(examples): Refactor examples for reference architectures #49 * change approach for CloudWatch logs group in CloudNGFW (as the same name PaloAltoCloudNGFW has to be used for both examples, groups was created on account used in workflows and in TFVARS we are not creating new log group, we are reusing existing one) * added support for all possible attributes for every resource used in module * README changed to new standard with new file .header.md Co-authored-by: michalbil <92343355+michalbil@users.noreply.github.com>
1 parent ded78a9 commit 1c4112b

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

65 files changed

+3962
-3870
lines changed

‎.releaserc.json

+5-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
{
22
"branches": [
3-
"main"
3+
"main",
4+
{
5+
"name": "rc",
6+
"prerelease": true
7+
}
48
],
59
"plugins": [
610
[

‎examples/centralized_design/README.md

+2-3
Large diffs are not rendered by default.

‎examples/centralized_design/example.tfvars

+160-162
Large diffs are not rendered by default.

‎examples/centralized_design/main.tf

+35-91
Original file line numberDiff line numberDiff line change
@@ -5,74 +5,18 @@ module "vpc" {
55

66
for_each = var.vpcs
77

8-
name = "${var.name_prefix}${each.value.name}"
9-
cidr_block = each.value.cidr
10-
nacls = each.value.nacls
11-
security_groups = each.value.security_groups
12-
create_internet_gateway = true
13-
enable_dns_hostnames = true
14-
enable_dns_support = true
15-
instance_tenancy = "default"
16-
}
17-
18-
### SUBNETS ###
19-
20-
locals {
21-
# Flatten the VPCs and their subnets into a list of maps, each containing the VPC name, subnet name, and subnet details.
22-
subnets_in_vpcs = flatten([for vk, vv in var.vpcs : [for sk, sv in vv.subnets :
23-
{
24-
cidr = sk
25-
nacl = sv.nacl
26-
az = sv.az
27-
subnet = sv.subnet_group
28-
vpc = vk
29-
create_subnet = try(sv.create_subnet, true)
30-
create_route_table = try(sv.create_route_table, sv.create_subnet, true)
31-
existing_route_table_id = try(sv.existing_route_table_id, null)
32-
associate_route_table = try(sv.associate_route_table, true)
33-
route_table_name = try(sv.route_table_name, null)
34-
local_tags = try(sv.local_tags, {})
35-
}
36-
]])
37-
# Create a map of subnets, keyed by the VPC name and subnet name.
38-
subnets_with_lists = { for subnet_in_vpc in local.subnets_in_vpcs : "${subnet_in_vpc.vpc}-${subnet_in_vpc.subnet}" => subnet_in_vpc... }
39-
subnets = { for key, value in local.subnets_with_lists : key => {
40-
vpc = distinct([for v in value : v.vpc])[0] # VPC name (always take first from the list as key is limitting number of VPCs)
41-
subnet = distinct([for v in value : v.subnet])[0] # Subnet name (always take first from the list as key is limitting number of subnets)
42-
az = [for v in value : v.az] # List of AZs
43-
cidr = [for v in value : v.cidr] # List of CIDRs
44-
nacl = compact([for v in value : v.nacl]) # List of NACLs
45-
create_subnet = [for v in value : try(v.create_subnet, true)] # List of create_subnet flags
46-
create_route_table = [for v in value : try(v.create_route_table, v.create_subnet, true)] # List of create_route_table flags
47-
existing_route_table_id = [for v in value : try(v.existing_route_table_id, null)] # List of existing_route_table_id values
48-
associate_route_table = [for v in value : try(v.associate_route_table, true)] # List of associate_route_table flags
49-
route_table_name = [for v in value : try(v.route_table_name, null)] # List of route_table_name values
50-
local_tags = [for v in value : try(v.local_tags, {})] # List of local_tags maps
51-
} }
52-
}
53-
54-
module "subnet_sets" {
55-
source = "../../modules/subnet_set"
56-
57-
for_each = local.subnets
58-
59-
name = each.value.subnet
60-
vpc_id = module.vpc[each.value.vpc].id
61-
has_secondary_cidrs = module.vpc[each.value.vpc].has_secondary_cidrs
62-
nacl_associations = {
63-
for index, az in each.value.az : az =>
64-
lookup(module.vpc[each.value.vpc].nacl_ids, each.value.nacl[index], null) if length(each.value.nacl) > 0
8+
region = var.region
9+
name = "${var.name_prefix}${each.value.name}"
10+
cidr_block = each.value.cidr_block
11+
subnets = each.value.subnets
12+
nacls = each.value.nacls
13+
security_groups = each.value.security_groups
14+
15+
options = {
16+
enable_dns_hostnames = true
17+
enable_dns_support = true
18+
instance_tenancy = "default"
6519
}
66-
cidrs = {
67-
for index, cidr in each.value.cidr : cidr => {
68-
az = each.value.az[index]
69-
create_subnet = each.value.create_subnet[index]
70-
create_route_table = each.value.create_route_table[index]
71-
existing_route_table_id = each.value.existing_route_table_id[index]
72-
associate_route_table = each.value.associate_route_table[index]
73-
route_table_name = each.value.route_table_name[index]
74-
local_tags = each.value.local_tags[index]
75-
} }
7620
}
7721

7822
### ROUTES ###
@@ -83,7 +27,7 @@ locals {
8327
#
8428
# tgw_default = {
8529
# vpc = "security_vpc"
86-
# subnet = "tgw_attach"
30+
# subnet_group = "tgw_attach"
8731
# to_cidr = "0.0.0.0/0"
8832
# next_hop_key = "security_gwlb_outbound"
8933
# next_hop_type = "gwlbe_endpoint"
@@ -106,7 +50,7 @@ locals {
10650
for vk, vv in var.vpcs : [
10751
for rk, rv in vv.routes : {
10852
vpc = rv.vpc
109-
subnet = rv.subnet_group
53+
subnet_group = rv.subnet_group
11054
to_cidr = rv.to_cidr
11155
next_hop_type = rv.next_hop_type
11256
next_hop_map = {
@@ -118,9 +62,9 @@ locals {
11862
}
11963
]]))
12064
vpc_routes = {
121-
for route in local.vpc_routes_with_next_hop_map : "${route.vpc}-${route.subnet}-${route.to_cidr}" => {
65+
for route in local.vpc_routes_with_next_hop_map : "${route.vpc}-${route.subnet_group}-${route.to_cidr}" => {
12266
vpc = route.vpc
123-
subnet = route.subnet
67+
subnet_group = route.subnet_group
12468
to_cidr = route.to_cidr
12569
next_hop_set = lookup(route.next_hop_map, route.next_hop_type, null)
12670
}
@@ -132,7 +76,7 @@ module "vpc_routes" {
13276

13377
for_each = local.vpc_routes
13478

135-
route_table_ids = module.subnet_sets["${each.value.vpc}-${each.value.subnet}"].unique_route_table_ids
79+
route_table_ids = { for k, v in module.vpc[each.value.vpc].route_tables : v.az => v.id if v.subnet_group == each.value.subnet_group }
13680
to_cidr = each.value.to_cidr
13781
next_hop_set = each.value.next_hop_set
13882
}
@@ -144,7 +88,7 @@ module "natgw_set" {
14488

14589
for_each = var.natgws
14690

147-
subnets = module.subnet_sets["${each.value.vpc}-${each.value.subnet_group}"].subnets
91+
subnets = { for k, v in module.vpc[each.value.vpc].subnets : v.az => v if v.subnet_group == each.value.subnet_group }
14892
}
14993

15094
### TGW ###
@@ -167,8 +111,8 @@ module "transit_gateway_attachment" {
167111
for_each = var.tgw.attachments
168112

169113
name = "${var.name_prefix}${each.value.name}"
170-
vpc_id = module.subnet_sets["${each.value.vpc}-${each.value.subnet_group}"].vpc_id
171-
subnets = module.subnet_sets["${each.value.vpc}-${each.value.subnet_group}"].subnets
114+
vpc_id = module.vpc[each.value.vpc].id
115+
subnets = { for k, v in module.vpc[each.value.vpc].subnets : k => v if v.subnet_group == each.value.subnet_group }
172116
transit_gateway_route_table = module.transit_gateway.route_tables[each.value.route_table]
173117
propagate_routes_to = {
174118
to1 = module.transit_gateway.route_tables[each.value.propagate_routes_to].id
@@ -198,8 +142,8 @@ module "gwlb" {
198142
for_each = var.gwlbs
199143

200144
name = "${var.name_prefix}${each.value.name}"
201-
vpc_id = module.subnet_sets["${each.value.vpc}-${each.value.subnet_group}"].vpc_id
202-
subnets = module.subnet_sets["${each.value.vpc}-${each.value.subnet_group}"].subnets
145+
vpc_id = module.vpc[each.value.vpc].id
146+
subnets = { for k, v in module.vpc[each.value.vpc].subnets : v.az => v if v.subnet_group == each.value.subnet_group }
203147
}
204148

205149
resource "aws_lb_target_group_attachment" "this" {
@@ -221,13 +165,13 @@ module "gwlbe_endpoint" {
221165

222166
name = "${var.name_prefix}${each.value.name}"
223167
gwlb_service_name = module.gwlb[each.value.gwlb].endpoint_service.service_name
224-
vpc_id = module.subnet_sets["${each.value.vpc}-${each.value.subnet_group}"].vpc_id
225-
subnets = module.subnet_sets["${each.value.vpc}-${each.value.subnet_group}"].subnets
168+
vpc_id = module.vpc[each.value.vpc].id
169+
subnets = { for k, v in module.vpc[each.value.vpc].subnets : v.az => v if v.subnet_group == each.value.subnet_group }
226170

227171
act_as_next_hop_for = each.value.act_as_next_hop ? {
228172
"from-igw-to-lb" = {
229173
route_table_id = module.vpc[each.value.vpc].internet_gateway_route_table.id
230-
to_subnets = module.subnet_sets["${each.value.from_igw_to_vpc}-${each.value.from_igw_to_subnet_group}"].subnets
174+
to_subnets = { for k, v in module.vpc[each.value.from_igw_to_vpc].subnets : v.az => v if v.subnet_group == each.value.from_igw_to_subnet_group }
231175
}
232176
# The routes in this section are special in that they are on the "edge", that is they are part of an IGW route table,
233177
# and AWS allows their destinations to only be:
@@ -336,7 +280,7 @@ module "vmseries" {
336280
device_index = v.device_index
337281
security_group_ids = try([module.vpc[each.value.common.vpc].security_group_ids[v.security_group]], [])
338282
source_dest_check = try(v.source_dest_check, false)
339-
subnet_id = module.subnet_sets["${v.vpc}-${v.subnet_group}"].subnets[each.value.az].id
283+
subnet_id = module.vpc[v.vpc].subnets["${v.subnet_group}${each.value.az}"].id
340284
create_public_ip = try(v.create_public_ip, false)
341285
}
342286
}
@@ -345,7 +289,7 @@ module "vmseries" {
345289

346290
iam_instance_profile = aws_iam_instance_profile.vm_series_iam_instance_profile.name
347291
ssh_key_name = var.ssh_key_name
348-
tags = var.global_tags
292+
tags = var.tags
349293
}
350294

351295
### Public ALB and NLB used in centralized model ###
@@ -356,13 +300,13 @@ module "public_alb" {
356300
for_each = { for k, v in var.vmseries : k => v }
357301

358302
lb_name = "${var.name_prefix}${each.value.application_lb.name}"
359-
subnets = { for k, v in module.subnet_sets["${each.value.vpc}-${each.value.application_lb.subnet_group}"].subnets : k => { id = v.id } }
303+
subnets = { for k, v in module.vpc[each.value.vpc].subnets : k => { id = v.id } if v.subnet_group == each.value.application_lb.subnet_group }
360304
vpc_id = module.vpc[each.value.vpc].id
361305
security_groups = [module.vpc[each.value.vpc].security_group_ids[each.value.application_lb.security_group]]
362306
rules = each.value.application_lb.rules
363307
targets = { for vmseries in local.vmseries_instances : "${vmseries.group}-${vmseries.instance}" => module.vmseries["${vmseries.group}-${vmseries.instance}"].interfaces["public"].private_ip }
364308

365-
tags = var.global_tags
309+
tags = var.tags
366310
}
367311

368312
module "public_nlb" {
@@ -372,7 +316,7 @@ module "public_nlb" {
372316

373317
name = "${var.name_prefix}${each.value.network_lb.name}"
374318
internal_lb = false
375-
subnets = { for k, v in module.subnet_sets["${each.value.vpc}-${each.value.network_lb.subnet_group}"].subnets : k => { id = v.id } }
319+
subnets = { for k, v in module.vpc[each.value.vpc].subnets : k => { id = v.id } if v.subnet_group == each.value.network_lb.subnet_group }
376320
vpc_id = module.vpc[each.value.vpc].id
377321

378322
balance_rules = { for k, v in each.value.network_lb.rules : k => {
@@ -384,7 +328,7 @@ module "public_nlb" {
384328
targets = { for vmseries in local.vmseries_instances : "${vmseries.group}-${vmseries.instance}" => module.vmseries["${vmseries.group}-${vmseries.instance}"].interfaces["public"].private_ip }
385329
} }
386330

387-
tags = var.global_tags
331+
tags = var.tags
388332
}
389333

390334
### SPOKE VM INSTANCES ####
@@ -446,9 +390,9 @@ resource "aws_instance" "spoke_vms" {
446390
ami = data.aws_ami.this.id
447391
instance_type = each.value.type
448392
key_name = var.ssh_key_name
449-
subnet_id = module.subnet_sets["${each.value.vpc}-${each.value.subnet_group}"].subnets[each.value.az].id
393+
subnet_id = module.vpc[each.value.vpc].subnets["${each.value.subnet_group}${each.value.az}"].id
450394
vpc_security_group_ids = [module.vpc[each.value.vpc].security_group_ids[each.value.security_group]]
451-
tags = merge({ Name = "${var.name_prefix}${each.key}" }, var.global_tags)
395+
tags = merge({ Name = "${var.name_prefix}${each.key}" }, var.tags)
452396
iam_instance_profile = aws_iam_instance_profile.spoke_vm_iam_instance_profile.name
453397

454398
root_block_device {
@@ -485,8 +429,8 @@ module "app_lb" {
485429

486430
name = "${var.name_prefix}${each.key}"
487431
internal_lb = true
488-
subnets = { for k, v in module.subnet_sets["${each.value.vpc}-${each.value.subnet_group}"].subnets : k => { id = v.id } }
489-
vpc_id = module.subnet_sets["${each.value.vpc}-${each.value.subnet_group}"].vpc_id
432+
subnets = { for k, v in module.vpc[each.value.vpc].subnets : k => { id = v.id } if v.subnet_group == each.value.subnet_group }
433+
vpc_id = module.vpc[each.value.vpc].id
490434

491435
balance_rules = {
492436
"SSH-traffic" = {
@@ -512,5 +456,5 @@ module "app_lb" {
512456
}
513457
}
514458

515-
tags = var.global_tags
459+
tags = var.tags
516460
}

‎examples/centralized_design/main_test.go

+42-19
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"github.com/PaloAltoNetworks/terraform-modules-swfw-tests-skeleton/pkg/testskeleton"
77
"github.com/gruntwork-io/terratest/modules/logger"
88
"github.com/gruntwork-io/terratest/modules/terraform"
9+
"gotest.tools/v3/assert"
910
)
1011

1112
func CreateTerraformOptions(t *testing.T) *terraform.Options {
@@ -29,33 +30,55 @@ func CreateTerraformOptions(t *testing.T) *terraform.Options {
2930
return terraformOptions
3031
}
3132

33+
func checkIfTerraformVersionIsSupported(t *testing.T) bool {
34+
terraformOptions := terraform.WithDefaultRetryableErrors(t, &terraform.Options{
35+
TerraformDir: ".",
36+
Logger: logger.Discard,
37+
Lock: true,
38+
})
39+
_, err := terraform.InitE(t, terraformOptions)
40+
if err != nil {
41+
assert.ErrorContains(t, err, "Unsupported Terraform Core version")
42+
return false
43+
}
44+
return true
45+
}
46+
3247
func TestValidate(t *testing.T) {
33-
testskeleton.ValidateCode(t, nil)
48+
if checkIfTerraformVersionIsSupported(t) {
49+
testskeleton.ValidateCode(t, nil)
50+
}
3451
}
3552

3653
func TestPlan(t *testing.T) {
37-
// define options for Terraform
38-
terraformOptions := CreateTerraformOptions(t)
39-
// prepare list of items to check
40-
assertList := []testskeleton.AssertExpression{}
41-
// plan test infrastructure and verify outputs
42-
testskeleton.PlanInfraCheckErrors(t, terraformOptions, assertList, "No errors are expected")
54+
if checkIfTerraformVersionIsSupported(t) {
55+
// define options for Terraform
56+
terraformOptions := CreateTerraformOptions(t)
57+
// prepare list of items to check
58+
assertList := []testskeleton.AssertExpression{}
59+
// plan test infrastructure and verify outputs
60+
testskeleton.PlanInfraCheckErrors(t, terraformOptions, assertList, "No errors are expected")
61+
}
4362
}
4463

4564
func TestApply(t *testing.T) {
46-
// define options for Terraform
47-
terraformOptions := CreateTerraformOptions(t)
48-
// prepare list of items to check
49-
assertList := []testskeleton.AssertExpression{}
50-
// deploy test infrastructure and verify outputs and check if there are no planned changes after deployment
51-
testskeleton.DeployInfraCheckOutputs(t, terraformOptions, assertList)
65+
if checkIfTerraformVersionIsSupported(t) {
66+
// define options for Terraform
67+
terraformOptions := CreateTerraformOptions(t)
68+
// prepare list of items to check
69+
assertList := []testskeleton.AssertExpression{}
70+
// deploy test infrastructure and verify outputs and check if there are no planned changes after deployment
71+
testskeleton.DeployInfraCheckOutputs(t, terraformOptions, assertList)
72+
}
5273
}
5374

5475
func TestIdempotence(t *testing.T) {
55-
// define options for Terraform
56-
terraformOptions := CreateTerraformOptions(t)
57-
// prepare list of items to check
58-
assertList := []testskeleton.AssertExpression{}
59-
// deploy test infrastructure and verify outputs and check if there are no planned changes after deployment
60-
testskeleton.DeployInfraCheckOutputsVerifyChanges(t, terraformOptions, assertList)
76+
if checkIfTerraformVersionIsSupported(t) {
77+
// define options for Terraform
78+
terraformOptions := CreateTerraformOptions(t)
79+
// prepare list of items to check
80+
assertList := []testskeleton.AssertExpression{}
81+
// deploy test infrastructure and verify outputs and check if there are no planned changes after deployment
82+
testskeleton.DeployInfraCheckOutputsVerifyChanges(t, terraformOptions, assertList)
83+
}
6184
}

0 commit comments

Comments
 (0)