feat(core): support legacy OCI chart media type with incremental indexing

[drey]

Summary

The OCI layer media type of a packaged chart stops being a constant and becomes a fact about a specific chart version: the indexer resolves it once per new tag, records it in HelmClusterAddonChart.status.versions[], and both controllers build their internal OCIRepository from that record. This makes charts pushed by older tooling — layer application/tar+gzip instead of application/vnd.cncf.helm.chart.content.v1.tar+gzip — deployable, and their values readable. Because resolving a media type costs a manifest request per tag while the catalog syncs every five minutes, the recorded verdicts double as the set of already-examined tags, so a steady-state pass with no new tags still costs exactly one registry request. status.versions[] gains mediaType, unavailableReason and unavailableMessage, and now lists every examined version rather than only the usable ones.

Why

The media type was hardcoded in two places — internal/services/oci_repo_service.go (the internal OCIRepository of a HelmClusterAddon) and images/chart-values-controller/internal/resolver/resolver.go (the auxiliary one used to read values.yaml). Both assumed the current CNCF type. A real marketplace chart does not have it:

$ crane manifest cr.yandex/yc-marketplace/yandex-cloud/cert-manager-webhook-yandex/cert-manager-webhook-yandex:1.0.8-1 \
    | jq '{config: .config.mediaType, layers: [.layers[].mediaType]}'
{
  "config": "application/vnd.cncf.helm.config.v1+json",
  "layers": [
    "application/tar+gzip"
  ]
}

For such a chart nelm-source-controller answers "failed to find layer with media type", the addon never gets an artifact, and chart-values-controller returns values_not_found — so a chart that is perfectly valid is simply undeployable.

Three constraints shaped the design rather than the obvious one-line fix:

Dropping layerSelector entirely is not safe. With no media type the source controller takes layers[0]. helm push of a chart with a .prov file produces a second layer and the order is not guaranteed by the spec, so the wrong layer can be selected silently.

application/tar+gzip identifies nothing. It is a generic type any tarball may carry, so accepting it means the layer alone can no longer tell a chart from an arbitrary artifact. The config media type is the authoritative marker.

The media type is per version, not per repository, so it has to be discovered per tag — and the previous indexer was one remote.List with a semver filter that never touched a manifest. Naively resolving would turn every five-minute sync into N manifest requests.

Key changes

Chart version API — api/v1alpha1/helm_cluster_addon_chart.go, crds/

• HelmClusterAddonChartVersion gains mediaType (only ever a supported layer type — an empty value means the version is not deployable), unavailableReason (enum RemovedFromRepository / UnsupportedMediaType / ResolvePending, absence means usable) and unavailableMessage.
• status.versions[] now lists every examined semver tag, not only the deployable ones; its doc comment and the Russian mirror say so. A separate unresolvedVersions[] was considered and rejected: a version an addon still references has to stay in versions[] anyway, so an entry's list would depend on whether an addon references it and entries would migrate between lists as spec.chart.version changes.
• New reasons ReasonPartialSync and ReasonChartVersionRemoved.
• No print column for the usable count: a CRD print column is JSONPath without aggregation, and omitempty leaves a usable entry with no field to filter on.

Chart artifact identification — internal/client/repository/oci_chart.go (new)

• Three checks on one manifest, in order: the descriptor must not be an index; config.mediaType must be in a closed list (application/vnd.cncf.helm.config.v1+json); the layer is the first entry of a closed priority list (…helm.chart.content.v1.tar+gzip, then application/tar+gzip) present in the manifest. Priority comes from the list, not from the order of layers, so an artifact carrying both resolves deterministically.
• An index tag produces a verdict, never an error — an error would be re-probed by the incremental loop on every sync forever.
• All three negative outcomes share the reason UnsupportedMediaType; which media type failed, and its observed value, go into unavailableMessage.

Incremental OCI indexing — internal/client/repository/{client,oci}.go

• FetchOptions{Known KnownCharts; Full bool} with NeedsExamination(chartName, tag): no entry → examine; mediaType set → skip; mediaType empty and reason UnsupportedMediaType → skip; anything else, including a recorded entry with no verdict at all (the upgrade migration) or ResolvePending → examine. Full overrides everything and is set by force reconcile.
• One HTTP request per examined tag: remote.Get returns both the descriptor media type and the manifest bytes, which are parsed with v1.ParseManifest — no desc.Image(), no second fetch. One remote.NewPuller is shared across the pass, because remote.Get builds a fresh puller per call and each one re-pings /v2/; on a bearer registry that was a token fetch per tag against a rate-limited endpoint.
• errgroup with SetLimit(8); no cap on tags per pass (a cap would only mask missing incrementality). Per-tag requests use a single attempt — a failure is recorded as pending and retried by the next sync anyway.
• Error taxonomy per tag: a supported layer → usable; no supported layer → UnsupportedMediaType; 5xx/timeout/429/network → ResolvePending with the error in the message, everything else in the pass still published; 404 → the tag vanished between listing and request, omitted entirely; 401/403 → escalated to the repository level as terminal. The only errors that leave resolveChartVersions are that escalation, a puller-construction failure and a cancelled parent context.
• Registry error text is capped at 256 bytes on a rune boundary before it reaches a status field.
• Removed the never-called isSemverCompliantTag.

