Skip to content

Latest commit

 

History

History
110 lines (77 loc) · 6.26 KB

styleguide.md

File metadata and controls

110 lines (77 loc) · 6.26 KB

Cozystack Review Guidelines

Project Architecture

Cozystack is a Kubernetes-based PaaS built on Helm umbrella charts and FluxCD. Packages live in packages/{core,system,apps,extra,library,tests}/. The library/ group holds reusable helper charts; the tests/ group exists because library charts are not directly testable. Each package wraps one or more upstream Helm charts. Go code in cmd/, internal/, pkg/ implements Kubernetes controllers and API server. Static CRDs are generated by hack/update-codegen.sh from types under api/v1alpha1/, api/backups/, and api/dashboard/. App Kinds (like Postgres, Kafka) live in api/apps/v1alpha1/ but are registered dynamically at runtime from ApplicationDefinition resources — they are not static CRDs.

Vendored Code — Critical Rules

Never suggest editing files inside any charts/ directory under packages/. Those are upstream Helm charts vendored via make update (which runs helm pull). Any direct edit is overwritten on the next update and provides zero value.

If you find an issue that appears to live in vendored chart code:

  • For configuration-level changes: suggest overrides in the package root values.yaml.
  • For structural changes: suggest a patch file in packages/<name>/patches/ applied by the Makefile.
  • For source-code changes in images: suggest a patch in packages/<name>/images/<name>/patches/.
  • For true upstream bugs: point to the upstream repository and suggest an upstream issue/PR.
  • Do NOT suggest creating charts/patches/ — patches never live inside charts/.

Similarly, never propose edits to:

  • vendor/ — Go dependencies. Changes go through go get and go mod tidy.
  • zz_generated.*.go — regenerated by make generate.
  • pkg/generated/ — auto-generated Kubernetes client code.
  • Image digest values in values.yaml — set by CI via make image, not by humans.
  • go.mod / go.sum by hand — use go get and go mod tidy.

Commit and PR Requirements

Each commit must follow Conventional Commits format: type(scope): brief description.

Valid types: feat, fix, security (a fix that comes through a security advisory), docs, style, refactor, perf, test, build, ci, chore.

Valid scopes:

  • System: dashboard, platform, cilium, kube-ovn, linstor, fluxcd, cluster-api
  • Apps: postgres, mariadb, redis, kafka, clickhouse, kubernetes, virtual-machine
  • Meta: api, hack, tests, ci, docs
  • Package-specific: any <package-name> matching a directory under packages/

Breaking changes: append ! after type/scope (feat(api)!: ...) or add a BREAKING CHANGE: footer.

Each commit must have a Signed-off-by: trailer (produced by git commit --signoff).

PR body must contain a release note block:

```release-note
type(scope): human-readable changelog entry
```

Flag any PR whose commits lack the Conventional Commits format or signoff, or whose body has no release-note block.

Helm Chart Conventions

Packages follow an umbrella chart pattern:

  • charts/ — vendored upstream, read-only
  • templates/ — Cozystack-specific extra manifests
  • values.yaml — override values for the upstream chart
  • values.schema.json — JSON Schema for dashboard UI and input validation

When reviewing values.schema.json:

  • make generate regenerates this file and strips title, description, and x-* custom annotations. Do not suggest adding fields that will be stripped on the next regeneration.
  • Focus on type correctness, required fields, enum values, and default values.
  • Flag breaking changes: removing a field, changing its type, or narrowing its enum.

Sensitive Components

packages/core/platform/: the platform chart that deploys everything. Changes here can require updates to the migration flow — the Helm hook in packages/core/platform/templates/migration-hook.yaml and the runner image with scripts in packages/core/platform/images/migrations/. Flag any change to this package that lacks a corresponding migration update or an explicit note that backward compatibility is preserved.

api/apps/v1alpha1/: app CRD Kinds are registered at runtime from ApplicationDefinition resources and the matching values.schema.json. Suggest changes to the relevant package schema rather than hand-editing generated types.

RBAC, ServiceAccounts, and SecurityContext: flag overly broad RBAC (* on resources or verbs without justification), missing securityContext, containers running as root without an explicit reason, and hostPath/hostNetwork usage without clear rationale.

Go Code Standards

  • Use controller-runtime patterns for reconcilers.
  • Use structured logging via logr — flag fmt.Print* and log.Print* in controller code.
  • Handle errors explicitly. Discarding meaningful errors with _ is a bug.
  • Propagate context.Context through call chains. Flag context.Background() created inside a reconciler or request handler.
  • Prefer ctrl.Result{RequeueAfter: ...} over empty requeue for predictable reconciliation loops.
  • Tests live beside the code (*_test.go). New behavior without tests is worth flagging.

What to Review Carefully

  • Logic errors, off-by-one bugs, nil dereferences.
  • Missing error handling, especially in reconcilers and API handlers.
  • Helm template correctness: missing quote, incorrect indentation, wrong scope in with/range.
  • Security: permissive RBAC, privileged containers, secrets in environment variables, hardcoded credentials.
  • Missing resource requests/limits on new workloads.
  • Breaking changes in values.schema.json: removed fields, tightened types, narrower enums.

Anti-patterns — Do Not Flag These

  • Large JSON files in dashboards/ — imported from upstream Grafana sources, not hand-written.
  • Files under any charts/ directory — vendored upstream, left as-is intentionally.
  • Whitespace or formatting in *.patch / *.diff files — machine-applied, not authored.
  • Missing comments on generated code.
  • go.sum changes accompanying go.mod changes — expected and correct.
  • Fork relationships for vendored tooling images — intentional (e.g., cozystack/kilo fork is expected).
  • Absence of unit tests for vendored chart overrides — covered by E2E tests in hack/e2e-chainsaw/.