Implement Docker's managed plugin format #25
Conversation
Signed-off-by: Nigel Brown <nigel@windsock.io>
nbrownuk
force-pushed
the
implement-managed-plugin
branch
from
e108a8c
to
fe243b5
Compare
|
@nbrownuk thanks for looking into this! I'm planning to review and try it out later this week. |
There was a problem hiding this comment.
@nbrownuk I've left a few comments. Overall, this looks great. Just a few minor thoughts on the build and installation instructions.
It would be good to include a note about how to find the logs for the plugin (I believe they're included w/ the Docker daemon logs--at least this is the case for Ubuntu.)
Makefile
Outdated
| -w /go/src/github.com/open-policy-agent/opa-docker-authz \ | ||
| golang:$(GO_VERSION) \ | ||
| ./build.sh | ||
| @sudo rm -rf ./vendor |
There was a problem hiding this comment.
I think it should be possible to avoid use of sudo in the build process. I would much rather prefer if the makefile could run without sudo. Can you refactor the build to avoid this?
| @@ -1,12 +1,46 @@ | |||
| .PHONY: all build | |||
|
|
|||
| VERSION := 0.2.2 | |||
| OPA_VERSION := 0.8.0 | |||
| GO_VERSION := 1.10 | |||
| REPO := openpolicyagent/opa-docker-authz | |||
There was a problem hiding this comment.
Perhaps we could have two repos: openpolicyagent/opa-docker-authz for the legacy (since it already exists, I am not sure we can change it) and openpolicyagent/opa-docker-authz-v2 for the managed plugin. WDYT?
There was a problem hiding this comment.
Seems sensible to me.
README.md
Outdated
| To have the `opa-docker-authz` plugin make use of the user-defined policy, specify the additional arguments during plugin installation: | ||
|
|
||
| ``` | ||
| $ docker plugin install openpolicyagent/opa-docker-authz:0.2.2 \ |
There was a problem hiding this comment.
I think it would be better if this step replaced the one above. As is, users will always need to create a policy file and then signal docker. We could make this easier to use by updating the plugin to fail-open/allow requests IF the policy file is missing (i.e., before reading the file, check if it exists, if not, allow the request, and log the behaviour.)
Dockerfile
Outdated
|
|
||
| MAINTAINER Torin Sandall <torinsandall@gmail.com> | ||
| RUN echo -e 'package docker.authz\n\ |
There was a problem hiding this comment.
See note below about making the installation instructions include creation of a policy. If we do that, this should be removed.
Removed the requirement for 'sudo' in the Makefile, altered plugin to fail open and authorize requests in the absence of a policy file, removed the creation of default policy in the Dockerfile, and updated the README with logging information and to reflect the various changes. Signed-off-by: Nigel Brown <nigel@windsock.io>
|
@tsandall Hopefully I addressed all of your concerns! Let me know what you think. |
|
@nbrownuk thanks for this work! I've cut the 0.3 release and pushed legacy and v2 images to docker hub. If you have time to test them out over the next little while, that would be much appreciated. Cheers! |
|
@tsandall Tested both new versions this morning, and all was well. If you need some help updating the tutorial in the docs, just let me know. |
Configures
opa-docker-authzas a Docker managed plugin (v2), and fixes #4.The changes allow the legacy plugin image to be created, in addition to the new v2 plugin, for backwards compatibility. The only issue is that both can't have the same name (i.e. openpolicyagent/opa-docker-authz). Maybe the managed plugin can be hosted on the Docker Store instead of the Docker Hub, where there are currently zero authorization plugins?