search

multinet-app / multinet-deploy

The ansible deployment tasks for multinet.app
0 stars 0 forks source link

Add Sentry DSN env var injection #42

Closed jjnesbitt closed 4 years ago

jjnesbitt commented 4 years ago

Prerequisite to multinet-app/multinet#284.

jjnesbitt commented 4 years ago

Looking at the docs page for lineinfile, it seems like you should also set the regexp parameter to prevent this line from being appended to the file on each deployment. (For example, see the very first example; that looks like what we want for this application.)

Could you look into that and let me know what you think?

I see what you mean, I can add that. Currently in the corresponding app PR, we set the SENTRY_DSN var to empty. Should we keep it that way? It's actually not an issue once we add the regexp parameter, but just thought I'd bring it up.

waxlamp commented 4 years ago

Looking at the docs page for lineinfile, it seems like you should also set the regexp parameter to prevent this line from being appended to the file on each deployment. (For example, see the very first example; that looks like what we want for this application.) Could you look into that and let me know what you think?

I see what you mean, I can add that. Currently in the corresponding app PR, we set the SENTRY_DSN var to empty. Should we keep it that way? It's actually not an issue once we add the regexp parameter, but just thought I'd bring it up.

I think it's ok to keep it that way. As you said, the regex ought to take care of it properly.

In general my personal feeling is that we could have just kept all of this out of the .env file altogether (using ansible variable injection, etc., instead), but it's no big deal. (In my view, the .env file is really for creating a dev environment rather than a deployment environment; if that starts to cause any weird problems we can look at refactoring dev and deploy stuff into separate channels within the codebase.) Securitywise, I believe these things are roughly the same (an attacker would either need to compromise our AWS setup or our GitHub account to learn the DSN, and these things seem roughly the same difficulty).

JackWilb commented 4 years ago

Test locally with vagrant and can confirm it seems to work. I had to modify the playbook and the tasks slightly to get it to run though