Skip to content

ci: adopt MegaLinter, and fix what it found - #516

Open
noctarius wants to merge 20 commits into
mainfrom
cleanup-megalinter
Open

noctarius wants to merge 20 commits into
mainfrom
cleanup-megalinter

Conversation

@noctarius

Copy link
Copy Markdown
Collaborator

Adopts MegaLinter as an aggregate lint gate over every language in the
repository, tunes it until what it reports is worth reading, and fixes what it
found on the way. The workflow is deliberately untriggered — it runs by hand
until the remaining findings have been read.

What the gate reports

findings linter time
defaults ~25,400 ~50 min
now ~1,000 ~20 min

Nothing was silenced without a reason recorded next to it. Every exclusion says
what the path is and why no linter has anything to say about it, and every
disabled check says which tool already answers the same question.

Two bugs it found, both dead code

  • csi-driver/scripts/config-gen-upgrade.sh never ran. IMAGE_TAG=<VALUE>
    is a redirection, not an assignment, so bash -n rejects the file. The
    version added in May 2025 failed to parse identically. Nothing referenced it,
    and the operator's storage node DaemonSet does its job in a container of the
    same name.
  • csi-driver/deploy/CRDs was a specification draft. Its own commit message
    says so. The group is simplyblock.com, which appears in no source; the real
    CRDs are storage.simplyblock.io. Three of the drafts declare kind: Rackup
    next to plural: lvols, plural: replications, and plural: lvolmigrations,
    which the API server rejects, so none was ever applied. Its README was a
    scratchpad of Docker Swarm container ids.

One real security fix

The rebalancer image installed fio, nvme-cli, and sudo with
--nogpgcheck into an image carrying Red Hat certification. The reason was
that oraclelinux-release sets gpgcheck=1 and points at a key file it does
not ship. Importing Oracle's key satisfies the repositories' own configuration.
Verified by building the runtime stage: dnf update and dnf install both
succeed with checking enabled.

Also pins the manager's runAsUser, which the image and the Helm chart both
already set and the kustomize base did not.

checkov: 649 findings to 116

It runs in project mode, so FILTER_REGEX_EXCLUDE never reached it and 286 of
its findings were in paths excluded from every other linter. Four things were
measured rather than assumed:

  • REPOSITORY_CHECKOV_FORWARD_EXCLUDED_DIRECTORIES defaults to true and is a
    no-op.
  • A directory: key in .checkov.yml is ignored, because the command-line
    --directory overrides it.
  • Repeating --directory reports each finding once per directory, inflating 592
    distinct findings to 1364.
  • skip-path takes a plain substring. A regex there aborts the run with
    re.PatternError from inside a forked worker, which reads as a hang.

What remains is 111 findings on the chart's control-plane workloads, which are
ordinary Deployments that could be hardened, and they need a cluster to verify.

Go

  • Every function body gets its own lines. 299 declarations across 116 files.
    Empty bodies and function literals are untouched.
  • hack/golangci-onelinefunc is the rule that keeps that true, as one
    analyzer with two front ends. Nothing off the shelf expresses it: revive has
    no such check and neither gofmt nor gofumpt reformats
    func f() int { return 1 }, both verified against a fixture. It is an AST
    rule because a signature can hold a brace before the body's
    (func f() map[string]struct{}). Not wired up yet — see below.
  • The comments meet the house style: 608 gate errors down to 90. British
    spellings, the comma e.g. takes, product names, and 62 quoted values moved
    into backticks, which is what the style guide asks for and what 1164 existing
    comment lines already do.

That no code changed is not an assumption. Every touched file was tokenized
before and after with go/scanner, comments skipped, and the token streams
compared. go vet ./... passes in all four modules and gofmt is clean.

Left deliberately undone

  • The onelinefunc rule is not enforced. The golangci-lint plugin route needs
    scripts/tools.{mk,sh,manifest,lock} changed, because $(GOLANGCI_LINT) is a
    release binary pinned by sha256 and used by three Makefiles, and atlas-lib
    has no .golangci.yml to enable it in. cmd/onelinefunc is the cheaper path
    and also covers test/integration, which the lint matrix does not.
  • 90 Go gate errors remain, and none is prose. They are the checkers reading
    code as English: nqn names a parameter, [nvme] is a godoc link, /nics is
    an endpoint, and six comment lines hold commented-out Go. Three exemptions in
    the checkers would take this to zero.
  • .github is outside checkov's scope, which costs 21 permissions: write-all findings on the other workflows.
  • simplyblock/spdkcsi:latest in deploy/kubernetes/controller.yaml and
    node.yaml, where every upstream sidecar names a version. Pinning it needs
    somebody to say which.
  • A hardcoded CLUSTER_SECRET in deploy/kubernetes/deploy.sh, which four
    secret scanners did not flag.
  • operator/test/e2e/rbac_test.go is not gofmt-clean on main, untouched
    here, which suggests golangci-lint's formatter is not reaching it.

MegaLinter covers every language in this repository at once, which nothing else
did. Adopting it against a tree that had never run it meant deciding what it is
allowed to say, because the defaults reported roughly 25,400 findings in about
50 minutes of linter time and a gate nobody can read is a gate nobody reads.

The configuration is tuned to roughly 1,880 findings in about 20 minutes, and
every choice is recorded where it is made rather than here:

- Excludes generated and published trees: the versioned chart directories and
  their indexes, the operator's kubebuilder output and consolidated installer,
  the exported OpenAPI document, the design documents the house style gate
  already owns, and the Helm templates that are not YAML until rendered.
- Runs one tool per job. revive lints Go, ruff lints Python, and yamllint is
  the only YAML gate; golangci-lint, flake8, isort, pylint, pyright, prettier's
  YAML pass, and rubocop are disabled because something else already answers
  the same question, in some cases better.
- Carries tool-native rule sets in revive.toml, .yamllint.yml,
  .markdownlint.yaml, .cspell.json, and .jscpd.json, each explaining which
  default it relaxes and what that default was reporting.
- Reports and does not write. The apply-fixes bot the MegaLinter template ships
  with is absent, and DISABLE_ERRORS keeps the backlog from failing every pull
  request on day one. Removing that line is what promotes this to a gate.

Two upstream bugs shaped the config. MegaLinter 9.4.0 advertises both a string
and a list for FILTER_REGEX_EXCLUDE and accepts only a string: the global value
is concatenated into a log message, and a per-linter value goes straight to
re.compile. Both raise a TypeError on a list.

