Page MenuHomePhabricator

[Docs] Revise identity deployment documentation
Changes PlannedPublic

Authored by varun on Jul 28 2023, 11:09 AM.
Tags
None
Referenced Files
F3174222: D8660.id29456.diff
Thu, Nov 7, 3:57 PM
F3169454: D8660.diff
Thu, Nov 7, 11:28 AM
Unknown Object (File)
Mon, Nov 4, 11:20 PM
Unknown Object (File)
Sun, Oct 27, 4:09 PM
Unknown Object (File)
Sep 28 2024, 1:22 AM
Unknown Object (File)
Sep 28 2024, 1:22 AM
Unknown Object (File)
Sep 28 2024, 1:22 AM
Unknown Object (File)
Sep 28 2024, 1:22 AM
Subscribers

Details

Summary

Change documentation to use environment variable for server_setup
secrets instead of a docker volume introduced in https://phab.comm.dev/D8641.
Also ensure that secrets directory is created before generating secrets to
avoid docker creating it as sudo user.

Addresses https://linear.app/comm/issue/ENG-4531

Test Plan

Run each identity section

Diff Detail

Repository
rCOMM Comm
Branch
jonringer/identity-docs-followup
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

docs/nix_services_deployment.md
23 ↗(On Diff #29191)

since I'm writing this locally as well, decided to omit the mention of the docker volume

Setting @bartek and @varun both as blocking since they had separate comments

docs/nix_services_deployment.md
12 ↗(On Diff #29191)

This isn't exactly what @varun asked for. Not sure if that was intentional or not

17 ↗(On Diff #29191)

I think we can just say "authentication of a password" instead of "authentication of a password user". We don't explain the concept of a "password user" anywhere else in the docs, nor should we need to in order to explain what OPAQUE is.

docs/nix_services_deployment.md
12 ↗(On Diff #29191)

the file option still needs a file, not a directory

21 ↗(On Diff #29191)

the keygen program creates the secrets directory i think. is there a way we can avoid calling mkdir ourselves and still avoid Docker creating it as sudo user?

edit: the documented steps actually don't even work for me. getting this error:

Creating secrets directory "secrets"
Error: Os { code: 13, kind: PermissionDenied, message: "Permission denied" }
35 ↗(On Diff #29191)

we don't need this, since we'll look for the file if the env var isn't present

varun requested changes to this revision.Jul 28 2023, 11:49 AM
This revision now requires changes to proceed.Jul 28 2023, 11:49 AM
jon marked an inline comment as done.

Address feedback

jon marked an inline comment as done.

Minor corrections

jon added inline comments.
docs/nix_services_deployment.md
17 ↗(On Diff #29191)

Rewritten

21 ↗(On Diff #29191)

do you mind doing ls -lg and see who owns the directory? If you already have a secrets directory, it could be causing issues.

I was also tempted with just saying to run cargo run -- keygen. But then I would need a step saying to download cargo.

35 ↗(On Diff #29191)

I either need to mount a secrets directory or expose the environment variable. Having neither will not work. I went with an environment variable since that is closer to what we want to use with ECS.

Docker volumes also seem more prone to UID and GID issues between the host and container.

jon marked an inline comment as done.

Other corrections

Please add me back before landing this, once you're sure it's 100%

bartek added inline comments.
docs/nix_services_deployment.md
12 ↗(On Diff #29456)

Personally, I tend to inform about the directory from which scripts should be executed, but don't feel strongly.

atul added inline comments.
docs/nix_services_deployment.md
12 ↗(On Diff #29456)

+1

varun edited reviewers, added: jon; removed: varun.
This revision is now accepted and ready to land.Aug 21 2023, 8:56 AM