allow dropping securityContext capabilities #11344

psschwei · 2021-05-13T13:49:25Z

Fixes #10812 (part 1)

Proposed Changes

This fix allows dropping security context capabilities from a container via containers[].securityContext.capabilities.drop

Release Note

Allow dropping capabilities from a container's security context

knative-prow-robot · 2021-05-13T13:49:36Z

Welcome @psschwei! It looks like this is your first PR to knative/serving 🎉

knative-prow-robot · 2021-05-13T13:49:37Z

Hi @psschwei. Thanks for your PR.

I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

julz

/ok-to-test

julz · 2021-05-13T13:59:43Z

pkg/apis/serving/fieldmask.go

+// CapabilitiesMask performs a _shallow_ copy of the Kubernetes SecurityContext object to a new
+// Kubernetes SecurityContext object bringing over only the fields allowed in the Knative API. This
+// does not validate the contents or the bounds of the provided fields.


Suggested change

// CapabilitiesMask performs a _shallow_ copy of the Kubernetes SecurityContext object to a new

// Kubernetes SecurityContext object bringing over only the fields allowed in the Knative API. This

// does not validate the contents or the bounds of the provided fields.

// CapabilitiesMask performs a _shallow_ copy of the Kubernetes Capabilities object to a new

// Kubernetes Capabilities object bringing over only the fields allowed in the Knative API. This

// does not validate the contents or the bounds of the provided fields.

julz · 2021-05-13T14:36:59Z

^ looks like a test flake in the new backround rate limit stuff

/test pull-knative-serving-unit-tests

markusthoemmes · 2021-05-17T06:31:13Z

Thanks for doing this! Summoning @dprotaso to gather API feedback.

codecov · 2021-05-17T06:35:45Z

Codecov Report

Merging #11344 (8ee752d) into main (4772eaa) will decrease coverage by 0.06%.
The diff coverage is 81.81%.

❗ Current head 8ee752d differs from pull request most recent head 101e01d. Consider uploading reports for the commit 101e01d to get more accurate results

@@            Coverage Diff             @@
##             main   #11344      +/-   ##
==========================================
- Coverage   87.74%   87.68%   -0.07%     
==========================================
  Files         191      191              
  Lines        9215     9225      +10     
==========================================
+ Hits         8086     8089       +3     
- Misses        876      880       +4     
- Partials      253      256       +3

Impacted Files	Coverage Δ
pkg/apis/serving/fieldmask.go	`94.85% <71.42%> (-0.64%)`	⬇️
pkg/apis/serving/k8s_validation.go	`97.95% <100.00%> (+0.02%)`	⬆️
pkg/reconciler/configuration/configuration.go	`85.93% <0.00%> (-2.35%)`	⬇️
pkg/activator/net/revision_backends.go	`91.51% <0.00%> (-0.90%)`	⬇️

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 4772eaa...101e01d. Read the comment docs.

markusthoemmes · 2021-05-17T12:00:31Z

@psschwei do you mind fixing the code style nits pointed out by the Github Action at https://github.com/knative/serving/pull/11344/checks?check_run_id=2598094297?

psschwei · 2021-05-17T16:18:11Z

/retest

dprotaso · 2021-05-21T01:20:42Z

/lgtm
/approve

@psschwei Can we update the schemas in a follow up PR?

@markusthoemmes are there docs describing how it should be done via tooling?

ie.

serving/config/core/300-resources/service.yaml

Lines 408 to 419 in d9da69e

    
           securityContext: 
        
             description: 'Security options the pod should run with. More info: https://kubernetes.io/docs/concepts/policy/security-context/ More info: https://kubernetes.io/docs/tasks/configure-pod-container/security-context/' 
        
             type: object 
        
             properties: 
        
               readOnlyRootFilesystem: 
        
                 description: Whether this container has a read-only root filesystem. Default is false. 
        
                 type: boolean 
        
               runAsUser: 
        
                 description: The UID to run the entrypoint of the container process. Defaults to user specified in image metadata if unspecified. May also be set in PodSecurityContext.  If set in both SecurityContext and PodSecurityContext, the value specified in SecurityContext takes precedence. 
        
                 type: integer 
        
                 format: int64 
        
             x-kubernetes-preserve-unknown-fields: true

