From df7d2173c93358ecd700f890c2fd6c58b9a7e8ef Mon Sep 17 00:00:00 2001 From: Miguel Ruiz Date: Fri, 20 Dec 2024 13:07:41 +0100 Subject: [PATCH 01/10] Add support for OCI tarball url Signed-off-by: Miguel Ruiz --- cmd/asset-syncer/server/utils.go | 69 ++++++++++++++++++++++++++++++-- 1 file changed, 66 insertions(+), 3 deletions(-) diff --git a/cmd/asset-syncer/server/utils.go b/cmd/asset-syncer/server/utils.go index 38625cc3ea7..aef22c79ce0 100644 --- a/cmd/asset-syncer/server/utils.go +++ b/cmd/asset-syncer/server/utils.go @@ -300,19 +300,82 @@ func (r *HelmRepo) Charts(ctx context.Context, fetchLatestOnly bool, chartResult // FetchFiles retrieves the important files of a chart and version from the repo func (r *HelmRepo) FetchFiles(cv models.ChartVersion, userAgent string, passCredentials bool) (map[string]string, error) { authorizationHeader := "" - chartTarballURL := chartTarballURL(r.AppRepositoryInternal, cv) + chartTarballURL, err := url.Parse(chartTarballURL(r.AppRepositoryInternal, cv)) + if err != nil { + return nil, err + } - if passCredentials || len(r.AuthorizationHeader) > 0 && isURLDomainEqual(chartTarballURL, r.URL) { + if passCredentials || len(r.AuthorizationHeader) > 0 && isURLDomainEqual(chartTarballURL.String(), r.URL) { authorizationHeader = r.AuthorizationHeader } + // If URL points to an OCI chart, we transform its URL to its tgz blob URL + if chartTarballURL.Scheme == "oci" { + // Override the chart tarball with the oci blob tarball + chartTarballURL, err = getOciTarballUrl(chartTarballURL, userAgent, authorizationHeader, r.netClient) + if err != nil { + return nil, err + } + } + return tarutil.FetchChartDetailFromTarballUrl( - chartTarballURL, + chartTarballURL.String(), userAgent, authorizationHeader, r.netClient) } +func getOciTarballUrl(chartTarballURL *url.URL, userAgent string, authorizationHeader string, netClient *http.Client) (*url.URL, error) { + // If URL points to an OCI chart, we transform its URL to its tgz blob URL + // Extract the tag from the chart Path + chartTag := "latest" + i := strings.Index(chartTarballURL.Path, ":") + if i >= 0 { + chartTag = chartTarballURL.Path[i+1:] + chartTarballURL.Path = chartTarballURL.Path[:i] + } + // TODO: I would like to refactor the OciAPIClient to be generic and allow generating an OCI client without all the oci-catalog specific code + // IMPORTANT: Currently, getOrasRepoClient(appname, userAgent) is too specific, I would need to be able to generate an OCI client for other general purposes by simply providing an url. + o := OciAPIClient{RegistryNamespaceUrl: chartTarballURL, HttpClient: netClient, GrpcClient: nil, AuthorizationHeader: authorizationHeader} + orasRepoClient, err := o.getOrasRepoClient(path.Join(chartTarballURL.Host, chartTarballURL.Path), "", "") + if err != nil { + panic(err) + } + ctx := context.TODO() + // TODO: This code is too similar to the function 'IsHelmChart'. + // Again, I would like to move both functions into a common 'fetchOciManifest' function in order to simplify the code structure + descriptor, rc, err := orasRepoClient.Manifests().FetchReference(ctx, chartTag) + if err != nil { + panic(err) + } + defer rc.Close() + + manifestData, err := content.ReadAll(rc, descriptor) + if err != nil { + panic(err) + } + + var manifest OCIManifest + err = json.Unmarshal(manifestData, &manifest) + + if len(manifest.Layers) != 1 || manifest.Layers[0].MediaType != "application/vnd.cncf.helm.chart.content.v1.tar+gzip" { + log.Errorf("Unexpected layer in index manifest: %v", manifest) + return nil, fmt.Errorf("unexpected layer in chart manifest") + } + + ociTarballUrl, err := buildChartOciTarballUrl("https", chartTarballURL.Host, chartTarballURL.Path, manifest.Layers[0].Digest) + if err != nil { + return nil, err + } + + return ociTarballUrl, nil +} + +func buildChartOciTarballUrl(schema string, registry string, repository string, blobDigest string) (*url.URL, error) { + ociTarballUrl := fmt.Sprintf("%s://%s/v2/%s/blobs/%s", schema, registry, repository, blobDigest) + return url.Parse(ociTarballUrl) +} + // TagList represents a list of tags as specified at // https://github.com/opencontainers/distribution-spec/blob/main/spec.md#content-discovery type TagList struct { From b56086d88c270c57f7084876fe58a4e3ba047c14 Mon Sep 17 00:00:00 2001 From: Miguel Ruiz Date: Fri, 20 Dec 2024 13:15:03 +0100 Subject: [PATCH 02/10] Replace Index with LastIndex Signed-off-by: Miguel Ruiz --- cmd/asset-syncer/server/utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/asset-syncer/server/utils.go b/cmd/asset-syncer/server/utils.go index aef22c79ce0..7bffe2a5aaf 100644 --- a/cmd/asset-syncer/server/utils.go +++ b/cmd/asset-syncer/server/utils.go @@ -329,7 +329,7 @@ func getOciTarballUrl(chartTarballURL *url.URL, userAgent string, authorizationH // If URL points to an OCI chart, we transform its URL to its tgz blob URL // Extract the tag from the chart Path chartTag := "latest" - i := strings.Index(chartTarballURL.Path, ":") + i := strings.LastIndex(chartTarballURL.Path, ":") if i >= 0 { chartTag = chartTarballURL.Path[i+1:] chartTarballURL.Path = chartTarballURL.Path[:i] From c9bf8f11caf0d301c1b6ce24c2bcfdc7d875a7f8 Mon Sep 17 00:00:00 2001 From: Miguel Ruiz Date: Fri, 20 Dec 2024 13:32:13 +0100 Subject: [PATCH 03/10] Adapt OciAPIClient to existing function Signed-off-by: Miguel Ruiz --- cmd/asset-syncer/server/utils.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/cmd/asset-syncer/server/utils.go b/cmd/asset-syncer/server/utils.go index 7bffe2a5aaf..fd0f7530ff1 100644 --- a/cmd/asset-syncer/server/utils.go +++ b/cmd/asset-syncer/server/utils.go @@ -329,15 +329,18 @@ func getOciTarballUrl(chartTarballURL *url.URL, userAgent string, authorizationH // If URL points to an OCI chart, we transform its URL to its tgz blob URL // Extract the tag from the chart Path chartTag := "latest" - i := strings.LastIndex(chartTarballURL.Path, ":") + i := strings.Index(chartTarballURL.Path, ":") if i >= 0 { chartTag = chartTarballURL.Path[i+1:] chartTarballURL.Path = chartTarballURL.Path[:i] } + j := strings.LastIndex(chartTarballURL.Path, "/") + RegistryNamespaceUrl, err := url.Parse(chartTarballURL.Path[j+1:]) + appName := chartTarballURL.Path[:j] // TODO: I would like to refactor the OciAPIClient to be generic and allow generating an OCI client without all the oci-catalog specific code // IMPORTANT: Currently, getOrasRepoClient(appname, userAgent) is too specific, I would need to be able to generate an OCI client for other general purposes by simply providing an url. - o := OciAPIClient{RegistryNamespaceUrl: chartTarballURL, HttpClient: netClient, GrpcClient: nil, AuthorizationHeader: authorizationHeader} - orasRepoClient, err := o.getOrasRepoClient(path.Join(chartTarballURL.Host, chartTarballURL.Path), "", "") + o := OciAPIClient{RegistryNamespaceUrl: RegistryNamespaceUrl, HttpClient: netClient, GrpcClient: nil} + orasRepoClient, err := o.getOrasRepoClient(appName, "") if err != nil { panic(err) } From 5caa0a5496eb54418a45380b17c5a4b6b86e39ac Mon Sep 17 00:00:00 2001 From: Miguel Ruiz Date: Mon, 23 Dec 2024 16:35:25 +0100 Subject: [PATCH 04/10] Fix deployment of OCI charts urls inside regular Helm index Signed-off-by: Miguel Ruiz --- cmd/asset-syncer/server/utils.go | 65 ++++++++++++------- .../plugins/helm/packages/v1alpha1/server.go | 2 +- .../packages/v1alpha1/utils/chart-client.go | 15 ++--- .../packages/v1alpha1/utils/fake/chart.go | 2 +- 4 files changed, 49 insertions(+), 35 deletions(-) diff --git a/cmd/asset-syncer/server/utils.go b/cmd/asset-syncer/server/utils.go index fd0f7530ff1..146282e22a8 100644 --- a/cmd/asset-syncer/server/utils.go +++ b/cmd/asset-syncer/server/utils.go @@ -311,21 +311,37 @@ func (r *HelmRepo) FetchFiles(cv models.ChartVersion, userAgent string, passCred // If URL points to an OCI chart, we transform its URL to its tgz blob URL if chartTarballURL.Scheme == "oci" { - // Override the chart tarball with the oci blob tarball - chartTarballURL, err = getOciTarballUrl(chartTarballURL, userAgent, authorizationHeader, r.netClient) - if err != nil { - return nil, err - } + return FetchChartDetailFromOciUrl(chartTarballURL, userAgent, authorizationHeader, r.netClient) + } else { + return FetchChartDetailFromTarballUrl(chartTarballURL, userAgent, authorizationHeader, r.netClient) + } +} + +// Fetches helm chart details from a gzipped tarball +// +// name is expected in format "foo/bar" or "foo%2Fbar" if url-escaped +func FetchChartDetailFromTarballUrl(chartTarballURL *url.URL, userAgent string, authz string, netClient *http.Client) (map[string]string, error) { + reqHeaders := make(map[string]string) + if len(userAgent) > 0 { + reqHeaders["User-Agent"] = userAgent + } + if len(authz) > 0 { + reqHeaders["Authorization"] = authz + } + + // use our "standard" http-client library + reader, _, err := httpclient.GetStream(chartTarballURL.String(), netClient, reqHeaders) + if reader != nil { + defer reader.Close() } - return tarutil.FetchChartDetailFromTarballUrl( - chartTarballURL.String(), - userAgent, - authorizationHeader, - r.netClient) + if err != nil { + return nil, err + } + return tarutil.FetchChartDetailFromTarball(reader) } -func getOciTarballUrl(chartTarballURL *url.URL, userAgent string, authorizationHeader string, netClient *http.Client) (*url.URL, error) { +func FetchChartDetailFromOciUrl(chartTarballURL *url.URL, userAgent string, authorizationHeader string, netClient *http.Client) (map[string]string, error) { // If URL points to an OCI chart, we transform its URL to its tgz blob URL // Extract the tag from the chart Path chartTag := "latest" @@ -334,12 +350,13 @@ func getOciTarballUrl(chartTarballURL *url.URL, userAgent string, authorizationH chartTag = chartTarballURL.Path[i+1:] chartTarballURL.Path = chartTarballURL.Path[:i] } + // Separate the appname from the oci Url j := strings.LastIndex(chartTarballURL.Path, "/") - RegistryNamespaceUrl, err := url.Parse(chartTarballURL.Path[j+1:]) - appName := chartTarballURL.Path[:j] + appName := chartTarballURL.Path[j+1:] + chartTarballURL.Path = chartTarballURL.Path[:j] // TODO: I would like to refactor the OciAPIClient to be generic and allow generating an OCI client without all the oci-catalog specific code // IMPORTANT: Currently, getOrasRepoClient(appname, userAgent) is too specific, I would need to be able to generate an OCI client for other general purposes by simply providing an url. - o := OciAPIClient{RegistryNamespaceUrl: RegistryNamespaceUrl, HttpClient: netClient, GrpcClient: nil} + o := OciAPIClient{RegistryNamespaceUrl: chartTarballURL, HttpClient: netClient, GrpcClient: nil} orasRepoClient, err := o.getOrasRepoClient(appName, "") if err != nil { panic(err) @@ -347,13 +364,13 @@ func getOciTarballUrl(chartTarballURL *url.URL, userAgent string, authorizationH ctx := context.TODO() // TODO: This code is too similar to the function 'IsHelmChart'. // Again, I would like to move both functions into a common 'fetchOciManifest' function in order to simplify the code structure - descriptor, rc, err := orasRepoClient.Manifests().FetchReference(ctx, chartTag) + manifestDescriptor, rc, err := orasRepoClient.Manifests().FetchReference(ctx, chartTag) if err != nil { panic(err) } defer rc.Close() - manifestData, err := content.ReadAll(rc, descriptor) + manifestData, err := content.ReadAll(rc, manifestDescriptor) if err != nil { panic(err) } @@ -366,17 +383,19 @@ func getOciTarballUrl(chartTarballURL *url.URL, userAgent string, authorizationH return nil, fmt.Errorf("unexpected layer in chart manifest") } - ociTarballUrl, err := buildChartOciTarballUrl("https", chartTarballURL.Host, chartTarballURL.Path, manifest.Layers[0].Digest) + blobDescriptor, err := orasRepoClient.Blobs().Resolve(ctx, manifest.Layers[0].Digest) + if err != nil { + return nil, err + } + reader, err := orasRepoClient.Blobs().Fetch(ctx, blobDescriptor) + if reader != nil { + defer reader.Close() + } if err != nil { return nil, err } - return ociTarballUrl, nil -} - -func buildChartOciTarballUrl(schema string, registry string, repository string, blobDigest string) (*url.URL, error) { - ociTarballUrl := fmt.Sprintf("%s://%s/v2/%s/blobs/%s", schema, registry, repository, blobDigest) - return url.Parse(ociTarballUrl) + return tarutil.FetchChartDetailFromTarball(reader) } // TagList represents a list of tags as specified at diff --git a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/server.go b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/server.go index 9012eef3de1..2c0e6a55468 100644 --- a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/server.go +++ b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/server.go @@ -963,7 +963,7 @@ func (s *Server) fetchChartWithRegistrySecrets(ctx context.Context, headers http }, appRepo, caCertSecret, authSecret, - s.chartClientFactory.New(appRepo.Spec.Type, userAgentString), + s.chartClientFactory.New(tarballURL, userAgentString), ) if err != nil { return nil, nil, connect.NewError(connect.CodeInternal, fmt.Errorf("Unable to fetch the chart %s from the namespace %q: %w", chartDetails.ChartName, appRepo.Namespace, err)) diff --git a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/chart-client.go b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/chart-client.go index 7b7d0c9acaa..1a8819d4930 100644 --- a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/chart-client.go +++ b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/chart-client.go @@ -184,16 +184,12 @@ func (c *OCIRepoClient) GetChart(details *ChartDetails, repoURL string) (*chart. if c.puller == nil { return nil, fmt.Errorf("unable to retrieve chart, Init should be called first") } - parsedURL, err := url.ParseRequestURI(strings.TrimSpace(repoURL)) - if err != nil { - return nil, err - } - unescapedChartName, err := url.QueryUnescape(details.ChartName) + parsedURL, err := url.Parse(strings.TrimSpace(details.TarballURL)) if err != nil { return nil, err } - ref := path.Join(parsedURL.Host, parsedURL.Path, fmt.Sprintf("%s:%s", unescapedChartName, details.Version)) + ref := path.Join(parsedURL.Host, parsedURL.Path) chartBuffer, _, err := c.puller.PullOCIChart(ref) if err != nil { return nil, err @@ -215,12 +211,11 @@ type ChartClientFactoryInterface interface { type ChartClientFactory struct{} // New for ClientResolver -func (c *ChartClientFactory) New(repoType, userAgent string) ChartClient { +func (c *ChartClientFactory) New(tarballUrl string, userAgent string) ChartClient { var client ChartClient - switch repoType { - case "oci": + if strings.HasPrefix(tarballUrl, "oci://") { client = NewOCIClient(userAgent) - default: + } else { client = NewChartClient(userAgent) } return client diff --git a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/fake/chart.go b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/fake/chart.go index ad8060a03e8..4f6a9e8ac9e 100644 --- a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/fake/chart.go +++ b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/fake/chart.go @@ -47,6 +47,6 @@ func (f *ChartClient) Init(appRepo *appRepov1.AppRepository, caCertSecret *corev type ChartClientFactory struct{} // New returns a fake ChartClient -func (c *ChartClientFactory) New(repoType, userAgent string) utils.ChartClient { +func (c *ChartClientFactory) New(tarballURL string, userAgent string) utils.ChartClient { return &ChartClient{} } From 021d6de04fb0e9af375ce3ddf0a05c0e018e0317 Mon Sep 17 00:00:00 2001 From: Miguel Ruiz Date: Mon, 23 Dec 2024 16:38:55 +0100 Subject: [PATCH 05/10] Move FetchChartDetailFromTarballUrl from tarutils to assets-syncer Signed-off-by: Miguel Ruiz --- pkg/tarutil/tarutil.go | 27 --------------------------- 1 file changed, 27 deletions(-) diff --git a/pkg/tarutil/tarutil.go b/pkg/tarutil/tarutil.go index c43e6355923..011478b72ce 100644 --- a/pkg/tarutil/tarutil.go +++ b/pkg/tarutil/tarutil.go @@ -8,40 +8,13 @@ import ( "bytes" "compress/gzip" "io" - "net/http" "path" "regexp" "strings" chart "github.com/vmware-tanzu/kubeapps/pkg/chart/models" - httpclient "github.com/vmware-tanzu/kubeapps/pkg/http-client" ) -// Fetches helm chart details from a gzipped tarball -// -// name is expected in format "foo/bar" or "foo%2Fbar" if url-escaped -func FetchChartDetailFromTarballUrl(chartTarballURL string, userAgent string, authz string, netClient *http.Client) (map[string]string, error) { - reqHeaders := make(map[string]string) - if len(userAgent) > 0 { - reqHeaders["User-Agent"] = userAgent - } - if len(authz) > 0 { - reqHeaders["Authorization"] = authz - } - - // use our "standard" http-client library - reader, _, err := httpclient.GetStream(chartTarballURL, netClient, reqHeaders) - if reader != nil { - defer reader.Close() - } - - if err != nil { - return nil, err - } - - return FetchChartDetailFromTarball(reader) -} - // Fetches helm chart details from a gzipped tarball // // name is expected in format "foo/bar" or "foo%2Fbar" if url-escaped From de760f33ad3b90747b7c150365d6c60aa8996443 Mon Sep 17 00:00:00 2001 From: Miguel Ruiz Date: Mon, 23 Dec 2024 17:22:12 +0100 Subject: [PATCH 06/10] Fix errors, linting and simplify Signed-off-by: Miguel Ruiz --- cmd/asset-syncer/server/utils.go | 89 +++---------------- .../packages/v1alpha1/utils/chart-client.go | 7 +- pkg/tarutil/tarutil.go | 27 ++++++ 3 files changed, 41 insertions(+), 82 deletions(-) diff --git a/cmd/asset-syncer/server/utils.go b/cmd/asset-syncer/server/utils.go index 23ed5726c70..c34ac1ab069 100644 --- a/cmd/asset-syncer/server/utils.go +++ b/cmd/asset-syncer/server/utils.go @@ -300,102 +300,39 @@ func (r *HelmRepo) Charts(ctx context.Context, fetchLatestOnly bool, chartResult // FetchFiles retrieves the important files of a chart and version from the repo func (r *HelmRepo) FetchFiles(cv models.ChartVersion, userAgent string, passCredentials bool) (map[string]string, error) { authorizationHeader := "" - chartTarballURL, err := url.Parse(chartTarballURL(r.AppRepositoryInternal, cv)) - if err != nil { - return nil, err - } + chartTarballURL := chartTarballURL(r.AppRepositoryInternal, cv) - if passCredentials || len(r.AuthorizationHeader) > 0 && isURLDomainEqual(chartTarballURL.String(), r.URL) { + if passCredentials || len(r.AuthorizationHeader) > 0 && isURLDomainEqual(chartTarballURL, r.URL) { authorizationHeader = r.AuthorizationHeader } // If URL points to an OCI chart, we transform its URL to its tgz blob URL - if chartTarballURL.Scheme == "oci" { + if strings.HasPrefix(chartTarballURL, "oci://") { return FetchChartDetailFromOciUrl(chartTarballURL, userAgent, authorizationHeader, r.netClient) } else { - return FetchChartDetailFromTarballUrl(chartTarballURL, userAgent, authorizationHeader, r.netClient) + return tarutil.FetchChartDetailFromTarballUrl(chartTarballURL, userAgent, authorizationHeader, r.netClient) } } -// Fetches helm chart details from a gzipped tarball -// -// name is expected in format "foo/bar" or "foo%2Fbar" if url-escaped -func FetchChartDetailFromTarballUrl(chartTarballURL *url.URL, userAgent string, authz string, netClient *http.Client) (map[string]string, error) { - reqHeaders := make(map[string]string) +// Fetches helm chart details from an OCI url +func FetchChartDetailFromOciUrl(chartTarballURL string, userAgent string, authz string, netClient *http.Client) (map[string]string, error) { + headers := http.Header{} if len(userAgent) > 0 { - reqHeaders["User-Agent"] = userAgent + headers.Add("User-Agent", userAgent) } if len(authz) > 0 { - reqHeaders["Authorization"] = authz + headers.Add("Authorization", authz) } - // use our "standard" http-client library - reader, _, err := httpclient.GetStream(chartTarballURL.String(), netClient, reqHeaders) - if reader != nil { - defer reader.Close() - } + puller := &helm.OCIPuller{Resolver: docker.NewResolver(docker.ResolverOptions{Headers: headers, Client: netClient})} - if err != nil { - return nil, err - } - return tarutil.FetchChartDetailFromTarball(reader) -} - -func FetchChartDetailFromOciUrl(chartTarballURL *url.URL, userAgent string, authorizationHeader string, netClient *http.Client) (map[string]string, error) { - // If URL points to an OCI chart, we transform its URL to its tgz blob URL - // Extract the tag from the chart Path - chartTag := "latest" - i := strings.Index(chartTarballURL.Path, ":") - if i >= 0 { - chartTag = chartTarballURL.Path[i+1:] - chartTarballURL.Path = chartTarballURL.Path[:i] - } - // Separate the appname from the oci Url - j := strings.LastIndex(chartTarballURL.Path, "/") - appName := chartTarballURL.Path[j+1:] - chartTarballURL.Path = chartTarballURL.Path[:j] - // TODO: I would like to refactor the OciAPIClient to be generic and allow generating an OCI client without all the oci-catalog specific code - // IMPORTANT: Currently, getOrasRepoClient(appname, userAgent) is too specific, I would need to be able to generate an OCI client for other general purposes by simply providing an url. - o := OciAPIClient{RegistryNamespaceUrl: chartTarballURL, HttpClient: netClient, GrpcClient: nil} - orasRepoClient, err := o.getOrasRepoClient(appName, "") - if err != nil { - panic(err) - } - ctx := context.TODO() - // TODO: This code is too similar to the function 'IsHelmChart'. - // Again, I would like to move both functions into a common 'fetchOciManifest' function in order to simplify the code structure - manifestDescriptor, rc, err := orasRepoClient.Manifests().FetchReference(ctx, chartTag) - if err != nil { - panic(err) - } - defer rc.Close() - - manifestData, err := content.ReadAll(rc, manifestDescriptor) - if err != nil { - panic(err) - } - - var manifest OCIManifest - err = json.Unmarshal(manifestData, &manifest) - - if len(manifest.Layers) != 1 || manifest.Layers[0].MediaType != "application/vnd.cncf.helm.chart.content.v1.tar+gzip" { - log.Errorf("Unexpected layer in index manifest: %v", manifest) - return nil, fmt.Errorf("unexpected layer in chart manifest") - } - - blobDescriptor, err := orasRepoClient.Blobs().Resolve(ctx, manifest.Layers[0].Digest) - if err != nil { - return nil, err - } - reader, err := orasRepoClient.Blobs().Fetch(ctx, blobDescriptor) - if reader != nil { - defer reader.Close() - } + ref := strings.TrimPrefix(strings.TrimSpace(chartTarballURL), "oci://") + chartBuffer, _, err := puller.PullOCIChart(ref) if err != nil { return nil, err } - return tarutil.FetchChartDetailFromTarball(reader) + return tarutil.FetchChartDetailFromTarball(chartBuffer) } // TagList represents a list of tags as specified at diff --git a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/chart-client.go b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/chart-client.go index 1a8819d4930..9a92405c613 100644 --- a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/chart-client.go +++ b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/chart-client.go @@ -8,7 +8,6 @@ import ( "io" "net/http" "net/url" - "path" "strings" "github.com/containerd/containerd/remotes/docker" @@ -184,12 +183,8 @@ func (c *OCIRepoClient) GetChart(details *ChartDetails, repoURL string) (*chart. if c.puller == nil { return nil, fmt.Errorf("unable to retrieve chart, Init should be called first") } - parsedURL, err := url.Parse(strings.TrimSpace(details.TarballURL)) - if err != nil { - return nil, err - } - ref := path.Join(parsedURL.Host, parsedURL.Path) + ref := strings.TrimPrefix(strings.TrimSpace(details.TarballURL), "oci://") chartBuffer, _, err := c.puller.PullOCIChart(ref) if err != nil { return nil, err diff --git a/pkg/tarutil/tarutil.go b/pkg/tarutil/tarutil.go index 011478b72ce..c43e6355923 100644 --- a/pkg/tarutil/tarutil.go +++ b/pkg/tarutil/tarutil.go @@ -8,13 +8,40 @@ import ( "bytes" "compress/gzip" "io" + "net/http" "path" "regexp" "strings" chart "github.com/vmware-tanzu/kubeapps/pkg/chart/models" + httpclient "github.com/vmware-tanzu/kubeapps/pkg/http-client" ) +// Fetches helm chart details from a gzipped tarball +// +// name is expected in format "foo/bar" or "foo%2Fbar" if url-escaped +func FetchChartDetailFromTarballUrl(chartTarballURL string, userAgent string, authz string, netClient *http.Client) (map[string]string, error) { + reqHeaders := make(map[string]string) + if len(userAgent) > 0 { + reqHeaders["User-Agent"] = userAgent + } + if len(authz) > 0 { + reqHeaders["Authorization"] = authz + } + + // use our "standard" http-client library + reader, _, err := httpclient.GetStream(chartTarballURL, netClient, reqHeaders) + if reader != nil { + defer reader.Close() + } + + if err != nil { + return nil, err + } + + return FetchChartDetailFromTarball(reader) +} + // Fetches helm chart details from a gzipped tarball // // name is expected in format "foo/bar" or "foo%2Fbar" if url-escaped From 7bb55adbd270410b77ccc4b2daefe61e920947ea Mon Sep 17 00:00:00 2001 From: Miguel Ruiz Date: Mon, 23 Dec 2024 18:26:43 +0100 Subject: [PATCH 07/10] Fix test error Signed-off-by: Miguel Ruiz --- .../plugins/helm/packages/v1alpha1/utils/chart-client.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/chart-client.go b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/chart-client.go index 9a92405c613..d2e91e39442 100644 --- a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/chart-client.go +++ b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/chart-client.go @@ -183,6 +183,9 @@ func (c *OCIRepoClient) GetChart(details *ChartDetails, repoURL string) (*chart. if c.puller == nil { return nil, fmt.Errorf("unable to retrieve chart, Init should be called first") } + if details.TarballURL == "" { + return nil, fmt.Errorf("calling GetChart '%s - %s' without any tarball url", details.ChartName, details.Version) + } ref := strings.TrimPrefix(strings.TrimSpace(details.TarballURL), "oci://") chartBuffer, _, err := c.puller.PullOCIChart(ref) From add30bf4696ee6c8db20f566fa8c8e4c3e9782a5 Mon Sep 17 00:00:00 2001 From: Miguel Ruiz Date: Mon, 23 Dec 2024 18:37:30 +0100 Subject: [PATCH 08/10] Update test and error condition Signed-off-by: Miguel Ruiz --- .../plugins/helm/packages/v1alpha1/utils/chart-client.go | 5 ++--- .../helm/packages/v1alpha1/utils/chart-client_test.go | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/chart-client.go b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/chart-client.go index d2e91e39442..15d2693d08e 100644 --- a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/chart-client.go +++ b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/chart-client.go @@ -183,10 +183,9 @@ func (c *OCIRepoClient) GetChart(details *ChartDetails, repoURL string) (*chart. if c.puller == nil { return nil, fmt.Errorf("unable to retrieve chart, Init should be called first") } - if details.TarballURL == "" { - return nil, fmt.Errorf("calling GetChart '%s - %s' without any tarball url", details.ChartName, details.Version) + if details == nil || details.TarballURL == "" { + return nil, fmt.Errorf("unable to retrieve chart, missing chart details") } - ref := strings.TrimPrefix(strings.TrimSpace(details.TarballURL), "oci://") chartBuffer, _, err := c.puller.PullOCIChart(ref) if err != nil { diff --git a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/chart-client_test.go b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/chart-client_test.go index 75aebb9aee8..9baf8c31bdd 100644 --- a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/chart-client_test.go +++ b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/chart-client_test.go @@ -218,7 +218,7 @@ func TestOCIClient(t *testing.T) { cli := NewOCIClient("foo") cli.(*OCIRepoClient).puller = &helmfake.OCIPuller{} _, err := cli.GetChart(nil, "foo") - if !strings.Contains(err.Error(), "invalid URI for request") { + if !strings.Contains(err.Error(), "missing chart details") { t.Errorf("Unexpected error %v", err) } }) From 4b45ee67bbc75cd520920b18b221e47b3b8e7ecb Mon Sep 17 00:00:00 2001 From: Miguel Ruiz Date: Tue, 24 Dec 2024 08:43:47 +0100 Subject: [PATCH 09/10] Fix tests Signed-off-by: Miguel Ruiz --- .../helm/packages/v1alpha1/utils/chart-client.go | 14 ++++++++------ .../packages/v1alpha1/utils/chart-client_test.go | 6 +++--- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/chart-client.go b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/chart-client.go index 15d2693d08e..c7c7ff2a6cb 100644 --- a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/chart-client.go +++ b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/chart-client.go @@ -8,6 +8,7 @@ import ( "io" "net/http" "net/url" + "path" "strings" "github.com/containerd/containerd/remotes/docker" @@ -100,7 +101,7 @@ func (c *HelmRepoClient) GetChart(details *ChartDetails, repoURL string) (*chart } log.Infof("Downloading %s ...", chartURL) - chart, err := fetchChart(c.netClient, chartURL) + chart, err := fetchChart(c.netClient, chartURL.String()) if err != nil { return nil, err } @@ -108,16 +109,16 @@ func (c *HelmRepoClient) GetChart(details *ChartDetails, repoURL string) (*chart return chart, nil } -func resolveChartURL(indexURL, chartURL string) (string, error) { +func resolveChartURL(indexURL, chartURL string) (*url.URL, error) { parsedIndexURL, err := url.Parse(strings.TrimSpace(indexURL)) if err != nil { - return "", err + return nil, err } parsedChartURL, err := parsedIndexURL.Parse(strings.TrimSpace(chartURL)) if err != nil { - return "", err + return nil, err } - return parsedChartURL.String(), nil + return parsedChartURL, nil } // fetchChart returns the Chart content given an URL @@ -186,7 +187,8 @@ func (c *OCIRepoClient) GetChart(details *ChartDetails, repoURL string) (*chart. if details == nil || details.TarballURL == "" { return nil, fmt.Errorf("unable to retrieve chart, missing chart details") } - ref := strings.TrimPrefix(strings.TrimSpace(details.TarballURL), "oci://") + chartURL, err := resolveChartURL(repoURL, details.TarballURL) + ref := path.Join(chartURL.Host, chartURL.Path) chartBuffer, _, err := c.puller.PullOCIChart(ref) if err != nil { return nil, err diff --git a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/chart-client_test.go b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/chart-client_test.go index 9baf8c31bdd..a480696958f 100644 --- a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/chart-client_test.go +++ b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/chart-client_test.go @@ -57,7 +57,7 @@ func Test_resolveChartURL(t *testing.T) { t.Run(tt.name, func(t *testing.T) { chartURL, err := resolveChartURL(tt.baseURL, tt.chartURL) assert.NoError(t, err) - assert.Equal(t, chartURL, tt.wantedURL, "url") + assert.Equal(t, chartURL.String(), tt.wantedURL, "url") }) } } @@ -231,7 +231,7 @@ func TestOCIClient(t *testing.T) { ExpectedName: "foo/bar/nginx:5.1.1", Content: map[string]*bytes.Buffer{"5.1.1": bytes.NewBuffer(data)}, } - ch, err := cli.GetChart(&ChartDetails{ChartName: "nginx", Version: "5.1.1"}, "http://foo/bar") + ch, err := cli.GetChart(&ChartDetails{ChartName: "nginx", Version: "5.1.1", TarballURL: "oci://foo/bar/nginx:5.1.1"}, "http://foo/bar") if ch == nil { t.Errorf("Unexpected error: %s", err) } @@ -248,7 +248,7 @@ func TestOCIClient(t *testing.T) { ExpectedName: "foo/bar/bar/nginx:5.1.1", Content: map[string]*bytes.Buffer{"5.1.1": bytes.NewBuffer(data)}, } - ch, err := cli.GetChart(&ChartDetails{ChartName: "nginx", Version: "5.1.1"}, "http://foo/bar%2Fbar") + ch, err := cli.GetChart(&ChartDetails{ChartName: "nginx", Version: "5.1.1", TarballURL: "oci://foo/bar%2Fbar/nginx:5.1.1"}, "http://foo/bar%2Fbar") if ch == nil { t.Errorf("Unexpected error: %s", err) } From c34dab38ea2ce5c19e6d241c9a098b2bb38ea5b8 Mon Sep 17 00:00:00 2001 From: Miguel Ruiz Date: Tue, 24 Dec 2024 08:51:14 +0100 Subject: [PATCH 10/10] Handle error lint Signed-off-by: Miguel Ruiz --- .../plugins/helm/packages/v1alpha1/utils/chart-client.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/chart-client.go b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/chart-client.go index c7c7ff2a6cb..093115444c9 100644 --- a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/chart-client.go +++ b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/utils/chart-client.go @@ -188,6 +188,10 @@ func (c *OCIRepoClient) GetChart(details *ChartDetails, repoURL string) (*chart. return nil, fmt.Errorf("unable to retrieve chart, missing chart details") } chartURL, err := resolveChartURL(repoURL, details.TarballURL) + if err != nil { + return nil, err + } + ref := path.Join(chartURL.Host, chartURL.Path) chartBuffer, _, err := c.puller.PullOCIChart(ref) if err != nil {