cspell disagreed with the house style at the mechanism level. The house style
exempts an identifier, path, command, or value by putting it in backticks, and
cspell honors neither backticks nor fenced blocks, so 381 of its 482 findings
were words already exempt. Its dictionary also held 59 words while the house
style mandates 185 product spellings, 174 of which were missing. Both are fixed
here, which takes cspell from 482 findings in 978 seconds to 41 in 5.

Kubebuilder and OLM join the house style's terminology list. Both are spelled
inconsistently in the existing prose, and nothing enforced either.
The findings MegaLinter and the house style gate reported between them, in the
six files they reported them in. Both gates were run against these files for
the first time, so all of this predates them.

What the house style gate rewrote, most of it in the CSI chart's parameter
table, where a description column had been written in lowercase throughout:

- Product names get the spelling their owner uses: Docker, CSI, RBAC, API, URI,
  RAID, CPU, Kubernetes, SPDK, NVMe PCIe, Kubebuilder, and Kustomize.
- The brand is lowercase mid-sentence, which is fifteen occurrences of
  "Simplyblock" in prose that is not a product name.
- American English: "labelled" becomes "labeled", "authorisation" becomes
  "authorization".
- "e.g." takes a comma, "low latency" is hyphenated before the noun it
  describes, and "NVMe-over-Fabrics" is written "NVMe over Fabrics".

Two came from cspell rather than the house style gate, which is the argument
for keeping both:

- "snasphot" was a plain typo, and the house style gate's misspelling list does
  not carry it.
- "a externallyManagedConfigmap" is now "an `externallyManagedConfigmap`". The
  backticks are the fix: the house style exempts an identifier written in them,
  and this one is the parameter the row documents.

The Helm value `externallyManagedConfigmap` keeps its spelling. Kubernetes
writes ConfigMap, so the name is wrong, but it is a chart parameter users have
in their values files and renaming it is a migration rather than a typo fix.

Table alignment and the list-item dashes in these files are left alone. Forty
findings of that kind predate this change, none of them on a line it touches,
and reflowing tables nobody is editing would bury the change above.
shellcheck reported a parse error on the script, and the reason it could be
parsed at all is that nothing has ever run it.

`IMAGE_TAG=<VALUE>` is not an assignment. bash reads `<VALUE>` as a redirection
and rejects the file, so `bash -n` fails on line 6 and the script exits before
its first command. That is not a recent regression: the version added in
a7d2770 carried `MAX_LVOL=<VALUE>` and `MAX_PROV=<VALUE>` on lines 9 and 10
and failed to parse just the same. No committed version of this file has ever
been runnable.

The placeholders do not make it a template either. It carries a `#!/bin/bash`
shebang and the executable bit, no extension marking it as one, and no
documentation anywhere saying which values to substitute.

Nothing references it, on this branch or on main. What it did — run
`node_configure.py --upgrade` in a per-node Job whose container it called
`s-node-api-config-generator` — is now the operator's work: the storage node
DaemonSet in operator/internal/utils/storage_nodeset_ds.go runs that script in
a container of exactly that name.

Its only two commits since it was added are a bulk label rename and the move
into csi-driver/, so nothing has been maintaining it either.
checkov runs in project mode, so MegaLinter hands it `--directory .` and never
builds a file list. FILTER_REGEX_EXCLUDE is applied while building that list,
which means it never reached checkov: 286 of its 649 findings were in paths
excluded from every other linter, including the vendored dependency charts and
the kubebuilder output.

Project mode is still the right mode. It is what lets checkov render the Helm
charts before scanning them, and what lets the CKV2_* checks reason across
resources, so the scope is set in .checkov.yml instead. checkov now covers
helm-charts, csi-driver, and operator, less the vendored subcharts, the
published chart output, and the generated CRDs, RBAC, and installer. That is
554 findings in 39 seconds, down from 649 in 306.

Four things were measured, because the descriptor's variables do not behave the
way their names suggest:

- REPOSITORY_CHECKOV_FORWARD_EXCLUDED_DIRECTORIES defaults to true and is a
  no-op. With a directory named in EXCLUDED_DIRECTORIES, checkov scanned it
  anyway, reported the same findings, and the command carried no --skip-path.
- A `directory:` key in .checkov.yml is dead config, because the command-line
  `--directory` MegaLinter always passes overrides it.
- Scoping by repeating `--directory` works but reports each finding once per
  directory, turning 592 distinct findings into 1364. Scoping by skip-path is
  both honest and faster, because it prunes the directory walk rather than
  filtering afterward.
- skip-path takes a plain path substring. A regex there aborts the run with
  `re.PatternError: nothing to repeat`, raised inside a forked worker, so the
  traceback names neither the pattern nor the file and the run looks like a
  hang rather than a failure.

.ruff_cache is ignored as well. The gate is what creates it, and while ruff
writes a .gitignore inside it, that is ruff's choice rather than a guarantee.
The development chart carries three paths that `make helm-sync` writes from the
operator's kubebuilder markers and API types: crds/, templates/roles/, and
templates/simplyblock-operator-webhook.yaml. A finding in any of them is fixed
at the marker and synced, never edited in the chart, which is the same reason
operator/config/rbac/ is already skipped.

Ten findings, all of them RBAC wildcards in generated roles. 554 to 544.
…s uid

checkov's findings against operator/ were 48. Two were real.

The rebalancer image installed fio, nvme-cli, and sudo with --nogpgcheck, which
is to say without checking who signed them, into an image that carries Red Hat
certification. The reason the flag was there is that oraclelinux-release sets
gpgcheck=1 on the repositories it adds and points them at
file:///etc/pki/rpm-gpg/RPM-GPG-KEY-oracle, a file it does not ship: the only
keys on disk are Red Hat's. Importing Oracle's key satisfies the repositories'
own configuration, so both flags are gone. Verified by building the runtime
stage: dnf update and dnf install both succeed with checking enabled.

The manager's pod security context asked for runAsNonRoot without saying which
user, leaving the id to whatever the image declares. It declares 65532, and the
Helm chart's own Deployment already pins that same id, so the kustomize base was
the one place the number was missing. dist/install.yaml is regenerated, and the
only line it gains is that one.

The rest were the scanner reading files that are not what it thinks they are:

- 38 came from two Kustomize patches. MegaLinter's image has no Kustomize
  binary, so checkov disables that framework and reads each overlay as a whole
  Deployment, then reports every field the patch does not repeat. It flagged
  manager_webhook_patch.yaml for a writable root filesystem next to a comment
  explaining that the filesystem is read-only. Those overlays are skipped now.
  config/manager/ is not, because it is a whole manifest and its findings mean
  something.
