tests: discover comparison components and build the CI matrix from them

[danish9039]

Pull Request Template for Kubeflow Manifests

✏️ Summary of Changes

Problem

Adding a component to the Helm and Kustomize comparison harness required eighteen edits across four shared files, and every one was an append to a list that other open branches were appending to at the same time.

file	edits per component
tests/helm_kustomize_compare.sh	case arm, two usage strings, default scenario, render branch
tests/helm_kustomize_compare_all.sh	scenarios map, all loop, help, error, prepare_component
tests/helm_kustomize_compare.py	allow-list, two usage strings, extras branch
.github/workflows/helm-kustomize-comparison.yml	paths: entries, a job

Four component pull requests were open simultaneously, so each merge forced the other three to rebase. The Notebooks chart #3525 merging mid-review demonstrated it once more: it registered itself in all four shared files and set every open Helm pull request conflicting at once.

The registration model also hid real defects, found while replacing it:

1. Seven of the ten components had no blocking comparison. Compare All Scenarios carried continue-on-error: true, so only dex, istio and kubeflow-dashboard could fail a pull request.
2. A Katib upstream synchronization ran no comparison. applications/katib/** was absent from the paths: filter, while scripts/synchronize-katib-manifests.sh writes to applications/katib/upstream, the baseline for all seven Katib scenarios.
3. Comparing Katib without naming a scenario always failed. The default-scenario case fell through to base, and Katib has no base scenario.
4. The comparator's per-component exceptions were unscoped, silent, and unchecked. Roughly 300 of its 579 lines were special cases: three divergent label filters, whole resource kinds dropped from the comparison, and opt-in maps where a missing entry silently meant no check. Measuring them revealed that most never affected any verdict, and that 21 CustomResourceDefinitions annotated helm.sh/resource-policy: keep (upstream cert-manager and Istio) were covered by no check at all.

Changes

1. Each chart declares how it is compared, in its own directory (tests: discover comparison components instead of registering them).

# common/oauth2-proxy/helm/ci/comparison.yaml, abridged
component: oauth2-proxy
releaseName: oauth2-proxy
namespace: oauth2-proxy
includeCustomResourceDefinitions: true
scenarios:
  m2m-dex-and-kind:
    kustomize:
    - common/oauth2-proxy/overlays/m2m-dex-and-kind
    values: ci/values-m2m-dex-and-kind.yaml

Eleven descriptors (Notebooks included after rebasing onto its merge) replace the four shared registries. tests/run_helm_kustomize_comparison.py discovers charts by glob, so adding a component touches only that component's directory. Both shell entry points are deleted and every call site invokes the Python entry point directly.

2. The workflow builds its jobs from those descriptors (tests: build the comparison job matrix from the descriptors).

strategy:
  fail-fast: false
  matrix:
    component: ${{ fromJSON(needs.discover-components.outputs.components) }}

Hand-written jobs become one discovery job and one matrix. Every component now has its own blocking job. The paths: filter is deliberately broad, because a narrower hand-maintained list is indistinguishable from a complete one when it is wrong. Workflow tests pinned to the removed job names are replaced by four that hold for every future component: the matrix is discovered rather than listed, no job or step is advisory, the comparison step carries no condition, and Kustomize reaches PATH.

3. Every chart-specific comparison exception becomes a declared allowance in that chart's descriptor (tests: declare comparison allowances in each chart's descriptor).

# common/dex/helm/ci/comparison.yaml, abridged
knownDifferences:
- skip: Namespace/auth
  reason: >
    The Kustomize base creates the auth namespace; the chart is installed
    into an existing namespace and renders none.
- resource: Deployment/auth/dex
  ignorePodTemplateAnnotations: [checksum/config, checksum/oidc-client, checksum/passwords]
  reason: >
    The chart triggers a rollout on configuration changes with checksum
    annotations; Kustomize achieves the same effect through content-hashed
    resource names and renders no annotations.

tests/helm_kustomize_compare.py becomes a generic engine that interprets these declarations; only universal facts about the renderers stay in code (the helm.sh label and annotation namespaces, Kustomize's content-hash suffixes). Three rules close the failure modes the code form allowed:

• an allowance names resources and carries a mandatory reason, which the loader enforces;
• a malformed allowance refuses to load, including misspelled fields;
• an allowance that matches nothing in any scenario fails the run, because a stale allowance is indistinguishable from a wrong one.

Every declaration was measured before being written, by rendering all scenarios and removing candidate rules one at a time. Only load-bearing rules were kept: eight of eleven components need no label filtering at all, the four whole-kind Namespace exclusions narrow to four named namespaces, and cert-manager's normalizer-undoing-a-normalizer disappears because the over-broad filter it corrected disappears. CustomResourceDefinition retention expectations are now declared per chart, so an undeclared helm.sh/resource-policy: keep annotation fails the comparison for every component, closing the gap that let 21 keep-annotated CustomResourceDefinitions ship unchecked.

4. Smaller fixes surfaced by the redesign.

• The six normalization pop calls for uid, status and friends are deleted: kustomize build and helm template are pure text renderers and cannot emit live-cluster fields (tests: delete normalization of fields the renderers never emit).
• A failed comparison names the differing resources instead of hiding them behind --verbose; the flag and the VERBOSE environment variable are removed (tests: always print which resources differ).
• A render failure reports the tool's own message instead of a Python traceback, and the remaining scenarios keep running (tests: report the tool's own failure and keep comparing).
• tests/kustomize_install.sh retries the download; the matrix fetches the asset once per component and a single connection reset failed the whole job (tests: retry the Kustomize download).
• The Dashboard and Notebooks synchronization scripts stop comparing parity themselves (helm: stop comparing parity inside the synchronization scripts). The comparison depends on the exact Helm version, which continuous integration pins and a contributor's machine does not; now that every component has its own blocking job, the scripts keep helm lint and print the command to compare manually.

6. Review findings are folded in, as a net deletion (tests: tighten comparison allowances per descriptor review, docs: correct comparison harness documentation and script comments, tests: name the Dashboard generator test by component).

• ignoredLabels now strips top-level metadata only; an entry declares podTemplates: true when the chart also writes the keys into workload template metadata. Measured: only the three cert-manager Deployments do, so the pod templates of every other chart are now compared strictly. The unused except mechanism is removed.
• trimDataWhitespace is deleted from the harness. The Katib configuration is compared as parsed YAML, scoped to the single katib-config.yaml data key, after measuring that parsed equality holds in all seven Katib scenarios.
• retainedCustomResourceDefinitions becomes {reason, names}, both validated at load, so the retention rationale is a reviewed field rather than a YAML comment.
• The Dex allowance reason claimed Kustomize uses content-hashed names; common/dex/base sets disableNameSuffixHash: true, so the reason is corrected. The Dashboard README now names dashboard-config as the one stable-named ConfigMap.
• The base scenarios are renamed to what they compare: platform-network-policies (kubeflow-namespaces) and platform-cluster-roles (kubeflow-roles).
• The nine test_-prefixed Helm test files are renamed to the repository's component-first convention (dex_login_test.py, kserve_manifest_test.py), for example tests/dashboard_helm_manifest_generator_test.py and tests/helm_kustomize_compare_test.py (tests: use component-first test file names).

5. The descriptor format is documented in tests/README.md — every field, the pattern syntax, the staleness rule, and how to add a chart — with a pointer from each chart README (tests: document the comparison descriptor format).

Verification

Run with Helm v4.2.2, the version the workflow pins.

• Harness migration: the deleted shell harness was reconstructed from origin/master, instrumented, and run against the new one: byte-identical rendered inputs and identical verdicts across all 38 comparisons (39 after Notebooks joined the matrix).
• Allowance translation: all 39 comparisons pass before and after; every declared allowance fires. Removing each of the 45 declared allowances in turn fails the comparison or the staleness gate, so none is decorative. Each loader guard was removed in turn and its test failed. Synthetic defects (a changed container port, a deleted resource) are detected on both sides.
• tests/comparison_descriptors_test.py (18 tests) and tests/helm_kustomize_compare_test.py (21 tests, both invocation forms the workflow uses) pass; actionlint, black, shellcheck and git diff --check are clean.
• scripts/synchronize-dashboard-manifests.sh re-run end to end produces no tree change, so it remains idempotent.

Note for reviewers with repository administration access

Job names change. Any branch protection required status check configured on Compare All Scenarios, Test Dex Helm behavior, Test Istio Helm behavior, Compare Dashboard Helm and Kustomize manifests, Compare Notebooks Helm and Kustomize manifests or Test comparison normalization must be remapped to Discover comparison components, Compare <component> and Test chart behavior. This is the one part of the change a contributor cannot make.

Deliberately out of scope

remove_empty_values still treats an empty list as equal to an absent field on both sides, the one remaining normalization that can hide a real difference (an empty jobNamespaces list renders a different operator configuration than an absent one). Narrowing it changes verdicts, so it is a follow-up; with this pull request in place, that follow-up is a data change reviewed per chart.

📦 Dependencies

None. This pull request stands alone.

It deletes shared files that the Pipelines chart #3552, the Dex CustomResourceDefinition lifecycle fix #3574 and the Spark Operator wrapper chart #3575 all touch; all three are conflicting today because the Notebooks chart #3525 merged into those files. Merging this first means each of them rebases onto it once and stops conflicting with the others.

🐛 Related Issues

None.

✅ Contributor Checklist

• I have tested these changes with kustomize. See Installation Prerequisites.
• All commits are signed-off to satisfy the DCO check.
• I have considered adding my company to the adopters page to support Kubeflow and help the community, since I expect help from the community for my issue (see 1. and 2.).

---

You can join the CNCF Slack and access our meetings at the Kubeflow Community website. Our channel on the CNCF Slack is here #kubeflow-community-distribution.

[google-oss-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign kimwnasptd for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[Copilot]

Pull request overview

Moves Helm–Kustomize comparison configuration into chart-local descriptors discovered by a new Python runner.

Changes:

• Adds descriptor discovery, validation, rendering, and scenario selection.
• Adds ten chart descriptors and corrects Katib’s default scenario.
• Removes shell runners and updates documentation, tests, workflow, and synchronization calls.

Reviewed changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/test_istio_helm_chart.py	Updates Istio workflow assertion.
tests/test_helm_kustomize_compare.py	Updates Dex workflow assertion.
tests/test_comparison_descriptors.py	Tests descriptor coverage and validation.
tests/run_helm_kustomize_comparison.py	Adds the discovered comparison runner.
tests/helm_kustomize_compare.sh	Removes the single-scenario shell runner.
tests/helm_kustomize_compare.py	Removes static component validation.
tests/helm_kustomize_compare_all.sh	Removes the aggregate shell runner.
scripts/synchronize-dashboard-manifests.sh	Uses the Python comparison runner.
experimental/helm/charts/kserve-ui/ci/comparison.yaml	Declares KServe UI comparison.
experimental/helm/charts/katib/ci/comparison.yaml	Declares Katib scenarios and default.
experimental/helm/charts/hub/ci/comparison.yaml	Declares Hub comparison scenarios.
common/oauth2-proxy/helm/README.md	Updates validation command.
common/oauth2-proxy/helm/ci/comparison.yaml	Declares OAuth2-Proxy comparison.
common/kubeflow-roles/helm/README.md	Updates validation command.
common/kubeflow-roles/helm/ci/comparison.yaml	Declares platform-role comparison.
common/kubeflow-namespace/helm/README.md	Updates validation commands.
common/kubeflow-namespace/helm/ci/comparison.yaml	Declares namespace scenarios.
common/istio/helm/README.md	Updates validation commands.
common/istio/helm/ci/comparison.yaml	Declares Istio scenarios.
common/dex/helm/README.md	Updates validation command.
common/dex/helm/ci/comparison.yaml	Declares Dex comparison.
common/cert-manager/helm/README.md	Updates validation commands.
common/cert-manager/helm/ci/comparison.yaml	Declares Cert Manager scenarios.
applications/dashboard/helm/README.md	Updates Dashboard validation commands.
applications/dashboard/helm/ci/comparison.yaml	Declares Dashboard comparison.
AGENTS.md	Documents the new runner.
.github/workflows/helm-kustomize-comparison.yml	Runs descriptor tests and the new runner.

[danish9039]

/retest

[Copilot]

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 36 out of 36 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

tests/run_helm_kustomize_comparison.py:135

• The loader rejects unknown fields only inside knownDifferences; unknown top-level, scenario, ignoredLabels, and helmOnlyResources fields are silently accepted. For example, value: instead of values: loads successfully and compares Helm defaults rather than the intended values file. Define allowed fields for every descriptor level and reject unknown keys so misspellings cannot silently alter coverage.
  tests/helm_kustomize_compare.py:84
• retainedCustomResourceDefinitions is represented as bare names, so these lifecycle allowances cannot carry the mandatory non-empty reason promised by the pull request and tests/README.md:70. The loader also performs no reason validation for this family. Represent each declaration as a resource-and-reason entry and validate it consistently with the other allowance families.

        self.retained_custom_resource_definitions = set(
            descriptor.get("retainedCustomResourceDefinitions") or []
        )

tests/helm_kustomize_compare.py:175

• The documented allowance syntax permits * wildcards, but helmOnlyResources is matched by exact string lookup here. A documented declaration such as Secret/kubeflow/* therefore never permits a matching resource and is reported as stale. Match each resource key through resource_matches, and validate these patterns during descriptor loading.

    def unexpected_helm_only(self, only_in_helm: set) -> set:
        """Filter Helm-side extras down to the undeclared ones."""
        declared = {
            entry["resource"]: f"helmOnlyResources[{index}]"
            for index, entry in enumerate(self.helm_only_resources)
        }
        for key in only_in_helm:
            if key in declared:
                self._fired.add(declared[key])
        return {key for key in only_in_helm if key not in declared}

[danish9039]

@juliusvonkohout for review