Merge pull request #1241 from Random-Liu/cherrypick-#1240-release-1.2

Random-Liu · web-flow · commit 0ee59257af5b · 2019-08-20T11:04:17.000-07:00
Cherrypick #1240 release 1.2
diff --git a/pkg/server/container_create.go b/pkg/server/container_create.go
@@ -371,14 +371,14 @@ func (c *criService) generateContainerSpec(id string, sandboxID string, sandboxP
 
 	if !c.config.DisableProcMount {
 		// Apply masked paths if specified.
-		// Note: If the container is privileged, then we clear any masked paths later on in the call to setOCIPrivileged()
+		// If the container is privileged, this will be cleared later on.
 		g.Config.Linux.MaskedPaths = nil
 		for _, path := range securityContext.GetMaskedPaths() {
 			g.AddLinuxMaskedPaths(path)
 		}
 
 		// Apply readonly paths if specified.
-		// Note: If the container is privileged, then we clear any readonly paths later on in the call to setOCIPrivileged()
+		// If the container is privileged, this will be cleared later on.
 		g.Config.Linux.ReadonlyPaths = nil
 		for _, path := range securityContext.GetReadonlyPaths() {
 			g.AddLinuxReadonlyPaths(path)
@@ -956,18 +956,17 @@ func generateApparmorSpecOpts(apparmorProf string, privileged, apparmorEnabled b
 		return nil, nil
 	}
 	switch apparmorProf {
-	case runtimeDefault:
-		// TODO (mikebrow): delete created apparmor default profile
-		return apparmor.WithDefaultProfile(appArmorDefaultProfileName), nil
-	case unconfinedProfile:
-		return nil, nil
-	case "":
-		// Based on kubernetes#51746, default apparmor profile should be applied
-		// for non-privileged container when apparmor is not specified.
+	// Based on kubernetes#51746, default apparmor profile should be applied
+	// for when apparmor is not specified.
+	case runtimeDefault, "":
 		if privileged {
+			// Do not set apparmor profile when container is privileged
 			return nil, nil
 		}
+		// TODO (mikebrow): delete created apparmor default profile
 		return apparmor.WithDefaultProfile(appArmorDefaultProfileName), nil
+	case unconfinedProfile:
+		return nil, nil
 	default:
 		// Require and Trim default profile name prefix
 		if !strings.HasPrefix(apparmorProf, profileNamePrefix) {
diff --git a/pkg/server/container_create_test.go b/pkg/server/container_create_test.go
@@ -982,10 +982,19 @@ func TestGenerateApparmorSpecOpts(t *testing.T) {
 			profile:  runtimeDefault,
 			specOpts: apparmor.WithDefaultProfile(appArmorDefaultProfileName),
 		},
+		"should not apparmor when apparmor is default and privileged is true": {
+			profile:    runtimeDefault,
+			privileged: true,
+		},
 		"should set specified profile when local profile is specified": {
 			profile:  profileNamePrefix + "test-profile",
 			specOpts: apparmor.WithProfile("test-profile"),
 		},
+		"should set apparmor when local profile is specified and privileged is true": {
+			profile:    profileNamePrefix + "test-profile",
+			privileged: true,
+			specOpts:   apparmor.WithProfile("test-profile"),
+		},
 		"should return error if specified profile is invalid": {
 			profile:   "test-profile",
 			expectErr: true,
