HYPERFLEET-1056 - feat: Helmfile infrastructure configurations#44
Open
ma-hill wants to merge 1 commit into
Open
HYPERFLEET-1056 - feat: Helmfile infrastructure configurations#44ma-hill wants to merge 1 commit into
ma-hill wants to merge 1 commit into
Conversation
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR migrates CI/developer workflow from chart-centric to a Helmfile-driven flow: Makefile selects HELMFILE_ENV and loads env.kind/env.gcp; Helmfile templates (helmfile.yaml.gotmpl) and base value templates orchestrate api, sentinel, and adapter releases and conditionally deploy a local rabbitmq chart. Terraform now writes Helm values to generated-values-from-terraform; a new script generates generated-values-rabbitmq for kind. Many e2e adapter configs, task templates, and resource manifests (Namespace, Job, Deployment, ManifestWork, ConfigMap) with CEL-based validation and status reporting were added; tf-helm-values.sh was removed. Sequence DiagramsequenceDiagram
participant User
participant Makefile
participant Terraform
participant Helmfile
participant Helm
participant Kind
User->>Makefile: make HELMFILE_ENV=gcp install-hyperfleet
Makefile->>Terraform: install-terraform (optional)
Terraform->>Makefile: write generated-values-from-terraform
Makefile->>Helmfile: helmfile -e gcp apply
Helmfile->>Helm: render & install hyperfleet-api, sentinels, adapters
Makefile->>Kind: build/load images & generate-rabbitmq-values (when HELMFILE_ENV=kind)
Helm->>User: Releases applied
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
ma-hill
force-pushed
the
HYPERFLEET-1056
branch
3 times, most recently
from
ccc611e to
1c95707
Compare
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CONTRIBUTING.md (1)
112-135:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThese Helmfile examples are not runnable as written.
The direct
helmfile ... templatecommands omitNAMESPACE, even though the new templates callrequiredEnv "NAMESPACE", and the lint example drops the=<env>assignment entirely. Copy-pasting this section will fail before rendering.Suggested fix
-helmfile -f helmfile/helmfile.yaml.gotmpl -e gcp template -helmfile -f helmfile/helmfile.yaml.gotmpl -e kind template -helmfile -f helmfile/helmfile.yaml.gotmpl -e e2e-gcp template -helmfile -f helmfile/helmfile.yaml.gotmpl -e e2e-kind template +NAMESPACE=<namespace> helmfile -f helmfile/helmfile.yaml.gotmpl -e gcp template +NAMESPACE=<namespace> helmfile -f helmfile/helmfile.yaml.gotmpl -e kind template +NAMESPACE=<namespace> helmfile -f helmfile/helmfile.yaml.gotmpl -e e2e-gcp template +NAMESPACE=<namespace> helmfile -f helmfile/helmfile.yaml.gotmpl -e e2e-kind template @@ -HELMFILE_ENV make lint-helmfile +HELMFILE_ENV=<env> make lint-helmfile🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CONTRIBUTING.md` around lines 112 - 135, The Helmfile examples omit the required NAMESPACE env var and the lint make target example drops the HELMFILE_ENV assignment; update the examples so every direct helmfile command and the make invocations set both HELMFILE_ENV and NAMESPACE (e.g. export or prefix HELMFILE_ENV=<env> NAMESPACE=<ns> before running) and ensure the make targets referenced (template-helmfile, lint-helmfile) are shown with the HELMFILE_ENV and NAMESPACE assignment so copy-paste will supply requiredEnv "NAMESPACE" for the templates.
🟠 Major comments (23)
scripts/generate-rabbitmq-values.sh-23-37 (1)
23-37:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate adapter/topic inputs before using them in paths and YAML.
Lines 74-88 write
adapter,topic, andNAMESPACEstraight into filenames and YAML without boundary checks. A malformed--adapter-topicslike../../outside=fooescapesOUT_DIR, and quotes/newlines can produce invalid or attacker-controlled values files. Reject anything outside a tight allowlist before writing.Possible hardening
+is_safe_token() { + [[ "$1" =~ ^[a-z0-9]([-.a-z0-9]*[a-z0-9])?$ ]] +} + IFS=',' read -ra MAPPINGS <<< "$ADAPTER_TOPICS" for mapping in "${MAPPINGS[@]}"; do adapter="${mapping%%=*}" topic="${mapping##*=}" + + if ! is_safe_token "$adapter" || ! is_safe_token "$topic" || ! is_safe_token "$NAMESPACE"; then + echo "ERROR: adapter, topic, and namespace must match [a-z0-9.-] and cannot contain path separators" >&2 + exit 1 + fi + file="${OUT_DIR}/${adapter}.yaml"As per coding guidelines, "Validate input at system boundaries (HTTP handlers, CLI parsers, webhook receivers)."
Also applies to: 74-88
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/generate-rabbitmq-values.sh` around lines 23 - 37, The script currently accepts --adapter-topics, NAMESPACE and OUT_DIR without validation which allows path traversal or injection into filenames/YAML; after flag parsing (use ADAPTER_TOPICS, NAMESPACE, OUT_DIR variables) validate and normalize inputs: reject ADAPTER_TOPICS entries that contain '/', '..', quotes, newlines or any chars outside a strict allowlist (e.g., only [A-Za-z0-9_.-]); split ADAPTER_TOPICS and validate each adapter and topic pair and fail with an error if any item is invalid; also validate NAMESPACE with the same allowlist and ensure OUT_DIR is a safe path (make absolute or ensure it doesn’t contain traversal segments) before writing files; perform these checks in the same script scope that uses ADAPTER_TOPICS/OUT_DIR/NAMESPACE so functions like require_value remain unchanged.helm/rabbitmq/values.yaml-12-14 (1)
12-14:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid default
guest/guestin RabbitMQ Helm values (breaks in-cluster auth if loopback-only is kept)RabbitMQ’s built-in
guestuser is loopback-only by default, so clients connecting via a Kubernetes Service (network/Pod IP, not localhost) typically can’t authenticate withauth.username: guest/auth.password: guest. The chart’shelm/rabbitmqdirectory only containstemplates/deployment.yaml/service.yaml(no rabbitmq.conf/loopback override files detected), so these defaults are likely used as-is—consider a non-guestdefault or require explicit credentials at install time.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helm/rabbitmq/values.yaml` around lines 12 - 14, Replace the insecure default auth.username: guest / auth.password: guest in the helm/rabbitmq values by removing the hardcoded guest credentials and either (a) make auth.username/auth.password required (empty by default) so installs fail fast and document this, or (b) set auth.password to a generated/templated secret value; update the chart templates (templates/deployment.yaml and any secret/template logic) to consume .Values.auth.username and .Values.auth.password and to error or reference a Secret when values are empty so the chart never relies on the loopback-only guest account.Makefile-170-179 (1)
170-179:⚠️ Potential issue | 🟠 Major | ⚡ Quick winE2E environments are skipped by generation and validation.
The PR advertises
e2e-gcpande2e-kindas supported Helmfile environments, but this logic only handles exactgcpandkind. Todaye2e-kindnever generates RabbitMQ values, and both e2e envs can passcheck-helmfile-env-generatedwithout validating the directory they depend on.Also applies to: 247-255
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` around lines 170 - 179, The generate-rabbitmq-values Makefile target incorrectly treats only exact HELMFILE_ENV values ('kind' and 'gcp'), skipping e2e-kind/e2e-gcp; update the conditional in the generate-rabbitmq-values rule (which references HELMFILE_ENV and BROKER_TYPE) to match prefixes or patterns (e.g., treat values starting with "kind" or "gcp" the same as exact names) so e2e-* envs are handled the same as kind/gcp, and apply the same pattern fix to the other similar block around the later rule lines (the other generate/validate checks mentioned). Also tighten check-helmfile-env-generated so it validates the expected output directory (GENERATED_RABBITMQ_DIR) actually exists and fails if missing, referencing the check-helmfile-env-generated target and GENERATED_RABBITMQ_DIR variable.terraform/helm-values-files.tf-38-56 (1)
38-56:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBootstrap the output directory before writing files.
These
local_fileresources write into../generated-values-from-terraform, but nothing here creates that directory. On a fresh checkout,terraform applycan fail before Helmfile ever sees the generated values.Does the Terraform hashicorp/local provider's local_file resource create missing parent directories for the filename automatically?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@terraform/helm-values-files.tf` around lines 38 - 56, The Terraform config writes files via local_file.adapter_values and local_file.sentinel_values into local.helm_values_dir but does not ensure the target directory exists; add a resource to create the directory (e.g., a null_resource.create_helm_values_dir with a provisioner "local-exec" running mkdir -p ${local.helm_values_dir}) and add depends_on = [null_resource.create_helm_values_dir] to both local_file.adapter_values and local_file.sentinel_values so the directory is created before attempting to write the files.Makefile-197-211 (1)
197-211:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix
helmfileconfig path for component-scoped install targets
install-api,install-sentinels, andinstall-adaptersrunhelmfile apply ...without-f, sohelmfilefalls back to looking for./helmfile.yaml/./helmfile.d. This repo doesn’t have a roothelmfile.yaml(onlyhelmfile/helmfile.yaml.gotmpl), so these targets can fail or apply the wrong state.Suggested fix
install-api: check-helmfile-env ## Install HyperFleet API - helmfile apply -e $(HELMFILE_ENV) -l component=api + helmfile -f helmfile/helmfile.yaml.gotmpl apply -e $(HELMFILE_ENV) -l component=api install-sentinels: check-helmfile-env ## Install Hyperfleet Sentinels - helmfile apply -e $(HELMFILE_ENV) -l component=sentinel + helmfile -f helmfile/helmfile.yaml.gotmpl apply -e $(HELMFILE_ENV) -l component=sentinel install-adapters: check-helmfile-env ## Install Hyperfleet Adapters - helmfile apply -e $(HELMFILE_ENV) -l component=adapter + helmfile -f helmfile/helmfile.yaml.gotmpl apply -e $(HELMFILE_ENV) -l component=adapter🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` around lines 197 - 211, The component-specific Make targets install-api, install-sentinels, and install-adapters currently call "helmfile apply" without the -f flag so helmfile may load a wrong/default file; update each target (install-api, install-sentinels, install-adapters) to pass the same -f helmfile/helmfile.yaml.gotmpl flag used by install-hyperfleet and keep the existing -e $(HELMFILE_ENV) and label filters so helmfile uses the repo's helmfile template rather than falling back to ./helmfile.yaml.helmfile/values/base-adapter.yaml.gotmpl-22-24 (1)
22-24:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove
SIMULATE_RESULTfrom the shared base values.This template is merged into every adapter release, so hardcoding
SIMULATE_RESULT=successmakes non-E2E environments opt into simulated behavior by default. Scope this flag to the E2E overlays, or make it an explicit opt-in instead of a base default.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helmfile/values/base-adapter.yaml.gotmpl` around lines 22 - 24, Remove the hardcoded env entry named SIMULATE_RESULT from the shared template (the env: - name: SIMULATE_RESULT value: success block) so it is not merged into all adapter releases; instead add a dedicated conditional or move this variable into the E2E overlay values and templates (e.g., guard with a value like .Values.e2e.enableSimulate or set SIMULATE_RESULT only in the E2E overlay values file) so non-E2E environments do not opt into simulated behavior by default.helmfile/environments/kind/kind-config.yaml.gotmpl-3-4 (1)
3-4:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate the env-selected broker and service types before rendering.
A bad
BROKER_TYPEhere still flows into the shared templates, buthelmfile/values/base-sentinel.yaml.gotmplonly handlesgooglepubsubandrabbitmq, so other values render an unusable broker config.API_SERVICE_TYPEis also forwarded without checks. Please enforce the supported values in this environment template instead of letting invalid input reach deployment time.As per coding guidelines, "Validate input at system boundaries (HTTP handlers, CLI parsers, webhook receivers)."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helmfile/environments/kind/kind-config.yaml.gotmpl` around lines 3 - 4, The environment template currently forwards BROKER_TYPE and API_SERVICE_TYPE without validating allowed values, causing invalid broker/service values to reach helm templates (base-sentinel only supports "googlepubsub" and "rabbitmq"); update the gotmpl around brokerType and serviceType to validate the env values against allowed lists and either coerce to a safe default or error early. Specifically, read env "BROKER_TYPE" and ensure it is one of ["googlepubsub","rabbitmq"] before assigning brokerType (otherwise set the default "rabbitmq" or abort render), and similarly validate env "API_SERVICE_TYPE" against allowed Kubernetes service types (e.g., "ClusterIP","NodePort","LoadBalancer") before assigning serviceType; implement these checks in the helmfile/environments/kind/kind-config.yaml.gotmpl template so invalid inputs never get forwarded to base-sentinel.yaml.gotempl or downstream charts.helmfile/environments/gcp/gcp-config.yaml.gotmpl-3-5 (1)
3-5:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail fast on unsupported
BROKER_TYPEandAPI_SERVICE_TYPEvalues.These env vars are accepted unchecked here. Downstream,
helmfile/values/base-sentinel.yaml.gotmplonly materializes broker settings forgooglepubsubandrabbitmq, so any otherBROKER_TYPEproduces a partial config, and an invalidAPI_SERVICE_TYPEis passed through to the Service manifest unchanged. Please validate both against an explicit allowlist at this boundary.As per coding guidelines, "Validate input at system boundaries (HTTP handlers, CLI parsers, webhook receivers)."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helmfile/environments/gcp/gcp-config.yaml.gotmpl` around lines 3 - 5, Add validation in the helmfile template to fail fast when env var values are unsupported: read BROKER_TYPE and API_SERVICE_TYPE into locals (e.g., $brokerType and $serviceType) and use template conditionals with an explicit allowlist (e.g., broker allowlist ["googlepubsub","rabbitmq"] and service allowlist ["LoadBalancer","ClusterIP","NodePort"]) and call the template fail function if a value is not in the allowlist; update helmfile/environments/gcp/gcp-config.yaml.gotmpl to perform these checks before emitting projectId/brokerType/serviceType (and reference values/base-sentinel.yaml.gotmpl behavior when choosing broker allowlist entries).helmfile/configs/e2e/adapters/cl-deployment/adapter-task-config.yaml-4-8 (1)
4-8:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate
clusterIdbefore reusing it in URLs and namespace-scoped resources.This adapter takes
event.idand propagates it into API URLs, namespace selection, and the status writeback path without a boundary check. A bad value here breaks the whole e2e chain, not just this deployment step.As per coding guidelines, "Validate input at system boundaries (HTTP handlers, CLI parsers, webhook receivers)."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helmfile/configs/e2e/adapters/cl-deployment/adapter-task-config.yaml` around lines 4 - 8, The param "clusterId" (sourced from event.id) must be validated before being used in API URLs, Kubernetes namespace selection, or status writeback paths; add a validation step where the adapter reads this param to enforce allowed characters/length (e.g., alphanumeric, dashes, max length) or match a strict regex, return a clear error if invalid, and sanitize/encode the value (URL-encode for API paths and validate against Kubernetes namespace rules) before constructing URLs or namespaces so no raw event.id is directly reused.helmfile/configs/e2e/adapters/cl-namespace/adapter-task-config.yaml-4-8 (1)
4-8:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate
clusterIdbefore templating it into URLs and Kubernetes metadata.
event.idis external input, and this file later injects it into/clusters/{{ .clusterId }}, the Namespace name, labels, and the status-update URL. Without a boundary check, malformed values can either break reconciliation or hit an unintended API path. Constrain it to the Kubernetes-safe/path-safe subset before any templating happens.As per coding guidelines, "Validate input at system boundaries (HTTP handlers, CLI parsers, webhook receivers)."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helmfile/configs/e2e/adapters/cl-namespace/adapter-task-config.yaml` around lines 4 - 8, The param "clusterId" (params -> name: "clusterId", referenced as {{ .clusterId }} and used in /clusters/{{ .clusterId }}, Namespace name/labels and status URLs) is coming from external event.id and must be validated/sanitized before templating: enforce it conforms to a Kubernetes DNS-1123 label (lowercase a-z0-9 and '-', <=63 chars, no leading/trailing '-'), and reject or canonicalize any value that fails; for URL paths either validate against a stricter path-safe subset or percent-encode after validation; implement this check at the parameter parsing/binding step (where params are read) and return a clear validation error or abort templating if clusterId is invalid.helmfile/configs/e2e/adapters/cl-job/adapter-task-config.yaml-4-8 (1)
4-8:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate
clusterIdbefore using it across HTTP and Kubernetes boundaries.
event.idis injected into/clusters/{{ .clusterId }}, the target namespace, selectors, and the status-update URL with no boundary validation. A malformed value can break both the API calls and the resource naming path for this adapter chain.As per coding guidelines, "Validate input at system boundaries (HTTP handlers, CLI parsers, webhook receivers)."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helmfile/configs/e2e/adapters/cl-job/adapter-task-config.yaml` around lines 4 - 8, Validate and sanitize the incoming clusterId (params -> name: "clusterId", source: "event.id") at the adapter boundary before any use in templates like /clusters/{{ .clusterId }}, namespace names, selectors or status-update URLs: enforce a strict whitelist (e.g., DNS-1123 label regex for Kubernetes names: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ with appropriate max length), reject or return a clear error for invalid values, URL-encode clusterId when inserting into HTTP paths, and/or transform to a safe canonical form; implement this check in the param parsing/handler that reads params.clusterId so the rest of the code can assume a validated, safe identifier.helmfile/configs/e2e/adapters/cl-deployment/adapter-task-resource-deployment.yaml-23-28 (1)
23-28:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThis Deployment still relies on a default root-capable security context.
The container/pod spec has no explicit hardening, so it inherits defaults that allow root and a writable root filesystem. That is both a security regression and a portability risk for clusters enforcing restricted admission.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helmfile/configs/e2e/adapters/cl-deployment/adapter-task-resource-deployment.yaml` around lines 23 - 28, The Deployment currently leaves the pod and container security context to defaults; add explicit pod-level and container-level securityContext entries to harden the workload: set pod spec.securityContext with runAsNonRoot: true and a non-root runAsUser/fsGroup (e.g., >1000), and on the container named "test" set securityContext with readOnlyRootFilesystem: true, allowPrivilegeEscalation: false, drop all capabilities (capabilities.drop: ["ALL"]) and optionally a seccompProfile/runtimeClass if required by your cluster; ensure these keys are added alongside the existing spec -> containers -> - name: test block so the pod will be non-root and use a read-only root filesystem.helmfile/configs/e2e/adapters/cl-job/adapter-task-resource-job.yaml-16-29 (1)
16-29:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHarden this Job instead of relying on the default root-capable security context.
Right now the pod/container run with the platform defaults, which is exactly what Trivy flagged. Even for e2e, this weakens the cluster baseline and can fail under restricted admission policies.
Suggested hardening
spec: backoffLimit: 0 template: spec: + securityContext: + seccompProfile: + type: RuntimeDefault restartPolicy: Never containers: - name: hello-world image: alpine:3.19 imagePullPolicy: IfNotPresent command: ["/bin/sh", "-c", "echo 'Hello, World!'"] + securityContext: + allowPrivilegeEscalation: false + readOnlyRootFilesystem: true + runAsNonRoot: true + runAsUser: 65532 + runAsGroup: 65532 + capabilities: + drop: ["ALL"] resources: requests: memory: "32Mi"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helmfile/configs/e2e/adapters/cl-job/adapter-task-resource-job.yaml` around lines 16 - 29, The Job's Pod uses platform defaults and runs root-capable containers; update the manifest (under spec and the container with name "hello-world") to harden its securityContext: add a pod-level securityContext with runAsNonRoot: true and runAsUser: 1000 (or another non-root UID), and add container-level securityContext for the "hello-world" container with runAsNonRoot: true, runAsUser: 1000, allowPrivilegeEscalation: false, readOnlyRootFilesystem: true, capabilities: drop: ["ALL"], and seccompProfile: { type: "RuntimeDefault" } (or Localhost with a profile if required by policy); ensure no privileged: true is set and remove any settings that would grant extra capabilities.helmfile/configs/e2e/adapters/cl-deployment/adapter-task-resource-deployment.yaml-25-26 (1)
25-26:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPin the nginx image and harden the test Deployment’s securityContext defaults.
- name: test image: nginx:latest
nginx:latestis a floating tag; HyperFleet standards call for container image digest pinning to avoid supply-chain/registry drift—use a pinned version orsha256digest instead.- This Deployment template lacks the required restrictive defaults (
podSecurityContextand containersecurityContext:runAsNonRoot,allowPrivilegeEscalation: false,capabilities.drop: ["ALL"],readOnlyRootFilesystem: true,seccompProfile.type: RuntimeDefault).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helmfile/configs/e2e/adapters/cl-deployment/adapter-task-resource-deployment.yaml` around lines 25 - 26, Replace the floating image tag for the container named "test" with a pinned image (use a specific version tag or an image@sha256:<digest>) and add restrictive security defaults to the Deployment template: add podSecurityContext with runAsNonRoot: true and a non-root runAsUser, and add container-level securityContext for the "test" container including runAsNonRoot: true, allowPrivilegeEscalation: false, capabilities.drop: ["ALL"], readOnlyRootFilesystem: true, and seccompProfile.type: RuntimeDefault; ensure these fields are added in the Deployment spec template for the pod and container corresponding to the "test" container.helmfile/configs/e2e/adapters/cl-deployment/adapter-task-config.yaml-39-52 (1)
39-52:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix
validationCheckgate for list-shapedclusterJobStatuscapture
clusterJobStatuscomes from a JSONPath filter (capture.field) where the adapter executor captures a single string only when the JSONPath yields exactly one match; otherwise it captures[]interface{...}. If the/clusters/.../statusesresponse contains multiplecl-job/Availablestatuses,validationCheck’sclusterJobStatus == "True"won’t match correctly and can keep the gate blocked—update the JSONPath to return a single scalar (e.g., add an index) or adjust the CEL expression to accept a list.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helmfile/configs/e2e/adapters/cl-deployment/adapter-task-config.yaml` around lines 39 - 52, The validation gate fails when capture.clusterJobStatus yields a list instead of a single string; update the capture or CEL to handle lists: either modify the clusterAdapterStatus.capture.field JSONPath (the capture named "clusterJobStatus") to return a single scalar (e.g., add an index selector so it returns the first match) or change the validationCheck expression to accept a list (use a CEL any/exists style check over clusterJobStatus to see if any element == "True") so the condition clusterJobStatus == "True" works for both single values and arrays.helmfile/configs/e2e/adapters/cl-maestro/adapter-task-config.yaml-11-14 (1)
11-14:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't overwrite the event generation.
paramsalready bindsgenerationfromevent.generation, thenclusterStatus.capturereuses the same name for the GET response field. That makesobserved_generationdepend on the follow-up API response instead of the triggering event.Proposed fix
- name: "clusterName" field: "name" - - name: "generation" + - name: "clusterGeneration" field: "generation"Also applies to: 32-33, 198-199
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helmfile/configs/e2e/adapters/cl-maestro/adapter-task-config.yaml` around lines 11 - 14, The capture in clusterStatus.capture is overwriting the event-bound params.generation (event.generation), so rename the GET response field there (e.g., change the captured key from "generation" to something like "api_generation" or "captured_generation") and ensure observed_generation continues to use the original params.generation (the event value) rather than the captured field; update all occurrences where clusterStatus.capture currently uses "generation" (including the other instances mentioned) to use the new name and adjust any downstream references accordingly.helmfile/configs/e2e/adapters/cl-maestro/adapter-task-config.yaml-43-45 (1)
43-45:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove the hard-coded placement target.
placementClusterNameis fixed to"cluster1", and Line 65 uses it as the Maestrotarget_cluster. Every event will reconcile against that same consumer, regardless of the actual placement result.Also applies to: 65-65
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helmfile/configs/e2e/adapters/cl-maestro/adapter-task-config.yaml` around lines 43 - 45, The placementClusterName is hard-coded to the literal "\"cluster1\"" which forces every event to reconcile to the same consumer; remove the fixed expression and wire placementClusterName to the placement adapter output (or the appropriate templated variable) so that target_cluster (where it's consumed) uses the actual placement result instead of the literal. Update the adapter-task-config entry for placementClusterName and any references to it (e.g., target_cluster) to read from the placement result variable rather than the hard-coded string.helmfile/configs/e2e/adapters/cl-maestro/adapter-task-resource-manifestwork.yaml-30-32 (1)
30-32:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake this file template-aware for tooling.
The standalone
{{ if ... }}block makes the raw file invalid YAML, which already shows up in static analysis. Any generic YAML lint/parse step over*.yamlwill choke on it until it is rendered.#!/bin/bash # Verify whether raw YAML files contain standalone Go-template control blocks # and whether repo tooling lint/parses them as plain YAML. fd -e yaml -e yml | xargs rg -n '^\s*\{\{\s*(if|else|end|range)\b' rg -n 'yamllint|kubeconform|kubectl apply -f|helm lint|helmfile lint|yq' -SExpected result: this file should appear in the first command. If the second command shows generic YAML tooling over
*.yaml, rename this template to a template-specific extension and update the manifest ref accordingly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helmfile/configs/e2e/adapters/cl-maestro/adapter-task-resource-manifestwork.yaml` around lines 30 - 32, This file contains a raw Go template control block ("{{ if .platformType }}" / "{{ end }}") which makes the YAML invalid to generic linters; to fix, rename the file to a template-aware extension (for example change adapter-task-resource-manifestwork.yaml -> adapter-task-resource-manifestwork.yaml.gotmpl or .tpl) and update any code/manifest references that consume this file to point to the new name so tooling skips plain-YAML parsing; ensure the template tokens ("{{ if .platformType }}" and "{{ end }}") remain unchanged inside the renamed file.helmfile/configs/e2e/adapters/cl-maestro/adapter-task-resource-manifestwork.yaml-6-6 (1)
6-6:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNormalize
clusterIdbefore using it inManifestWork.metadata.name(keepdiscovery.by_namein sync)In
helmfile/configs/e2e/adapters/cl-maestro/adapter-task-resource-manifestwork.yaml,ManifestWork.metadata.nameuses raw{{ .clusterId }}-{{ .adapter.name }}even though the template already lower-cases nested resource names via{{ .clusterId | lower }}-....helmfile/configs/e2e/adapters/cl-maestro/adapter-task-config.yamlthen discovers the ManifestWork using rawdiscovery.by_name: "{{ .clusterId }}-{{ .adapter.name }}", so any non-DNS-1123-safeclusterIdwill break create/discovery/status reporting. Apply DNS-1123-safe normalization toclusterIdfor the ManifestWork name (and update the pairedby_name), or document a strict upstream guarantee thatclusterIdis already Kubernetes-name safe.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helmfile/configs/e2e/adapters/cl-maestro/adapter-task-resource-manifestwork.yaml` at line 6, Normalize .clusterId to a DNS-1123-safe form before composing ManifestWork.metadata.name and use the same normalized value in discovery.by_name; specifically, update adapter-task-resource-manifestwork.yaml where ManifestWork.metadata.name is built (currently "{{ .clusterId }}-{{ .adapter.name }}") to apply the same normalization/filters used elsewhere (e.g., lowercasing and replacing/stripping invalid chars to produce a DNS-1123-compliant name) and then update the paired value in adapter-task-config.yaml (discovery.by_name) to use the identical normalized expression so create/discovery/status use the same safe name.CONTRIBUTING.md-247-260 (1)
247-260:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix the cleanup shell snippets.
export HELMFILE_ENV=gcp or HELMFILE_ENV=e2e-gcpis not valid shell, so these commands will not set the environment the way the section describes.Suggested fix
-export HELMFILE_ENV=gcp or HELMFILE_ENV=e2e-gcp +export HELMFILE_ENV=gcp # or: export HELMFILE_ENV=e2e-gcp @@ -export HELMFILE_ENV=kind or HELMFILE_ENV=e2e-kind +export HELMFILE_ENV=kind # or: export HELMFILE_ENV=e2e-kind🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CONTRIBUTING.md` around lines 247 - 260, The shell snippets use an invalid construct "export HELMFILE_ENV=gcp or HELMFILE_ENV=e2e-gcp"; update the examples so they show valid shell exports for HELMFILE_ENV (e.g., a single export per line with a comment or two separate example lines: one exporting gcp and one exporting e2e-gcp) and do the same for the kind/e2e-kind section; keep the NAMESPACE export as-is and ensure the subsequent commands (make uninstall-hyperfleet, make uninstall-maestro, make destroy-terraform) remain unchanged so the documentation demonstrates correct, runnable shell commands using the HELMFILE_ENV and NAMESPACE variables.README.md-205-219 (1)
205-219:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDocument the E2E generated-values flow too.
This section says generated values are only used for
HELMFILE_ENV=gcporHELMFILE_ENV=kind, but the README earlier introducese2e-gcpande2e-kindas supported Helmfile environments. As written, E2E readers are told the wrong prerequisite path for broker values.Suggested fix
-### Generated Helm Values - only used for HELMFILE_ENV=gcp or HELMFILE_ENV=kind +### Generated Helm Values - used for GCP and kind environment families -Broker configuration values are auto-generated and consumed by Helmfile: +Broker configuration values are auto-generated and consumed by Helmfile across the matching standard and E2E environments: -**For GCP environments (`gcp`):** +**For GCP environments (`gcp`, `e2e-gcp`):** - `make install-terraform` generates Google Pub/Sub configuration via `terraform/helm-values-files.tf` - Files are written to `generated-values-from-terraform/` - Contains: topic names, project ID, subscription IDs, Workload Identity bindings -- Helmfile automatically reads these values when deploying with `HELMFILE_ENV=gcp` +- Helmfile automatically reads these values when deploying with `HELMFILE_ENV=gcp` or `HELMFILE_ENV=e2e-gcp` -**For kind environments (`kind`):** +**For kind environments (`kind`, `e2e-kind`):** - `make generate-rabbitmq-values` generates RabbitMQ configuration - Files are written to `generated-values-rabbitmq/` - Contains: RabbitMQ URL, exchange names, queue names, routing keys -- Helmfile automatically reads these values when deploying with `HELMFILE_ENV=kind` +- Helmfile automatically reads these values when deploying with `HELMFILE_ENV=kind` or `HELMFILE_ENV=e2e-kind`🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 205 - 219, Update the "Generated Helm Values" section to include E2E environments by mentioning HELMFILE_ENV values e2e-gcp and e2e-kind alongside gcp and kind; explicitly state that for E2E workflows the same generation steps apply (e.g., `make install-terraform` → generated-values-from-terraform for GCP/e2e-gcp and `make generate-rabbitmq-values` → generated-values-rabbitmq for kind/e2e-kind) and that Helmfile reads those files when deploying with HELMFILE_ENV set to gcp, kind, e2e-gcp or e2e-kind so E2E readers are pointed to the correct prerequisite path for broker values.helmfile/environments/e2e-kind/adapter-configs.yaml.gotmpl-49-54 (1)
49-54:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep
broker.createenabled forcl-job.Every other adapter in this template sets
broker.create: true, butcl-jobomits it. If the chart defaults that flag to false, this adapter will render without broker wiring even though queue/exchange settings are present.Suggested fix
broker: + create: true rabbitmq: url: "amqp://guest:guest@rabbitmq:5672" queue: {{ $namespace }}-clusters-cl-job🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helmfile/environments/e2e-kind/adapter-configs.yaml.gotmpl` around lines 49 - 54, The cl-job adapter block is missing broker.create and may therefore not wire the RabbitMQ settings; update the cl-job adapter (the broker section for "cl-job") to include broker.create: true so that the RabbitMQ settings (broker.rabbitmq.url, queue, exchange, routingKey) are actually applied, matching the other adapters in this template.helmfile/environments/e2e-kind/adapter-configs.yaml.gotmpl-133-139 (1)
133-139:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix the
np-configmapqueue name.Line 137 points the nodepool adapter at
{{ $namespace }}-nodepools-cl-namespaceinstead of annp-configmapqueue. That misroutes messages and can collide with a different consumer naming scheme.Suggested fix
broker: create: true rabbitmq: url: "amqp://guest:guest@rabbitmq:5672" - queue: {{ $namespace }}-nodepools-cl-namespace + queue: {{ $namespace }}-nodepools-np-configmap exchange: {{ $namespace }}-nodepools routingKey: #🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helmfile/environments/e2e-kind/adapter-configs.yaml.gotmpl` around lines 133 - 139, The broker.rabbitmq.queue value is pointing at the wrong queue (currently "{{ $namespace }}-nodepools-cl-namespace"); update it to the np-configmap queue name (e.g. "{{ $namespace }}-np-configmap") so the nodepool adapter publishes to the correct queue; edit the broker -> rabbitmq -> queue entry to use "{{ $namespace }}-np-configmap" (leave exchange and routingKey as-is) and ensure any consumer/service that expects "np-configmap" matches this name to avoid collisions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Makefile`:
- Around line 117-130: The create-maestro-consumer Make target embeds
MAESTRO_CONSUMER directly into single-quoted shell JSON and grep patterns, which
allows a consumer name containing a single-quote to break quoting and enable
shell/JSON injection; fix it by (1) validating MAESTRO_CONSUMER against a strict
allowlist regex (e.g., alphanumerics, dashes, underscores) early in the
create-maestro-consumer flow and fail fast if it doesn't match, and (2) stop
inline single-quote interpolation when building the POST body and when checking
existence—construct the JSON payload safely (e.g., use jq or a small python
snippet to produce '{"name": <value>}' into a variable or stdin) and use a
JSON-aware check (e.g., pipe the API response through jq to test .name ==
MAESTRO_CONSUMER) instead of grep; update the parts of the
create-maestro-consumer target that reference MAESTRO_CONSUMER and the curl -d /
grep commands accordingly.
---
Outside diff comments:
In `@CONTRIBUTING.md`:
- Around line 112-135: The Helmfile examples omit the required NAMESPACE env var
and the lint make target example drops the HELMFILE_ENV assignment; update the
examples so every direct helmfile command and the make invocations set both
HELMFILE_ENV and NAMESPACE (e.g. export or prefix HELMFILE_ENV=<env>
NAMESPACE=<ns> before running) and ensure the make targets referenced
(template-helmfile, lint-helmfile) are shown with the HELMFILE_ENV and NAMESPACE
assignment so copy-paste will supply requiredEnv "NAMESPACE" for the templates.
---
Major comments:
In `@CONTRIBUTING.md`:
- Around line 247-260: The shell snippets use an invalid construct "export
HELMFILE_ENV=gcp or HELMFILE_ENV=e2e-gcp"; update the examples so they show
valid shell exports for HELMFILE_ENV (e.g., a single export per line with a
comment or two separate example lines: one exporting gcp and one exporting
e2e-gcp) and do the same for the kind/e2e-kind section; keep the NAMESPACE
export as-is and ensure the subsequent commands (make uninstall-hyperfleet, make
uninstall-maestro, make destroy-terraform) remain unchanged so the documentation
demonstrates correct, runnable shell commands using the HELMFILE_ENV and
NAMESPACE variables.
In `@helm/rabbitmq/values.yaml`:
- Around line 12-14: Replace the insecure default auth.username: guest /
auth.password: guest in the helm/rabbitmq values by removing the hardcoded guest
credentials and either (a) make auth.username/auth.password required (empty by
default) so installs fail fast and document this, or (b) set auth.password to a
generated/templated secret value; update the chart templates
(templates/deployment.yaml and any secret/template logic) to consume
.Values.auth.username and .Values.auth.password and to error or reference a
Secret when values are empty so the chart never relies on the loopback-only
guest account.
In `@helmfile/configs/e2e/adapters/cl-deployment/adapter-task-config.yaml`:
- Around line 4-8: The param "clusterId" (sourced from event.id) must be
validated before being used in API URLs, Kubernetes namespace selection, or
status writeback paths; add a validation step where the adapter reads this param
to enforce allowed characters/length (e.g., alphanumeric, dashes, max length) or
match a strict regex, return a clear error if invalid, and sanitize/encode the
value (URL-encode for API paths and validate against Kubernetes namespace rules)
before constructing URLs or namespaces so no raw event.id is directly reused.
- Around line 39-52: The validation gate fails when capture.clusterJobStatus
yields a list instead of a single string; update the capture or CEL to handle
lists: either modify the clusterAdapterStatus.capture.field JSONPath (the
capture named "clusterJobStatus") to return a single scalar (e.g., add an index
selector so it returns the first match) or change the validationCheck expression
to accept a list (use a CEL any/exists style check over clusterJobStatus to see
if any element == "True") so the condition clusterJobStatus == "True" works for
both single values and arrays.
In
`@helmfile/configs/e2e/adapters/cl-deployment/adapter-task-resource-deployment.yaml`:
- Around line 23-28: The Deployment currently leaves the pod and container
security context to defaults; add explicit pod-level and container-level
securityContext entries to harden the workload: set pod spec.securityContext
with runAsNonRoot: true and a non-root runAsUser/fsGroup (e.g., >1000), and on
the container named "test" set securityContext with readOnlyRootFilesystem:
true, allowPrivilegeEscalation: false, drop all capabilities (capabilities.drop:
["ALL"]) and optionally a seccompProfile/runtimeClass if required by your
cluster; ensure these keys are added alongside the existing spec -> containers
-> - name: test block so the pod will be non-root and use a read-only root
filesystem.
- Around line 25-26: Replace the floating image tag for the container named
"test" with a pinned image (use a specific version tag or an
image@sha256:<digest>) and add restrictive security defaults to the Deployment
template: add podSecurityContext with runAsNonRoot: true and a non-root
runAsUser, and add container-level securityContext for the "test" container
including runAsNonRoot: true, allowPrivilegeEscalation: false,
capabilities.drop: ["ALL"], readOnlyRootFilesystem: true, and
seccompProfile.type: RuntimeDefault; ensure these fields are added in the
Deployment spec template for the pod and container corresponding to the "test"
container.
In `@helmfile/configs/e2e/adapters/cl-job/adapter-task-config.yaml`:
- Around line 4-8: Validate and sanitize the incoming clusterId (params -> name:
"clusterId", source: "event.id") at the adapter boundary before any use in
templates like /clusters/{{ .clusterId }}, namespace names, selectors or
status-update URLs: enforce a strict whitelist (e.g., DNS-1123 label regex for
Kubernetes names: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ with appropriate max length),
reject or return a clear error for invalid values, URL-encode clusterId when
inserting into HTTP paths, and/or transform to a safe canonical form; implement
this check in the param parsing/handler that reads params.clusterId so the rest
of the code can assume a validated, safe identifier.
In `@helmfile/configs/e2e/adapters/cl-job/adapter-task-resource-job.yaml`:
- Around line 16-29: The Job's Pod uses platform defaults and runs root-capable
containers; update the manifest (under spec and the container with name
"hello-world") to harden its securityContext: add a pod-level securityContext
with runAsNonRoot: true and runAsUser: 1000 (or another non-root UID), and add
container-level securityContext for the "hello-world" container with
runAsNonRoot: true, runAsUser: 1000, allowPrivilegeEscalation: false,
readOnlyRootFilesystem: true, capabilities: drop: ["ALL"], and seccompProfile: {
type: "RuntimeDefault" } (or Localhost with a profile if required by policy);
ensure no privileged: true is set and remove any settings that would grant extra
capabilities.
In `@helmfile/configs/e2e/adapters/cl-maestro/adapter-task-config.yaml`:
- Around line 11-14: The capture in clusterStatus.capture is overwriting the
event-bound params.generation (event.generation), so rename the GET response
field there (e.g., change the captured key from "generation" to something like
"api_generation" or "captured_generation") and ensure observed_generation
continues to use the original params.generation (the event value) rather than
the captured field; update all occurrences where clusterStatus.capture currently
uses "generation" (including the other instances mentioned) to use the new name
and adjust any downstream references accordingly.
- Around line 43-45: The placementClusterName is hard-coded to the literal
"\"cluster1\"" which forces every event to reconcile to the same consumer;
remove the fixed expression and wire placementClusterName to the placement
adapter output (or the appropriate templated variable) so that target_cluster
(where it's consumed) uses the actual placement result instead of the literal.
Update the adapter-task-config entry for placementClusterName and any references
to it (e.g., target_cluster) to read from the placement result variable rather
than the hard-coded string.
In
`@helmfile/configs/e2e/adapters/cl-maestro/adapter-task-resource-manifestwork.yaml`:
- Around line 30-32: This file contains a raw Go template control block ("{{ if
.platformType }}" / "{{ end }}") which makes the YAML invalid to generic
linters; to fix, rename the file to a template-aware extension (for example
change adapter-task-resource-manifestwork.yaml ->
adapter-task-resource-manifestwork.yaml.gotmpl or .tpl) and update any
code/manifest references that consume this file to point to the new name so
tooling skips plain-YAML parsing; ensure the template tokens ("{{ if
.platformType }}" and "{{ end }}") remain unchanged inside the renamed file.
- Line 6: Normalize .clusterId to a DNS-1123-safe form before composing
ManifestWork.metadata.name and use the same normalized value in
discovery.by_name; specifically, update adapter-task-resource-manifestwork.yaml
where ManifestWork.metadata.name is built (currently "{{ .clusterId }}-{{
.adapter.name }}") to apply the same normalization/filters used elsewhere (e.g.,
lowercasing and replacing/stripping invalid chars to produce a
DNS-1123-compliant name) and then update the paired value in
adapter-task-config.yaml (discovery.by_name) to use the identical normalized
expression so create/discovery/status use the same safe name.
In `@helmfile/configs/e2e/adapters/cl-namespace/adapter-task-config.yaml`:
- Around line 4-8: The param "clusterId" (params -> name: "clusterId",
referenced as {{ .clusterId }} and used in /clusters/{{ .clusterId }}, Namespace
name/labels and status URLs) is coming from external event.id and must be
validated/sanitized before templating: enforce it conforms to a Kubernetes
DNS-1123 label (lowercase a-z0-9 and '-', <=63 chars, no leading/trailing '-'),
and reject or canonicalize any value that fails; for URL paths either validate
against a stricter path-safe subset or percent-encode after validation;
implement this check at the parameter parsing/binding step (where params are
read) and return a clear validation error or abort templating if clusterId is
invalid.
In `@helmfile/environments/e2e-kind/adapter-configs.yaml.gotmpl`:
- Around line 49-54: The cl-job adapter block is missing broker.create and may
therefore not wire the RabbitMQ settings; update the cl-job adapter (the broker
section for "cl-job") to include broker.create: true so that the RabbitMQ
settings (broker.rabbitmq.url, queue, exchange, routingKey) are actually
applied, matching the other adapters in this template.
- Around line 133-139: The broker.rabbitmq.queue value is pointing at the wrong
queue (currently "{{ $namespace }}-nodepools-cl-namespace"); update it to the
np-configmap queue name (e.g. "{{ $namespace }}-np-configmap") so the nodepool
adapter publishes to the correct queue; edit the broker -> rabbitmq -> queue
entry to use "{{ $namespace }}-np-configmap" (leave exchange and routingKey
as-is) and ensure any consumer/service that expects "np-configmap" matches this
name to avoid collisions.
In `@helmfile/environments/gcp/gcp-config.yaml.gotmpl`:
- Around line 3-5: Add validation in the helmfile template to fail fast when env
var values are unsupported: read BROKER_TYPE and API_SERVICE_TYPE into locals
(e.g., $brokerType and $serviceType) and use template conditionals with an
explicit allowlist (e.g., broker allowlist ["googlepubsub","rabbitmq"] and
service allowlist ["LoadBalancer","ClusterIP","NodePort"]) and call the template
fail function if a value is not in the allowlist; update
helmfile/environments/gcp/gcp-config.yaml.gotmpl to perform these checks before
emitting projectId/brokerType/serviceType (and reference
values/base-sentinel.yaml.gotmpl behavior when choosing broker allowlist
entries).
In `@helmfile/environments/kind/kind-config.yaml.gotmpl`:
- Around line 3-4: The environment template currently forwards BROKER_TYPE and
API_SERVICE_TYPE without validating allowed values, causing invalid
broker/service values to reach helm templates (base-sentinel only supports
"googlepubsub" and "rabbitmq"); update the gotmpl around brokerType and
serviceType to validate the env values against allowed lists and either coerce
to a safe default or error early. Specifically, read env "BROKER_TYPE" and
ensure it is one of ["googlepubsub","rabbitmq"] before assigning brokerType
(otherwise set the default "rabbitmq" or abort render), and similarly validate
env "API_SERVICE_TYPE" against allowed Kubernetes service types (e.g.,
"ClusterIP","NodePort","LoadBalancer") before assigning serviceType; implement
these checks in the helmfile/environments/kind/kind-config.yaml.gotmpl template
so invalid inputs never get forwarded to base-sentinel.yaml.gotempl or
downstream charts.
In `@helmfile/values/base-adapter.yaml.gotmpl`:
- Around line 22-24: Remove the hardcoded env entry named SIMULATE_RESULT from
the shared template (the env: - name: SIMULATE_RESULT value: success block) so
it is not merged into all adapter releases; instead add a dedicated conditional
or move this variable into the E2E overlay values and templates (e.g., guard
with a value like .Values.e2e.enableSimulate or set SIMULATE_RESULT only in the
E2E overlay values file) so non-E2E environments do not opt into simulated
behavior by default.
In `@Makefile`:
- Around line 170-179: The generate-rabbitmq-values Makefile target incorrectly
treats only exact HELMFILE_ENV values ('kind' and 'gcp'), skipping
e2e-kind/e2e-gcp; update the conditional in the generate-rabbitmq-values rule
(which references HELMFILE_ENV and BROKER_TYPE) to match prefixes or patterns
(e.g., treat values starting with "kind" or "gcp" the same as exact names) so
e2e-* envs are handled the same as kind/gcp, and apply the same pattern fix to
the other similar block around the later rule lines (the other generate/validate
checks mentioned). Also tighten check-helmfile-env-generated so it validates the
expected output directory (GENERATED_RABBITMQ_DIR) actually exists and fails if
missing, referencing the check-helmfile-env-generated target and
GENERATED_RABBITMQ_DIR variable.
- Around line 197-211: The component-specific Make targets install-api,
install-sentinels, and install-adapters currently call "helmfile apply" without
the -f flag so helmfile may load a wrong/default file; update each target
(install-api, install-sentinels, install-adapters) to pass the same -f
helmfile/helmfile.yaml.gotmpl flag used by install-hyperfleet and keep the
existing -e $(HELMFILE_ENV) and label filters so helmfile uses the repo's
helmfile template rather than falling back to ./helmfile.yaml.
In `@README.md`:
- Around line 205-219: Update the "Generated Helm Values" section to include E2E
environments by mentioning HELMFILE_ENV values e2e-gcp and e2e-kind alongside
gcp and kind; explicitly state that for E2E workflows the same generation steps
apply (e.g., `make install-terraform` → generated-values-from-terraform for
GCP/e2e-gcp and `make generate-rabbitmq-values` → generated-values-rabbitmq for
kind/e2e-kind) and that Helmfile reads those files when deploying with
HELMFILE_ENV set to gcp, kind, e2e-gcp or e2e-kind so E2E readers are pointed to
the correct prerequisite path for broker values.
In `@scripts/generate-rabbitmq-values.sh`:
- Around line 23-37: The script currently accepts --adapter-topics, NAMESPACE
and OUT_DIR without validation which allows path traversal or injection into
filenames/YAML; after flag parsing (use ADAPTER_TOPICS, NAMESPACE, OUT_DIR
variables) validate and normalize inputs: reject ADAPTER_TOPICS entries that
contain '/', '..', quotes, newlines or any chars outside a strict allowlist
(e.g., only [A-Za-z0-9_.-]); split ADAPTER_TOPICS and validate each adapter and
topic pair and fail with an error if any item is invalid; also validate
NAMESPACE with the same allowlist and ensure OUT_DIR is a safe path (make
absolute or ensure it doesn’t contain traversal segments) before writing files;
perform these checks in the same script scope that uses
ADAPTER_TOPICS/OUT_DIR/NAMESPACE so functions like require_value remain
unchanged.
In `@terraform/helm-values-files.tf`:
- Around line 38-56: The Terraform config writes files via
local_file.adapter_values and local_file.sentinel_values into
local.helm_values_dir but does not ensure the target directory exists; add a
resource to create the directory (e.g., a null_resource.create_helm_values_dir
with a provisioner "local-exec" running mkdir -p ${local.helm_values_dir}) and
add depends_on = [null_resource.create_helm_values_dir] to both
local_file.adapter_values and local_file.sentinel_values so the directory is
created before attempting to write the files.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 531ca917-2119-485a-a6aa-8cb630b7edf4
📒 Files selected for processing (65)
.gitignoreCONTRIBUTING.mdMakefileREADME.mdenv.gcpenv.kindhelm/adapter1/Chart.yamlhelm/adapter2/Chart.yamlhelm/adapter2/dryrun-adapter2-discovery.jsonhelm/adapter3/Chart.yamlhelm/api/values.yamlhelm/rabbitmq/Chart.yamlhelm/rabbitmq/README.mdhelm/rabbitmq/templates/_helpers.tplhelm/rabbitmq/templates/deployment.yamlhelm/rabbitmq/templates/service.yamlhelm/rabbitmq/values.yamlhelm/sentinel-clusters/Chart.yamlhelm/sentinel-clusters/values.yamlhelm/sentinel-nodepools/Chart.yamlhelm/sentinel-nodepools/values.yamlhelmfile/configs/base/adapters/adapter1/adapter-config.yamlhelmfile/configs/base/adapters/adapter1/adapter-task-config.yamlhelmfile/configs/base/adapters/adapter2/adapter-config.yamlhelmfile/configs/base/adapters/adapter2/adapter-task-config.yamlhelmfile/configs/base/adapters/adapter3/adapter-config.yamlhelmfile/configs/base/adapters/adapter3/adapter-task-config.yamlhelmfile/configs/e2e/adapters/cl-deployment/adapter-config.yamlhelmfile/configs/e2e/adapters/cl-deployment/adapter-task-config.yamlhelmfile/configs/e2e/adapters/cl-deployment/adapter-task-resource-deployment.yamlhelmfile/configs/e2e/adapters/cl-job/adapter-config.yamlhelmfile/configs/e2e/adapters/cl-job/adapter-task-config.yamlhelmfile/configs/e2e/adapters/cl-job/adapter-task-resource-job.yamlhelmfile/configs/e2e/adapters/cl-maestro/adapter-config.yamlhelmfile/configs/e2e/adapters/cl-maestro/adapter-task-config.yamlhelmfile/configs/e2e/adapters/cl-maestro/adapter-task-resource-manifestwork.yamlhelmfile/configs/e2e/adapters/cl-namespace/adapter-config.yamlhelmfile/configs/e2e/adapters/cl-namespace/adapter-task-config.yamlhelmfile/configs/e2e/adapters/np-configmap/adapter-config.yamlhelmfile/configs/e2e/adapters/np-configmap/adapter-task-config.yamlhelmfile/configs/e2e/adapters/np-configmap/adapter-task-resource-configmap.yamlhelmfile/configs/e2e/adapters/np-configmap/values.yamlhelmfile/environments/e2e-gcp/adapter-configs.yaml.gotmplhelmfile/environments/e2e-gcp/e2e-gcp-config.yaml.gotmplhelmfile/environments/e2e-gcp/sentinel-configs.yamlhelmfile/environments/e2e-kind/adapter-configs.yaml.gotmplhelmfile/environments/e2e-kind/e2e-kind-config.yaml.gotmplhelmfile/environments/e2e-kind/sentinel-configs.yamlhelmfile/environments/gcp/adapter-configs.yaml.gotmplhelmfile/environments/gcp/gcp-config.yaml.gotmplhelmfile/environments/gcp/sentinel-configs.yaml.gotmplhelmfile/environments/kind/adapter-configs.yaml.gotmplhelmfile/environments/kind/kind-config.yaml.gotmplhelmfile/environments/kind/sentinel-configs.yaml.gotmplhelmfile/helmfile.yaml.gotmplhelmfile/values/.gitkeephelmfile/values/base-adapter.yaml.gotmplhelmfile/values/base-api.yaml.gotmplhelmfile/values/base-sentinel.yaml.gotmplscripts/generate-rabbitmq-values.shscripts/kind-build-images.shscripts/tf-helm-values.shterraform/helm-values-files.tfterraform/outputs.tfterraform/versions.tf
💤 Files with no reviewable changes (10)
- helm/adapter2/dryrun-adapter2-discovery.json
- helm/adapter2/Chart.yaml
- helm/api/values.yaml
- helm/sentinel-nodepools/values.yaml
- helm/adapter1/Chart.yaml
- helm/adapter3/Chart.yaml
- scripts/tf-helm-values.sh
- helm/sentinel-clusters/Chart.yaml
- helm/sentinel-clusters/values.yaml
- helm/sentinel-nodepools/Chart.yaml
ma-hill
force-pushed
the
HYPERFLEET-1056
branch
3 times, most recently
from
7f53c69 to
95cbe54
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CONTRIBUTING.md`:
- Around line 170-171: Update the shell examples so they are copy/paste
runnable: for the RabbitMQ generation and kind quick-start, set HELMFILE_ENV
inline (e.g. invoke make with HELMFILE_ENV=gcp make generate-rabbitmq-values or
prefix the kind quick-start command with HELMFILE_ENV=...) or explicitly export
HELMFILE_ENV beforehand (export HELMFILE_ENV=gcp) so the command is not a no-op;
also fix the cleanup examples to use valid shell syntax (use export VAR=... or
VAR=...; followed by the command, and remove the literal "or" usage) and apply
the same changes to the repeated examples referenced for the later block that
also uses HELMFILE_ENV and the cleanup commands.
In
`@helmfile/configs/e2e/adapters/cl-deployment/adapter-task-resource-deployment.yaml`:
- Around line 23-28: Replace the use of unpinned images and add security
contexts: change the container image reference "nginx:latest" (and similarly
"alpine:latest" in the sibling job) to a pinned tag or digest (no :latest) and
add a pod-level and container-level securityContext entries for the
Deployment/Job spec; update the container named "test" to run as non-root
(runAsNonRoot: true / runAsUser), drop capabilities, set readOnlyRootFilesystem:
true and a restrictive seccomp/profile or allowPrivilegeEscalation: false as
appropriate, and add fsGroup/runAsGroup at the pod securityContext to harden
filesystem permissions.
In `@Makefile`:
- Line 158: Update the TODO comment in the Makefile that currently reads "TODO
add an uninstall-maestro-consumer target" to include a tracked ticket ID per
repo standards; change it to the TODO format with a ticket reference (e.g.
TODO(HYPERFLEET-xxxx): add uninstall-maestro-consumer target) so the comment
references a specific issue for tracking and compliance.
- Around line 176-184: The Makefile target generate-rabbitmq-values only
triggers for HELMFILE_ENV == kind, so calls with e2e-kind become no-ops; update
the conditional to treat both kind and e2e-kind as RabbitMQ environments (and
likewise ensure the opposite grouping treats gcp and e2e-gcp as Pub/Sub) so make
generate-rabbitmq-values HELMFILE_ENV=e2e-kind runs the same RabbitMQ
generation; apply the same pattern to the other identical conditional block
referenced (around the later 257-265 region) and update any
check-helmfile-env-generated validation logic to consider e2e-kind equivalent to
kind and e2e-gcp equivalent to gcp.
- Around line 9-13: Add loading of the top-level .env before the env-specific
includes and ensure HELMFILE_ENV has a default of gcp; specifically, set
HELMFILE_ENV ?= gcp (or confirm its existing default) and insert a -include .env
line before the conditional that includes env.kind/env.gcp so the precedence
becomes command-line → .env → env.gcp/env.kind → Makefile defaults (refer to the
HELMFILE_ENV variable and the existing -include env.kind / -include env.gcp
lines to locate the correct place to add this).
- Around line 101-112: The install-maestro target currently swallows failures
from the helm upgrade --install and the kubectl wait (using an if/then echo and
"|| true"), which lets the make recipe succeed even when Maestro isn't
installed; update the install-maestro recipe to allow failures to propagate by
removing the conditional that turns helm failures into warnings and by removing
the "|| true" on the kubectl wait, or explicitly convert those branches to a
failing exit (e.g., replace the echo-only failure branch with a construct that
echoes an error and exits non-zero) so that failures in the helm
upgrade/installation (the helm upgrade --install invocation) or pod readiness
check (kubectl wait) cause the make target to fail and stop subsequent targets
like install-maestro-all.
In `@README.md`:
- Around line 147-165: Update the README environment variable table to match the
Makefile: rename the MAESTRO_NS entry to MAESTRO_NAMESPACE (keeping the value
`maestro`) and change the KIND_CLUSTER_NAME default value from `hyperfleet` to
`dev-cluster` so the documented defaults for MAESTRO_NAMESPACE and
KIND_CLUSTER_NAME match the actual Makefile variables used by env.kind.
- Around line 206-220: The README currently documents generated Helm values only
for HELMFILE_ENV=gcp and HELMFILE_ENV=kind; update this section to explicitly
include both e2e-gcp and e2e-kind variants and their broker mappings so users
know which generation commands and output directories apply: state that
HELMFILE_ENV=gcp and HELMFILE_ENV=e2e-gcp use `make install-terraform` to
generate Google Pub/Sub values into `generated-values-from-terraform` (topic
names, project ID, subscription IDs, Workload Identity bindings), and that
HELMFILE_ENV=kind and HELMFILE_ENV=e2e-kind use `make generate-rabbitmq-values`
to generate RabbitMQ values into `generated-values-rabbitmq` (RabbitMQ URL,
exchanges, queues, routing keys); keep references to the exact command names
(`make install-terraform`, `make generate-rabbitmq-values`) and file dirs so
Helmfile consumers load the correct broker values for each environment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 6f48069e-37f5-49ca-ae2d-763e4fc157cb
📒 Files selected for processing (61)
.gitignoreCONTRIBUTING.mdMakefileREADME.mdenv.gcpenv.kindhelm/adapter1/Chart.yamlhelm/adapter2/Chart.yamlhelm/adapter2/dryrun-adapter2-discovery.jsonhelm/adapter3/Chart.yamlhelm/api/values.yamlhelm/rabbitmq/Chart.yamlhelm/rabbitmq/README.mdhelm/rabbitmq/templates/_helpers.tplhelm/rabbitmq/templates/deployment.yamlhelm/rabbitmq/templates/service.yamlhelm/rabbitmq/values.yamlhelm/sentinel-clusters/Chart.yamlhelm/sentinel-clusters/values.yamlhelm/sentinel-nodepools/Chart.yamlhelm/sentinel-nodepools/values.yamlhelmfile/configs/base/adapters/adapter1/adapter-config.yamlhelmfile/configs/base/adapters/adapter1/adapter-task-config.yamlhelmfile/configs/base/adapters/adapter2/adapter-config.yamlhelmfile/configs/base/adapters/adapter2/adapter-task-config.yamlhelmfile/configs/base/adapters/adapter3/adapter-config.yamlhelmfile/configs/base/adapters/adapter3/adapter-task-config.yamlhelmfile/configs/e2e/adapters/cl-deployment/adapter-config.yamlhelmfile/configs/e2e/adapters/cl-deployment/adapter-task-config.yamlhelmfile/configs/e2e/adapters/cl-deployment/adapter-task-resource-deployment.yamlhelmfile/configs/e2e/adapters/cl-job/adapter-config.yamlhelmfile/configs/e2e/adapters/cl-job/adapter-task-config.yamlhelmfile/configs/e2e/adapters/cl-job/adapter-task-resource-job.yamlhelmfile/configs/e2e/adapters/cl-maestro/adapter-config.yamlhelmfile/configs/e2e/adapters/cl-maestro/adapter-task-config.yamlhelmfile/configs/e2e/adapters/cl-maestro/adapter-task-resource-manifestwork.yamlhelmfile/configs/e2e/adapters/cl-namespace/adapter-config.yamlhelmfile/configs/e2e/adapters/cl-namespace/adapter-task-config.yamlhelmfile/configs/e2e/adapters/np-configmap/adapter-config.yamlhelmfile/configs/e2e/adapters/np-configmap/adapter-task-config.yamlhelmfile/configs/e2e/adapters/np-configmap/adapter-task-resource-configmap.yamlhelmfile/configs/e2e/adapters/np-configmap/values.yamlhelmfile/environments/e2e-gcp/adapter-configs.yaml.gotmplhelmfile/environments/e2e-gcp/sentinel-configs.yamlhelmfile/environments/e2e-kind/adapter-configs.yaml.gotmplhelmfile/environments/e2e-kind/sentinel-configs.yamlhelmfile/environments/gcp/adapter-configs.yaml.gotmplhelmfile/environments/gcp/sentinel-configs.yaml.gotmplhelmfile/environments/kind/adapter-configs.yaml.gotmplhelmfile/environments/kind/sentinel-configs.yaml.gotmplhelmfile/helmfile.yaml.gotmplhelmfile/values/.gitkeephelmfile/values/base-adapter.yaml.gotmplhelmfile/values/base-api.yaml.gotmplhelmfile/values/base-sentinel.yaml.gotmplscripts/generate-rabbitmq-values.shscripts/kind-build-images.shscripts/tf-helm-values.shterraform/helm-values-files.tfterraform/outputs.tfterraform/versions.tf
💤 Files with no reviewable changes (10)
- helm/adapter3/Chart.yaml
- helm/sentinel-clusters/values.yaml
- helm/sentinel-nodepools/values.yaml
- helm/sentinel-nodepools/Chart.yaml
- helm/api/values.yaml
- helm/adapter1/Chart.yaml
- scripts/tf-helm-values.sh
- helm/sentinel-clusters/Chart.yaml
- helm/adapter2/Chart.yaml
- helm/adapter2/dryrun-adapter2-discovery.json
✅ Files skipped from review due to trivial changes (9)
- helmfile/configs/e2e/adapters/np-configmap/adapter-config.yaml
- helm/rabbitmq/Chart.yaml
- helmfile/configs/e2e/adapters/cl-deployment/adapter-config.yaml
- helm/rabbitmq/values.yaml
- helmfile/environments/e2e-kind/sentinel-configs.yaml
- helm/rabbitmq/README.md
- .gitignore
- helmfile/configs/e2e/adapters/cl-job/adapter-config.yaml
- helmfile/configs/e2e/adapters/np-configmap/adapter-task-resource-configmap.yaml
🚧 Files skipped from review as they are similar to previous changes (26)
- env.gcp
- helmfile/environments/kind/adapter-configs.yaml.gotmpl
- helmfile/environments/gcp/sentinel-configs.yaml.gotmpl
- helmfile/values/base-api.yaml.gotmpl
- env.kind
- helmfile/environments/kind/sentinel-configs.yaml.gotmpl
- helmfile/values/base-sentinel.yaml.gotmpl
- terraform/outputs.tf
- scripts/kind-build-images.sh
- helmfile/configs/e2e/adapters/cl-maestro/adapter-config.yaml
- helmfile/environments/e2e-gcp/adapter-configs.yaml.gotmpl
- helmfile/environments/gcp/adapter-configs.yaml.gotmpl
- scripts/generate-rabbitmq-values.sh
- helmfile/configs/e2e/adapters/cl-namespace/adapter-config.yaml
- helmfile/configs/e2e/adapters/np-configmap/values.yaml
- helmfile/configs/e2e/adapters/np-configmap/adapter-task-config.yaml
- terraform/versions.tf
- helmfile/values/base-adapter.yaml.gotmpl
- helmfile/configs/e2e/adapters/cl-job/adapter-task-config.yaml
- helmfile/environments/e2e-gcp/sentinel-configs.yaml
- helmfile/configs/e2e/adapters/cl-namespace/adapter-task-config.yaml
- terraform/helm-values-files.tf
- helmfile/configs/e2e/adapters/cl-deployment/adapter-task-config.yaml
- helmfile/configs/e2e/adapters/cl-maestro/adapter-task-config.yaml
- helmfile/helmfile.yaml.gotmpl
- helm/rabbitmq/templates/_helpers.tpl
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@AGENTS.md`:
- Line 66: AGENTS.md still references removed commands/scripts and obsolete
environment vars despite renaming MAESTRO_NAMESPACE; update the guide to finish
migrating to the Helmfile workflow by removing references to
scripts/tf-helm-values.sh, make lint-helm, make ci-dry-run, make install-all,
install-all-rabbitmq and obsolete vars BROKER_TYPE and GCP_PROJECT_ID, replacing
them with the current Helmfile commands and variables used by the repo (use the
helmfile target names and values used elsewhere in the repo), update any example
invocations to call helmfile/Makefile targets that exist, and scan the file
(also around the other occurrence noted at line ~110) to ensure no other stale
references remain so the doc is the single source of truth.
In `@env.gcp`:
- Around line 25-26: The NAMESPACE assignment for the e2e branch builds a suffix
from $(USER) but only lowercases it, which can produce invalid DNS-1123 names;
update the NAMESPACE generation (the ifeq block that sets NAMESPACE when
HELMFILE_ENV contains e2e) to sanitize the $(USER) suffix by: lowercasing,
replacing any character not in [a-z0-9] with '-', collapsing consecutive '-'
into a single '-', trimming leading/trailing '-' and then truncating the suffix
so that when prefixed by "hyperfleet-e2e-" the full name is ≤63 characters, and
finally trimming again after truncation to remove any trailing '-' left by the
cut. Ensure these transformations are applied inline in the shell expression
used to compute NAMESPACE.
In
`@helmfile/configs/e2e/adapters/np-configmap/adapter-task-resource-configmap.yaml`:
- Around line 15-17: The template is using dotted keys (nodepool.id,
nodepool.name, cluster.id) which don't match the camelCase keys expected by the
CEL status checks (e.g., resources.?resource0.?data.?nodepoolId.hasValue());
change the keys to nodepoolId, nodepoolName and clusterId while keeping the same
templated values (e.g., "{{ .nodepoolId }}") so the CEL expressions can find the
data fields correctly.
In `@Makefile`:
- Around line 68-76: The create-kind-cluster and delete-kind-cluster targets
call the external tool kind directly but lack a check-kind prerequisite; add a
new phony target check-kind that verifies the kind binary is present (e.g., test
-x $(shell command -v kind) or similar), fail with a clear message if missing,
and then declare both create-kind-cluster and delete-kind-cluster to depend on
check-kind so the Makefile contract for external tools is satisfied; reference
the targets create-kind-cluster, delete-kind-cluster and the new check-kind when
making the change.
- Around line 80-90: Refactor the Makefile so check-helmfile-env only validates
local prerequisites (helmfile/helm + generated-values dirs) and no longer
depends on check-kubectl-context; add a new phony target
check-helmfile-apply-env that depends on check-helmfile-env and
check-kubectl-context; keep template-helmfile, build-helmfile, and lint-helmfile
depending on the new lightweight check-helmfile-env so they can run offline, and
update all cluster-affecting install targets (e.g., install-hyperfleet,
install-api, install-sentinels, install-adapters, install-rabbitmq) to depend on
check-helmfile-apply-env while leaving existing uninstall-* targets unchanged.
- Around line 236-239: Add a missing check-kind Makefile target and rewire
dependencies so kind commands are validated: create a check-kind target that
verifies the `kind` binary and make `create-kind-cluster` and
`delete-kind-cluster` depend on `check-kind`; leave `check-helm` as-is (the
current `command -v helmfile --version` check is fine). Also decouple helmfile
render/lint from kubectl by removing the `check-kubectl-context` dependency from
`check-helmfile-env` (or introduce a new `check-helmfile-env-nokubectl` target)
and have `template-helmfile`, `build-helmfile`, and `lint-helmfile` depend on
the kubectl-independent check so helmfile render/lint work without requiring
`kubectl config current-context`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 888d8fe1-6df3-4ffc-9194-ef18568b89fe
📒 Files selected for processing (62)
.gitignoreAGENTS.mdCONTRIBUTING.mdMakefileREADME.mdenv.gcpenv.kindhelm/adapter1/Chart.yamlhelm/adapter2/Chart.yamlhelm/adapter2/dryrun-adapter2-discovery.jsonhelm/adapter3/Chart.yamlhelm/api/values.yamlhelm/rabbitmq/Chart.yamlhelm/rabbitmq/README.mdhelm/rabbitmq/templates/_helpers.tplhelm/rabbitmq/templates/deployment.yamlhelm/rabbitmq/templates/service.yamlhelm/rabbitmq/values.yamlhelm/sentinel-clusters/Chart.yamlhelm/sentinel-clusters/values.yamlhelm/sentinel-nodepools/Chart.yamlhelm/sentinel-nodepools/values.yamlhelmfile/configs/base/adapters/adapter1/adapter-config.yamlhelmfile/configs/base/adapters/adapter1/adapter-task-config.yamlhelmfile/configs/base/adapters/adapter2/adapter-config.yamlhelmfile/configs/base/adapters/adapter2/adapter-task-config.yamlhelmfile/configs/base/adapters/adapter3/adapter-config.yamlhelmfile/configs/base/adapters/adapter3/adapter-task-config.yamlhelmfile/configs/e2e/adapters/cl-deployment/adapter-config.yamlhelmfile/configs/e2e/adapters/cl-deployment/adapter-task-config.yamlhelmfile/configs/e2e/adapters/cl-deployment/adapter-task-resource-deployment.yamlhelmfile/configs/e2e/adapters/cl-job/adapter-config.yamlhelmfile/configs/e2e/adapters/cl-job/adapter-task-config.yamlhelmfile/configs/e2e/adapters/cl-job/adapter-task-resource-job.yamlhelmfile/configs/e2e/adapters/cl-maestro/adapter-config.yamlhelmfile/configs/e2e/adapters/cl-maestro/adapter-task-config.yamlhelmfile/configs/e2e/adapters/cl-maestro/adapter-task-resource-manifestwork.yamlhelmfile/configs/e2e/adapters/cl-namespace/adapter-config.yamlhelmfile/configs/e2e/adapters/cl-namespace/adapter-task-config.yamlhelmfile/configs/e2e/adapters/np-configmap/adapter-config.yamlhelmfile/configs/e2e/adapters/np-configmap/adapter-task-config.yamlhelmfile/configs/e2e/adapters/np-configmap/adapter-task-resource-configmap.yamlhelmfile/configs/e2e/adapters/np-configmap/values.yamlhelmfile/environments/e2e-gcp/adapter-configs.yaml.gotmplhelmfile/environments/e2e-gcp/sentinel-configs.yamlhelmfile/environments/e2e-kind/adapter-configs.yaml.gotmplhelmfile/environments/e2e-kind/sentinel-configs.yamlhelmfile/environments/gcp/adapter-configs.yaml.gotmplhelmfile/environments/gcp/sentinel-configs.yaml.gotmplhelmfile/environments/kind/adapter-configs.yaml.gotmplhelmfile/environments/kind/sentinel-configs.yaml.gotmplhelmfile/helmfile.yaml.gotmplhelmfile/values/.gitkeephelmfile/values/base-adapter.yaml.gotmplhelmfile/values/base-api.yaml.gotmplhelmfile/values/base-sentinel.yaml.gotmplscripts/generate-rabbitmq-values.shscripts/kind-build-images.shscripts/tf-helm-values.shterraform/helm-values-files.tfterraform/outputs.tfterraform/versions.tf
💤 Files with no reviewable changes (10)
- helm/api/values.yaml
- helm/adapter2/dryrun-adapter2-discovery.json
- scripts/tf-helm-values.sh
- helm/adapter2/Chart.yaml
- helm/sentinel-nodepools/Chart.yaml
- helm/sentinel-clusters/Chart.yaml
- helm/adapter1/Chart.yaml
- helm/adapter3/Chart.yaml
- helm/sentinel-nodepools/values.yaml
- helm/sentinel-clusters/values.yaml
✅ Files skipped from review due to trivial changes (7)
- .gitignore
- helmfile/environments/e2e-kind/sentinel-configs.yaml
- helmfile/configs/e2e/adapters/cl-job/adapter-config.yaml
- helm/rabbitmq/README.md
- helm/rabbitmq/values.yaml
- helmfile/configs/e2e/adapters/np-configmap/adapter-config.yaml
- helmfile/values/base-sentinel.yaml.gotmpl
🚧 Files skipped from review as they are similar to previous changes (32)
- env.kind
- helm/rabbitmq/Chart.yaml
- scripts/kind-build-images.sh
- terraform/versions.tf
- helmfile/configs/e2e/adapters/np-configmap/values.yaml
- helmfile/configs/e2e/adapters/cl-job/adapter-task-resource-job.yaml
- helmfile/environments/gcp/sentinel-configs.yaml.gotmpl
- helmfile/values/base-api.yaml.gotmpl
- helmfile/environments/gcp/adapter-configs.yaml.gotmpl
- helmfile/configs/e2e/adapters/cl-maestro/adapter-config.yaml
- scripts/generate-rabbitmq-values.sh
- helmfile/values/base-adapter.yaml.gotmpl
- helmfile/configs/e2e/adapters/cl-deployment/adapter-config.yaml
- helmfile/configs/base/adapters/adapter2/adapter-task-config.yaml
- helmfile/configs/base/adapters/adapter3/adapter-config.yaml
- helmfile/configs/e2e/adapters/cl-maestro/adapter-task-config.yaml
- helmfile/environments/kind/adapter-configs.yaml.gotmpl
- helmfile/configs/e2e/adapters/cl-namespace/adapter-config.yaml
- terraform/outputs.tf
- helmfile/environments/e2e-gcp/sentinel-configs.yaml
- helmfile/configs/base/adapters/adapter1/adapter-task-config.yaml
- helmfile/configs/e2e/adapters/cl-namespace/adapter-task-config.yaml
- helmfile/configs/e2e/adapters/np-configmap/adapter-task-config.yaml
- helmfile/configs/base/adapters/adapter3/adapter-task-config.yaml
- helmfile/environments/e2e-kind/adapter-configs.yaml.gotmpl
- helmfile/configs/e2e/adapters/cl-deployment/adapter-task-resource-deployment.yaml
- helmfile/configs/e2e/adapters/cl-deployment/adapter-task-config.yaml
- helmfile/helmfile.yaml.gotmpl
- helmfile/environments/e2e-gcp/adapter-configs.yaml.gotmpl
- terraform/helm-values-files.tf
- helmfile/environments/kind/sentinel-configs.yaml.gotmpl
- helmfile/configs/e2e/adapters/cl-job/adapter-task-config.yaml
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (4)
Makefile (3)
68-77:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd a
check-kindprerequisite to the kind targets.Both targets invoke the external
kindbinary directly, so missing-tool failures degrade into raw shell errors instead of the standard preflight path.Suggested fix
+# ==== Kind Targets ==== +.PHONY: check-kind +check-kind: ## Verify kind is installed + `@command` -v kind >/dev/null 2>&1 || { echo "ERROR: kind is not installed"; exit 1; } + `@echo` "OK: kind found" + .PHONY: create-kind-cluster -create-kind-cluster: ## Create a new kind cluster +create-kind-cluster: check-kind ## Create a new kind cluster kind create cluster --name $(KIND_CLUSTER_NAME) kind export kubeconfig --name $(KIND_CLUSTER_NAME) --kubeconfig $(KUBECONFIG) `@echo` "OK: created kind cluster $(KIND_CLUSTER_NAME)" @@ .PHONY: delete-kind-cluster -delete-kind-cluster: ## Delete the kind cluster +delete-kind-cluster: check-kind ## Delete the kind cluster kind delete cluster --name $(KIND_CLUSTER_NAME) `@echo` "OK: deleted kind cluster $(KIND_CLUSTER_NAME)"As per coding guidelines, "Add a matching
check-*prerequisite target when introducing external tool dependencies".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` around lines 68 - 77, The make targets create-kind-cluster and delete-kind-cluster call the external kind binary directly; add a prerequisite named check-kind to both targets and implement a check-kind target that verifies the kind executable is present (e.g., using command -v or which) and prints a clear error if missing so failures use the standard preflight path; update the target definitions for create-kind-cluster and delete-kind-cluster to depend on check-kind and add the new check-kind target to the Makefile.
101-112:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not mask a failed Maestro install as success.
This recipe still swallows both
helm upgrade --installfailure andkubectl waitfailure, soinstall-maestro-allcan continue against a broken control plane.Suggested fix
`@echo` "Installing Maestro..." - if ! helm upgrade --install $(DRY_RUN_FLAG) $(MAESTRO_NAMESPACE)-maestro $(HELM_DIR)/maestro \ - --namespace $(MAESTRO_NAMESPACE) \ - --set agent.installWorkCRDs=false \ - --set agent.messageBroker.mqtt.host=maestro-mqtt.$(MAESTRO_NAMESPACE) \ - --wait --timeout 5m ; then \ - echo "Warning: maestro install failed on kind cluster; continuing"; \ - fi; + helm upgrade --install $(DRY_RUN_FLAG) $(MAESTRO_NAMESPACE)-maestro $(HELM_DIR)/maestro \ + --namespace $(MAESTRO_NAMESPACE) \ + --set agent.installWorkCRDs=false \ + --set agent.messageBroker.mqtt.host=maestro-mqtt.$(MAESTRO_NAMESPACE) \ + --wait --timeout 5m `@echo` "Waiting for Maestro pods to be ready..." - `@kubectl` wait --for=condition=ready pod -l app.kubernetes.io/instance=$(MAESTRO_NAMESPACE)-maestro --namespace $(MAESTRO_NAMESPACE) --timeout=180s || true + `@kubectl` wait --for=condition=ready pod -l app.kubernetes.io/instance=$(MAESTRO_NAMESPACE)-maestro --namespace $(MAESTRO_NAMESPACE) --timeout=180s🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` around lines 101 - 112, The install-maestro Makefile target currently masks failures from the helm upgrade --install command and the kubectl wait call; update the install-maestro recipe (target name: install-maestro) to fail fast instead of treating errors as warnings: remove the conditional that echoes "Warning: maestro install failed..." so that a non-zero exit from the helm upgrade --install (the helm command invocation) causes the Makefile to stop, and remove the trailing "|| true" on the kubectl wait invocation so that pod readiness failures also propagate an error. Ensure existing flags (DRY_RUN_FLAG, MAESTRO_NAMESPACE, HELM_DIR) are left intact and no other targets silently continue on these failures.
79-90:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep Helmfile render/lint local-only.
template-helmfile,build-helmfile, andlint-helmfileall route throughcheck-helmfile-env, which currently requires a live kubectl context. That blocks offline validation even though these targets only need local inputs.Suggested fix
.PHONY: check-helmfile-env -check-helmfile-env: check-helm check-kubectl-context check-helmfile-env-generated ## Verify kubectl context and generated values directory exists +check-helmfile-env: check-helm check-helmfile-env-generated ## Verify local helmfile prerequisites + +.PHONY: check-helmfile-apply-env +check-helmfile-apply-env: check-helmfile-env check-kubectl-context ## Verify cluster-targeting helmfile prerequisites @@ -install-hyperfleet: check-helmfile-env ## Install all HyperFleet components +install-hyperfleet: check-helmfile-apply-env ## Install all HyperFleet components @@ -install-api: check-helmfile-env ## Install HyperFleet API +install-api: check-helmfile-apply-env ## Install HyperFleet API @@ -install-sentinels: check-helmfile-env ## Install Hyperfleet Sentinels +install-sentinels: check-helmfile-apply-env ## Install Hyperfleet Sentinels @@ -install-adapters: check-helmfile-env ## Install Hyperfleet Adapters +install-adapters: check-helmfile-apply-env ## Install Hyperfleet AdaptersAlso applies to: 252-295
CONTRIBUTING.md (1)
259-272:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix the cleanup examples so they are valid shell.
export HELMFILE_ENV=gcp or HELMFILE_ENV=e2e-gcpand the kind variant are not executable shell syntax, so the cleanup flow is still not copy/pasteable.Suggested fix
-export HELMFILE_ENV=gcp or HELMFILE_ENV=e2e-gcp +export HELMFILE_ENV=gcp # or: export HELMFILE_ENV=e2e-gcp @@ -export HELMFILE_ENV=kind or HELMFILE_ENV=e2e-kind +export HELMFILE_ENV=kind # or: export HELMFILE_ENV=e2e-kind🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CONTRIBUTING.md` around lines 259 - 272, Replace the invalid shell examples for setting HELMFILE_ENV with valid export commands: instead of the literal "export HELMFILE_ENV=gcp or HELMFILE_ENV=e2e-gcp" (and the kind variant), show a single valid assignment like "export HELMFILE_ENV=gcp" and include a trailing inline comment or parenthetical note indicating the alternative value ("or e2e-gcp"); keep the existing use of export NAMESPACE and the subsequent cleanup commands (make uninstall-hyperfleet, make uninstall-maestro, make destroy-terraform) unchanged so the snippet becomes copy/pasteable shell.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CONTRIBUTING.md`:
- Around line 186-193: Move the export of the NAMESPACE environment variable so
it appears before any kubectl commands that reference ${NAMESPACE};
specifically, ensure the line "export NAMESPACE=<e2e_namespace>" is placed above
the kubectl port-forward invocations (e.g., the lines using "kubectl
port-forward -n ${NAMESPACE} svc/hyperfleet-api ..." and any other kubectl or
curl commands that dereference ${NAMESPACE}), and update the second E2E snippet
the same way (the block that mirrors lines 221-228) so both examples export
NAMESPACE first to avoid using an unset/wrong namespace.
In `@helmfile/configs/e2e/adapters/cl-job/adapter-task-resource-job.yaml`:
- Around line 16-29: The Job manifest's pod/container runs with default
privileges and a writable root FS; update the spec for the Job to add a minimal
pod-level and container-level securityContext: set podSecurityContext (e.g.,
runAsNonRoot: true and fsGroup), and on the container `hello-world` add
securityContext with runAsNonRoot: true, runAsUser (non-root uid),
readOnlyRootFilesystem: true, allowPrivilegeEscalation: false, drop all
capabilities, and a seccompProfile (RuntimeDefault) to ensure the container
cannot escalate privileges or write the root filesystem while keeping
resources/command unchanged; modify the `spec` and the `containers` entry
accordingly to reference these fields.
In `@Makefile`:
- Around line 162-168: The install-rabbitmq target currently runs for any
HELMFILE_ENV; update the install-rabbitmq Makefile target to validate
HELMFILE_ENV and refuse to run unless it is one of the allowed dev values (e.g.,
"kind" or "e2e-kind"): in the install-rabbitmq recipe check the HELMFILE_ENV
variable early (using a shell conditional or Makefile conditional) and print an
error and exit non‑zero if it is not an allowed value, otherwise proceed to run
the existing helm upgrade command (referencing install-rabbitmq, HELMFILE_ENV,
DRY_RUN_FLAG, HELM_DIR, and NAMESPACE to locate the logic to change).
In `@README.md`:
- Around line 76-80: The README claims .env overrides environment defaults but
the Makefile currently only includes env.gcp/env.kind; update the Makefile so
.env is loaded with higher precedence (sourced/applied before
env.$(HELMFILE_ENV) is read) or change the README to reflect current behavior.
Specifically, in the Makefile's variable loading logic for the
install-hyperfleet target (and any places where NAMESPACE or HELMFILE_ENV are
set), add a step to source or include .env if present before including
env.gcp/env.kind so command-line vars (NAMESPACE=..., HELMFILE_ENV=...) still
have highest priority, then .env, then env.$(HELMFILE_ENV), then Makefile
defaults.
---
Duplicate comments:
In `@CONTRIBUTING.md`:
- Around line 259-272: Replace the invalid shell examples for setting
HELMFILE_ENV with valid export commands: instead of the literal "export
HELMFILE_ENV=gcp or HELMFILE_ENV=e2e-gcp" (and the kind variant), show a single
valid assignment like "export HELMFILE_ENV=gcp" and include a trailing inline
comment or parenthetical note indicating the alternative value ("or e2e-gcp");
keep the existing use of export NAMESPACE and the subsequent cleanup commands
(make uninstall-hyperfleet, make uninstall-maestro, make destroy-terraform)
unchanged so the snippet becomes copy/pasteable shell.
In `@Makefile`:
- Around line 68-77: The make targets create-kind-cluster and
delete-kind-cluster call the external kind binary directly; add a prerequisite
named check-kind to both targets and implement a check-kind target that verifies
the kind executable is present (e.g., using command -v or which) and prints a
clear error if missing so failures use the standard preflight path; update the
target definitions for create-kind-cluster and delete-kind-cluster to depend on
check-kind and add the new check-kind target to the Makefile.
- Around line 101-112: The install-maestro Makefile target currently masks
failures from the helm upgrade --install command and the kubectl wait call;
update the install-maestro recipe (target name: install-maestro) to fail fast
instead of treating errors as warnings: remove the conditional that echoes
"Warning: maestro install failed..." so that a non-zero exit from the helm
upgrade --install (the helm command invocation) causes the Makefile to stop, and
remove the trailing "|| true" on the kubectl wait invocation so that pod
readiness failures also propagate an error. Ensure existing flags (DRY_RUN_FLAG,
MAESTRO_NAMESPACE, HELM_DIR) are left intact and no other targets silently
continue on these failures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 99badae1-aec4-4e30-8b8c-be12530a6339
📒 Files selected for processing (62)
.gitignoreAGENTS.mdCONTRIBUTING.mdMakefileREADME.mdenv.gcpenv.kindhelm/adapter1/Chart.yamlhelm/adapter2/Chart.yamlhelm/adapter2/dryrun-adapter2-discovery.jsonhelm/adapter3/Chart.yamlhelm/api/values.yamlhelm/rabbitmq/Chart.yamlhelm/rabbitmq/README.mdhelm/rabbitmq/templates/_helpers.tplhelm/rabbitmq/templates/deployment.yamlhelm/rabbitmq/templates/service.yamlhelm/rabbitmq/values.yamlhelm/sentinel-clusters/Chart.yamlhelm/sentinel-clusters/values.yamlhelm/sentinel-nodepools/Chart.yamlhelm/sentinel-nodepools/values.yamlhelmfile/configs/base/adapters/adapter1/adapter-config.yamlhelmfile/configs/base/adapters/adapter1/adapter-task-config.yamlhelmfile/configs/base/adapters/adapter2/adapter-config.yamlhelmfile/configs/base/adapters/adapter2/adapter-task-config.yamlhelmfile/configs/base/adapters/adapter3/adapter-config.yamlhelmfile/configs/base/adapters/adapter3/adapter-task-config.yamlhelmfile/configs/e2e/adapters/cl-deployment/adapter-config.yamlhelmfile/configs/e2e/adapters/cl-deployment/adapter-task-config.yamlhelmfile/configs/e2e/adapters/cl-deployment/adapter-task-resource-deployment.yamlhelmfile/configs/e2e/adapters/cl-job/adapter-config.yamlhelmfile/configs/e2e/adapters/cl-job/adapter-task-config.yamlhelmfile/configs/e2e/adapters/cl-job/adapter-task-resource-job.yamlhelmfile/configs/e2e/adapters/cl-maestro/adapter-config.yamlhelmfile/configs/e2e/adapters/cl-maestro/adapter-task-config.yamlhelmfile/configs/e2e/adapters/cl-maestro/adapter-task-resource-manifestwork.yamlhelmfile/configs/e2e/adapters/cl-namespace/adapter-config.yamlhelmfile/configs/e2e/adapters/cl-namespace/adapter-task-config.yamlhelmfile/configs/e2e/adapters/np-configmap/adapter-config.yamlhelmfile/configs/e2e/adapters/np-configmap/adapter-task-config.yamlhelmfile/configs/e2e/adapters/np-configmap/adapter-task-resource-configmap.yamlhelmfile/configs/e2e/adapters/np-configmap/values.yamlhelmfile/environments/e2e-gcp/adapter-configs.yaml.gotmplhelmfile/environments/e2e-gcp/sentinel-configs.yamlhelmfile/environments/e2e-kind/adapter-configs.yaml.gotmplhelmfile/environments/e2e-kind/sentinel-configs.yamlhelmfile/environments/gcp/adapter-configs.yaml.gotmplhelmfile/environments/gcp/sentinel-configs.yaml.gotmplhelmfile/environments/kind/adapter-configs.yaml.gotmplhelmfile/environments/kind/sentinel-configs.yaml.gotmplhelmfile/helmfile.yaml.gotmplhelmfile/values/.gitkeephelmfile/values/base-adapter.yaml.gotmplhelmfile/values/base-api.yaml.gotmplhelmfile/values/base-sentinel.yaml.gotmplscripts/generate-rabbitmq-values.shscripts/kind-build-images.shscripts/tf-helm-values.shterraform/helm-values-files.tfterraform/outputs.tfterraform/versions.tf
💤 Files with no reviewable changes (10)
- helm/adapter2/dryrun-adapter2-discovery.json
- helm/sentinel-nodepools/values.yaml
- helm/api/values.yaml
- helm/adapter3/Chart.yaml
- helm/sentinel-clusters/Chart.yaml
- helm/sentinel-nodepools/Chart.yaml
- helm/sentinel-clusters/values.yaml
- scripts/tf-helm-values.sh
- helm/adapter1/Chart.yaml
- helm/adapter2/Chart.yaml
✅ Files skipped from review due to trivial changes (10)
- helm/rabbitmq/values.yaml
- helmfile/values/base-adapter.yaml.gotmpl
- helmfile/configs/e2e/adapters/np-configmap/adapter-config.yaml
- helmfile/configs/base/adapters/adapter3/adapter-config.yaml
- helmfile/environments/e2e-gcp/sentinel-configs.yaml
- helmfile/configs/e2e/adapters/np-configmap/values.yaml
- helmfile/environments/e2e-kind/sentinel-configs.yaml
- AGENTS.md
- helmfile/configs/e2e/adapters/cl-job/adapter-config.yaml
- helm/rabbitmq/README.md
🚧 Files skipped from review as they are similar to previous changes (28)
- helmfile/configs/e2e/adapters/cl-namespace/adapter-config.yaml
- .gitignore
- helmfile/values/base-api.yaml.gotmpl
- helm/rabbitmq/Chart.yaml
- helmfile/environments/kind/sentinel-configs.yaml.gotmpl
- env.kind
- helmfile/environments/gcp/sentinel-configs.yaml.gotmpl
- helmfile/configs/base/adapters/adapter3/adapter-task-config.yaml
- helmfile/environments/e2e-kind/adapter-configs.yaml.gotmpl
- helmfile/configs/e2e/adapters/np-configmap/adapter-task-resource-configmap.yaml
- helmfile/environments/kind/adapter-configs.yaml.gotmpl
- helmfile/configs/base/adapters/adapter2/adapter-task-config.yaml
- helmfile/values/base-sentinel.yaml.gotmpl
- helmfile/configs/e2e/adapters/cl-deployment/adapter-config.yaml
- scripts/kind-build-images.sh
- terraform/outputs.tf
- helm/rabbitmq/templates/_helpers.tpl
- terraform/versions.tf
- helmfile/helmfile.yaml.gotmpl
- helmfile/configs/base/adapters/adapter1/adapter-task-config.yaml
- helmfile/configs/e2e/adapters/np-configmap/adapter-task-config.yaml
- helmfile/configs/e2e/adapters/cl-deployment/adapter-task-config.yaml
- helmfile/configs/e2e/adapters/cl-maestro/adapter-task-config.yaml
- terraform/helm-values-files.tf
- env.gcp
- helmfile/environments/e2e-gcp/adapter-configs.yaml.gotmpl
- helmfile/configs/e2e/adapters/cl-maestro/adapter-config.yaml
- scripts/generate-rabbitmq-values.sh
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (8)
helmfile/configs/e2e/adapters/cl-job/adapter-task-resource-job.yaml (1)
16-29:⚠️ Potential issue | 🟠 MajorSecurity context still missing (duplicate).
The past review flagged missing pod and container
securityContext(KSV-0014, KSV-0118). The Job still runs with default privileges and writable root FS.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helmfile/configs/e2e/adapters/cl-job/adapter-task-resource-job.yaml` around lines 16 - 29, Add hardened pod and container security contexts to the Job spec to address KSV-0014/KSV-0118: under spec (pod-level) add securityContext with runAsNonRoot: true and runAsUser (non-zero UID), and under the container definition for name "hello-world" add securityContext with readOnlyRootFilesystem: true, allowPrivilegeEscalation: false, runAsNonRoot: true, runAsUser matching the pod-level UID, and a capabilities section that drops all (and only adds if absolutely required). Also ensure any required seccompProfile/apparmor identifiers are set if your cluster requires them.CONTRIBUTING.md (2)
186-193:⚠️ Potential issue | 🟠 Major | ⚡ Quick winExport
NAMESPACEbefore any command that dereferences it.Both E2E snippets use
${NAMESPACE}inkubectlcommands before the docs tell the reader to set it, so copy/paste can hit the wrong namespace or fail outright.Suggested fix
# For E2E testing on kind - example: +export NAMESPACE=<e2e_namespace> export API_LOCAL_PORT=8000 export MAESTRO_LOCAL_PORT=8001 kubectl port-forward -n maestro svc/maestro ${MAESTRO_LOCAL_PORT}:8000 & kubectl port-forward -n ${NAMESPACE} svc/hyperfleet-api ${API_LOCAL_PORT}:8000 & -export NAMESPACE=<e2e_namespace> export MAESTRO_URL=http://localhost:${MAESTRO_LOCAL_PORT} export HYPERFLEET_API_URL=http://localhost:${API_LOCAL_PORT} @@ # For E2E testing on GKE - example: +export NAMESPACE=<e2e_namespace> kubectl patch svc maestro -n maestro -p '{"spec":{"type":"LoadBalancer"}}' export MAESTRO_EXTERNAL_IP=$(kubectl get svc maestro -n maestro -o jsonpath='{.status.loadBalancer.ingress[0].ip}') export MAESTRO_URL=http://${MAESTRO_EXTERNAL_IP}:8000 export API_EXTERNAL_IP=$(kubectl get svc hyperfleet-api -n $NAMESPACE -o jsonpath='{.status.loadBalancer.ingress[0].ip}') export HYPERFLEET_API_URL=http://${API_EXTERNAL_IP}:8000 -export NAMESPACE=<e2e_namespace>Also applies to: 221-228
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CONTRIBUTING.md` around lines 186 - 193, The kubectl port-forward commands reference ${NAMESPACE} before it's defined; move or add the export NAMESPACE=<e2e_namespace> line so the environment variable is set before any kubectl port-forward that uses ${NAMESPACE} (and apply the same change to the other E2E snippet that uses ${NAMESPACE} around the block referenced as 221-228); ensure the examples consistently define MAESTRO_LOCAL_PORT/API_LOCAL_PORT and export NAMESPACE prior to any command that dereferences it (look for the export NAMESPACE, MAESTRO_URL, HYPERFLEET_API_URL and kubectl port-forward lines to reorder).
259-272:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix the cleanup examples so they are executable as written.
export HELMFILE_ENV=gcp or HELMFILE_ENV=e2e-gcpis not valid shell, so these cleanup blocks cannot be copy/pasted successfully.Suggested fix
-export HELMFILE_ENV=gcp or HELMFILE_ENV=e2e-gcp +export HELMFILE_ENV=gcp # or: export HELMFILE_ENV=e2e-gcp @@ -export HELMFILE_ENV=kind or HELMFILE_ENV=e2e-kind +export HELMFILE_ENV=kind # or: export HELMFILE_ENV=e2e-kind🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CONTRIBUTING.md` around lines 259 - 272, The README shows non-executable lines like "export HELMFILE_ENV=gcp or HELMFILE_ENV=e2e-gcp"; update both Environment blocks to use valid shell export syntax (e.g., set HELMFILE_ENV to a single value such as "gcp" or "e2e-gcp" or show two separate example lines) and clarify NAMESPACE usage; specifically edit the text around the HELMFILE_ENV export and the blocks containing make uninstall-hyperfleet, make uninstall-maestro, and make destroy-terraform so the examples are copy/paste runnable (refer to the HELMFILE_ENV export lines and the make destroy-terraform / make uninstall-* commands).README.md (2)
206-220:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDocument the E2E environments in the generated-values flow.
This section narrows the broker setup to
gcpandkind, bute2e-gcpande2e-kindare also first-class Helmfile environments. As written, E2E users can follow the wrong setup path.As per coding guidelines, "Validate changes against HyperFleet architecture standards from the linked architecture repository" and "keep broker backend selection explicit via HELMFILE_ENV (gcp/e2e-gcp => Pub/Sub+Terraform; kind/e2e-kind => RabbitMQ via helm/rabbitmq/ chart)."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 206 - 220, Update the "Generated Helm Values" section to explicitly include the e2e environments so users pick the correct broker flow: state that HELMFILE_ENV values map as follows — gcp and e2e-gcp use Google Pub/Sub + Terraform via make install-terraform and terraform/helm-values-files.tf writing to generated-values-from-terraform/, while kind and e2e-kind use RabbitMQ generated by make generate-rabbitmq-values and written to generated-values-rabbitmq/ (RabbitMQ URL, exchanges, queues, routing keys). Also add a short note reminding readers to keep broker backend selection explicit via HELMFILE_ENV (gcp/e2e-gcp => Pub/Sub+Terraform; kind/e2e-kind => RabbitMQ via helm/rabbitmq chart) to align with HyperFleet architecture standards.
76-80:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThe documented precedence still doesn’t match the Makefile.
The Makefile only includes
env.gcp/env.kind, so advertising a.envlayer tells operators an override mechanism exists when it is ignored.Suggested fix
Configuration precedence (highest to lowest): 1. Command-line variables: `NAMESPACE=my-ns HELMFILE_ENV=gcp make install-hyperfleet` -2. `.env` file (if it exists) -3. Environment-specific files (`env.gcp` or `env.kind`) -4. Makefile defaults +2. Environment-specific files (`env.gcp` or `env.kind`) +3. Makefile defaultsMakefile (3)
155-161:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject non-RabbitMQ environments in
install-rabbitmq.The target is documented as
kind/e2e-kindonly, but it currently installs the dev RabbitMQ chart for anyHELMFILE_ENV, including GCP flows.Suggested fix
.PHONY: install-rabbitmq install-rabbitmq: check-helmfile-env check-hyperfleet-namespace ## Install RabbitMQ (HELMFILE_ENV=kind/e2e-kind only) +ifeq ($(filter kind e2e-kind,$(HELMFILE_ENV)),) + `@echo` "ERROR: install-rabbitmq only supports HELMFILE_ENV=kind or e2e-kind" + `@exit` 1 +else `@echo` "Installing RabbitMQ..." helm upgrade --install $(DRY_RUN_FLAG) rabbitmq $(HELM_DIR)/rabbitmq \ --namespace $(NAMESPACE) \ --wait --timeout 3m `@echo` "OK: RabbitMQ is ready" +endifAs per coding guidelines, "Validate changes against HyperFleet architecture standards from the linked architecture repository" and "keep broker backend selection explicit via HELMFILE_ENV (gcp/e2e-gcp => Pub/Sub+Terraform; kind/e2e-kind => RabbitMQ via helm/rabbitmq/ chart)."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` around lines 155 - 161, The install-rabbitmq target should refuse to run unless HELMFILE_ENV is an allowed RabbitMQ environment; modify the Makefile target install-rabbitmq to validate $(HELMFILE_ENV) (or fallback env variable used) and error out with a clear message if it is not "kind" or "e2e-kind" (or the exact allowed strings used elsewhere), so the Helm chart is only installed in RabbitMQ-appropriate flows; reference the install-rabbitmq target and the HELMFILE_ENV variable when adding this guard and ensure the check runs before invoking helm upgrade --install.
94-106:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t turn a failed Maestro install into success.
This target still swallows both the
helm upgrade --installfailure and the pod readiness failure, soinstall-maestrocan printOKwith no working Maestro release.Suggested fix
install-maestro: check-helm check-kubectl check-maestro-namespace install-applied-manifest-crd ## Install Maestro (server + agent) helm dependency update $(HELM_DIR)/maestro `@echo` "Installing Maestro..." - if ! helm upgrade --install $(DRY_RUN_FLAG) $(MAESTRO_NAMESPACE)-maestro $(HELM_DIR)/maestro \ - --namespace $(MAESTRO_NAMESPACE) \ - --set agent.installWorkCRDs=false \ - --set agent.messageBroker.mqtt.host=maestro-mqtt.$(MAESTRO_NAMESPACE) \ - --wait --timeout 5m ; then \ - echo "Warning: maestro install failed on kind cluster; continuing"; \ - fi; + helm upgrade --install $(DRY_RUN_FLAG) $(MAESTRO_NAMESPACE)-maestro $(HELM_DIR)/maestro \ + --namespace $(MAESTRO_NAMESPACE) \ + --set agent.installWorkCRDs=false \ + --set agent.messageBroker.mqtt.host=maestro-mqtt.$(MAESTRO_NAMESPACE) \ + --wait --timeout 5m `@echo` "Waiting for Maestro pods to be ready..." - `@kubectl` wait --for=condition=ready pod -l app.kubernetes.io/instance=$(MAESTRO_NAMESPACE)-maestro --namespace $(MAESTRO_NAMESPACE) --timeout=180s || true + `@kubectl` wait --for=condition=ready pod -l app.kubernetes.io/instance=$(MAESTRO_NAMESPACE)-maestro --namespace $(MAESTRO_NAMESPACE) --timeout=180s `@echo` "OK: Maestro pods are ready"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` around lines 94 - 106, The install-maestro Makefile target currently swallows failures from the helm upgrade --install command and from kubectl wait, allowing the target to report "OK" even on failure; modify the install-maestro target so that the helm command's exit code is propagated (remove the if ... then ... fi that converts failure into a warning) and remove the "|| true" from the kubectl wait invocation so pod readiness failures cause the target to fail; keep the existing echo messages but let make return a non-zero status when either the helm upgrade or kubectl wait commands fail (references: install-maestro target, the helm upgrade --install invocation, and the kubectl wait --for=condition=ready pod invocation).
73-83:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDecouple local Helmfile validation from cluster-context checks.
template-helmfile,build-helmfile, andlint-helmfileall funnel throughcheck-helmfile-env, which currently requireskubectl config current-context. That blocks local render/lint even though those paths only need local inputs.Suggested fix
-.PHONY: check-helmfile-env -check-helmfile-env: check-helmfile check-kubectl-context check-helmfile-env-generated ## Verify kubectl context and generated values directory exists +.PHONY: check-helmfile-env +check-helmfile-env: check-helmfile check-helmfile-env-generated ## Verify local Helmfile prerequisites + +.PHONY: check-helmfile-apply-env +check-helmfile-apply-env: check-helmfile-env check-kubectl-context ## Verify cluster context for apply/destroy flows @@ -install-hyperfleet: check-helmfile-env ## Install all HyperFleet components +install-hyperfleet: check-helmfile-apply-env ## Install all HyperFleet components @@ -install-api: check-helmfile-env ## Install HyperFleet API +install-api: check-helmfile-apply-env ## Install HyperFleet API @@ -install-sentinels: check-helmfile-env ## Install Hyperfleet Sentinels +install-sentinels: check-helmfile-apply-env ## Install Hyperfleet Sentinels @@ -install-adapters: check-helmfile-env ## Install Hyperfleet Adapters +install-adapters: check-helmfile-apply-env ## Install Hyperfleet AdaptersAlso applies to: 193-207, 249-250
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` around lines 73 - 83, The three targets template-helmfile, build-helmfile, and lint-helmfile should not require cluster context via check-helmfile-env; create a new check target (e.g., check-helmfile-local) that only validates local prerequisites (HELMFILE_ENV is set and helmfile templates exist) and does not run kubectl config current-context, then change the dependencies of template-helmfile, build-helmfile, and lint-helmfile to depend on check-helmfile-local instead of check-helmfile-env; also replace other occurrences (the other targets mentioned around the file) that perform local-only operations to use check-helmfile-local so cluster-context checks remain only on targets that actually need kubectl (leave check-helmfile-env for cluster-aware targets).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@helmfile/configs/e2e/adapters/cl-deployment/adapter-task-resource-deployment.yaml`:
- Line 26: The Deployment is using an unpinned image and lacks securityContext;
change the image from nginx:latest to a specific, supported tag (e.g.
nginx:1.xx.x or use nginxinc/nginx-unprivileged) and add Pod/Container
securityContext entries on the Deployment spec (the pod template and container
for this resource) to enforce running as non-root (runAsNonRoot: true and
runAsUser: <non-zero>), set readOnlyRootFilesystem: true at the container level,
and add the required emptyDir volume mounts for /tmp and /var/cache/nginx (or
switch to nginxinc/nginx-unprivileged to avoid extra mounts) so the Deployment
no longer runs as root or with writable root filesystem.
---
Duplicate comments:
In `@CONTRIBUTING.md`:
- Around line 186-193: The kubectl port-forward commands reference ${NAMESPACE}
before it's defined; move or add the export NAMESPACE=<e2e_namespace> line so
the environment variable is set before any kubectl port-forward that uses
${NAMESPACE} (and apply the same change to the other E2E snippet that uses
${NAMESPACE} around the block referenced as 221-228); ensure the examples
consistently define MAESTRO_LOCAL_PORT/API_LOCAL_PORT and export NAMESPACE prior
to any command that dereferences it (look for the export NAMESPACE, MAESTRO_URL,
HYPERFLEET_API_URL and kubectl port-forward lines to reorder).
- Around line 259-272: The README shows non-executable lines like "export
HELMFILE_ENV=gcp or HELMFILE_ENV=e2e-gcp"; update both Environment blocks to use
valid shell export syntax (e.g., set HELMFILE_ENV to a single value such as
"gcp" or "e2e-gcp" or show two separate example lines) and clarify NAMESPACE
usage; specifically edit the text around the HELMFILE_ENV export and the blocks
containing make uninstall-hyperfleet, make uninstall-maestro, and make
destroy-terraform so the examples are copy/paste runnable (refer to the
HELMFILE_ENV export lines and the make destroy-terraform / make uninstall-*
commands).
In `@helmfile/configs/e2e/adapters/cl-job/adapter-task-resource-job.yaml`:
- Around line 16-29: Add hardened pod and container security contexts to the Job
spec to address KSV-0014/KSV-0118: under spec (pod-level) add securityContext
with runAsNonRoot: true and runAsUser (non-zero UID), and under the container
definition for name "hello-world" add securityContext with
readOnlyRootFilesystem: true, allowPrivilegeEscalation: false, runAsNonRoot:
true, runAsUser matching the pod-level UID, and a capabilities section that
drops all (and only adds if absolutely required). Also ensure any required
seccompProfile/apparmor identifiers are set if your cluster requires them.
In `@Makefile`:
- Around line 155-161: The install-rabbitmq target should refuse to run unless
HELMFILE_ENV is an allowed RabbitMQ environment; modify the Makefile target
install-rabbitmq to validate $(HELMFILE_ENV) (or fallback env variable used) and
error out with a clear message if it is not "kind" or "e2e-kind" (or the exact
allowed strings used elsewhere), so the Helm chart is only installed in
RabbitMQ-appropriate flows; reference the install-rabbitmq target and the
HELMFILE_ENV variable when adding this guard and ensure the check runs before
invoking helm upgrade --install.
- Around line 94-106: The install-maestro Makefile target currently swallows
failures from the helm upgrade --install command and from kubectl wait, allowing
the target to report "OK" even on failure; modify the install-maestro target so
that the helm command's exit code is propagated (remove the if ... then ... fi
that converts failure into a warning) and remove the "|| true" from the kubectl
wait invocation so pod readiness failures cause the target to fail; keep the
existing echo messages but let make return a non-zero status when either the
helm upgrade or kubectl wait commands fail (references: install-maestro target,
the helm upgrade --install invocation, and the kubectl wait
--for=condition=ready pod invocation).
- Around line 73-83: The three targets template-helmfile, build-helmfile, and
lint-helmfile should not require cluster context via check-helmfile-env; create
a new check target (e.g., check-helmfile-local) that only validates local
prerequisites (HELMFILE_ENV is set and helmfile templates exist) and does not
run kubectl config current-context, then change the dependencies of
template-helmfile, build-helmfile, and lint-helmfile to depend on
check-helmfile-local instead of check-helmfile-env; also replace other
occurrences (the other targets mentioned around the file) that perform
local-only operations to use check-helmfile-local so cluster-context checks
remain only on targets that actually need kubectl (leave check-helmfile-env for
cluster-aware targets).
In `@README.md`:
- Around line 206-220: Update the "Generated Helm Values" section to explicitly
include the e2e environments so users pick the correct broker flow: state that
HELMFILE_ENV values map as follows — gcp and e2e-gcp use Google Pub/Sub +
Terraform via make install-terraform and terraform/helm-values-files.tf writing
to generated-values-from-terraform/, while kind and e2e-kind use RabbitMQ
generated by make generate-rabbitmq-values and written to
generated-values-rabbitmq/ (RabbitMQ URL, exchanges, queues, routing keys). Also
add a short note reminding readers to keep broker backend selection explicit via
HELMFILE_ENV (gcp/e2e-gcp => Pub/Sub+Terraform; kind/e2e-kind => RabbitMQ via
helm/rabbitmq chart) to align with HyperFleet architecture standards.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: ae627ce7-462f-4842-803f-84c14eb757de
📒 Files selected for processing (66)
.gitignoreAGENTS.mdCONTRIBUTING.mdMakefileREADME.mdenv.gcpenv.kindhelm/adapter1/Chart.yamlhelm/adapter1/values.yamlhelm/adapter2/Chart.yamlhelm/adapter2/dryrun-adapter2-discovery.jsonhelm/adapter2/values.yamlhelm/adapter3/Chart.yamlhelm/adapter3/values.yamlhelm/api/Chart.yamlhelm/api/values.yamlhelm/rabbitmq/Chart.yamlhelm/rabbitmq/README.mdhelm/rabbitmq/templates/_helpers.tplhelm/rabbitmq/templates/deployment.yamlhelm/rabbitmq/templates/service.yamlhelm/rabbitmq/values.yamlhelm/sentinel-clusters/Chart.yamlhelm/sentinel-clusters/values.yamlhelm/sentinel-nodepools/Chart.yamlhelm/sentinel-nodepools/values.yamlhelmfile/configs/base/adapters/adapter1/adapter-config.yamlhelmfile/configs/base/adapters/adapter1/adapter-task-config.yamlhelmfile/configs/base/adapters/adapter2/adapter-config.yamlhelmfile/configs/base/adapters/adapter2/adapter-task-config.yamlhelmfile/configs/base/adapters/adapter3/adapter-config.yamlhelmfile/configs/base/adapters/adapter3/adapter-task-config.yamlhelmfile/configs/e2e/adapters/cl-deployment/adapter-config.yamlhelmfile/configs/e2e/adapters/cl-deployment/adapter-task-config.yamlhelmfile/configs/e2e/adapters/cl-deployment/adapter-task-resource-deployment.yamlhelmfile/configs/e2e/adapters/cl-job/adapter-config.yamlhelmfile/configs/e2e/adapters/cl-job/adapter-task-config.yamlhelmfile/configs/e2e/adapters/cl-job/adapter-task-resource-job.yamlhelmfile/configs/e2e/adapters/cl-maestro/adapter-config.yamlhelmfile/configs/e2e/adapters/cl-maestro/adapter-task-config.yamlhelmfile/configs/e2e/adapters/cl-maestro/adapter-task-resource-manifestwork.yamlhelmfile/configs/e2e/adapters/cl-namespace/adapter-config.yamlhelmfile/configs/e2e/adapters/cl-namespace/adapter-task-config.yamlhelmfile/configs/e2e/adapters/np-configmap/adapter-config.yamlhelmfile/configs/e2e/adapters/np-configmap/adapter-task-config.yamlhelmfile/configs/e2e/adapters/np-configmap/adapter-task-resource-configmap.yamlhelmfile/configs/e2e/adapters/np-configmap/values.yamlhelmfile/environments/e2e-gcp/adapter-configs.yaml.gotmplhelmfile/environments/e2e-gcp/sentinel-configs.yamlhelmfile/environments/e2e-kind/adapter-configs.yaml.gotmplhelmfile/environments/e2e-kind/sentinel-configs.yamlhelmfile/environments/gcp/adapter-configs.yaml.gotmplhelmfile/environments/gcp/sentinel-configs.yaml.gotmplhelmfile/environments/kind/adapter-configs.yaml.gotmplhelmfile/environments/kind/sentinel-configs.yaml.gotmplhelmfile/helmfile.yaml.gotmplhelmfile/values/.gitkeephelmfile/values/base-adapter.yaml.gotmplhelmfile/values/base-api.yaml.gotmplhelmfile/values/base-sentinel.yaml.gotmplscripts/generate-rabbitmq-values.shscripts/kind-build-images.shscripts/tf-helm-values.shterraform/helm-values-files.tfterraform/outputs.tfterraform/versions.tf
💤 Files with no reviewable changes (14)
- helm/adapter3/values.yaml
- helm/api/Chart.yaml
- helm/sentinel-clusters/values.yaml
- helm/sentinel-clusters/Chart.yaml
- helm/adapter3/Chart.yaml
- helm/adapter2/values.yaml
- helm/adapter2/Chart.yaml
- helm/sentinel-nodepools/values.yaml
- helm/adapter1/values.yaml
- helm/sentinel-nodepools/Chart.yaml
- helm/api/values.yaml
- helm/adapter2/dryrun-adapter2-discovery.json
- scripts/tf-helm-values.sh
- helm/adapter1/Chart.yaml
✅ Files skipped from review due to trivial changes (9)
- helmfile/configs/e2e/adapters/np-configmap/values.yaml
- helmfile/environments/e2e-gcp/sentinel-configs.yaml
- helmfile/configs/base/adapters/adapter3/adapter-config.yaml
- helm/rabbitmq/values.yaml
- .gitignore
- helmfile/configs/e2e/adapters/np-configmap/adapter-config.yaml
- helm/rabbitmq/Chart.yaml
- AGENTS.md
- helm/rabbitmq/README.md
🚧 Files skipped from review as they are similar to previous changes (31)
- helmfile/environments/e2e-kind/sentinel-configs.yaml
- helmfile/environments/gcp/sentinel-configs.yaml.gotmpl
- helmfile/values/base-api.yaml.gotmpl
- helmfile/configs/e2e/adapters/cl-job/adapter-config.yaml
- env.kind
- helmfile/configs/e2e/adapters/cl-deployment/adapter-config.yaml
- helmfile/values/base-adapter.yaml.gotmpl
- helmfile/configs/e2e/adapters/cl-namespace/adapter-config.yaml
- scripts/kind-build-images.sh
- helmfile/environments/gcp/adapter-configs.yaml.gotmpl
- helmfile/configs/e2e/adapters/np-configmap/adapter-task-resource-configmap.yaml
- helmfile/environments/kind/sentinel-configs.yaml.gotmpl
- helmfile/environments/kind/adapter-configs.yaml.gotmpl
- terraform/helm-values-files.tf
- helmfile/configs/base/adapters/adapter1/adapter-task-config.yaml
- helmfile/configs/e2e/adapters/cl-maestro/adapter-config.yaml
- terraform/outputs.tf
- helmfile/values/base-sentinel.yaml.gotmpl
- helm/rabbitmq/templates/_helpers.tpl
- terraform/versions.tf
- helmfile/environments/e2e-gcp/adapter-configs.yaml.gotmpl
- helmfile/environments/e2e-kind/adapter-configs.yaml.gotmpl
- env.gcp
- helmfile/configs/e2e/adapters/cl-maestro/adapter-task-config.yaml
- helmfile/configs/e2e/adapters/np-configmap/adapter-task-config.yaml
- helmfile/configs/e2e/adapters/cl-deployment/adapter-task-config.yaml
- helmfile/configs/base/adapters/adapter2/adapter-task-config.yaml
- helmfile/configs/e2e/adapters/cl-job/adapter-task-config.yaml
- helmfile/configs/e2e/adapters/cl-namespace/adapter-task-config.yaml
- scripts/generate-rabbitmq-values.sh
- helmfile/helmfile.yaml.gotmpl
ma-hill
force-pushed
the
HYPERFLEET-1056
branch
3 times, most recently
from
15ba698 to
41bb909
Compare
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
Migrates infrastructure deployment from individual Makefile targets to Helmfile-based orchestration with environment-specific configurations. Previously, deploying HyperFleet required running separate make targets for each component with broker configuration generated by shell scripts. This introduces Helmfile as the deployment orchestrator, enabling declarative multi-component deployments across four environments (gcp, kind, e2e-gcp, e2e-kind) with environment-aware configuration via env files. The change reduces deployment complexity, adds E2E testing support for both GCP and kind clusters, and standardizes the deployment workflow across local development and CI/CD pipelines.
HYPERFLEET-1056
Changes
Helmfile Infrastructure
helmfile/helmfile.yaml.gotmplas the central deployment orchestration configuration with support for four environments (gcp, kind, e2e-gcp, e2e-kind)helmfile/environments/with separate configs for adapter configs, sentinel configs, and broker settings per environmenthelmfile/values/for API, Sentinel, and Adapter components with go templating for environment-aware substitutionhelmfile/configs/e2e/adapters/including cl-deployment, cl-job, cl-maestro, cl-namespace, and np-configmap test adaptershelm/adapter*/tohelmfile/configs/base/adapters/maintaining the same structureEnvironment Configuration
env.gcpwith defaults for GCP deployments (Google Pub/Sub broker, LoadBalancer service type, registry.ci.openshift.org registry)env.kindwith defaults for kind deployments (RabbitMQ broker, ClusterIP service type, IfNotPresent image pull policy)HELMFILE_ENVvalue (e.g.,hyperfleet-e2e-$USERfor e2e-gcp).envfile → environment-specific files (env.gcp/env.kind) → Makefile defaultsMakefile Refactoring
install-hyperfleet,uninstall-hyperfleet, and component-group targets (install-api, install-sentinels, install-adapters)HELMFILE_ENVvariable to select deployment environment (defaults togcp, auto-sources corresponding env file)template-helmfile,build-helmfile,lint-helmfilecreate-kind-cluster,delete-kind-clustertf-helm-valuesshell script invocation from Makefile (replaced by Terraform's built-inlocal_fileresources inhelm-values-files.tf)RabbitMQ Chart and Configuration
helm/rabbitmq/chart with deployment, service, and helper templates for self-hosted RabbitMQ in kind environmentsscripts/generate-rabbitmq-values.shto generate RabbitMQ broker configuration for kind deployments (replaces logic that was intf-helm-values.sh)manifests/rabbitmq.yamlstatic manifest (replaced by Helm chart)brokerType: "rabbitmq"is set in environment configTerraform Changes
terraform/helm-values-files.tfto auto-generate Helm values for Google Pub/Sub using Terraform'slocal_fileresourceapi_helm_values_yaml,sentinel_helm_values_yaml,adapter_helm_values_yamlto support the new file generation approachscripts/tf-helm-values.shfor Terraform-based value generation (shell script logic migrated to Terraform HCL)Documentation Updates
README.mdwith new Helmfile-based workflow, four deployment environments table, and environment configuration precedence rulesCONTRIBUTING.mdwith separate deployment workflows for kind and GKE, including E2E testing setup for both platformshelm/rabbitmq/README.mddocumenting the local RabbitMQ chart and its configurationscripts/kind-build-images.shwith usage documentation for building and loading images to kind clustersinstall-all,install-all-rabbitmqtargets in favor ofinstall-hyperfleetwithHELMFILE_ENVRemoved Files
helm/adapter1/Chart.yaml,helm/adapter2/Chart.yaml,helm/adapter3/Chart.yaml(moved to helmfile umbrella chart references)helm/sentinel-clusters/Chart.yaml,helm/sentinel-nodepools/Chart.yaml(moved to helmfile umbrella chart references)helm/api/values.yaml,helm/sentinel-*/values.yaml(replaced by helmfile value templates)helm/adapter2/dryrun-adapter2-discovery.json(unused test file)scripts/tf-helm-values.sh(functionality split between Terraform local_file resources and generate-rabbitmq-values.sh).gitignore Updates
generated-values-rabbitmq/to gitignore for RabbitMQ-generated Helm valuesNotes
This change introduces Helmfile as a new prerequisite for deployments. Users must install helmfile (
brew install helmfileor via other package managers) in addition to existing helm, kubectl, and terraform requirements.The old Makefile targets (
install-all,install-all-rabbitmq) are removed. The new workflow usesHELMFILE_ENV=<env> make install-hyperfleetinstead. See the updated CONTRIBUTING.md for migration examples.Maestro installation remains independent and is not managed by Helmfile. Use
make install-maestrobefore deploying HyperFleet components.Test Plan
make test-allpassesmake lintpassesmake template-helmfilefor gcp, kind, e2e-gcp, e2e-kind)make lint-helmfile)make validate-terraform)HELMFILE_ENV=kind make install-hyperfleet)HELMFILE_ENV=gcp make install-hyperfleet)HELMFILE_ENV=e2e-kind)HELMFILE_ENV=e2e-gcp)make generate-rabbitmq-values)