Skip to content
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

Remove public IPs from masters #1045

Merged
merged 5 commits into from
Feb 14, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions data/data/aws/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ module "bootstrap" {
cluster_name = "${var.cluster_name}"
iam_role = "${var.aws_master_iam_role_name}"
ignition = "${var.ignition_bootstrap}"
subnet_id = "${module.vpc.master_subnet_ids[0]}"
subnet_id = "${module.vpc.public_subnet_ids[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}"
Expand Down Expand Up @@ -46,7 +46,7 @@ module "masters" {
root_volume_iops = "${var.aws_master_root_volume_iops}"
root_volume_size = "${var.aws_master_root_volume_size}"
root_volume_type = "${var.aws_master_root_volume_type}"
subnet_ids = "${module.vpc.master_subnet_ids}"
subnet_ids = "${module.vpc.private_subnet_ids}"
target_group_arns = "${module.vpc.aws_lb_target_group_arns}"
target_group_arns_length = "${module.vpc.aws_lb_target_group_arns_length}"
ec2_ami = "${var.aws_ec2_ami_override}"
Expand Down
3 changes: 1 addition & 2 deletions data/data/aws/master/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,7 @@ resource "aws_instance" "master" {
subnet_id = "${element(var.subnet_ids, count.index)}"
user_data = "${var.user_data_ign}"

vpc_security_group_ids = ["${var.master_sg_ids}"]
associate_public_ip_address = true
vpc_security_group_ids = ["${var.master_sg_ids}"]

lifecycle {
# Ignore changes in the AMI which force recreation of the resource. This
Expand Down
8 changes: 4 additions & 4 deletions data/data/aws/vpc/common.tf
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ locals {
vpc_id = "${aws_vpc.new_vpc.id}"

// When referencing the _ids arrays or data source arrays via count = , always use the *_count variable rather than taking the length of the list
worker_subnet_ids = "${aws_subnet.worker_subnet.*.id}"
master_subnet_ids = "${aws_subnet.master_subnet.*.id}"
worker_subnet_count = "${local.new_az_count}"
master_subnet_count = "${local.new_az_count}"
private_subnet_ids = "${aws_subnet.private_subnet.*.id}"
public_subnet_ids = "${aws_subnet.public_subnet.*.id}"
private_subnet_count = "${local.new_az_count}"
public_subnet_count = "${local.new_az_count}"
}

# all data sources should be input variable-agnostic and used as canonical source for querying "state of resources" and building outputs
Expand Down
4 changes: 2 additions & 2 deletions data/data/aws/vpc/master-elb.tf
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
resource "aws_lb" "api_internal" {
name = "${var.cluster_name}-int"
load_balancer_type = "network"
subnets = ["${local.master_subnet_ids}"]
subnets = ["${local.private_subnet_ids}"]
internal = true
enable_cross_zone_load_balancing = true
idle_timeout = 3600
Expand All @@ -14,7 +14,7 @@ resource "aws_lb" "api_internal" {
resource "aws_lb" "api_external" {
name = "${var.cluster_name}-ext"
load_balancer_type = "network"
subnets = ["${local.master_subnet_ids}"]
subnets = ["${local.public_subnet_ids}"]
internal = false
enable_cross_zone_load_balancing = true
idle_timeout = 3600
Expand Down
8 changes: 6 additions & 2 deletions data/data/aws/vpc/outputs.tf
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,12 @@ output "vpc_id" {
value = "${data.aws_vpc.cluster_vpc.id}"
}

output "master_subnet_ids" {
value = "${local.master_subnet_ids}"
output "public_subnet_ids" {
value = "${local.public_subnet_ids}"
}

output "private_subnet_ids" {
value = "${local.private_subnet_ids}"
}

output "etcd_sg_id" {
Expand Down
10 changes: 5 additions & 5 deletions data/data/aws/vpc/vpc-private.tf
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,23 @@ resource "aws_route" "to_nat_gw" {
depends_on = ["aws_route_table.private_routes"]
}

resource "aws_subnet" "worker_subnet" {
resource "aws_subnet" "private_subnet" {
count = "${local.new_az_count}"

vpc_id = "${data.aws_vpc.cluster_vpc.id}"

cidr_block = "${cidrsubnet(local.new_worker_cidr_range, 3, count.index)}"
cidr_block = "${cidrsubnet(local.new_private_cidr_range, 3, count.index)}"

availability_zone = "${local.new_subnet_azs[count.index]}"

tags = "${merge(map(
"Name", "${var.cluster_name}-worker-${local.new_subnet_azs[count.index]}",
"Name", "${var.cluster_name}-private-${local.new_subnet_azs[count.index]}",
"kubernetes.io/role/internal-elb", "",
), var.tags)}"
}

resource "aws_route_table_association" "worker_routing" {
resource "aws_route_table_association" "private_routing" {
count = "${local.new_az_count}"
route_table_id = "${aws_route_table.private_routes.*.id[count.index]}"
subnet_id = "${aws_subnet.worker_subnet.*.id[count.index]}"
subnet_id = "${aws_subnet.private_subnet.*.id[count.index]}"
}
10 changes: 5 additions & 5 deletions data/data/aws/vpc/vpc-public.tf
Original file line number Diff line number Diff line change
Expand Up @@ -25,23 +25,23 @@ resource "aws_route" "igw_route" {
gateway_id = "${aws_internet_gateway.igw.id}"
}

resource "aws_subnet" "master_subnet" {
resource "aws_subnet" "public_subnet" {
count = "${local.new_az_count}"
Copy link
Contributor

@staebler staebler Jan 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to create a subnet for every availability zone when we are only going to use the first one? Are we keeping the public subnets around for the life of the cluster for other purposes?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to create a subnet for every availability zone when we are only going to use the first one?

We don't want to do this, but without #792 and #1121, fixing it is difficult.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how those would be related here. The public_subnet is only used by the bootstrap machine now. It is not affected in any way by the master machines configuration.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The public_subnet is only used by the bootstrap machine now. It is not affected in any way by the master machines configuration.

Won't gateways and such also live in the public subnets? We probably want one of those in each region that has master and/or compute nodes.

Copy link
Contributor

@staebler staebler Jan 31, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. It isn't clear to me how the masters and workers would use those gateways since the machines are not part of the public subnets.

vpc_id = "${data.aws_vpc.cluster_vpc.id}"

cidr_block = "${cidrsubnet(local.new_master_cidr_range, 3, count.index)}"
cidr_block = "${cidrsubnet(local.new_public_cidr_range, 3, count.index)}"

availability_zone = "${local.new_subnet_azs[count.index]}"

tags = "${merge(map(
"Name", "${var.cluster_name}-master-${local.new_subnet_azs[count.index]}",
"Name", "${var.cluster_name}-public-${local.new_subnet_azs[count.index]}",
), var.tags)}"
}

resource "aws_route_table_association" "route_net" {
count = "${local.new_az_count}"
route_table_id = "${aws_route_table.default.id}"
subnet_id = "${aws_subnet.master_subnet.*.id[count.index]}"
subnet_id = "${aws_subnet.public_subnet.*.id[count.index]}"
}

resource "aws_eip" "nat_eip" {
Expand All @@ -59,7 +59,7 @@ resource "aws_eip" "nat_eip" {
resource "aws_nat_gateway" "nat_gw" {
count = "${local.new_az_count}"
allocation_id = "${aws_eip.nat_eip.*.id[count.index]}"
subnet_id = "${aws_subnet.master_subnet.*.id[count.index]}"
subnet_id = "${aws_subnet.public_subnet.*.id[count.index]}"

tags = "${var.tags}"
}
4 changes: 2 additions & 2 deletions data/data/aws/vpc/vpc.tf
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
locals {
new_worker_cidr_range = "${cidrsubnet(data.aws_vpc.cluster_vpc.cidr_block,1,1)}"
new_master_cidr_range = "${cidrsubnet(data.aws_vpc.cluster_vpc.cidr_block,1,0)}"
new_private_cidr_range = "${cidrsubnet(data.aws_vpc.cluster_vpc.cidr_block,1,1)}"
new_public_cidr_range = "${cidrsubnet(data.aws_vpc.cluster_vpc.cidr_block,1,0)}"
}

resource "aws_vpc" "new_vpc" {
Expand Down
5 changes: 5 additions & 0 deletions docs/user/troubleshooting.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ If no pods are shown, etcd will need to be [investigated](#etcd-is-not-running).

### Unable to SSH into Master Nodes

For added security, SSH isn't available from the Internet by default. There are several options for enabling this functionality:

- Create a bastion host that is accessible from the Internet and has access to the cluster. If the bootstrap machine hasn't been automatically destroyed yet, it can double as a temporary bastion host since it is given a public IP address.
- Configure network peering or a VPN to allow remote access to the private network.

In order to SSH into the master nodes as user `core`, it is necessary to include an administrator's SSH key during the installation.
The selected key, if any, will be added to the `core` user's `~/.ssh/authorized_keys` via [Ignition](https://github.com/coreos/ignition) and is not configured via platform-specific approaches like [AWS key pairs][aws-key-pairs].
See [here][machine-config-daemon-ssh-keys] for information about managing SSH keys via the machine-config daemon.
Expand Down
2 changes: 1 addition & 1 deletion pkg/asset/machines/aws/machines.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func provider(clusterID, clusterName string, platform *aws.Platform, mpool *aws.
Subnet: awsprovider.AWSResourceReference{
Filters: []awsprovider.Filter{{
Name: "tag:Name",
Values: []string{fmt.Sprintf("%s-%s-%s", clusterName, role, az)},
Values: []string{fmt.Sprintf("%s-private-%s", clusterName, az)},
}},
},
Placement: awsprovider.Placement{Region: platform.Region, AvailabilityZone: az},
Expand Down