From 09cda512f9beddf39e6ca964c5bef93459e018b3 Mon Sep 17 00:00:00 2001 From: jdymitarai Date: Sat, 3 Oct 2026 13:15:41 +0900 Subject: [PATCH] fix(repo): write repositories.yaml atomically using AtomicWriteFile File.WriteFile used os.WriteFile, which truncates the destination file before writing data. If the write fails (due to quota, full disk, etc.) or the process is interrupted, repositories.yaml is left empty. Use fileutil.AtomicWriteFile to ensure updates to repositories.yaml are performed atomically via temporary file and rename, matching the behavior of index.go and chartrepo.go. Signed-off-by: jdymitarai --- pkg/repo/v1/repo.go | 5 ++++- pkg/repo/v1/repo_test.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/pkg/repo/v1/repo.go b/pkg/repo/v1/repo.go index be241a0ab..97adb2726 100644 --- a/pkg/repo/v1/repo.go +++ b/pkg/repo/v1/repo.go @@ -17,12 +17,15 @@ limitations under the License. package repo import ( + "bytes" "fmt" "os" "path/filepath" "time" "sigs.k8s.io/yaml" + + "helm.sh/helm/v4/internal/fileutil" ) // File represents the repositories.yaml file @@ -121,5 +124,5 @@ func (r *File) WriteFile(path string, perm os.FileMode) error { if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { return err } - return os.WriteFile(path, data, perm) + return fileutil.AtomicWriteFile(path, bytes.NewReader(data), perm) } diff --git a/pkg/repo/v1/repo_test.go b/pkg/repo/v1/repo_test.go index 2fe780ce3..006e64596 100644 --- a/pkg/repo/v1/repo_test.go +++ b/pkg/repo/v1/repo_test.go @@ -18,6 +18,7 @@ package repo import ( "os" + "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -168,6 +169,7 @@ func TestWriteFile(t *testing.T) { file, err := os.CreateTemp(t.TempDir(), "helm-repo") require.NoErrorf(t, err, "failed to create test-file") + require.NoError(t, file.Close()) defer os.Remove(file.Name()) require.NoErrorf(t, sampleRepository.WriteFile(file.Name(), 0o600), "failed to write file") @@ -207,3 +209,32 @@ func TestRemoveRepositoryInvalidEntries(t *testing.T) { assert.Truef(t, sampleRepository.Remove(removeRepository), "expected repository %s not found", removeRepository) assert.Falsef(t, sampleRepository.Has(removeRepository), "repository %s not deleted", removeRepository) } + +func TestWriteFile_Atomic(t *testing.T) { + dir := t.TempDir() + filePath := filepath.Join(dir, "repositories.yaml") + + initialRepo := NewFile() + initialRepo.Add(&Entry{ + Name: "initial", + URL: "https://example.com/initial", + }) + require.NoError(t, initialRepo.WriteFile(filePath, 0o644)) + + initialData, err := os.ReadFile(filePath) + require.NoError(t, err) + require.NotEmpty(t, initialData) + + updatedRepo := NewFile() + updatedRepo.Add(&Entry{ + Name: "updated", + URL: "https://example.com/updated", + }) + require.NoError(t, updatedRepo.WriteFile(filePath, 0o644)) + + loaded, err := LoadFile(filePath) + require.NoError(t, err) + assert.False(t, loaded.Has("initial")) + assert.True(t, loaded.Has("updated")) +} +