AWS: Enable clusters with no public endpoints

data/data/aws/bootstrap/main.tf

@@ -1,3 +1,7 @@
+locals {
+  public_endpoints = var.publish_strategy == "External" ? true : false
+}
+
 resource "aws_s3_bucket" "ignition" {
   acl = "private"
 
@@ -117,7 +121,7 @@ resource "aws_instance" "bootstrap" {
   subnet_id                   = var.subnet_id
   user_data                   = data.ignition_config.redirect.rendered
   vpc_security_group_ids      = flatten([var.vpc_security_group_ids, aws_security_group.bootstrap.id])
-  associate_public_ip_address = true
+  associate_public_ip_address = local.public_endpoints
 
   lifecycle {
     # Ignore changes in the AMI which force recreation of the resource. This
@@ -147,7 +151,9 @@ resource "aws_instance" "bootstrap" {
 }
 
 resource "aws_lb_target_group_attachment" "bootstrap" {
-  count = var.target_group_arns_length
+  // Because of the issue https://github.com/hashicorp/terraform/issues/12570, the consumers cannot use a dynamic list for count
+  // and therefore are force to implicitly assume that the list is of aws_lb_target_group_arns_length - 1, in case there is no api_external
+  count = local.public_endpoints ? var.target_group_arns_length : var.target_group_arns_length - 1
 
   target_group_arn = var.target_group_arns[count.index]
   target_id        = aws_instance.bootstrap.private_ip
@@ -173,7 +179,7 @@ resource "aws_security_group_rule" "ssh" {
   security_group_id = aws_security_group.bootstrap.id
 
   protocol    = "tcp"
-  cidr_blocks = ["0.0.0.0/0"]
+  cidr_blocks = local.public_endpoints ? ["0.0.0.0/0"] : var.vpc_cidrs
   from_port   = 22
   to_port     = 22
 }
@@ -183,7 +189,7 @@ resource "aws_security_group_rule" "bootstrap_journald_gateway" {
   security_group_id = aws_security_group.bootstrap.id
 
   protocol    = "tcp"
-  cidr_blocks = ["0.0.0.0/0"]
+  cidr_blocks = local.public_endpoints ? ["0.0.0.0/0"] : var.vpc_cidrs
   from_port   = 19531
   to_port     = 19531
 }

data/data/aws/bootstrap/variables.tf

@@ -62,9 +62,19 @@ variable "vpc_id" {
   description = "VPC ID is used to create resources like security group rules for bootstrap machine."
 }
 
+variable "vpc_cidrs" {
+  type        = list(string)
+  default     = []
+  description = "VPC CIDR blocks."
+}
+
 variable "vpc_security_group_ids" {
   type        = list(string)
   default     = []
   description = "VPC security group IDs for the bootstrap node."
 }
 
+variable "publish_strategy" {
+  type        = string
+  description = "The publishing strategy for endpoints like load balancers"
+}

data/data/aws/main.tf

@@ -18,11 +18,13 @@ module "bootstrap" {
   instance_type            = var.aws_bootstrap_instance_type
   cluster_id               = var.cluster_id
   ignition                 = var.ignition_bootstrap
-  subnet_id                = module.vpc.az_to_public_subnet_id[var.aws_master_availability_zones[0]]
+  subnet_id                = var.aws_publish_strategy == "External" ? module.vpc.az_to_public_subnet_id[var.aws_master_availability_zones[0]] : module.vpc.az_to_private_subnet_id[var.aws_master_availability_zones[0]]
   target_group_arns        = module.vpc.aws_lb_target_group_arns
   target_group_arns_length = module.vpc.aws_lb_target_group_arns_length
   vpc_id                   = module.vpc.vpc_id
+  vpc_cidrs                = module.vpc.vpc_cidrs
   vpc_security_group_ids   = [module.vpc.master_sg_id]
+  publish_strategy         = var.aws_publish_strategy
 
   tags = local.tags
 }
@@ -46,6 +48,7 @@ module "masters" {
   target_group_arns_length = module.vpc.aws_lb_target_group_arns_length
   ec2_ami                  = aws_ami_copy.main.id
   user_data_ign            = var.ignition_master
+  publish_strategy         = var.aws_publish_strategy
 }
 
 module "iam" {
@@ -70,17 +73,19 @@ module "dns" {
   etcd_ip_addresses        = flatten(module.masters.ip_addresses)
   tags                     = local.tags
   vpc_id                   = module.vpc.vpc_id
+  publish_strategy         = var.aws_publish_strategy
 }
 
 module "vpc" {
   source = "./vpc"
 
-  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
+  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
+  publish_strategy = var.aws_publish_strategy
 
   availability_zones = distinct(
     concat(

data/data/aws/master/main.tf

@@ -1,5 +1,9 @@
 locals {
   arn = "aws"
+
+  // Because of the issue https://github.com/hashicorp/terraform/issues/12570, the consumers cannot use a dynamic list for count
+  // and therefore are force to implicitly assume that the list is of aws_lb_target_group_arns_length - 1, in case there is no api_external
+  target_group_arns_length = var.publish_strategy == "External" ? var.target_group_arns_length : var.target_group_arns_length - 1
 }
 
 resource "aws_iam_instance_profile" "master" {
@@ -128,9 +132,9 @@ resource "aws_instance" "master" {
 }
 
 resource "aws_lb_target_group_attachment" "master" {
-  count = var.instance_count * var.target_group_arns_length
+  count = var.instance_count * local.target_group_arns_length
 
-  target_group_arn = var.target_group_arns[count.index % var.target_group_arns_length]
-  target_id        = aws_instance.master[floor(count.index / var.target_group_arns_length)].private_ip
+  target_group_arn = var.target_group_arns[count.index % local.target_group_arns_length]
+  target_id        = aws_instance.master[floor(count.index / local.target_group_arns_length)].private_ip
 }
 

data/data/aws/master/variables.tf

@@ -70,3 +70,13 @@ variable "user_data_ign" {
   type = string
 }
 
+variable "publish_strategy" {
+  type        = string
+  description = <<EOF
+The publishing strategy for endpoints like load balancers.
+
+Because of the issue https://github.com/hashicorp/terraform/issues/12570, the consumers cannot use a dynamic list for count
+and therefore are force to implicitly assume that the list is of aws_lb_target_group_arns_length - 1, in case there is no api_external. And that's where this variable
+helps to decide if the target_group_arns is of length (target_group_arns_length) or (target_group_arns_length - 1)
+EOF
+}

data/data/aws/route53/base.tf

@@ -1,4 +1,14 @@
+locals {
+
+  // Because of the issue https://github.com/hashicorp/terraform/issues/12570, the consumers cannot count 0/1
+  // based on if api_external_lb_dns_name for example, which will be null when there is no external lb for API.
+  // So publish_strategy serves an coordinated proxy for that decision.
+  public_endpoints = var.publish_strategy == "External" ? true : false
+}
+
 data "aws_route53_zone" "public" {
+  count = local.public_endpoints ? 1 : 0
+
   name = var.base_domain
 }
 
@@ -21,7 +31,9 @@ resource "aws_route53_zone" "int" {
 }
 
 resource "aws_route53_record" "api_external" {
-  zone_id = data.aws_route53_zone.public.zone_id
+  count = local.public_endpoints ? 1 : 0
+
+  zone_id = data.aws_route53_zone.public[0].zone_id
   name    = "api.${var.cluster_domain}"
   type    = "A"
 

data/data/aws/route53/variables.tf

@@ -54,3 +54,13 @@ variable "api_internal_lb_zone_id" {
   type        = string
 }
 
+variable "publish_strategy" {
+  type        = string
+  description = <<EOF
+The publishing strategy for endpoints like load balancers
+
+Because of the issue https://github.com/hashicorp/terraform/issues/12570, the consumers cannot count 0/1
+based on if api_external_lb_dns_name for example, which will be null when there is no external lb for API.
+So publish_strategy serves an coordinated proxy for that decision.
+EOF
+}

data/data/aws/variables-aws.tf

@@ -86,3 +86,8 @@ variable "aws_private_subnets" {
   default = null
   description = "(optional) Existing private subnets into which the cluster should be installed."
 }
+
+variable "aws_publish_strategy" {
+  type = string
+  description = "The cluster publishing strategy, either Internal or External"
+}

data/data/aws/vpc/common.tf

@@ -1,6 +1,10 @@
 # Canonical internal state definitions for this module.
 # read only: only locals and data source definitions allowed. No resources or module blocks in this file
 
+locals {
+  public_endpoints = var.publish_strategy == "External" ? true : false
+}
+
 # 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/data/aws/vpc/master-elb.tf

@@ -20,6 +20,8 @@ resource "aws_lb" "api_internal" {
 }
 
 resource "aws_lb" "api_external" {
+  count = local.public_endpoints ? 1 : 0
+
   name                             = "${var.cluster_id}-ext"
   load_balancer_type               = "network"
   subnets                          = data.aws_subnet.public.*.id
@@ -66,6 +68,8 @@ resource "aws_lb_target_group" "api_internal" {
 }
 
 resource "aws_lb_target_group" "api_external" {
+  count = local.public_endpoints ? 1 : 0
+
   name     = "${var.cluster_id}-aext"
   protocol = "TCP"
   port     = 6443
@@ -138,14 +142,14 @@ resource "aws_lb_listener" "api_internal_services" {
 }
 
 resource "aws_lb_listener" "api_external_api" {
-  count = var.public_master_endpoints ? 1 : 0
+  count = local.public_endpoints ? 1 : 0
 
-  load_balancer_arn = aws_lb.api_external.arn
+  load_balancer_arn = aws_lb.api_external[0].arn
   protocol          = "TCP"
   port              = "6443"
 
   default_action {
-    target_group_arn = aws_lb_target_group.api_external.arn
+    target_group_arn = aws_lb_target_group.api_external[0].arn
     type             = "forward"
   }
 }

data/data/aws/vpc/outputs.tf

@@ -2,6 +2,10 @@ output "vpc_id" {
   value = data.aws_vpc.cluster_vpc.id
 }
 
+output "vpc_cidrs" {
+  value = [data.aws_vpc.cluster_vpc.cidr_block]
+}
+
 output "az_to_private_subnet_id" {
   value = zipmap(data.aws_subnet.private.*.availability_zone, data.aws_subnet.private.*.id)
 }
@@ -27,6 +31,9 @@ output "worker_sg_id" {
 }
 
 output "aws_lb_target_group_arns" {
+  // The order of the list is very important because the consumers assume the 3rd item is the external aws_lb_target_group
+  // Because of the issue https://github.com/hashicorp/terraform/issues/12570, the consumers cannot use a dynamic list for count
+  // and therefore are force to implicitly assume that the list is of aws_lb_target_group_arns_length - 1, in case there is no api_external
   value = compact(
     concat(
       aws_lb_target_group.api_internal.*.arn,
@@ -42,11 +49,11 @@ output "aws_lb_target_group_arns_length" {
 }
 
 output "aws_lb_api_external_dns_name" {
-  value = aws_lb.api_external.dns_name
+  value = local.public_endpoints ? aws_lb.api_external[0].dns_name : null
 }
 
 output "aws_lb_api_external_zone_id" {
-  value = aws_lb.api_external.zone_id
+  value = local.public_endpoints ? aws_lb.api_external[0].zone_id : null
 }
 
 output "aws_lb_api_internal_dns_name" {

data/data/aws/vpc/variables.tf

@@ -11,14 +11,9 @@ variable "cluster_id" {
   type = string
 }
 
-variable "private_master_endpoints" {
-  description = "If set to true, private-facing ingress resources are created."
-  default     = true
-}
-
-variable "public_master_endpoints" {
-  description = "If set to true, public-facing ingress resources are created."
-  default     = true
+variable "publish_strategy" {
+  type        = string
+  description = "The publishing strategy for endpoints like load balancers"
 }
 
 variable "region" {

docs/user/customization.md

@@ -21,6 +21,8 @@ The following `install-config.yaml` properties are available:
     The installer may also support older API versions.
 * `additionalTrustBundle` (optional string): a PEM-encoded X.509 certificate bundle that will be added to the nodes' trusted certificate store.
 * `baseDomain` (required string): The base domain to which the cluster should belong.
+* `publish` (optional string): This controls how the user facing endpoints of the cluster like the Kubernetes API, OpenShift routes etc. are exposed.
+    Valid values are `External` (the default) and `Internal`.
 * `controlPlane` (optional [machine-pool](#machine-pools)): The configuration for the machines that comprise the control plane.
 * `compute` (optional array of [machine-pools](#machine-pools)): The configuration for the machines that comprise the compute nodes.
 * `imageContentSources` (optional array of objects): Sources and repositories for the release-image content.

pkg/asset/cluster/tfvars.go

@@ -179,7 +179,7 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error {
 		for i, m := range workers {
 			workerConfigs[i] = m.Spec.Template.Spec.ProviderSpec.Value.Object.(*awsprovider.AWSMachineProviderConfig)
 		}
-		data, err := awstfvars.TFVars(vpc, privateSubnets, publicSubnets, masterConfigs, workerConfigs)
+		data, err := awstfvars.TFVars(vpc, privateSubnets, publicSubnets, installConfig.Config.Publish, masterConfigs, workerConfigs)
 		if err != nil {
 			return errors.Wrapf(err, "failed to get %s Terraform variables", platform)
 		}

pkg/asset/installconfig/installconfig_test.go

@@ -91,6 +91,7 @@ func TestInstallConfigGenerate_FillsInDefaults(t *testing.T) {
 			None: &none.Platform{},
 		},
 		PullSecret: `{"auths":{"example.com":{"auth":"authorization value"}}}`,
+		Publish:    types.ExternalPublishingStrategy,
 	}
 	assert.Equal(t, expected, installConfig.Config, "unexpected config generated")
 }
@@ -154,6 +155,7 @@ pullSecret: "{\"auths\":{\"example.com\":{\"auth\":\"authorization value\"}}}"
 					},
 				},
 				PullSecret: `{"auths":{"example.com":{"auth":"authorization value"}}}`,
+				Publish:    types.ExternalPublishingStrategy,
 			},
 		},
 		{
@@ -235,6 +237,7 @@ network:
 					},
 				},
 				PullSecret: `{"auths":{"example.com":{"auth":"authorization value"}}}`,
+				Publish:    types.ExternalPublishingStrategy,
 			},
 		},
 	}

pkg/asset/machines/aws/machines.go

@@ -144,18 +144,21 @@ func tagsFromUserTags(clusterID string, usertags map[string]string) ([]awsprovid
 }
 
 // ConfigMasters sets the PublicIP flag and assigns a set of load balancers to the given machines
-func ConfigMasters(machines []machineapi.Machine, clusterID string) {
+func ConfigMasters(machines []machineapi.Machine, clusterID string, publish types.PublishingStrategy) {
+	lbrefs := []awsprovider.LoadBalancerReference{{
+		Name: fmt.Sprintf("%s-int", clusterID),
+		Type: awsprovider.NetworkLoadBalancerType,
+	}}
+
+	if publish == types.ExternalPublishingStrategy {
+		lbrefs = append(lbrefs, awsprovider.LoadBalancerReference{
+			Name: fmt.Sprintf("%s-ext", clusterID),
+			Type: awsprovider.NetworkLoadBalancerType,
+		})
+	}
+
 	for _, machine := range machines {
 		providerSpec := machine.Spec.ProviderSpec.Value.Object.(*awsprovider.AWSMachineProviderConfig)
-		providerSpec.LoadBalancers = []awsprovider.LoadBalancerReference{
-			{
-				Name: fmt.Sprintf("%s-ext", clusterID),
-				Type: awsprovider.NetworkLoadBalancerType,
-			},
-			{
-				Name: fmt.Sprintf("%s-int", clusterID),
-				Type: awsprovider.NetworkLoadBalancerType,
-			},
-		}
+		providerSpec.LoadBalancers = lbrefs
 	}
 }

pkg/asset/machines/master.go

@@ -178,7 +178,7 @@ func (m *Master) Generate(dependencies asset.Parents) error {
 		if err != nil {
 			return errors.Wrap(err, "failed to create master machine objects")
 		}
-		aws.ConfigMasters(machines, clusterID.InfraID)
+		aws.ConfigMasters(machines, clusterID.InfraID, ic.Publish)
 	case gcptypes.Name:
 		mpool := defaultGCPMachinePoolPlatform()
 		mpool.Set(ic.Platform.GCP.DefaultMachinePlatform)