From 86a808e3c73990dfcf04209dbf937697ec740e2a Mon Sep 17 00:00:00 2001 From: kimsungmin1 Date: Thu, 28 Aug 2025 14:43:49 +0900 Subject: [PATCH 01/12] fix scope when helm push to a registry that use token auth Signed-off-by: kimsungmin1 --- go.mod | 2 + go.sum | 4 + pkg/registry/client.go | 9 ++ pkg/registry/client_http_test.go | 2 +- pkg/registry/client_insecure_tls_test.go | 2 +- pkg/registry/client_scope_test.go | 109 +++++++++++++++++++++++ pkg/registry/client_tls_test.go | 2 +- pkg/registry/utils_test.go | 33 +++++-- 8 files changed, 154 insertions(+), 9 deletions(-) create mode 100644 pkg/registry/client_scope_test.go diff --git a/go.mod b/go.mod index 688094670..fe1e9315b 100644 --- a/go.mod +++ b/go.mod @@ -26,6 +26,7 @@ require ( github.com/mitchellh/copystructure v1.2.0 github.com/moby/term v0.5.2 github.com/opencontainers/image-spec v1.1.1 + github.com/phayes/freeport v0.0.0-20220201140144-74d24b5ae9f5 github.com/rubenv/sql-migrate v1.8.0 github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 github.com/spf13/cobra v1.9.1 @@ -75,6 +76,7 @@ require ( github.com/fxamacker/cbor/v2 v2.8.0 // indirect github.com/go-errors/errors v1.5.1 // indirect github.com/go-gorp/gorp/v3 v3.1.0 // indirect + github.com/go-jose/go-jose/v4 v4.0.5 // indirect github.com/go-logr/logr v1.4.3 // indirect github.com/go-logr/stdr v1.2.2 // indirect github.com/go-openapi/jsonpointer v0.21.1 // indirect diff --git a/go.sum b/go.sum index 5ac66f328..9e6fd5435 100644 --- a/go.sum +++ b/go.sum @@ -99,6 +99,8 @@ github.com/go-errors/errors v1.5.1 h1:ZwEMSLRCapFLflTpT7NKaAc7ukJ8ZPEjzlxt8rPN8b github.com/go-errors/errors v1.5.1/go.mod h1:sIVyrIiJhuEF+Pj9Ebtd6P/rEYROXFi3BopGUQ5a5Og= github.com/go-gorp/gorp/v3 v3.1.0 h1:ItKF/Vbuj31dmV4jxA1qblpSwkl9g1typ24xoe70IGs= github.com/go-gorp/gorp/v3 v3.1.0/go.mod h1:dLEjIyyRNiXvNZ8PSmzpt1GsWAUK8kjVhEpjH8TixEw= +github.com/go-jose/go-jose/v4 v4.0.5 h1:M6T8+mKZl/+fNNuFHvGIzDz7BTLQPIounk/b9dw3AaE= +github.com/go-jose/go-jose/v4 v4.0.5/go.mod h1:s3P1lRrkT8igV8D9OjyL4WRyHvjB6a4JSllnOrmmBOA= github.com/go-kit/kit v0.8.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as= github.com/go-logfmt/logfmt v0.3.0/go.mod h1:Qt1PoO58o5twSAckw1HlFXLmHsOX5/0LbT9GBnD5lWE= github.com/go-logfmt/logfmt v0.4.0/go.mod h1:3RMwSq7FuexP4Kalkev3ejPJsZTpXXBr9+V4qmtdjCk= @@ -246,6 +248,8 @@ github.com/opencontainers/image-spec v1.1.1 h1:y0fUlFfIZhPF1W537XOLg0/fcx6zcHCJw github.com/opencontainers/image-spec v1.1.1/go.mod h1:qpqAh3Dmcf36wStyyWU+kCeDgrGnAve2nCC8+7h8Q0M= github.com/peterbourgon/diskv v2.0.1+incompatible h1:UBdAOUP5p4RWqPBg048CAvpKN+vxiaj6gdUUzhl4XmI= github.com/peterbourgon/diskv v2.0.1+incompatible/go.mod h1:uqqh8zWWbv1HBMNONnaR/tNboyR3/BZd58JJSHlUSCU= +github.com/phayes/freeport v0.0.0-20220201140144-74d24b5ae9f5 h1:Ii+DKncOVM8Cu1Hc+ETb5K+23HdAMvESYE3ZJ5b5cMI= +github.com/phayes/freeport v0.0.0-20220201140144-74d24b5ae9f5/go.mod h1:iIss55rKnNBTvrwdmkUpLnDpZoAHvWaiq5+iMmen4AE= github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= diff --git a/pkg/registry/client.go b/pkg/registry/client.go index 0c9f256d3..46e633f7f 100644 --- a/pkg/registry/client.go +++ b/pkg/registry/client.go @@ -699,6 +699,8 @@ func (c *Client) Push(data []byte, ref string, options ...PushOption) (*PushResu repository.PlainHTTP = c.plainHTTP repository.Client = c.authorizer + ctx = WithScopeHint(ctx, repository, auth.ActionPush, auth.ActionPull) + manifestDescriptor, err = oras.ExtendedCopy(ctx, memoryStore, parsedRef.String(), repository, parsedRef.String(), oras.DefaultExtendedCopyOptions) if err != nil { return nil, err @@ -909,3 +911,10 @@ func (c *Client) tagManifest(ctx context.Context, memoryStore *memory.Store, return oras.TagBytes(ctx, memoryStore, ocispec.MediaTypeImageManifest, manifestData, parsedRef.String()) } + +func WithScopeHint(ctx context.Context, target any, actions ...string) context.Context { + if repo, ok := target.(*remote.Repository); ok { + return auth.AppendRepositoryScope(ctx, repo.Reference, actions...) + } + return ctx +} diff --git a/pkg/registry/client_http_test.go b/pkg/registry/client_http_test.go index 043fd4205..1412ceaa4 100644 --- a/pkg/registry/client_http_test.go +++ b/pkg/registry/client_http_test.go @@ -32,7 +32,7 @@ type HTTPRegistryClientTestSuite struct { func (suite *HTTPRegistryClientTestSuite) SetupSuite() { // init test client - dockerRegistry := setup(&suite.TestSuite, false, false) + dockerRegistry := setup(&suite.TestSuite, false, false, "htpasswd") // Start Docker registry go dockerRegistry.ListenAndServe() diff --git a/pkg/registry/client_insecure_tls_test.go b/pkg/registry/client_insecure_tls_test.go index accbf1670..c7f27b169 100644 --- a/pkg/registry/client_insecure_tls_test.go +++ b/pkg/registry/client_insecure_tls_test.go @@ -29,7 +29,7 @@ type InsecureTLSRegistryClientTestSuite struct { func (suite *InsecureTLSRegistryClientTestSuite) SetupSuite() { // init test client - dockerRegistry := setup(&suite.TestSuite, true, true) + dockerRegistry := setup(&suite.TestSuite, true, true, "htpasswd") // Start Docker registry go dockerRegistry.ListenAndServe() diff --git a/pkg/registry/client_scope_test.go b/pkg/registry/client_scope_test.go new file mode 100644 index 000000000..245af5e3e --- /dev/null +++ b/pkg/registry/client_scope_test.go @@ -0,0 +1,109 @@ +/* +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 registry + +import ( + "context" + "fmt" + "log" + "net/http" + "os" + "testing" + + "github.com/stretchr/testify/suite" +) + +type RegistryScopeTestSuite struct { + TestSuite +} + +func (suite *RegistryScopeTestSuite) SetupSuite() { + // set registry use token auth + dockerRegistry := setup(&suite.TestSuite, true, true, "token") + // Start Docker registry + go dockerRegistry.ListenAndServe() +} +func (suite *RegistryScopeTestSuite) TearDownSuite() { + teardown(&suite.TestSuite) + os.RemoveAll(suite.WorkspaceDir) +} + +func (suite *RegistryScopeTestSuite) Test_1_Cehck_Push_Request_Scope() { + + //set simple auth server to check the auth request scope + server := &http.Server{ + Addr: suite.AuthServerHost, + Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + suite.Equal(string("/auth?scope=repository%3Atestrepo%2Flocal-subchart%3Apull%2Cpush&service=testservice"), r.URL.String()) + w.WriteHeader(http.StatusOK) + }), + } + go func() { + err := server.ListenAndServe() + if err != nil && err != http.ErrServerClosed { + log.Fatalf("http server failed to ListenAndServe:%v", err) + } + }() + + // basic push, good ref + testingChartCreationTime := "1977-09-02T22:04:05Z" + chartData, err := os.ReadFile("../downloader/testdata/local-subchart-0.1.0.tgz") + suite.Nil(err, "no error loading test chart") + meta, err := extractChartMeta(chartData) + suite.Nil(err, "no error extracting chart meta") + ref := fmt.Sprintf("%s/testrepo/%s:%s", suite.DockerRegistryHost, meta.Name, meta.Version) + _, err = suite.RegistryClient.Push(chartData, ref, PushOptCreationTime(testingChartCreationTime)) + suite.NotNil(err, "error pushing good ref because auth server don't give proper token") + + //shutdown auth server + err = server.Shutdown(context.Background()) + suite.Nil(err, "shutdown simple auth server") + +} + +func (suite *RegistryScopeTestSuite) Test_2_Cehck_Pull_Request_Scope() { + + //set simple auth server to check the auth request scope + server := &http.Server{ + Addr: suite.AuthServerHost, + Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + suite.Equal(string("/auth?scope=repository%3Atestrepo%2Flocal-subchart%3Apull&service=testservice"), r.URL.String()) + w.WriteHeader(http.StatusOK) + }), + } + go func() { + err := server.ListenAndServe() + if err != nil && err != http.ErrServerClosed { + log.Fatalf("http server failed to ListenAndServe:%v", err) + } + }() + + // Load test chart (to build ref pushed in previous test) + // Simple pull, chart only + chartData, err := os.ReadFile("../downloader/testdata/local-subchart-0.1.0.tgz") + suite.Nil(err, "no error loading test chart") + meta, err := extractChartMeta(chartData) + suite.Nil(err, "no error extracting chart meta") + ref := fmt.Sprintf("%s/testrepo/%s:%s", suite.DockerRegistryHost, meta.Name, meta.Version) + _, err = suite.RegistryClient.Pull(ref) + suite.NotNil(err, "error pulling a simple chart because auth server don't give proper token") + + //shutdown auth server + err = server.Shutdown(context.Background()) + suite.Nil(err, "shutdown simple auth server") +} + +func TestRegistryScopeTestSuite(t *testing.T) { + suite.Run(t, new(RegistryScopeTestSuite)) +} diff --git a/pkg/registry/client_tls_test.go b/pkg/registry/client_tls_test.go index 0897858b5..ca2f6dfba 100644 --- a/pkg/registry/client_tls_test.go +++ b/pkg/registry/client_tls_test.go @@ -31,7 +31,7 @@ type TLSRegistryClientTestSuite struct { func (suite *TLSRegistryClientTestSuite) SetupSuite() { // init test client - dockerRegistry := setup(&suite.TestSuite, true, false) + dockerRegistry := setup(&suite.TestSuite, true, false, "htpasswd") // Start Docker registry go dockerRegistry.ListenAndServe() diff --git a/pkg/registry/utils_test.go b/pkg/registry/utils_test.go index f4ff5bd58..bf749f415 100644 --- a/pkg/registry/utils_test.go +++ b/pkg/registry/utils_test.go @@ -35,8 +35,10 @@ import ( "github.com/distribution/distribution/v3/configuration" "github.com/distribution/distribution/v3/registry" _ "github.com/distribution/distribution/v3/registry/auth/htpasswd" + _ "github.com/distribution/distribution/v3/registry/auth/token" _ "github.com/distribution/distribution/v3/registry/storage/driver/inmemory" "github.com/foxcpp/go-mockdns" + "github.com/phayes/freeport" "github.com/stretchr/testify/suite" "golang.org/x/crypto/bcrypt" @@ -56,12 +58,15 @@ var ( testHtpasswdFileBasename = "authtest.htpasswd" testUsername = "myuser" testPassword = "mypass" + testIssuer = "testissuer" + testService = "testservice" ) type TestSuite struct { suite.Suite Out io.Writer DockerRegistryHost string + AuthServerHost string CompromisedRegistryHost string WorkspaceDir string RegistryClient *Client @@ -70,7 +75,7 @@ type TestSuite struct { srv *mockdns.Server } -func setup(suite *TestSuite, tlsEnabled, insecure bool) *registry.Registry { +func setup(suite *TestSuite, tlsEnabled, insecure bool, auth string) *registry.Registry { suite.WorkspaceDir = testWorkspaceDir os.RemoveAll(suite.WorkspaceDir) os.Mkdir(suite.WorkspaceDir, 0700) @@ -147,11 +152,27 @@ func setup(suite *TestSuite, tlsEnabled, insecure bool) *registry.Registry { config.HTTP.DrainTimeout = time.Duration(10) * time.Second config.Storage = map[string]configuration.Parameters{"inmemory": map[string]interface{}{}} - config.Auth = configuration.Auth{ - "htpasswd": configuration.Parameters{ - "realm": "localhost", - "path": htpasswdPath, - }, + if auth == "token" { + port, err := freeport.GetFreePort() + suite.Nil(err, "no error finding free port for test auth server") + + suite.AuthServerHost = fmt.Sprintf("localhost:%d", port) + + config.Auth = configuration.Auth{ + "token": configuration.Parameters{ + "realm": "http://" + suite.AuthServerHost + "/auth", + "service": testService, + "issuer": testIssuer, + "rootcertbundle": tlsServerCert, + }, + } + } else { + config.Auth = configuration.Auth{ + "htpasswd": configuration.Parameters{ + "realm": "localhost", + "path": htpasswdPath, + }, + } } // config tls From 8bf1eac6848798b4fe4f343fd7667d9e77dc0c64 Mon Sep 17 00:00:00 2001 From: kimsungmin1 Date: Thu, 28 Aug 2025 14:53:14 +0900 Subject: [PATCH 02/12] add newline in license header Signed-off-by: kimsungmin1 --- pkg/registry/client_scope_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/registry/client_scope_test.go b/pkg/registry/client_scope_test.go index 245af5e3e..707e9eed6 100644 --- a/pkg/registry/client_scope_test.go +++ b/pkg/registry/client_scope_test.go @@ -1,9 +1,12 @@ /* 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. From 77e9a1d19b9970b2109e66dafba597d0c36b5ece Mon Sep 17 00:00:00 2001 From: kimsungmin1 Date: Fri, 29 Aug 2025 09:34:11 +0900 Subject: [PATCH 03/12] remove freeport dependency Signed-off-by: kimsungmin1 --- go.mod | 1 - go.sum | 2 -- pkg/registry/utils_test.go | 7 ++++--- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index fe1e9315b..acfad9c54 100644 --- a/go.mod +++ b/go.mod @@ -26,7 +26,6 @@ require ( github.com/mitchellh/copystructure v1.2.0 github.com/moby/term v0.5.2 github.com/opencontainers/image-spec v1.1.1 - github.com/phayes/freeport v0.0.0-20220201140144-74d24b5ae9f5 github.com/rubenv/sql-migrate v1.8.0 github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 github.com/spf13/cobra v1.9.1 diff --git a/go.sum b/go.sum index 9e6fd5435..1c5a292a7 100644 --- a/go.sum +++ b/go.sum @@ -248,8 +248,6 @@ github.com/opencontainers/image-spec v1.1.1 h1:y0fUlFfIZhPF1W537XOLg0/fcx6zcHCJw github.com/opencontainers/image-spec v1.1.1/go.mod h1:qpqAh3Dmcf36wStyyWU+kCeDgrGnAve2nCC8+7h8Q0M= github.com/peterbourgon/diskv v2.0.1+incompatible h1:UBdAOUP5p4RWqPBg048CAvpKN+vxiaj6gdUUzhl4XmI= github.com/peterbourgon/diskv v2.0.1+incompatible/go.mod h1:uqqh8zWWbv1HBMNONnaR/tNboyR3/BZd58JJSHlUSCU= -github.com/phayes/freeport v0.0.0-20220201140144-74d24b5ae9f5 h1:Ii+DKncOVM8Cu1Hc+ETb5K+23HdAMvESYE3ZJ5b5cMI= -github.com/phayes/freeport v0.0.0-20220201140144-74d24b5ae9f5/go.mod h1:iIss55rKnNBTvrwdmkUpLnDpZoAHvWaiq5+iMmen4AE= github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= diff --git a/pkg/registry/utils_test.go b/pkg/registry/utils_test.go index bf749f415..1a38f7b3e 100644 --- a/pkg/registry/utils_test.go +++ b/pkg/registry/utils_test.go @@ -38,7 +38,6 @@ import ( _ "github.com/distribution/distribution/v3/registry/auth/token" _ "github.com/distribution/distribution/v3/registry/storage/driver/inmemory" "github.com/foxcpp/go-mockdns" - "github.com/phayes/freeport" "github.com/stretchr/testify/suite" "golang.org/x/crypto/bcrypt" @@ -153,10 +152,12 @@ func setup(suite *TestSuite, tlsEnabled, insecure bool, auth string) *registry.R config.Storage = map[string]configuration.Parameters{"inmemory": map[string]interface{}{}} if auth == "token" { - port, err := freeport.GetFreePort() + ln, err := net.Listen("tcp", "127.0.0.1:0") suite.Nil(err, "no error finding free port for test auth server") + defer ln.Close() - suite.AuthServerHost = fmt.Sprintf("localhost:%d", port) + //set test auth server host + suite.AuthServerHost = ln.Addr().String() config.Auth = configuration.Auth{ "token": configuration.Parameters{ From 6d3278cf5fd9ce1d21b00c82cbfd99b81f422c26 Mon Sep 17 00:00:00 2001 From: kimsungmin1 Date: Tue, 9 Sep 2025 10:00:54 +0900 Subject: [PATCH 04/12] fix typo Signed-off-by: kimsungmin1 --- pkg/registry/client_scope_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/registry/client_scope_test.go b/pkg/registry/client_scope_test.go index e3e2422c0..60dbdf34d 100644 --- a/pkg/registry/client_scope_test.go +++ b/pkg/registry/client_scope_test.go @@ -41,7 +41,7 @@ func (suite *RegistryScopeTestSuite) TearDownSuite() { os.RemoveAll(suite.WorkspaceDir) } -func (suite *RegistryScopeTestSuite) Test_1_Cehck_Push_Request_Scope() { +func (suite *RegistryScopeTestSuite) Test_1_Check_Push_Request_Scope() { //set simple auth server to check the auth request scope server := &http.Server{ @@ -74,7 +74,7 @@ func (suite *RegistryScopeTestSuite) Test_1_Cehck_Push_Request_Scope() { } -func (suite *RegistryScopeTestSuite) Test_2_Cehck_Pull_Request_Scope() { +func (suite *RegistryScopeTestSuite) Test_2_Check_Pull_Request_Scope() { //set simple auth server to check the auth request scope server := &http.Server{ From eaff192cd5dd833d3709c2b0644c893a0601a461 Mon Sep 17 00:00:00 2001 From: kimsungmin1 Date: Thu, 11 Dec 2025 00:49:09 +0900 Subject: [PATCH 05/12] change client_scope_test.go to use httptest Signed-off-by: kimsungmin1 --- pkg/registry/client_scope_test.go | 61 ++++++++++++------------------- 1 file changed, 24 insertions(+), 37 deletions(-) diff --git a/pkg/registry/client_scope_test.go b/pkg/registry/client_scope_test.go index ef65a8288..59b3df941 100644 --- a/pkg/registry/client_scope_test.go +++ b/pkg/registry/client_scope_test.go @@ -17,10 +17,10 @@ limitations under the License. package registry import ( - "context" "fmt" - "log" + "net" "net/http" + "net/http/httptest" "os" "testing" @@ -43,20 +43,17 @@ func (suite *RegistryScopeTestSuite) TearDownSuite() { func (suite *RegistryScopeTestSuite) Test_1_Check_Push_Request_Scope() { - //set simple auth server to check the auth request scope - server := &http.Server{ - Addr: suite.AuthServerHost, - Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - suite.Equal(string("/auth?scope=repository%3Atestrepo%2Flocal-subchart%3Apull%2Cpush&service=testservice"), r.URL.String()) - w.WriteHeader(http.StatusOK) - }), - } - go func() { - err := server.ListenAndServe() - if err != nil && err != http.ErrServerClosed { - log.Fatalf("http server failed to ListenAndServe:%v", err) - } - }() + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + suite.Equal(string("/auth?scope=repository%3Atestrepo%2Flocal-subchart%3Apull%2Cpush&service=testservice"), r.URL.String()) + w.WriteHeader(http.StatusOK) + }) + listener, err := net.Listen("tcp", suite.AuthServerHost) + suite.Nil(err, "no error creating server listner") + + ts := httptest.NewUnstartedServer(handler) + ts.Listener = listener + ts.Start() + defer ts.Close() // basic push, good ref testingChartCreationTime := "1977-09-02T22:04:05Z" @@ -68,28 +65,21 @@ func (suite *RegistryScopeTestSuite) Test_1_Check_Push_Request_Scope() { _, err = suite.RegistryClient.Push(chartData, ref, PushOptCreationTime(testingChartCreationTime)) suite.NotNil(err, "error pushing good ref because auth server don't give proper token") - //shutdown auth server - err = server.Shutdown(context.Background()) - suite.Nil(err, "shutdown simple auth server") - } func (suite *RegistryScopeTestSuite) Test_2_Check_Pull_Request_Scope() { - //set simple auth server to check the auth request scope - server := &http.Server{ - Addr: suite.AuthServerHost, - Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - suite.Equal(string("/auth?scope=repository%3Atestrepo%2Flocal-subchart%3Apull&service=testservice"), r.URL.String()) - w.WriteHeader(http.StatusOK) - }), - } - go func() { - err := server.ListenAndServe() - if err != nil && err != http.ErrServerClosed { - log.Fatalf("http server failed to ListenAndServe:%v", err) - } - }() + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + suite.Equal(string("/auth?scope=repository%3Atestrepo%2Flocal-subchart%3Apull&service=testservice"), r.URL.String()) + w.WriteHeader(http.StatusOK) + }) + listener, err := net.Listen("tcp", suite.AuthServerHost) + suite.Nil(err, "no error creating server listner") + + ts := httptest.NewUnstartedServer(handler) + ts.Listener = listener + ts.Start() + defer ts.Close() // Load test chart (to build ref pushed in previous test) // Simple pull, chart only @@ -101,9 +91,6 @@ func (suite *RegistryScopeTestSuite) Test_2_Check_Pull_Request_Scope() { _, err = suite.RegistryClient.Pull(ref) suite.NotNil(err, "error pulling a simple chart because auth server don't give proper token") - //shutdown auth server - err = server.Shutdown(context.Background()) - suite.Nil(err, "shutdown simple auth server") } func TestRegistryScopeTestSuite(t *testing.T) { From e6ac04c9a76886a8c1b6109891900d23d1d62334 Mon Sep 17 00:00:00 2001 From: kimsungmin1 Date: Fri, 9 Jan 2026 20:10:24 +0900 Subject: [PATCH 06/12] change suite.Nil, suite.NotNill to more proper function(suite.NoError, suite.Error) Signed-off-by: kimsungmin1 --- pkg/registry/client_scope_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/registry/client_scope_test.go b/pkg/registry/client_scope_test.go index 59b3df941..348db2644 100644 --- a/pkg/registry/client_scope_test.go +++ b/pkg/registry/client_scope_test.go @@ -48,7 +48,7 @@ func (suite *RegistryScopeTestSuite) Test_1_Check_Push_Request_Scope() { w.WriteHeader(http.StatusOK) }) listener, err := net.Listen("tcp", suite.AuthServerHost) - suite.Nil(err, "no error creating server listner") + suite.NoError(err, "no error creating server listner") ts := httptest.NewUnstartedServer(handler) ts.Listener = listener @@ -58,12 +58,12 @@ func (suite *RegistryScopeTestSuite) Test_1_Check_Push_Request_Scope() { // basic push, good ref testingChartCreationTime := "1977-09-02T22:04:05Z" chartData, err := os.ReadFile("../downloader/testdata/local-subchart-0.1.0.tgz") - suite.Nil(err, "no error loading test chart") + suite.NoError(err, "no error loading test chart") meta, err := extractChartMeta(chartData) - suite.Nil(err, "no error extracting chart meta") + suite.NoError(err, "no error extracting chart meta") ref := fmt.Sprintf("%s/testrepo/%s:%s", suite.DockerRegistryHost, meta.Name, meta.Version) _, err = suite.RegistryClient.Push(chartData, ref, PushOptCreationTime(testingChartCreationTime)) - suite.NotNil(err, "error pushing good ref because auth server don't give proper token") + suite.Error(err, "error pushing good ref because auth server don't give proper token") } @@ -74,7 +74,7 @@ func (suite *RegistryScopeTestSuite) Test_2_Check_Pull_Request_Scope() { w.WriteHeader(http.StatusOK) }) listener, err := net.Listen("tcp", suite.AuthServerHost) - suite.Nil(err, "no error creating server listner") + suite.NoError(err, "no error creating server listner") ts := httptest.NewUnstartedServer(handler) ts.Listener = listener @@ -84,12 +84,12 @@ func (suite *RegistryScopeTestSuite) Test_2_Check_Pull_Request_Scope() { // Load test chart (to build ref pushed in previous test) // Simple pull, chart only chartData, err := os.ReadFile("../downloader/testdata/local-subchart-0.1.0.tgz") - suite.Nil(err, "no error loading test chart") + suite.NoError(err, "no error loading test chart") meta, err := extractChartMeta(chartData) - suite.Nil(err, "no error extracting chart meta") + suite.NoError(err, "no error extracting chart meta") ref := fmt.Sprintf("%s/testrepo/%s:%s", suite.DockerRegistryHost, meta.Name, meta.Version) _, err = suite.RegistryClient.Pull(ref) - suite.NotNil(err, "error pulling a simple chart because auth server don't give proper token") + suite.Error(err, "error pulling a simple chart because auth server don't give proper token") } From 504e26956ab245122a92830b055e165b3646f6f4 Mon Sep 17 00:00:00 2001 From: kimsm28 Date: Wed, 11 Mar 2026 10:56:03 +0900 Subject: [PATCH 07/12] fix typo, remove unnecessary code, fix to avoid to use the assertion in http hanlder Signed-off-by: kimsm28 --- pkg/registry/client.go | 11 +++++++++-- pkg/registry/client_scope_test.go | 21 +++++++++++++++------ pkg/registry/registry_test.go | 1 - 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/pkg/registry/client.go b/pkg/registry/client.go index ea111ff3a..0b5b5a98a 100644 --- a/pkg/registry/client.go +++ b/pkg/registry/client.go @@ -702,7 +702,7 @@ func (c *Client) Push(data []byte, ref string, options ...PushOption) (*PushResu repository.PlainHTTP = c.plainHTTP repository.Client = c.authorizer - ctx = WithScopeHint(ctx, repository, auth.ActionPush, auth.ActionPull) + ctx = withScopeHint(ctx, repository, auth.ActionPull, auth.ActionPush) manifestDescriptor, err = oras.ExtendedCopy(ctx, memoryStore, parsedRef.String(), repository, parsedRef.String(), oras.DefaultExtendedCopyOptions) if err != nil { @@ -917,7 +917,14 @@ func (c *Client) tagManifest(ctx context.Context, memoryStore *memory.Store, manifestData, parsedRef.String()) } -func WithScopeHint(ctx context.Context, target any, actions ...string) context.Context { +// add actions when request a registry authentication token(jwt) +// example1. when we want to pull 'testrepo/local-subchart' we can send bellow url, and 'pull' is the action +// auth?scope=repository%3Atestrepo%2Flocal-subchart%3Apull&service=testservice +// example2. when we want to push 'testrepo/local-subchart' we can send bellow url, and 'pull%2Cpush' are the actions +// auth?scope=repository%3Atestrepo%2Flocal-subchart%3Apull%2Cpush&service=testservice +// we can set the actions like bellow +// example) ctx = WithScopeHint(ctx, repository, auth.ActionPush, auth.ActionPull) +func withScopeHint(ctx context.Context, target any, actions ...string) context.Context { if repo, ok := target.(*remote.Repository); ok { return auth.AppendRepositoryScope(ctx, repo.Reference, actions...) } diff --git a/pkg/registry/client_scope_test.go b/pkg/registry/client_scope_test.go index 348db2644..876f92b10 100644 --- a/pkg/registry/client_scope_test.go +++ b/pkg/registry/client_scope_test.go @@ -43,12 +43,14 @@ func (suite *RegistryScopeTestSuite) TearDownSuite() { func (suite *RegistryScopeTestSuite) Test_1_Check_Push_Request_Scope() { + var requestUrl string + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - suite.Equal(string("/auth?scope=repository%3Atestrepo%2Flocal-subchart%3Apull%2Cpush&service=testservice"), r.URL.String()) + requestUrl = r.URL.String() w.WriteHeader(http.StatusOK) }) listener, err := net.Listen("tcp", suite.AuthServerHost) - suite.NoError(err, "no error creating server listner") + suite.NoError(err, "no error creating server listener") ts := httptest.NewUnstartedServer(handler) ts.Listener = listener @@ -63,18 +65,23 @@ func (suite *RegistryScopeTestSuite) Test_1_Check_Push_Request_Scope() { suite.NoError(err, "no error extracting chart meta") ref := fmt.Sprintf("%s/testrepo/%s:%s", suite.DockerRegistryHost, meta.Name, meta.Version) _, err = suite.RegistryClient.Push(chartData, ref, PushOptCreationTime(testingChartCreationTime)) - suite.Error(err, "error pushing good ref because auth server don't give proper token") + suite.Error(err, "error pushing good ref because auth server doesn't give proper token") + + //check the url that authentication server received + suite.Equal("/auth?scope=repository%3Atestrepo%2Flocal-subchart%3Apull%2Cpush&service=testservice", requestUrl) } func (suite *RegistryScopeTestSuite) Test_2_Check_Pull_Request_Scope() { + var requestUrl string + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - suite.Equal(string("/auth?scope=repository%3Atestrepo%2Flocal-subchart%3Apull&service=testservice"), r.URL.String()) + requestUrl = r.URL.String() w.WriteHeader(http.StatusOK) }) listener, err := net.Listen("tcp", suite.AuthServerHost) - suite.NoError(err, "no error creating server listner") + suite.NoError(err, "no error creating server listener") ts := httptest.NewUnstartedServer(handler) ts.Listener = listener @@ -89,8 +96,10 @@ func (suite *RegistryScopeTestSuite) Test_2_Check_Pull_Request_Scope() { suite.NoError(err, "no error extracting chart meta") ref := fmt.Sprintf("%s/testrepo/%s:%s", suite.DockerRegistryHost, meta.Name, meta.Version) _, err = suite.RegistryClient.Pull(ref) - suite.Error(err, "error pulling a simple chart because auth server don't give proper token") + suite.Error(err, "error pulling a simple chart because auth server doesn't give proper token") + //check the url that authentication server received + suite.Equal("/auth?scope=repository%3Atestrepo%2Flocal-subchart%3Apull&service=testservice", requestUrl) } func TestRegistryScopeTestSuite(t *testing.T) { diff --git a/pkg/registry/registry_test.go b/pkg/registry/registry_test.go index 26ab7b113..b492e3641 100644 --- a/pkg/registry/registry_test.go +++ b/pkg/registry/registry_test.go @@ -143,7 +143,6 @@ func setup(suite *TestRegistry, tlsEnabled, insecure bool, auth string) { if auth == "token" { ln, err := net.Listen("tcp", "127.0.0.1:0") suite.Nil(err, "no error finding free port for test auth server") - defer ln.Close() //set test auth server host suite.AuthServerHost = ln.Addr().String() From b9232c7853dafa6554f7f974a07bbbd8b6e015e4 Mon Sep 17 00:00:00 2001 From: kimsm28 Date: Wed, 11 Mar 2026 11:03:34 +0900 Subject: [PATCH 08/12] fix variable naming requestUrl -> requestURL Signed-off-by: kimsm28 --- pkg/registry/client_scope_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/registry/client_scope_test.go b/pkg/registry/client_scope_test.go index 876f92b10..f0dec311f 100644 --- a/pkg/registry/client_scope_test.go +++ b/pkg/registry/client_scope_test.go @@ -43,10 +43,10 @@ func (suite *RegistryScopeTestSuite) TearDownSuite() { func (suite *RegistryScopeTestSuite) Test_1_Check_Push_Request_Scope() { - var requestUrl string + var requestURL string handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - requestUrl = r.URL.String() + requestURL = r.URL.String() w.WriteHeader(http.StatusOK) }) listener, err := net.Listen("tcp", suite.AuthServerHost) @@ -68,16 +68,16 @@ func (suite *RegistryScopeTestSuite) Test_1_Check_Push_Request_Scope() { suite.Error(err, "error pushing good ref because auth server doesn't give proper token") //check the url that authentication server received - suite.Equal("/auth?scope=repository%3Atestrepo%2Flocal-subchart%3Apull%2Cpush&service=testservice", requestUrl) + suite.Equal("/auth?scope=repository%3Atestrepo%2Flocal-subchart%3Apull%2Cpush&service=testservice", requestURL) } func (suite *RegistryScopeTestSuite) Test_2_Check_Pull_Request_Scope() { - var requestUrl string + var requestURL string handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - requestUrl = r.URL.String() + requestURL = r.URL.String() w.WriteHeader(http.StatusOK) }) listener, err := net.Listen("tcp", suite.AuthServerHost) @@ -99,7 +99,7 @@ func (suite *RegistryScopeTestSuite) Test_2_Check_Pull_Request_Scope() { suite.Error(err, "error pulling a simple chart because auth server doesn't give proper token") //check the url that authentication server received - suite.Equal("/auth?scope=repository%3Atestrepo%2Flocal-subchart%3Apull&service=testservice", requestUrl) + suite.Equal("/auth?scope=repository%3Atestrepo%2Flocal-subchart%3Apull&service=testservice", requestURL) } func TestRegistryScopeTestSuite(t *testing.T) { From 026981e724ba59e67a7cbfae0b1dcc8b6e48e985 Mon Sep 17 00:00:00 2001 From: kimsm28 Date: Mon, 13 Apr 2026 13:15:49 +0900 Subject: [PATCH 09/12] fix registry test failures by adjusting DockerRegistryHost and auth server listener management - Change DockerRegistryHost to use 127.0.0.1 for HTTP tests and helm-test-registry for TLS tests to match certificate hostname - Add defer ln.Close() to prevent resource leak in token auth server setup - Restore requestURL variable and assertion in test body instead of handler to avoid potential race condition Signed-off-by: kimsm28 --- pkg/registry/client_scope_test.go | 3 --- pkg/registry/registry_test.go | 12 ++++++++---- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/pkg/registry/client_scope_test.go b/pkg/registry/client_scope_test.go index f0dec311f..3bde58a1d 100644 --- a/pkg/registry/client_scope_test.go +++ b/pkg/registry/client_scope_test.go @@ -44,7 +44,6 @@ func (suite *RegistryScopeTestSuite) TearDownSuite() { func (suite *RegistryScopeTestSuite) Test_1_Check_Push_Request_Scope() { var requestURL string - handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { requestURL = r.URL.String() w.WriteHeader(http.StatusOK) @@ -69,13 +68,11 @@ func (suite *RegistryScopeTestSuite) Test_1_Check_Push_Request_Scope() { //check the url that authentication server received suite.Equal("/auth?scope=repository%3Atestrepo%2Flocal-subchart%3Apull%2Cpush&service=testservice", requestURL) - } func (suite *RegistryScopeTestSuite) Test_2_Check_Pull_Request_Scope() { var requestURL string - handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { requestURL = r.URL.String() w.WriteHeader(http.StatusOK) diff --git a/pkg/registry/registry_test.go b/pkg/registry/registry_test.go index b492e3641..1111ac621 100644 --- a/pkg/registry/registry_test.go +++ b/pkg/registry/registry_test.go @@ -130,11 +130,14 @@ func setup(suite *TestRegistry, tlsEnabled, insecure bool, auth string) { suite.Nil(err, "no error finding free port for test registry") defer ln.Close() - // Change the registry host to another host which is not localhost. - // This is required because Docker enforces HTTP if the registry - // host is localhost/127.0.0.1. + // Use localhost for HTTP tests and helm-test-registry for TLS tests. + // TLS tests need a different hostname to match the certificate. port := ln.Addr().(*net.TCPAddr).Port - suite.DockerRegistryHost = fmt.Sprintf("helm-test-registry:%d", port) + if tlsEnabled { + suite.DockerRegistryHost = fmt.Sprintf("helm-test-registry:%d", port) + } else { + suite.DockerRegistryHost = fmt.Sprintf("127.0.0.1:%d", port) + } config.HTTP.Addr = ln.Addr().String() config.HTTP.DrainTimeout = time.Duration(10) * time.Second @@ -143,6 +146,7 @@ func setup(suite *TestRegistry, tlsEnabled, insecure bool, auth string) { if auth == "token" { ln, err := net.Listen("tcp", "127.0.0.1:0") suite.Nil(err, "no error finding free port for test auth server") + defer ln.Close() //set test auth server host suite.AuthServerHost = ln.Addr().String() From 48b9352e4567e0b16014e83afa9f8edf66a64447 Mon Sep 17 00:00:00 2001 From: kimsm28 Date: Mon, 13 Apr 2026 13:29:21 +0900 Subject: [PATCH 10/12] fix typos in withScopeHint function comment - Change 'bellow' to 'below' (spelling correction in multiple places) - Change 'WithScopeHint' to 'withScopeHint' to match actual function name Signed-off-by: kimsm28 --- pkg/registry/client.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/registry/client.go b/pkg/registry/client.go index 0b5b5a98a..bc87e1f9b 100644 --- a/pkg/registry/client.go +++ b/pkg/registry/client.go @@ -918,12 +918,12 @@ func (c *Client) tagManifest(ctx context.Context, memoryStore *memory.Store, } // add actions when request a registry authentication token(jwt) -// example1. when we want to pull 'testrepo/local-subchart' we can send bellow url, and 'pull' is the action +// example1. when we want to pull 'testrepo/local-subchart' we can send below url, and 'pull' is the action // auth?scope=repository%3Atestrepo%2Flocal-subchart%3Apull&service=testservice -// example2. when we want to push 'testrepo/local-subchart' we can send bellow url, and 'pull%2Cpush' are the actions +// example2. when we want to push 'testrepo/local-subchart' we can send below url, and 'pull%2Cpush' are the actions // auth?scope=repository%3Atestrepo%2Flocal-subchart%3Apull%2Cpush&service=testservice -// we can set the actions like bellow -// example) ctx = WithScopeHint(ctx, repository, auth.ActionPush, auth.ActionPull) +// we can set the actions like below +// example) ctx = withScopeHint(ctx, repository, auth.ActionPush, auth.ActionPull) func withScopeHint(ctx context.Context, target any, actions ...string) context.Context { if repo, ok := target.(*remote.Repository); ok { return auth.AppendRepositoryScope(ctx, repo.Reference, actions...) From 284e6647e903d5b4e0c933b13cb76a7f1e42474f Mon Sep 17 00:00:00 2001 From: kimsm28 Date: Mon, 13 Apr 2026 17:53:48 +0900 Subject: [PATCH 11/12] test: improve client_scope_test.go to avoid data races and brittle assertions - Fix data race by using channel instead of shared variable for requestURL - Make assertions more robust by parsing URL and checking query parameters instead of exact string matching Signed-off-by: kimsm28 --- pkg/registry/client_scope_test.go | 36 +++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/pkg/registry/client_scope_test.go b/pkg/registry/client_scope_test.go index 3bde58a1d..06ae157cd 100644 --- a/pkg/registry/client_scope_test.go +++ b/pkg/registry/client_scope_test.go @@ -21,8 +21,10 @@ import ( "net" "net/http" "net/http/httptest" + "net/url" "os" "testing" + "time" "github.com/stretchr/testify/suite" ) @@ -43,9 +45,9 @@ func (suite *RegistryScopeTestSuite) TearDownSuite() { func (suite *RegistryScopeTestSuite) Test_1_Check_Push_Request_Scope() { - var requestURL string + requestURL := make(chan string, 1) handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - requestURL = r.URL.String() + requestURL <- r.URL.String() w.WriteHeader(http.StatusOK) }) listener, err := net.Listen("tcp", suite.AuthServerHost) @@ -67,14 +69,25 @@ func (suite *RegistryScopeTestSuite) Test_1_Check_Push_Request_Scope() { suite.Error(err, "error pushing good ref because auth server doesn't give proper token") //check the url that authentication server received - suite.Equal("/auth?scope=repository%3Atestrepo%2Flocal-subchart%3Apull%2Cpush&service=testservice", requestURL) + select { + case urlStr := <-requestURL: + u, err := url.Parse(urlStr) + suite.NoError(err, "no error parsing requested URL") + + suite.Equal("/auth", u.Path) + suite.Equal("testservice", u.Query().Get("service")) + scope := u.Query().Get("scope") + suite.Contains(scope, "repository:testrepo/local-subchart:pull,push") + case <-time.After(5 * time.Second): + suite.T().Fatal("timeout waiting for auth request") + } } func (suite *RegistryScopeTestSuite) Test_2_Check_Pull_Request_Scope() { - var requestURL string + requestURL := make(chan string, 1) handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - requestURL = r.URL.String() + requestURL <- r.URL.String() w.WriteHeader(http.StatusOK) }) listener, err := net.Listen("tcp", suite.AuthServerHost) @@ -96,7 +109,18 @@ func (suite *RegistryScopeTestSuite) Test_2_Check_Pull_Request_Scope() { suite.Error(err, "error pulling a simple chart because auth server doesn't give proper token") //check the url that authentication server received - suite.Equal("/auth?scope=repository%3Atestrepo%2Flocal-subchart%3Apull&service=testservice", requestURL) + select { + case urlStr := <-requestURL: + u, err := url.Parse(urlStr) + suite.NoError(err, "no error parsing requested URL") + + suite.Equal("/auth", u.Path) + suite.Equal("testservice", u.Query().Get("service")) + scope := u.Query().Get("scope") + suite.Contains(scope, "repository:testrepo/local-subchart:pull") + case <-time.After(5 * time.Second): + suite.T().Fatal("timeout waiting for auth request") + } } func TestRegistryScopeTestSuite(t *testing.T) { From 07001b14672b64e208293f3a25b364d0f0c2932b Mon Sep 17 00:00:00 2001 From: kimsm28 Date: Wed, 15 Apr 2026 10:39:15 +0900 Subject: [PATCH 12/12] fix: replace WriteString(fmt.Sprintf()) with fmt.Fprintf() Replace sb.WriteString(fmt.Sprintf(...)) with fmt.Fprintf(&sb, ...) to follow staticcheck QF1012 recommendation. This is more efficient as it avoids creating an intermediate string. Signed-off-by: kimsm28 --- pkg/chart/common/util/jsonschema.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/chart/common/util/jsonschema.go b/pkg/chart/common/util/jsonschema.go index acd2ca100..108c1bcb8 100644 --- a/pkg/chart/common/util/jsonschema.go +++ b/pkg/chart/common/util/jsonschema.go @@ -82,7 +82,7 @@ func ValidateAgainstSchema(ch chart.Charter, values map[string]interface{}) erro slog.Debug("chart name", "chart-name", chrt.Name()) err := ValidateAgainstSingleSchema(values, chrt.Schema()) if err != nil { - sb.WriteString(fmt.Sprintf("%s:\n", chrt.Name())) + fmt.Fprintf(&sb, "%s:\n", chrt.Name()) sb.WriteString(err.Error()) } }