Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enhance authz interface for policies #1

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

enhance authz interface for policies #1

wants to merge 2 commits into from

Conversation

mapuri
Copy link

@mapuri mapuri commented Jan 16, 2016

This patch contains changes to:

  • introduce a policy map (as discussed in our earlier meetings and inline with discussion here Proposal: Policy Extension Point moby/moby#18647 (comment))
  • the authz plugin interface to allow passing policies from plugin to daemon
    • additionally, since multiple authz plugins can be chained, the policies are also passed to subsequent plugins in the chain giving them a chance to update the policy, if needed
  • add the policies returned by the authz plugin to the container's config. This is visible when we do docker inspect.

Next steps:

  • review this PR and see if it is inline with what we want to propose to docker folks
  • update the documentation on authz plugins
  • I will continue to work on implementing a authz plugin (in contiv/policyengine repo) that adheres to the updated interface
    • and possibly incorporate the structures from @jainvipin 's (soon to be sent) PR on concrete policy structures, discussion for which is already undergoing I believe on github-gist
  • make changes to libnetwork and volume driver interface to accept and pass policies down to drivers
  • see if we can implement the authz interface in volplugin and docknetplugin itself to avoid having a yet another daemon for storage and network policies.

Note: some of the changes are in vendor directory which implies this will possibly be merged as separate PRs in docker, if it get's accepted that is :).

/cc @shaleman @jainvipin @erikh @unclejack

@mapuri mapuri force-pushed the authz_work branch 3 times, most recently from 7105dbc to d176624 Compare January 16, 2016 01:57
@jainvipin
Copy link

@mapuri - basic structure looks right to me - thanks! Some comments:

// set container policies associated by authz plugin(s)
policies, ok := ctx.Value("policies").(map[container.PolicyType]string)
if !ok {
logrus.Warnf("incorrect type for policy map %T, expected %T", ctx.Value("policies"), config.Policies)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious why warn here? if policies are not associated with the ctx, then it is ok right?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is just a log message for type assertion failure.

Yes, it is ok for no policies to be associated. If policies are not associated then we just set the map as empty on the next line so that it is not nil.

@mapuri
Copy link
Author

mapuri commented Jan 16, 2016

@jainvipin thanks for the review.

I will update the documentation and test cases as well and push them in this PR.

@mapuri mapuri force-pushed the authz_work branch 7 times, most recently from e5534df to 124246d Compare January 23, 2016 00:26
@mapuri
Copy link
Author

mapuri commented Jan 23, 2016

@jainvipin @shaleman

I have updated the policy related structure to be in line with the proposal here https://gist.github.com/jainvipin/8b1677f041534df576b2

PTAL when you get a chance.

}

type firewallRule struct {
action string

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we incorporate direction and PeerGroupId to the code? Ref the updated doc in the link below...

@mapuri mapuri force-pushed the authz_work branch 2 times, most recently from 483a48d to d9c25e5 Compare February 2, 2016 02:15
@mapuri mapuri force-pushed the authz_work branch 2 times, most recently from 8399cb4 to 47346b8 Compare February 29, 2016 20:29
…o daemon

Also hooked container create handler to set the policies in container config.

Signed-off-by: Madhav Puri <[email protected]>
@mapuri mapuri force-pushed the authz_work branch 3 times, most recently from 8d94690 to bafd8e6 Compare March 8, 2016 23:16
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