Skip to content

Added environment variable to set docker-compose down options#4

Open
Corneel-D wants to merge 9 commits into
gleerman:masterfrom
Corneel-D:docker-compose-down-options
Open

Added environment variable to set docker-compose down options#4
Corneel-D wants to merge 9 commits into
gleerman:masterfrom
Corneel-D:docker-compose-down-options

Conversation

@Corneel-D
Copy link
Copy Markdown

Added an environment variable DOCKER_COMPOSE_DOWN_OPTIONS to set the docker-compose down options for our services. This makes it possible to e.g. easily remove all named volumes when stopping our services.

@gleerman
Copy link
Copy Markdown
Owner

gleerman commented Mar 11, 2020

I have three remarks about this PR.

My most major remark is that the change is not backward compatible. When the new env variable is not set, the program crashes:

----- DEPLOYING -----
Traceback (most recent call last):
  File "/srv/api/api.py", line 59, in <module>
    deployService.performDeploy()
  File "/srv/api/services/deploy.py", line 63, in performDeploy
    self.runCommand(self.getDownCommand())
  File "/srv/api/services/deploy.py", line 40, in getDownCommand
    options = os.environ['DOCKER_COMPOSE_DOWN_OPTIONS']
  File "/usr/lib/python3.8/os.py", line 675, in __getitem__
    raise KeyError(key) from None
KeyError: 'DOCKER_COMPOSE_DOWN_OPTIONS'

Secondly, I think there should be support to include the parameters in quotes. DOCKER_COMPOSE_DOWN_OPTIONS='--volumes' doesn't work, whereas DOCKER_COMPOSE_DOWN_OPTIONS=--volumes is working.

Lastly, I am slightly concerned about the ability to perform injected commands from within the docker-compose.yml file. For example, setting DOCKER_COMPOSE_DOWN_OPTIONS=; perform-evil-script.py. I'm not entirely sure how relevant this is, though, as the performer of the deployment startup command most likely has the rights to perform these kind of tasks. But they may not be aware of a secondary command being present.

I may prefer the option to add a flag DOCKER_COMPOSE_DOWN_FULL=true|false to solve the latter two concerns, resulting in the same feature being present. The first comment would then be solved by using a default value false when the environment variable is not present.

@Corneel-D
Copy link
Copy Markdown
Author

  • The crash without environment variable has been fixed
  • I tested and surrounding the parameters with quotes works correctly
  • The reason I choose to use a general flag is so it is also possible to easily set the other options. I understand the (valid) concern of injecting commands, although I think this kind of attack is already possible using the DOCKER_COMPOSE_SERVICES environment variable

@Corneel-D
Copy link
Copy Markdown
Author

I have split up the options into different environment variables:

  • DOCKER_COMPOSE_DOWN_OPTIONS_RMI
  • DOCKER_COMPOSE_DOWN_OPTIONS_VOLUMES
  • DOCKER_COMPOSE_DOWN_OPTIONS_REMOVE_ORPHANS
  • DOCKER_COMPOSE_DOWN_OPTIONS_TIMEOUT

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