From bd9218222d5c1c701257a70059f70425ca4e469e Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Mon, 30 Sep 2019 10:58:54 -0700 Subject: [PATCH] data/aws/vpc: Lay the groundwork for bring-your-own VPC/subnets The installer will no longer create (vs. the fully-IPI flow): * Internet gateways (aws_internet_gateway) * NAT gateways (aws_nat_gateway, aws_eip.nat_eip) * Subnets (aws_subnet) * Route tables (aws_route_table, aws_route, aws_route_table_association, aws_main_route_table_association) * VPCs (aws_vpc) * VPC DHCP options (aws_vpc_dhcp_options, vpc_dhcp_options_association) * VPC endpoints (aws_vpc_endpoint) because those resources define the VPC/subnet networking and we can't pick values for CIDRs and routing and such without a risk of picking something that collides with some other VPC/subnet consumer. The installer will continue to create (vs. the fully-IPI flow): * AMI copies (aws_ami_copy) * IAM roles and profiles (aws_iam_instance_profile, aws_iam_role, aws_iam_role_policy) * Instances (aws_instance, aws_network_interface.master) * Load balancers (aws_lb, aws_lb_listener, aws_lb_target_group, aws_lb_target_group_attachment) * Route 53 resources (aws_route53_zone, aws_route53_record) * S3 resources (aws_s3_bucket, aws_s3_bucket_object) * Security groups (aws_security_group, aws_security_group_rule) because other VPC/subnet consumers won't care about those. The installer will still need to grow new creation code (vs. the fully-IPI flow) for: * Adding kubernetes.io/cluster/.*: shared to the user-provided subnets. This will happen in Go immediately before calling out to Terraform to begin creating new resources, but I'm punting on that for this commit. I'd tried to use: id = var.vpc ? var.vpc : aws_vpc.new_vpc.*.id" and similar, hoping for something like Python's truthiness [1], but Terraform rejected that with [2]: The condition value is null. Conditions must either be true or false. aws_vpc.new_vpc[0].id and similar for the newly conditional resources (count = var.vpc == null ? 1 : 0, and similar) follows [3]: For resources where count is set -- even if the expression evaluates to 1 -- aws_instance.example returns a list of objects whose length is decided by the count. In this case aws_instance.example.id is an error, and must instead be written as aws_instance.example[0].id to access one of the objects before retrieving its id attribute value. [1]: https://docs.python.org/3.6/library/stdtypes.html#truth-value-testing [2]: https://prow.svc.ci.openshift.org/view/gcs/origin-ci-test/pr-logs/pull/openshift_installer/2438/pull-ci-openshift-installer-master-e2e-aws/7844 [3]: https://www.terraform.io/upgrade-guides/0-12.html#working-with-count-on-resources --- data/data/aws/main.tf | 10 +++++++--- data/data/aws/variables-aws.tf | 17 +++++++++++++++++ data/data/aws/vpc/common.tf | 19 ++++++++++++------- data/data/aws/vpc/master-elb.tf | 4 ++-- data/data/aws/vpc/outputs.tf | 8 ++++---- data/data/aws/vpc/variables.tf | 14 ++++++++++++++ data/data/aws/vpc/vpc-private.tf | 12 +++++++----- data/data/aws/vpc/vpc-public.tf | 32 ++++++++++++++++++++------------ data/data/aws/vpc/vpc.tf | 15 +++++++++++---- 9 files changed, 94 insertions(+), 37 deletions(-) diff --git a/data/data/aws/main.tf b/data/data/aws/main.tf index e24ba4d779d..4eea5257557 100644 --- a/data/data/aws/main.tf +++ b/data/data/aws/main.tf @@ -75,9 +75,13 @@ module "dns" { module "vpc" { source = "./vpc" - cidr_block = var.machine_cidr - cluster_id = var.cluster_id - region = var.aws_region + cidr_block = var.machine_cidr + cluster_id = var.cluster_id + region = var.aws_region + vpc = var.aws_vpc + public_subnets = var.aws_public_subnets + private_subnets = var.aws_private_subnets + availability_zones = distinct( concat( var.aws_master_availability_zones, diff --git a/data/data/aws/variables-aws.tf b/data/data/aws/variables-aws.tf index 149af1f161d..f10766a3f28 100644 --- a/data/data/aws/variables-aws.tf +++ b/data/data/aws/variables-aws.tf @@ -69,3 +69,20 @@ variable "aws_worker_availability_zones" { description = "The availability zones to provision for workers. Worker instances are created by the machine-API operator, but this variable controls their supporting infrastructure (subnets, routing, etc.)." } +variable "aws_vpc" { + type = string + default = null + description = "(optional) An existing network (VPC ID) into which the cluster should be installed." +} + +variable "aws_public_subnets" { + type = list(string) + default = null + description = "(optional) Existing public subnets into which the cluster should be installed." +} + +variable "aws_private_subnets" { + type = list(string) + default = null + description = "(optional) Existing private subnets into which the cluster should be installed." +} diff --git a/data/data/aws/vpc/common.tf b/data/data/aws/vpc/common.tf index 896a94db489..fbc320ee3bb 100644 --- a/data/data/aws/vpc/common.tf +++ b/data/data/aws/vpc/common.tf @@ -1,16 +1,21 @@ # Canonical internal state definitions for this module. # read only: only locals and data source definitions allowed. No resources or module blocks in this file -// Only reference data sources which are guaranteed to exist at any time (above) in this locals{} block -locals { - private_subnet_ids = aws_subnet.private_subnet.*.id - public_subnet_ids = aws_subnet.public_subnet.*.id -} - # all data sources should be input variable-agnostic and used as canonical source for querying "state of resources" and building outputs # (ie: we don't want "aws.new_vpc" and "data.aws_vpc.cluster_vpc", just "data.aws_vpc.cluster_vpc" used everwhere). data "aws_vpc" "cluster_vpc" { - id = aws_vpc.new_vpc.id + id = var.vpc == null ? aws_vpc.new_vpc[0].id : var.vpc } +data "aws_subnet" "public" { + count = var.public_subnets == null ? length(var.availability_zones) : length(var.public_subnets) + + id = var.public_subnets == null ? aws_subnet.public_subnet[count.index].id : var.public_subnets[count.index] +} + +data "aws_subnet" "private" { + count = var.private_subnets == null ? length(var.availability_zones) : length(var.private_subnets) + + id = var.private_subnets == null ? aws_subnet.private_subnet[count.index].id : var.private_subnets[count.index] +} diff --git a/data/data/aws/vpc/master-elb.tf b/data/data/aws/vpc/master-elb.tf index fc56d71610b..c1ffb26783e 100644 --- a/data/data/aws/vpc/master-elb.tf +++ b/data/data/aws/vpc/master-elb.tf @@ -1,7 +1,7 @@ resource "aws_lb" "api_internal" { name = "${var.cluster_id}-int" load_balancer_type = "network" - subnets = local.private_subnet_ids + subnets = data.aws_subnet.private.*.id internal = true enable_cross_zone_load_balancing = true idle_timeout = 3600 @@ -23,7 +23,7 @@ resource "aws_lb" "api_internal" { resource "aws_lb" "api_external" { name = "${var.cluster_id}-ext" load_balancer_type = "network" - subnets = local.public_subnet_ids + subnets = data.aws_subnet.public.*.id internal = false enable_cross_zone_load_balancing = true idle_timeout = 3600 diff --git a/data/data/aws/vpc/outputs.tf b/data/data/aws/vpc/outputs.tf index 946699559a0..0c6e91e17e3 100644 --- a/data/data/aws/vpc/outputs.tf +++ b/data/data/aws/vpc/outputs.tf @@ -3,19 +3,19 @@ output "vpc_id" { } output "az_to_private_subnet_id" { - value = zipmap(var.availability_zones, local.private_subnet_ids) + value = zipmap(data.aws_subnet.private.*.availability_zone, data.aws_subnet.private.*.id) } output "az_to_public_subnet_id" { - value = zipmap(var.availability_zones, local.public_subnet_ids) + value = zipmap(data.aws_subnet.public.*.availability_zone, data.aws_subnet.public.*.id) } output "public_subnet_ids" { - value = local.public_subnet_ids + value = data.aws_subnet.public.*.id } output "private_subnet_ids" { - value = local.private_subnet_ids + value = data.aws_subnet.private.*.id } output "master_sg_id" { diff --git a/data/data/aws/vpc/variables.tf b/data/data/aws/vpc/variables.tf index 40de23f9ac0..5009780908a 100644 --- a/data/data/aws/vpc/variables.tf +++ b/data/data/aws/vpc/variables.tf @@ -32,3 +32,17 @@ variable "tags" { description = "AWS tags to be applied to created resources." } +variable "vpc" { + type = string + description = "An existing network (VPC ID) into which the cluster should be installed." +} + +variable "public_subnets" { + type = list(string) + description = "Existing public subnets into which the cluster should be installed." +} + +variable "private_subnets" { + type = list(string) + description = "Existing private subnets into which the cluster should be installed." +} diff --git a/data/data/aws/vpc/vpc-private.tf b/data/data/aws/vpc/vpc-private.tf index fcce956263d..15f98bf7968 100644 --- a/data/data/aws/vpc/vpc-private.tf +++ b/data/data/aws/vpc/vpc-private.tf @@ -1,5 +1,6 @@ resource "aws_route_table" "private_routes" { - count = length(var.availability_zones) + count = var.private_subnets == null ? length(var.availability_zones) : 0 + vpc_id = data.aws_vpc.cluster_vpc.id tags = merge( @@ -11,7 +12,8 @@ resource "aws_route_table" "private_routes" { } resource "aws_route" "to_nat_gw" { - count = length(var.availability_zones) + count = var.private_subnets == null ? length(var.availability_zones) : 0 + route_table_id = aws_route_table.private_routes[count.index].id destination_cidr_block = "0.0.0.0/0" nat_gateway_id = element(aws_nat_gateway.nat_gw.*.id, count.index) @@ -23,7 +25,7 @@ resource "aws_route" "to_nat_gw" { } resource "aws_subnet" "private_subnet" { - count = length(var.availability_zones) + count = var.private_subnets == null ? length(var.availability_zones) : 0 vpc_id = data.aws_vpc.cluster_vpc.id @@ -41,8 +43,8 @@ resource "aws_subnet" "private_subnet" { } resource "aws_route_table_association" "private_routing" { - count = length(var.availability_zones) + count = var.private_subnets == null ? length(var.availability_zones) : 0 + route_table_id = aws_route_table.private_routes[count.index].id subnet_id = aws_subnet.private_subnet[count.index].id } - diff --git a/data/data/aws/vpc/vpc-public.tf b/data/data/aws/vpc/vpc-public.tf index 7eb8f44374f..e85ac8e92da 100644 --- a/data/data/aws/vpc/vpc-public.tf +++ b/data/data/aws/vpc/vpc-public.tf @@ -1,4 +1,6 @@ resource "aws_internet_gateway" "igw" { + count = var.vpc == null ? 1 : 0 + vpc_id = data.aws_vpc.cluster_vpc.id tags = merge( @@ -10,6 +12,8 @@ resource "aws_internet_gateway" "igw" { } resource "aws_route_table" "default" { + count = var.vpc == null ? 1 : 0 + vpc_id = data.aws_vpc.cluster_vpc.id tags = merge( @@ -21,14 +25,18 @@ resource "aws_route_table" "default" { } resource "aws_main_route_table_association" "main_vpc_routes" { + count = var.vpc == null ? 1 : 0 + vpc_id = data.aws_vpc.cluster_vpc.id - route_table_id = aws_route_table.default.id + route_table_id = aws_route_table.default[0].id } resource "aws_route" "igw_route" { + count = var.vpc == null ? 1 : 0 + destination_cidr_block = "0.0.0.0/0" - route_table_id = aws_route_table.default.id - gateway_id = aws_internet_gateway.igw.id + route_table_id = aws_route_table.default[0].id + gateway_id = aws_internet_gateway.igw[0].id timeouts { create = "20m" @@ -36,11 +44,10 @@ resource "aws_route" "igw_route" { } resource "aws_subnet" "public_subnet" { - count = length(var.availability_zones) - vpc_id = data.aws_vpc.cluster_vpc.id - - cidr_block = cidrsubnet(local.new_public_cidr_range, 3, count.index) + count = var.public_subnets == null ? length(var.availability_zones) : 0 + vpc_id = data.aws_vpc.cluster_vpc.id + cidr_block = cidrsubnet(local.new_public_cidr_range, 3, count.index) availability_zone = var.availability_zones[count.index] tags = merge( @@ -52,13 +59,14 @@ resource "aws_subnet" "public_subnet" { } resource "aws_route_table_association" "route_net" { - count = length(var.availability_zones) - route_table_id = aws_route_table.default.id + count = var.public_subnets == null ? length(var.availability_zones) : 0 + + route_table_id = aws_route_table.default[0].id subnet_id = aws_subnet.public_subnet[count.index].id } resource "aws_eip" "nat_eip" { - count = length(var.availability_zones) + count = var.public_subnets == null ? length(var.availability_zones) : 0 vpc = true tags = merge( @@ -75,7 +83,8 @@ resource "aws_eip" "nat_eip" { } resource "aws_nat_gateway" "nat_gw" { - count = length(var.availability_zones) + count = var.public_subnets == null ? length(var.availability_zones) : 0 + allocation_id = aws_eip.nat_eip[count.index].id subnet_id = aws_subnet.public_subnet[count.index].id @@ -86,4 +95,3 @@ resource "aws_nat_gateway" "nat_gw" { var.tags, ) } - diff --git a/data/data/aws/vpc/vpc.tf b/data/data/aws/vpc/vpc.tf index 5036ce7b48e..a2534912f2a 100644 --- a/data/data/aws/vpc/vpc.tf +++ b/data/data/aws/vpc/vpc.tf @@ -4,6 +4,8 @@ locals { } resource "aws_vpc" "new_vpc" { + count = var.vpc == null ? 1 : 0 + cidr_block = var.cidr_block enable_dns_hostnames = true enable_dns_support = true @@ -17,7 +19,9 @@ resource "aws_vpc" "new_vpc" { } resource "aws_vpc_endpoint" "s3" { - vpc_id = aws_vpc.new_vpc.id + count = var.vpc == null ? 1 : 0 + + vpc_id = data.aws_vpc.cluster_vpc.id service_name = "com.amazonaws.${var.region}.s3" route_table_ids = concat( aws_route_table.private_routes.*.id, @@ -26,6 +30,8 @@ resource "aws_vpc_endpoint" "s3" { } resource "aws_vpc_dhcp_options" "main" { + count = var.vpc == null ? 1 : 0 + domain_name = var.region == "us-east-1" ? "ec2.internal" : format("%s.compute.internal", var.region) domain_name_servers = ["AmazonProvidedDNS"] @@ -33,7 +39,8 @@ resource "aws_vpc_dhcp_options" "main" { } resource "aws_vpc_dhcp_options_association" "main" { - vpc_id = aws_vpc.new_vpc.id - dhcp_options_id = aws_vpc_dhcp_options.main.id -} + count = var.vpc == null ? 1 : 0 + vpc_id = data.aws_vpc.cluster_vpc.id + dhcp_options_id = aws_vpc_dhcp_options.main[0].id +}