295 - Clean up docker-compose.env

nozomione commented 9 months ago

Issue Number

Closing: #295

Purpose/Implementation Notes

Added the following env variables to docker-componse.env and adjusted the codebase accordingly:

Name	Value	Note
`STAGE_SENTRY_DSN`	`''`	Always an empty string
`STAGE_SENTRY_ENV`	`local-refinebio-client`	constant unless changed
`STAGE_API_HOST`	`STAGE_API_HOST`	`staging` API host name
`STAGE_API_VERSION`	STAGE_API_VERSION	current `staging` API version

Types of changes

What types of changes does your code introduce?

Breaking change (fix or feature that would cause existing functionality to not work as expected)

Functional tests

Ran all the scripts locally and successfully started/removed the containers for client and storybook.

Checklist

[x] Lint and unit tests pass locally with my changes
[ ] I have added tests that prove my fix is effective or that my feature works
[ ] I have added necessary documentation (if appropriate)
[ ] Any dependent changes have been merged and published in downstream modules

vercel[bot] commented 9 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
refinebio-web	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Aug 8, 2024 4:24am

davidsmejia commented 9 months ago

Hey, so we actually want to keep the docker-compose.yml file. We just want it to contain default values where appropriate and remove it from being tracked from source control.

The docker-compose.yml file should look like this:

# Environment variables for docker-compose.yml
CLIENT_PORT=3002
STORYBOOK_PORT=6006
STAGE_SENTRY_DSN=''
STAGE_SENTRY_ENV=local-refinebio-client

Then we add it to .gitignore and we can populate it with sensitive values and they wont get tracked in version control.

Let me know if you have any questions.

nozomione commented 9 months ago

Hey, so we actually want to keep the docker-compose.yml file. We just want it to contain default values where appropriate and remove it from being tracked from source control.

The docker-compose.yml file should look like this:
# Environment variables for docker-compose.yml
CLIENT_PORT=3002
STORYBOOK_PORT=6006
STAGE_SENTRY_DSN=''
STAGE_SENTRY_ENV=local-refinebio-client
Then we add it to .gitignore and we can populate it with sensitive values and they wont get tracked in version control.

Let me know if you have any questions.

I see. The docker-componse.env file is required to compose Docker containers in all environments (dev in localhost, staging and production in Vercel preview etc). So if we are going to add it to .gitignore, we'll probably need some additional setup (the scpca-portal repo is committing it). Let me know how you want to go about it.

davidsmejia commented 9 months ago

I see. The "docker-componse.env" file is required to compose Docker containers in all environments (dev in localhost, staging and production in Vercel preview etc). So if we are going to add it to .gitignore, we'll probably need some additional setup (the scpca-portal repo is committing it). Let me know how you want to go about it.

I don't think this will be a problem as we aren't using docker outside of local development.

Any env var in a deployed environment would be populated by vercel. It passes the env vars that are set in the admin panel to our build step which then get passed into our application context via next.config.js.

We should be good to make this change.

nozomione commented 9 months ago

I see. The "docker-componse.env" file is required to compose Docker containers in all environments (dev in localhost, staging and production in Vercel preview etc). So if we are going to add it to .gitignore, we'll probably need some additional setup (the scpca-portal repo is committing it). Let me know how you want to go about it.

I don't think this will be a problem as we aren't using docker outside of local development.

Any env var in a deployed environment would be populated by vercel. It passes the env vars that are set in the admin panel to our build step which then get passed into our application context via next.config.js.

We should be good to make this change.

What if other developers want to run dev (locahost) for contributions? For example, in thescpca-portal repo, the docker-componse.env (includes the ports information and sentry) is currently committed, so that could be easily achieved and any errors can be reported to us promptly to monitor them.

davidsmejia commented 9 months ago

What if other developers want to run dev (locahost) for contributions? For example, in thescpca-portal repo, the docker-componse.env (includes the ports information and sentry) is currently committed, so that could be easily achieved and any errors can be reported to us promptly.

Yeah I will be removing that from there as well.

Some points here:

We dont really want to collect any contributors errors in sentry as that information isn't actionable.
We really only care about errors in prod / staging but we need local support to know everything is wired up correctly prior to deployments.
Sentry not being configured correctly doesn't break the site. External contributors ought to be able to clone the repo and run the project just fine.
If they want to collect their own errors, they can populate the empty SENTRY_DSN env var with their own DSN.
Any active SENTRY_DSN should be kept in a secret / env vars to help mitigate spam. Though this hasn't been an issue to date.

