Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dockerized testenv #8

Closed
wants to merge 13 commits into from
Closed

Conversation

andresriancho
Copy link
Contributor

This pull request allows users to run testenv inside docker. In order to use
this new feature users need to run the following command:

docker-compose --project-name test up

And then access http://localhost:8998/ , the port where the testenv will listen
is defined in the docker-compose.yml file:

mydb:
  image: mysql
  environment:
    - MYSQL_ROOT_PASSWORD=testpass

testenv:
  image: andresriancho/testenv:latest
  ports:
    - "8998:80"
  links:
    - mydb
  environment:
    - APACHE_RUN_USER=www-data
    - APACHE_RUN_GROUP=www-data
    - APACHE_LOG_DIR=/var/log/apache2/

In the future, and if this PR is accepted, the testenv image would be in sqlmapproject
instead of andresriancho.

At the moment only the mysql part was implemented (because that's what I'm using in w3af),
but adding more databases should take less than an hour (each).

Docker has many incredible features, one of the most important ones for the testenv is
the fact that we can change the mysql version by specifying it in the docker compose file:

mydb:
  image: mysql:5.5
  environment:
    - MYSQL_ROOT_PASSWORD=testpass

No installation required, no configuration changes, just adding :5.5 will run a different mysql
version in the container.

The testenv main server is defined in Dockerfile which would be the file to extend when
adding more databases. People extending this should follow the design concept of using other
containers for the databases (whenever possible). These external containers most likely exist
and can be re-used, instead of installing inside the main container.

The final part in all this magic is the continuous delivery aspect which is implemented in circle.yml,
note that this file is run by CircleCI (free CI system for open source projects) and the main steps
are:

  • Build the latest testenv image
  • Make sure it's working (curl)
  • Push it to the registry

So each time someone makes a change to the repo (for now mine, but in the future the testenv) the
image in the registry is updated and all the users get the latest when running
docker-compose --project-name test up

If in the future you decide to use CircleCI and want to push to the docker registry, you'll have to
set the right environment variables in your build configuration (so CI can auth to the registry). Those
variables are DOCKER_EMAIL and DOCKER_AUTH

This work is part of my efforts to dockerize all of w3af's test dependencies

@stamparm
Copy link
Member

