HYPERFLEET-313: Restructure chart with base + overlay pattern for multi-cloud #1
Conversation
- Add Chart.yaml with file:// dependencies for API, Adapter, Sentinel - Add values.yaml with RabbitMQ as default broker - Add values-dev.yaml.example for developer customization - Add values-pubsub.yaml.example for GKE with Workload Identity - Add global broker configuration (rabbitmq/googlepubsub) - Add RabbitMQ deployment template for local development - Add helper templates for global image overrides - Update README with Pub/Sub quick start and configuration docs - Add .gitignore for personal dev files 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
WalkthroughThe changes add a Helm-based chart structure for HyperFleet using a base+overlay pattern, including a deprecated root Chart.yaml that points to cloud-specific overlays. A new charts/hyperfleet-base chart provides metadata, helpers, values, NOTES, and templates (including an optional in-cluster RabbitMQ Service and Deployment). A hyperfleet-gcp overlay chart and multiple values overlays/examples (Pub/Sub and RabbitMQ) are added. A Makefile with targets for deps, lint, template, test, install/upgrade/uninstall, and cleanup is introduced. README.md is expanded with usage, migration, and configuration guidance. .gitignore entries for Helm files were added. Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
Take another look into this PR. Each deployment will use different adapter charts, should we have charts per cloud providers? |
Introduces the base chart containing cloud-agnostic components: - HyperFleet API (cluster lifecycle management) - Sentinel (resource polling and event publishing) - Landing Zone adapter (namespace creation) - RabbitMQ (optional, for development) Includes broker abstraction layer supporting RabbitMQ and Google Pub/Sub. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
GCP-specific overlay that depends on hyperfleet-base and adds: - validation-gcp adapter - Google Pub/Sub as default broker - Workload Identity configuration Includes values-dev.yaml for RabbitMQ-based development. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- examples/gcp-dev/values.yaml: GCP + RabbitMQ for development - examples/gcp-prod/values.yaml: GCP + Pub/Sub for production - Makefile: targets for deps, lint, template, install, clean 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Update README with new base + overlay chart structure - Add deprecation notice to root Chart.yaml - Update .gitignore for new directory structure - Remove old root-level templates and values files Migration: helm install hyperfleet charts/hyperfleet-gcp -f values.yaml 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
ciaranRoche
changed the title
Each component (API, Sentinel, adapters) has independent release cycles and different image tags. Global tag override doesn't make sense. - Remove global.image.tag from values - Remove hyperfleet-base.imageTag helper - Keep global.image.registry (useful for custom registry) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI Agents
In @Chart.yaml:
- Around line 30-53: The Chart.yaml uses git+https:// repository URLs for
dependencies (hyperfleet-api, sentinel, landing-zone, validation-gcp) which Helm
does not natively support and also pins to the main branch; replace these with
proper, supported sources by publishing those charts to an HTTP(S) Helm chart
repo or an OCI registry and update the repository fields to those URLs, or
switch to file:// paths for local development, or explicitly document and
require the helm-git plugin if you must keep git sources; also change branch
refs to semantic version tags (or exact chart versions) instead of ref=main to
ensure reproducible dependency resolution.
In @charts/hyperfleet-base/templates/_helpers.tpl:
- Around line 58-66: The helper "hyperfleet-base.rabbitmqUrl" currently embeds
.Values.rabbitmq.auth.username and .Values.rabbitmq.auth.password directly into
the returned URL, which causes NOTES.txt to print credentials; change the helper
so it does not output raw credentials—either return a URL with the password
masked (e.g., replace password with "***" or "<redacted>") or return a URL that
references the Secret name (not the secret value) so consumers know to read the
Secret; then update NOTES.txt to use the new helper output and/or add a clear
warning that credentials are stored in a Kubernetes Secret (and will not be
shown), referencing the helper name "hyperfleet-base.rabbitmqUrl" and the
.Values.rabbitmq.auth.* keys and the Secret that holds them.
In @charts/hyperfleet-base/templates/rabbitmq.yaml:
- Around line 59-62: The RABBITMQ_DEFAULT_USER and RABBITMQ_DEFAULT_PASS env
entries currently inject plaintext credentials; instead create a Kubernetes
Secret (e.g., template name rabbitmq-secret.yaml) that contains the username and
password (stringData) and then update the Deployment in
charts/hyperfleet-base/templates/rabbitmq.yaml to remove the hardcoded value
fields and reference the Secret via env.valueFrom.secretKeyRef (or
envFrom.secretRef) for RABBITMQ_DEFAULT_USER and RABBITMQ_DEFAULT_PASS so the
pod reads creds from the created Secret.
- Around line 27-47: The manifest currently creates a Deployment named {{
.Release.Name }}-rabbitmq with replicas: 1 and no volume mounts; replace this
with a StatefulSet resource (keep the metadata.name and labels) that declares a
volumeClaimTemplates entry to provision a PersistentVolumeClaim for RabbitMQ’s
data directory (/var/lib/rabbitmq), mount that PVC into the pod template spec,
and increase replicas to 3 for HA; also ensure the container startup/config in
the template enables RabbitMQ clustering (or switch to the Bitnami RabbitMQ
chart if preferred) so state and clustering are preserved across restarts and
reschedules.
- Around line 48-64: The rabbitmq container spec (container name "rabbitmq",
image field using .Values.rabbitmq.image) lacks a securityContext, explicit
imagePullPolicy, and resource requests; update the container entry to add
imagePullPolicy: IfNotPresent (or from values), add a securityContext with
runAsNonRoot: true, runAsUser (non-root UID) and readOnlyRootFilesystem: true
where compatible, and ensure .Values.rabbitmq.resources includes both requests
and limits (or transform .Values.rabbitmq.resources via toYaml to include
requests) so the Pod has resource requests for scheduling.
In @examples/gcp-prod/values.yaml:
- Around line 39-42: Replace the hardcoded secret in
database.postgresql.password: remove the literal "CHANGE_ME_IN_PRODUCTION" value
and either (a) set the key to a failing placeholder (e.g., a clearly invalid
token like "__REQUIRED_CHANGE__") so deployments break until replaced, (b)
change it to reference an external secret/source used by the chart (e.g., a
secretRef/lookup into the release-managed Kubernetes Secret or external secret
manager) or (c) omit the password key entirely and document that the operator
must supply a secret; locate and update the database.postgresql.password entry
and adjust README/values documentation to instruct users how to provide the
password via secrets.
- Around line 94-97: The production example currently sets
validation-gcp.deploymentMode to "dummy" which is inconsistent with production;
either change validation-gcp.deploymentMode from "dummy" to "real" to reflect
production usage (so the adapter will be used when validation-gcp-adapter.yaml
exists), or if keeping "dummy" is intentional, replace the terse comment with a
clear explanatory comment referencing why dummy is used, the missing
validation-gcp-adapter.yaml, and the exact condition/timeline for switching to
"real" mode.
In @Makefile:
- Around line 67-71: The Makefile targets (e.g., lint-gcp) reference a missing
charts/hyperfleet-gcp/values-dev.yaml (and it’s listed in .gitignore), so either
add a committed template/default values-dev.yaml into charts/hyperfleet-gcp, or
change the Makefile targets to fall back to an existing example (e.g.,
examples/gcp-prod/values.yaml) when values-dev.yaml is absent, or add a
documented prep step in README/CONTRIBUTING that instructs users to
generate/copy charts/hyperfleet-gcp/values-dev.yaml before running
lint-gcp/deploy targets; update the lint-gcp, and any other Makefile targets
referencing values-dev.yaml to reflect the chosen solution and include
references to charts/hyperfleet-gcp/values-dev.yaml and .gitignore so reviewers
can verify the fix.
🧹 Nitpick comments (13)
README.md (1)
9-17: Specify language for fenced code block.The directory structure should have a language identifier for proper syntax highlighting and rendering.
🔎 Proposed fix
-``` +```text hyperfleet-chart/ charts/ hyperfleet-base/ # Core platform (API, Sentinel, Landing Zone)Based on static analysis hint from markdownlint-cli2.
charts/hyperfleet-gcp/values.yaml (1)
79-83: Clarify deploymentMode "dummy" as default.The
deploymentModeis set to "dummy" by default with a comment suggesting "real" mode requires additional files. Consider whether this default is appropriate for a cloud overlay that users might deploy directly, or if it should prompt users to explicitly choose their deployment mode.This could be confusing for users who expect production-ready defaults in a GCP overlay chart. Consider either:
- Setting "real" as the default with clear prerequisites in documentation
- Adding a comment explaining that "dummy" is for initial testing/validation only
Makefile (2)
122-128: Consider requiring values file customization for install target.The
installtarget usesvalues-dev.yamldirectly without prompting users to customize it. Consider whether this should follow the same pattern asinstall-prod(lines 130-137) with a warning, or if it's intended for truly automated development deployments.If this is for development automation, the current approach is fine. If users should customize it, consider adding a check:
🔎 Proposed enhancement
install: deps-gcp ## Install hyperfleet-gcp to cluster @echo "$(GREEN)Installing hyperfleet-gcp...$(NC)" + @if [ ! -f "charts/hyperfleet-gcp/values-dev.yaml" ]; then \ + echo "$(RED)ERROR: values-dev.yaml not found$(NC)"; \ + echo "Copy and customize: cp charts/hyperfleet-gcp/values.yaml charts/hyperfleet-gcp/values-dev.yaml"; \ + exit 1; \ + fi helm install $(RELEASE_NAME) charts/hyperfleet-gcp \
16-20: Optional: Add conventional "all" phony target.Static analysis suggests adding an "all" phony target as a convention. While not strictly required, it's a common Makefile practice to have a default target that runs a meaningful set of operations.
🔎 Proposed addition
.PHONY: help deps lint template test clean \ + all \ deps-base deps-gcp \ lint-base lint-gcp \ template-base template-gcp template-gcp-dev \ check-helm-git +##@ General + +all: deps lint test ## Run full build pipeline (deps + lint + test) + # Default valuesBased on static analysis hint from checkmake.
charts/hyperfleet-base/values.yaml (3)
62-62: Avoid hardcoded passwords in version-controlled files.Even for development defaults, hardcoded passwords in values files can be accidentally used in production or leak into logs/configs. Consider using a generated value or requiring explicit override.
🔎 Suggested approach
postgresql: enabled: true - password: "dev-password-change-me" + password: "" # Required: Set via --set or values overrideAlternatively, document that this must be overridden and consider adding a template validation to fail if password is empty in production contexts.
137-139: RabbitMQ credentials also hardcoded.Same concern as the PostgreSQL password. These credentials should be overridden and not committed with default values that could leak.
🔎 Suggested approach
auth: - username: hyperfleet - password: hyperfleet-dev-password + username: "" # Required: Set via --set or values override + password: "" # Required: Set via --set or values override
54-57: Usinglatestimage tags is discouraged.Using
latesttags makes deployments non-reproducible and can cause unexpected behavior when images are updated. Consider using explicit version tags or SHA digests.🔎 Suggested approach
image: registry: quay.io/openshift-hyperfleet repository: hyperfleet-api - tag: latest + tag: "0.1.0" # Pin to specific versionApply similar changes to sentinel and landing-zone image configurations.
Also applies to: 74-77, 104-107
examples/gcp-dev/values.yaml (1)
50-52: Same hardcoded password concern in example file.Consider using environment variable substitution or a reference to a secret instead of hardcoding, even in examples.
charts/hyperfleet-gcp/templates/NOTES.txt (1)
25-41: Consider templating the PROJECT placeholder in gcloud commands.The
PROJECTplaceholder in the Workload Identity commands could be templated using the configured projectId for a better user experience.🔎 Suggested improvement
# Sentinel gcloud iam service-accounts add-iam-policy-binding \ - sentinel@PROJECT.iam.gserviceaccount.com \ + sentinel@{{ .Values.base.global.broker.googlepubsub.projectId }}.iam.gserviceaccount.com \ --role roles/iam.workloadIdentityUser \ - --member "serviceAccount:PROJECT.svc.id.goog[{{ .Release.Namespace }}/sentinel]" + --member "serviceAccount:{{ .Values.base.global.broker.googlepubsub.projectId }}.svc.id.goog[{{ .Release.Namespace }}/sentinel]"Apply similar changes to other gcloud commands. Add a guard to only show this section when projectId is set.
charts/hyperfleet-base/Chart.yaml (1)
22-37: All dependencies use unpinnedref=mainreferences.Same reproducibility concern as noted for hyperfleet-gcp. For production releases, pin all dependencies to specific tags or commit SHAs.
Also note: sentinel is at version
0.1.0while other dependencies are1.0.0. Ensure this version mismatch is intentional and document if sentinel is in earlier development stages.🔎 Suggested approach for pinned refs
- name: hyperfleet-api version: "1.0.0" - repository: "git+https://github.com/openshift-hyperfleet/hyperfleet-api@charts?ref=main" + repository: "git+https://github.com/openshift-hyperfleet/hyperfleet-api@charts?ref=v1.0.0" condition: hyperfleet-api.enabled - name: sentinel version: "0.1.0" - repository: "git+https://github.com/openshift-hyperfleet/hyperfleet-sentinel@deployments/helm/sentinel?ref=main" + repository: "git+https://github.com/openshift-hyperfleet/hyperfleet-sentinel@deployments/helm/sentinel?ref=v0.1.0" condition: sentinel.enabled - name: landing-zone version: "1.0.0" - repository: "git+https://github.com/openshift-hyperfleet/adapter-landing-zone@charts?ref=main" + repository: "git+https://github.com/openshift-hyperfleet/adapter-landing-zone@charts?ref=v1.0.0" condition: landing-zone.enabledcharts/hyperfleet-gcp/Chart.yaml (1)
30-32: Consider pinning git references to specific versions or commits for reproducible builds.Using
ref=mainmeanshelm dependency updatewill pull whatever is currently on the main branch, making builds non-deterministic. This pattern is used across multiple chart dependencies (hyperfleet-api, sentinel, landing-zone, validation-gcp). To improve reproducibility, pin to specific release tags or commit SHAs. Note: This requires the helm-git plugin (documented in Prerequisites) for git+ URL support.🔎 Suggested approach
- name: validation-gcp version: "1.0.0" - repository: "git+https://github.com/openshift-hyperfleet/adapter-validation-gcp@charts?ref=main" + repository: "git+https://github.com/openshift-hyperfleet/adapter-validation-gcp@charts?ref=v1.0.0" condition: validation-gcp.enabledChart.yaml (1)
1-53: Clarify purpose of deprecated chart with active dependencies.This root chart is marked deprecated (lines 1-12) and directs users to cloud-specific overlays, but still defines four active dependencies (lines 30-53). If the chart is truly deprecated and should not be used, consider either:
- Removing all dependencies and templates to prevent accidental use
- Adding a clear warning that installation will fail or is unsupported
charts/hyperfleet-base/templates/rabbitmq.yaml (1)
65-74: Improve health probes for RabbitMQ.The current
tcpSocketprobes only verify that the port is open, not that RabbitMQ is actually healthy and able to process messages. Consider using HTTP probes against the management API or exec probes withrabbitmq-diagnostics.🔎 Example HTTP-based health checks
readinessProbe: - tcpSocket: - port: amqp + httpGet: + path: /api/health/checks/local-alarms + port: management + scheme: HTTP initialDelaySeconds: 10 periodSeconds: 5 livenessProbe: - tcpSocket: - port: amqp + httpGet: + path: /api/health/checks/node-is-running + port: management + scheme: HTTP initialDelaySeconds: 30 periodSeconds: 10Note: This requires the management plugin to be enabled (default in most RabbitMQ images).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
.gitignoreChart.yamlMakefileREADME.mdcharts/hyperfleet-base/Chart.yamlcharts/hyperfleet-base/templates/NOTES.txtcharts/hyperfleet-base/templates/_helpers.tplcharts/hyperfleet-base/templates/rabbitmq.yamlcharts/hyperfleet-base/values.yamlcharts/hyperfleet-gcp/Chart.yamlcharts/hyperfleet-gcp/templates/NOTES.txtcharts/hyperfleet-gcp/values.yamlexamples/gcp-dev/values.yamlexamples/gcp-prod/values.yaml
🧰 Additional context used
🪛 checkmake (0.2.2)
Makefile
[warning] 122-122: Target body for "install" exceeds allowed length of 5 (6).
(maxbodylength)
[warning] 156-156: Target body for "clean" exceeds allowed length of 5 (8).
(maxbodylength)
[warning] 16-16: Missing required phony target "all"
(minphony)
🪛 markdownlint-cli2 (0.18.1)
README.md
9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 YAMLlint (1.37.1)
charts/hyperfleet-base/templates/rabbitmq.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
🔇 Additional comments (15)
.gitignore (1)
1-12: LGTM!The gitignore patterns appropriately exclude development-specific values files and Helm-generated artifacts (lock files, dependency charts, and packages) from version control.
charts/hyperfleet-gcp/values.yaml (1)
30-36: Verify deployment behavior with empty projectId.The
projectIdis marked as REQUIRED in the comment but defaults to an empty string. Ensure that the chart provides clear validation errors or usage instructions when users attempt to deploy without setting this value, rather than failing silently at runtime.Consider adding a validation check in the chart templates to fail fast with a helpful error message if projectId is not provided.
charts/hyperfleet-base/values.yaml (2)
43-46: AWS broker configuration placeholder looks good.The AWS SNS/SQS stub is appropriately disabled by default and provides a clear extension point for future cloud support.
22-41: Broker abstraction layer is well-structured.The broker configuration with type selection and per-broker settings provides a clean abstraction for multi-cloud deployments. The defaults favor RabbitMQ for development, which aligns with the stated goals.
examples/gcp-dev/values.yaml (2)
13-17: Placeholder conventions are clear and helpful.The
YOUR_QUAY_USERNAMEanddev-*_GIT_SHAplaceholders clearly indicate where users should substitute their own values. Good example structure.Also applies to: 28-29, 33-36, 43-44, 57-60
54-62: validation-gcp configuration looks correct.The dummy deployment mode for development and broker type alignment with base configuration is appropriate.
charts/hyperfleet-base/templates/_helpers.tpl (3)
4-6: Standard naming helpers are correctly implemented.The name truncation to 63 characters and suffix trimming follow Kubernetes naming conventions properly.
Also applies to: 11-22
34-41: Labels helper includes appropriate standard labels.The common labels follow Kubernetes recommended labeling conventions with
helm.sh/chart,app.kubernetes.io/version, andapp.kubernetes.io/part-of.
47-53: Image registry helper logic is correct.Properly prioritizes global registry over local, with appropriate nil checks.
charts/hyperfleet-base/templates/NOTES.txt (2)
17-23: Broker configuration output is informative.Good use of conditional logic to display relevant broker details based on type. Note: the RabbitMQ URL will include credentials (see related comment on _helpers.tpl).
1-15: NOTES.txt provides helpful post-install guidance.Clear component listing, status commands, and production overlay recommendations. Well-structured deployment notes.
Also applies to: 25-36
charts/hyperfleet-gcp/Chart.yaml (1)
24-27: Local file path dependency is appropriate for development.The
file://../hyperfleet-basereference works for local development and when packaging charts together. Ensure the CI/CD pipeline handles this correctly during chart publishing.charts/hyperfleet-gcp/templates/NOTES.txt (2)
13-18: Good fallback message for missing projectId.The
| default "NOT SET - Please configure!"pattern provides clear feedback when configuration is incomplete.
47-51: Log viewing commands are helpful.Good inclusion of kubectl logs commands for all components. The label selectors align with expected Kubernetes labeling.
charts/hyperfleet-base/Chart.yaml (1)
1-18: Chart metadata is well-structured.Proper apiVersion v2, clear description, maintainer info, and relevant keywords. The home URL provides discoverability.
examples/gcp-dev/values.yaml
Outdated
| repository: sentinel | ||
| tag: "dev-SENTINEL_GIT_SHA" | ||
| broker: | ||
| type: rabbitmq |
There was a problem hiding this comment.
How about use pub/sub instead of rabbitmq? Especially it's for GCP dev
There was a problem hiding this comment.
there are just examples, at the moment i have dev and prod, dev using rabbitmq and prod using pub sub, what ill do is be more explicit with the examples, have a pubsub example and a rabbitmq example
- Rename examples: gcp-dev → gcp-rabbitmq, gcp-prod → gcp-pubsub - Rename values-dev.yaml → values-rabbitmq.yaml for clarity - Rename landing-zone → adapter-landing-zone for adapter consistency - Use alias in Chart.yaml since upstream chart is named 'landing-zone' - Add global.hyperfleetApi for shared API endpoint config - Remove component-level broker.type (inherit from global) - Remove createTopicIfMissing/createSubscriptionIfMissing (requires extra perms) - Add explicit broker.type to validation-gcp for RabbitMQ mode - Add test-helm target for Makefile consistency with other repos - Update README with broker config explanation and renamed paths - Fix NOTES.txt to reference adapter-landing-zone 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @charts/hyperfleet-base/values.yaml:
- Around line 62-66: Replace the hardcoded database.postgresql.password value in
values.yaml with a secure secret mechanism: remove the "password" key and
instead configure database.postgresql.auth with either auth.existingSecret set
to empty string to let the PostgreSQL subchart auto-generate and store the
password, or reference an external secret by setting auth.existingSecret to the
secret name and secretKeys.adminPasswordKey to the key name; update README.md to
document how to provide or pre-create the external secret when installing the
chart.
- Line 61: The chart values use a floating image tag ("tag: latest"), which
prevents deterministic deployments; update the tag keys (e.g., the "tag" entries
at the locations shown) to pin each component to a specific semantic version or
immutable digest (e.g., v1.2.3 or sha256:...) instead of "latest", and
optionally add a separate values-dev.yaml to override tags to "latest" for
development environments; ensure all occurrences noted in the comment are
replaced.
- Around line 135-137: The values.yaml currently hardcodes RabbitMQ credentials
under auth.username/auth.password which exposes secrets; replace the literal
password by wiring auth.password to a secure source (e.g., reference a
Kubernetes Secret or use Helm chart values injected by a pre-install hook),
update chart templates that consume auth.password to read from that Secret
instead of values.yaml, and add logic to create the Secret (via a Helm hook or a
separate manifest) that generates a random password at install time and stores
it in the Secret so secrets are not committed to version control.
In @README.md:
- Around line 9-17: Update the fenced code block that shows the directory
structure in README.md to include a language identifier (e.g., change the
opening ``` to ```text) so the block is rendered and linted correctly; locate
the directory-structure block (the triple-backtick block containing
"hyperfleet-chart/" and its nested entries) and add the language tag to the
opening fence.
🧹 Nitpick comments (5)
charts/hyperfleet-gcp/values-rabbitmq.yaml (1)
21-37: Consider consolidating duplicate RabbitMQ URLs.The same AMQP URL (
amqp://hyperfleet:hyperfleet-dev-password@hyperfleet-rabbitmq:5672/) is repeated three times (lines 25, 30, 37). While acceptable for a values file, consider using YAML anchors to reduce duplication and ensure consistency:♻️ Optional refactor using YAML anchors
base: global: broker: type: rabbitmq rabbitmq: enabled: true googlepubsub: enabled: false + # RabbitMQ connection shared by all components + rabbitmqUrl: &rabbitmq-url "amqp://hyperfleet:hyperfleet-dev-password@hyperfleet-rabbitmq:5672/" sentinel: broker: topic: "hyperfleet-clusters" rabbitmq: - url: "amqp://hyperfleet:hyperfleet-dev-password@hyperfleet-rabbitmq:5672/" + url: *rabbitmq-url adapter-landing-zone: broker: rabbitmq: - url: "amqp://hyperfleet:hyperfleet-dev-password@hyperfleet-rabbitmq:5672/" + url: *rabbitmq-urlMakefile (1)
16-21: Consider adding an 'all' target for Makefile conventions.The static analysis tool noted a missing
allphony target. While yourhelptarget serves well as the default, adding analltarget is a common Makefile convention.♻️ Optional addition
.PHONY: help deps lint template test test-helm clean \ deps-base deps-gcp \ lint-base lint-gcp \ template-base template-gcp template-gcp-rabbitmq \ install install-pubsub upgrade uninstall status \ - test-templates check-helm-git + test-templates check-helm-git all + +all: test-helm ## Run all tests (default target)charts/hyperfleet-base/values.yaml (3)
132-134: Pin RabbitMQ to a specific version.The
3-managementtag is a floating tag that tracks the latest 3.x release. For reproducible deployments, pin to a specific patch version.📌 Suggested fix
rabbitmq: enabled: true image: repository: rabbitmq - tag: 3-management + tag: "3.13.0-management" # or latest stable 3.x version
126-131: Consider strengthening the dev-only documentation.Based on learnings, the RabbitMQ deployment is strictly for development/testing and should never be used in production. The current comment is adequate, but a more prominent warning could prevent misuse.
💡 Suggested enhancement
# ============================================================================= # RabbitMQ Configuration (Development) # ============================================================================= -# Simple in-cluster RabbitMQ for development and testing. -# Disable for production and use external managed broker. +# WARNING: Development and testing ONLY! +# This simple in-cluster RabbitMQ is NOT suitable for production use. +# For production, disable this and configure an external managed broker +# (e.g., Google Pub/Sub via cloud overlays). rabbitmq: - enabled: true + enabled: true # Set to false for productionBased on learnings, production deployments use Google Pub/Sub instead.
55-71: Consider adding resource limits for core components.The HyperFleet API, Sentinel, and Landing Zone Adapter lack resource limits and requests, while RabbitMQ has them defined. Defining resources helps with proper scheduling, quality-of-service guarantees, and prevents resource exhaustion.
♻️ Suggested additions
Add a
resourcessection to each component:hyperfleet-api: # ... existing config ... resources: limits: cpu: 1000m memory: 1Gi requests: cpu: 200m memory: 256Mi sentinel: # ... existing config ... resources: limits: cpu: 500m memory: 512Mi requests: cpu: 100m memory: 128Mi adapter-landing-zone: # ... existing config ... resources: limits: cpu: 500m memory: 512Mi requests: cpu: 100m memory: 128MiThese can be adjusted based on actual usage patterns and can be overridden in cloud-specific overlays for production.
Also applies to: 75-98, 102-124
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
Chart.yamlMakefileREADME.mdcharts/hyperfleet-base/Chart.yamlcharts/hyperfleet-base/templates/NOTES.txtcharts/hyperfleet-base/values.yamlcharts/hyperfleet-gcp/values-rabbitmq.yamlcharts/hyperfleet-gcp/values.yamlexamples/gcp-pubsub/values.yamlexamples/gcp-rabbitmq/values.yaml
✅ Files skipped from review due to trivial changes (2)
- examples/gcp-rabbitmq/values.yaml
- charts/hyperfleet-base/templates/NOTES.txt
🚧 Files skipped from review as they are similar to previous changes (2)
- charts/hyperfleet-base/Chart.yaml
- charts/hyperfleet-gcp/values.yaml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-07T16:25:34.939Z
Learnt from: ciaranRoche
Repo: openshift-hyperfleet/hyperfleet-chart PR: 1
File: charts/hyperfleet-base/templates/rabbitmq.yaml:27-47
Timestamp: 2026-01-07T16:25:34.939Z
Learning: In the HyperFleet Helm charts (hyperfleet-base), the RabbitMQ deployment is intended only for development/testing environments and will never be used in production. Production deployments use Google Pub/Sub instead.
Applied to files:
charts/hyperfleet-base/values.yamlcharts/hyperfleet-gcp/values-rabbitmq.yamlChart.yamlexamples/gcp-pubsub/values.yaml
🪛 checkmake (0.2.2)
Makefile
[warning] 125-125: Target body for "install" exceeds allowed length of 5 (6).
(maxbodylength)
[warning] 159-159: Target body for "clean" exceeds allowed length of 5 (8).
(maxbodylength)
[warning] 16-16: Missing required phony target "all"
(minphony)
🪛 Checkov (3.2.334)
charts/hyperfleet-gcp/values-rabbitmq.yaml
[medium] 25-26: Basic Auth Credentials
(CKV_SECRET_4)
🪛 markdownlint-cli2 (0.18.1)
README.md
9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (16)
charts/hyperfleet-gcp/values-rabbitmq.yaml (3)
1-8: LGTM! Clear usage documentation for development.The header comments clearly communicate this is for development/GCP deployments with RabbitMQ, and the usage example is helpful.
32-37: Hardcoded credentials are acceptable for development.The static analysis tool flagged hardcoded credentials, but these are appropriate for this development-focused values file. Based on learnings, RabbitMQ is intended only for development/testing environments, and production deployments use Google Pub/Sub instead.
42-48: The validation-gcp adapter withdeploymentMode: "dummy"and RabbitMQ is the documented, intended configuration for testing environments. The file header explicitly states that RabbitMQ is used for in-cluster messaging in non-production deployments. The dummy mode is well-documented across example files and the main values.yaml with the note: "Use 'dummy' for testing, 'real' when validation-gcp-adapter.yaml exists." This configuration requires no changes.Chart.yaml (1)
1-12: Excellent deprecation notice and migration guidance.The deprecation header is clear, actionable, and provides concrete examples for migrating to the new chart structure. This will help users transition smoothly.
README.md (3)
19-33: Clear and well-structured component documentation.The base and GCP overlay component descriptions are concise and accurately reflect the architecture introduced in this PR.
112-116: Excellent clarification on dual RabbitMQ configuration.This note distinguishes between
global.broker.type(tells components which broker to use) andrabbitmq.enabled(deploys the broker). This is a common source of confusion and the explanation is very helpful.
193-203: Migration guidance aligns well with deprecated Chart.yaml.The migration examples match the deprecation notice in the root Chart.yaml, providing consistent messaging across documentation.
Makefile (4)
1-15: Well-documented Makefile with clear usage guidance.The header comments provide excellent context about the base+overlay pattern and include prerequisite requirements with actionable install commands.
41-50: Excellent prerequisite validation with actionable guidance.The
check-helm-gittarget validates the required plugin and provides clear installation instructions if missing. This proactive approach prevents cryptic errors downstream.
52-62: Well-designed dependency chain.The chained dependency targets (
deps→deps-base→deps-gcp) ensure correct ordering and include the helm-git check. The approach is clean and maintainable.
68-72: Lint targets use correct value files.The past review comment about missing
values-dev.yamlis resolved - the Makefile correctly referencesvalues-rabbitmq.yamlandexamples/gcp-pubsub/values.yaml, which exist in the PR.examples/gcp-pubsub/values.yaml (4)
1-16: Comprehensive prerequisites and usage documentation.The header provides clear prerequisites for GCP deployment with Pub/Sub, including Workload Identity, service accounts, and Terraform-provisioned resources. The usage example is actionable.
18-36: Well-structured global configuration.The global settings correctly configure Pub/Sub as the broker type and include the HyperFleet API endpoint configuration for component communication.
48-68: Sentinel configuration follows GCP best practices.The Sentinel configuration correctly sets up Workload Identity annotations and Pub/Sub-specific settings (maxOutstandingMessages, numGoroutines). The use of USER suffix in topic names supports multi-tenant development.
69-88: Landing zone adapter includes dead letter queue configuration.The adapter-landing-zone configuration properly includes
deadLetterTopicfor handling failed messages, which is important for production resilience.charts/hyperfleet-base/values.yaml (1)
32-33: Verify howtopicPrefixwill be used in application implementations.The configuration exists in values.yaml with an empty default, but it's not referenced in the Helm chart templates themselves. The multi-tenant isolation concern is valid as a design consideration, but the actual risk depends on how applications consuming this chart implement broker integration. Consider documenting whether deployed applications automatically inject namespace or other tenant context into topic names when
topicPrefixis empty.
Add a more descriptive comment explaining that "dummy" mode returns mock validation responses, and when to switch to "real" mode. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
ciaranRoche
force-pushed
the
HYPERFLEET-313
branch
from
37a0128 to
6f44433
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @examples/gcp-pubsub/values.yaml:
- Around line 67-68: Remove the dead Pub/Sub broker parameters
maxOutstandingMessages and numGoroutines from the configuration: delete these
keys from the example values.yaml and from the base chart
(charts/hyperfleet-base/values.yaml), and run a quick grep across the chart
templates for maxOutstandingMessages and numGoroutines to ensure no templates or
helpers reference them (if any references exist, either remove or replace those
template usages).
In @README.md:
- Around line 44-64: In the Quick Start section standardize the values file
pattern by making both examples reference the examples/ directory (e.g., change
the RabbitMQ command to use ../../examples/gcp-rabbitmq/values.yaml so it
matches the Pub/Sub line which uses ../../examples/gcp-pubsub/values.yaml);
update the README entries for both RabbitMQ and Pub/Sub to use the same -f
../../examples/... path and add a brief parenthetical next to the filenames
clarifying if these example files are "comprehensive example" vs "chart-local
variant" so readers understand why multiple variants exist.
🧹 Nitpick comments (1)
README.md (1)
34-42: Add context for helm-git plugin requirement.The Prerequisites section lists the helm-git plugin (line 38) but doesn't explain why it's needed. While the Chart Dependencies section (lines 140–160) shows git+ repository URLs, readers may not immediately understand the connection. Consider adding a brief note such as: "The helm-git plugin is required to pull chart dependencies from GitHub repositories."
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.mdcharts/hyperfleet-base/templates/NOTES.txtcharts/hyperfleet-base/values.yamlcharts/hyperfleet-gcp/values.yamlexamples/gcp-pubsub/values.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- charts/hyperfleet-base/templates/NOTES.txt
- charts/hyperfleet-base/values.yaml
- charts/hyperfleet-gcp/values.yaml
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ciaranRoche
Repo: openshift-hyperfleet/hyperfleet-chart PR: 1
File: charts/hyperfleet-base/templates/rabbitmq.yaml:27-47
Timestamp: 2026-01-07T16:25:34.939Z
Learning: In the HyperFleet Helm charts (hyperfleet-base), the RabbitMQ deployment is intended only for development/testing environments and will never be used in production. Production deployments use Google Pub/Sub instead.
📚 Learning: 2026-01-07T16:25:34.939Z
Learnt from: ciaranRoche
Repo: openshift-hyperfleet/hyperfleet-chart PR: 1
File: charts/hyperfleet-base/templates/rabbitmq.yaml:27-47
Timestamp: 2026-01-07T16:25:34.939Z
Learning: In the HyperFleet Helm charts (hyperfleet-base), the RabbitMQ deployment is intended only for development/testing environments and will never be used in production. Production deployments use Google Pub/Sub instead.
Applied to files:
examples/gcp-pubsub/values.yamlREADME.md
🔇 Additional comments (6)
examples/gcp-pubsub/values.yaml (6)
1-17: LGTM!The documentation header is clear and comprehensive, providing appropriate prerequisites and usage guidance for GCP + Pub/Sub deployments.
18-36: LGTM!Global configuration correctly enables Google Pub/Sub and disables RabbitMQ for production GCP deployment. Placeholders for registry and project ID are appropriate for an example file.
37-48: LGTM!The hyperfleet-api configuration correctly addresses previous security concerns by removing hardcoded passwords and providing clear guidance for secret management.
70-89: LGTM!The adapter-landing-zone configuration is well-structured with proper Workload Identity setup and complete Pub/Sub configuration including dead-letter topic.
90-92: LGTM!RabbitMQ is correctly disabled for this production GCP + Pub/Sub example. Based on learnings, RabbitMQ is only intended for development/testing environments.
93-116: LGTM!The validation-gcp configuration has been improved with a clear, expanded comment explaining the "dummy" mode and when to switch to "real" mode, addressing previous review feedback.
| maxOutstandingMessages: 1000 | ||
| numGoroutines: 10 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find relevant files
echo "=== Checking for hyperfleet-base chart ==="
fd -type f -name "values.yaml" | head -20
echo ""
echo "=== Checking for examples/gcp-pubsub/values.yaml ==="
fd -path "*/examples/gcp-pubsub/values.yaml"
echo ""
echo "=== Checking directory structure ==="
ls -la charts/ 2>/dev/null || echo "No charts directory"Repository: openshift-hyperfleet/hyperfleet-chart
Length of output: 808
🏁 Script executed:
#!/bin/bash
# Search for maxOutstandingMessages and numGoroutines in the codebase
echo "=== Searching for maxOutstandingMessages ==="
rg "maxOutstandingMessages" --type yaml -B 2 -A 2
echo ""
echo "=== Searching for numGoroutines ==="
rg "numGoroutines" --type yaml -B 2 -A 2
echo ""
echo "=== Checking if these are Go Pub/Sub library parameters ==="
rg "maxOutstandingMessages|numGoroutines" --type go || echo "Not found in Go files"Repository: openshift-hyperfleet/hyperfleet-chart
Length of output: 1593
🏁 Script executed:
#!/bin/bash
# Check the example file that's mentioned in the review
echo "=== Checking examples/gcp-pubsub/values.yaml ==="
if [ -f "examples/gcp-pubsub/values.yaml" ]; then
cat -n examples/gcp-pubsub/values.yaml | sed -n '60,75p'
else
echo "File not found. Searching for it:"
fd "values.yaml" | grep -i example
fiRepository: openshift-hyperfleet/hyperfleet-chart
Length of output: 701
🏁 Script executed:
#!/bin/bash
# Check if maxOutstandingMessages and numGoroutines are used in templates
echo "=== Checking sentinel-related templates in hyperfleet-base ==="
find charts/hyperfleet-base/templates -type f -name "*.yaml" -o -name "*.tpl" | while read f; do
if grep -l "sentinel\|googlepubsub" "$f" 2>/dev/null; then
echo "=== Found in: $f ==="
cat -n "$f"
fi
done
echo ""
echo "=== Checking if maxOutstandingMessages/numGoroutines are referenced in templates ==="
rg "maxOutstandingMessages|numGoroutines" charts/hyperfleet-base/templates/ || echo "Not found in templates"Repository: openshift-hyperfleet/hyperfleet-chart
Length of output: 252
🏁 Script executed:
#!/bin/bash
# Check the base chart values.yaml structure for sentinel config
echo "=== Sentinel section in base chart values.yaml ==="
cat -n charts/hyperfleet-base/values.yaml | grep -A 30 "sentinel:" | head -50Repository: openshift-hyperfleet/hyperfleet-chart
Length of output: 1206
🏁 Script executed:
#!/bin/bash
# Search for documentation or comments about these parameters
echo "=== Searching for documentation/comments about Pub/Sub parameters ==="
rg -B 5 "maxOutstandingMessages" charts/ || echo "No context found"Repository: openshift-hyperfleet/hyperfleet-chart
Length of output: 521
Remove unused Pub/Sub broker parameters from sentinel configuration.
The maxOutstandingMessages and numGoroutines parameters are defined in the base chart but not used in any templates. These should be removed from both the base chart (charts/hyperfleet-base/values.yaml) and this example file to avoid dead configuration code.
🤖 Prompt for AI Agents
In @examples/gcp-pubsub/values.yaml around lines 67 - 68, Remove the dead
Pub/Sub broker parameters maxOutstandingMessages and numGoroutines from the
configuration: delete these keys from the example values.yaml and from the base
chart (charts/hyperfleet-base/values.yaml), and run a quick grep across the
chart templates for maxOutstandingMessages and numGoroutines to ensure no
templates or helpers reference them (if any references exist, either remove or
replace those template usages).
| ## Quick Start | ||
|
|
||
| ### GCP Development (RabbitMQ) | ||
|
|
||
| ```bash | ||
| cd charts/hyperfleet-gcp | ||
| helm dependency update | ||
| helm install hyperfleet . -f values-rabbitmq.yaml \ | ||
| -n hyperfleet-system --create-namespace | ||
| ``` | ||
|
|
||
| ### GCP Production (Pub/Sub) | ||
|
|
||
| ```bash | ||
| cd charts/hyperfleet-gcp | ||
| helm dependency update | ||
| helm install hyperfleet . \ | ||
| -f ../../examples/gcp-pubsub/values.yaml \ | ||
| --set base.global.broker.googlepubsub.projectId=YOUR_PROJECT \ | ||
| -n hyperfleet-system --create-namespace | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify values file locations referenced in README.md Quick Start section
echo "=== Files in charts/hyperfleet-gcp ==="
if [ -d charts/hyperfleet-gcp ]; then
find charts/hyperfleet-gcp -maxdepth 1 -name "values*.yaml" -type f 2>/dev/null | sort
else
echo "Directory not found: charts/hyperfleet-gcp"
fi
echo ""
echo "=== All values.yaml files in examples/ ==="
if [ -d examples ]; then
find examples -name "values.yaml" -type f 2>/dev/null | sort
else
echo "Directory not found: examples"
fi
echo ""
echo "=== Chart directory structure ==="
ls -la charts/hyperfleet-gcp 2>/dev/null | head -20
echo ""
echo "=== Examples directory structure ==="
ls -la examples 2>/dev/nullRepository: openshift-hyperfleet/hyperfleet-chart
Length of output: 970
🏁 Script executed:
#!/bin/bash
# Compare RabbitMQ values files to understand their relationship
echo "=== charts/hyperfleet-gcp/values-rabbitmq.yaml ==="
cat charts/hyperfleet-gcp/values-rabbitmq.yaml
echo ""
echo "=== examples/gcp-rabbitmq/values.yaml ==="
cat examples/gcp-rabbitmq/values.yaml
echo ""
echo "=== File size comparison ==="
wc -l charts/hyperfleet-gcp/values-rabbitmq.yaml examples/gcp-rabbitmq/values.yamlRepository: openshift-hyperfleet/hyperfleet-chart
Length of output: 3400
🏁 Script executed:
#!/bin/bash
echo "=== examples/gcp-pubsub/values.yaml ==="
head -30 examples/gcp-pubsub/values.yamlRepository: openshift-hyperfleet/hyperfleet-chart
Length of output: 960
Consolidate values file pattern in Quick Start for consistency.
The Quick Start examples use inconsistent patterns:
- RabbitMQ (line 51):
-f values-rabbitmq.yaml(chart-shipped variant) - Pub/Sub (line 61):
-f ../../examples/gcp-pubsub/values.yaml(examples directory)
Both files exist and the paths work correctly, but the mixed approach creates confusion. The examples directory contains both examples/gcp-rabbitmq/values.yaml and examples/gcp-pubsub/values.yaml, suggesting both should follow the same pattern. Standardize on one approach (e.g., both use named example files in examples/, or both use chart-local values) and clarify the distinction if variants serve different purposes (minimal vs. comprehensive configurations).
🤖 Prompt for AI Agents
In @README.md around lines 44 - 64, In the Quick Start section standardize the
values file pattern by making both examples reference the examples/ directory
(e.g., change the RabbitMQ command to use
../../examples/gcp-rabbitmq/values.yaml so it matches the Pub/Sub line which
uses ../../examples/gcp-pubsub/values.yaml); update the README entries for both
RabbitMQ and Pub/Sub to use the same -f ../../examples/... path and add a brief
parenthetical next to the filenames clarifying if these example files are
"comprehensive example" vs "chart-local variant" so readers understand why
multiple variants exist.
3 participants
Summary
Restructures the HyperFleet Helm chart from a single umbrella chart into a base + cloud overlay pattern to support multi-cloud deployments.
Architecture
Key Changes
Base chart (
hyperfleet-base): Cloud-agnostic core componentsGCP overlay (
hyperfleet-gcp): GCP-specific configurationfile://../hyperfleet-baseBroker independence: Any broker works with any cloud
helm install hyperfleet charts/hyperfleet-gcp -f charts/hyperfleet-gcp/values-dev.yamlhelm install hyperfleet charts/hyperfleet-gcp -f examples/gcp-prod/values.yamlMigration
Files
charts/hyperfleet-base/charts/hyperfleet-gcp/examples/gcp-dev/values.yamlexamples/gcp-prod/values.yamlMakefileREADME.mdChart.yamlFuture Cloud Support
Additional overlays can follow the same pattern:
hyperfleet-aws- AWS with SNS/SQS, IRSAhyperfleet-e2e- Testing DeploymentTest plan
make lintpasses for both dev and prod valueshelm install hyperfleet charts/hyperfleet-gcp -f charts/hyperfleet-gcp/values-dev.yamlhelm install hyperfleet charts/hyperfleet-gcp -f examples/gcp-prod/values.yamlVerification
Summary by CodeRabbit
Documentation
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.