Skip to content
This repository has been archived by the owner on Sep 17, 2021. It is now read-only.

feature/docker #433

Merged
merged 37 commits into from
Nov 10, 2016
Merged

feature/docker #433

merged 37 commits into from
Nov 10, 2016

Conversation

johnbuhay
Copy link
Contributor

This feature introduces files and updates for dockerizing SecurityMonkey!

Note-able changes are:

  • env-config/config-deploy.py: Configuration using environment variables
  • docker folder: entrypoint scripts
  • docker-nginx folder: nginx configuration requirements
  • docker-compose.yml: for local development
  • Documentation* added to docs/ * I am not too familiar with reStructuredText

@scriptsrc
Copy link
Contributor

Baking in an SSL cert makes me uncomfortable.

I understand this was taken from ZeroToDocker:

https://github.com/Netflix-Skunkworks/zerotodocker/blob/master/security_monkey/0.3.4/security_monkey-nginx/server.key

Is there a better way to do that?

@scriptsrc
Copy link
Contributor

Also, should this be a separate repo? Is there a reason to merge this in with the codebase?

@johnbuhay
Copy link
Contributor Author

I completely agree with you on the SSL baking.

I have just added a few more commits that disable the ssl in nginx if they are not present.

This way the certificates are not baked in and ssl is enabled by default, so long as the user makes them available.

@johnbuhay
Copy link
Contributor Author

One reason for merging this into the codebase is for local development with the use of Docker. With the docker-compose.yml file, you can bring up an entire SecurityMonkey environment as it would be deployed. This biggest benefit is changes to your local repo can be built and tested in the container environment much faster than waiting for CI.

At the very least, the changes in config-deploy.py are required. This way the same container can be shipped to any environment, and it will be configured to that environments specifics with the use of variables. This is much easier to maintain because minimal volumes are needed.

Should you prefer, I can cut another PR for just the config-deploy.py update for merging. Since I am probably the only one doing local development with Docker at this time; this would be the least impactful to the current codebase.

Apologies if I was too verbose.

@scriptsrc
Copy link
Contributor

Super cool. Once you send those couple changes, I'll get this merged in.

I'll probably add a ReadMe in the docker folder identifying that it's for local dev as well.

@johnbuhay
Copy link
Contributor Author

More details on a few changes since we last discussed:

  • SSL certs are not used in the Dockerfile, intention here is to enable the use of any docker orchestration tool to manage those types of secrets
    • entrypoint for NGINX checks for the required SSL certs and logs a clear message to stdout if they are missing (NGINX will also generate its own error message)
  • config-deploy.py was reverted to its original state, and one containing my changes for Docker has been included as env-config/config-docker.py
  • Updated documentation

@scriptsrc
Copy link
Contributor

alt tag

The config-deploy.py in your feature/docker is behind the config-deploy.py in the develop branch. Could you rebase for me?

jnbnyc added 21 commits November 9, 2016 17:11
default settings with environment variables
for postgres settings
Netflix-Skunkworks/zerotodocker
Netflix-Skunkworks/zerotodocker
variable in these entrypoints as this should be
set before these entrypoints exist
variable to override default settings defined
in SECURITY_MONKEY_SETTINGS

2 - Update entrypoints to use environment
variables
Netflix-Skunkworks/zerotodocker
Netflix-Skunkworks/zerotodocker
by Netflix-Skunkworks. These are meant to
act as a placeholder for the example.
.dockerignore to avoid committing
secrets
SecurityMonkey from this directory,
as opposed to checking out the repository
directly, since this code has not yet been
merged. This can be used to build and
develop locally.
variables as override to default
@johnbuhay johnbuhay closed this Nov 9, 2016
@johnbuhay
Copy link
Contributor Author

Fixed config-deploy.py

@johnbuhay johnbuhay reopened this Nov 9, 2016
@scriptsrc scriptsrc merged commit 1f42816 into Netflix:develop Nov 10, 2016
@scriptsrc scriptsrc mentioned this pull request Dec 2, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants