Skip to content

Dockerize vulnerablecode - #256

Merged
pombredanne merged 6 commits into
aboutcode-org:developfrom
sbs2001:dockerize
Sep 24, 2020
Merged

Dockerize vulnerablecode#256
pombredanne merged 6 commits into
aboutcode-org:developfrom
sbs2001:dockerize

Conversation

@sbs2001

@sbs2001 sbs2001 commented Sep 20, 2020

Copy link
Copy Markdown
Collaborator

Fixes #229

Signed-off-by: Shivam Sandbhor shivam.sandbhor@gmail.com

Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>
@sbs2001
sbs2001 requested review from singh1114 and tdruez September 20, 2020 06:31
@sbs2001

sbs2001 commented Sep 20, 2020

Copy link
Copy Markdown
Collaborator Author

This is a repaste of the issue I am facing from gitter :

All works fine if the project is ran using docker-compose but I want the project to work without docker too, for that to happen I have specify "HOST":"localhost" in the settings.py manually which is not ideal. How should I tell django where to look for db depending upon whether it's ran in a container or normally ? I'm thinking of using an env variable in the docker-compose file and add some logic in settings.py to interpret this env variable. Is there a better way ?

When the project is ran in docker containers  the `db`
container is not on `localhost` as specified in `settings.py`.
To counter this an env variable `DB_HOST` is used to specify where
the db is hosted.

Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>
Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>
Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>
@sbs2001 sbs2001 changed the title [DO NOT MERGE] 🚀 Dockerize vulnerablecode Dockerize vulnerablecode Sep 21, 2020

@pombredanne pombredanne left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!
LGTM except for some weird spacing and a suggestion on Docker image.

Comment thread README.md Outdated
Comment thread README.md
Comment thread docker-compose.yml
Comment thread Dockerfile
Comment thread Dockerfile Outdated
RUN mkdir /vulnerablecode
WORKDIR /vulnerablecode
ADD . /vulnerablecode/
RUN pip install -r requirements.txt No newline at end of file

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add some labels to the image such that we know its base image, origin and license, author, maintainers, etc? (See how complicated a Docker image analysis can be with https://github.com/nexB/scancode.io/blob/main/scanpipe/pipelines/docker.py )

@sbs2001
sbs2001 force-pushed the dockerize branch 2 times, most recently from ddbc85c to 1d6e67e Compare September 21, 2020 10:36
Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>

@pombredanne pombredanne left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my suggestions inline. Note that at this stage we may want to start using ReST for the README and docs like for the other projects ... but this should another PR.

Comment thread README.md
The easiest way to set up VulnerableCode is by using the docker containers.
For this you need to have the following installed.
- Docker Engine. Find instructions to install it here
- Docker Compose. Find instructions to install it here

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about this:

Suggested change
- Docker Compose. Find instructions to install it here
An easy way to set up VulnerableCode is with docker containers and docker compose.
For this you need to have the following installed.
- Docker Engine at https://docs.docker.com/engine/install/
- Docker Compose at https://docs.docker.com/compose/install/

Comment thread docker-compose.yml Outdated
- DJANGO_DEV=1
- VC_DB_HOST=db
build: .
command: bash -c "python manage.py makemigrations && python manage.py migrate && python manage.py runserver 0.0.0.0:8000"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the "web" service have to "makemigrations"?

Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>

@pombredanne pombredanne left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM++ 🙇
Merging

Comment thread Dockerfile
LABEL "base_image": "pkg:docker/python@sha256%3Ae9b7e3b4e9569808066c5901b8a9ad315a9f14ae8d3949ece22ae339fff2cad0"
LABEL "dockerfile_url": "https://github.com/nexB/vulnerablecode/blob/develop/Dockerfile"
LABEL "homepage_url": "https://github.com/nexB/vulnerablecode"
LABEL "license": "Apache-2.0" No newline at end of file

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome :)

@pombredanne
pombredanne merged commit 6bf44e0 into aboutcode-org:develop Sep 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Better deployment, more convenient usage

3 participants