Page MenuHomePhabricator

[keyserver] Run webapp through comm services cluster
ClosedPublic

Authored by will on Jul 29 2024, 8:43 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Oct 19, 8:24 AM
Unknown Object (File)
Fri, Oct 18, 9:45 PM
Unknown Object (File)
Fri, Oct 18, 9:45 PM
Unknown Object (File)
Fri, Oct 18, 9:45 PM
Unknown Object (File)
Fri, Oct 18, 9:45 PM
Unknown Object (File)
Fri, Oct 18, 9:45 PM
Unknown Object (File)
Fri, Oct 18, 9:45 PM
Unknown Object (File)
Fri, Oct 18, 9:45 PM
Subscribers

Details

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

services/terraform/remote/service_webapp.tf
19–25

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 edited the summary of this revision. (Show Details)

move dotenv to env.tf

will edited the test plan for this revision. (Show Details)
will requested review of this revision.Jul 29 2024, 9:13 PM

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

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.

services/terraform/remote/aws_iam.tf
74 ↗(On Diff #42922)

Got it. I'll rename it on next rebase

Planning to change .env to secrets.json approach

review feedback. use secrets.json approach instead of .env

will planned changes to this revision.Aug 1 2024, 11:18 AM
varun added 1 blocking reviewer(s): bartek.

setting @bartek as blocking since he left the original feedback

services/terraform/remote/secrets.json
28–40 ↗(On Diff #43027)

These are included later in the json file. I'll remove before landing

bartek added inline comments.
services/terraform/remote/secrets.json
91 ↗(On Diff #43027)

Was the KMS key somehow auto-regenerated? I guess you didn't modify anything since the arn remains unchanged

This revision is now accepted and ready to land.Aug 2 2024, 10:35 AM
services/terraform/remote/secrets.json
91 ↗(On Diff #43027)

Did not modify the KMS key