[quality] Gate proposal: values.schema.json rules and chart template arms without a helm-unittest assertion

Aperta
#672 1 commento 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
3/5
Tempo stimato
1-2 giorni
Idoneità per principianti
15/100
Tipo di issue
Funzionalità
Chiarezza
Da chiarire
Stato di attività
Attiva
Stack tecnologico
json, typescript, yaml
Ambito
testing-qa

Direzione di ricerca

Start with the schema rules in charts/prow-github-app/values.schema.json and the existing failedTemplate cases in charts/prow-github-app/tests/schema_test.yaml and topology_test.yaml. The issue asks for a governance decision first: whether to add a merge-blocking vitest gate (tests/chartAssertions.test.ts) and the eleven missing negative cases. Do not start coding until a maintainer accepts or declines it. If accepted, done means each listed rule has a helm-unittest case that fails when the rule is loosened.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

agent/quality kind/cleanup stage/045-needs-decision testing

Finding

This proposal was first filed on 2026-11-03 and folded by the hive into #665 (comment <!-- hive-finding-folded -->); #665 was then closed by #666, so the decision it asked for was never taken. Re-filed standalone so it can be accepted or declined.

It is the fourth finding in the same class on charts/prow-github-app — a template arm or schema rule that helm unittest renders but never asserts — so per the quality lane's rules it is a gate proposal, not a fourth tests-only PR.

Prior instances (each landed as a held tests-only PR after being verified by mutating the template and watching the new assertion fail):

  1. #665 → #666 (merged): checksum roll annotations, six placement pass-throughs, the release-contains fullname arm.

  2. #668 → #669 (merged): nameOverride selector agreement, service/pdb/cronjob/prow pass-throughs, common labels.

  3. #671 (merged): both install-notes arms, the Redis note, the cronjob-disabled arm, probe paths, securityContext, resources.

  4. This instance — charts/prow-github-app/values.schema.json rejection rules. At main@91a1669 (all three PRs above merged) the chart suites contain seven failedTemplate cases: six in tests/schema_test.yaml (≥2 replicas without Redis, a non-rediss?:// URL, empty credentials, an empty schedule, one misspelled key at the root, one under cronjob) and one in tests/topology_test.yaml (terminationGracePeriodSeconds: 15). No case asserts that the schema rejects any of:

    • image.pullPolicy outside ["Always","IfNotPresent","Never"]
    • prow.mergeMethod outside ["","merge","squash","rebase"]
    • github.apiUrl not matching ^$|^https?://
    • redis.existingSecretKey: "" (minLength: 1)
    • cronjob.activeDeadlineSeconds: 0 (minimum: 1)
    • cronjob.successfulJobsHistoryLimit / failedJobsHistoryLimit: -1 (minimum: 0)
    • service.port: 0 (minimum: 1)
    • replicaCount: 0 (minimum: 1)
    • image.repository: "" (minLength: 1)
    • github.existingSecret: "" with no app credentials (anyOf[0] minLength: 1)
    • a misspelled key under any of image, github, prow, redis, service, ingress, serviceAccount, podDisruptionBudget (additionalProperties: false — only the root and cronjob blocks have a negative)

    Every existing mention of those paths in tests/*_test.yaml sets a valid value (grep -n 'pullPolicy\|mergeMethod\|apiUrl\|existingSecretKey\|activeDeadlineSeconds\|JobsHistoryLimit\|service.port\|image.repository' charts/prow-github-app/tests/*_test.yaml), so loosening or deleting any rule above passes helm unittest, ct lint and both kind installs.

Evidence method (reproducible at main@91a1669): list every enum|pattern|minimum|minLength|additionalProperties|required in values.schema.json; for each, grep tests/*_test.yaml for a case that set:s the path to an invalid value and asserts failedTemplate (grep -c failedTemplate charts/prow-github-app/tests/*.yaml → 6 + 1). Unit evidence: helm unittest charts/prow-github-app. End-to-end evidence: the chart CI job installs charts/prow-github-app/ci/single-values.yaml and the HA values file into kind — both are schema-valid, so no install exercises a rejection rule. The schema rejections are reached by neither suite; the schema is the chart's only input validation.

The missing gate

Nothing in CI measures what the chart suite exercises, which is why this class recurs one file at a time. The repository already gates repository text with vitest contract tests (__tests__/workflows.test.ts "action.yml mirrors the inputs src/ reads"; __tests__/coverageIgnoreHints.test.ts), so the cheapest gate with the same shape is:

__tests__/chartAssertions.test.ts (runs under npm test in the existing build-test job — no workflow change, no new dependency; js-yaml is already in package.json):

  • parse charts/prow-github-app/values.schema.json; collect every property path carrying enum, pattern, minimum, minLength, required or additionalProperties: false (including under if/then/anyOf);
  • scan charts/prow-github-app/templates/* for .Values.<path> inside {{ if | with | range | else if }};
  • parse charts/prow-github-app/tests/*_test.yaml and collect set: keys, split by whether the case asserts failedTemplate;
  • fail with the list of paths that (a) gate a template arm but appear in no test, or (b) carry a schema constraint but appear in no failedTemplate case. A path-shaped allowlist constant in the test records any deliberate exception with its reason.

Scope stays additive: no config keys change, no chart files move, the chart job is untouched.

Decision requested

  • Accept — the quality lane opens one held PR adding __tests__/chartAssertions.test.ts plus the eleven schema negatives above in tests/schema_test.yaml, so the gate is green on arrival.
  • Decline — the lane resumes individual chart findings (next: the eleven schema negatives as a plain held PR, no gate).

Risk: governance — this adds a merge-blocking rule on chart contributions, which is the maintainers' call, not the lane's.

Priority

  • Impact: medium (a loosened schema rule reaches users as a mis-rendered install with no error)
  • Effort: low (one vitest file, ~11 helm-unittest cases)

Filed by quality agent (hold-gated mode)


🐝 Hive Agent: quality | Instance: hosted-available-lke648397-260827-5q9t | SHA: 91a1669

— hive: agent=quality backend=copilot model=claude-fable-5.1 copilot=1.0.88

Lingua principale
TypeScript
Stelle
132
Fork
24
Merge medio
1g 6h
PR unite (30g)
232

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di cncf/prow-github-actions

Tutte le issue di cncf/prow-github-actions

Issue simili

Altre issue su TypeScript

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.