Skip to content

server/*: add fine-grained SupplementalGroups control for enhanced security #8268

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 3 commits into from
Jun 26, 2024
Merged

server/*: add fine-grained SupplementalGroups control for enhanced security #8268

merged 3 commits into from
Jun 26, 2024

Conversation

@sohankunkerkar
Copy link
Member

@sohankunkerkar sohankunkerkar commented Jun 6, 2024 •
edited
Loading

This PR implements the CRI part of the KEP-3619, which aims to provide fine-grained control over supplemental groups for container processes.

What type of PR is this?

/kind feature

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #8230

Special notes for your reviewer:

xref: https://github.com/kubernetes/enhancements/tree/master/keps/sig-node/3619-supplemental-groups-policy

Does this PR introduce a user-facing change?

Add fine-grained SupplementalGroups control for enhanced security (KEP-3619)

@openshift-ci openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Jun 6, 2024
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 6, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci openshift-ci bot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. kind/feature Categorizes issue or PR as related to a new feature. labels Jun 6, 2024
@sohankunkerkar sohankunkerkar force-pushed the suppplemental-groups branch from ab1739e to 6b13931 Compare June 6, 2024 14:55
@codecov
Copy link

codecov bot commented Jun 6, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 50.00000% with 16 lines in your changes missing coverage. Please review.

Project coverage is 49.51%. Comparing base (86c3283) to head (11562fd).
Report is 8 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #8268      +/-   ##
==========================================
- Coverage   49.51%   49.51%   -0.01%     
==========================================
  Files         153      153              
  Lines       16968    17063      +95     
==========================================
+ Hits         8402     8448      +46     
- Misses       7517     7552      +35     
- Partials     1049     1063      +14

This was referenced Jun 6, 2024
Open
Merged
@sohankunkerkar sohankunkerkar force-pushed the suppplemental-groups branch from 6b13931 to 36d3056 Compare June 6, 2024 17:50
haircommander
haircommander reviewed Jun 7, 2024
View reviewed changes
@sohankunkerkar sohankunkerkar force-pushed the suppplemental-groups branch 4 times, most recently from f667009 to 33f1029 Compare June 10, 2024 20:15
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 15, 2024
@sohankunkerkar sohankunkerkar force-pushed the suppplemental-groups branch 2 times, most recently from 0fa5994 to 51e2c49 Compare June 16, 2024 14:56
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 16, 2024
@sohankunkerkar sohankunkerkar force-pushed the suppplemental-groups branch 3 times, most recently from fec9e76 to 5785616 Compare June 17, 2024 15:32
@sohankunkerkar sohankunkerkar changed the title [WIP] server/*: add fine-grained SupplementalGroups control for enhanced security server/*: add fine-grained SupplementalGroups control for enhanced security Jun 17, 2024
@sohankunkerkar sohankunkerkar marked this pull request as ready for review June 17, 2024 18:36
@sohankunkerkar sohankunkerkar requested a review from mrunalp as a code owner June 17, 2024 18:36
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 17, 2024
@openshift-ci openshift-ci bot requested review from QiWang19 and kwilczynski June 17, 2024 18:37
@sohankunkerkar sohankunkerkar force-pushed the suppplemental-groups branch 2 times, most recently from 1f42d1a to e057151 Compare June 18, 2024 20:34
@sohankunkerkar
Copy link
Member Author

/retest

everpeace
everpeace reviewed Jun 25, 2024
View reviewed changes
Copy link
Contributor

@everpeace everpeace left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@sohankunkerkar Thanks for supporting the KEP! I left a few small comments. PTAL. And, would you mind including "KEP-3619" in the release note like:

Add fine-grained SupplementalGroups control for enhanced security (KEP-3619)

@kwilczynski Thank you for pinging me!

@sohankunkerkar sohankunkerkar force-pushed the suppplemental-groups branch 2 times, most recently from c24cca5 to da46137 Compare June 26, 2024 13:59
@sohankunkerkar
Copy link
Member Author

/retest

@sohankunkerkar
Copy link
Member Author

@cri-o/cri-o-maintainers PTAL

@sohankunkerkar
Copy link
Member Author

/retest

1 similar comment
@sohankunkerkar
Copy link
Member Author

/retest

haircommander
haircommander reviewed Jun 26, 2024
View reviewed changes
@haircommander
Copy link
Member

/retest
/approve
/lgtm

@openshift-ci openshift-ci bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 26, 2024
kwilczynski
kwilczynski reviewed Jun 26, 2024
View reviewed changes
Add space to the error message
11562fd 
Signed-off-by: Krzysztof Wilczyński <[email protected]>
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jun 26, 2024
@kwilczynski
Copy link
Contributor

/approve
/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 26, 2024
@kwilczynski
Copy link
Contributor

@everpeace, thank you for lending us a hand with the reviews! Appreciated. 🙇

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 26, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: haircommander, kwilczynski, sohankunkerkar

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 [haircommander,kwilczynski,sohankunkerkar]

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

@kwilczynski
Copy link
Contributor

/retest

1 similar comment
@sohankunkerkar
Copy link
Member Author

/retest

@openshift-merge-bot openshift-merge-bot bot merged commit 722d68b into cri-o:main Jun 26, 2024
This was referenced Jul 17, 2024
Merged
Merged
@github-actions github-actions bot mentioned this pull request Sep 25, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@haircommander haircommander haircommander left review comments

@mrunalp mrunalp Awaiting requested review from mrunalp mrunalp is a code owner

@QiWang19 QiWang19 Awaiting requested review from QiWang19

+2 more reviewers

@kwilczynski kwilczynski kwilczynski left review comments

@everpeace everpeace everpeace left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@kwilczynski kwilczynski

@haircommander haircommander

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add support for Supplemental Groups Policy

5 participants

@sohankunkerkar @kwilczynski @haircommander @everpeace @openshift-merge-robot