argo-cd icon indicating copy to clipboard operation
argo-cd copied to clipboard

feat(hydrator): add inline parameter support to Source Hydrator (#24228)

Open sangyeong01 opened this issue 4 months ago • 6 comments

Summary

Add Helm support to Source Hydrator (alpha feature) to enable using Helm charts with environment-specific value files and parameters.

Currently, Source Hydrator only supports basic Git repository fields (repoURL, targetRevision, path) in the DrySource specification, making it impossible to use with Helm charts that require valueFiles, parameters, or other Helm-specific configurations.

This PR adds a Helm field to the DrySource type and updates the hydrator logic to properly handle Helm configurations, enabling teams to use Source Hydrator with Helm-based applications.

Closes #24228

Changes

  • API: Added Helm *ApplicationSourceHelm field to DrySource type in pkg/apis/application/v1alpha1/types.go
  • Logic: Updated controller/hydrator/hydrator.go to copy Helm configuration from DrySource to the hydrated ApplicationSource

Example Usage

Before (not supported):

apiVersion: argoproj.io/v1alpha1
kind: Application
spec:
  sourceHydrator:
    drySource:
      repoURL: https://github.com/example/charts.git
      targetRevision: main
      path: my-chart
      # ❌ helm field not supported

After (now supported):

apiVersion: argoproj.io/v1alpha1
kind: Application
spec:
  sourceHydrator:
    drySource:
      repoURL: https://github.com/example/charts.git
      targetRevision: main
      path: my-chart
      helm:  # ✅ Now works
        valueFiles:
          - values-prod.yaml
        parameters:
          - name: image.tag
            value: v1.2.3
        releaseName: my-release

Use Case

Many teams use Helm with environment-specific value files (e.g., values-dev.yaml, values-prod.yaml). Without this support, Source Hydrator cannot be used for Helm applications, limiting its adoption for teams that want to use both Helm and the hydration workflow.

Testing

  • [x] Manual testing with Helm chart and multiple value files
  • [x] Verified existing functionality remains unaffected
  • [x] Confirmed omitempty tag ensures backward compatibility

Future Work

  • Add unit tests (will be included in follow-up PR)
  • Update CRD schema (will be included in follow-up PR)
  • Add documentation (will be included in follow-up PR)

Checklist

  • [x] Either (a) I've created an [enhancement proposal](https://github.com/argoproj/argo-cd/issues/new/choose) and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
  • [x] The title of the PR states what changed and the related issues number (used for the release note).
  • [x] The title of the PR conforms to the [Title of the PR](https://argo-cd.readthedocs.io/en/latest/developer-guide/submit-your-pr/#title-of-the-pr)
  • [x] I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
  • [x] I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
  • [x] Does this PR require documentation updates?
  • [x] I've updated documentation as required by this PR.
  • [x] I have signed off all my commits as required by [DCO](https://github.com/argoproj/argoproj/blob/master/community/CONTRIBUTING.md#legal)
  • [x] I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • [x] My build is green ([troubleshooting builds](https://argo-cd.readthedocs.io/en/latest/developer-guide/ci/)).
  • [x] My new feature complies with the [feature status](https://github.com/argoproj/argoproj/blob/master/community/feature-status.md) guidelines.
  • [x] I have added a brief description of why this PR is necessary and/or what this PR solves.
  • [ ] Optional. My organization is added to USERS.md.
  • [ ] Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity).

Notes

This PR focuses on the core API and logic changes. Follow-up PRs will include:

  1. Unit and integration tests
  2. CRD schema updates
  3. Documentation updates
  4. CLI/UI updates if needed

The changes are minimal and backward-compatible, using the omitempty JSON tag to ensure existing DrySource configurations continue to work without modification.

sangyeong01 avatar Aug 26 '25 16:08 sangyeong01

:x: Preview Environment deleted from Bunnyshell

Available commands (reply to this comment):

  • :rocket: /bns:deploy to deploy the environment

bunnyshell[bot] avatar Aug 26 '25 16:08 bunnyshell[bot]

Codecov Report

:x: Patch coverage is 84.61538% with 2 lines in your changes missing coverage. Please review. :warning: Please upload report for BASE (master@e81872f). Learn more about missing BASE report. :warning: Report is 20 commits behind head on master.

Files with missing lines Patch % Lines
pkg/apis/application/v1alpha1/types.go 77.77% 1 Missing and 1 partial :warning:
Additional details and impacted files
@@            Coverage Diff            @@
##             master   #24277   +/-   ##
=========================================
  Coverage          ?   62.46%           
=========================================
  Files             ?      351           
  Lines             ?    49502           
  Branches          ?        0           
=========================================
  Hits              ?    30922           
  Misses            ?    15619           
  Partials          ?     2961           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Aug 26 '25 17:08 codecov[bot]

Thanks for the PR! I've marked it as draft while it's being iterated over.

I think we should add kustomize and directory fields at the same time for feature parity with the source field.

I don't think we can save CRD updates, tests, and docs for a follow-up PR. Those need to be part of the initial PR.

crenshaw-dev avatar Aug 26 '25 19:08 crenshaw-dev

@crenshaw-dev, I’ve addressed all requested changes, including adding docs and e2e tests. Could you please take another look when you have a chance? Thanks!

sangyeong01 avatar Sep 20 '25 06:09 sangyeong01

Hi @crenshaw-dev,

Thanks for the follow-up. I've updated the PR to address the additional changes you requested. Please let me know what you think when you have a moment. Thanks!

sangyeong01 avatar Sep 25 '25 05:09 sangyeong01

LGTM

wtam2018 avatar Nov 17 '25 03:11 wtam2018

Looking good! Added a couple questions about tests.

Mind adding a note to docs/operator-manual/application.yaml that these fields are available? No need to copy the full spec from source, just add a note that the helm, kustomize, directory, and plugin fields are available.

Added a note in commit 607f7a085. The docs now mention that drySource.helm, drySource.kustomize, drySource.directory, and drySource.plugin fields are available and reference the source field sections above for the full spec.

sangyeong01 avatar Dec 04 '25 17:12 sangyeong01

@sangyeong01, @emirot is gonna give this a local test, but code LGTM. Looking forward to getting it into 3.3!

crenshaw-dev avatar Dec 04 '25 17:12 crenshaw-dev

I have added an ArgoCD helm application with your changes and I could see the helm chart got hydrated https://github.com/emirot/argocd-test-hydrator/tree/environments/dev/development

ArgoCD app:

project: default
destination:
  server: https://kubernetes.default.svc
  namespace: argocd
sourceHydrator:
  drySource:
    repoURL: https://github.com/emirot/argocd-test-hydrator
    targetRevision: HEAD
    path: hello-world
    helm:
      valueFiles:
        - values.yaml
      releaseName: my-release
  syncSource:
    targetBranch: environments/dev
    path: development

However in my case it looks like the app is in a infinite Hydrating state
Screenshot 2025-12-04 at 1 32 33 PM @sangyeong01 what did I miss? Did you see that behavior?

emirot avatar Dec 04 '25 21:12 emirot

@emirot good catch! I proposed a fix here: https://github.com/sangyeong01/argo-cd/pull/1

crenshaw-dev avatar Dec 05 '25 01:12 crenshaw-dev

@emirot good catch! I proposed a fix here: sangyeong01#1

Thanks for the fix! I tested this by deploying a local build . Before the fix, it was stuck in Hydrating state with repeated "reason":"spec.sourceHydrator differs" logs. After the fix, it reaches Hydrated and stays stable. Good to merge.

sangyeong01 avatar Dec 05 '25 04:12 sangyeong01

Great! @sangyeong01 you can either merge my PR into your branch or copy/paste my code into this PR. :-)

crenshaw-dev avatar Dec 05 '25 13:12 crenshaw-dev

Thanks @crenshaw-dev ! I've merged your PR into my branch. The fix is now included in this PR.

sangyeong01 avatar Dec 05 '25 13:12 sangyeong01

Thanks @sangyeong01

juwon8891 avatar Dec 17 '25 06:12 juwon8891