From a79b7be961456578e79fa49f6c38866d73715d15 Mon Sep 17 00:00:00 2001 From: DevContainer User Date: Thu, 5 Mar 2026 13:49:54 +0000 Subject: [PATCH] docs: add architecture decision records for error boundaries and hooks architecture Co-Authored-By: Claude Opus 4.6 --- .../adr/006-dual-error-boundaries.md | 151 +++++++++++++++++ docs/architecture/adr/007-hooks-vs-context.md | 157 ++++++++++++++++++ docs/architecture/adr/README.md | 2 + 3 files changed, 310 insertions(+) create mode 100644 docs/architecture/adr/006-dual-error-boundaries.md create mode 100644 docs/architecture/adr/007-hooks-vs-context.md diff --git a/docs/architecture/adr/006-dual-error-boundaries.md b/docs/architecture/adr/006-dual-error-boundaries.md new file mode 100644 index 0000000..dc38788 --- /dev/null +++ b/docs/architecture/adr/006-dual-error-boundaries.md @@ -0,0 +1,151 @@ +# ADR 006: Error Boundary with Dual Variants + +**Status**: Accepted + +**Date**: 2026-03-05 + +**Deciders**: Development Team + +--- + +## Context + +The Sealed Secrets plugin registers components at two distinct integration points in Headlamp: + +1. **Route-level**: Full-page views (`SealedSecretList`, `SealingKeysView`) registered via `registerRoute` +2. **Section-level**: Injected detail sections (`SecretDetailsSection`) registered via `registerDetailsViewSection` + +Each integration point has different error recovery requirements: + +- **Route-level errors** typically stem from API connectivity issues (controller not found, RBAC misconfiguration). Users need troubleshooting guidance and a retry mechanism. +- **Section-level errors** are isolated failures within a host page. The error should be contained without disrupting the rest of the detail view. A simple reload is sufficient. + +A single error boundary class cannot serve both needs because the error messaging, recovery actions, and visual treatment differ significantly. + +--- + +## Decision + +Implement a `BaseErrorBoundary` abstract class with a `renderError()` template method, then derive two concrete variants: + +- **`ApiErrorBoundary`**: Used at route level. Displays connectivity troubleshooting guidance (check controller namespace, RBAC permissions, pod status) with a Retry button that resets the error state. +- **`GenericErrorBoundary`**: Used at section level. Displays a compact error message with a Reload button. Designed to fail gracefully without affecting the parent detail page. + +Both variants use `getDerivedStateFromError` for error capture and expose a reset mechanism via `setState({ hasError: false })`. + +```typescript +abstract class BaseErrorBoundary extends React.Component { + static getDerivedStateFromError(error: Error): State { + return { hasError: true, error }; + } + + abstract renderError(error: Error): React.ReactNode; + + render() { + if (this.state.hasError) { + return this.renderError(this.state.error); + } + return this.props.children; + } +} +``` + +--- + +## Consequences + +### Positive + +✅ **Appropriate error recovery**: Each integration point gets tailored error messages and recovery actions + +✅ **Fault isolation**: Section-level errors don't crash the entire detail page + +✅ **Shared base class**: Common error capture logic is defined once in `BaseErrorBoundary` + +✅ **Consistent with React patterns**: Error boundaries are the recommended React mechanism for catching render errors + +### Negative + +⚠️ **Class components required**: React error boundaries must be class components, breaking the otherwise all-functional-component convention + +⚠️ **Two components to maintain**: Changes to error handling patterns must be applied to both variants + +### Mitigation + +- The class component exception is documented and limited to `ErrorBoundary.tsx` +- Both variants share `BaseErrorBoundary`, so common logic changes propagate automatically + +--- + +## Alternatives Considered + +### 1. Single generic error boundary + +**Pros**: +- Simpler — one component for all uses +- Less code to maintain + +**Cons**: +- Cannot provide context-specific troubleshooting guidance +- Route-level errors need different recovery UX than section-level errors +- Generic messages are unhelpful for API connectivity issues + +**Rejected**: The error recovery requirements differ too much between route and section contexts. + +--- + +### 2. try/catch in each component + +**Pros**: +- No class components needed +- Per-component error handling + +**Cons**: +- Cannot catch render-phase errors (React limitation) +- Duplicated error handling logic across every component +- Inconsistent error UX + +**Rejected**: React error boundaries are the only mechanism for catching render errors. + +--- + +### 3. React error boundary library (react-error-boundary) + +**Pros**: +- Functional component API via `ErrorBoundary` wrapper +- Built-in reset mechanisms +- Well-maintained + +**Cons**: +- External dependency not available in plugin runtime +- Plugin cannot add npm dependencies beyond Headlamp peer dependencies + +**Rejected**: Dependency constraint makes this infeasible. + +--- + +## Implementation + +- `ApiErrorBoundary` wraps `SealedSecretList` and `SealingKeysView` in `index.tsx` +- `GenericErrorBoundary` wraps `SecretDetailsSection` in `index.tsx` +- Both are defined in `src/components/ErrorBoundary.tsx` +- Uses MUI `Alert`, `Box`, `Button`, `Typography` for styled error display + +--- + +## References + +- [React Error Boundaries](https://react.dev/reference/react/Component#catching-rendering-errors-with-an-error-boundary) +- [Headlamp Plugin Registration API](https://headlamp.dev/docs/latest/development/plugins/) + +--- + +## Related ADRs + +- [ADR 005: Custom React Hooks](005-react-hooks-extraction.md) — Hooks architecture that error boundaries wrap + +--- + +## Changelog + +- **2026-03-05**: Initial decision diff --git a/docs/architecture/adr/007-hooks-vs-context.md b/docs/architecture/adr/007-hooks-vs-context.md new file mode 100644 index 0000000..5ec3902 --- /dev/null +++ b/docs/architecture/adr/007-hooks-vs-context.md @@ -0,0 +1,157 @@ +# ADR 007: Custom Hooks Architecture vs Data Context + +**Status**: Accepted + +**Date**: 2026-03-05 + +**Deciders**: Development Team + +--- + +## Context + +All other Headlamp plugins in this project family (polaris, rook, intel-gpu, kube-vip, tns-csi) use a single React Context provider (`*DataProvider`) to centralize data fetching and share state across components. This is the established pattern. + +The Sealed Secrets plugin has different requirements: + +1. **Multiple independent data domains**: Controller health, RBAC permissions, SealedSecret CRUD, and encryption are logically separate concerns with different lifecycles. +2. **CRD class extension**: `SealedSecret` extends Headlamp's `KubeObject` class, providing its own `useList()` hook — making a centralized fetch redundant for the primary resource. +3. **Write-heavy workflows**: Unlike read-only plugins, sealed-secrets creates, encrypts, and rotates resources. The encryption workflow involves multi-step state (certificate fetch → encrypt → create resource). +4. **Independent refresh cadences**: Controller health polls every 30 seconds; SealedSecret list is reactive via `useList()`; RBAC checks run once on mount. + +A single context provider would either become a monolithic "god context" or force artificial coupling between unrelated concerns. + +--- + +## Decision + +Use **independent custom hooks** instead of a shared data context: + +- **`useControllerHealth(autoRefresh?, intervalMs?)`**: Polls controller `/healthz` endpoint. Returns `{ healthy, checking, error, refresh }`. +- **`usePermissions()`**: Queries RBAC capabilities on mount. Returns permission flags for create, delete, encrypt operations. +- **`useSealedSecretEncryption()`**: Orchestrates the encryption workflow (fetch cert → encrypt values → build manifest). Returns workflow state and action functions. +- **`SealedSecret.useList()`**: Headlamp's built-in `KubeObject.useList()` — reactive to cluster changes, no custom fetch needed. + +Each hook manages its own loading, error, and refresh state. Components compose multiple hooks as needed. + +```typescript +function SealedSecretList() { + const [secrets, error] = SealedSecret.useList(); + const { healthy } = useControllerHealth(true); + const { canCreate } = usePermissions(); + // Each concern is independent +} +``` + +--- + +## Consequences + +### Positive + +✅ **Separation of concerns**: Each hook encapsulates a single domain (health, permissions, encryption, CRUD) + +✅ **Independent lifecycles**: Controller health polls at 30s; RBAC checks once; list is reactive — no unnecessary coupling + +✅ **Composable**: Components pick only the hooks they need, avoiding unnecessary data in scope + +✅ **Testable in isolation**: Each hook can be unit-tested independently without mocking an entire context provider + +✅ **Leverages Headlamp's KubeObject**: `SealedSecret.useList()` provides reactive list updates without custom fetch logic + +### Negative + +⚠️ **Diverges from project convention**: Other plugins use the `*DataProvider` pattern — contributors must learn a different approach for this plugin + +⚠️ **No single source of truth**: State is distributed across hooks rather than centralized — harder to debug "what data does the plugin have right now?" + +⚠️ **Potential duplicate fetches**: If two components both call `useControllerHealth()`, the health endpoint is polled twice + +### Mitigation + +- The convention divergence is documented in `CLAUDE.md` and this ADR +- Controller health polling is lightweight (single `/healthz` call) +- `SealedSecret.useList()` is internally deduplicated by Headlamp's hook system + +--- + +## Alternatives Considered + +### 1. Single SealedSecretsDataProvider context + +**Pros**: +- Consistent with other plugins in the project +- Single source of truth for all sealed-secrets data +- Deduplicates fetches automatically + +**Cons**: +- Would become a "god context" with 10+ fields spanning unrelated concerns +- All consumers re-render when any field changes (health poll triggers list re-render) +- Encryption workflow state doesn't belong in shared context (it's dialog-scoped) +- `SealedSecret.useList()` already provides reactive CRUD — wrapping it in context adds indirection + +**Rejected**: The data domains are too independent; a single context would create artificial coupling. + +--- + +### 2. Multiple specialized contexts + +**Pros**: +- Separation of concerns (like hooks) +- Consistent with React Context pattern + +**Cons**: +- Three or four nested providers in `index.tsx` — deep nesting +- More boilerplate than hooks (provider + context + consumer hook per domain) +- No benefit over standalone hooks when providers don't need to share state + +**Rejected**: Contexts add boilerplate without benefit when data domains are independent. + +--- + +### 3. State management library (Zustand, Jotai) + +**Pros**: +- Lightweight, no provider nesting +- Built-in deduplication and memoization + +**Cons**: +- External dependency not available in plugin runtime +- Plugins cannot add npm dependencies beyond Headlamp peer dependencies + +**Rejected**: Dependency constraint makes this infeasible. + +--- + +## Implementation + +``` +src/hooks/ +├── useControllerHealth.ts # Health polling with configurable interval +├── usePermissions.ts # RBAC capability check (runs once) +└── useSealedSecretEncryption.ts # Multi-step encryption workflow +``` + +- Components in `src/components/` import hooks directly +- No provider wrapping needed in `index.tsx` (except error boundaries) +- `SealedSecret` class in `src/lib/SealedSecretCRD.ts` extends `KubeObject` for `useList()`/`useGet()` + +--- + +## References + +- [React Custom Hooks](https://react.dev/learn/reusing-logic-with-custom-hooks) +- [Headlamp KubeObject API](https://headlamp.dev/docs/latest/development/api/classes/lib_k8s_cluster.KubeObject/) + +--- + +## Related ADRs + +- [ADR 005: Custom React Hooks](005-react-hooks-extraction.md) — Details the hook extraction process +- [ADR 006: Dual Error Boundaries](006-dual-error-boundaries.md) — Error handling that wraps hook-based components + +--- + +## Changelog + +- **2026-03-05**: Initial decision diff --git a/docs/architecture/adr/README.md b/docs/architecture/adr/README.md index 140b828..bde921f 100644 --- a/docs/architecture/adr/README.md +++ b/docs/architecture/adr/README.md @@ -26,6 +26,8 @@ Each ADR follows this structure: | [003](003-client-side-crypto.md) | Client-Side Encryption | Accepted | 2026-02-11 | | [004](004-rbac-integration.md) | RBAC-Aware UI | Accepted | 2026-02-11 | | [005](005-react-hooks-extraction.md) | Custom React Hooks | Accepted | 2026-02-12 | +| [006](006-dual-error-boundaries.md) | Error Boundary with Dual Variants | Accepted | 2026-03-05 | +| [007](007-hooks-vs-context.md) | Custom Hooks Architecture vs Data Context | Accepted | 2026-03-05 | ## Creating New ADRs