From 5b217f4a3c20d63549595b3c9aa1ed13938c01f4 Mon Sep 17 00:00:00 2001 From: Erin-Boehmer Date: Tue, 20 Sep 2022 12:09:50 -0400 Subject: [PATCH 1/3] Make api gateway optional and adjust api_base_auth var naming Co-authored-by: Hugues Alary --- README.md | 3 +- examples/eks_airflow/metaflow.tf | 3 +- main.tf | 3 +- modules/metadata-service/README.md | 3 +- modules/metadata-service/api-gateway.tf | 99 ++++++++++++++----------- modules/metadata-service/outputs.tf | 4 +- modules/metadata-service/variables.tf | 18 +++-- outputs.tf | 2 +- variables.tf | 18 +++-- 9 files changed, 91 insertions(+), 62 deletions(-) diff --git a/README.md b/README.md index 35eef1c..afb7f4c 100644 --- a/README.md +++ b/README.md @@ -96,7 +96,6 @@ You can find a more complete example that uses this module but also includes set | Name | Description | Type | Default | Required | |------|-------------|------|---------|:--------:| | [access\_list\_cidr\_blocks](#input\_access\_list\_cidr\_blocks) | List of CIDRs we want to grant access to our Metaflow Metadata Service. Usually this is our VPN's CIDR blocks. | `list(string)` | `[]` | no | -| [api\_basic\_auth](#input\_api\_basic\_auth) | Enable basic auth for API Gateway? (requires key export) | `bool` | `true` | no | | [batch\_type](#input\_batch\_type) | AWS Batch Compute Type ('ec2', 'fargate') | `string` | `"ec2"` | no | | [compute\_environment\_desired\_vcpus](#input\_compute\_environment\_desired\_vcpus) | Desired Starting VCPUs for Batch Compute Environment [0-16] for EC2 Batch Compute Environment (ignored for Fargate) | `number` | `8` | no | | [compute\_environment\_egress\_cidr\_blocks](#input\_compute\_environment\_egress\_cidr\_blocks) | CIDR blocks to which egress is allowed from the Batch Compute environment's security group | `list(string)` | 
[
  "0.0.0.0/0"
]
| no | @@ -112,6 +111,8 @@ You can find a more complete example that uses this module but also includes set | [launch\_template\_http\_put\_response\_hop\_limit](#input\_launch\_template\_http\_put\_response\_hop\_limit) | The desired HTTP PUT response hop limit for instance metadata requests. Can be an integer from 1 to 64 | `number` | `2` | no | | [launch\_template\_http\_tokens](#input\_launch\_template\_http\_tokens) | Whether or not the metadata service requires session tokens, also referred to as Instance Metadata Service Version 2 (IMDSv2). Can be 'optional' or 'required' | `string` | `"optional"` | no | | [metadata\_service\_container\_image](#input\_metadata\_service\_container\_image) | Container image for metadata service | `string` | `""` | no | +| [metadata\_service\_enable\_api\_basic\_auth](#input\_metadata\_service\_enable\_api\_basic\_auth) | Enable basic auth for API Gateway? (requires key export) | `bool` | `true` | no | +| [metadata\_service\_enable\_api\_gateway](#input\_metadata\_service\_enable\_api\_gateway) | Enable API Gateway for public metadata service endpoint | `bool` | `true` | no | | [resource\_prefix](#input\_resource\_prefix) | string prefix for all resources | `string` | `"metaflow"` | no | | [resource\_suffix](#input\_resource\_suffix) | string suffix for all resources | `string` | `""` | no | | [subnet1\_id](#input\_subnet1\_id) | First subnet used for availability zone redundancy | `string` | n/a | yes | diff --git a/examples/eks_airflow/metaflow.tf b/examples/eks_airflow/metaflow.tf index 80938a9..07020c9 100644 --- a/examples/eks_airflow/metaflow.tf +++ b/examples/eks_airflow/metaflow.tf @@ -48,11 +48,12 @@ module "metaflow-metadata-service" { resource_suffix = local.resource_suffix access_list_cidr_blocks = [] - api_basic_auth = true database_name = module.metaflow-datastore.database_name database_password = module.metaflow-datastore.database_password database_username = module.metaflow-datastore.database_username datastore_s3_bucket_kms_key_arn = module.metaflow-datastore.datastore_s3_bucket_kms_key_arn + enable_api_basic_auth = true + enable_api_gateway = true fargate_execution_role_arn = aws_iam_role.ecs_execution_role.arn metaflow_vpc_id = module.vpc.vpc_id metadata_service_container_image = module.metaflow-common.default_metadata_service_container_image diff --git a/main.tf b/main.tf index 918ab59..23b6029 100644 --- a/main.tf +++ b/main.tf @@ -19,11 +19,12 @@ module "metaflow-metadata-service" { resource_suffix = local.resource_suffix access_list_cidr_blocks = var.access_list_cidr_blocks - api_basic_auth = var.api_basic_auth database_name = module.metaflow-datastore.database_name database_password = module.metaflow-datastore.database_password database_username = module.metaflow-datastore.database_username datastore_s3_bucket_kms_key_arn = module.metaflow-datastore.datastore_s3_bucket_kms_key_arn + enable_api_basic_auth = var.metadata_service_enable_api_basic_auth + enable_api_gateway = var.metadata_service_enable_api_gateway fargate_execution_role_arn = module.metaflow-computation.ecs_execution_role_arn iam_partition = var.iam_partition metadata_service_container_image = local.metadata_service_container_image diff --git a/modules/metadata-service/README.md b/modules/metadata-service/README.md index 862123c..8f68903 100644 --- a/modules/metadata-service/README.md +++ b/modules/metadata-service/README.md @@ -16,11 +16,12 @@ If the `access_list_cidr_blocks` variable is set, only traffic originating from | Name | Description | Type | Default | Required | |------|-------------|------|---------|:--------:| | [access\_list\_cidr\_blocks](#input\_access\_list\_cidr\_blocks) | List of CIDRs we want to grant access to our Metaflow Metadata Service. Usually this is our VPN's CIDR blocks. | `list(string)` | n/a | yes | -| [api\_basic\_auth](#input\_api\_basic\_auth) | Enable basic auth for API Gateway? (requires key export) | `bool` | `true` | no | | [database\_name](#input\_database\_name) | The database name | `string` | `"metaflow"` | no | | [database\_password](#input\_database\_password) | The database password | `string` | n/a | yes | | [database\_username](#input\_database\_username) | The database username | `string` | n/a | yes | | [datastore\_s3\_bucket\_kms\_key\_arn](#input\_datastore\_s3\_bucket\_kms\_key\_arn) | The ARN of the KMS key used to encrypt the Metaflow datastore S3 bucket | `string` | n/a | yes | +| [enable\_api\_basic\_auth](#input\_enable\_api\_basic\_auth) | Enable basic auth for API Gateway? (requires key export) | `bool` | `true` | no | +| [enable\_api\_gateway](#input\_enable\_api\_gateway) | Enable API Gateway for public metadata service endpoint | `bool` | `true` | no | | [fargate\_execution\_role\_arn](#input\_fargate\_execution\_role\_arn) | The IAM role that grants access to ECS and Batch services which we'll use as our Metadata Service API's execution\_role for our Fargate instance | `string` | n/a | yes | | [iam\_partition](#input\_iam\_partition) | IAM Partition (Select aws-us-gov for AWS GovCloud, otherwise leave as is) | `string` | `"aws"` | no | | [is\_gov](#input\_is\_gov) | Set to true if IAM partition is 'aws-us-gov' | `bool` | `false` | no | diff --git a/modules/metadata-service/api-gateway.tf b/modules/metadata-service/api-gateway.tf index e816ed2..fdf07fb 100644 --- a/modules/metadata-service/api-gateway.tf +++ b/modules/metadata-service/api-gateway.tf @@ -1,6 +1,6 @@ resource "aws_api_gateway_rest_api_policy" "this" { - count = length(var.access_list_cidr_blocks) > 0 ? 1 : 0 - rest_api_id = aws_api_gateway_rest_api.this.id + count = var.enable_api_gateway && length(var.access_list_cidr_blocks) > 0 ? 1 : 0 + rest_api_id = aws_api_gateway_rest_api.this[0].id policy = < Date: Tue, 20 Sep 2022 12:20:46 -0400 Subject: [PATCH 2/3] Format api-gateway.tf file --- modules/metadata-service/api-gateway.tf | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/metadata-service/api-gateway.tf b/modules/metadata-service/api-gateway.tf index fdf07fb..f01cee9 100644 --- a/modules/metadata-service/api-gateway.tf +++ b/modules/metadata-service/api-gateway.tf @@ -62,7 +62,7 @@ resource "aws_api_gateway_vpc_link" "this" { } resource "aws_api_gateway_method" "this" { - count = var.enable_api_gateway ? 1 : 0 + count = var.enable_api_gateway ? 1 : 0 http_method = "ANY" resource_id = aws_api_gateway_resource.this[0].id rest_api_id = aws_api_gateway_rest_api.this[0].id @@ -75,7 +75,7 @@ resource "aws_api_gateway_method" "this" { } resource "aws_api_gateway_method" "db" { - count = var.enable_api_gateway ? 1 : 0 + count = var.enable_api_gateway ? 1 : 0 http_method = "GET" resource_id = aws_api_gateway_resource.db[0].id rest_api_id = aws_api_gateway_rest_api.this[0].id @@ -159,7 +159,7 @@ resource "aws_api_gateway_deployment" "this" { } resource "aws_api_gateway_stage" "this" { - count = var.enable_api_gateway ? 1 : 0 + count = var.enable_api_gateway ? 1 : 0 deployment_id = aws_api_gateway_deployment.this[0].id rest_api_id = aws_api_gateway_rest_api.this[0].id stage_name = local.api_gateway_stage_name From f1afe74b0078c1f680b75163fb36f1efaa91cfab Mon Sep 17 00:00:00 2001 From: Erin-Boehmer Date: Tue, 20 Sep 2022 13:52:12 -0400 Subject: [PATCH 3/3] Use metadata_service_enable_api_basic_auth var name from root of module --- outputs.tf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/outputs.tf b/outputs.tf index 810e470..9b3b339 100644 --- a/outputs.tf +++ b/outputs.tf @@ -74,7 +74,7 @@ output "metaflow_profile_json" { "METAFLOW_BATCH_CONTAINER_REGISTRY" = element(split("/", aws_ecr_repository.metaflow_batch_image[0].repository_url), 0), "METAFLOW_BATCH_CONTAINER_IMAGE" = element(split("/", aws_ecr_repository.metaflow_batch_image[0].repository_url), 1) } : {}, - var.enable_api_basic_auth ? { + var.metadata_service_enable_api_basic_auth ? { "METAFLOW_SERVICE_AUTH_KEY" = "## Replace with output from 'aws apigateway get-api-key --api-key ${module.metaflow-metadata-service.api_gateway_rest_api_id_key_id} --include-value | grep value' ##" } : {}, var.batch_type == "fargate" ? {