Robot Upgrade: argo-cd chart upgrade from 8.1.0 to 9.4.5#3951
Robot Upgrade: argo-cd chart upgrade from 8.1.0 to 9.4.5#3951weizhoublue wants to merge 1 commit intomainfrom
Conversation
Summary of ChangesHello @weizhoublue, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a substantial upgrade to the Argo CD Helm chart, advancing it from version 8.1.0 to 9.4.5. The update encompasses not only the core Argo CD application but also critical underlying components like Redis-HA and Dex, ensuring compatibility and leveraging the latest features. A significant addition is the experimental support for Kubernetes Gateway API, providing more flexible and powerful ingress routing capabilities. Furthermore, the changes introduce a suite of new configuration options for fine-tuning deployments and improve the clarity of the project's documentation. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the Argo CD Helm chart to version 9.4.5, aligning with Argo CD appVersion v3.3.2, and includes a significant update to the redis-ha subchart to version 4.34.11. Key changes involve adding extensive support for Kubernetes Gateway API resources (HTTPRoute, GRPCRoute, and BackendTLSPolicy) with corresponding documentation and configuration options, and introducing new global and component-specific labels and network policy controls across various Argo CD components (controller, repoServer, server, dex, redis, applicationSet, notifications, commitServer). The README.md is updated with detailed examples for AKS Application Routing and Gateway API configurations, along with important breaking change notes for redis-ha upgrades and the removal of many parameters from .Values.configs.params to streamline configuration. Review comments highlighted and corrected a typo in an annotation key (annnotaions: to annotations:) and addressed a potential security vulnerability by recommending type casting for splitBrainDetection.retryInterval to prevent arbitrary command execution. Additionally, several network policy ingress rules were made conditional on their respective metrics enablement flags to prevent creating rules for non-existent ports.
| {{- toYaml . | nindent 4 }} | ||
| {{- end }} | ||
| {{- with .Values.controller.vpa.annotations }} | ||
| annnotaions: |
There was a problem hiding this comment.
There is a typo in the annotations key. annnotaions: should be annotations:.
annotations:| if [ "$ROLE" != "master" ]; then | ||
| reinit | ||
| echo "waiting for redis to become master" | ||
| sleep {{ .Values.splitBrainDetection.retryInterval }} |
There was a problem hiding this comment.
The sleep command uses the .Values.splitBrainDetection.retryInterval value directly without validation or proper shell escaping. If this value is overridden with a malicious string (e.g., 10; id), it could lead to arbitrary command execution within the container. It is recommended to cast the value to an integer to ensure it only contains numeric characters.
sleep {{ .Values.splitBrainDetection.retryInterval | int }}
| if [ "$REDIS_MASTER" != "$MASTER" ]; then | ||
| reinit | ||
| echo "Redis master and local master are not the same. waiting." | ||
| sleep {{ .Values.splitBrainDetection.retryInterval }} |
There was a problem hiding this comment.
The sleep command uses the .Values.splitBrainDetection.retryInterval value directly without validation or proper shell escaping. If this value is overridden with a malicious string (e.g., 10; id), it could lead to arbitrary command execution within the container. It is recommended to cast the value to an integer to ensure it only contains numeric characters.
sleep {{ .Values.splitBrainDetection.retryInterval | int }}
| - from: | ||
| - namespaceSelector: {} | ||
| ports: | ||
| - port: metrics |
There was a problem hiding this comment.
The ingress rule for metrics should be conditional on applicationSet.metrics.enabled. Otherwise, if metrics are disabled but another feature like ingress is enabled, this rule will be created for a port that doesn't exist, which could cause issues.
{{- if .Values.applicationSet.metrics.enabled }}
- from:
- namespaceSelector: {}
ports:
- port: metrics
{{- end }}| - from: | ||
| - namespaceSelector: {} | ||
| ports: | ||
| - port: metrics |
There was a problem hiding this comment.
The ingress rule for metrics should be conditional on repoServer.metrics.enabled. Otherwise, this rule will be created for a port that might not exist if metrics are disabled.
{{- if .Values.repoServer.metrics.enabled }}
- from:
- namespaceSelector: {}
ports:
- port: metrics
{{- end }}| - from: | ||
| - namespaceSelector: {} | ||
| ports: | ||
| - port: metrics | ||
| protocol: TCP |
There was a problem hiding this comment.
The ingress rule for metrics should be conditional on dex.metrics.enabled. Otherwise, this rule will be created for a port that might not exist if metrics are disabled.
{{- if .Values.dex.metrics.enabled }}
- from:
- namespaceSelector: {}
ports:
- port: metrics
protocol: TCP
{{- end }}| - from: | ||
| - namespaceSelector: {} | ||
| ports: | ||
| - port: metrics | ||
| protocol: TCP |
There was a problem hiding this comment.
The ingress rule for metrics should be conditional on redis.metrics.enabled. Otherwise, this rule will be created for a port that might not exist if metrics are disabled.
{{- if .Values.redis.metrics.enabled }}
- from:
- namespaceSelector: {}
ports:
- port: metrics
protocol: TCP
{{- end }}
github-actions
bot
force-pushed
the
upgrade/argo-cd/9.4.5
branch
from
4c8ab5f to
cccf863
Compare
Signed-off-by: robot <robot@example.com>
github-actions
bot
force-pushed
the
upgrade/argo-cd/9.4.5
branch
from
cccf863 to
f686d8e
Compare
2 participants
I am robot, upgrade: project argo-cd chart upgrade from 8.1.0 to 9.4.5