Skip to content

[quality] chart: the pod checksum roll annotations, scheduling pass-throughs and release-named fullname arm are asserted by no helm-unittest suite #665

Closed

Description

@hivecommons-hive

Finding

charts/prow-github-app/templates/deployment.yaml renders checksum/config (sha256 of .Values.prow + .Values.github.apiUrl) and, without github.existingSecret, checksum/secret so a changed ConfigMap or inline Secret rolls the pods. It also passes nodeSelector, tolerations, affinity, topologySpreadConstraints, podLabels and podAnnotations through, and _helpers.tpl:10 returns the bare release name when it already contains the chart name.

None of that is asserted:

  • Unit (helm unittest 1.2.1 at main@19d23da, 4 suites / 41 tests green): no suite reads spec.template.metadata.annotations, sets any of the six placement/pod values, or renders with a release name containing prow-github-app.
  • End to end (test.yml chart job, run 41345516301): the two kind installs use ci/single-values.yaml and ci/ha-external-redis-values.yaml, which set none of those values, and the job never reads the annotations.

Mutation check against the current suites: dropping .Values.github.apiUrl from the config digest, deleting the nodeSelector block, forcing the fullname contains arm to false, and removing checksum/secret each still pass all 41 tests.

Not a defect; a test gap. Side observation (not a finding): cronjob.yaml does not apply nodeSelector/tolerations/affinity to cron pods and values.yaml makes no claim that it should.

Recommendation

Add helm-unittest cases under charts/prow-github-app/tests/:

  • a placement suite pinning the default absence and the pass-through of the six placement/pod values on the server pod (and that podLabels do not leak into spec.selector);
  • topology cases pinning the checksum/config / checksum/secret digests for the fixture values, that a prow.* or github.apiUrl change moves only checksum/config, an inline credential change moves only checksum/secret, and github.existingSecret drops checksum/secret;
  • a wiring case rendering with release name prow-github-app-prod.

Priority

  • Impact: low (pass-through values and an operational roll annotation)
  • Effort: low

Filed by quality agent (hold-gated mode)


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

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

Activity

  1. added
    testingCreated by Hive for agent-filed issue provenance
    agent/qualityCreated by Hive for agent-filed issue provenance
    on Nov 2, 2026
  2. added
    kind/cleanupCategorizes issue or PR as related to cleaning up code, process, or technical debt.
    and removed on Nov 2, 2026
  3. hivecommons-hive commented on Nov 3, 2026

    @hivecommons-hive
    ContributorAuthor

    Folded finding. This was filed as a separate issue, but 1 open agent-filed issue (plus 4 closed as completed in the last 14 days) already cite test.yml, so it was folded into the oldest of them instead of opening another. Treat this issue as the tracker for that component: prefer one structural fix that covers every folded variant over a PR per variant.

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

    Finding

    This 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 filed as a gate proposal rather than a fourth tests-only PR. Accept or decline below.

    Prior instances (all at main@19d23da, each verified by mutating the template and watching the new assertion fail):

    1. [quality] chart: the pod checksum roll annotations, scheduling pass-throughs and release-named fullname arm are asserted by no helm-unittest suite #665 → held PR test(chart): pin the pod checksum roll annotations, the scheduling pass-throughs and the release-named fullname arm #666: deployment.yaml checksum roll annotations, six placement pass-throughs, the _helpers.tpl release-contains fullname arm.

    2. [quality] chart: nameOverride, service.type/port, podDisruptionBudget.minAvailable, cronjob history limits and prow.jobs/prow.config are asserted by no helm-unittest suite or kind install #668 → held PR test(chart): pin nameOverride selector agreement, service/pdb/cronjob/prow pass-throughs and the common labels #669: nameOverride selector agreement, service/pdb/cronjob/prow pass-throughs, common labels.

    3. held PR test(chart): pin the NOTES install arms and the pod hardening, probe paths and resource pass-throughs #671: both NOTES.txt install arms, the Redis note, the cronjob-disabled arm, probe paths, securityContext, resources.

    4. This instance — values.schema.json rejection rules. tests/schema_test.yaml (7 negatives) plus one in topology_test.yaml cover replicaCount ≥ 2 without Redis, a non-rediss?:// URL, empty credentials, an empty schedule, terminationGracePeriodSeconds < 20, and one misspelled key at the top level and one under cronjob. No test asserts that the schema rejects any of these:

      • 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)

      Loosening or deleting any of those rules today passes helm unittest, ct lint and both kind installs.

    Evidence method (reproducible): checkout main@19d23da with #666/#669/#670/#671 merged on top (all merge clean); list every {{ if|with|range … .Values.<path> }} in templates/ and every enum|pattern|minimum|minLength|additionalProperties|required in values.schema.json; grep tests/*_test.yaml for a set: of that path (for schema rules, one whose asserts include failedTemplate). Unit evidence: helm unittest charts/prow-github-app (52 tests with the held PRs). End-to-end evidence: the chart job of test.yml installs ci/single-values.yaml and ci/ha into kind — those values are all schema-valid, so no install exercises a rejection rule. Priority 3 for the arms (exercised by the kind install, unasserted by unit); the schema rejections are reached by neither suite.

    The missing gate

    Nothing in CI measures what the chart suite exercises, which is why the same class keeps recurring one file at a time. The repository already gates repo 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):

    • parse charts/prow-github-app/values.schema.json; collect every property path that carries 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 (js-yaml is already a dependency) and collect set: keys, split by whether a 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, and the chart job is untouched.

    Decision requested

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

    Priority

    • Impact: medium (schema rules are the chart's only input validation; a loosened rule reaches users as a mis-rendered install)
    • 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: unknown

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

  4. added
    hive/likely-doneHive verified that a merged PR references or claims this issue; pending confirmation
    and removed on Nov 3, 2026
  5. hivecommons-hive commented on Nov 3, 2026

    @hivecommons-hive
    ContributorAuthor

    Fixed by #666 (#666, merged). Closing because Hive's PR claim metadata says this PR was the full fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    agent/qualityCreated by Hive for agent-filed issue provenancehive/likely-doneHive verified that a merged PR references or claims this issue; pending confirmationkind/cleanupCategorizes issue or PR as related to cleaning up code, process, or technical debt.testingCreated by Hive for agent-filed issue provenance

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions