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

Partly remove support for seccomp annotations #109819

Merged
merged 1 commit into from Aug 1, 2022
Merged

Partly remove support for seccomp annotations #109819

merged 1 commit into from Aug 1, 2022

Conversation

saschagrunert
Copy link
Member

@saschagrunert saschagrunert commented May 5, 2022 •
edited by liggitt

What type of PR is this?

/kind api-change

What this PR does / why we need it:

We now partly drop the support for seccomp annotations which is planned for v1.25 as part of the KEP:

Making the annotations fully non-functional will be deferred to a future release.

Which issue(s) this PR fixes:

Refers to kubernetes/enhancements#135
Docs PR: kubernetes/website#33524
Fixes #95171

Special notes for your reviewer:

Pod security policies are not touched by this change and therefore we have to keep the annotation key constants.

Warnings emitted for annotation-only use since 1.22 in #102491, and specific warnings about removal of support in 1.25 emitted since 1.23 in #104389

Does this PR introduce a user-facing change?

Action required: support for the alpha seccomp annotations `seccomp.security.alpha.kubernetes.io/pod` and `container.seccomp.security.alpha.kubernetes.io`, deprecated since v1.19, has been partially removed. Kubelets no longer support the annotations, use of the annotations in static pods is no longer supported, and the seccomp annotations are no longer auto-populated when pods with seccomp fields are created. Auto-population of the seccomp fields from the annotations is planned to be removed in 1.27. Pods should use the corresponding pod or container `securityContext.seccompProfile` field instead.

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

KEP: https://github.com/kubernetes/enhancements/issues/135

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. area/kubelet labels May 5, 2022
@k8s-ci-robot k8s-ci-robot added sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/testing Categorizes an issue or PR as relevant to SIG Testing. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels May 5, 2022
@saschagrunert saschagrunert mentioned this pull request May 5, 2022
Open
22 tasks
@saschagrunert saschagrunert added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels May 5, 2022
@saschagrunert
Copy link
Member Author

/test pull-kubernetes-node-e2e-containerd

@sftim sftim mentioned this pull request May 5, 2022
Merged
@k8s-triage-robot
Copy link

This PR may require API review.

If so, when the changes are ready, complete the pre-review checklist and request an API review.

Status of requested reviews is tracked in the API Review project.

@liggitt liggitt self-assigned this May 5, 2022
@liggitt liggitt added this to Assigned in API Reviews May 5, 2022
@liggitt liggitt added this to the v1.25 milestone May 5, 2022
@saschagrunert saschagrunert force-pushed the seccomp-annotation branch 2 times, most recently from aec38c5 to 72d1e1c Compare May 5, 2022 14:18
@saschagrunert
Copy link
Member Author

Tests are green, this is ready for review. PTAL @kubernetes/api-reviewers @kubernetes/sig-node-pr-reviews

@saschagrunert saschagrunert mentioned this pull request May 6, 2022
Merged
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 1, 2022
@liggitt
Copy link
Member

liggitt commented Aug 1, 2022

/lgtm
/approve

@liggitt liggitt added the kind/deprecation Categorizes issue or PR as related to a feature/enhancement marked for deprecation. label Aug 1, 2022
@liggitt liggitt moved this from Changes requested to API review completed, 1.25 in API Reviews Aug 1, 2022
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 1, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt, mrunalp, saschagrunert, tallclair

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 Aug 1, 2022
@liggitt
Copy link
Member

liggitt commented Aug 1, 2022

I've gotten a little lost in all this, so let me restate my understanding of the timeline for seccomp annotations:

  • 1.3: Kubelet respects annotations on the container (and sandbox?)
  • 1.19, GA: Validate and sync annotations to and from fields.
  • 1.22: Warn when annotations are used without fields.
  • 1.23: Warn that annotations will cease to function as of 1.25.
  • 1.25, this PR: Stop copying fields to annotations.

If I understand correctly, the intent at this point is that in 1.25:

  • When present, annotations are copied into missing fields with a warning.
  • When present, annotations are validated to match fields.
  • Kubelet respects [only] fields on the container (and sandbox?)

All of that looks correct, expect the last bit - Kubelet respects the fields on the pod and container, just not the annotations.

