Skip to content

feat: namespace-scoped application family - #77

Open
drey wants to merge 383 commits into
mainfrom
feat/ns-scoped-applications
Open

drey wants to merge 383 commits into
mainfrom
feat/ns-scoped-applications

Conversation

@drey

@drey drey commented Sep 11, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Adds a namespace-scoped resource family so a namespace owner can install a Helm chart without cluster-wide rights: five new CRDs (HelmApplication, HelmApplicationRepository, HelmApplicationChart, HelmClusterApplicationRepository, HelmClusterApplicationChart) and their controllers. The existing addon code is generalized rather than copied — both families now run through the same reconcilers.

Why

Installing a chart required cluster-admin, because every resource the module owns is cluster-scoped. A namespace owner needs a way to install charts in their own namespace without gaining access to the whole cluster.

Two requirements shaped the design. A release must be applied under a bounded identity, or a chart carrying a cluster-scoped object would escape the namespace boundary. And the new family must not become a second copy of the addon code, which lives on just as long and would have to be edited in both places.

Key changes

API (api/v1alpha1, api/naming, crds/). Five new CRDs with their Russian doc counterparts. RepositorySpec/RepositoryStatus and the chart catalog status are now shared types: all eight CRDs generate byte-identical to before, except one description line in the addon catalog. The catalog naming scheme moved to api/naming and became injective: the hash now covers the repository and chart joined by a byte no name can hold, so two pairs can no longer produce one object name.

Controller (images/operator-helm-controller/internal). Domain contracts in internal/source (repository, catalog, release, access) with a per-kind adapter in internal/adapter. The catalogs collapsed into one generic internal/catalog package, and the repository and release reconcilers are now shared. Adds controllers for the five new kinds and a HelmApplication webhook. A transitional step renames existing catalog objects to the current scheme on every repository reconcile, independent of the remote, carrying their status over before the old object is deleted.

Release identity (internal/services/access_service.go). Each application gets a token-less ServiceAccount in d8-operator-helm, a Role seeded in the application's namespace with full rights inside it, and a RoleBinding. The Role is created once and never read back, so a namespace owner may narrow it afterwards. The internal HelmRelease carries spec.serviceAccountName and spec.storageNamespace, and the release storage lives in the application's namespace.

Chart values service (images/chart-values-controller/internal). A repository kind is described by one family value instead of branching. The HTTP contract gained a namespace field, and authorization is decided per family: permission to create a HelmClusterAddon for addons, permission to create a HelmApplication in the requested namespace for applications. The namespace is part of a chart's identity only for a namespaced family; for a cluster-scoped one it is only the authorization context. Auxiliary object names now key on the repository namespace, and the cluster-scoped name shape is unchanged.

Templates (templates/). RBAC for both controllers, the webhook configuration and the ValidatingAdmissionPolicy cover the new kinds. The controller's rights over roles are narrowed to create, escalate and bind, since the Role is only ever seeded and never read back. Seven user-facing ClusterRoles are added for both Deckhouse role models, so a namespace owner can reach the new kinds at all: write access sits at admin level, the controller-written chart catalogs are read-only, and the cluster-scoped repository stays with cluster editors. The admission policy now also closes the application and application-repository statuses, which the seeded Role can otherwise reach.

