Introduce helm charts for WSO2 Integrator: WebSubHub#1
Conversation
WalkthroughIntroduces Helm chart configurations for websubhub and websubhub-consolidator components with deployment templates, service definitions, ConfigMap templates, and configuration files. Adds repository-wide code ownership and macOS-specific git ignore entries. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (5)
distributed/websubhub/templates/service.yaml (1)
27-30: Consider adding a name to the service port.Adding a
namefield to the port definition improves clarity and is beneficial for service mesh integrations (like Istio) and monitoring tools that reference ports by name.🔎 Proposed enhancement
ports: - protocol: TCP + name: http port: {{ .Values.deployment.config.server.port }} targetPort: {{ .Values.deployment.config.server.port }}distributed/websubhub-consolidator/templates/service.yaml (1)
27-30: Consider adding a name to the service port.Adding a
namefield enhances clarity and supports service mesh integrations and monitoring tools.🔎 Proposed enhancement
ports: - protocol: TCP + name: http port: {{ .Values.deployment.config.server.port }} targetPort: {{ .Values.deployment.config.server.port }}distributed/websubhub-consolidator/values.yaml (1)
19-19: Consider parameterizing environment-specific URLs.The Solace connection URLs (
tcp://solace-pubsubplus-dev:55555andhttp://solace-pubsubplus-dev:8080/SEMP/v2/config) contain environment-specific hostnames. Consider documenting that these must be overridden for production deployments, or provide environment-specific value files.Also applies to: 27-27
distributed/websubhub/values.yaml (1)
26-26: Hard-coded development environment URLs.The Solace URLs contain "-dev" suffix and are hard-coded. For production deployments, these should be configurable or use environment-specific values files.
Also applies to: 34-34
distributed/websubhub/confs/Config.toml (1)
3-3: Consider using SERVER_ID environment variable for dynamic pod identification.The server.id is templated from values, which is hard-coded as "websubhub-1". For a StatefulSet with multiple replicas, consider reading from the SERVER_ID environment variable (already set in statefulset.yaml line 69) to ensure each pod has a unique identifier.
This relates to the hard-coded server.id issue in values.yaml.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
.github/CODEOWNERS.github/issue_template.md.github/pull_request_template.md.gitignoredistributed/websubhub-consolidator/Chart.yamldistributed/websubhub-consolidator/confs/Config.tomldistributed/websubhub-consolidator/templates/conf.yamldistributed/websubhub-consolidator/templates/deployment.yamldistributed/websubhub-consolidator/templates/service.yamldistributed/websubhub-consolidator/values.yamldistributed/websubhub/Chart.yamldistributed/websubhub/confs/Config.tomldistributed/websubhub/templates/conf.yamldistributed/websubhub/templates/service.yamldistributed/websubhub/templates/statefulset.yamldistributed/websubhub/values.yaml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
distributed/websubhub/templates/statefulset.yaml
[error] 22-22: syntax error: expected , but found ''
(syntax)
distributed/websubhub/templates/service.yaml
[error] 22-22: syntax error: expected , but found ''
(syntax)
distributed/websubhub-consolidator/templates/service.yaml
[error] 22-22: syntax error: expected , but found ''
(syntax)
distributed/websubhub-consolidator/templates/deployment.yaml
[error] 22-22: syntax error: expected , but found ''
(syntax)
🔇 Additional comments (6)
.gitignore (1)
25-27: Well-placed addition of macOS-specific ignore patterns.The addition of
.DS_Store(and the comment header) is appropriate for cross-platform development..DS_Storeis a standard macOS metadata file that should never be committed, and excluding it prevents accidental pollution of the repository on macOS systems..github/CODEOWNERS (1)
1-7: LGTM!The CODEOWNERS file is correctly formatted with proper syntax and helpful comments. The default ownership rule assigns both maintainers to all repository paths, which is appropriate for a new Helm charts repository.
distributed/websubhub-consolidator/confs/Config.toml (2)
15-16: Credentials are sourced from values.yaml.These auth configuration lines reference
.Values.deployment.config.store.solace.auth.usernameand.Values.deployment.config.store.solace.auth.password. Ensure the critical security issue in values.yaml (hardcoded credentials) is addressed. Once values.yaml uses Secrets, this template should be updated to inject credentials securely (e.g., via environment variables or mounted Secrets).Also applies to: 24-25
1-28: LGTM: Helm templating in TOML is correct.The use of Helm templating within the TOML configuration file is appropriate. The
tplfunction in conf.yaml will correctly process these templates at render time.distributed/websubhub/templates/statefulset.yaml (1)
22-22: Static analysis false positive - Helm templating is valid.The YAMLlint syntax error is a false positive. Helm template expressions like
{{ .Release.Name }}are valid and will be rendered before the YAML is applied to Kubernetes. You can safely ignore this linting error or configure YAMLlint to skip Helm template files.distributed/websubhub-consolidator/templates/deployment.yaml (1)
22-22: Static analysis false positive - Helm templating is valid.The YAMLlint syntax error is a false positive. Helm template expressions are valid and rendered before YAML is applied to Kubernetes. This can be safely ignored or YAMLlint can be configured to skip Helm templates.
| # ------------------------------------------------------------------------------------- | ||
| # | ||
| # Copyright (c) 2025, WSO2 LLC. (http://www.wso2.com). All Rights Reserved. | ||
| # | ||
| # This software is the property of WSO2 LLC. and its suppliers, if any. | ||
| # Dissemination of any information or reproduction of any material contained | ||
| # herein is strictly forbidden, unless permitted by WSO2 in accordance with the | ||
| # WSO2 Commercial License available at https://wso2.com/licenses/eula/3.2 | ||
| # | ||
| # -------------------------------------------------------------------------------------- |
There was a problem hiding this comment.
License inconsistency across chart files.
The Chart.yaml uses the WSO2 Commercial License (EULA 3.2), while template files use Apache License 2.0. Ensure consistent licensing across all chart files.
🤖 Prompt for AI Agents
In distributed/websubhub-consolidator/Chart.yaml lines 1-10, the file header
declares the WSO2 Commercial License (EULA 3.2) while other chart/template files
use Apache-2.0; update Chart.yaml to use the same license text and metadata as
the other charts (replace the WSO2 EULA header with the Apache License 2.0
header and ensure the "license" or "annotations" fields in Chart.yaml reflect
Apache-2.0) so all chart files consistently declare Apache-2.0.
| # | ||
| # -------------------------------------------------------------------------------------- | ||
|
|
||
| apiVersion: v1 |
There was a problem hiding this comment.
Update to Helm 3 apiVersion.
The chart uses apiVersion: v1 (Helm 2 format). Use apiVersion: v2 for Helm 3 compatibility and to access features like dependencies.
🔎 Proposed fix
-apiVersion: v1
+apiVersion: v2📝 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.
| apiVersion: v1 | |
| apiVersion: v2 |
🤖 Prompt for AI Agents
In distributed/websubhub-consolidator/Chart.yaml around line 12, the chart
declares apiVersion: v1 (Helm 2 format); update it to apiVersion: v2 to be Helm
3 compatible and enable Helm 3 features such as dependencies. Edit the
Chart.yaml to replace the apiVersion value from v1 to v2 and verify the chart
still validates with helm lint and installs with Helm 3.
| apiVersion: v1 | ||
| kind: ConfigMap | ||
| metadata: | ||
| name: websubhub-consolidator-svc-cm |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use Release.Name in ConfigMap name for uniqueness.
The hardcoded ConfigMap name websubhub-consolidator-svc-cm prevents multiple releases in the same namespace. Use {{ .Release.Name }}-cm to ensure uniqueness across releases.
🔎 Proposed fix
- name: websubhub-consolidator-svc-cm
+ name: {{ .Release.Name }}-cmNote: Update any references to this ConfigMap in Deployment/StatefulSet templates to use the same naming pattern.
🤖 Prompt for AI Agents
In distributed/websubhub-consolidator/templates/conf.yaml around line 22, the
ConfigMap name is hardcoded as "websubhub-consolidator-svc-cm", which prevents
deploying multiple releases in the same namespace; change the name to use the
Helm release name template (e.g. "{{ .Release.Name }}-cm") for uniqueness and
update any Deployment/StatefulSet or other template references to the same
templated name so they point to the renamed ConfigMap.
| - name: {{ .Release.Name }} | ||
| image: {{ .Values.deployment.image.repository }}:{{ .Values.deployment.image.tag }} | ||
| imagePullPolicy: {{ .Values.deployment.image.pullPolicy }} | ||
| ports: | ||
| - name: http | ||
| containerPort: {{ .Values.deployment.config.server.port }} | ||
| livenessProbe: | ||
| httpGet: | ||
| path: /health/liveness | ||
| port: http | ||
| scheme: HTTP | ||
| initialDelaySeconds: 30 | ||
| periodSeconds: 10 | ||
| timeoutSeconds: 5 | ||
| failureThreshold: 3 | ||
| readinessProbe: | ||
| httpGet: | ||
| path: /health/readiness | ||
| port: http | ||
| scheme: HTTP | ||
| initialDelaySeconds: 15 | ||
| periodSeconds: 5 | ||
| timeoutSeconds: 3 | ||
| failureThreshold: 3 | ||
| env: | ||
| - name: "BAL_CONFIG_FILES" | ||
| value: "/home/wso2/conf/Config.toml" | ||
| volumeMounts: | ||
| - name: "config-config-map-volume" | ||
| mountPath: "/home/wso2/conf" | ||
| readOnly: true |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Missing resource limits and requests.
Unlike the websubhub StatefulSet, this Deployment has no resource specifications defined (not even commented). For production deployments, resource limits are essential for cluster stability and proper scheduling.
🔎 Add resources block
Add resource specifications to values.yaml for the consolidator:
deployment:
resources:
requests:
cpu: "500m"
memory: "512Mi"
limits:
cpu: "1"
memory: "1Gi"Then reference in the Deployment template after line 66:
resources:
requests:
cpu: {{ .Values.deployment.resources.requests.cpu }}
memory: {{ .Values.deployment.resources.requests.memory }}
limits:
cpu: {{ .Values.deployment.resources.limits.cpu }}
memory: {{ .Values.deployment.resources.limits.memory }}🤖 Prompt for AI Agents
distributed/websubhub-consolidator/templates/deployment.yaml lines 36-66: The
Deployment is missing resource requests/limits which are required for production
scheduling and cluster stability; add a deployment.resources section to
values.yaml (example keys: requests.cpu, requests.memory, limits.cpu,
limits.memory with values like "500m"/"512Mi" and "1"/"1Gi") and then insert a
resources block into this Deployment template immediately after line 66 using
those Helm values (map requests and limits to
.Values.deployment.resources.requests.* and
.Values.deployment.resources.limits.*), ensuring the template renders correctly
when values are provided.
| volumes: | ||
| - name: "config-config-map-volume" | ||
| configMap: | ||
| name: "websubhub-consolidator-svc-cm" |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Hard-coded ConfigMap name should use Helm templating.
The ConfigMap name "websubhub-consolidator-svc-cm" is hard-coded, preventing chart reusability. Use {{ .Release.Name }}-config or a similar pattern for consistency with Helm best practices.
🔎 Use templated ConfigMap name
configMap:
- name: "websubhub-consolidator-svc-cm"
+ name: "{{ .Release.Name }}-config"Ensure the ConfigMap template uses the same naming convention.
🤖 Prompt for AI Agents
In distributed/websubhub-consolidator/templates/deployment.yaml around line 71,
the ConfigMap name is hard-coded as "websubhub-consolidator-svc-cm"; change it
to use Helm templating (for example {{ .Release.Name }}-config or {{ include
"chart.fullname" . }}-config) so the chart is reusable, and update any other
references in this template (volumes, envFrom, etc.) to use the exact same
template expression to keep names consistent.
| apiVersion: v1 | ||
| kind: ConfigMap | ||
| metadata: | ||
| name: websubhub-svc-cm |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use Release.Name in ConfigMap name for uniqueness.
The hardcoded ConfigMap name websubhub-svc-cm will cause conflicts if multiple releases of this chart are deployed in the same namespace. Use {{ .Release.Name }}-cm to ensure uniqueness.
🔎 Proposed fix
- name: websubhub-svc-cm
+ name: {{ .Release.Name }}-cmNote: Ensure that any references to this ConfigMap in StatefulSet/Deployment templates are also updated to use the same naming pattern.
📝 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.
| name: websubhub-svc-cm | |
| name: {{ .Release.Name }}-cm |
🤖 Prompt for AI Agents
In distributed/websubhub/templates/conf.yaml around line 22, the ConfigMap name
is hardcoded as "websubhub-svc-cm" which can collide across releases; change it
to use the release name (e.g. "{{ .Release.Name }}-cm") so each release gets a
unique ConfigMap name, and update any StatefulSet/Deployment/other templates
that reference this ConfigMap to use the same "{{ .Release.Name }}-cm" pattern
so references remain consistent.
| volumes: | ||
| - name: "config-config-map-volume" | ||
| configMap: | ||
| name: "websubhub-svc-cm" |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Hard-coded ConfigMap name should use Helm templating.
The ConfigMap name "websubhub-svc-cm" is hard-coded. This prevents chart reusability and could cause conflicts when installing multiple releases. Use {{ .Release.Name }}-config or similar pattern.
🔎 Use templated ConfigMap name
configMap:
- name: "websubhub-svc-cm"
+ name: "{{ .Release.Name }}-config"Note: You'll also need to update the ConfigMap template (templates/conf.yaml) to use the same naming convention.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In distributed/websubhub/templates/statefulset.yaml around line 83, the
ConfigMap name is hard-coded as "websubhub-svc-cm"; replace it with a Helm
template expression (for example {{ .Release.Name }}-config or the chart's
defined name template) so the release-specific name is used, and update the
ConfigMap resource template (templates/conf.yaml) to emit the exact same
templated name to keep references consistent across the chart.
| config: | ||
| server: | ||
| port: 9000 | ||
| id: "websubhub-1" |
There was a problem hiding this comment.
Hard-coded server ID conflicts with StatefulSet replicas.
The server.id is hard-coded as "websubhub-1", but the deployment uses 2 replicas (line 2). In a StatefulSet, each pod should have a unique identifier. Consider removing this from values and relying on the SERVER_ID environment variable (set from metadata.name in statefulset.yaml line 69-72) instead.
🔎 Suggested approach
Remove the hard-coded id from values.yaml and update Config.toml to use the SERVER_ID environment variable, or make it a template that generates unique IDs per replica.
🤖 Prompt for AI Agents
In distributed/websubhub/values.yaml around line 17, the server id is hard-coded
as "websubhub-1" which will collide with StatefulSet replicas; remove this
hard-coded id from values.yaml and update Config.toml (or its template) to read
SERVER_ID from the environment (or use a Helm template that derives a unique id
per replica) so each pod gets a unique server.id supplied by the statefulset
metadata.name-based SERVER_ID env var referenced in statefulset.yaml lines
~69-72.
| username: "admin" | ||
| password: "password" |
There was a problem hiding this comment.
Credentials should use Kubernetes Secrets, not plaintext values.
Hard-coded credentials in values files are a security risk. These should reference Kubernetes Secrets instead, especially for production deployments.
🔎 Recommended approach
- Create a Secret object in a separate template (e.g.,
templates/secret.yaml) - Reference the secret in the StatefulSet via
valueFrom.secretKeyRef - Update Config.toml template to read from environment variables instead of directly from values
Example Secret reference in StatefulSet:
env:
- name: SOLACE_USERNAME
valueFrom:
secretKeyRef:
name: websubhub-solace-credentials
key: username
- name: SOLACE_PASSWORD
valueFrom:
secretKeyRef:
name: websubhub-solace-credentials
key: passwordAlso applies to: 37-38
🤖 Prompt for AI Agents
In distributed/websubhub/values.yaml around lines 29-30 (and similarly lines
37-38) the chart currently exposes plaintext credentials; replace this by
creating a templates/secret.yaml that contains the username/password as a
Kubernetes Secret, remove the hard-coded username/password from values.yaml (or
replace them with a secret name reference), update the StatefulSet/Deployment
template to populate env vars using valueFrom.secretKeyRef (pointing to the new
secret and keys), and modify the Config.toml template to read credentials from
those environment variables instead of embedding values directly.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @distribution/websubhub-consolidator/confs/Config.toml:
- Around line 24-25: The Config.toml currently injects admin credentials
directly via auth.username and auth.password using {{
.Values.deployment.config.store.solace.admin.auth.username }}; move these into
Kubernetes Secrets and reference them via environment variables instead. Update
the Helm chart to create or reference a Secret containing the admin
username/password (from .Values.deployment.secrets or similar), inject them into
the Pod spec as env vars using secretKeyRef, and change the Config.toml template
to read auth.username and auth.password from those env vars (or render via
tpl/envsubst at deploy time) so no admin credentials are directly templated into
the ConfigMap.
- Around line 15-16: The Config.toml currently inlines credentials via
auth.username and auth.password; move these values into a Kubernetes Secret
(e.g., {{ .Release.Name }}-solace-credentials with keys auth-username and
auth-password), update the Deployment to inject them as environment variables
(e.g., SOLACE_AUTH_USERNAME and SOLACE_AUTH_PASSWORD via secretKeyRef), and
modify Config.toml (or the app’s config loader) to read auth.username and
auth.password from those environment variables instead of templated values.
In @distribution/websubhub/confs/Config.toml:
- Around line 23-24: The Config.toml currently injects admin credentials
directly into auth.username and auth.password; instead create a Kubernetes
Secret containing the Solace admin credentials (e.g., keys solace-admin-username
and solace-admin-password), update the Helm charts/manifests to populate
container environment variables (e.g., SOLACE_ADMIN_USERNAME and
SOLACE_ADMIN_PASSWORD) from that Secret, and change Config.toml to read those
values from environment variables rather than templating the raw values into
auth.username and auth.password; also ensure any existing references to the
templated values are removed and deployments/statefulsets use envFrom or env
entries to source the Secret.
In @distribution/websubhub/templates/ingress.yaml:
- Around line 18-20: Replace hardcoded ingressClassName and host in the Ingress
template with values-driven keys and add TLS and annotations support: read
ingress.className -> replace ingressClassName, ingress.host -> replace host
under rules, ingress.annotations -> add to metadata.annotations, and if
ingress.tls.enabled is true emit a tls section referencing
ingress.tls.secretName; ensure the template honors ingress.enabled so the
resource is only created when enabled and document the new values.yaml keys
(ingress.enabled, className, host, tls.enabled, tls.secretName, annotations).
🧹 Nitpick comments (5)
distribution/websubhub/templates/conf.yaml (1)
23-23: Remove extra space before colon.Line 23 has an extra space before the colon in
namespace :. Standard YAML formatting uses no space before colons.♻️ Proposed fix
- namespace : {{ .Release.Namespace }} + namespace: {{ .Release.Namespace }}distribution/websubhub-consolidator/templates/service.yaml (1)
27-30: Consider adding a name to the service port.While optional, naming service ports is a Kubernetes best practice that improves clarity for service discovery, debugging, and when multiple ports are exposed.
♻️ Proposed fix
ports: - protocol: TCP + name: http port: {{ .Values.deployment.config.server.port }} targetPort: {{ .Values.deployment.config.server.port }}distribution/websubhub/templates/service.yaml (1)
27-30: Consider adding a name to the service port.While optional, naming service ports is a Kubernetes best practice that improves clarity for service discovery, debugging, and when multiple ports are exposed.
♻️ Proposed fix
ports: - protocol: TCP + name: http port: {{ .Values.deployment.config.server.port }} targetPort: {{ .Values.deployment.config.server.port }}distribution/websubhub-consolidator/templates/conf.yaml (1)
23-23: Remove extra space before colon.Line 23 has an extra space before the colon in
namespace :. Standard YAML formatting uses no space before colons.♻️ Proposed fix
- namespace : {{ .Release.Namespace }} + namespace: {{ .Release.Namespace }}distribution/websubhub/confs/Config.toml (1)
34-34: Consider simplifying array templating.The inline range logic for
statusCodesis correct but complex. Consider using a more readable approach if TOML parsing supports JSON arrays.Alternative approach
If the TOML parser supports it:
-statusCodes = [{{ range $index, $code := .Values.deployment.config.delivery.retry.statusCodes }}{{if $index}}, {{end}}{{ $code }}{{end}}] +statusCodes = {{ .Values.deployment.config.delivery.retry.statusCodes | toJson }}Otherwise, the current implementation is acceptable.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
distribution/websubhub-consolidator/Chart.yamldistribution/websubhub-consolidator/confs/Config.tomldistribution/websubhub-consolidator/templates/conf.yamldistribution/websubhub-consolidator/templates/deployment.yamldistribution/websubhub-consolidator/templates/service.yamldistribution/websubhub-consolidator/values.yamldistribution/websubhub/Chart.yamldistribution/websubhub/confs/Config.tomldistribution/websubhub/templates/conf.yamldistribution/websubhub/templates/ingress.yamldistribution/websubhub/templates/service.yamldistribution/websubhub/templates/statefulset.yamldistribution/websubhub/values.yaml
✅ Files skipped from review due to trivial changes (3)
- distribution/websubhub-consolidator/values.yaml
- distribution/websubhub/Chart.yaml
- distribution/websubhub-consolidator/Chart.yaml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
distribution/websubhub-consolidator/templates/deployment.yaml
[error] 22-22: syntax error: expected , but found ''
(syntax)
distribution/websubhub/templates/service.yaml
[error] 22-22: syntax error: expected , but found ''
(syntax)
distribution/websubhub/templates/conf.yaml
[warning] 23-23: too many spaces before colon
(colons)
[error] 22-22: syntax error: expected , but found ''
(syntax)
distribution/websubhub/templates/statefulset.yaml
[error] 22-22: syntax error: expected , but found ''
(syntax)
distribution/websubhub/templates/ingress.yaml
[error] 15-15: syntax error: expected , but found ''
(syntax)
distribution/websubhub-consolidator/templates/conf.yaml
[warning] 23-23: too many spaces before colon
(colons)
[error] 22-22: syntax error: expected , but found ''
(syntax)
distribution/websubhub-consolidator/templates/service.yaml
[error] 22-22: syntax error: expected , but found ''
(syntax)
🔇 Additional comments (7)
distribution/websubhub/values.yaml (2)
17-17: Verify the use of a pre-release image tag.The image tag
1.0.0-M1-RC6appears to be a milestone release candidate. Ensure this pre-release version is intentional and appropriate for your deployment environment.
1-10: Verify license header inconsistency.The values.yaml file uses the WSO2 Commercial License header, while the template files (conf.yaml, service.yaml) use Apache License 2.0. Confirm this licensing difference is intentional.
distribution/websubhub-consolidator/templates/deployment.yaml (2)
60-71: LGTM: Configuration management properly implemented.The environment variable and volume mount configuration correctly wires the ConfigMap to the application's expected configuration path.
36-66: Missing resource limits and requests.The Deployment lacks resource specifications, which are essential for production deployments to ensure cluster stability and proper pod scheduling.
Likely an incorrect or invalid review comment.
distribution/websubhub/templates/statefulset.yaml (2)
61-67: Resource limits are now properly configured.The resource requests and limits are correctly implemented and uncommented. This resolves the concern from the previous review.
Note: The past review comment indicated these were commented out, but they are now active in the current code.
68-83: LGTM: StatefulSet configuration properly implemented.The environment variables and volume configuration are correctly set up. Using
metadata.nameforSERVER_IDis appropriate for StatefulSet pod identity tracking.distribution/websubhub/confs/Config.toml (1)
14-15: Credentials should use Kubernetes Secrets and environment variables.Credentials are currently templated into the ConfigMap, which is not secure. Move these to Kubernetes Secrets and reference them via environment variables instead.
Likely an incorrect or invalid review comment.
| auth.username = "{{ .Values.deployment.config.store.solace.auth.username }}" | ||
| auth.password = "{{ .Values.deployment.config.store.solace.auth.password }}" |
There was a problem hiding this comment.
Credentials should use Kubernetes Secrets and environment variables.
Authentication credentials are templated directly into the ConfigMap, exposing them in plain text. Move credentials to Kubernetes Secrets and reference them via environment variables in the Deployment template.
🔐 Recommended approach
- Create a Secret in the Helm chart:
apiVersion: v1
kind: Secret
metadata:
name: {{ .Release.Name }}-solace-credentials
type: Opaque
stringData:
auth-username: {{ .Values.deployment.config.store.solace.auth.username }}
auth-password: {{ .Values.deployment.config.store.solace.auth.password }}- Update the Deployment to inject env vars from the Secret:
env:
- name: SOLACE_AUTH_USERNAME
valueFrom:
secretKeyRef:
name: {{ .Release.Name }}-solace-credentials
key: auth-username
- name: SOLACE_AUTH_PASSWORD
valueFrom:
secretKeyRef:
name: {{ .Release.Name }}-solace-credentials
key: auth-password- Update Config.toml to read from environment variables (syntax depends on application framework).
🤖 Prompt for AI Agents
In @distribution/websubhub-consolidator/confs/Config.toml around lines 15 - 16,
The Config.toml currently inlines credentials via auth.username and
auth.password; move these values into a Kubernetes Secret (e.g., {{
.Release.Name }}-solace-credentials with keys auth-username and auth-password),
update the Deployment to inject them as environment variables (e.g.,
SOLACE_AUTH_USERNAME and SOLACE_AUTH_PASSWORD via secretKeyRef), and modify
Config.toml (or the app’s config loader) to read auth.username and auth.password
from those environment variables instead of templated values.
| auth.username = "{{ .Values.deployment.config.store.solace.admin.auth.username }}" | ||
| auth.password = "{{ .Values.deployment.config.store.solace.admin.auth.password }}" |
There was a problem hiding this comment.
Admin credentials should use Kubernetes Secrets and environment variables.
Admin authentication credentials are templated into the ConfigMap. Apply the same Secret-based approach as recommended for the regular auth credentials.
🤖 Prompt for AI Agents
In @distribution/websubhub-consolidator/confs/Config.toml around lines 24 - 25,
The Config.toml currently injects admin credentials directly via auth.username
and auth.password using {{
.Values.deployment.config.store.solace.admin.auth.username }}; move these into
Kubernetes Secrets and reference them via environment variables instead. Update
the Helm chart to create or reference a Secret containing the admin
username/password (from .Values.deployment.secrets or similar), inject them into
the Pod spec as env vars using secretKeyRef, and change the Config.toml template
to read auth.username and auth.password from those env vars (or render via
tpl/envsubst at deploy time) so no admin credentials are directly templated into
the ConfigMap.
| auth.username = "{{ .Values.deployment.config.store.solace.admin.auth.username }}" | ||
| auth.password = "{{ .Values.deployment.config.store.solace.admin.auth.password }}" |
There was a problem hiding this comment.
Admin credentials should use Kubernetes Secrets and environment variables.
Admin credentials are currently templated into the ConfigMap. Move these to Kubernetes Secrets and reference them via environment variables for better security.
🤖 Prompt for AI Agents
In @distribution/websubhub/confs/Config.toml around lines 23 - 24, The
Config.toml currently injects admin credentials directly into auth.username and
auth.password; instead create a Kubernetes Secret containing the Solace admin
credentials (e.g., keys solace-admin-username and solace-admin-password), update
the Helm charts/manifests to populate container environment variables (e.g.,
SOLACE_ADMIN_USERNAME and SOLACE_ADMIN_PASSWORD) from that Secret, and change
Config.toml to read those values from environment variables rather than
templating the raw values into auth.username and auth.password; also ensure any
existing references to the templated values are removed and
deployments/statefulsets use envFrom or env entries to source the Secret.
| ingressClassName: traefik | ||
| rules: | ||
| - host: "websubhub.dev" |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Make ingress configuration values configurable and add TLS support.
The ingressClassName and host are hardcoded, limiting chart reusability across environments. Additionally, there's no TLS configuration, which is essential for production deployments.
♻️ Proposed configuration improvements
Add to values.yaml:
ingress:
enabled: true
className: "traefik"
host: "websubhub.dev"
tls:
enabled: false
secretName: ""
annotations: {}Update the Ingress template:
+{{- if .Values.ingress.enabled }}
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
name: {{ .Release.Name }}-ingress
namespace: {{ .Release.Namespace }}
+ {{- with .Values.ingress.annotations }}
+ annotations:
+ {{- toYaml . | nindent 4 }}
+ {{- end }}
spec:
- ingressClassName: traefik
+ ingressClassName: {{ .Values.ingress.className }}
+ {{- if .Values.ingress.tls.enabled }}
+ tls:
+ - hosts:
+ - {{ .Values.ingress.host }}
+ secretName: {{ .Values.ingress.tls.secretName }}
+ {{- end }}
rules:
- - host: "websubhub.dev"
+ - host: {{ .Values.ingress.host | quote }}
http:
paths:
- path: /
pathType: Prefix
backend:
service:
name: {{ .Release.Name }}-service
port:
number: {{ .Values.deployment.config.server.port }}
+{{- end }}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @distribution/websubhub/templates/ingress.yaml around lines 18 - 20, Replace
hardcoded ingressClassName and host in the Ingress template with values-driven
keys and add TLS and annotations support: read ingress.className -> replace
ingressClassName, ingress.host -> replace host under rules, ingress.annotations
-> add to metadata.annotations, and if ingress.tls.enabled is true emit a tls
section referencing ingress.tls.secretName; ensure the template honors
ingress.enabled so the resource is only created when enabled and document the
new values.yaml keys (ingress.enabled, className, host, tls.enabled,
tls.secretName, annotations).
1 participant
Purpose
Part of: wso2/product-integrator-websubhub#3
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.