[WIP]: internal/filesystem consolidation, part 2

.golangci.yml

@@ -31,6 +31,7 @@ linters:
     - revive
     # Gocritic
     - gocritic
+    - forbidigo
 
     # 3. We used to use these, but have now removed them
 
@@ -41,6 +42,12 @@ linters:
     # - nakedret
 
   settings:
+    forbidigo:
+      forbid:
+        # FIXME: there are still calls to os.WriteFile in tests under `cmd`
+        - pattern: ^os\.WriteFile.*$
+          pkg: github.com/containerd/nerdctl/v2/pkg
+          msg: os.WriteFile is neither atomic nor durable - use nerdctl filesystem.WriteFile instead
     staticcheck:
       checks:
         # Below is the default set

cmd/nerdctl/helpers/flagutil.go

@@ -21,6 +21,7 @@ import (
 
 	"github.com/spf13/cobra"
 
+	"github.com/containerd/nerdctl/v2/pkg"
 	"github.com/containerd/nerdctl/v2/pkg/api/types"
 )
 
@@ -112,6 +113,11 @@ func ProcessRootCmdFlags(cmd *cobra.Command) (types.GlobalCommandOptions, error)
 		return types.GlobalCommandOptions{}, err
 	}
 
+	err = pkg.InitFS(dataRoot)
+	if err != nil {
+		return types.GlobalCommandOptions{}, err
+	}
+
 	return types.GlobalCommandOptions{
 		Debug:            debug,
 		DebugFull:        debugFull,

docs/dev/auditing_dockerfile.md

@@ -255,7 +255,7 @@ On a warm cache, it is still over 150MB and 30+ seconds.
 In and of itself, this is hard to reduce, as we need these...
 
 Actions:
-- [ ] we could cache the module download location to reduce round-trips on modules that are shared accross
+- [ ] we could cache the module download location to reduce round-trips on modules that are shared across
 different projects
 - [ ] we are likely installing nerdctl modules six times - (once per architecture during the build phase, then once per
 ubuntu version and architecture during the tests runs (this is not even accounted for in the audit above)) - it should

docs/dev/store.md

@@ -23,7 +23,7 @@ containers can be named the same), etc.
 However, storing data on the filesystem in a reliable way comes with challenges:
 - incomplete writes may happen (because of a system restart, or an application crash), leaving important structured files
 in a broken state
-- concurrent writes, or reading while writing would obviously be a problem as well, be it accross goroutines, or between
+- concurrent writes, or reading while writing would obviously be a problem as well, be it across goroutines, or between
 concurrent executions of the nerdctl binary, or embedded in a third-party application that does concurrently access resources
 
 The `pkg/store` package does provide a "storage" abstraction that takes care of these issues, generally providing

docs/dir.md

@@ -65,7 +65,7 @@ Data volume
 
 Can be overridden with `nerdctl --cni-netconfpath=<NETCONFPATH>` flag and environment variable `$NETCONFPATH`.
 
-At the top-level of <NETCONFPATH>, network (files) are shared accross all namespaces.
+At the top-level of <NETCONFPATH>, network (files) are shared across all namespaces.
 Sub-folders inside <NETCONFPATH> are only available to the namespace bearing the same name,
 and its networks definitions are private.
 

pkg/buildkitutil/buildkitutil_test.go

@@ -29,6 +29,8 @@ import (
 	"testing"
 
 	"gotest.tools/v3/assert"
+
+	"github.com/containerd/nerdctl/v2/pkg/internal/filesystem"
 )
 
 func TestBuildKitFile(t *testing.T) {
@@ -55,7 +57,7 @@ func TestBuildKitFile(t *testing.T) {
 		{
 			name: "only Dockerfile is present",
 			prepare: func(t *testing.T) error {
-				return os.WriteFile(filepath.Join(tmp, DefaultDockerfileName), []byte{}, 0644)
+				return filesystem.WriteFile(filepath.Join(tmp, DefaultDockerfileName), []byte{}, 0644)
 			},
 			args:       args{".", ""},
 			wantAbsDir: tmp,
@@ -65,7 +67,7 @@ func TestBuildKitFile(t *testing.T) {
 		{
 			name: "only Containerfile is present",
 			prepare: func(t *testing.T) error {
-				return os.WriteFile(filepath.Join(tmp, "Containerfile"), []byte{}, 0644)
+				return filesystem.WriteFile(filepath.Join(tmp, "Containerfile"), []byte{}, 0644)
 			},
 			args:       args{".", ""},
 			wantAbsDir: tmp,
@@ -75,11 +77,11 @@ func TestBuildKitFile(t *testing.T) {
 		{
 			name: "both Dockerfile and Containerfile are present",
 			prepare: func(t *testing.T) error {
-				var err = os.WriteFile(filepath.Join(tmp, "Dockerfile"), []byte{}, 0644)
+				var err = filesystem.WriteFile(filepath.Join(tmp, "Dockerfile"), []byte{}, 0644)
 				if err != nil {
 					return err
 				}
-				return os.WriteFile(filepath.Join(tmp, "Containerfile"), []byte{}, 0644)
+				return filesystem.WriteFile(filepath.Join(tmp, "Containerfile"), []byte{}, 0644)
 			},
 			args:       args{".", ""},
 			wantAbsDir: tmp,
@@ -89,11 +91,11 @@ func TestBuildKitFile(t *testing.T) {
 		{
 			name: "Dockerfile and Containerfile have different contents",
 			prepare: func(t *testing.T) error {
-				var err = os.WriteFile(filepath.Join(tmp, "Dockerfile"), []byte{'d'}, 0644)
+				var err = filesystem.WriteFile(filepath.Join(tmp, "Dockerfile"), []byte{'d'}, 0644)
 				if err != nil {
 					return err
 				}
-				return os.WriteFile(filepath.Join(tmp, "Containerfile"), []byte{'c'}, 0644)
+				return filesystem.WriteFile(filepath.Join(tmp, "Containerfile"), []byte{'c'}, 0644)
 			},
 			args:       args{".", ""},
 			wantAbsDir: tmp,
@@ -103,7 +105,7 @@ func TestBuildKitFile(t *testing.T) {
 		{
 			name: "Custom file is specfied",
 			prepare: func(t *testing.T) error {
-				return os.WriteFile(filepath.Join(tmp, "CustomFile"), []byte{}, 0644)
+				return filesystem.WriteFile(filepath.Join(tmp, "CustomFile"), []byte{}, 0644)
 			},
 			args:       args{".", "CustomFile"},
 			wantAbsDir: tmp,
@@ -113,7 +115,7 @@ func TestBuildKitFile(t *testing.T) {
 		{
 			name: "Absolute path is specified along with custom file",
 			prepare: func(t *testing.T) error {
-				return os.WriteFile(filepath.Join(tmp, "CustomFile"), []byte{}, 0644)
+				return filesystem.WriteFile(filepath.Join(tmp, "CustomFile"), []byte{}, 0644)
 			},
 			args:       args{tmp, "CustomFile"},
 			wantAbsDir: tmp,
@@ -123,7 +125,7 @@ func TestBuildKitFile(t *testing.T) {
 		{
 			name: "Absolute path is specified along with Docker file",
 			prepare: func(t *testing.T) error {
-				return os.WriteFile(filepath.Join(tmp, "Dockerfile"), []byte{}, 0644)
+				return filesystem.WriteFile(filepath.Join(tmp, "Dockerfile"), []byte{}, 0644)
 			},
 			args:       args{tmp, "."},
 			wantAbsDir: tmp,
@@ -133,7 +135,7 @@ func TestBuildKitFile(t *testing.T) {
 		{
 			name: "Absolute path is specified with Container file in the path",
 			prepare: func(t *testing.T) error {
-				return os.WriteFile(filepath.Join(tmp, ContainerfileName), []byte{}, 0644)
+				return filesystem.WriteFile(filepath.Join(tmp, ContainerfileName), []byte{}, 0644)
 			},
 			args:       args{tmp, "."},
 			wantAbsDir: tmp,

pkg/cmd/builder/build.go

@@ -41,6 +41,7 @@ import (
 	"github.com/containerd/nerdctl/v2/pkg/buildkitutil"
 	"github.com/containerd/nerdctl/v2/pkg/clientutil"
 	"github.com/containerd/nerdctl/v2/pkg/containerutil"
+	"github.com/containerd/nerdctl/v2/pkg/internal/filesystem"
 	"github.com/containerd/nerdctl/v2/pkg/platformutil"
 	"github.com/containerd/nerdctl/v2/pkg/referenceutil"
 	"github.com/containerd/nerdctl/v2/pkg/strutil"
@@ -110,7 +111,7 @@ func Build(ctx context.Context, client *containerd.Client, options types.Builder
 		if err != nil {
 			return err
 		}
-		if err := os.WriteFile(options.IidFile, []byte(id), 0644); err != nil {
+		if err := filesystem.WriteFile(options.IidFile, []byte(id), 0644); err != nil {
 			return err
 		}
 	}

pkg/cmd/compose/compose.go

@@ -34,6 +34,7 @@ import (
 	"github.com/containerd/nerdctl/v2/pkg/composer"
 	"github.com/containerd/nerdctl/v2/pkg/composer/serviceparser"
 	"github.com/containerd/nerdctl/v2/pkg/imgutil"
+	"github.com/containerd/nerdctl/v2/pkg/internal/filesystem"
 	"github.com/containerd/nerdctl/v2/pkg/ipfs"
 	"github.com/containerd/nerdctl/v2/pkg/netutil"
 	"github.com/containerd/nerdctl/v2/pkg/referenceutil"
@@ -136,7 +137,7 @@ func New(client *containerd.Client, globalOptions types.GlobalCommandOptions, op
 					return err
 				}
 				defer os.RemoveAll(dir)
-				if err := os.WriteFile(filepath.Join(dir, "api"), []byte(ipfsAddress), 0600); err != nil {
+				if err := filesystem.WriteFile(filepath.Join(dir, "api"), []byte(ipfsAddress), 0600); err != nil {
 					return err
 				}
 				ipfsPath = dir

pkg/cmd/container/create.go

@@ -51,6 +51,7 @@ import (
 	"github.com/containerd/nerdctl/v2/pkg/imgutil"
 	"github.com/containerd/nerdctl/v2/pkg/imgutil/load"
 	"github.com/containerd/nerdctl/v2/pkg/inspecttypes/dockercompat"
+	"github.com/containerd/nerdctl/v2/pkg/internal/filesystem"
 	"github.com/containerd/nerdctl/v2/pkg/ipcutil"
 	"github.com/containerd/nerdctl/v2/pkg/labels"
 	"github.com/containerd/nerdctl/v2/pkg/logging"
@@ -951,7 +952,7 @@ func generateLogConfig(dataStore string, id string, logDriver string, logOpt []s
 		}
 
 		logConfigFilePath := logging.LogConfigFilePath(dataStore, ns, id)
-		if err = os.WriteFile(logConfigFilePath, logConfigB, 0600); err != nil {
+		if err = filesystem.WriteFile(logConfigFilePath, logConfigB, 0600); err != nil {
 			return logConfig, err
 		}
 

pkg/cmd/image/pull.go

@@ -26,6 +26,7 @@ import (
 
 	"github.com/containerd/nerdctl/v2/pkg/api/types"
 	"github.com/containerd/nerdctl/v2/pkg/imgutil"
+	"github.com/containerd/nerdctl/v2/pkg/internal/filesystem"
 	"github.com/containerd/nerdctl/v2/pkg/ipfs"
 	"github.com/containerd/nerdctl/v2/pkg/referenceutil"
 	"github.com/containerd/nerdctl/v2/pkg/signutil"
@@ -62,7 +63,7 @@ func EnsureImage(ctx context.Context, client *containerd.Client, rawRef string,
 				return nil, err
 			}
 			defer os.RemoveAll(dir)
-			if err := os.WriteFile(filepath.Join(dir, "api"), []byte(options.IPFSAddress), 0600); err != nil {
+			if err := filesystem.WriteFile(filepath.Join(dir, "api"), []byte(options.IPFSAddress), 0600); err != nil {
 				return nil, err
 			}
 			ipfsPath = dir

pkg/cmd/image/push.go

@@ -46,6 +46,7 @@ import (
 	nerdconverter "github.com/containerd/nerdctl/v2/pkg/imgutil/converter"
 	"github.com/containerd/nerdctl/v2/pkg/imgutil/dockerconfigresolver"
 	"github.com/containerd/nerdctl/v2/pkg/imgutil/push"
+	"github.com/containerd/nerdctl/v2/pkg/internal/filesystem"
 	"github.com/containerd/nerdctl/v2/pkg/ipfs"
 	"github.com/containerd/nerdctl/v2/pkg/platformutil"
 	"github.com/containerd/nerdctl/v2/pkg/referenceutil"
@@ -85,7 +86,7 @@ func Push(ctx context.Context, client *containerd.Client, rawRef string, options
 				return err
 			}
 			defer os.RemoveAll(dir)
-			if err := os.WriteFile(filepath.Join(dir, "api"), []byte(options.IpfsAddress), 0600); err != nil {
+			if err := filesystem.WriteFile(filepath.Join(dir, "api"), []byte(options.IpfsAddress), 0600); err != nil {
 				return err
 			}
 			ipfsPath = dir

pkg/cmd/ipfs/registry_serve.go

@@ -24,6 +24,7 @@ import (
 	"github.com/containerd/log"
 
 	"github.com/containerd/nerdctl/v2/pkg/api/types"
+	"github.com/containerd/nerdctl/v2/pkg/internal/filesystem"
 	"github.com/containerd/nerdctl/v2/pkg/ipfs"
 )
 
@@ -35,7 +36,7 @@ func RegistryServe(options types.IPFSRegistryServeOptions) error {
 			return err
 		}
 		defer os.RemoveAll(dir)
-		if err := os.WriteFile(filepath.Join(dir, "api"), []byte(options.IPFSAddress), 0600); err != nil {
+		if err := filesystem.WriteFile(filepath.Join(dir, "api"), []byte(options.IPFSAddress), 0600); err != nil {
 			return err
 		}
 		ipfsPath = dir

pkg/composer/serviceparser/serviceparser_test.go

@@ -18,7 +18,6 @@ package serviceparser
 
 import (
 	"fmt"
-	"os"
 	"path/filepath"
 	"runtime"
 	"strconv"
@@ -27,6 +26,7 @@ import (
 	"github.com/compose-spec/compose-go/v2/types"
 	"gotest.tools/v3/assert"
 
+	"github.com/containerd/nerdctl/v2/pkg/internal/filesystem"
 	"github.com/containerd/nerdctl/v2/pkg/strutil"
 	"github.com/containerd/nerdctl/v2/pkg/testutil"
 )
@@ -521,7 +521,7 @@ configs:
 	assert.NilError(t, err)
 
 	for _, f := range []string{"secret1", "secret2", "secret3", "config1", "config2"} {
-		err = os.WriteFile(filepath.Join(project.WorkingDir, f), []byte("content-"+f), 0444)
+		err = filesystem.WriteFile(filepath.Join(project.WorkingDir, f), []byte("content-"+f), 0444)
 		assert.NilError(t, err)
 	}
 

pkg/containerutil/container_network_manager.go

@@ -39,6 +39,7 @@ import (
 	"github.com/containerd/nerdctl/v2/pkg/clientutil"
 	"github.com/containerd/nerdctl/v2/pkg/dnsutil/hostsstore"
 	"github.com/containerd/nerdctl/v2/pkg/idutil/containerwalker"
+	"github.com/containerd/nerdctl/v2/pkg/internal/filesystem"
 	"github.com/containerd/nerdctl/v2/pkg/labels"
 	"github.com/containerd/nerdctl/v2/pkg/mountutil"
 	"github.com/containerd/nerdctl/v2/pkg/netutil"
@@ -829,7 +830,7 @@ func writeEtcHostnameForContainer(globalOptions types.GlobalCommandOptions, host
 	}
 
 	hostnamePath := filepath.Join(stateDir, "hostname")
-	if err := os.WriteFile(hostnamePath, []byte(hostname+"\n"), 0644); err != nil {
+	if err := filesystem.WriteFile(hostnamePath, []byte(hostname+"\n"), 0644); err != nil {
 		return nil, err
 	}
 

pkg/dnsutil/hostsstore/hostsstore.go

@@ -40,6 +40,7 @@ import (
 	"github.com/containerd/errdefs"
 	"github.com/containerd/log"
 
+	"github.com/containerd/nerdctl/v2/pkg/internal/filesystem"
 	"github.com/containerd/nerdctl/v2/pkg/store"
 )
 
@@ -116,11 +117,11 @@ func (x *hostsStore) Acquire(meta Meta) (err error) {
 		// Because of the way we call network manager ContainerNetworkingOpts then SetupNetworking in sequence
 		// we need to make sure we do not overwrite an already allocated hosts file.
 		if _, err = os.Stat(loc); os.IsNotExist(err) {
-			if err = os.WriteFile(loc, []byte{}, 0o644); err != nil {
+			if err = filesystem.WriteFile(loc, []byte{}, 0o644); err != nil {
 				return errors.Join(store.ErrSystemFailure, err)
 			}
 
-			// os.WriteFile relies on syscall.Open. Unless there are ACLs, the effective mode of the file will be matched
+			// WriteFile relies on syscall.Open. Unless there are ACLs, the effective mode of the file will be matched
 			// against the current process umask.
 			// See https://www.man7.org/linux/man-pages/man2/open.2.html for details.
 			// Since we must make sure that these files are world readable, explicitly chmod them here.
@@ -185,12 +186,12 @@ func (x *hostsStore) AllocHostsFile(id string, content []byte) (location string,
 			return err
 		}
 
-		err = os.WriteFile(loc, content, 0o644)
+		err = filesystem.WriteFile(loc, content, 0o644)
 		if err != nil {
 			err = errors.Join(store.ErrSystemFailure, err)
 		}
 
-		// os.WriteFile relies on syscall.Open. Unless there are ACLs, the effective mode of the file will be matched
+		// WriteFile relies on syscall.Open. Unless there are ACLs, the effective mode of the file will be matched
 		// against the current process umask.
 		// See https://www.man7.org/linux/man-pages/man2/open.2.html for details.
 		// Since we must make sure that these files are world readable, explicitly chmod them here.
@@ -351,7 +352,14 @@ func (x *hostsStore) updateAllHosts() (err error) {
 			return err
 		}
 
-		err = os.WriteFile(loc, buf.Bytes(), 0o644)
+		// Because the file is mounted, we cannot do atomic writes here as that would change inode.
+		// The practical implications of this are that a partial / interrupted write would leave the hosts file with
+		// an invalid entry and/or missing entries. At worse, this would lead to a container losing localhost network
+		// capabilities.
+		// Proper consistency requires that we would have a rollback mechanism in case of recoverable failure,
+		// and a disaster management / cleanup mechanism, presumably at the top-level of the operation.
+		// nolint:forbidigo
+		err = filesystem.WriteFile(loc, buf.Bytes(), 0o644)
 		if err != nil {
 			log.L.WithError(err).Errorf("failed to write hosts file for %q", entry)
 		}

pkg/fs.go

@@ -0,0 +1,23 @@
+/*
+   Copyright The containerd Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package pkg
+
+import "github.com/containerd/nerdctl/v2/pkg/internal/filesystem"
+
+func InitFS(path string) error {
+	return filesystem.SetFSOpsDirectory(path)
+}