From f7e2f40afdd504ce5f6628799df8e9024fa336d0 Mon Sep 17 00:00:00 2001 From: OlTrenin Date: Fri, 26 Dec 2025 19:34:35 +0100 Subject: [PATCH 1/2] fix(plugins): reset cache repo modifications before update Signed-off-by: OlTrenin --- internal/plugin/installer/vcs_installer.go | 54 +++++++++++- .../plugin/installer/vcs_installer_test.go | 87 +++++++++++++++++-- 2 files changed, 133 insertions(+), 8 deletions(-) diff --git a/internal/plugin/installer/vcs_installer.go b/internal/plugin/installer/vcs_installer.go index 3601ec7a8..e7124a6c4 100644 --- a/internal/plugin/installer/vcs_installer.go +++ b/internal/plugin/installer/vcs_installer.go @@ -21,6 +21,7 @@ import ( stdfs "io/fs" "log/slog" "os" + "os/exec" "sort" "github.com/Masterminds/semver/v3" @@ -95,12 +96,63 @@ func (i *VCSInstaller) Install() error { return fs.CopyDir(i.Repo.LocalPath(), i.Path()) } +// resetRepo discards all local modifications in the repository. +// This is used to clean the cached repository before updating. +func resetRepo(repo vcs.Repo) error { + // Check the VCS type to determine the appropriate reset command + switch repo.Vcs() { + case vcs.Git: + // For Git, use 'git reset --hard' to discard all local changes + cmd := exec.Command("git", "reset", "--hard") + cmd.Dir = repo.LocalPath() + if output, err := cmd.CombinedOutput(); err != nil { + return fmt.Errorf("git reset failed: %w, output: %s", err, output) + } + return nil + case vcs.Hg: + // For Mercurial, use 'hg revert --all --no-backup' + cmd := exec.Command("hg", "revert", "--all", "--no-backup") + cmd.Dir = repo.LocalPath() + if output, err := cmd.CombinedOutput(); err != nil { + return fmt.Errorf("hg revert failed: %w, output: %s", err, output) + } + return nil + case vcs.Bzr: + // For Bazaar, use 'bzr revert' + cmd := exec.Command("bzr", "revert") + cmd.Dir = repo.LocalPath() + if output, err := cmd.CombinedOutput(); err != nil { + return fmt.Errorf("bzr revert failed: %w, output: %s", err, output) + } + return nil + case vcs.Svn: + // For SVN, use 'svn revert -R .' + cmd := exec.Command("svn", "revert", "-R", ".") + cmd.Dir = repo.LocalPath() + if output, err := cmd.CombinedOutput(); err != nil { + return fmt.Errorf("svn revert failed: %w, output: %s", err, output) + } + return nil + default: + return fmt.Errorf("unsupported VCS type: %v", repo.Vcs()) + } +} + // Update updates a remote repository func (i *VCSInstaller) Update() error { slog.Debug("updating", "source", i.Repo.Remote()) + + // Reset any local modifications in the cache directory before updating. + // The cached repository is managed by Helm and should not contain user modifications. + // Any modifications made by Helm itself (e.g., to plugin.yaml during installation) + // should be discarded before attempting to update. if i.Repo.IsDirty() { - return errors.New("plugin repo was modified") + slog.Debug("resetting local modifications in cache", "path", i.Repo.LocalPath()) + if err := resetRepo(i.Repo); err != nil { + return fmt.Errorf("failed to reset local modifications: %w", err) + } } + if err := i.Repo.Update(); err != nil { return err } diff --git a/internal/plugin/installer/vcs_installer_test.go b/internal/plugin/installer/vcs_installer_test.go index d542a0f75..ea11b7e53 100644 --- a/internal/plugin/installer/vcs_installer_test.go +++ b/internal/plugin/installer/vcs_installer_test.go @@ -175,15 +175,88 @@ func TestVCSInstallerUpdate(t *testing.T) { t.Fatal(err) } - // Test update failure - if err := os.Remove(filepath.Join(vcsInstaller.Repo.LocalPath(), "plugin.yaml")); err != nil { + // Test that local modifications are automatically reset during update + // Remove plugin.yaml to simulate modifications + pluginYamlPath := filepath.Join(vcsInstaller.Repo.LocalPath(), "plugin.yaml") + if err := os.Remove(pluginYamlPath); err != nil { t.Fatal(err) } - // Testing update for error - if err := Update(vcsInstaller); err == nil { - t.Fatalf("expected error for plugin modified, got none") - } else if err.Error() != "plugin repo was modified" { - t.Fatalf("expected error for plugin modified, got (%v)", err) + + // Verify the repo is dirty + if !vcsInstaller.Repo.IsDirty() { + t.Fatal("expected repo to be dirty after removing plugin.yaml") + } + + // Update should succeed because local modifications are automatically reset + if err := Update(vcsInstaller); err != nil { + t.Fatalf("update should succeed after automatic reset, got error: %v", err) + } + + // Verify plugin.yaml was restored after reset + if _, err := os.Stat(pluginYamlPath); err != nil { + t.Fatalf("plugin.yaml should be restored after update, got error: %v", err) + } + +} + +func TestResetRepo(t *testing.T) { + // Use a real git repository by cloning a test repo + ensure.HelmHome(t) + + source := "https://github.com/adamreese/helm-env" + + i, err := NewForSource(source, "") + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + vcsInstaller, ok := i.(*VCSInstaller) + if !ok { + t.Fatal("expected a VCSInstaller") } + // Clone the repository + if err := vcsInstaller.sync(vcsInstaller.Repo); err != nil { + if strings.Contains(err.Error(), "Could not resolve host: github.com") { + t.Skip("Unable to run test without Internet access") + } + t.Fatalf("Failed to sync repo: %v", err) + } + + // Modify an existing file to make the repo dirty + pluginYaml := filepath.Join(vcsInstaller.Repo.LocalPath(), "plugin.yaml") + originalContent, err := os.ReadFile(pluginYaml) + if err != nil { + t.Fatalf("Failed to read plugin.yaml: %v", err) + } + + modifiedContent := append(originalContent, []byte("\n# Test modification\n")...) + if err := os.WriteFile(pluginYaml, modifiedContent, 0644); err != nil { + t.Fatalf("Failed to modify plugin.yaml: %v", err) + } + + // Verify the repo is dirty + if !vcsInstaller.Repo.IsDirty() { + t.Fatal("Expected repo to be dirty after modifying plugin.yaml") + } + + // Reset the repo + if err := resetRepo(vcsInstaller.Repo); err != nil { + t.Fatalf("resetRepo failed: %v", err) + } + + // Verify the repo is clean + if vcsInstaller.Repo.IsDirty() { + t.Fatal("Expected repo to be clean after reset") + } + + // Verify the plugin.yaml was restored to original content + restoredContent, err := os.ReadFile(pluginYaml) + if err != nil { + t.Fatalf("Failed to read plugin.yaml after reset: %v", err) + } + + if string(restoredContent) != string(originalContent) { + t.Fatal("Expected plugin.yaml to be restored to original content after reset") + } } From 98598c66a395df8632c8b9a6f491d4fd439d4af1 Mon Sep 17 00:00:00 2001 From: OlTrenin Date: Sun, 4 Jan 2026 14:08:26 +0100 Subject: [PATCH 2/2] fix(plugins): reset only plugin.yaml instead of entire repo Signed-off-by: OlTrenin --- internal/plugin/installer/vcs_installer.go | 37 ++++++++++--------- .../plugin/installer/vcs_installer_test.go | 12 +++--- 2 files changed, 26 insertions(+), 23 deletions(-) diff --git a/internal/plugin/installer/vcs_installer.go b/internal/plugin/installer/vcs_installer.go index e7124a6c4..f8b042e4b 100644 --- a/internal/plugin/installer/vcs_installer.go +++ b/internal/plugin/installer/vcs_installer.go @@ -96,38 +96,41 @@ func (i *VCSInstaller) Install() error { return fs.CopyDir(i.Repo.LocalPath(), i.Path()) } -// resetRepo discards all local modifications in the repository. +// resetPluginYaml discards local modifications to plugin.yaml file. // This is used to clean the cached repository before updating. -func resetRepo(repo vcs.Repo) error { +// plugin.yaml is the only file that Helm modifies during installation. +func resetPluginYaml(repo vcs.Repo) error { + pluginYaml := "plugin.yaml" + // Check the VCS type to determine the appropriate reset command switch repo.Vcs() { case vcs.Git: - // For Git, use 'git reset --hard' to discard all local changes - cmd := exec.Command("git", "reset", "--hard") + // For Git, use 'git checkout -- plugin.yaml' to discard changes to this file + cmd := exec.Command("git", "checkout", "--", pluginYaml) cmd.Dir = repo.LocalPath() if output, err := cmd.CombinedOutput(); err != nil { - return fmt.Errorf("git reset failed: %w, output: %s", err, output) + return fmt.Errorf("git checkout failed: %w, output: %s", err, output) } return nil case vcs.Hg: - // For Mercurial, use 'hg revert --all --no-backup' - cmd := exec.Command("hg", "revert", "--all", "--no-backup") + // For Mercurial, use 'hg revert --no-backup plugin.yaml' + cmd := exec.Command("hg", "revert", "--no-backup", pluginYaml) cmd.Dir = repo.LocalPath() if output, err := cmd.CombinedOutput(); err != nil { return fmt.Errorf("hg revert failed: %w, output: %s", err, output) } return nil case vcs.Bzr: - // For Bazaar, use 'bzr revert' - cmd := exec.Command("bzr", "revert") + // For Bazaar, use 'bzr revert plugin.yaml' + cmd := exec.Command("bzr", "revert", pluginYaml) cmd.Dir = repo.LocalPath() if output, err := cmd.CombinedOutput(); err != nil { return fmt.Errorf("bzr revert failed: %w, output: %s", err, output) } return nil case vcs.Svn: - // For SVN, use 'svn revert -R .' - cmd := exec.Command("svn", "revert", "-R", ".") + // For SVN, use 'svn revert plugin.yaml' + cmd := exec.Command("svn", "revert", pluginYaml) cmd.Dir = repo.LocalPath() if output, err := cmd.CombinedOutput(); err != nil { return fmt.Errorf("svn revert failed: %w, output: %s", err, output) @@ -142,14 +145,14 @@ func resetRepo(repo vcs.Repo) error { func (i *VCSInstaller) Update() error { slog.Debug("updating", "source", i.Repo.Remote()) - // Reset any local modifications in the cache directory before updating. + // Reset plugin.yaml if it was modified by Helm during installation. // The cached repository is managed by Helm and should not contain user modifications. - // Any modifications made by Helm itself (e.g., to plugin.yaml during installation) - // should be discarded before attempting to update. + // plugin.yaml is the only file that Helm modifies during installation, + // so we only need to reset this specific file. if i.Repo.IsDirty() { - slog.Debug("resetting local modifications in cache", "path", i.Repo.LocalPath()) - if err := resetRepo(i.Repo); err != nil { - return fmt.Errorf("failed to reset local modifications: %w", err) + slog.Debug("resetting plugin.yaml in cache", "path", i.Repo.LocalPath()) + if err := resetPluginYaml(i.Repo); err != nil { + return fmt.Errorf("failed to reset plugin.yaml: %w", err) } } diff --git a/internal/plugin/installer/vcs_installer_test.go b/internal/plugin/installer/vcs_installer_test.go index ea11b7e53..c0c1046d9 100644 --- a/internal/plugin/installer/vcs_installer_test.go +++ b/internal/plugin/installer/vcs_installer_test.go @@ -199,7 +199,7 @@ func TestVCSInstallerUpdate(t *testing.T) { } -func TestResetRepo(t *testing.T) { +func TestResetPluginYaml(t *testing.T) { // Use a real git repository by cloning a test repo ensure.HelmHome(t) @@ -223,7 +223,7 @@ func TestResetRepo(t *testing.T) { t.Fatalf("Failed to sync repo: %v", err) } - // Modify an existing file to make the repo dirty + // Modify plugin.yaml to make the repo dirty pluginYaml := filepath.Join(vcsInstaller.Repo.LocalPath(), "plugin.yaml") originalContent, err := os.ReadFile(pluginYaml) if err != nil { @@ -240,14 +240,14 @@ func TestResetRepo(t *testing.T) { t.Fatal("Expected repo to be dirty after modifying plugin.yaml") } - // Reset the repo - if err := resetRepo(vcsInstaller.Repo); err != nil { - t.Fatalf("resetRepo failed: %v", err) + // Reset only plugin.yaml + if err := resetPluginYaml(vcsInstaller.Repo); err != nil { + t.Fatalf("resetPluginYaml failed: %v", err) } // Verify the repo is clean if vcsInstaller.Repo.IsDirty() { - t.Fatal("Expected repo to be clean after reset") + t.Fatal("Expected repo to be clean after resetting plugin.yaml") } // Verify the plugin.yaml was restored to original content