Conditionally create webhook-related resources #158
Conversation
…d service) are only created when webhook.enabled=true. fix #157
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a Helm conditional around webhook-related Kubernetes resources in the job-update-webhook-certificates template so those resources are rendered only when Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Helm as Helm renderer
participant Template as job-update-webhook-certificates.yaml
participant K8s as Kubernetes API
Helm->>Template: render template
alt .Values.webhook.enabled == true
Template-->>Helm: include webhook resources
Helm->>K8s: create/update webhook Jobs/ServiceAccount/Secrets
note right of K8s #DFF2BF: webhook resources created
else .Values.webhook.enabled == false
Template-->>Helm: skip webhook resources
note right of Helm #FEE4CB: no webhook resources rendered
end
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Comment |
Summary of ChangesHello @chrislusf, 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 resolves issue #157 by introducing conditional deployment for webhook-related Kubernetes resources. It specifically modifies the Helm chart to ensure that the job responsible for updating webhook certificates is only created when webhooks are explicitly enabled via 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.
Pull Request Overview
This PR adds conditional rendering to the webhook certificate update job based on the webhook.enabled configuration value. This prevents unnecessary Kubernetes resources from being created when the webhook functionality is disabled.
- Wraps the entire webhook certificate update job YAML template with
{{- if .Values.webhook.enabled }}conditional blocks
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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 introduces a condition to create webhook-related resources only when .Values.webhook.enabled is true. The change is implemented by wrapping the contents of deploy/helm/templates/webhook/job-update-webhook-certificates.yaml in an if block, which correctly makes the creation of jobs, service accounts, and roles conditional. My review includes suggestions to improve whitespace handling in the Helm template, which is a best practice for producing clean and predictable YAML output. The PR description also mentions a webhook Service. As I cannot see its definition in the changed files, please ensure it is also conditionally created to fully resolve the issue.
| @@ -1,3 +1,4 @@ | |||
| {{- if .Values.webhook.enabled }} | |||
There was a problem hiding this comment.
To prevent rendering an extra newline at the beginning of the file and to ensure the output is completely empty when webhook.enabled is false, it's a good practice to use whitespace control. Adding a hyphen to the closing brace will chomp the trailing newline after this directive.
{{- if .Values.webhook.enabled -}}| - kind: ServiceAccount | ||
| name: {{ include "seaweedfs-operator.fullname" . }}-update-webhook-certificates | ||
| namespace: {{ .Release.Namespace }} | ||
| {{- end }} |
There was a problem hiding this comment.
Similarly to the opening if block, adding a hyphen here will ensure no unnecessary whitespace is rendered. This makes the template's output cleaner and more predictable. Using {{- end -}} will consume the newline before it, and combined with the change at the start of the file, will result in a completely empty file when the condition is not met.
{{- end -}}
fix #157
webhook-related resources (jobs, service account, roles, webhooks, and service) are only created when webhook.enabled=true.
Summary by CodeRabbit