Skip to content

chore: main -> test#466

Closed
RamanKharchee wants to merge 45 commits into
testfrom
main
Closed

chore: main -> test#466
RamanKharchee wants to merge 45 commits into
testfrom
main

Conversation

@RamanKharchee

Copy link
Copy Markdown
Contributor

Summary

Type of change

  • Bug fix (non-breaking)
  • New feature (non-breaking)
  • Breaking change (chart upgrade requires user action)
  • Documentation only
  • CI / tooling

Chart version

  • I bumped version in charts/nudgebee-agent/Chart.yaml (required for user-visible changes)
  • No version bump needed (docs/CI only)

Test plan

  • helm lint charts/nudgebee-agent passes
  • ct lint --config .github/configs/ct.yaml passes
  • helm template output reviewed
  • Installed on a real/kind cluster and verified the change

Related issues

mayankpande88 and others added 30 commits February 11, 2026 18:30
…393)

* feat: add enableWritePermissions flag for runner service account

Adds runner.enableWritePermissions (default: false) that grants
additional write permissions needed for kubectl executor, workload
lifecycle management, and node operations.

When enabled, adds: node delete, service/secret/namespace/endpoint/
serviceaccount writes, resourcequotas, limitranges, statefulset
scaling, workload creation (apps group), ingress/networkpolicy writes,
RBAC role management, and rollout create/delete.

