Skip to content

Conversation

ukitazume
Copy link
Contributor

@ukitazume ukitazume commented Jan 4, 2022

Nat Gateway is not available in ap-northeast-1a, so I exclude the zone for the list.

the data source of availability_zones has no fit filter to exclude Nat gateway unavailable zone, so I specify it by zone_id.
https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/availability_zones

Without this commit, it runs the below error on ap-northeast-1.

module.secure-for-cloud_example_single-account.module.resource_group.aws_resourcegroups_group.sysdig_secure_for_cloud: Creation complete after 1s [id=sfc]               ╷
│ Error: Error creating NAT Gateway: NotAvailableInZone: Nat Gateway is not available in this availability zone
│       status code: 400, request id: 83a3257b-edf2-413e-941a-60a3f3546dea
│
│   with module.secure-for-cloud_example_single-account.module.ecs_fargate_cluster.module.vpc.aws_nat_gateway.this[0],
│   on .terraform/modules/secure-for-cloud_example_single-account.ecs_fargate_cluster.vpc/main.tf line 1096, in resource "aws_nat_gateway" "this":
│ 1096: resource "aws_nat_gateway" "this" {

@wideawakening
Copy link
Contributor

what a tricky thing indeed :/

checked TF resources and although there is an state:available property in the aws_availability_zones datasource guess its something generic not taking into consideration nats support.

mainly to avoid maintaining a list of explicit zones, and as it may also reduce some costs, letme give it a try to the "Single NAT Gateway" scenario and see if it may serve as a workaround
https://registry.terraform.io/modules/terraform-aws-modules/vpc/aws/latest#nat-gateway-scenarios

@ukitazume
Copy link
Contributor Author

ukitazume commented Jan 4, 2022

it looks so better than listed up by ourselves. thanks. I will wait for your fix.

@ukitazume ukitazume marked this pull request as draft January 4, 2022 23:02
@ukitazume ukitazume closed this Jan 5, 2022
@ukitazume
Copy link
Contributor Author

@wideawakening
I think the single NATs gateways is not good for az redundant.

I checked the VPC terraform module code. It accepts the AZ via a variable.
the module https://github.com/terraform-aws-modules/terraform-aws-vpc/blob/master/variables.tf#L265-L269

I think we accept the az like the VPC module.

@wideawakening
Copy link
Contributor

I think we accept the az like the VPC module.

that's another valid solution indeed, can expose azs as var on our internal module/infrastructure/ecs-fargate-cluster and let user provide it. as a fallback we will fetch all zones, as current setup. would that make sense for you?

I think the single NATs gateways is not good for az redundant.

regarding the VPC configuration, nailing it down to customer config is gonna be a tricky thing to shape-up, let alone a production network configuration.

currently it's just a quick setup to get this up running for a trial test, but don't think it suits a production environment setup.
the output of the ecs-fargate-cluster is required for the cloud-connector ECS task definition as per

  • vpc_id for the security group ingress/egress configuration (which currently allows all connections)
  • service network_configuration

as for your use case, would it make sense to let user input those two variables as a pre-existing VPC?
we could make the usage of the ecs-fargate-cluster optional and let user provide those two variables given a create_vpc:false configuration.

let us know your thoughts about those two options

@wideawakening wideawakening reopened this Jan 5, 2022
@ukitazume
Copy link
Contributor Author

Yes, that makes sense.

I'd like erraform-aws-secure-for-cloud/examples/* to create the VPC and subnets but now it fails on ap-northeast-1 due to no NATS-gateway availability on ap-norhteast-1a.
The terraform resource of az has no options and filters to exlucde no NATS available zones. So, I'd like to specify AZs they use. It's not nice for the first use, but it provides a workaround and we can document it.

As the options,
I'd like to omit to prepare a VPC for sfc when connecting Sysdig and AWS and want this module cover that because it could be isolated from another AWS resources on the same account.

I think it should be production ready as redundant and security by default and would have some options for reducing costs and customization.

@wideawakening
Copy link
Contributor

closing this PR in favor of #53 to handle testing gh actions (secrets are not shared when merging forks)

  • tried the problematic region but was not able to get get the problematic zone_id "apne1-az3" :/
provider "aws" {
  region = "ap-northeast-1"
}

resulted in the datasource

$ terraform state show data.aws_availability_zones.zones 

# data.aws_availability_zones.zones:
data "aws_availability_zones" "zones" {
    group_names = [
        "ap-northeast-1",
    ]
    id          = "ap-northeast-1"
    names       = [
        "ap-northeast-1a",
        "ap-northeast-1c",
        "ap-northeast-1d",
    ]
    zone_ids    = [
        "apne1-az4",
        "apne1-az1",
        "apne1-az2",
    ]
}

  • anyhow, exposed the ecs vpc region azs as variable (with the default as fallback) and exposed it on the examples too.
    you should be able to take advantage of the ecs_vpc_region_azs

  • with the current variable setup I did enforce it

export TF_VAR_vpc_region_azs=[\"apne1-az1\",\"apne1-az3\"]

and got the desired error

│ Error: error creating subnet: InvalidParameterValue: Value (apne1-az3) for parameter availabilityZoneId is invalid. Subnets can currently only be created in the following availability zones: apne1-az1, apne1-az2, apne1-az4.
│ 	status code: 400, request id: 6e32d757-2e61-4220-8106-22ccf814e1fe
│ 
│   with module.vpc.aws_subnet.public[1],
│   on .terraform/modules/vpc/main.tf line 376, in resource "aws_subnet" "public":
│  376: resource "aws_subnet" "public" {

  • left nat gateway scenario untouched to current One NAT Gateway per subnet (default behavior).

  • added troubleshooting for this too on the main README

hope that serves @ukitazume

@ukitazume
Copy link
Contributor Author

Hi @wideawakening ,
sorry for checking your improvement.
I tried to use v0.5.0 and it works with the new variable ecs_vpc_region_azs. thanks a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants