From 5e35c3b8e393476e277644af043ac411832b5c08 Mon Sep 17 00:00:00 2001 From: George Jenkins Date: Sat, 30 Nov 2024 23:46:18 -0800 Subject: [PATCH] Cleanup repotest Server constructors Signed-off-by: George Jenkins --- cmd/helm/dependency_build_test.go | 5 +- cmd/helm/dependency_update_test.go | 15 +-- cmd/helm/install_test.go | 5 +- cmd/helm/pull_test.go | 10 +- cmd/helm/repo_add_test.go | 30 +---- cmd/helm/repo_remove_test.go | 10 +- cmd/helm/repo_update_test.go | 20 +-- cmd/helm/show_test.go | 5 +- pkg/downloader/chart_downloader_test.go | 12 +- pkg/downloader/manager_test.go | 30 ++--- pkg/repo/repotest/server.go | 170 +++++++++++++----------- pkg/repo/repotest/server_test.go | 7 +- 12 files changed, 126 insertions(+), 193 deletions(-) diff --git a/cmd/helm/dependency_build_test.go b/cmd/helm/dependency_build_test.go index 37e3242c4..92ab252b8 100644 --- a/cmd/helm/dependency_build_test.go +++ b/cmd/helm/dependency_build_test.go @@ -29,11 +29,8 @@ import ( ) func TestDependencyBuildCmd(t *testing.T) { - srv, err := repotest.NewTempServerWithCleanup(t, "testdata/testcharts/*.tgz") + srv := repotest.NewTempServer(t, "testdata/testcharts/*.tgz") defer srv.Stop() - if err != nil { - t.Fatal(err) - } rootDir := srv.Root() srv.LinkIndices() diff --git a/cmd/helm/dependency_update_test.go b/cmd/helm/dependency_update_test.go index 1a1e0468f..1713b5681 100644 --- a/cmd/helm/dependency_update_test.go +++ b/cmd/helm/dependency_update_test.go @@ -32,10 +32,7 @@ import ( ) func TestDependencyUpdateCmd(t *testing.T) { - srv, err := repotest.NewTempServerWithCleanup(t, "testdata/testcharts/*.tgz") - if err != nil { - t.Fatal(err) - } + srv := repotest.NewTempServer(t, "testdata/testcharts/*.tgz") defer srv.Stop() t.Logf("Listening on directory %s", srv.Root()) @@ -151,10 +148,7 @@ func TestDependencyUpdateCmd_DoNotDeleteOldChartsOnError(t *testing.T) { defer resetEnv()() ensure.HelmHome(t) - srv, err := repotest.NewTempServerWithCleanup(t, "testdata/testcharts/*.tgz") - if err != nil { - t.Fatal(err) - } + srv := repotest.NewTempServer(t, "testdata/testcharts/*.tgz") defer srv.Stop() t.Logf("Listening on directory %s", srv.Root()) @@ -248,10 +242,7 @@ func TestDependencyUpdateCmd_WithRepoThatWasNotAdded(t *testing.T) { } func setupMockRepoServer(t *testing.T) *repotest.Server { - srv, err := repotest.NewTempServerWithCleanup(t, "testdata/testcharts/*.tgz") - if err != nil { - t.Fatal(err) - } + srv := repotest.NewTempServer(t, "testdata/testcharts/*.tgz") t.Logf("Listening on directory %s", srv.Root()) diff --git a/cmd/helm/install_test.go b/cmd/helm/install_test.go index 682b25164..a7c6c9fde 100644 --- a/cmd/helm/install_test.go +++ b/cmd/helm/install_test.go @@ -27,10 +27,7 @@ import ( ) func TestInstall(t *testing.T) { - srv, err := repotest.NewTempServerWithCleanup(t, "testdata/testcharts/*.tgz*") - if err != nil { - t.Fatal(err) - } + srv := repotest.NewTempServer(t, "testdata/testcharts/*.tgz*") defer srv.Stop() srv.WithMiddleware(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { diff --git a/cmd/helm/pull_test.go b/cmd/helm/pull_test.go index ae70595f9..753b64fdc 100644 --- a/cmd/helm/pull_test.go +++ b/cmd/helm/pull_test.go @@ -28,10 +28,7 @@ import ( ) func TestPullCmd(t *testing.T) { - srv, err := repotest.NewTempServerWithCleanup(t, "testdata/testcharts/*.tgz*") - if err != nil { - t.Fatal(err) - } + srv := repotest.NewTempServer(t, "testdata/testcharts/*.tgz*") defer srv.Stop() ociSrv, err := repotest.NewOCIServer(t, srv.Root()) @@ -252,10 +249,7 @@ func TestPullCmd(t *testing.T) { } func TestPullWithCredentialsCmd(t *testing.T) { - srv, err := repotest.NewTempServerWithCleanup(t, "testdata/testcharts/*.tgz*") - if err != nil { - t.Fatal(err) - } + srv := repotest.NewTempServer(t, "testdata/testcharts/*.tgz*") defer srv.Stop() srv.WithMiddleware(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { diff --git a/cmd/helm/repo_add_test.go b/cmd/helm/repo_add_test.go index 2386bb01f..7e0026470 100644 --- a/cmd/helm/repo_add_test.go +++ b/cmd/helm/repo_add_test.go @@ -34,22 +34,15 @@ import ( ) func TestRepoAddCmd(t *testing.T) { - srv, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*") - if err != nil { - t.Fatal(err) - } + srv := repotest.NewTempServer(t, "testdata/testserver/*.*") defer srv.Stop() // A second test server is setup to verify URL changing - srv2, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*") - if err != nil { - t.Fatal(err) - } + srv2 := repotest.NewTempServer(t, "testdata/testserver/*.*") defer srv2.Stop() tmpdir := filepath.Join(t.TempDir(), "path-component.yaml/data") - err = os.MkdirAll(tmpdir, 0777) - if err != nil { + if err := os.MkdirAll(tmpdir, 0777); err != nil { t.Fatal(err) } repoFile := filepath.Join(tmpdir, "repositories.yaml") @@ -81,10 +74,7 @@ func TestRepoAddCmd(t *testing.T) { } func TestRepoAdd(t *testing.T) { - ts, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*") - if err != nil { - t.Fatal(err) - } + ts := repotest.NewTempServer(t, "testdata/testserver/*.*") defer ts.Stop() rootDir := t.TempDir() @@ -135,10 +125,7 @@ func TestRepoAdd(t *testing.T) { } func TestRepoAddCheckLegalName(t *testing.T) { - ts, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*") - if err != nil { - t.Fatal(err) - } + ts := repotest.NewTempServer(t, "testdata/testserver/*.*") defer ts.Stop() defer resetEnv()() @@ -192,10 +179,7 @@ func TestRepoAddConcurrentHiddenFile(t *testing.T) { } func repoAddConcurrent(t *testing.T, testName, repoFile string) { - ts, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*") - if err != nil { - t.Fatal(err) - } + ts := repotest.NewTempServer(t, "testdata/testserver/*.*") defer ts.Stop() var wg sync.WaitGroup @@ -243,7 +227,7 @@ func TestRepoAddFileCompletion(t *testing.T) { } func TestRepoAddWithPasswordFromStdin(t *testing.T) { - srv := repotest.NewTempServerWithCleanupAndBasicAuth(t, "testdata/testserver/*.*") + srv := repotest.NewTempServer(t, "testdata/testserver/*.*", repotest.WithBasicAuth()) defer srv.Stop() defer resetEnv()() diff --git a/cmd/helm/repo_remove_test.go b/cmd/helm/repo_remove_test.go index e2795e738..0b72b218a 100644 --- a/cmd/helm/repo_remove_test.go +++ b/cmd/helm/repo_remove_test.go @@ -30,10 +30,7 @@ import ( ) func TestRepoRemove(t *testing.T) { - ts, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*") - if err != nil { - t.Fatal(err) - } + ts := repotest.NewTempServer(t, "testdata/testserver/*.*") defer ts.Stop() rootDir := t.TempDir() @@ -162,10 +159,7 @@ func testCacheFiles(t *testing.T, cacheIndexFile string, cacheChartsFile string, } func TestRepoRemoveCompletion(t *testing.T) { - ts, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*") - if err != nil { - t.Fatal(err) - } + ts := repotest.NewTempServer(t, "testdata/testserver/*.*") defer ts.Stop() rootDir := t.TempDir() diff --git a/cmd/helm/repo_update_test.go b/cmd/helm/repo_update_test.go index 1ddf0f5ed..85010f030 100644 --- a/cmd/helm/repo_update_test.go +++ b/cmd/helm/repo_update_test.go @@ -106,10 +106,7 @@ func TestUpdateCustomCacheCmd(t *testing.T) { cachePath := filepath.Join(rootDir, "updcustomcache") os.Mkdir(cachePath, os.ModePerm) - ts, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*") - if err != nil { - t.Fatal(err) - } + ts := repotest.NewTempServer(t, "testdata/testserver/*.*") defer ts.Stop() o := &repoUpdateOptions{ @@ -130,10 +127,7 @@ func TestUpdateCharts(t *testing.T) { defer resetEnv()() ensure.HelmHome(t) - ts, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*") - if err != nil { - t.Fatal(err) - } + ts := repotest.NewTempServer(t, "testdata/testserver/*.*") defer ts.Stop() r, err := repo.NewChartRepository(&repo.Entry{ @@ -165,10 +159,7 @@ func TestUpdateChartsFail(t *testing.T) { defer resetEnv()() ensure.HelmHome(t) - ts, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*") - if err != nil { - t.Fatal(err) - } + ts := repotest.NewTempServer(t, "testdata/testserver/*.*") defer ts.Stop() var invalidURL = ts.URL() + "55" @@ -198,10 +189,7 @@ func TestUpdateChartsFailWithError(t *testing.T) { defer resetEnv()() ensure.HelmHome(t) - ts, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*") - if err != nil { - t.Fatal(err) - } + ts := repotest.NewTempServer(t, "testdata/testserver/*.*") defer ts.Stop() var invalidURL = ts.URL() + "55" diff --git a/cmd/helm/show_test.go b/cmd/helm/show_test.go index 93ec08d0f..d9c3208e8 100644 --- a/cmd/helm/show_test.go +++ b/cmd/helm/show_test.go @@ -26,10 +26,7 @@ import ( ) func TestShowPreReleaseChart(t *testing.T) { - srv, err := repotest.NewTempServerWithCleanup(t, "testdata/testcharts/*.tgz*") - if err != nil { - t.Fatal(err) - } + srv := repotest.NewTempServer(t, "testdata/testcharts/*.tgz*") defer srv.Stop() if err := srv.LinkIndices(); err != nil { diff --git a/pkg/downloader/chart_downloader_test.go b/pkg/downloader/chart_downloader_test.go index 131e21306..ac7474cb5 100644 --- a/pkg/downloader/chart_downloader_test.go +++ b/pkg/downloader/chart_downloader_test.go @@ -171,7 +171,7 @@ func TestIsTar(t *testing.T) { } func TestDownloadTo(t *testing.T) { - srv := repotest.NewTempServerWithCleanupAndBasicAuth(t, "testdata/*.tgz*") + srv := repotest.NewTempServer(t, "testdata/*.tgz*", repotest.WithBasicAuth()) defer srv.Stop() if err := srv.CreateIndex(); err != nil { t.Fatal(err) @@ -218,11 +218,8 @@ func TestDownloadTo(t *testing.T) { func TestDownloadTo_TLS(t *testing.T) { // Set up mock server w/ tls enabled - srv, err := repotest.NewTempServerWithCleanup(t, "testdata/*.tgz*") + srv := repotest.NewTempServer(t, "testdata/*.tgz*") srv.Stop() - if err != nil { - t.Fatal(err) - } srv.StartTLS() defer srv.Stop() if err := srv.CreateIndex(); err != nil { @@ -274,10 +271,7 @@ func TestDownloadTo_VerifyLater(t *testing.T) { dest := t.TempDir() // Set up a fake repo - srv, err := repotest.NewTempServerWithCleanup(t, "testdata/*.tgz*") - if err != nil { - t.Fatal(err) - } + srv := repotest.NewTempServer(t, "testdata/*.tgz*") defer srv.Stop() if err := srv.LinkIndices(); err != nil { t.Fatal(err) diff --git a/pkg/downloader/manager_test.go b/pkg/downloader/manager_test.go index db2487d16..253aa10fc 100644 --- a/pkg/downloader/manager_test.go +++ b/pkg/downloader/manager_test.go @@ -292,10 +292,7 @@ version: 0.1.0` func TestUpdateBeforeBuild(t *testing.T) { // Set up a fake repo - srv, err := repotest.NewTempServerWithCleanup(t, "testdata/*.tgz*") - if err != nil { - t.Fatal(err) - } + srv := repotest.NewTempServer(t, "testdata/*.tgz*") defer srv.Stop() if err := srv.LinkIndices(); err != nil { t.Fatal(err) @@ -347,13 +344,11 @@ func TestUpdateBeforeBuild(t *testing.T) { } // Update before Build. see issue: https://github.com/helm/helm/issues/7101 - err = m.Update() - if err != nil { + if err := m.Update(); err != nil { t.Fatal(err) } - err = m.Build() - if err != nil { + if err := m.Build(); err != nil { t.Fatal(err) } } @@ -363,10 +358,7 @@ func TestUpdateBeforeBuild(t *testing.T) { // to be fetched. func TestUpdateWithNoRepo(t *testing.T) { // Set up a fake repo - srv, err := repotest.NewTempServerWithCleanup(t, "testdata/*.tgz*") - if err != nil { - t.Fatal(err) - } + srv := repotest.NewTempServer(t, "testdata/*.tgz*") defer srv.Stop() if err := srv.LinkIndices(); err != nil { t.Fatal(err) @@ -422,8 +414,7 @@ func TestUpdateWithNoRepo(t *testing.T) { } // Test the update - err = m.Update() - if err != nil { + if err := m.Update(); err != nil { t.Fatal(err) } } @@ -436,10 +427,7 @@ func TestUpdateWithNoRepo(t *testing.T) { // If each of these main fields (name, version, repository) is not supplied by dep param, default value will be used. func checkBuildWithOptionalFields(t *testing.T, chartName string, dep chart.Dependency) { // Set up a fake repo - srv, err := repotest.NewTempServerWithCleanup(t, "testdata/*.tgz*") - if err != nil { - t.Fatal(err) - } + srv := repotest.NewTempServer(t, "testdata/*.tgz*") defer srv.Stop() if err := srv.LinkIndices(); err != nil { t.Fatal(err) @@ -487,14 +475,12 @@ func checkBuildWithOptionalFields(t *testing.T, chartName string, dep chart.Depe } // First build will update dependencies and create Chart.lock file. - err = m.Build() - if err != nil { + if err := m.Build(); err != nil { t.Fatal(err) } // Second build should be passed. See PR #6655. - err = m.Build() - if err != nil { + if err := m.Build(); err != nil { t.Fatal(err) } } diff --git a/pkg/repo/repotest/server.go b/pkg/repo/repotest/server.go index 4a86707cf..a1f8c5b43 100644 --- a/pkg/repo/repotest/server.go +++ b/pkg/repo/repotest/server.go @@ -41,36 +41,113 @@ import ( "helm.sh/helm/v3/pkg/repo" ) -// NewTempServerWithCleanup creates a server inside of a temp dir. +type ServerOption func(*testing.T, *Server) + +func WithNoAutostart() ServerOption { + return func(_ *testing.T, server *Server) { + server.autostart = false + } +} + +func WithBasicAuth() ServerOption { + return func(t *testing.T, server *Server) { + server.WithMiddleware(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + username, password, ok := r.BasicAuth() + if !ok || username != "username" || password != "password" { + t.Errorf("Expected request to use basic auth and for username == 'username' and password == 'password', got '%v', '%s', '%s'", ok, username, password) + } + })) + + } +} + +// NewTempServer creates a server inside of a temp dir. // // If the passed in string is not "", it will be treated as a shell glob, and files // will be copied from that path to the server's docroot. // // The caller is responsible for stopping the server. // The temp dir will be removed by testing package automatically when test finished. -func NewTempServerWithCleanup(t *testing.T, glob string) (*Server, error) { - srv, err := NewTempServer(glob) +func NewTempServer(t *testing.T, glob string, options ...ServerOption) *Server { + + tdir, err := os.MkdirTemp("", "helm-repotest-") + if err != nil { + t.Fatal(err) + } + + srv := newServer(t, tdir, options...) + + if glob != "" { + if _, err := srv.CopyCharts(glob); err != nil { + t.Fatal(err) + } + } + t.Cleanup(func() { os.RemoveAll(srv.docroot) }) - return srv, err + + if srv.autostart { + srv.Start() + } + + return srv } -// Set up a fake repo with basic auth enabled -func NewTempServerWithCleanupAndBasicAuth(t *testing.T, glob string) *Server { - srv, err := NewTempServerWithCleanup(t, glob) - srv.Stop() +// NewServer creates a repository server for testing. +// +// docroot should be a temp dir managed by the caller. +// +// By default the server will be started, serving files off of the docroot. +// +// Use CopyCharts to move charts into the repository and then index them +// for service. +func NewServer(t *testing.T, docroot string, options ...ServerOption) *Server { + srv := newServer(t, docroot, options...) + + if srv.autostart { + srv.Start() + } + + return srv +} + +// Create the server, but don't yet start it +func newServer(t *testing.T, docroot string, options ...ServerOption) *Server { + root, err := filepath.Abs(docroot) if err != nil { t.Fatal(err) } - srv.WithMiddleware(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { - username, password, ok := r.BasicAuth() - if !ok || username != "username" || password != "password" { - t.Errorf("Expected request to use basic auth and for username == 'username' and password == 'password', got '%v', '%s', '%s'", ok, username, password) - } - })) - srv.Start() + + srv := &Server{ + docroot: root, + autostart: true, + } + + // Add the testing repository as the only repo. + if err := setTestingRepository(srv.URL(), filepath.Join(srv.docroot, "repositories.yaml")); err != nil { + t.Fatal(err) + } + + for _, option := range options { + option(t, srv) + } + return srv } +// Server is an implementation of a repository server for testing. +type Server struct { + docroot string + srv *httptest.Server + middleware http.HandlerFunc + autostart bool +} + +// WithMiddleware injects middleware in front of the server. This can be used to inject +// additional functionality like layering in an authentication frontend. +func (s *Server) WithMiddleware(middleware http.HandlerFunc) { + s.middleware = middleware +} + type OCIServer struct { *registry.Registry RegistryURL string @@ -238,69 +315,6 @@ func (srv *OCIServer) Run(t *testing.T, opts ...OCIServerOpt) { result.Chart.Digest, result.Chart.Size) } -// NewTempServer creates a server inside of a temp dir. -// -// If the passed in string is not "", it will be treated as a shell glob, and files -// will be copied from that path to the server's docroot. -// -// The caller is responsible for destroying the temp directory as well as stopping -// the server. -// -// Deprecated: use NewTempServerWithCleanup -func NewTempServer(glob string) (*Server, error) { - tdir, err := os.MkdirTemp("", "helm-repotest-") - if err != nil { - return nil, err - } - srv := NewServer(tdir) - - if glob != "" { - if _, err := srv.CopyCharts(glob); err != nil { - srv.Stop() - return srv, err - } - } - - return srv, nil -} - -// NewServer creates a repository server for testing. -// -// docroot should be a temp dir managed by the caller. -// -// This will start the server, serving files off of the docroot. -// -// Use CopyCharts to move charts into the repository and then index them -// for service. -func NewServer(docroot string) *Server { - root, err := filepath.Abs(docroot) - if err != nil { - panic(err) - } - srv := &Server{ - docroot: root, - } - srv.Start() - // Add the testing repository as the only repo. - if err := setTestingRepository(srv.URL(), filepath.Join(root, "repositories.yaml")); err != nil { - panic(err) - } - return srv -} - -// Server is an implementation of a repository server for testing. -type Server struct { - docroot string - srv *httptest.Server - middleware http.HandlerFunc -} - -// WithMiddleware injects middleware in front of the server. This can be used to inject -// additional functionality like layering in an authentication frontend. -func (s *Server) WithMiddleware(middleware http.HandlerFunc) { - s.middleware = middleware -} - // Root gets the docroot for the server. func (s *Server) Root() string { return s.docroot diff --git a/pkg/repo/repotest/server_test.go b/pkg/repo/repotest/server_test.go index a7d7f5b95..7949362e5 100644 --- a/pkg/repo/repotest/server_test.go +++ b/pkg/repo/repotest/server_test.go @@ -34,7 +34,7 @@ func TestServer(t *testing.T) { rootDir := t.TempDir() - srv := NewServer(rootDir) + srv := newServer(t, rootDir) defer srv.Stop() c, err := srv.CopyCharts("testdata/*.tgz") @@ -99,10 +99,7 @@ func TestServer(t *testing.T) { func TestNewTempServer(t *testing.T) { ensure.HelmHome(t) - srv, err := NewTempServerWithCleanup(t, "testdata/examplechart-0.1.0.tgz") - if err != nil { - t.Fatal(err) - } + srv := NewTempServer(t, "testdata/examplechart-0.1.0.tgz") defer srv.Stop() res, err := http.Head(srv.URL() + "/examplechart-0.1.0.tgz")