Fix database container #95

ctmbl · 2023-04-20T09:10:31Z

close #94

We should however take care of choosing an updated version of bitnami/mongodb image, 4.4 is pretty old... https://hub.docker.com/r/bitnami/mongodb

⚠️ IMPORTANT from bitnami documentation

NOTE: As this is a non-root container, the mounted files and directories must have the proper permissions for the UID 1001.

meaning rw I think (but maybe x too didn't test it)
This is part of this debate and I think we should take greater care to use unprivileged users in our images... we should open an issue about that
At the moment it only means that when deploying we must ensure that this condition above is true, but a deployment script could take care of this.

…conatiner

atxr

Thanks for solving these issues I introduced earlier
I didn't approve yet because we need to tackle the bitnami/mongodb docker version before
Personally, I don't see any problem with latest, but if you want to choose a fixed version, let's update to 6.0 at least!
About the permissions, did you manage to test the persistence with this PR?
Finally, we definitely need to solve this root-container issue you're right :)

ctmbl · 2023-05-04T13:52:34Z

Personally, I don't see any problem with latest, but if you want to choose a fixed version, let's update to 6.0 at least!

We're not in a big project so this shouldn't be a problem, especially because 6.0 has recently been released, so 7.0 should be far.

On one hand we should keep in mind that latest always points to the more recent tag and a major version update in semantic versioning means that some changes are not backward compatible https://semver.org/ so we're taking the risk to break the system without introducing a bug ourselves.
On the other hand latest ensure a better security because we'll update the system almost automatically, and currently 6.0 has been released quite recently and that could come with several bug discovery.

To conclude this is a complicate choice to make, and I personally don't like using latest but for this one time the security-risk/broken-api-risk tip the scales in latest's favour I think!
EDIT: I added it in b61bd31 don't hesitate to object if you do not agree finally!

About the permissions, did you manage to test the persistence with this PR?

I manage to... but as I discussed with some of you in private about the blog bot logs the permissions on the host must match the uid used in the container, this is something that must be taken into account either on the host side (by pre-creating the mounted folders and setting them owned by the userid of the container user) or at the container start time (by running the container as the userid that owns the folder on the host) both should work, in either way this will be addressed in another PR by writing a run.sh script I think!

Finally, we definitely need to solve this root-container issue you're right :)

Absolutely let's open an issue!

pros and cons have been discussed here: iScsc#95 (comment)

ctmbl · 2023-05-04T14:08:11Z

sorry for the force push I add foolishly pushed a wrong commit (you know the revert of #34 to deploy locally with docker while #86 is not merge ahah)

atxr

Nice! Then, it's all green for me! 🟢

gbrivady · 2023-05-04T17:18:38Z

Works with the npm running method, commits and discussions/decisions you discutted seems sensible to me, so good to go

Edit : can't approve tho

ctmbl · 2023-05-04T21:37:00Z

@gbrivady yeah I knew that but I think it's time to change it so I promoted you to the iscsc.fr-maintainers team now your approval will allow this PR to be merged!

However, how did you start the DB if you've tried it with the npm command?

Also one important think you must test is the DB persistence, to properly test it you must also properly setup your ./mongodb folder, don't know if you've already done it but here are the steps (implicitly explained in my first message):

mkdir -p mongodb/prod
mkdir mongodb/dev
chmod -R 664 mongodb
sudo chown -R :1001 mongodb

The goal of these lines is to make the mongodb folder (which will be mounted so shared by the container) owned and writable by the user 1001 in the container!

PS: if you'd like to test it properly with docker, without #86 being merged the best way is to :

git revert 8087def

unluckily you'll have a conflict, to solve it:

in docker-compose.yml keep only the network section change, get rid of everything else, it's about SSL certifcates
other conflicting files aren't code so add them as is we don't matter

git revert --continue

the you can start the docker

docker-compose --env-file .env.production up --build

you may have some env var wrongly setup but it's easily solvable

gbrivady · 2023-05-04T22:02:28Z

@gbrivady yeah I knew that but I think it's time to change it so I promoted you to the iscsc.fr-maintainers team now your approval will allow this PR to be merged!

However, how did you start the DB if you've tried it with the npm command?

Also one important think you must test is the DB persistence, to properly test it you must also properly setup your ./mongodb folder, don't know if you've already done it but here are the steps (implicitly explained in my first message):
mkdir -p mongodb/prod
mkdir mongodb/dev
chmod -R 664 mongodb
sudo chown -R :1001 mongodb
The goal of these lines is to make the mongodb folder (which will be mounted so shared by the container) owned and writable by the user 1001 in the container!

PS: if you'd like to test it properly with docker, without #86 being merged the best way is to :
git revert 8087def
unluckily you'll have a conflict, to solve it:

in docker-compose.yml keep only the network section change, get rid of everything else, it's about SSL certifcates

other conflicting files aren't code so add them as is we don't matter
git revert --continue
the you can start the docker
docker-compose --env-file .env.production up --build
you may have some env var wrongly setup but it's easily solvable

Yeah I might definitely have not checked it correctly, will try again tomorrow

ctmbl · 2023-05-05T12:37:06Z

@gbrivady sorry for the messy testing process #86 and #95 are changing many things in the dev workflow so things are a bit messy right now, when you'll be testing it don't hesitate to ping me on discord to solve the problems you may have!

gbrivady · 2023-05-05T16:09:29Z

Add to fix some stuff in my .env, but got the site and DB running according to your comments, so I guess good to go?

ctmbl · 2023-05-05T17:11:22Z

perfect! tks a lot for testing it!

This merge is needed to bring the recent changes of iScsc#95 and solve merges conflicts of the PR

* Fix non persistent DB by mounting the right volume in the container * Configure MongoDB port through additional flags to pass to mongod in conatiner * Run prettier * Add mongodb folder to gitignore * Set bitnami/mongodb version tag to latest pros and cons have been discussed here: #95 (comment)

Remove env-cmd call in npm 'start' script --> handle by docker compose Add nginx dev config (see commit body) Took the one before [https](8087def) was introduced It was introduced with [dev and prod mode](f20a338) Add backend dev Dockerfile Add frontend dev Dockerfile Add dev docker compose Separate dev and prod mode for the nginx template created 2 separate template conf and pass an env var MODE from .env to docker to run.sh for the nginx config Update shared volumes in docker compose dev to only share source code Fix nginx container error because targets not found name of react and not containers was wrong Remove commented dead config from nginx conf dev template Add explicit logging config to nginx conf dev template Adapt docker-compose-dev.yml to local containerized DB Refactor docker-compose-dev.yml to be easier to understand I simply refactor each service config attributes order the order I chose is arbitrary but seems logical - [A|B] means A OR B, because they are mutually exclusive ```yml services: <service name>: depends_on: [build|image]: networks: restart: env_file: ports: volumes: ``` Remove a useless tabulation in docker-compose-dev.yml Refactor docker-compose.yml the same way than docker-compose-dev.yml I simply refactor each service config attributes order the order I chose is arbitrary but seems logical - [A|B] means A OR B, because they are mutually exclusive ```yml services: <service name>: depends_on: [build|image]: networks: restart: env_file: ports: volumes: ``` Set the frontend's `proxy` with env var instead of hardcoded value remove the hardcoded value of `proxy` in package.json the frontend fails to reach the backend in container because thanks to docker networks the backend exists at `node-app[-dev]:$NODE_PORT` from the frontend container, not at `localhost:$NODE_PORT` add http-proxy-middleware to package.json and create a ./frontend/src/setupProxy.js to configure the proxy following [react tutorial](https://create-react-app.dev/docs/proxying-api-requests-in-development/#configuring-the-proxy-manually) PS: the changes in `package-lock.json` seems to come from the fact that the newly installed package shares dependencies with other and they ahve different needs over these shared dependencies Update .env.example Fix nginx container crash because it can't access to logs folder git addugit addu I keep these lines commented because I want to add logging withb it iScsc#10 Configure MongoDB port through additional flags to pass to mongod in conatiner Fix non persistent DB by mounting the right volume in the container Run prettier Specify read-write permissions on mounted volumes Remove containers restart option in dev mode Fix database container (iScsc#95) * Fix non persistent DB by mounting the right volume in the container * Configure MongoDB port through additional flags to pass to mongod in conatiner * Run prettier * Add mongodb folder to gitignore * Set bitnami/mongodb version tag to latest pros and cons have been discussed here: iScsc#95 (comment) Improve DB_PORT passing to MongoDB I just better read the doc https://hub.docker.com/r/bitnami/mongodb/ search for MONGODB_PORT_NUMBER Write a first version of README about dev mode containerized Add setup-db-folder.sh script Update README with DB folder setup and clean it Add an important waning about mongodb folder permissions to README Fix setup-db-folder.sh script Update bitnami mongoDB used image to latest Remove restart attribute in docker compose dev, useless in dev mode Update GH Action to exclude mongodb folder from prettier checking Run prettier Try a prettier GH Action fix Armor nginx run script against unexpected MODE value leading to conf template not found Fix variables wrongly substituted in nginx dev conf and comment logs format This log_format named 'main' isn't used anyway so it is useless, I keep those lines commented anyway for a later PR where I'll properly set the logging in dev and prod mode Improve setup database script

* Add Node development environment (#86 still not merged) Remove env-cmd call in npm 'start' script --> handle by docker compose Add nginx dev config (see commit body) Took the one before [https](8087def) was introduced It was introduced with [dev and prod mode](f20a338) Add backend dev Dockerfile Add frontend dev Dockerfile Add dev docker compose Separate dev and prod mode for the nginx template created 2 separate template conf and pass an env var MODE from .env to docker to run.sh for the nginx config Update shared volumes in docker compose dev to only share source code Fix nginx container error because targets not found name of react and not containers was wrong Remove commented dead config from nginx conf dev template Add explicit logging config to nginx conf dev template Adapt docker-compose-dev.yml to local containerized DB Refactor docker-compose-dev.yml to be easier to understand I simply refactor each service config attributes order the order I chose is arbitrary but seems logical - [A|B] means A OR B, because they are mutually exclusive ```yml services: <service name>: depends_on: [build|image]: networks: restart: env_file: ports: volumes: ``` Remove a useless tabulation in docker-compose-dev.yml Refactor docker-compose.yml the same way than docker-compose-dev.yml I simply refactor each service config attributes order the order I chose is arbitrary but seems logical - [A|B] means A OR B, because they are mutually exclusive ```yml services: <service name>: depends_on: [build|image]: networks: restart: env_file: ports: volumes: ``` Set the frontend's `proxy` with env var instead of hardcoded value remove the hardcoded value of `proxy` in package.json the frontend fails to reach the backend in container because thanks to docker networks the backend exists at `node-app[-dev]:$NODE_PORT` from the frontend container, not at `localhost:$NODE_PORT` add http-proxy-middleware to package.json and create a ./frontend/src/setupProxy.js to configure the proxy following [react tutorial](https://create-react-app.dev/docs/proxying-api-requests-in-development/#configuring-the-proxy-manually) PS: the changes in `package-lock.json` seems to come from the fact that the newly installed package shares dependencies with other and they ahve different needs over these shared dependencies Update .env.example Fix nginx container crash because it can't access to logs folder git addugit addu I keep these lines commented because I want to add logging withb it #10 Configure MongoDB port through additional flags to pass to mongod in conatiner Fix non persistent DB by mounting the right volume in the container Run prettier Specify read-write permissions on mounted volumes Remove containers restart option in dev mode Fix database container (#95) * Fix non persistent DB by mounting the right volume in the container * Configure MongoDB port through additional flags to pass to mongod in conatiner * Run prettier * Add mongodb folder to gitignore * Set bitnami/mongodb version tag to latest pros and cons have been discussed here: #95 (comment) Improve DB_PORT passing to MongoDB I just better read the doc https://hub.docker.com/r/bitnami/mongodb/ search for MONGODB_PORT_NUMBER Write a first version of README about dev mode containerized Add setup-db-folder.sh script Update README with DB folder setup and clean it Add an important waning about mongodb folder permissions to README Fix setup-db-folder.sh script Update bitnami mongoDB used image to latest Remove restart attribute in docker compose dev, useless in dev mode Update GH Action to exclude mongodb folder from prettier checking Run prettier Try a prettier GH Action fix Armor nginx run script against unexpected MODE value leading to conf template not found Fix variables wrongly substituted in nginx dev conf and comment logs format This log_format named 'main' isn't used anyway so it is useless, I keep those lines commented anyway for a later PR where I'll properly set the logging in dev and prod mode Improve setup database script * Update docker-compose-dev.yml for a flask backend * Update backend/Dockerfile.dev to a python image running flask * Fix FLASK_RUN_PORT env var name * Update nginx conf to proxy to the flask app * Fix flaks container not accessible from outside the container * Update lasting wrong NODE_PORT var in .env.example Co-authored-by: Alexandre Tullot <alexandre.tullot@protonmail.com> --------- Co-authored-by: Alexandre Tullot <alexandre.tullot@protonmail.com>

ctmbl added 2 commits April 20, 2023 10:57

Fix non persistent DB by mounting the right volume in the container

f2febe7

Configure MongoDB port through additional flags to pass to mongod in …

1946db7

…conatiner

ctmbl added bug Something isn't working Priority: High The Issue must be addressed as soon as possible Severity: Critical The bug or Issue prevent the website from running or present a critical security issue DB about the website database labels Apr 20, 2023

ctmbl requested review from atxr, amtoine and J3y0 April 20, 2023 09:10

ctmbl self-assigned this Apr 20, 2023

ctmbl added 2 commits April 20, 2023 18:11

Run prettier

d605d61

Add mongodb folder to gitignore

8cd22fe

atxr reviewed May 2, 2023

View reviewed changes

atxr mentioned this pull request May 2, 2023

Create a containerize environment for development workflow #86

Merged

6 tasks

ctmbl added a commit to ctmbl/iscsc.fr that referenced this pull request May 4, 2023

Set bitnami/mongodb version tag to latest

01a9222

pros and cons have been discussed here: iScsc#95 (comment)

ctmbl force-pushed the fix-db-container branch from 01a9222 to b61bd31 Compare May 4, 2023 14:05

Set bitnami/mongodb version tag to latest

b61bd31

pros and cons have been discussed here: iScsc#95 (comment)

atxr approved these changes May 4, 2023

View reviewed changes

ctmbl requested a review from gbrivady May 4, 2023 21:30

This was referenced May 5, 2023

Security enhancement: use non-root containers for nginx/frontend/backend #98

Open

Monitor and log website traffic #10

Open

gbrivady approved these changes May 5, 2023

View reviewed changes

ctmbl merged commit eb1bd3c into iScsc:main May 5, 2023

ctmbl deleted the fix-db-container branch May 5, 2023 17:13

ctmbl added a commit to ctmbl/iscsc.fr that referenced this pull request May 10, 2023

Merge branch 'main' into dev-container

07e0eae

This merge is needed to bring the recent changes of iScsc#95 and solve merges conflicts of the PR

ctmbl mentioned this pull request May 12, 2023

FEATURE: add npm run rule to prettify the code #99

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix database container #95

Fix database container #95

ctmbl commented Apr 20, 2023

atxr left a comment •

edited

Loading

ctmbl commented May 4, 2023 •

edited

Loading

ctmbl commented May 4, 2023

atxr left a comment

gbrivady commented May 4, 2023 •

edited

Loading

ctmbl commented May 4, 2023 •

edited

Loading

gbrivady commented May 4, 2023

ctmbl commented May 5, 2023

gbrivady commented May 5, 2023

ctmbl commented May 5, 2023

Fix database container #95

Fix database container #95

Conversation

ctmbl commented Apr 20, 2023

atxr left a comment • edited Loading

Choose a reason for hiding this comment

ctmbl commented May 4, 2023 • edited Loading

ctmbl commented May 4, 2023

atxr left a comment

Choose a reason for hiding this comment

gbrivady commented May 4, 2023 • edited Loading

ctmbl commented May 4, 2023 • edited Loading

gbrivady commented May 4, 2023

ctmbl commented May 5, 2023

gbrivady commented May 5, 2023

ctmbl commented May 5, 2023

atxr left a comment •

edited

Loading

ctmbl commented May 4, 2023 •

edited

Loading

gbrivady commented May 4, 2023 •

edited

Loading

ctmbl commented May 4, 2023 •

edited

Loading