From d95e6cac516792d94d9e6bf17511f3167e828a66 Mon Sep 17 00:00:00 2001 From: Pinchy Date: Sun, 22 Mar 2026 09:02:27 -0500 Subject: [PATCH 1/6] feat: add OCI/HTTPS Helm chart support and tag/SHA clone fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #337 (zapier/kubechecks) ## Problem kubechecks tried to git-clone every source URL, including OCI registries (oci://...) and HTTPS Helm repos (https://charts.example.io). These are not git repos and the clone fails. Additionally, git sources that use tag-based or commit-SHA targetRevision also failed because go-git was only trying branch refs. ## Changes ### pkg/argo_client/manifests.go - Add helper that returns true when: - (it's a Helm chart source, not a git path), AND - RepoURL starts with (OCI registry), OR - RepoURL is an HTTPS URL without a .git suffix and source.Path is empty (HTTPS Helm repo) - Modify to skip for external Helm chart sources: - When : create an empty temp dir and use it as . ArgoCD's repo-server fetches the chart from the registry itself using its own credentials and OCI support. - When : log a warning and return nil (graceful degradation — skip manifest generation for that source). ### pkg/git/repo.go - Add helper to detect go-git "reference not found" errors (handles both plumbing.ErrReferenceNotFound and string match). - Add helper to detect commit SHAs (hex strings ≥7 chars). - Modify to fall back when branch clone fails with ref-not-found: 1. Retry as a tag reference () 2. If still not found AND name looks like a hex SHA: clone default branch then checkout the specific commit hash. ### Tests added - : OCI, HTTPS Helm, git-with-.git, no Chart field, HTTPS with path set - : full SHA, short SHA, too-short, branch name, tag name, mixed-case hex, non-hex chars - : nil, plumbing sentinel, generic error, string matches --- pkg/argo_client/manifests.go | 90 +++++++++++++++++++++++++------ pkg/argo_client/manifests_test.go | 61 +++++++++++++++++++++ pkg/git/repo.go | 66 +++++++++++++++++++++-- pkg/git/repo_test.go | 45 ++++++++++++++++ 4 files changed, 242 insertions(+), 20 deletions(-) diff --git a/pkg/argo_client/manifests.go b/pkg/argo_client/manifests.go index 0149a36f..263e8e2a 100644 --- a/pkg/argo_client/manifests.go +++ b/pkg/argo_client/manifests.go @@ -100,6 +100,27 @@ 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 are unambiguous + 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 + } + return false +} + // 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 +130,28 @@ 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. + // When ArgoCDSendFullRepository is true we send an empty directory — ArgoCD's + // repo-server will fetch the chart from the registry on its own. + // When ArgoCDSendFullRepository is false we have no sensible local directory + // to package, so we skip manifest generation for this source. + if isExternalHelmChart(source) { + if !a.cfg.ArgoCDSendFullRepository { + log.Warn(). + Str("app", app.Name). + Str("repoURL", source.RepoURL). + Str("chart", source.Chart). + Msg("skipping manifest generation for external Helm chart source: ArgoCDSendFullRepository is false") + return nil, nil + } + log.Info(). + Str("app", app.Name). + Str("repoURL", source.RepoURL). + Str("chart", source.Chart). + Msg("external Helm chart source detected; using empty temp dir — ArgoCD repo-server will fetch the chart") + } + clusterCloser, clusterClient := a.GetClusterClient() defer pkg.WithErrorLogging(clusterCloser.Close, "failed to close connection") @@ -131,26 +174,39 @@ 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 - } - - 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") - } - var packageDir string - if a.cfg.ArgoCDSendFullRepository { - log.Debug().Caller().Str("app", app.Name).Msg("sending full repository") - packageDir = repo.Directory + + if isExternalHelmChart(source) { + // For external Helm charts, create an empty temp dir. ArgoCD's repo-server + // will fetch the chart itself; we just need a valid (possibly empty) directory + // to satisfy the GenerateManifestWithFiles streaming protocol. + emptyDir, err := os.MkdirTemp("", "kubechecks-helm-external-*") + if err != nil { + return nil, errors.Wrap(err, "failed to create temp dir for external Helm chart") + } + defer pkg.WipeDir(emptyDir) + packageDir = emptyDir } else { - log.Debug().Caller().Str("app", app.Name).Msg("packaging app") - packageDir, err = packageApp(ctx, source, refs, repo, getRepo) + repoTarget := source.TargetRevision + if pkg.AreSameRepos(source.RepoURL, pullRequest.CloneURL) && areSameTargetRef(source.TargetRevision, pullRequest.BaseRef) { + repoTarget = pullRequest.HeadRef + } + + 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..efd22a08 100644 --- a/pkg/argo_client/manifests_test.go +++ b/pkg/argo_client/manifests_test.go @@ -20,6 +20,67 @@ 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, + }, + } + + 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) + }) + } +} From f7c0c18929d04023fceb5256e02a11abd4c49f06 Mon Sep 17 00:00:00 2001 From: Pinchy Date: Sun, 22 Mar 2026 15:05:36 -0500 Subject: [PATCH 2/6] ci: add Docker build and push workflow for GHCR - Triggers on push to main/feat branches and on PRs - Builds Docker image using existing Dockerfile (target: production) - Pushes to ghcr.io/christmas-island/kubechecks only on main push - PRs: build only (no push) to verify compilation - Tags: 'latest' on main, short SHA on all builds - Multi-arch build: linux/amd64 + linux/arm64 - Separate job for Go tests (make test) - Uses GHA cache for faster builds --- .github/workflows/build.yaml | 94 ++++++++++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 .github/workflows/build.yaml diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml new file mode 100644 index 00000000..ca9dfa84 --- /dev/null +++ b/.github/workflows/build.yaml @@ -0,0 +1,94 @@ +name: build + +on: + push: + branches: + - main + - feat/** + pull_request: + types: + - opened + - reopened + - synchronize + +permissions: + contents: read + packages: write + +jobs: + test: + name: Go Tests + runs-on: ubuntu-24.04 + + steps: + - name: Checkout code + uses: actions/checkout@v5 + + - name: Parse tool versions + uses: wistia/parse-tool-versions@v1.0 + + - name: Set up Go + uses: actions/setup-go@v6 + with: + go-version: "1.25.x" + cache: false + + - name: Cache Go modules + build cache + uses: actions/cache@v4 + with: + path: | + ~/.cache/go-build + ~/go/pkg/mod + key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} + restore-keys: | + ${{ runner.os }}-go- + + - name: Run tests + run: make test + + build-and-push: + name: Build Docker Image + runs-on: ubuntu-24.04 + + steps: + - name: Checkout code + uses: actions/checkout@v5 + + - name: Set up QEMU + uses: docker/setup-qemu-action@v3 + + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@v3 + + - name: Log in to GHCR + if: github.event_name == 'push' && github.ref == 'refs/heads/main' + uses: docker/login-action@v3 + with: + registry: ghcr.io + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + + - name: Extract Docker metadata + id: meta + uses: docker/metadata-action@v5 + with: + images: ghcr.io/christmas-island/kubechecks + tags: | + type=raw,value=latest,enable=${{ github.ref == 'refs/heads/main' }} + type=sha,format=short + + - name: Build and push Docker image + uses: docker/build-push-action@v6 + with: + context: . + file: Dockerfile + target: production + platforms: linux/amd64,linux/arm64 + push: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }} + tags: ${{ steps.meta.outputs.tags }} + labels: ${{ steps.meta.outputs.labels }} + build-args: | + GIT_COMMIT=${{ github.sha }} + GIT_TAG=${{ github.ref_name }} + cache-from: type=gha + cache-to: type=gha,mode=max From d27d19a7d64ccabeab63fbe7f66046bd9ca519b0 Mon Sep 17 00:00:00 2001 From: Pinchy Date: Sun, 22 Mar 2026 16:14:22 -0500 Subject: [PATCH 3/6] chore: remove build workflow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Not needed for this fork — upstream handles CI. --- .github/workflows/build.yaml | 94 ------------------------------------ 1 file changed, 94 deletions(-) delete mode 100644 .github/workflows/build.yaml diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml deleted file mode 100644 index ca9dfa84..00000000 --- a/.github/workflows/build.yaml +++ /dev/null @@ -1,94 +0,0 @@ -name: build - -on: - push: - branches: - - main - - feat/** - pull_request: - types: - - opened - - reopened - - synchronize - -permissions: - contents: read - packages: write - -jobs: - test: - name: Go Tests - runs-on: ubuntu-24.04 - - steps: - - name: Checkout code - uses: actions/checkout@v5 - - - name: Parse tool versions - uses: wistia/parse-tool-versions@v1.0 - - - name: Set up Go - uses: actions/setup-go@v6 - with: - go-version: "1.25.x" - cache: false - - - name: Cache Go modules + build cache - uses: actions/cache@v4 - with: - path: | - ~/.cache/go-build - ~/go/pkg/mod - key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} - restore-keys: | - ${{ runner.os }}-go- - - - name: Run tests - run: make test - - build-and-push: - name: Build Docker Image - runs-on: ubuntu-24.04 - - steps: - - name: Checkout code - uses: actions/checkout@v5 - - - name: Set up QEMU - uses: docker/setup-qemu-action@v3 - - - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v3 - - - name: Log in to GHCR - if: github.event_name == 'push' && github.ref == 'refs/heads/main' - uses: docker/login-action@v3 - with: - registry: ghcr.io - username: ${{ github.actor }} - password: ${{ secrets.GITHUB_TOKEN }} - - - name: Extract Docker metadata - id: meta - uses: docker/metadata-action@v5 - with: - images: ghcr.io/christmas-island/kubechecks - tags: | - type=raw,value=latest,enable=${{ github.ref == 'refs/heads/main' }} - type=sha,format=short - - - name: Build and push Docker image - uses: docker/build-push-action@v6 - with: - context: . - file: Dockerfile - target: production - platforms: linux/amd64,linux/arm64 - push: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }} - tags: ${{ steps.meta.outputs.tags }} - labels: ${{ steps.meta.outputs.labels }} - build-args: | - GIT_COMMIT=${{ github.sha }} - GIT_TAG=${{ github.ref_name }} - cache-from: type=gha - cache-to: type=gha,mode=max From cd25e1a1bcbcf1f27448feac997a8f3568be671b Mon Sep 17 00:00:00 2001 From: Pinchy Date: Sun, 22 Mar 2026 16:49:27 -0500 Subject: [PATCH 4/6] fix: write stub Chart.yaml for external Helm chart sources ArgoCD's repo-server expects a Chart.yaml in the temp dir even for external Helm charts. Without it, GenerateManifestWithFiles fails with 'error reading helm chart from ... Chart.yaml: no such file'. Write a minimal Chart.yaml stub (apiVersion, name, version, type) so the streaming protocol succeeds and repo-server can fetch the actual chart from the registry. --- pkg/argo_client/manifests.go | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/pkg/argo_client/manifests.go b/pkg/argo_client/manifests.go index 263e8e2a..487d9e92 100644 --- a/pkg/argo_client/manifests.go +++ b/pkg/argo_client/manifests.go @@ -177,14 +177,30 @@ func (a *ArgoClient) generateManifests(ctx context.Context, app v1alpha1.Applica var packageDir string if isExternalHelmChart(source) { - // For external Helm charts, create an empty temp dir. ArgoCD's repo-server - // will fetch the chart itself; we just need a valid (possibly empty) directory - // to satisfy the GenerateManifestWithFiles streaming protocol. + // For external Helm charts, create a temp dir with a stub Chart.yaml. + // ArgoCD's repo-server will fetch the actual chart itself; we just need + // a valid directory with a Chart.yaml to satisfy the + // GenerateManifestWithFiles streaming protocol and avoid + // "error reading helm chart from ... Chart.yaml: no such file" errors. emptyDir, err := os.MkdirTemp("", "kubechecks-helm-external-*") if err != nil { return nil, errors.Wrap(err, "failed to create temp dir for external Helm chart") } defer pkg.WipeDir(emptyDir) + + chartVersion := source.TargetRevision + if chartVersion == "" { + chartVersion = "0.0.0" + } + chartName := source.Chart + if chartName == "" { + chartName = "external-chart" + } + stubChart := fmt.Sprintf("apiVersion: v2\nname: %s\nversion: %s\ntype: application\n", chartName, chartVersion) + if err := os.WriteFile(filepath.Join(emptyDir, "Chart.yaml"), []byte(stubChart), 0644); err != nil { + return nil, errors.Wrap(err, "failed to write stub Chart.yaml for external Helm chart") + } + packageDir = emptyDir } else { repoTarget := source.TargetRevision From 60e3a992c1063566545155836805feb9da6426ab Mon Sep 17 00:00:00 2001 From: Pinchy Date: Sun, 22 Mar 2026 17:59:56 -0500 Subject: [PATCH 5/6] fix: use GenerateManifest for external Helm chart sources MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of streaming a stub Chart.yaml via GenerateManifestWithFiles, delegate directly to repoClient.GenerateManifest() for external Helm chart sources (OCI registries and HTTPS Helm repos). This is the correct approach because: - GenerateManifestWithFiles requires local chart files. Streaming a stub causes 'helm template' to run against the stub dir, not the actual chart — producing incorrect or missing manifests. - GenerateManifest tells repo-server to fetch the chart from the registry using its own credentials and OCI/Helm support, exactly as ArgoCD does during a normal sync. Known limitations (acknowledged): - Chart dependencies referencing private repos require those repos to be configured in ArgoCD with appropriate credentials. - Registry auth failures will surface as manifest generation errors (same behavior as ArgoCD itself). - TargetRevision must be a valid chart version (not a branch name) for Helm chart sources — consistent with ArgoCD's requirements. --- pkg/argo_client/manifests.go | 163 ++++++++++++++++++++++++++--------- 1 file changed, 123 insertions(+), 40 deletions(-) diff --git a/pkg/argo_client/manifests.go b/pkg/argo_client/manifests.go index 487d9e92..766417d2 100644 --- a/pkg/argo_client/manifests.go +++ b/pkg/argo_client/manifests.go @@ -121,6 +121,123 @@ func isExternalHelmChart(source v1alpha1.ApplicationSource) bool { 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. @@ -132,24 +249,16 @@ func (a *ArgoClient) generateManifests(ctx context.Context, app v1alpha1.Applica log.Info().Str("app", app.Name).Msg("generating manifests") // External Helm chart sources (OCI or HTTPS Helm repos) cannot be git-cloned. - // When ArgoCDSendFullRepository is true we send an empty directory — ArgoCD's - // repo-server will fetch the chart from the registry on its own. - // When ArgoCDSendFullRepository is false we have no sensible local directory - // to package, so we skip manifest generation for this source. + // 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) { - if !a.cfg.ArgoCDSendFullRepository { - log.Warn(). - Str("app", app.Name). - Str("repoURL", source.RepoURL). - Str("chart", source.Chart). - Msg("skipping manifest generation for external Helm chart source: ArgoCDSendFullRepository is false") - return nil, nil - } log.Info(). Str("app", app.Name). Str("repoURL", source.RepoURL). Str("chart", source.Chart). - Msg("external Helm chart source detected; using empty temp dir — ArgoCD repo-server will fetch the chart") + Msg("external Helm chart source detected; delegating to repo-server GenerateManifest") + return a.generateManifestForExternalChart(ctx, app, source, refs) } clusterCloser, clusterClient := a.GetClusterClient() @@ -176,33 +285,7 @@ func (a *ArgoClient) generateManifests(ctx context.Context, app v1alpha1.Applica var packageDir string - if isExternalHelmChart(source) { - // For external Helm charts, create a temp dir with a stub Chart.yaml. - // ArgoCD's repo-server will fetch the actual chart itself; we just need - // a valid directory with a Chart.yaml to satisfy the - // GenerateManifestWithFiles streaming protocol and avoid - // "error reading helm chart from ... Chart.yaml: no such file" errors. - emptyDir, err := os.MkdirTemp("", "kubechecks-helm-external-*") - if err != nil { - return nil, errors.Wrap(err, "failed to create temp dir for external Helm chart") - } - defer pkg.WipeDir(emptyDir) - - chartVersion := source.TargetRevision - if chartVersion == "" { - chartVersion = "0.0.0" - } - chartName := source.Chart - if chartName == "" { - chartName = "external-chart" - } - stubChart := fmt.Sprintf("apiVersion: v2\nname: %s\nversion: %s\ntype: application\n", chartName, chartVersion) - if err := os.WriteFile(filepath.Join(emptyDir, "Chart.yaml"), []byte(stubChart), 0644); err != nil { - return nil, errors.Wrap(err, "failed to write stub Chart.yaml for external Helm chart") - } - - packageDir = emptyDir - } else { + { repoTarget := source.TargetRevision if pkg.AreSameRepos(source.RepoURL, pullRequest.CloneURL) && areSameTargetRef(source.TargetRevision, pullRequest.BaseRef) { repoTarget = pullRequest.HeadRef From 4be635f28bc66f967a1fce26718ead89324d59e2 Mon Sep 17 00:00:00 2001 From: Pinchy Date: Tue, 24 Mar 2026 16:10:45 -0500 Subject: [PATCH 6/6] fix: detect schemeless OCI registry URLs as external Helm charts URLs like 'docker.io/envoyproxy' (without oci:// prefix) are valid ArgoCD OCI Helm source URLs. Without this fix, kubechecks tries to git-clone them, which fails. Adds detection for URLs with no scheme where source.Chart is set and the URL doesn't end in .git. --- pkg/argo_client/manifests.go | 9 ++++++++- pkg/argo_client/manifests_test.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/pkg/argo_client/manifests.go b/pkg/argo_client/manifests.go index 766417d2..b9153c61 100644 --- a/pkg/argo_client/manifests.go +++ b/pkg/argo_client/manifests.go @@ -109,7 +109,7 @@ func isExternalHelmChart(source v1alpha1.ApplicationSource) bool { if source.Chart == "" { return false } - // OCI registries are unambiguous + // OCI registries with explicit oci:// scheme if strings.HasPrefix(source.RepoURL, "oci://") { return true } @@ -118,6 +118,13 @@ func isExternalHelmChart(source v1alpha1.ApplicationSource) bool { 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 } diff --git a/pkg/argo_client/manifests_test.go b/pkg/argo_client/manifests_test.go index efd22a08..5a6d241e 100644 --- a/pkg/argo_client/manifests_test.go +++ b/pkg/argo_client/manifests_test.go @@ -71,6 +71,37 @@ func TestIsExternalHelmChart(t *testing.T) { }, 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 {