fix(datadog): update datadog clustername handling - #47
Merged
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 5 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Confidence score: 3/5
- In
HELM/newrelic/default/qbm.yml, New Relic installation still prompts users to re-entercluster_name, which can override or discard the platform-supplied identity and misattribute telemetry—preserve the supplied cluster name without requiring manual re-entry.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="HELM/newrelic/default/qbm.yml">
<violation number="1">
P2: New Relic installs still require users to retype `cluster_name`, so this change drops the platform-supplied cluster identity described by the PR and can leave telemetry attributed to a manually entered name. Keep `cluster_name` as a `contextVariables` entry sourced from `cluster.name` instead of adding it as a required user variable.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
… and New Relic What: - HELM/datadog/7 (1.0.1 -> 2.0.0): `cluster_name` is now required with no default; the `qovery-cluster` placeholder is removed. Pattern is unchanged. - HELM/newrelic/default (1.0.1 -> 2.0.0): `cluster_name` moves from a required user variable to a `contextVariables` entry sourced from `cluster.name`, so the platform supplies it instead of the user retyping it. - catalog.json regenerated. Why: Datadog installs that never touched the field tagged all telemetry `qovery-cluster`, a name that matches no real cluster, making the data hard to attribute. New Relic had the mirror problem: it forced the user to type a name the platform already knows. Notes: - Both are major bumps. The variable contract changes, and telemetry re-tags under a different cluster identity, so dashboards and monitors keyed on the old value stop matching after the upgrade. - Datadog deliberately does NOT use `contextVariables`. Datadog requires lowercase RFC1123 for `clusterName`, while Qovery cluster names are unvalidated (q-core's `ClusterRequest.name` is a bare String) — `_Undeletable_cluster` exists today and fails that pattern. `ContextVariable.overridable` is parsed but never read in q-core, so a user hitting the mismatch would have no way to correct it. New Relic's accepted charset is wide enough for raw cluster names. - New Relic is the first Helm blueprint to use `contextVariables` (the 12 existing users are all Terraform), and q-core resolves them only on the catalog-query path. The rendered value must be confirmed with `mise run deploy-service-rc` before merge.
…cluster singleton What: Set `datadog.operator.enabled: false` in HELM/datadog/7/values.yaml, and document it. No version bump — 2.0.0 in this branch is unreleased. Why: Chart 3.240.0 defaults `datadog.operator.enabled` to true, so every deploy shipped the Datadog Operator plus 7 cluster-scoped CRDs (datadogagents, datadogagentinternals, datadogcsidrivers, datadogdashboards, datadoggenericresources, datadogmonitors, datadogslos). CRDs are singular per cluster, so the first environment to install owned them and every later Datadog install on the same cluster failed: CustomResourceDefinition "datadogagents.datadoghq.com" exists and cannot be imported into the current release: invalid ownership metadata The Operator was never used — the agent is deployed directly by this chart and no DatadogAgent CRs exist on any cluster checked. Notes: - Verified by rendering the blueprint's own values.yaml against datadog 3.240.0: CRDs 7 -> 0, ClusterRole/Binding 5 -> 4, Deployments 2 -> 1 (operator gone), agent DaemonSet and DD_CLUSTER_NAME unchanged. - The failure mode was invisible in practice: on _Undeletable_cluster the owning release (helm-z62cf6ba0-datadog, deployed Apr 23) sits in a PAUSED environment — DaemonSet scaled to 0 via the qovery-pause node selector — yet still held CRD ownership and blocked installs in every other environment. The Helm error names no environment, so the cause is not discoverable from the deployment log. - Upgrading an existing 1.x install removes those CRDs. Safe where no DatadogAgent CRs exist; a cluster genuinely using the Operator would need them kept.
What: Restores HELM/newrelic/default to its state on main (metadata.version back to 1.0.1, `cluster_name` back to a required user variable) and regenerates catalog.json. The Datadog changes in this branch are untouched. Why: The New Relic change made it the first Helm blueprint to use `contextVariables`, and that could not be verified end to end. q-core resolves context variables only on the catalog-query path (BlueprintCatalogQueryUseCase) — nothing on the dispatch path, where values are taken verbatim from the client request. If they are not resolved at deploy, New Relic would install with an empty cluster name, which is worse than the required field it replaced. Three rc deploy attempts across two environments never created a service, so the mechanism stayed unproven. Rather than land unverified behaviour, this PR is narrowed to the Datadog fixes, which are verified. Notes: - Datadog's fixes were confirmed by server-side dry-run against a live cluster that already holds conflicting Datadog CRDs: `main` reproduces the reported ownership error, this branch installs cleanly with 0 CRDs in the manifest. - New Relic still forces the user to type a name the platform knows. Re-propose the auto-fill once context variables are confirmed to resolve at dispatch, or once QOV-2188 makes `overridable` real. - Separate defect worth its own ticket: blueprint dispatch silently failed for both Helm blueprints in two environments, returning HTTP 200 with no service created and no error to the caller. That blocks the PR-prerelease testing flow added in #45.
…in 63 characters What: Set `clusterAgent.admissionController.enabled: false` in HELM/datadog/7/values.yaml and document it. No version bump — 2.0.0 in this branch is unreleased. Why: The chart names that component's Service `<release>-cluster-agent-admission-controller`. Qovery release names are `helm-z<id>-<service name>`, so the Service overshot Kubernetes' 63-character cap and the deploy failed after the chart had already been resolved: Service "helm-z75e3f52b-rc-datadog-operatorfix-cluster-agent-admission-controller" is invalid: metadata.name: must be no more than 63 characters The 35-character suffix left only 13 characters for the service name, which no realistic name respects. Nothing is lost by disabling it. The controller injects APM/DogStatsD config and unified service tags only into pods labelled `admission.datadoghq.com/enabled=true` (`mutateUnlabelled` defaults to false), and Qovery services do not carry that label, so on this blueprint it injects into nothing. The README already directs users to point tracers at the node agent manually. It is not involved in metrics, logs, kube-state-metrics, live containers or cluster checks. Notes: - Verified by rendering the blueprint's values against datadog 3.240.0 with the exact release name that failed: longest resource name drops from 72 to 61 characters, nothing exceeds 63, CRDs stay at 0, and the agent DaemonSet plus cluster agent Deployment are unchanged. - A `fullnameOverride` would also have shortened the names but was rejected: every ClusterRole/ClusterRoleBinding the chart emits is release-name-prefixed today, and a fixed override would make them collide across environments — the same class of failure the operator change in this branch removes. - The ceiling is not gone, only raised. The longest remaining suffix is `-kpi-telemetry-configmap` (24 chars), so a service name beyond ~24 characters still breaches the limit. Documented in the README. - The controller also claims a cluster-wide `datadog-webhook` object, created at runtime by the cluster agent rather than templated by the chart, so Helm never owns it and two installs in different namespaces would silently contend for it. Further evidence for QOV-2194.
Astach
force-pushed
the
fix/blueprint-cluster-name-context
branch
from
August 27, 2026 13:06
dc71922 to
8cf5d2b
Compare
…of requiring it
What:
`cluster_name` becomes optional with no default. `values.yaml` falls back to the
engine-injected `qovery_cluster_name`, passed through Tera's `slugify`:
clusterName: {% if cluster_name %}{{ cluster_name | slugify }}{% else %}{{ qovery_cluster_name | slugify }}{% endif %}
AGENTS.md gains a section on what the engine actually does when rendering `values.yaml`.
No version bump — 2.0.0 in this branch is unreleased.
Why:
This is what QOV-2188 comment 22403 asked for: the field should show the real cluster name,
not a placeholder. Earlier in this branch it was made required-with-no-default instead,
on the belief that auto-filling was unsafe because Qovery cluster names are unconstrained
while Datadog demands lowercase RFC1123. Both halves of that objection turn out to be
solvable:
- The engine injects `qovery_cluster_name` into every blueprint's Tera context (engine
lib-engine/src/blueprint/task.rs `inject_context_variables`, called before spec resolution
and shared by the Helm and Terraform paths), so the cluster's own name is already available.
The injection is keyed on that hardcoded name — not on `contextVariables`, which only feeds
the console form.
- `slugify` makes it RFC1123: `_Undeletable_cluster` -> `undeletable-cluster`.
Notes:
- Verified by rendering this file with tera 1.20 (the engine's version, default features) and
feeding the result to `helm template` against datadog 3.240.0: cluster_name omitted ->
`undeletable-cluster`, empty string -> `undeletable-cluster`, `my-prod-eu` -> `my-prod-eu`.
`DD_CLUSTER_NAME` lands as `undeletable-cluster`; CRDs stay at 0; longest resource name 61.
- `{% if %}` rather than `| default(value=…)` is deliberate. Tera's `default` substitutes for an
undefined variable only, and both the Console (no `default:` to prefill) and the generated rc
payload send `cluster_name` as an empty string, which `default` would pass through as an empty
tag.
- The `pattern` is kept, so an explicit override must already be valid RFC1123; `slugify` on that
branch is then a no-op. Inference is the only path that relies on it.
- slugify is lossy: two cluster names could collapse to the same tag, and renaming the Qovery
cluster re-tags telemetry. Both are acceptable next to a placeholder that matches no cluster.
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
Astach
changed the title
What:
`pattern` becomes `^([a-z0-9]([a-z0-9-]{0,38}[a-z0-9])?)?$` — the previous expression wrapped
in an optional group, so "" is valid. Nothing else about the accepted set changes.
Why:
Making `cluster_name` optional in the previous commit left it unusable. An untouched optional
field is submitted as "" rather than omitted, and q-core validates a submitted "":
BlueprintVariableValidation.validate — a variable absent from the request is skipped
(`if (v == null) { if (m.required) ...; return@forEach }`), but one that is present goes to
FieldSchemaValidation.validateValue, and validateString applies `pattern` with no exemption
for blanks.
So `""` failed with PatternMismatch and the create request was rejected before the engine ran,
meaning the values.yaml fallback to `qovery_cluster_name` could never fire. The generated rc
payload sends exactly that, confirmed: {"name":"cluster_name","value":""}.
Notes:
- Verified the new pattern differs from the old in one case only. Accepts "" and every value the
old one did, including a 40-character name; still rejects 41 characters, uppercase, underscores,
a leading or trailing hyphen, and spaces.
- Re-verified the template after the change: omitted -> undeletable-cluster, "" ->
undeletable-cluster, my-prod-eu -> my-prod-eu.
- Audited every qbm.yml for the same combination (optional + pattern + no default). This variable
is the only one, so no other blueprint carries the trap.
- The deeper asymmetry is q-core's: an omitted variable skips validation while a submitted "" is
validated, and no manifest default is applied in either case. Blueprint authors have to encode
the blank case in both the pattern and the template. Noted on QOV-2196.
What:
Quote `datadog.clusterName`, `datadog.apiKey` and `datadog.site`. `logs.enabled` and
`apm.portEnabled` stay unquoted — the chart wants real booleans there. Also condensed the
comment block above clusterName, which had grown to nine lines.
Why:
`slugify` emits `[a-z0-9-]`, which includes strings YAML reads as implicit non-strings. A cluster
named `True`, `ON` or `123` slugifies to `true`/`on`/`123`, YAML parses it as a bool or number,
and the chart's type check rejects it before anything reaches Kubernetes:
executing "check-cluster-name" at <.Values.datadog.clusterName>:
wrong type for value; expected string; got bool
Reproduced against datadog 3.240.0: `True`, `123` and `ON` all failed at
templates/daemonset.yaml:150:12; `prod-eu` passed. After quoting, all five names tested render,
including `_Undeletable_cluster` -> "undeletable-cluster".
apiKey and site are the same class of hazard. A 32-character hex key that happened to be all
digits would be parsed as a number, and `site` is only safe today because `allowedValues` limits it
to domains — quoting removes the dependency on that. Quoting is unconditionally safe for
clusterName because slugify cannot emit a quote character.
Notes:
- Verified the two intentional booleans still render as `true`/`false`, not strings.
- `datadog_api_key` has no `pattern`, so an arbitrary value is interpolated into values.yaml.
Quoting stops a numeric-looking key being mangled, but a value containing a quote or newline
could still break or inject YAML. Constraining it to hex belongs in a separate change, since a
pattern risks rejecting key formats I have not verified.
What: Replace the named example cluster in HELM/datadog/7/values.yaml, HELM/datadog/7/README.md and AGENTS.md with a description of the constraint itself (uppercase, underscores, spaces). Why: A specific cluster from an internal sandbox does not belong in catalog documentation that ships publicly. The rule is what matters, not the instance that exposed it.
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
Why: Overriding the Datadog cluster tag rejected values Datadog is happy with — anything over 40 characters, or containing an underscore or a dot. The README compounded it by promising "valid RFC1123", which is neither what the schema enforced nor what Datadog requires. QOV-2188. What: An explicit cluster tag now accepts what Datadog accepts: up to 80 characters of lowercase letters, digits, hyphens and underscores, optionally dot-separated. It is sent through unchanged. Leaving the field empty still infers the Qovery cluster's name. Notes: The pattern and maxLength now mirror the chart's own check-cluster-name helper instead of a hand-rolled rule; verified they agree with it on every value tested. slugify consequently applies only to the inferred name. It used to run on explicit input too, which was invisible while the pattern allowed only RFC1123, but would now corrupt legal values (`my_cluster` -> `my-cluster`, `a.b.c` -> `a-b-c`).
How to test this blueprint before mergingUse a test organization, never production. Requirements:
Payloads are pre-filled from this branch's
|
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Confidence score: 3/5
HELM/datadog/7/README.md: An emptycluster_namecombined with a Qovery cluster name longer than 80 slugified characters can fail the chart’scheck-cluster-namevalidation, despite being documented as supported; limit or validate the fallback name before use.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="HELM/datadog/7/README.md">
<violation number="1" location="HELM/datadog/7/README.md:15">
P2: When `cluster_name` is empty and the Qovery cluster name slugifies beyond 80 characters, the fallback can fail the chart's `check-cluster-name` validation even though this table presents it as supported. Limit or validate the fallback value, or document that long Qovery cluster names cannot use the default.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
| | `datadog_api_key` | string | yes | — | Datadog API key (required) | | ||
| | `datadog_site` | string | | `datadoghq.com` | Datadog site (`datadoghq.com`, `datadoghq.eu`, `us3/us5/ap1`, gov) | | ||
| | `cluster_name` | string | | `qovery-cluster` | Cluster name tag on all telemetry (lowercase RFC1123) | | ||
| | `cluster_name` | string | | Qovery cluster name | Cluster name tag on all telemetry, max 80 chars. Empty = the Qovery cluster's name, slugified | |
There was a problem hiding this comment.
P2: When cluster_name is empty and the Qovery cluster name slugifies beyond 80 characters, the fallback can fail the chart's check-cluster-name validation even though this table presents it as supported. Limit or validate the fallback value, or document that long Qovery cluster names cannot use the default.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At HELM/datadog/7/README.md, line 15:
<comment>When `cluster_name` is empty and the Qovery cluster name slugifies beyond 80 characters, the fallback can fail the chart's `check-cluster-name` validation even though this table presents it as supported. Limit or validate the fallback value, or document that long Qovery cluster names cannot use the default.</comment>
<file context>
@@ -12,7 +12,7 @@ A **Datadog API key** supplied as the sensitive `datadog_api_key` variable, plus
| `datadog_api_key` | string | yes | — | Datadog API key (required) |
| `datadog_site` | string | | `datadoghq.com` | Datadog site (`datadoghq.com`, `datadoghq.eu`, `us3/us5/ap1`, gov) |
-| `cluster_name` | string | | Qovery cluster name | Cluster name tag on all telemetry. Empty = the Qovery cluster's name, slugified |
+| `cluster_name` | string | | Qovery cluster name | Cluster name tag on all telemetry, max 80 chars. Empty = the Qovery cluster's name, slugified |
| `enable_logs` | string | | `true` | Collect container logs from all pods (`true`/`false`) |
| `enable_apm` | string | | `false` | Enable the APM trace-agent port (`true`/`false`) |
</file context>
gdubroeucq
approved these changes
Aug 28, 2026
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
2 participants
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.
What
Four fixes to
HELM/datadog/7(1.0.1 → 2.0.0), plus an AGENTS.md section on how the enginerenders
values.yaml.cluster_nameis optional and defaults to the Qovery cluster's own name, slugified toRFC1123. Previously a hardcoded
qovery-clusterplaceholder.datadog.operator.enabled: false).values.yamlare quoted.Why
The blueprint could not deploy, and tagged telemetry with a name matching no real cluster.
singular per cluster, so the first environment to install owned them and every later
install failed on Helm ownership — including when the owning environment was paused.
The Operator was unused; the agent is deployed directly by the chart.
<release>-cluster-agent-admission-controller,which exceeded Kubernetes' 63-character limit for all but very short service names. It
only injects into pods labelled
admission.datadoghq.com/enabled=true, so nothing is lost.trueor123became a bool or number and the chart'scheck-cluster-namerejected it.Notes
Follow-ups: QOV-2194 (cluster-scoped blueprints), QOV-2196 (API deploys ignore variable defaults).