* chore: update image tags for main release

  Updated image tags to latest versions from ECR for main branch.

  🤖 Generated with [Claude Code](https://claude.ai/code)

  Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
These scans are now handled server-side by ml-k8s-server via
Vertical Rightsizing Refresh and Volume Rightsizing Refresh Hasura crons.
Removing agent-side scheduling to avoid duplicate execution.
  Updated image tags to latest versions from ECR for main branch.

  🤖 Generated with [Claude Code](https://claude.ai/code)

  Co-Authored-By: Claude <noreply@anthropic.com>
fix: remove agent-side krr_scan and volume_analyzer cron schedules
Without --no-paginate, the JMESPath query runs per-page, returning
multiple image tags. The multi-line value causes InvalidImageTag errors.
fix: add --no-paginate to nudgebee-agent ECR query
feat(chart): migrate to Go nudgebee-agent, drop Robusta plumbing
#424)

* chore(chart): remove grafanaRenderer sidecar and kubewatch sync config

- grafanaRenderer was a server-side rendering sidecar used by Robusta to
  generate Grafana graph snapshots for Findings. Go agent doesn't use it;
  default was enableContainer: false anyway.
- kubewatch.config.sync: unused block (workload snapshot + scheduled sync
  knobs). Not consumed by the fork's current config schema.

* chore: update image tags for main release

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
fix(chart): conditional MUTATE_ENABLED + rename /etc/robusta path
Brings the Go-agent cutover (#422, #424, #425) into test, dropping
test's Robusta-era keys that overlapped with deleted blocks.

Resolution notes:
- prometheus-alert-manager-config.yaml: deleted (per #422 — inert in
  typical installs due to alertmanagerConfigNamespaceSelector mismatch).
- prometheus-alert-rule.yaml: kept test's rate/ratio-based formulas
  (from #417), removed dead victoria_metrics_enabled gate, and applied
  Gemini code-review fix to ApplicationAPIFailures denominator
  (aggregate by container_id+method+path so a per-endpoint failure
  isn't diluted by container-wide traffic).
- values.yaml: took main's cleaned version (Robusta plumbing gone),
  added back nodeAgent.service block that daemonset.yaml expects.
- workflows: took main's GHCR-based tag-bump path.
- Chart.yaml: bumped to 0.1.1 (next 0.1.x after main's 0.1.0).
The script built the helm command as a quoted string and ran it via
\`eval \$a\`. Two real problems with that pattern:

- **Command injection**: any flag value containing shell metacharacters
  ($, backticks, ;, \$(...)) gets evaluated by eval as part of the
  command. A malicious -a / -p / -s value runs arbitrary commands.
- **Correctness**: even with trusted operators, legitimate values
  containing spaces, quotes, or commas silently misbehave — values
  files with spaces in the path, Grafana passwords with shell-special
  characters, Prometheus URLs with query strings, etc.

Refactor each optional flag block into a (possibly empty) bash array,
then build the helm command with \`"\${arr[@]}"\` expansion. Bash
preserves quoting per-element, empty arrays expand to nothing, and
no eval is needed.

Addresses Gemini review comment on PR #427 (installation.sh:298).
The bitnami clickhouse pod already has fsGroup: 1001, which K8s applies
via the CSI driver before the main container starts. The chown init
container (running bitnami-os-shell as root) was doing the same work
manually — leftover defense for storage backends that don't honor
fsGroup. Modern CSI drivers (EBS, GKE PD, Azure Disk, etc.) all do.

Also drops the empty-dir extraVolume that only existed to give the
init container a /tmp scratch space, and removes one of the remaining
registry.nudgebee.com/* image dependencies (bitnami-os-shell).
* chore: update image tags for prod release

  Updated image tags to latest versions from ECR for prod branch.

  🤖 Generated with [Claude Code](https://claude.ai/code)

  Co-Authored-By: Claude <noreply@anthropic.com>

* chore: update image tags for prod release

  Updated image tags to latest versions from ECR for prod branch.

  🤖 Generated with [Claude Code](https://claude.ai/code)

  Co-Authored-By: Claude <noreply@anthropic.com>

* fix: switch OTel Collector ClickHouse exporter from native TCP to HTTP

Use HTTP interface (port 8123) instead of native TCP (port 9000) to
eliminate false-positive "deprecated interserver protocol" warnings
from ClickHouse logs.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix: fix for version

* chore: update image tags for prod release

  Updated image tags to latest versions from ECR for prod branch.

  🤖 Generated with [Claude Code](https://claude.ai/code)

  Co-Authored-By: Claude <noreply@anthropic.com>

* Merge pull request #392 from nudgebee/feat/node-agent-headless-service

feat: add optional headless service for node-agent prometheus scraping

* fix: set loki datasource isDefault=false to prevent Grafana crash

loki-stack chart creates a Loki datasource ConfigMap with isDefault: true
by default. Combined with kube-prometheus-stack's Prometheus datasource
(also isDefault: true), Grafana fails during provisioning with:
"Only one datasource per organization can be marked as default"

* chore: update image tags for prod release

  Updated image tags to latest versions from ECR for prod branch.

  🤖 Generated with [Claude Code](https://claude.ai/code)

  Co-Authored-By: Claude <noreply@anthropic.com>

* style: quote namespace variable and improve readability of loki helm command

* chore: bump chart version to 0.0.117

* fix: readme

* chore: updated release version

* chore: update image tags for prod release

  Updated image tags to latest versions from ECR for prod branch.

  🤖 Generated with [Claude Code](https://claude.ai/code)

  Co-Authored-By: Claude <noreply@anthropic.com>

* fix: expose rate_limit params for crash loop and image pull backoff triggers

Rate limit defaults were hidden in Python trigger classes, making it
hard for customers to discover and override them. Now explicitly set
in values.yaml with inline comments.

* chore: update image tags for prod release

  Updated image tags to latest versions from ECR for prod branch.

  🤖 Generated with [Claude Code](https://claude.ai/code)

  Co-Authored-By: Claude <noreply@anthropic.com>

* chore: updated version

* chore: update image tags for prod release

  Updated image tags to latest versions from ECR for prod branch.

  🤖 Generated with [Claude Code](https://claude.ai/code)

  Co-Authored-By: Claude <noreply@anthropic.com>

* fix: add --no-paginate to nudgebee-agent ECR query in release workflows

Without --no-paginate, the JMESPath query runs per-page, returning
multiple image tags. The multi-line value causes InvalidImageTag errors.

* chore: update image tags for prod release

  Updated image tags to latest versions from ECR for prod branch.

  🤖 Generated with [Claude Code](https://claude.ai/code)

  Co-Authored-By: Claude <noreply@anthropic.com>

* chore: updated version

* fix: default runner.clickhouse_enabled to false to prevent secret lookup failure

When opentelemetry-collector.enabled is set to false, the ClickHouse
subchart is not deployed and its secret does not exist. The runner
template OR condition (otel-collector.enabled OR clickhouse_enabled)
still evaluated to true due to the old default, causing the runner pod
to fail on a missing ClickHouse secret.

* chore: bump chart version to 0.0.121

* fix: remove invalid matchers field from AlertmanagerConfig receiver

matchers is not part of the receiver schema in v1alpha1, causing helm
install/upgrade to fail with "field not declared in schema".

* chore: update image tags for prod release

  Updated image tags to latest versions from ECR for prod branch.

  🤖 Generated with [Claude Code](https://claude.ai/code)

  Co-Authored-By: Claude <noreply@anthropic.com>

* ci: add kubeconform validation and helm template rendering to CI

- Add helm template + kubeconform steps to both dev and prod workflows
  to validate rendered manifests against K8s and CRD schemas (strict mode)
- Enable validate-chart-schema in ct.yaml
- Add missing dependency repos and helm dependency build step
- Bump action versions (checkout@v4, setup-helm@v4, setup-python@v5,
  chart-testing-action@v2.7.0, Helm v3.14.0)
- Default create_nb_alert_config to false

* feat: add new flags to installation script for self-hosted and component control

Add 6 new flags for self-hosted/on-prem deployments and component toggling:
- `-w` relay WebSocket address
- `-c` collector endpoint URL
- `-i` custom image registry
- `-x` disable OpenCost
- `-t` disable OpenTelemetry Collector and ClickHouse
- `-g` disable Prometheus stack (skips auto-discovery/install)

Fixes nudgebee/nudgebee#10415

* fix: reduce alert-rule noise with rate/ratio-based thresholds

The default PrometheusRule shipped by this chart was generating
hundreds of firings per day on real clusters, with most carrying
no actionable signal. The four rules used absolute-count or sparse
expressions that fire on benign log lines and routine 4xx responses.

Re-tune each rule against signal characteristics rather than
arbitrary thresholds, validated against historical data on a dev
cluster (24-hour window):

  HighErrorCriticalLogs    250 -> 18 distinct containers (-93%)
  ApplicationAPIFailures   68  -> 19 distinct containers (-72%)
  KubePodStuckTerminating  73  -> 31 distinct pods       (-58%)
  KubeHpaMaxedOut          unchanged

Changes per rule:

* HighErrorCriticalLogs: was `increase(err logs)[5m] > 1` — fires
  on a single error message in 5 minutes. Replaced with deviation
  from baseline: current 5m rate must exceed 3x the same metric
  at `[1h] offset 1h`. The offset prevents an ongoing spike from
  inflating the baseline and silencing sustained incidents. An
  activity floor of 0.1 err/s and a `baseline > 0` guard avoid
  ratio blowups on idle and cold-start containers. The container_id
  exclusion regex now also drops log-collection infrastructure
  (fluent-bit, fluentd, loki, opensearch, elasticsearch, logstash,
  datadog, newrelic, otel, promtail) — these emit steady error-level
  log volume from transport/ingest failures and are not app signal.
  Severity demoted to `warning`: log-level alerting is structurally
  noisy (apps lie about levels) and almost always co-occurs with a
  higher-confidence signal.

* ApplicationAPIFailures: was `increase(4xx|5xx)[5m] > 1` — fires
  on a single request failure regardless of total volume. Replaced
  with a 5xx rate ratio: `5xx rate / total rate > 5%` plus an
  activity floor of 0.1 req/s. 4xx is removed entirely — those are
  client errors (bad request, unauthorised) and don't signal an
  application failure. `for` raised from 5m to 10m to ride over slow
  rolling updates.

* KubePodStuckTerminating: original expression
  `count(kube_pod_deletion_timestamp) * count(NodeLost==0) > 0`
  fires for any deleting pod, including pods in normal graceful
  termination. Replaced with an age-based expression
  `(time() - max_over_time(kube_pod_deletion_timestamp[20m])) > 1800`,
  i.e. pods stuck for 30 minutes — 60x the Kubernetes default
  terminationGracePeriodSeconds. `max_over_time(...[20m])` smooths
  across kube-state-metrics scrape gaps so the alert stays firing
  rather than flapping on every missed scrape (this was the main
  cause of the 537 state changes per 24h observed previously).
  The NodeLost suppression uses the cleaner `unless on(...) ... == 1`
  idiom. Severity demoted to `warning` — a single per-pod page for a
  stuck pod is rarely 3am-actionable and usually resolves with a
  manual force-delete.

* KubeHpaMaxedOut: unchanged behaviourally; only whitespace cleaned.

Container-id exclusion regexes for the log and HTTP rules are now
defined once at the top of the template via Helm template variables
(`$excludeLog`, `$excludeApi`) and reused, so additions stay in sync
across the duplicated denominator/numerator selectors.

Each new expression was tested directly against VictoriaMetrics
(which serves the same PromQL surface) to confirm the queries parse
and produce the expected reduction in firings.

* chore: bump chart version to 0.0.123

* fix: replace probabilistic with tail sampling and add memory limiter to otel pipeline

The shipped otel-collector pipeline was uniformly dropping 90% of all
traces via probabilistic_sampler regardless of status or latency. That
discards 90% of error and slow traces — the exact traces an SRE wants
to see — alongside the noise it was meant to remove. Replace with
tail_sampling: keep 100% of error traces, 100% of traces above 1s, and
sample 10% of healthy traffic.

Other improvements bundled with the sampling rewrite:

* memory_limiter is now defined AND threaded into all three pipelines
  as the first processor, so the collector back-pressures receivers
  under memory load instead of OOM-killing.

* filter/drop_health_check rewritten from 15 hardcoded path equality
  checks (covering /health, /healthz, /live, /ready, /metrics across
  three OTel attribute conventions) to three regex matches that also
  catch /livez, /readyz, /ping, /status, /api/health, /healthcheck,
  and namespaced variants.

* filter/drop_invalid_timestamps added to the traces pipeline. Drops
  spans with zero or inverted timestamps from broken upstream
  instrumentation.

Trade-off: tail_sampling holds spans for `decision_wait` (10s) before
deciding, costing ~50-100MB per replica with `num_traces: 50000`.
Constraint: tail sampling requires all spans of a trace to land on
the same collector instance. The chart's default deployment is a
single replica so works out of the box; customers scaling to >1
replica must add a consistent-hash load balancer (otel `loadbalancing`
exporter) in front, otherwise sampling decisions are made on partial
traces. Documented inline in values.yaml.

* fix: aggregate 5xx ratio across status codes in ApplicationAPIFailures

The numerator was grouping by (container_id, method, path, status) which
splits 500/502/503/etc into independent buckets. A service with 6% total
5xx but a 3% 500 + 3% 503 split would not fire because neither individual
status code crosses the 5% threshold. Drop status from the grouping so
the ratio measures aggregate 5xx rate per (container, method, path).

Reported by gemini-code-assist on PR #417.

* fix(otel): make filter/drop_health_check OTTL conditions parse

The bare IsMatch() calls introduced in 0.0.123 fail to parse with the
filterprocessor's OTTL grammar, which requires conditions to be of the
form `<expr> <op> <value>`. The collector crash-loops with:

  processors::filter/drop_health_check: unable to parse OTTL statement:
  unexpected token "<EOF>" (expected <opcomparison> Value)

Append `== true` to each IsMatch() condition. Bumps chart to 0.0.124.

* chore: update image tags for prod release

  Updated image tags to latest versions from ECR for prod branch.

  🤖 Generated with [Claude Code](https://claude.ai/code)

  Co-Authored-By: Claude <noreply@anthropic.com>

* fix(install): replace eval-based helm command with bash array

The script built the helm command as a quoted string and ran it via
\`eval \$a\`. Two real problems with that pattern:

- **Command injection**: any flag value containing shell metacharacters
  ($, backticks, ;, \$(...)) gets evaluated by eval as part of the
  command. A malicious -a / -p / -s value runs arbitrary commands.
- **Correctness**: even with trusted operators, legitimate values
  containing spaces, quotes, or commas silently misbehave — values
  files with spaces in the path, Grafana passwords with shell-special
  characters, Prometheus URLs with query strings, etc.

Refactor each optional flag block into a (possibly empty) bash array,
then build the helm command with \`"\${arr[@]}"\` expansion. Bash
preserves quoting per-element, empty arrays expand to nothing, and
no eval is needed.

Addresses Gemini review comment on PR #427 (installation.sh:298).

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Shiv <3078106+blue4209211@users.noreply.github.com>
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
The README only linked to external docs. Add a helm command + reference
to installation.sh so users can install from the README directly.

Uses `helm upgrade --install` (idempotent) and quotes the auth-key
placeholder so values with shell-special characters work.
* fix(chart): default runner.es.url and runner.signoz.url to empty

Chart defaulted runner.es.url to a hardcoded
"https://elasticsearch-es-internal-http.monitoring.svc:9200" and
runner.signoz.url to "https://signoz.signoz.svc:8080". The runner-secret
conditionally emits ELASTICSEARCH_URL / SIGNOZ_URL only when those
values are set — but since they always were set (to a guessed default),
every install ended up with both env vars present. The Go agent then
enabled ES/SigNoz handling and tried to connect to non-existent services
on most clusters.

Default both to "" so they're opt-in. Customers who actually have ES or
SigNoz set the URL explicitly; everyone else gets clean defaults.

* chore: update image tags for main release

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
)

* fix(chart): wire globalConfig.prometheus_url to PROMETHEUS_URL env

The Go agent reads PROMETHEUS_URL from the environment
(pkg/config/config.go:127). The cutover removed the chart's
globalConfig.prometheus_url plumbing — customers who set
globalConfig.prometheus_url (installation.sh does, dev-gke values
does) were having it silently dropped.

Restore globalConfig.prometheus_url + globalConfig.prometheus_headers
as documented keys, and surface them as PROMETHEUS_URL /
PROMETHEUS_HEADERS in the runner-secret. Conditional render — keys
only appear when set.

* chore: update image tags for main release

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* chore: prepare repo for open-source release

- add Apache-2.0 LICENSE, CONTRIBUTING, SECURITY, CODE_OF_CONDUCT
- add CODEOWNERS, PR template, issue templates, dependabot config
- expand README with components table, prerequisites, data disclosure, badges
- Chart.yaml: add home/sources/maintainers/keywords/artifacthub annotations
- values.yaml: repoint registry.nudgebee.com images to public ghcr.io/nudgebee
- values.yaml: drop hardcoded ClickHouse password; subchart auto-generates

* chore: update image tags for main release

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* feat(runner): add runner component source and CI

Add the Go source for the runner under runner/, alongside the Helm chart
that deploys it. The runner is the in-cluster agent that connects to the
NudgeBee backend over WebSocket and executes diagnostic and remediation
actions.

Adds path-scoped CI (runner/**): a multi-arch image build/push and a
lint + test workflow (gofmt, go vet, golangci-lint, race tests).

The runner image name and Go module path are unchanged, so the chart's
image-tag tooling needs no changes.

* fix(runner): address automated review findings

- events lister: sort by LastSeen desc before capping (previously returned
  an arbitrary subset, not the most recent)
- snake: iterate a rune slice once (fixes byte/rune index mismatch on
  multi-byte input and removes per-iteration []rune realloc)
- kube exec: guard nil ProcessState (panic when the binary fails to start)
- /api/actions: cap request body at 1 MiB via http.MaxBytesReader
- discovery: Forget keys on deletion/skip/success paths to bound the
  rate limiter's backoff map
…bot (#434)

* chore(repo): hygiene — gitignore, workflow names/permissions, dependabot

- .gitignore: add OS/editor cruft, packaged charts, coverage output, and
  root-anchored patterns for local-only secret/data files
- helm-dev-lint / helm-prod-test: distinct names (both were 'CI')
- all workflows: top-level read-only default permissions (write-needing
  jobs already declare job-level scopes)
- dependabot: add gomod + docker ecosystems for runner/

* ci: add OpenSSF Scorecard workflow + badge

* chore: update image tags for main release

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* ci: harden workflows for OpenSSF Scorecard

Address Dangerous-Workflow, Pinned-Dependencies, and Token-Permissions
findings from the OpenSSF Scorecard.

- update-image-tags.yml: fix script injection. github.head_ref /
  github.base_ref are attacker-controllable PR inputs; move them out of
  inline ${{ }} in run:/github-script into env vars referenced as quoted
  "$HEAD_REF"/$BASE_REF (shell) and ${process.env.BASE_REF} (JS).
- Pin all GitHub Actions to commit SHAs (# vX comments; Dependabot keeps
  them current).
- Pin runner/Dockerfile base images to multi-arch manifest-list digests
  (golang:1.26-alpine, alpine:3.23).
- Add top-level `permissions: contents: read` to runner-image.yaml and
  runner-lint-test.yaml (the only workflows still defaulting to write-all);
  job-level packages:write escalation preserved.

* ci: add CodeQL workflow for Go (runner/) and Actions

Default-setup CodeQL doesn't pick up the runner because Linguist
classifies this repo as Go Template + Shell (the Helm chart dominates).
Explicit workflow scans runner/ Go code and the workflow files
themselves on push, PR, and weekly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: shiv <blue.4209211@gmail.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bumps [helm/chart-releaser-action](https://github.com/helm/chart-releaser-action) from 1.4.1 to 1.7.0.
- [Release notes](https://github.com/helm/chart-releaser-action/releases)
- [Commits](helm/chart-releaser-action@98bccfd...cae68fe)

---
updated-dependencies:
- dependency-name: helm/chart-releaser-action
  dependency-version: 1.7.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* fix(runner): resolve pod owner up to controlling Deployment

The Pod converter forwarded raw pod.OwnerReferences as-is, so any
Deployment-owned pod was emitted with `owner = [{kind: "ReplicaSet",
name: "<deploy>-<hash>"}]`. The collector keys
k8s_pods.workload_{type,name} off owner[0], so the backend stopped
writing any rows with workload_type="Deployment" — every UI/triage
query that filters by Deployment returns empty.

Fix the resolution at emit time using the ReplicaSet informer (already
registered in RegisterAll). When the immediate owner is a ReplicaSet,
read the RS from cache and emit the RS's own controller ownerReference
(typically the Deployment) instead. Authoritative — handles RSes
created outside the Deployment controller correctly and doesn't depend
on the controller's pod-template-hash naming staying stable.

- converters.go: add replicaSetLookupFn type, controllerOwner helper,
  and ownerInfosWithRSLookup(owners, namespace, rsLookup) variant.
- service.go: split convertPod into newPodConverter(rsLookup) factory
  (matching the existing newDeploymentConverter pattern); add
  replicaSetLookup() closure backed by the RS informer indexer; wire
  RegisterPods to use the factory.
- Tests cover three paths: RS→Deployment resolution, cache-miss
  fallback to the immediate RS (startup-race safety), and
  non-ReplicaSet owners (DaemonSet/StatefulSet/Job) passthrough.

* fix(runner): factory-wide cache sync + idiomatic controllerOwner return

Address PR review feedback:

1. Race on auxiliary informers
   Switch Run() from cache.WaitForCacheSync(handler-only) to
   s.factory.WaitForCacheSync(stopCh). The factory tracks every
   informer ever instantiated — including the ReplicaSet/Pod
   informers wired up via the lookup closures — so a caller that
   uses RegisterPods without RegisterReplicaSets no longer races
   the initial pod snapshot against an unsynced RS cache.

2. controllerOwner return shape
   Return (OwnerReference, bool) instead of *OwnerReference. Avoids
   handing out a pointer into a caller-owned slice, matches the
   "comma-ok" idiom, and lets the caller stack-allocate.
- proxy.go: drop arithmetic in make() capacity hint to clear CodeQL
  go/allocation-size-overflow (#45). HTTP header counts can't realistically
  overflow int, but removing the addition makes the analyzer signal go
  away permanently. Net behavior identical — map grows for extras as
  needed.
- go.mod: bump go directive 1.26.0 -> 1.26.3 (CI uses GOTOOLCHAIN auto)
  to pick up stdlib fixes: GO-2026-4866/4869/4870/4946/4947/4971 (x509,
  TLS, tar, net).
- go.mod: bump golang.org/x/net v0.49.0 -> v0.55.0 for GO-2026-4918
  (HTTP/2 transport infinite loop) and GO-2026-5026 (idna).
- go mod tidy bumps x/sys, x/term, x/text as transitive collateral.

govulncheck ./... → No vulnerabilities found (was 8 reachable + 8 latent).
go test -race ./... passes.

Co-authored-by: Shiv <3078106+blue4209211@users.noreply.github.com>
Bumps [helm/chart-testing-action](https://github.com/helm/chart-testing-action) from 2.7.0 to 2.8.0.
- [Release notes](https://github.com/helm/chart-testing-action/releases)
- [Commits](helm/chart-testing-action@0d28d31...6ec842c)

---
updated-dependencies:
- dependency-name: helm/chart-testing-action
  dependency-version: 2.8.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Mayank Pande <mayankpande88@gmail.com>
Bumps [actions/setup-python](https://github.com/actions/setup-python) from 5.6.0 to 6.2.0.
- [Release notes](https://github.com/actions/setup-python/releases)
- [Commits](actions/setup-python@a26af69...a309ff8)

---
updated-dependencies:
- dependency-name: actions/setup-python
  dependency-version: '6'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Mayank Pande <mayankpande88@gmail.com>
Bumps [actions/checkout](https://github.com/actions/checkout) from 3.6.0 to 6.0.2.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v3.6.0...de0fac2)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-version: '6'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Mayank Pande <mayankpande88@gmail.com>
* chore(deps): bump azure/setup-helm from 3.5 to 5

Bumps [azure/setup-helm](https://github.com/azure/setup-helm) from 3.5 to 5.
- [Release notes](https://github.com/azure/setup-helm/releases)
- [Changelog](https://github.com/Azure/setup-helm/blob/main/CHANGELOG.md)
- [Commits](Azure/setup-helm@v3.5...dda3372)

---
updated-dependencies:
- dependency-name: azure/setup-helm
  dependency-version: '5'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>

* chore: update image tags for main release

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Mayank Pande <mayankpande88@gmail.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Clears 13 osv-scanner findings (GO-2026-5005..5033) flagged by
OpenSSF Scorecard's Vulnerabilities check. All were uncalled (govulncheck
still showed 0 reachable), but Scorecard counts declared deps regardless
of reachability.

osv-scanner -r . → No issues found.
go test ./... passes under go1.26.3.
Clears OpenSSF Scorecard's Fuzzing check (alert #41). Both parsers
handle env-var input and were previously untested under random input;
the fuzz targets assert they never panic and that returned keys/values
are always non-empty and trimmed.

Verified locally: ~2.2M execs in 10s per target, no crashes.
mayankpande88 and others added 15 commits June 2, 2026 09:53
…#446)

* fix(chart): make profiler image configurable + auto-bump

The agent's PROFILER_IMAGE env defaulted to a binary-baked URL
(registry.dev.nudgebee.pollux.in/nudgebee-profiler-{}:latest) whose
:latest manifest had a missing layer, so debugger pods hit
ImagePullBackOff and the pod_profiler action failed with
"wait debugger ready: context deadline exceeded".

- values.yaml: new runner.profilerImage defaulting to
  ghcr.io/nudgebee/application-profiler-{}:dc5fd6d
- _helpers.tpl: render PROFILER_IMAGE only when set; empty value
  falls back to the agent binary's compiled default
- update-image-tags.yml: auto-bump runner.profilerImage to newest
  short-SHA from ghcr.io/nudgebee/application-profiler-bpf (all
  variants share the same SHA per release.yml)

* fix(runner): handle legacy create_or_replace_alert_rule payload shape

The api-server's eventrule path sends create_or_replace_alert_rule with a
flat {alert, expr, duration, annotations, labels} payload — the Robusta
playbook shape — not a full PrometheusRule manifest. The current handler
requires a manifest with metadata.name+namespace+spec, so every call from
the api-server failed with "rule.metadata.name and namespace are required"
and the corresponding event_rules DB row was created without an actual
PrometheusRule CR in the cluster.

This change keeps both shapes behind the same action name:

- Manifest path (existing): unchanged for callers that pass a full CR.
- Legacy path (new): translates {alert, expr, duration, annotations,
  labels} into a single rule appended to a canonical shared CR named
  nudgebee-prometheus.rules in the agent's install namespace
  (INSTALLATION_NAMESPACE, falls back to ScannerNamespace). If a rule
  with the same alert name already exists in any group, it's replaced in
  place; otherwise it's appended to the first group. If the CR doesn't
  exist, a new one is created carrying the release.app=nudgebee-
  resource-management label so an in-cluster legacy runner can still
  find it via the same label selector.

delete_alert_rule gets the same shape-routing: {alert} drops the rule
from the canonical CR (idempotent for missing CR / missing rule);
{namespace, name} still deletes a whole CR.

Auth: create_or_replace_alert_rule and delete_alert_rule are added to
the light-action allowlist. Most Group-D mutations stay RSA-required,
but these two travel api-server -> relay -> agent and are gated by the
relay's shared secret today; without the carve-out the agent silently
rejects every call ("not in light-action allowlist"). The new comment
above the allowlist write spells out the trust posture so a future
reader doesn't accidentally un-carve them.

Tests cover create-when-absent, append-when-new, replace-in-place,
delete-by-alert, missing-CR no-op, unknown-alert no-op, and the
handler-level shape router for both create and delete.

* chore: update image tags for main release

* fix(runner): retry-on-conflict around legacy alert-rule CR + safer str helper

Review feedback from #446:

- CreateOrReplaceAlertRule and DeleteAlertRule mutate a shared CR
  (nudgebee-prometheus.rules). Two concurrent UI sessions will land
  between Get and Update and hit 409 Conflict. Wrap the read-modify-
  write cycle in retry.RetryOnConflict so the second writer rereads
  and re-applies its patch instead of failing the user-facing action.
  The AlreadyExists branch in the create-fallback (two callers racing
  to create the CR) is converted to NewConflict so the same retry loop
  handles it.

- Replace direct r["alert"] != alert comparisons with the package's
  str(r, "alert") helper. Same defensive idiom used everywhere else
  in handlers.go; keeps the equality check on string-typed values only.

- main.go: log a warning at startup when both INSTALLATION_NAMESPACE
  and SCANNER_NAMESPACE are unset, so an operator sees the misconfig
  before the first action fails.

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Shiv <3078106+blue4209211@users.noreply.github.com>
* fix(chart): make profiler image configurable + auto-bump

The agent's PROFILER_IMAGE env defaulted to a binary-baked URL
(registry.dev.nudgebee.pollux.in/nudgebee-profiler-{}:latest) whose
:latest manifest had a missing layer, so debugger pods hit
ImagePullBackOff and the pod_profiler action failed with
"wait debugger ready: context deadline exceeded".

- values.yaml: new runner.profilerImage defaulting to
  ghcr.io/nudgebee/application-profiler-{}:dc5fd6d
- _helpers.tpl: render PROFILER_IMAGE only when set; empty value
  falls back to the agent binary's compiled default
- update-image-tags.yml: auto-bump runner.profilerImage to newest
  short-SHA from ghcr.io/nudgebee/application-profiler-bpf (all
  variants share the same SHA per release.yml)

* chore: update image tags for main release

* chore: update image tags for main release

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Shiv <3078106+blue4209211@users.noreply.github.com>
* ci(release): sign chart packages with cosign (keyless)

Addresses OpenSSF Scorecard's Signed-Releases check. GitHub releases
previously shipped only the bare chart .tgz with no signature.

Both release workflows now install cosign and, after chart-releaser
stages packages under .cr-release-packages/, keyless-sign each tarball
(Fulcio cert + Rekor transparency log) and upload <chart>.tgz.sig and
<chart>.tgz.pem to the matching release. Adds id-token: write so the
job can mint the OIDC token for keyless signing.

README documents verification via `cosign verify-blob` with the
workflow identity + GitHub Actions OIDC issuer.

Takes effect on the next release; existing releases stay unsigned.

* chore: update image tags for main release

* docs: anchor and escape dots in cosign identity regexp

Addresses review feedback: unescaped dots in the
--certificate-identity-regexp matched any char. Escape the literal
dots in github.com/.github/.yml and anchor with ^ so only the exact
workflow identity verifies.

* chore: update image tags for main release

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
The cosign signing step failed on real releases: cosign-installer now
installs cosign v3, which deprecated --output-signature/--output-certificate
in favour of the new Sigstore bundle format. Without --bundle, cosign tried
to write a bundle to an empty path ("create bundle file: open : no such file
or directory").

- Pin cosign-release: v3.0.6 so behaviour is deterministic.
- Sign into <chart>.tgz.sigstore.json via --new-bundle-format --bundle
  (signature + cert + Rekor entry in one file). OpenSSF Scorecard's
  Signed-Releases probe recognises .sigstore.json as a signature.
- Upload the single bundle instead of the old .sig/.pem pair.
- README: verify via `cosign verify-blob --bundle ...sigstore.json`.

Flags validated against cosign v3.0.6.
The rc-4 run published a full (non-pre) release: chart-releaser creates
the release as a normal release, and the "mark as pre-release" step ran
*after* signing — so when signing failed, pre-release marking + notes
were skipped, leaving a full, unsigned release.

Reorder so pre-release marking and notes happen immediately after
chart-releaser, with signing as the final step. A signing failure now
leaves a correctly-marked, fully-noted pre-release behind instead of a
half-configured full release.
The matcher fired on every K8s Event of type=Warning and produced ~6k
findings/24h on dev clusters, dominated by probe-flap noise (kubelet
Unhealthy probes, calico-node, fluent-bit, loki-canary). The high-signal
warning conditions are already covered by dedicated matchers:
pod_crash_loop, pod_oom_killed, image_pull_backoff, node_not_ready,
job_failure.

Also fixes a subject-shape bug exposed by the matcher: the engine
override only read involvedObject (core/v1 deprecated field) and ignored
regarding (events.k8s.io/v1). When the payload used the modern API, the
override fell through and SubjectFromObj returned the Event's own
metadata name (e.g. notifications-58f67d7655-q87d2.18b5319ebc909bed)
instead of the involved pod. With the matcher gone, the override and
its fetchSubjectEvents skip are deleted too.

This is a re-apply of nudgebee-agent PR #52, which was merged into the
pre-OSS history but dropped when the upstream repo was re-initialized
on 2026-05-21. The runner code lives under k8s-agent/runner since #433
(2026-05-26) and never picked it up.
* chore: point CODEOWNERS at real maintainers

The previous CODEOWNERS routed all paths to @nudgebee/maintainers, a
team that does not exist — so the file was inert and the OpenSSF
Scorecard Branch-Protection check stayed capped. List the two
write-access maintainers directly so code-owner reviews can actually be
required and satisfied (each reviews the other's PRs).

* chore: use @nudgebee/oss team for CODEOWNERS

Created the @nudgebee/oss team (write access, maintainer members) so a
real group owns the code rather than individual handles or the
non-existent @nudgebee/maintainers. Each PR is reviewed by a teammate
(authors can't self-satisfy code-owner review).
…r calls (#453)

* fix(runner): allowlist pod_script_run_enricher for unsigned api-server calls

* chore: update image tags for main release

* docs(runner): note pod_script_run_enricher allowlist security debt

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* fix(chart): decouple MUTATE_ENABLED from .Values.rsa

MUTATE_ENABLED was only set when the optional .Values.rsa block was
configured, which left the runner's mutate subsystem off on every fresh
install. That made the light-action carve-outs added in #446
(create_or_replace_alert_rule, delete_alert_rule) unreachable: a UI-
initiated event-rule create lands in the api-server DB but the agent
never registers the handlers, so the PrometheusRule CR is never written.
Worked around on dev with `kubectl set env deploy/... MUTATE_ENABLED=true`.

Move MUTATE_ENABLED out of the rsa-conditional block so it is always
true; keep RSA_PRIVATE_KEY_PATH inside the block because it points at a
file that only exists when the rsa secret is mounted. The agent's auth
boundary lives in the validator, not at registration time — only
explicitly-allowlisted light actions accept unsigned requests; all
other Group-D mutations still fall through to the HMAC/RSA-partial-keys
path and 403 without a signed request, so enabling the subsystem here
does not loosen the security posture for installs without rsa.

A short comment above the env block spells the posture out so a future
reader doesn't re-couple it.

Verified:
- helm lint passes
- ct lint --lint-conf .github/configs/lintconf.yaml --check-version-
  increment=false (same flags helm-dev-lint.yml uses) passes
- helm template with no rsa renders MUTATE_ENABLED=true and no
  RSA_PRIVATE_KEY_PATH
- helm template with rsa renders both env vars

Chart version bumped 0.1.1 -> 0.1.2 (user-visible env change).

* fix(chart): expose MUTATE_ENABLED as runner.mutateEnabled (default true)

Review feedback on #452: a compliance-driven operator should be able to
ship a strictly read-only deployment with no mutate handlers registered
at all, not just the always-on default we landed in the first revision.

- values.yaml: new `runner.mutateEnabled: true` with a comment pointing
  at the auth boundary so an operator knows what flipping it does.
- _helpers.tpl: select between "true" and "false" with `eq ... false`,
  not `default true`. `default` treats an explicit `false` as unset and
  re-enables the subsystem; the `eq` form preserves an explicit opt-out.

Default behaviour is unchanged: MUTATE_ENABLED=true so the light-action
alert-rule path keeps working out of the box. Verified with
`helm template` for all three states (unset / =true / =false).

* chore: update image tags for main release

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…455)

The chart's runner.image.tag was set by update-image-tags.yml at PR time, which
records the newest *already-published* GHCR image — i.e. the previous build,
because a PR's own runner image isn't built until it lands on main. So a chart
cut at commit N shipped the runner image from N-1. This off-by-one shipped
rc-5 (commit 5b2521d, with the pod_script_run_enricher allowlist fix #453) with
the pre-#453 runner image (42d1e24), so every env on rc-5 ran an agent without
the fix.

Pin authoritatively at release time instead: pick the newest nudgebee-agent
image whose commit is an ancestor of the release commit, and fail the release if
none exists. A released chart can no longer ship a runner image that predates or
isn't part of its own code. Applied to both release-rc.yml and release.yml.

Verified against history: HEAD=5b2521dc now selects 11-50_5b2521dc (not the old
08-54_42d1e24), and HEAD=42d1e24 correctly selects its own image, not a newer
non-ancestor.
* feat(runner): expose net/http/pprof on the runner http server

The runner shows transient heap bursts (live heap ~60-90MB, heap_sys
spikes to >1.5GB then releases) several times a day, driven by on-demand
dispatched tasks. A point-in-time heap snapshot misses the cause because
the memory is freed between bursts.

Register the standard pprof profiles on the existing :5000 mux so
`go tool pprof` can attribute allocations. /debug/pprof/allocs is
cumulative over the process lifetime, so it captures the burst hotspots
even after GC reclaims them.

* fix(runner): gate pprof behind PPROF_ENABLED (default off)

Address review: net/http/pprof was registered unconditionally on the
:5000 listener, exposing unauthenticated profiling endpoints in-cluster
(DoS / info-disclosure risk).

Gate the handlers on a new PPROF_ENABLED env (config.PprofEnabled,
default false), surfaced via the chart value runner.pprof (default
false). Enable only for active debugging.
…runner heap spikes (#459)

* fix(servicemap): aggregate edge metrics server-side to stop heap spikes

The service_map action drove the runner's transient memory spikes. A heap
profile captured during a live 923MB spike showed
servicemap.parsePromRangeResponse → json.Unmarshal holding 344MB (73% of
heap), with up to MaxParallel=8 fetches running concurrently — pushing the
runner toward ~1.5GB before GC reclaimed it.

Cause: the connection/request queries fetched raw, unaggregated series.
container_http_requests_total alone is ~20–40k series in a busy cluster
(status/method/instance/pod/le cardinality), all decoded into
map[string]string + [][]any. build.go then collapses every series down to
its edge (src/dst workload) via la.<field> += r.Last — so the entire raw
cardinality was pulled and decoded only to be summed away.

Fix: aggregate server-side by the same edge labels build.go groups on —
sum by(edge) for the additive counts/bytes, max by(edge) for the latency
ratio (mirroring build.go's `if r.Last > la.latency`). Prometheus now
returns one series per edge (hundreds) instead of the raw cardinality.

Validated against live Prometheus: container_http_requests_total
19,456 → 589 series (~32x); other edge queries similarly collapsed. Output
is identical because build.go already summed/maxed across the collapsed
series; only the fetch shape and memory footprint change. Range query and
parser are unchanged, so coverage and values are exactly preserved.

* chore: update image tags for main release

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* fix(rbac): grant karpenter permissions unconditionally

The karpenter.sh / karpenter.k8s.aws / karpenter.azure.com rules in the
runner and forwarder ClusterRoles were gated on .Capabilities.APIVersions.
That gate is evaluated once at chart-render time and fails to grant access
in two common cases:

- GitOps/templated installs (ArgoCD, Flux, `helm template | kubectl apply`)
  where .Capabilities does not include CRDs, so the rule is dropped silently.
- Karpenter installed after this chart, leaving stale RBAC until the next
  helm upgrade.

Result: the runner hits "nodeclaims.karpenter.sh is forbidden" even on
clusters where karpenter is installed.

An RBAC rule referencing an absent apiGroup/resource is inert, so granting
these permissions unconditionally is functionally equivalent to the gated
behavior but becomes active the moment karpenter is present, on every
install path. The forwarder gate additionally only checked v1beta1, missing
karpenter v1 clusters; dropping it fixes that too.

* fix(rbac): read-only karpenter for forwarder, gate runner writes

Address review feedback:

- Forwarder (kubewatch) only watches resources, so grant it read-only
  (get/list/watch) access to karpenter resources instead of write verbs.
- Runner keeps unconditional read access (fixes the original "forbidden to
  list nodeclaims" error) but gates write verbs behind
  runner.enableWritePermissions, consistent with the Argo Rollouts block.
- Normalize list-item indentation to 6 spaces to match the rest of the file.

* chore: update image tags for main release

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* chore(deps): bump docker/setup-qemu-action from 4.0.0 to 4.1.0

Bumps [docker/setup-qemu-action](https://github.com/docker/setup-qemu-action) from 4.0.0 to 4.1.0.
- [Release notes](https://github.com/docker/setup-qemu-action/releases)
- [Commits](docker/setup-qemu-action@ce36039...0611638)

---
updated-dependencies:
- dependency-name: docker/setup-qemu-action
  dependency-version: 4.1.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* chore: update image tags for main release

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
… cost polling (#456)

* feat(opencost): disable in-cluster OpenCost by default; runner skips discovery when off

OpenCost now runs centrally on the Nudgebee server side (one multi-tenant
instance per environment) instead of one per cluster.

- values.yaml: default opencost.enabled=false (agent ships no in-cluster OpenCost).
- runner: gate cost discovery+polling on OPENCOST_ENABLED (default true). When
  false, skip OPENCOST_ENDPOINT and the cluster-wide `app=opencost` autodiscovery
  so the runner can't latch onto a neighbouring namespace's OpenCost and keep
  reporting opencostConnection=true (which would suppress the server-side
  takeover). With it off the agent reports cost off and the server enrolls the
  cluster automatically.
- runner.yaml: wire OPENCOST_ENABLED into the runner secret from opencost.enabled.

Per-cluster rollback: --set opencost.enabled=true restores the in-cluster path
(and re-enables runner discovery). The OpenCost chart dependency is retained.

* chore: update image tags for main release

* feat(opencost): keep opencost.enabled=true by default (per-cluster opt-out)

Default stays enabled; --set opencost.enabled=false moves a cluster to the
central server-side OpenCost. Runner OPENCOST_ENABLED gating (this PR) makes that
opt-out actually stop cost discovery/polling.

* fix(opencost): log a warning on invalid OPENCOST_ENABLED value

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@github-advanced-security

Copy link
Copy Markdown

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

  • The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
  • Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
  • You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

@cla-assistant

cla-assistant Bot commented Jun 7, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 4 committers have signed the CLA.

✅ mayankpande88
❌ github-actions[bot]
❌ dependabot[bot]
❌ blue4209211
You have signed the CLA already but the status is still pending? Let us recheck it.

1 similar comment
@cla-assistant

cla-assistant Bot commented Jun 7, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 4 committers have signed the CLA.

✅ mayankpande88
❌ github-actions[bot]
❌ dependabot[bot]
❌ blue4209211
You have signed the CLA already but the status is still pending? Let us recheck it.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request migrates the nudgebee-agent runner component from Python to Go. It removes several deprecated Helm templates and helper functions (such as the robusta config files, apiServer deployment, and playbooks-config-secret) and introduces a robust Go-based agent featuring sub-packages for WebSocket dispatching, Kubernetes resource discovery, AlertManager webhook forwarding, and observability proxies (Prometheus, Loki, ClickHouse, etc.). Feedback was provided on the ClickHouse client to ensure trailing semicolons are trimmed from queries before appending the FORMAT JSONCompact suffix to prevent syntax errors.

Comment on lines +97 to +105
q := strings.TrimSpace(query)
if q == "" {
return errResult("clickhouse: empty query"), nil
}
// Append FORMAT JSONCompact unless the caller already has one. JSONCompact
// gives us {meta:[{name,type}], data:[[...]], rows:N} which we map directly.
if !strings.Contains(strings.ToUpper(q), "FORMAT ") {
q += " FORMAT JSONCompact"
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If the query ends with a trailing semicolon (e.g., SELECT * FROM t;), appending FORMAT JSONCompact directly will result in a syntax error in ClickHouse. It is safer to trim any trailing semicolon before appending the format suffix.

Suggested change
q := strings.TrimSpace(query)
if q == "" {
return errResult("clickhouse: empty query"), nil
}
// Append FORMAT JSONCompact unless the caller already has one. JSONCompact
// gives us {meta:[{name,type}], data:[[...]], rows:N} which we map directly.
if !strings.Contains(strings.ToUpper(q), "FORMAT ") {
q += " FORMAT JSONCompact"
}
q := strings.TrimSpace(query)
q = strings.TrimSuffix(q, ";")
if q == "" {
return errResult("clickhouse: empty query"), nil
}
// Append FORMAT JSONCompact unless the caller already has one. JSONCompact
// gives us {meta:[{name,type}], data:[[...]], rows:N} which we map directly.
if !strings.Contains(strings.ToUpper(q), "FORMAT ") {
q += " FORMAT JSONCompact"
}

@blue4209211 blue4209211 closed this Jun 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants