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

Add support privilege when create service #1129

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
@jimmyxian
Copy link
Contributor

jimmyxian commented Jul 6, 2016

Implement: #1030
Signed-off-by: Xian Chaobo xianchaobo@huawei.com

@@ -175,6 +175,9 @@ message ContainerSpec {

// PullOptions parameterize the behavior of image pulls.
PullOptions pull_options = 10;

// Privileged give extended privileges to the container.
bool Privileged = 11;

This comment has been minimized.

@aaronlehmann

aaronlehmann Jul 6, 2016

Collaborator

Field names in protobuf should be lowercase (privileged). They automatically get converted to go-style CamelCase names when the Go code is generated.

This comment has been minimized.

@jimmyxian

jimmyxian Jul 6, 2016

Contributor

@aaronlehmann Thanks for the review. I will modify this now. :)

@jimmyxian jimmyxian force-pushed the jimmyxian:add-support-privileged branch from c68ad23 to 9342392 Jul 6, 2016

@jimmyxian

This comment has been minimized.

Copy link
Contributor

jimmyxian commented Jul 6, 2016

@aaronlehmann Review again. Thanks

add support privilege to container
Signed-off-by: Xian Chaobo <xianchaobo@huawei.com>

@jimmyxian jimmyxian force-pushed the jimmyxian:add-support-privileged branch from 9342392 to e5fa1d9 Jul 12, 2016

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Jul 12, 2016

Current coverage is 55.09%

Merging #1129 into master will decrease coverage by 0.02%

@@             master      #1129   diff @@
==========================================
  Files            77         77          
  Lines         12079      12080     +1   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits           6658       6655     -3   
- Misses         4505       4508     +3   
- Partials        916        917     +1   

Sunburst

Powered by Codecov. Last updated by f98068e...e5fa1d9

@jimmyxian

This comment has been minimized.

Copy link
Contributor

jimmyxian commented Jul 12, 2016

@stevvooe

This comment has been minimized.

Copy link
Contributor

stevvooe commented Jul 13, 2016

@jimmyxian Adding privileged greatly impacts the security model. Proper considerations need to be made before adding support for this operation.

@dperny

This comment has been minimized.

Copy link
Member

dperny commented Aug 29, 2016

I think this was a pull request before its time. There will come a day when we add --privileged to swarmkit, but today is not that day.

Closing. Feel free to reopen later on (or right now), if you disagree.

@dperny dperny closed this Aug 29, 2016

@aluzzardi

This comment has been minimized.

Copy link
Contributor

aluzzardi commented Aug 29, 2016

The demand for --privileged is pretty strong actually. @mgoelzer: thoughts?

@stevvooe

This comment has been minimized.

Copy link
Contributor

stevvooe commented Aug 29, 2016

@aluzzardi What about the profile proposal?

@diogomonica

This comment has been minimized.

Copy link
Contributor

diogomonica commented Aug 30, 2016

I strongly urge us not to take the easy way out on this one. This will haunt us forever.

If the demand is strong, I suggest we prioritize coming up with decent profiles, or at least an early extensible version that fits the use-case.

@mgoelzer

This comment has been minimized.

Copy link
Contributor

mgoelzer commented Aug 30, 2016

I think we should add --privileged. My concern with finer grained solutions is that, although I agree they are more elegant, it will take a long time to come to agreement on how one should work. What do users do during that period of time?

@stevvooe

This comment has been minimized.

Copy link
Contributor

stevvooe commented Aug 30, 2016

@mgoelzer There are no required changes for the profile model. The profile mode hardwires --privileged to a particular profile. The initial proposal only has default and privileged.

@aluzzardi

This comment has been minimized.

Copy link
Contributor

aluzzardi commented Aug 30, 2016

What I don't like about profiles is that now "privileged" is just a string. The behavior becomes not portable.

@tiny1990

This comment has been minimized.

Copy link

tiny1990 commented Dec 29, 2016

@jimmyxian not merge?

@Kent-H

This comment has been minimized.

Copy link

Kent-H commented May 19, 2017

Have any steps been made towards this? What about the related --cap-add, --cap-drop, --pid=host, & --security-opt? (Listing off the requirements for my current use case.)

Personally, I believe this should have been merged. Mirroring the current docker run capabilities is likely more helpful for end-users than waiting for a security model rewrite (which does not seem to have a timeline).

I tend to view swarm as nothing more than a control layer for docker. Given this, my suggestion is that swarm mode should mirror as much of the docker functionality as possible (i.e. - just store and pass on every docker run parameter, unless there's no direct mapping to swarm mode (--rm, etc.)

My opinion is obviously biased. Could we get this re-opened for discussion?

@lipingxue

This comment has been minimized.

Copy link

lipingxue commented Aug 30, 2017

Is there any progress on this to support --privileged option?

cirocosta pushed a commit to cirocosta/swarmkit that referenced this pull request Jun 4, 2018

cirocosta pushed a commit to cirocosta/swarmkit that referenced this pull request Jun 5, 2018

@codeskyblue

This comment has been minimized.

Copy link

codeskyblue commented Oct 19, 2018

ping

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment