Add securityContext to Knative Eventing core objects #5863

evankanderson · 2021-11-01T15:51:17Z

Proposed Changes

🧹 Set SecurityContext on all resources installed by core or in-memory controllers.
- This should allow running pods under the restricted Pod Security Standard

Pre-review Checklist

[na] At least 80% unit test coverage
[na] E2E tests for any new behavior
[na] Docs PR for any user-facing impact
[na] Spec PR for any new API feature
[na] Conformance test for any change to the spec

Release Note

All core Knative Eventing Pods should now be able to run in the restricted pod security standard profile.

Docs

No docs impact

codecov · 2021-11-01T15:58:39Z

Codecov Report

Merging #5863 (b0ba615) into main (4de0da0) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #5863   +/-   ##
=======================================
  Coverage   81.96%   81.96%           
=======================================
  Files         220      220           
  Lines        7458     7458           
=======================================
  Hits         6113     6113           
  Misses        918      918           
  Partials      427      427

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4de0da0...b0ba615. Read the comment docs.

evankanderson · 2021-11-01T16:19:30Z

Apparently I can cut and paste runasNonRoot very effectively. Corrected!

evankanderson · 2021-11-01T18:05:45Z

/assign @lionelvillard

odacremolbap

Not sure if intended, but while heartbeats manifest has been updated with resources/security, appender and event_display has not. recordevents (not sure if that's generally considered a tool people could use or an internal testing tool only) also lacks them.

evankanderson · 2021-11-02T05:49:32Z

I thought that heartbeats was related to one of the GA components, while recordeevents and appender did not seem to be documented on knative.dev.

evankanderson · 2021-11-02T05:50:04Z

/assign @julz @devguyio

matzew · 2021-11-02T07:23:21Z

/assign

I will take a look at this

matzew · 2021-11-02T07:31:49Z

/hold

matzew · 2021-11-02T08:03:55Z

config/brokers/mt-channel-broker/deployments/broker-filter.yaml

+          readOnlyRootFilesystem: true
+          runAsNonRoot: true


some of this was already done before and got reverted - see: https://github.com/knative/eventing/pull/5629/files

FWIW we reverted because we didn't have time - at the time - to figure out mem/cpu requests that were low enough to work reliably in GitHub actions, the securityContext stuff should IMO be good either way, and I think having resources present - even if low - is better than not having them at all (so being entirely at the mercy of the scheduler happening not to collocate too many jobs on a node).

tl;dr imo we should just make sure this reliably works in GitHub actions, and lower mem/cpu until it does :)

matzew · 2021-11-02T08:49:59Z

2021/11/02 09:08:48 error processing import paths in "config/500-controller.yaml": error resolving image references: Put https://index.docker.io/v2/matzew/apiserver_receive_adapter-53e790fb0f6460420543f068b229321d/blobs/uploads/03d941d0-a316-469c-8740-a659a9f27e49?_state=0lAdx7jka1qpJmLkNLSzDpjLciu6ww-yU6JXcCYOVdt7Ik5hbWUiOiJtYXR6ZXcvYXBpc2VydmVyX3JlY2VpdmVfYWRhcHRlci01M2U3OTBmYjBmNjQ2MDQyMDU0M2YwNjhiMjI5MzIxZCIsIlVVSUQiOiIwM2Q5NDFkMC1hMzE2LTQ2OWMtODc0MC1hNjU5YTlmMjdlNDkiLCJPZmZzZXQiOjIzNTA3NTY2LCJTdGFydGVkQXQiOiIyMDIxLTExLTAyVDA4OjA3OjU5WiJ9&digest=sha256%3A8f43c9c4ec1e0540726c3a0ba0c218f38189d29581240ab48168bcce899c815d: EOF

hrm - getting that from ko apply ... I guess unrelated... but...

matzew · 2021-11-02T09:39:23Z

config/channels/in-memory-channel/deployments/controller.yaml

+          requests:
+            cpu: 100m
+            memory: 100Mi


as said before, not sure we really have facts for bumping the numbers to this

fwiw I think any request is better than no request at all here, otherwise it really is just luck causing the controller to be on a node with any mem/cpu left at all iyswim

These requests were copied from serving; I agree that they could probably be tuned (lower, I'm hoping), but leaving them out entirely doesn't do the scheduler any favors (and just pushes the issue to any responsible downstream distributors).

I agree that it would have been good if this had been in earlier in the release cycle; I forgot about it until 2 days ago.

evankanderson · 2021-11-02T13:54:13Z

Do you want me to split this into two (securityContext and resources) if you want, though it seems unlikely that either will be part of the release.

evankanderson · 2021-11-02T16:15:47Z

I'm going to make this one be just securityContext and open a new one for resources.

Checking in now will give max time to bake for 1.1

evankanderson · 2021-11-02T21:21:20Z

Nov  2 16:41:17.698 install_latest_release [OUT] Installing Knative Eventing from: latest-release
Nov  2 16:41:17.702 install_latest_release [OUT] --2021-11-02 16:41:17--  https://github.com/knative/eventing/releases/download/v0.27.0/eventing.yaml
Nov  2 16:41:17.710 install_latest_release [OUT] Resolving github.com (github.com)... 140.82.112.4
Nov  2 16:41:17.737 install_latest_release [OUT] Connecting to github.com (github.com)|140.82.112.4|:443... connected.
Nov  2 16:41:17.867 install_latest_release [OUT] HTTP request sent, awaiting response... 404 Not Found
Nov  2 16:41:17.986 install_latest_release [OUT] 2021-11-02 16:41:17 ERROR 404: Not Found.

I don't think that's this PR.

/retest pull-knative-eventing-upgrade-tests

knative-prow-robot · 2021-11-02T21:21:22Z

@evankanderson: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:

/test pull-knative-eventing-build-tests
/test pull-knative-eventing-conformance-tests
/test pull-knative-eventing-integration-tests
/test pull-knative-eventing-reconciler-tests
/test pull-knative-eventing-unit-tests
/test pull-knative-eventing-upgrade-tests

The following commands are available to trigger optional jobs:

/test pull-knative-eventing-go-coverage

Use /test all to run all jobs.

In response to this:

Nov  2 16:41:17.698 install_latest_release [OUT] Installing Knative Eventing from: latest-release
Nov  2 16:41:17.702 install_latest_release [OUT] --2021-11-02 16:41:17--  https://github.com/knative/eventing/releases/download/v0.27.0/eventing.yaml
Nov  2 16:41:17.710 install_latest_release [OUT] Resolving github.com (github.com)... 140.82.112.4
Nov  2 16:41:17.737 install_latest_release [OUT] Connecting to github.com (github.com)|140.82.112.4|:443... connected.
Nov  2 16:41:17.867 install_latest_release [OUT] HTTP request sent, awaiting response... 404 Not Found
Nov  2 16:41:17.986 install_latest_release [OUT] 2021-11-02 16:41:17 ERROR 404: Not Found.

I don't think that's this PR.

/retest pull-knative-eventing-upgrade-tests

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.

evankanderson · 2021-11-02T23:24:52Z

/test pull-knative-eventing-upgrade-tests

evankanderson · 2021-11-03T03:49:28Z

(This needs knative/hack#101 to pass)

evankanderson · 2021-11-03T17:32:15Z

/test pull-knative-eventing-upgrade-tests

evankanderson · 2021-11-03T20:36:03Z

(Tests now pass)

evankanderson · 2021-11-05T23:35:54Z

@matzew

Does making this just contain securityContext changes and having it early in the cycle reduce your concerns enough to remove the hold (and maybe approve it)?

matzew · 2021-11-08T17:45:03Z

/unhold

odacremolbap

+1

(although I liked the resources version best, even if they were only a starting point to to get tuned later)

knative-prow-robot · 2021-11-08T20:07:24Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: evankanderson, odacremolbap

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [evankanderson]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

* adding/updating securityContext, as needed, to allow running as 'restricted' standard. * adding seccompProfile PR references from knative/eventing repo: * knative/eventing#5863 * knative/eventing#6533 Signed-off-by: Matthias Wessendorf <mwessend@redhat.com>

* 🛂 Addressing PodSecurity violation warnings: * adding/updating securityContext, as needed, to allow running as 'restricted' standard. * adding seccompProfile PR references from knative/eventing repo: * knative/eventing#5863 * knative/eventing#6533 Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * revert zipkin changes Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Update control-plane/config/post-install/500-storage-version-migrator.yaml Co-authored-by: Pierangelo Di Pilato <pierangelodipilato@gmail.com> Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> Co-authored-by: Pierangelo Di Pilato <pierangelodipilato@gmail.com>

* adding/updating securityContext, as needed, to allow running as 'restricted' standard. * adding seccompProfile PR references from knative/eventing repo: * knative/eventing#5863 * knative/eventing#6533 Signed-off-by: Matthias Wessendorf <mwessend@redhat.com>

* passport_control: Addressing PodSecurity violation warnings: * adding/updating securityContext, as needed, to allow running as 'restricted' standard. * adding seccompProfile PR references from knative/eventing repo: * knative/eventing#5863 * knative/eventing#6533 Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * revert zipkin changes Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Update control-plane/config/post-install/500-storage-version-migrator.yaml Co-authored-by: Pierangelo Di Pilato <pierangelodipilato@gmail.com> Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> Co-authored-by: Matthias Wessendorf <mwessend@redhat.com> Co-authored-by: Pierangelo Di Pilato <pierangelodipilato@gmail.com>

google-cla bot added the cla: yes Indicates the PR's author has signed the CLA. label Nov 1, 2021

knative-prow-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Nov 1, 2021

evankanderson force-pushed the kube-from-the-dryer branch from b5f22ad to 2b05b8a Compare November 1, 2021 16:13

Add securityContext and resources to Knative Eventing core objects

4e10b12

evankanderson force-pushed the kube-from-the-dryer branch from 2b05b8a to 4e10b12 Compare November 1, 2021 16:24

knative-prow-robot assigned lionelvillard Nov 1, 2021

odacremolbap reviewed Nov 1, 2021

View reviewed changes

knative-prow-robot assigned devguyio and julz Nov 2, 2021

knative-prow-robot assigned matzew Nov 2, 2021

knative-prow-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 2, 2021

matzew reviewed Nov 2, 2021

View reviewed changes

Evan Anderson added 2 commits November 2, 2021 09:22

Back out resoucre limits, just do securityContexts

ba527b1

Add securityContext to allow running as 'restricted' standard

b0ba615

knative-prow-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 2, 2021

evankanderson changed the title ~~Add securityContext and resources to Knative Eventing core objects~~ Add securityContext to Knative Eventing core objects Nov 5, 2021

knative-prow-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 8, 2021

odacremolbap approved these changes Nov 8, 2021

View reviewed changes

knative-prow-robot assigned odacremolbap Nov 8, 2021

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 8, 2021

xtreme-sameer-vohra mentioned this pull request Nov 8, 2021

All eventing-rabbitmq pods use a restricted pod security standard profile knative-extensions/eventing-rabbitmq#496

Closed

knative-prow-robot merged commit 36bf0bd into knative:main Nov 8, 2021

ikavgo mentioned this pull request Dec 7, 2021

Add securityContext to Knative Eventing RabbitMQ objects knative-extensions/eventing-rabbitmq#541

Merged

matzew mentioned this pull request Sep 27, 2022

Addressing PodSecurity violation warnings: knative-extensions/eventing-kafka-broker#2685

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add securityContext to Knative Eventing core objects #5863

Add securityContext to Knative Eventing core objects #5863

evankanderson commented Nov 1, 2021 •

edited

Loading

codecov bot commented Nov 1, 2021 •

edited

Loading

evankanderson commented Nov 1, 2021

evankanderson commented Nov 1, 2021

odacremolbap left a comment •

edited

Loading

evankanderson commented Nov 2, 2021

evankanderson commented Nov 2, 2021

matzew commented Nov 2, 2021

matzew commented Nov 2, 2021

matzew Nov 2, 2021

julz Nov 2, 2021

matzew commented Nov 2, 2021

matzew Nov 2, 2021

julz Nov 2, 2021

evankanderson Nov 2, 2021

evankanderson commented Nov 2, 2021

evankanderson commented Nov 2, 2021

evankanderson commented Nov 2, 2021

knative-prow-robot commented Nov 2, 2021

evankanderson commented Nov 2, 2021

evankanderson commented Nov 3, 2021

evankanderson commented Nov 3, 2021

evankanderson commented Nov 3, 2021

evankanderson commented Nov 5, 2021

matzew commented Nov 8, 2021

odacremolbap left a comment

knative-prow-robot commented Nov 8, 2021

Add securityContext to Knative Eventing core objects #5863

Add securityContext to Knative Eventing core objects #5863

Conversation

evankanderson commented Nov 1, 2021 • edited Loading

Proposed Changes

Pre-review Checklist

codecov bot commented Nov 1, 2021 • edited Loading

Codecov Report

evankanderson commented Nov 1, 2021

evankanderson commented Nov 1, 2021

odacremolbap left a comment • edited Loading

Choose a reason for hiding this comment

evankanderson commented Nov 2, 2021

evankanderson commented Nov 2, 2021

matzew commented Nov 2, 2021

matzew commented Nov 2, 2021

matzew Nov 2, 2021

Choose a reason for hiding this comment

julz Nov 2, 2021

Choose a reason for hiding this comment

matzew commented Nov 2, 2021

matzew Nov 2, 2021

Choose a reason for hiding this comment

julz Nov 2, 2021

Choose a reason for hiding this comment

evankanderson Nov 2, 2021

Choose a reason for hiding this comment

evankanderson commented Nov 2, 2021

evankanderson commented Nov 2, 2021

evankanderson commented Nov 2, 2021

knative-prow-robot commented Nov 2, 2021

evankanderson commented Nov 2, 2021

evankanderson commented Nov 3, 2021

evankanderson commented Nov 3, 2021

evankanderson commented Nov 3, 2021

evankanderson commented Nov 5, 2021

matzew commented Nov 8, 2021

odacremolbap left a comment

Choose a reason for hiding this comment

knative-prow-robot commented Nov 8, 2021

evankanderson commented Nov 1, 2021 •

edited

Loading

codecov bot commented Nov 1, 2021 •

edited

Loading

odacremolbap left a comment •

edited

Loading