My comments:

  1. Adding CircleCI files is an overkill (at least for a sqlmap's testenv) - IMO
  2. Docker files should go into a separate directory (i.e. docker) not laying around in the repository's root directory
  3. I am pro KISS. One docker file (per DBMS?) should be the goal (with auxiliary scripts in some subdirectory of docker) - e.g. https://github.com/andresriancho/testenv/blob/master/Dockerfile and https://github.com/andresriancho/testenv/blob/master/docker/run.sh should stay in e.g. form docker/mysql and docker/scripts/mysql.sh. Other stuff should be removed
  4. I would suggest also that docker files could also be run independently of the testenv alltogether. Docker file could check if its a standalone and pull/download the testenv (or parts of it) if needed. That would show the real power of Docker.

p.s. in plain speak: can't automatically pull the request because of all those files laying around the root directory (as we in sqlmap tend to keep stuff structured as much as we can). Also, introducing of CircleCI is unneeded (at least for us)

@andresriancho
Copy link
Contributor Author

  1. Adding CircleCI files is an overkill (at least for a sqlmap's testenv) - IMO

Just for the record, the files related with CircleCI builds are:

  • ci/dockercfg.template
  • circle.yml

These files are required to have continuous delivery of testenv to the docker registry. So, I believe that the question here is: "Do we need/want to have testenv delivered to the docker registry on each change?"

IMHO, if it's already done, why not keep it?

If we decide that that's something we want I could replace the ci/dockercfg.template file with something inside the circle.yml file, but the circle configuration file needs to exist and be in the root directory (that's a CircleCI requirement)

  1. Docker files should go into a separate directory (i.e. docker) not laying around in the repository's root directory

Agreed. Moved docker-compose.yml inside docker (andresriancho@0072711)

It's not possible to move the "Dockerfile" outside the repository root because of how "docker build" works together with COPY . /var/www/sqlmap/, there is no way to copy from a parent directory, like we would have to do if the Dockerfile is in docker and the code is in the root: COPY ../ /var/www/sqlmap/

  1. I am pro KISS. One docker file (per DBMS?) should be the goal (with auxiliary scripts in some subdirectory of docker) - e.g. https://github.com/andresriancho/testenv/blob/master/Dockerfile and https://github.com/andresriancho/testenv/blob/master/docker/run.sh should stay in e.g. form docker/mysql and docker/scripts/mysql.sh. Other stuff should be removed

This is a little bit complicated to achieve given the previous limitation of Dockerfiles. The workaround would be to create a build.sh script that lives inside the docker directory and performs these steps:

  • Take a command line argument which specifies the dockerfile to build, for example: build.sh mysql
  • Copy the docker/mysql/Dockerfile to the repository root
  • cd into the repository root
  • Run the docker build command
  • cd back into the original directory

I'm unsure about how you guys use the testenv, but... when you run the sqlmap test suite, don't you need all services (mysql, pgsql, etc.) up and running? If the answer is yes, why would we split this into different docker images? Also, why isn't this split in deployment.sh?

Another question that appears is if the different docker images for each DBMS wouldn't have a lot in common, and a change in one would require a change in many (not sure about this one)

Finally, would you push "testenv-mysql", "testenv-pgsql", etc. to the registry?

I believe that the benefits of splitting this up are:

  • Less memory usage when running the docker-compose command, since it will only start the dbms you want to work with

And the problems are:

  • Potential code duplication between dockerfiles for each dbms
  • We have to maintain one docker-compose.yml for each dbms
  • "Complex" usage, since the user would have to run N command, one for each dbms he wants, in some cases leading to frustration:
    • Start docker-compose for mysql
    • Test some stuff, all works, happy happy joy, etc.
    • Test some other stuff, error, problems
    • Hate
    • Ah! I had to start the docker-compose for pgsql!
  • In the previous example, the memory usage would also suffer because the user would be running two instances of the apache docker image, one for mysql and one for pgsql
  1. I would suggest also that docker files could also be run independently of the testenv alltogether. Docker file could check if its a standalone and pull/download the testenv (or parts of it) if needed. That would show the real power of Docker.

Yes, that could be done too (if you don't like docker stuff to be in this repo).

IMHO the power of docker is to have continuous delivery of docker images on each change you make to the registry. This allows users to run the testenv easily and without having to build it themselves.

p.s. in plain speak: can't automatically pull the request because of all those files laying around the root directory (as we in sqlmap tend to keep stuff structured as much as we can). Also, introducing of CircleCI is unneeded (at least for us)

"all those files", oh my, you're OCD is worse than mine! After the change I introduced, the only files are Dockerfile (which has a reason to be there, COPY) and circle.yml (which needs to be there because of circleci requirement).

@andresriancho
Copy link
Contributor Author

@stamparm ping 📣

@stamparm
Copy link
Member

stamparm commented Aug 5, 2015

Just to discuss with @inquisb. Will let you know.

@stamparm
Copy link
Member

stamparm commented Aug 5, 2015

@andresriancho p.s. my OCD powers are strong :). I am fully aware of that fact.

@stamparm
Copy link
Member

stamparm commented Aug 9, 2015

Sorry for waiting. We internally decided to stick with most simple solution. One Docker file and that's it. No Circle or similar.

@andresriancho
Copy link
Contributor Author

@stamparm so, the files that I should keep in this PR are:

  • Dockerfile (in root dir)
  • docker/docker-compose.yml
  • docker/run.sh

Also, the fix for schema/mysql.sql which is not related to docker.

Do we agree on that? If so I'll remove the extra files and PR is merged, correct?

PS: I'll most likely keep the circle.yml stuff in my fork

@stamparm
Copy link
Member

@andresriancho yup. It sounds ok. Later we'll do more modifications

@andresriancho
Copy link
Contributor Author

Closing this PR, opening a new one.

andresriancho added a commit to andresriancho/testenv that referenced this pull request Aug 17, 2015
@andresriancho andresriancho mentioned this pull request Aug 17, 2015
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.

2 participants