Details

Reviewers

bartek
varun

Commits

rCOMM42499907ef11: [keyserver] Run webapp through comm services cluster

Summary

Run webapp on the comm services cluster. Will not land until web.comm.app has been properly set up on prod

This will require a new keyserver image push with https://phab.comm.dev/D12927

Depends on D12906

Test Plan

terraform apply. Switched comm.software to point to web app load balancer and accessed the working web app.

Used wyilio/keysever:1.0 for testing

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

will created this revision.Jul 29 2024, 8:43 PM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptJul 29 2024, 8:43 PM

will edited the summary of this revision. (Show Details)Jul 29 2024, 8:44 PM

will added a parent revision: D12927: [keyserver] Do not verify identity logged for webapp and landing.

will added inline comments.Jul 29 2024, 8:48 PM

services/terraform/remote/service_webapp.tf
19–25 ↗	(On Diff #42920)	This addresses the possibility for devs to not have the env file but detected if the file doesn't exist and creating it. This will cause the dev to have to terraform apply twice before resolving the missing .env file, but this shouldn't be too much of a problem. Tried using the sops provider but it strips `\`` from the env file so found this to be a working method. The `dotenv` also requires a `.env` file and it cannot be passed by value or sops @bartek Let me know if there's an easier way around this I'm missing

will added a child revision: D12929: [terraform] use module approach for landing.Jul 29 2024, 8:50 PM

move dotenv to env.tf

Harbormaster completed remote builds in B30777: Diff 42920.Jul 29 2024, 9:10 PM

will edited the summary of this revision. (Show Details)Jul 29 2024, 9:12 PM

will edited the test plan for this revision. (Show Details)

Harbormaster completed remote builds in B30779: Diff 42922.Jul 29 2024, 9:13 PM

will requested review of this revision.Jul 29 2024, 9:13 PM

will edited the summary of this revision. (Show Details)Jul 29 2024, 9:15 PM

will added a child revision: D12930: [terraform] deploy landing on comm services cluster.

General question:
What's the advantage of sops-encrypting new .env files instead of reusing our sops_file resource for secrets.json, having webapp-specific config inside it, and then doing

webapp_environment_vars = merge(local.secrets.webapp_secrets,  { ... })

I'm not saying your approach is bad, I just want to understand the motivation

services/terraform/remote/aws_iam.tf
74 ↗	(On Diff #42922)	Here, in `remote`, this name is way too general. We have other "ecs task roles" for specific services (`"aws_iam_role" "feature_flags_service"`, `reports_service`) and some more general like `"services_ddb_full_access"`. So leaving this could be VERY confusing

In D12928#366013, @bartek wrote:
General question:
What's the advantage of sops-encrypting new .env files instead of reusing our sops_file resource for secrets.json, having webapp-specific config inside it, and then doing
webapp_environment_vars = merge(local.secrets.webapp_secrets,  { ... })
?

I'm not saying your approach is bad, I just want to understand the motivation

Wasn't something I exactly considered. I think my main motivation was keeping it as a .env format, but let me know if you think going with the secrets.json approach would be better. It would simplify the process of the null resource creating a .env file every time.