From d514effb97c18225605523ef23f446f09ee2676a Mon Sep 17 00:00:00 2001 From: Evans Mungai Date: Thu, 13 Nov 2025 18:28:38 +0000 Subject: [PATCH 1/2] fix: Do not copy .git directory when installing plugins Signed-off-by: Evans Mungai --- internal/plugin/installer/http_installer.go | 2 +- .../plugin/installer/http_installer_test.go | 90 +++++++++++ internal/plugin/installer/installer.go | 4 + internal/plugin/installer/local_installer.go | 2 +- .../plugin/installer/local_installer_test.go | 69 +++++---- internal/plugin/installer/oci_installer.go | 2 +- .../plugin/installer/oci_installer_test.go | 143 ++++++++++++++++++ internal/plugin/installer/vcs_installer.go | 2 +- .../plugin/installer/vcs_installer_test.go | 54 +++++++ internal/third_party/dep/fs/fs.go | 32 +++- internal/third_party/dep/fs/fs_test.go | 136 ++++++++++++++++- 11 files changed, 493 insertions(+), 43 deletions(-) diff --git a/internal/plugin/installer/http_installer.go b/internal/plugin/installer/http_installer.go index bb96314f4..eb71839a1 100644 --- a/internal/plugin/installer/http_installer.go +++ b/internal/plugin/installer/http_installer.go @@ -137,7 +137,7 @@ func (i *HTTPInstaller) Install() error { } slog.Debug("copying", "source", src, "path", i.Path()) - return fs.CopyDir(src, i.Path()) + return fs.CopyDir(src, i.Path(), copyDirOptions) } // Update updates a local repository diff --git a/internal/plugin/installer/http_installer_test.go b/internal/plugin/installer/http_installer_test.go index be40b1b90..891a5fceb 100644 --- a/internal/plugin/installer/http_installer_test.go +++ b/internal/plugin/installer/http_installer_test.go @@ -31,6 +31,8 @@ import ( "syscall" "testing" + "github.com/stretchr/testify/assert" + "helm.sh/helm/v4/internal/test/ensure" "helm.sh/helm/v4/pkg/getter" "helm.sh/helm/v4/pkg/helmpath" @@ -44,6 +46,43 @@ type TestHTTPGetter struct { MockError error } +// createTestTarball creates a gzipped tarball from a list of files +func createTestTarball(t *testing.T, files []struct { + Name string + Body string + Mode int64 +}) []byte { + t.Helper() + + var buf bytes.Buffer + gw := gzip.NewWriter(&buf) + tw := tar.NewWriter(gw) + + for _, file := range files { + hdr := &tar.Header{ + Name: file.Name, + Mode: file.Mode, + Size: int64(len(file.Body)), + Typeflag: tar.TypeReg, + } + if err := tw.WriteHeader(hdr); err != nil { + t.Fatal(err) + } + if _, err := tw.Write([]byte(file.Body)); err != nil { + t.Fatal(err) + } + } + + if err := tw.Close(); err != nil { + t.Fatal(err) + } + if err := gw.Close(); err != nil { + t.Fatal(err) + } + + return buf.Bytes() +} + func (t *TestHTTPGetter) Get(_ string, _ ...getter.Option) (*bytes.Buffer, error) { return t.MockResponse, t.MockError } @@ -127,6 +166,57 @@ func TestHTTPInstaller(t *testing.T) { } +func TestHTTPInstaller_IgnoresGitDir(t *testing.T) { + ensure.HelmHome(t) + + // Create a tarball with .git directory + files := []struct { + Name string + Body string + Mode int64 + }{ + {"test-plugin/plugin.yaml", "name: test-plugin\nversion: 1.0.0", 0644}, + {"test-plugin/.git/config", "git config", 0644}, + } + tarballData := createTestTarball(t, files) + + srv := mockArchiveServer() + defer srv.Close() + source := srv.URL + "/plugins/test-plugin-1.0.0.tar.gz" + + if err := os.MkdirAll(helmpath.DataPath("plugins"), 0755); err != nil { + t.Fatalf("Could not create %s: %s", helmpath.DataPath("plugins"), err) + } + + i, err := NewForSource(source, "") + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + httpInstaller, ok := i.(*HTTPInstaller) + if !ok { + t.Fatal("expected a HTTPInstaller") + } + + httpInstaller.getter = &TestHTTPGetter{ + MockResponse: bytes.NewBuffer(tarballData), + } + + if err := Install(i); err != nil { + t.Fatal(err) + } + + // Verify .git was not copied + installedGitDir := filepath.Join(i.Path(), ".git") + _, err = os.Stat(installedGitDir) + assert.True(t, os.IsNotExist(err), "expected .git directory to not exist, got %v", err) + + // Verify plugin.yaml was copied + if _, err := os.Stat(filepath.Join(i.Path(), "plugin.yaml")); err != nil { + t.Fatal("plugin.yaml should have been copied") + } +} + func TestHTTPInstallerNonExistentVersion(t *testing.T) { ensure.HelmHome(t) srv := mockArchiveServer() diff --git a/internal/plugin/installer/installer.go b/internal/plugin/installer/installer.go index c7c1a8801..dbb5d81b3 100644 --- a/internal/plugin/installer/installer.go +++ b/internal/plugin/installer/installer.go @@ -25,12 +25,16 @@ import ( "strings" "helm.sh/helm/v4/internal/plugin" + "helm.sh/helm/v4/internal/third_party/dep/fs" "helm.sh/helm/v4/pkg/registry" ) // ErrMissingMetadata indicates that plugin.yaml is missing. var ErrMissingMetadata = errors.New("plugin metadata (plugin.yaml) missing") +// copyDirOptions is the default options for copying a plugin directory. +var copyDirOptions = fs.CopyDirOptions{DirsInSrcToIgnore: []string{".git"}} + // Debug enables verbose output. var Debug bool diff --git a/internal/plugin/installer/local_installer.go b/internal/plugin/installer/local_installer.go index 1c8314282..fdc6450bd 100644 --- a/internal/plugin/installer/local_installer.go +++ b/internal/plugin/installer/local_installer.go @@ -155,7 +155,7 @@ func (i *LocalInstaller) installFromArchive() error { // Copy to the final destination slog.Debug("copying", "source", pluginDir, "path", i.Path()) - return fs.CopyDir(pluginDir, i.Path()) + return fs.CopyDir(pluginDir, i.Path(), copyDirOptions) } // Update updates a local repository diff --git a/internal/plugin/installer/local_installer_test.go b/internal/plugin/installer/local_installer_test.go index 2decb695f..894d38b0b 100644 --- a/internal/plugin/installer/local_installer_test.go +++ b/internal/plugin/installer/local_installer_test.go @@ -16,13 +16,12 @@ limitations under the License. package installer // import "helm.sh/helm/v4/internal/plugin/installer" import ( - "archive/tar" - "bytes" - "compress/gzip" "os" "path/filepath" "testing" + "github.com/stretchr/testify/assert" + "helm.sh/helm/v4/internal/test/ensure" "helm.sh/helm/v4/pkg/helmpath" ) @@ -76,11 +75,6 @@ func TestLocalInstallerTarball(t *testing.T) { tempDir := t.TempDir() tarballPath := filepath.Join(tempDir, "test-plugin-1.0.0.tar.gz") - // Create tarball content - var buf bytes.Buffer - gw := gzip.NewWriter(&buf) - tw := tar.NewWriter(gw) - files := []struct { Name string Body string @@ -90,29 +84,8 @@ func TestLocalInstallerTarball(t *testing.T) { {"test-plugin/bin/test-plugin", "#!/bin/bash\necho test", 0755}, } - for _, file := range files { - hdr := &tar.Header{ - Name: file.Name, - Mode: file.Mode, - Size: int64(len(file.Body)), - } - if err := tw.WriteHeader(hdr); err != nil { - t.Fatal(err) - } - if _, err := tw.Write([]byte(file.Body)); err != nil { - t.Fatal(err) - } - } - - if err := tw.Close(); err != nil { - t.Fatal(err) - } - if err := gw.Close(); err != nil { - t.Fatal(err) - } - // Write tarball to file - if err := os.WriteFile(tarballPath, buf.Bytes(), 0644); err != nil { + if err := os.WriteFile(tarballPath, createTestTarball(t, files), 0644); err != nil { t.Fatal(err) } @@ -146,3 +119,39 @@ func TestLocalInstallerTarball(t *testing.T) { t.Fatalf("plugin not found at %s: %v", i.Path(), err) } } + +func TestLocalInstaller_IgnoresGitDir(t *testing.T) { + ensure.HelmHome(t) + + // Create a plugin directory with .git + pluginDir := t.TempDir() + if err := os.WriteFile(filepath.Join(pluginDir, "plugin.yaml"), []byte("name: test-plugin\nversion: 1.0.0"), 0644); err != nil { + t.Fatal(err) + } + gitDir := filepath.Join(pluginDir, ".git") + if err := os.MkdirAll(gitDir, 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(gitDir, "config"), []byte("git config"), 0644); err != nil { + t.Fatal(err) + } + + i, err := NewForSource(pluginDir, "") + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + if err := Install(i); err != nil { + t.Fatal(err) + } + + // Verify .git was not copied + installedGitDir := filepath.Join(i.Path(), ".git") + _, err = os.Stat(installedGitDir) + assert.True(t, os.IsNotExist(err), "expected .git directory to not exist, got %v", err) + + // Verify plugin.yaml was copied + if _, err := os.Stat(filepath.Join(i.Path(), "plugin.yaml")); err != nil { + t.Fatal("plugin.yaml should have been copied") + } +} diff --git a/internal/plugin/installer/oci_installer.go b/internal/plugin/installer/oci_installer.go index afbb42ca5..4be217317 100644 --- a/internal/plugin/installer/oci_installer.go +++ b/internal/plugin/installer/oci_installer.go @@ -177,7 +177,7 @@ func (i *OCIInstaller) Install() error { } slog.Debug("copying", "source", src, "path", i.Path()) - return fs.CopyDir(src, i.Path()) + return fs.CopyDir(src, i.Path(), copyDirOptions) } // Update updates a plugin by reinstalling it diff --git a/internal/plugin/installer/oci_installer_test.go b/internal/plugin/installer/oci_installer_test.go index 1280cf97d..acf52e232 100644 --- a/internal/plugin/installer/oci_installer_test.go +++ b/internal/plugin/installer/oci_installer_test.go @@ -33,6 +33,7 @@ import ( "github.com/opencontainers/go-digest" ocispec "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/stretchr/testify/assert" "helm.sh/helm/v4/internal/test/ensure" "helm.sh/helm/v4/pkg/cli" @@ -100,6 +101,64 @@ command: "$HELM_PLUGIN_DIR/bin/%s" return buf.Bytes() } +// createTestPluginTarGzWithGit creates a test plugin tar.gz with plugin.yaml and .git directory +func createTestPluginTarGzWithGit(t *testing.T, pluginName string) []byte { + t.Helper() + + var buf bytes.Buffer + gzWriter := gzip.NewWriter(&buf) + tarWriter := tar.NewWriter(gzWriter) + + // Add plugin.yaml + pluginYAML := fmt.Sprintf(`name: %s +version: "1.0.0" +description: "Test plugin for OCI installer" +command: "$HELM_PLUGIN_DIR/bin/%s" +`, pluginName, pluginName) + header := &tar.Header{ + Name: "plugin.yaml", + Mode: 0644, + Size: int64(len(pluginYAML)), + Typeflag: tar.TypeReg, + } + if err := tarWriter.WriteHeader(header); err != nil { + t.Fatal(err) + } + if _, err := tarWriter.Write([]byte(pluginYAML)); err != nil { + t.Fatal(err) + } + + // Add .git directory + gitDirHeader := &tar.Header{ + Name: ".git/", + Mode: 0755, + Typeflag: tar.TypeDir, + } + if err := tarWriter.WriteHeader(gitDirHeader); err != nil { + t.Fatal(err) + } + + // Add .git/config + gitConfig := "git config content" + gitConfigHeader := &tar.Header{ + Name: ".git/config", + Mode: 0644, + Size: int64(len(gitConfig)), + Typeflag: tar.TypeReg, + } + if err := tarWriter.WriteHeader(gitConfigHeader); err != nil { + t.Fatal(err) + } + if _, err := tarWriter.Write([]byte(gitConfig)); err != nil { + t.Fatal(err) + } + + tarWriter.Close() + gzWriter.Close() + + return buf.Bytes() +} + // mockOCIRegistryWithArtifactType creates a mock OCI registry server using the new artifact type approach func mockOCIRegistryWithArtifactType(t *testing.T, pluginName string) (*httptest.Server, string) { t.Helper() @@ -804,3 +863,87 @@ func TestOCIInstaller_Install_ValidationErrors(t *testing.T) { }) } } + +func TestOCIInstaller_IgnoresGitDir(t *testing.T) { + ensure.HelmHome(t) + + pluginName := "test-plugin" + pluginData := createTestPluginTarGzWithGit(t, pluginName) + + // Create a mock OCI registry server + layerDigest := fmt.Sprintf("sha256:%x", sha256Sum(pluginData)) + configData := []byte("{}") + configDigest := fmt.Sprintf("sha256:%x", sha256Sum(configData)) + + manifest := ocispec.Manifest{ + MediaType: ocispec.MediaTypeImageManifest, + ArtifactType: "application/vnd.helm.plugin.v1+json", + Config: ocispec.Descriptor{ + MediaType: "application/vnd.oci.empty.v1+json", + Digest: digest.Digest(configDigest), + Size: int64(len(configData)), + }, + Layers: []ocispec.Descriptor{ + { + MediaType: "application/vnd.oci.image.layer.v1.tar", + Digest: digest.Digest(layerDigest), + Size: int64(len(pluginData)), + }, + }, + } + + manifestData, err := json.Marshal(manifest) + if err != nil { + t.Fatal(err) + } + manifestDigest := fmt.Sprintf("sha256:%x", sha256Sum(manifestData)) + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet && strings.Contains(r.URL.Path, "/v2/") && !strings.Contains(r.URL.Path, "/manifests/") && !strings.Contains(r.URL.Path, "/blobs/"): + w.Header().Set("Docker-Distribution-API-Version", "registry/2.0") + w.WriteHeader(http.StatusOK) + case r.Method == http.MethodGet && strings.Contains(r.URL.Path, "/manifests/"): + w.Header().Set("Content-Type", ocispec.MediaTypeImageManifest) + w.Header().Set("Docker-Content-Digest", manifestDigest) + w.WriteHeader(http.StatusOK) + w.Write(manifestData) + case r.Method == http.MethodGet && strings.Contains(r.URL.Path, "/blobs/"+layerDigest): + w.Header().Set("Content-Type", "application/vnd.oci.image.layer.v1.tar") + w.WriteHeader(http.StatusOK) + w.Write(pluginData) + case r.Method == http.MethodGet && strings.Contains(r.URL.Path, "/blobs/"+configDigest): + w.Header().Set("Content-Type", "application/vnd.oci.empty.v1+json") + w.WriteHeader(http.StatusOK) + w.Write(configData) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer srv.Close() + + registryURL, err := url.Parse(srv.URL) + if err != nil { + t.Fatalf("Failed to parse server URL: %v", err) + } + + source := fmt.Sprintf("oci://%s/%s:1.0.0", registryURL.Host, pluginName) + i, err := NewForSource(source, "") + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + if err := Install(i); err != nil { + t.Fatal(err) + } + + // Verify .git was not copied + installedGitDir := filepath.Join(i.Path(), ".git") + _, err = os.Stat(installedGitDir) + assert.True(t, os.IsNotExist(err), "expected .git directory to not exist, got %v", err) + + // Verify plugin.yaml was copied + if _, err := os.Stat(filepath.Join(i.Path(), "plugin.yaml")); err != nil { + t.Fatal("plugin.yaml should have been copied") + } +} diff --git a/internal/plugin/installer/vcs_installer.go b/internal/plugin/installer/vcs_installer.go index 3601ec7a8..e153d7c63 100644 --- a/internal/plugin/installer/vcs_installer.go +++ b/internal/plugin/installer/vcs_installer.go @@ -92,7 +92,7 @@ func (i *VCSInstaller) Install() error { } slog.Debug("copying files", "source", i.Repo.LocalPath(), "destination", i.Path()) - return fs.CopyDir(i.Repo.LocalPath(), i.Path()) + return fs.CopyDir(i.Repo.LocalPath(), i.Path(), copyDirOptions) } // Update updates a remote repository diff --git a/internal/plugin/installer/vcs_installer_test.go b/internal/plugin/installer/vcs_installer_test.go index d542a0f75..b3aced859 100644 --- a/internal/plugin/installer/vcs_installer_test.go +++ b/internal/plugin/installer/vcs_installer_test.go @@ -23,6 +23,7 @@ import ( "testing" "github.com/Masterminds/vcs" + "github.com/stretchr/testify/assert" "helm.sh/helm/v4/internal/test/ensure" "helm.sh/helm/v4/pkg/helmpath" @@ -187,3 +188,56 @@ func TestVCSInstallerUpdate(t *testing.T) { } } + +func TestVCSInstaller_IgnoresGitDir(t *testing.T) { + ensure.HelmHome(t) + + if err := os.MkdirAll(helmpath.DataPath("plugins"), 0755); err != nil { + t.Fatalf("Could not create %s: %s", helmpath.DataPath("plugins"), err) + } + + // Create a test plugin directory with .git + testRepoPath := t.TempDir() + if err := os.WriteFile(filepath.Join(testRepoPath, "plugin.yaml"), []byte("name: test-plugin\nversion: 1.0.0"), 0644); err != nil { + t.Fatal(err) + } + gitDir := filepath.Join(testRepoPath, ".git") + if err := os.MkdirAll(gitDir, 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(gitDir, "config"), []byte("git config"), 0644); err != nil { + t.Fatal(err) + } + + source := "https://github.com/test/test-plugin" + repo := &testRepo{ + local: testRepoPath, + tags: []string{"1.0.0"}, + } + + i, err := NewForSource(source, "1.0.0") + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + vcsInstaller, ok := i.(*VCSInstaller) + if !ok { + t.Fatal("expected a VCSInstaller") + } + + vcsInstaller.Repo = repo + + if err := Install(i); err != nil { + t.Fatal(err) + } + + // Verify .git was not copied + installedGitDir := filepath.Join(i.Path(), ".git") + _, err = os.Stat(installedGitDir) + assert.True(t, os.IsNotExist(err), "expected .git directory to not exist, got %v", err) + + // Verify plugin.yaml was copied + if _, err := os.Stat(filepath.Join(i.Path(), "plugin.yaml")); err != nil { + t.Fatal("plugin.yaml should have been copied") + } +} diff --git a/internal/third_party/dep/fs/fs.go b/internal/third_party/dep/fs/fs.go index 6e2720f3b..563b75c16 100644 --- a/internal/third_party/dep/fs/fs.go +++ b/internal/third_party/dep/fs/fs.go @@ -36,9 +36,11 @@ import ( "fmt" "io" "io/fs" + "log/slog" "os" "path/filepath" "runtime" + "slices" "syscall" ) @@ -68,7 +70,7 @@ func RenameWithFallback(src, dst string) error { func renameByCopy(src, dst string) error { var cerr error if dir, _ := IsDir(src); dir { - cerr = CopyDir(src, dst) + cerr = CopyDir(src, dst, CopyDirOptions{}) if cerr != nil { cerr = fmt.Errorf("copying directory failed: %w", cerr) } @@ -95,12 +97,35 @@ var ( errDstExist = errors.New("destination already exists") ) +// CopyDirOptions contains options for controlling the behavior of CopyDir. +type CopyDirOptions struct { + // DirsInSrcToIgnore is a list of directory names (not paths) to skip during copying. + // Any directory whose base name matches an entry in this list will be ignored, + // along with all its contents. The check is performed recursively at each level + // of the directory tree. + DirsInSrcToIgnore []string +} + // CopyDir recursively copies a directory tree, attempting to preserve permissions. // Source directory must exist, destination directory must *not* exist. -func CopyDir(src, dst string) error { +// +// See CopyDirOptions for details on available options. +// +// Example: +// +// CopyDir("/src", "/dst", CopyDirOptions{DirsInSrcToIgnore: []string{".git", ".vscode"}}) +// This will copy all files and directories from /src to /dst, but skip any +// directories named ".git" or ".vscode" and their contents. +func CopyDir(src, dst string, options CopyDirOptions) error { src = filepath.Clean(src) dst = filepath.Clean(dst) + // Check if this directory name should be ignored + if len(options.DirsInSrcToIgnore) > 0 && slices.Contains(options.DirsInSrcToIgnore, filepath.Base(src)) { + slog.Debug("skipping directory", "directory", src, "dirsToIgnore", options.DirsInSrcToIgnore) + return nil + } + // We use os.Lstat() here to ensure we don't fall in a loop where a symlink // actually links to a one of its parent directories. fi, err := os.Lstat(src) @@ -129,11 +154,12 @@ func CopyDir(src, dst string) error { } for _, entry := range entries { + // Skip entries whose name matches any in dirsToIgnore srcPath := filepath.Join(src, entry.Name()) dstPath := filepath.Join(dst, entry.Name()) if entry.IsDir() { - if err = CopyDir(srcPath, dstPath); err != nil { + if err = CopyDir(srcPath, dstPath, options); err != nil { return fmt.Errorf("copying directory failed: %w", err) } } else { diff --git a/internal/third_party/dep/fs/fs_test.go b/internal/third_party/dep/fs/fs_test.go index 610771bc3..1b34eb4da 100644 --- a/internal/third_party/dep/fs/fs_test.go +++ b/internal/third_party/dep/fs/fs_test.go @@ -114,7 +114,7 @@ func TestCopyDir(t *testing.T) { } destdir := filepath.Join(dir, "dest") - if err := CopyDir(srcdir, destdir); err != nil { + if err := CopyDir(srcdir, destdir, CopyDirOptions{}); err != nil { t.Fatal(err) } @@ -151,6 +151,130 @@ func TestCopyDir(t *testing.T) { } } +func TestCopyDir_IgnoreDirectories(t *testing.T) { + dir := t.TempDir() + + srcdir := filepath.Join(dir, "src") + if err := os.MkdirAll(srcdir, 0755); err != nil { + t.Fatal(err) + } + + // Create a .git directory that should be ignored + gitdir := filepath.Join(srcdir, ".git") + if err := os.MkdirAll(gitdir, 0755); err != nil { + t.Fatal(err) + } + gitFile := filepath.Join(gitdir, "config") + if err := os.WriteFile(gitFile, []byte("git config"), 0644); err != nil { + t.Fatal(err) + } + + // Create a nested .git directory + nestedDir := filepath.Join(srcdir, "subdir") + if err := os.MkdirAll(nestedDir, 0755); err != nil { + t.Fatal(err) + } + // Add a regular file in subdir to ensure the directory itself gets copied + nestedRegularFile := filepath.Join(nestedDir, "file.txt") + if err := os.WriteFile(nestedRegularFile, []byte("nested regular file"), 0644); err != nil { + t.Fatal(err) + } + nestedGitDir := filepath.Join(nestedDir, ".git") + if err := os.MkdirAll(nestedGitDir, 0755); err != nil { + t.Fatal(err) + } + nestedGitFile := filepath.Join(nestedGitDir, "config") + if err := os.WriteFile(nestedGitFile, []byte("nested git config"), 0644); err != nil { + t.Fatal(err) + } + + // Create a regular directory that should NOT be ignored + regulardir := filepath.Join(srcdir, "regular") + if err := os.MkdirAll(regulardir, 0755); err != nil { + t.Fatal(err) + } + regularFile := filepath.Join(regulardir, "file.txt") + if err := os.WriteFile(regularFile, []byte("regular file"), 0644); err != nil { + t.Fatal(err) + } + + // Create a regular file at the root + rootFile := filepath.Join(srcdir, "root.txt") + if err := os.WriteFile(rootFile, []byte("root file"), 0644); err != nil { + t.Fatal(err) + } + + // Create another directory to ignore (e.g., .vscode) + vscodeDir := filepath.Join(srcdir, ".vscode") + if err := os.MkdirAll(vscodeDir, 0755); err != nil { + t.Fatal(err) + } + vscodeFile := filepath.Join(vscodeDir, "settings.json") + if err := os.WriteFile(vscodeFile, []byte("vscode settings"), 0644); err != nil { + t.Fatal(err) + } + + destdir := filepath.Join(dir, "dest") + dirsToIgnore := []string{".git", ".vscode"} + if err := CopyDir(srcdir, destdir, CopyDirOptions{DirsInSrcToIgnore: dirsToIgnore}); err != nil { + t.Fatal(err) + } + + // Verify .git directory was NOT copied + destGitDir := filepath.Join(destdir, ".git") + if _, err := os.Stat(destGitDir); err == nil { + t.Fatalf("expected .git directory to be ignored, but it was copied to %s", destGitDir) + } + + // Verify nested .git directory was NOT copied + destNestedGitDir := filepath.Join(destdir, "subdir", ".git") + if _, err := os.Stat(destNestedGitDir); err == nil { + t.Fatalf("expected nested .git directory to be ignored, but it was copied to %s", destNestedGitDir) + } + + // Verify subdir itself WAS copied (it contains a regular file) + destSubdirFile := filepath.Join(destdir, "subdir", "file.txt") + gotSubdir, err := os.ReadFile(destSubdirFile) + if err != nil { + t.Fatalf("expected nested regular file to be copied, but it was not found at %s: %v", destSubdirFile, err) + } + if string(gotSubdir) != "nested regular file" { + t.Fatalf("expected file contents 'nested regular file', got %s", string(gotSubdir)) + } + + // Verify .vscode directory was NOT copied + destVscodeDir := filepath.Join(destdir, ".vscode") + if _, err := os.Stat(destVscodeDir); err == nil { + t.Fatalf("expected .vscode directory to be ignored, but it was copied to %s", destVscodeDir) + } + + // Verify regular directory WAS copied + destRegularDir := filepath.Join(destdir, "regular") + if _, err := os.Stat(destRegularDir); err != nil { + t.Fatalf("expected regular directory to be copied, but it was not found at %s: %v", destRegularDir, err) + } + + // Verify regular file WAS copied + destRegularFile := filepath.Join(destdir, "regular", "file.txt") + got, err := os.ReadFile(destRegularFile) + if err != nil { + t.Fatalf("expected regular file to be copied, but it was not found at %s: %v", destRegularFile, err) + } + if string(got) != "regular file" { + t.Fatalf("expected file contents 'regular file', got %s", string(got)) + } + + // Verify root file WAS copied + destRootFile := filepath.Join(destdir, "root.txt") + got, err = os.ReadFile(destRootFile) + if err != nil { + t.Fatalf("expected root file to be copied, but it was not found at %s: %v", destRootFile, err) + } + if string(got) != "root file" { + t.Fatalf("expected file contents 'root file', got %s", string(got)) + } +} + func TestCopyDirFail_SrcInaccessible(t *testing.T) { if runtime.GOOS == "windows" { // XXX: setting permissions works differently in @@ -177,7 +301,7 @@ func TestCopyDirFail_SrcInaccessible(t *testing.T) { dir := t.TempDir() dstdir = filepath.Join(dir, "dst") - if err := CopyDir(srcdir, dstdir); err == nil { + if err := CopyDir(srcdir, dstdir, CopyDirOptions{}); err == nil { t.Fatalf("expected error for CopyDir(%s, %s), got none", srcdir, dstdir) } } @@ -212,7 +336,7 @@ func TestCopyDirFail_DstInaccessible(t *testing.T) { }) defer cleanup() - if err := CopyDir(srcdir, dstdir); err == nil { + if err := CopyDir(srcdir, dstdir, CopyDirOptions{}); err == nil { t.Fatalf("expected error for CopyDir(%s, %s), got none", srcdir, dstdir) } } @@ -230,7 +354,7 @@ func TestCopyDirFail_SrcIsNotDir(t *testing.T) { dstdir = filepath.Join(dir, "dst") - if err = CopyDir(srcdir, dstdir); err == nil { + if err = CopyDir(srcdir, dstdir, CopyDirOptions{}); err == nil { t.Fatalf("expected error for CopyDir(%s, %s), got none", srcdir, dstdir) } @@ -256,7 +380,7 @@ func TestCopyDirFail_DstExists(t *testing.T) { t.Fatal(err) } - if err = CopyDir(srcdir, dstdir); err == nil { + if err = CopyDir(srcdir, dstdir, CopyDirOptions{}); err == nil { t.Fatalf("expected error for CopyDir(%s, %s), got none", srcdir, dstdir) } @@ -306,7 +430,7 @@ func TestCopyDirFailOpen(t *testing.T) { dstdir = filepath.Join(dir, "dst") - if err = CopyDir(srcdir, dstdir); err == nil { + if err = CopyDir(srcdir, dstdir, CopyDirOptions{}); err == nil { t.Fatalf("expected error for CopyDir(%s, %s), got none", srcdir, dstdir) } } From f3a04ae56bd5423b4205ecc2dd924e96c0cae31b Mon Sep 17 00:00:00 2001 From: Evans Mungai Date: Thu, 13 Nov 2025 18:52:48 +0000 Subject: [PATCH 2/2] Fix failing tests Signed-off-by: Evans Mungai --- internal/plugin/installer/local_installer.go | 6 +-- internal/plugin/installer/oci_installer.go | 6 +++ .../plugin/installer/oci_installer_test.go | 5 +- internal/plugin/installer/vcs_installer.go | 3 +- .../plugin/installer/vcs_installer_test.go | 54 ------------------- 5 files changed, 15 insertions(+), 59 deletions(-) diff --git a/internal/plugin/installer/local_installer.go b/internal/plugin/installer/local_installer.go index fdc6450bd..b07461fef 100644 --- a/internal/plugin/installer/local_installer.go +++ b/internal/plugin/installer/local_installer.go @@ -84,7 +84,7 @@ func (i *LocalInstaller) Install() error { return i.installFromDirectory() } -// installFromDirectory creates a symlink to the plugin directory +// installFromDirectory copies the plugin directory func (i *LocalInstaller) installFromDirectory() error { stat, err := os.Stat(i.Source) if err != nil { @@ -97,8 +97,8 @@ func (i *LocalInstaller) installFromDirectory() error { if !isPlugin(i.Source) { return ErrMissingMetadata } - slog.Debug("symlinking", "source", i.Source, "path", i.Path()) - return os.Symlink(i.Source, i.Path()) + slog.Debug("copying", "source", i.Source, "path", i.Path()) + return fs.CopyDir(i.Source, i.Path(), copyDirOptions) } // installFromArchive extracts and installs a plugin from a tarball diff --git a/internal/plugin/installer/oci_installer.go b/internal/plugin/installer/oci_installer.go index 4be217317..4dac57021 100644 --- a/internal/plugin/installer/oci_installer.go +++ b/internal/plugin/installer/oci_installer.go @@ -24,6 +24,7 @@ import ( "log/slog" "os" "path/filepath" + "strings" "helm.sh/helm/v4/internal/plugin" "helm.sh/helm/v4/internal/plugin/cache" @@ -221,6 +222,11 @@ func extractTar(r io.Reader, targetDir string) error { return err } + // Skip .git directories and their contents + if strings.HasPrefix(header.Name, ".git/") || header.Name == ".git" { + continue + } + path, err := cleanJoin(targetDir, header.Name) if err != nil { return err diff --git a/internal/plugin/installer/oci_installer_test.go b/internal/plugin/installer/oci_installer_test.go index acf52e232..395e74c6a 100644 --- a/internal/plugin/installer/oci_installer_test.go +++ b/internal/plugin/installer/oci_installer_test.go @@ -888,6 +888,9 @@ func TestOCIInstaller_IgnoresGitDir(t *testing.T) { MediaType: "application/vnd.oci.image.layer.v1.tar", Digest: digest.Digest(layerDigest), Size: int64(len(pluginData)), + Annotations: map[string]string{ + ocispec.AnnotationTitle: pluginName + "-1.0.0.tgz", + }, }, }, } @@ -928,7 +931,7 @@ func TestOCIInstaller_IgnoresGitDir(t *testing.T) { } source := fmt.Sprintf("oci://%s/%s:1.0.0", registryURL.Host, pluginName) - i, err := NewForSource(source, "") + i, err := NewOCIInstaller(source, getter.WithPlainHTTP(true)) if err != nil { t.Fatalf("unexpected error: %s", err) } diff --git a/internal/plugin/installer/vcs_installer.go b/internal/plugin/installer/vcs_installer.go index e153d7c63..7b3699268 100644 --- a/internal/plugin/installer/vcs_installer.go +++ b/internal/plugin/installer/vcs_installer.go @@ -92,7 +92,8 @@ func (i *VCSInstaller) Install() error { } slog.Debug("copying files", "source", i.Repo.LocalPath(), "destination", i.Path()) - return fs.CopyDir(i.Repo.LocalPath(), i.Path(), copyDirOptions) + // VCS plugins need .git for updates, so don't filter it out + return fs.CopyDir(i.Repo.LocalPath(), i.Path(), fs.CopyDirOptions{}) } // Update updates a remote repository diff --git a/internal/plugin/installer/vcs_installer_test.go b/internal/plugin/installer/vcs_installer_test.go index b3aced859..d542a0f75 100644 --- a/internal/plugin/installer/vcs_installer_test.go +++ b/internal/plugin/installer/vcs_installer_test.go @@ -23,7 +23,6 @@ import ( "testing" "github.com/Masterminds/vcs" - "github.com/stretchr/testify/assert" "helm.sh/helm/v4/internal/test/ensure" "helm.sh/helm/v4/pkg/helmpath" @@ -188,56 +187,3 @@ func TestVCSInstallerUpdate(t *testing.T) { } } - -func TestVCSInstaller_IgnoresGitDir(t *testing.T) { - ensure.HelmHome(t) - - if err := os.MkdirAll(helmpath.DataPath("plugins"), 0755); err != nil { - t.Fatalf("Could not create %s: %s", helmpath.DataPath("plugins"), err) - } - - // Create a test plugin directory with .git - testRepoPath := t.TempDir() - if err := os.WriteFile(filepath.Join(testRepoPath, "plugin.yaml"), []byte("name: test-plugin\nversion: 1.0.0"), 0644); err != nil { - t.Fatal(err) - } - gitDir := filepath.Join(testRepoPath, ".git") - if err := os.MkdirAll(gitDir, 0755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(gitDir, "config"), []byte("git config"), 0644); err != nil { - t.Fatal(err) - } - - source := "https://github.com/test/test-plugin" - repo := &testRepo{ - local: testRepoPath, - tags: []string{"1.0.0"}, - } - - i, err := NewForSource(source, "1.0.0") - if err != nil { - t.Fatalf("unexpected error: %s", err) - } - - vcsInstaller, ok := i.(*VCSInstaller) - if !ok { - t.Fatal("expected a VCSInstaller") - } - - vcsInstaller.Repo = repo - - if err := Install(i); err != nil { - t.Fatal(err) - } - - // Verify .git was not copied - installedGitDir := filepath.Join(i.Path(), ".git") - _, err = os.Stat(installedGitDir) - assert.True(t, os.IsNotExist(err), "expected .git directory to not exist, got %v", err) - - // Verify plugin.yaml was copied - if _, err := os.Stat(filepath.Join(i.Path(), "plugin.yaml")); err != nil { - t.Fatal("plugin.yaml should have been copied") - } -}