containers / containers/nri-plugins

Helm charts: fix YAML types for nodeSelector/affinity and add schema validation

Open
#668 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
114
Forks
44
Avg merge
1d 8h
Merged PRs (30d)
20

Description

## Problem

Several Helm chart values across the repo use incorrect YAML types that could cause invalid manifest rendering if users override them with non-empty values.

Affected charts (at minimum): `memory-qos`, `sgx-epc`, `memtierd`, `resctrl-mon`.

### 1. `nodeSelector` and `affinity` default to `[]` (list) instead of `{}` (map)

In `values.yaml`:
```yaml
affinity: []
nodeSelector: []
```

Kubernetes expects `nodeSelector` to be a `map[string]string` and `affinity` to be an object. While the `{{- with }}` template construct treats empty lists as falsy (so defaults don't render), the declared type is semantically wrong. Users copying the pattern could supply a list value by mistake.

### 2. `values.schema.json` is missing `tolerations`, `affinity`, and `nodeSelector` properties

The schema validates `image`, `resources`, `nri`, and `podPriorityClassNodeCritical` but does not describe scheduling-related values. Adding these with correct types (`tolerations`: array of objects, `affinity`: object, `nodeSelector`: object) would catch invalid user inputs at `helm install` time.

### 3. `Chart.yaml` uses `version: v0.0.0` with a leading `v`

Helm requires [SemVer](https://semver.org/) for the `version` field. The leading `v` is technically non-compliant and rejected by strict `helm lint`/`helm package`. All charts use this pattern and should be updated to `0.0.0` (or an appropriate version).

## Suggested fix

A single PR could update all affected charts consistently:
- Change `affinity: []` → `affinity: {}` and `nodeSelector: []` → `nodeSelector: {}` in all `values.yaml` files
- Add `tolerations`, `affinity`, `nodeSelector` to all `values.schema.json` files with correct types
- Change `version: v0.0.0` → `version: 0.0.0` in all `Chart.yaml` files

## Context

Discovered via automated code review (Copilot) on #666. These are pre-existing repo-wide patterns, not regressions introduced by that PR.

Contributor guide

Open the contributing guide

Research direction

Start by locating the values.yaml, values.schema.json, and Chart.yaml files for the memory-qos, sgx-epc, memtierd, and resctrl-mon charts, then compare their scheduling values and chart versions. Run strict helm lint or helm package checks after the edits. Done means the affected charts use map-typed defaults, validate tolerations, affinity, and nodeSelector, and use SemVer-compliant versions.

Written by the indexing model from the issue text.

Assessment

Tech stack
helm, yaml
Domain
devops, infrastructure
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
64/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.