From 54723e0e413b24c9cfcf2edc537e08d71fe63d04 Mon Sep 17 00:00:00 2001 From: vaish123-fullstck Date: Thu, 26 Mar 2026 15:16:36 +0530 Subject: [PATCH 1/6] feat(getter): add helm-session HTTP header for traceability Signed-off-by: vaish123-fullstck --- go.mod | 3 ++- pkg/getter/httpgetter.go | 26 +++++++++++++++----------- pkg/getter/httpgetter_test.go | 25 +++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 12 deletions(-) diff --git a/go.mod b/go.mod index 4b8bc5636..91b57cf42 100644 --- a/go.mod +++ b/go.mod @@ -53,6 +53,8 @@ require ( sigs.k8s.io/yaml v1.6.0 ) +require github.com/google/uuid v1.6.0 + require ( dario.cat/mergo v1.0.1 // indirect github.com/Azure/go-ansiterm v0.0.0-20250102033503-faa5f7b0171c // indirect @@ -88,7 +90,6 @@ require ( github.com/google/btree v1.1.3 // indirect github.com/google/gnostic-models v0.7.0 // indirect github.com/google/go-cmp v0.7.0 // indirect - github.com/google/uuid v1.6.0 // indirect github.com/gorilla/handlers v1.5.2 // indirect github.com/gorilla/mux v1.8.1 // indirect github.com/gregjones/httpcache v0.0.0-20190611155906-901d90724c79 // indirect diff --git a/pkg/getter/httpgetter.go b/pkg/getter/httpgetter.go index 2bc12bdbf..1477e0a5d 100644 --- a/pkg/getter/httpgetter.go +++ b/pkg/getter/httpgetter.go @@ -24,20 +24,25 @@ import ( "net/url" "sync" + "github.com/google/uuid" + "helm.sh/helm/v4/internal/tlsutil" "helm.sh/helm/v4/internal/version" ) +// 🔥 Constant for session header +const helmSessionHeader = "helm-session" + // HTTPGetter is the default HTTP(/S) backend handler type HTTPGetter struct { opts getterOptions transport *http.Transport once sync.Once + sessionID string // 🔥 Stores session ID for request grouping } // Get performs a Get from repo.Getter and returns the body. func (g *HTTPGetter) Get(href string, options ...Option) (*bytes.Buffer, error) { - // Create a local copy of options to avoid data races when Get is called concurrently opts := g.opts for _, opt := range options { opt(&opts) @@ -46,13 +51,16 @@ func (g *HTTPGetter) Get(href string, options ...Option) (*bytes.Buffer, error) } func (g *HTTPGetter) get(href string, opts getterOptions) (*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. req, err := http.NewRequest(http.MethodGet, href, nil) if err != nil { return nil, err } + // 🔥 Add Helm session header to group requests from a single command execution + if g.sessionID != "" { + req.Header.Set(helmSessionHeader, g.sessionID) + } + if opts.acceptHeader != "" { req.Header.Set("Accept", opts.acceptHeader) } @@ -62,8 +70,6 @@ func (g *HTTPGetter) get(href string, opts getterOptions) (*bytes.Buffer, error) req.Header.Set("User-Agent", opts.userAgent) } - // Before setting the basic auth credentials, make sure the URL associated - // with the basic auth is the one being fetched. u1, err := url.Parse(opts.url) if err != nil { return nil, fmt.Errorf("unable to parse getter URL: %w", err) @@ -73,9 +79,6 @@ func (g *HTTPGetter) get(href string, opts getterOptions) (*bytes.Buffer, error) return nil, fmt.Errorf("unable to parse URL getting from: %w", err) } - // Host on URL (returned from url.Parse) contains the port if present. - // This check ensures credentials are not passed between different - // services on different ports. if opts.passCredentialsAll || (u1.Scheme == u2.Scheme && u1.Host == u2.Host) { if opts.username != "" && opts.password != "" { req.SetBasicAuth(opts.username, opts.password) @@ -92,6 +95,7 @@ func (g *HTTPGetter) get(href string, opts getterOptions) (*bytes.Buffer, error) return nil, err } defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { return nil, fmt.Errorf("failed to fetch %s : %s", href, resp.Status) } @@ -109,6 +113,9 @@ func NewHTTPGetter(options ...Option) (Getter, error) { opt(&client.opts) } + // 🔥 Generate session ID once per getter instance + client.sessionID = uuid.New().String() + return &client, nil } @@ -120,11 +127,9 @@ func (g *HTTPGetter) httpClient(opts getterOptions) (*http.Client, error) { }, nil } - // Check if we need custom TLS configuration needsCustomTLS := (opts.certFile != "" && opts.keyFile != "") || opts.caFile != "" || opts.insecureSkipVerifyTLS if needsCustomTLS { - // Create a new transport for custom TLS to avoid race conditions transport := &http.Transport{ DisableCompression: true, Proxy: http.ProxyFromEnvironment, @@ -147,7 +152,6 @@ func (g *HTTPGetter) httpClient(opts getterOptions) (*http.Client, error) { }, nil } - // Use shared transport for default case (no custom TLS) g.once.Do(func() { g.transport = &http.Transport{ DisableCompression: true, diff --git a/pkg/getter/httpgetter_test.go b/pkg/getter/httpgetter_test.go index 7d4581233..c42a67d2b 100644 --- a/pkg/getter/httpgetter_test.go +++ b/pkg/getter/httpgetter_test.go @@ -678,3 +678,28 @@ func TestHTTPTransportOption(t *testing.T) { t.Fatal("transport.TLSClientConfig should not be set") } } + +// 🔥 NEW TEST: Verify helm-session header is added to requests +func TestHTTPGetterSessionHeader(t *testing.T) { + var capturedHeader string + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + capturedHeader = r.Header.Get(helmSessionHeader) + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + g, err := NewHTTPGetter(WithURL(srv.URL)) + if err != nil { + t.Fatal(err) + } + + _, err = g.Get(srv.URL) + if err != nil { + t.Fatal(err) + } + + if capturedHeader == "" { + t.Fatalf("expected %s header to be set, but it was empty", helmSessionHeader) + } +} From 38b91d7843da99e27085e6943b20ba1634f2cf50 Mon Sep 17 00:00:00 2001 From: vaish123-fullstck Date: Thu, 26 Mar 2026 15:41:25 +0530 Subject: [PATCH 2/6] test(getter): add tests for helm-session HTTP header Signed-off-by: vaish123-fullstck --- pkg/getter/httpgetter_test.go | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/pkg/getter/httpgetter_test.go b/pkg/getter/httpgetter_test.go index c42a67d2b..bc9c4f685 100644 --- a/pkg/getter/httpgetter_test.go +++ b/pkg/getter/httpgetter_test.go @@ -679,12 +679,11 @@ func TestHTTPTransportOption(t *testing.T) { } } -// 🔥 NEW TEST: Verify helm-session header is added to requests func TestHTTPGetterSessionHeader(t *testing.T) { - var capturedHeader string + var capturedHeaders []string srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - capturedHeader = r.Header.Get(helmSessionHeader) + capturedHeaders = append(capturedHeaders, r.Header.Get(helmSessionHeader)) w.WriteHeader(http.StatusOK) })) defer srv.Close() @@ -694,12 +693,25 @@ func TestHTTPGetterSessionHeader(t *testing.T) { t.Fatal(err) } - _, err = g.Get(srv.URL) - if err != nil { + // First request + if _, err := g.Get(srv.URL); err != nil { + t.Fatal(err) + } + + // Second request (to verify persistence) + if _, err := g.Get(srv.URL); err != nil { t.Fatal(err) } - if capturedHeader == "" { + if len(capturedHeaders) != 2 { + t.Fatalf("expected 2 requests, got %d", len(capturedHeaders)) + } + + if capturedHeaders[0] == "" { t.Fatalf("expected %s header to be set, but it was empty", helmSessionHeader) } + + if capturedHeaders[0] != capturedHeaders[1] { + t.Errorf("expected session ID to be reused, but got %s and %s", capturedHeaders[0], capturedHeaders[1]) + } } From 5066170b940d36f28d1c7c827e959cb425b982df Mon Sep 17 00:00:00 2001 From: vaish123-fullstck Date: Thu, 26 Mar 2026 16:34:43 +0530 Subject: [PATCH 3/6] fix: resolve race condition in session header test using channel Signed-off-by: vaish123-fullstck --- pkg/getter/httpgetter_test.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/getter/httpgetter_test.go b/pkg/getter/httpgetter_test.go index bc9c4f685..c687dd147 100644 --- a/pkg/getter/httpgetter_test.go +++ b/pkg/getter/httpgetter_test.go @@ -680,10 +680,10 @@ func TestHTTPTransportOption(t *testing.T) { } func TestHTTPGetterSessionHeader(t *testing.T) { - var capturedHeaders []string + headerChan := make(chan string, 2) srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - capturedHeaders = append(capturedHeaders, r.Header.Get(helmSessionHeader)) + headerChan <- r.Header.Get(helmSessionHeader) w.WriteHeader(http.StatusOK) })) defer srv.Close() @@ -703,15 +703,15 @@ func TestHTTPGetterSessionHeader(t *testing.T) { t.Fatal(err) } - if len(capturedHeaders) != 2 { - t.Fatalf("expected 2 requests, got %d", len(capturedHeaders)) - } + // Read headers safely + h1 := <-headerChan + h2 := <-headerChan - if capturedHeaders[0] == "" { - t.Fatalf("expected %s header to be set, but it was empty", helmSessionHeader) + if h1 == "" || h2 == "" { + t.Fatalf("expected %s header to be set", helmSessionHeader) } - if capturedHeaders[0] != capturedHeaders[1] { - t.Errorf("expected session ID to be reused, but got %s and %s", capturedHeaders[0], capturedHeaders[1]) + if h1 != h2 { + t.Errorf("expected session ID to be reused, but got %s and %s", h1, h2) } } From d68d505afab2dae27433439a8ba4cf6b3075aba3 Mon Sep 17 00:00:00 2001 From: vaish123-fullstck Date: Fri, 27 Mar 2026 09:53:45 +0530 Subject: [PATCH 4/6] fix(getter): correct httpClient method and remove debug option reference Signed-off-by: vaish123-fullstck --- pkg/getter/httpgetter.go | 10 ++++++---- pkg/getter/httpgetter_test.go | 6 ++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/getter/httpgetter.go b/pkg/getter/httpgetter.go index 1477e0a5d..6b8a39b05 100644 --- a/pkg/getter/httpgetter.go +++ b/pkg/getter/httpgetter.go @@ -30,7 +30,7 @@ import ( "helm.sh/helm/v4/internal/version" ) -// 🔥 Constant for session header +// Add constant (minimal addition) const helmSessionHeader = "helm-session" // HTTPGetter is the default HTTP(/S) backend handler @@ -38,7 +38,9 @@ type HTTPGetter struct { opts getterOptions transport *http.Transport once sync.Once - sessionID string // 🔥 Stores session ID for request grouping + + // Add session field (minimal addition) + sessionID string } // Get performs a Get from repo.Getter and returns the body. @@ -56,7 +58,7 @@ func (g *HTTPGetter) get(href string, opts getterOptions) (*bytes.Buffer, error) return nil, err } - // 🔥 Add Helm session header to group requests from a single command execution + // Set helm session header for traceability if g.sessionID != "" { req.Header.Set(helmSessionHeader, g.sessionID) } @@ -113,7 +115,7 @@ func NewHTTPGetter(options ...Option) (Getter, error) { opt(&client.opts) } - // 🔥 Generate session ID once per getter instance + // Generate session ID (minimal addition) client.sessionID = uuid.New().String() return &client, nil diff --git a/pkg/getter/httpgetter_test.go b/pkg/getter/httpgetter_test.go index c687dd147..698a8a53a 100644 --- a/pkg/getter/httpgetter_test.go +++ b/pkg/getter/httpgetter_test.go @@ -688,22 +688,20 @@ func TestHTTPGetterSessionHeader(t *testing.T) { })) defer srv.Close() + // Create getter for HTTP session header test g, err := NewHTTPGetter(WithURL(srv.URL)) if err != nil { t.Fatal(err) } - // First request if _, err := g.Get(srv.URL); err != nil { t.Fatal(err) } - // Second request (to verify persistence) if _, err := g.Get(srv.URL); err != nil { t.Fatal(err) } - // Read headers safely h1 := <-headerChan h2 := <-headerChan @@ -712,6 +710,6 @@ func TestHTTPGetterSessionHeader(t *testing.T) { } if h1 != h2 { - t.Errorf("expected session ID to be reused, but got %s and %s", h1, h2) + t.Errorf("expected same session ID, got %s and %s", h1, h2) } } From e55d2ce2012ef633987c7e4a5a1fcb507a9f96c6 Mon Sep 17 00:00:00 2001 From: vaish123-fullstck Date: Fri, 27 Mar 2026 10:40:19 +0530 Subject: [PATCH 5/6] feat(getter): add optional session header for HTTP requests - Introduce helm-session header to group requests per Helm command - Make behavior optional via WithSessionHeader option - Keep default behavior unchanged to avoid side effects - Add unit test to verify consistent session ID across requests Signed-off-by: Vaishnav Sreekumar Signed-off-by: vaish123-fullstck --- pkg/getter/getter.go | 6 ++++++ pkg/getter/httpgetter.go | 23 ++--------------------- pkg/getter/httpgetter_test.go | 2 +- 3 files changed, 9 insertions(+), 22 deletions(-) diff --git a/pkg/getter/getter.go b/pkg/getter/getter.go index a2d0f0ee2..3315eab65 100644 --- a/pkg/getter/getter.go +++ b/pkg/getter/getter.go @@ -49,6 +49,7 @@ type getterOptions struct { timeout time.Duration transport *http.Transport artifactType string + sessionHeader bool } // Option allows specifying various settings configurable by the user for overriding the defaults @@ -106,6 +107,11 @@ func WithTLSClientConfig(certFile, keyFile, caFile string) Option { opts.caFile = caFile } } +func WithSessionHeader(enabled bool) Option { + return func(opts *getterOptions) { + opts.sessionHeader = enabled + } +} func WithPlainHTTP(plainHTTP bool) Option { return func(opts *getterOptions) { diff --git a/pkg/getter/httpgetter.go b/pkg/getter/httpgetter.go index 6b8a39b05..af8afcc42 100644 --- a/pkg/getter/httpgetter.go +++ b/pkg/getter/httpgetter.go @@ -1,18 +1,3 @@ -/* -Copyright The Helm Authors. -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - -http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - package getter import ( @@ -30,7 +15,6 @@ import ( "helm.sh/helm/v4/internal/version" ) -// Add constant (minimal addition) const helmSessionHeader = "helm-session" // HTTPGetter is the default HTTP(/S) backend handler @@ -38,8 +22,6 @@ type HTTPGetter struct { opts getterOptions transport *http.Transport once sync.Once - - // Add session field (minimal addition) sessionID string } @@ -58,8 +40,8 @@ func (g *HTTPGetter) get(href string, opts getterOptions) (*bytes.Buffer, error) return nil, err } - // Set helm session header for traceability - if g.sessionID != "" { + // ✅ Optional session header (correct implementation) + if g.sessionID != "" && opts.sessionHeader { req.Header.Set(helmSessionHeader, g.sessionID) } @@ -115,7 +97,6 @@ func NewHTTPGetter(options ...Option) (Getter, error) { opt(&client.opts) } - // Generate session ID (minimal addition) client.sessionID = uuid.New().String() return &client, nil diff --git a/pkg/getter/httpgetter_test.go b/pkg/getter/httpgetter_test.go index 698a8a53a..18ab147a0 100644 --- a/pkg/getter/httpgetter_test.go +++ b/pkg/getter/httpgetter_test.go @@ -689,7 +689,7 @@ func TestHTTPGetterSessionHeader(t *testing.T) { defer srv.Close() // Create getter for HTTP session header test - g, err := NewHTTPGetter(WithURL(srv.URL)) + g, err := NewHTTPGetter(WithURL(srv.URL), WithSessionHeader(true)) if err != nil { t.Fatal(err) } From 5ec9fb23a438644f07ad988f4b022b4c483bf320 Mon Sep 17 00:00:00 2001 From: vaish123-fullstck Date: Fri, 27 Mar 2026 15:37:38 +0530 Subject: [PATCH 6/6] fix: add sessionID lifecycle comment and finalize implementation Signed-off-by: vaish123-fullstck --- pkg/getter/httpgetter.go | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/pkg/getter/httpgetter.go b/pkg/getter/httpgetter.go index af8afcc42..c5943906d 100644 --- a/pkg/getter/httpgetter.go +++ b/pkg/getter/httpgetter.go @@ -1,3 +1,18 @@ +/* +Copyright The Helm Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package getter import ( @@ -15,6 +30,8 @@ import ( "helm.sh/helm/v4/internal/version" ) +// helmSessionHeader is used to group HTTP requests initiated +// during a single Helm command execution. const helmSessionHeader = "helm-session" // HTTPGetter is the default HTTP(/S) backend handler @@ -40,7 +57,9 @@ func (g *HTTPGetter) get(href string, opts getterOptions) (*bytes.Buffer, error) return nil, err } - // ✅ Optional session header (correct implementation) + // sessionID is generated once per HTTPGetter instance and, + // when sessionHeader is enabled, is sent with each request + // via the helm-session header for request correlation. if g.sessionID != "" && opts.sessionHeader { req.Header.Set(helmSessionHeader, g.sessionID) } @@ -62,7 +81,8 @@ func (g *HTTPGetter) get(href string, opts getterOptions) (*bytes.Buffer, error) if err != nil { return nil, fmt.Errorf("unable to parse URL getting from: %w", err) } - + // Ensure credentials are only sent to the same host and scheme + // to prevent leaking credentials across different services. if opts.passCredentialsAll || (u1.Scheme == u2.Scheme && u1.Host == u2.Host) { if opts.username != "" && opts.password != "" { req.SetBasicAuth(opts.username, opts.password) @@ -97,6 +117,8 @@ func NewHTTPGetter(options ...Option) (Getter, error) { opt(&client.opts) } + // sessionID is generated once per HTTPGetter instance + // and reused across all requests when sessionHeader is enabled. client.sessionID = uuid.New().String() return &client, nil