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

Add support privilege when create service #1129

Closed
wants to merge 1 commit into from

Conversation

jimmyxian
Copy link
Contributor

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;
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

@jimmyxian
Copy link
Contributor Author

@aaronlehmann Review again. Thanks

Signed-off-by: Xian Chaobo <xianchaobo@huawei.com>
@codecov-io
Copy link

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
Copy link
Contributor Author

ping @stevvooe @aluzzardi @aaronlehmann

@stevvooe
Copy link
Contributor

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

@dperny
Copy link
Collaborator

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
Copy link
Member

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

@stevvooe
Copy link
Contributor

@aluzzardi What about the profile proposal?

@diogomonica
Copy link
Contributor

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.

@ghost
Copy link

ghost 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
Copy link
Contributor

@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
Copy link
Member

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

@tiny1990
Copy link

@jimmyxian not merge?

@kent-h
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
Copy link

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
Copy link

ping

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

Successfully merging this pull request may close these issues.

None yet