From 5d84ee32cefd0ec74a74f5c83f14e04c4f844391 Mon Sep 17 00:00:00 2001 From: JaeguKim Date: Sun, 15 Feb 2026 03:03:20 +0900 Subject: [PATCH] fix(logging): replace global slog usage with dependency-injected loggers Signed-off-by: JaeguKim --- internal/chart/v3/lint/rules/template.go | 2 +- internal/chart/v3/lint/rules/values.go | 2 +- internal/chart/v3/loader/directory.go | 2 +- internal/chart/v3/util/dependencies.go | 35 +++++----- internal/chart/v3/util/dependencies_test.go | 27 ++++---- internal/plugin/installer/http_installer.go | 12 +++- internal/plugin/installer/installer.go | 17 ++++- internal/plugin/installer/local_installer.go | 16 +++-- internal/plugin/installer/oci_installer.go | 18 +++-- internal/plugin/installer/vcs_installer.go | 22 +++++-- internal/plugin/runtime_extismv1.go | 18 +++-- internal/plugin/runtime_subprocess.go | 27 +++++--- internal/plugin/runtime_subprocess_getter.go | 4 +- internal/release/v2/util/manifest_sorter.go | 11 ++-- .../release/v2/util/manifest_sorter_test.go | 2 +- internal/sympath/walk.go | 15 +++-- internal/sympath/walk_test.go | 2 +- internal/version/version.go | 13 ++-- pkg/action/action.go | 3 +- pkg/action/get_metadata.go | 11 +++- pkg/action/uninstall.go | 8 +-- pkg/action/upgrade.go | 2 +- pkg/chart/common.go | 32 ++++++--- pkg/chart/common/capabilities.go | 2 +- pkg/chart/common/util/jsonschema.go | 16 +++-- pkg/chart/common/util/jsonschema_test.go | 16 ++++- pkg/chart/v2/lint/rules/template.go | 2 +- pkg/chart/v2/lint/rules/values.go | 2 +- pkg/chart/v2/loader/directory.go | 2 +- pkg/chart/v2/util/dependencies.go | 35 +++++----- pkg/chart/v2/util/dependencies_test.go | 27 ++++---- pkg/cmd/flags.go | 27 ++++---- pkg/cmd/helpers.go | 6 +- pkg/cmd/helpers_test.go | 2 +- pkg/cmd/install.go | 20 +++--- pkg/cmd/load_plugins.go | 20 +++--- pkg/cmd/plugin.go | 11 ++-- pkg/cmd/plugin_install.go | 7 +- pkg/cmd/plugin_list.go | 4 +- pkg/cmd/plugin_test.go | 9 +-- pkg/cmd/plugin_uninstall.go | 21 +++--- pkg/cmd/plugin_update.go | 15 +++-- pkg/cmd/pull.go | 3 +- pkg/cmd/registry_login.go | 6 +- pkg/cmd/rollback.go | 4 +- pkg/cmd/root.go | 30 +++------ pkg/cmd/root_test.go | 35 ++++++++-- pkg/cmd/search.go | 7 +- pkg/cmd/search_hub.go | 7 +- pkg/cmd/search_repo.go | 13 ++-- pkg/cmd/show.go | 16 ++--- pkg/cmd/template.go | 6 +- pkg/cmd/uninstall.go | 2 +- pkg/cmd/upgrade.go | 18 ++--- pkg/cmd/version.go | 4 +- pkg/downloader/cache.go | 12 +++- pkg/downloader/chart_downloader.go | 28 +++++--- pkg/engine/engine.go | 29 +++++--- pkg/engine/engine_test.go | 3 +- pkg/engine/lookup_func.go | 20 +++--- pkg/ignore/rules.go | 16 +++-- pkg/kube/client.go | 16 ++--- pkg/kube/ready.go | 66 ++++++++++++------- pkg/kube/wait.go | 53 ++++++++------- pkg/registry/client.go | 21 +++++- pkg/registry/client_test.go | 35 +++++++++- pkg/registry/transport.go | 16 +++-- pkg/release/v1/util/manifest_sorter.go | 11 ++-- pkg/release/v1/util/manifest_sorter_test.go | 2 +- pkg/repo/v1/chartrepo.go | 2 - pkg/repo/v1/index.go | 18 +++-- pkg/storage/driver/cfgmaps.go | 4 +- pkg/storage/driver/memory.go | 5 +- pkg/storage/driver/secrets.go | 4 +- pkg/storage/driver/sql.go | 1 - pkg/storage/storage.go | 7 +- 76 files changed, 660 insertions(+), 405 deletions(-) diff --git a/internal/chart/v3/lint/rules/template.go b/internal/chart/v3/lint/rules/template.go index a8ae910eb..e019fc397 100644 --- a/internal/chart/v3/lint/rules/template.go +++ b/internal/chart/v3/lint/rules/template.go @@ -88,7 +88,7 @@ func TemplatesWithSkipSchemaValidation(linter *support.Linter, values map[string // lint ignores import-values // See https://github.com/helm/helm/issues/9658 - if err := chartutil.ProcessDependencies(chart, values); err != nil { + if err := chartutil.ProcessDependencies(chart, values, nil); err != nil { return } diff --git a/internal/chart/v3/lint/rules/values.go b/internal/chart/v3/lint/rules/values.go index b4a2edb0c..b681c75f1 100644 --- a/internal/chart/v3/lint/rules/values.go +++ b/internal/chart/v3/lint/rules/values.go @@ -78,7 +78,7 @@ func validateValuesFile(valuesPath string, overrides map[string]any, skipSchemaV } if !skipSchemaValidation { - return util.ValidateAgainstSingleSchema(coalescedValues, schema) + return util.ValidateAgainstSingleSchema(coalescedValues, schema, nil) } return nil diff --git a/internal/chart/v3/loader/directory.go b/internal/chart/v3/loader/directory.go index dfe3af3b2..e0c649921 100644 --- a/internal/chart/v3/loader/directory.go +++ b/internal/chart/v3/loader/directory.go @@ -114,7 +114,7 @@ func LoadDir(dir string) (*chart.Chart, error) { files = append(files, &archive.BufferedFile{Name: n, ModTime: fi.ModTime(), Data: data}) return nil } - if err = sympath.Walk(topdir, walk); err != nil { + if err = sympath.Walk(topdir, walk, nil); err != nil { return c, err } diff --git a/internal/chart/v3/util/dependencies.go b/internal/chart/v3/util/dependencies.go index b31f7eb96..55a618f7f 100644 --- a/internal/chart/v3/util/dependencies.go +++ b/internal/chart/v3/util/dependencies.go @@ -30,14 +30,15 @@ import ( // ProcessDependencies checks through this chart's dependencies, processing accordingly. func ProcessDependencies(c *chart.Chart, v common.Values) error { - if err := processDependencyEnabled(c, v, ""); err != nil { + logger := slog.Default() + if err := processDependencyEnabled(c, v, "", logger); err != nil { return err } - return processDependencyImportValues(c, true) + return processDependencyImportValues(c, true, logger) } // processDependencyConditions disables charts based on condition path value in values -func processDependencyConditions(reqs []*chart.Dependency, cvals common.Values, cpath string) { +func processDependencyConditions(reqs []*chart.Dependency, cvals common.Values, cpath string, logger *slog.Logger) { if reqs == nil { return } @@ -53,10 +54,10 @@ func processDependencyConditions(reqs []*chart.Dependency, cvals common.Values, r.Enabled = bv break } - slog.Warn("returned non-bool value", "path", c, "chart", r.Name) + logger.Warn("returned non-bool value", "path", c, "chart", r.Name) } else if errors.As(err, &errNoValue) { // this is a real error - slog.Warn("the method PathValue returned error", slog.Any("error", err)) + logger.Warn("the method PathValue returned error", slog.Any("error", err)) } } } @@ -64,7 +65,7 @@ func processDependencyConditions(reqs []*chart.Dependency, cvals common.Values, } // processDependencyTags disables charts based on tags in values -func processDependencyTags(reqs []*chart.Dependency, cvals common.Values) { +func processDependencyTags(reqs []*chart.Dependency, cvals common.Values, logger *slog.Logger) { if reqs == nil { return } @@ -84,7 +85,7 @@ func processDependencyTags(reqs []*chart.Dependency, cvals common.Values) { hasFalse = true } } else { - slog.Warn("returned non-bool value", "tag", k, "chart", r.Name) + logger.Warn("returned non-bool value", "tag", k, "chart", r.Name) } } } @@ -143,7 +144,7 @@ func copyMetadata(metadata *chart.Metadata) *chart.Metadata { } // processDependencyEnabled removes disabled charts from dependencies -func processDependencyEnabled(c *chart.Chart, v map[string]any, path string) error { +func processDependencyEnabled(c *chart.Chart, v map[string]any, path string, logger *slog.Logger) error { if c.Metadata.Dependencies == nil { return nil } @@ -186,8 +187,8 @@ Loop: return err } // flag dependencies as enabled/disabled - processDependencyTags(c.Metadata.Dependencies, cvals) - processDependencyConditions(c.Metadata.Dependencies, cvals, path) + processDependencyTags(c.Metadata.Dependencies, cvals, logger) + processDependencyConditions(c.Metadata.Dependencies, cvals, path, logger) // make a map of charts to remove rm := map[string]struct{}{} for _, r := range c.Metadata.Dependencies { @@ -216,7 +217,7 @@ Loop: // recursively call self to process sub dependencies for _, t := range cd { subpath := path + t.Metadata.Name + "." - if err := processDependencyEnabled(t, cvals, subpath); err != nil { + if err := processDependencyEnabled(t, cvals, subpath, logger); err != nil { return err } } @@ -250,7 +251,7 @@ func set(path []string, data map[string]any) map[string]any { } // processImportValues merges values from child to parent based on the chart's dependencies' ImportValues field. -func processImportValues(c *chart.Chart, merge bool) error { +func processImportValues(c *chart.Chart, merge bool, logger *slog.Logger) error { if c.Metadata.Dependencies == nil { return nil } @@ -283,7 +284,7 @@ func processImportValues(c *chart.Chart, merge bool) error { // get child table vv, err := cvals.Table(r.Name + "." + child) if err != nil { - slog.Warn( + logger.Warn( "ImportValues missing table from chart", slog.String("chart", "chart"), slog.String("name", r.Name), @@ -305,7 +306,7 @@ func processImportValues(c *chart.Chart, merge bool) error { }) vm, err := cvals.Table(r.Name + "." + child) if err != nil { - slog.Warn("ImportValues missing table", slog.Any("error", err)) + logger.Warn("ImportValues missing table", slog.Any("error", err)) continue } if merge { @@ -373,12 +374,12 @@ func istable(v any) bool { } // processDependencyImportValues imports specified chart values from child to parent. -func processDependencyImportValues(c *chart.Chart, merge bool) error { +func processDependencyImportValues(c *chart.Chart, merge bool, logger *slog.Logger) error { for _, d := range c.Dependencies() { // recurse - if err := processDependencyImportValues(d, merge); err != nil { + if err := processDependencyImportValues(d, merge, logger); err != nil { return err } } - return processImportValues(c, merge) + return processImportValues(c, merge, logger) } diff --git a/internal/chart/v3/util/dependencies_test.go b/internal/chart/v3/util/dependencies_test.go index c8a176725..f238309db 100644 --- a/internal/chart/v3/util/dependencies_test.go +++ b/internal/chart/v3/util/dependencies_test.go @@ -15,6 +15,7 @@ limitations under the License. package util import ( + "log/slog" "os" "path/filepath" "sort" @@ -117,7 +118,7 @@ func TestDependencyEnabled(t *testing.T) { for _, tc := range tests { c := loadChart(t, "testdata/subpop") t.Run(tc.name, func(t *testing.T) { - if err := processDependencyEnabled(c, tc.v, ""); err != nil { + if err := processDependencyEnabled(c, tc.v, "", slog.New(slog.DiscardHandler)); err != nil { t.Fatalf("error processing enabled dependencies %v", err) } @@ -219,7 +220,7 @@ func TestProcessDependencyImportValues(t *testing.T) { e["SCBexported2A"] = "blaster" e["global.SC1exported2.all.SC1exported3"] = "SC1expstr" - if err := processDependencyImportValues(c, false); err != nil { + if err := processDependencyImportValues(c, false, slog.New(slog.DiscardHandler)); err != nil { t.Fatalf("processing import values dependencies %v", err) } cc := common.Values(c.Values) @@ -259,7 +260,7 @@ func TestProcessDependencyImportValues(t *testing.T) { } c = loadChart(t, "testdata/subpop") - if err := processDependencyImportValues(c, true); err != nil { + if err := processDependencyImportValues(c, true, slog.New(slog.DiscardHandler)); err != nil { t.Fatalf("processing import values dependencies %v", err) } cc = common.Values(c.Values) @@ -275,10 +276,10 @@ func TestProcessDependencyImportValues(t *testing.T) { func TestProcessDependencyImportValuesFromSharedDependencyToAliases(t *testing.T) { c := loadChart(t, "testdata/chart-with-import-from-aliased-dependencies") - if err := processDependencyEnabled(c, c.Values, ""); err != nil { + if err := processDependencyEnabled(c, c.Values, "", slog.New(slog.DiscardHandler)); err != nil { t.Fatalf("expected no errors but got %q", err) } - if err := processDependencyImportValues(c, true); err != nil { + if err := processDependencyImportValues(c, true, slog.New(slog.DiscardHandler)); err != nil { t.Fatalf("processing import values dependencies %v", err) } e := make(map[string]string) @@ -327,7 +328,7 @@ func TestProcessDependencyImportValuesMultiLevelPrecedence(t *testing.T) { e["app2.service.port"] = "8080" e["app3.service.port"] = "9090" e["app4.service.port"] = "1234" - if err := processDependencyImportValues(c, true); err != nil { + if err := processDependencyImportValues(c, true, slog.New(slog.DiscardHandler)); err != nil { t.Fatalf("processing import values dependencies %v", err) } cc := common.Values(c.Values) @@ -354,7 +355,7 @@ func TestProcessDependencyImportValuesForEnabledCharts(t *testing.T) { c := loadChart(t, "testdata/import-values-from-enabled-subchart/parent-chart") nameOverride := "parent-chart-prod" - if err := processDependencyImportValues(c, true); err != nil { + if err := processDependencyImportValues(c, true, slog.New(slog.DiscardHandler)); err != nil { t.Fatalf("processing import values dependencies %v", err) } @@ -362,7 +363,7 @@ func TestProcessDependencyImportValuesForEnabledCharts(t *testing.T) { t.Fatalf("expected 2 dependencies for this chart, but got %d", len(c.Dependencies())) } - if err := processDependencyEnabled(c, c.Values, ""); err != nil { + if err := processDependencyEnabled(c, c.Values, "", slog.New(slog.DiscardHandler)); err != nil { t.Fatalf("expected no errors but got %q", err) } @@ -427,7 +428,7 @@ func TestDependentChartAliases(t *testing.T) { t.Fatalf("expected 2 dependencies for this chart, but got %d", len(c.Dependencies())) } - if err := processDependencyEnabled(c, c.Values, ""); err != nil { + if err := processDependencyEnabled(c, c.Values, "", slog.New(slog.DiscardHandler)); err != nil { t.Fatalf("expected no errors but got %q", err) } @@ -469,7 +470,7 @@ func TestDependentChartWithSubChartsAbsentInDependency(t *testing.T) { t.Fatalf("expected 2 dependencies for this chart, but got %d", len(c.Dependencies())) } - if err := processDependencyEnabled(c, c.Values, ""); err != nil { + if err := processDependencyEnabled(c, c.Values, "", slog.New(slog.DiscardHandler)); err != nil { t.Fatalf("expected no errors but got %q", err) } @@ -506,7 +507,7 @@ func TestDependentChartsWithSubchartsAllSpecifiedInDependency(t *testing.T) { t.Fatalf("expected 2 dependencies for this chart, but got %d", len(c.Dependencies())) } - if err := processDependencyEnabled(c, c.Values, ""); err != nil { + if err := processDependencyEnabled(c, c.Values, "", slog.New(slog.DiscardHandler)); err != nil { t.Fatalf("expected no errors but got %q", err) } @@ -526,7 +527,7 @@ func TestDependentChartsWithSomeSubchartsSpecifiedInDependency(t *testing.T) { t.Fatalf("expected 2 dependencies for this chart, but got %d", len(c.Dependencies())) } - if err := processDependencyEnabled(c, c.Values, ""); err != nil { + if err := processDependencyEnabled(c, c.Values, "", slog.New(slog.DiscardHandler)); err != nil { t.Fatalf("expected no errors but got %q", err) } @@ -559,7 +560,7 @@ func TestChartWithDependencyAliasedTwiceAndDoublyReferencedSubDependency(t *test t.Fatalf("expected one dependency for this chart, but got %d", len(c.Dependencies())) } - if err := processDependencyEnabled(c, c.Values, ""); err != nil { + if err := processDependencyEnabled(c, c.Values, "", slog.New(slog.DiscardHandler)); err != nil { t.Fatalf("expected no errors but got %q", err) } diff --git a/internal/plugin/installer/http_installer.go b/internal/plugin/installer/http_installer.go index 5a2912d2e..d184d918b 100644 --- a/internal/plugin/installer/http_installer.go +++ b/internal/plugin/installer/http_installer.go @@ -42,6 +42,14 @@ type HTTPInstaller struct { // Cached data to avoid duplicate downloads pluginData []byte provData []byte + logger *slog.Logger +} + +func (i *HTTPInstaller) log() *slog.Logger { + if i.logger != nil { + return i.logger + } + return slog.New(slog.DiscardHandler) } // NewHTTPInstaller creates a new HttpInstaller. @@ -113,7 +121,7 @@ func (i *HTTPInstaller) Install() error { if i.provData != nil { provPath := tarballPath + ".prov" if err := os.WriteFile(provPath, i.provData, 0644); err != nil { - slog.Debug("failed to save provenance file", "error", err) + i.log().Debug("failed to save provenance file", "error", err) } } @@ -137,7 +145,7 @@ func (i *HTTPInstaller) Install() error { return err } - slog.Debug("copying", "source", src, "path", i.Path()) + i.log().Debug("copying", "source", src, "path", i.Path()) return fs.CopyDir(src, i.Path()) } diff --git a/internal/plugin/installer/installer.go b/internal/plugin/installer/installer.go index 69a797ad9..80a1523ca 100644 --- a/internal/plugin/installer/installer.go +++ b/internal/plugin/installer/installer.go @@ -31,6 +31,17 @@ import ( // ErrMissingMetadata indicates that plugin.yaml is missing. var ErrMissingMetadata = errors.New("plugin metadata (plugin.yaml) missing") +// installerLogger extracts a *slog.Logger from an Installer if it provides one. +func installerLogger(i Installer) *slog.Logger { + type logProvider interface { + log() *slog.Logger + } + if lp, ok := i.(logProvider); ok { + return lp.log() + } + return slog.New(slog.DiscardHandler) +} + // Options contains options for plugin installation. type Options struct { // Verify enables signature verification before installation @@ -77,7 +88,7 @@ func InstallWithOptions(i Installer, opts Options) (*VerificationResult, error) return nil, err } if _, pathErr := os.Stat(i.Path()); !os.IsNotExist(pathErr) { - slog.Warn("plugin already exists", slog.String("path", i.Path()), slog.Any("error", pathErr)) + installerLogger(i).Warn("plugin already exists", slog.String("path", i.Path()), slog.Any("error", pathErr)) return nil, errors.New("plugin already exists") } @@ -128,7 +139,7 @@ func InstallWithOptions(i Installer, opts Options) (*VerificationResult, error) // Update updates a plugin. func Update(i Installer) error { if _, pathErr := os.Stat(i.Path()); os.IsNotExist(pathErr) { - slog.Warn("plugin does not exist", slog.String("path", i.Path()), slog.Any("error", pathErr)) + installerLogger(i).Warn("plugin does not exist", slog.String("path", i.Path()), slog.Any("error", pathErr)) return errors.New("plugin does not exist") } return i.Update() @@ -159,7 +170,7 @@ func NewForSource(source, version string) (installer Installer, err error) { func FindSource(location string) (Installer, error) { installer, err := existingVCSRepo(location) if err != nil && err.Error() == "Cannot detect VCS" { - slog.Warn( + installerLogger(installer).Warn( "cannot get information about plugin source", slog.String("location", location), slog.Any("error", err), diff --git a/internal/plugin/installer/local_installer.go b/internal/plugin/installer/local_installer.go index 71407380f..48d183afc 100644 --- a/internal/plugin/installer/local_installer.go +++ b/internal/plugin/installer/local_installer.go @@ -39,6 +39,14 @@ type LocalInstaller struct { extractor Extractor pluginData []byte // Cached plugin data provData []byte // Cached provenance data + logger *slog.Logger +} + +func (i *LocalInstaller) log() *slog.Logger { + if i.logger != nil { + return i.logger + } + return slog.New(slog.DiscardHandler) } // NewLocalInstaller creates a new LocalInstaller. @@ -97,7 +105,7 @@ func (i *LocalInstaller) installFromDirectory() error { if !isPlugin(i.Source) { return ErrMissingMetadata } - slog.Debug("symlinking", "source", i.Source, "path", i.Path()) + i.log().Debug("symlinking", "source", i.Source, "path", i.Path()) return os.Symlink(i.Source, i.Path()) } @@ -129,7 +137,7 @@ func (i *LocalInstaller) installFromArchive() error { if provData, err := os.ReadFile(provSource); err == nil { provPath := tarballPath + ".prov" if err := os.WriteFile(provPath, provData, 0644); err != nil { - slog.Debug("failed to save provenance file", "error", err) + i.log().Debug("failed to save provenance file", "error", err) } } @@ -154,13 +162,13 @@ func (i *LocalInstaller) installFromArchive() error { } // Copy to the final destination - slog.Debug("copying", "source", pluginDir, "path", i.Path()) + i.log().Debug("copying", "source", pluginDir, "path", i.Path()) return fs.CopyDir(pluginDir, i.Path()) } // Update updates a local repository func (i *LocalInstaller) Update() error { - slog.Debug("local repository is auto-updated") + i.log().Debug("local repository is auto-updated") return nil } diff --git a/internal/plugin/installer/oci_installer.go b/internal/plugin/installer/oci_installer.go index 50d01522a..5a887a6a2 100644 --- a/internal/plugin/installer/oci_installer.go +++ b/internal/plugin/installer/oci_installer.go @@ -48,6 +48,14 @@ type OCIInstaller struct { // Cached data to avoid duplicate downloads pluginData []byte provData []byte + logger *slog.Logger +} + +func (i *OCIInstaller) log() *slog.Logger { + if i.logger != nil { + return i.logger + } + return slog.New(slog.DiscardHandler) } // NewOCIInstaller creates a new OCIInstaller with optional getter options @@ -85,7 +93,7 @@ func NewOCIInstaller(source string, options ...getter.Option) (*OCIInstaller, er // Install downloads and installs a plugin from OCI registry // Implements Installer. func (i *OCIInstaller) Install() error { - slog.Debug("pulling OCI plugin", "source", i.Source) + i.log().Debug("pulling OCI plugin", "source", i.Source) // Ensure plugin data is cached if i.pluginData == nil { @@ -124,7 +132,7 @@ func (i *OCIInstaller) Install() error { if i.provData != nil { provPath := tarballPath + ".prov" if err := os.WriteFile(provPath, i.provData, 0644); err != nil { - slog.Debug("failed to save provenance file", "error", err) + i.log().Debug("failed to save provenance file", "error", err) } } @@ -177,7 +185,7 @@ func (i *OCIInstaller) Install() error { return err } - slog.Debug("copying", "source", src, "path", i.Path()) + i.log().Debug("copying", "source", src, "path", i.Path()) return fs.CopyDir(src, i.Path()) } @@ -264,7 +272,7 @@ func (i *OCIInstaller) SupportsVerification() bool { // GetVerificationData downloads and caches plugin and provenance data from OCI registry for verification func (i *OCIInstaller) GetVerificationData() (archiveData, provData []byte, filename string, err error) { - slog.Debug("getting verification data for OCI plugin", "source", i.Source) + i.log().Debug("getting verification data for OCI plugin", "source", i.Source) // Download plugin data once and cache it if i.pluginData == nil { @@ -297,6 +305,6 @@ func (i *OCIInstaller) GetVerificationData() (archiveData, provData []byte, file } filename = fmt.Sprintf("%s-%s.tgz", metadata.Name, metadata.Version) - slog.Debug("got verification data for OCI plugin", "filename", filename) + i.log().Debug("got verification data for OCI plugin", "filename", filename) return i.pluginData, i.provData, filename, nil } diff --git a/internal/plugin/installer/vcs_installer.go b/internal/plugin/installer/vcs_installer.go index 3601ec7a8..0f6fceac0 100644 --- a/internal/plugin/installer/vcs_installer.go +++ b/internal/plugin/installer/vcs_installer.go @@ -36,6 +36,14 @@ type VCSInstaller struct { Repo vcs.Repo Version string base + logger *slog.Logger +} + +func (i *VCSInstaller) log() *slog.Logger { + if i.logger != nil { + return i.logger + } + return slog.New(slog.DiscardHandler) } func existingVCSRepo(location string) (Installer, error) { @@ -91,13 +99,13 @@ func (i *VCSInstaller) Install() error { return ErrMissingMetadata } - slog.Debug("copying files", "source", i.Repo.LocalPath(), "destination", i.Path()) + i.log().Debug("copying files", "source", i.Repo.LocalPath(), "destination", i.Path()) return fs.CopyDir(i.Repo.LocalPath(), i.Path()) } // Update updates a remote repository func (i *VCSInstaller) Update() error { - slog.Debug("updating", "source", i.Repo.Remote()) + i.log().Debug("updating", "source", i.Repo.Remote()) if i.Repo.IsDirty() { return errors.New("plugin repo was modified") } @@ -131,7 +139,7 @@ func (i *VCSInstaller) solveVersion(repo vcs.Repo) (string, error) { if err != nil { return "", err } - slog.Debug("found refs", "refs", refs) + i.log().Debug("found refs", "refs", refs) // Convert and filter the list to semver.Version instances semvers := getSemVers(refs) @@ -142,7 +150,7 @@ func (i *VCSInstaller) solveVersion(repo vcs.Repo) (string, error) { if constraint.Check(v) { // If the constraint passes get the original reference ver := v.Original() - slog.Debug("setting to version", "version", ver) + i.log().Debug("setting to version", "version", ver) return ver, nil } } @@ -152,17 +160,17 @@ func (i *VCSInstaller) solveVersion(repo vcs.Repo) (string, error) { // setVersion attempts to checkout the version func (i *VCSInstaller) setVersion(repo vcs.Repo, ref string) error { - slog.Debug("setting version", "version", i.Version) + i.log().Debug("setting version", "version", i.Version) return repo.UpdateVersion(ref) } // sync will clone or update a remote repo. func (i *VCSInstaller) sync(repo vcs.Repo) error { if _, err := os.Stat(repo.LocalPath()); errors.Is(err, stdfs.ErrNotExist) { - slog.Debug("cloning", "source", repo.Remote(), "destination", repo.LocalPath()) + i.log().Debug("cloning", "source", repo.Remote(), "destination", repo.LocalPath()) return repo.Get() } - slog.Debug("updating", "source", repo.Remote(), "destination", repo.LocalPath()) + i.log().Debug("updating", "source", repo.Remote(), "destination", repo.LocalPath()) return repo.Update() } diff --git a/internal/plugin/runtime_extismv1.go b/internal/plugin/runtime_extismv1.go index cd9a02535..3a23b3915 100644 --- a/internal/plugin/runtime_extismv1.go +++ b/internal/plugin/runtime_extismv1.go @@ -126,6 +126,14 @@ type ExtismV1PluginRuntime struct { dir string rc *RuntimeConfigExtismV1 r *RuntimeExtismV1 + logger *slog.Logger +} + +func (p *ExtismV1PluginRuntime) log() *slog.Logger { + if p.logger != nil { + return p.logger + } + return slog.New(slog.DiscardHandler) } var _ Plugin = (*ExtismV1PluginRuntime)(nil) @@ -143,13 +151,13 @@ func (p *ExtismV1PluginRuntime) Invoke(ctx context.Context, input *Input) (*Outp var tmpDir string if p.rc.FileSystem.CreateTempDir { tmpDirInner, err := os.MkdirTemp(os.TempDir(), "helm-plugin-*") - slog.Debug("created plugin temp dir", slog.String("dir", tmpDirInner), slog.String("plugin", p.metadata.Name)) + p.log().Debug("created plugin temp dir", slog.String("dir", tmpDirInner), slog.String("plugin", p.metadata.Name)) if err != nil { return nil, fmt.Errorf("failed to create temp dir for extism compilation cache: %w", err) } defer func() { if err := os.RemoveAll(tmpDir); err != nil { - slog.Warn("failed to remove plugin temp dir", slog.String("dir", tmpDir), slog.String("plugin", p.metadata.Name), slog.String("error", err.Error())) + p.log().Warn("failed to remove plugin temp dir", slog.String("dir", tmpDir), slog.String("plugin", p.metadata.Name), slog.String("error", err.Error())) } }() @@ -174,7 +182,7 @@ func (p *ExtismV1PluginRuntime) Invoke(ctx context.Context, input *Input) (*Outp } pe.SetLogger(func(logLevel extism.LogLevel, s string) { - slog.Debug(s, slog.String("level", logLevel.String()), slog.String("plugin", p.metadata.Name)) + p.log().Debug(s, slog.String("level", logLevel.String()), slog.String("plugin", p.metadata.Name)) }) inputData, err := json.Marshal(input.Message) @@ -182,7 +190,7 @@ func (p *ExtismV1PluginRuntime) Invoke(ctx context.Context, input *Input) (*Outp return nil, fmt.Errorf("failed to json marshal plugin input message: %T: %w", input.Message, err) } - slog.Debug("plugin input", slog.String("plugin", p.metadata.Name), slog.String("inputData", string(inputData))) + p.log().Debug("plugin input", slog.String("plugin", p.metadata.Name), slog.String("inputData", string(inputData))) entryFuncName := p.rc.EntryFuncName if entryFuncName == "" { @@ -200,7 +208,7 @@ func (p *ExtismV1PluginRuntime) Invoke(ctx context.Context, input *Input) (*Outp } } - slog.Debug("plugin output", slog.String("plugin", p.metadata.Name), slog.Int("exitCode", int(exitCode)), slog.String("outputData", string(outputData))) + p.log().Debug("plugin output", slog.String("plugin", p.metadata.Name), slog.Int("exitCode", int(exitCode)), slog.String("outputData", string(outputData))) outputMessage := reflect.New(pluginTypesIndex[p.metadata.Type].outputType) if err := json.Unmarshal(outputData, outputMessage.Interface()); err != nil { diff --git a/internal/plugin/runtime_subprocess.go b/internal/plugin/runtime_subprocess.go index cd1a0842c..188dae9b4 100644 --- a/internal/plugin/runtime_subprocess.go +++ b/internal/plugin/runtime_subprocess.go @@ -84,6 +84,14 @@ type SubprocessPluginRuntime struct { pluginDir string RuntimeConfig RuntimeConfigSubprocess EnvVars map[string]string + logger *slog.Logger +} + +func (r *SubprocessPluginRuntime) log() *slog.Logger { + if r.logger != nil { + return r.logger + } + return slog.New(slog.DiscardHandler) } var _ Plugin = (*SubprocessPluginRuntime)(nil) @@ -125,7 +133,7 @@ func (r *SubprocessPluginRuntime) InvokeWithEnv(main string, argv []string, env cmd.Stdout = stdout cmd.Stderr = stderr - if err := executeCmd(cmd, r.metadata.Name); err != nil { + if err := executeCmd(cmd, r.metadata.Name, r.logger); err != nil { return err } @@ -154,7 +162,7 @@ func (r *SubprocessPluginRuntime) InvokeHook(event string) error { cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr - slog.Debug("executing plugin hook command", slog.String("pluginName", r.metadata.Name), slog.String("command", cmd.String())) + r.log().Debug("executing plugin hook command", slog.String("pluginName", r.metadata.Name), slog.String("command", cmd.String())) if err := cmd.Run(); err != nil { if eerr, ok := err.(*exec.ExitError); ok { os.Stderr.Write(eerr.Stderr) @@ -168,10 +176,13 @@ func (r *SubprocessPluginRuntime) InvokeHook(event string) error { // TODO decide the best way to handle this code // right now we implement status and error return in 3 slightly different ways in this file // then replace the other three with a call to this func -func executeCmd(prog *exec.Cmd, pluginName string) error { +func executeCmd(prog *exec.Cmd, pluginName string, logger *slog.Logger) error { + if logger == nil { + logger = slog.New(slog.DiscardHandler) + } if err := prog.Run(); err != nil { if eerr, ok := err.(*exec.ExitError); ok { - slog.Debug( + logger.Debug( "plugin execution failed", slog.String("pluginName", pluginName), slog.String("error", err.Error()), @@ -216,8 +227,8 @@ func (r *SubprocessPluginRuntime) runCLI(input *Input) (*Output, error) { cmd.Stdout = input.Stdout cmd.Stderr = input.Stderr - slog.Debug("executing plugin command", slog.String("pluginName", r.metadata.Name), slog.String("command", cmd.String())) - if err := executeCmd(cmd, r.metadata.Name); err != nil { + r.log().Debug("executing plugin command", slog.String("pluginName", r.metadata.Name), slog.String("command", cmd.String())) + if err := executeCmd(cmd, r.metadata.Name, r.logger); err != nil { return nil, err } @@ -265,8 +276,8 @@ func (r *SubprocessPluginRuntime) runPostrenderer(input *Input) (*Output, error) cmd.Stdout = postRendered cmd.Stderr = stderr - slog.Debug("executing plugin command", slog.String("pluginName", r.metadata.Name), slog.String("command", cmd.String())) - if err := executeCmd(cmd, r.metadata.Name); err != nil { + r.log().Debug("executing plugin command", slog.String("pluginName", r.metadata.Name), slog.String("command", cmd.String())) + if err := executeCmd(cmd, r.metadata.Name, r.logger); err != nil { return nil, err } diff --git a/internal/plugin/runtime_subprocess_getter.go b/internal/plugin/runtime_subprocess_getter.go index c7262e0dd..3f88451d2 100644 --- a/internal/plugin/runtime_subprocess_getter.go +++ b/internal/plugin/runtime_subprocess_getter.go @@ -88,8 +88,8 @@ func (r *SubprocessPluginRuntime) runGetter(input *Input) (*Output, error) { cmd.Stdout = &buf cmd.Stderr = os.Stderr - slog.Debug("executing plugin command", slog.String("pluginName", r.metadata.Name), slog.String("command", cmd.String())) - if err := executeCmd(cmd, r.metadata.Name); err != nil { + r.log().Debug("executing plugin command", slog.String("pluginName", r.metadata.Name), slog.String("command", cmd.String())) + if err := executeCmd(cmd, r.metadata.Name, r.logger); err != nil { return nil, err } diff --git a/internal/release/v2/util/manifest_sorter.go b/internal/release/v2/util/manifest_sorter.go index f269dda6d..4a0fe6f06 100644 --- a/internal/release/v2/util/manifest_sorter.go +++ b/internal/release/v2/util/manifest_sorter.go @@ -74,7 +74,10 @@ var events = map[string]v2.HookEvent{ // // Files that do not parse into the expected format are simply placed into a map and // returned. -func SortManifests(files map[string]string, _ common.VersionSet, ordering KindSortOrder) ([]*v2.Hook, []Manifest, error) { +func SortManifests(files map[string]string, _ common.VersionSet, ordering KindSortOrder, logger *slog.Logger) ([]*v2.Hook, []Manifest, error) { + if logger == nil { + logger = slog.New(slog.DiscardHandler) + } result := &result{} var sortedFilePaths []string @@ -101,7 +104,7 @@ func SortManifests(files map[string]string, _ common.VersionSet, ordering KindSo path: filePath, } - if err := manifestFile.sort(result); err != nil { + if err := manifestFile.sort(result, logger); err != nil { return result.hooks, result.generic, err } } @@ -136,7 +139,7 @@ func SortManifests(files map[string]string, _ common.VersionSet, ordering KindSo // metadata: // annotations: // helm.sh/hook-output-log-policy: hook-succeeded,hook-failed -func (file *manifestFile) sort(result *result) error { +func (file *manifestFile) sort(result *result, logger *slog.Logger) error { // Go through manifests in order found in file (function `SplitManifests` creates integer-sortable keys) var sortedEntryKeys []string for entryKey := range file.entries { @@ -196,7 +199,7 @@ func (file *manifestFile) sort(result *result) error { } if isUnknownHook { - slog.Info("skipping unknown hooks", "hookTypes", hookTypes) + logger.Info("skipping unknown hooks", "hookTypes", hookTypes) continue } diff --git a/internal/release/v2/util/manifest_sorter_test.go b/internal/release/v2/util/manifest_sorter_test.go index 28f0b34cc..632324731 100644 --- a/internal/release/v2/util/manifest_sorter_test.go +++ b/internal/release/v2/util/manifest_sorter_test.go @@ -138,7 +138,7 @@ metadata: manifests[o.path] = o.manifest } - hs, generic, err := SortManifests(manifests, nil, InstallOrder) + hs, generic, err := SortManifests(manifests, nil, InstallOrder, nil) if err != nil { t.Fatalf("Unexpected error: %s", err) } diff --git a/internal/sympath/walk.go b/internal/sympath/walk.go index 812bb68ce..dbad472db 100644 --- a/internal/sympath/walk.go +++ b/internal/sympath/walk.go @@ -33,12 +33,15 @@ import ( // are filtered by walkFn. The files are walked in lexical order, which makes the // output deterministic but means that for very large directories Walk can be // inefficient. Walk follows symbolic links. -func Walk(root string, walkFn filepath.WalkFunc) error { +func Walk(root string, walkFn filepath.WalkFunc, logger *slog.Logger) error { + if logger == nil { + logger = slog.New(slog.DiscardHandler) + } info, err := os.Lstat(root) if err != nil { err = walkFn(root, nil, err) } else { - err = symwalk(root, info, walkFn) + err = symwalk(root, info, walkFn, logger) } if err == filepath.SkipDir { return nil @@ -63,7 +66,7 @@ func readDirNames(dirname string) ([]string, error) { } // symwalk recursively descends path, calling walkFn. -func symwalk(path string, info os.FileInfo, walkFn filepath.WalkFunc) error { +func symwalk(path string, info os.FileInfo, walkFn filepath.WalkFunc, logger *slog.Logger) error { // Recursively walk symlinked directories. if IsSymlink(info) { resolved, err := filepath.EvalSymlinks(path) @@ -71,11 +74,11 @@ func symwalk(path string, info os.FileInfo, walkFn filepath.WalkFunc) error { return fmt.Errorf("error evaluating symlink %s: %w", path, err) } // This log message is to highlight a symlink that is being used within a chart, symlinks can be used for nefarious reasons. - slog.Info("found symbolic link in path. Contents of linked file included and used", "path", path, "resolved", resolved) + logger.Info("found symbolic link in path. Contents of linked file included and used", "path", path, "resolved", resolved) if info, err = os.Lstat(resolved); err != nil { return err } - if err := symwalk(path, info, walkFn); err != nil && err != filepath.SkipDir { + if err := symwalk(path, info, walkFn, logger); err != nil && err != filepath.SkipDir { return err } return nil @@ -102,7 +105,7 @@ func symwalk(path string, info os.FileInfo, walkFn filepath.WalkFunc) error { return err } } else { - err = symwalk(filename, fileInfo, walkFn) + err = symwalk(filename, fileInfo, walkFn, logger) if err != nil { if (!fileInfo.IsDir() && !IsSymlink(fileInfo)) || err != filepath.SkipDir { return err diff --git a/internal/sympath/walk_test.go b/internal/sympath/walk_test.go index 1eba8b996..d1fd5d5ec 100644 --- a/internal/sympath/walk_test.go +++ b/internal/sympath/walk_test.go @@ -136,7 +136,7 @@ func TestWalk(t *testing.T) { return mark(info, err, &errors, true) } // Expect no errors. - err := Walk(tree.name, markFn) + err := Walk(tree.name, markFn, nil) if err != nil { t.Fatalf("no error expected, found: %s", err) } diff --git a/internal/version/version.go b/internal/version/version.go index 007f79f16..7dea10bbe 100644 --- a/internal/version/version.go +++ b/internal/version/version.go @@ -75,8 +75,13 @@ func GetUserAgent() string { return "Helm/" + strings.TrimPrefix(GetVersion(), "v") } -// Get returns build info -func Get() BuildInfo { +// Get returns build info. +// An optional logger may be provided for diagnostic messages. If nil, a discard +// logger is used. +func Get(logger *slog.Logger) BuildInfo { + if logger == nil { + logger = slog.New(slog.DiscardHandler) + } makeKubeClientVersionString := func() string { // Test builds don't include debug info / module info @@ -88,13 +93,13 @@ func Get() BuildInfo { vstr, err := K8sIOClientGoModVersion() if err != nil { - slog.Error("failed to retrieve k8s.io/client-go version", slog.Any("error", err)) + logger.Error("failed to retrieve k8s.io/client-go version", slog.Any("error", err)) return "" } v, err := semver.NewVersion(vstr) if err != nil { - slog.Error("unable to parse k8s.io/client-go version", slog.String("version", vstr), slog.Any("error", err)) + logger.Error("unable to parse k8s.io/client-go version", slog.String("version", vstr), slog.Any("error", err)) return "" } diff --git a/pkg/action/action.go b/pkg/action/action.go index 8c1888144..835860132 100644 --- a/pkg/action/action.go +++ b/pkg/action/action.go @@ -157,7 +157,6 @@ func ConfigurationSetLogger(h slog.Handler) ConfigurationOption { func NewConfiguration(options ...ConfigurationOption) *Configuration { c := &Configuration{} - c.SetLogger(slog.Default().Handler()) for _, o := range options { o(c) @@ -461,7 +460,7 @@ func (cfg *Configuration) renderResources(ch *chart.Chart, values common.Values, // Sort hooks, manifests, and partials. Only hooks and manifests are returned, // as partials are not used after renderer.Render. Empty manifests are also // removed here. - hs, manifests, err := releaseutil.SortManifests(files, nil, releaseutil.InstallOrder) + hs, manifests, err := releaseutil.SortManifests(files, nil, releaseutil.InstallOrder, nil) if err != nil { // By catching parse errors here, we can prevent bogus releases from going // to Kubernetes. diff --git a/pkg/action/get_metadata.go b/pkg/action/get_metadata.go index 5312dac7f..95ddf4df1 100644 --- a/pkg/action/get_metadata.go +++ b/pkg/action/get_metadata.go @@ -52,6 +52,10 @@ type Metadata struct { Status string `json:"status" yaml:"status"` DeployedAt string `json:"deployedAt" yaml:"deployedAt"` ApplyMethod string `json:"applyMethod,omitempty" yaml:"applyMethod,omitempty"` + + // logger is the structured logger for this metadata instance. + // It is not serialized. + logger *slog.Logger `json:"-" yaml:"-"` } // NewGetMetadata creates a new GetMetadata object with the given configuration. @@ -106,16 +110,21 @@ func (g *GetMetadata) Run(name string) (*Metadata, error) { Status: rac.Status(), DeployedAt: rac.DeployedAt().Format(time.RFC3339), ApplyMethod: rac.ApplyMethod(), + logger: g.cfg.Logger(), }, nil } // FormattedDepNames formats metadata.dependencies names into a comma-separated list. func (m *Metadata) FormattedDepNames() string { + logger := m.logger + if logger == nil { + logger = slog.New(slog.DiscardHandler) + } depsNames := make([]string, 0, len(m.Dependencies)) for _, dep := range m.Dependencies { ac, err := ci.NewDependencyAccessor(dep) if err != nil { - slog.Error("unable to access dependency metadata", "error", err) + logger.Error("unable to access dependency metadata", "error", err) continue } depsNames = append(depsNames, ac.Name()) diff --git a/pkg/action/uninstall.go b/pkg/action/uninstall.go index 79156991c..39c186a40 100644 --- a/pkg/action/uninstall.go +++ b/pkg/action/uninstall.go @@ -258,7 +258,7 @@ func (u *Uninstall) deleteRelease(rel *release.Release) (kube.ResourceList, stri var errs []error manifests := releaseutil.SplitManifests(rel.Manifest) - _, files, err := releaseutil.SortManifests(manifests, nil, releaseutil.UninstallOrder) + _, files, err := releaseutil.SortManifests(manifests, nil, releaseutil.UninstallOrder, nil) if err != nil { // We could instead just delete everything in no particular order. // FIXME: One way to delete at this point would be to try a label-based @@ -283,12 +283,12 @@ func (u *Uninstall) deleteRelease(rel *release.Release) (kube.ResourceList, stri return nil, "", []error{fmt.Errorf("unable to build kubernetes objects for delete: %w", err)} } if len(resources) > 0 { - _, errs = u.cfg.KubeClient.Delete(resources, parseCascadingFlag(u.DeletionPropagation)) + _, errs = u.cfg.KubeClient.Delete(resources, u.parseCascadingFlag(u.DeletionPropagation)) } return resources, kept.String(), errs } -func parseCascadingFlag(cascadingFlag string) v1.DeletionPropagation { +func (u *Uninstall) parseCascadingFlag(cascadingFlag string) v1.DeletionPropagation { switch cascadingFlag { case "orphan": return v1.DeletePropagationOrphan @@ -297,7 +297,7 @@ func parseCascadingFlag(cascadingFlag string) v1.DeletionPropagation { case "background": return v1.DeletePropagationBackground default: - slog.Debug("uninstall: given cascade value, defaulting to delete propagation background", "value", cascadingFlag) + u.cfg.Logger().Debug("uninstall: given cascade value, defaulting to delete propagation background", "value", cascadingFlag) return v1.DeletePropagationBackground } } diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 00939ffa6..1806386d0 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -277,7 +277,7 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chartv2.Chart, vals map[str return nil, nil, false, err } - if err := chartutil.ProcessDependencies(chart, vals); err != nil { + if err := chartutil.ProcessDependencies(chart, vals, nil); err != nil { return nil, nil, false, err } diff --git a/pkg/chart/common.go b/pkg/chart/common.go index cec2c7091..496e17bdc 100644 --- a/pkg/chart/common.go +++ b/pkg/chart/common.go @@ -31,20 +31,28 @@ var NewAccessor func(chrt Charter) (Accessor, error) = NewDefaultAccessor //noli func NewDefaultAccessor(chrt Charter) (Accessor, error) { switch v := chrt.(type) { case v2chart.Chart: - return &v2Accessor{&v}, nil + return &v2Accessor{chrt: &v}, nil case *v2chart.Chart: - return &v2Accessor{v}, nil + return &v2Accessor{chrt: v}, nil case v3chart.Chart: - return &v3Accessor{&v}, nil + return &v3Accessor{chrt: &v}, nil case *v3chart.Chart: - return &v3Accessor{v}, nil + return &v3Accessor{chrt: v}, nil default: return nil, errors.New("unsupported chart type") } } type v2Accessor struct { - chrt *v2chart.Chart + chrt *v2chart.Chart + logger *slog.Logger +} + +func (r *v2Accessor) log() *slog.Logger { + if r.logger != nil { + return r.logger + } + return slog.New(slog.DiscardHandler) } func (r *v2Accessor) Name() string { @@ -63,7 +71,7 @@ func (r *v2Accessor) MetadataAsMap() map[string]any { ret, err := structToMap(r.chrt.Metadata) if err != nil { - slog.Error("error converting metadata to map", "error", err) + r.log().Error("error converting metadata to map", "error", err) } return ret } @@ -113,7 +121,15 @@ func (r *v2Accessor) Deprecated() bool { } type v3Accessor struct { - chrt *v3chart.Chart + chrt *v3chart.Chart + logger *slog.Logger +} + +func (r *v3Accessor) log() *slog.Logger { + if r.logger != nil { + return r.logger + } + return slog.New(slog.DiscardHandler) } func (r *v3Accessor) Name() string { @@ -132,7 +148,7 @@ func (r *v3Accessor) MetadataAsMap() map[string]any { ret, err := structToMap(r.chrt.Metadata) if err != nil { - slog.Error("error converting metadata to map", "error", err) + r.log().Error("error converting metadata to map", "error", err) } return ret } diff --git a/pkg/chart/common/capabilities.go b/pkg/chart/common/capabilities.go index 20f4953cf..cf240b88c 100644 --- a/pkg/chart/common/capabilities.go +++ b/pkg/chart/common/capabilities.go @@ -177,6 +177,6 @@ func newCapabilities(kubeVersionMajor, kubeVersionMinor uint64) (*Capabilities, Minor: strconv.FormatUint(kubeVersionMinor, 10), }, APIVersions: DefaultVersionSet, - HelmVersion: helmversion.Get(), + HelmVersion: helmversion.Get(nil), }, nil } diff --git a/pkg/chart/common/util/jsonschema.go b/pkg/chart/common/util/jsonschema.go index 63ca0c274..9fd1ad85b 100644 --- a/pkg/chart/common/util/jsonschema.go +++ b/pkg/chart/common/util/jsonschema.go @@ -74,20 +74,21 @@ func newHTTPURLLoader() *HTTPURLLoader { // ValidateAgainstSchema checks that values does not violate the structure laid out in schema func ValidateAgainstSchema(ch chart.Charter, values map[string]any) error { + logger := slog.Default() chrt, err := chart.NewAccessor(ch) if err != nil { return err } var sb strings.Builder if chrt.Schema() != nil { - slog.Debug("chart name", "chart-name", chrt.Name()) + logger.Debug("chart name", "chart-name", chrt.Name()) err := ValidateAgainstSingleSchema(values, chrt.Schema()) if err != nil { fmt.Fprintf(&sb, "%s:\n", chrt.Name()) sb.WriteString(err.Error()) } } - slog.Debug("number of dependencies in the chart", "chart", chrt.Name(), "dependencies", len(chrt.Dependencies())) + logger.Debug("number of dependencies in the chart", "chart", chrt.Name(), "dependencies", len(chrt.Dependencies())) // For each dependency, recursively call this function with the coalesced values for _, subchart := range chrt.Dependencies() { sub, err := chart.NewAccessor(subchart) @@ -122,6 +123,7 @@ func ValidateAgainstSchema(ch chart.Charter, values map[string]any) error { // ValidateAgainstSingleSchema checks that values does not violate the structure laid out in this schema func ValidateAgainstSingleSchema(values common.Values, schemaJSON []byte) (reterr error) { + logger := slog.Default() defer func() { if r := recover(); r != nil { reterr = fmt.Errorf("unable to validate schema: %s", r) @@ -134,14 +136,14 @@ func ValidateAgainstSingleSchema(values common.Values, schemaJSON []byte) (reter if err != nil { return err } - slog.Debug("unmarshalled JSON schema", "schema", schemaJSON) + logger.Debug("unmarshalled JSON schema", "schema", schemaJSON) // Configure compiler with loaders for different URL schemes loader := jsonschema.SchemeURLLoader{ "file": jsonschema.FileLoader{}, "http": newHTTPURLLoader(), "https": newHTTPURLLoader(), - "urn": urnLoader{}, + "urn": urnLoader{logger: logger}, } compiler := jsonschema.NewCompiler() @@ -178,7 +180,9 @@ var URNResolver URNResolverFunc = func(urn string) (any, error) { // urnLoader implements resolution for the urn: scheme by delegating to // URNResolver. If unresolved, it logs a warning and returns a permissive // boolean-true schema to avoid hard failures (back-compat behavior). -type urnLoader struct{} +type urnLoader struct { + logger *slog.Logger +} // warnedURNs ensures we log the unresolved-URN warning only once per URN. var warnedURNs sync.Map @@ -188,7 +192,7 @@ func (l urnLoader) Load(urlStr string) (any, error) { return doc, nil } if _, loaded := warnedURNs.LoadOrStore(urlStr, struct{}{}); !loaded { - slog.Warn("unresolved URN reference ignored; using permissive schema", "urn", urlStr) + l.logger.Warn("unresolved URN reference ignored; using permissive schema", "urn", urlStr) } return jsonschema.UnmarshalJSON(strings.NewReader("true")) } diff --git a/pkg/chart/common/util/jsonschema_test.go b/pkg/chart/common/util/jsonschema_test.go index 838d152a1..87f7c98fa 100644 --- a/pkg/chart/common/util/jsonschema_test.go +++ b/pkg/chart/common/util/jsonschema_test.go @@ -235,8 +235,13 @@ func TestValidateAgainstSchema2020Negative(t *testing.T) { } var errString string +<<<<<<< HEAD if err := ValidateAgainstSchema(chrt, vals); err == nil { t.Fatal("Expected an error, but got nil") +======= + if err := ValidateAgainstSchema(chrt, vals, nil); err == nil { + t.Fatalf("Expected an error, but got nil") +>>>>>>> 6f8673662 (fix(logging): replace global slog usage with dependency-injected loggers) } else { errString = err.Error() } @@ -295,7 +300,7 @@ func TestValidateAgainstSingleSchema_UnresolvedURN_Ignored(t *testing.T) { "$ref": "urn:example:helm:schemas:v1:helm-schema-validation-conditions:v1/helmSchemaValidation-true" }`) vals := map[string]any{"any": "value"} - if err := ValidateAgainstSingleSchema(vals, schema); err != nil { + if err := ValidateAgainstSingleSchema(vals, schema, nil); err != nil { t.Fatalf("expected no error when URN unresolved is ignored, got: %v", err) } } @@ -328,7 +333,7 @@ func TestValidateAgainstSchema_MissingSubchartValues_NoPanic(t *testing.T) { } }() - if err := ValidateAgainstSchema(chrt, vals); err != nil { + if err := ValidateAgainstSchema(chrt, vals, nil); err != nil { t.Fatalf("expected no error when subchart values are missing, got: %v", err) } } @@ -356,7 +361,7 @@ func TestValidateAgainstSchema_SubchartNil_NoPanic(t *testing.T) { } }() - if err := ValidateAgainstSchema(chrt, vals); err != nil { + if err := ValidateAgainstSchema(chrt, vals, nil); err != nil { t.Fatalf("expected no error when subchart values are nil, got: %v", err) } } @@ -385,7 +390,12 @@ func TestValidateAgainstSchema_InvalidSubchartValuesType_NoPanic(t *testing.T) { }() // We expect a non-nil error (invalid type), but crucially no panic. +<<<<<<< HEAD if err := ValidateAgainstSchema(chrt, vals); err == nil { t.Fatal("expected an error when subchart values have invalid type, got nil") +======= + if err := ValidateAgainstSchema(chrt, vals, nil); err == nil { + t.Fatalf("expected an error when subchart values have invalid type, got nil") +>>>>>>> 6f8673662 (fix(logging): replace global slog usage with dependency-injected loggers) } } diff --git a/pkg/chart/v2/lint/rules/template.go b/pkg/chart/v2/lint/rules/template.go index 94210dec8..bb4d991d2 100644 --- a/pkg/chart/v2/lint/rules/template.go +++ b/pkg/chart/v2/lint/rules/template.go @@ -119,7 +119,7 @@ func (t *templateLinter) Lint() { // lint ignores import-values // See https://github.com/helm/helm/issues/9658 - if err := chartutil.ProcessDependencies(chart, t.values); err != nil { + if err := chartutil.ProcessDependencies(chart, t.values, nil); err != nil { return } diff --git a/pkg/chart/v2/lint/rules/values.go b/pkg/chart/v2/lint/rules/values.go index 2c766068c..0842fbc80 100644 --- a/pkg/chart/v2/lint/rules/values.go +++ b/pkg/chart/v2/lint/rules/values.go @@ -78,7 +78,7 @@ func validateValuesFile(valuesPath string, overrides map[string]any, skipSchemaV } if !skipSchemaValidation { - return util.ValidateAgainstSingleSchema(coalescedValues, schema) + return util.ValidateAgainstSingleSchema(coalescedValues, schema, nil) } return nil diff --git a/pkg/chart/v2/loader/directory.go b/pkg/chart/v2/loader/directory.go index 82578d924..191a6f4c7 100644 --- a/pkg/chart/v2/loader/directory.go +++ b/pkg/chart/v2/loader/directory.go @@ -114,7 +114,7 @@ func LoadDir(dir string) (*chart.Chart, error) { files = append(files, &archive.BufferedFile{Name: n, ModTime: fi.ModTime(), Data: data}) return nil } - if err = sympath.Walk(topdir, walk); err != nil { + if err = sympath.Walk(topdir, walk, nil); err != nil { return c, err } diff --git a/pkg/chart/v2/util/dependencies.go b/pkg/chart/v2/util/dependencies.go index f28a4f4b1..f2740726d 100644 --- a/pkg/chart/v2/util/dependencies.go +++ b/pkg/chart/v2/util/dependencies.go @@ -30,14 +30,15 @@ import ( // ProcessDependencies checks through this chart's dependencies, processing accordingly. func ProcessDependencies(c *chart.Chart, v common.Values) error { - if err := processDependencyEnabled(c, v, ""); err != nil { + logger := slog.Default() + if err := processDependencyEnabled(c, v, "", logger); err != nil { return err } - return processDependencyImportValues(c, true) + return processDependencyImportValues(c, true, logger) } // processDependencyConditions disables charts based on condition path value in values -func processDependencyConditions(reqs []*chart.Dependency, cvals common.Values, cpath string) { +func processDependencyConditions(reqs []*chart.Dependency, cvals common.Values, cpath string, logger *slog.Logger) { if reqs == nil { return } @@ -53,10 +54,10 @@ func processDependencyConditions(reqs []*chart.Dependency, cvals common.Values, r.Enabled = bv break } - slog.Warn("returned non-bool value", "path", c, "chart", r.Name) + logger.Warn("returned non-bool value", "path", c, "chart", r.Name) } else if !errors.As(err, &errNoValue) { // this is a real error - slog.Warn("the method PathValue returned error", slog.Any("error", err)) + logger.Warn("the method PathValue returned error", slog.Any("error", err)) } } } @@ -64,7 +65,7 @@ func processDependencyConditions(reqs []*chart.Dependency, cvals common.Values, } // processDependencyTags disables charts based on tags in values -func processDependencyTags(reqs []*chart.Dependency, cvals common.Values) { +func processDependencyTags(reqs []*chart.Dependency, cvals common.Values, logger *slog.Logger) { if reqs == nil { return } @@ -84,7 +85,7 @@ func processDependencyTags(reqs []*chart.Dependency, cvals common.Values) { hasFalse = true } } else { - slog.Warn("returned non-bool value", "tag", k, "chart", r.Name) + logger.Warn("returned non-bool value", "tag", k, "chart", r.Name) } } } @@ -143,7 +144,7 @@ func copyMetadata(metadata *chart.Metadata) *chart.Metadata { } // processDependencyEnabled removes disabled charts from dependencies -func processDependencyEnabled(c *chart.Chart, v map[string]any, path string) error { +func processDependencyEnabled(c *chart.Chart, v map[string]any, path string, logger *slog.Logger) error { if c.Metadata.Dependencies == nil { return nil } @@ -186,8 +187,8 @@ Loop: return err } // flag dependencies as enabled/disabled - processDependencyTags(c.Metadata.Dependencies, cvals) - processDependencyConditions(c.Metadata.Dependencies, cvals, path) + processDependencyTags(c.Metadata.Dependencies, cvals, logger) + processDependencyConditions(c.Metadata.Dependencies, cvals, path, logger) // make a map of charts to remove rm := map[string]struct{}{} for _, r := range c.Metadata.Dependencies { @@ -216,7 +217,7 @@ Loop: // recursively call self to process sub dependencies for _, t := range cd { subpath := path + t.Metadata.Name + "." - if err := processDependencyEnabled(t, cvals, subpath); err != nil { + if err := processDependencyEnabled(t, cvals, subpath, logger); err != nil { return err } } @@ -250,7 +251,7 @@ func set(path []string, data map[string]any) map[string]any { } // processImportValues merges values from child to parent based on the chart's dependencies' ImportValues field. -func processImportValues(c *chart.Chart, merge bool) error { +func processImportValues(c *chart.Chart, merge bool, logger *slog.Logger) error { if c.Metadata.Dependencies == nil { return nil } @@ -283,7 +284,7 @@ func processImportValues(c *chart.Chart, merge bool) error { // get child table vv, err := cvals.Table(r.Name + "." + child) if err != nil { - slog.Warn( + logger.Warn( "ImportValues missing table from chart", slog.String("chart", r.Name), slog.Any("error", err), @@ -304,7 +305,7 @@ func processImportValues(c *chart.Chart, merge bool) error { }) vm, err := cvals.Table(r.Name + "." + child) if err != nil { - slog.Warn("ImportValues missing table", slog.Any("error", err)) + logger.Warn("ImportValues missing table", slog.Any("error", err)) continue } if merge { @@ -372,12 +373,12 @@ func istable(v any) bool { } // processDependencyImportValues imports specified chart values from child to parent. -func processDependencyImportValues(c *chart.Chart, merge bool) error { +func processDependencyImportValues(c *chart.Chart, merge bool, logger *slog.Logger) error { for _, d := range c.Dependencies() { // recurse - if err := processDependencyImportValues(d, merge); err != nil { + if err := processDependencyImportValues(d, merge, logger); err != nil { return err } } - return processImportValues(c, merge) + return processImportValues(c, merge, logger) } diff --git a/pkg/chart/v2/util/dependencies_test.go b/pkg/chart/v2/util/dependencies_test.go index 0e4df8528..3c6f8e761 100644 --- a/pkg/chart/v2/util/dependencies_test.go +++ b/pkg/chart/v2/util/dependencies_test.go @@ -15,6 +15,7 @@ limitations under the License. package util import ( + "log/slog" "os" "path/filepath" "sort" @@ -117,7 +118,7 @@ func TestDependencyEnabled(t *testing.T) { for _, tc := range tests { c := loadChart(t, "testdata/subpop") t.Run(tc.name, func(t *testing.T) { - if err := processDependencyEnabled(c, tc.v, ""); err != nil { + if err := processDependencyEnabled(c, tc.v, "", slog.New(slog.DiscardHandler)); err != nil { t.Fatalf("error processing enabled dependencies %v", err) } @@ -219,7 +220,7 @@ func TestProcessDependencyImportValues(t *testing.T) { e["SCBexported2A"] = "blaster" e["global.SC1exported2.all.SC1exported3"] = "SC1expstr" - if err := processDependencyImportValues(c, false); err != nil { + if err := processDependencyImportValues(c, false, slog.New(slog.DiscardHandler)); err != nil { t.Fatalf("processing import values dependencies %v", err) } cc := common.Values(c.Values) @@ -259,7 +260,7 @@ func TestProcessDependencyImportValues(t *testing.T) { } c = loadChart(t, "testdata/subpop") - if err := processDependencyImportValues(c, true); err != nil { + if err := processDependencyImportValues(c, true, slog.New(slog.DiscardHandler)); err != nil { t.Fatalf("processing import values dependencies %v", err) } cc = common.Values(c.Values) @@ -275,10 +276,10 @@ func TestProcessDependencyImportValues(t *testing.T) { func TestProcessDependencyImportValuesFromSharedDependencyToAliases(t *testing.T) { c := loadChart(t, "testdata/chart-with-import-from-aliased-dependencies") - if err := processDependencyEnabled(c, c.Values, ""); err != nil { + if err := processDependencyEnabled(c, c.Values, "", slog.New(slog.DiscardHandler)); err != nil { t.Fatalf("expected no errors but got %q", err) } - if err := processDependencyImportValues(c, true); err != nil { + if err := processDependencyImportValues(c, true, slog.New(slog.DiscardHandler)); err != nil { t.Fatalf("processing import values dependencies %v", err) } e := make(map[string]string) @@ -327,7 +328,7 @@ func TestProcessDependencyImportValuesMultiLevelPrecedence(t *testing.T) { e["app2.service.port"] = "8080" e["app3.service.port"] = "9090" e["app4.service.port"] = "1234" - if err := processDependencyImportValues(c, true); err != nil { + if err := processDependencyImportValues(c, true, slog.New(slog.DiscardHandler)); err != nil { t.Fatalf("processing import values dependencies %v", err) } cc := common.Values(c.Values) @@ -354,7 +355,7 @@ func TestProcessDependencyImportValuesForEnabledCharts(t *testing.T) { c := loadChart(t, "testdata/import-values-from-enabled-subchart/parent-chart") nameOverride := "parent-chart-prod" - if err := processDependencyImportValues(c, true); err != nil { + if err := processDependencyImportValues(c, true, slog.New(slog.DiscardHandler)); err != nil { t.Fatalf("processing import values dependencies %v", err) } @@ -362,7 +363,7 @@ func TestProcessDependencyImportValuesForEnabledCharts(t *testing.T) { t.Fatalf("expected 2 dependencies for this chart, but got %d", len(c.Dependencies())) } - if err := processDependencyEnabled(c, c.Values, ""); err != nil { + if err := processDependencyEnabled(c, c.Values, "", slog.New(slog.DiscardHandler)); err != nil { t.Fatalf("expected no errors but got %q", err) } @@ -427,7 +428,7 @@ func TestDependentChartAliases(t *testing.T) { t.Fatalf("expected 2 dependencies for this chart, but got %d", len(c.Dependencies())) } - if err := processDependencyEnabled(c, c.Values, ""); err != nil { + if err := processDependencyEnabled(c, c.Values, "", slog.New(slog.DiscardHandler)); err != nil { t.Fatalf("expected no errors but got %q", err) } @@ -469,7 +470,7 @@ func TestDependentChartWithSubChartsAbsentInDependency(t *testing.T) { t.Fatalf("expected 2 dependencies for this chart, but got %d", len(c.Dependencies())) } - if err := processDependencyEnabled(c, c.Values, ""); err != nil { + if err := processDependencyEnabled(c, c.Values, "", slog.New(slog.DiscardHandler)); err != nil { t.Fatalf("expected no errors but got %q", err) } @@ -506,7 +507,7 @@ func TestDependentChartsWithSubchartsAllSpecifiedInDependency(t *testing.T) { t.Fatalf("expected 2 dependencies for this chart, but got %d", len(c.Dependencies())) } - if err := processDependencyEnabled(c, c.Values, ""); err != nil { + if err := processDependencyEnabled(c, c.Values, "", slog.New(slog.DiscardHandler)); err != nil { t.Fatalf("expected no errors but got %q", err) } @@ -526,7 +527,7 @@ func TestDependentChartsWithSomeSubchartsSpecifiedInDependency(t *testing.T) { t.Fatalf("expected 2 dependencies for this chart, but got %d", len(c.Dependencies())) } - if err := processDependencyEnabled(c, c.Values, ""); err != nil { + if err := processDependencyEnabled(c, c.Values, "", slog.New(slog.DiscardHandler)); err != nil { t.Fatalf("expected no errors but got %q", err) } @@ -559,7 +560,7 @@ func TestChartWithDependencyAliasedTwiceAndDoublyReferencedSubDependency(t *test t.Fatalf("expected one dependency for this chart, but got %d", len(c.Dependencies())) } - if err := processDependencyEnabled(c, c.Values, ""); err != nil { + if err := processDependencyEnabled(c, c.Values, "", slog.New(slog.DiscardHandler)); err != nil { t.Fatalf("expected no errors but got %q", err) } diff --git a/pkg/cmd/flags.go b/pkg/cmd/flags.go index 5a220d1ce..45d1137de 100644 --- a/pkg/cmd/flags.go +++ b/pkg/cmd/flags.go @@ -56,41 +56,44 @@ func addValueOptionsFlags(f *pflag.FlagSet, v *values.Options) { f.StringArrayVar(&v.LiteralValues, "set-literal", []string{}, "set a literal STRING value on the command line") } -func AddWaitFlag(cmd *cobra.Command, wait *kube.WaitStrategy) { +func AddWaitFlag(cmd *cobra.Command, wait *kube.WaitStrategy, logger *slog.Logger) { cmd.Flags().Var( - newWaitValue(kube.HookOnlyStrategy, wait), + newWaitValue(kube.HookOnlyStrategy, wait, logger), "wait", "wait until resources are ready (up to --timeout). Use '--wait' alone for 'watcher' strategy, or specify one of: 'watcher', 'hookOnly', 'legacy'. Default when flag is omitted: 'hookOnly'.", ) cmd.Flags().Lookup("wait").NoOptDefVal = string(kube.StatusWatcherStrategy) } -type waitValue kube.WaitStrategy +type waitValue struct { + ws *kube.WaitStrategy + logger *slog.Logger +} -func newWaitValue(defaultValue kube.WaitStrategy, ws *kube.WaitStrategy) *waitValue { +func newWaitValue(defaultValue kube.WaitStrategy, ws *kube.WaitStrategy, logger *slog.Logger) *waitValue { *ws = defaultValue - return (*waitValue)(ws) + return &waitValue{ws: ws, logger: logger} } func (ws *waitValue) String() string { - if ws == nil { + if ws == nil || ws.ws == nil { return "" } - return string(*ws) + return string(*ws.ws) } func (ws *waitValue) Set(s string) error { switch s { case string(kube.StatusWatcherStrategy), string(kube.LegacyStrategy), string(kube.HookOnlyStrategy): - *ws = waitValue(s) + *ws.ws = kube.WaitStrategy(s) return nil case "true": - slog.Warn("--wait=true is deprecated (boolean value) and can be replaced with --wait=watcher") - *ws = waitValue(kube.StatusWatcherStrategy) + ws.logger.Warn("--wait=true is deprecated (boolean value) and can be replaced with --wait=watcher") + *ws.ws = kube.StatusWatcherStrategy return nil case "false": - slog.Warn("--wait=false is deprecated (boolean value) and can be replaced with --wait=hookOnly") - *ws = waitValue(kube.HookOnlyStrategy) + ws.logger.Warn("--wait=false is deprecated (boolean value) and can be replaced with --wait=hookOnly") + *ws.ws = kube.HookOnlyStrategy return nil default: return fmt.Errorf("invalid wait input %q. Valid inputs are %s, %s, and %s", s, kube.StatusWatcherStrategy, kube.HookOnlyStrategy, kube.LegacyStrategy) diff --git a/pkg/cmd/helpers.go b/pkg/cmd/helpers.go index e555dd18b..121ac729d 100644 --- a/pkg/cmd/helpers.go +++ b/pkg/cmd/helpers.go @@ -42,14 +42,14 @@ func addDryRunFlag(cmd *cobra.Command) { // Determine the `action.DryRunStrategy` given -dry-run=` flag (or absence of) // Legacy usage of the flag: boolean values, and `--dry-run` (without value) are supported, and log warnings emitted -func cmdGetDryRunFlagStrategy(cmd *cobra.Command, isTemplate bool) (action.DryRunStrategy, error) { +func cmdGetDryRunFlagStrategy(cmd *cobra.Command, isTemplate bool, logger *slog.Logger) (action.DryRunStrategy, error) { f := cmd.Flag("dry-run") v := f.Value.String() switch v { case f.NoOptDefVal: - slog.Warn(`--dry-run is deprecated and should be replaced with '--dry-run=client'`) + logger.Warn(`--dry-run is deprecated and should be replaced with '--dry-run=client'`) return action.DryRunClient, nil case string(action.DryRunClient): return action.DryRunClient, nil @@ -77,7 +77,7 @@ func cmdGetDryRunFlagStrategy(cmd *cobra.Command, isTemplate bool) (action.DryRu if b { result = action.DryRunClient } - slog.Warn(fmt.Sprintf(`boolean '--dry-run=%v' flag is deprecated and must be replaced with '--dry-run=%s'`, v, result)) + logger.Warn(fmt.Sprintf(`boolean '--dry-run=%v' flag is deprecated and must be replaced with '--dry-run=%s'`, v, result)) return result, nil } diff --git a/pkg/cmd/helpers_test.go b/pkg/cmd/helpers_test.go index 08065499e..e8ad7c6ed 100644 --- a/pkg/cmd/helpers_test.go +++ b/pkg/cmd/helpers_test.go @@ -286,7 +286,7 @@ func TestCmdGetDryRunFlagStrategy(t *testing.T) { cmd.Flags().Parse([]string{"helm", tc.DryRunFlagArg}) t.Run(name, func(t *testing.T) { - dryRunStrategy, err := cmdGetDryRunFlagStrategy(cmd, tc.IsTemplate) + dryRunStrategy, err := cmdGetDryRunFlagStrategy(cmd, tc.IsTemplate, logger) if tc.ExpectedError { assert.Error(t, err) } else { diff --git a/pkg/cmd/install.go b/pkg/cmd/install.go index 67e2a9fab..acbda344f 100644 --- a/pkg/cmd/install.go +++ b/pkg/cmd/install.go @@ -150,13 +150,13 @@ func newInstallCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { } client.SetRegistryClient(registryClient) - dryRunStrategy, err := cmdGetDryRunFlagStrategy(cmd, false) + dryRunStrategy, err := cmdGetDryRunFlagStrategy(cmd, false, cfg.Logger()) if err != nil { return err } client.DryRunStrategy = dryRunStrategy - rel, err := runInstall(args, client, valueOpts, out) + rel, err := runInstall(args, client, valueOpts, out, cfg.Logger()) if err != nil { return fmt.Errorf("INSTALLATION FAILED: %w", err) } @@ -172,7 +172,7 @@ func newInstallCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { } f := cmd.Flags() - addInstallFlags(cmd, f, client, valueOpts) + addInstallFlags(cmd, f, client, valueOpts, cfg.Logger()) // hide-secret is not available in all places the install flags are used so // it is added separately f.BoolVar(&client.HideSecret, "hide-secret", false, "hide Kubernetes Secrets when also using the --dry-run flag") @@ -183,7 +183,7 @@ func newInstallCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { return cmd } -func addInstallFlags(cmd *cobra.Command, f *pflag.FlagSet, client *action.Install, valueOpts *values.Options) { +func addInstallFlags(cmd *cobra.Command, f *pflag.FlagSet, client *action.Install, valueOpts *values.Options, logger *slog.Logger) { f.BoolVar(&client.CreateNamespace, "create-namespace", false, "create the release namespace if not present") f.BoolVar(&client.ForceReplace, "force-replace", false, "force resource updates by replacement") f.BoolVar(&client.ForceReplace, "force", false, "deprecated") @@ -232,7 +232,7 @@ func addInstallFlags(cmd *cobra.Command, f *pflag.FlagSet, client *action.Instal addValueOptionsFlags(f, valueOpts) addChartPathOptionsFlags(f, &client.ChartPathOptions) - AddWaitFlag(cmd, &client.WaitStrategy) + AddWaitFlag(cmd, &client.WaitStrategy, logger) cmd.MarkFlagsMutuallyExclusive("force-replace", "force-conflicts") cmd.MarkFlagsMutuallyExclusive("force", "force-conflicts") @@ -251,10 +251,10 @@ func addInstallFlags(cmd *cobra.Command, f *pflag.FlagSet, client *action.Instal } } -func runInstall(args []string, client *action.Install, valueOpts *values.Options, out io.Writer) (*release.Release, error) { - slog.Debug("Original chart version", "version", client.Version) +func runInstall(args []string, client *action.Install, valueOpts *values.Options, out io.Writer, logger *slog.Logger) (*release.Release, error) { + logger.Debug("Original chart version", "version", client.Version) if client.Version == "" && client.Devel { - slog.Debug("setting version to >0.0.0-0") + logger.Debug("setting version to >0.0.0-0") client.Version = ">0.0.0-0" } @@ -269,7 +269,7 @@ func runInstall(args []string, client *action.Install, valueOpts *values.Options return nil, err } - slog.Debug("Chart path", "path", cp) + logger.Debug("Chart path", "path", cp) p := getter.All(settings) vals, err := valueOpts.MergeValues(p) @@ -293,7 +293,7 @@ func runInstall(args []string, client *action.Install, valueOpts *values.Options } if ac.Deprecated() { - slog.Warn("this chart is deprecated") + logger.Warn("this chart is deprecated") } if req := ac.MetaDependencies(); len(req) > 0 { diff --git a/pkg/cmd/load_plugins.go b/pkg/cmd/load_plugins.go index 029dd04f5..065f819c0 100644 --- a/pkg/cmd/load_plugins.go +++ b/pkg/cmd/load_plugins.go @@ -52,7 +52,7 @@ const ( // This follows a different pattern than the other commands because it has // to inspect its environment and then add commands to the base command // as it finds them. -func loadCLIPlugins(baseCmd *cobra.Command, out io.Writer) { +func loadCLIPlugins(baseCmd *cobra.Command, out io.Writer, logger *slog.Logger) { // If HELM_NO_PLUGINS is set to 1, do not load plugins. if os.Getenv("HELM_NO_PLUGINS") == "1" { return @@ -64,7 +64,7 @@ func loadCLIPlugins(baseCmd *cobra.Command, out io.Writer) { } found, err := plugin.FindPlugins(dirs, descriptor) if err != nil { - slog.Error("failed to load plugins", slog.String("error", err.Error())) + logger.Error("failed to load plugins", slog.String("error", err.Error())) return } @@ -136,7 +136,7 @@ func loadCLIPlugins(baseCmd *cobra.Command, out io.Writer) { for _, cmd := range baseCmd.Commands() { if cmd.Name() == c.Name() { - slog.Error("failed to load plugins: name conflicts", slog.String("name", c.Name())) + logger.Error("failed to load plugins: name conflicts", slog.String("name", c.Name())) return } } @@ -151,7 +151,7 @@ func loadCLIPlugins(baseCmd *cobra.Command, out io.Writer) { if (err == nil && ((subCmd.HasParent() && subCmd.Parent().Name() == "completion") || subCmd.Name() == cobra.ShellCompRequestCmd)) || /* for the tests */ subCmd == baseCmd.Root() { - loadCompletionForPlugin(c, plug) + loadCompletionForPlugin(c, plug, logger) } } } @@ -218,14 +218,14 @@ type pluginCommand struct { // loadCompletionForPlugin will load and parse any completion.yaml provided by the plugin // and add the dynamic completion hook to call the optional plugin.complete -func loadCompletionForPlugin(pluginCmd *cobra.Command, plug plugin.Plugin) { +func loadCompletionForPlugin(pluginCmd *cobra.Command, plug plugin.Plugin, logger *slog.Logger) { // Parse the yaml file providing the plugin's sub-commands and flags cmds, err := loadFile(strings.Join( []string{plug.Dir(), pluginStaticCompletionFile}, string(filepath.Separator))) if err != nil { // The file could be missing or invalid. No static completion for this plugin. - slog.Debug("plugin completion file loading", slog.String("error", err.Error())) + logger.Debug("plugin completion file loading", slog.String("error", err.Error())) // Continue to setup dynamic completion. cmds = &pluginCommand{} } @@ -233,18 +233,18 @@ func loadCompletionForPlugin(pluginCmd *cobra.Command, plug plugin.Plugin) { // Preserve the Usage string specified for the plugin cmds.Name = pluginCmd.Use - addPluginCommands(plug, pluginCmd, cmds) + addPluginCommands(plug, pluginCmd, cmds, logger) } // addPluginCommands is a recursive method that adds each different level // of sub-commands and flags for the plugins that have provided such information -func addPluginCommands(plug plugin.Plugin, baseCmd *cobra.Command, cmds *pluginCommand) { +func addPluginCommands(plug plugin.Plugin, baseCmd *cobra.Command, cmds *pluginCommand, logger *slog.Logger) { if cmds == nil { return } if len(cmds.Name) == 0 { - slog.Debug("sub-command name field missing", slog.String("commandPath", baseCmd.CommandPath())) + logger.Debug("sub-command name field missing", slog.String("commandPath", baseCmd.CommandPath())) return } @@ -313,7 +313,7 @@ func addPluginCommands(plug plugin.Plugin, baseCmd *cobra.Command, cmds *pluginC Run: func(_ *cobra.Command, _ []string) {}, } baseCmd.AddCommand(subCmd) - addPluginCommands(plug, subCmd, &cmd) + addPluginCommands(plug, subCmd, &cmd, logger) } } diff --git a/pkg/cmd/plugin.go b/pkg/cmd/plugin.go index ba904ef5f..5b6207ea3 100644 --- a/pkg/cmd/plugin.go +++ b/pkg/cmd/plugin.go @@ -17,6 +17,7 @@ package cmd import ( "io" + "log/slog" "github.com/spf13/cobra" @@ -27,17 +28,17 @@ const pluginHelp = ` Manage client-side Helm plugins. ` -func newPluginCmd(out io.Writer) *cobra.Command { +func newPluginCmd(out io.Writer, logger *slog.Logger) *cobra.Command { cmd := &cobra.Command{ Use: "plugin", Short: "install, list, or uninstall Helm plugins", Long: pluginHelp, } cmd.AddCommand( - newPluginInstallCmd(out), - newPluginListCmd(out), - newPluginUninstallCmd(out), - newPluginUpdateCmd(out), + newPluginInstallCmd(out, logger), + newPluginListCmd(out, logger), + newPluginUninstallCmd(out, logger), + newPluginUpdateCmd(out, logger), newPluginPackageCmd(out), newPluginVerifyCmd(out), ) diff --git a/pkg/cmd/plugin_install.go b/pkg/cmd/plugin_install.go index c248ed818..143930262 100644 --- a/pkg/cmd/plugin_install.go +++ b/pkg/cmd/plugin_install.go @@ -45,6 +45,7 @@ type pluginInstallOptions struct { plainHTTP bool password string username string + logger *slog.Logger } const pluginInstallDesc = ` @@ -58,8 +59,8 @@ treated as "local dev" and do not require signatures. Use --verify=false to explicitly skip signature verification (NOT recommended). ` -func newPluginInstallCmd(out io.Writer) *cobra.Command { - o := &pluginInstallOptions{} +func newPluginInstallCmd(out io.Writer, logger *slog.Logger) *cobra.Command { + o := &pluginInstallOptions{logger: logger} cmd := &cobra.Command{ Use: "install [options] ", Short: "install a Helm plugin", @@ -169,7 +170,7 @@ func (o *pluginInstallOptions) run(out io.Writer) error { fmt.Fprintf(out, "Plugin Hash Verified: %s\n", verifyResult.FileHash) } - slog.Debug("loading plugin", "path", i.Path()) + o.logger.Debug("loading plugin", "path", i.Path()) p, err := plugin.LoadDir(i.Path()) if err != nil { return fmt.Errorf("plugin is installed but unusable: %w", err) diff --git a/pkg/cmd/plugin_list.go b/pkg/cmd/plugin_list.go index 74e969e04..726d63b79 100644 --- a/pkg/cmd/plugin_list.go +++ b/pkg/cmd/plugin_list.go @@ -29,7 +29,7 @@ import ( "helm.sh/helm/v4/internal/plugin/schema" ) -func newPluginListCmd(out io.Writer) *cobra.Command { +func newPluginListCmd(out io.Writer, logger *slog.Logger) *cobra.Command { var pluginType string cmd := &cobra.Command{ Use: "list", @@ -37,7 +37,7 @@ func newPluginListCmd(out io.Writer) *cobra.Command { Short: "list installed Helm plugins", ValidArgsFunction: noMoreArgsCompFunc, RunE: func(_ *cobra.Command, _ []string) error { - slog.Debug("pluginDirs", "directory", settings.PluginsDirectory) + logger.Debug("pluginDirs", "directory", settings.PluginsDirectory) dirs := filepath.SplitList(settings.PluginsDirectory) descriptor := plugin.Descriptor{ Type: pluginType, diff --git a/pkg/cmd/plugin_test.go b/pkg/cmd/plugin_test.go index 0a6435d99..39a63e3a1 100644 --- a/pkg/cmd/plugin_test.go +++ b/pkg/cmd/plugin_test.go @@ -18,6 +18,7 @@ package cmd import ( "bytes" "fmt" + "log/slog" "os" "runtime" "strings" @@ -92,7 +93,7 @@ func TestLoadCLIPlugins(t *testing.T) { out bytes.Buffer cmd cobra.Command ) - loadCLIPlugins(&cmd, &out) + loadCLIPlugins(&cmd, &out, slog.Default()) fullEnvOutput := strings.Join([]string{ "HELM_PLUGIN_NAME=fullenv", @@ -171,7 +172,7 @@ func TestLoadPluginsWithSpace(t *testing.T) { out bytes.Buffer cmd cobra.Command ) - loadCLIPlugins(&cmd, &out) + loadCLIPlugins(&cmd, &out, slog.Default()) envs := strings.Join([]string{ "fullenv", @@ -250,7 +251,7 @@ func TestLoadCLIPluginsForCompletion(t *testing.T) { cmd := &cobra.Command{ Use: "completion", } - loadCLIPlugins(cmd, &out) + loadCLIPlugins(cmd, &out, slog.Default()) tests := []staticCompletionDetails{ {"args", []string{}, []string{}, []staticCompletionDetails{}}, @@ -343,7 +344,7 @@ func TestLoadCLIPlugins_HelmNoPlugins(t *testing.T) { out := bytes.NewBuffer(nil) cmd := &cobra.Command{} - loadCLIPlugins(cmd, out) + loadCLIPlugins(cmd, out, slog.Default()) plugins := cmd.Commands() if len(plugins) != 0 { diff --git a/pkg/cmd/plugin_uninstall.go b/pkg/cmd/plugin_uninstall.go index c75cf6264..db18bf74f 100644 --- a/pkg/cmd/plugin_uninstall.go +++ b/pkg/cmd/plugin_uninstall.go @@ -29,11 +29,12 @@ import ( ) type pluginUninstallOptions struct { - names []string + names []string + logger *slog.Logger } -func newPluginUninstallCmd(out io.Writer) *cobra.Command { - o := &pluginUninstallOptions{} +func newPluginUninstallCmd(out io.Writer, logger *slog.Logger) *cobra.Command { + o := &pluginUninstallOptions{logger: logger} cmd := &cobra.Command{ Use: "uninstall ...", @@ -61,7 +62,7 @@ func (o *pluginUninstallOptions) complete(args []string) error { } func (o *pluginUninstallOptions) run(out io.Writer) error { - slog.Debug("loading installer plugins", "dir", settings.PluginsDirectory) + o.logger.Debug("loading installer plugins", "dir", settings.PluginsDirectory) plugins, err := plugin.LoadAllDir(settings.PluginsDirectory, plugin.LogIgnorePluginLoadErrorFilterFunc) if err != nil { return err @@ -69,7 +70,7 @@ func (o *pluginUninstallOptions) run(out io.Writer) error { var errorPlugins []error for _, name := range o.names { if found := findPlugin(plugins, name); found != nil { - if err := uninstallPlugin(found); err != nil { + if err := uninstallPlugin(found, o.logger); err != nil { errorPlugins = append(errorPlugins, fmt.Errorf("failed to uninstall plugin %s, got error (%v)", name, err)) } else { fmt.Fprintf(out, "Uninstalled plugin: %s\n", name) @@ -84,7 +85,7 @@ func (o *pluginUninstallOptions) run(out io.Writer) error { return nil } -func uninstallPlugin(p plugin.Plugin) error { +func uninstallPlugin(p plugin.Plugin, logger *slog.Logger) error { if err := os.RemoveAll(p.Dir()); err != nil { return err } @@ -102,18 +103,18 @@ func uninstallPlugin(p plugin.Plugin) error { // Remove tarball file tarballPath := filepath.Join(pluginsDir, versionedBasename) if _, err := os.Stat(tarballPath); err == nil { - slog.Debug("removing versioned tarball", "path", tarballPath) + logger.Debug("removing versioned tarball", "path", tarballPath) if err := os.Remove(tarballPath); err != nil { - slog.Debug("failed to remove tarball file", "path", tarballPath, "error", err) + logger.Debug("failed to remove tarball file", "path", tarballPath, "error", err) } } // Remove provenance file provPath := filepath.Join(pluginsDir, versionedBasename+".prov") if _, err := os.Stat(provPath); err == nil { - slog.Debug("removing versioned provenance", "path", provPath) + logger.Debug("removing versioned provenance", "path", provPath) if err := os.Remove(provPath); err != nil { - slog.Debug("failed to remove provenance file", "path", provPath, "error", err) + logger.Debug("failed to remove provenance file", "path", provPath, "error", err) } } } diff --git a/pkg/cmd/plugin_update.go b/pkg/cmd/plugin_update.go index 83ef35107..ffd701224 100644 --- a/pkg/cmd/plugin_update.go +++ b/pkg/cmd/plugin_update.go @@ -29,11 +29,12 @@ import ( ) type pluginUpdateOptions struct { - names []string + names []string + logger *slog.Logger } -func newPluginUpdateCmd(out io.Writer) *cobra.Command { - o := &pluginUpdateOptions{} +func newPluginUpdateCmd(out io.Writer, logger *slog.Logger) *cobra.Command { + o := &pluginUpdateOptions{logger: logger} cmd := &cobra.Command{ Use: "update ...", @@ -61,7 +62,7 @@ func (o *pluginUpdateOptions) complete(args []string) error { } func (o *pluginUpdateOptions) run(out io.Writer) error { - slog.Debug("loading installed plugins", "path", settings.PluginsDirectory) + o.logger.Debug("loading installed plugins", "path", settings.PluginsDirectory) plugins, err := plugin.LoadAllDir(settings.PluginsDirectory, plugin.LogIgnorePluginLoadErrorFilterFunc) if err != nil { return err @@ -70,7 +71,7 @@ func (o *pluginUpdateOptions) run(out io.Writer) error { for _, name := range o.names { if found := findPlugin(plugins, name); found != nil { - if err := updatePlugin(found); err != nil { + if err := updatePlugin(found, o.logger); err != nil { errorPlugins = append(errorPlugins, fmt.Errorf("failed to update plugin %s, got error (%v)", name, err)) } else { fmt.Fprintf(out, "Updated plugin: %s\n", name) @@ -85,7 +86,7 @@ func (o *pluginUpdateOptions) run(out io.Writer) error { return nil } -func updatePlugin(p plugin.Plugin) error { +func updatePlugin(p plugin.Plugin, logger *slog.Logger) error { exactLocation, err := filepath.EvalSymlinks(p.Dir()) if err != nil { return err @@ -103,7 +104,7 @@ func updatePlugin(p plugin.Plugin) error { return err } - slog.Debug("loading plugin", "path", i.Path()) + logger.Debug("loading plugin", "path", i.Path()) updatedPlugin, err := plugin.LoadDir(i.Path()) if err != nil { return err diff --git a/pkg/cmd/pull.go b/pkg/cmd/pull.go index bb7a8d1c0..39edab296 100644 --- a/pkg/cmd/pull.go +++ b/pkg/cmd/pull.go @@ -20,7 +20,6 @@ import ( "fmt" "io" "log" - "log/slog" "github.com/spf13/cobra" @@ -61,7 +60,7 @@ func newPullCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { RunE: func(_ *cobra.Command, args []string) error { client.Settings = settings if client.Version == "" && client.Devel { - slog.Debug("setting version to >0.0.0-0") + cfg.Logger().Debug("setting version to >0.0.0-0") client.Version = ">0.0.0-0" } diff --git a/pkg/cmd/registry_login.go b/pkg/cmd/registry_login.go index 1350fb244..93756ca78 100644 --- a/pkg/cmd/registry_login.go +++ b/pkg/cmd/registry_login.go @@ -63,7 +63,7 @@ func newRegistryLoginCmd(cfg *action.Configuration, out io.Writer) *cobra.Comman RunE: func(_ *cobra.Command, args []string) error { hostname := args[0] - username, password, err := getUsernamePassword(o.username, o.password, o.passwordFromStdinOpt) + username, password, err := getUsernamePassword(o.username, o.password, o.passwordFromStdinOpt, cfg.Logger()) if err != nil { return err } @@ -91,7 +91,7 @@ func newRegistryLoginCmd(cfg *action.Configuration, out io.Writer) *cobra.Comman } // Adapted from https://github.com/oras-project/oras -func getUsernamePassword(usernameOpt string, passwordOpt string, passwordFromStdinOpt bool) (string, string, error) { +func getUsernamePassword(usernameOpt string, passwordOpt string, passwordFromStdinOpt bool, logger *slog.Logger) (string, string, error) { var err error username := usernameOpt password := passwordOpt @@ -127,7 +127,7 @@ func getUsernamePassword(usernameOpt string, passwordOpt string, passwordFromStd } } } else { - slog.Warn("using --password via the CLI is insecure. Use --password-stdin") + logger.Warn("using --password via the CLI is insecure. Use --password-stdin") } return username, password, nil diff --git a/pkg/cmd/rollback.go b/pkg/cmd/rollback.go index 01d8b1866..979b6ec39 100644 --- a/pkg/cmd/rollback.go +++ b/pkg/cmd/rollback.go @@ -66,7 +66,7 @@ func newRollbackCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { client.Version = ver } - dryRunStrategy, err := cmdGetDryRunFlagStrategy(cmd, false) + dryRunStrategy, err := cmdGetDryRunFlagStrategy(cmd, false, cfg.Logger()) if err != nil { return err } @@ -93,7 +93,7 @@ func newRollbackCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { f.BoolVar(&client.CleanupOnFail, "cleanup-on-fail", false, "allow deletion of new resources created in this rollback when rollback fails") f.IntVar(&client.MaxHistory, "history-max", settings.MaxHistory, "limit the maximum number of revisions saved per release. Use 0 for no limit") addDryRunFlag(cmd) - AddWaitFlag(cmd, &client.WaitStrategy) + AddWaitFlag(cmd, &client.WaitStrategy, cfg.Logger()) cmd.MarkFlagsMutuallyExclusive("force-replace", "force-conflicts") cmd.MarkFlagsMutuallyExclusive("force", "force-conflicts") diff --git a/pkg/cmd/root.go b/pkg/cmd/root.go index 04ba91c1f..808065fff 100644 --- a/pkg/cmd/root.go +++ b/pkg/cmd/root.go @@ -102,7 +102,7 @@ By default, the default directories depend on the Operating System. The defaults var settings = cli.New() -func NewRootCmd(out io.Writer, args []string, logSetup func(bool)) (*cobra.Command, error) { +func NewRootCmd(out io.Writer, args []string, logSetup func(bool) *slog.Logger) (*cobra.Command, error) { actionConfig := action.NewConfiguration() cmd, err := newRootCmdWithConfig(actionConfig, out, args, logSetup) if err != nil { @@ -121,16 +121,15 @@ func NewRootCmd(out io.Writer, args []string, logSetup func(bool)) (*cobra.Comma return cmd, nil } -// SetupLogging sets up Helm logging used by the Helm client. +// SetupLogging creates the Helm logger used by the Helm client. // This function is passed to the NewRootCmd function to enable logging. Any other // application that uses the NewRootCmd function to setup all the Helm commands may // use this function to setup logging or their own. Using a custom logging setup function // enables applications using Helm commands to integrate with their existing logging // system. // The debug argument is the value if Helm is set for debugging (i.e. --debug flag) -func SetupLogging(debug bool) { - logger := logging.NewLogger(func() bool { return debug }) - slog.SetDefault(logger) +func SetupLogging(debug bool) *slog.Logger { + return logging.NewLogger(func() bool { return debug }) } // configureColorOutput configures the color output based on the ColorMode setting @@ -147,7 +146,7 @@ func configureColorOutput(settings *cli.EnvSettings) { } } -func newRootCmdWithConfig(actionConfig *action.Configuration, out io.Writer, args []string, logSetup func(bool)) (*cobra.Command, error) { +func newRootCmdWithConfig(actionConfig *action.Configuration, out io.Writer, args []string, logSetup func(bool) *slog.Logger) (*cobra.Command, error) { cmd := &cobra.Command{ Use: "helm", Short: "The Helm package manager for Kubernetes.", @@ -177,17 +176,8 @@ func newRootCmdWithConfig(actionConfig *action.Configuration, out io.Writer, arg flags.ParseErrorsAllowlist.UnknownFlags = true flags.Parse(args) - logSetup(settings.Debug) - - // newRootCmdWithConfig is only called from NewRootCmd. NewRootCmd sets up - // NewConfiguration without a custom logger. So, the slog default is used. logSetup - // can change the default logger to the one in the logger package. This happens for - // the Helm client. This means the actionConfig logger is different from the slog - // default logger. If they are different we sync the actionConfig logger to the slog - // current default one. - if actionConfig.Logger() != slog.Default() { - actionConfig.SetLogger(slog.Default().Handler()) - } + logger := logSetup(settings.Debug) + actionConfig.SetLogger(logger.Handler()) // Validate color mode setting switch settings.ColorMode { @@ -273,7 +263,7 @@ func newRootCmdWithConfig(actionConfig *action.Configuration, out io.Writer, arg newLintCmd(out), newPackageCmd(out), newRepoCmd(out), - newSearchCmd(out), + newSearchCmd(out, logger), newVerifyCmd(out), // release commands @@ -290,7 +280,7 @@ func newRootCmdWithConfig(actionConfig *action.Configuration, out io.Writer, arg newCompletionCmd(out), newEnvCmd(out), - newPluginCmd(out), + newPluginCmd(out, logger), newVersionCmd(out), // Hidden documentation generator command: 'helm docs' @@ -303,7 +293,7 @@ func newRootCmdWithConfig(actionConfig *action.Configuration, out io.Writer, arg ) // Find and add CLI plugins - loadCLIPlugins(cmd, out) + loadCLIPlugins(cmd, out, logger) // Check for expired repositories checkForExpiredRepos(settings.RepositoryConfig) diff --git a/pkg/cmd/root_test.go b/pkg/cmd/root_test.go index 316e6bd2e..bd6bb7598 100644 --- a/pkg/cmd/root_test.go +++ b/pkg/cmd/root_test.go @@ -142,10 +142,37 @@ func TestRootCmdLogger(t *testing.T) { t.Errorf("expected no error, got: '%v'", err) } - l1 := actionConfig.Logger() - l2 := slog.Default() + l := actionConfig.Logger() - if l1.Handler() != l2.Handler() { - t.Error("expected actionConfig logger to be the slog default logger") + // The actionConfig logger should be set (not the discard handler) + if l.Handler() == slog.New(slog.DiscardHandler).Handler() { + t.Error("expected actionConfig logger to be set, got discard handler") + } + + // The actionConfig logger should NOT be the global default (we no longer set slog.SetDefault) + if l.Handler() == slog.Default().Handler() { + t.Error("expected actionConfig logger to NOT be the slog default logger") + } +} + +func TestRootCmdCustomLogger(t *testing.T) { + args := []string{} + buf := new(bytes.Buffer) + actionConfig := action.NewConfiguration() + + // SDK users should be able to inject a custom logger + customHandler := slog.NewTextHandler(buf, &slog.HandlerOptions{Level: slog.LevelDebug}) + customLogSetup := func(_ bool) *slog.Logger { + return slog.New(customHandler) + } + + _, err := newRootCmdWithConfig(actionConfig, buf, args, customLogSetup) + if err != nil { + t.Errorf("expected no error, got: '%v'", err) + } + + // Verify the custom handler was injected into actionConfig + if actionConfig.Logger().Handler() != customHandler { + t.Error("expected actionConfig logger to use the custom handler") } } diff --git a/pkg/cmd/search.go b/pkg/cmd/search.go index 4d110286d..3a5aea26f 100644 --- a/pkg/cmd/search.go +++ b/pkg/cmd/search.go @@ -18,6 +18,7 @@ package cmd import ( "io" + "log/slog" "github.com/spf13/cobra" ) @@ -28,7 +29,7 @@ they can be stored including the Artifact Hub and repositories you have added. Use search subcommands to search different locations for charts. ` -func newSearchCmd(out io.Writer) *cobra.Command { +func newSearchCmd(out io.Writer, logger *slog.Logger) *cobra.Command { cmd := &cobra.Command{ Use: "search [keyword]", @@ -36,8 +37,8 @@ func newSearchCmd(out io.Writer) *cobra.Command { Long: searchDesc, } - cmd.AddCommand(newSearchHubCmd(out)) - cmd.AddCommand(newSearchRepoCmd(out)) + cmd.AddCommand(newSearchHubCmd(out, logger)) + cmd.AddCommand(newSearchRepoCmd(out, logger)) return cmd } diff --git a/pkg/cmd/search_hub.go b/pkg/cmd/search_hub.go index f9adb73f4..d9894cfb2 100644 --- a/pkg/cmd/search_hub.go +++ b/pkg/cmd/search_hub.go @@ -56,10 +56,11 @@ type searchHubOptions struct { outputFormat output.Format listRepoURL bool failOnNoResult bool + logger *slog.Logger } -func newSearchHubCmd(out io.Writer) *cobra.Command { - o := &searchHubOptions{} +func newSearchHubCmd(out io.Writer, logger *slog.Logger) *cobra.Command { + o := &searchHubOptions{logger: logger} cmd := &cobra.Command{ Use: "hub [KEYWORD]", @@ -90,7 +91,7 @@ func (o *searchHubOptions) run(out io.Writer, args []string) error { q := strings.Join(args, " ") results, err := c.Search(q) if err != nil { - slog.Debug("search failed", slog.Any("error", err)) + o.logger.Debug("search failed", slog.Any("error", err)) return fmt.Errorf("unable to perform search against %q", o.searchEndpoint) } diff --git a/pkg/cmd/search_repo.go b/pkg/cmd/search_repo.go index 53626f1b6..08e88e714 100644 --- a/pkg/cmd/search_repo.go +++ b/pkg/cmd/search_repo.go @@ -73,10 +73,11 @@ type searchRepoOptions struct { repoCacheDir string outputFormat output.Format failOnNoResult bool + logger *slog.Logger } -func newSearchRepoCmd(out io.Writer) *cobra.Command { - o := &searchRepoOptions{} +func newSearchRepoCmd(out io.Writer, logger *slog.Logger) *cobra.Command { + o := &searchRepoOptions{logger: logger} cmd := &cobra.Command{ Use: "repo [keyword]", @@ -131,17 +132,17 @@ func (o *searchRepoOptions) run(out io.Writer, args []string) error { } func (o *searchRepoOptions) setupSearchedVersion() { - slog.Debug("original chart version", "version", o.version) + o.logger.Debug("original chart version", "version", o.version) if o.version != "" { return } if o.devel { // search for releases and prereleases (alpha, beta, and release candidate releases). - slog.Debug("setting version to >0.0.0-0") + o.logger.Debug("setting version to >0.0.0-0") o.version = ">0.0.0-0" } else { // search only for stable releases, prerelease versions will be skipped - slog.Debug("setting version to >0.0.0") + o.logger.Debug("setting version to >0.0.0") o.version = ">0.0.0" } } @@ -190,7 +191,7 @@ func (o *searchRepoOptions) buildIndex() (*search.Index, error) { f := filepath.Join(o.repoCacheDir, helmpath.CacheIndexFile(n)) ind, err := repo.LoadIndexFile(f) if err != nil { - slog.Warn("repo is corrupt or missing", slog.String("repo", n), slog.Any("error", err)) + o.logger.Warn("repo is corrupt or missing", slog.String("repo", n), slog.Any("error", err)) continue } diff --git a/pkg/cmd/show.go b/pkg/cmd/show.go index d7249c3fe..723c7fcca 100644 --- a/pkg/cmd/show.go +++ b/pkg/cmd/show.go @@ -88,7 +88,7 @@ func newShowCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { if err != nil { return err } - output, err := runShow(args, client) + output, err := runShow(args, client, cfg.Logger()) if err != nil { return err } @@ -109,7 +109,7 @@ func newShowCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { if err != nil { return err } - output, err := runShow(args, client) + output, err := runShow(args, client, cfg.Logger()) if err != nil { return err } @@ -130,7 +130,7 @@ func newShowCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { if err != nil { return err } - output, err := runShow(args, client) + output, err := runShow(args, client, cfg.Logger()) if err != nil { return err } @@ -151,7 +151,7 @@ func newShowCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { if err != nil { return err } - output, err := runShow(args, client) + output, err := runShow(args, client, cfg.Logger()) if err != nil { return err } @@ -172,7 +172,7 @@ func newShowCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { if err != nil { return err } - output, err := runShow(args, client) + output, err := runShow(args, client, cfg.Logger()) if err != nil { return err } @@ -211,10 +211,10 @@ func addShowFlags(subCmd *cobra.Command, client *action.Show) { } } -func runShow(args []string, client *action.Show) (string, error) { - slog.Debug("original chart version", "version", client.Version) +func runShow(args []string, client *action.Show, logger *slog.Logger) (string, error) { + logger.Debug("original chart version", "version", client.Version) if client.Version == "" && client.Devel { - slog.Debug("setting version to >0.0.0-0") + logger.Debug("setting version to >0.0.0-0") client.Version = ">0.0.0-0" } diff --git a/pkg/cmd/template.go b/pkg/cmd/template.go index 047fd60df..7bca28198 100644 --- a/pkg/cmd/template.go +++ b/pkg/cmd/template.go @@ -92,7 +92,7 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { } client.SetRegistryClient(registryClient) - dryRunStrategy, err := cmdGetDryRunFlagStrategy(cmd, true) + dryRunStrategy, err := cmdGetDryRunFlagStrategy(cmd, true, cfg.Logger()) if err != nil { return err } @@ -105,7 +105,7 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { client.Replace = true // Skip the name check client.APIVersions = common.VersionSet(extraAPIs) client.IncludeCRDs = includeCrds - rel, err := runInstall(args, client, valueOpts, out) + rel, err := runInstall(args, client, valueOpts, out, cfg.Logger()) if err != nil && !settings.Debug { if rel != nil { @@ -203,7 +203,7 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { } f := cmd.Flags() - addInstallFlags(cmd, f, client, valueOpts) + addInstallFlags(cmd, f, client, valueOpts, cfg.Logger()) f.StringArrayVarP(&showFiles, "show-only", "s", []string{}, "only show manifests rendered from the given templates") f.StringVar(&client.OutputDir, "output-dir", "", "writes the executed templates to files in output-dir instead of stdout") f.BoolVar(&validate, "validate", false, "deprecated") diff --git a/pkg/cmd/uninstall.go b/pkg/cmd/uninstall.go index 49f7bd19d..10d744102 100644 --- a/pkg/cmd/uninstall.go +++ b/pkg/cmd/uninstall.go @@ -82,7 +82,7 @@ func newUninstallCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { f.StringVar(&client.DeletionPropagation, "cascade", "background", "Must be \"background\", \"orphan\", or \"foreground\". Selects the deletion cascading strategy for the dependents. Defaults to background. Use \"foreground\" with --wait to ensure resources with finalizers are fully deleted before returning.") f.DurationVar(&client.Timeout, "timeout", 300*time.Second, "time to wait for any individual Kubernetes operation (like Jobs for hooks)") f.StringVar(&client.Description, "description", "", "add a custom description") - AddWaitFlag(cmd, &client.WaitStrategy) + AddWaitFlag(cmd, &client.WaitStrategy, cfg.Logger()) return cmd } diff --git a/pkg/cmd/upgrade.go b/pkg/cmd/upgrade.go index 43e19ab22..d61b6a444 100644 --- a/pkg/cmd/upgrade.go +++ b/pkg/cmd/upgrade.go @@ -112,7 +112,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { } client.SetRegistryClient(registryClient) - dryRunStrategy, err := cmdGetDryRunFlagStrategy(cmd, false) + dryRunStrategy, err := cmdGetDryRunFlagStrategy(cmd, false, cfg.Logger()) if err != nil { return err } @@ -125,7 +125,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { histClient := action.NewHistory(cfg) histClient.Max = 1 versions, err := histClient.Run(args[0]) - if errors.Is(err, driver.ErrReleaseNotFound) || isReleaseUninstalled(versions) { + if errors.Is(err, driver.ErrReleaseNotFound) || isReleaseUninstalled(versions, cfg.Logger()) { // Only print this to stdout for table output if outfmt == output.Table { fmt.Fprintf(out, "Release %q does not exist. Installing it now.\n", args[0]) @@ -157,11 +157,11 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { instClient.ForceConflicts = client.ForceConflicts instClient.ServerSideApply = client.ServerSideApply != "false" - if isReleaseUninstalled(versions) { + if isReleaseUninstalled(versions, cfg.Logger()) { instClient.Replace = true } - rel, err := runInstall(args, instClient, valueOpts, out) + rel, err := runInstall(args, instClient, valueOpts, out, cfg.Logger()) if err != nil { return err } @@ -178,7 +178,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { } if client.Version == "" && client.Devel { - slog.Debug("setting version to >0.0.0-0") + cfg.Logger().Debug("setting version to >0.0.0-0") client.Version = ">0.0.0-0" } @@ -232,7 +232,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { } if ac.Deprecated() { - slog.Warn("this chart is deprecated") + cfg.Logger().Warn("this chart is deprecated") } // Create context and prepare the handle of SIGTERM @@ -305,7 +305,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { addValueOptionsFlags(f, valueOpts) bindOutputFlag(cmd, &outfmt) bindPostRenderFlag(cmd, &client.PostRenderer, settings) - AddWaitFlag(cmd, &client.WaitStrategy) + AddWaitFlag(cmd, &client.WaitStrategy, cfg.Logger()) cmd.MarkFlagsMutuallyExclusive("force-replace", "force-conflicts") cmd.MarkFlagsMutuallyExclusive("force", "force-conflicts") @@ -322,10 +322,10 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { return cmd } -func isReleaseUninstalled(versionsi []ri.Releaser) bool { +func isReleaseUninstalled(versionsi []ri.Releaser, logger *slog.Logger) bool { versions, err := releaseListToV1List(versionsi) if err != nil { - slog.Error("cannot convert release list to v1 release list", "error", err) + logger.Error("cannot convert release list to v1 release list", "error", err) return false } return len(versions) > 0 && versions[len(versions)-1].Info.Status == common.StatusUninstalled diff --git a/pkg/cmd/version.go b/pkg/cmd/version.go index 80fb0d712..faf0c2588 100644 --- a/pkg/cmd/version.go +++ b/pkg/cmd/version.go @@ -83,14 +83,14 @@ func (o *versionOptions) run(out io.Writer) error { if err != nil { return err } - return tt.Execute(out, version.Get()) + return tt.Execute(out, version.Get(nil)) } fmt.Fprintln(out, formatVersion(o.short)) return nil } func formatVersion(short bool) string { - v := version.Get() + v := version.Get(nil) if short { if len(v.GitCommit) >= 7 { return fmt.Sprintf("%s+g%s", v.Version, v.GitCommit[:7]) diff --git a/pkg/downloader/cache.go b/pkg/downloader/cache.go index 92d477e49..2f941d7ef 100644 --- a/pkg/downloader/cache.go +++ b/pkg/downloader/cache.go @@ -49,7 +49,15 @@ var CacheProv = ".prov" // DiskCache is a cache that stores data on disk. type DiskCache struct { - Root string + Root string + Logger *slog.Logger +} + +func (c *DiskCache) log() *slog.Logger { + if c.Logger != nil { + return c.Logger + } + return slog.New(slog.DiscardHandler) } // Get returns a reader for the given key. @@ -79,7 +87,7 @@ func (c *DiskCache) Put(key [sha256.Size]byte, data io.Reader, cacheType string) // TODO: verify the key and digest of the key are the same. p := c.fileName(key, cacheType) if err := os.MkdirAll(filepath.Dir(p), 0755); err != nil { - slog.Error("failed to create cache directory") + c.log().Error("failed to create cache directory") return p, err } return p, fileutil.AtomicWriteFile(p, data, 0644) diff --git a/pkg/downloader/chart_downloader.go b/pkg/downloader/chart_downloader.go index 9c26f925e..c58fa5036 100644 --- a/pkg/downloader/chart_downloader.go +++ b/pkg/downloader/chart_downloader.go @@ -85,6 +85,16 @@ type ChartDownloader struct { // Cache specifies the cache implementation to use. Cache Cache + + // Logger is the structured logger for this downloader instance. + Logger *slog.Logger +} + +func (c *ChartDownloader) log() *slog.Logger { + if c.Logger != nil { + return c.Logger + } + return slog.New(slog.DiscardHandler) } // DownloadTo retrieves a chart. Depending on the settings, it may also download a provenance file. @@ -104,7 +114,7 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven return "", nil, errors.New("content cache must be set") } c.Cache = &DiskCache{Root: c.ContentCache} - slog.Debug("set up default downloader cache") + c.log().Debug("set up default downloader cache") } hash, u, err := c.ResolveChartVersion(ref, version) if err != nil { @@ -140,7 +150,7 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven if err == nil { found = true data = bytes.NewBuffer(fdata) - slog.Debug("found chart in cache", "id", hash) + c.log().Debug("found chart in cache", "id", hash) } } } @@ -180,7 +190,7 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven if err == nil { found = true body = bytes.NewBuffer(fdata) - slog.Debug("found provenance in cache", "id", hash) + c.log().Debug("found provenance in cache", "id", hash) } } } @@ -220,7 +230,7 @@ func (c *ChartDownloader) DownloadToCache(ref, version string) (string, *provena return "", nil, errors.New("content cache must be set") } c.Cache = &DiskCache{Root: c.ContentCache} - slog.Debug("set up default downloader cache") + c.log().Debug("set up default downloader cache") } digestString, u, err := c.ResolveChartVersion(ref, version) @@ -252,11 +262,11 @@ func (c *ChartDownloader) DownloadToCache(ref, version string) (string, *provena if len(digest) > 0 { pth, err = c.Cache.Get(digest32, CacheChart) if err == nil { - slog.Debug("found chart in cache", "id", digestString) + c.log().Debug("found chart in cache", "id", digestString) } } if len(digest) == 0 || err != nil { - slog.Debug("attempting to download chart", "ref", ref, "version", version) + c.log().Debug("attempting to download chart", "ref", ref, "version", version) if err != nil && !os.IsNotExist(err) { return "", nil, err } @@ -276,7 +286,7 @@ func (c *ChartDownloader) DownloadToCache(ref, version string) (string, *provena if err != nil { return "", nil, err } - slog.Debug("put downloaded chart in cache", "id", hex.EncodeToString(digest32[:])) + c.log().Debug("put downloaded chart in cache", "id", hex.EncodeToString(digest32[:])) } // If provenance is requested, verify it. @@ -285,7 +295,7 @@ func (c *ChartDownloader) DownloadToCache(ref, version string) (string, *provena ppth, err := c.Cache.Get(digest32, CacheProv) if err == nil { - slog.Debug("found provenance in cache", "id", digestString) + c.log().Debug("found provenance in cache", "id", digestString) } else { if !os.IsNotExist(err) { return pth, ver, err @@ -304,7 +314,7 @@ func (c *ChartDownloader) DownloadToCache(ref, version string) (string, *provena if err != nil { return "", nil, err } - slog.Debug("put downloaded provenance file in cache", "id", hex.EncodeToString(digest32[:])) + c.log().Debug("put downloaded provenance file in cache", "id", hex.EncodeToString(digest32[:])) } if c.Verify != VerifyLater { diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 6fd2beed8..3df24e6aa 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -47,6 +47,15 @@ type Engine struct { EnableDNS bool // CustomTemplateFuncs is defined by users to provide custom template funcs CustomTemplateFuncs template.FuncMap + // logger is the structured logger for this engine instance + logger *slog.Logger +} + +func (e Engine) log() *slog.Logger { + if e.logger != nil { + return e.logger + } + return slog.New(slog.DiscardHandler) } // New creates a new instance of Engine using the passed in rest config. @@ -77,7 +86,7 @@ func New(config *rest.Config) Engine { // section contains a value named "bar", that value will be passed on to the // bar chart during render time. func (e Engine) Render(chrt ci.Charter, values common.Values) (map[string]string, error) { - tmap := allTemplates(chrt, values) + tmap := allTemplates(chrt, values, e.log()) return e.render(tmap) } @@ -208,7 +217,7 @@ func (e Engine) initFunMap(t *template.Template) { if val == nil { if e.LintMode { // Don't fail on missing required values when linting - slog.Warn("missing required value", "message", warn) + e.log().Warn("missing required value", "message", warn) return "", nil } return val, errors.New(warnWrap(warn)) @@ -216,7 +225,7 @@ func (e Engine) initFunMap(t *template.Template) { if val == "" { if e.LintMode { // Don't fail on missing required values when linting - slog.Warn("missing required values", "message", warn) + e.log().Warn("missing required values", "message", warn) return "", nil } return val, errors.New(warnWrap(warn)) @@ -229,7 +238,7 @@ func (e Engine) initFunMap(t *template.Template) { funcMap["fail"] = func(msg string) (string, error) { if e.LintMode { // Don't fail when linting - slog.Info("funcMap fail", "message", msg) + e.log().Info("funcMap fail", "message", msg) return "", nil } return "", errors.New(warnWrap(msg)) @@ -238,7 +247,7 @@ func (e Engine) initFunMap(t *template.Template) { // If we are not linting and have a cluster connection, provide a Kubernetes-backed // implementation. if !e.LintMode && e.clientProvider != nil { - funcMap["lookup"] = newLookupFunction(*e.clientProvider) + funcMap["lookup"] = newLookupFunction(*e.clientProvider, e.log()) } // When DNS lookups are not enabled override the sprig function and return @@ -521,9 +530,9 @@ func (p byPathLen) Less(i, j int) bool { // allTemplates returns all templates for a chart and its dependencies. // // As it goes, it also prepares the values in a scope-sensitive manner. -func allTemplates(c ci.Charter, vals common.Values) map[string]renderable { +func allTemplates(c ci.Charter, vals common.Values, logger *slog.Logger) map[string]renderable { templates := make(map[string]renderable) - recAllTpls(c, templates, vals) + recAllTpls(c, templates, vals, logger) return templates } @@ -531,12 +540,12 @@ func allTemplates(c ci.Charter, vals common.Values) map[string]renderable { // // As it recurses, it also sets the values to be appropriate for the template // scope. -func recAllTpls(c ci.Charter, templates map[string]renderable, values common.Values) map[string]any { +func recAllTpls(c ci.Charter, templates map[string]renderable, values common.Values, logger *slog.Logger) map[string]any { vals := values.AsMap() subCharts := make(map[string]any) accessor, err := ci.NewAccessor(c) if err != nil { - slog.Error("error accessing chart", "error", err) + logger.Error("error accessing chart", "error", err) } chartMetaData := accessor.MetadataAsMap() chartMetaData["IsRoot"] = accessor.IsRoot() @@ -561,7 +570,7 @@ func recAllTpls(c ci.Charter, templates map[string]renderable, values common.Val for _, child := range accessor.Dependencies() { // TODO: Handle error sub, _ := ci.NewAccessor(child) - subCharts[sub.Name()] = recAllTpls(child, templates, next) + subCharts[sub.Name()] = recAllTpls(child, templates, next, logger) } newParentID := accessor.ChartFullPath() diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go index 869b5d202..5f7b9e804 100644 --- a/pkg/engine/engine_test.go +++ b/pkg/engine/engine_test.go @@ -19,6 +19,7 @@ package engine import ( "errors" "fmt" + "log/slog" "path" "strings" "sync" @@ -590,7 +591,7 @@ func TestAllTemplates(t *testing.T) { } dep1.AddDependency(dep2) - tpls := allTemplates(ch1, common.Values{}) + tpls := allTemplates(ch1, common.Values{}, slog.New(slog.DiscardHandler)) if len(tpls) != 5 { t.Errorf("Expected 5 charts, got %d", len(tpls)) } diff --git a/pkg/engine/lookup_func.go b/pkg/engine/lookup_func.go index 52b6ffdaf..45265e81f 100644 --- a/pkg/engine/lookup_func.go +++ b/pkg/engine/lookup_func.go @@ -36,7 +36,7 @@ type lookupFunc = func(apiversion string, resource string, namespace string, nam // // If the resource does not exist, no error is raised. func NewLookupFunction(config *rest.Config) lookupFunc { //nolint:revive - return newLookupFunction(clientProviderFromConfig{config: config}) + return newLookupFunction(clientProviderFromConfig{config: config}, slog.New(slog.DiscardHandler)) } type ClientProvider interface { @@ -51,10 +51,10 @@ type clientProviderFromConfig struct { } func (c clientProviderFromConfig) GetClientFor(apiVersion, kind string) (dynamic.NamespaceableResourceInterface, bool, error) { - return getDynamicClientOnKind(apiVersion, kind, c.config) + return getDynamicClientOnKind(apiVersion, kind, c.config, slog.New(slog.DiscardHandler)) } -func newLookupFunction(clientProvider ClientProvider) lookupFunc { +func newLookupFunction(clientProvider ClientProvider, logger *slog.Logger) lookupFunc { return func(apiversion string, kind string, namespace string, name string) (map[string]any, error) { var client dynamic.ResourceInterface c, namespaced, err := clientProvider.GetClientFor(apiversion, kind) @@ -94,11 +94,11 @@ func newLookupFunction(clientProvider ClientProvider) lookupFunc { } // getDynamicClientOnKind returns a dynamic client on an Unstructured type. This client can be further namespaced. -func getDynamicClientOnKind(apiversion string, kind string, config *rest.Config) (dynamic.NamespaceableResourceInterface, bool, error) { +func getDynamicClientOnKind(apiversion string, kind string, config *rest.Config, logger *slog.Logger) (dynamic.NamespaceableResourceInterface, bool, error) { gvk := schema.FromAPIVersionAndKind(apiversion, kind) - apiRes, err := getAPIResourceForGVK(gvk, config) + apiRes, err := getAPIResourceForGVK(gvk, config, logger) if err != nil { - slog.Error( + logger.Error( "unable to get apiresource", slog.String("groupVersionKind", gvk.String()), slog.Any("error", err), @@ -112,23 +112,23 @@ func getDynamicClientOnKind(apiversion string, kind string, config *rest.Config) } intf, err := dynamic.NewForConfig(config) if err != nil { - slog.Error("unable to get dynamic client", slog.Any("error", err)) + logger.Error("unable to get dynamic client", slog.Any("error", err)) return nil, false, err } res := intf.Resource(gvr) return res, apiRes.Namespaced, nil } -func getAPIResourceForGVK(gvk schema.GroupVersionKind, config *rest.Config) (metav1.APIResource, error) { +func getAPIResourceForGVK(gvk schema.GroupVersionKind, config *rest.Config, logger *slog.Logger) (metav1.APIResource, error) { res := metav1.APIResource{} discoveryClient, err := discovery.NewDiscoveryClientForConfig(config) if err != nil { - slog.Error("unable to create discovery client", slog.Any("error", err)) + logger.Error("unable to create discovery client", slog.Any("error", err)) return res, err } resList, err := discoveryClient.ServerResourcesForGroupVersion(gvk.GroupVersion().String()) if err != nil { - slog.Error( + logger.Error( "unable to retrieve resource list", slog.String("GroupVersion", gvk.GroupVersion().String()), slog.Any("error", err), diff --git a/pkg/ignore/rules.go b/pkg/ignore/rules.go index a8160da2a..cf855d15e 100644 --- a/pkg/ignore/rules.go +++ b/pkg/ignore/rules.go @@ -36,6 +36,14 @@ const HelmIgnore = ".helmignore" // Empty() will create an immutable empty ruleset. type Rules struct { patterns []*pattern + logger *slog.Logger +} + +func (r *Rules) log() *slog.Logger { + if r.logger != nil { + return r.logger + } + return slog.New(slog.DiscardHandler) } // Empty builds an empty ruleset. @@ -101,7 +109,7 @@ func (r *Rules) Ignore(path string, fi os.FileInfo) bool { } for _, p := range r.patterns { if p.match == nil { - slog.Info("this will be ignored no matcher supplied", "patterns", p.raw) + r.log().Info("this will be ignored no matcher supplied", "patterns", p.raw) return false } @@ -176,7 +184,7 @@ func (r *Rules) parseRule(rule string) error { rule = after ok, err := filepath.Match(rule, n) if err != nil { - slog.Error("failed to compile", slog.String("rule", rule), slog.Any("error", err)) + r.log().Error("failed to compile", slog.String("rule", rule), slog.Any("error", err)) return false } return ok @@ -186,7 +194,7 @@ func (r *Rules) parseRule(rule string) error { p.match = func(n string, _ os.FileInfo) bool { ok, err := filepath.Match(rule, n) if err != nil { - slog.Error( + r.log().Error( "failed to compile", slog.String("rule", rule), slog.Any("error", err), @@ -202,7 +210,7 @@ func (r *Rules) parseRule(rule string) error { n = filepath.Base(n) ok, err := filepath.Match(rule, n) if err != nil { - slog.Error("failed to compile", slog.String("rule", rule), slog.Any("error", err)) + r.log().Error("failed to compile", slog.String("rule", rule), slog.Any("error", err)) return false } return ok diff --git a/pkg/kube/client.go b/pkg/kube/client.go index c955e8875..cf2204905 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -191,7 +191,7 @@ func (c *Client) GetWaiterWithOptions(strategy WaitStrategy, opts ...WaitOption) if err != nil { return nil, err } - return &legacyWaiter{kubeClient: kc, ctx: c.WaitContext}, nil + return &legacyWaiter{kubeClient: kc, ctx: c.WaitContext, logger: c.Logger()}, nil case StatusWatcherStrategy: return c.newStatusWatcher(opts...) case HookOnlyStrategy: @@ -226,11 +226,9 @@ func New(getter genericclioptions.RESTClientGetter) *Client { getter = genericclioptions.NewConfigFlags(true) } factory := cmdutil.NewFactory(getter) - c := &Client{ + return &Client{ Factory: factory, } - c.SetLogger(slog.Default().Handler()) - return c } // getKubeClient get or create a new KubernetesClientSet @@ -601,7 +599,7 @@ func (c *Client) update(originals, targets ResourceList, createApplyFunc CreateA if original == nil { kind := target.Mapping.GroupVersionKind.Kind - slog.Warn("resource exists on cluster but not in original release, using cluster state as baseline", + c.Logger().Warn("resource exists on cluster but not in original release, using cluster state as baseline", "namespace", target.Namespace, "name", target.Name, "kind", kind) currentObj, err := helper.Get(target.Namespace, target.Name) @@ -890,7 +888,7 @@ func (c *Client) Update(originals, targets ResourceList, options ...ClientUpdate c.Logger().Debug("using client-side apply for resource update", slog.Bool("threeWayMergeForUnstructured", updateOptions.threeWayMergeForUnstructured)) return func(original, target *resource.Info) error { - return patchResourceClientSide(original.Object, target, updateOptions.threeWayMergeForUnstructured) + return patchResourceClientSide(original.Object, target, updateOptions.threeWayMergeForUnstructured, c.Logger()) } } @@ -1120,7 +1118,7 @@ func replaceResource(target *resource.Info, fieldValidationDirective FieldValida } -func patchResourceClientSide(original runtime.Object, target *resource.Info, threeWayMergeForUnstructured bool) error { +func patchResourceClientSide(original runtime.Object, target *resource.Info, threeWayMergeForUnstructured bool, logger *slog.Logger) error { patch, patchType, err := createPatch(original, target, threeWayMergeForUnstructured) if err != nil { @@ -1129,7 +1127,7 @@ func patchResourceClientSide(original runtime.Object, target *resource.Info, thr kind := target.Mapping.GroupVersionKind.Kind if patch == nil || string(patch) == "{}" { - slog.Debug("no changes detected", "kind", kind, "name", target.Name) + logger.Debug("no changes detected", "kind", kind, "name", target.Name) // This needs to happen to make sure that Helm has the latest info from the API // Otherwise there will be no labels and other functions that use labels will panic if err := target.Get(); err != nil { @@ -1139,7 +1137,7 @@ func patchResourceClientSide(original runtime.Object, target *resource.Info, thr } // send patch to server - slog.Debug("patching resource", "kind", kind, "name", target.Name, "namespace", target.Namespace) + logger.Debug("patching resource", "kind", kind, "name", target.Name, "namespace", target.Namespace) helper := resource.NewHelper(target.Client, target.Mapping).WithFieldManager(getManagedFieldsManager()) obj, err := helper.Patch(target.Namespace, target.Name, patchType, patch, nil) if err != nil { diff --git a/pkg/kube/ready.go b/pkg/kube/ready.go index a1a3d4a9a..d0164626c 100644 --- a/pkg/kube/ready.go +++ b/pkg/kube/ready.go @@ -73,6 +73,22 @@ type ReadyChecker struct { client kubernetes.Interface checkJobs bool pausedAsReady bool + logger *slog.Logger +} + +// log returns the configured logger or a discard logger if none is set. +func (c *ReadyChecker) log() *slog.Logger { + if c.logger != nil { + return c.logger + } + return slog.New(slog.DiscardHandler) +} + +// WithLogger returns a ReadyCheckerOption that sets the logger. +func WithLogger(logger *slog.Logger) ReadyCheckerOption { + return func(c *ReadyChecker) { + c.logger = logger + } } // IsReady checks if v is ready. It supports checking readiness for pods, @@ -226,21 +242,21 @@ func (c *ReadyChecker) isPodReady(pod *corev1.Pod) bool { return true } } - slog.Debug("Pod is not ready", "namespace", pod.GetNamespace(), "name", pod.GetName()) + c.log().Debug("Pod is not ready", "namespace", pod.GetNamespace(), "name", pod.GetName()) return false } func (c *ReadyChecker) jobReady(job *batchv1.Job) (bool, error) { if job.Status.Failed > *job.Spec.BackoffLimit { - slog.Debug("Job is failed", "namespace", job.GetNamespace(), "name", job.GetName()) + c.log().Debug("Job is failed", "namespace", job.GetNamespace(), "name", job.GetName()) // If a job is failed, it can't recover, so throw an error return false, fmt.Errorf("job is failed: %s/%s", job.GetNamespace(), job.GetName()) } if job.Spec.Completions != nil && job.Status.Succeeded < *job.Spec.Completions { - slog.Debug("Job is not completed", "namespace", job.GetNamespace(), "name", job.GetName()) + c.log().Debug("Job is not completed", "namespace", job.GetNamespace(), "name", job.GetName()) return false, nil } - slog.Debug("Job is completed", "namespace", job.GetNamespace(), "name", job.GetName()) + c.log().Debug("Job is completed", "namespace", job.GetNamespace(), "name", job.GetName()) return true, nil } @@ -252,7 +268,7 @@ func (c *ReadyChecker) serviceReady(s *corev1.Service) bool { // Ensure that the service cluster IP is not empty if s.Spec.ClusterIP == "" { - slog.Debug("Service does not have cluster IP address", "namespace", s.GetNamespace(), "name", s.GetName()) + c.log().Debug("Service does not have cluster IP address", "namespace", s.GetNamespace(), "name", s.GetName()) return false } @@ -260,25 +276,25 @@ func (c *ReadyChecker) serviceReady(s *corev1.Service) bool { if s.Spec.Type == corev1.ServiceTypeLoadBalancer { // do not wait when at least 1 external IP is set if len(s.Spec.ExternalIPs) > 0 { - slog.Debug("Service has external IP addresses", "namespace", s.GetNamespace(), "name", s.GetName(), "externalIPs", s.Spec.ExternalIPs) + c.log().Debug("Service has external IP addresses", "namespace", s.GetNamespace(), "name", s.GetName(), "externalIPs", s.Spec.ExternalIPs) return true } if s.Status.LoadBalancer.Ingress == nil { - slog.Debug("Service does not have load balancer ingress IP address", "namespace", s.GetNamespace(), "name", s.GetName()) + c.log().Debug("Service does not have load balancer ingress IP address", "namespace", s.GetNamespace(), "name", s.GetName()) return false } } - slog.Debug("Service is ready", "namespace", s.GetNamespace(), "name", s.GetName(), "clusterIP", s.Spec.ClusterIP, "externalIPs", s.Spec.ExternalIPs) + c.log().Debug("Service is ready", "namespace", s.GetNamespace(), "name", s.GetName(), "clusterIP", s.Spec.ClusterIP, "externalIPs", s.Spec.ExternalIPs) return true } func (c *ReadyChecker) volumeReady(v *corev1.PersistentVolumeClaim) bool { if v.Status.Phase != corev1.ClaimBound { - slog.Debug("PersistentVolumeClaim is not bound", "namespace", v.GetNamespace(), "name", v.GetName()) + c.log().Debug("PersistentVolumeClaim is not bound", "namespace", v.GetNamespace(), "name", v.GetName()) return false } - slog.Debug("PersistentVolumeClaim is bound", "namespace", v.GetNamespace(), "name", v.GetName(), "phase", v.Status.Phase) + c.log().Debug("PersistentVolumeClaim is bound", "namespace", v.GetNamespace(), "name", v.GetName(), "phase", v.Status.Phase) return true } @@ -289,23 +305,23 @@ func (c *ReadyChecker) deploymentReady(rs *appsv1.ReplicaSet, dep *appsv1.Deploy } // Verify the generation observed by the deployment controller matches the spec generation if dep.Status.ObservedGeneration != dep.Generation { - slog.Debug("Deployment is not ready, observedGeneration does not match spec generation", "namespace", dep.GetNamespace(), "name", dep.GetName(), "actualGeneration", dep.Status.ObservedGeneration, "expectedGeneration", dep.Generation) + c.log().Debug("Deployment is not ready, observedGeneration does not match spec generation", "namespace", dep.GetNamespace(), "name", dep.GetName(), "actualGeneration", dep.Status.ObservedGeneration, "expectedGeneration", dep.Generation) return false } expectedReady := *dep.Spec.Replicas - deploymentutil.MaxUnavailable(*dep) if rs.Status.ReadyReplicas < expectedReady { - slog.Debug("Deployment does not have enough pods ready", "namespace", dep.GetNamespace(), "name", dep.GetName(), "readyPods", rs.Status.ReadyReplicas, "totalPods", expectedReady) + c.log().Debug("Deployment does not have enough pods ready", "namespace", dep.GetNamespace(), "name", dep.GetName(), "readyPods", rs.Status.ReadyReplicas, "totalPods", expectedReady) return false } - slog.Debug("Deployment is ready", "namespace", dep.GetNamespace(), "name", dep.GetName(), "readyPods", rs.Status.ReadyReplicas, "totalPods", expectedReady) + c.log().Debug("Deployment is ready", "namespace", dep.GetNamespace(), "name", dep.GetName(), "readyPods", rs.Status.ReadyReplicas, "totalPods", expectedReady) return true } func (c *ReadyChecker) daemonSetReady(ds *appsv1.DaemonSet) bool { // Verify the generation observed by the daemonSet controller matches the spec generation if ds.Status.ObservedGeneration != ds.Generation { - slog.Debug("DaemonSet is not ready, observedGeneration does not match spec generation", "namespace", ds.GetNamespace(), "name", ds.GetName(), "observedGeneration", ds.Status.ObservedGeneration, "expectedGeneration", ds.Generation) + c.log().Debug("DaemonSet is not ready, observedGeneration does not match spec generation", "namespace", ds.GetNamespace(), "name", ds.GetName(), "observedGeneration", ds.Status.ObservedGeneration, "expectedGeneration", ds.Generation) return false } @@ -316,7 +332,7 @@ func (c *ReadyChecker) daemonSetReady(ds *appsv1.DaemonSet) bool { // Make sure all the updated pods have been scheduled if ds.Status.UpdatedNumberScheduled != ds.Status.DesiredNumberScheduled { - slog.Debug("DaemonSet does not have enough Pods scheduled", "namespace", ds.GetNamespace(), "name", ds.GetName(), "scheduledPods", ds.Status.UpdatedNumberScheduled, "totalPods", ds.Status.DesiredNumberScheduled) + c.log().Debug("DaemonSet does not have enough Pods scheduled", "namespace", ds.GetNamespace(), "name", ds.GetName(), "scheduledPods", ds.Status.UpdatedNumberScheduled, "totalPods", ds.Status.DesiredNumberScheduled) return false } maxUnavailable, err := intstr.GetScaledValueFromIntOrPercent(ds.Spec.UpdateStrategy.RollingUpdate.MaxUnavailable, int(ds.Status.DesiredNumberScheduled), true) @@ -329,10 +345,10 @@ func (c *ReadyChecker) daemonSetReady(ds *appsv1.DaemonSet) bool { expectedReady := int(ds.Status.DesiredNumberScheduled) - maxUnavailable if int(ds.Status.NumberReady) < expectedReady { - slog.Debug("DaemonSet does not have enough Pods ready", "namespace", ds.GetNamespace(), "name", ds.GetName(), "readyPods", ds.Status.NumberReady, "totalPods", expectedReady) + c.log().Debug("DaemonSet does not have enough Pods ready", "namespace", ds.GetNamespace(), "name", ds.GetName(), "readyPods", ds.Status.NumberReady, "totalPods", expectedReady) return false } - slog.Debug("DaemonSet is ready", "namespace", ds.GetNamespace(), "name", ds.GetName(), "readyPods", ds.Status.NumberReady, "totalPods", expectedReady) + c.log().Debug("DaemonSet is ready", "namespace", ds.GetNamespace(), "name", ds.GetName(), "readyPods", ds.Status.NumberReady, "totalPods", expectedReady) return true } @@ -386,13 +402,13 @@ func (c *ReadyChecker) crdReady(crd apiextv1.CustomResourceDefinition) bool { func (c *ReadyChecker) statefulSetReady(sts *appsv1.StatefulSet) bool { // Verify the generation observed by the statefulSet controller matches the spec generation if sts.Status.ObservedGeneration != sts.Generation { - slog.Debug("StatefulSet is not ready, observedGeneration doest not match spec generation", "namespace", sts.GetNamespace(), "name", sts.GetName(), "actualGeneration", sts.Status.ObservedGeneration, "expectedGeneration", sts.Generation) + c.log().Debug("StatefulSet is not ready, observedGeneration doest not match spec generation", "namespace", sts.GetNamespace(), "name", sts.GetName(), "actualGeneration", sts.Status.ObservedGeneration, "expectedGeneration", sts.Generation) return false } // If the update strategy is not a rolling update, there will be nothing to wait for if sts.Spec.UpdateStrategy.Type != appsv1.RollingUpdateStatefulSetStrategyType { - slog.Debug("StatefulSet skipped ready check", "namespace", sts.GetNamespace(), "name", sts.GetName(), "updateStrategy", sts.Spec.UpdateStrategy.Type) + c.log().Debug("StatefulSet skipped ready check", "namespace", sts.GetNamespace(), "name", sts.GetName(), "updateStrategy", sts.Spec.UpdateStrategy.Type) return true } @@ -418,29 +434,29 @@ func (c *ReadyChecker) statefulSetReady(sts *appsv1.StatefulSet) bool { // Make sure all the updated pods have been scheduled if int(sts.Status.UpdatedReplicas) < expectedReplicas { - slog.Debug("StatefulSet does not have enough Pods scheduled", "namespace", sts.GetNamespace(), "name", sts.GetName(), "readyPods", sts.Status.UpdatedReplicas, "totalPods", expectedReplicas) + c.log().Debug("StatefulSet does not have enough Pods scheduled", "namespace", sts.GetNamespace(), "name", sts.GetName(), "readyPods", sts.Status.UpdatedReplicas, "totalPods", expectedReplicas) return false } if int(sts.Status.ReadyReplicas) != replicas { - slog.Debug("StatefulSet does not have enough Pods ready", "namespace", sts.GetNamespace(), "name", sts.GetName(), "readyPods", sts.Status.ReadyReplicas, "totalPods", replicas) + c.log().Debug("StatefulSet does not have enough Pods ready", "namespace", sts.GetNamespace(), "name", sts.GetName(), "readyPods", sts.Status.ReadyReplicas, "totalPods", replicas) return false } // This check only makes sense when all partitions are being upgraded otherwise during a // partitioned rolling upgrade, this condition will never evaluate to true, leading to // error. if partition == 0 && sts.Status.CurrentRevision != sts.Status.UpdateRevision { - slog.Debug("StatefulSet is not ready, currentRevision does not match updateRevision", "namespace", sts.GetNamespace(), "name", sts.GetName(), "currentRevision", sts.Status.CurrentRevision, "updateRevision", sts.Status.UpdateRevision) + c.log().Debug("StatefulSet is not ready, currentRevision does not match updateRevision", "namespace", sts.GetNamespace(), "name", sts.GetName(), "currentRevision", sts.Status.CurrentRevision, "updateRevision", sts.Status.UpdateRevision) return false } - slog.Debug("StatefulSet is ready", "namespace", sts.GetNamespace(), "name", sts.GetName(), "readyPods", sts.Status.ReadyReplicas, "totalPods", replicas) + c.log().Debug("StatefulSet is ready", "namespace", sts.GetNamespace(), "name", sts.GetName(), "readyPods", sts.Status.ReadyReplicas, "totalPods", replicas) return true } func (c *ReadyChecker) replicationControllerReady(rc *corev1.ReplicationController) bool { // Verify the generation observed by the replicationController controller matches the spec generation if rc.Status.ObservedGeneration != rc.Generation { - slog.Debug("ReplicationController is not ready, observedGeneration doest not match spec generation", "namespace", rc.GetNamespace(), "name", rc.GetName(), "actualGeneration", rc.Status.ObservedGeneration, "expectedGeneration", rc.Generation) + c.log().Debug("ReplicationController is not ready, observedGeneration doest not match spec generation", "namespace", rc.GetNamespace(), "name", rc.GetName(), "actualGeneration", rc.Status.ObservedGeneration, "expectedGeneration", rc.Generation) return false } return true @@ -449,7 +465,7 @@ func (c *ReadyChecker) replicationControllerReady(rc *corev1.ReplicationControll func (c *ReadyChecker) replicaSetReady(rs *appsv1.ReplicaSet) bool { // Verify the generation observed by the replicaSet controller matches the spec generation if rs.Status.ObservedGeneration != rs.Generation { - slog.Debug("ReplicaSet is not ready, observedGeneration doest not match spec generation", "namespace", rs.GetNamespace(), "name", rs.GetName(), "actualGeneration", rs.Status.ObservedGeneration, "expectedGeneration", rs.Generation) + c.log().Debug("ReplicaSet is not ready, observedGeneration doest not match spec generation", "namespace", rs.GetNamespace(), "name", rs.GetName(), "actualGeneration", rs.Status.ObservedGeneration, "expectedGeneration", rs.Generation) return false } return true diff --git a/pkg/kube/wait.go b/pkg/kube/wait.go index b5e91d8f3..de65b53cf 100644 --- a/pkg/kube/wait.go +++ b/pkg/kube/wait.go @@ -51,22 +51,31 @@ type legacyWaiter struct { c ReadyChecker kubeClient *kubernetes.Clientset ctx context.Context + logger *slog.Logger +} + +// log returns the configured logger or a discard logger if none is set. +func (hw *legacyWaiter) log() *slog.Logger { + if hw.logger != nil { + return hw.logger + } + return slog.New(slog.DiscardHandler) } func (hw *legacyWaiter) Wait(resources ResourceList, timeout time.Duration) error { - hw.c = NewReadyChecker(hw.kubeClient, PausedAsReady(true)) + hw.c = NewReadyChecker(hw.kubeClient, PausedAsReady(true), WithLogger(hw.log())) return hw.waitForResources(resources, timeout) } func (hw *legacyWaiter) WaitWithJobs(resources ResourceList, timeout time.Duration) error { - hw.c = NewReadyChecker(hw.kubeClient, PausedAsReady(true), CheckJobs(true)) + hw.c = NewReadyChecker(hw.kubeClient, PausedAsReady(true), CheckJobs(true), WithLogger(hw.log())) return hw.waitForResources(resources, timeout) } // waitForResources polls to get the current status of all pods, PVCs, Services and // Jobs(optional) until all are ready or a timeout is reached func (hw *legacyWaiter) waitForResources(created ResourceList, timeout time.Duration) error { - slog.Debug("beginning wait for resources", "count", len(created), "timeout", timeout) + hw.log().Debug("beginning wait for resources", "count", len(created), "timeout", timeout) ctx, cancel := hw.contextWithTimeout(timeout) defer cancel() @@ -84,10 +93,10 @@ func (hw *legacyWaiter) waitForResources(created ResourceList, timeout time.Dura if waitRetries > 0 && hw.isRetryableError(err, v) { numberOfErrors[i]++ if numberOfErrors[i] > waitRetries { - slog.Debug("max number of retries reached", "resource", v.Name, "retries", numberOfErrors[i]) + hw.log().Debug("max number of retries reached", "resource", v.Name, "retries", numberOfErrors[i]) return false, err } - slog.Debug("retrying resource readiness", "resource", v.Name, "currentRetries", numberOfErrors[i]-1, "maxRetries", waitRetries) + hw.log().Debug("retrying resource readiness", "resource", v.Name, "currentRetries", numberOfErrors[i]-1, "maxRetries", waitRetries) return false, nil } numberOfErrors[i] = 0 @@ -103,7 +112,7 @@ func (hw *legacyWaiter) isRetryableError(err error, resource *resource.Info) boo if err == nil { return false } - slog.Debug( + hw.log().Debug( "error received when checking resource status", slog.String("resource", resource.Name), slog.Any("error", err), @@ -112,7 +121,7 @@ func (hw *legacyWaiter) isRetryableError(err error, resource *resource.Info) boo if errors.As(err, &ev) { statusCode := ev.Status().Code retryable := hw.isRetryableHTTPStatusCode(statusCode) - slog.Debug( + hw.log().Debug( "status code received", slog.String("resource", resource.Name), slog.Int("statusCode", int(statusCode)), @@ -120,7 +129,7 @@ func (hw *legacyWaiter) isRetryableError(err error, resource *resource.Info) boo ) return retryable } - slog.Debug("retryable error assumed", "resource", resource.Name) + hw.log().Debug("retryable error assumed", "resource", resource.Name) return true } @@ -130,7 +139,7 @@ func (hw *legacyWaiter) isRetryableHTTPStatusCode(httpStatusCode int32) bool { // WaitForDelete polls to check if all the resources are deleted or a timeout is reached func (hw *legacyWaiter) WaitForDelete(deleted ResourceList, timeout time.Duration) error { - slog.Debug("beginning wait for resources to be deleted", "count", len(deleted), "timeout", timeout) + hw.log().Debug("beginning wait for resources to be deleted", "count", len(deleted), "timeout", timeout) startTime := time.Now() ctx, cancel := hw.contextWithTimeout(timeout) @@ -148,9 +157,9 @@ func (hw *legacyWaiter) WaitForDelete(deleted ResourceList, timeout time.Duratio elapsed := time.Since(startTime).Round(time.Second) if err != nil { - slog.Debug("wait for resources failed", slog.Duration("elapsed", elapsed), slog.Any("error", err)) + hw.log().Debug("wait for resources failed", slog.Duration("elapsed", elapsed), slog.Any("error", err)) } else { - slog.Debug("wait for resources succeeded", slog.Duration("elapsed", elapsed)) + hw.log().Debug("wait for resources succeeded", slog.Duration("elapsed", elapsed)) } return err @@ -242,7 +251,7 @@ func (hw *legacyWaiter) watchUntilReady(timeout time.Duration, info *resource.In return nil } - slog.Debug("watching for resource changes", "kind", kind, "resource", info.Name, "timeout", timeout) + hw.log().Debug("watching for resource changes", "kind", kind, "resource", info.Name, "timeout", timeout) // Use a selector on the name of the resource. This should be unique for the // given version and kind @@ -270,7 +279,7 @@ func (hw *legacyWaiter) watchUntilReady(timeout time.Duration, info *resource.In // we get. We care mostly about jobs, where what we want to see is // the status go into a good state. For other types, like ReplicaSet // we don't really do anything to support these as hooks. - slog.Debug("add/modify event received", "resource", info.Name, "eventType", e.Type) + hw.log().Debug("add/modify event received", "resource", info.Name, "eventType", e.Type) switch kind { case "Job": @@ -280,11 +289,11 @@ func (hw *legacyWaiter) watchUntilReady(timeout time.Duration, info *resource.In } return true, nil case watch.Deleted: - slog.Debug("deleted event received", "resource", info.Name) + hw.log().Debug("deleted event received", "resource", info.Name) return true, nil case watch.Error: // Handle error and return with an error. - slog.Error("error event received", "resource", info.Name) + hw.log().Error("error event received", "resource", info.Name) return true, fmt.Errorf("failed to deploy %s", info.Name) default: return false, nil @@ -306,12 +315,12 @@ func (hw *legacyWaiter) waitForJob(obj runtime.Object, name string) (bool, error if c.Type == batchv1.JobComplete && c.Status == "True" { return true, nil } else if c.Type == batchv1.JobFailed && c.Status == "True" { - slog.Error("job failed", "job", name, "reason", c.Reason) + hw.log().Error("job failed", "job", name, "reason", c.Reason) return true, fmt.Errorf("job %s failed: %s", name, c.Reason) } } - slog.Debug("job status update", "job", name, "active", o.Status.Active, "failed", o.Status.Failed, "succeeded", o.Status.Succeeded) + hw.log().Debug("job status update", "job", name, "active", o.Status.Active, "failed", o.Status.Failed, "succeeded", o.Status.Succeeded) return false, nil } @@ -326,17 +335,17 @@ func (hw *legacyWaiter) waitForPodSuccess(obj runtime.Object, name string) (bool switch o.Status.Phase { case corev1.PodSucceeded: - slog.Debug("pod succeeded", "pod", o.Name) + hw.log().Debug("pod succeeded", "pod", o.Name) return true, nil case corev1.PodFailed: - slog.Error("pod failed", "pod", o.Name) + hw.log().Error("pod failed", "pod", o.Name) return true, fmt.Errorf("pod %s failed", o.Name) case corev1.PodPending: - slog.Debug("pod pending", "pod", o.Name) + hw.log().Debug("pod pending", "pod", o.Name) case corev1.PodRunning: - slog.Debug("pod running", "pod", o.Name) + hw.log().Debug("pod running", "pod", o.Name) case corev1.PodUnknown: - slog.Debug("pod unknown", "pod", o.Name) + hw.log().Debug("pod unknown", "pod", o.Name) } return false, nil diff --git a/pkg/registry/client.go b/pkg/registry/client.go index f2bfd13b4..e43eacf64 100644 --- a/pkg/registry/client.go +++ b/pkg/registry/client.go @@ -76,6 +76,7 @@ type ( credentialsStore credentials.Store httpClient *http.Client plainHTTP bool + logger *slog.Logger } // ClientOption allows specifying various settings configurable by the user for overriding the defaults @@ -217,6 +218,20 @@ func ClientOptPlainHTTP() ClientOption { } } +// ClientOptLogger returns a function that sets the logger on client options set +func ClientOptLogger(logger *slog.Logger) ClientOption { + return func(client *Client) { + client.logger = logger + } +} + +func (c *Client) log() *slog.Logger { + if c.logger != nil { + return c.logger + } + return slog.New(slog.DiscardHandler) +} + type ( // LoginOption allows specifying various settings on login LoginOption func(*loginOperation) @@ -229,10 +244,10 @@ type ( // warnIfHostHasPath checks if the host contains a repository path and logs a warning if it does. // Returns true if the host contains a path component (i.e., contains a '/'). -func warnIfHostHasPath(host string) bool { +func warnIfHostHasPath(host string, logger *slog.Logger) bool { if strings.Contains(host, "/") { registryHost := strings.Split(host, "/")[0] - slog.Warn("registry login currently only supports registry hostname, not a repository path", "host", host, "suggested", registryHost) + logger.Warn("registry login currently only supports registry hostname, not a repository path", "host", host, "suggested", registryHost) return true } return false @@ -244,7 +259,7 @@ func (c *Client) Login(host string, options ...LoginOption) error { option(&loginOperation{host, c}) } - warnIfHostHasPath(host) + warnIfHostHasPath(host, c.log()) reg, err := remote.NewRegistry(host) if err != nil { diff --git a/pkg/registry/client_test.go b/pkg/registry/client_test.go index 702dfff69..179481e46 100644 --- a/pkg/registry/client_test.go +++ b/pkg/registry/client_test.go @@ -18,6 +18,7 @@ package registry import ( "io" + "log/slog" "net/http" "net/http/httptest" "path/filepath" @@ -121,6 +122,38 @@ func TestLogin_ResetsForceAttemptOAuth2_OnFailure(t *testing.T) { } } +// TestClientOptLogger verifies that the logger option properly injects a custom logger. +func TestClientOptLogger(t *testing.T) { + t.Parallel() + + var buf strings.Builder + customHandler := slog.NewTextHandler(&buf, &slog.HandlerOptions{Level: slog.LevelWarn}) + logger := slog.New(customHandler) + + credFile := filepath.Join(t.TempDir(), "config.json") + c, err := NewClient( + ClientOptWriter(io.Discard), + ClientOptCredentialsFile(credFile), + ClientOptLogger(logger), + ) + require.NoError(t, err) + + // The client's log() should return the custom logger + if c.log().Handler() != customHandler { + t.Error("expected client logger to use the custom handler") + } + + // Without the option, log() should return a discard handler + c2, err := NewClient( + ClientOptWriter(io.Discard), + ClientOptCredentialsFile(credFile), + ) + require.NoError(t, err) + + // Verify logging to the nil-logger client doesn't panic + c2.log().Warn("should not panic") +} + // TestWarnIfHostHasPath verifies that warnIfHostHasPath correctly detects path components. func TestWarnIfHostHasPath(t *testing.T) { t.Parallel() @@ -159,7 +192,7 @@ func TestWarnIfHostHasPath(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := warnIfHostHasPath(tt.host) + got := warnIfHostHasPath(tt.host, slog.New(slog.DiscardHandler)) if got != tt.wantWarn { t.Errorf("warnIfHostHasPath(%q) = %v, want %v", tt.host, got, tt.wantWarn) } diff --git a/pkg/registry/transport.go b/pkg/registry/transport.go index e4177efb3..40ac3885d 100644 --- a/pkg/registry/transport.go +++ b/pkg/registry/transport.go @@ -48,6 +48,14 @@ const payloadSizeLimit int64 = 16 * 1024 // 16 KiB // request and add hooks to report HTTP tracing events. type LoggingTransport struct { http.RoundTripper + Logger *slog.Logger +} + +func (t *LoggingTransport) log() *slog.Logger { + if t.Logger != nil { + return t.Logger + } + return slog.New(slog.DiscardHandler) } // NewTransport creates and returns a new instance of LoggingTransport @@ -69,14 +77,14 @@ func NewTransport(debug bool) *retry.Transport { func (t *LoggingTransport) RoundTrip(req *http.Request) (resp *http.Response, err error) { id := requestCount.Add(1) - 1 - slog.Debug(req.Method, "id", id, "url", req.URL, "header", logHeader(req.Header)) + t.log().Debug(req.Method, "id", id, "url", req.URL, "header", logHeader(req.Header)) resp, err = t.RoundTripper.RoundTrip(req) if err != nil { - slog.Debug("Response"[:len(req.Method)], "id", id, "error", err) + t.log().Debug("Response"[:len(req.Method)], "id", id, "error", err) } else if resp != nil { - slog.Debug("Response"[:len(req.Method)], "id", id, "status", resp.Status, "header", logHeader(resp.Header), "body", logResponseBody(resp)) + t.log().Debug("Response"[:len(req.Method)], "id", id, "status", resp.Status, "header", logHeader(resp.Header), "body", logResponseBody(resp)) } else { - slog.Debug("Response"[:len(req.Method)], "id", id, "response", "nil") + t.log().Debug("Response"[:len(req.Method)], "id", id, "response", "nil") } return resp, err diff --git a/pkg/release/v1/util/manifest_sorter.go b/pkg/release/v1/util/manifest_sorter.go index 6f7b4ea8b..1cbed7103 100644 --- a/pkg/release/v1/util/manifest_sorter.go +++ b/pkg/release/v1/util/manifest_sorter.go @@ -74,7 +74,10 @@ var events = map[string]release.HookEvent{ // // Files that do not parse into the expected format are simply placed into a map and // returned. -func SortManifests(files map[string]string, _ common.VersionSet, ordering KindSortOrder) ([]*release.Hook, []Manifest, error) { +func SortManifests(files map[string]string, _ common.VersionSet, ordering KindSortOrder, logger *slog.Logger) ([]*release.Hook, []Manifest, error) { + if logger == nil { + logger = slog.New(slog.DiscardHandler) + } result := &result{} var sortedFilePaths []string @@ -101,7 +104,7 @@ func SortManifests(files map[string]string, _ common.VersionSet, ordering KindSo path: filePath, } - if err := manifestFile.sort(result); err != nil { + if err := manifestFile.sort(result, logger); err != nil { return result.hooks, result.generic, err } } @@ -136,7 +139,7 @@ func SortManifests(files map[string]string, _ common.VersionSet, ordering KindSo // metadata: // annotations: // helm.sh/hook-output-log-policy: hook-succeeded,hook-failed -func (file *manifestFile) sort(result *result) error { +func (file *manifestFile) sort(result *result, logger *slog.Logger) error { // Go through manifests in order found in file (function `SplitManifests` creates integer-sortable keys) var sortedEntryKeys []string for entryKey := range file.entries { @@ -196,7 +199,7 @@ func (file *manifestFile) sort(result *result) error { } if isUnknownHook { - slog.Info("skipping unknown hooks", "hookTypes", hookTypes) + logger.Info("skipping unknown hooks", "hookTypes", hookTypes) continue } diff --git a/pkg/release/v1/util/manifest_sorter_test.go b/pkg/release/v1/util/manifest_sorter_test.go index 4360013e5..6245e4ccf 100644 --- a/pkg/release/v1/util/manifest_sorter_test.go +++ b/pkg/release/v1/util/manifest_sorter_test.go @@ -138,7 +138,7 @@ metadata: manifests[o.path] = o.manifest } - hs, generic, err := SortManifests(manifests, nil, InstallOrder) + hs, generic, err := SortManifests(manifests, nil, InstallOrder, nil) if err != nil { t.Fatalf("Unexpected error: %s", err) } diff --git a/pkg/repo/v1/chartrepo.go b/pkg/repo/v1/chartrepo.go index deef7474e..58e752321 100644 --- a/pkg/repo/v1/chartrepo.go +++ b/pkg/repo/v1/chartrepo.go @@ -23,7 +23,6 @@ import ( "encoding/json" "fmt" "io" - "log/slog" "net/url" "os" "path/filepath" @@ -269,7 +268,6 @@ func ResolveReferenceURL(baseURL, refURL string) (string, error) { func (e *Entry) String() string { buf, err := json.Marshal(e) if err != nil { - slog.Error("failed to marshal entry", slog.Any("error", err)) panic(err) } return string(buf) diff --git a/pkg/repo/v1/index.go b/pkg/repo/v1/index.go index ba747d702..d3250281c 100644 --- a/pkg/repo/v1/index.go +++ b/pkg/repo/v1/index.go @@ -90,6 +90,16 @@ type IndexFile struct { // Annotations are additional mappings uninterpreted by Helm. They are made available for // other applications to add information to the index file. Annotations map[string]string `json:"annotations,omitempty"` + + // Logger is the structured logger for this index file. It is not serialized. + Logger *slog.Logger `json:"-"` +} + +func (i IndexFile) log() *slog.Logger { + if i.Logger != nil { + return i.Logger + } + return slog.Default() } // NewIndexFile initializes an index. @@ -154,7 +164,7 @@ func (i IndexFile) MustAdd(md *chart.Metadata, filename, baseURL, digest string) // Deprecated: Use index.MustAdd instead. func (i IndexFile) Add(md *chart.Metadata, filename, baseURL, digest string) { if err := i.MustAdd(md, filename, baseURL, digest); err != nil { - slog.Error("skipping loading invalid entry for chart", "name", md.Name, "version", md.Version, "file", filename, "error", err) + i.log().Error("skipping loading invalid entry for chart", "name", md.Name, "version", md.Version, "file", filename, "error", err) } } @@ -217,7 +227,7 @@ func (i IndexFile) Get(name, version string) (*ChartVersion, error) { if constraint.Check(test) { if len(version) != 0 { - slog.Warn("unable to find exact version requested; falling back to closest available version", "chart", name, "requested", version, "selected", ver.Version) + i.log().Warn("unable to find exact version requested; falling back to closest available version", "chart", name, "requested", version, "selected", ver.Version) } return ver, nil } @@ -359,7 +369,7 @@ func loadIndex(data []byte, source string) (*IndexFile, error) { for name, cvs := range i.Entries { for idx, v := range slices.Backward(cvs) { if v == nil { - slog.Warn("skipping loading invalid entry for chart: empty entry", "name", name, "source", source) + i.log().Warn("skipping loading invalid entry for chart: empty entry", "name", name, "source", source) cvs = append(cvs[:idx], cvs[idx+1:]...) continue } @@ -371,7 +381,7 @@ func loadIndex(data []byte, source string) (*IndexFile, error) { v.APIVersion = chart.APIVersionV1 } if err := v.Validate(); ignoreSkippableChartValidationError(err) != nil { - slog.Warn("skipping loading invalid entry for chart", "name", name, "version", v.Version, "source", source, "error", err) + i.log().Warn("skipping loading invalid entry for chart", "name", name, "version", v.Version, "source", source, "error", err) cvs = append(cvs[:idx], cvs[idx+1:]...) } } diff --git a/pkg/storage/driver/cfgmaps.go b/pkg/storage/driver/cfgmaps.go index 00a0832b3..573788868 100644 --- a/pkg/storage/driver/cfgmaps.go +++ b/pkg/storage/driver/cfgmaps.go @@ -53,11 +53,9 @@ type ConfigMaps struct { // NewConfigMaps initializes a new ConfigMaps wrapping an implementation of // the kubernetes ConfigMapsInterface. func NewConfigMaps(impl corev1.ConfigMapInterface) *ConfigMaps { - c := &ConfigMaps{ + return &ConfigMaps{ impl: impl, } - c.SetLogger(slog.Default().Handler()) - return c } // Name returns the name of the driver. diff --git a/pkg/storage/driver/memory.go b/pkg/storage/driver/memory.go index 7ea4a014a..fc00bc1b7 100644 --- a/pkg/storage/driver/memory.go +++ b/pkg/storage/driver/memory.go @@ -17,7 +17,6 @@ limitations under the License. package driver import ( - "log/slog" "strconv" "strings" "sync" @@ -50,9 +49,7 @@ type Memory struct { // NewMemory initializes a new memory driver. func NewMemory() *Memory { - m := &Memory{cache: map[string]memReleases{}, namespace: "default"} - m.SetLogger(slog.Default().Handler()) - return m + return &Memory{cache: map[string]memReleases{}, namespace: "default"} } // SetNamespace sets a specific namespace in which releases will be accessed. diff --git a/pkg/storage/driver/secrets.go b/pkg/storage/driver/secrets.go index 5e12684df..d4bc8e608 100644 --- a/pkg/storage/driver/secrets.go +++ b/pkg/storage/driver/secrets.go @@ -52,11 +52,9 @@ type Secrets struct { // NewSecrets initializes a new Secrets wrapping an implementation of // the kubernetes SecretsInterface. func NewSecrets(impl corev1.SecretInterface) *Secrets { - s := &Secrets{ + return &Secrets{ impl: impl, } - s.SetLogger(slog.Default().Handler()) - return s } // Name returns the name of the driver. diff --git a/pkg/storage/driver/sql.go b/pkg/storage/driver/sql.go index 21d9f6679..09c94548b 100644 --- a/pkg/storage/driver/sql.go +++ b/pkg/storage/driver/sql.go @@ -296,7 +296,6 @@ func NewSQL(connectionString string, namespace string) (*SQL, error) { } driver.namespace = namespace - driver.SetLogger(slog.Default().Handler()) return driver, nil } diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go index d5d2ea317..77b55c030 100644 --- a/pkg/storage/storage.go +++ b/pkg/storage/storage.go @@ -339,14 +339,9 @@ func Init(d driver.Driver) *Storage { Driver: d, } - var h slog.Handler // Get logger from driver if it implements the LoggerSetterGetter interface if ls, ok := d.(logging.LoggerSetterGetter); ok { - h = ls.Logger().Handler() - } else { - // If the driver does not implement the LoggerSetterGetter interface, set the default logger - h = slog.Default().Handler() + s.SetLogger(ls.Logger().Handler()) } - s.SetLogger(h) return s }