From 4a0967200a6bfea056ed7a635f560dee0d6ecf99 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Tue, 27 Nov 2018 22:20:14 -0800 Subject: [PATCH] data/aws/vpc: Add an S3 endpoint to new VPCs As suggested by Stephen Cuppett, this allows registry <-> S3 transfers to bypass the (NAT) gateways. Traffic over the NAT gateways costs money, so the new endpoint should make S3 access from the cluster cheaper (and possibly more reliable). This also allows for additional security policy flexibility, although I'm not taking advantage of that in this commit. Docs for VPC endpoints are in [1,2,3,4]. Endpoints do not currently support cross-region requests [1]. And based on discussion with Stephen, adding an endpoint may *break* access to S3 on other regions. But I can't find docs to back that up, and [3] has: We use the most specific route that matches the traffic to determine how to route the traffic (longest prefix match). If you have an existing route in your route table for all internet traffic (0.0.0.0/0) that points to an internet gateway, the endpoint route takes precedence for all traffic destined for the service, because the IP address range for the service is more specific than 0.0.0.0/0. All other internet traffic goes to your internet gateway, including traffic that's destined for the service in other regions. which suggests that access to S3 on other regions may be unaffected. In any case, our registry buckets, and likely any other buckets associated with the cluster, will be living in the same region. concat is documented in [5]. The wrapping brackets avoid [6]: level=error msg="Error: module.vpc.aws_vpc_endpoint.s3: route_table_ids: should be a list" although I think that's a Terraform bug. See also 8a37f72b (modules/aws/bootstrap: Pull AWS bootstrap setup into a module, 2018-09-05, #217), which talks about this same issue. [1]: https://docs.aws.amazon.com/vpc/latest/userguide/vpc-endpoints-s3.html [2]: https://docs.aws.amazon.com/vpc/latest/userguide/vpc-endpoints.html [3]: https://docs.aws.amazon.com/vpc/latest/userguide/vpce-gateway.html [4]: https://www.terraform.io/docs/providers/aws/r/vpc_endpoint.html [5]: https://www.terraform.io/docs/configuration/interpolation.html#concat-list1-list2- [6]: https://storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_installer/745/pull-ci-openshift-installer-master-e2e-aws/1673/build-log.txt --- data/data/aws/main.tf | 1 + data/data/aws/vpc/variables.tf | 5 +++++ data/data/aws/vpc/vpc.tf | 6 ++++++ 3 files changed, 12 insertions(+) diff --git a/data/data/aws/main.tf b/data/data/aws/main.tf index 428ad87a751..59ce2ea3f0f 100644 --- a/data/data/aws/main.tf +++ b/data/data/aws/main.tf @@ -89,6 +89,7 @@ module "vpc" { cluster_id = "${var.cluster_id}" cluster_name = "${var.cluster_name}" external_vpc_id = "${var.aws_external_vpc_id}" + region = "${var.aws_region}" external_master_subnet_ids = "${compact(var.aws_external_master_subnet_ids)}" external_worker_subnet_ids = "${compact(var.aws_external_worker_subnet_ids)}" diff --git a/data/data/aws/vpc/variables.tf b/data/data/aws/vpc/variables.tf index 73b39e627ad..cfce7db467a 100644 --- a/data/data/aws/vpc/variables.tf +++ b/data/data/aws/vpc/variables.tf @@ -51,3 +51,8 @@ variable "public_master_endpoints" { description = "If set to true, public-facing ingress resources are created." default = true } + +variable "region" { + type = "string" + description = "The target AWS region for the cluster." +} diff --git a/data/data/aws/vpc/vpc.tf b/data/data/aws/vpc/vpc.tf index 5ef3c9402ee..4b30178efe5 100644 --- a/data/data/aws/vpc/vpc.tf +++ b/data/data/aws/vpc/vpc.tf @@ -16,3 +16,9 @@ resource "aws_vpc" "new_vpc" { "openshiftClusterID", "${var.cluster_id}" ), var.extra_tags)}" } + +resource "aws_vpc_endpoint" "s3" { + vpc_id = "${aws_vpc.new_vpc.id}" + service_name = "com.amazonaws.${var.region}.s3" + route_table_ids = ["${concat(aws_route_table.private_routes.*.id, aws_route_table.default.*.id)}"] +}