feature: appArmor for both cri manager and container manager #672
Conversation
Codecov Report
@@ Coverage Diff @@
## master #672 +/- ##
==========================================
- Coverage 10.16% 10.16% -0.01%
==========================================
Files 71 71
Lines 3422 3425 +3
==========================================
Hits 348 348
- Misses 3035 3038 +3
Partials 39 39
Continue to review full report at Codecov.
|
|
@allencloud @Letty5411 PTAL. |
| @@ -63,6 +63,7 @@ func (cc *CreateCommand) addFlags() { | |||
| flagSet.StringVar(&cc.utsMode, "uts", "", "UTS namespace to use") | |||
| flagSet.StringSliceVar(&cc.sysctls, "sysctl", nil, "Sysctl options") | |||
| flagSet.StringSliceVar(&cc.network, "net", nil, "Set networks to container") | |||
| flagSet.StringSliceVar(&cc.securityOpt, "security-opt", nil, "Security Options") | |||
There was a problem hiding this comment.
I think the command line usage was not readable enough for users to understand even developers. I will profile an issue to fix this.
|
@allencloud this PR has passed all corresponding tests in cri-tools and we may use some code in this PR for our further development. Maybe we should merge this ASAP :) |
daemon/mgr/container.go
Outdated
| @@ -330,6 +330,10 @@ func (mgr *ContainerManager) Create(ctx context.Context, name string, config *ty | |||
| meta.Config.NetworkDisabled = true | |||
| } | |||
|
|
|||
| if err := parseSecurityOpt(meta, config.HostConfig); err != nil { | |||
There was a problem hiding this comment.
I think this will panic. Since in the containercreate function, HostConfig could be nil. And if you use a nil to get SecurityOpt, it would panic.
Although at the very beginning, we checked that if config.HostConfig == nil, we could make function parseSecurityOpt more robust. How about just inputting config.HostConfig. SecurityOpt ? Then like func parseSecurityOpt(meta *ContainerMeta, securityOpts []string) error
|
LGTM generally except one nits in function |
Signed-off-by: YaoZengzeng <[email protected]>
|
@allencloud updated. |
|
LGTM |
pouchrobot
added
the
LGTM
Signed-off-by: YaoZengzeng [email protected]
1.Describe what this PR did
With this PR, we could enable one of the security option AppArmor.
Now we could apply AppArmor with options like:
Actually in docker, we should apply default AppArmor profile "docker-default" to every container which not explicitly specify it or explicitly specify it with "docker-default". We should implement it in pouch as well.
2.Does this pull request fix one issue?
fixes part of #635
3.Describe how you did it
4.Describe how to verify it
5.Special notes for reviews