Merge pull request #8357 from thaJeztah/1.6_backport_oci_readonly_mounts

dmcgowan · web-flow · commit c0efc63d3907 · 2023-04-06T09:25:38.000-07:00
[release/1.6 backport] oci: Use WithReadonlyTempMount when adding users/groups
diff --git a/oci/spec_opts.go b/oci/spec_opts.go
@@ -663,8 +663,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)
 		}
@@ -724,8 +723,7 @@ func WithUserID(uid uint32) SpecOpts {
 			return err
 		}
 
-		mounts = tryReadonlyMounts(mounts)
-		return mount.WithTempMount(ctx, mounts, setUser)
+		return mount.WithReadonlyTempMount(ctx, mounts, setUser)
 	}
 }
 
@@ -769,8 +767,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 {
@@ -848,8 +845,7 @@ func WithAdditionalGIDs(userstr string) SpecOpts {
 			return err
 		}
 
-		mounts = tryReadonlyMounts(mounts)
-		return mount.WithTempMount(ctx, mounts, setAdditionalGids)
+		return mount.WithReadonlyTempMount(ctx, mounts, setAdditionalGids)
 	}
 }
 
@@ -910,8 +906,7 @@ func WithAppendAdditionalGroups(groups ...string) SpecOpts {
 			return err
 		}
 
-		mounts = tryReadonlyMounts(mounts)
-		return mount.WithTempMount(ctx, mounts, setAdditionalGids)
+		return mount.WithReadonlyTempMount(ctx, mounts, setAdditionalGids)
 	}
 }
 
@@ -1389,21 +1384,3 @@ func WithDevShmSize(kb int64) SpecOpts {
 		return ErrNoShmMount
 	}
 }
-
-// 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
-}

Original file line number	Diff line number	Diff line change
`@@ -663,8 +663,7 @@ func WithUser(userstr string) SpecOpts {`
`663`	`663`	`return err`
`664`	`664`	`}`
`665`	`665`
`666`		`- mounts = tryReadonlyMounts(mounts)`
`667`		`- return mount.WithTempMount(ctx, mounts, f)`
	`666`	`+ return mount.WithReadonlyTempMount(ctx, mounts, f)`
`668`	`667`	`default:`
`669`	`668`	`return fmt.Errorf("invalid USER value %s", userstr)`
`670`	`669`	`}`
`@@ -724,8 +723,7 @@ func WithUserID(uid uint32) SpecOpts {`
`724`	`723`	`return err`
`725`	`724`	`}`
`726`	`725`
`727`		`- mounts = tryReadonlyMounts(mounts)`
`728`		`- return mount.WithTempMount(ctx, mounts, setUser)`
	`726`	`+ return mount.WithReadonlyTempMount(ctx, mounts, setUser)`
`729`	`727`	`}`
`730`	`728`	`}`
`731`	`729`
`@@ -769,8 +767,7 @@ func WithUsername(username string) SpecOpts {`
`769`	`767`	`return err`
`770`	`768`	`}`
`771`	`769`
`772`		`- mounts = tryReadonlyMounts(mounts)`
`773`		`- return mount.WithTempMount(ctx, mounts, setUser)`
	`770`	`+ return mount.WithReadonlyTempMount(ctx, mounts, setUser)`
`774`	`771`	`} else if s.Windows != nil {`
`775`	`772`	`s.Process.User.Username = username`
`776`	`773`	`} else {`
`@@ -848,8 +845,7 @@ func WithAdditionalGIDs(userstr string) SpecOpts {`
`848`	`845`	`return err`
`849`	`846`	`}`
`850`	`847`
`851`		`- mounts = tryReadonlyMounts(mounts)`
`852`		`- return mount.WithTempMount(ctx, mounts, setAdditionalGids)`
	`848`	`+ return mount.WithReadonlyTempMount(ctx, mounts, setAdditionalGids)`
`853`	`849`	`}`
`854`	`850`	`}`
`855`	`851`
`@@ -910,8 +906,7 @@ func WithAppendAdditionalGroups(groups ...string) SpecOpts {`
`910`	`906`	`return err`
`911`	`907`	`}`
`912`	`908`
`913`		`- mounts = tryReadonlyMounts(mounts)`
`914`		`- return mount.WithTempMount(ctx, mounts, setAdditionalGids)`
	`909`	`+ return mount.WithReadonlyTempMount(ctx, mounts, setAdditionalGids)`
`915`	`910`	`}`
`916`	`911`	`}`
`917`	`912`
`@@ -1389,21 +1384,3 @@ func WithDevShmSize(kb int64) SpecOpts {`
`1389`	`1384`	`return ErrNoShmMount`
`1390`	`1385`	`}`
`1391`	`1386`	`}`
`1392`		`-`
`1393`		`-// tryReadonlyMounts is used by the options which are trying to get user/group`
`1394`		`-// information from container's rootfs. Since the option does read operation`
`1395`		`-// only, this helper will append ReadOnly mount option to prevent linux kernel`
`1396`		`-// from syncing whole filesystem in umount syscall.`
`1397`		`-//`
`1398`		`-// TODO(fuweid):`
`1399`		`-//`
`1400`		`-// Currently, it only works for overlayfs. I think we can apply it to other`
`1401`		-// kinds of filesystem. Maybe we can return `ro` option by `snapshotter.Mount`
`1402`		`-// API, when the caller passes that experimental annotation`
`1403`		-// `containerd.io/snapshot/readonly.mount` something like that.
`1404`		`-func tryReadonlyMounts(mounts []mount.Mount) []mount.Mount {`
`1405`		`- if len(mounts) == 1 && mounts[0].Type == "overlay" {`
`1406`		`- mounts[0].Options = append(mounts[0].Options, "ro")`
`1407`		`- }`
`1408`		`- return mounts`
`1409`		`-}`