Add opensearch.tf as shared module and runs service in dev environment. VPC features are disabled when is_dev is True.
Details
Details
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- wyilio/terraform_opensearch
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
I'd try doing it a bit differently.
Looking at terraform docs, the aws_opensearch_domain has the access_policies field optional.
Also, there is a separate aws_opensearch_domain_policy resource which looks like a counterpart for the access_policies field.
I'd leave only the aws_opensearch_domain resource in the shared module and move all IAM stuff to remote/aws_iam.tf and create aws_opensearch_domain_policy there to connect IAM with the domain. This way you probably don't have to enable support for "sts" in localstack.
Also, I have mixed feelings about searching for VPCs using data in the shared module. Generally, this is alright but might have issues when creating a new environment from scratch (data is looking for resources that are yet going to be created by other resources). Not a big issue for us though, we're not planning to do it at all.
Since the subnets are already well-defined in the remote configuration, we can use another approach - just pass subnet_ids array as a variable to the shared module and default to an empty array. You can then pass them in the remote's main.tf.
IMO the latter approach is easier but leaving the decision here up to you.
| services/terraform/modules/shared/opensearch.tf | ||
|---|---|---|
| 21–24 | Are we going to set this tag anywhere for our subnets? Or is it automatically added? | |
| 53–65 | Is the Identity Service the only entity that accesses OpenSearch endpoints? If yes, this policy can be even stricter for staging/prod to let only specific ECS containers access it. | |
| 77–81 | We use us-east-2, but other regions might have 2 subnets. This could fail then. The suggestion should work then | |
Comment Actions
Mostly looking good, just a suggestion and question inline
| services/terraform/modules/shared/opensearch.tf | ||
|---|---|---|
| 1 ↗ | (On Diff #32950) | I'd pass the VPC id directly to avoid the data.aws_vpc.identity-search but up to you. Adding a default makes sense anyway. |
| 21 ↗ | (On Diff #32950) | What's the advantage of [count.index] over just [0]? |
Comment Actions
overall LGTM, but I have some questions...
should we add tags to the new resources? something like this for the security group:
tags = {
Name = "${var.vpc}-opensearch-${var.domain}"
Environment = var.is_dev ? "development" : "production"
}and this for the domain:
tags = {
Name = var.domain
Environment = var.is_dev ? "development" : "production"
}also do we need an output?
output "opensearch_domain_endpoint" {
value = aws_opensearch_domain.identity-search.endpoint
description = "The endpoint of the OpenSearch domain"
}also setting @bartek as blocking since he's the terraform expert
| services/terraform/modules/shared/opensearch.tf | ||
|---|---|---|
| 21 ↗ | (On Diff #32950) | i don't think there is one, but could be wrong... |