Repository: containerd/nydus-snapshotter

Issue:
Nydusd fails to start when snapshotter root path leads to an oversized Unix socket path
### Summary

When the path to the nydus-snapshotter root directory is too long, the nydusd daemon will fail to start due to the wrong path length.

### Logs
nerdctl Error:
`FATA[0004] wait for instance id x: wait until daemon is RUNNING: get daemon state: daemon socket ${rootPath}/socket/${xid}/api.sock: not found`

nydusd Error:
`Error: Error { kind: InvalidInput, message: "path must be shorter than SUN_LEN" }`
`time="xxxxxxx" level=error msg="Process 190090 has been a zombie"`
`time="xxxxxxx" level=error msg="Nydusd d2a7fb7lnnuukgvqigt0 probably not started"`
`time="xxxxxxx" level=error msg="Process 190090 has been a zombie"`

The daemon failed to start because the combined Unix socket path exceeds the system's SUN_LEN limit.

I think we should do a check for possible unix path lengths when nydus-snapshotter starts, it would be bad if it fails when running the nydusd daemon


Discussion:
@imeoer: Hi @Zephyrcf, thanks for the issue! What are `rootPath` and `xid` like?@Zephyrcf: > Hi [@Zephyrcf](https://github.com/Zephyrcf), thanks for the issue! What are `rootPath` and `xid` like?

rootPath is the root parameter configured in nydus-snapshotter config. xid is randomly generated through *xid.New()* and is used to identify a nydusd daemon.


Solution patch:
diff --git a/config/config.go b/config/config.go
index f91bb67..935e856 100644
--- a/config/config.go
+++ b/config/config.go
@@ -47,6 +47,16 @@ const (
 	// to manage those snapshots.
 	DaemonModeNone    DaemonMode = DaemonMode(constant.DaemonModeNone)
 	DaemonModeInvalid DaemonMode = DaemonMode(constant.DaemonModeInvalid)
+	// MaxRootPathLen defines the maximum allowed length of the root portion of a Unix domain socket path.
+	//
+	// This value is calculated based on the hard length limit of `sun_path` on Linux systems (108 bytes).
+	// Nydusd's socket path format is "${rootPath}/socket/${xid}/api?.sock".
+	// - The length of the fixed part "/socket/${xid}/api?.sock" is 38 bytes.
+	//
+	// Since the maximum upper limit of the total path length is 108 bytes, in order to avoid exceeding the limit, the maximum allowed length of rootPath is:
+	// 108 - len("/socket/${xid}/api?.sock") = 108 - 38 = 70.
+	// Therefore, we must set the effective maximum length of the root path to 70 bytes.
+	MaxRootPathLen = 70
 )
 
 func parseDaemonMode(m string) (DaemonMode, error) {
@@ -274,9 +284,13 @@ func ValidateConfig(c *SnapshotterConfig) error {
 		}
 	}
 
-	if len(c.Root) == 0 {
+	rootPathLen := len(c.Root)
+	if rootPathLen == 0 {
 		return errors.New("empty root directory")
 	}
+	if rootPathLen > MaxRootPathLen {
+		return errors.Errorf("root directory path is too long: %d bytes, max is %d bytes", rootPathLen, MaxRootPathLen)
+	}
 
 	if c.DaemonConfig.FsDriver != FsDriverFscache && c.DaemonConfig.FsDriver != FsDriverFusedev &&
 		c.DaemonConfig.FsDriver != FsDriverBlockdev && c.DaemonConfig.FsDriver != FsDriverNodev &&
diff --git a/config/daemonconfig/daemonconfig.go b/config/daemonconfig/daemonconfig.go
index f515185..c028627 100644
--- a/config/daemonconfig/daemonconfig.go
+++ b/config/daemonconfig/daemonconfig.go
@@ -26,6 +26,7 @@ const (
 	backendTypeLocalfs  StorageBackendType = "localfs"
 	backendTypeOss      StorageBackendType = "oss"
 	backendTypeRegistry StorageBackendType = "registry"
+	backendTypeS3       StorageBackendType = "s3"
 )
 
 type DaemonConfig interface {
@@ -83,14 +84,17 @@ type BackendConfig struct {
 	BlobRedirectedHost string         `json:"blob_redirected_host,omitempty"`
 	Mirrors            []MirrorConfig `json:"mirrors,omitempty"`
 
-	// OSS backend configs
+	// Shared by oss and s3 backend configs
 	EndPoint        string `json:"endpoint,omitempty"`
 	AccessKeyID     string `json:"access_key_id,omitempty" secret:"true"`
 	AccessKeySecret string `json:"access_key_secret,omitempty" secret:"true"`
 	BucketName      string `json:"bucket_name,omitempty"`
 	ObjectPrefix    string `json:"object_prefix,omitempty"`
 
-	// Shared by registry and oss backend
+	// S3-specific config
+	Region string `json:"region,omitempty"`
+
+	// Shared by registry, oss, and s3
 	Scheme     string `json:"scheme,omitempty"`
 	SkipVerify bool   `json:"skip_verify,omitempty"`
 
@@ -174,10 +178,11 @@ func SupplementDaemonConfig(c DaemonConfig, imageID, snapshotID string,
 		c.Supplement(registryHost, image.Repo, snapshotID, params)
 		c.FillAuth(keyChain)
 
-	// Localfs and OSS backends don't need any update,
+	// Localfs, OSS and S3 backends don't need any update,
 	// just use the provided config in template
 	case backendTypeLocalfs:
 	case backendTypeOss:
+	case backendTypeS3:
 	default:
 		return errors.Errorf("unknown backend type %s", backendType)
 	}



Test patch:
diff --git a/config/config_test.go b/config/config_test.go
index 5a513df..947a95a 100644
--- a/config/config_test.go
+++ b/config/config_test.go
@@ -253,4 +253,15 @@ func TestProcessConfigurations(t *testing.T) {
 
 	err = ProcessConfigurations(&snapshotterConfig3)
 	A.NoError(err)
+
+	var snapshotterConfig4 SnapshotterConfig
+	oversizedPath := "/path/to/very/long/root/directory/that/exceed/the/max/nydus-snapshotter"
+	A.Equal(MaxRootPathLen+1, len(oversizedPath))
+
+	snapshotterConfig4.Root = oversizedPath
+
+	err = MergeConfig(&snapshotterConfig4, &defaultSnapshotterConfig)
+	A.NoError(err)
+	err = ValidateConfig(&snapshotterConfig4)
+	A.Error(err)
 }
diff --git a/tests/helpers/kind.sh b/tests/helpers/kind.sh
index a83c831..1bff45c 100755
--- a/tests/helpers/kind.sh
+++ b/tests/helpers/kind.sh
@@ -53,7 +53,7 @@ configure::dockerd '{
   "cgroup-parent": "/actions_job",
   "insecure-registries" : [ "'"$registry_url"'" ]
 }'
-http::healthcheck "$registry_url"/v2/ 5 1 "$DOCKER_USER" "$DOCKER_PASSWORD"
+http::healthcheck "$registry_url"/v2/ 10 2 "$DOCKER_USER" "$DOCKER_PASSWORD"
 log::info "Login"
 docker::login "$DOCKER_USER" "$DOCKER_PASSWORD" "$registry_url"
 
diff --git a/tests/helpers/lib.sh b/tests/helpers/lib.sh
index 28a48ae..5791dec 100755
--- a/tests/helpers/lib.sh
+++ b/tests/helpers/lib.sh
@@ -133,7 +133,7 @@ _http::get(){
   shift
 
   local header
-  local command=(curl -fsSL --retry "$retry" --retry-delay "$delay" -o "$output")
+  local command=(curl -fsSL --retry-connrefused --retry "$retry" --retry-delay "$delay" -o "$output")
   # Add a basic auth user if necessary
   [ "$user" == "" ] || command+=(--user "$user:$password")
   # Force tls v1.2 and no redirect to http if url scheme is https