From 4285a195222086972282bf22e643c513f2924363 Mon Sep 17 00:00:00 2001 From: Tom Wieczorek Date: Fri, 26 Apr 2024 22:14:23 +0200 Subject: [PATCH] Remove *os.File hack Use file.AtomicOpener directly when writing the keepalived config file. This was the only hack that relied on using *os.File directly. Signed-off-by: Tom Wieczorek --- internal/pkg/file/atomic.go | 5 +---- internal/pkg/file/atomic_test.go | 18 +++++++++--------- pkg/component/controller/cplb_unix.go | 23 ++++++++++------------- 3 files changed, 20 insertions(+), 26 deletions(-) diff --git a/internal/pkg/file/atomic.go b/internal/pkg/file/atomic.go index fb0e8a03e47a..829376e62234 100644 --- a/internal/pkg/file/atomic.go +++ b/internal/pkg/file/atomic.go @@ -323,10 +323,7 @@ func remove(fd *os.File) error { // https://github.com/golang/go/issues/22397#issuecomment-498856679 func WriteAtomically(fileName string, perm os.FileMode, write func(file io.Writer) error) (err error) { return AtomicWithTarget(fileName).WithPermissions(perm).Do(func(unbuffered AtomicWriter) error { - // Pass the real file to the callback, so that callers that are cheating - // and relied on this implementation detail won't break. - fd := unbuffered.(*Atomic).fd - return write(fd) + return write(unbuffered) }) } diff --git a/internal/pkg/file/atomic_test.go b/internal/pkg/file/atomic_test.go index 372877ee59da..2286a2c4277f 100644 --- a/internal/pkg/file/atomic_test.go +++ b/internal/pkg/file/atomic_test.go @@ -134,9 +134,9 @@ func TestWriteAtomically(t *testing.T) { file := filepath.Join(dir, "file") errs := flatten(WriteAtomically(file, 0644, func(file io.Writer) error { - c, ok := file.(io.Closer) - require.True(t, ok, "Not closeable: %T", file) - require.NoError(t, c.Close()) + a, ok := file.(*Atomic) + require.True(t, ok, "Not an Atomic: %T", file) + require.NoError(t, a.fd.Close()) return nil })) @@ -169,9 +169,9 @@ func TestWriteAtomically(t *testing.T) { var tempPath string err := WriteAtomically(file, 0644, func(file io.Writer) error { - n, ok := file.(interface{ Name() string }) - require.True(t, ok, "Doesn't have a name: %T", file) - tempPath = n.Name() + a, ok := file.(*Atomic) + require.True(t, ok, "Not an Atomic: %T", file) + tempPath = a.fd.Name() require.Equal(t, dir, filepath.Dir(tempPath)) if runtime.GOOS == "windows" { t.Skip("Cannot remove a file which is still opened on Windows") @@ -245,9 +245,9 @@ func TestWriteAtomically(t *testing.T) { var tempPath string errs := flatten(WriteAtomically(file, 0755, func(file io.Writer) error { - n, ok := file.(interface{ Name() string }) - require.True(t, ok, "Doesn't have a name: %T", file) - tempPath = n.Name() + a, ok := file.(*Atomic) + require.True(t, ok, "Not an Atomic: %T", file) + tempPath = a.fd.Name() require.Equal(t, dir, filepath.Dir(tempPath)) if runtime.GOOS == "windows" { diff --git a/pkg/component/controller/cplb_unix.go b/pkg/component/controller/cplb_unix.go index e189bc4e1680..9c4ef9b36934 100644 --- a/pkg/component/controller/cplb_unix.go +++ b/pkg/component/controller/cplb_unix.go @@ -23,9 +23,7 @@ import ( "context" "errors" "fmt" - "io" "net" - "os" "path/filepath" "slices" "syscall" @@ -310,17 +308,16 @@ func (*Keepalived) getLinkAddresses(link netlink.Link) ([]netlink.Addr, []string } func (k *Keepalived) generateKeepalivedTemplate() error { - if err := file.WriteAtomically(k.configFilePath, 0400, func(file io.Writer) error { - if err := file.(*os.File).Chown(k.uid, -1); err != nil { - return err - } - - w := bufio.NewWriter(file) - if err := keepalivedConfigTemplate.Execute(w, k.keepalivedConfig); err != nil { - return err - } - return w.Flush() - }); err != nil { + if err := file.AtomicWithTarget(k.configFilePath). + WithPermissions(0400). + WithOwner(k.uid). + Do(func(unbuffered file.AtomicWriter) error { + w := bufio.NewWriter(unbuffered) + if err := keepalivedConfigTemplate.Execute(w, k.keepalivedConfig); err != nil { + return err + } + return w.Flush() + }); err != nil { return fmt.Errorf("failed to write keepalived config file: %w", err) }