- The remaining four are left visible and unfixed. A sample CR names no
  namespace on purpose, the manager genuinely needs its service account token,
  and `controller:latest` is the placeholder Kustomize replaces, so neither the
  tag nor the digest check has anything to say about the shipped image.

Three findings are deliberate and now carry the reason next to the thing they
justify: an OLM bundle built FROM scratch has no user database to add a user to,
patching a UBI base is an update layer that installs nothing by design, and the
rebalancer's sudo grants exactly two binaries through a sudoers file, which is
narrower than the alternative of running the container as root.

operator: 48 findings to 4. The repository: 544 to 500.
Two hundred and fifty-seven of checkov's remaining findings were on pods and
claims that exist for the length of a test run and are then deleted. Asking a
throwaway pod for a seccomp profile, a liveness probe, and a read-only root
filesystem is not a finding anybody will act on, and it was half of what the
gate reported.

Skipped:

- csi-driver/e2e/, whose nine templates the Ginkgo suite applies and removes.
  All 179 findings there came from those, at roughly 20 apiece.
- deploy/kubernetes/testpod, testclone, and testrestore, plus the WordPress and
  MySQL demo in mysql-pvc.yaml. The install walkthrough has a reader apply these
  by hand to prove the driver works.

deploy.sh is what settles which of those manifests an installation applies. Its
list names driver, config-map, secret, controller-rbac, node-rbac, controller,
node, storageclass, and the snapshot controller, and none of the fixtures above
appear in it. controller.yaml and node.yaml therefore stay in scope: those
describe workloads that run, and node.yaml's privileges are the interesting
kind.

500 findings to 243, and what is left is all on manifests that deploy something.
yamllint reported that csi-driver/deploy/CRDs/pool.yaml is not valid YAML: it
ends on three lines of stray `~~~`, a markdown fence that was pasted in and
never taken out. Fixing those three lines would have made a dead file parse.

The commit that added the directory says what it is: "new CRDs for operator -
serve as a specification draft" (72d9288, November 2025). It is a sketch of an
API that was later built elsewhere, and everything about it says so.

The group is simplyblock.com, which appears in no Go or Python source in this
repository. The operator's CRDs are storage.simplyblock.io, generated into
operator/config/crd/bases from the API types. Of the kinds drafted here only
StorageNode shares a name with a real one, and it is in the wrong group;
SimplyBlockStorageCluster is neither the right kind nor the right spelling of
the brand.

Three of the drafts declare `kind: Rackup`. lvol.yaml pairs it with `plural:
lvols`, replication.yaml with `plural: replications`, and lvolmigration.yaml
with `plural: lvolmigrations`, so all three are a Backup template copied with
the typo left in. The API server rejects a CustomResourceDefinition shaped like
that, which means none of these was ever applied to a cluster.

Nothing references the directory. deploy.sh, which is the script that installs
this driver by hand, lists what it applies and no CRD from here is in it. The
README.MD is not documentation either: it is a scratchpad of Docker Swarm
container ids, a line reading `???`, and a fragment of somebody's `docker ps`.

The real specification lives in operator/docs/designs, and the real CRDs are
generated. Git keeps this draft if anyone wants to read it.
CKV_K8S_21 reported 36 resources in a namespace called `default`, and every one
of them is an artifact of how it was read. A Helm template renders to `default`
when nothing passes --namespace, which is why the resources came out named
Deployment.default.simplyblock-webappapi. An installation names its namespace,
and the manifests under csi-driver/deploy omit it deliberately so a reader can
apply them wherever they happen to be working.

CKV_K8S_43 wants an image digest. The charts take a repository and a tag as
values, which is how a user points them at their own registry or pins their own
build, so a digest would remove the thing that makes them configurable. The tags
are still checked: CKV_K8S_14 stays enabled, and it found
simplyblock/spdkcsi:latest in csi-driver/deploy/kubernetes/controller.yaml and
node.yaml, where every upstream sidecar beside it names a version.

The upstream snapshot controller is skipped by path. It is
kubernetes-csi/external-snapshotter's manifest copied into both trees with its
header comment intact, and its 37 findings belong to the project that wrote it.
Changing them here would fork a vendored file and lose the change at the next
copy.

243 findings to 158. What is left is not a configuration problem: 69 of them are
the five control-plane Deployments and the MinIO StatefulSet in
controlplane_deploy.yaml, and 42 more are the CSI controller and node plugins,
whose privileges are the ones this product actually needs.
A node plugin mounts filesystems and opens raw block devices on the host. It
runs privileged, shares the host network namespace, and holds CAP_SYS_ADMIN,
and the controller is not far behind. Twenty-five of the 42 findings on these
two manifests were checkov reading that design back as a list of faults: not
privileged, no added capabilities, no host network, a read-only root
filesystem, a high uid. A CSI driver cannot be any of those things.

Skipping by path is the only way to express it, because checkov cannot disable
a check for one file, and it costs the seventeen findings on these manifests
that were not about privilege. The Helm chart under csi-driver/charts is the
supported way to install this driver and these are the walkthrough's manual
path, so the resource limits and probes they lack are worth less than the same
gap in the chart would be.

One of the seventeen is worth acting on separately, and is recorded here so it
is not lost with the rest: controller.yaml and node.yaml both run
`simplyblock/spdkcsi:latest`, while every upstream sidecar beside them names a
version (csi-provisioner:v4.0.1, csi-snapshotter:v8.2.0, csi-attacher:v4.5.1).
A floating tag on the one image this project builds means an install cannot be
reproduced and a rollback has nothing to roll back to. Pinning it needs somebody
to say which version, so it is not done here.

158 findings to 116, and 111 of those are the chart.
299 function declarations across 116 files kept their body on the signature's
line, as `func (GroupByHardware) Name() string { return "identical hardware" }`
does. They are split, and nothing else about them changes.

The rewrite was mechanical: for each declaration whose body opened and closed on
one line, a newline went in after the brace and before it, and gofmt did the
indentation. It is safe by construction, because a one-line body cannot contain
a line comment: the comment would swallow its own closing brace and the file
would not have parsed.

Verified rather than assumed. All four modules build, `go vet ./...` passes in
each of them, which type-checks the test files that hold 60 of these, and the
atlas-lib suite is green. An AST scan of the result finds none left, and the
same scan found the same 299 before, from an independently written program.

Empty bodies are untouched: `func (noopReporter) Report() {}` is how a no-op
implementation of an interface is written, and 14 of those stay as they are.
Function literals are untouched too, because `defer func() { cancel() }()` and a
closure passed to a Ginkgo It read better on one line.

