You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit b69f2df
Browse filesBrowse the repository at this point in the historyBrowse files
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]>
@@ -50,7 +50,7 @@ The platform exposes infrastructure services via the Kubernetes API with ready-m
50
50
51
51
Every package is a Helm chart following the umbrella chart pattern:
52
52
53
-
```
53
+
```text
54
54
packages/<category>/<package-name>/
55
55
├── Chart.yaml # Chart definition and parameter docs
56
56
├── Makefile # Development workflow targets
@@ -110,21 +110,48 @@ A chart also declares the limits on its own resource name there, with `## @name
110
110
-**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).
111
111
-**Go tests:** standard `go test`, with Ginkgo/Gomega for controllers.
112
112
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
+
113
131
## Conventions
114
132
115
133
### Helm Charts
134
+
116
135
- Follow **umbrella chart** pattern for system components
117
136
- Include upstream charts in `charts/` directory (vendored, not referenced)
118
137
- Override configuration in root `values.yaml`
119
138
- 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.
120
144
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
122
146
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.
124
148
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>`.
126
152
127
153
### Go Code
154
+
128
155
- Follow standard **Go conventions** and idioms
129
156
- Use **controller-runtime** patterns for Kubernetes controllers
130
157
- Prefer **kubebuilder** for API definitions and controllers
@@ -170,20 +197,24 @@ Before working on these areas, read the relevant doc first:
170
197
## Things Agents Should Not Do
171
198
172
199
### Never Edit These
200
+
173
201
- Do not modify files in `/vendor/` (Go dependencies)
174
202
- Do not edit generated files: `zz_generated.*.go`
175
203
- Do not change `go.mod`/`go.sum` manually (use `go get`)
176
204
- Do not edit upstream charts in `packages/*/charts/` directly (use patches)
177
205
- Do not modify image digests in `values.yaml` (generated by build)
178
206
179
207
### Version Control
208
+
180
209
- Do not commit built artifacts from `_out`
181
210
- Do not commit test artifacts or temporary files
182
211
183
212
### Git Operations
213
+
184
214
- Do not force push to main/master
185
215
- Do not update git config
186
216
- Do not perform destructive operations without explicit request
187
217
188
218
### Core Components
219
+
189
220
- Do not modify `packages/core/platform/` without understanding migration impact
0 commit comments