Add all securityContext fields in injected containers by rlenglet · Pull Request #17427 · istio/istio

rlenglet · 2019-09-26T22:46:14Z

[x] Configuration Infrastructure
[ ] Docs
[ ] Installation
[ ] Networking
[ ] Performance and Scalability
[ ] Policies and Telemetry
[ ] Security
[ ] Test and Release
[ ] User Experience
[ ] Developer Infrastructure

rlenglet · 2019-09-26T22:52:40Z

/test pilot-e2e-envoyv2-v1alpha3

rlenglet · 2019-09-27T04:06:24Z

/test lint

jammerful · 2019-09-30T15:03:56Z

@rlenglet I'm interested in seeing this PR merged, as right now istio doesn't work with Pod Security Policy even with the istio cni installed. Do you have a sense of when this might be merged and when this might make it into a release? Thanks.

rlenglet · 2019-09-30T15:26:49Z

@jammerful you can apply a similar change into your deployment's injection template. You don't have to wait for a new release to apply this change locally. I'm expecting to ship this in 1.3.3, around mid-October.

jammerful · 2019-09-30T15:39:25Z

@rlenglet I was trying to avoid having to change the config map for the side car injector, though I will test that this changed does allow my pod security policy to be used.
On a side note, I stumbled upon re-invocation of mutating admission controllers, is it correct if the pod security policy's mutating admission controller is marked a reinvocationPolicy: "IfNeeded" the istio side car injector's and the pod security policy's mutating admission controllers will work together allowing, technically not requiring setting the fields in this PR explicitly? That being said I still like the fields being set explicitly as it is deterministic.

rlenglet · 2019-09-30T19:55:08Z

On a side note, I stumbled upon re-invocation of mutating admission controllers, is it correct if the pod security policy's mutating admission controller is marked a reinvocationPolicy: "IfNeeded" the istio side car injector's and the pod security policy's mutating admission controllers will work together allowing, technically not requiring setting the fields in this PR explicitly?

That is very interesting, thanks. However, users may have other admission controllers that could trigger a re-invocation, so I think we can't avoid setting the fields.

jammerful · 2019-10-01T03:21:27Z

That is very interesting, thanks. However, users may have other admission controllers that could trigger a re-invocation, so I think we can't avoid setting the fields.

Yup this is still needed, as I noted this ensures that istio always runs with the minimal set of permissions (principle of least privilege) no matter what the users kubernetes configuration is (that is what I meant by "deterministic"). In any case, please let me know if you need any help with this, I would be happy to help out to get this over the line.

howardjohn · 2019-10-06T21:37:12Z

lgtm, mention me once its not WIP if you need an approval

ericvn · 2019-10-19T18:19:12Z

Note that this PR is missing the cherrypick label for 1.4. I assume it should be added there as well.

thecodejunkie · 2019-10-28T08:46:44Z

Running into this issue as well, in a PSP enabled cluster with automatic sidecar injection. Unable to validate against any PSP -> ReplicaSet fails to create pods. Will this be fixed for 1.4?

rlenglet · 2019-12-28T01:47:17Z

/test e2e-dashboard_istio

rlenglet · 2019-12-28T04:56:49Z

The automated tests are passing.
And I manually tested that this PR allows running pods with automatic injection and Istio CNI enabled with a very restrictive PodSecurityPolicy.

rlenglet · 2019-12-28T05:02:11Z

I tested that this simpler capabilities combination works:

        securityContext:
          capabilities:
            add:
            - NET_ADMIN
            - NET_RAW
            drop:
            - ALL

I will update the PR.

Fixes #17318

rlenglet · 2019-12-28T06:29:59Z

@howardjohn @istio/wg-environments-maintainers @istio/wg-user-experience-maintainers This is ready for review. I am still working on the matching istio/installer PR. Thanks!

istio-testing · 2019-12-28T06:31:30Z

@rlenglet: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
e2e-bookInfoTests-trustdomain_istio	`188e736`	link	`/test e2e-bookInfoTests-trustdomain`

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

rlenglet · 2019-12-28T06:37:12Z

/test pilot-multicluster-e2e_istio

istio-testing · 2019-12-28T15:15:08Z

In response to a cherrypick label: #17427 failed to apply on top of branch "release-1.3":

Applying: Update injection unit tests
Using index info to reconstruct a base tree...
A	pkg/kube/inject/testdata/inject/app_probe/hello-probes-with-flag-set-in-annotation.yaml.injected
A	pkg/kube/inject/testdata/inject/app_probe/hello-probes-with-flag-unset-in-annotation.yaml.injected
A	pkg/kube/inject/testdata/inject/app_probe/hello-probes.yaml.injected
A	pkg/kube/inject/testdata/inject/app_probe/hello-readiness.yaml.injected
A	pkg/kube/inject/testdata/inject/app_probe/https-probes.yaml.injected
A	pkg/kube/inject/testdata/inject/app_probe/named_port.yaml.injected
A	pkg/kube/inject/testdata/inject/app_probe/one_container.yaml.injected
A	pkg/kube/inject/testdata/inject/app_probe/ready_live.yaml.injected
A	pkg/kube/inject/testdata/inject/app_probe/ready_only.yaml.injected
A	pkg/kube/inject/testdata/inject/app_probe/two_container.yaml.injected
A	pkg/kube/inject/testdata/inject/auth.cert-dir.yaml.injected
A	pkg/kube/inject/testdata/inject/auth.non-default-service-account.yaml.injected
A	pkg/kube/inject/testdata/inject/auth.yaml.injected
A	pkg/kube/inject/testdata/inject/cronjob-with-app.yaml.injected
A	pkg/kube/inject/testdata/inject/cronjob.yaml.injected
A	pkg/kube/inject/testdata/inject/daemonset.yaml.injected
A	pkg/kube/inject/testdata/inject/deploymentconfig-multi.yaml.injected
A	pkg/kube/inject/testdata/inject/deploymentconfig.yaml.injected
A	pkg/kube/inject/testdata/inject/enable-core-dump.yaml.injected
A	pkg/kube/inject/testdata/inject/format-duration.yaml.injected
A	pkg/kube/inject/testdata/inject/frontend.yaml.injected
A	pkg/kube/inject/testdata/inject/hello-always.yaml.injected
A	pkg/kube/inject/testdata/inject/hello-config-map-name.yaml.injected
A	pkg/kube/inject/testdata/inject/hello-ignore.yaml.injected
A	pkg/kube/inject/testdata/inject/hello-mtls-not-ready.yaml.injected
A	pkg/kube/inject/testdata/inject/hello-multi.yaml.injected
A	pkg/kube/inject/testdata/inject/hello-namespace.yaml.injected
A	pkg/kube/inject/testdata/inject/hello-never.yaml.injected
A	pkg/kube/inject/testdata/inject/hello-proxy-override.yaml.injected
A	pkg/kube/inject/testdata/inject/hello-template-in-values.yaml.injected
A	pkg/kube/inject/testdata/inject/hello-tproxy-debug.yaml.injected
A	pkg/kube/inject/testdata/inject/hello-tproxy.yaml.injected
A	pkg/kube/inject/testdata/inject/hello.yaml.injected
A	pkg/kube/inject/testdata/inject/job.yaml.injected
A	pkg/kube/inject/testdata/inject/kubevirtInterfaces.yaml.injected
A	pkg/kube/inject/testdata/inject/kubevirtInterfaces_list.yaml.injected
A	pkg/kube/inject/testdata/inject/list-frontend.yaml.injected
A	pkg/kube/inject/testdata/inject/list.yaml.injected
A	pkg/kube/inject/testdata/inject/multi-container.yaml.injected
A	pkg/kube/inject/testdata/inject/multi-init.yaml.injected
A	pkg/kube/inject/testdata/inject/pod-with-app.yaml.injected
A	pkg/kube/inject/testdata/inject/pod.yaml.injected
A	pkg/kube/inject/testdata/inject/replicaset.yaml.injected
A	pkg/kube/inject/testdata/inject/replicationcontroller.yaml.injected
A	pkg/kube/inject/testdata/inject/statefulset.yaml.injected
A	pkg/kube/inject/testdata/inject/status_annotations.yaml.injected
A	pkg/kube/inject/testdata/inject/status_params.yaml.injected
A	pkg/kube/inject/testdata/inject/traffic-annotations-empty-includes.yaml.injected
A	pkg/kube/inject/testdata/inject/traffic-annotations-wildcards.yaml.injected
A	pkg/kube/inject/testdata/inject/traffic-annotations.yaml.injected
A	pkg/kube/inject/testdata/inject/traffic-params-empty-includes.yaml.injected
A	pkg/kube/inject/testdata/inject/traffic-params.yaml.injected
A	pkg/kube/inject/testdata/webhook/daemonset.yaml.injected
A	pkg/kube/inject/testdata/webhook/deploymentconfig-multi.yaml.injected
A	pkg/kube/inject/testdata/webhook/deploymentconfig.yaml.injected
A	pkg/kube/inject/testdata/webhook/frontend.yaml.injected
A	pkg/kube/inject/testdata/webhook/hello-config-map-name.yaml.injected
A	pkg/kube/inject/testdata/webhook/hello-mtls-not-ready.yaml.injected
A	pkg/kube/inject/testdata/webhook/hello-multi.yaml.injected
A	pkg/kube/inject/testdata/webhook/hello-probes.yaml.injected
A	pkg/kube/inject/testdata/webhook/job.yaml.injected
A	pkg/kube/inject/testdata/webhook/list-frontend.yaml.injected
A	pkg/kube/inject/testdata/webhook/list.yaml.injected
A	pkg/kube/inject/testdata/webhook/multi-init.yaml.injected
A	pkg/kube/inject/testdata/webhook/replicaset.yaml.injected
A	pkg/kube/inject/testdata/webhook/replicationcontroller.yaml.injected
A	pkg/kube/inject/testdata/webhook/resource_annotations.yaml.injected
A	pkg/kube/inject/testdata/webhook/statefulset.yaml.injected
A	pkg/kube/inject/testdata/webhook/status_annotations.yaml.injected
A	pkg/kube/inject/testdata/webhook/traffic-annotations-empty-includes.yaml.injected
A	pkg/kube/inject/testdata/webhook/traffic-annotations-wildcards.yaml.injected
A	pkg/kube/inject/testdata/webhook/traffic-annotations.yaml.injected
A	pkg/kube/inject/testdata/webhook/user-volume.yaml.injected
Falling back to patching base and 3-way merge...
CONFLICT (modify/delete): pkg/kube/inject/testdata/webhook/hello-mtls-not-ready.yaml.injected deleted in HEAD and modified in Update injection unit tests. Version Update injection unit tests of pkg/kube/inject/testdata/webhook/hello-mtls-not-ready.yaml.injected left in tree.
CONFLICT (modify/delete): pkg/kube/inject/testdata/inject/hello-mtls-not-ready.yaml.injected deleted in HEAD and modified in Update injection unit tests. Version Update injection unit tests of pkg/kube/inject/testdata/inject/hello-mtls-not-ready.yaml.injected left in tree.
Auto-merging pilot/pkg/kube/inject/testdata/webhook/user-volume.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/webhook/traffic-annotations.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/webhook/traffic-annotations-wildcards.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/webhook/traffic-annotations-empty-includes.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/webhook/status_annotations.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/webhook/statefulset.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/webhook/resource_annotations.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/webhook/replicationcontroller.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/webhook/replicaset.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/webhook/multi-init.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/webhook/list.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/webhook/list-frontend.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/webhook/job.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/webhook/hello-probes.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/webhook/hello-multi.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/webhook/hello-config-map-name.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/webhook/frontend.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/webhook/deploymentconfig.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/webhook/deploymentconfig-multi.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/webhook/daemonset.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/traffic-params.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/traffic-params-empty-includes.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/traffic-annotations.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/traffic-annotations-wildcards.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/traffic-annotations-empty-includes.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/status_params.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/status_annotations.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/statefulset.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/replicationcontroller.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/replicaset.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/pod.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/pod-with-app.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/multi-init.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/multi-container.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/list.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/list-frontend.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/kubevirtInterfaces_list.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/kubevirtInterfaces.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/job.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/hello.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/hello-tproxy.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/hello-tproxy-debug.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/hello-template-in-values.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/hello-proxy-override.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/hello-never.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/hello-namespace.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/hello-multi.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/hello-ignore.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/hello-config-map-name.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/hello-always.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/frontend.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/format-duration.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/enable-core-dump.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/deploymentconfig.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/deploymentconfig-multi.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/daemonset.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/cronjob.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/cronjob-with-app.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/auth.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/auth.non-default-service-account.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/auth.cert-dir.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/app_probe/two_container.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/app_probe/ready_only.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/app_probe/ready_live.yaml.injected
CONFLICT (content): Merge conflict in pilot/pkg/kube/inject/testdata/inject/app_probe/ready_live.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/app_probe/one_container.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/app_probe/named_port.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/app_probe/https-probes.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/app_probe/hello-readiness.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/app_probe/hello-probes.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/app_probe/hello-probes-with-flag-unset-in-annotation.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/app_probe/hello-probes-with-flag-set-in-annotation.yaml.injected
error: Failed to merge in the changes.
Patch failed at 0002 Update injection unit tests

istio-testing · 2019-12-28T15:15:19Z

In response to a cherrypick label: new pull request created: #19832

rlenglet added area/config cherrypick/release-1.2 labels Sep 26, 2019

rlenglet requested a review from a team as a code owner September 26, 2019 22:46

istio-testing added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 26, 2019

googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Sep 26, 2019

rlenglet added the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label Sep 27, 2019

rlenglet changed the title ~~Add all securityContext fields in injected containers~~ [WIP] Add all securityContext fields in injected containers Sep 27, 2019

rlenglet force-pushed the add-all-security-context-fields branch from 21e1770 to 495e055 Compare September 27, 2019 01:31

rlenglet requested review from a team, geeknoid, howardjohn, linsun and rshriram as code owners September 27, 2019 01:31

istio-testing added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 27, 2019

rlenglet force-pushed the add-all-security-context-fields branch 2 times, most recently from f56a615 to 188e736 Compare September 27, 2019 02:06

rlenglet mentioned this pull request Oct 18, 2019

Include all securityContext fields in injection template #17318

Closed

ericvn added the cherrypick/release-1.4 label Oct 19, 2019

rlenglet force-pushed the add-all-security-context-fields branch from bdb1c97 to 507d556 Compare December 28, 2019 01:40

rlenglet removed the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label Dec 28, 2019

rlenglet changed the title ~~[WIP] Add all securityContext fields in injected containers~~ Add all securityContext fields in injected containers Dec 28, 2019

rlenglet removed the request for review from geeknoid December 28, 2019 04:58

rlenglet changed the title ~~Add all securityContext fields in injected containers~~ [WIP] Add all securityContext fields in injected containers Dec 28, 2019

istio-testing added the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label Dec 28, 2019

rlenglet added 2 commits December 27, 2019 21:34

Add all securityContext fields in injected containers

754d2ed

Fixes #17318

Update injection unit tests

2b9638d

rlenglet force-pushed the add-all-security-context-fields branch from 6c3fff9 to 2b9638d Compare December 28, 2019 05:53

rlenglet changed the title ~~[WIP] Add all securityContext fields in injected containers~~ Add all securityContext fields in injected containers Dec 28, 2019

istio-testing removed the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label Dec 28, 2019

rlenglet added this to the 1.5 milestone Dec 28, 2019

rlenglet mentioned this pull request Dec 28, 2019

Add all securityContext fields in injected containers istio/installer#628

Merged

liamawhite approved these changes Dec 28, 2019

View reviewed changes

howardjohn approved these changes Dec 28, 2019

View reviewed changes

istio-testing merged commit 99c0ec6 into master Dec 28, 2019

istio-testing deleted the add-all-security-context-fields branch December 28, 2019 15:14

istio-testing mentioned this pull request Dec 28, 2019

[release-1.4] Add all securityContext fields in injected containers #19832

Merged

rlenglet mentioned this pull request Dec 28, 2019

[release-1.3] Add all securityContext fields in injected containers #19836

Merged

rlenglet mentioned this pull request Jan 10, 2020

Control plane components need overlay to run with restrictive PodSecurityPolicy #19720

Closed

This was referenced Feb 27, 2026

🌱 CNCF mission generation 2026-02-27 kubestellar/console-kb#6

Closed

🌱 CNCF mission generation 2026-02-27 kubestellar/console-kb#11

Merged

Conversation

rlenglet commented Sep 26, 2019

Uh oh!

rlenglet commented Sep 26, 2019

Uh oh!

rlenglet commented Sep 27, 2019

Uh oh!

jammerful commented Sep 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rlenglet commented Sep 30, 2019

Uh oh!

jammerful commented Sep 30, 2019

Uh oh!

rlenglet commented Sep 30, 2019

Uh oh!

jammerful commented Oct 1, 2019

Uh oh!

howardjohn commented Oct 6, 2019

Uh oh!

ericvn commented Oct 19, 2019

Uh oh!

thecodejunkie commented Oct 28, 2019

Uh oh!

rlenglet commented Dec 28, 2019

Uh oh!

rlenglet commented Dec 28, 2019

Uh oh!

rlenglet commented Dec 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rlenglet commented Dec 28, 2019

Uh oh!

istio-testing commented Dec 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rlenglet commented Dec 28, 2019

Uh oh!

istio-testing commented Dec 28, 2019

Uh oh!

istio-testing commented Dec 28, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

jammerful commented Sep 30, 2019 •

edited

Loading

rlenglet commented Dec 28, 2019 •

edited

Loading

istio-testing commented Dec 28, 2019 •

edited

Loading