Skip to content

Apparmor cleanup #131989

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
Merged
merged 4 commits into from
Jul 18, 2025
Merged

Apparmor cleanup #131989

merged 4 commits into from
Jul 18, 2025

Conversation

@tallclair
Copy link
Member

@tallclair tallclair commented May 27, 2025 •
edited
Loading

What type of PR is this?

/kind cleanup
/kind api-change

What this PR does / why we need it:

Implement phase 2 of the AppArmor field transition, but continue to suppress annotation warnings when there is a corresponding field. This change is consistent with the way we handle seccomp annotations.

  1. Stop copying fields to annotations (but continue copying annotations -> fields)
  2. Warn when using AppArmor annotations on PodTemplates without a corresponding container-level field

Which issue(s) this PR fixes:

For kubernetes/enhancements#24 (comment)

Does this PR introduce a user-facing change?

[Action Required] AppArmor profiles specified in the pod or container SecurityContext are no longer copied to deprecated AppArmor annotations (prefix `container.apparmor.security.beta.kubernetes.io/`). Anything that inspects the deprecated annotations must be migrated to use the SecurityContext fields instead.

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

- [KEP]: https://github.com/kubernetes/enhancements/blob/master/keps/sig-node/24-apparmor/README.md#removing-annotation-support

/sig node
/milestone v1.34
/priority important-longterm

@k8s-ci-robot k8s-ci-robot added the release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. label May 27, 2025
@k8s-ci-robot k8s-ci-robot added this to the v1.34 milestone May 27, 2025
@k8s-ci-robot k8s-ci-robot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API sig/node Categorizes an issue or PR as relevant to SIG Node. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. labels May 27, 2025
@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented May 27, 2025

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-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels May 27, 2025
@k8s-ci-robot k8s-ci-robot requested review from sjenning and thockin May 27, 2025 19:29
@tallclair
Copy link
Member Author

tallclair commented May 27, 2025

/assign @liggitt
/cc @vinayakankugoyal

tallclair
tallclair commented May 27, 2025
View reviewed changes
@k8s-triage-robot
Copy link

k8s-triage-robot commented May 27, 2025

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.

@vinayakankugoyal
Copy link
Contributor

vinayakankugoyal commented May 28, 2025

/lgtm

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

k8s-ci-robot commented May 28, 2025

LGTM label has been added.

Git tree hash: 79118bbf786e61ad87208ad4d421bbbec8ee4192

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 30, 2025
@k8s-ci-robot k8s-ci-robot requested a review from liggitt May 30, 2025 23:37
tallclair
tallclair commented May 30, 2025
View reviewed changes
pkg/api/pod/warnings.go
// use of container AppArmor annotation without accompanying field

isPodTemplate := fieldPath != nil // Pod warnings are emitted through applyAppArmorVersionSkew instead.
hasAppArmorField := hasPodAppArmorProfile || (c.SecurityContext != nil && c.SecurityContext.AppArmorProfile != nil)
Copy link
Member Author

@tallclair tallclair May 30, 2025

Choose a reason for hiding this comment

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

Note to reviewers: we weren't checking that the container annotation matched the pod profile here. Rather than extracting the logic from the pod strategy that does that comparison, I decided to just always emit a warning on pod templates when there is a pod-level field but a container level annotation.

Copy link
Member

@liggitt liggitt Jul 17, 2025

Choose a reason for hiding this comment

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

only warn if the annotation produced a different effective app armor policy for the container... can relocate the helper the pod strategy is using to figure that out to this package if you want

@tallclair
Copy link
Member Author

tallclair commented May 30, 2025

Reverted the changes to warn unconditionally on annotation usage. PTAL.

This was referenced Jun 9, 2025
Open
Merged
@Rajalakshmi-Girish Rajalakshmi-Girish moved this to Pending inclusion in [sig-release] Bug Triage Jun 30, 2025
@sarthaksarthak9
Copy link
Member

sarthaksarthak9 commented Jul 2, 2025

Hello @tallclair
This pr has not been updated for 1 month, so I'd like to check what's the status. If there's anything we can do, please let us know.
The code freeze is starting 02:00 UTC Friday 25th July 2025 (about 3 weeks from now). Please make sure the PR has both lgtm and approved labels before the code freeze. Thanks!

@sarthaksarthak9 sarthaksarthak9 moved this from Pending inclusion to Tracked in [sig-release] Bug Triage Jul 2, 2025
@tallclair
Copy link
Member Author

tallclair commented Jul 7, 2025

@liggitt would you mind reviewing this? Or should I find someone else for approval?

@liggitt liggitt moved this to In progress in API Reviews Jul 17, 2025
@liggitt liggitt moved this from In progress to Changes requested in API Reviews Jul 17, 2025
@BenTheElder BenTheElder mentioned this pull request Jul 17, 2025
Closed
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 17, 2025
@liggitt liggitt moved this from Changes requested to In progress in API Reviews Jul 17, 2025
@liggitt
Copy link
Member

liggitt commented Jul 18, 2025

/lgtm
/approve
/retest

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

k8s-ci-robot commented Jul 18, 2025

LGTM label has been added.

Git tree hash: 202adeb4e3ce2fc556bbcc8b662a7e8345259249

@liggitt liggitt moved this from In progress to API review completed, 1.34 in API Reviews Jul 18, 2025
@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Jul 18, 2025

[APPROVALNOTIFIER] This PR is APPROVED

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

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 18, 2025
@k8s-triage-robot
Copy link

k8s-triage-robot commented Jul 18, 2025

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

  • The PR does have any do-not-merge/* labels
  • The PR does not have the needs-ok-to-test label
  • The PR is mergeable (does not have a needs-rebase label)
  • The PR is approved (has cncf-cla: yes, lgtm, approved labels)
  • The PR is failing tests required for merge

You can:

/retest

@k8s-ci-robot k8s-ci-robot merged commit 963a9ac into kubernetes:master Jul 18, 2025
13 checks passed
@github-project-automation github-project-automation bot moved this from Tracked to Done in [sig-release] Bug Triage Jul 18, 2025
@github-project-automation github-project-automation bot moved this from other-sig (sig-node-approved) to Done in SIG Node: code and documentation PRs Jul 18, 2025
@JaydipGabani JaydipGabani mentioned this pull request Sep 11, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sjenning sjenning Awaiting requested review from sjenning

@thockin thockin Awaiting requested review from thockin

@liggitt liggitt Awaiting requested review from liggitt

@vinayakankugoyal vinayakankugoyal Awaiting requested review from vinayakankugoyal

Assignees

@liggitt liggitt

@vinayakankugoyal vinayakankugoyal

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/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. 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. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. sig/node Categorizes an issue or PR as relevant to SIG Node. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

API Reviews
Status: API review completed, 1.34
SIG Node: code and documentation PRs
Archived in project
[sig-release] Bug Triage
Archived in project

Milestone

v1.34

Development

Successfully merging this pull request may close these issues.

6 participants

@tallclair @k8s-ci-robot @k8s-triage-robot @vinayakankugoyal @sarthaksarthak9 @liggitt