From 15f6c819abce2353e4da352907e16a5da04796bf Mon Sep 17 00:00:00 2001 From: medmes Date: Mon, 11 May 2026 12:01:42 +0200 Subject: [PATCH 1/2] docs(claude): add CLAUDE.md baseline and Go conventions rule Introduces CLAUDE.md with module layout, make targets, single-test commands, KCP/SKR architecture, module installation flow, architectural guardrails, security guardrails, and links to agent_docs/ depth files. Adds .claude/rules/go-conventions.md (loads on *.go): points to .golangci.yaml as source of truth, documents nolint policy and FIPS constraint that are not machine-enforceable. Adds agent_docs/ depth files (architecture, reconcilers, CRD conventions, testing, codegen) for on-demand context beyond CLAUDE.md budget. Adds docs/CLAUDE.md with documentation writing style and templates. Adds CI gate (.github/workflows/check-generated-code.yml) that blocks PRs where generated files (CRD YAML, deepcopy) are out of sync with api/ types. --- .claude/rules/go-conventions.md | 26 ++++ .github/workflows/check-generated-code.yml | 40 +++++ CLAUDE.md | 162 +++++++++++++++++++++ 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 +++++++++++++ docs/CLAUDE.md | 62 ++++++++ 9 files changed, 756 insertions(+) create mode 100644 .claude/rules/go-conventions.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 create mode 100644 docs/CLAUDE.md diff --git a/.claude/rules/go-conventions.md b/.claude/rules/go-conventions.md new file mode 100644 index 0000000000..9d04d638e1 --- /dev/null +++ b/.claude/rules/go-conventions.md @@ -0,0 +1,26 @@ +--- +paths: + - "**/*.go" +--- + +# Go code conventions — lifecycle-manager + +`make lint` is the authoritative check. The full linter config is in `.golangci.yaml`. + +## Import aliases + +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. + +Import ordering is enforced by `gci`: **standard → third-party → project** (`github.com/kyma-project/lifecycle-manager`) **→ blank → dot**. + +## nolint policy + +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 (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). 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..c09ea9ea22 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,162 @@ +# 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/`. 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 | +|---|---| +| `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 +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) \ + 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`. + +## 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. + +## Code conventions + +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 + +| 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) | +| 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: + +- **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. 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. 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 fba8b390f129546ee3da37a8e988c5776df83ddb Mon Sep 17 00:00:00 2001 From: medmes Date: Mon, 11 May 2026 12:01:55 +0200 Subject: [PATCH 2/2] docs(claude): add review agents and CVE triage context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds two complementary review agents: - operator-reviewer (Sonnet): checklist-based compliance review for reconciler changes, CRD types, finalizers, SKR context lifecycle. - principal-engineer (Opus): design-level judgment — abstraction fitness, architectural fit, observability, error philosophy, maintainability. Adds .claude/cve-triage/context.md covering three CVE surfaces: container image (BDBA), Go module SCA (Mend), and Go SAST (Checkmarx). Includes FIPS constraint on crypto fixes and Checkmarx false-positive patterns specific to Kubernetes operators. --- .claude/agents/operator-reviewer.md | 81 +++++++++++++++ .claude/agents/principal-engineer.md | 81 +++++++++++++++ .claude/cve-triage/context.md | 144 +++++++++++++++++++++++++++ 3 files changed, 306 insertions(+) create mode 100644 .claude/agents/operator-reviewer.md create mode 100644 .claude/agents/principal-engineer.md create mode 100644 .claude/cve-triage/context.md diff --git a/.claude/agents/operator-reviewer.md b/.claude/agents/operator-reviewer.md new file mode 100644 index 0000000000..ce1d473eb1 --- /dev/null +++ b/.claude/agents/operator-reviewer.md @@ -0,0 +1,81 @@ +--- +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. + +## 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/.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/cve-triage/context.md b/.claude/cve-triage/context.md new file mode 100644 index 0000000000..bc23185f75 --- /dev/null +++ b/.claude/cve-triage/context.md @@ -0,0 +1,144 @@ +# CVE Triage Context — lifecycle-manager + +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. + +--- + +## 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, no APT +- **Builder**: Go static binary (`CGO_ENABLED=0`, `GOFIPS140=v1.0.0`) +- **Runtime user**: UID 65532 (nonroot) + +#### 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 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` + +### Image CVE triage logic + +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. + +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 + +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)` + +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 + +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`: + +| 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` — 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 floating tag