From 0d2f0116eac616d9b3a3b8836a8d854bbe40be74 Mon Sep 17 00:00:00 2001 From: Scott Rigby Date: Wed, 5 Jan 2022 18:41:02 -0500 Subject: [PATCH 01/27] hack in progress Signed-off-by: Scott Rigby --- internal/resolver/resolver.go | 68 +++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index 70ce6a55b..6df33fd68 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -17,7 +17,9 @@ package resolver import ( "bytes" + "context" "encoding/json" + "net/http" "os" "path/filepath" "strings" @@ -33,6 +35,10 @@ import ( "helm.sh/helm/v3/pkg/helmpath" "helm.sh/helm/v3/pkg/provenance" "helm.sh/helm/v3/pkg/repo" + + orasregistry "oras.land/oras-go/pkg/registry" + orasremote "oras.land/oras-go/pkg/registry/remote" + orasauth "oras.land/oras-go/pkg/registry/remote/auth" ) const FeatureGateOCI = gates.Gate("HELM_EXPERIMENTAL_OCI") @@ -52,6 +58,7 @@ func New(chartpath, cachepath string) *Resolver { } // Resolve resolves dependencies and returns a lock file with the resolution. +// To-do: clarify that we use "Repository" in struct as URL for registrytoo, even though strictly speaking it's not a helm "repository" func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string) (*chart.Lock, error) { // Now we clone the dependencies, locking as we go. @@ -139,6 +146,67 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string return nil, errors.Wrapf(FeatureGateOCI.Error(), "repository %s is an OCI registry", d.Repository) } + + // Call ORAS tag API + // See https://github.com/oras-project/oras-go/pull/89 + // - using string: concat d.Repository + d.Name + // - does latest version exist, find out how Masterminds/semver checks this given contstraint string, and get the version + // To-do: use registry.ctx() + // How to get the opts though without context from a *Client? + ctx := context.Background() + // Do we need to extract something from this string in order to work? + ociRepository := d.Repository + parsedRepository, err := orasregistry.ParseReference(ociRepository) + if err != nil { + return nil, errors.Wrapf(err, "no cached repository for %s found. (try 'helm repo update')", repoName) + } + + // To:do: Can we get client values from registry.NewClient()? + // If so how to pass client ops without an existing *Client? + // Example code for this: + // client, err := registry.NewClient() + // if err != nil { + // return nil, err + // } + client := &orasauth.Client{ + Header: http.Header{ + "User-Agent": {"oras-go"}, + }, + Cache: orasauth.DefaultCache, + } + + repository := orasremote.Repository{ + Reference: parsedRepository, + Client: client, + } + + // Get tags from ORAS tag API + // To-do: The block farther below comment says + // "The version are already sorted and hence the first one to satisfy the constraint is used" + // So how do we ensure these are sorted? + tags, err := orasregistry.Tags(ctx, &repository) + if err != nil { + return nil, err + } + + // Mock chart version objects + vs = make(repo.ChartVersions, len(tags)) + for i, tag := range tags { + // Change underscore (_) back to plus (+) for Helm + // See https://github.com/helm/helm/issues/10166 + tag = strings.ReplaceAll(tag, "_", "+") + + // Then add those on what comes from equivalent of tags + // To-do: do we need anything here other than Version and Name? + vs[i].Version = tag + vs[i].Name = d.Name + } + + // Ultimately either here before continue, or below, we want to accomplish: + // 1. if there's a reference that matches the version (did it work or not? could it locate image? 401 or 403 etc) + // 2. if so, proceed + // But below, if `len(ver.URLs) == 0` it's not considered valid, so we continue + // what is the equivalent of this for OCI? There is no tarball URL(s), right? } locked[i] = &chart.Dependency{ From 042e13d0d10dda30eadbc7f9069417a33bffd919 Mon Sep 17 00:00:00 2001 From: Scott Rigby Date: Wed, 5 Jan 2022 18:46:13 -0500 Subject: [PATCH 02/27] Temp use Andy's fork PR branch Signed-off-by: Scott Rigby --- go.mod | 2 ++ go.sum | 2 ++ 2 files changed, 4 insertions(+) diff --git a/go.mod b/go.mod index d328ad8bb..427cbc04a 100644 --- a/go.mod +++ b/go.mod @@ -45,3 +45,5 @@ require ( oras.land/oras-go v1.1.0-rc3 sigs.k8s.io/yaml v1.3.0 ) + +replace oras.land/oras-go => github.com/sabre1041/oras-go v1.0.1-0.20220105042547-7ebb8809edae diff --git a/go.sum b/go.sum index c56a45885..19662166d 100644 --- a/go.sum +++ b/go.sum @@ -953,6 +953,8 @@ github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQD github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf35Ld67mk= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/ryanuber/columnize v0.0.0-20160712163229-9b3edd62028f/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts= +github.com/sabre1041/oras-go v1.0.1-0.20220105042547-7ebb8809edae h1:ZhMHqeREElljvMEkMcIak/rRWxWe7ut0x1jb0yFQ8CI= +github.com/sabre1041/oras-go v1.0.1-0.20220105042547-7ebb8809edae/go.mod h1:MZN6VbUUZjfWRF1EJKPbAWhJtE+R8JVicRmeYZp8qDg= github.com/safchain/ethtool v0.0.0-20190326074333-42ed695e3de8/go.mod h1:Z0q5wiBQGYcxhMZ6gUqHn6pYNLypFAvaL3UvgZLR0U4= github.com/sagikazarmark/crypt v0.3.0/go.mod h1:uD/D+6UF4SrIR1uGEv7bBNkNqLGqUr43MRiaGWX1Nig= github.com/satori/go.uuid v1.2.0/go.mod h1:dA0hQrYB0VpLJoorglMZABFdXlWrHn1NEOzdhQKdks0= From 16453c372c41a82e676ba95a18b88890060f16b0 Mon Sep 17 00:00:00 2001 From: Scott Rigby Date: Wed, 5 Jan 2022 18:46:57 -0500 Subject: [PATCH 03/27] It appears we never got to this block below. Quick rec by Farina. Untested if necessary Signed-off-by: Scott Rigby --- pkg/downloader/manager.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index fc6c7a432..c85b0b707 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -594,6 +594,8 @@ func (m *Manager) resolveRepoNames(deps []*chart.Dependency) (map[string]string, continue } + // See https://helm.sh/docs/topics/registries/#specifying-dependencies + // See createTestingMetadataForOCI() if registry.IsOCI(dd.Repository) { reposMap[dd.Name] = dd.Repository continue From 3dc9930488aeaf2fb6a15c266255ae9691167f8f Mon Sep 17 00:00:00 2001 From: Scott Rigby Date: Thu, 6 Jan 2022 19:08:46 -0500 Subject: [PATCH 04/27] Revert "hack in progress" This reverts commit c0be414e4b8d2928018504c010cb04b1b2450bf3. Taking a different approach, but keep this work in git history for now until we know the new approach works. Signed-off-by: Scott Rigby --- internal/resolver/resolver.go | 68 ----------------------------------- 1 file changed, 68 deletions(-) diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index 6df33fd68..70ce6a55b 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -17,9 +17,7 @@ package resolver import ( "bytes" - "context" "encoding/json" - "net/http" "os" "path/filepath" "strings" @@ -35,10 +33,6 @@ import ( "helm.sh/helm/v3/pkg/helmpath" "helm.sh/helm/v3/pkg/provenance" "helm.sh/helm/v3/pkg/repo" - - orasregistry "oras.land/oras-go/pkg/registry" - orasremote "oras.land/oras-go/pkg/registry/remote" - orasauth "oras.land/oras-go/pkg/registry/remote/auth" ) const FeatureGateOCI = gates.Gate("HELM_EXPERIMENTAL_OCI") @@ -58,7 +52,6 @@ func New(chartpath, cachepath string) *Resolver { } // Resolve resolves dependencies and returns a lock file with the resolution. -// To-do: clarify that we use "Repository" in struct as URL for registrytoo, even though strictly speaking it's not a helm "repository" func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string) (*chart.Lock, error) { // Now we clone the dependencies, locking as we go. @@ -146,67 +139,6 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string return nil, errors.Wrapf(FeatureGateOCI.Error(), "repository %s is an OCI registry", d.Repository) } - - // Call ORAS tag API - // See https://github.com/oras-project/oras-go/pull/89 - // - using string: concat d.Repository + d.Name - // - does latest version exist, find out how Masterminds/semver checks this given contstraint string, and get the version - // To-do: use registry.ctx() - // How to get the opts though without context from a *Client? - ctx := context.Background() - // Do we need to extract something from this string in order to work? - ociRepository := d.Repository - parsedRepository, err := orasregistry.ParseReference(ociRepository) - if err != nil { - return nil, errors.Wrapf(err, "no cached repository for %s found. (try 'helm repo update')", repoName) - } - - // To:do: Can we get client values from registry.NewClient()? - // If so how to pass client ops without an existing *Client? - // Example code for this: - // client, err := registry.NewClient() - // if err != nil { - // return nil, err - // } - client := &orasauth.Client{ - Header: http.Header{ - "User-Agent": {"oras-go"}, - }, - Cache: orasauth.DefaultCache, - } - - repository := orasremote.Repository{ - Reference: parsedRepository, - Client: client, - } - - // Get tags from ORAS tag API - // To-do: The block farther below comment says - // "The version are already sorted and hence the first one to satisfy the constraint is used" - // So how do we ensure these are sorted? - tags, err := orasregistry.Tags(ctx, &repository) - if err != nil { - return nil, err - } - - // Mock chart version objects - vs = make(repo.ChartVersions, len(tags)) - for i, tag := range tags { - // Change underscore (_) back to plus (+) for Helm - // See https://github.com/helm/helm/issues/10166 - tag = strings.ReplaceAll(tag, "_", "+") - - // Then add those on what comes from equivalent of tags - // To-do: do we need anything here other than Version and Name? - vs[i].Version = tag - vs[i].Name = d.Name - } - - // Ultimately either here before continue, or below, we want to accomplish: - // 1. if there's a reference that matches the version (did it work or not? could it locate image? 401 or 403 etc) - // 2. if so, proceed - // But below, if `len(ver.URLs) == 0` it's not considered valid, so we continue - // what is the equivalent of this for OCI? There is no tarball URL(s), right? } locked[i] = &chart.Dependency{ From 39792b5ad004a99b3ef649a649a0f1834b07a0f6 Mon Sep 17 00:00:00 2001 From: Scott Rigby Date: Thu, 6 Jan 2022 19:10:15 -0500 Subject: [PATCH 05/27] Revert "It appears we never got to this block below. Quick rec by Farina. Untested if necessary" This reverts commit f616a01808da3428c6191e5196f32ca72eb22254. Removing for now until we know we need it. Signed-off-by: Scott Rigby --- pkg/downloader/manager.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index c85b0b707..52f1a1312 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -578,7 +578,8 @@ func (m *Manager) resolveRepoNames(deps []*chart.Dependency) (map[string]string, missing := []string{} for _, dd := range deps { // Don't map the repository, we don't need to download chart from charts directory - if dd.Repository == "" { + // When OCI is used there is no Helm repository + if dd.Repository == "" || registry.IsOCI(dd.Repository) { continue } // if dep chart is from local path, verify the path is valid @@ -594,8 +595,6 @@ func (m *Manager) resolveRepoNames(deps []*chart.Dependency) (map[string]string, continue } - // See https://helm.sh/docs/topics/registries/#specifying-dependencies - // See createTestingMetadataForOCI() if registry.IsOCI(dd.Repository) { reposMap[dd.Name] = dd.Repository continue From 9a7c362dd115431f9f5c17606113769ec036530c Mon Sep 17 00:00:00 2001 From: Andrew Block Date: Thu, 6 Jan 2022 22:31:32 -0600 Subject: [PATCH 06/27] Initial tag listing support Signed-off-by: Andrew Block --- internal/experimental/registry/client.go | 36 +++++++++++++++++++++--- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/internal/experimental/registry/client.go b/internal/experimental/registry/client.go index 3bdfac20b..fdfab76f4 100644 --- a/internal/experimental/registry/client.go +++ b/internal/experimental/registry/client.go @@ -31,6 +31,9 @@ import ( dockerauth "oras.land/oras-go/pkg/auth/docker" "oras.land/oras-go/pkg/content" "oras.land/oras-go/pkg/oras" + "oras.land/oras-go/pkg/registry" + registrremote "oras.land/oras-go/pkg/registry/remote" + registryauth "oras.land/oras-go/pkg/registry/remote/auth" "helm.sh/helm/v3/internal/version" "helm.sh/helm/v3/pkg/chart" @@ -49,10 +52,11 @@ type ( Client struct { debug bool // path to repository config file e.g. ~/.docker/config.json - credentialsFile string - out io.Writer - authorizer auth.Client - resolver remotes.Resolver + credentialsFile string + out io.Writer + authorizer auth.Client + registryAuthorizer *registryauth.Client + resolver remotes.Resolver } // ClientOption allows specifying various settings configurable by the user for overriding the defaults @@ -88,6 +92,15 @@ func NewClient(options ...ClientOption) (*Client, error) { } client.resolver = resolver } + if client.registryAuthorizer == nil { + client.registryAuthorizer = ®istryauth.Client{ + Header: http.Header{ + "User-Agent": {version.GetUserAgent()}, + }, + Cache: registryauth.DefaultCache, + } + + } return client, nil } @@ -539,3 +552,18 @@ func PushOptStrictMode(strictMode bool) PushOption { operation.strictMode = strictMode } } + +// Tags lists all tags for a given repository +func (c *Client) Tags(ref string) ([]string, error) { + parsedReference, err := registry.ParseReference(ref) + if err != nil { + return nil, err + } + + repository := registrremote.Repository{ + Reference: parsedReference, + Client: c.registryAuthorizer, + } + + return registry.Tags(ctx(c.out, c.debug), &repository) +} From e3f2fb42357fbe67257b0eeacd82cb6cba98c7c1 Mon Sep 17 00:00:00 2001 From: Scott Rigby Date: Fri, 7 Jan 2022 03:51:52 -0500 Subject: [PATCH 07/27] Add OCI tag verions to the Dependency object before Resolve. TODO: fix HTTP HTTPS error for local registries Signed-off-by: Scott Rigby --- internal/resolver/resolver.go | 17 ++++++++++++++++- pkg/chart/dependency.go | 4 ++++ pkg/downloader/manager.go | 20 ++++++++++++++++++++ 3 files changed, 40 insertions(+), 1 deletion(-) diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index 70ce6a55b..7fc27acac 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -139,6 +139,20 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string return nil, errors.Wrapf(FeatureGateOCI.Error(), "repository %s is an OCI registry", d.Repository) } + + // This works, fake it to test mocking the version + // tags := []string{"0.1.0", "0.1.1", "0.2.0"} + tags := d.OCITagVersions + vs = make(repo.ChartVersions, len(tags)) + for ti, t := range tags { + // Mock chart version objects + version := &repo.ChartVersion{ + Metadata: &chart.Metadata{ + Version: t, + }, + } + vs[ti] = version + } } locked[i] = &chart.Dependency{ @@ -149,7 +163,8 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string // The version are already sorted and hence the first one to satisfy the constraint is used for _, ver := range vs { v, err := semver.NewVersion(ver.Version) - if err != nil || len(ver.URLs) == 0 { + // OCI does not need URLs + if err != nil || (!registry.IsOCI(d.Repository) && len(ver.URLs) == 0) { // Not a legit entry. continue } diff --git a/pkg/chart/dependency.go b/pkg/chart/dependency.go index b2819f373..890fa971f 100644 --- a/pkg/chart/dependency.go +++ b/pkg/chart/dependency.go @@ -47,6 +47,10 @@ type Dependency struct { ImportValues []interface{} `json:"import-values,omitempty"` // Alias usable alias to be used for the chart Alias string `json:"alias,omitempty"` + // OCI tag versions available to check against the constraint when Version + // contains a semantic version range. + // See (*Manager).resolve. + OCITagVersions []string `json:"-"` } // Validate checks for common problems with the dependency datastructure in diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index 52f1a1312..d8c0eeb9c 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -233,6 +233,26 @@ func (m *Manager) loadChartDir() (*chart.Chart, error) { // This returns a lock file, which has all of the dependencies normalized to a specific version. func (m *Manager) resolve(req []*chart.Dependency, repoNames map[string]string) (*chart.Lock, error) { res := resolver.New(m.ChartPath, m.RepositoryCache) + // NOTE: When OCI dependencies specify a semver range in Version, + // (*Resolver).Resolve must somehow get the list of OCI tag versions + // available to check against this constraint. However for backward + // compatibility we can not change that function signature to pass a + // *registry.Client required to get tags from the ORAS tag listing API. + // However we can add a new backward compatible struct field to + // *chart.Dependency, so that we can pass these along to + // (*Resolver).resolve from here through reqs []*chart.Dependency. + for i, d := range req { + if registry.IsOCI(d.Repository) { + // TODO(scottrigby): fix HTTP error + // > Error: Get "https://localhost:5000/v2/helm-charts/tags/list": http: server gave HTTP response to HTTPS client + ref := strings.Trim(d.Repository, "oci://") + tags, err := m.RegistryClient.Tags(ref) + if err != nil { + return nil, err + } + req[i].OCITagVersions = tags + } + } return res.Resolve(req, repoNames) } From a8df413c4116fde82c00ba72563ff5ff5d1b8300 Mon Sep 17 00:00:00 2001 From: Scott Rigby Date: Fri, 7 Jan 2022 15:38:41 -0500 Subject: [PATCH 08/27] Update ORAS to v1.1.0-rc1 Now that https://github.com/oras-project/oras-go/pull/89 is merged and released Signed-off-by: Scott Rigby --- go.mod | 2 -- go.sum | 2 -- 2 files changed, 4 deletions(-) diff --git a/go.mod b/go.mod index 427cbc04a..d328ad8bb 100644 --- a/go.mod +++ b/go.mod @@ -45,5 +45,3 @@ require ( oras.land/oras-go v1.1.0-rc3 sigs.k8s.io/yaml v1.3.0 ) - -replace oras.land/oras-go => github.com/sabre1041/oras-go v1.0.1-0.20220105042547-7ebb8809edae diff --git a/go.sum b/go.sum index 19662166d..c56a45885 100644 --- a/go.sum +++ b/go.sum @@ -953,8 +953,6 @@ github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQD github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf35Ld67mk= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/ryanuber/columnize v0.0.0-20160712163229-9b3edd62028f/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts= -github.com/sabre1041/oras-go v1.0.1-0.20220105042547-7ebb8809edae h1:ZhMHqeREElljvMEkMcIak/rRWxWe7ut0x1jb0yFQ8CI= -github.com/sabre1041/oras-go v1.0.1-0.20220105042547-7ebb8809edae/go.mod h1:MZN6VbUUZjfWRF1EJKPbAWhJtE+R8JVicRmeYZp8qDg= github.com/safchain/ethtool v0.0.0-20190326074333-42ed695e3de8/go.mod h1:Z0q5wiBQGYcxhMZ6gUqHn6pYNLypFAvaL3UvgZLR0U4= github.com/sagikazarmark/crypt v0.3.0/go.mod h1:uD/D+6UF4SrIR1uGEv7bBNkNqLGqUr43MRiaGWX1Nig= github.com/satori/go.uuid v1.2.0/go.mod h1:dA0hQrYB0VpLJoorglMZABFdXlWrHn1NEOzdhQKdks0= From 1fabbabae9fd0c76fccf2a6658de867d8cf96ab9 Mon Sep 17 00:00:00 2001 From: Scott Rigby Date: Fri, 7 Jan 2022 15:47:30 -0500 Subject: [PATCH 09/27] Fix Trim to TrimPrefix Signed-off-by: Scott Rigby --- pkg/downloader/manager.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index d8c0eeb9c..8fd17fa14 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -245,7 +245,7 @@ func (m *Manager) resolve(req []*chart.Dependency, repoNames map[string]string) if registry.IsOCI(d.Repository) { // TODO(scottrigby): fix HTTP error // > Error: Get "https://localhost:5000/v2/helm-charts/tags/list": http: server gave HTTP response to HTTPS client - ref := strings.Trim(d.Repository, "oci://") + ref := strings.TrimPrefix(d.Repository, "oci://") tags, err := m.RegistryClient.Tags(ref) if err != nil { return nil, err From 834a11db567b1e154ed976c03a7556c336ad4c7b Mon Sep 17 00:00:00 2001 From: Andrew Block Date: Sat, 8 Jan 2022 22:10:21 -0600 Subject: [PATCH 10/27] Added registryClient to resolver Signed-off-by: Andrew Block --- internal/resolver/resolver.go | 14 ++++++++------ internal/resolver/resolver_test.go | 4 +++- pkg/downloader/manager.go | 2 +- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index 7fc27acac..634b351a6 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -39,15 +39,17 @@ const FeatureGateOCI = gates.Gate("HELM_EXPERIMENTAL_OCI") // Resolver resolves dependencies from semantic version ranges to a particular version. type Resolver struct { - chartpath string - cachepath string + chartpath string + cachepath string + registryClient *registry.Client } -// New creates a new resolver for a given chart and a given helm home. -func New(chartpath, cachepath string) *Resolver { +// New creates a new resolver for a given chart, helm home and registry client. +func New(chartpath, cachepath string, registryClient *registry.Client) *Resolver { return &Resolver{ - chartpath: chartpath, - cachepath: cachepath, + chartpath: chartpath, + cachepath: cachepath, + registryClient: registryClient, } } diff --git a/internal/resolver/resolver_test.go b/internal/resolver/resolver_test.go index 419f8f316..6fbc2e86a 100644 --- a/internal/resolver/resolver_test.go +++ b/internal/resolver/resolver_test.go @@ -19,6 +19,7 @@ import ( "runtime" "testing" + "helm.sh/helm/v3/internal/experimental/registry" "helm.sh/helm/v3/pkg/chart" ) @@ -139,7 +140,8 @@ func TestResolve(t *testing.T) { } repoNames := map[string]string{"alpine": "kubernetes-charts", "redis": "kubernetes-charts"} - r := New("testdata/chartpath", "testdata/repository") + registryClient, _ := registry.NewClient() + r := New("testdata/chartpath", "testdata/repository", registryClient) for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { l, err := r.Resolve(tt.req, repoNames) diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index 8fd17fa14..74286b3fd 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -232,7 +232,7 @@ func (m *Manager) loadChartDir() (*chart.Chart, error) { // // This returns a lock file, which has all of the dependencies normalized to a specific version. func (m *Manager) resolve(req []*chart.Dependency, repoNames map[string]string) (*chart.Lock, error) { - res := resolver.New(m.ChartPath, m.RepositoryCache) + res := resolver.New(m.ChartPath, m.RepositoryCache, m.RegistryClient) // NOTE: When OCI dependencies specify a semver range in Version, // (*Resolver).Resolve must somehow get the list of OCI tag versions // available to check against this constraint. However for backward From 0fae7f5008b4dc64dc5f830b819094c78a313d06 Mon Sep 17 00:00:00 2001 From: Andrew Block Date: Sun, 9 Jan 2022 14:47:08 -0600 Subject: [PATCH 11/27] Updated tag resolver logic Signed-off-by: Andrew Block --- internal/experimental/registry/client.go | 28 ++++++++++++++++++++++-- internal/resolver/resolver.go | 11 +++++++--- pkg/chart/dependency.go | 4 ---- pkg/downloader/manager.go | 20 ----------------- 4 files changed, 34 insertions(+), 29 deletions(-) diff --git a/internal/experimental/registry/client.go b/internal/experimental/registry/client.go index fdfab76f4..a9badb9d5 100644 --- a/internal/experimental/registry/client.go +++ b/internal/experimental/registry/client.go @@ -22,8 +22,10 @@ import ( "io" "io/ioutil" "net/http" + "sort" "strings" + "github.com/Masterminds/semver/v3" "github.com/containerd/containerd/remotes" ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" @@ -553,7 +555,7 @@ func PushOptStrictMode(strictMode bool) PushOption { } } -// Tags lists all tags for a given repository +// Tags provides an all semver compliant tags for a given repository func (c *Client) Tags(ref string) ([]string, error) { parsedReference, err := registry.ParseReference(ref) if err != nil { @@ -565,5 +567,27 @@ func (c *Client) Tags(ref string) ([]string, error) { Client: c.registryAuthorizer, } - return registry.Tags(ctx(c.out, c.debug), &repository) + registrtyTags, err := registry.Tags(ctx(c.out, c.debug), &repository) + if err != nil { + return nil, err + } + + var tagVersions []*semver.Version + for _, tag := range registrtyTags { + tagVersion, err := semver.StrictNewVersion(tag) + if err != nil { + tagVersions = append(tagVersions, tagVersion) + } + } + + sort.Sort(semver.Collection(tagVersions)) + + tags := make([]string, len(tagVersions)) + + for iTv, tv := range tagVersions { + tags[iTv] = tv.String() + } + + return tags, nil + } diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index 634b351a6..27ca9929d 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -18,6 +18,7 @@ package resolver import ( "bytes" "encoding/json" + "fmt" "os" "path/filepath" "strings" @@ -136,15 +137,19 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string } found = false } else { + fmt.Println("Entering OCI block") version = d.Version if !FeatureGateOCI.IsEnabled() { return nil, errors.Wrapf(FeatureGateOCI.Error(), "repository %s is an OCI registry", d.Repository) } - // This works, fake it to test mocking the version - // tags := []string{"0.1.0", "0.1.1", "0.2.0"} - tags := d.OCITagVersions + // Retrive list of tags for repository + tags, err := r.registryClient.Tags(d.Repository) + if err != nil { + return nil, errors.Wrapf(err, "could not retrieve list of tags for repository", d.Repository) + } + vs = make(repo.ChartVersions, len(tags)) for ti, t := range tags { // Mock chart version objects diff --git a/pkg/chart/dependency.go b/pkg/chart/dependency.go index 890fa971f..b2819f373 100644 --- a/pkg/chart/dependency.go +++ b/pkg/chart/dependency.go @@ -47,10 +47,6 @@ type Dependency struct { ImportValues []interface{} `json:"import-values,omitempty"` // Alias usable alias to be used for the chart Alias string `json:"alias,omitempty"` - // OCI tag versions available to check against the constraint when Version - // contains a semantic version range. - // See (*Manager).resolve. - OCITagVersions []string `json:"-"` } // Validate checks for common problems with the dependency datastructure in diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index 74286b3fd..d26dfd845 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -233,26 +233,6 @@ func (m *Manager) loadChartDir() (*chart.Chart, error) { // This returns a lock file, which has all of the dependencies normalized to a specific version. func (m *Manager) resolve(req []*chart.Dependency, repoNames map[string]string) (*chart.Lock, error) { res := resolver.New(m.ChartPath, m.RepositoryCache, m.RegistryClient) - // NOTE: When OCI dependencies specify a semver range in Version, - // (*Resolver).Resolve must somehow get the list of OCI tag versions - // available to check against this constraint. However for backward - // compatibility we can not change that function signature to pass a - // *registry.Client required to get tags from the ORAS tag listing API. - // However we can add a new backward compatible struct field to - // *chart.Dependency, so that we can pass these along to - // (*Resolver).resolve from here through reqs []*chart.Dependency. - for i, d := range req { - if registry.IsOCI(d.Repository) { - // TODO(scottrigby): fix HTTP error - // > Error: Get "https://localhost:5000/v2/helm-charts/tags/list": http: server gave HTTP response to HTTPS client - ref := strings.TrimPrefix(d.Repository, "oci://") - tags, err := m.RegistryClient.Tags(ref) - if err != nil { - return nil, err - } - req[i].OCITagVersions = tags - } - } return res.Resolve(req, repoNames) } From df98e18eb73cd08feee1c9e7ddf0e629ad510ee7 Mon Sep 17 00:00:00 2001 From: Andrew Block Date: Sun, 9 Jan 2022 18:47:31 -0600 Subject: [PATCH 12/27] Working oci code without providing versions Signed-off-by: Andrew Block --- internal/experimental/registry/client.go | 9 +++++---- internal/experimental/registry/util.go | 10 ++++++++++ internal/resolver/resolver.go | 4 ++-- pkg/action/install.go | 5 ++--- pkg/getter/ocigetter.go | 25 ++++++++++++++++++++++-- 5 files changed, 42 insertions(+), 11 deletions(-) diff --git a/internal/experimental/registry/client.go b/internal/experimental/registry/client.go index a9badb9d5..b3bc49612 100644 --- a/internal/experimental/registry/client.go +++ b/internal/experimental/registry/client.go @@ -567,20 +567,21 @@ func (c *Client) Tags(ref string) ([]string, error) { Client: c.registryAuthorizer, } - registrtyTags, err := registry.Tags(ctx(c.out, c.debug), &repository) + registryTags, err := registry.Tags(ctx(c.out, c.debug), &repository) if err != nil { return nil, err } var tagVersions []*semver.Version - for _, tag := range registrtyTags { + for _, tag := range registryTags { tagVersion, err := semver.StrictNewVersion(tag) - if err != nil { + if err == nil { tagVersions = append(tagVersions, tagVersion) } } - sort.Sort(semver.Collection(tagVersions)) + // Sort the collection + sort.Sort(sort.Reverse(semver.Collection(tagVersions))) tags := make([]string, len(tagVersions)) diff --git a/internal/experimental/registry/util.go b/internal/experimental/registry/util.go index c421e5639..909a803a3 100644 --- a/internal/experimental/registry/util.go +++ b/internal/experimental/registry/util.go @@ -36,6 +36,16 @@ func IsOCI(url string) bool { return strings.HasPrefix(url, fmt.Sprintf("%s://", OCIScheme)) } +// ContainsTag determines whether a tag is found in a provided list of tags +func ContainsTag(tags []string, tag string) bool { + for _, t := range tags { + if tag == t { + return true + } + } + return false +} + // extractChartMeta is used to extract a chart metadata from a byte array func extractChartMeta(chartData []byte) (*chart.Metadata, error) { ch, err := loader.LoadArchive(bytes.NewReader(chartData)) diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index 27ca9929d..36a4803e1 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -144,10 +144,10 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string "repository %s is an OCI registry", d.Repository) } - // Retrive list of tags for repository + // Retrieve list of tags for repository tags, err := r.registryClient.Tags(d.Repository) if err != nil { - return nil, errors.Wrapf(err, "could not retrieve list of tags for repository", d.Repository) + return nil, errors.Wrapf(err, "could not retrieve list of tags for repository %s", d.Repository) } vs = make(repo.ChartVersions, len(tags)) diff --git a/pkg/action/install.go b/pkg/action/install.go index 922f728b0..250dfae95 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -695,10 +695,9 @@ func (c *ChartPathOptions) LocateChart(name string, settings *cli.EnvSettings) ( } if registry.IsOCI(name) { - if version == "" { - return "", errors.New("version is explicitly required for OCI registries") + if version != "" { + dl.Options = append(dl.Options, getter.WithTagName(version)) } - dl.Options = append(dl.Options, getter.WithTagName(version)) } if c.Verify { diff --git a/pkg/getter/ocigetter.go b/pkg/getter/ocigetter.go index 45c92749c..fbfcf7133 100644 --- a/pkg/getter/ocigetter.go +++ b/pkg/getter/ocigetter.go @@ -20,6 +20,8 @@ import ( "fmt" "strings" + "github.com/pkg/errors" + "helm.sh/helm/v3/internal/experimental/registry" ) @@ -50,10 +52,29 @@ func (g *OCIGetter) get(href string) (*bytes.Buffer, error) { registry.PullOptWithProv(true)) } - if version := g.opts.version; version != "" { - ref = fmt.Sprintf("%s:%s", ref, version) + // Retrieve list of repository tags + tags, err := client.Tags(ref) + if err != nil { + return nil, err } + //Determine if version provided. If not + providedVersion := g.opts.version + if g.opts.version == "" { + if len(tags) > 0 { + providedVersion = tags[0] + } else { + return nil, errors.Errorf("Unable to locate any tags in provided repository: %s", ref) + } + + } else { + if !registry.ContainsTag(tags, providedVersion) { + return nil, errors.Errorf("Could not located provided version %s in repository %s", providedVersion, ref) + } + } + + ref = fmt.Sprintf("%s:%s", ref, providedVersion) + result, err := client.Pull(ref, pullOpts...) if err != nil { return nil, err From 4f62d3dc1b8b95246eb345f8e9117adee6d52f91 Mon Sep 17 00:00:00 2001 From: Andrew Block Date: Sun, 9 Jan 2022 19:51:03 -0600 Subject: [PATCH 13/27] Started work on 'helm pull' Signed-off-by: Andrew Block --- internal/resolver/resolver.go | 2 -- pkg/action/pull.go | 12 ++++++------ 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index 36a4803e1..5afb439e3 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -18,7 +18,6 @@ package resolver import ( "bytes" "encoding/json" - "fmt" "os" "path/filepath" "strings" @@ -137,7 +136,6 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string } found = false } else { - fmt.Println("Entering OCI block") version = d.Version if !FeatureGateOCI.IsEnabled() { return nil, errors.Wrapf(FeatureGateOCI.Error(), diff --git a/pkg/action/pull.go b/pkg/action/pull.go index 2f5127ea9..f09da82d0 100644 --- a/pkg/action/pull.go +++ b/pkg/action/pull.go @@ -92,13 +92,13 @@ func (p *Pull) Run(chartRef string) (string, error) { } if registry.IsOCI(chartRef) { - if p.Version == "" { - return out.String(), errors.Errorf("--version flag is explicitly required for OCI registries") - } - c.Options = append(c.Options, - getter.WithRegistryClient(p.cfg.RegistryClient), - getter.WithTagName(p.Version)) + getter.WithRegistryClient(p.cfg.RegistryClient)) + + if p.Version != "" { + c.Options = append(c.Options, + getter.WithTagName(p.Version)) + } } if p.Verify { From b6bf3905f37b13863efc5c5309bb154853ac6daa Mon Sep 17 00:00:00 2001 From: Scott Rigby Date: Mon, 10 Jan 2022 14:47:45 -0500 Subject: [PATCH 14/27] Implement reusable GetTagMatchingVersionOrConstraint Largely borrowed from (IndexFile).Get. However there is not currently a nice way to make this code also usable to the repo package, as IndexFile depends on a list of index Entries containing a nexted version. We could refactor this later to somehow use the same shared function, but for now keeping separate. Signed-off-by: Scott Rigby --- internal/experimental/registry/util.go | 31 ++++++++++++++++++++++++++ pkg/getter/ocigetter.go | 31 +++++++++++++------------- 2 files changed, 46 insertions(+), 16 deletions(-) diff --git a/internal/experimental/registry/util.go b/internal/experimental/registry/util.go index 909a803a3..36713676a 100644 --- a/internal/experimental/registry/util.go +++ b/internal/experimental/registry/util.go @@ -23,6 +23,8 @@ import ( "io" "strings" + "github.com/Masterminds/semver" + "github.com/pkg/errors" "github.com/sirupsen/logrus" orascontext "oras.land/oras-go/pkg/context" "oras.land/oras-go/pkg/registry" @@ -46,6 +48,35 @@ func ContainsTag(tags []string, tag string) bool { return false } +func GetTagMatchingVersionOrConstraint(tags []string, versionString string) (string, error) { + var constraint *semver.Constraints + if versionString == "" { + // If string is empty, set wildcard constraint + constraint, _ = semver.NewConstraint("*") + } else { + // Otherwise set constraint to the string given + var err error + constraint, err = semver.NewConstraint(versionString) + if err != nil { + return "", err + } + } + + // Otherwise try to find the first available version matching the string, + // in case it is a constraint + for _, v := range tags { + test, err := semver.NewVersion(v) + if err != nil { + continue + } + if constraint.Check(test) { + return v, nil + } + } + + return "", errors.Errorf("Could not locate a version matching provided version string %s", versionString) +} + // extractChartMeta is used to extract a chart metadata from a byte array func extractChartMeta(chartData []byte) (*chart.Metadata, error) { ch, err := loader.LoadArchive(bytes.NewReader(chartData)) diff --git a/pkg/getter/ocigetter.go b/pkg/getter/ocigetter.go index fbfcf7133..0de5987db 100644 --- a/pkg/getter/ocigetter.go +++ b/pkg/getter/ocigetter.go @@ -30,7 +30,7 @@ type OCIGetter struct { opts options } -//Get performs a Get from repo.Getter and returns the body. +// Get performs a Get from repo.Getter and returns the body. func (g *OCIGetter) Get(href string, options ...Option) (*bytes.Buffer, error) { for _, opt := range options { opt(&g.opts) @@ -57,23 +57,22 @@ func (g *OCIGetter) get(href string) (*bytes.Buffer, error) { if err != nil { return nil, err } - - //Determine if version provided. If not - providedVersion := g.opts.version - if g.opts.version == "" { - if len(tags) > 0 { - providedVersion = tags[0] - } else { - return nil, errors.Errorf("Unable to locate any tags in provided repository: %s", ref) - } - - } else { - if !registry.ContainsTag(tags, providedVersion) { - return nil, errors.Errorf("Could not located provided version %s in repository %s", providedVersion, ref) - } + if len(tags) == 0 { + return nil, errors.Errorf("Unable to locate any tags in provided repository: %s", ref) } - ref = fmt.Sprintf("%s:%s", ref, providedVersion) + // Determine if version provided + // If empty, try to get the highest available tag + // If exact version, try to find it + // If semver constraint string, try to find a match + providedVersion := g.opts.version + + tag, err := registry.GetTagMatchingVersionOrConstraint(tags, providedVersion) + if err != nil { + return nil, err + } + + ref = fmt.Sprintf("%s:%s", ref, tag) result, err := client.Pull(ref, pullOpts...) if err != nil { From 4c8a3faaa28e67316297d68fcb1cca0a54a8ae56 Mon Sep 17 00:00:00 2001 From: Scott Rigby Date: Mon, 10 Jan 2022 17:54:46 -0500 Subject: [PATCH 15/27] Fix import Signed-off-by: Scott Rigby --- internal/experimental/registry/util.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/experimental/registry/util.go b/internal/experimental/registry/util.go index 36713676a..dc4133ac1 100644 --- a/internal/experimental/registry/util.go +++ b/internal/experimental/registry/util.go @@ -23,7 +23,7 @@ import ( "io" "strings" - "github.com/Masterminds/semver" + "github.com/Masterminds/semver/v3" "github.com/pkg/errors" "github.com/sirupsen/logrus" orascontext "oras.land/oras-go/pkg/context" From 291c17fcc5627c85a0c378e0ba83eb0c5ae6dffc Mon Sep 17 00:00:00 2001 From: Andrew Block Date: Tue, 11 Jan 2022 19:13:08 -0600 Subject: [PATCH 16/27] Enabled auth and support http registries for OCI Signed-off-by: Andrew Block --- internal/experimental/registry/client.go | 42 ++++++++++++++++--- internal/experimental/registry/client_test.go | 20 ++++++++- 2 files changed, 54 insertions(+), 8 deletions(-) diff --git a/internal/experimental/registry/client.go b/internal/experimental/registry/client.go index b3bc49612..2de348ccd 100644 --- a/internal/experimental/registry/client.go +++ b/internal/experimental/registry/client.go @@ -17,6 +17,7 @@ limitations under the License. package registry // import "helm.sh/helm/v3/internal/experimental/registry" import ( + "context" "encoding/json" "fmt" "io" @@ -34,7 +35,7 @@ import ( "oras.land/oras-go/pkg/content" "oras.land/oras-go/pkg/oras" "oras.land/oras-go/pkg/registry" - registrremote "oras.land/oras-go/pkg/registry/remote" + registryremote "oras.land/oras-go/pkg/registry/remote" registryauth "oras.land/oras-go/pkg/registry/remote/auth" "helm.sh/helm/v3/internal/version" @@ -100,6 +101,23 @@ func NewClient(options ...ClientOption) (*Client, error) { "User-Agent": {version.GetUserAgent()}, }, Cache: registryauth.DefaultCache, + Credential: func(ctx context.Context, reg string) (registryauth.Credential, error) { + dockerClient, ok := client.authorizer.(*dockerauth.Client) + if !ok { + return registryauth.EmptyCredential, errors.New("unable to obtain docker client") + } + + username, password, err := dockerClient.Credential(reg) + if err != nil { + return registryauth.EmptyCredential, errors.New("unable to retrieve credentials") + } + + return registryauth.Credential{ + Username: username, + Password: password, + }, nil + + }, } } @@ -555,21 +573,33 @@ func PushOptStrictMode(strictMode bool) PushOption { } } -// Tags provides an all semver compliant tags for a given repository +// Tags provides a sorted list all semver compliant tags for a given repository func (c *Client) Tags(ref string) ([]string, error) { parsedReference, err := registry.ParseReference(ref) if err != nil { return nil, err } - repository := registrremote.Repository{ + repository := registryremote.Repository{ Reference: parsedReference, Client: c.registryAuthorizer, } - registryTags, err := registry.Tags(ctx(c.out, c.debug), &repository) - if err != nil { - return nil, err + var registryTags []string + + for { + registryTags, err = registry.Tags(ctx(c.out, c.debug), &repository) + if err != nil { + // Fallback to http based request + if !repository.PlainHTTP && strings.Contains(err.Error(), "server gave HTTP response") { + repository.PlainHTTP = true + continue + } + return nil, err + } + + break + } var tagVersions []*semver.Version diff --git a/internal/experimental/registry/client_test.go b/internal/experimental/registry/client_test.go index 356f3eaba..baf9b4291 100644 --- a/internal/experimental/registry/client_test.go +++ b/internal/experimental/registry/client_test.go @@ -294,7 +294,23 @@ func (suite *RegistryClientTestSuite) Test_2_Pull() { suite.Equal(provData, result.Prov.Data) } -func (suite *RegistryClientTestSuite) Test_3_Logout() { +func (suite *RegistryClientTestSuite) Test_3_Tags() { + + // Load test chart (to build ref pushed in previous test) + chartData, err := ioutil.ReadFile("../../../pkg/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", suite.DockerRegistryHost, meta.Name) + + // Query for tags and validate length + tags, err := suite.RegistryClient.Tags(ref) + suite.Nil(err, "no error retrieving tags") + suite.Equal(1, len(tags)) + +} + +func (suite *RegistryClientTestSuite) Test_4_Logout() { err := suite.RegistryClient.Logout("this-host-aint-real:5000") suite.NotNil(err, "error logging out of registry that has no entry") @@ -302,7 +318,7 @@ func (suite *RegistryClientTestSuite) Test_3_Logout() { suite.Nil(err, "no error logging out of registry") } -func (suite *RegistryClientTestSuite) Test_4_ManInTheMiddle() { +func (suite *RegistryClientTestSuite) Test_5_ManInTheMiddle() { ref := fmt.Sprintf("%s/testrepo/supposedlysafechart:9.9.9", suite.CompromisedRegistryHost) // returns content that does not match the expected digest From 828941b273e1c6393bec6723ae2cc394c0536db4 Mon Sep 17 00:00:00 2001 From: Andrew Block Date: Tue, 11 Jan 2022 20:54:47 -0600 Subject: [PATCH 17/27] Readded resolver OCI logic Signed-off-by: Andrew Block --- internal/resolver/resolver.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index 5afb439e3..79d72917d 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -143,8 +143,8 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string } // Retrieve list of tags for repository - tags, err := r.registryClient.Tags(d.Repository) - if err != nil { + ref := fmt.Sprintf("%s/%s", strings.TrimPrefix(d.Repository, fmt.Sprintf("%s://", registry.OCIScheme)), d.Name) + tags, err := r.registryClient.Tags(ref) if err != nil { return nil, errors.Wrapf(err, "could not retrieve list of tags for repository %s", d.Repository) } From 9c3b0008896d840237617bae8cc9823b77b536d4 Mon Sep 17 00:00:00 2001 From: Andrew Block Date: Tue, 11 Jan 2022 21:05:28 -0600 Subject: [PATCH 18/27] Fixed bad commit Signed-off-by: Andrew Block --- internal/resolver/resolver.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index 79d72917d..cf370e927 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -18,6 +18,7 @@ package resolver import ( "bytes" "encoding/json" + "fmt" "os" "path/filepath" "strings" @@ -144,7 +145,8 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string // Retrieve list of tags for repository ref := fmt.Sprintf("%s/%s", strings.TrimPrefix(d.Repository, fmt.Sprintf("%s://", registry.OCIScheme)), d.Name) - tags, err := r.registryClient.Tags(ref) if err != nil { + tags, err := r.registryClient.Tags(ref) + if err != nil { return nil, errors.Wrapf(err, "could not retrieve list of tags for repository %s", d.Repository) } From 1b3e0bc46a3e22cfa8f0adce08a18c8436152b30 Mon Sep 17 00:00:00 2001 From: Scott Rigby Date: Wed, 12 Jan 2022 12:38:32 -0500 Subject: [PATCH 19/27] Update oras-go to v1.1.0 Signed-off-by: Scott Rigby --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index d328ad8bb..49f96ea97 100644 --- a/go.mod +++ b/go.mod @@ -42,6 +42,6 @@ require ( k8s.io/client-go v0.23.1 k8s.io/klog/v2 v2.30.0 k8s.io/kubectl v0.23.1 - oras.land/oras-go v1.1.0-rc3 + oras.land/oras-go v1.1.0 sigs.k8s.io/yaml v1.3.0 ) diff --git a/go.sum b/go.sum index c56a45885..88da64deb 100644 --- a/go.sum +++ b/go.sum @@ -1746,8 +1746,8 @@ k8s.io/utils v0.0.0-20201110183641-67b214c5f920/go.mod h1:jPW/WVKK9YHAvNhRxK0md/ k8s.io/utils v0.0.0-20210802155522-efc7438f0176/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA= k8s.io/utils v0.0.0-20210930125809-cb0fa318a74b h1:wxEMGetGMur3J1xuGLQY7GEQYg9bZxKn3tKo5k/eYcs= k8s.io/utils v0.0.0-20210930125809-cb0fa318a74b/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA= -oras.land/oras-go v1.1.0-rc3 h1:+HdHR0Lgm/0jmjF4SqUif8Ky2XNWhrcP4hxGVvtKIUI= -oras.land/oras-go v1.1.0-rc3/go.mod h1:1A7vR/0KknT2UkJVWh+xMi95I/AhK8ZrxrnUSmXN0bQ= +oras.land/oras-go v1.1.0 h1:tfWM1RT7PzUwWphqHU6ptPU3ZhwVnSw/9nEGf519rYg= +oras.land/oras-go v1.1.0/go.mod h1:1A7vR/0KknT2UkJVWh+xMi95I/AhK8ZrxrnUSmXN0bQ= rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0= rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA= From c7b2a9d48795e1191b80a1f94acca5b4f084d612 Mon Sep 17 00:00:00 2001 From: Matt Farina Date: Mon, 10 Jan 2022 11:20:50 -0500 Subject: [PATCH 20/27] Fixing issue where OCI handling early causes a bad message Note, there is OCI handling later in the funtion that should handle the situation instead. Closes #10534 Signed-off-by: Matt Farina --- pkg/downloader/manager.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index d26dfd845..aa2c4a2f4 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -578,8 +578,7 @@ func (m *Manager) resolveRepoNames(deps []*chart.Dependency) (map[string]string, missing := []string{} for _, dd := range deps { // Don't map the repository, we don't need to download chart from charts directory - // When OCI is used there is no Helm repository - if dd.Repository == "" || registry.IsOCI(dd.Repository) { + if dd.Repository == "" { continue } // if dep chart is from local path, verify the path is valid From 4d50526a2bcf788d2518cb42dd468863bd3f0332 Mon Sep 17 00:00:00 2001 From: Scott Rigby Date: Wed, 12 Jan 2022 14:36:54 -0500 Subject: [PATCH 21/27] Move OCI tag semver range logic from OCIGetter to ChartDownloader Signed-off-by: Scott Rigby --- pkg/action/pull.go | 5 ----- pkg/downloader/chart_downloader.go | 35 +++++++++++++++++++++++++++++- pkg/getter/ocigetter.go | 24 -------------------- 3 files changed, 34 insertions(+), 30 deletions(-) diff --git a/pkg/action/pull.go b/pkg/action/pull.go index f09da82d0..6de08a6da 100644 --- a/pkg/action/pull.go +++ b/pkg/action/pull.go @@ -94,11 +94,6 @@ func (p *Pull) Run(chartRef string) (string, error) { if registry.IsOCI(chartRef) { c.Options = append(c.Options, getter.WithRegistryClient(p.cfg.RegistryClient)) - - if p.Version != "" { - c.Options = append(c.Options, - getter.WithTagName(p.Version)) - } } if p.Verify { diff --git a/pkg/downloader/chart_downloader.go b/pkg/downloader/chart_downloader.go index 93afb1461..cd5a5d83d 100644 --- a/pkg/downloader/chart_downloader.go +++ b/pkg/downloader/chart_downloader.go @@ -139,12 +139,41 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven return destfile, ver, nil } +func (c *ChartDownloader) getOciUri(ref, version string, u *url.URL) (*url.URL, error) { + // Retrieve list of repository tags + tags, err := c.RegistryClient.Tags(ref) + if err != nil { + return nil, err + } + if len(tags) == 0 { + return nil, errors.Errorf("Unable to locate any tags in provided repository: %s", ref) + } + + // Determine if version provided + // If empty, try to get the highest available tag + // If exact version, try to find it + // If semver constraint string, try to find a match + providedVersion := version + + tag, err := registry.GetTagMatchingVersionOrConstraint(tags, providedVersion) + if err != nil { + return nil, err + } + + // TODO Find a net/url equivalent of this + //ref = fmt.Sprintf("%s:%s", ref, tag) + u.Path = fmt.Sprintf("%s:%s", u.Path, tag) + + return u, err +} + // ResolveChartVersion resolves a chart reference to a URL. // // It returns the URL and sets the ChartDownloader's Options that can fetch // the URL using the appropriate Getter. // -// A reference may be an HTTP URL, a 'reponame/chartname' reference, or a local path. +// A reference may be an HTTP URL, an oci reference URL, a 'reponame/chartname' +// reference, or a local path. // // A version is a SemVer string (1.2.3-beta.1+f334a6789). // @@ -159,6 +188,10 @@ func (c *ChartDownloader) ResolveChartVersion(ref, version string) (*url.URL, er return nil, errors.Errorf("invalid chart URL format: %s", ref) } + if registry.IsOCI(u.Path) { + return c.getOciUri(ref, version, u) + } + rf, err := loadRepoConfig(c.RepositoryConfig) if err != nil { return u, err diff --git a/pkg/getter/ocigetter.go b/pkg/getter/ocigetter.go index 0de5987db..dbf382102 100644 --- a/pkg/getter/ocigetter.go +++ b/pkg/getter/ocigetter.go @@ -20,8 +20,6 @@ import ( "fmt" "strings" - "github.com/pkg/errors" - "helm.sh/helm/v3/internal/experimental/registry" ) @@ -52,28 +50,6 @@ func (g *OCIGetter) get(href string) (*bytes.Buffer, error) { registry.PullOptWithProv(true)) } - // Retrieve list of repository tags - tags, err := client.Tags(ref) - if err != nil { - return nil, err - } - if len(tags) == 0 { - return nil, errors.Errorf("Unable to locate any tags in provided repository: %s", ref) - } - - // Determine if version provided - // If empty, try to get the highest available tag - // If exact version, try to find it - // If semver constraint string, try to find a match - providedVersion := g.opts.version - - tag, err := registry.GetTagMatchingVersionOrConstraint(tags, providedVersion) - if err != nil { - return nil, err - } - - ref = fmt.Sprintf("%s:%s", ref, tag) - result, err := client.Pull(ref, pullOpts...) if err != nil { return nil, err From ba4020770ec21286dbf9cb4b6c6c82b42ee8d4d3 Mon Sep 17 00:00:00 2001 From: Scott Rigby Date: Wed, 12 Jan 2022 14:45:10 -0500 Subject: [PATCH 22/27] Fix linting Signed-off-by: Scott Rigby --- pkg/downloader/chart_downloader.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/downloader/chart_downloader.go b/pkg/downloader/chart_downloader.go index cd5a5d83d..879c36d36 100644 --- a/pkg/downloader/chart_downloader.go +++ b/pkg/downloader/chart_downloader.go @@ -139,7 +139,7 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven return destfile, ver, nil } -func (c *ChartDownloader) getOciUri(ref, version string, u *url.URL) (*url.URL, error) { +func (c *ChartDownloader) getOciURI(ref, version string, u *url.URL) (*url.URL, error) { // Retrieve list of repository tags tags, err := c.RegistryClient.Tags(ref) if err != nil { @@ -189,7 +189,7 @@ func (c *ChartDownloader) ResolveChartVersion(ref, version string) (*url.URL, er } if registry.IsOCI(u.Path) { - return c.getOciUri(ref, version, u) + return c.getOciURI(ref, version, u) } rf, err := loadRepoConfig(c.RepositoryConfig) From 23989f9e2411aa603066356076f43840bcd2d212 Mon Sep 17 00:00:00 2001 From: Andrew Block Date: Wed, 12 Jan 2022 14:32:42 -0600 Subject: [PATCH 23/27] Updates to chart downloader Signed-off-by: Andrew Block --- pkg/downloader/chart_downloader.go | 6 ++---- pkg/downloader/manager.go | 1 + 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/pkg/downloader/chart_downloader.go b/pkg/downloader/chart_downloader.go index 879c36d36..b8853a1a6 100644 --- a/pkg/downloader/chart_downloader.go +++ b/pkg/downloader/chart_downloader.go @@ -141,7 +141,7 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven func (c *ChartDownloader) getOciURI(ref, version string, u *url.URL) (*url.URL, error) { // Retrieve list of repository tags - tags, err := c.RegistryClient.Tags(ref) + tags, err := c.RegistryClient.Tags(strings.TrimPrefix(ref, fmt.Sprintf("%s://", registry.OCIScheme))) if err != nil { return nil, err } @@ -160,8 +160,6 @@ func (c *ChartDownloader) getOciURI(ref, version string, u *url.URL) (*url.URL, return nil, err } - // TODO Find a net/url equivalent of this - //ref = fmt.Sprintf("%s:%s", ref, tag) u.Path = fmt.Sprintf("%s:%s", u.Path, tag) return u, err @@ -188,7 +186,7 @@ func (c *ChartDownloader) ResolveChartVersion(ref, version string) (*url.URL, er return nil, errors.Errorf("invalid chart URL format: %s", ref) } - if registry.IsOCI(u.Path) { + if registry.IsOCI(u.String()) { return c.getOciURI(ref, version, u) } diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index aa2c4a2f4..cc59ae864 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -332,6 +332,7 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error { Keyring: m.Keyring, RepositoryConfig: m.RepositoryConfig, RepositoryCache: m.RepositoryCache, + RegistryClient: m.RegistryClient, Getters: m.Getters, Options: []getter.Option{ getter.WithBasicAuth(username, password), From 1a9cb93551dd112ab91c0dd0982819979787e637 Mon Sep 17 00:00:00 2001 From: Andrew Block Date: Wed, 12 Jan 2022 15:49:12 -0600 Subject: [PATCH 24/27] Handling name of OCI file Signed-off-by: Andrew Block --- pkg/action/pull.go | 1 + pkg/downloader/chart_downloader.go | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/action/pull.go b/pkg/action/pull.go index 6de08a6da..55a127d4d 100644 --- a/pkg/action/pull.go +++ b/pkg/action/pull.go @@ -87,6 +87,7 @@ func (p *Pull) Run(chartRef string) (string, error) { getter.WithTLSClientConfig(p.CertFile, p.KeyFile, p.CaFile), getter.WithInsecureSkipVerifyTLS(p.InsecureSkipTLSverify), }, + RegistryClient: p.cfg.RegistryClient, RepositoryConfig: p.Settings.RepositoryConfig, RepositoryCache: p.Settings.RepositoryCache, } diff --git a/pkg/downloader/chart_downloader.go b/pkg/downloader/chart_downloader.go index b8853a1a6..8bcd76395 100644 --- a/pkg/downloader/chart_downloader.go +++ b/pkg/downloader/chart_downloader.go @@ -103,7 +103,8 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven name := filepath.Base(u.Path) if u.Scheme == registry.OCIScheme { - name = fmt.Sprintf("%s-%s.tgz", name, version) + idx := strings.LastIndexByte(name, ':') + name = fmt.Sprintf("%s-%s.tgz", name[:idx], name[idx+1:]) } destfile := filepath.Join(dest, name) From bd754a054c73a6c6dc4d342cda0a0ecdd4ce96d0 Mon Sep 17 00:00:00 2001 From: Scott Rigby Date: Wed, 12 Jan 2022 17:42:52 -0500 Subject: [PATCH 25/27] Bring exact version check logic from IndexFile.Get into registry tag check Signed-off-by: Scott Rigby --- internal/experimental/registry/util.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/internal/experimental/registry/util.go b/internal/experimental/registry/util.go index dc4133ac1..8c6eb1958 100644 --- a/internal/experimental/registry/util.go +++ b/internal/experimental/registry/util.go @@ -54,6 +54,14 @@ func GetTagMatchingVersionOrConstraint(tags []string, versionString string) (str // If string is empty, set wildcard constraint constraint, _ = semver.NewConstraint("*") } else { + // when customer input exact version, check whether have exact match + // one first + for _, v := range tags { + if versionString == v { + return v, nil + } + } + // Otherwise set constraint to the string given var err error constraint, err = semver.NewConstraint(versionString) From ee382eb169bab70a3b44571ce5c363b80f1399e1 Mon Sep 17 00:00:00 2001 From: Scott Rigby Date: Wed, 12 Jan 2022 17:43:10 -0500 Subject: [PATCH 26/27] Remove unneeded assignment Signed-off-by: Scott Rigby --- pkg/downloader/chart_downloader.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/downloader/chart_downloader.go b/pkg/downloader/chart_downloader.go index 8bcd76395..8ca5cacb6 100644 --- a/pkg/downloader/chart_downloader.go +++ b/pkg/downloader/chart_downloader.go @@ -154,9 +154,7 @@ func (c *ChartDownloader) getOciURI(ref, version string, u *url.URL) (*url.URL, // If empty, try to get the highest available tag // If exact version, try to find it // If semver constraint string, try to find a match - providedVersion := version - - tag, err := registry.GetTagMatchingVersionOrConstraint(tags, providedVersion) + tag, err := registry.GetTagMatchingVersionOrConstraint(tags, version) if err != nil { return nil, err } From 808a2d1908088137a16d086dd86802ece7bd13b3 Mon Sep 17 00:00:00 2001 From: Scott Rigby Date: Wed, 12 Jan 2022 17:59:56 -0500 Subject: [PATCH 27/27] Change underscore (_) back to plus (+) for Helm Signed-off-by: Scott Rigby --- internal/experimental/registry/client.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/experimental/registry/client.go b/internal/experimental/registry/client.go index 2de348ccd..1b686b8ba 100644 --- a/internal/experimental/registry/client.go +++ b/internal/experimental/registry/client.go @@ -604,7 +604,9 @@ func (c *Client) Tags(ref string) ([]string, error) { var tagVersions []*semver.Version for _, tag := range registryTags { - tagVersion, err := semver.StrictNewVersion(tag) + // Change underscore (_) back to plus (+) for Helm + // See https://github.com/helm/helm/issues/10166 + tagVersion, err := semver.StrictNewVersion(strings.ReplaceAll(tag, "_", "+")) if err == nil { tagVersions = append(tagVersions, tagVersion) }