The rule that keeps this true follows in the next commit.

The house style gate reports 178 findings in the comments of these files, none
of them on a line this change touched, all of them British spellings and
quotation marks that predate the gate. They are left for a deliberate pass.
The rule the previous commit satisfies, written down so it stays satisfied: a
function declaration's body does not share a line with its signature.

Nothing off the shelf expresses it. revive carries no such check among its
rules, and neither gofmt nor gofumpt reformats `func f() int { return 1 }`.
Both were run against a fixture rather than taken on trust, and both left it
alone.

It is an AST rule rather than a pattern because a signature can legally hold a
brace before the body's. `func f() map[string]struct{} { return nil }` is in the
test fixture for exactly that reason, and a regex reading left to right finds
the wrong brace. go/ast finds the right one.

Two front ends over one analyzer:

- cmd/onelinefunc runs it alone, through singlechecker, so a Makefile target or
  a CI step can enforce it without rebuilding golangci-lint. It also reaches
  test/integration, which repo_lint.yaml's matrix does not lint at all.
- plugin.go registers it as a golangci-lint module plugin, for the route that
  puts it beside the linters each component's .golangci.yml already enables.

Neither is wired up yet, deliberately. The golangci-lint route costs more than
it looks: $(GOLANGCI_LINT) is a release binary pinned by sha256 in
scripts/tools.lock and used by three Makefiles, a plugin cannot be downloaded,
and atlas-lib has no .golangci.yml to enable it in.

It lives in hack/ with a go.mod of its own because golangci-lint compiles a
plugin into a bespoke binary and nothing here imports it. The house rule against
new modules is about shared code the operator and the CSI driver would each
need a replace directive for, which a build-time tool neither of them links is
not.
394 fixes across 81 files, all of them in comments. The house style gate reports
608 errors over the repository's 698 non-generated Go files; this clears the ones
that are prose, and leaves the ones that are not.

Applied: British spellings (behaviour, cancelled, initialise, honour, dialled,
signalled, backwards, centre and the rest), the comma that e.g. and i.e. take,
the adverb that is not hyphenated to its adjective, product names spelled the
way their owners spell them (K8s, Docker, JSON, I/O, Kubernetes, Kubebuilder,
Slack), and the brand lowercase mid-sentence.

Not applied, deliberately: all 144 quoted-mark findings. The rule puts a comma
or a full stop inside the closing quotation mark, which is right for a quoted
phrase and wrong for a quoted value, and a Go comment documenting a field is
full of quoted values. Left to the fixer it rewrote `"^sb-fio-baseline-.*".`
into a regex with an extra character, `("",` into `(","`, a
`fmt.Sprintf("/api/v2/clusters/%s/storage-pools/%s",` into a format string
ending in a comma, and `"live", "connecting", "resetting",` into a list whose
separators had moved inside the values. The house style already answers this:
a value takes backticks rather than quotation marks. Doing that is a separate
pass, and until then these stay as they are.

Eighteen fixes were applied and then reverted, because the checkers cannot tell
prose from a name:

- `[nvme]` and `[nvmeof]` are godoc links, on lines that still said
  [nvme.Siblings] and [nvmeof.WaitForDevice] after the rewrite.
- `nqn` names a parameter in seventeen doc comments, and godoc refers to a
  parameter by the name it has.
- `/nics` is an endpoint, not a plural.
- `public.ecr.aws/simply-block` is a registry path, which the adverb-hyphen rule
  turned into `simply block`.
- `"xfs"` is what mkfs is passed, and `delete(hostDomains, ip)` is an expression
  quoted in a comment.

That no code changed is not an assumption: every one of the 81 files was
tokenized before and after with go/scanner, comments skipped, and the token
streams compared. They are identical. `go vet ./...` passes in all four modules
and gofmt is clean.
The quoted-mark rule puts a comma or a full stop inside the closing quotation
mark. That is right for a quoted phrase and wrong for a quoted value, and the
144 findings it raised over the Go comments were nearly all values: the status
strings a StorageNode reports, the durations and sizes a field accepts, device
paths, name templates, a regex.

The house style answers this rather than arguing with it: a value or an
identifier takes backticks instead of quotation marks. 1164 comment lines in
this repository already do, `nvme0n1` and `ng0n1` among them, so 62 more follow
the convention rather than inventing one.

    // OnlineNodes is the count of StorageNode CRs with status `online`.
    // MaxAge is the maximum age of backups to retain (e.g., `7d`, `12h`, `30m`).
    // Defaults to `^sb-fio-baseline-.*`.

Seven were genuine quotations and took the rule as written, the mark moving
inside: "the peer that can answer for node worker-3," "realign when nothing has
moved," "was this address here before."

Twenty findings are left and none of them is prose. Six comment lines hold
commented-out Go or a literal document, and the rule is reading their string
literals:

    // log.Error(err, "StoragePool update failed", "status", status, ...)
    // endpoint := fmt.Sprintf("/api/v2/clusters/%s/storage-pools/%s", ...)
    // {"status":"success","data":{"resultType":"vector","result":[...]}}
    // (e.g., ["nvme0n1","nvme1n1"]).

Moving a comma into any of those would corrupt what the line is showing, so they
stay. Teaching the checker to skip a comment line that parses as code is the
fix, and it belongs in the checker.

Every one of the 43 files was tokenized before and after with comments skipped,
and the token streams are identical. `go vet ./...` passes in all four modules
and gofmt is clean.
The configuration is worth landing and the findings are worth reading, but a
job that runs for tens of minutes on every push to main and every pull request,
to report roughly a thousand findings nobody is working down yet, teaches people
to scroll past it. It runs by hand until that reading has happened.

workflow_dispatch keeps it available in the meantime. The file records what it
should become, which is a push and a pull_request trigger on main, and says that
DISABLE_ERRORS comes out of .mega-linter.yml at the same time so a finding fails
the job rather than being noted in it.
Copilot AI lite review requested due to automatic review settings September 10, 2026 20:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

It touches a very large surface area (CI/linting gates, Docker builds, generated manifests, and widespread edits), so a final human pass is needed to confirm the new gate behavior and release impact.

Pull request overview

This PR introduces MegaLinter as a repo-wide lint gate (kept manual-only for now), adds/tunes linter configuration to make results actionable, and applies the resulting mechanical fixes across Go, YAML, Markdown, shell, and Docker build artifacts. It also removes dead-code artifacts (unused script + draft CRDs) and includes a container-hardening change for the rebalancer image build.

Changes:

  • Add MegaLinter-facing config (revive, yamllint, markdownlint, jscpd) and ignore patterns; update terminology checks.
  • Apply large-scale formatting/style fixes (Go one-line funcs → multi-line bodies; comment/doc spelling/terminology normalization).
  • Harden/clean up container + manifest artifacts (GPG verification for Oracle repos; pin runAsUser; remove dead CSI-driver CRD drafts and unused script).
File summaries
File Description
test/integration/suites/volstack_test.go Go formatting: expand one-line helper into multi-line form
test/integration/onnode/volstack_test.go Go formatting: expand one-line methods into multi-line form
test/integration/fabric/target.go Go formatting: expand one-line accessors into multi-line form
test/integration/fabric/nodeshell.go Go formatting: expand one-line accessors into multi-line form
test/integration/controlplane/state.go Go formatting: expand one-line method into multi-line form
test/integration/controlplane/server.go Go formatting: expand one-line methods into multi-line form
test/integration/cluster/talos.go Go formatting: expand one-line accessors into multi-line form
test/integration/cluster/copy.go Go formatting: expand one-line helper into multi-line form
revive.toml Add revive ruleset tuned for this repo (MegaLinter GO_REVIVE)
operator/test/utils/utils.go Comment/style normalization (terminology capitalization)
operator/test/e2e/rbac_test.go Comment style normalization (American English)
operator/README.md Docs style normalization (American English + punctuation)
operator/internal/webhook/storagenode_validator.go Comment punctuation normalization
operator/internal/webhook/simplyblock_volume_placement_injector.go Comment style normalization (but introduces a minor grammar issue)
operator/internal/webhook/simplyblock_volume_placement_injector_test.go Comment terminology normalization (“K8s”)
operator/internal/webhook/simplyblock_rebalancer_injector.go Comment terminology + quoting normalization
operator/internal/webhook/certmanager.go Comment style + Go formatting: expand one-line method
operator/internal/webhook/certmanager_test.go Comment terminology normalization (American English)
operator/internal/webapi/rebalancing.go Comment normalization + wording updates around NQN terminology
operator/internal/webapi/client.go Comment punctuation normalization
operator/internal/volumemigration/validate.go Comment terminology normalization
operator/internal/volumemigration/validate_test.go Comment punctuation normalization
operator/internal/volumemigration/utils.go Comment quoting normalization
operator/internal/volumemigration/subsystem.go Comment quoting normalization
operator/internal/volumemigration/subsystem_test.go Comment quoting normalization
operator/internal/volumemigration/scoring.go Comment terminology normalization (American English)
operator/internal/volumemigration/release.go Comment terminology normalization + wording tweaks
operator/internal/volumemigration/paths.go Comment quoting/punctuation normalization
operator/internal/volumemigration/paths_test.go Comment punctuation normalization
operator/internal/volumemigration/config_test.go Comment terminology normalization (American English)
operator/internal/utils/objects.go Comment clarity/terminology normalization
operator/internal/upgrade/tui/reporter.go Go formatting: expand one-line method
operator/internal/upgrade/subject.go Go formatting: expand one-line methods
operator/internal/upgrade/steps/upgrade.go Go formatting: expand one-line interface methods
operator/internal/upgrade/steps/ownership.go Go formatting: expand one-line interface methods
operator/internal/upgrade/steps/described.go Go formatting: expand one-line methods
operator/internal/upgrade/steps/crds.go Go formatting: expand one-line interface methods
operator/internal/upgrade/runner_test.go Go formatting: expand one-line methods
operator/internal/upgrade/rule.go Go formatting: expand one-line method
operator/internal/upgrade/report.go Go formatting: expand one-line methods
operator/internal/upgrade/release/release.go Go formatting: expand one-line method
operator/internal/upgrade/registry_test.go Go formatting: expand one-line methods
operator/internal/upgrade/readonly.go Go formatting: expand one-line constructor
operator/internal/upgrade/position.go Go formatting: expand one-line methods
operator/internal/upgrade/plan.go Go formatting: expand one-line methods
operator/internal/upgrade/phase.go Go formatting: expand one-line method
operator/internal/upgrade/keys/keys.go Go formatting: expand one-line methods
operator/internal/upgrade/helm/config.go Go formatting: expand one-line methods
operator/internal/upgrade/finding.go Go formatting: expand one-line methods
operator/internal/upgrade/discover/release.go Go formatting: expand one-line methods
operator/internal/upgrade/discover/kind.go Go formatting: expand one-line methods
operator/internal/upgrade/derive/rule.go Go formatting: expand one-line methods
operator/internal/upgrade/crds/crds.go Go formatting: expand one-line methods
operator/internal/upgrade/check.go Go formatting: expand one-line methods
operator/internal/upgrade/blocked_test.go Go formatting: expand one-line methods
operator/internal/tlsutil/tlsutil.go Comment wording normalization
operator/internal/nodeprobe/report.go Go formatting: expand one-line method
operator/internal/metricsapi/storage.go Go formatting: expand one-line REST interface methods
operator/internal/metricsapi/storage_test.go Go formatting: expand one-line method
operator/internal/metricsapi/server.go Go formatting: expand one-line method
operator/internal/discovery/rules.go Go formatting: expand one-line methods
operator/internal/discovery/placement.go Go formatting: expand one-line methods
operator/internal/discovery/grouping.go Go formatting: expand one-line methods
operator/internal/csilink/hub.go Comment normalization + Go formatting: expand one-line method
operator/internal/csilink/hub_test.go Comment terminology normalization (American English)
operator/internal/cpinformer/subscriptions/volume.go Go formatting: expand one-line method
operator/internal/cpinformer/subscriptions/volume_test.go Go formatting: expand one-line helper
operator/internal/cpinformer/subscriptions/node.go Go formatting: expand one-line methods
operator/internal/cpinformer/subscriptions/node_test.go Go formatting: expand one-line helper
operator/internal/cpinformer/subscriptions/device.go Go formatting: expand one-line methods
operator/internal/cpinformer/subscriptions/device_test.go Go formatting: expand one-line helper
operator/internal/cpinformer/subscriptions/cache.go Go formatting: expand one-line methods
operator/internal/cpinformer/store_test.go Comment quoting normalization
operator/internal/cpinformer/sse.go Comment quoting/terminology normalization
operator/internal/cpinformer/manager.go Go formatting: expand one-line method
operator/internal/cpinformer/manager_test.go Go formatting: expand one-line methods
operator/internal/controllers/deployment/operatorops_unit_test.go Go formatting: expand one-line helper
operator/internal/controller/volumerebalancer_realignment_test.go Comment punctuation/terminology normalization
operator/internal/controller/volumerebalancer_controller.go Comment punctuation normalization
operator/internal/controller/volumemigration_migration_paths_test.go Comment terminology normalization
operator/internal/controller/volumemigration_helpers_test.go Comment normalization + Go formatting: expand one-line methods
operator/internal/controller/volumemigration_controller.go Comment wording/quoting normalization
operator/internal/controller/volumemigration_controller_unit_test.go Comment terminology/quoting normalization
operator/internal/controller/storagenodeops_controller.go Comment punctuation/quoting normalization
operator/internal/controller/storagenodeops_controller_unit_test.go Comment normalization (but introduces “Slack” capitalization issue)
operator/internal/controller/storagenode_latency_controller.go Comment terminology normalization (simplyblock/K8s)
operator/internal/controller/storagenode_controller.go Comment quoting/punctuation normalization
operator/internal/controller/storagedevice_controller_unit_test.go Go formatting: expand one-line helpers/methods
operator/internal/controller/storageclusterops_noderollingrestart.go Comment terminology normalization (American English)
operator/internal/controller/storageclusterops_controller.go Comment quoting normalization
operator/internal/controller/storageclusterops_controller_unit_test.go Comment wording normalization
operator/internal/controller/storagebackupsync_controller.go Comment punctuation + terminology normalization
operator/internal/controller/storagebackupsync_controller_unit_test.go Comment punctuation normalization
operator/internal/controller/storagebackup_controller.go Comment punctuation normalization
operator/internal/controller/simplyblocktask_controller.go Comment terminology normalization (“Kubernetes”)
operator/internal/controller/simplyblockstoragepool_controller.go Comment indentation cleanup (commented-out code)
operator/internal/controller/simplyblockstoragenodeset_drain.go Comment punctuation normalization
operator/internal/controller/simplyblockstoragenodeset_drain_unit_test.go Comment terminology normalization (American English)
operator/internal/controller/simplyblockstoragenodeset_controller_unit_test.go Comment terminology/punctuation normalization
operator/internal/controller/replicationslot_controller_unit_test.go Comment quoting + terminology normalization
operator/internal/controller/replicationpolicy_controller.go Comment quoting normalization
operator/internal/controller/replicationops_controller.go Comment quoting normalization
operator/internal/controller/replicationops_controller_unit_test.go Comment quoting normalization
operator/internal/controller/persistentvolumeclaim_controller.go Comment punctuation normalization
operator/internal/controller/nodedrain_controller_unit_test.go Comment quoting + terminology normalization
operator/internal/controller/benchmark_provisioner.go Comment quoting + terminology normalization (simplyblock)
operator/internal/controller/backuppolicy_controller_unit_test.go Comment punctuation normalization
operator/internal/autoplacement/utils.go Comment punctuation + terminology normalization
operator/internal/autoplacement/types.go Comment terminology normalization (“K8s”)
operator/internal/autoplacement/storage_node_selector.go Comment punctuation/terminology normalization
operator/internal/autoplacement/rebalancer.go Comment punctuation/terminology normalization
operator/internal/autoplacement/logical_volume_selector.go Comment punctuation/quoting normalization
operator/internal/autoplacement/baseline.go Comment terminology normalization (American English)
operator/internal/autoplacement/baseline_strategy.go Comment terminology normalization (American English)
operator/internal/autoplacement/autobalancing_test.go Go formatting: expand one-line helper
operator/Dockerfile.simplyblock-rebalancer Security hardening: import Oracle GPG key; remove --nogpgcheck; add checkov skip rationale
operator/Dockerfile Add checkov skip rationale for security patch layer
operator/dist/install.yaml Pin runAsUser: 65532 in generated install manifest
operator/config/manager/manager.yaml Pin runAsUser: 65532 in kustomize manager base
operator/cmd/simplyblock-upgrade/run.go Go formatting: expand one-line method
operator/cmd/simplyblock-rebalancer/validate_migration.go Comment terminology update (but command name becomes inaccurate)
operator/cmd/simplyblock-rebalancer/validate_migration_test.go Comment terminology normalization
operator/cmd/simplyblock-rebalancer/main.go Comment terminology/quoting normalization
operator/cmd/simplyblock-nodeprobe/main_test.go Go formatting: expand one-line helper
operator/bundle.Dockerfile Add checkov skip rationale for metadata-only bundle image
operator/api/v1alpha1/volumemigration_types.go Comment terminology normalization (American English)
operator/api/v1alpha1/task_types.go Comment wording normalization (JSON capitalization)
operator/api/v1alpha1/storagepool_types.go Comment wording/quoting normalization
operator/api/v1alpha1/storagenodeops_types.go Comment quoting normalization
operator/api/v1alpha1/storagenode_types.go Comment punctuation/quoting normalization
operator/api/v1alpha1/storageclusterops_types.go Comment terminology normalization (American English)
operator/api/v1alpha1/storagebackup_types.go Comment wording normalization
operator/api/v1alpha1/replicationpolicy_types.go Comment quoting normalization
operator/api/v1alpha1/replicationops_types.go Comment quoting/punctuation normalization
operator/api/v1alpha1/controlplane_types.go Comment punctuation normalization
operator/api/v1alpha1/backuprestore_types.go Comment wording normalization
operator/api/v1alpha1/backuppolicy_types.go Comment terminology normalization (American English)
helm-charts/README.md Docs terminology normalization (simplyblock lowercase)
hack/golangci-onelinefunc/plugin.go New golangci-lint plugin wrapper for onelinefunc analyzer
hack/golangci-onelinefunc/go.sum New module dependency locks for the plugin tool module
hack/golangci-onelinefunc/go.mod New tool module (Go version currently mismatched vs repo modules)
hack/golangci-onelinefunc/cmd/onelinefunc/main.go Standalone CLI front-end for the analyzer
hack/golangci-onelinefunc/analyzer.go New AST analyzer implementing the one-line func-body rule
csi-driver/scripts/config-gen-upgrade.sh Remove dead/unparseable script
csi-driver/README.md Docs wording/typography normalization
csi-driver/internal/guardian/guardian_test.go Go formatting: expand one-line helper
csi-driver/internal/fabric/repair_test.go Go formatting: expand one-line methods
csi-driver/internal/csi/controller/mock_controlplane_test.go Go formatting: expand one-line methods
csi-driver/internal/csi/controller/errorclass_rpc.go Go formatting: expand one-line methods
csi-driver/internal/controlplane/cluster.go Go formatting: expand one-line methods
csi-driver/deploy/CRDs/synch-replication.yaml Remove draft/dead CRD manifest
csi-driver/deploy/CRDs/replication.yaml Remove draft/dead CRD manifest
csi-driver/deploy/CRDs/README.MD Remove scratchpad README content from draft CRD dir
csi-driver/deploy/CRDs/pool.yaml Remove draft/dead CRD manifest
csi-driver/deploy/CRDs/lvolmigration.yaml Remove draft/dead CRD manifest
csi-driver/deploy/CRDs/lvol.yaml Remove draft/dead CRD manifest
csi-driver/deploy/CRDs/Devices.yaml Remove draft/dead CRD manifest
csi-driver/deploy/CRDs/ClusterTasks.yaml Remove draft/dead CRD manifest
atlas-lib/volstack/runner.go Go formatting: expand one-line constructor
atlas-lib/volstack/runner_test.go Go formatting: expand one-line methods/helpers
atlas-lib/volstack/record.go Go formatting: expand one-line constructor
atlas-lib/volstack/plans/plans.go Go formatting: expand one-line naming helpers
atlas-lib/volstack/plans/plans_test.go Go formatting: expand one-line methods
atlas-lib/volstack/layers/staging_probe_test.go Go formatting: expand one-line methods
atlas-lib/volstack/layers/members.go Go formatting: expand one-line methods/constructors
atlas-lib/volstack/layers/members_test.go Go formatting: expand one-line methods/helpers
atlas-lib/volstack/layers/lvmvolumegroup.go Go formatting: expand one-line method
atlas-lib/volstack/layers/lvmvolumegroup_test.go Go formatting: expand one-line helper
atlas-lib/volstack/layers/lvmvolume.go Go formatting: expand one-line methods/helpers
atlas-lib/volstack/layers/lvmvolume_test.go Go formatting: expand one-line helper
atlas-lib/volstack/layers/lvmphysicalvolume.go Go formatting: expand one-line methods
atlas-lib/volstack/layers/lvmphysicalvolume_test.go Go formatting: expand one-line methods
atlas-lib/volstack/layers/filesystem.go Go formatting: expand one-line methods/constructors
atlas-lib/volstack/layers/filesystem_test.go Go formatting: expand one-line helper
atlas-lib/volstack/layers/filesystem_strategy.go Go formatting: expand one-line methods
atlas-lib/volstack/layers/fabric.go Go formatting: expand one-line methods/constructors
atlas-lib/volstack/layers/fabric_test.go Go formatting: expand one-line methods
atlas-lib/volstack/layer.go Go formatting: expand one-line method
atlas-lib/storage/storagerpc/storagerpc_test.go Go formatting: expand one-line method
atlas-lib/storage/storagerpc/server.go Go formatting: expand one-line method
atlas-lib/storage/storagerpc/doc.go Comment terminology normalization (American English)
atlas-lib/storage/storagerpc/convert.go Comment terminology normalization (American English)
atlas-lib/storage/storagerpc/convert_test.go Comment wording normalization
atlas-lib/storage/storagerpc/client.go Comment quoting normalization
atlas-lib/storage/accessor_test.go Go formatting: expand one-line method
atlas-lib/prometheus/client.go Go formatting: expand one-line helper
atlas-lib/prometheus/capacity.go Go formatting: expand one-line method
atlas-lib/pci/scan.go Go formatting: expand one-line methods
atlas-lib/nvmeof/wait_test.go Go formatting: expand one-line method
atlas-lib/nvmeof/repair_test.go Go formatting: expand one-line methods
atlas-lib/nvmeof/reconcile.go Go formatting: expand one-line methods
atlas-lib/nvmeof/fabrics_test.go Go formatting: expand one-line method
atlas-lib/nvmeof/detach_test.go Go formatting: expand one-line methods
atlas-lib/nvme/siblings.go Go formatting: expand one-line methods
atlas-lib/lvm/volume_test.go Go formatting: expand one-line methods
atlas-lib/link/session.go Comment terminology normalization + Go formatting: expand one-line methods
atlas-lib/link/registry.go Comment punctuation + terminology normalization
atlas-lib/link/peer.go Comment quoting + Go formatting: expand one-line methods
atlas-lib/link/kubeauth.go Comment quoting normalization
atlas-lib/link/hub.go Go formatting: expand one-line methods
atlas-lib/link/errors.go Go formatting: expand one-line method
atlas-lib/link/doc.go Comment wording normalization (“backward”)
atlas-lib/link/dial.go Comment terminology normalization
atlas-lib/kube/derived.go Go formatting: expand one-line methods/functions
atlas-lib/inventory/inventory.go Go formatting: expand one-line methods
atlas-lib/inventory/hugepages.go Go formatting: expand one-line methods
atlas-lib/inventory/devices_fixture_test.go Go formatting: expand one-line helpers
atlas-lib/errs/deferrers/deferrers_test.go Go formatting: expand one-line method
atlas-lib/errs/class/class.go Go formatting: expand one-line methods/functions
atlas-lib/errs/class/class_test.go Go formatting: expand one-line methods
atlas-lib/controlplane/errors.go Go formatting: expand one-line method
atlas-lib/blockdev/usage_test.go Go formatting: expand one-line helper
atlas-lib/blockdev/local_other.go Go formatting: expand one-line functions
atlas-lib/blockdev/local_linux.go Go formatting: expand one-line methods
atlas-lib/blockdev/fixture_test.go Go formatting: expand one-line methods/helpers
atlas-lib/blockdev/content.go Go formatting: expand one-line helper
atlas-lib/blockdev/candidate.go Go formatting: expand one-line method
atlas-lib/blockdev/candidate_test.go Go formatting: expand one-line helpers/methods
atlas-lib/blockdev/boundary_test.go Go formatting: expand one-line method
atlas-lib/blockdev/blkid.go Go formatting: expand one-line constructors
AGENTS.md Terminology normalization (“Kubebuilder”)
.yamllint.yml Add repository yamllint configuration for MegaLinter
.markdownlint.yaml Add markdownlint configuration tuned for repo Markdown
.jscpd.json Add duplication detection config (paths excluded, thresholds set)
.gitignore Ignore MegaLinter outputs and ruff cache artifacts
.claude/skills/house-style/scripts/check-terminology.py Extend terminology whitelist (Kubebuilder, OLM)
Review details
  • Files reviewed: 238/244 changed files
  • Comments generated: 5
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread hack/golangci-onelinefunc/go.mod Outdated
Comment thread operator/cmd/simplyblock-rebalancer/validate_migration.go Outdated
Comment thread operator/internal/controller/storagenodeops_controller_unit_test.go Outdated
Comment thread operator/internal/webapi/rebalancing.go Outdated
Comment thread operator/internal/webhook/simplyblock_volume_placement_injector.go Outdated
Three CI checks failed on drift, all of them for the same reason: a doc comment
on an API type is not only a comment. controller-gen copies it into the CRD's
description, so editing the comments in operator/api/v1alpha1 rewrote the
generated schemas, and the checked-in copies no longer matched.

Regenerated: operator/config/crd/bases, the embedded manifests under
operator/internal/upgrade/crds, the chart's crds/ through `make helm-sync`, and
dist/install.yaml. Every changed line is description text. Running the
generators a second time changes nothing further.

One description is not a straight propagation. `Fabric is the transport fabric
(e.g. tcp)` had become `(e.g., TCP)`, which put the protocol in prose next to a
field whose own default is `tcp`, and `kubectl explain` shows the two together.
It reads `(e.g., `tcp`)` now: the comma the rule asks for, and the value in
backticks, which is what the field actually takes.

Backticks in a description are not new here. The neighboring text already
carried `tune2fs -m 0` that way before this branch.
Copilot found five things on the pull request and four of them are mine: places
where a house style fixer rewrote a word that was not prose, and my review of
the substitution list did not catch it. I checked the risky-looking terms and
waved the ordinary-looking ones through, which is how `slack` and `forwards`
got past.

Restored:

- `nvme connect` and `nvme list` are commands. The terminology rule spells the
  protocol NVMe, but these two name a binary and a subcommand, and the point of
  writing them exactly is that somebody can grep a log for them.
- `slack` is spare capacity in that test's fixture, not the product.
- `reads and forwards` is a verb. The American English rule reads the -wards
  ending as an adverb, which is right for backwards and afterwards and wrong
  here.