Tests and docs. New e2e suites helmapplication and helmapplicationrepository. The derived-name scheme is mirrored in tests/e2e/internal/naming (the operator's internal package cannot be imported) and tied to the operator's own implementation by twin tests. README and EXAMPLE in both languages describe the new family and correct the claim that cluster-admin is required. CI gained a unit-test workflow and a generated-files check, and the Go lint job is enabled again now that the linter is built from source rather than downloaded.

Review focus / risks

  • Every chart catalog object is renamed, the released addon ones included. The name a repository and chart pair produces was not injective, so two repositories could fight over one catalog object; the hash now covers the pair joined by a byte no name can hold. Existing objects therefore move. On the first reconcile after the upgrade each repository recreates its catalog objects under the new name, carries the old status over — a version a consumer still holds keeps its RemovedFromRepository marker and media type — and deletes the old object. The rename runs before anything touches the remote, so a repository not yet due for a sync, or one whose registry is gone for good, still migrates. Everything about it is marked TRANSITIONAL and comes out once every cluster has reconciled once: MigrateNames in internal/catalog/catalog.go, its interface entry, and Inputs.MigrateErr.
  • Impersonation is never actually exercised. The tests confirm serviceAccountName and storageNamespace are set, but the werf/3p-helm-controller fork holds full rights itself: an implementation honouring the storage namespace while ignoring the service account would pass every check in this branch. Proving it needs a chart containing a cluster-scoped object and an assertion of Ready=False with forbidden. This is the one load-bearing assumption no test closes.
  • Creating a HelmApplication is effectively namespace-admin. The seeded Role grants */*/* within the namespace. It is a deliberate decision, documented, but worth confirming the boundary is acceptable.
  • The user-facing roles rest on review alone. Seven ClusterRoles cover both Deckhouse role models. Nothing checks them automatically: the module linter skips templates/rbacv2/** entirely, its wildcard rule only reads files named rbac-for-us.yaml, and its classic-model rule verifies the name and the access-level annotation but not the rules. No e2e exercises the aggregation chain either. Worth reading the labels against the upstream selectors by hand, in particular that write access lands at admin level and never at an editor one.
  • The released addon family. No expectation changed in the thirteen addon test files touched; conditions, reasons, requeues, finalizers and the HTTP contract match. Three deliberate differences: catalog object names move as above, an addon release name longer than 53 characters is now truncated instead of being rejected by Helm, and three catalog error messages were reworded. Every other derived name is frozen and pinned by a test.
  • The namespace divergence for helmclusterapplicationrepository. The request must carry a namespace and is authorized against it, but the namespace is dropped when the chart is resolved. Worth confirming the divergence is genuinely not exploitable.
  • Repository isolation is only partly proven. The e2e creates same-named repositories in two namespaces but with the same URL, so their chart sets coincide: a controller pruning objects by label without a namespace restriction would still pass that test.

@drey
drey force-pushed the feat/ns-scoped-applications branch from 1530990 to ffe72c0 Compare September 11, 2026 12:47
drey added 28 commits September 23, 2026 18:06
The chart label carries the name verbatim and is the query surface a user and the
UI find a catalog object by, so it can be neither sanitized nor omitted. An index
may publish a name no label value can hold; offering no such chart costs less than
losing the pass, which left every chart listed after it unwritten and never reached
the pruning loop.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
A throttle is deliberately not terminal, but the nil that says so let the response
reach the YAML decoder, where the JSON body a registry sends with it decodes into
an empty index without error. The repository then read as offering no charts and
its whole catalog was pruned.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
A release installed from a registry keeps an OCI digest in its history. Once the
index republishes that version as an archive, the revision it is compared against
is a bare version with no digest part to index into.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
…d three

Rejecting every kube- namespace would move an addon already installed in one to
permanently failed, and the webhook refuses every edit to such an object, so there
is no way back short of deleting it.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
An addon names no service account, so helm-controller applies its chart as itself
over every resource in the cluster. Whoever may create one may therefore grant
themselves anything, which is why the legacy access levels reserve it too.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
The one kind missing from the list the policy closes.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
A root CEL rule is re-evaluated on every write to the object, a status patch
included, so a minimum added to a kind already in use would reject every status
patch of a repository created under a shorter name — and a name is immutable. The
reference from an addon loses the same minimum, or such a repository would exist
but stay unreferenceable. The cap stays: the name travels on as a label value.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
WriteTimeout runs from the moment the headers are read, and the handler's own work
— a registry probe, then a chart artifact download — can take longer than it did.
The client was reset instead of being told to retry or handed the values.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
The kind does cap its name at the length a label value holds, so a name the API
server would have rejected travelled as far as an access review and came back as
repository_not_found — the very answer this check exists to prevent. Only the
minimum is per-kind.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
Signed-off-by: Ilya Drey <ilya.drey@flant.com>
The RBACv2 capability ClusterRoles carry ru.meta.deckhouse.io/title and description
by contract, so the Cyrillic the linter finds is the annotation Deckhouse reads.
The one finding left standing is noted at the top of the file: it is a warning, and
the linter reporting it takes no exclusion list.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
Signed-off-by: Ilya Drey <ilya.drey@flant.com>
Signed-off-by: Ilya Drey <ilya.drey@flant.com>
…th the spec

A pass that will be retried, or one waiting on an internal object still working, no longer records the spec as observed: only a release that settles on Ready or gives up for good does. Comparing the observed generation against metadata.generation now tells work in flight apart from a verdict about the spec in front of it, which is what kstatus readers assume.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
The Lease is the only thing keeping two cluster addons off the same repository/chart pair, and nothing but e2e exercised it. The suite pins both races it settles - the lost create and the rejected take-over - along with the take-over of an orphaned claim, the owner guard on release, and the label-scoped cleanup of claims a chart change left behind.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
The package holds no sources: it declares the interfaces through which services and reconcilers see a release, a repository and a catalog without knowing the kind. The old name collided with the internal flux sources the release path actually works with, so every file that touched both had to tell source.Release from a sourcev1 object by eye. The doc comment now states the rule that decides where a contract lives rather than naming the one package that consumed them.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
The two collaborators.go files lived by two conventions at once: half the interfaces were named for an action (RepositoryResolver, CatalogSynchronizer), half carried a Manager suffix that named nothing. One rule now: the interface is what the collaborator owns, the method is a verb that does not repeat it, so EnsureHelmChart on ChartManager becomes Ensure on Charts. Every Deps field is now named after its own type.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
BaseService existed to share two helpers and a client. The helpers move to utils.DeleteIfPresent, which three of the future packages need and which is now covered by its own test; the client, the scheme and the target namespace each service keeps itself, unexported, the way ForceService already did. Nothing outside the package ever read the exported fields.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
The gateway to the internal flux objects is now its own package: the HelmChart, OCIRepository, HelmRelease and HelmRepository the module writes, their outcome types, and the reconcile-request stamping that prods their controllers. The types are named for what they own rather than suffixed Service, and the tests move with them.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
The ServiceAccount, Role and RoleBinding a release is applied under are now their own package. The outcome it produces is named for what the reconciler asks of it - an identity - rather than for the API group it happens to use, which is what the release reconciler already calls the collaborator.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
Two collaborators only the cluster addon family needs: the Lease that keeps two addons off one repository/chart pair, and the creation of a target namespace the addon is allowed to bring itself. Each is now a package whose doc comment says which family needs it and why the others leave the collaborator empty.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
The last two collaborators - reading a remote repository and writing the catalog, and the secrets the flux objects need to reach the same registry - become reposync, and the services package is gone. ConfigOutcome moves to the repository reconciler as ConfigFailure: nothing but the reconciler ever built one.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
Every collaborator that took a *runtime.Scheme stored it and never used it: nothing sets an owner reference, because the internal objects live in the operator namespace and outlive no parent there. Seven constructors and every wiring site lose the parameter.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
Inputs was eleven fields of which a pass filled three or four, and which ones were filled was the real discriminator: a maintenance pass carried a chart outcome field it could never set, and nothing stopped a caller setting both. A pass now ends in one of four outcomes, each carrying only what that ending can describe, and the internal source is a second sealed choice inside the one outcome that has a chart. The nil outcome the type system cannot rule out is answered with an error to the work queue rather than an empty status.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
The doc comments written during the split overstated four things: rbac refuses a foreign Role and RoleBinding but not the ServiceAccount, which lives where nobody else can write; the chart claim exists for a product rule, not for a name collision the internal objects never have; reposync is where a whole repository is read, but internalobj probes a single artifact; and contract depends on the repository client too. The chart claim lease name joins the frozen internal names, and the delete-precondition test now fails if the revision stops being named.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
Signed-off-by: Ilya Drey <ilya.drey@flant.com>
Signed-off-by: Ilya Drey <ilya.drey@flant.com>
Signed-off-by: Ilya Drey <ilya.drey@flant.com>
drey added 30 commits October 19, 2026 16:26
Neither an application nor an addon target may use default. This reverses
78f08be.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
helm-controller keeps such a release under Reconciling=True forever, so the owner
reported work in flight with an empty message while nothing could change the
outcome. Message patterns, each naming the conditions it may appear in, now mark
the state as stalled; the first one covers a forbidden verb on a resource.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
A post-render failure is reported the same way an RBAC refusal is: the internal
object keeps retrying under Reconciling=True while only an edit to the chart or
its values changes the outcome.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
Signed-off-by: Ilya Drey <ilya.drey@flant.com>
…timestamp for migration

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
Signed-off-by: Ilya Drey <ilya.drey@flant.com>
Signed-off-by: Ilya Drey <ilya.drey@flant.com>
Signed-off-by: Ilya Drey <ilya.drey@flant.com>
…rator

The ClusterAdmin role granted writes on HelmClusterAddon and on both
cluster-scoped repositories. Creating an addon runs an arbitrary chart with
cluster-wide rights, and redirecting a repository substitutes the artifact of
an existing one, so both stay with the built-in cluster-admin role.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
The admin capability writes applications only. Reading a repository discloses
the registry password it stores in plaintext, so both writing and reading one
move one level up, to superadmin. The capabilities are renamed after the action
they grant, as the platform modules name theirs.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
Nothing in the granular model wrote HelmClusterAddon or the cluster-scoped
repositories, so the rights lived with the built-in cluster-admin role alone.
A capability of the superadmin level hands them to d8:subsystem:delivery:superadmin
instead, the way the virtualization module ships its own internals.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
A subject of kind User is stored with rbac.authorization.k8s.io, so comparing
the read-back binding against a literal that omits the group failed on a
binding the controller had correctly left alone.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
The label puts the binding behind the platform's label-objects admission
policy, so no user can delete it and the self-healing scenario cannot be
staged any more. The spec now states what holds instead: the deletion is
refused and the binding stays as the reconciler wrote it. The label itself
is asserted next to the managed-by one.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
The suite always pointed the ModuleSource at the shared dev registry, so a
build published anywhere else could not be exercised. The repository and the
certificate authority now come from the environment, keeping the former
address as the default, and the hint about resolving a digest names whichever
repository is in use.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
containerd reads a registry certificate and its credentials only at startup,
so a module published to a registry of one's own could not be pulled by the
node. When the registry is named, the certificate is mounted and the registry
patched into the node configuration before the cluster is created; otherwise
the configuration stays as it was.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
The kind cluster the suite runs against follows the channel the module is
released to.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
…licies

Both decide the limits of the namespace from inside it and sit above what the
identity is meant to hold, so they join Namespace, ResourceQuota, LimitRange and
ProjectNamespace. The flat list of core-group kinds becomes a dict of group to
resources driving the match constraints and the Role policy alike, so the next
kind is one entry rather than a new clause in both.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
…ead of 422

The policy that keeps the internal objects and the chart catalog to the
controllers carried no reason, so the API server fell back to Invalid and a
refusal arrived as 422. The three policies covering the application identity
already say Forbidden; this one now says it too.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
The watcher kept every error for the whole suite, so one bad line failed the
assertion of every scenario that followed and the suite teardown besides, and
an Ordered container skipped its remaining specs. Reading the errors now takes
them, leaving each assertion to report what appeared since the previous one.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
The repository kinds carry no lower bound on their own names, so a two-letter
repository was created but could never be referenced. Drop the floor from the
chart reference and from the chart-values request validation that mirrored it.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
Dropping the "default" default left the field optional with no lower bound, so
an addon without a namespace passed admission and then kept requeueing on a
Get by an empty name, which the API server never answers terminally.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
Signed-off-by: Ilya Drey <ilya.drey@flant.com>
The installer tag defaulted to RockSolid while the ModuleConfig hardcoded
Stable, so a cluster raised on one channel reconfigured itself onto another.
Both now follow the flag, which defaults to Stable as the usage text promised.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
Signed-off-by: Ilya Drey <ilya.drey@flant.com>
The flag used to skip verification when the operator fetched the index of an
http(s) repository, which bought nothing: flux verifies the certificate of the
chart that index points to either way, so such a repository never worked.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
Both fields used to reach the internal OCIRepository, where flux reads insecure
only when no CertSecretRef is set, so a spec asking for no verification got one
anyway. The CA reference is now left out whenever the flag is on.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
Signed-off-by: Ilya Drey <ilya.drey@flant.com>
The only caller is the console, which proxies the request with the end
user's Dex token untouched. kube-apiserver wraps its OIDC authenticator
audience-agnostic, so a token review asking for any audience of our own
would reject that token, and asking for none already falls back to the
API server's own --api-audiences.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
The chart address comes from the repository index, so the probe's error text
told the caller what answered at that address. The reply now carries the
verdict alone and the full error goes to the log.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
Every internal object derived from an addon carries its name as the value
of the helm.deckhouse.io/cluster-addon label, and a label value stops at
63 characters. Without the bound the first write of a longer-named addon
- the chart claim Lease - is refused as Invalid, and the reconcile loops
on that error without reaching a status.

The rule is evaluated on every write, so an addon that already carries a
longer name can no longer be updated. The four other source kinds carry
the same rule.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant