From 89f391e674c0d273754b8c69551b3cb6f80f6235 Mon Sep 17 00:00:00 2001 From: Sowmith Kuppa Date: Fri, 1 Aug 2025 15:28:21 -0400 Subject: [PATCH 1/3] Add debug logging for helm pull command Implemented debug output for HTTP-level logs in the helm pull command by integrating cli.EnvSettings into the chart downloader. This addresses the issue where the --debug flag was ignored during helm pull. Fixes #31098 Signed-off-by: Sowmith Kuppa --- pkg/action/pull.go | 1 + pkg/downloader/chart_downloader.go | 71 +++++++++++++++++++++++++++++- pkg/getter/getter.go | 1 + pkg/getter/httpgetter.go | 18 ++++++-- 4 files changed, 87 insertions(+), 4 deletions(-) diff --git a/pkg/action/pull.go b/pkg/action/pull.go index a2f53af0d..644812ca7 100644 --- a/pkg/action/pull.go +++ b/pkg/action/pull.go @@ -88,6 +88,7 @@ func (p *Pull) Run(chartRef string) (string, error) { RegistryClient: p.cfg.RegistryClient, RepositoryConfig: p.Settings.RepositoryConfig, RepositoryCache: p.Settings.RepositoryCache, + Debug: p.Settings.Debug, } if registry.IsOCI(chartRef) { diff --git a/pkg/downloader/chart_downloader.go b/pkg/downloader/chart_downloader.go index 04c56e614..b3b2d324c 100644 --- a/pkg/downloader/chart_downloader.go +++ b/pkg/downloader/chart_downloader.go @@ -21,6 +21,8 @@ import ( "io" "io/fs" "net/url" + "net/http" + "net/http/httputil" "os" "path/filepath" "strings" @@ -69,11 +71,44 @@ type ChartDownloader struct { Getters getter.Providers // Options provide parameters to be passed along to the Getter being initialized. Options []getter.Option + Debug bool //Added to capture the --debug flag RegistryClient *registry.Client RepositoryConfig string RepositoryCache string } +type debugTransport struct { + *http.Transport + out io.Writer +} + +func (t *debugTransport) RoundTrip(req *http.Request) (*http.Response, error) { + + fmt.Fprintf(t.out, "DEBUG: HTTP request to %s\n", req.URL.String()) + // Log the request + reqDump, err := httputil.DumpRequestOut(req, false) + if err == nil { + fmt.Fprintf(t.out, "%s\n", reqDump) + } + // Perform the request + resp, err := t.Transport.RoundTrip(req) + if err != nil { + fmt.Fprintf(t.out, "HTTP request failed: %v\n", err) + return nil, err + } + // Log the response + respDump, err := httputil.DumpResponse(resp, false) + if err == nil { + fmt.Fprintf(t.out, "HTTP Response: \n%s\n", respDump) + } + if resp.StatusCode >= 300 && resp.StatusCode < 400 { + location, _ := resp.Header["Location"] + fmt.Fprintf(t.out, "DEBUG: Redirect to: %v\n", location) + } + + return resp, err +} + // DownloadTo retrieves a chart. Depending on the settings, it may also download a provenance file. // // If Verify is set to VerifyNever, the verification will be nil. @@ -88,18 +123,37 @@ type ChartDownloader struct { func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *provenance.Verification, error) { u, err := c.ResolveChartVersion(ref, version) if err != nil { + fmt.Fprintf(c.Out, "DEBUG: Failed to resolve chart version: %v\n", err) return "", nil, err } - + fmt.Fprintf(c.Out, "DEBUG: Resolved chart URL: %s\n", u.String()) g, err := c.Getters.ByScheme(u.Scheme) if err != nil { + fmt.Fprintf(c.Out, "DEBUG: Failed to get getter for scheme %s: %v\n", u.Scheme, err) return "", nil, err } + fmt.Fprintf(c.Out, "DEBUG: Using getter for scheme: %s\n", u.Scheme) c.Options = append(c.Options, getter.WithAcceptHeader("application/gzip,application/octet-stream")) + // If debug is enabled, wrap the getter's HTTP client with a debug transport + if c.Debug { + dt := &debugTransport{ + Transport: http.DefaultTransport.(*http.Transport).Clone(), + out: c.Out, + } + dt.Transport.DisableKeepAlives = true + c.Options = append(c.Options, getter.WithClient(&http.Client{ + Transport: dt, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + fmt.Fprintf(c.Out, "DEBUG: Following redirect to %s\n", req.URL.String()) + return nil + }, + })) + } data, err := g.Get(u.String(), c.Options...) if err != nil { + fmt.Fprintf(c.Out, "DEBUG: Failed to fetch chart: %v\n", err) return "", nil, err } @@ -117,6 +171,21 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven // If provenance is requested, verify it. ver := &provenance.Verification{} if c.Verify > VerifyNever { + // If debug is enabled, use the same debug transport for provenance file + if c.Debug { + dt := &debugTransport{ + Transport: http.DefaultTransport.(*http.Transport).Clone(), + out: c.Out, + } + dt.Transport.DisableKeepAlives = true + c.Options = append(c.Options, getter.WithClient(&http.Client{ + Transport: dt, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + fmt.Fprintf(c.Out, "DEBUG: Following redirect to %s\n", req.URL.String()) + return nil + }, + })) + } body, err := g.Get(u.String() + ".prov") if err != nil { if c.Verify == VerifyAlways { diff --git a/pkg/getter/getter.go b/pkg/getter/getter.go index 5605e043f..3637caedd 100644 --- a/pkg/getter/getter.go +++ b/pkg/getter/getter.go @@ -47,6 +47,7 @@ type options struct { registryClient *registry.Client timeout time.Duration transport *http.Transport + client *http.Client } // Option allows specifying various settings configurable by the user for overriding the defaults diff --git a/pkg/getter/httpgetter.go b/pkg/getter/httpgetter.go index 925df201e..e98379fb4 100644 --- a/pkg/getter/httpgetter.go +++ b/pkg/getter/httpgetter.go @@ -35,6 +35,7 @@ type HTTPGetter struct { once sync.Once } + // Get performs a Get from repo.Getter and returns the body. func (g *HTTPGetter) Get(href string, options ...Option) (*bytes.Buffer, error) { for _, opt := range options { @@ -80,9 +81,13 @@ func (g *HTTPGetter) get(href string) (*bytes.Buffer, error) { } } - client, err := g.httpClient() - if err != nil { - return nil, err + client := g.opts.client + if client == nil { + var err error + client, err = g.httpClient() + if err != nil { + return nil, err + } } resp, err := client.Do(req) @@ -155,3 +160,10 @@ func (g *HTTPGetter) httpClient() (*http.Client, error) { return client, nil } + +// WithClient sets the HTTP client for the getter +func WithClient(client *http.Client) Option { + return func(opts *options) { + opts.client = client + } +} From df457d5c56249c53d09831421ecd672c31800e87 Mon Sep 17 00:00:00 2001 From: Sowmith Kuppa Date: Mon, 4 Aug 2025 15:57:26 -0400 Subject: [PATCH 2/3] Add debug logging for helm pull command Implemented debug output feature for HTTP-level logs in the helm pull command. This addresses the issue where the --debug flag was ignored during helm pull. Fixes #31098 Signed-off-by: Sowmith Kuppa --- pkg/downloader/chart_downloader.go | 97 ++++++++++++++++++------------ pkg/getter/getter.go | 2 +- pkg/getter/httpgetter.go | 1 - 3 files changed, 59 insertions(+), 41 deletions(-) diff --git a/pkg/downloader/chart_downloader.go b/pkg/downloader/chart_downloader.go index b3b2d324c..f7eb7d4b0 100644 --- a/pkg/downloader/chart_downloader.go +++ b/pkg/downloader/chart_downloader.go @@ -20,9 +20,9 @@ import ( "fmt" "io" "io/fs" - "net/url" "net/http" "net/http/httputil" + "net/url" "os" "path/filepath" "strings" @@ -71,7 +71,7 @@ type ChartDownloader struct { Getters getter.Providers // Options provide parameters to be passed along to the Getter being initialized. Options []getter.Option - Debug bool //Added to capture the --debug flag + Debug bool //Added to capture the --debug flag RegistryClient *registry.Client RepositoryConfig string RepositoryCache string @@ -79,33 +79,38 @@ type ChartDownloader struct { type debugTransport struct { *http.Transport - out io.Writer + out io.Writer + debug bool } func (t *debugTransport) RoundTrip(req *http.Request) (*http.Response, error) { - - fmt.Fprintf(t.out, "DEBUG: HTTP request to %s\n", req.URL.String()) - // Log the request - reqDump, err := httputil.DumpRequestOut(req, false) - if err == nil { - fmt.Fprintf(t.out, "%s\n", reqDump) + if t.debug { + fmt.Fprintf(t.out, "DEBUG: HTTP request to %s\n", req.URL.String()) + // Log the request + reqDump, err := httputil.DumpRequestOut(req, false) + if err == nil { + fmt.Fprintf(t.out, "%s\n", reqDump) + } } // Perform the request resp, err := t.Transport.RoundTrip(req) if err != nil { - fmt.Fprintf(t.out, "HTTP request failed: %v\n", err) + if t.debug { + fmt.Fprintf(t.out, "HTTP request failed: %v\n", err) + } return nil, err } // Log the response - respDump, err := httputil.DumpResponse(resp, false) - if err == nil { - fmt.Fprintf(t.out, "HTTP Response: \n%s\n", respDump) + if t.debug { + respDump, err := httputil.DumpResponse(resp, false) + if err == nil { + fmt.Fprintf(t.out, "HTTP Response: \n%s\n", respDump) + } + if resp.StatusCode >= 300 && resp.StatusCode < 400 { + location := resp.Header["Location"] + fmt.Fprintf(t.out, "DEBUG: Redirect to: %v\n", location) + } } - if resp.StatusCode >= 300 && resp.StatusCode < 400 { - location, _ := resp.Header["Location"] - fmt.Fprintf(t.out, "DEBUG: Redirect to: %v\n", location) - } - return resp, err } @@ -123,17 +128,24 @@ func (t *debugTransport) RoundTrip(req *http.Request) (*http.Response, error) { func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *provenance.Verification, error) { u, err := c.ResolveChartVersion(ref, version) if err != nil { - fmt.Fprintf(c.Out, "DEBUG: Failed to resolve chart version: %v\n", err) + if c.Debug { + fmt.Fprintf(c.Out, "DEBUG: Failed to resolve chart version: %v\n", err) + } return "", nil, err } - fmt.Fprintf(c.Out, "DEBUG: Resolved chart URL: %s\n", u.String()) + if c.Debug { + fmt.Fprintf(c.Out, "DEBUG: Resolved chart URL: %s\n", u.String()) + } g, err := c.Getters.ByScheme(u.Scheme) if err != nil { - fmt.Fprintf(c.Out, "DEBUG: Failed to get getter for scheme %s: %v\n", u.Scheme, err) + if c.Debug { + fmt.Fprintf(c.Out, "DEBUG: Failed to get getter for scheme %s: %v\n", u.Scheme, err) + } return "", nil, err } - fmt.Fprintf(c.Out, "DEBUG: Using getter for scheme: %s\n", u.Scheme) - + if c.Debug { + fmt.Fprintf(c.Out, "DEBUG: Using getter for scheme: %s\n", u.Scheme) + } c.Options = append(c.Options, getter.WithAcceptHeader("application/gzip,application/octet-stream")) // If debug is enabled, wrap the getter's HTTP client with a debug transport @@ -141,19 +153,24 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven dt := &debugTransport{ Transport: http.DefaultTransport.(*http.Transport).Clone(), out: c.Out, + debug: c.Debug, } - dt.Transport.DisableKeepAlives = true + dt.DisableKeepAlives = true c.Options = append(c.Options, getter.WithClient(&http.Client{ - Transport: dt, - CheckRedirect: func(req *http.Request, via []*http.Request) error { - fmt.Fprintf(c.Out, "DEBUG: Following redirect to %s\n", req.URL.String()) - return nil - }, - })) + Transport: dt, + CheckRedirect: func(req *http.Request, _ []*http.Request) error { + if c.Debug { + fmt.Fprintf(c.Out, "DEBUG: Following redirect to %s\n", req.URL.String()) + } + return nil + }, + })) } data, err := g.Get(u.String(), c.Options...) if err != nil { - fmt.Fprintf(c.Out, "DEBUG: Failed to fetch chart: %v\n", err) + if c.Debug { + fmt.Fprintf(c.Out, "DEBUG: Failed to fetch chart: %v\n", err) + } return "", nil, err } @@ -175,16 +192,18 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven if c.Debug { dt := &debugTransport{ Transport: http.DefaultTransport.(*http.Transport).Clone(), - out: c.Out, + out: c.Out, } - dt.Transport.DisableKeepAlives = true + dt.DisableKeepAlives = true c.Options = append(c.Options, getter.WithClient(&http.Client{ - Transport: dt, - CheckRedirect: func(req *http.Request, via []*http.Request) error { - fmt.Fprintf(c.Out, "DEBUG: Following redirect to %s\n", req.URL.String()) - return nil - }, - })) + Transport: dt, + CheckRedirect: func(req *http.Request, _ []*http.Request) error { + if c.Debug { + fmt.Fprintf(c.Out, "DEBUG: Following redirect to %s\n", req.URL.String()) + } + return nil + }, + })) } body, err := g.Get(u.String() + ".prov") if err != nil { diff --git a/pkg/getter/getter.go b/pkg/getter/getter.go index 3637caedd..f2f7eb5ca 100644 --- a/pkg/getter/getter.go +++ b/pkg/getter/getter.go @@ -47,7 +47,7 @@ type options struct { registryClient *registry.Client timeout time.Duration transport *http.Transport - client *http.Client + client *http.Client } // Option allows specifying various settings configurable by the user for overriding the defaults diff --git a/pkg/getter/httpgetter.go b/pkg/getter/httpgetter.go index e98379fb4..da85d3723 100644 --- a/pkg/getter/httpgetter.go +++ b/pkg/getter/httpgetter.go @@ -35,7 +35,6 @@ type HTTPGetter struct { once sync.Once } - // Get performs a Get from repo.Getter and returns the body. func (g *HTTPGetter) Get(href string, options ...Option) (*bytes.Buffer, error) { for _, opt := range options { From d5ea54d2a1215f11e65e9c3d49010ce4a77623cc Mon Sep 17 00:00:00 2001 From: Sowmith Kuppa Date: Wed, 6 Aug 2025 22:51:55 -0400 Subject: [PATCH 3/3] Add detailed slog debug logs for helm pull with --debug Fixes #31098 Signed-off-by: Sowmith Kuppa --- pkg/action/pull.go | 1 - pkg/downloader/chart_downloader.go | 82 ++++++++++++++---------------- pkg/getter/getter.go | 14 +++++ pkg/getter/httpgetter.go | 12 +++++ 4 files changed, 63 insertions(+), 46 deletions(-) diff --git a/pkg/action/pull.go b/pkg/action/pull.go index 644812ca7..a2f53af0d 100644 --- a/pkg/action/pull.go +++ b/pkg/action/pull.go @@ -88,7 +88,6 @@ func (p *Pull) Run(chartRef string) (string, error) { RegistryClient: p.cfg.RegistryClient, RepositoryConfig: p.Settings.RepositoryConfig, RepositoryCache: p.Settings.RepositoryCache, - Debug: p.Settings.Debug, } if registry.IsOCI(chartRef) { diff --git a/pkg/downloader/chart_downloader.go b/pkg/downloader/chart_downloader.go index f7eb7d4b0..c7c7c2e3a 100644 --- a/pkg/downloader/chart_downloader.go +++ b/pkg/downloader/chart_downloader.go @@ -20,6 +20,7 @@ import ( "fmt" "io" "io/fs" + "log/slog" "net/http" "net/http/httputil" "net/url" @@ -70,8 +71,8 @@ type ChartDownloader struct { // Getter collection for the operation Getters getter.Providers // Options provide parameters to be passed along to the Getter being initialized. - Options []getter.Option - Debug bool //Added to capture the --debug flag + Options []getter.Option + // Debug bool //Added to capture the --debug flag RegistryClient *registry.Client RepositoryConfig string RepositoryCache string @@ -79,36 +80,37 @@ type ChartDownloader struct { type debugTransport struct { *http.Transport - out io.Writer - debug bool + out io.Writer } func (t *debugTransport) RoundTrip(req *http.Request) (*http.Response, error) { - if t.debug { - fmt.Fprintf(t.out, "DEBUG: HTTP request to %s\n", req.URL.String()) + debug := os.Getenv("HELM_DEBUG") == "true" + + if debug { + slog.Debug("HTTP request", "url", req.URL.String()) // Log the request reqDump, err := httputil.DumpRequestOut(req, false) if err == nil { - fmt.Fprintf(t.out, "%s\n", reqDump) + slog.Debug("HTTP request dump", "dump", string(reqDump)) } } // Perform the request resp, err := t.Transport.RoundTrip(req) if err != nil { - if t.debug { - fmt.Fprintf(t.out, "HTTP request failed: %v\n", err) + if debug { + slog.Debug("HTTP request failed", "error", err) } return nil, err } // Log the response - if t.debug { + if debug { respDump, err := httputil.DumpResponse(resp, false) if err == nil { - fmt.Fprintf(t.out, "HTTP Response: \n%s\n", respDump) + slog.Debug("HTTP response dump", "dump", string(respDump)) } if resp.StatusCode >= 300 && resp.StatusCode < 400 { location := resp.Header["Location"] - fmt.Fprintf(t.out, "DEBUG: Redirect to: %v\n", location) + slog.Debug("HTTP redirect", "location", location) } } return resp, err @@ -126,51 +128,43 @@ func (t *debugTransport) RoundTrip(req *http.Request) (*http.Response, error) { // Returns a string path to the location where the file was downloaded and a verification // (if provenance was verified), or an error if something bad happened. func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *provenance.Verification, error) { - u, err := c.ResolveChartVersion(ref, version) - if err != nil { - if c.Debug { - fmt.Fprintf(c.Out, "DEBUG: Failed to resolve chart version: %v\n", err) - } - return "", nil, err - } - if c.Debug { - fmt.Fprintf(c.Out, "DEBUG: Resolved chart URL: %s\n", u.String()) - } - g, err := c.Getters.ByScheme(u.Scheme) - if err != nil { - if c.Debug { - fmt.Fprintf(c.Out, "DEBUG: Failed to get getter for scheme %s: %v\n", u.Scheme, err) - } - return "", nil, err - } - if c.Debug { - fmt.Fprintf(c.Out, "DEBUG: Using getter for scheme: %s\n", u.Scheme) - } - c.Options = append(c.Options, getter.WithAcceptHeader("application/gzip,application/octet-stream")) + debug := os.Getenv("HELM_DEBUG") == "true" // If debug is enabled, wrap the getter's HTTP client with a debug transport - if c.Debug { + if debug { dt := &debugTransport{ Transport: http.DefaultTransport.(*http.Transport).Clone(), out: c.Out, - debug: c.Debug, } dt.DisableKeepAlives = true c.Options = append(c.Options, getter.WithClient(&http.Client{ Transport: dt, CheckRedirect: func(req *http.Request, _ []*http.Request) error { - if c.Debug { - fmt.Fprintf(c.Out, "DEBUG: Following redirect to %s\n", req.URL.String()) - } + slog.Debug("Following redirect", "url", req.URL.String()) return nil }, })) } + + c.Options = append(c.Options, getter.WithAcceptHeader("application/gzip,application/octet-stream")) + + u, err := c.ResolveChartVersion(ref, version) + if err != nil { + slog.Debug("Failed to resolve chart version", "error", err) + return "", nil, err + } + slog.Debug("Resolved chart URL", "url", u.String()) + + g, err := c.Getters.ByScheme(u.Scheme) + if err != nil { + slog.Debug("Failed to get getter for scheme", "scheme", u.Scheme, "error", err) + return "", nil, err + } + slog.Debug("Using getter for scheme", "scheme", u.Scheme) + data, err := g.Get(u.String(), c.Options...) if err != nil { - if c.Debug { - fmt.Fprintf(c.Out, "DEBUG: Failed to fetch chart: %v\n", err) - } + slog.Debug("Failed to fetch chart", "error", err) return "", nil, err } @@ -189,7 +183,7 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven ver := &provenance.Verification{} if c.Verify > VerifyNever { // If debug is enabled, use the same debug transport for provenance file - if c.Debug { + if debug { dt := &debugTransport{ Transport: http.DefaultTransport.(*http.Transport).Clone(), out: c.Out, @@ -198,9 +192,7 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven c.Options = append(c.Options, getter.WithClient(&http.Client{ Transport: dt, CheckRedirect: func(req *http.Request, _ []*http.Request) error { - if c.Debug { - fmt.Fprintf(c.Out, "DEBUG: Following redirect to %s\n", req.URL.String()) - } + slog.Debug("Following redirect for prov file", "url", req.URL.String()) return nil }, })) diff --git a/pkg/getter/getter.go b/pkg/getter/getter.go index f2f7eb5ca..b1eb1ef12 100644 --- a/pkg/getter/getter.go +++ b/pkg/getter/getter.go @@ -19,7 +19,9 @@ package getter import ( "bytes" "fmt" + "log/slog" "net/http" + "os" "slices" "time" @@ -192,6 +194,18 @@ const ( var defaultOptions = []Option{WithTimeout(time.Second * DefaultHTTPTimeout)} +func init() { + level := slog.LevelInfo + if os.Getenv("HELM_DEBUG") == "true" { + level = slog.LevelDebug + } + + logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{ + Level: level, + })) + slog.SetDefault(logger) +} + func Getters(extraOpts ...Option) Providers { return Providers{ Provider{ diff --git a/pkg/getter/httpgetter.go b/pkg/getter/httpgetter.go index da85d3723..ae951b37f 100644 --- a/pkg/getter/httpgetter.go +++ b/pkg/getter/httpgetter.go @@ -20,8 +20,10 @@ import ( "crypto/tls" "fmt" "io" + "log/slog" "net/http" "net/url" + "os" "sync" "helm.sh/helm/v4/internal/tlsutil" @@ -46,6 +48,8 @@ func (g *HTTPGetter) Get(href string, options ...Option) (*bytes.Buffer, error) func (g *HTTPGetter) get(href string) (*bytes.Buffer, error) { // Set a helm specific user agent so that a repo server and metrics can // separate helm calls from other tools interacting with repos. + debug := os.Getenv("HELM_DEBUG") == "true" + req, err := http.NewRequest(http.MethodGet, href, nil) if err != nil { return nil, err @@ -89,12 +93,20 @@ func (g *HTTPGetter) get(href string) (*bytes.Buffer, error) { } } + if debug { + slog.Debug("HTTP GET request", "url", href) + } + resp, err := client.Do(req) if err != nil { + if debug { + slog.Debug("HTTP GET failed", "error", err) + } return nil, err } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { + slog.Debug("HTTP GET non-OK response", "url", href, "status", resp.Status) return nil, fmt.Errorf("failed to fetch %s : %s", href, resp.Status) }