-
Notifications
You must be signed in to change notification settings - Fork 29
Robot Upgrade: cert-manager chart upgrade from v1.16.4 to v1.19.1 #3676
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
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 orchestrates a comprehensive upgrade of the Highlights
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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request upgrades the cert-manager Helm chart from v1.16.4 to v1.19.1. The changes are extensive, reflecting the updates in the upstream chart, including version bumps, documentation updates, new features like global.hostUsers, and template improvements for better flexibility and consistency. The review identified a few minor issues, primarily related to documentation inconsistencies and trailing whitespace in template files, which should be addressed to improve clarity and maintain code quality. Overall, the upgrade appears to be handled correctly.
| priorityClassName: {{ . | quote }} | ||
| {{- end }} | ||
| {{- if (hasKey .Values.global "hostUsers") }} | ||
| hostUsers: {{ .Values.global.hostUsers }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| priorityClassName: {{ . | quote }} | ||
| {{- end }} | ||
| {{- if (hasKey .Values.global "hostUsers") }} | ||
| hostUsers: {{ .Values.global.hostUsers }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| priorityClassName: {{ . | quote }} | ||
| {{- end }} | ||
| {{- if (hasKey .Values.global "hostUsers") }} | ||
| hostUsers: {{ .Values.global.hostUsers }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| priorityClassName: {{ . | quote }} | ||
| {{- end }} | ||
| {{- if (hasKey .Values.global "hostUsers") }} | ||
| hostUsers: {{ .Values.global.hostUsers }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "default": "http-metrics", | ||
| "description": "The target port to set on the ServiceMonitor. This must match the port that the cert-manager controller is listening on for metrics." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The JSON schema for prometheus.servicemonitor.targetPort is missing a type definition. While the default is now a string (http-metrics), it should also support integer values for flexibility. The README correctly documents this as string,integer. Please add the type definition to ensure the schema is accurate and useful for validation tools.
"default": "http-metrics",
"description": "The target port to set on the ServiceMonitor. This must match the port that the cert-manager controller is listening on for metrics.",
"type": ["string", "integer"]| # Set all pods to run in a user namespace without host access. | ||
| # Experimental: may be removed once the Kubernetes User Namespaces feature is GA. | ||
| # | ||
| # Requirements: | ||
| # - Kubernetes ≥ 1.33, or | ||
| # - Kubernetes 1.27–1.32 with UserNamespacesSupport feature gate enabled. | ||
| # | ||
| # Set to false to run pods in a user namespace without host access. | ||
| # | ||
| # See [limitations](https://kubernetes.io/docs/concepts/workloads/pods/user-namespaces/#limitations) for details. | ||
| # +docs:property | ||
| # hostUsers: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation for global.hostUsers appears to be contradictory. Line 39 states it's to "run in a user namespace without host access", but line 46 says to "Set to false" to achieve this. This is confusing for users. Please clarify the description to indicate which value (true or false) enables the feature.
# Set all pods to run in a user namespace without host access.
# Experimental: may be removed once the Kubernetes User Namespaces feature is GA.
#
# Requirements:
# - Kubernetes ≥ 1.33, or
# - Kubernetes 1.27–1.32 with UserNamespacesSupport feature gate enabled.
#
# Set to `true` to run pods in a user namespace without host access.
#
# See [limitations](https://kubernetes.io/docs/concepts/workloads/pods/user-namespaces/#limitations) for details.
# +docs:property
# hostUsers: false82d32ca to
4771506
Compare
9817626 to
6dee6dd
Compare
a40ed6c to
12c7239
Compare
a1e51ca to
50692c0
Compare
bb1e64e to
983fc47
Compare
ca5cfc2 to
4a5f710
Compare
88902fe to
96699a4
Compare
Signed-off-by: robot <[email protected]>
96699a4 to
d6f6fd5
Compare
I am robot, upgrade: project cert-manager chart upgrade from v1.16.4 to v1.19.1