Conversation
|
Caution Review failedThe pull request is closed. WalkthroughA new Apex Fusion Helm chart (chart metadata, values, README) and templates (helpers, StatefulSet, Services, ConfigMaps, ServiceAccount, PodMonitor) were added under Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Helm as Helm (chart)
participant Templates as Templates (_helpers.tpl, *.yaml)
participant K8s as Kubernetes API
Dev->>Helm: helm install / helm template (apex-fusion)
Helm->>Templates: evaluate helpers & templates with values
Templates->>Templates: resolve names, labels, configmaps, checksums
Templates->>K8s: render manifests (StatefulSet, Services, ConfigMaps, SA, PodMonitor)
K8s-->>Dev: manifests / apply results
note right of Templates `#ccffcc`: New/changed interactions:\n- configuration & proxy ConfigMaps\n- optional proxy sidecar\n- headless Service and PodMonitor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (8)
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: 4
🧹 Nitpick comments (1)
extensions/apex-fusion/README.md (1)
26-27: Document the path to add mainnet support.The README restricts apex-fusion to testnet networks and defers mainnet support to a future release. Consider adding a brief note on the required steps to enable mainnet (e.g., "mainnet support will be added by updating the proxy configuration and node parameters").
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.github/workflows/check_extensions.yml(1 hunks)extensions/apex-fusion/Chart.yaml(1 hunks)extensions/apex-fusion/README.md(1 hunks)extensions/apex-fusion/templates/_helpers.tpl(1 hunks)extensions/apex-fusion/templates/configmap-configuration.yaml(1 hunks)extensions/apex-fusion/templates/configmap-proxy.yaml(1 hunks)extensions/apex-fusion/templates/podmonitor.yaml(1 hunks)extensions/apex-fusion/templates/service-headless.yaml(1 hunks)extensions/apex-fusion/templates/service.yaml(1 hunks)extensions/apex-fusion/templates/serviceaccount.yaml(1 hunks)extensions/apex-fusion/templates/statefulset.yaml(1 hunks)extensions/apex-fusion/values.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
extensions/apex-fusion/templates/serviceaccount.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
extensions/apex-fusion/templates/configmap-proxy.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
extensions/apex-fusion/templates/podmonitor.yaml
[error] 8-8: syntax error: could not find expected ':'
(syntax)
extensions/apex-fusion/templates/service.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
extensions/apex-fusion/templates/service-headless.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
extensions/apex-fusion/templates/statefulset.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
extensions/apex-fusion/templates/configmap-configuration.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
🔇 Additional comments (11)
.github/workflows/check_extensions.yml (1)
45-47: Workflow integration looks good.The matrix entry follows the established pattern for extensions and will trigger validation on the new apex-fusion chart.
extensions/apex-fusion/Chart.yaml (1)
1-12: Chart metadata is well-formed.Structure and versioning are appropriate for a new extension. The RC1 pre-release version signals that this chart may receive updates.
extensions/apex-fusion/values.yaml (2)
5-6: Verify the container image repository.The image repository
ghcr.io/blinklabs-io/cardano-nodeappears to be a cardano-node image rather than an Apex-Fusion-specific image. Clarify whether:
- Apex Fusion is built on top of cardano-node (expected)
- The image repository is correct for this extension
- Different networks (vector-testnet, prime-testnet, prime-mainnet) use the same image
60-62: Clarify network magic and per-network configuration.The chart specifies a single hardcoded
networkMagic: 1forvector-testnet. The README mentions three supported networks (vector-testnet, prime-testnet, prime-mainnet), which typically have different network magic numbers. Verify:
- Whether all three networks use magic 1
- Whether different network environments require different magic values
- If so, consider documenting or providing preset values for each network
extensions/apex-fusion/templates/podmonitor.yaml (1)
1-14: Template structure is sound, but helpers must be verified.The PodMonitor template uses correct Helm syntax and follows consistent labeling patterns. However, validation requires confirming that the referenced template helpers (
vector-node.fullname,vector-node.labels,vector-node.selectorLabels) are defined intemplates/_helpers.tpl.extensions/apex-fusion/templates/serviceaccount.yaml (1)
1-12: ServiceAccount template follows Helm best practices.Conditional rendering allows users to supply their own ServiceAccount when desired. However, verify that:
- Template helpers (
vector-node.serviceAccountName,vector-node.labels) are defined- The ServiceAccount permissions (via Role/RoleBinding) are adequately scoped elsewhere in the chart (not visible in provided files)
extensions/apex-fusion/templates/configmap-proxy.yaml (1)
1-13: Proxy ConfigMap template is defensive and well-structured.The template properly uses
required()to validate mandatory fields, has appropriate conditional gating, and handles indentation correctly. Verify thatvector-node.proxyConfigNameis defined intemplates/_helpers.tpl.extensions/apex-fusion/templates/service.yaml (1)
1-53: Service template design is idiomatic and extensible.The template correctly uses local variables for clarity, gates optional fields, and supports multiple service types. Port definitions align with values.yaml. However, verify:
- All helper functions (
vector-node.fullname,vector-node.labels,vector-node.selectorLabels) are defined- Selector labels match the pod labels in
templates/statefulset.yaml(not provided in review)- Port definitions match containerPort names in the StatefulSet
extensions/apex-fusion/templates/configmap-configuration.yaml (1)
3-6: Good validation logic for ConfigMap creation.The validation checks ensure that the ConfigMap name resolves correctly and that required files are provided, preventing runtime errors.
extensions/apex-fusion/templates/statefulset.yaml (2)
6-13: Good practice: checksums for ConfigMap changes trigger pod restarts.The checksum annotations for proxy and configuration ConfigMaps ensure that pods are restarted when configuration changes, which is the correct pattern for propagating config updates.
71-94: Cardano-specific configuration is correct for Apex Fusion VECTOR chain.The Cardano-specific references in the template are appropriate. Apex Fusion's VECTOR chain is explicitly built on Cardano node technology—the vector-node software is a fork of the Cardano node codebase. The chart correctly uses the cardano-node image (
ghcr.io/blinklabs-io/cardano-node:10.1.4), and the configuration paths and environment variables (CARDANO_NETWORK,CARDANO_NODE_SOCKET_PATH) properly reflect this architecture. No adaptation or changes are needed.Likely an incorrect or invalid review comment.
| {{- $fullName := printf "%s-headless" (include "vector-node.fullname" .) -}} | ||
| apiVersion: v1 | ||
| kind: Service | ||
| metadata: | ||
| name: {{ $fullName }} | ||
| labels: | ||
| {{- include "vector-node.labels" . | nindent 4 }} | ||
| spec: | ||
| clusterIP: None | ||
| publishNotReadyAddresses: true | ||
| selector: | ||
| {{- include "vector-node.selectorLabels" . | nindent 4 }} |
There was a problem hiding this comment.
Fix naming inconsistency: template helpers reference "vector-node" but chart is "apex-fusion".
All helper template references use the vector-node.* prefix, but this chart is named apex-fusion. This naming inconsistency will cause confusion and maintenance issues.
Update all helper references to use a consistent prefix matching the chart name. For example:
vector-node.fullname→apex-fusion.fullnamevector-node.labels→apex-fusion.labelsvector-node.selectorLabels→apex-fusion.selectorLabels
The helper definitions in _helpers.tpl will also need to be renamed accordingly.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
🤖 Prompt for AI Agents
In extensions/apex-fusion/templates/service-headless.yaml around lines 1 to 12,
template helpers reference the incorrect prefix "vector-node" instead of the
chart name "apex-fusion"; update all helper calls in this file to use the
"apex-fusion" prefix (e.g., apex-fusion.fullname, apex-fusion.labels,
apex-fusion.selectorLabels) and ensure the corresponding helper function names
in templates/_helpers.tpl are renamed/mapped the same so names are consistent
across the chart.
1 participant
Summary by CodeRabbit
New Features
Documentation
Chores