feat(must-gather): expose known must-gather env vars as explicit fields in the values file

[rm3l]

Description of the change

This makes it easier to customize.

Which issue(s) does this PR fix or relate to

—

How to test changes / Special notes to the reviewer

Checklist

• For each Chart updated, version bumped in the corresponding Chart.yaml according to Semantic Versioning.
• For each Chart updated, variables are documented in the values.yaml and added to the corresponding README.md. The pre-commit utility can be used to generate the necessary content. Run pre-commit run --all-files to run the hooks and then push any resulting changes. The pre-commit Workflow will enforce this and warn you if needed.
• JSON Schema template updated and re-generated the raw schema via the pre-commit hook.
• Tests pass using the Chart Testing tool and the ct lint command.
• If you updated the orchestrator-infra chart, make sure the versions of the Knative CRDs are aligned with the versions of the CRDs installed by the OpenShift Serverless operators declared in the values.yaml file. See Installing Knative Eventing and Knative Serving CRDs for more details.

[rhdh-qodo-merge]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX Issues (0)

1. Heap dump key breaking 🐞 Bug ☼ Reliability

Description

The chart removes gather.withHeapDumps and replaces it with gather.heapDump.enabled, but
gather has additionalProperties: false in the values schema, so any existing user values files
that still set gather.withHeapDumps will fail schema validation and won’t enable heap dumps after
upgrade.

Code

charts/must-gather/values.schema.tmpl.json[R208-244]

+                "heapDump": {
+                    "title": "Heap dump collection configuration.",
+                    "type": "object",
+                    "additionalProperties": false,
+                    "properties": {
+                        "enabled": {
+                            "title": "Enable collection of heap dumps (disabled by default).",
+                            "type": "boolean",
+                            "default": false
+                        },
+                        "method": {
+                            "title": "Method for heap dump collection: 'inspector' (default) or 'sigusr2'.",
+                            "type": "string",
+                            "default": ""
+                        },
+                        "timeout": {
+                            "title": "Timeout in seconds for heap dump collection (default: 600).",
+                            "type": "string",
+                            "default": ""
+                        },
+                        "bufferSize": {
+                            "title": "WebSocket buffer size in bytes for inspector method (default: 16777216 = 16MB).",
+                            "type": "string",
+                            "default": ""
+                        },
+                        "remoteDir": {
+                            "title": "Directory in container where heap dumps are written for SIGUSR2 method (default: /tmp).",
+                            "type": "string",
+                            "default": ""
+                        },
+                        "instances": {
+                            "title": "Filter for specific heap dump instances (comma-separated pod names or patterns).",
+                            "type": "string",
+                            "default": ""
+                        }
+                    }
                },

Evidence

gather is schema-locked (additionalProperties: false) and no longer defines withHeapDumps, so
legacy values containing that key will be rejected by schema validation. The default values and
templates also no longer reference withHeapDumps, so even if schema validation is bypassed,
upgrades will silently stop enabling heap dumps unless users migrate to the new key.

charts/must-gather/values.schema.tmpl.json[169-244]
charts/must-gather/values.yaml[66-95]
charts/must-gather/templates/deployment.yaml[92-107]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`gather.withHeapDumps` was removed and replaced by `gather.heapDump.enabled`. Because the `gather` schema is strict (`additionalProperties: false`), existing user values files that still set `gather.withHeapDumps` will fail schema validation (and heap dumps will not be enabled in templates).

### Issue Context
This is a breaking values interface change. If you intend to keep it non-breaking for at least one release, you need to accept the old key in schema and map it to the new behavior in templates.

### Fix Focus Areas
- charts/must-gather/values.schema.tmpl.json[169-244]
- charts/must-gather/values.schema.json[957-1031]
- charts/must-gather/templates/deployment.yaml[92-107]
- charts/must-gather/templates/rbac.yaml[47-53]
- charts/must-gather/templates/clusterrbac.yaml[60-66]
- charts/must-gather/templates/NOTES.txt[24-30]

### Suggested implementation direction
- Re-introduce `gather.withHeapDumps` in the schema (mark as `deprecated: true` if you want), so existing values pass validation.
- In templates, compute an effective heap-dump enablement flag like:
 - `$heapDumpEnabled := or .Values.gather.heapDump.enabled .Values.gather.withHeapDumps`
 and use `$heapDumpEnabled` everywhere instead of `.Values.gather.heapDump.enabled`.
- Optionally add a NOTES.txt message when the legacy key is used to guide migration.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Unvalidated heapDump.method 🐞 Bug ≡ Correctness

Description

gather.heapDump.method is documented as only allowing "inspector" or "sigusr2", but the JSON
schema accepts any string, so typos/misconfigurations will pass helm lint/schema validation and
only fail later at runtime.

Code

charts/must-gather/values.schema.tmpl.json[R218-242]

