Skip to content

Commit b69f2df

Browse files
committed
docs(agents): define testing and authoring conventions
Require test-first for testable behavior without inventing failing tests for documentation or unmodified upstream updates. Distinguish an intentional contract change from weakening a test to hide a regression. Keep new image definitions vendor-neutral and render inputs explicit. A lookup needs an essential use case and maintainer approval because changes to its input object do not trigger a HelmRelease upgrade. Validation: markdownlint, 0 findings (MD013 excluded). Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <[email protected]>
1 parent 22e45c6 commit b69f2df

1 file changed

Lines changed: 36 additions & 5 deletions

File tree

‎docs/agents/overview.md‎

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ The platform exposes infrastructure services via the Kubernetes API with ready-m
1616

1717
## Code Layout
1818

19-
```
19+
```text
2020
.
2121
├── packages/ # Main directory for cozystack packages
2222
│ ├── core/ # Core platform logic charts (installer, platform)
@@ -50,7 +50,7 @@ The platform exposes infrastructure services via the Kubernetes API with ready-m
5050

5151
Every package is a Helm chart following the umbrella chart pattern:
5252

53-
```
53+
```text
5454
packages/<category>/<package-name>/
5555
├── Chart.yaml # Chart definition and parameter docs
5656
├── Makefile # Development workflow targets
@@ -110,21 +110,48 @@ A chart also declares the limits on its own resource name there, with `## @name
110110
- **E2E tests:** Kyverno Chainsaw suites in `hack/e2e-chainsaw/` (one directory per app), run with `chainsaw test`. Cluster bootstrap (`hack/e2e-install-cozystack.bats`) and the OpenAPI checks (`hack/e2e-test-openapi.bats`) remain BATS. Conventions for writing and stabilising them — and the CI that runs them — live in [`e2e-testing.md`](./e2e-testing.md).
111111
- **Go tests:** standard `go test`, with Ginkgo/Gomega for controllers.
112112

113+
### Test first, failure path first
114+
115+
These rules apply to changes with testable behavior. Documentation-only edits and unmodified upstream updates need the relevant checks, not an artificial failing test. First-party adaptations made during a dependency bump remain covered by these rules.
116+
117+
1. Write the test before changing the implementation and watch it fail for the intended reason, not on a typo or a missing fixture. Then make it green, then refactor under it.
118+
2. Cover relevant failure paths before the happy path: empty or null values, invalid input, a number where a string is expected, a missing resource, a timeout or an API error. Assert the resulting message, exit status and state left untouched.
119+
3. Fix regressions in the implementation. Do not skip, weaken or delete a test, or silence a linter, merely to get a green run. When the intended contract changes, update or replace the obsolete assertions and retain coverage of applicable failure cases.
120+
4. A stub (a not-implemented branch, an empty implementation) lands together with a test that fails on it and states the behaviour expected once it is done.
121+
5. A bug found along the way that is out of the PR's scope gets an issue and a test pinning the current broken behaviour, marked `FIXME(#N)`. The PR that fixes the bug flips the test to pin the fixed behaviour.
122+
123+
A negated BATS assertion can never be red; see [§11 of `e2e-testing.md`](./e2e-testing.md#11-never-negate-a-bats-assertion-with-).
124+
125+
### Linters
126+
127+
Fix every linter finding before pushing, the cosmetic ones included. When a rule genuinely conflicts with the project, change the linter configuration and state the reason there, instead of suppressing the finding inline.
128+
129+
Markdown follows these markdownlint rules: every code fence names a language (`text` when nothing else fits), table separators have spaces around the pipes (`| --- |`), and there is a blank line before and after every list and after every heading. Line length (MD013) is the exception: prose stays one line per paragraph, as [Prose Formatting](../../AGENTS.md#prose-formatting) explains.
130+
113131
## Conventions
114132

115133
### Helm Charts
134+
116135
- Follow **umbrella chart** pattern for system components
117136
- Include upstream charts in `charts/` directory (vendored, not referenced)
118137
- Override configuration in root `values.yaml`
119138
- Use `values.schema.json` for input validation and dashboard UI rendering
139+
- An unquoted numeric value (`name: 123`) reaches a template as a number: `len` on it aborts the render with `len of type float64` (`int64` for a value given with `--set`) before any `fail` message is reached, and `if not $v` treats `0` as unset. Coerce with `printf "%v"` before validating a value that may be an unquoted scalar, and test for emptiness with `eq $v ""` after the coercion. A YAML `null` or a missing key coerces to the string `<nil>`, which that test reports as non-empty, so reject it first with `kindIs "invalid"` on the raw value, before `printf`
140+
141+
**`lookup` is prohibited by default.** Do not add a call or expand an existing one unless it is essential to the required behavior and a maintainer approves that specific use. It introduces a live-state input that offline rendering cannot reproduce; a change to the object it reads does not itself trigger a HelmRelease upgrade. Existing calls are not precedent, and convenience is not sufficient justification.
142+
143+
Prefer values (`_cluster`, `_namespace` or `valuesFrom`) for external data and `.Capabilities.APIVersions.Has` for API availability. Perform checks at runtime in a hook or Job, or let a controller own the object, when that meets the requirement. For an approved exception, explain why these alternatives are insufficient and test both the empty/offline path and the live-object path with a `kubernetesProvider` mock, including the case of an unavailable API when applicable.
120144

121-
**No `lookup` that copies another object into the render.** A `lookup` reads cluster state at render time, and nothing re-renders the HelmRelease when that state changes, so whatever it copied into the output (a bucket name, an endpoint, a CA, a list of StorageClasses) stays stale until something unrelated upgrades the release. Lookups of this kind already in the charts have caused failures, and several have since been moved to values, so they are not precedent: do not add one of this kind unless a maintainer has authorized that specific one.
145+
### Container images
122146

123-
A `lookup` is fine when a repeated render gives the same output and carries nothing from another object into it: checking that an object exists, keeping a generated Secret's value, keeping an immutable field of the chart's own live object, or a guard that `fail`s on an unsafe change. Keep its empty branch rendering, because offline `helm template` and helm-unittest without a `kubernetesProvider` mock see only that branch; test the other branch with such a mock.
147+
A new first-party image is built from a `Containerfile`, following the [vendor-neutral naming principle](../../AGENTS.md#working-principles). The package `Makefile` passes it to `docker buildx build` with `--file`. Existing `Dockerfile`s and vendored build definitions are not renamed.
124148

125-
To use a fact about the cluster instead, read it from values: `_cluster` and `_namespace` in the `cozystack-values` Secret, or `valuesFrom` on a specific Secret, whose content change is a values change and so upgrades the release. Ask whether an API is served with `.Capabilities.APIVersions.Has`, since a `lookup` on an unserved group aborts the whole render. Otherwise check at run time in a hook or Job, or let a controller own the object.
149+
### Examples and fixtures
150+
151+
Addresses and names in docs, examples and test fixtures come from the reserved documentation ranges, never from a routable value that merely looks realistic: IPv4 from RFC 5737 (`192.0.2.0/24`, `198.51.100.0/24`, `203.0.113.0/24`), IPv6 from RFC 3849 (`2001:db8::/32`), domains from RFC 2606 and RFC 6761 (`example.com`, `.example`, `.test`, `.invalid`), ASNs from RFC 5398 (`64496`–`64511`, `65536`–`65551`), MAC addresses from RFC 7042 (`00:00:5E:00:53:00`–`00:00:5E:00:53:FF`). Where no reserved range exists, use an obviously synthetic placeholder such as `<NODE_IP>`.
126152

127153
### Go Code
154+
128155
- Follow standard **Go conventions** and idioms
129156
- Use **controller-runtime** patterns for Kubernetes controllers
130157
- Prefer **kubebuilder** for API definitions and controllers
@@ -170,20 +197,24 @@ Before working on these areas, read the relevant doc first:
170197
## Things Agents Should Not Do
171198

172199
### Never Edit These
200+
173201
- Do not modify files in `/vendor/` (Go dependencies)
174202
- Do not edit generated files: `zz_generated.*.go`
175203
- Do not change `go.mod`/`go.sum` manually (use `go get`)
176204
- Do not edit upstream charts in `packages/*/charts/` directly (use patches)
177205
- Do not modify image digests in `values.yaml` (generated by build)
178206

179207
### Version Control
208+
180209
- Do not commit built artifacts from `_out`
181210
- Do not commit test artifacts or temporary files
182211

183212
### Git Operations
213+
184214
- Do not force push to main/master
185215
- Do not update git config
186216
- Do not perform destructive operations without explicit request
187217

188218
### Core Components
219+
189220
- Do not modify `packages/core/platform/` without understanding migration impact

0 commit comments

Comments
 (0)