Catalog merge and prune protection — internal/services/repo_sync_service.go

• knownCharts reads the recorded verdicts back out of the chart statuses before fetching — the status is the only store of that state, and a separate fingerprint field was rejected as one more thing that can drift from the data it summarizes.
• existing.Status.Versions = lo.Map(...) becomes a merge: a still-listed version comes from the fetch result, a disappeared one is dropped unless an addon references it, in which case it is retained with RemovedFromRepository and keeps its media type. Without that media type the addon's internal OCIRepository could not be constructed at all, so pruning it would block every change to a running addon rather than just the pull. A referenced version whose fresh verdict is UnsupportedMediaType keeps its old media type for the same reason.
• The same protection covers the chart object: a HelmClusterAddonChart referenced by an addon is not deleted even when the repository lists no tags.
• In-use versions are found through the field index, moved out of the webhook package into a new internal/index so the service can use it without importing the webhook. The module's other field index (.spec.chart.helmClusterAddonRepository, previously in internal/utils/mapper.go) moved there too, so both live in one place under one naming convention. Both spec.chart.version and status.lastAppliedChart.version count (they differ during an upgrade), and the last-applied reference is checked to belong to this chart.
• The merged list is written in a deterministic order — semver descending, parsability as the primary key so the comparator stays transitive. An unstable order would produce a status patch and a log line on every sync for a catalog that did not change.
• Dropped the if len(chart.Versions) == 0 { continue } guard, which is what lets a chart whose every tag is pending survive the pruning loop instead of being deleted.
• SyncOutcome.FetchAttempted distinguishes "the registry was never contacted" from "the fetch succeeded"; FetchOutcome.Pending carries the count of unexamined tags as data, never as an error.

Repository status — internal/reconcile/helmclusteraddonrepository/evaluate.go

• lastSuccessfulSyncTime advances only on a pass with Pending == 0 — its doc comment says "fully brought up to date", which is false with unresolved tags. The frozen Last Sync column becomes the signal, and it converges on its own.
• Synced=False/PartialSync only while no full pass has ever happened. On the first pass lastSuccessfulSyncTime is empty either way, so the signal above does not exist yet and a user would see Synced=True over a silently incomplete catalog; afterwards it stops flapping over one junk tag. Reconciling=True was rejected for this: pending tags are retried forever, so the condition would never clear and kubectl wait would hang over a single junk tag. Nothing escalates — the failure counter is untouched and Stalled is never reached.
• nextFailureCount carries the counter forward when no fetch was attempted, so a failed status read can no longer reset ConsecutiveFetchFailures, clear Stalled and report Ready=True without the registry being contacted.

Deploy path — internal/reconcile/helmclusteraddon/reconciler.go, internal/services/oci_repo_service.go

• The gate becomes exactly "for an OCI repository, usable ⇔ mediaType != \"\"", which is the same as "we know enough to construct the internal OCIRepository". It gets four cases right at once: a normal version passes; a version retained after its tag disappeared passes deliberately, so the addon keeps reconciling its values, maintenance mode and deletion while the source controller reports the real pull failure; a non-chart or never-resolved tag is rejected with a message naming the reason; and a Helm repository keeps the old presence-only gate, since its versions never carry a media type.
• applyOCIRepositorySpec takes the recorded media type and never writes an empty one — there is deliberately no fallback, since falling back to "first layer" is the behaviour this PR removes.
• The addon reports ChartVersionRemoved when its version is retained but no longer offered, instead of only the source controller's bare "not found".
• Bug fix on the way: the OCI branch passed err — a guaranteed-nil leftover from GetRepositoryType — to status.Failed instead of addonChartErr, so the real cause was lost.

chart-values-controller — internal/resolver/resolver.go, templates/chart-values-controller/rbac-for-us.yaml

• Reads the media type from the same catalog status instead of resolving it again: that would duplicate the discovery logic and spend registry requests on an answer already in the cluster. Adds get/list/watch on helmclusteraddoncharts (the client cache starts an informer on first read, so get alone would fail at runtime).
• Outcome mapping distinguishes "no verdict yet" from "a negative verdict": a missing chart object, a missing entry, ResolvePending, or an entry with no reason at all (the pre-upgrade state) → OutcomePending, so the caller retries; any other empty-media-type entry → OutcomeValuesNotFound naming the reason. A RemovedFromRepository version keeps its media type and is still served.