+                        "method": {
+                            "title": "Method for heap dump collection: 'inspector' (default) or 'sigusr2'.",
+                            "type": "string",
+                            "default": ""
+                        },
+                        "timeout": {
+                            "title": "Timeout in seconds for heap dump collection (default: 600).",
+                            "type": "string",
+                            "default": ""
+                        },
+                        "bufferSize": {
+                            "title": "WebSocket buffer size in bytes for inspector method (default: 16777216 = 16MB).",
+                            "type": "string",
+                            "default": ""
+                        },
+                        "remoteDir": {
+                            "title": "Directory in container where heap dumps are written for SIGUSR2 method (default: /tmp).",
+                            "type": "string",
+                            "default": ""
+                        },
+                        "instances": {
+                            "title": "Filter for specific heap dump instances (comma-separated pod names or patterns).",
+                            "type": "string",
+                            "default": ""
+                        }

Evidence

The values schema defines heapDump.method as a plain string with no enum constraint, while the
chart README explicitly restricts valid values to two options. This creates a validation gap where
invalid configuration is not caught early.

charts/must-gather/values.schema.tmpl.json[208-244]
charts/must-gather/README.md[112-118]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The schema allows any string for `gather.heapDump.method`, but documentation restricts it to two valid values. This allows invalid values to pass schema validation and break at runtime.

### Issue Context
This chart already uses enums for other fields (e.g., `gather.logLevel`). Align heapDump.method with the documented contract.

### Fix Focus Areas
- charts/must-gather/values.schema.tmpl.json[208-244]
- charts/must-gather/values.schema.json[960-996]

### Suggested implementation direction
- Add an `enum` constraint for `method`, e.g.:
 - `"enum": ["", "inspector", "sigusr2"]`
- (Optional) In templates, add a defensive `fail` if `heapDump.enabled` is true and `method` is set to something outside the allowed set (in case schema validation is bypassed).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

Previous review results

Review updated until commit 4a97bcb

Results up to commit c73d18a

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX Issues (0)

1. Heap dump key breaking 🐞 Bug ☼ Reliability

Description

The chart removes gather.withHeapDumps and replaces it with gather.heapDump.enabled, but
gather has additionalProperties: false in the values schema, so any existing user values files
that still set gather.withHeapDumps will fail schema validation and won’t enable heap dumps after
upgrade.

Code

charts/must-gather/values.schema.tmpl.json[R208-244]

+                "heapDump": {
+                    "title": "Heap dump collection configuration.",
+                    "type": "object",
+                    "additionalProperties": false,
+                    "properties": {
+                        "enabled": {
+                            "title": "Enable collection of heap dumps (disabled by default).",
+                            "type": "boolean",
+                            "default": false
+                        },
+                        "method": {
+                            "title": "Method for heap dump collection: 'inspector' (default) or 'sigusr2'.",
+                            "type": "string",
+                            "default": ""
+                        },
+                        "timeout": {
+                            "title": "Timeout in seconds for heap dump collection (default: 600).",
+                            "type": "string",
+                            "default": ""
+                        },
+                        "bufferSize": {
+                            "title": "WebSocket buffer size in bytes for inspector method (default: 16777216 = 16MB).",
+                            "type": "string",
+                            "default": ""
+                        },
+                        "remoteDir": {
+                            "title": "Directory in container where heap dumps are written for SIGUSR2 method (default: /tmp).",
+                            "type": "string",
+                            "default": ""
+                        },
+                        "instances": {
+                            "title": "Filter for specific heap dump instances (comma-separated pod names or patterns).",
+                            "type": "string",
+                            "default": ""
+                        }
+                    }
                },

Evidence

gather is schema-locked (additionalProperties: false) and no longer defines withHeapDumps, so
legacy values containing that key will be rejected by schema validation. The default values and
templates also no longer reference withHeapDumps, so even if schema validation is bypassed,
upgrades will silently stop enabling heap dumps unless users migrate to the new key.

charts/must-gather/values.schema.tmpl.json[169-244]
charts/must-gather/values.yaml[66-95]
charts/must-gather/templates/deployment.yaml[92-107]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`gather.withHeapDumps` was removed and replaced by `gather.heapDump.enabled`. Because the `gather` schema is strict (`additionalProperties: false`), existing user values files that still set `gather.withHeapDumps` will fail schema validation (and heap dumps will not be enabled in templates).

### Issue Context
This is a breaking values interface change. If you intend to keep it non-breaking for at least one release, you need to accept the old key in schema and map it to the new behavior in templates.

### Fix Focus Areas
- charts/must-gather/values.schema.tmpl.json[169-244]
- charts/must-gather/values.schema.json[957-1031]
- charts/must-gather/templates/deployment.yaml[92-107]
- charts/must-gather/templates/rbac.yaml[47-53]
- charts/must-gather/templates/clusterrbac.yaml[60-66]
- charts/must-gather/templates/NOTES.txt[24-30]

### Suggested implementation direction
- Re-introduce `gather.withHeapDumps` in the schema (mark as `deprecated: true` if you want), so existing values pass validation.
- In templates, compute an effective heap-dump enablement flag like:
 - `$heapDumpEnabled := or .Values.gather.heapDump.enabled .Values.gather.withHeapDumps`
 and use `$heapDumpEnabled` everywhere instead of `.Values.gather.heapDump.enabled`.