- `uuid` and `uuids` name parameters. "UUID matches uuid" had become "UUID
  matches UUID", which says nothing.
- Nine occurrences of NQN that my earlier revert downgraded. Reverting
  NQN back to nqn was right where the word names a parameter and wrong in
  paths.go, release.go, and rebalancing.go, where it names the concept or the
  field the comment documents. The blanket replacement did not distinguish them;
  these are restored to what origin/main had.

A systematic check replaces the eyeballing that missed them: for every
lowercase-to-cased substitution, does the lowercase form also appear as a
declared identifier in the same file? It reports none now.

The tool module's Go directive drops from 1.26.5, which was whatever the local
toolchain happened to be, to the 1.26.2 the four component modules share.

The house style gate now fails on these files, with fourteen errors that are all
correct code: six nqn, three uuid, and one each of slack, ip, nics, forwards,
and a quoted error body. A fifteenth, MigrationStatusCancelled, predates this
branch and is an exported name, so renaming it is an API change. Until the
checkers learn that a doc comment names its parameters as they are spelled, that
[nvme] is a godoc link, and that a comment line can be commented-out code, that
is the floor for Go and no amount of editing the source lowers it.
The plugin was added two commits ago and nothing ran it. It runs now, in
operator, csi-driver, and atlas-lib alike.

