Skip to content
This repository has been archived by the owner on Jun 14, 2018. It is now read-only.

added priviledged=true to the init container and to all tests #922

Merged
merged 4 commits into from
Jul 11, 2017

Conversation

ayj
Copy link
Contributor

@ayj ayj commented Jul 11, 2017

Commit #921 on behalf of @bjartek.

Add privileged=true to init-containers to unblock deployments with SELINUX (e.g. OpenShift). As @bjartek suggested in his original PR, this is functionally sufficient, but longer term someone needs to sort through what exact permissions are needed with SELinux and add the necessary RBAC rules to OpenShift similar to what what it is for Kubernetes in the main repo.

Fixes istio/old_issues_repo#34

@ayj ayj requested a review from rshriram July 11, 2017 16:56
@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

@istio-testing
Copy link
Contributor

Jenkins job pilot/presubmit passed

@kyessenov
Copy link
Contributor

The mystery is still why SELinux requires more capabilities than NET_ADMIN for iptables.
Let's get this in, and solve the mystery once we involve more SELinux users/folks.

@kyessenov kyessenov self-requested a review July 11, 2017 17:55
@istio-testing
Copy link
Contributor

Jenkins job pilot/e2e-smoketest passed

@@ -131,6 +131,7 @@ func injectIntoPodTemplateSpec(p *Params, t *v1.PodTemplateSpec) error {
"capabilities": map[string]interface{}{
"add": []string{"NET_ADMIN"},
},
"privileged": true,
Copy link
Member

Choose a reason for hiding this comment

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

Quick feedback. We know that stuff works in Kube without this. And we know that for OpenShift, we need this. Rather than elevating privileges for all cases, can we constrain this to just OpenShift only? This need not be done in this PR, as I don't want to add onto @bjartek 's commits. Lets get this in and may be figure out an istioctl option for this?

@rshriram rshriram merged commit 12f2435 into istio:master Jul 11, 2017
@ayj ayj deleted the bjartek-pr921 branch July 11, 2017 20:20
@ldemailly
Copy link
Member

next time let's make sure to add the credit in the commit message itself
(and hopefully soon we don't have to do this 'takeover' business)

@bjartek
Copy link

bjartek commented Jul 12, 2017

Let me know if you need more help with istio/Openshift :)

@ldemailly
Copy link
Member

so it turns out this is required after all ? should we revert this ?

https://groups.google.com/d/msg/istio-users/8gI2DdnvOag/Ds_6oG-bBgAJ

@bjartek
Copy link

bjartek commented Jul 14, 2017 via email

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

Successfully merging this pull request may close these issues.

None yet

7 participants