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

Enable SELinux relabeling in CSI volumes #64026

Merged
merged 2 commits into from
May 23, 2018
Merged

Enable SELinux relabeling in CSI volumes #64026

merged 2 commits into from
May 23, 2018

Conversation

jsafrane
Copy link
Member

@jsafrane jsafrane commented May 18, 2018 •
edited
Loading

What this PR does / why we need it:
CSI volume plugin should provide correct information in GetAttributes call so kubelet can ask container runtime to relabel the volume. Therefore CSI volume plugin needs to check if a random volume mounted by a CSI driver supports SELinux or not by checking for "seclabel" mount or superblock option.

Which issue(s) this PR fixes
Fixes #63965

Release note:

Fixed SELinux relabeling of CSI volumes.

@saad-ali @vladimirvivien @davidz627
@cofyc, FYI, I'm changing struct mountInfo.

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels May 18, 2018
@jsafrane
Copy link
Member Author

/retest

vladimirvivien
vladimirvivien reviewed May 21, 2018
View reviewed changes
Copy link
Member

@vladimirvivien vladimirvivien 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.

please address minor change

pkg/volume/csi/csi_mounter.go Outdated
path := c.GetPath()
supportSelinux, err := mounter.GetSELinuxSupport(path)
if err != nil {
glog.V(2).Infof("Error checking for SELinux support: %s", err)
Copy link
Member

Choose a reason for hiding this comment

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

To be consistent with other logging in csi glog.V(2).Info(log("Error checking for SELinux support: %s", err))

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

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

New changes are detected. LGTM label has been removed.

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 22, 2018
@jsafrane
Copy link
Member Author

/assign @thockin @smarterclayton
for approval

I'll create pkg/util/nsenter/OWNERS in a separate pr.

@jsafrane jsafrane added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 22, 2018
@jsafrane
Copy link
Member Author

/retest

@childsb
Copy link
Contributor

childsb commented May 22, 2018

from sig-storage

/approve

@smarterclayton
Copy link
Contributor

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: childsb, jsafrane, smarterclayton

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 May 22, 2018
@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 63914, 63887, 64116, 64026, 62933). If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-github-robot k8s-github-robot merged commit 36b1f67 into kubernetes:master May 23, 2018
msau42
msau42 reviewed May 23, 2018
View reviewed changes
pkg/util/mount/mount_linux.go
@@ -642,22 +633,46 @@ func parseMountInfo(filename string) ([]mountInfo, error) {
}
// See `man proc` for authoritative description of format of the file.
fields := strings.Fields(line)
if len(fields) < 7 {
return nil, fmt.Errorf("wrong number of fields in (expected %d, got %d): %s", 8, len(fields), line)
if len(fields) < 10 {
Copy link
Member

Choose a reason for hiding this comment

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

make this a constant?

@jsafrane jsafrane mentioned this pull request May 23, 2018
Merged
@jsafrane jsafrane mentioned this pull request May 29, 2018
Merged
@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Jun 1, 2018
k8s-github-robot pushed a commit that referenced this pull request Jun 1, 2018
Merge pull request #64432 from jsafrane/automated-cherry-pick-of-#640…
d8f9e51 
…26-upstream-release-1.10

Automatic merge from submit-queue.

Automated cherry pick of #64026: Add SELinux support to CSI

Cherry pick of #64026 on release-1.10.

#64026: Add GetSELinuxSupport to mounter.

```release-note
Fixed SELinux relabeling of CSI volumes.
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msau42 msau42 msau42 left review comments

@vladimirvivien vladimirvivien vladimirvivien left review comments

@brendandburns brendandburns Awaiting requested review from brendandburns

@davidz627 davidz627 Awaiting requested review from davidz627

@pmorie pmorie Awaiting requested review from pmorie

@rootfs rootfs Awaiting requested review from rootfs

@saad-ali saad-ali Awaiting requested review from saad-ali

Assignees

@vladimirvivien vladimirvivien

@smarterclayton smarterclayton

@thockin thockin

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. lgtm "Looks good to me", 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. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CSI plugin should support SELinux
8 participants
@jsafrane @k8s-ci-robot @childsb @smarterclayton @k8s-github-robot @vladimirvivien @msau42 @thockin