add nats merge config #490
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dosubot
bot
added
the
size:L
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe pull request introduces several updates to the NATS application, primarily focusing on versioning and configuration enhancements. The Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
packages/apps/nats/README.md (1)
7-15: Parameter documentation needs clarificationA few issues with the parameter documentation:
- The description for
replicasis incorrect - it says "Persistent Volume size for NATS" but should describe the number of NATS instances- The
usersparameter could benefit from more detailed documentation about its structure and purpose- Consider adding examples for the
config.mergeparameter to illustrate common use casesConsider updating the table with these improvements:
| Name | Description | Value | | ------------------- | -------------------------------------------------- | ------- | | `external` | Enable external access from outside the cluster | `false` | -| `replicas` | Persistent Volume size for NATS | `2` | +| `replicas` | Number of NATS server instances to run | `2` | | `storageClass` | StorageClass used to store the data | `""` | -| `users` | Users configuration | `{}` | +| `users` | NATS users and their credentials configuration | `{}` | | `jetstream.size` | Jetstream persistent storage size | `10Gi` | | `jetstream.enabled` | Enable or disable Jetstream | `true` | -| `config.merge` | Additional configuration to merge into NATS config | `{}` | +| `config.merge` | Additional NATS server configuration to merge (e.g., authentication, limits) | `{}` |packages/apps/nats/values.yaml (1)
30-35: Enhance config.merge documentation with examplesThe
config.mergeparameter would benefit from example configurations in the comments to guide users.Add example configurations:
config: ## @param config.merge Additional configuration to merge into NATS config ## Allows you to customize NATS server settings by merging additional configurations. ## For example, you can add extra parameters, configure authentication, or set custom settings. + ## Examples: + ## merge: + ## max_payload: 1MB + ## max_connections: 1000 + ## tls: + ## cert_file: /path/to/cert ## Default: {} merge: {}packages/apps/nats/values.schema.json (1)
35-44: Add additionalProperties validation for config.mergeThe
config.mergeobject currently allows any properties without validation.Consider adding basic type validation:
"config": { "type": "object", "properties": { "merge": { "type": "object", "description": "Additional configuration to merge into NATS config", - "default": {} + "default": {}, + "additionalProperties": { + "anyOf": [ + { "type": "string" }, + { "type": "number" }, + { "type": "boolean" }, + { "type": "object" } + ] + } } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
packages/apps/nats/Chart.yaml(1 hunks)packages/apps/nats/README.md(1 hunks)packages/apps/nats/templates/nats.yaml(2 hunks)packages/apps/nats/values.schema.json(1 hunks)packages/apps/nats/values.yaml(1 hunks)packages/apps/versions_map(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/apps/nats/Chart.yaml
🔇 Additional comments (4)
packages/apps/versions_map (1)
55-56: LGTM! Version mapping changes look correct.
The changes appropriately:
- Finalize version 0.3.1 with specific commit hash
- Add new version 0.4.0 as HEAD for the new features
packages/apps/nats/values.yaml (1)
19-28: Consider security implications of Jetstream defaults
While the Jetstream configuration is well-structured, enabling it by default (enabled: true) with a fixed storage size (10Gi) might not be suitable for all deployments.
Let's check if there are any existing deployments that might be affected:
Consider:
- Making
enabled: falseby default to require explicit opt-in - Adding a warning comment about storage requirements
- Adding validation for minimum storage size
packages/apps/nats/templates/nats.yaml (2)
Line range hint 43-69: Overall template structure looks good
The template changes maintain proper YAML structure and integrate well with the existing configuration. The conditional logic for merging accounts and configurations is well-organized.
66-69: Verify jetstream configuration values
The template now references dynamic values for jetstream configuration. Please ensure these values are properly defined in values.yaml with appropriate defaults.
Let's verify the configuration values:
✅ Verification successful
Jetstream configuration values are properly defined and documented
The verification shows that both jetstream.enabled and jetstream.size values are:
- Defined in values.yaml with a default size of "10Gi"
- Validated in values.schema.json with proper type definitions and descriptions
- Documented in README.md with their default values
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if jetstream configuration values are properly defined
# Check values.yaml for jetstream configuration
echo "Checking values.yaml for jetstream configuration..."
rg -A 5 'jetstream:' packages/apps/nats/values.yaml
# Check values schema
echo "Checking schema validation..."
rg -A 10 'jetstream' packages/apps/nats/values.schema.json
# Check documentation
echo "Checking documentation..."
rg 'jetstream\.(size|enabled)' packages/apps/nats/README.md
Length of output: 1183
Comment on lines
+20
to
34
| "jetstream": { | ||
| "type": "object", | ||
| "properties": { | ||
| "size": { | ||
| "type": "string", | ||
| "description": "Jetstream persistent storage size", | ||
| "default": "10Gi" | ||
| }, | ||
| "enabled": { | ||
| "type": "boolean", | ||
| "description": "Enable or disable Jetstream", | ||
| "default": true | ||
| } | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add required properties and pattern validation
The Jetstream configuration could benefit from additional schema validation.
Consider adding these improvements:
"jetstream": {
"type": "object",
+ "required": ["size", "enabled"],
"properties": {
"size": {
"type": "string",
"description": "Jetstream persistent storage size",
+ "pattern": "^[0-9]+(Gi|Mi)$",
"default": "10Gi"
},
"enabled": {
"type": "boolean",
"description": "Enable or disable Jetstream",
"default": true
}
}
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "jetstream": { | |
| "type": "object", | |
| "properties": { | |
| "size": { | |
| "type": "string", | |
| "description": "Jetstream persistent storage size", | |
| "default": "10Gi" | |
| }, | |
| "enabled": { | |
| "type": "boolean", | |
| "description": "Enable or disable Jetstream", | |
| "default": true | |
| } | |
| } | |
| } | |
| }, | |
| "jetstream": { | |
| "type": "object", | |
| "required": ["size", "enabled"], | |
| "properties": { | |
| "size": { | |
| "type": "string", | |
| "description": "Jetstream persistent storage size", | |
| "pattern": "^[0-9]+(Gi|Mi)$", | |
| "default": "10Gi" | |
| }, | |
| "enabled": { | |
| "type": "boolean", | |
| "description": "Enable or disable Jetstream", | |
| "default": true | |
| } | |
| } | |
| }, |
Comment on lines
+54
to
+56
| {{- if .Values.nats.config.merge }} | ||
| {{ toYaml .Values.config.merge }} | ||
| {{- end }} |
There was a problem hiding this comment.
Fix inconsistent config path reference
There's a path mismatch in the config merge section. The condition checks .Values.nats.config.merge but the template uses .Values.config.merge. This will cause the merge to fail.
Apply this fix:
{{- if .Values.nats.config.merge }}
- {{ toYaml .Values.config.merge }}
+ {{ toYaml .Values.nats.config.merge }}
{{- end }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {{- if .Values.nats.config.merge }} | |
| {{ toYaml .Values.config.merge }} | |
| {{- end }} | |
| {{- if .Values.nats.config.merge }} | |
| {{ toYaml .Values.nats.config.merge }} | |
| {{- end }} |
Signed-off-by: Andrei Kvapil <kvapss@gmail.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced a new patch application step in the update process for KubeOVN. - Enhanced flexibility in the `kube-ovn-cni` configuration by allowing users to specify the Maximum Transmission Unit (MTU) for improved network performance. - **Bug Fixes** - Applied a patch to ensure the new MTU configuration is properly integrated into the deployment process. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes - **New Features** - Introduced a new variable `$host` for improved configuration management. - Added a `valuesFrom` section to the `dashboard` release, allowing external value sourcing. - Enhanced Keycloak integration with new client scopes, roles, and configurations for Kubeapps. - Added support for custom pod specifications and environment variables in Redis configurations. - Introduced a new Kubernetes configuration file for managing access to resources via Role and Secret. - Updated image versions across various components to ensure compatibility and leverage new features. - **Bug Fixes** - Implemented error handling to ensure required configurations are present. - Improved handling of request headers for the `/logos` endpoint in Nginx configuration. - Adjusted security context configurations to enhance deployment security. - **Documentation** - Updated configuration files to reflect new dependencies and structures for better clarity. - Enhanced README documentation with upgrade instructions and security defaults. - Expanded notes on handling persistent volumes and data migration during upgrades. These enhancements improve the overall functionality and reliability of the platform. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Upgraded MetalLB application version to `v0.14.8`. - Introduced a new `frr-k8s` dependency for enhanced BGP management. - Added new configuration options for TLS settings and extra containers in the controller. - Implemented new Custom Resource Definitions (CRDs) for managing FRR configurations and node states. - **Bug Fixes** - Improved validation logic for service account names to ensure consistency. - **Documentation** - Updated README files for the MetalLB and `frr-k8s` charts to reflect new features and configuration options. - **Refactor** - Enhanced RBAC configurations for better resource management and security. - Improved webhook configurations for better validation and consistency. - **Chores** - Updated various YAML configuration files to include namespace specifications for clarity. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced new configuration options for socket-based load balancing tracing and initial fetch timeout settings in the Cilium deployment. - Enhanced validation checks for deprecated options to prevent misconfigurations. - **Bug Fixes** - Improved error messaging for deprecated or invalid settings. - **Documentation** - Updated version numbers in README and configuration files to reflect the new version (1.16.4). - **Chores** - Updated Dockerfile and image tags to reference the latest version (1.16.4). <!-- end of auto-generated comment: release notes by coderabbit.ai -->
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes - **New Features** - Updated Piraeus Operator chart to version 2.7.1. - Introduced new Custom Resource Definitions (CRDs) for enhanced management of LINSTOR resources. - **Improvements** - Updated image tags for various components to their latest versions. - Added `nodeSelector` and `affinity` fields for improved pod scheduling in deployments. These enhancements provide users with better resource management and operational capabilities. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> - **New Features** - Introduced a new pre-commit hook (`run-make-generate`) to automate the generation process in application directories. - **Documentation** - Enhanced readability of the Managed NATS Service README by adjusting formatting and removing unnecessary headers. - **Bug Fixes** - Corrected JSON structure in the Postgres values schema to ensure validity. - **Chores** - Updated pre-commit configuration for improved consistency and functionality. - Reorganized properties in the NATS values schema, removing the `users` property to reflect changes in user management capabilities. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced build process for Kubeapps with improved modularity and patch integration. - Introduced version specification for Kubeapps builds. - **Bug Fixes** - Streamlined plugin build commands for better performance and clarity. - **Refactor** - Restructured Dockerfile to utilize different base images and optimize the build stages. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes - **New Features** - Integrated OpenID Connect (OIDC) for enhanced authentication. - Added dynamic Role resource for tenant-specific access to Kubernetes secrets. - Introduced new Keycloak realm groups for improved role management. - **Improvements** - Enhanced error handling for service readiness checks. - Streamlined configuration files for better clarity and management of OIDC settings. - Updated handling of API server address and improved configuration adaptability based on OIDC settings. - **Bug Fixes** - Removed deprecated configurations related to Keycloak, simplifying deployment. These updates aim to improve security, usability, and overall system performance. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced new Makefiles for `keycloak`, `keycloak-configure`, and `keycloak-operator` packages, establishing environment variables for deployment. - Each Makefile includes common scripts to streamline build and environment settings. - **Bug Fixes** - No specific bug fixes were mentioned. - **Documentation** - No updates to documentation were noted. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced management of Kubernetes secrets for `k8s-client`, `kubeapps-client`, and `kubeapps-auth-config`. - Improved handling of client secrets by reusing existing configurations when available. - **Bug Fixes** - Addressed issues with static secret definitions, streamlining the configuration process. - **Chores** - Removed outdated secret and Keycloak client definitions for cleaner configuration management. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Summary by CodeRabbit - **New Features** - Enhanced management of Keycloak credentials by checking for existing passwords stored in Kubernetes Secrets. - Improved password management logic, allowing for the reuse of existing passwords or the generation of new ones as needed. - **Bug Fixes** - Streamlined secret handling to avoid unnecessary random password generation, improving security and maintainability. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Floppy Disk <kklinch0@gmail.com>
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes - **New Features** - Updated container images for various components to their latest versions, enhancing performance and security. - **Bug Fixes** - Addressed potential issues by upgrading image tags and digests for components such as CozyStack, ClickHouse, PostgreSQL, and others. - **Documentation** - Updated `values.yaml` configurations for multiple packages to reflect the latest image versions and digests. These updates ensure improved functionality and reliability across the application. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
I saw your call for adopters - I am sort of in production now, but not with any services that I can advertise. This Urmanac is something I'm testing on WASM workloads. I also have hosted some Ruby services on my cluster. I am still in the proof-of-concept phase with my production workloads, working towards a service level of 99.5% or better. I am running SpinKube on Cozystack, with my own Talos Linux image that I have built to add the Spin and Tailscale extensions. (The urmanac is in beta at: https://beta.urmanac.com - urmanac.com is a dead link for now.) What's holding me back currently is hardware, not so much the software stack. I have deployed Cozystack on some severely under-powered machines. Every time I push it to the limit, my load averages shoot up into the 100's and I unfortunately bring my control plane and services down. I will probably get better results when I am able to separate the KubeVirt clusters from the data plane and control plane. When the load rises too high, etcd becomes unresponsive, and it goes downhill from there. I am very impressed with the architecture of Cozystack and I have made some contributions to Cozystack on behalf of the FluxCD community! I am in firm support of your goal to join the CNCF. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added "Urmanac" to the Cozystack Adopters list, including contact information and a description of its use of Cozystack. - **Documentation** - Reformatted the existing entry for "gohost" for consistency. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Kingdon Barrett <kingdon+github@tuesdaystudios.com>
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced Keycloak client configuration with new secrets for `k8s-client`, `kubeapps-client`, and `kubeapps-auth-config`. - Introduced new `ClusterKeycloak` and `ClusterKeycloakRealm` resources for improved management. - Updated Keycloak client scopes with additional attributes and protocol mappers. - Added multiple CiliumNetworkPolicy and CiliumClusterwideNetworkPolicy configurations for better traffic control. - **Improvements** - Logic added to check for existing Kubernetes secrets and generate new ones as needed, ensuring seamless configuration management. - Enhanced network policies to provide comprehensive control over ingress and egress traffic for various services within the tenant's namespace. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes
New Features
jetstream.sizeandjetstream.enabled, enhancing persistent storage options.config.mergeparameter for additional customization of NATS server settings.Bug Fixes
Documentation
README.mdto reflect new configuration options and parameters for the Managed NATS Service.Versioning
0.3.1to0.4.0.