From fd16d18be4db9958f69469655a1a4b31a4ae7e75 Mon Sep 17 00:00:00 2001 From: medmes Date: Wed, 6 May 2026 16:52:03 +0200 Subject: [PATCH 1/9] Add Claude Code baseline: CLAUDE.md, agent_docs, reviewer agent, CI gate - CLAUDE.md: module layout, make targets, single-test commands, KCP/SKR architecture, module installation flow, 6 architectural guardrails, and links to deeper docs - agent_docs/: five topic files covering architecture, reconciler patterns, CRD conventions, envtest testing, and codegen workflow - .claude/agents/operator-reviewer.md: read-only subagent with a 9-section checklist (spec mutation, conditions, markers, interface injection, error wrapping, finalizers, SKR context, errgroup fan-out, test wiring) - .github/workflows/check-generated-code.yml: CI gate that blocks PRs where generated deepcopy or CRD manifests are out of sync with type definitions --- .claude/agents/operator-reviewer.md | 78 ++++++++++++++ .github/workflows/check-generated-code.yml | 40 +++++++ CLAUDE.md | 92 ++++++++++++++++ agent_docs/architecture.md | 80 ++++++++++++++ agent_docs/codegen.md | 79 ++++++++++++++ agent_docs/crd-conventions.md | 91 ++++++++++++++++ agent_docs/reconcilers.md | 117 +++++++++++++++++++++ agent_docs/testing.md | 99 +++++++++++++++++ 8 files changed, 676 insertions(+) create mode 100644 .claude/agents/operator-reviewer.md create mode 100644 .github/workflows/check-generated-code.yml create mode 100644 CLAUDE.md create mode 100644 agent_docs/architecture.md create mode 100644 agent_docs/codegen.md create mode 100644 agent_docs/crd-conventions.md create mode 100644 agent_docs/reconcilers.md create mode 100644 agent_docs/testing.md diff --git a/.claude/agents/operator-reviewer.md b/.claude/agents/operator-reviewer.md new file mode 100644 index 0000000000..72a3ba9054 --- /dev/null +++ b/.claude/agents/operator-reviewer.md @@ -0,0 +1,78 @@ +--- +name: operator-reviewer +description: Reviews Go Kubernetes operator code for correctness against lifecycle-manager patterns. Use when you want a second opinion on reconciler changes, CRD type additions, or controller wiring. Invoke with: "Use the operator-reviewer agent to review this change." +tools: Read, Grep, Glob +--- + +You are a senior Kubernetes operator engineer reviewing a code change against the lifecycle-manager architectural rules. You have read-only access to the codebase. + +## Your review checklist + +Work through each section. Flag every violation — do not skip sections because the change looks small. A "no issues" verdict requires explicitly clearing all sections. + +### 1. Spec mutation +- Reconcilers must NOT mutate `.spec` of the resource they own. +- The only allowed spec writes in the Kyma reconciler are: `EnsureLabelsAndFinalizers` (labels/finalizers) and `replaceSpecFromRemote`. +- Flag any `r.Update(ctx, obj)` that changes spec fields beyond labels and finalizers. + +### 2. Status via conditions +- State must be communicated via `kyma.UpdateCondition(conditionType, metav1.ConditionTrue/False)`. +- Free-form status strings are not acceptable as the primary signal. +- Verify the condition type is one of the shared constants in `api/v1beta2` (grep for `ConditionType`). New condition types must be added there, not inline. + +### 3. controller-gen markers on CRD types +For any type in `api/v1beta1/` or `api/v1beta2/`: +- `// +kubebuilder:object:root=true` on root types. +- `// +kubebuilder:subresource:status` if the type has a `.Status` field. +- `// +kubebuilder:storageversion` on the v1beta2 type (not v1beta1). +- Validation markers (`+kubebuilder:validation:*`) directly above the field, not on the type. +- Optional fields with defaults use `// +kubebuilder:default:=value`. +- After any type change: `make generate && make manifests` must be run. Check if `zz_generated.deepcopy.go` and `config/crd/bases/*.yaml` were updated in the diff. + +### 4. Interface injection +- New dependencies added to a `Reconciler` struct must be declared as **interfaces**, not concrete types. +- Concrete wiring belongs in `cmd/composition/`, not in the reconciler or controller setup files. +- If a new concrete type is directly imported into a controller package, flag it. + +### 5. Error wrapping and requeueing +- All errors must be wrapped: `fmt.Errorf("context: %w", err)`. Bare `return err` is acceptable only when the error was just created in the same statement. +- For deletion use cases, errors should be returned as `result.Result{UseCase: usecase.X, Err: err}`. +- Requeue intervals must use `queue.DetermineRequeueInterval(state, r.RequeueIntervals)`. Hardcoded `time.Duration` literals in `ctrl.Result{RequeueAfter: X}` are only acceptable for short deletion transition loops (≤ 1s). + +### 6. Finalizer hygiene +- Finalizers use `shared.KymaFinalizer` (or the type-appropriate constant from `api/shared/`). +- After removing a finalizer, `r.Update(ctx, obj)` must be called (not `r.Status().Update`). +- Finalizer removal must happen after all cleanup is confirmed complete — never before. + +### 7. SKR context lifecycle +- `SkrContextFactory.InvalidateCache(kyma.GetNamespacedName())` must be called before returning on any `apierrors.IsUnauthorized` or connection-related error. +- `SkrContextFactory.Init(ctx, ...)` must be called before `SkrContextFactory.Get(...)` in the reconcile path. + +### 8. Concurrent operations pattern +- Fan-out work in `handleProcessingState` uses `errgroup.Group`. New parallel operations must be added as `errGroup.Go(func() error { ... })`. +- Do not add blocking sequential calls inside the errgroup section. + +### 9. Test coverage +- Integration tests for new controller behaviour go in `tests/integration/controller//`. +- Suite wiring must use the same composer functions as production (`cmd/composition/`), not custom stubs. +- `DualClusterFactory` from `tests/integration/commontestutils/skrcontextimpl/` must be used for anything that touches the SKR. + +## Output format + +``` +## Operator Review + +### Violations +- [CRITICAL] : — +- [WARNING] : — + +### Cleared +- Spec mutation: ✓ +- Status conditions: ✓ +- ... + +### Verdict +PASS / FAIL / NEEDS DISCUSSION +``` + +If there are no violations in a section, mark it ✓ in "Cleared". A FAIL verdict requires at least one CRITICAL. A NEEDS DISCUSSION verdict means no hard rule is broken but there is a pattern concern worth raising before merge. diff --git a/.github/workflows/check-generated-code.yml b/.github/workflows/check-generated-code.yml new file mode 100644 index 0000000000..24cbed1c23 --- /dev/null +++ b/.github/workflows/check-generated-code.yml @@ -0,0 +1,40 @@ +name: Check generated code + +permissions: { } + +on: + pull_request: + types: [ opened, synchronize, reopened, ready_for_review ] + paths: + - 'api/**' + - 'internal/**' + - 'config/crd/**' + - 'config/rbac/**' + +jobs: + check-generated: + name: "Verify generated files are up-to-date" + runs-on: ubuntu-latest + defaults: + run: + working-directory: lifecycle-manager + steps: + - name: Checkout lifecycle-manager + uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 + - name: Set up Go + uses: actions/setup-go@7a3fe6cf4cb3a834922a1244abfce67bcef6a0c5 # v6.2.0 + with: + go-version-file: 'lifecycle-manager/go.mod' + - name: Install controller-gen + run: make controller-gen + - name: Run make generate + run: make generate + - name: Run make manifests + run: make manifests + - name: Check for diff + run: | + if ! git diff --exit-code; then + echo "" + echo "Generated files are out of sync. Run 'make generate && make manifests' locally and commit the result." + exit 1 + fi diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000000..a352e8d269 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,92 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +## Module & language + +- Main module: `github.com/kyma-project/lifecycle-manager` (Go 1.26.1, per `versions.yaml`) +- The API types live in a **separate Go module**: `github.com/kyma-project/lifecycle-manager/api` — run `go` commands targeting API code from within `api/`, not from the repo root. +- Additional sub-modules: `maintenancewindows/`, `skr-webhook/` +- Key dependencies: `controller-runtime v0.23.3`, `k8s.io/* v0.35.4`, `cert-manager v1.20.2`, `istio v1.29.2`, `ocm.software/ocm v0.40.0`, `runtime-watcher/listener v1.4.0` +- Tool versions are pinned in `versions.yaml`; `make ` downloads and caches them to `bin/`. + +## Common make targets + +Run from `lifecycle-manager/`: + +| Target | What it does | +|---|---| +| `make generate` | Regenerate `zz_generated.deepcopy.go` via controller-gen | +| `make manifests` | Regenerate CRD YAML (`config/crd/bases/`) and RBAC (`config/rbac/`) via controller-gen | +| `make test` | Unit tests + envtest integration tests (also runs generate/manifests/fmt/vet) | +| `make unittest-klm` | Unit tests only for the main module (no envtest) | +| `make unittest-api` | Unit tests for the `api/` sub-module | +| `make build` | Compile `bin/manager` | +| `make lint` | golangci-lint across main module, `api/`, and `maintenancewindows/` | +| `make fmt` | `go fmt ./...` | + +**After any change to a type in `api/`**: run both `make generate` and `make manifests`. +A CI workflow (`check-generated-code.yml`) blocks PRs where generated files are out of sync. + +### Running a single test + +Unit test: +```sh +go test -run TestFoo ./internal/... +``` + +Single controller's integration tests (requires envtest assets): +```sh +KUBEBUILDER_ASSETS=$(./bin/setup-envtest use 1.32.0 -p path) \ + go test ./tests/integration/controller/kyma/... -v -ginkgo.focus "some spec description" +``` + +Replace `kyma` with `manifest`, `watcher`, `modulereleasemeta`, or `moduletemplate`. Run `make envtest` once after checkout to populate `bin/setup-envtest`. + +## Architecture overview + +See [`agent_docs/architecture.md`](agent_docs/architecture.md) for the full component map. + +- lifecycle-manager runs on **KCP** (Kyma Control Plane) and manages a fleet of **SKR** clusters (Satellite Kyma Runtimes). +- The `Kyma` CR on KCP is the source of truth for *which modules to install*; its `.spec` is **overwritten** from the remote SKR copy on every reconcile (`remote.ReplaceSpec`). Never depend on the KCP-side spec persisting across reconcile calls. +- Controllers: `kyma`, `manifest`, `watcher`, `mandatorymodule` (install + delete), `purge`, `istiogatewaysecret`. + +### Module installation flow + +`Kyma.spec.modules` → `ModuleReleaseMeta` (channel → version mapping) → `ModuleTemplate` (OCI descriptor) → `Manifest` CR (created on KCP with OwnerReference to `Kyma`) → manifest controller applies workloads to SKR. + +If no `ModuleReleaseMeta` exists for a module, the controller falls back to listing all `ModuleTemplate` CRs and filtering by `spec.channel`. Missing channel entries put the Kyma CR into `Error` state. + +### Mandatory modules + +Fetched via the `operator.kyma-project.io/mandatory-module` label on `ModuleTemplate`. No channel concept; highest version wins when multiple exist. Mandatory module status does not appear in `Kyma.status`. Deletion is handled by a separate controller that adds a finalizer to the `ModuleTemplate` and waits for all associated `Manifest` CRs to be gone before releasing it. + +### Purge controller + +Forcefully removes all module resources from a remote cluster when a `Kyma` CR has been stuck in deletion longer than the grace period (default: 5 minutes). Removes finalizers from all remote CRs so garbage collection can proceed. + +## Architectural guardrails + +1. **Reconcilers must not mutate `.spec`** of the object they own. The only exceptions are `EnsureLabelsAndFinalizers` (labels/finalizers) and `replaceSpecFromRemote` (Kyma spec). Status is written freely. + +2. **Communicate state via conditions, not free-form strings.** Use `kyma.UpdateCondition(type, status)` with the shared condition types in `api/v1beta2` (`ConditionTypeModules`, `ConditionTypeModuleCatalog`, `ConditionTypeSKRWebhook`, etc.). + +3. **controller-gen markers are the source of truth for CRD schema.** Never hand-edit files in `config/crd/bases/`. See [`agent_docs/crd-conventions.md`](agent_docs/crd-conventions.md). + +4. **All services are injected via interfaces.** The `Reconciler` struct holds interface fields (`SkrContextFactory`, `SKRWebhookManager`, `DeletionService`, etc.). Add new dependencies as interfaces; never call concrete types directly from the reconcile loop. + +5. **Requeueing uses `queue.DetermineRequeueInterval`**, not hardcoded durations. Short explicit intervals (e.g. `1 * time.Second`) are only used during deletion transitions. + +6. **Error wrapping**: always use `fmt.Errorf("context: %w", err)`. For deletion use cases, return `result.Result{UseCase: usecase.X, Err: err}` and let the caller decide on requeue/metrics. + +## Where to look for more context + +| Topic | File | +|---|---| +| Component map, KCP/SKR split, DI wiring | [`agent_docs/architecture.md`](agent_docs/architecture.md) | +| Reconciler patterns, state machine, SKR context | [`agent_docs/reconcilers.md`](agent_docs/reconcilers.md) | +| CRD naming, versioning, markers | [`agent_docs/crd-conventions.md`](agent_docs/crd-conventions.md) | +| Running tests, envtest setup, Ginkgo conventions | [`agent_docs/testing.md`](agent_docs/testing.md) | +| Code generation, when to run it, troubleshooting drift | [`agent_docs/codegen.md`](agent_docs/codegen.md) | +| Controller responsibilities in depth | [`docs/contributor/02-controllers.md`](docs/contributor/02-controllers.md) | +| KCP↔SKR synchronization protocol | [`docs/contributor/08-kcp-skr-synchronization.md`](docs/contributor/08-kcp-skr-synchronization.md) | diff --git a/agent_docs/architecture.md b/agent_docs/architecture.md new file mode 100644 index 0000000000..98c6681314 --- /dev/null +++ b/agent_docs/architecture.md @@ -0,0 +1,80 @@ +# Architecture + +## Component map + +``` +kyma-operator-manager/ (monorepo) +├── lifecycle-manager/ ← this operator (runs on KCP) +│ ├── api/ ← separate Go module; CRD types only +│ ├── internal/ ← controllers, services, repositories +│ ├── pkg/ ← reusable packages (queue, status, templatelookup, watcher, …) +│ ├── cmd/ ← main.go + dependency-injection composition +│ ├── config/ ← kustomize bases: crd/, rbac/, webhook/, manager/, overlays/ +│ ├── tests/integration/ ← per-controller envtest suites +│ ├── maintenancewindows/ ← separate Go module +│ └── skr-webhook/ ← separate Go module +│ +├── template-operator/ ← reference module operator (used in tests) +├── modulectl/ ← CLI for scaffolding/publishing modules +└── runtime-watcher/listener/ ← library consumed by lifecycle-manager +``` + +## KCP / SKR split + +lifecycle-manager operates across two cluster roles: + +| Term | Meaning | +|---|---| +| **KCP** | Kyma Control Plane — the cluster where lifecycle-manager runs | +| **SKR** | Satellite Kyma Runtime — a customer-managed cluster | + +Each SKR is represented on KCP by a `Kyma` CR. The `Kyma` spec is the desired state; it is +populated from the SKR-side `Kyma` copy on every reconcile (`replaceSpecFromRemote`). Lifecycle- +manager installs/removes modules on the SKR by creating `Manifest` CRs on KCP; the manifest +controller then applies the actual workloads to the SKR. + +Connectivity to SKRs goes through `SkrContextFactory` (`internal/remote/`). It caches REST +clients per Kyma name and invalidates them on auth errors. + +## Controllers + +| Controller | Package | Owns | Watches | +|---|---|---|---| +| `kyma` | `internal/controller/kyma` | `Kyma` | `Kyma`, `ModuleTemplate`, `ModuleReleaseMeta` | +| `manifest` | `internal/controller/manifest` | `Manifest` | `Manifest` | +| `watcher` | `internal/controller/watcher` | `Watcher` | `Watcher` | +| `mandatorymodule/installation` | `internal/controller/mandatorymodule` | `Manifest` | `Kyma` | +| `mandatorymodule/deletion` | `internal/controller/mandatorymodule` | `Manifest` | `Kyma` | +| `purge` | `internal/controller/purge` | `Kyma` | `Kyma` | +| `istiogatewaysecret` | `internal/controller/istiogatewaysecret` | Secret | Secret | + +## Service layer + +Business logic lives in `internal/service/`, not in controllers. Controllers call services; +services do not call other services directly. + +Key services and their jobs: + +| Service | Location | Job | +|---|---|---| +| `SkrContextFactory` | `internal/remote/` | Provide authenticated SKR clients | +| `RemoteCatalog` | `internal/remote/` | Sync `ModuleTemplate` CRs to SKR | +| `SKRWebhookManager` | `internal/service/watcher/` | Install/remove runtime-watcher webhook on SKR | +| `SkrSyncService` | `internal/service/skrsync/` | Sync CRDs and image-pull secrets to SKR | +| `DeletionService` | `internal/controller/kyma/deletion/` | Orchestrate Kyma deletion steps | +| `RestrictedModules` | `internal/service/restrictedmodule/` | Default restricted module entries | + +## Watcher integration + +The `runtime-watcher` component runs on each SKR and pushes change events back to KCP via +a webhook. lifecycle-manager installs this webhook through `SKRWebhookManager`. The `Watcher` +CRD (`api/v1beta2`) configures which resources on the SKR are watched and where events are +forwarded. The watcher listener library (`github.com/kyma-project/runtime-watcher/listener`) is +consumed directly in `internal/controller/watcher/`. + +## Dependency injection + +`cmd/composition/` holds pure-function composers that wire up each controller from its +dependencies. No `init()` side effects. The composition functions (`ComposeKymaDeletionService`, +`ComposeSkrWebhookManager`, etc.) are also called from integration test suites to get +production-equivalent wiring under envtest. diff --git a/agent_docs/codegen.md b/agent_docs/codegen.md new file mode 100644 index 0000000000..11e28e71fe --- /dev/null +++ b/agent_docs/codegen.md @@ -0,0 +1,79 @@ +# Code generation + +## Two separate codegen steps + +| Command | What it generates | When to run | +|---|---|---| +| `make generate` | `zz_generated.deepcopy.go` in each `api/` version package | Any change to a type struct in `api/v1beta1/` or `api/v1beta2/` | +| `make manifests` | CRD YAML in `config/crd/bases/`, RBAC in `config/rbac/common/` | Any change to a kubebuilder marker or any type in `api/` | + +**After touching any type in `api/`**, run both: +```sh +cd lifecycle-manager +make generate +make manifests +``` + +The CI workflow `check-generated-code.yml` runs `make generate && make manifests && git diff +--exit-code` on every PR and will block merge if generated files are out of sync. + +## What controller-gen reads + +`make generate` invocation: +``` +$(CONTROLLER_GEN) object:headerFile="hack/boilerplate.go.txt" paths="./..." +``` + +`make manifests` invocation: +``` +$(CONTROLLER_GEN) rbac:roleName=controller-manager crd webhook \ + paths="./..." \ + output:crd:artifacts:config=config/crd/bases \ + output:rbac:dir=config/rbac/common +``` + +controller-gen scans all Go files under `./...` for `// +kubebuilder:*` comments and Go type +definitions. The `api/` directory is a **separate Go module**, so controller-gen is run from the +main module root, which has `api/` as a replace directive in `go.mod` — this means it traverses +into `api/`. + +## Generated file locations + +``` +api/v1beta1/zz_generated.deepcopy.go ← make generate +api/v1beta2/zz_generated.deepcopy.go ← make generate +api/shared/zz_generated.deepcopy.go ← make generate + +config/crd/bases/ + operator.kyma-project.io_kymas.yaml + operator.kyma-project.io_manifests.yaml + operator.kyma-project.io_moduletemplates.yaml + operator.kyma-project.io_modulereleasemetas.yaml + operator.kyma-project.io_watchers.yaml + +config/rbac/common/ + role.yaml ← make manifests (RBAC markers) +``` + +None of these files should be edited by hand. + +## Tool version + +controller-gen version is pinned in `versions.yaml`: +```yaml +controllerTools: "0.18.0" +``` + +`make controller-gen` downloads and caches it to `bin/controller-gen`. Run this once after +a fresh checkout or when `versions.yaml` changes. + +## Troubleshooting drift + +If the CI check fails with a diff in a generated file: +1. Run `make generate && make manifests` locally. +2. `git diff` to confirm the generated change is as expected. +3. Commit the generated files alongside the type change. + +If the diff is unexpected (e.g. a formatting-only change), check whether `controller-gen` was +upgraded in `versions.yaml` and whether `make controller-gen` needs to be re-run to pick up the +new binary. diff --git a/agent_docs/crd-conventions.md b/agent_docs/crd-conventions.md new file mode 100644 index 0000000000..35ae5b3414 --- /dev/null +++ b/agent_docs/crd-conventions.md @@ -0,0 +1,91 @@ +# CRD conventions + +## API group + +All lifecycle-manager CRDs live under `operator.kyma-project.io`. + +The constant for the group is `shared.OperatorGroup = "operator.kyma-project.io"` in +`api/shared/`. + +## Versions + +| Version | Status | +|---|---| +| `v1beta1` | Deprecated; kept for conversion | +| `v1beta2` | Current storage version (marked `+kubebuilder:storageversion`) | + +When adding a field, add it to `v1beta2`. Do not add new fields to `v1beta1`. Conversion webhooks +handle clients that still send v1beta1 objects. + +## Required markers on every CRD type + +```go +// +kubebuilder:object:root=true +// +kubebuilder:subresource:status +// +kubebuilder:storageversion // on the current storage version only +// +kubebuilder:printcolumn:name="State",type=string,JSONPath=".status.state" +// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp" +``` + +`+kubebuilder:subresource:status` is mandatory for any type that has a `.status` field — +without it, `r.Status().Update()` writes to the main object instead of the status subresource +and will overwrite spec on conflict. + +## Field validation markers + +Use `+kubebuilder:validation:*` markers directly above the field, not on the type: + +```go +// +kubebuilder:validation:Pattern:=^[a-z]+$ +// +kubebuilder:validation:MaxLength:=32 +// +kubebuilder:validation:MinLength:=3 +Channel string `json:"channel"` +``` + +For optional fields with a default: + +```go +// +kubebuilder:default:=CreateAndDelete +CustomResourcePolicy `json:"customResourcePolicy,omitempty"` +``` + +## List types + +For fields that are lists keyed by a sub-field, declare the list type and key to enable +strategic merge patch: + +```go +// +listType=map +// +listMapKey=name +Modules []Module `json:"modules,omitempty"` +``` + +## Pruning unknown fields + +To preserve an opaque/dynamic field that controller-gen would otherwise strip: + +```go +// +kubebuilder:pruning:PreserveUnknownFields +Source machineryruntime.RawExtension `json:"source"` +``` + +## Generated files — do not hand-edit + +| Generated file | Source | +|---|---| +| `config/crd/bases/*.yaml` | `make manifests` | +| `config/rbac/common/*.yaml` | `make manifests` | +| `api/v1beta2/zz_generated.deepcopy.go` | `make generate` | +| `api/v1beta1/zz_generated.deepcopy.go` | `make generate` | + +Editing these files by hand will be overwritten on the next `make manifests` or +`make generate` run, and the CI check will catch any drift on PRs. + +## Naming conventions + +- Kind: PascalCase singular (`Kyma`, `Manifest`, `ModuleTemplate`) +- Resource (plural): lowercase (`kymas`, `manifests`, `moduletemplates`) +- controller-gen derives the plural automatically; override only if needed with + `// +kubebuilder:resource:plural=customname` +- Shared constants (labels, annotations, finalizer names) live in `api/shared/` and are used + by both the operator and external consumers of the API module. diff --git a/agent_docs/reconcilers.md b/agent_docs/reconcilers.md new file mode 100644 index 0000000000..4578ba7e40 --- /dev/null +++ b/agent_docs/reconcilers.md @@ -0,0 +1,117 @@ +# Reconciler patterns + +## Reconciler struct + +Every controller follows the same shape: + +```go +type Reconciler struct { + client.Client // embedded — use r.Get, r.List, r.Update, r.Delete directly + event.Event // emit Kubernetes events via r.Warning / r.Normal + queue.RequeueIntervals // Success, Busy, Error, Warning durations from flags + + // dependencies as interfaces — never concrete types + SkrContextFactory remote.SkrContextProvider + DeletionService DeletionService + SKRWebhookManager SKRWebhookManager + // ... +} +``` + +Concrete dependencies are wired in `cmd/composition/` using pure composer functions. If you add +a new service dependency, add it as an interface field here and add a composer call there. + +## Reconcile entry point + +```go +func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) +``` + +The pattern is always: +1. Fetch the object (`r.Get`). Return `ctrl.Result{}, nil` on not-found. +2. Initialise conditions (`status.InitConditions`). +3. Check skip/deletion annotations. +4. Do work; update status via `r.updateStatus` or `r.updateStatusWithError`. +5. Return `ctrl.Result{RequeueAfter: interval}, err`. + +## Kyma state machine + +``` +"" ──► Processing ──► Ready + ├──► Warning + ├──► Error (returns ErrKymaInErrorState so rate limiter applies) + └──► Deleting ──► (finalizer removed, no requeue) +``` + +`processKymaState` switches on `kyma.Status.State`. Both `Error` and `Warning` fall through to +`handleProcessingState` — errors don't stop reconciliation; they just set a condition. + +## Spec is read from SKR + +The Kyma `spec` on KCP is **overwritten** at the start of every reconcile from the SKR-side copy: + +```go +remote.ReplaceSpec(controlPlaneKyma, remoteKyma) +``` + +Do not write business logic that depends on the KCP spec persisting across calls without this +override. The remote Kyma is the user-facing API surface. + +## Status updates + +Always update status through the helpers — never call `r.Status().Update()` directly: + +```go +r.updateStatus(ctx, kyma, shared.StateReady, "kyma is ready") +r.updateStatusWithError(ctx, kyma, err) +``` + +These call `status.Helper(r).UpdateStatusForExistingModules(...)`, which also fires a Kubernetes +event. Condition updates are done in place on the object (`kyma.UpdateCondition(type, status)`) +before calling the helper. + +## Finalizers + +The kyma finalizer constant is `shared.KymaFinalizer`. Add/check it with: + +```go +controllerutil.ContainsFinalizer(obj, shared.KymaFinalizer) +controllerutil.AddFinalizer(obj, shared.KymaFinalizer) +controllerutil.RemoveFinalizer(obj, shared.KymaFinalizer) +``` + +Finalizers are added in `kyma.EnsureLabelsAndFinalizers()` and removed at the end of +`handleDeletingState`. After removing a finalizer, always call `r.Update(ctx, obj)` (not +`r.Status().Update`). + +## Requeueing + +Use `queue.DetermineRequeueInterval(state, r.RequeueIntervals)` for normal requeue intervals. +Short explicit intervals (e.g. `1 * time.Second`) are only used during deletion transitions +where the next step is expected to be fast. + +Returning a non-nil error triggers the controller-runtime rate limiter. Only return an error when +the condition is transient and should be rate-limited. For permanent/expected states (e.g. module +not yet ready), return `ctrl.Result{RequeueAfter: interval}, nil`. + +## SKR context lifecycle + +```go +r.SkrContextFactory.Init(ctx, kyma.GetNamespacedName()) // fetch/refresh credentials +skrCtx, err := r.SkrContextFactory.Get(kyma.GetNamespacedName()) +// on auth errors: +r.SkrContextFactory.InvalidateCache(kyma.GetNamespacedName()) +``` + +Always invalidate the cache on `apierrors.IsUnauthorized` or connection-related errors before +returning. The next reconcile will re-establish the client. + +## Parallel operations in handleProcessingState + +`handleProcessingState` uses `errgroup.Group` to fan out: +- `reconcileManifests` (module state) +- `RemoteCatalog.SyncModuleCatalog` (module catalog on SKR) +- `SKRWebhookManager.Reconcile` (watcher webhook on SKR) + +These run concurrently. If any returns an error, `errGroup.Wait()` returns it and the reconcile +sets state to Error. Add new concurrent operations as new `errGroup.Go(func() error { ... })`. diff --git a/agent_docs/testing.md b/agent_docs/testing.md new file mode 100644 index 0000000000..64485248d8 --- /dev/null +++ b/agent_docs/testing.md @@ -0,0 +1,99 @@ +# Testing + +## Test types + +| Type | Location | Runner | +|---|---|---| +| Unit tests | `internal/`, `pkg/`, `api/` | `go test` (no envtest) | +| Integration tests | `tests/integration/controller//` | envtest + Ginkgo v2 | +| E2E tests | `.github/workflows/test-e2e.yml` | real clusters (CI only) | + +## Running tests + +**All tests (unit + integration):** +```sh +cd lifecycle-manager +make test +``` + +This also runs `make generate`, `make manifests`, `make fmt`, `make vet` first — the full +pre-flight. Use it before opening a PR. + +**Unit tests only (fast, no envtest):** +```sh +make unittest-klm # main module +make unittest-api # api/ sub-module +make unittest-maintenancewindows +``` + +**A single controller's integration tests:** +```sh +cd lifecycle-manager +KUBEBUILDER_ASSETS=$(./bin/setup-envtest use 1.32.0 -p path) \ + go test ./tests/integration/controller/kyma/... -v +``` + +Replace `kyma` with `manifest`, `watcher`, `modulereleasemeta`, or `moduletemplate` for other +controllers. + +Add `-ginkgo.focus "some test description"` to run a specific `It` block. + +## envtest setup + +Each controller suite has a `suite_test.go` that bootstraps envtest. The pattern is: + +```go +kcpEnv = &envtest.Environment{ + CRDDirectoryPaths: []string{ + filepath.Join(integration.GetProjectRoot(), "config", "crd", "bases"), + }, + CRDs: externalCRDs, // cert-manager, istio loaded from config/samples/tests/crds/ +} +restCfg, err = kcpEnv.Start() +``` + +`integration.GetProjectRoot()` walks up from the test binary until it finds `go.mod`, so tests +can be run from any working directory. + +External CRD snapshots live in `config/samples/tests/crds/`: +- `cert-manager-v1.10.1.crds.yaml` +- `istio-v1.17.1.crds.yaml` + +Do not update these snapshots without updating the corresponding import in the suite. + +## Wiring controllers in tests + +Integration tests wire up the full controller using the same composer functions as production: + +```go +err = (&kyma.Reconciler{ + Client: kcpClient, + SkrContextFactory: testSkrContextFactory, // DualClusterFactory — starts a second envtest for SKR + // ... +}).SetupWithManager(mgr, ...) +``` + +`DualClusterFactory` (`tests/integration/commontestutils/skrcontextimpl/`) spins up a second +envtest environment to simulate the SKR cluster. It replaces the real remote client; no actual +remote cluster is needed. + +## Ginkgo conventions + +- Each suite file: `package _test` (external test package). +- Suite bootstrap: `TestAPIs(t *testing.T)` calls `RunSpecs`. +- Short requeue intervals in tests: `Success: 1s, Busy/Error/Warning: 100ms` to keep tests fast. +- `Eventually(func, Timeout, Interval)` from `pkg/testutils` for async assertions. +- `Timeout` and `Interval` constants are defined in `pkg/testutils/` and imported via + `. "github.com/kyma-project/lifecycle-manager/pkg/testutils"`. + +## Tool versions + +envtest K8s version is read from `versions.yaml`: + +```yaml +envtest_k8s: "1.32.0" +envtest: "0.21" +``` + +`make envtest` downloads `setup-envtest` at the version in `versions.yaml`. The binary is cached +in `bin/`. You only need to run this once after a fresh checkout. From 71442e30a876b0fa997086aebb476e9aede9dd29 Mon Sep 17 00:00:00 2001 From: medmes Date: Wed, 6 May 2026 17:21:17 +0200 Subject: [PATCH 2/9] Add code conventions and docs guidelines from PR #3139 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - CLAUDE.md: GOFIPS140=v1.0.0 requirement for direct go commands, import alias table (apimetav1, apierrors, machineryruntime, ctrl, ctrlruntime, etc.), import ordering (standard→third-party→project), lint limits (120-char lines, 80-line funlen, cyclop 20, nolint requires explanation) - docs/CLAUDE.md: SAP/Kyma documentation writing style, templates, and formatting guidelines (sourced from PR #3139 by @lindnerby) --- CLAUDE.md | 39 ++++++++++++++++++++++++++++--- docs/CLAUDE.md | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 3 deletions(-) create mode 100644 docs/CLAUDE.md diff --git a/CLAUDE.md b/CLAUDE.md index a352e8d269..e96fff0c06 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -12,7 +12,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co ## Common make targets -Run from `lifecycle-manager/`: +Run from `lifecycle-manager/`. All `go` commands use `GOFIPS140=v1.0.0 go` (FIPS-enabled builds) — the Makefile sets this automatically, but use it explicitly when running `go` commands directly. | Target | What it does | |---|---| @@ -32,13 +32,13 @@ A CI workflow (`check-generated-code.yml`) blocks PRs where generated files are Unit test: ```sh -go test -run TestFoo ./internal/... +GOFIPS140=v1.0.0 go test -run TestFoo ./internal/... ``` Single controller's integration tests (requires envtest assets): ```sh KUBEBUILDER_ASSETS=$(./bin/setup-envtest use 1.32.0 -p path) \ - go test ./tests/integration/controller/kyma/... -v -ginkgo.focus "some spec description" + GOFIPS140=v1.0.0 go test ./tests/integration/controller/kyma/... -v -ginkgo.focus "some spec description" ``` Replace `kyma` with `manifest`, `watcher`, `modulereleasemeta`, or `moduletemplate`. Run `make envtest` once after checkout to populate `bin/setup-envtest`. @@ -79,6 +79,38 @@ Forcefully removes all module resources from a remote cluster when a `Kyma` CR h 6. **Error wrapping**: always use `fmt.Errorf("context: %w", err)`. For deletion use cases, return `result.Result{UseCase: usecase.X, Err: err}` and let the caller decide on requeue/metrics. +## Code conventions + +### Import aliases + +The `importas` linter enforces strict aliases — violations fail CI. Key ones you will use in almost every file: + +| Package | Alias | +|---|---| +| `k8s.io/apimachinery/pkg/apis/meta/v1` | `apimetav1` | +| `k8s.io/apimachinery/pkg/api/errors` | `apierrors` | +| `k8s.io/apimachinery/pkg/runtime` | `machineryruntime` | +| `k8s.io/apimachinery/pkg/labels` | `k8slabels` | +| `k8s.io/api/core/v1` | `apicorev1` | +| `k8s.io/api/apps/v1` | `apiappsv1` | +| `k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1` | `apiextensionsv1` | +| `sigs.k8s.io/controller-runtime` | `ctrl` | +| `sigs.k8s.io/controller-runtime/pkg/controller` | `ctrlruntime` | +| `sigs.k8s.io/controller-runtime/pkg/log` | `logf` | + +Full alias list is in `.golangci.yaml` under `linters-settings.importas.alias`. + +### Import ordering + +Enforced by `gci`: **standard → third-party → project** (`github.com/kyma-project/lifecycle-manager`) **→ blank → dot**. + +### Lint limits + +- Line length: **120 characters** (revive `line-length-limit`) +- Function length: **80 lines** (`funlen`) — use `//nolint:funlen // ` only for composition root wiring +- Cyclomatic complexity: **20** (`cyclop`) +- All linters enabled by default; check `.golangci.yaml` before adding `//nolint`. Every `//nolint` directive **must** include an explanation comment. + ## Where to look for more context | Topic | File | @@ -90,3 +122,4 @@ Forcefully removes all module resources from a remote cluster when a `Kyma` CR h | Code generation, when to run it, troubleshooting drift | [`agent_docs/codegen.md`](agent_docs/codegen.md) | | Controller responsibilities in depth | [`docs/contributor/02-controllers.md`](docs/contributor/02-controllers.md) | | KCP↔SKR synchronization protocol | [`docs/contributor/08-kcp-skr-synchronization.md`](docs/contributor/08-kcp-skr-synchronization.md) | +| Documentation writing style and templates | [`docs/CLAUDE.md`](docs/CLAUDE.md) | diff --git a/docs/CLAUDE.md b/docs/CLAUDE.md new file mode 100644 index 0000000000..5e6bd1c88f --- /dev/null +++ b/docs/CLAUDE.md @@ -0,0 +1,62 @@ +# Documentation Guidelines + +When writing documentation for this project, follow these rules. + +## General rules + +Keep documentation clear, precise, and easy to read. Use active voice and present tense. Avoid future tense. + +When a word has multiple meanings, prefer the unambiguous alternative: + +| Avoid | Use instead | +|---|---| +| "as" / "since" | "because" (causation), "while" (temporal), "like" (comparison) | +| "may not" | "might not" (possibility) or "must not" (prohibition) | +| "should" | "we recommend" (recommendation) or "must" (requirement) | +| "once" (temporal vs. conditional) | "after" or "when" | +| "i.e." / "e.g." | "that means" / "for example" | +| "allows you to" / "enables you to" | "you can" | +| "leverage" / "utilize" | "use" | + +Always state the purpose before the instruction: "To [purpose], [instruction]." +Always state the condition before the conclusion: "If [condition], [instruction]." + +Use full sentences to introduce lists. All list items must follow a consistent pattern — never mix sentences and fragments in the same list. + +## Templates + +Use the document templates from the [kyma-project template repository](https://github.com/kyma-project/template-repository/tree/main/docs/user/assets/templates): + +- **concept.md** — for explaining foundational ideas and principles +- **task.md** — for step-by-step instructions and how-to guides +- **troubleshooting.md** — for diagnostic information and solutions to common issues +- **custom-resource.md** — for documenting custom resource configuration and usage + +## Style and terminology + +Follow the [Kyma style and terminology guidelines](https://github.com/kyma-project/community/blob/main/docs/guidelines/content-guidelines/04-style-and-terminology.md): + +- Use **imperative mood** for instructions — no "please" +- Address readers as "you", not "we" or "let's" +- Use **sentence case** for standard text; **Title Case** for component names and headings +- Use **CamelCase** for Kubernetes resources (`ConfigMap`, `APIRule`) +- Always capitalize "Kubernetes"; never abbreviate it +- Do not capitalize "namespace" +- "must" for mandatory requirements; "can" for optional features +- "using" or "with" instead of "via" +- "connect/connection" instead of "integrate/integration" +- American English spelling +- Avoid parentheses — use lists instead +- Include serial commas + +## Formatting + +Follow the [Kyma formatting guidelines](https://github.com/kyma-project/community/blob/main/docs/guidelines/content-guidelines/03-formatting.md): + +- **Bold** for parameters, HTTP headers, events, roles, UI elements, and variables/placeholders +- `Code font` for code examples, values, endpoints, filenames, paths, repository names, status codes, flags, and custom resources +- **Ordered lists** for sequential procedures; **unordered lists** for non-sequential items +- Action verbs and present tense in headings (for example, "Expose a Service") +- Tables for comparisons and structured information +- Callout panels: `[!NOTE]` for specific information, `[!WARNING]` for critical alerts, `[!TIP]` for helpful advice +- Break lengthy paragraphs into lists or tables for readability From 060a2301f6c54599c81389d0ccc5a8e2f8e21995 Mon Sep 17 00:00:00 2001 From: medmes Date: Fri, 8 May 2026 11:53:50 +0200 Subject: [PATCH 3/9] Add guidance for Claude Code model usage in CLAUDE.md --- CLAUDE.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index e96fff0c06..2982f2a488 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -123,3 +123,12 @@ Enforced by `gci`: **standard → third-party → project** (`github.com/kyma-pr | Controller responsibilities in depth | [`docs/contributor/02-controllers.md`](docs/contributor/02-controllers.md) | | KCP↔SKR synchronization protocol | [`docs/contributor/08-kcp-skr-synchronization.md`](docs/contributor/08-kcp-skr-synchronization.md) | | Documentation writing style and templates | [`docs/CLAUDE.md`](docs/CLAUDE.md) | + +## Model usage + +Follow the Kyma team's Claude Code workflow: + +- **Planning complex tasks** — switch to Opus: `/model claude-opus-4-7` +- **Implementation** — use the default Sonnet: `/model claude-sonnet-4-6` + +Use Opus when you need to understand an unfamiliar subsystem, design a non-trivial change, or reason about cross-cutting impacts. Switch back to Sonnet once the approach is clear and you are writing code. From c6c581b71a928c0bf98873b7ba4b6126e71bc2fa Mon Sep 17 00:00:00 2001 From: medmes Date: Fri, 8 May 2026 15:27:23 +0200 Subject: [PATCH 4/9] docs(claude): add security guardrails and CVE triage context Documents TLS 1.3, FIPS, container security context, NetworkPolicy, RBAC, and secret-handling constraints so AI assistants don't accidentally weaken existing CVE mitigations. Adds .claude/cve-triage/context.md (inspired by kyma-project/telemetry-manager) for scanner triage workflow. --- .claude/cve-triage/context.md | 54 +++++++++++++++++++++++++++++++++++ CLAUDE.md | 45 +++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+) create mode 100644 .claude/cve-triage/context.md diff --git a/.claude/cve-triage/context.md b/.claude/cve-triage/context.md new file mode 100644 index 0000000000..c9c2126723 --- /dev/null +++ b/.claude/cve-triage/context.md @@ -0,0 +1,54 @@ +# CVE Triage Context — lifecycle-manager + +This file provides context for triaging CVEs raised by automated security scanners (BDBA, Mend, Checkmarx One) against lifecycle-manager artifacts. + +## Container images + +### lifecycle-manager (KCP operator) +- **Registry**: `europe-docker.pkg.dev/kyma-project/prod/lifecycle-manager` +- **Scanned tags**: `latest` and the two most recent release tags (see `sec-scanners-config.yaml`) +- **Base image**: `gcr.io/distroless/static:nonroot` — no shell, no libc, no package manager +- **Builder**: Go static binary (`CGO_ENABLED=0`, `GOFIPS140=v1.0.0`) +- **Runtime user**: UID 65532 (nonroot) +- **Role**: KCP-side orchestration — reads Kyma CRs, resolves ModuleTemplates via OCI, creates Manifest CRs, manages SKR webhook deployment via mTLS + +### skr-webhook (runtime-watcher binary, deployed to SKR clusters) +- **Source repo**: `github.com/kyma-project/runtime-watcher` — lifecycle-manager deploys it via `SKRWebhookManager` +- **Role**: `ValidatingWebhook` on each SKR; forwards resource change events to KCP over mTLS +- **TLS**: Minimum TLS 1.3 enforced; `NextProtos: ["http/1.1"]` is a **CVE-2023-44487** (HTTP/2 Rapid Reset) mitigation — do not remove +- For CVEs in the skr-webhook image, file issues against `kyma-project/runtime-watcher` + +## Scanner configuration + +Defined in `sec-scanners-config.yaml`: +- **BDBA** — binary/container scan; scans the production image +- **Mend** — Go module SCA; excludes test files +- **Checkmarx One** — SAST; `go-default` preset, excludes test files +- **OSSF Scorecard** — supply-chain security; runs weekly + +## Triage decision tree + +1. **Is the vulnerable package actually present?** + - Distroless base: OS-level CVEs (glibc, bash, apt, openssl) are not applicable — mark as **not applicable** + - For Go module CVEs: run `go list -m -json all | jq 'select(.Path == "")'` to confirm the module is actually in the dependency graph + +2. **Is it reachable from production code?** + - If the vulnerable function is only reachable from `_test.go` files or `tests/` → **not applicable in production** + - Check whether untrusted input reaches the vulnerable code path + +3. **Is there a fixed version?** + - Check `go.mod` for a replace directive that may block the upgrade + - If fixed: open a `deps` PR — `go get @ && go mod tidy` + - Title format: `deps: bump to (CVE-XXXX-XXXXX)` + +4. **No fix available?** + - Assess exploitability in context (network-accessible? requires auth? distroless removes significant attack surface) + - Document the assessment in the CVE tracking issue with justification + - Suppression in scanner config requires Kyma security team approval + +## Key security constraints (do not weaken during fixes) + +- `GOFIPS140=v1.0.0` in `Dockerfile` and `Makefile` — required; removing it is not a valid CVE mitigation +- TLS 1.3 minimum on all endpoints — do not downgrade to accept a scanner's "recommended" TLS 1.2 fallback +- `rotationPolicy: Always` on cert-manager certificates — required for key hygiene +- Container base image pinned to sha256 digest — update the digest, never switch to a tag diff --git a/CLAUDE.md b/CLAUDE.md index 2982f2a488..112e770913 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -124,6 +124,51 @@ Enforced by `gci`: **standard → third-party → project** (`github.com/kyma-pr | KCP↔SKR synchronization protocol | [`docs/contributor/08-kcp-skr-synchronization.md`](docs/contributor/08-kcp-skr-synchronization.md) | | Documentation writing style and templates | [`docs/CLAUDE.md`](docs/CLAUDE.md) | +## Security guardrails + +These constraints exist for specific CVE mitigations or compliance requirements — do not remove or weaken them without understanding what they protect against. + +### FIPS compliance +- **Never remove `GOFIPS140=v1.0.0`** from `Dockerfile` or `Makefile`. Mandatory for SAP/Kyma production builds. The Go FIPS module restricts crypto to FIPS-140-approved algorithms. +- FIPS mode is monitored at runtime via the `lifecycle_mgr_fips_mode` Prometheus metric (`internal/pkg/metrics/fipsMode.go`). A value of `0` means FIPS is off — that is an incident. +- Do not add Go dependencies that use non-FIPS-approved crypto (custom cipher suites, `golang.org/x/crypto` elliptic curves that bypass the stdlib FIPS module). + +### TLS enforcement +- **`config/watcher/gateway.yaml`** enforces TLS 1.3 exclusively (`minProtocolVersion: TLSV1_3`, `maxProtocolVersion: TLSV1_3`) with `mode: MUTUAL`. Do not downgrade to TLS 1.2. +- **`forwardClientCertDetails: SANITIZE_SET`** on the Istio Gateway prevents client cert header spoofing — keep it. +- Certificates use 4096-bit RSA with `rotationPolicy: Always` (key re-generated on every renewal). Do not reduce key size or remove the rotation policy. See `config/certmanager/certificate_watcher.yaml`. + +### Container security context +Every container (including sidecars and init containers) must include: +```yaml +securityContext: + allowPrivilegeEscalation: false + readOnlyRootFilesystem: true + runAsNonRoot: true + capabilities: + drop: ["ALL"] + seccompProfile: + type: RuntimeDefault +``` +Reference: `skr-webhook/resources.yaml` and `config/manager/manager.yaml`. + +### Container base image +`Dockerfile` pins `gcr.io/distroless/static:nonroot` to a **sha256 digest**. Never switch to a tag (`:latest`, `:nonroot`) — only the digest form is acceptable. When updating the base image, update the digest and document the CVE or reason. + +### NetworkPolicies +`skr-webhook/resources.yaml` contains four strict NetworkPolicies. Do not relax egress to `0.0.0.0/0` or remove namespace/pod selectors. All rules are intentional: +- Ingress: Gardener VPN source only + Prometheus scrape port +- Egress: Kubernetes API server (443) + DNS only + +### RBAC +`config/rbac/manager_role.yaml` uses explicit resource and verb lists — no wildcards. When adding a permission: use the minimum verb set, add a separate `rules` entry per API group. Never use `resources: ["*"]` or `verbs: ["*"]`. + +### Secret handling +TLS keys and sensitive credentials must be mounted as Kubernetes Secret volumes — never passed as environment variables. See `config/certmanager/certificate_watcher.yaml` for the cert-manager pattern. + +### CVE triage +Three scanners run against this repo (`sec-scanners-config.yaml`): **Checkmarx One** (SAST), **BDBA** (container CVE scan), **Mend** (Go module SCA). When triaging a CVE finding, see [`.claude/cve-triage/context.md`](.claude/cve-triage/context.md). + ## Model usage Follow the Kyma team's Claude Code workflow: From 05c1bbd494f9901ba7ed6ba660ab299fa4b8115c Mon Sep 17 00:00:00 2001 From: medmes Date: Fri, 8 May 2026 15:30:42 +0200 Subject: [PATCH 5/9] docs(claude): expand CVE triage to cover Go module and SAST surfaces Adds dedicated triage logic for Mend SCA (Go module deps) and Checkmarx SAST findings, not just BDBA container image CVEs. Key additions: FIPS constraint on crypto fixes, go mod why workflow for transitive deps, Checkmarx false-positive patterns specific to Kubernetes operators. --- .claude/cve-triage/context.md | 152 +++++++++++++++++++++++++++------- 1 file changed, 121 insertions(+), 31 deletions(-) diff --git a/.claude/cve-triage/context.md b/.claude/cve-triage/context.md index c9c2126723..bc23185f75 100644 --- a/.claude/cve-triage/context.md +++ b/.claude/cve-triage/context.md @@ -1,54 +1,144 @@ # CVE Triage Context — lifecycle-manager -This file provides context for triaging CVEs raised by automated security scanners (BDBA, Mend, Checkmarx One) against lifecycle-manager artifacts. +This file provides context for triaging CVEs raised by automated security scanners against lifecycle-manager. There are **two distinct CVE surfaces** — container/OS level and Go code level — each with different triage logic. -## Container images +--- -### lifecycle-manager (KCP operator) +## Surface 1: Container images (BDBA findings) + +BDBA scans the built container image for OS-level and binary CVEs. + +### Images tracked + +#### lifecycle-manager (KCP operator) - **Registry**: `europe-docker.pkg.dev/kyma-project/prod/lifecycle-manager` - **Scanned tags**: `latest` and the two most recent release tags (see `sec-scanners-config.yaml`) -- **Base image**: `gcr.io/distroless/static:nonroot` — no shell, no libc, no package manager +- **Base image**: `gcr.io/distroless/static:nonroot` — no shell, no libc, no package manager, no APT - **Builder**: Go static binary (`CGO_ENABLED=0`, `GOFIPS140=v1.0.0`) - **Runtime user**: UID 65532 (nonroot) -- **Role**: KCP-side orchestration — reads Kyma CRs, resolves ModuleTemplates via OCI, creates Manifest CRs, manages SKR webhook deployment via mTLS -### skr-webhook (runtime-watcher binary, deployed to SKR clusters) +#### skr-webhook (runtime-watcher binary, deployed to SKR clusters) - **Source repo**: `github.com/kyma-project/runtime-watcher` — lifecycle-manager deploys it via `SKRWebhookManager` - **Role**: `ValidatingWebhook` on each SKR; forwards resource change events to KCP over mTLS -- **TLS**: Minimum TLS 1.3 enforced; `NextProtos: ["http/1.1"]` is a **CVE-2023-44487** (HTTP/2 Rapid Reset) mitigation — do not remove +- **TLS note**: `NextProtos: ["http/1.1"]` is a **CVE-2023-44487** (HTTP/2 Rapid Reset) mitigation — do not remove - For CVEs in the skr-webhook image, file issues against `kyma-project/runtime-watcher` -## Scanner configuration +### Image CVE triage logic -Defined in `sec-scanners-config.yaml`: -- **BDBA** — binary/container scan; scans the production image -- **Mend** — Go module SCA; excludes test files -- **Checkmarx One** — SAST; `go-default` preset, excludes test files -- **OSSF Scorecard** — supply-chain security; runs weekly +1. **Is the vulnerable package present in the image?** + - `gcr.io/distroless/static` contains: `ca-certificates`, `tzdata`, `glibc` stubs only — no shell, no apt, no openssl, no libc runtime beyond what Go's static binary needs + - CVEs in bash, curl, wget, openssl, apt, python, etc. → **not applicable** (not in the image) + - When unsure: `docker run --rm --entrypoint=sh europe-docker.pkg.dev/.../lifecycle-manager:latest` will fail — there is no shell + +2. **Is it a glibc CVE?** + - `CGO_ENABLED=0` means the Go binary does not link against glibc — glibc CVEs → **not applicable** + +3. **No fix available for a present package?** + - Assess whether the vulnerable code path is reachable (network-accessible? requires auth? no shell = no local privilege escalation) + - Document assessment in the CVE tracking issue; suppression requires Kyma security team approval + +--- + +## Surface 2: Go module dependencies (Mend SCA findings) + +Mend scans `go.mod` and the full module graph for known vulnerable versions. This is where most actionable findings come from. + +### Key Go module facts + +- Main module: `github.com/kyma-project/lifecycle-manager` (root `go.mod`) +- API sub-module: `api/go.mod` — has its own dependency graph; Mend scans both +- Test files excluded from Mend scan (`*_test.go`, `tests/`, `testutils/`) + +### Go module CVE triage logic + +1. **Is the module actually in the dependency graph?** + ```sh + go list -m -json all | jq 'select(.Path == "")' + # Run from the affected module root (root or api/) + ``` + If the module is not listed → **not applicable** (Mend may flag transitive deps from indirect paths not actually used). + +2. **Is it a direct or transitive dependency?** + ```sh + go mod why + # Shows the import chain: lifecycle-manager → ... → vulnerable-module + ``` + Transitive deps are harder to upgrade directly — check if the parent dependency has released a fix first. -## Triage decision tree +3. **Is the vulnerable code path reachable from production code?** + - Check `go mod why` import chain — if it only passes through test utilities → **not applicable in production** + - For network-parsing CVEs: check whether lifecycle-manager calls the vulnerable function with untrusted input -1. **Is the vulnerable package actually present?** - - Distroless base: OS-level CVEs (glibc, bash, apt, openssl) are not applicable — mark as **not applicable** - - For Go module CVEs: run `go list -m -json all | jq 'select(.Path == "")'` to confirm the module is actually in the dependency graph +4. **Is there a fixed version?** + - Check `go.mod` for a `replace` directive that may block the upgrade + - If fixed: + ```sh + GOFIPS140=v1.0.0 go get @ + go mod tidy + make test # verify nothing breaks + ``` + - Open a `deps` PR: title format `deps: bump to (CVE-XXXX-XXXXX)` -2. **Is it reachable from production code?** - - If the vulnerable function is only reachable from `_test.go` files or `tests/` → **not applicable in production** - - Check whether untrusted input reaches the vulnerable code path +5. **FIPS constraint on crypto fixes** — critical: + - If the CVE is in a crypto package, the replacement **must be FIPS-140-approved** + - Do not substitute stdlib `crypto/*` with `golang.org/x/crypto` custom implementations — many are not FIPS-approved + - Safe: upgrading to a newer version of the same `crypto/tls`, `crypto/sha256`, etc. stdlib package + - Unsafe: replacing `crypto/rand` with a third-party PRNG, adding non-stdlib elliptic curve implementations + - When unsure: check the [Go FIPS module documentation](https://pkg.go.dev/crypto/internal/fips140) and consult the Kyma security team -3. **Is there a fixed version?** - - Check `go.mod` for a replace directive that may block the upgrade - - If fixed: open a `deps` PR — `go get @ && go mod tidy` - - Title format: `deps: bump to (CVE-XXXX-XXXXX)` +6. **No fix available?** + - Document: is it exploitable given lifecycle-manager's deployment context? (runs in KCP, not exposed to untrusted internet traffic directly) + - Add suppression to `sec-scanners-config.yaml` with justification — requires Kyma security team approval + +--- + +## Surface 3: Go source code (Checkmarx One SAST findings) + +Checkmarx performs static analysis on Go source looking for code-level vulnerabilities: injection, path traversal, insecure deserialization, hardcoded secrets, etc. + +### Checkmarx scope + +- Preset: `go-default` +- Excludes: `**/test/**`, `**/*_test.go`, `**/testutils/**`, `tests/**` +- Scans: all production Go source in the main module + +### SAST CVE triage logic + +1. **Is the finding a true positive?** + - Checkmarx commonly false-positives on: log statements (not injection), label/annotation values (not executed), YAML marshalling (not SQL/shell injection) + - Verify: does the flagged code path accept input from an untrusted external source (HTTP request body, Kubernetes CR fields from untrusted users)? + - lifecycle-manager processes Kubernetes resources — its "input" comes from the Kubernetes API server, which enforces RBAC before any data reaches the operator + +2. **Common Go patterns that trigger false positives here:** + - `fmt.Sprintf` with CR field values → Checkmarx may flag as "string injection" — not exploitable if the result is used as a Kubernetes resource name (API server validates names) + - `os.ReadFile` with a config path set at startup → not a path traversal if the path is operator-controlled, not user-supplied + +3. **True positives to take seriously:** + - Hardcoded credentials or tokens in source (not test fixtures) + - `exec.Command` or `os.Exec` with any user-controlled input + - `crypto/md5` or `crypto/sha1` used for security purposes (not checksums) + - HTTP client that skips TLS verification (`InsecureSkipVerify: true`) outside of test code + +4. **Remediation pattern:** + - Fix in source, run `make lint` and `make test` before opening a PR + - If Checkmarx flags a pattern that is genuinely safe in context, add a suppression comment with justification — but only after confirming with the security team + +--- + +## Scanner configuration reference + +Defined in `sec-scanners-config.yaml`: -4. **No fix available?** - - Assess exploitability in context (network-accessible? requires auth? distroless removes significant attack surface) - - Document the assessment in the CVE tracking issue with justification - - Suppression in scanner config requires Kyma security team approval +| Scanner | Type | Scope | Excludes | +|---|---|---|---| +| **BDBA** | Container binary scan | Production images | — | +| **Mend** | Go module SCA | `go.mod` dependency graph | Test files | +| **Checkmarx One** | Go SAST | Go source (`go-default` preset) | Test files | +| **OSSF Scorecard** | Supply-chain security | GitHub repo posture | — (weekly) | ## Key security constraints (do not weaken during fixes) -- `GOFIPS140=v1.0.0` in `Dockerfile` and `Makefile` — required; removing it is not a valid CVE mitigation -- TLS 1.3 minimum on all endpoints — do not downgrade to accept a scanner's "recommended" TLS 1.2 fallback +- `GOFIPS140=v1.0.0` in `Dockerfile` and `Makefile` — never remove; removing it is not a valid CVE mitigation +- TLS 1.3 minimum on all endpoints — do not downgrade even if a scanner recommends TLS 1.2 compatibility - `rotationPolicy: Always` on cert-manager certificates — required for key hygiene -- Container base image pinned to sha256 digest — update the digest, never switch to a tag +- Container base image pinned to sha256 digest — update the digest, never switch to a floating tag From 33dab86801b01cf5b639ceda5fb8f301f251fe8a Mon Sep 17 00:00:00 2001 From: medmes Date: Fri, 8 May 2026 15:39:01 +0200 Subject: [PATCH 6/9] docs(claude): align operator-reviewer agent with official schema Adds model: claude-sonnet-4-6 (explicit, no accidental Opus usage), color: blue (visual ID in UI), maxTurns: 20 (caps read-only review loop). --- .claude/agents/operator-reviewer.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.claude/agents/operator-reviewer.md b/.claude/agents/operator-reviewer.md index 72a3ba9054..ce1d473eb1 100644 --- a/.claude/agents/operator-reviewer.md +++ b/.claude/agents/operator-reviewer.md @@ -2,6 +2,9 @@ name: operator-reviewer description: Reviews Go Kubernetes operator code for correctness against lifecycle-manager patterns. Use when you want a second opinion on reconciler changes, CRD type additions, or controller wiring. Invoke with: "Use the operator-reviewer agent to review this change." tools: Read, Grep, Glob +model: claude-sonnet-4-6 +color: blue +maxTurns: 20 --- You are a senior Kubernetes operator engineer reviewing a code change against the lifecycle-manager architectural rules. You have read-only access to the codebase. From 1113b413f3c72959f4b0b1607b0d9bad0e7029bd Mon Sep 17 00:00:00 2001 From: medmes Date: Fri, 8 May 2026 15:43:38 +0200 Subject: [PATCH 7/9] docs(claude): add principal-engineer agent for design-level review Complements operator-reviewer (rule compliance, Sonnet) with a design judgment agent (architectural fit, abstractions, maintainability, Opus). Documents the two-agent workflow in CLAUDE.md. --- .claude/agents/principal-engineer.md | 81 ++++++++++++++++++++++++++++ CLAUDE.md | 11 ++++ 2 files changed, 92 insertions(+) create mode 100644 .claude/agents/principal-engineer.md diff --git a/.claude/agents/principal-engineer.md b/.claude/agents/principal-engineer.md new file mode 100644 index 0000000000..5854088f14 --- /dev/null +++ b/.claude/agents/principal-engineer.md @@ -0,0 +1,81 @@ +--- +name: principal-engineer +description: Senior engineering design review. Use when you want judgment on whether an approach is architecturally sound, not just rule-compliant. Invoke before or after operator-reviewer when the change is non-trivial — new abstractions, new controllers, new cross-cutting patterns, significant refactors. Ask: "Use the principal-engineer agent to review this design." +tools: Read, Grep, Glob +model: claude-opus-4-7 +color: purple +maxTurns: 25 +--- + +You are a principal software engineer with deep experience building production Kubernetes operators. You review code and design decisions at a higher level than a rule-checklist: your job is to ask whether the approach is right, not just whether it follows the existing rules. + +You have read-only access to the codebase. Browse as much context as you need before forming an opinion. Do not rush to verdict. + +## What you evaluate + +### 1. Simplicity and necessity +- Is this the simplest solution that correctly solves the problem? +- Is any new abstraction, interface, or type actually necessary, or is it invented complexity? +- Could this be done with less code and fewer moving parts? +- If a helper was extracted, does it have a single clear reason to exist, or is it just factored-out noise? + +### 2. Abstraction fitness +- Are the new types and interfaces at the right level? Do they express domain concepts (reconciliation, module state, SKR connectivity) or implementation details? +- Does the naming reflect what the code *is* and *does*, without leaking the internal how? +- Would a new contributor understand the intent from the type and method names alone, without reading the implementation? + +### 3. Architectural fit +- Does this change follow the established patterns (interface injection, SSA, conditions, requeueing via `queue.DetermineRequeueInterval`)? +- If it deviates from a pattern, is the deviation justified and localized, or does it set a precedent that will be copied incorrectly? +- Does new state belong where it was placed? (Reconciler struct vs. service layer vs. caller) + +### 4. Error philosophy +- Are errors wrapped with enough context to trace the failure without logs (`fmt.Errorf("context: %w", err)`)? +- Is the error classification correct — transient vs. permanent, requeue vs. terminal? +- Does the code distinguish between "caller did something wrong" and "external system is unavailable"? + +### 5. Observability +- Are meaningful state transitions visible via conditions or metrics? +- If this introduces new failure modes, is there a signal an oncall engineer can act on? +- Is there a log statement at the right level (not too verbose, not silent on important transitions)? + +### 6. Concurrency and lifecycle +- Is shared state protected? Are there hidden races in concurrent reconcile paths? +- Does the change respect the SKR context lifecycle (Init → Get, InvalidateCache on auth failure)? +- Are finalizers added before any work that creates external state that needs cleanup? + +### 7. Testability +- Is the change testable with the existing test infrastructure (envtest, DualClusterFactory)? +- Are new dependencies injectable as interfaces, or are they hardcoded concrete types? +- Is the happy path tested? Is the primary failure mode tested? + +### 8. Maintenance cost +- Will this code be easy to change in 12 months by someone who didn't write it? +- Are there hidden assumptions that aren't expressed as types, constants, or comments? +- Does this increase or decrease the cognitive load of the reconcile loop? + +## Output format + +``` +## Principal Engineer Review + +### Design assessment +[2-4 sentences on the overall approach — is the design sound?] + +### Concerns +- [HIGH] : — +- [MEDIUM] : — +- [LOW] : — + +### What works well +- + +### Verdict +APPROVE / REQUEST CHANGES / REJECT + +[1-2 sentences on the decisive factor for the verdict] +``` + +A REJECT verdict means the fundamental approach needs rethinking before implementation details matter — suggest the alternative. REQUEST CHANGES means the approach is sound but specific design decisions need addressing. APPROVE means you would merge this, even if small things could be better. + +Do not give a verdict before you have read enough code to understand the context. If the diff alone is insufficient, read the surrounding files first. diff --git a/CLAUDE.md b/CLAUDE.md index 112e770913..e7af83fd04 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -176,4 +176,15 @@ Follow the Kyma team's Claude Code workflow: - **Planning complex tasks** — switch to Opus: `/model claude-opus-4-7` - **Implementation** — use the default Sonnet: `/model claude-sonnet-4-6` +## Review agents + +Two complementary agents are available in `.claude/agents/`: + +| Agent | When to use | Model | +|---|---|---| +| `operator-reviewer` | After writing code — checks rule compliance (spec mutation, conditions, markers, finalizers) | Sonnet | +| `principal-engineer` | Before or after coding — judges whether the design is right, not just rule-compliant | Opus | + +Run `operator-reviewer` for any PR touching reconciler logic or CRD types. Run `principal-engineer` for non-trivial changes — new abstractions, new controllers, significant refactors, or anything where you want a second opinion on the approach itself. + Use Opus when you need to understand an unfamiliar subsystem, design a non-trivial change, or reason about cross-cutting impacts. Switch back to Sonnet once the approach is clear and you are writing code. From 4832eeca1dd9042aedc7cef323b0ccfe48dee2dc Mon Sep 17 00:00:00 2001 From: medmes Date: Fri, 8 May 2026 15:49:23 +0200 Subject: [PATCH 8/9] docs(claude): move Go conventions to path-scoped rules file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Splits import aliases, ordering, and lint limits out of CLAUDE.md into .claude/rules/go-conventions.md with paths: ["**/*.go"]. Rules load only when editing Go files, saving ~28 lines from the always-loaded context. CLAUDE.md: 190 → 162 lines. --- .claude/rules/go-conventions.md | 42 +++++++++++++++++++++++++++++++++ CLAUDE.md | 30 +---------------------- 2 files changed, 43 insertions(+), 29 deletions(-) create mode 100644 .claude/rules/go-conventions.md diff --git a/.claude/rules/go-conventions.md b/.claude/rules/go-conventions.md new file mode 100644 index 0000000000..5f66dd1b8f --- /dev/null +++ b/.claude/rules/go-conventions.md @@ -0,0 +1,42 @@ +--- +paths: + - "**/*.go" +--- + +# Go code conventions — lifecycle-manager + +These rules are enforced by CI (`make lint`). Violations fail the build. + +## Import aliases + +The `importas` linter enforces strict aliases. Key ones used in almost every file: + +| Package | Alias | +|---|---| +| `k8s.io/apimachinery/pkg/apis/meta/v1` | `apimetav1` | +| `k8s.io/apimachinery/pkg/api/errors` | `apierrors` | +| `k8s.io/apimachinery/pkg/runtime` | `machineryruntime` | +| `k8s.io/apimachinery/pkg/labels` | `k8slabels` | +| `k8s.io/api/core/v1` | `apicorev1` | +| `k8s.io/api/apps/v1` | `apiappsv1` | +| `k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1` | `apiextensionsv1` | +| `sigs.k8s.io/controller-runtime` | `ctrl` | +| `sigs.k8s.io/controller-runtime/pkg/controller` | `ctrlruntime` | +| `sigs.k8s.io/controller-runtime/pkg/log` | `logf` | + +Full alias list: `.golangci.yaml` → `linters-settings.importas.alias`. + +## Import ordering + +Enforced by `gci`: **standard → third-party → project** (`github.com/kyma-project/lifecycle-manager`) **→ blank → dot**. + +## Lint limits + +- Line length: **120 characters** (`revive line-length-limit`) +- Function length: **80 lines** (`funlen`) — `//nolint:funlen // ` only for composition root wiring +- Cyclomatic complexity: **20** (`cyclop`) +- All linters enabled by default; check `.golangci.yaml` before adding any `//nolint`. Every directive **must** include an explanation comment. + +## FIPS + +Use `GOFIPS140=v1.0.0 go` for any `go` command run directly (Makefile sets this automatically). Do not add dependencies that use non-FIPS-approved crypto. diff --git a/CLAUDE.md b/CLAUDE.md index e7af83fd04..c09ea9ea22 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -81,35 +81,7 @@ Forcefully removes all module resources from a remote cluster when a `Kyma` CR h ## Code conventions -### Import aliases - -The `importas` linter enforces strict aliases — violations fail CI. Key ones you will use in almost every file: - -| Package | Alias | -|---|---| -| `k8s.io/apimachinery/pkg/apis/meta/v1` | `apimetav1` | -| `k8s.io/apimachinery/pkg/api/errors` | `apierrors` | -| `k8s.io/apimachinery/pkg/runtime` | `machineryruntime` | -| `k8s.io/apimachinery/pkg/labels` | `k8slabels` | -| `k8s.io/api/core/v1` | `apicorev1` | -| `k8s.io/api/apps/v1` | `apiappsv1` | -| `k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1` | `apiextensionsv1` | -| `sigs.k8s.io/controller-runtime` | `ctrl` | -| `sigs.k8s.io/controller-runtime/pkg/controller` | `ctrlruntime` | -| `sigs.k8s.io/controller-runtime/pkg/log` | `logf` | - -Full alias list is in `.golangci.yaml` under `linters-settings.importas.alias`. - -### Import ordering - -Enforced by `gci`: **standard → third-party → project** (`github.com/kyma-project/lifecycle-manager`) **→ blank → dot**. - -### Lint limits - -- Line length: **120 characters** (revive `line-length-limit`) -- Function length: **80 lines** (`funlen`) — use `//nolint:funlen // ` only for composition root wiring -- Cyclomatic complexity: **20** (`cyclop`) -- All linters enabled by default; check `.golangci.yaml` before adding `//nolint`. Every `//nolint` directive **must** include an explanation comment. +Go import aliases, import ordering, and lint limits load automatically when editing `.go` files — see [`.claude/rules/go-conventions.md`](.claude/rules/go-conventions.md). ## Where to look for more context From 3c7e1d33966209357cbfec67b17af72b9f030852 Mon Sep 17 00:00:00 2001 From: medmes Date: Fri, 8 May 2026 15:56:59 +0200 Subject: [PATCH 9/9] docs(claude): remove duplicated alias table from go-conventions rule The table was incomplete (10/75 aliases) and would drift from .golangci.yaml. Rule now points to .golangci.yaml as the authoritative source and retains only what's not machine-enforceable: the nolint policy and FIPS constraint. --- .claude/rules/go-conventions.md | 36 +++++++++------------------------ 1 file changed, 10 insertions(+), 26 deletions(-) diff --git a/.claude/rules/go-conventions.md b/.claude/rules/go-conventions.md index 5f66dd1b8f..9d04d638e1 100644 --- a/.claude/rules/go-conventions.md +++ b/.claude/rules/go-conventions.md @@ -5,38 +5,22 @@ paths: # Go code conventions — lifecycle-manager -These rules are enforced by CI (`make lint`). Violations fail the build. +`make lint` is the authoritative check. The full linter config is in `.golangci.yaml`. ## Import aliases -The `importas` linter enforces strict aliases. Key ones used in almost every file: +Strict aliases are enforced by `importas` — violations fail CI. The **complete alias list** is in `.golangci.yaml` under `linters-settings.importas.alias` (75 entries). When adding an import, check that file first. -| Package | Alias | -|---|---| -| `k8s.io/apimachinery/pkg/apis/meta/v1` | `apimetav1` | -| `k8s.io/apimachinery/pkg/api/errors` | `apierrors` | -| `k8s.io/apimachinery/pkg/runtime` | `machineryruntime` | -| `k8s.io/apimachinery/pkg/labels` | `k8slabels` | -| `k8s.io/api/core/v1` | `apicorev1` | -| `k8s.io/api/apps/v1` | `apiappsv1` | -| `k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1` | `apiextensionsv1` | -| `sigs.k8s.io/controller-runtime` | `ctrl` | -| `sigs.k8s.io/controller-runtime/pkg/controller` | `ctrlruntime` | -| `sigs.k8s.io/controller-runtime/pkg/log` | `logf` | +Import ordering is enforced by `gci`: **standard → third-party → project** (`github.com/kyma-project/lifecycle-manager`) **→ blank → dot**. -Full alias list: `.golangci.yaml` → `linters-settings.importas.alias`. +## nolint policy -## Import ordering - -Enforced by `gci`: **standard → third-party → project** (`github.com/kyma-project/lifecycle-manager`) **→ blank → dot**. - -## Lint limits - -- Line length: **120 characters** (`revive line-length-limit`) -- Function length: **80 lines** (`funlen`) — `//nolint:funlen // ` only for composition root wiring -- Cyclomatic complexity: **20** (`cyclop`) -- All linters enabled by default; check `.golangci.yaml` before adding any `//nolint`. Every directive **must** include an explanation comment. +Every `//nolint` directive **must** include an explanation: +```go +//nolint:funlen // composition root wiring — acceptable exception +``` +Bare `//nolint:funlen` fails review. Check `.golangci.yaml` before adding any suppression. ## FIPS -Use `GOFIPS140=v1.0.0 go` for any `go` command run directly (Makefile sets this automatically). Do not add dependencies that use non-FIPS-approved crypto. +Use `GOFIPS140=v1.0.0 go` for any `go` command run directly (the Makefile sets this automatically). Do not add dependencies that bypass the FIPS-approved stdlib crypto (no third-party elliptic curves, no custom cipher suites).