-
Notifications
You must be signed in to change notification settings - Fork 950
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.
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
feature: selinux for cri manager #1092
Conversation
52b378a
to
c97f1b9
Compare
Codecov Report
@@ Coverage Diff @@
## master #1092 +/- ##
==========================================
- Coverage 15.8% 15.48% -0.32%
==========================================
Files 163 161 -2
Lines 8793 8800 +7
==========================================
- Hits 1390 1363 -27
- Misses 7300 7334 +34
Partials 103 103
|
@allencloud PTAL |
meta.AppArmorProfile = value | ||
case "seccomp": | ||
meta.SeccompProfile = value | ||
case "no-new-privileges": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prevent processes in container to gain new privileges via the --security-opt=no-new-privileges
flag
just attach a record which may do us a favour.
daemon/mgr/container_utils.go
Outdated
case "label": | ||
labelOpts = append(labelOpts, value) | ||
default: | ||
return fmt.Errorf("invalid type %s in --security-opt %s: unknown type from apparmor and seccomp", key, securityOpt) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the code change, I do not think this line is correct any longer.
unknown type from apparmor and seccomp
except apparmor
and seccomp
, it seems that we should add no-new-privileges
and label
. Right? @YaoZengzeng
daemon/mgr/container_utils.go
Outdated
meta.SeccompProfile = value | ||
case "no-new-privileges": | ||
noNewPrivileges, err := strconv.ParseBool(value) | ||
if labelOpts != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can judge if labelOpts == nil
first to make return fast to simplify code and reduce ident.
if labelOpts == nil {
return nil
}
meta.ProcessLabel, meta.MountLabel, err = label.InitLabels(labelOpts)
if err != nil{
return fmt.Errorf("failed to init labels: %v", err)
}
return nil
Signed-off-by: YaoZengzeng <yaozengzeng@zju.edu.cn>
@allencloud Updated. |
LGTM |
Signed-off-by: YaoZengzeng yaozengzeng@zju.edu.cn
Ⅰ. Describe what this PR did
Ⅱ. Does this pull request fix one issue?
Ⅲ. Describe how you did it
With this PR, we could enable one of the security context --- SELinux, if it's configured in CRI.
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews