Skip to content

Commit 21e1124

Browse files
apostasieapostasie
committed
Move from os filesystem operations to internal/filesystem
Signed-off-by: apostasie <[email protected]>
1 parent f38310b commit 21e1124

File tree

31 files changed

+392
-69
lines changed

31 files changed

+392
-69
lines changed

‎pkg/buildkitutil/buildkitutil_test.go

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ import (
2929
"testing"
3030

3131
"gotest.tools/v3/assert"
32+
33+
"github.com/containerd/nerdctl/v2/pkg/internal/filesystem"
3234
)
3335

3436
func TestBuildKitFile(t *testing.T) {
@@ -55,7 +57,7 @@ func TestBuildKitFile(t *testing.T) {
5557
{
5658
name: "only Dockerfile is present",
5759
prepare: func(t *testing.T) error {
58-
return os.WriteFile(filepath.Join(tmp, DefaultDockerfileName), []byte{}, 0644)
60+
return filesystem.WriteFile(filepath.Join(tmp, DefaultDockerfileName), []byte{}, 0644)
5961
},
6062
args: args{".", ""},
6163
wantAbsDir: tmp,
@@ -65,7 +67,7 @@ func TestBuildKitFile(t *testing.T) {
6567
{
6668
name: "only Containerfile is present",
6769
prepare: func(t *testing.T) error {
68-
return os.WriteFile(filepath.Join(tmp, "Containerfile"), []byte{}, 0644)
70+
return filesystem.WriteFile(filepath.Join(tmp, "Containerfile"), []byte{}, 0644)
6971
},
7072
args: args{".", ""},
7173
wantAbsDir: tmp,
@@ -75,11 +77,11 @@ func TestBuildKitFile(t *testing.T) {
7577
{
7678
name: "both Dockerfile and Containerfile are present",
7779
prepare: func(t *testing.T) error {
78-
var err = os.WriteFile(filepath.Join(tmp, "Dockerfile"), []byte{}, 0644)
80+
var err = filesystem.WriteFile(filepath.Join(tmp, "Dockerfile"), []byte{}, 0644)
7981
if err != nil {
8082
return err
8183
}
82-
return os.WriteFile(filepath.Join(tmp, "Containerfile"), []byte{}, 0644)
84+
return filesystem.WriteFile(filepath.Join(tmp, "Containerfile"), []byte{}, 0644)
8385
},
8486
args: args{".", ""},
8587
wantAbsDir: tmp,
@@ -89,11 +91,11 @@ func TestBuildKitFile(t *testing.T) {
8991
{
9092
name: "Dockerfile and Containerfile have different contents",
9193
prepare: func(t *testing.T) error {
92-
var err = os.WriteFile(filepath.Join(tmp, "Dockerfile"), []byte{'d'}, 0644)
94+
var err = filesystem.WriteFile(filepath.Join(tmp, "Dockerfile"), []byte{'d'}, 0644)
9395
if err != nil {
9496
return err
9597
}
96-
return os.WriteFile(filepath.Join(tmp, "Containerfile"), []byte{'c'}, 0644)
98+
return filesystem.WriteFile(filepath.Join(tmp, "Containerfile"), []byte{'c'}, 0644)
9799
},
98100
args: args{".", ""},
99101
wantAbsDir: tmp,
@@ -103,7 +105,7 @@ func TestBuildKitFile(t *testing.T) {
103105
{
104106
name: "Custom file is specfied",
105107
prepare: func(t *testing.T) error {
106-
return os.WriteFile(filepath.Join(tmp, "CustomFile"), []byte{}, 0644)
108+
return filesystem.WriteFile(filepath.Join(tmp, "CustomFile"), []byte{}, 0644)
107109
},
108110
args: args{".", "CustomFile"},
109111
wantAbsDir: tmp,
@@ -113,7 +115,7 @@ func TestBuildKitFile(t *testing.T) {
113115
{
114116
name: "Absolute path is specified along with custom file",
115117
prepare: func(t *testing.T) error {
116-
return os.WriteFile(filepath.Join(tmp, "CustomFile"), []byte{}, 0644)
118+
return filesystem.WriteFile(filepath.Join(tmp, "CustomFile"), []byte{}, 0644)
117119
},
118120
args: args{tmp, "CustomFile"},
119121
wantAbsDir: tmp,
@@ -123,7 +125,7 @@ func TestBuildKitFile(t *testing.T) {
123125
{
124126
name: "Absolute path is specified along with Docker file",
125127
prepare: func(t *testing.T) error {
126-
return os.WriteFile(filepath.Join(tmp, "Dockerfile"), []byte{}, 0644)
128+
return filesystem.WriteFile(filepath.Join(tmp, "Dockerfile"), []byte{}, 0644)
127129
},
128130
args: args{tmp, "."},
129131
wantAbsDir: tmp,
@@ -133,7 +135,7 @@ func TestBuildKitFile(t *testing.T) {
133135
{
134136
name: "Absolute path is specified with Container file in the path",
135137
prepare: func(t *testing.T) error {
136-
return os.WriteFile(filepath.Join(tmp, ContainerfileName), []byte{}, 0644)
138+
return filesystem.WriteFile(filepath.Join(tmp, ContainerfileName), []byte{}, 0644)
137139
},
138140
args: args{tmp, "."},
139141
wantAbsDir: tmp,

‎pkg/cmd/builder/build.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import (
4141
"github.com/containerd/nerdctl/v2/pkg/buildkitutil"
4242
"github.com/containerd/nerdctl/v2/pkg/clientutil"
4343
"github.com/containerd/nerdctl/v2/pkg/containerutil"
44+
"github.com/containerd/nerdctl/v2/pkg/internal/filesystem"
4445
"github.com/containerd/nerdctl/v2/pkg/platformutil"
4546
"github.com/containerd/nerdctl/v2/pkg/referenceutil"
4647
"github.com/containerd/nerdctl/v2/pkg/strutil"
@@ -110,7 +111,7 @@ func Build(ctx context.Context, client *containerd.Client, options types.Builder
110111
if err != nil {
111112
return err
112113
}
113-
if err := os.WriteFile(options.IidFile, []byte(id), 0644); err != nil {
114+
if err := filesystem.WriteFile(options.IidFile, []byte(id), 0644); err != nil {
114115
return err
115116
}
116117
}

‎pkg/cmd/compose/compose.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import (
3434
"github.com/containerd/nerdctl/v2/pkg/composer"
3535
"github.com/containerd/nerdctl/v2/pkg/composer/serviceparser"
3636
"github.com/containerd/nerdctl/v2/pkg/imgutil"
37+
"github.com/containerd/nerdctl/v2/pkg/internal/filesystem"
3738
"github.com/containerd/nerdctl/v2/pkg/ipfs"
3839
"github.com/containerd/nerdctl/v2/pkg/netutil"
3940
"github.com/containerd/nerdctl/v2/pkg/referenceutil"
@@ -136,7 +137,7 @@ func New(client *containerd.Client, globalOptions types.GlobalCommandOptions, op
136137
return err
137138
}
138139
defer os.RemoveAll(dir)
139-
if err := os.WriteFile(filepath.Join(dir, "api"), []byte(ipfsAddress), 0600); err != nil {
140+
if err := filesystem.WriteFile(filepath.Join(dir, "api"), []byte(ipfsAddress), 0600); err != nil {
140141
return err
141142
}
142143
ipfsPath = dir

‎pkg/cmd/container/create.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ import (
5151
"github.com/containerd/nerdctl/v2/pkg/imgutil"
5252
"github.com/containerd/nerdctl/v2/pkg/imgutil/load"
5353
"github.com/containerd/nerdctl/v2/pkg/inspecttypes/dockercompat"
54+
"github.com/containerd/nerdctl/v2/pkg/internal/filesystem"
5455
"github.com/containerd/nerdctl/v2/pkg/ipcutil"
5556
"github.com/containerd/nerdctl/v2/pkg/labels"
5657
"github.com/containerd/nerdctl/v2/pkg/logging"
@@ -951,7 +952,7 @@ func generateLogConfig(dataStore string, id string, logDriver string, logOpt []s
951952
}
952953

953954
logConfigFilePath := logging.LogConfigFilePath(dataStore, ns, id)
954-
if err = os.WriteFile(logConfigFilePath, logConfigB, 0600); err != nil {
955+
if err = filesystem.WriteFile(logConfigFilePath, logConfigB, 0600); err != nil {
955956
return logConfig, err
956957
}
957958

‎pkg/cmd/image/pull.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626

2727
"github.com/containerd/nerdctl/v2/pkg/api/types"
2828
"github.com/containerd/nerdctl/v2/pkg/imgutil"
29+
"github.com/containerd/nerdctl/v2/pkg/internal/filesystem"
2930
"github.com/containerd/nerdctl/v2/pkg/ipfs"
3031
"github.com/containerd/nerdctl/v2/pkg/referenceutil"
3132
"github.com/containerd/nerdctl/v2/pkg/signutil"
@@ -62,7 +63,7 @@ func EnsureImage(ctx context.Context, client *containerd.Client, rawRef string,
6263
return nil, err
6364
}
6465
defer os.RemoveAll(dir)
65-
if err := os.WriteFile(filepath.Join(dir, "api"), []byte(options.IPFSAddress), 0600); err != nil {
66+
if err := filesystem.WriteFile(filepath.Join(dir, "api"), []byte(options.IPFSAddress), 0600); err != nil {
6667
return nil, err
6768
}
6869
ipfsPath = dir

‎pkg/cmd/image/push.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import (
4646
nerdconverter "github.com/containerd/nerdctl/v2/pkg/imgutil/converter"
4747
"github.com/containerd/nerdctl/v2/pkg/imgutil/dockerconfigresolver"
4848
"github.com/containerd/nerdctl/v2/pkg/imgutil/push"
49+
"github.com/containerd/nerdctl/v2/pkg/internal/filesystem"
4950
"github.com/containerd/nerdctl/v2/pkg/ipfs"
5051
"github.com/containerd/nerdctl/v2/pkg/platformutil"
5152
"github.com/containerd/nerdctl/v2/pkg/referenceutil"
@@ -85,7 +86,7 @@ func Push(ctx context.Context, client *containerd.Client, rawRef string, options
8586
return err
8687
}
8788
defer os.RemoveAll(dir)
88-
if err := os.WriteFile(filepath.Join(dir, "api"), []byte(options.IpfsAddress), 0600); err != nil {
89+
if err := filesystem.WriteFile(filepath.Join(dir, "api"), []byte(options.IpfsAddress), 0600); err != nil {
8990
return err
9091
}
9192
ipfsPath = dir

‎pkg/cmd/ipfs/registry_serve.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"github.com/containerd/log"
2525

2626
"github.com/containerd/nerdctl/v2/pkg/api/types"
27+
"github.com/containerd/nerdctl/v2/pkg/internal/filesystem"
2728
"github.com/containerd/nerdctl/v2/pkg/ipfs"
2829
)
2930

@@ -35,7 +36,7 @@ func RegistryServe(options types.IPFSRegistryServeOptions) error {
3536
return err
3637
}
3738
defer os.RemoveAll(dir)
38-
if err := os.WriteFile(filepath.Join(dir, "api"), []byte(options.IPFSAddress), 0600); err != nil {
39+
if err := filesystem.WriteFile(filepath.Join(dir, "api"), []byte(options.IPFSAddress), 0600); err != nil {
3940
return err
4041
}
4142
ipfsPath = dir

‎pkg/composer/serviceparser/serviceparser_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ package serviceparser
1818

1919
import (
2020
"fmt"
21-
"os"
2221
"path/filepath"
2322
"runtime"
2423
"strconv"
@@ -27,6 +26,7 @@ import (
2726
"github.com/compose-spec/compose-go/v2/types"
2827
"gotest.tools/v3/assert"
2928

29+
"github.com/containerd/nerdctl/v2/pkg/internal/filesystem"
3030
"github.com/containerd/nerdctl/v2/pkg/strutil"
3131
"github.com/containerd/nerdctl/v2/pkg/testutil"
3232
)
@@ -521,7 +521,7 @@ configs:
521521
assert.NilError(t, err)
522522

523523
for _, f := range []string{"secret1", "secret2", "secret3", "config1", "config2"} {
524-
err = os.WriteFile(filepath.Join(project.WorkingDir, f), []byte("content-"+f), 0444)
524+
err = filesystem.WriteFile(filepath.Join(project.WorkingDir, f), []byte("content-"+f), 0444)
525525
assert.NilError(t, err)
526526
}
527527

‎pkg/containerutil/container_network_manager.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import (
3939
"github.com/containerd/nerdctl/v2/pkg/clientutil"
4040
"github.com/containerd/nerdctl/v2/pkg/dnsutil/hostsstore"
4141
"github.com/containerd/nerdctl/v2/pkg/idutil/containerwalker"
42+
"github.com/containerd/nerdctl/v2/pkg/internal/filesystem"
4243
"github.com/containerd/nerdctl/v2/pkg/labels"
4344
"github.com/containerd/nerdctl/v2/pkg/mountutil"
4445
"github.com/containerd/nerdctl/v2/pkg/netutil"
@@ -829,7 +830,7 @@ func writeEtcHostnameForContainer(globalOptions types.GlobalCommandOptions, host
829830
}
830831

831832
hostnamePath := filepath.Join(stateDir, "hostname")
832-
if err := os.WriteFile(hostnamePath, []byte(hostname+"\n"), 0644); err != nil {
833+
if err := filesystem.WriteFile(hostnamePath, []byte(hostname+"\n"), 0644); err != nil {
833834
return nil, err
834835
}
835836

‎pkg/dnsutil/hostsstore/hostsstore.go

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import (
4040
"github.com/containerd/errdefs"
4141
"github.com/containerd/log"
4242

43+
"github.com/containerd/nerdctl/v2/pkg/internal/filesystem"
4344
"github.com/containerd/nerdctl/v2/pkg/store"
4445
)
4546

@@ -116,11 +117,11 @@ func (x *hostsStore) Acquire(meta Meta) (err error) {
116117
// Because of the way we call network manager ContainerNetworkingOpts then SetupNetworking in sequence
117118
// we need to make sure we do not overwrite an already allocated hosts file.
118119
if _, err = os.Stat(loc); os.IsNotExist(err) {
119-
if err = os.WriteFile(loc, []byte{}, 0o644); err != nil {
120+
if err = filesystem.WriteFile(loc, []byte{}, 0o644); err != nil {
120121
return errors.Join(store.ErrSystemFailure, err)
121122
}
122123

123-
// os.WriteFile relies on syscall.Open. Unless there are ACLs, the effective mode of the file will be matched
124+
// WriteFile relies on syscall.Open. Unless there are ACLs, the effective mode of the file will be matched
124125
// against the current process umask.
125126
// See https://www.man7.org/linux/man-pages/man2/open.2.html for details.
126127
// 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,
185186
return err
186187
}
187188

188-
err = os.WriteFile(loc, content, 0o644)
189+
err = filesystem.WriteFile(loc, content, 0o644)
189190
if err != nil {
190191
err = errors.Join(store.ErrSystemFailure, err)
191192
}
192193

193-
// os.WriteFile relies on syscall.Open. Unless there are ACLs, the effective mode of the file will be matched
194+
// WriteFile relies on syscall.Open. Unless there are ACLs, the effective mode of the file will be matched
194195
// against the current process umask.
195196
// See https://www.man7.org/linux/man-pages/man2/open.2.html for details.
196197
// Since we must make sure that these files are world readable, explicitly chmod them here.
@@ -351,6 +352,13 @@ func (x *hostsStore) updateAllHosts() (err error) {
351352
return err
352353
}
353354

355+
// Because the file is mounted, we cannot do atomic writes here as that would change inode.
356+
// The practical implications of this are that a partial / interrupted write would leave the hosts file with
357+
// an invalid entry and/or missing entries. At worse, this would lead to a container losing localhost network
358+
// capabilities.
359+
// Proper consistency requires that we would have a rollback mechanism in case of recoverable failure,
360+
// and a disaster management / cleanup mechanism, presumably at the top-level of the operation.
361+
// nolint:forbidigo
354362
err = os.WriteFile(loc, buf.Bytes(), 0o644)
355363
if err != nil {
356364
log.L.WithError(err).Errorf("failed to write hosts file for %q", entry)

0 commit comments

Comments
 (0)