golangci-lint cannot load a plugin at run time: the linter has to be compiled
in. `.custom-gcl.yml` describes that build against the v2.11.4 that
scripts/tools.manifest already pins, and scripts/tools.mk grows a $(CUSTOM_GCL)
target that produces .bin/custom-gcl from the plugin's sources. Every lint
target now runs that binary instead of the downloaded one, which does not know
the linter and refuses a config that enables it.

atlas-lib had no .golangci.yml at all, and so ran golangci-lint's standard set.
The new file says `default: standard` rather than `default: none`, because the
point is to add one linter, not to quietly stop running the ones the module has
been checked with all along.

repo_lint.yaml stops using golangci-lint-action, which installs the released
binary. It builds custom-gcl and runs it directly rather than through
`make -C <component> lint`, because atlas-lib's lint target also runs code
generation that this job has never provisioned and the action never triggered.

Verified from an empty .bin: `make custom-gcl` builds it, `custom-gcl linters`
lists onelinefunc as enabled in each of the three modules, and a full run in
each reports 0 issues. A temporary one-line function in
operator/internal/discovery is reported, so the rule fires rather than merely
loading.

Two things this cost, both worth knowing. The build clones and compiles
golangci-lint, so the first lint of the day takes a minute or two; the target is
gated on the plugin's sources and .custom-gcl.yml, so it does not rebuild
otherwise. And the version in .custom-gcl.yml has to move with the one in
scripts/tools.manifest, because the plugin compiles against golangci-lint's own
module and a mismatch fails the build.
When checkov's scoping moved from `--directory` arguments to skip-path in
.checkov.yml, the two keys went and the paragraph explaining them stayed. It
claimed the scope is set by argument, which it is not, and ended on "the ones
added below" with nothing below it. It had also run into the next comment,
having eaten the blank line between them.

It now says where the scope actually lives, and keeps the three measurements as
what they are useful for: a record of the routes that do not work, so nobody
spends the afternoon rediscovering that FORWARD_EXCLUDED_DIRECTORIES is a no-op
or that repeating `--directory` reports every finding once per directory.

The finding count in the note below it was from the first clean run, before the
exclusions and the fixes. It is roughly a thousand now, not 1,880.
Fifteen conflicts, and only three of them were real. The other twelve were
generated CRDs in the three trees that carry them, resolved by regenerating from
the merged sources rather than by choosing sides in a schema.

The three source conflicts were all the same shape, main changing the code and
this branch changing the spelling or the formatting around it, so both sides are
kept: the status field is RollingRestart as main renamed it and the comment
above it says initialization; StorageDevice and the metrics kinds are v1alpha2
as main moved them, in bodies this branch had already split.

main also brought nine function declarations whose body sits on the signature's
line, which is what this branch forbids. Three arrived as conflicts and six did
not: NeedLeaderElection in the StorageDevice collector and the conversion
service, and four methods on DeviceStorage. All are split.

The six were nearly missed twice. The standalone checker was run with stderr
redirected to /dev/null, which is where singlechecker writes its diagnostics, so
it reported nothing. golangci-lint then found three, and finding three more
after those were fixed is what gave it away: max-same-issues defaults to three,
so the run had been truncating a rule whose whole output is one identical
message per site. The three .golangci.yml files now set that limit and
max-issues-per-linter to zero, because a gate that says "3 issues" when there
are thirty is worse than one that takes a minute longer.

Verified on the merged tree: four modules build, `go vet ./...` passes in each,
gofmt is clean, custom-gcl reports 0 issues in all three components with no caps
applied, and a second run of the generators changes nothing.
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.

2 participants