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.

Sign up for GitHub

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

Jump to bottom

[Pod Security] Baseline + restricted policy checks for seccomp #103341

Merged
merged 2 commits into from
Jul 7, 2021
Merged

[Pod Security] Baseline + restricted policy checks for seccomp #103341

merged 2 commits into from
Jul 7, 2021

Conversation

sejr
Copy link
Contributor

@sejr sejr commented Jun 30, 2021 •
edited
Loading

What type of PR is this?

/kind feature

What this PR does / why we need it:

This PR adds a restricted policy check for seccomp profiles.

Which issue(s) this PR fixes:

Fixes #103205
Fixes #103388

Special notes for your reviewer:

This is a tentative PR, but would like some clarification. From the issue:

  • generateFail should generate test cases
    • pod that doesn't set any seccomp profile (default is unconfined)

Would it make sense to create a helper similar to ensureSELinuxOptions to create this default seccomp profile? I attempted that, but was getting tons of errors relating to other restricted policy tests (ie not seccomp related). For the time being, I don't have a "minimal valid pod" test that would trigger this error.

Does this PR introduce a user-facing change?

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

- [KEP]: https://git.k8s.io/enhancements/keps/sig-auth/2579-psp-replacement

/sig auth security

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/feature Categorizes issue or PR as related to a new feature. sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/security Categorizes an issue or PR as relevant to SIG Security. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jun 30, 2021
@k8s-ci-robot
Copy link
Contributor

@sejr: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

@k8s-ci-robot k8s-ci-robot added the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Jun 30, 2021
@k8s-ci-robot
Copy link
Contributor

Hi @sejr. Thanks for your PR.

I'm waiting for a kubernetes 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.

@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jun 30, 2021
@liggitt liggitt self-assigned this Jun 30, 2021
@liggitt
Copy link
Member

liggitt commented Jun 30, 2021

/ok-to-test

@k8s-ci-robot k8s-ci-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 Jun 30, 2021
@sejr sejr force-pushed the podsecurity-restricted-seccomp branch from de9998e to 12ebe9e Compare June 30, 2021 15:19
tallclair
tallclair reviewed Jun 30, 2021
View reviewed changes
Copy link
Member

@tallclair tallclair left a comment

Choose a reason for hiding this comment

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

Thanks! I think this is actually going to be one of the more complicated checks, requiring multiple versions.

@liggitt liggitt assigned tallclair and unassigned liggitt Jun 30, 2021
@sejr sejr force-pushed the podsecurity-restricted-seccomp branch from 12ebe9e to 5d5959e Compare July 2, 2021 03:30
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 2, 2021
@sejr sejr force-pushed the podsecurity-restricted-seccomp branch 2 times, most recently from 69d47f5 to 6907db4 Compare July 2, 2021 03:33
@sejr

This comment has been minimized.

Sign in to view
@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 2, 2021
@sejr sejr force-pushed the podsecurity-restricted-seccomp branch 2 times, most recently from 60376d3 to 1f37838 Compare July 2, 2021 04:39
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 2, 2021
@sejr sejr force-pushed the podsecurity-restricted-seccomp branch from 9cadce2 to d746dfa Compare July 6, 2021 20:28
liggitt
liggitt reviewed Jul 6, 2021
View reviewed changes
@liggitt
Copy link
Member

liggitt commented Jul 6, 2021

sorry, conflicted with #103517, regenerating fixtures should clear that up (might want to just drop/rebuild the generated commit rather than rebase it)

@sejr sejr force-pushed the podsecurity-restricted-seccomp branch from d746dfa to f2a3643 Compare July 6, 2021 21:34
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 6, 2021
@tallclair tallclair added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Jul 6, 2021
@sejr sejr force-pushed the podsecurity-restricted-seccomp branch 2 times, most recently from 67b99e7 to bed4d5a Compare July 6, 2021 23:01
@sejr sejr force-pushed the podsecurity-restricted-seccomp branch 3 times, most recently from 6f76a0a to 5c01183 Compare July 7, 2021 00:43
@sejr sejr force-pushed the podsecurity-restricted-seccomp branch from 5c01183 to 5bc1dfb Compare July 7, 2021 00:45
@liggitt
Copy link
Member

liggitt commented Jul 7, 2021

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 7, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt, sejr

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:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 7, 2021
@liggitt
Copy link
Member

liggitt commented Jul 7, 2021

k8s.io/kubernetes/vendor/k8s.io/client-go/metadata/metadatainformer: TestMetadataSharedInformerFactory flake

/retest

@sejr sejr force-pushed the podsecurity-restricted-seccomp branch from 5bc1dfb to f905ce8 Compare July 7, 2021 01:35
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 7, 2021
@liggitt
Copy link
Member

liggitt commented Jul 7, 2021

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 7, 2021
@k8s-ci-robot k8s-ci-robot merged commit 9e87082 into kubernetes:master Jul 7, 2021
@sejr sejr deleted the podsecurity-restricted-seccomp branch July 7, 2021 12:06
@liggitt liggitt mentioned this pull request Jul 10, 2021
Closed
12 tasks
@sejr sejr mentioned this pull request Jul 27, 2021
Closed
6 tasks
@ccampo133 ccampo133 mentioned this pull request Sep 16, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liggitt liggitt liggitt left review comments

@tallclair tallclair tallclair left review comments

@deads2k deads2k Awaiting requested review from deads2k

@krmayankk krmayankk Awaiting requested review from krmayankk

Assignees

@liggitt liggitt

@tallclair tallclair

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note-none Denotes a PR that doesn't merit a release note. sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/security Categorizes an issue or PR as relevant to SIG Security. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
SIG Auth
Archived in project
Milestone
v1.22
Development

Successfully merging this pull request may close these issues.

[PodSecurity] baseline - seccomp: forbid explicit unconfined [PodSecurity] restricted - seccomp
4 participants
@sejr @k8s-ci-robot @liggitt @tallclair