Similar changes should appear in the Configuration & Revision

knative-prow-robot · 2021-05-21T01:20:50Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dprotaso

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:

~~pkg/apis/OWNERS~~ [dprotaso]

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

markusthoemmes · 2021-05-21T06:30:49Z

There are no docs for this just yet. It's on my TODO to make this more collaborator friendly.

psschwei · 2021-05-21T19:26:11Z

@psschwei Can we update the schemas in a follow up PR?

Done -- see #11394

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

knative-prow-robot added area/API API objects and controllers size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 13, 2021

knative-prow-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label May 13, 2021

julz reviewed May 13, 2021

View reviewed changes

knative-prow-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 13, 2021

psschwei force-pushed the add-drop-capability branch from 70ea252 to 84282bc Compare May 13, 2021 14:29

markusthoemmes assigned dprotaso May 17, 2021

allow dropping securityContext capabilities

101e01d

psschwei force-pushed the add-drop-capability branch from 8ee752d to 101e01d Compare May 17, 2021 13:37

psschwei mentioned this pull request May 17, 2021

add flag to disable test cleanup in e2e tests knative/pkg#2124

Closed

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

knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 21, 2021

knative-prow-robot merged commit 1b2b759 into knative:main May 21, 2021

psschwei mentioned this pull request May 21, 2021

Adding securityContext.capabilities.drop to schemas #11394

Merged

psschwei mentioned this pull request May 25, 2021

Add feature gate to allow adding capabilities to securityContext #11410

Merged

psschwei deleted the add-drop-capability branch October 15, 2021 15:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

allow dropping securityContext capabilities #11344

allow dropping securityContext capabilities #11344

Uh oh!

psschwei commented May 13, 2021

Uh oh!

knative-prow-robot commented May 13, 2021

Uh oh!

knative-prow-robot commented May 13, 2021

Uh oh!

julz left a comment

Uh oh!

julz May 13, 2021

Uh oh!

psschwei May 13, 2021

Uh oh!

julz commented May 13, 2021

Uh oh!

markusthoemmes commented May 17, 2021

Uh oh!

codecov bot commented May 17, 2021 •

edited

Loading

Uh oh!

markusthoemmes commented May 17, 2021

Uh oh!

psschwei commented May 17, 2021

Uh oh!

dprotaso commented May 21, 2021

Uh oh!

knative-prow-robot commented May 21, 2021

Uh oh!

markusthoemmes commented May 21, 2021

Uh oh!

psschwei commented May 21, 2021

Uh oh!

Uh oh!

allow dropping securityContext capabilities #11344

allow dropping securityContext capabilities #11344

Uh oh!

Conversation

psschwei commented May 13, 2021

Proposed Changes

Uh oh!

knative-prow-robot commented May 13, 2021

Uh oh!

knative-prow-robot commented May 13, 2021

Uh oh!

julz left a comment

Choose a reason for hiding this comment

Uh oh!

julz May 13, 2021

Choose a reason for hiding this comment

Uh oh!

psschwei May 13, 2021

Choose a reason for hiding this comment

Uh oh!

julz commented May 13, 2021

Uh oh!

markusthoemmes commented May 17, 2021

Uh oh!

codecov bot commented May 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

markusthoemmes commented May 17, 2021

Uh oh!

psschwei commented May 17, 2021

Uh oh!

dprotaso commented May 21, 2021

Uh oh!

knative-prow-robot commented May 21, 2021

Uh oh!

markusthoemmes commented May 21, 2021

Uh oh!

psschwei commented May 21, 2021

Uh oh!

Uh oh!

codecov bot commented May 17, 2021 •

edited

Loading