-
Notifications
You must be signed in to change notification settings - Fork 1.1k
ACLs in Swarm see #1199 (ReOpen for #1365) #1366
Conversation
448de54
to
de27892
Compare
@vieux @aluzzardi should this be looked at together with moby/moby#13697 and moby/moby#17412 (@NathanMcCauley is currently working with them on that proposal) |
@doronp done! :) But it seems that some of your commits are still not signed though :( Also would be better if you can squash some commits (like all those @thaJeztah Thanks this is helpful! We'll take a look at it as this seems to be closely related. |
FYI there's also moby/moby#17412 and moby/moby#17687 dealing with this. |
@chanwit : Added BATS tests, however since the tests are run without the --multiTenant flag they fail and hence the build fails. Can you put the Flag on for those tests? |
@doronp Digging into this one. We are seeing the first bits being merged in docker with moby/moby#15365 (for |
Indeed it would be neat to harness the docker side authZ support. 1 - Swarm will pass the requests to the engines in its cluster and those will communicate with the AuthZ back-end via the plugin - Swarm will then aggregate and handle the responses. I suspect there will be lots of inefficacy in this approach - Swarm will have to access the engines directly every time - it would not be able to use the internal cluster data as to complete operation as it does today*. This will probably have tremendous performance impact especially on bigger cluster (which is a reasonable assumption if someone uses ACL at all) *We can keep the operations such as ps in Swarm and only make it do AuthZ via the engines but it is still inefficient. 2 - Take the AuthZ(+AuthN) plugin from docker and make swarm do that job itself - Swarm is just like an engine in that case I am going on digging the possible implementation options. let me know what you think. |
879f26f
to
497e328
Compare
@vieux @abronan @aluzzardi commit 497e328 is the plugin package import - It does not interfere with functionality and passes all tests. It will help a lot to review and merge it ASAP - better not mix it with the AuthZ plugin code (last commit is WIP) Thanks. |
c83d47b
to
2ebbb60
Compare
ping @docker/swarm-maintainers |
c916115
to
d09fab7
Compare
@MHBauer There was a huge rebase maily #1880 That made separating Godeps and AuthZ plugin actual usage less relevant. About the next step of actually importing and using AuthZ plugin from the Engine: cannot save github.com/docker/docker/pkg/authorization at revision 6eb38359954269103f2a3094283c4cb0a1cb63f9: already have github.com/docker/docker/pkg/discovery at revision b65fd8e879545e8c9b859ea9b6b825ac50c79e46. Does that mean that docker authZ and swarm should decide on one revision? Its a bit of a blocker... About the failing tests - I am not sure this is related to my changes. |
@@ -7,6 +7,8 @@ import ( | |||
"net/http/pprof" | |||
|
|||
log "github.com/Sirupsen/logrus" | |||
// "github.com/docker/docker/pkg/authorization" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't you remove it for now instead of commenting out ? Did you comment out the sections only due to the godep issue you mentioned above?
@ezrasilvera Good catch. The comment out is due to Godeps indeed. Without that code there is no commit really, with it the error you get is misleading since I could not update the Godeps yet. I will try to determine if I can persuade one side to use the others version to resolve that. |
d1d732a
to
419dc7f
Compare
So I changed the revision in the Swarm side it compiles now with the AuthZ import and passes basic manual tests. |
@MHBauer Ping |
Be forewarned, moby/moby#21897 is about to move the auth stuff around. |
I think the point of #1366 (comment) was if we could just init the Middleware directly without copying the init into swarm. |
We'd have to reach directly into I can see that was considered, because it is commented out in |
@MHBauer The reason In order to avoid that (almost a) copy there needs to be done another change of behaviour in Swarm which is that the handler being used should be able to return error and be like the one in Docker. When that is done the behaviour could be the same as in Dockers middle wear. And then we can avoid that private function there and use Dockers. I was planning to do that on another PR since it is not a small change. Cleaned and re based. |
… from docker engine Signed-off-by: Doron Podoleanu <doronp@il.ibm.com>
@dongluochen - @abronan advised me you can help with this. What are the next steps on my side? 10x. |
Hi, may I ask what status of the ACL integration for Swarm is right now? I have developed a Docker auth plugin called casbin-authz-plugin, and I want it to work on Docker Swarm too. What is the suggested way to do this? Thanks! |
@hsluoyz Is your plugin for Docker Swarm-mode? You can find the difference of classic Swarm and Swarm-mode here. You may open issue at https://github.com/moby/moby for Swarm mode authentication. |
Hi @dongluochen ,
No, my plugin is still a local Docker engine plugin base on: https://github.com/docker/go-plugins-helpers/tree/master/authorization
So I think I should develop the auth plugin based on the new Swarm-mode on the moby repo.
I didn't find any information about how to develop an authorization plugin for Docker Swarm mode, I will open an issue in moby to ask about it as you suggested, thanks! |
ACLs in Swarm see #1199 #1365
Authorization (ACLs) support in Swarm as a first step towards multi tenancy. #1199
This PR includes (organized by Commits for convenience)
1 - Validation API & Default implementation- Approve/Disapprove/condition of 'Who wants to to what'?
2 - Handler API & Default implementation - Use labels to enforce decision of 1
3 - Feature flag --multiTenant to get this feature going (regular Swarm otherwise)
(Re open PR for internal PM reasons)