davidsmejia commented 9 months ago

Just to clarify, the file will be committed but untracked which means when you populate values and do git diff in the project root and docker-compose.env wont show up. But the file will exist and ready for use with docker compose.

nozomione commented 9 months ago

In terms of security, since we are using the refinebio API, I recommended to monitor the error activity on the local dev so that we'll be able to properly identity or prevent any risks or faults.

Just to clarify, the file will be committed but untracked which means when you populate values and do git diff in the project root and docker-compose.env wont show up. But the file will exist and ready for use with docker compose.

As for the untracked files, I was talking about a cloned/forked repository codebase for starting docker containers (with the required ports). So, in this case, we'll need to update the README and add instructions for port setup for contributors.

davidsmejia commented 9 months ago

In terms of security, since we are using the refinebio API, I recommended to monitor the error activity on the local dev so that we'll be able to properly identity or prevent any risks or faults.

Please create an issue on refinebio-admin if you would like to provide more detail about how this would work or benefit us.

As for the untracked files, I was talking about a cloned/forked repository codebase for starting docker containers (with the required ports). So, in this case, we'll need to update the README and add instructions for port setup for contributors.

Please see point number 3 in my earlier comment about this.

Yeah I agree we can add a blurb to the README about populating sentry credentials.

Just to reiterate and clarify, before docker-compose.env is untracked (not deleted or removed from the project). It should contain all of the env var names with values, sensitive information should be omitted and replaced with just an empty string. Specifically, in this PR it should just be SENTRY_DSN that has it's value replace with an empty string.

nozomione commented 9 months ago

Based on slack chat w/ @davidsmejia, the following changes will be made and this workflow should be documented somewhere (e.g. the egg repo):

Regarding Sentry:

No longer reporting dev (localhost) errors to Sentry (only staging and prod)
- Except, if prefer to track errors in dev for testing etc:
- Internal developers: they can add the sentry DSN value (stored in our password manager) to the docker-compose.env file to their local repository (*but it should never be committed)
- External contributors: they can use their own Sentry DSN (reference the # 4 in comment)

Regarding docker-componse.env:

Adding this file to .gitignore (ref #comment) so that it won't be tracked
The already committed docker-componse.env in the dev branch will be treated as a template going forward
- Except, if we need to update this file, do the following steps:
  1. remove it from .gitignore to track it in a local repository
  2. update it and commit that change to the remote repository (dev)
  3. in a local repository, add it back again to .gitignore to untrack and commit the .gitignore to the remote repository (dev)

The docker-compose.env file:

# Environment variables for docker-compose.yml
CLIENT_PORT=3002 // constant unless changed
STORYBOOK_PORT=6006 // constant unless changed
STAGE_SENTRY_DSN='' // always an empty string
STAGE_SENTRY_ENV=local-refinebio-client // constant unless changed

nozomione commented 7 months ago

@davidsmejia I applied the feedback and updated docker-componse.env with new required values (e.g., removed the Sentry DNS)(d910e5619c5e3aea23c8db025247f840b4b74db3).

This file needs to be commuted to dev before adding it to .gitignore in order to use it as a template file (https://github.com/AlexsLemonade/refinebio-web/pull/296#issuecomment-1821804612), since we need this file to start the project for development.

Please take a look.

davidsmejia commented 7 months ago

This looks good, the only thing missing would be the STAGE_API_HOST and STAGE_API_VERSION which should just point to staging and be v1.

nozomione commented 7 months ago

the STAGE_API_HOST and STAGE_API_VERSION which should just point to staging and be v1.

@davidsmejia I've added those env vars and adjusted the fetchAsync helper. Also updated the implementation section of this PR and it's ready for another look!

🗒️ refinebio staging API is outdated at the moment, so STAGE_API_HOST is temporarily pointing to prod for development purpose until it's updated (the same applies to env value for Vercel for staging).

davidsmejia commented 1 month ago

If you have had this repo cloned before docker-compose.env was ignored you can run: git update-index --skip-worktree docker-compose.env to have git skip tracking changes to this file. if you need to undo that you can use the same command but with --no-skip-worktree to make edits.

AlexsLemonade / refinebio-web