Shared naming — api/naming/ (new)

• GetHelmClusterAddonChartName moves out of operator-helm-controller/internal/utils into the api module: it is a truncated hash, both controllers must derive it identically, and it lived in an internal package of another module. Golden tests pin its output, so no existing object is renamed.

Tests — +~1800 lines

• The OCI client is tested against a real in-process ggcr registry with real pushed artifacts, including a legacy-media-type one and a multi-layer one where the chart layer is not first. The incremental tests assert the number of manifest requests (0 for a recorded verdict, exactly 1 for a pending or unmigrated entry), which is the only thing that actually proves the economics; the counter is reset after the fixture pushes because remote.Write reads manifests itself.
• Table tests for the deploy gate's four cases, the merge and prune-protection rules, and every PartialSync branch including many consecutive partial passes never reaching Stalled.
• The e2e lifecycle assertion now counts usable versions rather than a non-empty list.

Review focus / risks

• status.versions[] changed meaning with no schema signal. It used to list available versions; it now lists examined ones, ordered semver-descending rather than in registry order. Any consumer — the Deckhouse UI, scripts, dashboards — that treats the list as "available" without checking unavailableReason will show unusable entries. Worth a release note.
• Verify the upgrade window is acceptable. Existing entries carry neither field, which the incremental table reads as "never examined", so the first sync after rollout resolves them with no force reconcile and no manual step (pinned by the "1.0.3": {} case in oci_test.go). Until that pass completes, every OCI addon reports Ready=False with "the repository catalog has not resolved it yet" and values requests return a retryable pending. Nothing destructive happens — with no artifact the reconciler never calls EnsureHelmRelease, the running release is untouched, and deletion still works because reconcileDelete runs before the gate — but the window is up to one sync interval per repository, and the first post-upgrade pass examines every tag at concurrency 8.
• New controller with an old CRD is the one bad skew direction. Structural pruning would drop mediaType on every status write, so every tag would be re-examined every five minutes and every OCI addon would stay gated off. Module CRDs are applied ahead of workloads so this should not happen, but it degrades badly rather than gracefully. The reverse is benign and self-healing.
• Force reconcile is now load-bearing as the only repair lever. A wrongly cached UnsupportedMediaType is sticky by design, and ValidatingAdmissionPolicy forbids everyone except the module's service accounts from editing or deleting a HelmClusterAddonChart — so force reconcile is the only way out. Known gap left in: the annotation is consumed whenever a pass was attempted, including one that failed before the fetch, so a force request can be swallowed. Cheap to gate on the fetch having run.
• A re-pushed tag is not detected until force reconcile. Deliberate — mutation detection costs a request per known tag per pass, which defeats the whole point — but it means a tag overwritten with different content keeps its recorded media type indefinitely. Confirm that trade-off.
• Deleting a repository still breaks the addons using it, and this PR does not close that. The chart objects are owned by the repository (BlockOwnerDeletion), so they are garbage-collected with it and the addon then fails on the repository lookup itself; the auth/TLS secrets go too, so the addon's surviving internal OCIRepository starts failing on a private registry. The workload keeps running and the addon stays deletable, and re-creating the repository under the same name recovers everything within a sync. Pre-existing behaviour, unchanged here — the prune protection added in this PR guards the sync path, not owner-reference GC. Closing it properly needs an admission rule or a finalizer on the repository, using the same field index this PR extracts.
• Status object size. versions[] now retains every examined tag with up to 256 bytes of message each. Not a realistic shape today (cosign and non-semver tags are filtered before resolution), but a chart repository carrying thousands of non-chart semver tags would grow the object toward the etcd limit, at which point the status patch fails permanently.
• The shared puller is a new concurrency surface. It is documented as caching its fetcher per repository behind a sync.Map/sync.Once and the suite passes under -race, but only the happy path is covered — token refresh partway through a long pass is not.
• No e2e for a legacy chart end to end. The resolution half is proven against a real registry in unit tests; that nelm-source-controller accepts application/tar+gzip in layerSelector rests on the crane evidence above and on reading its layer-selection code. Pushing a legacy fixture into the test registry was left as separate test-infrastructure work.
• task generate:api was never run in a working environment (remote taskfiles are disabled locally, so update-codegen.sh was invoked directly and prettier skipped). The CRD and deepcopy output is committed and consistent with the markers, but task ci:generate:api is the real check — worth running once before merge to avoid a whitespace-only red CI.