diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 000000000..d77c7a3df --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,182 @@ +# Cluster Policy Controller - AI Agent Development Guide + +> **For AI Agents:** This file contains critical context for understanding and working with the cluster-policy-controller codebase. Read this fully before making changes. Pay special attention to "Never Do" and "Common Agent Mistakes" sections. + +The cluster-policy-controller maintains policy resources necessary to create pods in an OpenShift cluster: UID/SELinux allocation, quota management, CSR approval, and Pod Security Admission label synchronization. + +## Quick Reference + +### Essential Commands +```bash +# Build +make build # Build binary + +# Test +make test # Run unit tests (./pkg/... ./cmd/...) +make verify # Run all verification checks + +# Image +make images # Build container image + +# Clean +make clean # Remove built binary +``` + +**Note**: This lists the most commonly used commands. Consult the [Makefile](./Makefile) for +additional build, test, and verification targets. + +### Additional Documentation +- **[ARCHITECTURE.md](./ARCHITECTURE.md)** - System design, controller architecture, data flow +- **[CONTRIBUTING.md](./CONTRIBUTING.md)** - Development workflow, testing, PR process +- **[README.md](./README.md)** - Quick start and deployment context + +## Overview + +The cluster-policy-controller is a multi-controller binary that runs inside the `kube-controller-manager` static pod in the `openshift-kube-controller-manager` namespace. It is managed by the [`cluster-kube-controller-manager-operator`](https://github.com/openshift/cluster-kube-controller-manager-operator/). + +**Purpose**: Maintain policy resources (UID ranges, SELinux labels, quotas, PSA labels) that are prerequisites for pod creation in an OpenShift cluster. + +**Key Capabilities**: +- Allocate UID ranges and SELinux MCS labels for namespaces +- Manage ResourceQuota with OpenShift-specific image stream quota support +- Reconcile ClusterResourceQuota usage across namespaces +- Approve CertificateSigningRequests for monitoring components +- If PSA enforcement is turned on, synchronize Pod Security Admission (PSA) labels based on SCC assignments +- If PSA enforcement is turned off, synchronize only warn and audit PSA labels based on SCC assignments +- Label privileged system namespaces with PSA privileged level + +### Controllers + +Six controllers are registered in `pkg/cmd/controller/config.go`. See [ARCHITECTURE.md — Controller Details](./ARCHITECTURE.md#controller-details) for each controller's purpose, resources watched, and data flow. + +## Architecture Patterns + +### OpenShift library-go Pattern + +This binary follows the **OpenShift library-go controller pattern**: + +**Key Characteristics**: +1. **Factory-based Controllers**: Use `factory.New()` pattern from library-go +2. **Informer-driven**: React to cluster changes via Kubernetes informers +3. **Event Recording**: Use `events.Recorder` for audit trail +4. **Shared Context**: All controllers share `EnhancedControllerContext` for informers and clients +5. **Controller Enablement**: Controllers can be enabled/disabled via `OpenShiftControllerManagerConfig.Controllers` + +For controller initialization patterns, the `EnhancedControllerContext`, and the startup flow, see [ARCHITECTURE.md — Controller Architecture](./ARCHITECTURE.md#controller-architecture) and [Startup and Lifecycle](./ARCHITECTURE.md#startup-and-lifecycle). + +## Development Guidelines + +### What to Always Do + +1. Use Informers and Listers (Almost Never Direct API Calls in Sync Loops) +2. Use library-go Factory Pattern +3. Use Server-Side Apply for Updates +4. Record Events for Significant Actions +5. Handle Conflicts Gracefully +6. Respect Controller Enablement + +### What to Ask First + +#### 1. Before Adding New Dependencies +**Ask**: "Is this already available in library-go or the Kubernetes packages we vendor?" +- Prefer library-go patterns over custom implementations +- Check `go.mod` for existing dependencies +- New dependencies must be vendored (`go mod vendor`) + +#### 2. Before Changing Informer Resync Periods +**Ask**: "What is the impact on API server load?" +- Default is 10 minutes for most informers +- The namespace SCC allocation controller uses 8-hour repair cycles +- Shorter resync = more API server load + +#### 3. Before Modifying UID/MCS Allocation Logic +**Ask**: "Does this change affect existing clusters with allocated ranges?" +- UID ranges are permanent once assigned to namespaces +- The bitmap-based allocator in `RangeAllocation` is a critical data structure +- Changes can break existing namespace security configurations + +#### 4. Before Changing PSA Label Sync Behavior +**Ask**: "Does this affect the `security.openshift.io/scc.podSecurityLabelSync` contract?" +- Namespaces opt in/out via this label +- System namespaces (`openshift-*`) have special handling +- Exempted namespaces are hardcoded in `nsexemptions/` + +#### 5. Before Adding New Service Accounts +**Ask**: "Does the service account exist in the `openshift-infra` namespace?" +- Controllers use dedicated service accounts via `ClientBuilder` +- These must be provisioned by the kube-controller-manager-operator + +### What to Never Do + +1. Almost Never Make Direct API Calls in Sync Loops +2. Never Bypass the ControllerInitializers Registry +3. Never Ignore Error Returns +4. Never Modify the Vendor Directory Manually + +#### 5. Never Skip the Health Check +The binary waits for API server health before starting controllers (`WaitForHealthyAPIServer`). Don't remove or bypass this - it prevents controllers from starting before the API server is ready. + +#### 6. Never Break the UID Allocation Bitmap +The `RangeAllocation` resource stores a bitmap of allocated UID offsets. Operations on this bitmap must be: +- Atomic (update with optimistic concurrency via resourceVersion) +- Consistent with namespace annotations +- Repaired periodically (every 8 hours) + +#### 7. Never Assume Feature Gate State +```go +// GOOD: Check feature gates explicitly +featureGates := sets.NewString(controllerCtx.OpenshiftControllerConfig.FeatureGates...) +if featureGates.Has("OpenShiftPodSecurityAdmission=false") { + // advisory mode +} +``` + +## Common Agent Mistakes to Avoid + +These are specific mistakes AI agents frequently make in this codebase: + +- **Don't suggest `client.Get()` or `client.List()` in sync functions** - Use informers and listers instead. Direct API calls in controller sync loops cause performance issues and are against the library-go pattern. Always use the cached listers from informers. + +- **Don't propose adding controllers without mentioning `pkg/cmd/controller/config.go`** - All controllers must be registered in the `ControllerInitializers` map, or they won't run. Simply creating a controller file isn't enough. + +- **Don't forget the service account constant** - Every controller should use a dedicated service account name constant in `config.go` for `ClientBuilder.Client()`. Missing this means the controller can't authenticate. + +- **Don't modify `nsexemptions/`** - The namespace exemption list determines which namespaces are never synced by the PSA label syncer. Changes here affect cluster security posture. + +- **Don't use table-driven tests without the `t.Run()` pattern** - All tests should use subtests for better failure isolation: + +- **Don't change the field manager string** - Controllers use specific field manager names (e.g., `"cluster-policy-controller"`, `"pod-security-admission-label-synchronization-controller"`) for server-side apply. Changing these causes ownership conflicts and can break label management. + +## Security Notes + +- **UID Range Allocation** - Each namespace gets a unique UID block from `1000000000-1999999999/10000`. This is a finite resource that cannot be reclaimed without manual intervention. +- **SELinux MCS Labels** - Each namespace gets unique MCS labels derived from its UID range offset. These must be unique to maintain SELinux isolation. +- **PSA Label Sync** - The PSA labels determine what security profiles pods in a namespace must adhere to. Incorrect labels can either block legitimate workloads or allow privileged access. +- **CSR Approval** - The CSR approver only handles monitoring-related CSRs with specific labels and subject names. It does not approve arbitrary CSRs. +- **RBAC** - Each controller uses a separate service account with minimal permissions. + +**Security-Critical Code**: +- `pkg/security/controller/` - UID and MCS allocation (affects container isolation) +- `pkg/psalabelsyncer/` - PSA label computation (affects admission control) +- `pkg/psalabelsyncer/scctopsamapping.go` - SCC to PSA level mapping +- `pkg/cmd/controller/csr.go` - CSR approval subjects + +## Testing + +Unit tests are co-located with code in `pkg/` directories. Use `find . -name '*_test.go'` to discover test files. + +**Framework**: Go `testing` (some test files also use testify assertions) + +**Running**: +```bash +make test # Run all unit tests +go test ./pkg/... # Run package tests directly +``` + +**CI/CD**: Tests run via OpenShift CI (Prow). Configuration is in the [openshift/release](https://github.com/openshift/release) repository. + +## Questions? + +- **Slack**: #forum-ocp-apiserver (OpenShift internal) +- **Component**: see OWNERS file +- **Approvers**: see OWNERS file diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md new file mode 100644 index 000000000..6eaedc418 --- /dev/null +++ b/ARCHITECTURE.md @@ -0,0 +1,520 @@ +# Cluster Policy Controller - Architecture + +This document explains the architecture, design decisions, and operational model of the cluster-policy-controller. + +## Table of Contents +- [Overview](#overview) +- [System Architecture](#system-architecture) +- [Controller Architecture](#controller-architecture) +- [Startup and Lifecycle](#startup-and-lifecycle) +- [Controller Details](#controller-details) +- [Informer and Client Infrastructure](#informer-and-client-infrastructure) +- [Design Decisions](#design-decisions) +- [Failure Modes and Recovery](#failure-modes-and-recovery) + +## Overview + +### What is the Cluster Policy Controller? + +The cluster-policy-controller is a multi-controller binary that enforces cluster-wide policies required for pod creation in OpenShift. It runs as a container inside the `kube-controller-manager` static pod in the `openshift-kube-controller-manager` namespace. + +Unlike an operator that manages its own operand, this controller IS the workload - it runs six independent controllers in a single process, each watching different resources and enforcing different policies. + +### Why Does It Exist? + +OpenShift extends Kubernetes with security and quota features that have no upstream equivalent: +- **UID Range Allocation**: Every namespace needs a unique UID range for container security isolation (SCC enforcement) +- **SELinux MCS Labels**: Required for SELinux-based container isolation, derived from UID range +- **ClusterResourceQuota**: OpenShift's cluster-scoped quota that aggregates across namespaces +- **Image Stream Quotas**: Resource quota evaluation for OpenShift ImageStreams +- **PSA Label Sync**: Bridges the gap between OpenShift SCCs and Kubernetes Pod Security Admission +- **CSR Approval**: Automated certificate approval for monitoring infrastructure + +These policies are foundational - without them, pods cannot be created (missing UID range), quota cannot be enforced (missing evaluators), or monitoring breaks (unsigned certificates). + +## System Architecture + +### High-Level Architecture + +```text +┌─────────────────────────────────────────────────────────────────────┐ +│ OpenShift Cluster │ +│ │ +│ ┌───────────────────────────────────────────────────────────────┐ │ +│ │ openshift-kube-controller-manager namespace │ │ +│ │ │ │ +│ │ ┌─────────────────────────────────────────────────────────┐ │ │ +│ │ │ kube-controller-manager static pod │ │ │ +│ │ │ │ │ │ +│ │ │ ┌───────────────────────────────────────────────────┐ │ │ │ +│ │ │ │ cluster-policy-controller container │ │ │ │ +│ │ │ │ │ │ │ │ +│ │ │ │ ┌──────────────┐ ┌──────────────────────────┐ │ │ │ │ +│ │ │ │ │ Namespace │ │ Pod Security Admission │ │ │ │ │ +│ │ │ │ │ SCC │ │ Label Syncer │ │ │ │ │ +│ │ │ │ │ Allocation │ │ │ │ │ │ │ +│ │ │ │ └──────────────┘ └──────────────────────────┘ │ │ │ │ +│ │ │ │ ┌──────────────┐ ┌──────────────────────────┐ │ │ │ │ +│ │ │ │ │ Resource │ │ Privileged Namespace │ │ │ │ │ +│ │ │ │ │ Quota │ │ Pod Security Admission │ │ │ │ │ +│ │ │ │ │ Manager │ │ Label Syncer │ │ │ │ │ +│ │ │ │ └──────────────┘ └──────────────────────────┘ │ │ │ │ +│ │ │ │ ┌──────────────┐ ┌──────────────────────────┐ │ │ │ │ +│ │ │ │ │ CSR Approver │ │ Cluster Quota │ │ │ │ │ +│ │ │ │ │ │ │ Reconciliation │ │ │ │ │ +│ │ │ │ └──────────────┘ └──────────────────────────┘ │ │ │ │ +│ │ │ └───────────────────────────────────────────────────┘ │ │ │ +│ │ └─────────────────────────────────────────────────────────┘ │ │ +│ └───────────────────────────────────────────────────────────────┘ │ +│ │ +│ Resources Watched / Managed: │ +│ ┌─────────────┐ ┌────────────┐ ┌────────┐ ┌───────────────────┐ │ +│ │ Namespaces │ │ SCCs │ │ CSRs │ │ ClusterResource- │ │ +│ │ (all) │ │ │ │ │ │ Quotas │ │ +│ └─────────────┘ └────────────┘ └────────┘ └───────────────────┘ │ +│ ┌─────────────┐ ┌────────────┐ ┌────────────────────────────────┐ │ +│ │ Resource │ │ Image │ │ ServiceAccounts, Roles, │ │ +│ │ Quotas │ │ Streams │ │ RoleBindings, ClusterRoles, │ │ +│ │ │ │ │ │ ClusterRoleBindings │ │ +│ └─────────────┘ └────────────┘ └────────────────────────────────┘ │ +│ ┌────────────────────────────────┐ │ +│ │ RangeAllocation (UID bitmap) │ │ +│ └────────────────────────────────┘ │ +└─────────────────────────────────────────────────────────────────────┘ +``` + +### Data Flow + +```text +Binary starts: cluster-policy-controller start + │ + ▼ +Parse OpenShiftControllerManagerConfig (from ComponentConfig) + │ + ▼ +Wait for API server healthy (/healthz, poll every 1s, timeout 5min) + │ + ▼ +Create EnhancedControllerContext + ├── KubernetesInformers (10min resync) + ├── ImageInformers (10min resync) + ├── QuotaInformers (10min resync) + ├── SecurityInformers (10min resync) + ├── MetadataInformers (10min resync) + └── ClientBuilder (per-SA clients, QPS/10 + 1) + │ + ▼ +For each ControllerInitializer: + ├── Check IsControllerEnabled (config.Controllers list) + ├── Call InitFunc(ctx, controllerCtx) + └── Each controller runs in its own goroutine + │ + ▼ +StartInformers() - starts all informer factories + │ + ▼ +Block on ctx.Done() until shutdown +``` + +## Controller Architecture + +### Controller Registry + +All controllers are registered in the `ControllerInitializers` map in `pkg/cmd/controller/config.go`. Each controller: +- Has a unique name (used for enablement and logging) +- Uses a dedicated service account for RBAC isolation +- Gets its own goroutine +- Shares the informer cache (reads are cheap, writes go to API server) + +### InitFunc Signature + +```go +type InitFunc func(ctx context.Context, controllerCtx *EnhancedControllerContext) (bool, error) +``` + +Returns: +- `(true, nil)` - controller started successfully +- `(true, error)` - controller attempted to start but failed (fatal) +- `(false, nil)` - controller chose not to start (skipped) + +## Startup and Lifecycle + +### Process Lifecycle + +The binary is managed by the `cluster-kube-controller-manager-operator` as part of the kube-controller-manager static pod. It does not self-update, does not manage its own deployment, and does not have operator status conditions. It relies on the operator for lifecycle management. + +### Health Check + +Before starting controllers, the binary waits for the API server: + +```go +func WaitForHealthyAPIServer(client rest.Interface) error { + return wait.PollImmediate(time.Second, 5*time.Minute, func() (bool, error) { + healthStatus := 0 + client.Get().AbsPath("/healthz").Do(ctx).StatusCode(&healthStatus) + return healthStatus == http.StatusOK, nil + }) +} +``` + +This is critical because the binary starts alongside the API server in the same static pod. + +### Controller Enablement + +Controllers can be enabled/disabled via the `Controllers` field in `OpenShiftControllerManagerConfig`: +- `["*"]` (default) - all controllers enabled +- `["-openshift.io/resourcequota"]` - disable specific controller +- `["openshift.io/namespace-security-allocation"]` - enable only specific controllers + +## Controller Details + +### 1. Namespace Security Allocation Controller + +**Key**: `openshift.io/namespace-security-allocation` +**Location**: `pkg/security/controller/namespace_scc_allocation_controller.go` +**Service Account**: `namespace-security-allocation-controller` + +**Purpose**: Allocate UID ranges and SELinux MCS labels to namespaces that don't have them. + +**Resources Watched**: Namespaces, RangeAllocation +**Resources Modified**: Namespaces (annotations), RangeAllocation + +**How It Works**: + +```text +Namespace created/updated (via informer) + │ + ▼ +Does namespace have securityv1.UIDRangeAnnotation? + ├── Yes → Skip (already allocated) + └── No → Allocate + │ + ▼ + Read RangeAllocation "scc-uid" from API + │ + ▼ + Parse bitmap (big.Int) from RangeAllocation.Data + │ + ▼ + Find next unset bit (linear scan) + │ + ▼ + Set bit in bitmap, Update RangeAllocation (optimistic lock) + │ + ▼ + Calculate UID block from bit offset + │ + ▼ + Calculate MCS label from UID block offset + │ + ▼ + Apply annotations to namespace (server-side apply): + - openshift.io/sa.scc.uid-range: "1000010000/10000" + - openshift.io/sa.scc.supplemental-groups: "1000010000/10000" + - openshift.io/sa.scc.mcs: "s0:c1,c0" +``` + +**Repair Cycle**: Every 8 hours, rebuilds the entire bitmap from namespace annotations to fix any inconsistencies. Initial repair runs at startup with retry. + +**Concurrency**: Runs with 1 worker. The bitmap update uses optimistic concurrency (resourceVersion on RangeAllocation). + +### 2. Resource Quota Manager + +**Key**: `openshift.io/resourcequota` +**Location**: `pkg/cmd/controller/quota.go` (init), Kubernetes `kresourcequota.Controller` (implementation) +**Service Account**: `resourcequota-controller` + +**Purpose**: Enforce ResourceQuota with OpenShift-specific image stream evaluators. + +**How It Works**: + +This wraps the standard Kubernetes ResourceQuota controller and adds image stream quota evaluators: + +```text +Standard K8s quota controller + + Image stream evaluators (from pkg/quota/quotaimageexternal/) + ├── ImageStreamImport evaluator + └── ImageStreamTag evaluator +``` + +### 3. Cluster Quota Reconciliation Controller + +**Key**: `openshift.io/cluster-quota-reconciliation` +**Location**: `pkg/quota/clusterquotareconciliation/reconciliation_controller.go` +**Service Account**: `cluster-quota-reconciliation-controller` + +**Purpose**: Maintain ClusterResourceQuota objects by aggregating usage across all matched namespaces. + +**How It Works**: + +```text +ClusterResourceQuota defines: + - Hard limits (e.g., pods: 100 across selected namespaces) + - Namespace selector (label or annotation based) + │ + ▼ +ClusterQuotaMappingController + - Maps ClusterResourceQuotas to namespaces + - Watches Namespaces and ClusterResourceQuotas + │ + ▼ +ClusterQuotaReconciliationController + - For each mapped namespace: evaluate used resources + - Aggregate usage across all namespaces + - Update ClusterResourceQuota.Status with total used +``` + +**Components**: +- `ClusterQuotaMappingController` (from library-go): Maps quotas to namespaces +- `ClusterQuotaReconciliationController`: Computes and updates usage + +### 4. CSR Approver Controller + +**Key**: `openshift.io/cluster-csr-approver` +**Location**: `pkg/cmd/controller/csr.go` +**Service Account**: `cluster-csr-approver-controller` + +**Purpose**: Automatically approve CertificateSigningRequests for monitoring components. + +**How It Works**: + +```text +CertificateSigningRequest created + │ + ▼ +Label filter: metrics.openshift.io/csr.subject ∈ {prometheus, metrics-server} + ├── No match → Ignore + └── Match → Validate + │ + ▼ + Check requester is openshift-monitoring:cluster-monitoring-operator + │ + ▼ + Check subject is one of: + - CN=system:serviceaccount:openshift-monitoring:prometheus-k8s + - CN=system:serviceaccount:openshift-monitoring:metrics-server + │ + ▼ + Approve CSR +``` + +**Scope**: Only approves CSRs from the `openshift-monitoring` namespace with specific labels and subjects. This is not a general-purpose CSR approver. + +### 5. Pod Security Admission Label Syncer + +**Key**: `openshift.io/podsecurity-admission-label-syncer` +**Location**: `pkg/psalabelsyncer/podsecurity_label_sync_controller.go` +**Service Account**: `podsecurity-admission-label-syncer-controller` + +**Purpose**: Synchronize Kubernetes Pod Security Admission (PSA) labels on namespaces based on the SecurityContextConstraints (SCCs) available to service accounts in those namespaces. + +**How It Works**: + +```text +Namespace change (or SA/SCC/RoleBinding change) + │ + ▼ +Is namespace controlled? + - Not exempted (nsexemptions list) + - Label security.openshift.io/scc.podSecurityLabelSync != "false" + - Not an openshift-* NS (unless explicitly opted in with label "true") + - Controller owns at least one PSA label + │ + ▼ +Wait for UID range annotation (from SCC allocation controller) + │ + ▼ +List all ServiceAccounts in namespace + │ + ▼ +For each SA: determine allowed SCCs via RBAC + (SAToSCCCache: RoleBindings → Roles → SCC "use" verb) + │ + ▼ +For each allowed SCC: convert to PSA level + (scctopsamapping.go: SCC fields → privileged/baseline/restricted) + │ + ▼ +Take the most permissive PSA level across all SAs + │ + ▼ +- If OpenShiftPodSecurityAdmission=true server-side apply pod-security.kubernetes.io/enforce, + pod-security.kubernetes.io/warn and pod-security.kubernetes.io/audit labels. +- If OpenShiftPodSecurityAdmission=false server-side apply pod-security.kubernetes.io/warn and + pod-security.kubernetes.io/audit labels. We do not set pod-security.kubernetes.io/enforce label. +- Set version labels to "latest" +- Set annotation: security.openshift.io/MinimallySufficientPodSecurityStandard +``` + +**Two Modes** (controlled by feature gate): +- **Enforcing** (default, `OpenShiftPodSecurityAdmission=true`): Sets enforce + warn + audit labels +- **Advising** (`OpenShiftPodSecurityAdmission=false`): Sets only warn + audit labels (no enforcement) + +**SAToSCCCache**: Maintains a mapping of ServiceAccount → accessible SCCs by indexing RoleBindings and ClusterRoleBindings. Watches for RBAC changes and re-enqueues affected namespaces. + +**Field Ownership**: Uses server-side apply with field manager `pod-security-admission-label-synchronization-controller`. Includes logic to migrate ownership from the legacy `cluster-policy-controller` field manager. + +### 6. Privileged Namespaces PSA Label Syncer + +**Key**: `openshift.io/privileged-namespaces-psa-label-syncer` +**Location**: `pkg/psalabelsyncer/privileged_namespaces_controller.go` +**Service Account**: `privileged-namespaces-psa-label-syncer` + +**Purpose**: Label system namespaces with PSA privileged level. + +**Target Namespaces**: Built-in Kubernetes namespaces (`default`, `kube-system`, `kube-public`). + +**Applied Labels**: +```yaml +pod-security.kubernetes.io/enforce: privileged +pod-security.kubernetes.io/audit: privileged +pod-security.kubernetes.io/warn: privileged +``` + +This controller is simpler than the PSA label syncer - it doesn't analyze SCCs, it just labels known system namespaces as privileged. + +## Informer and Client Infrastructure + +### Client QPS Management + +The controller context divides API server QPS across clients: + +This prevents any single controller from monopolizing API server bandwidth. + +### GenericResourceInformer + +A union informer that tries multiple backends: +1. KubernetesInformers (core K8s resources) +2. ImageInformers (OpenShift images) +3. QuotaInformers (OpenShift quotas) +4. MetadataInformers (fallback for any GVR) + +Used primarily by the quota controllers for evaluating resources across different API groups. + +### Informer Lifecycle + +```text +1. Controllers are initialized (each registers informer event handlers) +2. StartInformers() starts all factories (starts watches) +3. InformersStarted channel is closed (signals controllers can proceed) +4. Controllers process events from their work queues +``` + +Informers are started AFTER all controllers are initialized to ensure no events are missed. + +## Design Decisions + +### Why a Multi-Controller Binary? + +**Decision**: Run 6 controllers in a single binary instead of 6 separate deployments. + +**Rationale**: +- **Shared informers**: All controllers watch namespaces - sharing the informer cache saves significant API server load and memory +- **Operational simplicity**: One binary to deploy, monitor, and debug +- **Historical**: These controllers were originally part of the OpenShift controller manager + +**Trade-offs**: +- Shared informers reduce memory and API calls +- Failure in one controller doesn't affect others (each runs in its own goroutine) +- But a crash in the binary takes down all controllers +- Cannot scale controllers independently + +### Why Run Inside kube-controller-manager? + +**Decision**: Run as a container in the kube-controller-manager static pod, not as a standalone deployment. + +**Rationale**: +- These controllers must run exactly once (no replicas) +- They must start alongside the control plane +- The kube-controller-manager-operator already manages the static pod lifecycle +- Health checking is integrated with the existing control plane health + +**Trade-offs**: +- Lifecycle tied to kube-controller-manager +- Cannot be independently updated without operator involvement +- Guaranteed to run on control plane nodes + +### Why Server-Side Apply for Namespace Updates? + +**Decision**: Use server-side apply (SSA) instead of read-modify-write for namespace annotations and labels. + +**Rationale**: +- **Conflict-free**: SSA uses field-level ownership, not object-level resourceVersion +- **Partial updates**: Only touches the fields the controller manages +- **Ownership tracking**: Clear audit trail of which controller owns which fields + +**Trade-offs**: +- SSA is the modern Kubernetes pattern +- Requires careful field manager naming +- Migration from legacy Update() to Apply() required ownership transfer logic (see `forceHistoricalLabelsOwnership`) + +### Why Per-Controller Service Accounts? + +**Decision**: Each controller uses a dedicated service account from `openshift-infra`. + +**Rationale**: +- **Least privilege**: Each controller gets only the RBAC permissions it needs +- **Audit trail**: API server audit logs show which controller made each call +- **Blast radius**: Compromising one SA doesn't grant access to all resources + +**Trade-offs**: +- More service accounts to manage +- More RBAC resources to maintain +- But much better security posture + +## Failure Modes and Recovery + +### Controller Crash + +**Symptom**: The entire binary crashes (since all controllers run in one process). + +**Recovery**: +- The static pod is automatically restarted by kubelet +- Controllers resume from their informer caches +- Work queue items are re-processed +- The UID allocator runs its initial repair on restart + +### API Server Unavailable + +**Symptom**: Informers can't connect, API calls fail. + +**Recovery**: +- The health check (`WaitForHealthyAPIServer`) retries for up to 5 minutes +- After startup, informers automatically reconnect via watch +- Work queue items are requeued on failure with exponential backoff + +### UID Range Exhaustion + +**Symptom**: `uid range exceeded` error in logs. New namespaces don't get UID annotations. + +**Recovery**: +- This is a hard limit - the range `1000000000-1999999999/10000` supports ~100,000 namespaces +- Requires widening the range (config change + operator restart) +- Existing allocations are preserved + +### Stale RangeAllocation Bitmap + +**Symptom**: Bitmap doesn't match namespace annotations (e.g., after etcd restore). + +**Recovery**: +- The 8-hour repair cycle rebuilds the bitmap from namespace annotations +- Can force immediate repair by restarting the binary +- The repair is idempotent and safe to run at any time + +### PSA Label Conflicts + +**Symptom**: Multiple field managers trying to set the same PSA labels. + +**Recovery**: +- The controller detects when another user manages a label and backs off +- The `forceHistoricalLabelsOwnership` function handles migration from legacy field managers +- Logs a warning: "someone else is already managing the PSA labels" + +### Quota Reconciliation Lag + +**Symptom**: ClusterResourceQuota status shows stale usage counts. + +**Recovery**: +- The resync period ensures eventual consistency +- Discovery sync picks up new resource types +- Force immediate reconciliation by editing the ClusterResourceQuota diff --git a/CLAUDE.md b/CLAUDE.md new file mode 120000 index 000000000..47dc3e3d8 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1 @@ +AGENTS.md \ No newline at end of file diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 000000000..9a6cb02a1 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,419 @@ +# Contributing to Cluster Policy Controller + +This document serves as a guide for contributing to the cluster-policy-controller, which is maintained by the OpenShift Control Plane group. + +This document is explicitly for contributions to the cluster-policy-controller repository and not for high-level feature proposals within OpenShift. + +Feature proposals should follow the OpenShift Enhancement Proposal process outlined in https://github.com/openshift/enhancements/blob/master/dev-guide/feature-zero-to-hero.md#openshift-feature-development-zero-to-hero-guide. +If you are looking for a review on an OpenShift Enhancement Proposal that involves changes to the cluster-policy-controller, please request a review in the [`#forum-ocp-apiserver`](https://redhat.enterprise.slack.com/archives/CB48XQ4KZ) Slack channel. + +## Table of Contents + +- [Code Conventions](#code-conventions) +- [Testing Guidelines](#testing-guidelines) +- [Pull Request Process and Guidelines](#pull-request-process-and-guidelines) +- [Review Expectations](#review-expectations) +- [Local Development Guide](#local-development-guide) + +--- + +## Code Conventions + +We largely follow the [Kubernetes Code Conventions](https://github.com/kubernetes/community/blob/main/contributors/guide/coding-conventions.md#code-conventions). + +Review both the Kubernetes Code Conventions and the ones specified here. If any conventions are at odds with one another, prefer the conventions explicitly documented here. + +### Bash + +- Follow the [shell styleguide](https://google.github.io/styleguide/shellguide.html). +- Use [`shellcheck`](https://github.com/koalaman/shellcheck) to identify common mistakes or caveats. +- Ensure that all scripts run consistently across Linux and MacOS. + +### Golang (Go) + +- Review [Effective Go](https://go.dev/doc/effective_go). +- Review common [Go Code Review Comments](https://go.dev/wiki/CodeReviewComments). +- Review and avoid [Go Landmines](https://gist.github.com/lavalamp/4bd23295a9f32706a48f) +- Comment your code following the [Go comment conventions](https://go.dev/doc/comment). + - Comments should be meaningful and add context and/or explain choices that cannot be expressed through clear code. + - All exported types, functions, and methods must have descriptive comments. + - All unexported types, functions, and methods should have descriptive comments. +- When adding command-line flags, use dashes/hyphens (`-`) and not underscores (`_`). +- Naming + - Please consider package name when selecting an interface name, and avoid redundancy. For example, `storage.Interface` is better than `storage.StorageInterface`. + - Do not use uppercase characters, underscores, or dashes in package names. + - Please consider parent directory name when choosing a package name. For example, `pkg/controllers/autoscaler/foo.go` should say `package autoscaler` not `package autoscalercontroller`. + - Unless there's a good reason, the package foo line should match the name of the directory in which the .go file exists. + - Importers can use a different name if they need to disambiguate. + - Locks should be called `lock` and should never be embedded (always `lock sync.Mutex`). When multiple locks are present, give each lock a distinct name following Go conventions: `stateLock`, `mapLock` etc. +- Error handling + - Wrap errors with meaningful context before returning or logging them. +- When logging, follow the [Kubernetes Logging Conventions](https://github.com/kubernetes/community/blob/main/contributors/devel/sig-instrumentation/logging.md). +- When patching OpenShift-maintained forks of "upstream" repositories, patches should be as small as reasonably possible and should minimize touch points with code that is likely to change and impact the rebasing process. + +### General + +Regardless of the programming language, make sure to take the following into consideration: +- Keep readability / maintainability in mind when writing code. + - Clever code and abstractions are often harder to reason about after the fact. Keep clever code and abstractions to the minimum necessary to accomplish the end-goal. +- Do not reinvent the wheel. Where possible, use existing standard library or vendored library functionality. If you are adding a net-new dependency, stop and think if you _really_ need to add the new dependency to achieve your goals. +- When writing tests, focus on testing the functional behaviors your code exercises. Avoid writing tests that are testing that the standard library works as expected or is trivial. Do not write tests just for line coverage. + +### Directory and File Conventions + +- Avoid package sprawl. Find an appropriate subdirectory for new packages. + - Libraries with no appropriate home belong in new package subdirectories of `pkg/`. +- Avoid general utility packages. Packages called "util" are suspect. Instead, derive a name that describes your desired function. +- All filenames should be lowercase. +- Go source files and directories use underscores, not dashes. + - Package directories should generally avoid using separators as much as possible. When package names are multiple words, they usually should be in nested subdirectories. + +--- + +## Testing Guidelines + +These are high-level testing guidelines. The cluster-policy-controller has additional testing specifics documented in the [Testing Your Changes](#testing-your-changes) section below. + +- All changes must include unit test additions/changes. + - Exceptions are at reviewer/approver discretion. +- Table-driven unit tests are preferred for testing multiple scenarios/inputs. For an example, see existing tests in `pkg/psalabelsyncer/` or `pkg/security/controller/`. +- Unit tests must pass on all platforms (at the very least, Linux + MacOS). +- Significant features should come with integration and/or end-to-end (e2e) tests where appropriate. + - End-to-end tests _may_ be scoped as a separate work item when the end-to-end tests for the component must be added to the openshift/origin repository instead of the component repository. It is up to reviewer/approver discretion whether a contribution can be merged without end-to-end tests being implemented. +- Do not expect an asynchronous thing to happen immediately. Do not wait for one second and expect a pod to be running. Wait and retry instead. + +If necessary, manual integration testing can be done by creating a cluster using the [`Cluster Bot` Slack App](https://redhat.enterprise.slack.com/archives/D03KX7M1CRJ). +Once you have a cluster created, you can follow some of the instructions in https://github.com/openshift/enhancements/blob/master/dev-guide/operators.md for guidance on how to build component images and modify cluster-operators to deploy those images. + +Most component repos have existing tooling to run unit tests. For cluster-policy-controller: + +```bash +# Run unit tests +make test + +# Run all verification +make verify +``` + +--- + +## Pull Request Process and Guidelines + +This section assumes that you have a functional understanding of `git` and how to create a pull request on GitHub. + +If you do not, start with [GitHub's "Getting Started" guide](https://docs.github.com/en/get-started/start-your-journey). + +### Prerequisites + +Before you commit any changes or create any pull requests, you must adhere to OpenShift contribution policies. +Currently, that means enabling commit signature verification. + +See https://docs.google.com/document/d/1184EPSGunUkcSQYUK8T4a6iyawwi6f2zxdbB2jtG9nQ/edit?usp=sharing for more details on how to adhere to the commit signature verification policy of OpenShift. + +**To enable commit signing**: + +```bash +git config --global user.signingkey +git config --global commit.gpgsign true +``` + +### Creating a Pull Request + +When creating a pull request, include the following: + +- A brief, but descriptive, title. + - All pull requests _should_ link to a Jira ticket associated with the work. There is automation that performs this linking when prefixing the title with the Jira ticket identifier like: `OCPBUGS-XXXX: my pull request title`. For pull requests that have no Jira ticket associated with it, you can prefix it with `NO-JIRA:` to signal that there is not a Jira ticket associated with it. +- A useful description of the changes being made and why they are important. Include links to supporting documents and any additional context that reviewers may need. + +**Example PR Title**: +``` +OCPBUGS-12345: fix UID range allocation for namespaces with existing annotations +``` + +or + +``` +NO-JIRA: fix typo in CONTRIBUTING.md +``` + +### Rebase Checklist + +When rebasing to a new Kubernetes version, copy this checklist into the PR: + +- [ ] Select the desired [kubernetes release branch](https://github.com/kubernetes/kubernetes/branches), and use its `go.mod` and `CHANGELOG` as references for the rest of the work. +- [ ] Bump go version, all `k8s.io/`, `github.com/openshift/`, and any other relevant dependencies as needed. +- [ ] Run `go mod vendor && go mod tidy`, commit that separately from all other changes. +- [ ] Bump image versions (Dockerfile, ci...) if needed. +- [ ] Run `make build verify test`. +- [ ] Make code changes as needed until the above pass. +- [ ] Any other minor update, like documentation. + +### CI / CD + +For CI/CD, OpenShift uses Prow to run various checks. This can include unit tests, e2e tests, linters, etc. + +The jobs configured for each repository are in https://github.com/openshift/release/tree/main/ci-operator/config/openshift . If you find yourself needing to add additional jobs, review the documentation at https://docs.ci.openshift.org/how-tos/contributing-openshift-release/ . + +There are often a mixture of required and optional checks as well as merge criteria that must be met before a pull request can merge. +When any of these checks fail, the GitHub Prow bot will leave a comment on the PR with links to the run of that check that failed. + +As the PR author, it is your responsibility to evaluate the failed checks and determine if there are any changes necessary to pass the checks. +If you suspect that the check failure was a flake, you can trigger retests by commenting `/retest` (or `/retest-required` for retesting only the required checks) on the PR. + +**Common Prow Commands**: +- `/retest` - Rerun all failed tests +- `/retest-required` - Rerun only required failed tests +- `/test ` - Run a specific test job + +### Verifying your changes / Creating an OpenShift cluster from a PR + +As part of merging a PR, there is a requirement to verify that the changes you've made are working as expected using the `/verified` comment command. + +While there are a lot of scenarios where the existing CI/CD checks may be sufficient to verify your changes are working (and can be denoted by commenting `/verified by ci`), +there may be scenarios where manual verification is required. + +You can use the `Cluster Bot` Slack App to create a cluster from a PR by sending it a message in the format of `launch ${OCP_VERSION},${PR_LINK} ${PLATFORM},${VARIANT}`. +As an example, `launch 4.22,https://github.com/openshift/cluster-policy-controller/pull/123 aws` would launch an OpenShift 4.22 cluster with the changes made in the PR running on AWS. +For more information on what `Cluster Bot` can do, you can send it a message saying `help` and it will respond with additional documentation on how it can be used. + +Once you've verified your changes work as expected, you can mark the PR as verified by commenting `/verified by @{your_github_handle}` on the PR. + +### Additional Resources + +For more information regarding more general OpenShift pull request processes, the following resources are helpful: + +- https://docs.ci.openshift.org/architecture/jira +- https://docs.ci.openshift.org/ +- https://steps.ci.openshift.org/ + +--- + +## Review Expectations + +### Requesting a review + +If you are not a member of the OpenShift control plane team and you need a review on a PR, post it in the [#forum-ocp-apiserver](https://redhat.enterprise.slack.com/archives/CB48XQ4KZ) Slack channel or reach out to folks outlined in the OWNERS file directly. + +If you are a member of the OpenShift control plane team, reviews should come from your feature team. In the event your feature team does not have someone that can approve a PR, post it in the [#control-plane](https://redhat.enterprise.slack.com/archives/CC3CZCQHM) Slack channel. + +OpenShift uses AI code review tools as part of the code review process. +Before requesting a review, address all feedback from the code review agent(s). +It is up to your discretion as the contributor how you would like to address that feedback. +Responding with an explanation as to why you are not going to take action on a comment made by the agent is an acceptable way to "address" its feedback. + +### Interacting with reviewers + +When interacting with reviewers/approvers: + +- Be professional. +- Be respectful of differing opinions, viewpoints, and experiences. +- Gracefully give and receive constructive feedback. +- Focus on what is best for the product/organization, not just us as individuals. + +A special note on the usage of AI - to respect the time of those that are reviewing your contribution, please do not use AI to respond to review comments. + +--- + +## Local Development Guide + +This section provides detailed guidance for local development, building, and testing the cluster-policy-controller. + +### Prerequisites + +#### Required Tools + +- **Go**: Version specified in `go.mod` — [Installation guide](https://go.dev/doc/install) +- **Podman or Docker**: For building container images +- **oc CLI**: OpenShift command-line tool +- **Git**: For version control with commit signing enabled +- **Make**: For build automation + +#### Verify Prerequisites + +```bash +# Check Go version +go version # Should match the version in go.mod + +# Check cluster access (for deployment testing) +oc version +oc whoami + +# Verify commit signing is enabled +git config --get user.signingkey +git config --get commit.gpgsign # Should return 'true' +``` + +### Getting Started + +#### Fork and Clone + +1. **Fork the repository** on GitHub + +2. **Clone your fork**: + ```bash + git clone https://github.com//cluster-policy-controller.git + cd cluster-policy-controller + ``` + +3. **Add upstream remote**: + ```bash + git remote add upstream https://github.com/openshift/cluster-policy-controller.git + git fetch upstream + ``` + +#### Understand the Codebase + +Review the documentation: +- [AGENTS.md](./AGENTS.md) - Code patterns and conventions for AI agents +- [ARCHITECTURE.md](./ARCHITECTURE.md) - System design and architecture +- [README.md](./README.md) - Quick overview + +Key directories: +``` +cluster-policy-controller/ +├── cmd/cluster-policy-controller/ # Main entrypoint +├── pkg/cmd/ # Controller initialization +├── pkg/security/ # UID/MCS allocation +├── pkg/quota/ # Quota controllers +├── pkg/psalabelsyncer/ # PSA label sync +└── vendor/ # Vendored dependencies +``` + +### Building + +#### Build the Binary + +```bash +# Build the binary +make build + +# The binary is created at: +# ./cluster-policy-controller +``` + +#### Build the Container Image + +```bash +# Build using the RHEL Dockerfile +podman build -f Dockerfile.rhel -t cluster-policy-controller:dev . +``` + +### Testing Your Changes + +#### Unit Tests + +```bash +# Run all unit tests +make test + +# Run specific package tests +go test ./pkg/security/... +go test ./pkg/psalabelsyncer/... +go test ./pkg/quota/... + +# Run with verbose output +go test -v ./pkg/security/controller/... + +# Run with coverage +go test -coverprofile=coverage.out ./pkg/... ./cmd/... +go tool cover -html=coverage.out +``` + +#### Verification + +```bash +# Run all verification checks +make verify +``` + +### Deploying Changes to a Cluster + +The cluster-policy-controller runs inside the kube-controller-manager static pod. To test changes: + +1. **Build and push a container image**: + ```bash + export QUAY_USER= + export IMAGE_TAG=dev-$(git rev-parse --short HEAD) + + podman build -f Dockerfile.rhel -t quay.io/${QUAY_USER}/cluster-policy-controller:${IMAGE_TAG} . + podman push quay.io/${QUAY_USER}/cluster-policy-controller:${IMAGE_TAG} + ``` + +2. **Follow the OpenShift test deployment guide**: + See [How can I test changes to an OpenShift operator/operand/release component?](https://github.com/openshift/enhancements/blob/master/dev-guide/operators.md#how-can-i-test-changes-to-an-openshift-operatoroperandrelease-component) + +3. **Or use Cluster Bot**: + Send the PR link to the Cluster Bot Slack App to launch a test cluster with your changes. + +### Debugging + +#### View Logs + +The binary runs as a container in the kube-controller-manager static pod: + +```bash +# Find the pod +oc get pods -n openshift-kube-controller-manager + +# View logs for the cluster-policy-controller container +oc logs -n openshift-kube-controller-manager \ + \ + -c cluster-policy-controller -f +``` + +#### Check Controller Status + +```bash +# Check namespace UID allocation +oc get namespace -o jsonpath='{.metadata.annotations}' + +# Check ClusterResourceQuota status +oc get clusterresourcequota -o yaml + +# Check PSA labels on a namespace +oc get namespace --show-labels + +# Check CSR approval +oc get csr +``` + +#### Common Issues + +**Issue**: Namespace missing UID range annotation +```bash +# Check if the SCC allocation controller is running +oc logs -n openshift-kube-controller-manager \ + -c cluster-policy-controller | grep "namespace-security-allocation" + +# Check RangeAllocation +oc get rangeallocations scc-uid -o yaml +``` + +**Issue**: PSA labels not being set +```bash +# Check if the namespace has the opt-in label +oc get namespace -o jsonpath='{.metadata.labels.security\.openshift\.io/scc\.podSecurityLabelSync}' + +# Check if the namespace has a UID range annotation (required) +oc get namespace -o jsonpath='{.metadata.annotations.openshift\.io/sa\.scc\.uid-range}' +``` + +--- + +## Additional Resources + +- [OpenShift Operator Best Practices](https://github.com/openshift/enhancements/blob/master/dev-guide/operators.md) +- [Kubernetes Controller Best Practices](https://kubernetes.io/docs/concepts/architecture/controller/) +- [OpenShift CI Documentation](https://docs.ci.openshift.org/) +- [Pod Security Admission](https://kubernetes.io/docs/concepts/security/pod-security-admission/) +- [OpenShift Security Context Constraints](https://docs.openshift.com/container-platform/latest/authentication/managing-security-context-constraints.html) + +--- + +**Questions?** + +- Post in [#forum-ocp-apiserver](https://redhat.enterprise.slack.com/archives/CB48XQ4KZ) Slack channel +- Open an issue on GitHub +- Review the OWNERS file for maintainer contacts + +**Thank you for contributing!** diff --git a/README.md b/README.md index 21394f774..4438411bf 100644 --- a/README.md +++ b/README.md @@ -6,6 +6,11 @@ Controllers managed by cluster-policy-controller are: * cluster csr approver controller - csr approver for monitoring scraping * podsecurity admission label syncer controller - configure the PodSecurity admission namespace label for namespaces with "security.openshift.io/scc.podSecurityLabelSync: true" label +For more details please see: +- [AGENTS.md](./AGENTS.md) - Code patterns and conventions for AI agents +- [ARCHITECTURE.md](./ARCHITECTURE.md) - System design and architecture +- [CONTRIBUTING.md](./CONTRIBUTING.md) - Contribution guide + ## Run The `cluster-policy-controller` runs as a container in the `openshift-kube-controller-manager namespace`, in the kube-controller-manager static pod. This pod is defined and managed by the [`kube-controller-manager`](https://github.com/openshift/cluster-kube-controller-manager-operator/)