fix(argo-cd): do not use hardcoded commit server URL #3367
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
3bd9bfe to
4b19374
Compare
This commit adds the required template logic to the Argo CD Helm chart to use the proper, release name dependent URL for the commit server. The current implementation uses the default hardcoded URL `argocd-commit-server:8086` from <https://github.com/argoproj/argo-cd/blob/v3.0.9/common/common.go#L31>. Signed-off-by: Alexej Disterhoft <alexej.disterhoft@redcare-pharmacy.com>
Signed-off-by: Alexej Disterhoft <alexej.disterhoft@redcare-pharmacy.com>
Signed-off-by: Alexej Disterhoft <alexej.disterhoft@redcare-pharmacy.com>
284ef73 to
5a969c8
Compare
There was a problem hiding this comment.
Pull Request Overview
Addresses the hardcoded commit server URL by making the service port and port name configurable, propagating the dynamic address into presets and controller environments, and bumping the chart version.
- Introduce
commitServer.service.portandportNamevalues invalues.yaml - Update Service template to use those values, add env var in application-controller, and set the
commit.serverpreset in helpers - Bump chart version to 8.1.2, update documentation and changelog
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| charts/argo-cd/values.yaml | Added commitServer.service.port and portName entries to replace the hardcoded port |
| charts/argo-cd/templates/argocd-commit-server/service.yaml | Templatized Service port name and port values |
| charts/argo-cd/templates/argocd-application-controller/statefulset.yaml | Inject ARGOCD_APPLICATION_CONTROLLER_COMMIT_SERVER via ConfigMap |
| charts/argo-cd/templates/argocd-application-controller/deployment.yaml | Inject ARGOCD_APPLICATION_CONTROLLER_COMMIT_SERVER via ConfigMap |
| charts/argo-cd/templates/_helpers.tpl | Set the commit.server preset dynamically based on service fullname and port |
| charts/argo-cd/README.md | Documented the new commitServer.service.port and portName values |
| charts/argo-cd/Chart.yaml | Bumped chart version to 8.1.2 and updated the changelog entry |
Comments suppressed due to low confidence (1)
charts/argo-cd/templates/argocd-commit-server/service.yaml:23
- Hardcoding the targetPort value to "server" breaks custom portName overrides. Use the templated portName (e.g.,
{{ .Values.commitServer.service.portName }}) instead of a static string.
targetPort: server
yu-croco
reviewed
Jun 24, 2025
Comment on lines
+347
to
+352
| - name: ARGOCD_APPLICATION_CONTROLLER_COMMIT_SERVER | ||
| valueFrom: | ||
| configMapKeyRef: | ||
| name: argocd-cmd-params-cm | ||
| key: commit.server | ||
| optional: true |
There was a problem hiding this comment.
Though Argo CD itself has interface for this, ref, we follow upstream's manifests. Could you please refactor the upstream's manifest at first? Once upstream releases it, we will also follow it.
There was a problem hiding this comment.
Right, thanks for the heads up - I've raised a PR with the required changes upstream: argoproj/argo-cd#23536
There was a problem hiding this comment.
fyi: upstream changes have been merged
14 tasks
mkilchhofer
approved these changes
Jun 27, 2025
There was a problem hiding this comment.
I think since the changes are already accepted upstream and it only added the env vars (no new Golang logic), we can already merge this even before upstream releases a new version.
LGTM and thanks @nobbs for your contribution
yu-croco
approved these changes
Jun 27, 2025
yaringol
pushed a commit
to yaringol/argo-helm
that referenced
this pull request
Jul 12, 2025
* fix(argo-cd): do not use hardcoded commit server URL This commit adds the required template logic to the Argo CD Helm chart to use the proper, release name dependent URL for the commit server. The current implementation uses the default hardcoded URL `argocd-commit-server:8086` from <https://github.com/argoproj/argo-cd/blob/v3.0.9/common/common.go#L31>. Signed-off-by: Alexej Disterhoft <alexej.disterhoft@redcare-pharmacy.com> * chore(argo-cd): update version to 8.1.2 Signed-off-by: Alexej Disterhoft <alexej.disterhoft@redcare-pharmacy.com> * docs(argo-cd): add commit server service port and port name to README Signed-off-by: Alexej Disterhoft <alexej.disterhoft@redcare-pharmacy.com> --------- Signed-off-by: Alexej Disterhoft <alexej.disterhoft@redcare-pharmacy.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Checklist: