Dockerize server (#249, #236)

[irsol]

Description

App and MongoDB can run as Docker containers.

Issue Addressed

• Issue number [BUG] Server Discovery and Monitoring engine is deprecated #249

• Issue number [FEATURE] Setup with docker #236

Additionally

Upgraded Node version to 12.0.0.

---

This change is 

[sash-ko]

Reviewed 10 of 10 files at r1.
Reviewable status: complete! all files reviewed, all discussions resolved

[sash-ko]

Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @irsol)

---

Dockerfile, line 8 at r1 (raw file):

RUN npm install
ENV SECRET='abadjadjadja1223232412424'
ENV developmentDB = 'mongodb://mongodb:27017/ntwitter'

Use capital letter for environment variables, for example, DEVELOPMEN_DB

---

Dockerfile, line 12 at r1 (raw file):

COPY . .
#Expose port and start application
EXPOSE 3000

Don't change ports if there is not real reason for this

---

Dockerfile, line 14 at r1 (raw file):

EXPOSE 3000

CMD [ "node", "server.js" ]

node server.js just runs you server. npm start execute start from the scri[ts package.json. In package.json it is defined nodemon server.js. Bear in mind, that nodemon is not needed for Docker it is still needed when running the service without Docker

---

readme.md, line 54 at r1 (raw file):

};

Usage via Docker

Add here how to run your server in Docker and mongo without (check your config.js)

---

server.js, line 2 at r1 (raw file):

const express = require('express');
var cookieParser = require('cookie-parser');

See comment about sessions below

---

server.js, line 15 at r1 (raw file):

const promiseRetry = require('promise-retry');

app.use(cookieParser("super55"));

I think you should not include cookies and sessions into this PR

---

config/config.js, line 6 at r1 (raw file):

const clientID = process.env.GITHUB_CLIENT_ID;
const clientSecret = process.env.GITHUB_CLIENT_SECRET;
const developmentDB = process.env.developmentDB || 'mongodb://localhost:27017/ntwitter';

See comment about environment variables

[sash-ko]

Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @irsol)

---

config/config.js, line 20 at r1 (raw file):

    },
    github: {
      clientID: "e3930cf94c772ba10ef1",

All these secrets should go to .env file ( checkout this blog post Node.js Everywhere with Environment Variables! ). But for this, you need to create another issue and another pull request (just a recommendation)

[sash-ko]

Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @irsol)

---

Dockerfile, line 7 at r1 (raw file):

COPY package*.json ./
RUN npm install
ENV SECRET='abadjadjadja1223232412424'

When I run the server without Docker it is missing SECRET variable (see file expess.js where it is used) and I get an error Error: secret option required for sessions. You probably need to define this variable in .env file

[sash-ko]

Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @irsol)

---

server.js, line 16 at r1 (raw file):

app.use(cookieParser("super55"));
app.use(cookieSession({

See comment about SECRET how to solve problem that will appear once you remove those lines

[irsol]

Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @irsol)

server.js, line 16 at r1 (raw file):

app.use(cookieParser("super55"));
app.use(cookieSession({

See comment about SECRET how to solve problem that will appear once you remove those lines

Created new issue #252 .

[sash-ko]

Reviewable status: all files reviewed, 11 unresolved discussions (waiting on @irsol and @sash-ko)

---

server.js, line 36 at r1 (raw file):

  maxTimeout: 5000
};

Add comments with an explanation why you do this (promiseRetry)

[irsol]

Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @irsol)

Dockerfile, line 7 at r1 (raw file):

COPY package*.json ./
RUN npm install
ENV SECRET='abadjadjadja1223232412424'

When I run the server without Docker it is missing SECRET variable (see file expess.js where it is used) and I get an error Error: secret option required for sessions. You probably need to define this variable in .env file

Created new issue #252 .

[irsol]

environment variable

Renamed env variables.

[vinitkumar]

@irsol @sash-ko Sorry guys! I have been busy so I didn't look into the notifications. Posting some comments on the PR. @irsol if you able to resolve this, I will be more than happy to merge this PR.

[vinitkumar]

Reviewable status: 8 of 11 files reviewed, 12 unresolved discussions (waiting on @irsol and @sash-ko)

---

Dockerfile, line 12 at r1 (raw file):

Previously, sash-ko (Oleksandr Lysenko) wrote…

Don't change ports if there is not real reason for this

Same. Don't change that port.3000 has always been the port so keep it as it is.

---

docker-compose.yml, line 6 at r2 (raw file):

    build: .
    ports:
      - "8080:3000"

Keep it as 3000. Don't change it.

[lgtm-com]

This pull request introduces 5 alerts when merging 34b494e into 2e4f8f6 - view on LGTM.com

new alerts:

• 3 for Hard-coded credentials
• 2 for Missing CSRF middleware

[lgtm-com]

This pull request introduces 5 alerts when merging 34b494e into c41d553 - view on LGTM.com

new alerts:

• 3 for Hard-coded credentials
• 2 for Missing CSRF middleware