- Optionally add a NOTES.txt message when the legacy key is used to guide migration.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Unvalidated heapDump.method 🐞 Bug ≡ Correctness

Description

gather.heapDump.method is documented as only allowing "inspector" or "sigusr2", but the JSON
schema accepts any string, so typos/misconfigurations will pass helm lint/schema validation and
only fail later at runtime.

Code

charts/must-gather/values.schema.tmpl.json[R218-242]

+                        "method": {
+                            "title": "Method for heap dump collection: 'inspector' (default) or 'sigusr2'.",
+                            "type": "string",
+                            "default": ""
+                        },
+                        "timeout": {
+                            "title": "Timeout in seconds for heap dump collection (default: 600).",
+                            "type": "string",
+                            "default": ""
+                        },
+                        "bufferSize": {
+                            "title": "WebSocket buffer size in bytes for inspector method (default: 16777216 = 16MB).",
+                            "type": "string",
+                            "default": ""
+                        },
+                        "remoteDir": {
+                            "title": "Directory in container where heap dumps are written for SIGUSR2 method (default: /tmp).",
+                            "type": "string",
+                            "default": ""
+                        },
+                        "instances": {
+                            "title": "Filter for specific heap dump instances (comma-separated pod names or patterns).",
+                            "type": "string",
+                            "default": ""
+                        }

Evidence

The values schema defines heapDump.method as a plain string with no enum constraint, while the
chart README explicitly restricts valid values to two options. This creates a validation gap where
invalid configuration is not caught early.

charts/must-gather/values.schema.tmpl.json[208-244]
charts/must-gather/README.md[112-118]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The schema allows any string for `gather.heapDump.method`, but documentation restricts it to two valid values. This allows invalid values to pass schema validation and break at runtime.

### Issue Context
This chart already uses enums for other fields (e.g., `gather.logLevel`). Align heapDump.method with the documented contract.

### Fix Focus Areas
- charts/must-gather/values.schema.tmpl.json[208-244]
- charts/must-gather/values.schema.json[960-996]

### Suggested implementation direction
- Add an `enum` constraint for `method`, e.g.:
 - `"enum": ["", "inspector", "sigusr2"]`
- (Optional) In templates, add a defensive `fail` if `heapDump.enabled` is true and `method` is set to something outside the allowed set (in case schema validation is bypassed).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[rhdh-qodo-merge]

Review Summary by Qodo

Expose heap dump environment variables as explicit configuration fields

✨ Enhancement

Walkthroughs

Description

• Refactor heap dump configuration from boolean to structured object
• Add explicit fields for heap dump method, timeout, buffer size, and instances
• Enable granular customization of heap dump collection behavior
• Update chart version to 0.3.0 and regenerate schema documentation

Diagram

flowchart LR
  A["withHeapDumps<br/>boolean flag"] -->|"refactored to"| B["heapDump object<br/>with sub-fields"]
  B --> C["enabled"]
  B --> D["method"]
  B --> E["timeout"]
  B --> F["bufferSize"]
  B --> G["remoteDir"]
  B --> H["instances"]
  C -->|"maps to"| I["HEAP_DUMP env vars<br/>in deployment"]
  D -->|"maps to"| I
  E -->|"maps to"| I
  F -->|"maps to"| I
  G -->|"maps to"| I
  H -->|"maps to"| I

Loading

File Changes

1. charts/must-gather/Chart.yaml ⚙️ Configuration changes +1/-1

Bump chart version to 0.3.0

charts/must-gather/Chart.yaml

---

2. charts/must-gather/README.md 📝 Documentation +10/-3

Update version and document heap dump fields

charts/must-gather/README.md

---

3. charts/must-gather/templates/NOTES.txt ✨ Enhancement +1/-1

Update heap dump condition reference

charts/must-gather/templates/NOTES.txt

---

View more (6)

4. charts/must-gather/templates/clusterrbac.yaml ✨ Enhancement +1/-1

Update heap dump condition in RBAC rules

charts/must-gather/templates/clusterrbac.yaml

---

5. charts/must-gather/templates/rbac.yaml ✨ Enhancement +1/-1

Update heap dump condition in RBAC rules

charts/must-gather/templates/rbac.yaml

---

6. charts/must-gather/templates/deployment.yaml ✨ Enhancement +22/-2

Add heap dump environment variables to deployment

charts/must-gather/templates/deployment.yaml

---

7. charts/must-gather/values.schema.json 📝 Documentation +37/-5

Add heapDump object schema with sub-properties

charts/must-gather/values.schema.json

---

8. charts/must-gather/values.schema.tmpl.json 📝 Documentation +36/-4

Add heapDump object schema template definition

charts/must-gather/values.schema.tmpl.json

---

9. charts/must-gather/values.yaml ✨ Enhancement +15/-1

Replace withHeapDumps with heapDump object structure

charts/must-gather/values.yaml

---

[openshift-ci]

New changes are detected. LGTM label has been removed.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[rm3l]

/agentic_review

[rhdh-qodo-merge]

Persistent review updated to latest commit 4a97bcb