-
Notifications
You must be signed in to change notification settings - Fork 112
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
feat: add support for deployment annotations and labels #129
base: main
Are you sure you want to change the base?
feat: add support for deployment annotations and labels #129
Conversation
WalkthroughThe pull request introduces enhanced configuration options for the n8n Helm chart's Kubernetes deployments. The changes focus on adding flexibility to deployment metadata by introducing two new optional configuration parameters: Changes
Sequence DiagramsequenceDiagram
participant User as Helm Values
participant Chart as Helm Chart
participant K8s as Kubernetes Deployment
User->>Chart: Provide deploymentLabels
User->>Chart: Provide deploymentAnnotations
Chart->>K8s: Conditionally render labels
Chart->>K8s: Conditionally render annotations
The sequence diagram illustrates how users can provide optional deployment metadata through Helm values, which are then conditionally rendered into the Kubernetes deployment resources during chart installation. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
charts/n8n/values.yaml (2)
180-185
: LGTM! Consider enhancing documentation with common use cases.The implementation of
deploymentAnnotations
follows Helm best practices. The examples provided are good starting points.Consider adding more examples for common use cases:
# Prometheus scraping prometheus.io/scrape: "true" prometheus.io/port: "9100" # AWS Load Balancer Controller alb.ingress.kubernetes.io/target-type: ip # Argo CD argocd.argoproj.io/sync-wave: "1"
186-191
: LGTM! Consider documenting label best practices.The implementation of
deploymentLabels
follows Helm best practices. The examples provided are clear and practical.Consider adding a comment about Kubernetes label best practices:
deploymentLabels: {} # Follow Kubernetes label syntax: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set # example: # app.kubernetes.io/component: backend # app.kubernetes.io/environment: production # app.kubernetes.io/team: platformcharts/n8n/templates/deployment.worker.yaml (1)
7-13
: Consider adding usage examples in READMETo improve user experience, consider adding example usage of the new deployment annotations and labels in the README.md file. This would help users understand how to leverage these new features effectively.
Example addition to README.md:
+ ## Deployment Annotations and Labels + + You can add custom annotations and labels at the deployment level using `deploymentAnnotations` and `deploymentLabels`: + + ```yaml + deploymentAnnotations: + fluxcd.io/automated: "true" + fluxcd.io/tag.nginx: semver:~1.0 + + deploymentLabels: + environment: production + team: platform + ```🧰 Tools
🪛 yamllint (1.35.1)
[warning] 9-9: wrong indentation: expected 2 but found 4
(indentation)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
charts/n8n/templates/deployment.webhooks.yaml
(1 hunks)charts/n8n/templates/deployment.worker.yaml
(1 hunks)charts/n8n/templates/deployment.yaml
(1 hunks)charts/n8n/values.yaml
(1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
charts/n8n/templates/deployment.worker.yaml
[warning] 9-9: wrong indentation: expected 2 but found 4
(indentation)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
charts/n8n/templates/deployment.webhooks.yaml
[warning] 9-9: wrong indentation: expected 2 but found 4
(indentation)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
charts/n8n/templates/deployment.yaml
[warning] 7-7: wrong indentation: expected 2 but found 4
(indentation)
[warning] 8-8: wrong indentation: expected 2 but found 4
(indentation)
[warning] 9-9: wrong indentation: expected 2 but found 4
(indentation)
🔇 Additional comments (5)
charts/n8n/values.yaml (1)
180-191
: Verify template implementation.Let's verify that these new fields are correctly implemented in the deployment templates.
✅ Verification successful
Template implementation is correctly done across all deployment manifests
The verification shows that both
deploymentAnnotations
anddeploymentLabels
are properly implemented:
- The fields are consistently used in all three deployment templates:
charts/n8n/templates/deployment.yaml
charts/n8n/templates/deployment.webhooks.yaml
charts/n8n/templates/deployment.worker.yaml
- The implementation follows Helm best practices:
- Conditional blocks with proper
if
statements- Correct indentation using
nindent 4
- Proper use of
toYaml
for mapping the values🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of deploymentAnnotations and deploymentLabels in deployment templates # Check if the new fields are used in deployment templates echo "Checking deployment templates for new fields..." rg -l 'deploymentAnnotations|deploymentLabels' 'charts/n8n/templates/' # Verify the syntax used to include these fields echo "Verifying template syntax..." rg -A 5 '\.Values\.(deploymentAnnotations|deploymentLabels)' 'charts/n8n/templates/'Length of output: 2897
charts/n8n/templates/deployment.yaml (1)
7-13
: Implementation looks good and follows best practices!The implementation correctly adds support for deployment-level annotations and labels while maintaining backward compatibility. The conditional blocks ensure that the fields are only included when values are provided.
Let's verify if the corresponding values are defined in values.yaml:
✅ Verification successful
Values are properly defined with examples in values.yaml
The verification confirms that both
deploymentLabels
anddeploymentAnnotations
are properly defined in the values.yaml file with:
- Default empty map values (
{}
)- Helpful example comments showing the expected format
deploymentAnnotations
example shows monitoring and FluxCD use casesdeploymentLabels
example shows environment and team labeling use cases🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if deploymentLabels and deploymentAnnotations are defined in values.yaml rg -A 5 "deploymentLabels:|deploymentAnnotations:" charts/n8n/values.yamlLength of output: 331
🧰 Tools
🪛 yamllint (1.35.1)
[warning] 7-7: wrong indentation: expected 2 but found 4
(indentation)
[warning] 8-8: wrong indentation: expected 2 but found 4
(indentation)
[warning] 9-9: wrong indentation: expected 2 but found 4
(indentation)
charts/n8n/templates/deployment.webhooks.yaml (1)
8-14
: Implementation is consistent with the main deployment templateThe changes maintain consistency across all deployment templates, which is a good practice.
🧰 Tools
🪛 yamllint (1.35.1)
[warning] 9-9: wrong indentation: expected 2 but found 4
(indentation)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
charts/n8n/templates/deployment.worker.yaml (2)
8-14
: Implementation is consistent with other deployment templatesThe changes maintain consistency across all deployment templates, which is a good practice.
🧰 Tools
🪛 yamllint (1.35.1)
[warning] 9-9: wrong indentation: expected 2 but found 4
(indentation)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
7-13
: Verify coexistence with pod-level metadataThe implementation looks correct, but let's verify that both deployment and pod level metadata can coexist without conflicts.
✅ Verification successful
Pod and deployment level metadata are properly separated
The verification shows that:
- Pod-level metadata (
podAnnotations
,podLabels
) are defined separately invalues.yaml
with empty default values- All deployment templates (main, webhooks, worker) consistently handle pod-level metadata within the pod template spec
- The indentation levels are correct:
- Deployment metadata at 4 spaces
- Pod metadata at 8 spaces
This confirms that deployment and pod level metadata can safely coexist as they are rendered at different levels in the YAML hierarchy.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if there are any potential conflicts between pod and deployment level metadata rg -A 10 "podAnnotations:|podLabels:" charts/n8n/values.yaml # Verify the template rendering with both levels of metadata echo "Checking if pod-level metadata is preserved in the template..." rg "pod(Annotations|Labels)" charts/n8n/templates/deployment*.yamlLength of output: 1312
🧰 Tools
🪛 yamllint (1.35.1)
[warning] 9-9: wrong indentation: expected 2 but found 4
(indentation)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
This chart is currently redesigned to support the changed config option of n8n. Currently, it makes no sense to accept this PR, but I will consider your contribution for the new chart version. Check the new values.yaml to see if this is already supported. #125 (comment) |
Thanks for the clarification, I've commented on the discussion based on your proposal. Please kindly check |
Add support for custom deployment annotations and labels
Description
This PR adds support for custom annotations and labels at the deployment level. Previously, the chart only supported pod-level annotations and labels. This enhancement allows users to add Kubernetes annotations and labels specifically at the deployment level, which is useful for various deployment management and integration scenarios.
Changes Made
deploymentAnnotations
anddeploymentLabels
fields in values.yamlExample Usage
Summary by CodeRabbit
New Features
Improvements