In a future release, we would drop applySeccompVersionSkew entirely, but leave the annotation validation and field<->annotation validation as-is.

Would this be on or after 1.27, which is N+2 in my thinking above?

Yes, likely 1.27.

My sense is that we don't have to leave the annotation code forever, but I could be wrong. Perhaps a few versions later (N+4?) we could:

  • Stop validating that annotations match fields.
  • Stop warning when annotations are used without fields.

I just don't see much benefit to dropping that code and allowing in data that some API clients could be confused/broken by.

@k8s-ci-robot k8s-ci-robot merged commit 37b100b into kubernetes:master Aug 1, 2022
SIG Node CI/Test Board automation moved this from Archive-it to Done Aug 1, 2022
SIG Node PR Triage automation moved this from Needs Reviewer to Done Aug 1, 2022
SIG Auth Old automation moved this from Needs Triage to Closed / Done Aug 1, 2022
@saschagrunert saschagrunert deleted the seccomp-annotation branch August 1, 2022 14:50
saschagrunert added a commit to saschagrunert/website that referenced this pull request Aug 2, 2022
@saschagrunert
Partly remove support for seccomp annotations
35731da 
From the release notes of
kubernetes/kubernetes#109819, we have to update
according to the following situation:

```
Action required: support for the alpha seccomp annotations
`seccomp.security.alpha.kubernetes.io/pod` and
`container.seccomp.security.alpha.kubernetes.io`, deprecated since
v1.19, has been partially removed. Kubelets no longer support the
annotations, use of the annotations in static pods is no longer
supported, and the seccomp annotations are no longer auto-populated when
pods with seccomp fields are created. Auto-population of the seccomp
fields from the annotations is planned to be removed in 1.27. Pods
should use the corresponding pod or container
`securityContext.seccompProfile` field instead.
```

Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
saschagrunert added a commit to saschagrunert/website that referenced this pull request Aug 2, 2022
@saschagrunert
Partly remove support for seccomp annotations
3ce2650 
From the release notes of
kubernetes/kubernetes#109819, we have to update
according to the following situation:

```
Action required: support for the alpha seccomp annotations
`seccomp.security.alpha.kubernetes.io/pod` and
`container.seccomp.security.alpha.kubernetes.io`, deprecated since
v1.19, has been partially removed. Kubelets no longer support the
annotations, use of the annotations in static pods is no longer
supported, and the seccomp annotations are no longer auto-populated when
pods with seccomp fields are created. Auto-population of the seccomp
fields from the annotations is planned to be removed in 1.27. Pods
should use the corresponding pod or container
`securityContext.seccompProfile` field instead.
```

Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
saschagrunert added a commit to saschagrunert/website that referenced this pull request Aug 2, 2022
@saschagrunert
Partly remove support for seccomp annotations
5f94ccd 
From the release notes of
kubernetes/kubernetes#109819, we have to update
according to the following situation:

```
Action required: support for the alpha seccomp annotations
`seccomp.security.alpha.kubernetes.io/pod` and
`container.seccomp.security.alpha.kubernetes.io`, deprecated since
v1.19, has been partially removed. Kubelets no longer support the
annotations, use of the annotations in static pods is no longer
supported, and the seccomp annotations are no longer auto-populated when
pods with seccomp fields are created. Auto-population of the seccomp
fields from the annotations is planned to be removed in 1.27. Pods
should use the corresponding pod or container
`securityContext.seccompProfile` field instead.
```

Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
saschagrunert added a commit to saschagrunert/website that referenced this pull request Aug 2, 2022
@saschagrunert
Partly remove support for seccomp annotations
b04a9eb 
From the release notes of
kubernetes/kubernetes#109819, we have to update
according to the following situation:

```
Action required: support for the alpha seccomp annotations
`seccomp.security.alpha.kubernetes.io/pod` and
`container.seccomp.security.alpha.kubernetes.io`, deprecated since
v1.19, has been partially removed. Kubelets no longer support the
annotations, use of the annotations in static pods is no longer
supported, and the seccomp annotations are no longer auto-populated when
pods with seccomp fields are created. Auto-population of the seccomp
fields from the annotations is planned to be removed in 1.27. Pods
should use the corresponding pod or container
`securityContext.seccompProfile` field instead.
```

Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
saschagrunert added a commit to saschagrunert/website that referenced this pull request Aug 2, 2022
@saschagrunert
Partly remove support for seccomp annotations
9b54716 
From the release notes of
kubernetes/kubernetes#109819, we have to update
according to the following situation:

