Add tests and a dynamic config section generator #14

kevin-hanselman · 2019-03-30T19:06:22Z

I added a simple Bash script to test the proxy locally. Eventually I'd like to port it to Python.

$ ./run_tests.sh
Starting docksockprox_test with args: ...
PASS | assert 'docker version' is allowed
PASS | assert 'docker run --rm alpine' is forbidden
PASS | assert 'docker pull alpine' is forbidden
PASS | assert 'docker logs docksockprox_test' is forbidden
PASS | assert 'docker wait docksockprox_test' is forbidden
PASS | assert 'docker rm -f docksockprox_test' is forbidden
PASS | assert 'docker restart docksockprox_test' is forbidden
PASS | assert 'docker network ls' is forbidden
PASS | assert 'docker config ls' is forbidden
PASS | assert 'docker service ls' is forbidden
PASS | assert 'docker stack ls' is forbidden
PASS | assert 'docker secret ls' is forbidden
PASS | assert 'docker plugin ls' is forbidden
PASS | assert 'docker info' is forbidden
PASS | assert 'docker system info' is forbidden
PASS | assert 'docker build .' is forbidden
PASS | assert 'docker swarm init' is forbidden
Removing docksockprox_test...
Starting docksockprox_test with args: -e CONTAINERS=1...
PASS | assert 'docker logs docksockprox_test' is allowed
PASS | assert 'docker inspect docksockprox_test' is allowed
PASS | assert 'docker wait docksockprox_test' is forbidden
PASS | assert 'docker run --rm alpine' is forbidden
PASS | assert 'docker rm -f docksockprox_test' is forbidden
PASS | assert 'docker restart docksockprox_test' is forbidden
Removing docksockprox_test...
Starting docksockprox_test with args: -e POST=1...
PASS | assert 'docker rm -f docksockprox_test' is forbidden
PASS | assert 'docker pull alpine' is forbidden
PASS | assert 'docker run --rm alpine' is forbidden
PASS | assert 'docker network create foobar' is forbidden
Removing docksockprox_test...
Starting docksockprox_test with args: -e NETWORKS=1 -e POST=1...
PASS | assert 'docker network ls' is allowed
PASS | assert 'docker network create foo' is allowed
PASS | assert 'docker network rm foo' is allowed
Removing docksockprox_test...

I also added a Python script that downloads the Docker API Swagger YAML files and generates most of the proxy rules. Eventually this script could be expanded to make rules for sub-endpoints (e.g. /containers/{id}/json), enabling fine-grained access control of the entire Docker API.

$ python gen_cfg_section.py 1.27 1.39 --yaml-dir ./yamls/
Downloading Docker API version 1.27...
Writing file './yamls/docker_v1_27.yaml'...
Downloading Docker API version 1.28...
Writing file './yamls/docker_v1_28.yaml'...
Downloading Docker API version 1.29...
Writing file './yamls/docker_v1_29.yaml'...
Downloading Docker API version 1.30...
Writing file './yamls/docker_v1_30.yaml'...
Downloading Docker API version 1.31...
Writing file './yamls/docker_v1_31.yaml'...
Downloading Docker API version 1.32...
Writing file './yamls/docker_v1_32.yaml'...
Downloading Docker API version 1.33...
Writing file './yamls/docker_v1_33.yaml'...
Downloading Docker API version 1.34...
Writing file './yamls/docker_v1_34.yaml'...
Downloading Docker API version 1.35...
Writing file './yamls/docker_v1_35.yaml'...
Downloading Docker API version 1.36...
Writing file './yamls/docker_v1_36.yaml'...
Downloading Docker API version 1.37...
Writing file './yamls/docker_v1_37.yaml'...
Downloading Docker API version 1.38...
Writing file './yamls/docker_v1_38.yaml'...
Downloading Docker API version 1.39...
Writing file './yamls/docker_v1_39.yaml'...
    http-request allow if { path,url_dec -m reg -i ^(/v[\d\.]+)?/_ping } { env(PING) -m bool }
    http-request allow if { path,url_dec -m reg -i ^(/v[\d\.]+)?/auth } { env(AUTH) -m bool }
    http-request allow if { path,url_dec -m reg -i ^(/v[\d\.]+)?/build } { env(BUILD) -m bool }
    http-request allow if { path,url_dec -m reg -i ^(/v[\d\.]+)?/commit } { env(COMMIT) -m bool }
    http-request allow if { path,url_dec -m reg -i ^(/v[\d\.]+)?/configs } { env(CONFIGS) -m bool }
    http-request allow if { path,url_dec -m reg -i ^(/v[\d\.]+)?/containers } { env(CONTAINERS) -m bool }
    http-request allow if { path,url_dec -m reg -i ^(/v[\d\.]+)?/distribution } { env(DISTRIBUTION) -m bool }
    http-request allow if { path,url_dec -m reg -i ^(/v[\d\.]+)?/events } { env(EVENTS) -m bool }
    http-request allow if { path,url_dec -m reg -i ^(/v[\d\.]+)?/exec } { env(EXEC) -m bool }
    http-request allow if { path,url_dec -m reg -i ^(/v[\d\.]+)?/images } { env(IMAGES) -m bool }
    http-request allow if { path,url_dec -m reg -i ^(/v[\d\.]+)?/info } { env(INFO) -m bool }
    http-request allow if { path,url_dec -m reg -i ^(/v[\d\.]+)?/networks } { env(NETWORKS) -m bool }
    http-request allow if { path,url_dec -m reg -i ^(/v[\d\.]+)?/nodes } { env(NODES) -m bool }
    http-request allow if { path,url_dec -m reg -i ^(/v[\d\.]+)?/plugins } { env(PLUGINS) -m bool }
    http-request allow if { path,url_dec -m reg -i ^(/v[\d\.]+)?/secrets } { env(SECRETS) -m bool }
    http-request allow if { path,url_dec -m reg -i ^(/v[\d\.]+)?/services } { env(SERVICES) -m bool }
    http-request allow if { path,url_dec -m reg -i ^(/v[\d\.]+)?/session } { env(SESSION) -m bool }
    http-request allow if { path,url_dec -m reg -i ^(/v[\d\.]+)?/swarm } { env(SWARM) -m bool }
    http-request allow if { path,url_dec -m reg -i ^(/v[\d\.]+)?/system } { env(SYSTEM) -m bool }
    http-request allow if { path,url_dec -m reg -i ^(/v[\d\.]+)?/tasks } { env(TASKS) -m bool }
    http-request allow if { path,url_dec -m reg -i ^(/v[\d\.]+)?/version } { env(VERSION) -m bool }
    http-request allow if { path,url_dec -m reg -i ^(/v[\d\.]+)?/volumes } { env(VOLUMES) -m bool }

One thing I noticed while building this script is that /post doesn't show up in any of the Swagger YAML files. Can anyone explain this? Should the /post rule be deleted?

yajo

Thanks! This contribution is greatly appreciated.

You have to care about some points commented inline, mostly targeted at easy to use and maintain, and also these:

Install python3 in the dockerfile.
Add the proposed script as an entrypoint.
Document usage in README.

I suggest you also to use jinja2 to generate the config file. It will be easier to read and maintain.

Thanks!

yajo · 2019-04-01T07:27:48Z

gen_cfg_section.py

+        formatter_class=argparse.ArgumentDefaultsHelpFormatter
+    )
+    parser.add_argument(
+        'api_versions',


Not needed. You can detect the version directly from the docker socket. Then you can use that single version instead of worrying about many.

To be clear: you would like a version of this script to run in the proxy container, interrogate the current Docker API version, load the appropriate (pre-downloaded) Swagger YAML file, generate the haproxy.cfg file, and start the proxy?

yajo · 2019-04-01T07:30:45Z

gen_cfg_section.py

+        )
+    )
+    parser.add_argument(
+        '--yaml-dir',


Not needed. Swagger YAML files can be in a directory you want. Just document that in the README and let users mount whatever they want in that volume.

You mention elsewhere that you would like the YAML files to be downloaded and added to the Docker image via the Dockerfile. It seems like the method you proposed above is unnecessary if we add all of the YAML files directly to the image in such a fashion.

Yes, it wouldn't be needed. I just imagined that you personally wanted to do it for some reason, given you took the time to add an option for that, but you're right that this is not very important when the other changes are done.

yajo · 2019-04-01T07:31:13Z

gen_cfg_section.py

+        help='Save and load Swagger YAML files from this directory'
+    )
+    parser.add_argument(
+        '--add-comments',


Not needed. The comments are meaningless if you get the api version from the docker socket.

yajo · 2019-04-01T07:31:27Z

gen_cfg_section.py

+    return parser.parse_args()
+
+
+def parse_api_versions(api_versions):


Not needed (see above)

yajo · 2019-04-01T07:34:14Z

run_tests.sh

@@ -0,0 +1,84 @@
+#!/bin/bash


Would you mind changing this to a python unit test?

I recommend you to use plumbum to execute commands. Combininb plumbum with unit tests is a pleasure (see an example here).

Yeah, as mentioned in the PR I wanted to do this eventually. I agree this script isn't very maintainable.

yajo · 2019-04-01T07:36:42Z

gen_cfg_section.py

+            with open(yaml_path) as fd:
+                return yaml.safe_load(fd)
+
+    swagger_url = 'https://docs.docker.com/engine/api/v{}/swagger.yaml'.format(


Do not depend on network connectivity to get this. Given the big chances of this container being restricted by network (due to the risk inherent to opening a docker socket), it's not safe.

Instead, download all available definitions in a folder in the image on build (you have to add that in the dockerfile).

yajo · 2019-04-01T07:38:51Z

gen_cfg_section.py

@@ -0,0 +1,122 @@
+import argparse


Add shebang and make executable.

kevin-hanselman · 2019-04-03T15:36:51Z

@yajo can you comment on the question I posed in the original PR:

One thing I noticed while building this script is that /post doesn't show up in any of the Swagger YAML files. Can anyone explain this? Should the /post rule be deleted?

yajo · 2019-04-04T07:50:53Z

Should the /post rule be deleted?

Yes, it's a bug. The POST env var is for letting POST requests instead of just GET ones. It should be deleted.

From #14

* Add first version of tests From #14 * Expand tests * Add GH CI * Apply suggestions * Apply autopretty template + fix prettier * Fix isort * Apply autoprettier * Fix VSCode settings * Make tests run in parallel * Build docker image before testing * Update workspace settings * Try multi-platform builds and push to ghcr.io * Push to docker hub as well from ci * Upgrade autopretty * Update pyproject configurations * Improve test configuration and execution TT26468 * Provide initial conftest * Improve tests * Add python3 in image * Remove POST rule from proxy * Build image before testing and push at the end Builds the image (in single arch) before testing Loads the image into local docker (See https://github.com/docker/build-push-action#export-image-to-docker) Rebuilds and pushes the final image in multi-arch at the end. * Fix python path * Remove build fixture from tests to see if image is built in CI * Organize docker tests definition and document * Restore fixture allowing usage for local testing This reverts commit dc0b60e and allows using `--prebuild` CLI flag for pytest when doing local tests. Co-authored-by: Jairo Llopis <jairo.llopis@tecnativa.com>

joao-p-marques · 2021-01-28T08:29:36Z

Closed in favor of #34

add tests and a dynamic config section generator

5f5ce51

yajo suggested changes Apr 1, 2019

View reviewed changes

funkyfuture mentioned this pull request May 13, 2019

I'd need finer-grained restrictions #15

Open

joao-p-marques added a commit that referenced this pull request Nov 9, 2020

Add first version of tests

0a0b6f8

From #14

joao-p-marques mentioned this pull request Nov 9, 2020

Add tests and CI #34

Merged

6 tasks

joao-p-marques closed this Jan 28, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add tests and a dynamic config section generator #14

Add tests and a dynamic config section generator #14

kevin-hanselman commented Mar 30, 2019

yajo left a comment

yajo Apr 1, 2019

kevin-hanselman Apr 2, 2019

yajo Apr 2, 2019

yajo Apr 1, 2019

kevin-hanselman Apr 2, 2019

yajo Apr 2, 2019

yajo Apr 1, 2019

yajo Apr 1, 2019

yajo Apr 1, 2019

kevin-hanselman Apr 2, 2019

yajo Apr 1, 2019

yajo Apr 1, 2019

kevin-hanselman commented Apr 3, 2019

yajo commented Apr 4, 2019

joao-p-marques commented Jan 28, 2021

		return parser.parse_args()


		def parse_api_versions(api_versions):

Add tests and a dynamic config section generator #14

Add tests and a dynamic config section generator #14

Conversation

kevin-hanselman commented Mar 30, 2019

yajo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevin-hanselman commented Apr 3, 2019

yajo commented Apr 4, 2019

joao-p-marques commented Jan 28, 2021