thanks @dsheets for bringing this up! I agree that this is a much more intuitive UX.

My only concern is the failure mode if a docker daemon restarts. If the daemon was using a dynamically enabled authz plugin, I think we should also ensure that the plugin is enabled on restart - I'm not sure what the best way of accomplishing this would be, though.

@riyazdf good catch! I've updated the patch to append an authz plugin to the middleware chain whenever it is enabled (both during startup and after docker plugin install or docker plugin enable). I used the Handle method of the plugin store for this (relied upon by ipam and network plugins for registration). The CLI and daemon.json remain the only ways to explicitly specify the order of authz plugins (if plugins modify content this is important). As such, using the CLI or daemon.json is still best practice and the preferred way to specify authz plugins.

I'm going to be offline until July 7 but @yallop should be able to answer questions or address concerns in the interim.

A few tests failing.

looks like a failure on the v1 authz plugin teardown that affects followup tests, though I'm not sure how this change affects this directly. cc @yallop

18:31:37 ----------------------------------------------------------------------
18:31:37 FAIL: docker_cli_authz_unix_test.go:73: DockerAuthzSuite.TearDownTest
18:31:37 
18:31:37 docker_cli_authz_unix_test.go:76:
18:31:37     s.ds.TearDownTest(c)
18:31:37 environment/clean.go:143:
18:31:37     t.Fatalf("%v", err)
18:31:37 ... Error: json: cannot unmarshal object into Go value of type []types.NetworkResource
18:31:37 
18:31:37 
18:31:37 ----------------------------------------------------------------------
18:31:37 PANIC: docker_cli_authz_unix_test.go:282: DockerAuthzSuite.TestAuthZPluginAPIDenyResponse
18:31:37 
18:31:37 [d36ba59acdb28] waiting for daemon to start
18:31:37 [d36ba59acdb28] daemon started
18:31:37 
18:31:37 [d36ba59acdb28] exiting daemon
18:31:37 ... Panic: Fixture has panicked (see related PANIC)
18:31:37 
18:31:37 ----------------------------------------------------------------------

ping @dsheets can you work on the failing tests?

Yes, I'm looking at the failing test. It looks like the command-line-specified authz plugin is being retained as enabled. This causes the test teardown to fail as the authz plugin disallows access to endpoints like /networks and /plugins which are required for teardown.

The test was failing because the global test daemon and the authz test suite daemon share the plugin spec directory and V1 plugins are activated during queries or retrieval (Get). V1 plugins have no means to disable an installed plugin and so the V1 model and global plugin kinds like authz don't work well together. Any subsystem that retrieved V1 plugins or requested their type would end up activating those plugins and, with global plugin kinds like authz, inserting the plugin into a global processing chain. This caused the available but not-enabled authz plugin under test in docker_cli_authz_unix_test.go to become enabled in the global test daemon during tear down where plugins get enumerated. The plugin would then block further resource enumerations during teardown as designed.

I've fixed this issue by factoring out plugin V2+ handler registration from all-plugin handler registration and only registering an authz V2+ plugin handler to inject middleware. As V1 authz plugins can only be enabled via the daemon command-line, config file, or daemon reload, the handler does not need to run for V1 plugins. V2 plugins have two states: enabled and disabled. If a V2 plugin is enabled, the handler will ensure that it is loaded into the authz middleware chain. If the plugin is disabled, the handler will not be triggered.

ping @cpuguy83 PTAL

Any review?

This PR now depends on #34941. I've fixed a number of pre-existing and latent issues that testing state transitions revealed. There are still at least two additional test cases I would like to write before having this merged but I believe the work on names, v1/v2 collisions, and persistent chains can be reviewed now.

Is this ready to go? Needs a rebase.

I don't think @dsheets wants to work on this any more (correct me if I'm wrong, @dsheets!)

I did a quick rebase https://github.com/djs55/docker/tree/authz-dynamic-enable

To my untrained eye the only place where there was an interesting conflict was in pkg/authorization/plugin.go where ddae20c added a field initErr error to the type authorizationPlugin struct . It seemed in this version to be unused so I removed it in the patch which also removed the once sync.Once from the struct ("authz plugins: use fixed, canonical names")

I will not shepherd this PR any more. I recommend checking the osxfs handover document to see if there were remaining TODO items for this work.

Closing this as this is now quite out of date and no one is actively working on it.