```
Action required: support for the alpha seccomp annotations
`seccomp.security.alpha.kubernetes.io/pod` and
`container.seccomp.security.alpha.kubernetes.io`, deprecated since
v1.19, has been partially removed. Kubelets no longer support the
annotations, use of the annotations in static pods is no longer
supported, and the seccomp annotations are no longer auto-populated when
pods with seccomp fields are created. Auto-population of the seccomp
fields from the annotations is planned to be removed in 1.27. Pods
should use the corresponding pod or container
`securityContext.seccompProfile` field instead.
```

Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
saschagrunert added a commit to saschagrunert/website that referenced this pull request Aug 2, 2022
@saschagrunert
Partly remove support for seccomp annotations
45d0bde 
From the release notes of
kubernetes/kubernetes#109819, we have to update
according to the following situation:

```
Action required: support for the alpha seccomp annotations
`seccomp.security.alpha.kubernetes.io/pod` and
`container.seccomp.security.alpha.kubernetes.io`, deprecated since
v1.19, has been partially removed. Kubelets no longer support the
annotations, use of the annotations in static pods is no longer
supported, and the seccomp annotations are no longer auto-populated when
pods with seccomp fields are created. Auto-population of the seccomp
fields from the annotations is planned to be removed in 1.27. Pods
should use the corresponding pod or container
`securityContext.seccompProfile` field instead.
```

Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
@SataQiu SataQiu mentioned this pull request Jan 5, 2023
Merged
@saschagrunert saschagrunert mentioned this pull request Jan 9, 2023
Closed
9 tasks
@saschagrunert saschagrunert mentioned this pull request Jan 10, 2023
Merged
saschagrunert added a commit to saschagrunert/kubernetes that referenced this pull request Jan 12, 2023
@saschagrunert
Make seccomp annotations non-functional
af1f6a2 
This cleanup has been planned to finish the corresponding KEP:
kubernetes#91286

As follow-up on the partly removal of the seccomp annotations in
kubernetes#109819, we now drop
the version skew handling completely, but still warn as well as keep
the validation in place if both (annotation and field) are set.

The Pod Security Admission code has been already changed in
kubernetes#114846.

Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
danielvegamyhre pushed a commit to danielvegamyhre/kubernetes that referenced this pull request Jan 14, 2023
@saschagrunert @danielvegamyhre
Make seccomp annotations non-functional
b8e2971 
This cleanup has been planned to finish the corresponding KEP:
kubernetes#91286

As follow-up on the partly removal of the seccomp annotations in
kubernetes#109819, we now drop
the version skew handling completely, but still warn as well as keep
the validation in place if both (annotation and field) are set.

The Pod Security Admission code has been already changed in
kubernetes#114846.

Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
@tallclair tallclair mentioned this pull request Jan 26, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cbandy cbandy cbandy left review comments

@liggitt liggitt liggitt left review comments

@tallclair tallclair tallclair left review comments

@mrunalp mrunalp mrunalp approved these changes

@andrewsykim andrewsykim Awaiting requested review from andrewsykim

@dchen1107 dchen1107 Awaiting requested review from dchen1107

Assignees

@liggitt liggitt

@tallclair tallclair

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/kubelet area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/deprecation Categorizes issue or PR as related to a feature/enhancement marked for deprecation. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
API Reviews
Status: API review completed, 1.25
SIG Auth
Archived in project
SIG Node CI/Test Board
Done
SIG Node CI/Test Board
Archived in project
SIG Node PR Triage
Done
SIG Auth Old
Closed / Done
Milestone
v1.25
Development

Successfully merging this pull request may close these issues.

Remove version skew and annotation support for seccomp (v1.23)
8 participants
@saschagrunert @k8s-triage-robot @endocrimes @tallclair @mrunalp @liggitt @cbandy @k8s-ci-robot