corresponding controller changes

Signed-off-by: Kent Rancourt <kent.rancourt@gmail.com>

internal/controller/promotion/argocd.go

@@ -299,7 +299,7 @@ func applyArgoCDSourceUpdate(
 			return source, nil
 		}
 		for _, chart := range newFreight.Charts {
-			if chart.RegistryURL == source.RepoURL && chart.Name == source.Chart {
+			if chart.RepoURL == source.RepoURL && chart.Name == source.Chart {
 				source.TargetRevision = chart.Version
 				break
 			}

internal/controller/promotion/argocd_test.go

@@ -614,9 +614,9 @@ func TestApplyArgoCDSourceUpdate(t *testing.T) {
 			newFreight: kargoapi.FreightReference{
 				Charts: []kargoapi.Chart{
 					{
-						RegistryURL: "fake-url",
-						Name:        "fake-chart",
-						Version:     "fake-version",
+						RepoURL: "fake-url",
+						Name:    "fake-chart",
+						Version: "fake-version",
 					},
 				},
 			},

internal/controller/promotion/helm.go

@@ -3,6 +3,7 @@ package promotion
 import (
 	"fmt"
 	"os"
+	"path"
 	"path/filepath"
 
 	"github.com/pkg/errors"
@@ -191,7 +192,8 @@ func buildChartDependencyChanges(
 	// Build a table of charts --> versions
 	versionsByChart := make(map[string]string, len(charts))
 	for _, chart := range charts {
-		key := fmt.Sprintf("%s:%s", chart.RegistryURL, chart.Name)
+		// path.Join accounts for the possibility that chart.Name is empty
+		key := path.Join(chart.RepoURL, chart.Name)
 		versionsByChart[key] = chart.Version
 	}
 
@@ -222,7 +224,7 @@ func buildChartDependencyChanges(
 				errors.Wrapf(err, "error unmarshaling %q", absChartYAMLPath)
 		}
 		for i, dependency := range chartYAMLObj.Dependencies {
-			chartKey := fmt.Sprintf("%s:%s", dependency.Repository, dependency.Name)
+			chartKey := fmt.Sprintf("%s/%s", dependency.Repository, dependency.Name)
 			version, found := versionsByChart[chartKey]
 			if !found {
 				continue

internal/controller/promotion/helm_test.go

@@ -360,7 +360,7 @@ func TestBuildChartDependencyChanges(t *testing.T) {
 	err = os.WriteFile(
 		filepath.Join(testFooChartDir, "Chart.yaml"),
 		[]byte(`dependencies:
-- repository: fake-registry
+- repository: fake-repo
   name: fake-chart
   version: placeholder
 `),
@@ -376,10 +376,10 @@ func TestBuildChartDependencyChanges(t *testing.T) {
 		filepath.Join(testBarChartDir, "Chart.yaml"),
 		// This fake chart has TWO dependencies -- one of which shouldn't be updated
 		[]byte(`dependencies:
-- repository: another-fake-registry
+- repository: another-fake-repo
   name: another-fake-chart
   version: placeholder
-- repository: yet-another-fake-registry
+- repository: yet-another-fake-repo
   name: yet-another-fake-chart
   version: placeholder
 `),
@@ -390,28 +390,28 @@ func TestBuildChartDependencyChanges(t *testing.T) {
 	// New charts
 	charts := []kargoapi.Chart{
 		{
-			RegistryURL: "fake-registry",
-			Name:        "fake-chart",
-			Version:     "fake-version",
+			RepoURL: "fake-repo",
+			Name:    "fake-chart",
+			Version: "fake-version",
 		},
 		{
-			RegistryURL: "another-fake-registry",
-			Name:        "another-fake-chart",
-			Version:     "another-fake-version",
+			RepoURL: "another-fake-repo",
+			Name:    "another-fake-chart",
+			Version: "another-fake-version",
 		},
 	}
 
 	// Instructions for how to update Chart.yaml files
 	chartUpdates := []kargoapi.HelmChartDependencyUpdate{
 		{
-			RegistryURL: "fake-registry",
-			Name:        "fake-chart",
-			ChartPath:   "charts/foo",
+			Repository: "fake-repo",
+			Name:       "fake-chart",
+			ChartPath:  "charts/foo",
 		},
 		{
-			RegistryURL: "another-fake-registry",
-			Name:        "another-fake-chart",
-			ChartPath:   "charts/bar",
+			Repository: "another-fake-repo",
+			Name:       "another-fake-chart",
+			ChartPath:  "charts/bar",
 		},
 		// Note there is no mention of how to update bar's second dependency, so
 		// we expect it to be left alone.

internal/controller/stages/health.go

@@ -3,6 +3,7 @@ package stages
 import (
 	"context"
 	"fmt"
+	"path"
 
 	kargoapi "github.com/akuity/kargo/api/v1alpha1"
 	argocd "github.com/akuity/kargo/internal/controller/argocd/api/v1alpha1"
@@ -119,21 +120,24 @@ func (r *reconciler) checkHealth(
 		}
 
 		var desiredRevision string
-		sourceGitRepoURL := git.NormalizeGitURL(app.Spec.Source.RepoURL)
-		for _, commit := range currentFreight.Commits {
-			if git.NormalizeGitURL(commit.RepoURL) == sourceGitRepoURL {
-				if commit.HealthCheckCommit != "" {
-					desiredRevision = commit.HealthCheckCommit
-				} else {
-					desiredRevision = commit.ID
+		if app.Spec.Source.Chart == "" {
+			// This source points to a git repository
+			sourceGitRepoURL := git.NormalizeGitURL(app.Spec.Source.RepoURL)
+			for _, commit := range currentFreight.Commits {
+				if git.NormalizeGitURL(commit.RepoURL) == sourceGitRepoURL {
+					if commit.HealthCheckCommit != "" {
+						desiredRevision = commit.HealthCheckCommit
+					} else {
+						desiredRevision = commit.ID
+					}
 				}
+				break
 			}
-			break
-		}
-		if desiredRevision == "" {
+		} else {
+			// This source points to a Helm chart
 			for _, chart := range currentFreight.Charts {
-				if chart.RegistryURL == app.Spec.Source.RepoURL &&
-					chart.Name == app.Spec.Source.Chart {
+				// path.Join accounts for the possibility that chart.Name is empty
+				if path.Join(chart.RepoURL, chart.Name) == path.Join(app.Spec.Source.RepoURL, app.Spec.Source.Chart) {
 					desiredRevision = chart.Version
 					break
 				}

internal/controller/warehouses/helm.go

@@ -2,9 +2,9 @@ package warehouses
 
 import (
 	"context"
+	"path"
 
 	"github.com/pkg/errors"
-	log "github.com/sirupsen/logrus"
 
 	kargoapi "github.com/akuity/kargo/api/v1alpha1"
 	"github.com/akuity/kargo/internal/credentials"
@@ -26,18 +26,20 @@ func (r *reconciler) getLatestCharts(
 
 		sub := s.Chart
 
-		logger := logging.LoggerFromContext(ctx).WithFields(log.Fields{
-			"registry": sub.RegistryURL,
-			"chart":    sub.Name,
-		})
+		logger := logging.LoggerFromContext(ctx).WithField("repoURL", sub.RepoURL)
+		if sub.Name != "" {
+			logger = logger.WithField("chart", sub.Name)
+		}
 
+		// path.Join accounts for the possibility that chart.Name is empty
+		chartURL := path.Join(sub.RepoURL, sub.Name)
 		creds, ok, err :=
-			r.credentialsDB.Get(ctx, namespace, credentials.TypeHelm, sub.RegistryURL)
+			r.credentialsDB.Get(ctx, namespace, credentials.TypeHelm, chartURL)
 		if err != nil {
 			return nil, errors.Wrapf(
 				err,
-				"error obtaining credentials for chart registry %q",
-				sub.RegistryURL,
+				"error obtaining credentials for chart %q",
+				chartURL,
 			)
 		}
 
@@ -54,26 +56,39 @@ func (r *reconciler) getLatestCharts(
 
 		vers, err := r.getLatestChartVersionFn(
 			ctx,
-			sub.RegistryURL,
+			sub.RepoURL,
 			sub.Name,
 			sub.SemverConstraint,
 			helmCreds,
 		)
 		if err != nil {
+			if sub.Name == "" {
+				return nil, errors.Wrapf(
+					err,
+					"error searching for latest version of chart in repository %q",
+					sub.RepoURL,
+				)
+			}
 			return nil, errors.Wrapf(
 				err,
-				"error searching for latest version of chart %q in registry %q",
+				"error searching for latest version of chart %q in repository %q",
 				sub.Name,
-				sub.RegistryURL,
+				sub.RepoURL,
 			)
 		}
 
 		if vers == "" {
 			logger.Error("found no suitable chart version")
+			if sub.Name == "" {
+				return nil, errors.Errorf(
+					"found no suitable version of chart in repository %q",
+					sub.RepoURL,
+				)
+			}
 			return nil, errors.Errorf(
-				"found no suitable version of chart %q in registry %q",
+				"found no suitable version of chart %q in repository %q",
 				sub.Name,
-				sub.RegistryURL,
+				sub.RepoURL,
 			)
 		}
 		logger.WithField("version", vers).
@@ -82,9 +97,9 @@ func (r *reconciler) getLatestCharts(
 		charts = append(
 			charts,
 			kargoapi.Chart{
-				RegistryURL: sub.RegistryURL,
-				Name:        sub.Name,
-				Version:     vers,
+				RepoURL: sub.RepoURL,
+				Name:    sub.Name,
+				Version: vers,
 			},
 		)
 	}

internal/controller/warehouses/helm_test.go

@@ -26,7 +26,7 @@ func TestGetLatestCharts(t *testing.T) {
 		assertions func([]kargoapi.Chart, error)
 	}{
 		{
-			name: "error getting registry credentials",
+			name: "error getting repository credentials",
 			credentialsDB: &credentials.FakeDB{
 				GetFn: func(
 					context.Context,
@@ -43,7 +43,7 @@ func TestGetLatestCharts(t *testing.T) {
 				require.Contains(
 					t,
 					err.Error(),
-					"error obtaining credentials for chart registry",
+					"error obtaining credentials for chart",
 				)
 				require.Contains(t, err.Error(), "something went wrong")
 			},
@@ -135,9 +135,9 @@ func TestGetLatestCharts(t *testing.T) {
 				require.Equal(
 					t,
 					kargoapi.Chart{
-						RegistryURL: "fake-url",
-						Name:        "fake-chart",
-						Version:     "1.0.0",
+						RepoURL: "fake-url",
+						Name:    "fake-chart",
+						Version: "1.0.0",
 					},
 					charts[0],
 				)
@@ -156,8 +156,8 @@ func TestGetLatestCharts(t *testing.T) {
 				[]kargoapi.RepoSubscription{
 					{
 						Chart: &kargoapi.ChartSubscription{
-							RegistryURL: "fake-url",
-							Name:        "fake-chart",
+							RepoURL: "fake-url",
+							Name:    "fake-chart",
 						},
 					},
 				},

internal/controller/warehouses/warehouses.go

@@ -77,7 +77,7 @@ type reconciler struct {
 
 	getLatestChartVersionFn func(
 		ctx context.Context,
-		registryURL string,
+		repoURL string,
 		chart string,
 		semverConstraint string,
 		creds *helm.Credentials,

internal/controller/warehouses/warehouses_test.go

@@ -334,9 +334,9 @@ func TestGetLatestFreightFromRepos(t *testing.T) {
 				) ([]kargoapi.Chart, error) {
 					return []kargoapi.Chart{
 						{
-							RegistryURL: "fake-registry",
-							Name:        "fake-chart",
-							Version:     "fake-version",
+							RepoURL: "fake-repo",
+							Name:    "fake-chart",
+							Version: "fake-version",
 						},
 					}, nil
 				},
@@ -371,9 +371,9 @@ func TestGetLatestFreightFromRepos(t *testing.T) {
 						},
 						Charts: []kargoapi.Chart{
 							{
-								RegistryURL: "fake-registry",
-								Name:        "fake-chart",
-								Version:     "fake-version",
+								RepoURL: "fake-repo",
+								Name:    "fake-chart",
+								Version: "fake-version",
 							},
 						},
 					},

internal/credentials/credentials.go

@@ -262,11 +262,9 @@ func getCredentialsSecret(
 	repoURL string,
 	acceptPrefixMatch bool,
 ) (*corev1.Secret, error) {
-	if credType == TypeGit {
-		// This is important. We don't want the presence or absence of ".git" at the
-		// end of the URL to affect credential lookups.
-		repoURL = git.NormalizeGitURL(repoURL)
-	}
+	repoURL = normalizeChartRepositoryURL( // This should be safe even on non-chart repo URLs
+		git.NormalizeGitURL(repoURL), // This should be safe even on non-Git URLs
+	)
 
 	secrets := corev1.SecretList{}
 	if err := kubeClient.List(
@@ -291,17 +289,33 @@ func getCredentialsSecret(
 		if !ok {
 			continue
 		}
-		url := string(urlBytes)
+		url := normalizeChartRepositoryURL( // This should be safe even on non-chart repo URLs
+			git.NormalizeGitURL( // This should be safe even on non-Git URLs
+				string(urlBytes),
+			),
+		)
 		if acceptPrefixMatch && strings.HasPrefix(repoURL, url) {
 			return &secret, nil
 		}
-		if !acceptPrefixMatch && git.NormalizeGitURL(url) == repoURL {
+		if !acceptPrefixMatch && url == repoURL {
 			return &secret, nil
 		}
 	}
 	return nil, nil
 }
 
+// NormalizeURL normalizes a chart repository URL for purposes of comparison.
+// Crucially, this function removes the oci:// prefix from the URL if there is
+// one.
+func normalizeChartRepositoryURL(repo string) string {
+	return strings.TrimPrefix(
+		strings.ToLower(
+			strings.TrimSpace(repo),
+		),
+		"oci://",
+	)
+}
+
 func secretToCreds(secret *corev1.Secret) Credentials {
 	return Credentials{
 		Username:      string(secret.Data["username"]),