Skip to content

feat: migrate to upstream flux controllers - #78

Merged
drey merged 19 commits into
feat/ns-scoped-applicationsfrom
feat/upstream-flux
Sep 18, 2026
Merged

drey merged 19 commits into
feat/ns-scoped-applicationsfrom
feat/upstream-flux

Conversation

@drey

@drey drey commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator

Summary

Moves the module off the werf forks of flux and onto upstream fluxcd/helm-controller v1.6.4 and fluxcd/source-controller v1.9.5. The internal CustomResourceDefinitions are no longer hand-maintained copies: a small generator derives them from the upstream ones at pinned tags, and CI fails if the committed files drift. The proxy's rename table follows the upstream groups and identifiers, both module controllers build against the upstream API modules, both images are built from upstream, and the source image loses the nelm in its name along with the fork it was named after.

Branched from feat/ns-scoped-applications (#77), not from main.

Why

The forks exist to replace Helm with the nelm engine. Carrying them costs a rebase of two controllers against every upstream release, and the internal CRDs had to be edited by hand to match. Moving to upstream drops both costs and gives up nelm's behaviour deliberately — that is the goal of the change, not a side effect of it.

Key changes

Generator (tools/internalcrds). A new Go module that reads upstream's config/crd/bases and writes the internal definitions: group to {source,helm}.internal.operator-helm.deckhouse.io, every kind prefixed InternalNelmOperator, short names and categories dropped so the module cannot take hr, hc and the all category away from a real flux in the same cluster. Two guards fail the build rather than emit a half-renamed file: one rejects any leftover toolkit.fluxcd.io string in the output, the other rejects a kind the generator does not know, so a future flux version cannot silently introduce an unrenamed identity. task generate:internal-crds regenerates, task ci:generate:internal-crds verifies the committed files, and the lint workflow runs it.

Proxy (images/kube-api-rewriter/pkg/operatornelm). Only the left side of the rename table moves. Every stored identity — the internal group, the kind prefix, resource names, the annotation namespace, finalizer strings — is byte-identical to before, because each one names objects, finalizers and release secrets that already exist in every cluster running this module. The one deliberate exception is reconcile.fluxcd.io/resetAt, which never had a rule at all: the fork wrote reconcile.werf.io/resetAt straight through to the cluster. Neither controller reads or writes it, so it joins its two siblings in the internal namespace instead of freezing on the domain we are leaving.

Controllers (images/operator-helm-controller, images/chart-values-controller). Import paths swapped to fluxcd/helm-controller/api v1.6.4, fluxcd/source-controller/api v1.9.5, fluxcd/pkg/apis/meta and fluxcd/pkg/chartutil at the versions helm-controller itself pins. helm.sh/helm/v3 is gone from the code, the module files and the module graph: upstream's chartutil takes Helm v4 types, and carrying both majors would leave every later reader guessing which one is authoritative.

Build and chart. Both werf.inc.yaml files clone upstream at the pinned tags; images/nelm-source-controller/ became images/source-controller/, which renames the built image, because .werf/images.yaml derives the image name from the directory. The chart followed: the templates directory, four spellings of the name, two Helm template names, the ServiceAccount named in the ValidatingAdmissionPolicy allow-list, the e2e config, oss.yaml and build/components/versions.yml. A new task check-flux-tags, run from CI, fails if the tags in the werf files and build/components/versions.yml disagree.

Verification

task test:unit, task lint, task ci:generate:api and task ci:generate:internal-crds all exit 0; regenerating the internal CRDs from the pinned tags reproduces the committed files byte for byte. The chart renders and the Deployment, ServiceAccount, RBAC bindings and admission-policy allow-list all resolve to one name. Every flag the deployments pass exists in upstream's main.go at these tags, and upstream's own RBAC requirements are covered by the ClusterRole.

Not verified here: werf build has not run (no werf, no registry), and neither has e2e against a cluster.

Review focus / risks

  • The engine swap only shows on a cluster. Releases written by nelm must be read by helm-controller on Helm 4, with drift detection enabled on every release. The first pass after the upgrade may see every field as drift and issue an upgrade per release. This is the accepted headline risk of the change and the e2e run on an upgraded cluster is the only place it surfaces.
  • Origin labels on chart resources. helm-controller labels every rendered resource with <group>/name and <group>/namespace. The proxy rewrites request bodies only for the resource types on its own list, so a chart's Ingress, CronJob, HorizontalPodAutoscaler or arbitrary CRD instance keeps the label verbatim. The mechanism is not new — the fork left helm.werf.io/name on exactly the same resources — but the leaked string is now a genuine flux identity, which in a cluster running a real flux reads as someone else's origin label. Widening the proxy's list means editing shared vendored code, which this change does not touch. The first apply of each release after the upgrade replaces the old labels, since they come from the release manifest.
  • The upgrade window on the renamed Service. --storage-adv-addr is built from the Service name, so artifact URLs already recorded in HelmChart.status.artifact.url point at a Service that no longer exists until source-controller re-reconciles at startup. Its storage is an emptyDir, so a fresh pod republishes everything. The ServiceAccount is likewise replaced rather than kept alongside the old one, with no transitional period.
  • SOURCE_REPO. The images clone through it and it defaults to GitHub. If the build pipeline points it at a mirror, that mirror must carry fluxcd/helm-controller and fluxcd/source-controller at those paths. There is no other SOURCE_REPO clone in the repository to compare against.
  • Version and dependency movement. The internal CRDs now serve one version per kind instead of two, because upstream removed the beta versions; the surviving version is the one that was already the storage version in this module since its first commit, so nothing stored is lost. The dependency upgrade carries controller-runtime 0.23.1 → 0.24.1 and the k8s.io/* set to 0.36.x; none of the behaviour that changed there is used by this module. go mod tidy was deliberately not run, so both go.sum files still carry fork checksums.

🤖 Generated with Claude Code

drey added 19 commits September 17, 2026 15:14
Every identity is addressed by path rather than replaced as text: a
document's descriptions must survive untouched, and a textual pass over
thousands of lines would be impossible to review.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
They were written by hand once, from the fork's output. Deriving them from
a pinned upstream tag makes the next version bump a tag change, and lets a
reviewer check the rename table instead of seven thousand lines of yaml.

The generator also fails if any rendered document still names an upstream
flux group, so a kind or nested group a future flux release adds in a place
the rename table does not yet cover cannot slip through silently.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
The leftover-string guard only ever caught an unrenamed group: a bare kind
carries no toolkit.fluxcd.io substring for it to see. checkKnownKind now
rejects a document whose spec.names.kind is not one Rename recognizes, which
is where a new upstream kind first becomes visible, since it arrives as its
own definition file.

Also wires tools/internalcrds's tests into the root test:unit aggregate, so
CI actually runs them, and drops the indent numbers from a comment per the
no-constant-values-in-comments rule.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
Upstream serves one version per kind at the pinned tags; the beta versions
the fork carried are gone, and declaring one would answer discovery for a
version nothing can serve.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
Only the left side of the table changes: what upstream writes. The right
side is what sits on objects in every cluster running the module, so moving it
would need a migration.

Reconnaissance also found reconcile.fluxcd.io/resetAt, an annotation the fork
already wrote unrewritten under its own domain with no rule at all; it now maps
to that same already-stored value instead of the internal one, so nothing already
on live objects moves. Short names and the category are dropped along with it,
since the generated definitions never carried them either.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
This module never writes reconcile.fluxcd.io/resetAt (setReconcileRequestAnnotations
only ever stamps forceAt and requestedAt), so unlike those two there is no value
already stored under the fork's domain to preserve. Renaming it to the internal
namespace keeps all three reconcile annotations consistent instead of leaving
one permanently pinned to the domain this migration is removing.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
The fields the module sets on the internal HelmRelease are all standard in
helm.toolkit.fluxcd.io/v2, so the swap is a change of module path.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
helm.sh/helm/v3 was only carried for chartutil.ReadValues, whose result was
immediately converted to the v4 type. helm.sh/helm/v4/pkg/chart/common has the
same ReadValues, so use it directly and drop the v3 dependency entirely.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
The source image loses the nelm in its name along with the fork it was
named after.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
The name appeared in four spellings plus two Helm template names, and the service account it names is listed in the admission policy: the old account is removed and the new one created, with no transitional period. oss.yaml and build/components/versions.yml now point both entries at the upstream fluxcd projects instead of the werf forks.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
The helm-controller and source-controller tags were pinned in the
Taskfile vars, the two werf.inc.yaml files, versions.yml and oss.yaml
with nothing checking they agreed. Bumping the werf files without the
Taskfile would build one version's binary while regenerating the
internal CRDs from another's schema, silently.

Make build/components/versions.yml the single source
generate:internal-crds reads its tags from, and add a check-flux-tags
task, run from ci:generate:internal-crds, that fails CI when either
werf.inc.yaml literal drifts from versions.yml.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
tools/internalcrds is a Go module of its own now, but it was attached
to test:unit as a raw shell line and absent from includes, so task
format and task lint never touched it. Give it a Taskfile.dist.yaml
and .golangci.yaml modeled on the sibling Go modules, include it from
the root Taskfile, and reference it from format, lint and test:unit
like every other module.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
Three comments described something other than what the code does: the
substitute annotation is a real instruction to flux's own
kustomize-controller, not an example upstream declares; the slice
branch of renameKindReferences matches any list element by value with
no key to scope it, not just the sourceRef/chartRef enums it happens
to reach today; and the resetAt rationale was duplicated verbatim
between the rule and its test.

Also make Rename fail when spec.names.kind is missing or not a
string, instead of silently writing the bare kindPrefix: run already
guarded this with checkKnownKind, but Rename is exported and called
directly by tests, so the guard belongs in the function itself.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
An empty pin on both sides compared equal and passed, and a definition
without a plural produced the bare prefix as its resource name.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
A validation rule comparing against an upstream kind could never hold once
the enum beside it was renamed, and the documentation named kinds this
cluster does not serve. The leftover guard now covers kinds as well.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
The rule table and the definitions are two halves of one agreement, and
until now nothing compared them: a changed plural, a moved version or a
missing definition passed both generator guards and surfaced only as a
"not found" against a live cluster.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
Four fields new to helm-controller since the fork spell their values in
lower case, which the module linter rejects. The exemptions naming a
second version go with it: no definition serves one any more.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
Signed-off-by: Ilya Drey <ilya.drey@flant.com>
It instructs a real flux kustomize-controller to leave the definition
alone, and nothing applies these through one. Removing it leaves no
upstream identity in the output at all, so the guard no longer needs
its exception.

Signed-off-by: Ilya Drey <ilya.drey@flant.com>
@drey
drey merged commit 57d414d into feat/ns-scoped-applications Sep 18, 2026
8 of 9 checks passed
@drey
drey deleted the feat/upstream-flux branch September 18, 2026 12:21
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