Write manifests atomically

Use a temporary-write-and-rename approach to write manifests to disk, so
that the appliers never see partially written files. Google's renameio
library does this by creating temorary files whose names start with a
dot. So let the appliers ignore anything that starts with a dot. That
way, there's also no more need to ignore chmod events. They actually
*can* matter, when files become readable or unreadable for the current
process.

Signed-off-by: Tom Wieczorek <[email protected]>

go.mod

@@ -18,6 +18,7 @@ require (
 	github.com/evanphx/json-patch v4.12.0+incompatible
 	github.com/fsnotify/fsnotify v1.5.4
 	github.com/go-openapi/jsonpointer v0.19.5
+	github.com/google/renameio/v2 v2.0.0
 	github.com/gorilla/mux v1.8.0
 	github.com/imdario/mergo v0.3.13
 	github.com/k0sproject/dig v0.2.0
@@ -41,6 +42,7 @@ require (
 	go.etcd.io/etcd/client/pkg/v3 v3.5.4
 	go.etcd.io/etcd/client/v3 v3.5.4
 	go.etcd.io/etcd/etcdutl/v3 v3.5.4
+	go.uber.org/multierr v1.8.0
 	go.uber.org/zap v1.21.0
 	golang.org/x/crypto v0.0.0-20220411220226-7b82a4e95df4
 	golang.org/x/sync v0.0.0-20210220032951-036812b2e83c
@@ -247,7 +249,6 @@ require (
 	go.opentelemetry.io/proto/otlp v0.11.0 // indirect
 	go.starlark.net v0.0.0-20200306205701-8dd3e2ee1dd5 // indirect
 	go.uber.org/atomic v1.9.0 // indirect
-	go.uber.org/multierr v1.8.0 // indirect
 	golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3 // indirect
 	golang.org/x/net v0.0.0-20220425223048-2871e0cb64e4 // indirect
 	golang.org/x/oauth2 v0.0.0-20220223155221-ee480838109b // indirect

go.sum

@@ -736,6 +736,8 @@ github.com/google/pprof v0.0.0-20210601050228-01bbb1931b22/go.mod h1:kpwsk12EmLe
 github.com/google/pprof v0.0.0-20210609004039-a478d1d731e9/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE=
 github.com/google/pprof v0.0.0-20210720184732-4bb14d4b1be1/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE=
 github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI=
+github.com/google/renameio/v2 v2.0.0 h1:UifI23ZTGY8Tt29JbYFiuyIU3eX+RNFtUwefq9qAhxg=
+github.com/google/renameio/v2 v2.0.0/go.mod h1:BtmJXm5YlszgC+TD4HOEEUFgkJP3nLxehU6hfe7jRt4=
 github.com/google/rpmpack v0.0.0-20191226140753-aa36bfddb3a0/go.mod h1:RaTPr0KUf2K7fnZYLNDrr8rxAamWs3iNywJLtQ2AzBg=
 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4=
 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ=

internal/pkg/file/atomic_posix.go

@@ -0,0 +1,55 @@
+//go:build !windows
+// +build !windows
+
+/*
+Copyright 2022 k0s 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 file
+
+import (
+	"io"
+	"os"
+	"path/filepath"
+
+	"github.com/google/renameio/v2"
+	"go.uber.org/multierr"
+)
+
+// WriteAtomically will atomically create or replace a file. The contents of the
+// file will be those that the write callback writes to the Writer that gets
+// passed in. WriteAtomically will buffer the contents in a hidden (i.e. its
+// name will start with a dot), temporary file. When write returns without an
+// error, the temporary file will be renamed to fileName, otherwise it will be
+// deleted without touching the target file.
+func WriteAtomically(fileName string, perm os.FileMode, write func(file io.Writer) error) (err error) {
+	file, err := renameio.NewPendingFile(
+		fileName,
+		renameio.WithTempDir(filepath.Dir(fileName)),
+		renameio.WithPermissions(perm),
+	)
+	if err != nil {
+		return err
+	}
+	defer func() {
+		err = multierr.Append(err, file.Cleanup())
+	}()
+
+	if err := write(file); err != nil {
+		return err
+	}
+
+	return file.CloseAtomicallyReplace()
+}

internal/pkg/file/atomic_windows.go

@@ -0,0 +1,40 @@
+//go:build windows
+// +build windows
+
+/*
+Copyright 2022 k0s 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 file
+
+import (
+	"io"
+	"os"
+
+	"go.uber.org/multierr"
+)
+
+// WriteAtomically is not atomic on windows, but writes the file directly.
+//
+// https://github.com/google/renameio/blob/v2.0.0/README.md#windows-support
+// https://github.com/google/renameio/pull/20
+func WriteAtomically(fileName string, perm os.FileMode, write func(file io.Writer) error) error {
+	file, err := os.OpenFile(fileName, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, perm)
+	if err != nil {
+		return err
+	}
+
+	return multierr.Append(write(file), file.Close())
+}

internal/pkg/file/file.go

@@ -18,7 +18,10 @@ package file
 
 import (
 	"fmt"
+	"io"
 	"os"
+	"path/filepath"
+	"strconv"
 
 	"github.com/k0sproject/k0s/internal/pkg/users"
 )
@@ -33,6 +36,49 @@ func Exists(fileName string) bool {
 	return !info.IsDir()
 }
 
+type DottedBaseName int
+
+const (
+	// NotDotted indicates that a path is not considered to be hidden on
+	// Unix-like operating systems, i.e. it doesn't start with a dot.
+	NotDotted DottedBaseName = iota + 1
+
+	// Dotted indicates that a path is considered to be hidden on Unix-like
+	// operating systems, i.e. it starts with a dot.
+	Dotted
+
+	// RelativeBase indicates that it cannot be decided if the path is
+	// considered to be hidden on Unix-like operating systems or not, since it's
+	// base name is relative.
+	RelativeBase
+)
+
+func (d DottedBaseName) String() string {
+	switch d {
+	case NotDotted:
+		return "NotDotted"
+	case Dotted:
+		return "Dotted"
+	case RelativeBase:
+		return "RelativeBase"
+	}
+
+	return strconv.FormatInt(int64(d), 10)
+}
+
+// IsDottedBaseName checks if the given path is considered to be hidden on
+// Unix-like operating systems, i.e. if its base name starts with a dot.
+func IsDottedBaseName(path string) DottedBaseName {
+	base := filepath.Base(filepath.Clean(path))
+	if base == "." || base == ".." {
+		return RelativeBase
+	}
+	if base[0] == '.' {
+		return Dotted
+	}
+	return NotDotted
+}
+
 // Chown changes file/dir mode
 func Chown(file, owner string, permissions os.FileMode) error {
 	// Chown the file properly for the owner
@@ -84,3 +130,18 @@ func WriteTmpFile(data string, prefix string) (path string, err error) {
 
 	return tmpFile.Name(), nil
 }
+
+// WriteContentAtomically will atomically create or replace a file with the
+// given content. WriteContentAtomically will buffer the contents in a hidden
+// (i.e. its name will start with a dot), temporary file. When write returns
+// without an error, the temporary file will be renamed to fileName, otherwise
+// it will be deleted without touching the target file.
+//
+// WriteContentAtomically is *not* atomic on Windows, but will write directly to
+// the file.
+func WriteContentAtomically(fileName string, content []byte, perm os.FileMode) error {
+	return WriteAtomically(fileName, perm, func(file io.Writer) error {
+		_, err := file.Write(content)
+		return err
+	})
+}

internal/pkg/file/file_test.go

@@ -16,9 +16,10 @@ limitations under the License.
 package file
 
 import (
+	"fmt"
 	"os"
-	"path"
 	"path/filepath"
+	"runtime"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -35,7 +36,7 @@ func TestExists(t *testing.T) {
 		t.Errorf("test non-existing: got %t, wanted %t", got, want)
 	}
 
-	existingFileName := path.Join(dir, "existing")
+	existingFileName := filepath.Join(dir, "existing")
 	require.NoError(t, os.WriteFile(existingFileName, []byte{}, 0644))
 
 	// test existing
@@ -58,3 +59,64 @@ func TestExists(t *testing.T) {
 	}
 
 }
+
+func TestIsDottedBaseName(t *testing.T) {
+	roots := []struct {
+		name   string
+		dotted DottedBaseName
+		prefix string
+	}{
+		{"absolute", NotDotted, "/rooted/path/"},
+		{"relative", RelativeBase, ""},
+	}
+
+	if runtime.GOOS == "windows" {
+		roots[0].prefix = `C:\rooted\path\`
+	}
+
+	paths := []struct {
+		name, path string
+		dotted     DottedBaseName
+		skip       int
+	}{
+		{"Empty", "", NotDotted, 1},
+		{"Dot", ".", RelativeBase, 1},
+		{"DotDot", "..", RelativeBase, 2},
+		{"DotDotDot", "...", Dotted, 0},
+		{"A", "A", NotDotted, 0},
+		{"DotA", ".A", Dotted, 0},
+		{"ADot", "A.", NotDotted, 0},
+	}
+
+	assert.Equal(t, "...", filepath.Join("...", "."))
+	assert.Equal(t, "...", filepath.Base("..."))
+
+	for _, root := range roots {
+		for _, dir := range paths {
+			for _, file := range paths {
+				t.Run(fmt.Sprintf("%s_%sDir_%sFile", root.name, dir.name, file.name), func(t *testing.T) {
+					expectedDotted := file.dotted
+					if file.skip > 0 {
+						expectedDotted = dir.dotted
+						if file.skip+dir.skip > 1 {
+							expectedDotted = root.dotted
+						}
+					}
+
+					path := root.prefix
+					if dir.path != "" {
+						path += dir.path
+						path += string(filepath.Separator)
+					}
+					path += file.path
+
+					dotted := IsDottedBaseName(path)
+					t.Logf("%v := IsDottedBaseName(%q)", dotted, path)
+					if expectedDotted != dotted {
+						assert.Fail(t, dotted.String(), "Expected %q to be %v", path, expectedDotted)
+					}
+				})
+			}
+		}
+	}
+}

internal/pkg/templatewriter/templatewriter.go

@@ -18,11 +18,11 @@ package templatewriter
 import (
 	"fmt"
 	"io"
-	"os"
 	"text/template"
 
 	"github.com/Masterminds/sprig"
 
+	"github.com/k0sproject/k0s/internal/pkg/file"
 	"github.com/k0sproject/k0s/pkg/constant"
 )
 
@@ -34,14 +34,9 @@ type TemplateWriter struct {
 	Path     string
 }
 
-// Write writes executes the template and writes the results on disk
+// Write executes the template and writes the results on disk
 func (p *TemplateWriter) Write() error {
-	podFile, err := os.OpenFile(p.Path, os.O_RDWR|os.O_CREATE|os.O_TRUNC, constant.CertMode)
-	if err != nil {
-		return fmt.Errorf("failed to open pod file for %s: %w", p.Name, err)
-	}
-	defer podFile.Close()
-	return p.WriteToBuffer(podFile)
+	return file.WriteAtomically(p.Path, constant.CertMode, p.WriteToBuffer)
 }
 
 // WriteToBuffer writes executed template tot he given writer

pkg/applier/applier.go

@@ -23,7 +23,9 @@ import (
 	"path"
 	"path/filepath"
 
-	"github.com/sirupsen/logrus"
+	"github.com/k0sproject/k0s/internal/pkg/file"
+	"github.com/k0sproject/k0s/pkg/kubernetes"
+
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/cli-runtime/pkg/resource"
@@ -32,7 +34,7 @@ import (
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/restmapper"
 
-	"github.com/k0sproject/k0s/pkg/kubernetes"
+	"github.com/sirupsen/logrus"
 )
 
 // manifestFilePattern is the glob pattern that all applicable manifest files need to match.
@@ -104,10 +106,12 @@ func (a *Applier) Apply(ctx context.Context) error {
 	if err != nil {
 		return err
 	}
-	files, err := filepath.Glob(path.Join(a.Dir, manifestFilePattern))
+
+	files, err := a.listFiles()
 	if err != nil {
 		return err
 	}
+
 	resources, err := a.parseFiles(files)
 	if err != nil {
 		return err
@@ -176,6 +180,23 @@ func (a *Applier) parseFiles(files []string) ([]*unstructured.Unstructured, erro
 	return resources, nil
 }
 
+func (a *Applier) listFiles() ([]string, error) {
+	matches, err := filepath.Glob(path.Join(a.Dir, manifestFilePattern))
+	if err != nil {
+		return nil, err
+	}
+
+	// filter out hidden files
+	var filtered []string
+	for _, match := range matches {
+		if file.IsDottedBaseName(match) == file.NotDotted {
+			filtered = append(filtered, match)
+		}
+	}
+
+	return filtered, nil
+}
+
 type restClientGetter struct {
 	clientFactory kubernetes.ClientFactoryInterface
 }