[terraform] use `.env` in self-host folder for keyserver config variables instead of specifying values in terraform.tfvars or terraform.tfvars.json
ClosedPublic
Actions

Authored by will on Jul 18 2024, 3:21 PM.

Details

Reviewers

varun
ashoat
bartek

Commits

rCOMMd39ad4ec4ac3: [terraform] use `.env` in self-host folder for keyserver config variables…

Summary

This addresses https://linear.app/comm/issue/ENG-8844/make-terraform-config-add-task-def-variables-through-a-env-file

We use a local .env file (functionally identical to one we would use in comm/keyserver/.env) to include the env variable for the primary/secondar node ecs tasks

Originally, we were using terraform.tfvars to specify the values for these environmental variables and hardcoding a set list of env variables that would be included in our primary/secondary ecs tasks.

The new method allows the user to specify any env variable which will then appear in the ecs task by modifying the .env file in comm/services/terraform/self-host/.env

Test Plan

terraform apply. Configuration is working and successfully populates the task def. New tasks launch and run successfully without error from startup.
If tasks were unable to read these environmental variables (i.e. COMM_JSONCONFIG_secrets_user_credentials), the tasks would fail.

Depends on D12731

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

will created this revision.Jul 18 2024, 3:21 PM

Herald added a subscriber: tomek. · View Herald TranscriptJul 18 2024, 3:21 PM

will added inline comments.Jul 18 2024, 3:35 PM

services/terraform/self-host/aws_db.tf
2 ↗	(On Diff #42506)	See `services/terraform/self-host/env.tf` for what this is
11 ↗	(On Diff #42506)	will change to user specified port before landing
16 ↗	(On Diff #42506)	We now specify this through the `COMM_DATABASE_PORT` env variable. In `env.tf`, I remove the risk of the user not specifying this and having it default to 3306 by including it in default_environment_vars = { "COMM_DATABASE_PORT" = "3307" } which is automatically merged into a shared map of variables
services/terraform/self-host/env.tf
4–6 ↗	(On Diff #42506)	Default vars if user not specified. Did database port to 3307 because it defaults to 3306 on keyserver if not specified and we want to enable TLS with 3307
7–21 ↗	(On Diff #42506)	These were originally all just one large merged map called shared_environment_vars. However, on terraform apply, realized that because the aws rds resource is dependent on `shared_environment_vars` for the `COMM_DATABASE_PORT` and shared_environment_vars` is dependent on the aws rds resource for the value for `COMM_DATABASE_HOST`, we had a cycle. Resolved this by separating and using `local_with_default_environment_vars` for aws rds configuration
services/terraform/self-host/keyserver_primary.tf
9 ↗	(On Diff #42506)	Removing this before landing. Already specified in shared

will retitled this revision from [terraform] use `.env` in self-host folder for keyserver config variables instead of specifying terraform.tfvars or terraform.tfvars.json to [terraform] use `.env` in self-host folder for keyserver config variables instead of specifying values in terraform.tfvars or terraform.tfvars.json.Jul 18 2024, 3:36 PM

Harbormaster completed remote builds in B30498: Diff 42506.Jul 18 2024, 3:38 PM

will requested review of this revision.Jul 18 2024, 3:38 PM

will edited the summary of this revision. (Show Details)Jul 18 2024, 3:38 PM

varun accepted this revision.Jul 18 2024, 4:06 PM

varun added inline comments.

services/terraform/self-host/keyserver_primary.tf
25 ↗	(On Diff #42506)	do we use this anywhere? should we be using this?

This revision is now accepted and ready to land.Jul 18 2024, 4:06 PM

Great work on this – looks really elegant!

Where did we land the MariaDB params – can those be specified only in the .env file, or will they need to be additionally specified to Terraform somehow?

In D12800#362553, @ashoat wrote:

Great work on this – looks really elegant!

Where did we land the MariaDB params – can those be specified only in the .env file, or will they need to be additionally specified to Terraform somehow?

With this diff, the MariaDB params can now only be specified with the .env file.

Here's how we grab them:

locals {
  mariadb_database_name = local.local_with_default_environment_vars.COMM_DATABASE_DATABASE
  mariadb_username      = local.local_with_default_environment_vars.COMM_DATABASE_USER
  mariadb_password      = local.local_with_default_environment_vars.COMM_DATABASE_PASSWORD
  mariadb_port          = jsondecode(local.local_with_default_environment_vars.COMM_DATABASE_PORT)
}

Lovely! Forgot that we don't use COMM_JSONCONFIG for the MariaDB config in the Docker configuration. I think we initially made this decision because it made it easier to access them from our Docker Compose config. Seems like there's a similar benefit for making it easy to access from the Terraform config.