Add testCase for OCIChartRepository

This implementation also makes sure that HelmOCIRepository_reconciler
uses an OCIChartRepository for validation

Signed-off-by: Soule BA <[email protected]>

Author: souleb

controllers/helmrepository_controller_oci.go

@@ -3,7 +3,7 @@ package controllers
 import (
 	"context"
 	"fmt"
-	"net/url"
+	"strings"
 	"time"
 
 	"github.com/fluxcd/pkg/apis/meta"
@@ -13,6 +13,7 @@ import (
 	"github.com/fluxcd/pkg/runtime/predicates"
 	sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
 	serror "github.com/fluxcd/source-controller/internal/error"
+	"github.com/fluxcd/source-controller/internal/helm/repository"
 	sreconcile "github.com/fluxcd/source-controller/internal/reconcile"
 	"github.com/fluxcd/source-controller/internal/reconcile/summarize"
 	helmgetter "helm.sh/helm/v3/pkg/getter"
@@ -267,30 +268,29 @@ func (r *HelmRepositoryOCIReconciler) reconcileSource(ctx context.Context, obj *
 // validateSource the HelmRepository object by checking the url and connecting to the underlying registry
 // with he provided credentials.
 func (r *HelmRepositoryOCIReconciler) validateSource(ctx context.Context, obj *sourcev1.HelmRepository, loginOpts ...registry.LoginOption) (sreconcile.Result, error) {
-	target, err := url.Parse(obj.Spec.URL)
+	chartRepo, err := repository.NewOCIChartRepository(obj.Spec.URL, repository.WithOCIRegistryClient(r.RegistryClient))
 	if err != nil {
-		e := &serror.Event{
-			Err:    fmt.Errorf("failed to parse URL '%s': %w", obj.Spec.URL, err),
-			Reason: "ValidationError",
-		}
-		conditions.MarkFalse(obj, sourcev1.SourceValidCondition, e.Reason, e.Err.Error())
-		return sreconcile.ResultEmpty, e
-	}
-
-	// Check if the registry is supported
-	if !registry.IsOCI(obj.Spec.URL) {
-		e := &serror.Event{
-			Err:    fmt.Errorf("unsupported registry scheme '%s'", target.Scheme),
-			Reason: "ValidationError",
+		if strings.Contains(err.Error(), "parse") {
+			e := &serror.Event{
+				Err:    fmt.Errorf("failed to parse URL '%s': %w", obj.Spec.URL, err),
+				Reason: "ValidationError",
+			}
+			conditions.MarkFalse(obj, sourcev1.SourceValidCondition, e.Reason, e.Err.Error())
+			return sreconcile.ResultEmpty, e
+		} else if strings.Contains(err.Error(), "the url scheme is not supported") {
+			e := &serror.Event{
+				Err:    err,
+				Reason: "ValidationError",
+			}
+			conditions.MarkFalse(obj, sourcev1.SourceValidCondition, e.Reason, e.Err.Error())
+			return sreconcile.ResultEmpty, e
 		}
-		conditions.MarkFalse(obj, sourcev1.SourceValidCondition, e.Reason, e.Err.Error())
-		return sreconcile.ResultEmpty, e
 	}
 
-	err = r.RegistryClient.Login(target.Host+target.Path, loginOpts...)
+	err = chartRepo.Login(loginOpts...)
 	if err != nil {
 		e := &serror.Event{
-			Err:    fmt.Errorf("failed to login to registry '%s': %w", target.String(), err),
+			Err:    fmt.Errorf("failed to login to registry: %w", err),
 			Reason: "ValidationError",
 		}
 		conditions.MarkFalse(obj, sourcev1.SourceValidCondition, e.Reason, e.Err.Error())

internal/helm/repository/oci_chart_repository.go

@@ -21,41 +21,49 @@ import (
 	"crypto/tls"
 	"fmt"
 	"net/url"
+	"sort"
 	"strings"
 
 	"helm.sh/helm/v3/pkg/chart"
 	"helm.sh/helm/v3/pkg/getter"
 	"helm.sh/helm/v3/pkg/registry"
 	"helm.sh/helm/v3/pkg/repo"
 
+	"github.com/Masterminds/semver/v3"
+	"github.com/fluxcd/pkg/version"
 	"github.com/fluxcd/source-controller/internal/transport"
 )
 
+type RegistryClient interface {
+	Login(host string, opts ...registry.LoginOption) error
+	Logout(host string, opts ...registry.LogoutOption) error
+	Tags(url string) ([]string, error)
+}
+
 // OCIChartRepository represents a Helm chart repository, and the configuration
 // required to download the repository tags and charts from the repository.
 // All methods are thread safe unless defined otherwise.
 type OCIChartRepository struct {
-	// URL the ChartRepository's index.yaml can be found at,
-	// without the index.yaml suffix.
+	// URL is the location of the repository.
 	URL url.URL
-	// Client to use while downloading the Index or a chart from the URL.
+	// Client to use while accessing the repository's contents.
 	Client getter.Getter
-	// Options to configure the Client with while downloading the Index
+	// Options to configure the Client with while downloading tags
 	// or a chart from the URL.
 	Options []getter.Option
 
 	tlsConfig *tls.Config
 
 	// RegistryClient is a client to use while downloading tags or charts from a registry.
-	RegistryClient *registry.Client
+	RegistryClient RegistryClient
 }
 
 // OCIChartRepositoryOption is a function that can be passed to NewOCIChartRepository
 // to configure an OCIChartRepository.
 type OCIChartRepositoryOption func(*OCIChartRepository) error
 
 // WithOCIRegistryClient returns a ChartRepositoryOption that will set the registry client
-func WithOCIRegistryClient(client *registry.Client) OCIChartRepositoryOption {
+func WithOCIRegistryClient(client RegistryClient) OCIChartRepositoryOption {
 	return func(r *OCIChartRepository) error {
 		r.RegistryClient = client
 		return nil
@@ -139,7 +147,7 @@ func (r *OCIChartRepository) Get(name, ver string) (*repo.ChartVersion, error) {
 	// 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
-	tag, err := registry.GetTagMatchingVersionOrConstraint(cvs, ver)
+	tag, err := getLastMatchingVersionOrConstraint(cvs, ver)
 	return &repo.ChartVersion{
 		URLs: []string{fmt.Sprintf("%s/%s:%s", r.URL.String(), name, tag)},
 		Metadata: &chart.Metadata{
@@ -174,6 +182,10 @@ func (r *OCIChartRepository) DownloadChart(chart *repo.ChartVersion) (*bytes.Buf
 	}
 
 	ref := chart.URLs[0]
+	if !registry.IsOCI(ref) {
+		return nil, fmt.Errorf("chart '%s' is not an OCI chart", chart.Name)
+	}
+
 	u, err := url.Parse(ref)
 	if err != nil {
 		err = fmt.Errorf("invalid chart URL format '%s': %w", ref, err)
@@ -187,3 +199,67 @@ func (r *OCIChartRepository) DownloadChart(chart *repo.ChartVersion) (*bytes.Buf
 	// trim the oci scheme prefix if needed
 	return r.Client.Get(strings.TrimPrefix(u.String(), fmt.Sprintf("%s://", registry.OCIScheme)), clientOpts...)
 }
+
+// Login attempts to login to the OCI registry.
+func (r *OCIChartRepository) Login(opts ...registry.LoginOption) error {
+	err := r.RegistryClient.Login(r.URL.Host, opts...)
+	if err != nil {
+		return err
+	}
+	return nil
+}
+
+// Logout attempts to logout from the OCI registry.
+func (r *OCIChartRepository) Logout() error {
+	err := r.RegistryClient.Logout(r.URL.Host)
+	if err != nil {
+		return err
+	}
+	return nil
+}
+
+func getLastMatchingVersionOrConstraint(cvs []string, ver string) (string, error) {
+	// Check for exact matches first
+	if ver != "" {
+		for _, cv := range cvs {
+			if ver == cv {
+				return cv, nil
+			}
+		}
+	}
+
+	// Continue to look for a (semantic) version match
+	verConstraint, err := semver.NewConstraint("*")
+	if err != nil {
+		return "", err
+	}
+	latestStable := ver == "" || ver == "*"
+	if !latestStable {
+		verConstraint, err = semver.NewConstraint(ver)
+		if err != nil {
+			return "", err
+		}
+	}
+
+	matchingVersions := make([]string, 0, len(cvs))
+	for _, cv := range cvs {
+		v, err := version.ParseVersion(cv)
+		if err != nil {
+			continue
+		}
+
+		if !verConstraint.Check(v) {
+			continue
+		}
+
+		matchingVersions = append(matchingVersions, cv)
+	}
+	if len(matchingVersions) == 0 {
+		return "", fmt.Errorf("could not locate a version matching provided version string %s", ver)
+	}
+
+	// Sort versions
+	sort.Sort(sort.Reverse(sort.StringSlice(matchingVersions)))
+
+	return matchingVersions[0], nil
+}