diff --git a/pkg/argo_client/manifests.go b/pkg/argo_client/manifests.go index 0149a36f..b9153c61 100644 --- a/pkg/argo_client/manifests.go +++ b/pkg/argo_client/manifests.go @@ -100,6 +100,151 @@ func preprocessSources(app *v1alpha1.Application, pullRequest vcs.PullRequest) ( return contentSources, refSources } +// isExternalHelmChart returns true when a source refers to a Helm chart that is +// fetched directly from a Helm registry (OCI or HTTPS), not from a Git repository. +// Such sources cannot be git-cloned; ArgoCD's repo-server is responsible for +// pulling the chart using its own credentials and OCI support. +func isExternalHelmChart(source v1alpha1.ApplicationSource) bool { + // source.Chart is only set for Helm chart sources (not git-path sources) + if source.Chart == "" { + return false + } + // OCI registries with explicit oci:// scheme + if strings.HasPrefix(source.RepoURL, "oci://") { + return true + } + // HTTPS Helm repos: no .git suffix and no source path means it is a plain + // Helm repository URL (e.g. https://charts.example.io) + if strings.HasPrefix(source.RepoURL, "https://") && !strings.HasSuffix(source.RepoURL, ".git") && source.Path == "" { + return true + } + // OCI registry shorthand without scheme (e.g. "docker.io/envoyproxy", + // "ghcr.io/org/charts"). ArgoCD accepts these for OCI Helm sources. + // If Chart is set and the URL has no scheme and doesn't end in .git, + // it's an OCI/Helm registry reference, not a git repo. + if source.Chart != "" && !strings.Contains(source.RepoURL, "://") && !strings.HasSuffix(source.RepoURL, ".git") { + return true + } + return false +} + +// generateManifestForExternalChart handles manifest generation for external Helm chart +// sources (OCI registries and HTTPS Helm repos). Instead of streaming local files via +// GenerateManifestWithFiles, it calls GenerateManifest directly — the repo-server will +// fetch the chart from the registry using its own credentials and OCI/Helm support. +// This avoids needing a local copy of the chart and correctly handles chart dependencies. +func (a *ArgoClient) generateManifestForExternalChart(ctx context.Context, app v1alpha1.Application, source v1alpha1.ApplicationSource, refs []v1alpha1.ApplicationSource) ([]string, error) { + clusterCloser, clusterClient := a.GetClusterClient() + defer pkg.WithErrorLogging(clusterCloser.Close, "failed to close connection") + + clusterData, err := clusterClient.Get(ctx, &cluster.ClusterQuery{Name: app.Spec.Destination.Name, Server: app.Spec.Destination.Server}) + if err != nil { + getManifestsFailed.WithLabelValues(app.Name).Inc() + return nil, errors.Wrap(err, "failed to get cluster") + } + + settingsCloser, settingsClient := a.GetSettingsClient() + defer pkg.WithErrorLogging(settingsCloser.Close, "failed to close connection") + + argoSettings, err := settingsClient.Get(ctx, &settings.SettingsQuery{}) + if err != nil { + getManifestsFailed.WithLabelValues(app.Name).Inc() + return nil, errors.Wrap(err, "failed to get settings") + } + + settingsMgr := argosettings.NewSettingsManager(ctx, a.k8s, a.cfg.ArgoCDNamespace) + argoDB := db.NewDB(a.cfg.ArgoCDNamespace, settingsMgr, a.k8s) + + helmRepos, err := argoDB.ListHelmRepositories(ctx) + if err != nil { + return nil, fmt.Errorf("error listing helm repositories: %w", err) + } + + proj, err := func() (*v1alpha1.AppProject, error) { + closer, projectClient, err := a.client.NewProjectClient() + if err != nil { + return nil, errors.Wrap(err, "failed to get project client") + } + defer pkg.WithErrorLogging(closer.Close, "failed to close connection") + return projectClient.Get(ctx, &project.ProjectQuery{Name: app.Spec.Project}) + }() + if err != nil { + return nil, fmt.Errorf("error getting app project: %w", err) + } + + permittedHelmRepos, err := argo.GetPermittedRepos(proj, helmRepos) + if err != nil { + return nil, fmt.Errorf("error getting permitted helm repos: %w", err) + } + + helmRepositoryCredentials, err := argoDB.GetAllHelmRepositoryCredentials(ctx) + if err != nil { + return nil, fmt.Errorf("error getting helm repository credentials: %w", err) + } + permittedHelmCredentials, err := argo.GetPermittedReposCredentials(proj, helmRepositoryCredentials) + if err != nil { + return nil, fmt.Errorf("error getting permitted repos credentials: %w", err) + } + + enabledSourceTypes, err := settingsMgr.GetEnabledSourceTypes() + if err != nil { + return nil, fmt.Errorf("error getting settings enabled source types: %w", err) + } + + helmOptions, err := settingsMgr.GetHelmSettings() + if err != nil { + return nil, fmt.Errorf("error getting helm settings: %w", err) + } + + refSources, err := argo.GetRefSources(context.Background(), app.Spec.Sources, app.Spec.Project, argoDB.GetRepository, []string{}) + if err != nil { + return nil, fmt.Errorf("failed to get ref sources: %w", err) + } + + // Resolve the repository object so repo-server has credentials for the chart registry. + repo, err := argoDB.GetRepository(ctx, source.RepoURL, app.Spec.Project) + if err != nil { + log.Warn().Err(err).Str("repoURL", source.RepoURL).Msg("could not resolve repository credentials; proceeding without them") + repo = &v1alpha1.Repository{Repo: source.RepoURL} + } + + q := repoapiclient.ManifestRequest{ + Repo: repo, + Revision: source.TargetRevision, + AppLabelKey: argoSettings.AppLabelKey, + AppName: app.Name, + Namespace: app.Spec.Destination.Namespace, + ApplicationSource: &source, + Repos: permittedHelmRepos, + KustomizeOptions: argoSettings.KustomizeOptions, + KubeVersion: clusterData.Info.ServerVersion, + ApiVersions: clusterData.Info.APIVersions, + HelmRepoCreds: permittedHelmCredentials, + HelmOptions: helmOptions, + TrackingMethod: argoSettings.TrackingMethod, + EnabledSourceTypes: enabledSourceTypes, + ProjectName: proj.Name, + ProjectSourceRepos: proj.Spec.SourceRepos, + HasMultipleSources: app.Spec.HasMultipleSources(), + RefSources: refSources, + } + + repoClient, conn, err := a.createRepoServerClient() + if err != nil { + return nil, errors.Wrap(err, "error creating repo client") + } + defer pkg.WithErrorLogging(conn.Close, "failed to close connection") + + log.Debug().Caller().Str("app", app.Name).Msg("calling GenerateManifest for external Helm chart") + resp, err := repoClient.GenerateManifest(ctx, &q) + if err != nil { + getManifestsFailed.WithLabelValues(app.Name).Inc() + return nil, fmt.Errorf("failed to generate manifests for external Helm chart %s: %w", app.Name, err) + } + getManifestsSuccess.WithLabelValues(app.Name).Inc() + return resp.Manifests, nil +} + // generateManifests generates an Application along with all of its files, and sends it to the ArgoCD // Repository service to be transformed into raw kubernetes manifests. This allows us to take advantage of server // configuration and credentials. @@ -109,6 +254,20 @@ func (a *ArgoClient) generateManifests(ctx context.Context, app v1alpha1.Applica // 2. there must be one and only one non-ref source // 3. ref sources that match the pull requests' repo and target branch need to have their target branch swapped to the head branch of the pull request log.Info().Str("app", app.Name).Msg("generating manifests") + + // External Helm chart sources (OCI or HTTPS Helm repos) cannot be git-cloned. + // Instead of streaming files via GenerateManifestWithFiles (which would require + // a local copy of the chart), we delegate directly to GenerateManifest — the + // repo-server will fetch the chart from the registry using its own credentials. + if isExternalHelmChart(source) { + log.Info(). + Str("app", app.Name). + Str("repoURL", source.RepoURL). + Str("chart", source.Chart). + Msg("external Helm chart source detected; delegating to repo-server GenerateManifest") + return a.generateManifestForExternalChart(ctx, app, source, refs) + } + clusterCloser, clusterClient := a.GetClusterClient() defer pkg.WithErrorLogging(clusterCloser.Close, "failed to close connection") @@ -131,26 +290,29 @@ func (a *ArgoClient) generateManifests(ctx context.Context, app v1alpha1.Applica settingsMgr := argosettings.NewSettingsManager(ctx, a.k8s, a.cfg.ArgoCDNamespace) argoDB := db.NewDB(a.cfg.ArgoCDNamespace, settingsMgr, a.k8s) - repoTarget := source.TargetRevision - if pkg.AreSameRepos(source.RepoURL, pullRequest.CloneURL) && areSameTargetRef(source.TargetRevision, pullRequest.BaseRef) { - repoTarget = pullRequest.HeadRef - } + var packageDir string - log.Debug().Caller().Str("app", app.Name).Msg("get repo") - repo, err := getRepo(ctx, source.RepoURL, repoTarget) - if err != nil { - return nil, errors.Wrap(err, "failed to get repo") - } + { + repoTarget := source.TargetRevision + if pkg.AreSameRepos(source.RepoURL, pullRequest.CloneURL) && areSameTargetRef(source.TargetRevision, pullRequest.BaseRef) { + repoTarget = pullRequest.HeadRef + } - var packageDir string - if a.cfg.ArgoCDSendFullRepository { - log.Debug().Caller().Str("app", app.Name).Msg("sending full repository") - packageDir = repo.Directory - } else { - log.Debug().Caller().Str("app", app.Name).Msg("packaging app") - packageDir, err = packageApp(ctx, source, refs, repo, getRepo) + log.Debug().Caller().Str("app", app.Name).Msg("get repo") + repo, err := getRepo(ctx, source.RepoURL, repoTarget) if err != nil { - return nil, errors.Wrap(err, "failed to package application") + return nil, errors.Wrap(err, "failed to get repo") + } + + if a.cfg.ArgoCDSendFullRepository { + log.Debug().Caller().Str("app", app.Name).Msg("sending full repository") + packageDir = repo.Directory + } else { + log.Debug().Caller().Str("app", app.Name).Msg("packaging app") + packageDir, err = packageApp(ctx, source, refs, repo, getRepo) + if err != nil { + return nil, errors.Wrap(err, "failed to package application") + } } } diff --git a/pkg/argo_client/manifests_test.go b/pkg/argo_client/manifests_test.go index d05dfcd2..5a6d241e 100644 --- a/pkg/argo_client/manifests_test.go +++ b/pkg/argo_client/manifests_test.go @@ -20,6 +20,98 @@ import ( "github.com/zapier/kubechecks/pkg/vcs" ) +func TestIsExternalHelmChart(t *testing.T) { + testcases := map[string]struct { + source v1alpha1.ApplicationSource + expected bool + }{ + "oci-chart": { + source: v1alpha1.ApplicationSource{ + RepoURL: "oci://registry.example.io/charts", + Chart: "my-chart", + }, + expected: true, + }, + "https-helm-repo": { + source: v1alpha1.ApplicationSource{ + RepoURL: "https://charts.example.io", + Chart: "my-chart", + }, + expected: true, + }, + "https-git-repo-with-chart-field": { + // .git suffix indicates it is a git repo, not a Helm HTTPS repo + source: v1alpha1.ApplicationSource{ + RepoURL: "https://github.com/org/repo.git", + Chart: "my-chart", + }, + expected: false, + }, + "git-path-source-no-chart": { + // source.Chart is empty — it's a git-path source + source: v1alpha1.ApplicationSource{ + RepoURL: "oci://registry.example.io/charts", + Path: "helm/app", + }, + expected: false, + }, + "plain-git-repo": { + source: v1alpha1.ApplicationSource{ + RepoURL: "git@github.com:org/repo.git", + Path: "app/", + }, + expected: false, + }, + "https-helm-repo-with-path": { + // Has source.Path set — treat as git-path source, not external Helm chart + source: v1alpha1.ApplicationSource{ + RepoURL: "https://charts.example.io", + Chart: "my-chart", + Path: "some/path", + }, + expected: false, + }, + "oci-shorthand-docker-io": { + // OCI registry without scheme (ArgoCD shorthand format) + source: v1alpha1.ApplicationSource{ + RepoURL: "docker.io/envoyproxy", + Chart: "gateway-helm", + }, + expected: true, + }, + "oci-shorthand-ghcr": { + source: v1alpha1.ApplicationSource{ + RepoURL: "ghcr.io/org/charts", + Chart: "my-chart", + }, + expected: true, + }, + "oci-shorthand-no-chart": { + // No Chart field — not an external Helm chart + source: v1alpha1.ApplicationSource{ + RepoURL: "docker.io/envoyproxy", + Path: "some/path", + }, + expected: false, + }, + "oci-shorthand-git-suffix": { + // .git suffix means it's a git repo even without scheme + source: v1alpha1.ApplicationSource{ + RepoURL: "github.com/org/repo.git", + Chart: "my-chart", + }, + expected: false, + }, + } + + for name, tc := range testcases { + t.Run(name, func(t *testing.T) { + actual := isExternalHelmChart(tc.source) + assert.Equal(t, tc.expected, actual) + }) + } +} + func TestAreSameTargetRef(t *testing.T) { testcases := map[string]struct { ref1, ref2 string diff --git a/pkg/git/repo.go b/pkg/git/repo.go index 652ee988..ef7ca219 100644 --- a/pkg/git/repo.go +++ b/pkg/git/repo.go @@ -103,11 +103,43 @@ func (r *Repo) Clone(ctx context.Context) error { cloneOpts.ReferenceName = plumbing.NewBranchReferenceName(r.BranchName) } - // Clone the repository + // Clone the repository, falling back to tag or commit-SHA references when + // the initial branch-based clone fails with "reference not found". repo, err := gogit.PlainCloneContext(ctx, r.Directory, false, cloneOpts) if err != nil { - log.Error().Err(err).Msg("unable to clone repository with go-git") - return errors.Wrap(err, "failed to clone repository") + if r.BranchName != "HEAD" && r.BranchName != "" && isRefNotFound(err) { + log.Info().Str("branch", r.BranchName).Msg("branch reference not found, retrying as tag") + + // Retry as a tag reference + cloneOpts.ReferenceName = plumbing.NewTagReferenceName(r.BranchName) + repo, err = gogit.PlainCloneContext(ctx, r.Directory, false, cloneOpts) + + if err != nil && isRefNotFound(err) && isHexString(r.BranchName) { + // Looks like a commit SHA — clone the default branch then checkout the SHA + log.Info().Str("sha", r.BranchName).Msg("tag reference not found, retrying as commit SHA") + cloneOpts.ReferenceName = "" + repo, err = gogit.PlainCloneContext(ctx, r.Directory, false, cloneOpts) + if err == nil { + worktree, wtErr := repo.Worktree() + if wtErr != nil { + err = errors.Wrap(wtErr, "failed to get worktree for SHA checkout") + } else { + checkoutErr := worktree.Checkout(&gogit.CheckoutOptions{ + Hash: plumbing.NewHash(r.BranchName), + Force: true, + }) + if checkoutErr != nil { + err = errors.Wrapf(checkoutErr, "failed to checkout commit SHA %s", r.BranchName) + } + } + } + } + } + + if err != nil { + log.Error().Err(err).Msg("unable to clone repository with go-git") + return errors.Wrap(err, "failed to clone repository") + } } // Set up refs/remotes/origin/HEAD symbolic reference @@ -565,6 +597,34 @@ func (r *Repo) CleanupTempBranch(ctx context.Context, tempBranch, baseBranch str return nil } +// isRefNotFound returns true when the error indicates a git reference was not +// found on the remote (either NoMatchingRefSpecError or plumbing.ErrReferenceNotFound). +func isRefNotFound(err error) bool { + if err == nil { + return false + } + if errors.Is(err, plumbing.ErrReferenceNotFound) { + return true + } + // go-git wraps the error in places; also check string match as last resort + return strings.Contains(err.Error(), "reference not found") || + strings.Contains(err.Error(), "couldn't find remote ref") +} + +// isHexString returns true when s looks like a git commit SHA (hex characters, +// typically 40 chars for full SHA or at least 7 for abbreviated SHA). +func isHexString(s string) bool { + if len(s) < 7 { + return false + } + for _, c := range s { + if !((c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F')) { + return false + } + } + return true +} + // sanitizeBranchName removes characters that are invalid in git branch names func sanitizeBranchName(name string) string { // Replace invalid characters with hyphens diff --git a/pkg/git/repo_test.go b/pkg/git/repo_test.go index b4f0b653..99c6fd2d 100644 --- a/pkg/git/repo_test.go +++ b/pkg/git/repo_test.go @@ -2,8 +2,10 @@ package git import ( "context" + "errors" "testing" + "github.com/go-git/go-git/v5/plumbing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -70,3 +72,46 @@ func TestBuildCloneURL(t *testing.T) { }) } } + +func TestIsHexString(t *testing.T) { + testcases := map[string]struct { + input string + expected bool + }{ + "full-sha": {"a3f1c2d4e5b6a7f8c9d0e1f2a3b4c5d6e7f8a9b0", true}, + "short-sha": {"a3f1c2d", true}, + "too-short": {"a3f1c", false}, + "branch-name": {"main", false}, + "tag-name": {"v1.2.3", false}, + "mixed-case-hex": {"A3F1C2D4E5B6A7F8", true}, + "non-hex-chars": {"a3f1g2d4", false}, + "empty": {"", false}, + } + + for name, tc := range testcases { + t.Run(name, func(t *testing.T) { + actual := isHexString(tc.input) + assert.Equal(t, tc.expected, actual) + }) + } +} + +func TestIsRefNotFound(t *testing.T) { + testcases := map[string]struct { + err error + expected bool + }{ + "nil": {nil, false}, + "plumbing-err-ref-not-found": {plumbing.ErrReferenceNotFound, true}, + "generic-error": {errors.New("some other error"), false}, + "string-match-reference-not-found": {errors.New("reference not found"), true}, + "string-match-remote-ref": {errors.New("couldn't find remote ref refs/heads/nonexistent"), true}, + } + + for name, tc := range testcases { + t.Run(name, func(t *testing.T) { + actual := isRefNotFound(tc.err) + assert.Equal(t, tc.expected, actual) + }) + } +}