oci: Use WithReadonlyTempMount when adding users/groups

rumpl · rumpl · commit cab056226fd8 · 2023-04-05T12:09:36.000+02:00
Signed-off-by: Djordje Lukic &lt;djordje.lukic@docker.com&gt;
diff --git a/oci/spec_opts.go b/oci/spec_opts.go
@@ -683,8 +683,7 @@ func WithUser(userstr string) SpecOpts {
 				return err
 			}
 
-			mounts = tryReadonlyMounts(mounts)
-			return mount.WithTempMount(ctx, mounts, f)
+			return mount.WithReadonlyTempMount(ctx, mounts, f)
 		default:
 			return fmt.Errorf("invalid USER value %s", userstr)
 		}
@@ -744,8 +743,7 @@ func WithUserID(uid uint32) SpecOpts {
 			return err
 		}
 
-		mounts = tryReadonlyMounts(mounts)
-		return mount.WithTempMount(ctx, mounts, setUser)
+		return mount.WithReadonlyTempMount(ctx, mounts, setUser)
 	}
 }
 
@@ -789,8 +787,7 @@ func WithUsername(username string) SpecOpts {
 				return err
 			}
 
-			mounts = tryReadonlyMounts(mounts)
-			return mount.WithTempMount(ctx, mounts, setUser)
+			return mount.WithReadonlyTempMount(ctx, mounts, setUser)
 		} else if s.Windows != nil {
 			s.Process.User.Username = username
 		} else {
@@ -868,8 +865,7 @@ func WithAdditionalGIDs(userstr string) SpecOpts {
 			return err
 		}
 
-		mounts = tryReadonlyMounts(mounts)
-		return mount.WithTempMount(ctx, mounts, setAdditionalGids)
+		return mount.WithReadonlyTempMount(ctx, mounts, setAdditionalGids)
 	}
 }
 
@@ -930,8 +926,7 @@ func WithAppendAdditionalGroups(groups ...string) SpecOpts {
 			return err
 		}
 
-		mounts = tryReadonlyMounts(mounts)
-		return mount.WithTempMount(ctx, mounts, setAdditionalGids)
+		return mount.WithReadonlyTempMount(ctx, mounts, setAdditionalGids)
 	}
 }
 
@@ -1426,24 +1421,6 @@ func WithDevShmSize(kb int64) SpecOpts {
 	}
 }
 
-// tryReadonlyMounts is used by the options which are trying to get user/group
-// information from container's rootfs. Since the option does read operation
-// only, this helper will append ReadOnly mount option to prevent linux kernel
-// from syncing whole filesystem in umount syscall.
-//
-// TODO(fuweid):
-//
-// Currently, it only works for overlayfs. I think we can apply it to other
-// kinds of filesystem. Maybe we can return `ro` option by `snapshotter.Mount`
-// API, when the caller passes that experimental annotation
-// `containerd.io/snapshot/readonly.mount` something like that.
-func tryReadonlyMounts(mounts []mount.Mount) []mount.Mount {
-	if len(mounts) == 1 && mounts[0].Type == "overlay" {
-		mounts[0].Options = append(mounts[0].Options, "ro")
-	}
-	return mounts
-}
-
 // WithWindowsDevice adds a device exposed to a Windows (WCOW or LCOW) Container
 func WithWindowsDevice(idType, id string) SpecOpts {
 	return func(_ context.Context, _ Client, _ *containers.Container, s *Spec) error {

Original file line number	Diff line number	Diff line change
`@@ -683,8 +683,7 @@ func WithUser(userstr string) SpecOpts {`
`683`	`683`	`return err`
`684`	`684`	`}`
`685`	`685`
`686`		`- mounts = tryReadonlyMounts(mounts)`
`687`		`- return mount.WithTempMount(ctx, mounts, f)`
	`686`	`+ return mount.WithReadonlyTempMount(ctx, mounts, f)`
`688`	`687`	`default:`
`689`	`688`	`return fmt.Errorf("invalid USER value %s", userstr)`
`690`	`689`	`}`
`@@ -744,8 +743,7 @@ func WithUserID(uid uint32) SpecOpts {`
`744`	`743`	`return err`
`745`	`744`	`}`
`746`	`745`
`747`		`- mounts = tryReadonlyMounts(mounts)`
`748`		`- return mount.WithTempMount(ctx, mounts, setUser)`
	`746`	`+ return mount.WithReadonlyTempMount(ctx, mounts, setUser)`
`749`	`747`	`}`
`750`	`748`	`}`
`751`	`749`
`@@ -789,8 +787,7 @@ func WithUsername(username string) SpecOpts {`
`789`	`787`	`return err`
`790`	`788`	`}`
`791`	`789`
`792`		`- mounts = tryReadonlyMounts(mounts)`
`793`		`- return mount.WithTempMount(ctx, mounts, setUser)`
	`790`	`+ return mount.WithReadonlyTempMount(ctx, mounts, setUser)`
`794`	`791`	`} else if s.Windows != nil {`
`795`	`792`	`s.Process.User.Username = username`
`796`	`793`	`} else {`
`@@ -868,8 +865,7 @@ func WithAdditionalGIDs(userstr string) SpecOpts {`
`868`	`865`	`return err`
`869`	`866`	`}`
`870`	`867`
`871`		`- mounts = tryReadonlyMounts(mounts)`
`872`		`- return mount.WithTempMount(ctx, mounts, setAdditionalGids)`
	`868`	`+ return mount.WithReadonlyTempMount(ctx, mounts, setAdditionalGids)`
`873`	`869`	`}`
`874`	`870`	`}`
`875`	`871`
`@@ -930,8 +926,7 @@ func WithAppendAdditionalGroups(groups ...string) SpecOpts {`
`930`	`926`	`return err`
`931`	`927`	`}`
`932`	`928`
`933`		`- mounts = tryReadonlyMounts(mounts)`
`934`		`- return mount.WithTempMount(ctx, mounts, setAdditionalGids)`
	`929`	`+ return mount.WithReadonlyTempMount(ctx, mounts, setAdditionalGids)`
`935`	`930`	`}`
`936`	`931`	`}`
`937`	`932`
`@@ -1426,24 +1421,6 @@ func WithDevShmSize(kb int64) SpecOpts {`
`1426`	`1421`	`}`
`1427`	`1422`	`}`
`1428`	`1423`
`1429`		`-// tryReadonlyMounts is used by the options which are trying to get user/group`
`1430`		`-// information from container's rootfs. Since the option does read operation`
`1431`		`-// only, this helper will append ReadOnly mount option to prevent linux kernel`
`1432`		`-// from syncing whole filesystem in umount syscall.`
`1433`		`-//`
`1434`		`-// TODO(fuweid):`
`1435`		`-//`
`1436`		`-// Currently, it only works for overlayfs. I think we can apply it to other`
`1437`		-// kinds of filesystem. Maybe we can return `ro` option by `snapshotter.Mount`
`1438`		`-// API, when the caller passes that experimental annotation`
`1439`		-// `containerd.io/snapshot/readonly.mount` something like that.
`1440`		`-func tryReadonlyMounts(mounts []mount.Mount) []mount.Mount {`
`1441`		`- if len(mounts) == 1 && mounts[0].Type == "overlay" {`
`1442`		`- mounts[0].Options = append(mounts[0].Options, "ro")`
`1443`		`- }`
`1444`		`- return mounts`
`1445`		`-}`
`1446`		`-`
`1447`	`1424`	`// WithWindowsDevice adds a device exposed to a Windows (WCOW or LCOW) Container`
`1448`	`1425`	`func WithWindowsDevice(idType, id string) SpecOpts {`
`1449`	`1426`	`return func(_ context.Context, _ Client, _ containers.Container, s Spec) error {`