Skip to content
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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dadangnh
Copy link

@dadangnh dadangnh commented Jan 3, 2025

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

  • Added new deploymentAnnotations and deploymentLabels fields in values.yaml
  • Updated all deployment manifests (main, webhook, and worker) to support the new fields
  • Maintained backwards compatibility by making the new fields optional
  • Kept existing pod-level annotations and labels functionality unchanged

Example Usage

# values.yaml or values override
deploymentAnnotations:
  fluxcd.io/automated: "true"
  notification.monitor.io/slack: "true"

deploymentLabels:
  environment: production
  team: platform

Summary by CodeRabbit

  • New Features

    • Added support for custom deployment annotations and labels in the Helm chart
    • Enabled flexible configuration of metadata for n8n Kubernetes deployments
  • Improvements

    • Enhanced deployment configuration with optional label and annotation customization
    • Improved deployment resource metadata management through Helm values

@dadangnh dadangnh requested a review from a team as a code owner January 3, 2025 11:59
Copy link

coderabbitai bot commented Jan 3, 2025

Walkthrough

The 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: deploymentAnnotations and deploymentLabels. These parameters allow users to dynamically add custom annotations and labels to deployment resources across different components (main deployment, webhooks, and worker) without modifying the core template structure.

Changes

File Change Summary
charts/n8n/values.yaml Added new optional configuration sections deploymentAnnotations and deploymentLabels as empty objects
charts/n8n/templates/deployment.yaml
charts/n8n/templates/deployment.webhooks.yaml
charts/n8n/templates/deployment.worker.yaml
Added conditional rendering for deployment labels and annotations using .Values.deploymentLabels and .Values.deploymentAnnotations

Sequence Diagram

sequenceDiagram
    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
Loading

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a 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: platform
charts/n8n/templates/deployment.worker.yaml (1)

7-13: Consider adding usage examples in README

To 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

📥 Commits

Reviewing files that changed from the base of the PR and between 6ab4ff4 and 62d8102.

📒 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 and deploymentLabels 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 and deploymentAnnotations 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 cases
    • deploymentLabels 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.yaml

Length 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 template

The 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 templates

The 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 metadata

The 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:

  1. Pod-level metadata (podAnnotations, podLabels) are defined separately in values.yaml with empty default values
  2. All deployment templates (main, webhooks, worker) consistently handle pod-level metadata within the pod template spec
  3. 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*.yaml

Length 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)

@Vad1mo
Copy link
Member

Vad1mo commented Jan 3, 2025

This chart is currently redesigned to support the changed config option of n8n.
Please follow the issue #129 for more details and the current state of development.

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)

@dadangnh
Copy link
Author

dadangnh commented Jan 4, 2025

Thanks for the clarification, I've commented on the discussion based on your proposal. Please kindly check

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants