Skip to content

Conversation

@robinlieb
Copy link
Contributor

@robinlieb robinlieb commented Nov 10, 2025

Description

This PR adds the capability to mutate OCI references in Argo CD resources and replaces the given OCI reference to the Zarf registry URL.

This change adds the possibility to use Manifest and Helm Charts in the repoURL in the Application, like explained in the OCI Docs in Argo CD with the first two examples. To archive that it also changes the patching of the URL in repository secrets to use the registry instead of the git server if oci is used.

Additionally, added the patching of the repo url for Argo CD repo-creds secret type.

Related Issue

Relates to #3046

Checklist before merging

@robinlieb robinlieb requested review from a team as code owners November 10, 2025 19:29
@netlify
Copy link

netlify bot commented Nov 10, 2025

Deploy Preview for zarf-docs canceled.

Name Link
🔨 Latest commit c9b5774
🔍 Latest deploy log https://app.netlify.com/projects/zarf-docs/deploys/69442847e26de20008e06330

Copy link
Member

@brandtkeller brandtkeller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made a first pass - spent a healthy amount of time educating myself on argo further.

Left a nit that I think we can cleanup in a few places. Separately want to consider some additional scope for this change:

  1. E2E testing - unit tests get us some good feedback loops on the internal dependency and behaviors but this is largely an external integration. I would love to think about how we test this is operational.
  2. Documentation - right now this can be appending to the existing tutorial. There may be more opportunity for a more granular pass at argocd orchestration docs.

Two minor requests:

  1. Can you comment on an issue if/when you pick it up for work (or at least with the PR) - In doing so we can assign you and help prevent any colliding submissions (as well as remember to track for reviews on the project board.
  2. Add context to the PR description. It took me a fair bit of reading to tell that the associated issue was asking for a feature around oci registries when you actually solved for oci images. The former isn't actually as valid and you solved the problem correctly here IMO - but I had to hunt down the delta and process.

Thank you for the continued support and contributions - I very much appreciate it!

patches = append(patches, operations.ReplacePatchOperation("/data/username", base64.StdEncoding.EncodeToString([]byte(gitServer.PullUsername))))
patches = append(patches, operations.ReplacePatchOperation("/data/password", base64.StdEncoding.EncodeToString([]byte(gitServer.PullPassword))))

if isOCI {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: for code patterns used across zarf (of those replaced thus far) we have been upgrading to use the happy path is left-aligned concept whereby - when possible we process conditions and return early and avoid else returns.

@github-project-automation github-project-automation bot moved this to In progress in Zarf Nov 11, 2025
@robinlieb robinlieb force-pushed the feat/argocd-applications-oci-source branch 2 times, most recently from ab7df2b to 9007e84 Compare November 13, 2025 19:06
@robinlieb
Copy link
Contributor Author

robinlieb commented Nov 13, 2025

@brandtkeller thanks for the initial review. I added changes which addresses all of the mentioned points.
Added two applications in the Argo CD examples showcasing the the use of Helm Charts and manifests from OCI.
Added E2E test on base of this example.
Also refactored the patch logic for Argo CD resources since this contained some duplication.

For the other two points, will definitely keep that in mind for future work.

Update: For the sake of proper documentation, should I create a new issue specifying the OCI support for manifests?

@al-jeyapal
Copy link

The issue we might have with this is that ArgoCD will try to hit the registry over HTTPS. We were getting this in the Argo logs:

Failed to load target state:
 failed to generate manifest for source 1 of 2:
   rpc error: code = Unknown desc = failed to resolve revision "0.0.0":
     cannot get digest for revision 0.0.0:
       Head "https://zarf-docker-registry.zarf.svc.cluster.local:5000/v2/helm-charts/manifests/0.0.0":
         http: server gave HTTP response to HTTPS client

Helm allows you to use http via:

 helm pull --plain-http oci://...

However it doesn't look like Argo exposes an option for that. It exposes a --insecure-skip-server-verification but that only allows it to trust unverified certs.

We're currently trying to work around this by putting an internal caddy service to act as a HTTPS proxy in front of the zarf registry (on #4327). Still working through the details.

The easier option would be to have a HTTPS endpoint exposed on the zarf registry (just internally as a service not as an ingress/GatewayAPI route), even if it just uses a self-signed certificate. Not sure if that's a route we want to take though.

@brandtkeller brandtkeller added this to the v0.67.1 Release milestone Dec 4, 2025
@robinlieb robinlieb force-pushed the feat/argocd-applications-oci-source branch 2 times, most recently from 1128adc to a5388db Compare December 12, 2025 20:24
@codecov
Copy link

codecov bot commented Dec 16, 2025

Codecov Report

❌ Patch coverage is 71.08434% with 24 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/internal/agent/hooks/argocd-application.go 69.23% 10 Missing and 6 partials ⚠️
src/internal/agent/hooks/argocd-repository.go 83.33% 2 Missing and 2 partials ⚠️
src/internal/agent/hooks/argocd-appproject.go 40.00% 2 Missing and 1 partial ⚠️
src/internal/agent/hooks/flux-ocirepo.go 50.00% 1 Missing ⚠️
Files with missing lines Coverage Δ
src/internal/agent/hooks/common.go 85.00% <ø> (ø)
src/internal/agent/hooks/flux-gitrepo.go 85.93% <ø> (ø)
src/internal/agent/hooks/flux-ocirepo.go 82.64% <50.00%> (ø)
src/internal/agent/hooks/argocd-appproject.go 71.73% <40.00%> (-4.46%) ⬇️
src/internal/agent/hooks/argocd-repository.go 76.56% <83.33%> (-0.37%) ⬇️
src/internal/agent/hooks/argocd-application.go 72.54% <69.23%> (-3.51%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: Robin Lieb <34332703+robinlieb@users.noreply.github.com>
Signed-off-by: Robin Lieb <34332703+robinlieb@users.noreply.github.com>
Signed-off-by: Robin Lieb <34332703+robinlieb@users.noreply.github.com>
@robinlieb robinlieb force-pushed the feat/argocd-applications-oci-source branch from a5388db to d159081 Compare December 16, 2025 16:50
@robinlieb
Copy link
Contributor Author

The failed pipeline Is probably related to the version bump of Argo CD, which throws an error with the gitea.localhost setup in the test. Not sure yet what exactly changed in Argo CD but will investigate further on how to resolve that.

Signed-off-by: Robin Lieb <34332703+robinlieb@users.noreply.github.com>
@robinlieb robinlieb force-pushed the feat/argocd-applications-oci-source branch from 34ce98d to c9b5774 Compare December 18, 2025 16:13
@robinlieb
Copy link
Contributor Author

External test fails because gitea.localhost gets resolved to 127.0.0.1 when calling git / libcurl in Argo CD Repo Server. This behavior changed with Helm 7.4.0 / Argo CD v2.12.0, where Argo CD moved to ubuntu:24.04 base image, which is stricter / RFC 6761 conform in terms of localhost resolution.
Added curloptResolve option for git config in repo sever.
Alternative approach would move from gitea.localhost to e.g. gitea.local in tests, but that would require to add this entry in /etc/hosts on local machines running external tests and in pipeline.

@al-jeyapal
Copy link

Thanks for the work on this @robinlieb. Out of curiosity, how did you work around ArgoCD forcing Helm to use HTTPS when interacting with OCI? We had to put a HTTPS proxy in place to work around that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

3 participants