Conversation
Propose adopting a Kubernetes operator for OpenFGA lifecycle management, covering migration handling, declarative CRDs for stores/models/tuples, and the operator deployment model as a Helm subchart. Also adds the ADR process documentation, template, and chart analysis. ADR-001: Adopt a Kubernetes Operator ADR-002: Operator-Managed Migrations ADR-003: Declarative Store Lifecycle CRDs ADR-004: Operator Deployment as Helm Subchart
WalkthroughIntroduces Architecture Decision Records (ADRs) framework for the project. Adds ADR template and five decision documents covering OpenFGA Operator adoption, operator-managed database migrations, declarative store lifecycle management via CRDs, and operator deployment strategy. Includes README documenting ADR practices and contributor guidelines. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10–15 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds an Architecture Decision Record (ADR) process and proposes a staged approach to adopting an OpenFGA Kubernetes operator for Helm-chart lifecycle management (migrations, store/model/tuple CRDs, and operator deployment model).
Changes:
- Introduces an ADR template and README describing the ADR workflow and lifecycle.
- Adds ADR-001..004 documenting the operator proposal, including migration handling and declarative CRDs.
- Documents operator distribution as a conditional Helm subchart and outlines CRD installation/upgrade approaches.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/adr/README.md | Documents the ADR process and provides an index of proposed ADRs. |
| docs/adr/000-template.md | Adds a standard ADR template for future decisions. |
| docs/adr/001-adopt-openfga-operator.md | Proposes adopting an operator and outlines staged scope and tradeoffs. |
| docs/adr/002-operator-managed-migrations.md | Proposes replacing Helm-hook migrations with operator-managed migrations. |
| docs/adr/003-declarative-store-lifecycle-crds.md | Proposes CRDs for store/model/tuple lifecycle management via the operator. |
| docs/adr/004-operator-deployment-model.md | Proposes deploying the operator as a conditional Helm subchart dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| spec: | ||
| # Reference to the OpenFGA instance | ||
| openfgaRef: | ||
| url: openfga.openfga-system.svc:8081 |
There was a problem hiding this comment.
In this CRD example, openfgaRef.url is shown as a bare host:port without a scheme, which is not a URL and may be ambiguous for consumers (HTTP vs HTTPS). Consider either using a full URL (e.g., include http:///https://) or renaming the field in the proposed API to something like address/endpoint to match the expected format.
| url: openfga.openfga-system.svc:8081 | |
| url: http://openfga.openfga-system.svc:8081 |
| The operator will support both modes via a `watchNamespace` configuration: | ||
|
|
||
| ```yaml | ||
| # Operator values | ||
| operator: | ||
| watchNamespace: "" # empty = watch own namespace only (all-in-one mode) | ||
| # watchNamespace: "" # or set to a specific namespace | ||
| # watchAllNamespaces: true # watch all namespaces (standalone mode) |
There was a problem hiding this comment.
This values snippet uses operator.watchNamespace, but earlier the ADR notes that subchart configuration is typically passed via prefixed keys (e.g., openfga-operator.*) while operator.enabled is the dependency condition. Consider aligning the example with the intended values contract (e.g., show openfga-operator.watchNamespace / openfga-operator.watchAllNamespaces, or explicitly clarify which keys live in the parent chart vs the subchart).
| The operator will support both modes via a `watchNamespace` configuration: | |
| ```yaml | |
| # Operator values | |
| operator: | |
| watchNamespace: "" # empty = watch own namespace only (all-in-one mode) | |
| # watchNamespace: "" # or set to a specific namespace | |
| # watchAllNamespaces: true # watch all namespaces (standalone mode) | |
| The operator will support both modes via subchart values such as `openfga-operator.watchNamespace`: | |
| ```yaml | |
| # Subchart values (`operator.enabled` remains the parent-chart dependency condition) | |
| openfga-operator: | |
| watchNamespace: "" # empty = watch own namespace only (all-in-one mode) | |
| # watchNamespace: "my-namespace" # or set to a specific namespace | |
| # watchAllNamespaces: true # watch all namespaces (standalone mode) |
There was a problem hiding this comment.
🧹 Nitpick comments (5)
docs/adr/004-operator-deployment-model.md (2)
69-90: Optional: Add language identifier to fenced code block.The chart structure diagram could specify
textas the language identifier:-``` +```text helm-charts/ ├── charts/Minor linter compliance improvement.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/adr/004-operator-deployment-model.md` around lines 69 - 90, Update the fenced code block showing the helm-charts directory diagram in docs/adr/004-operator-deployment-model.md by adding the language identifier "text" to the opening fence (i.e., change the ``` block that precedes the helm-charts/ tree to ```text) so the diagram is lint-friendly; leave the content of the tree (helm-charts/, charts/, openfga/, openfga-operator/, crds/, etc.) unchanged.
140-146: ClarifywatchNamespaceconfiguration example.The example shows three different modes but the YAML structure is ambiguous:
- Line 143 sets
watchNamespace: ""(empty = own namespace)- Lines 144-145 show commented alternatives but it's unclear if these are meant to be mutually exclusive settings or if there's a separate
watchAllNamespacesbooleanConsider restructuring to show the three modes more clearly:
# Operator values operator: - watchNamespace: "" # empty = watch own namespace only (all-in-one mode) - # watchNamespace: "" # or set to a specific namespace - # watchAllNamespaces: true # watch all namespaces (standalone mode) + # Mode 1: Watch own namespace only (all-in-one mode, default) + watchNamespace: "" + + # Mode 2: Watch a specific namespace + # watchNamespace: "other-namespace" + + # Mode 3: Watch all namespaces (standalone mode) + # watchAllNamespaces: trueThis makes it clearer that these are three mutually exclusive configurations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/adr/004-operator-deployment-model.md` around lines 140 - 146, The operator values example is ambiguous about mutually exclusive settings; update the operator config examples so they clearly show three separate, mutually exclusive modes: (1) all-in-one mode using watchNamespace: "" (or omitted) to watch only the operator's namespace, (2) single-namespace mode using watchNamespace: "<namespace>" to watch a specific namespace, and (3) standalone/global mode using watchAllNamespaces: true to watch all namespaces. Reference the operator block and the keys watchNamespace and watchAllNamespaces (operator -> watchNamespace / watchAllNamespaces) and present each mode as its own distinct commented YAML snippet so readers understand they should use only one of these options.docs/adr/README.md (1)
33-39: Optional: Consider adding language identifier to fenced code block.The lifecycle diagram could specify
textas the language identifier to satisfy linters:-``` +```text Proposed → Accepted → (optionally) Superseded or DeprecatedThis is purely for linter compliance and doesn't affect readability.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/adr/README.md` around lines 33 - 39, The fenced code block containing the lifecycle diagram (the triple-backtick block whose content starts with "Proposed → Accepted → (optionally) Superseded or Deprecated") should include a language identifier for linter compliance; change the opening fence from ``` to ```text so the block becomes ```text ... ``` to satisfy linters without altering the diagram content or readability.docs/adr/003-declarative-store-lifecycle-crds.md (1)
54-75: Optional: Add language identifier to YAML code blocks.The FGAStore CRD example could specify
yamlas the language identifier for better syntax highlighting:-``` +```yaml apiVersion: openfga.dev/v1alpha1 kind: FGAStoreSimilar changes could be applied to the FGAModel (lines 85-116) and FGATuples (lines 127-155) examples. This is a minor improvement for readability and linter compliance.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/adr/003-declarative-store-lifecycle-crds.md` around lines 54 - 75, Add the YAML language identifier to the code fences for all CRD examples to enable proper syntax highlighting: update the FGAStore example (the block showing apiVersion: openfga.dev/v1alpha1 / kind: FGAStore), the FGAModel example (the block describing model schema), and the FGATuples example (the block showing tuples/state) so each opening triple-backtick uses ```yaml; ensure any other fenced blocks in this file that contain YAML are similarly updated.docs/adr/002-operator-managed-migrations.md (1)
76-94: Optional: Add language identifier to fenced code blocks.The operator reconciliation diagram could specify
textas the language identifier:-``` +```text ┌────────────────────────────────────────────────────────┐ │ Operator Reconciliation │Similar changes could be applied to the sequence diagrams (lines 130-141, 147-163) for linter compliance. This is purely cosmetic.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/adr/002-operator-managed-migrations.md` around lines 76 - 94, The fenced diagrams lack a language identifier; update the three fenced code blocks that render the "Operator Reconciliation" diagram and the two sequence diagrams so their opening backticks include the language identifier "text" (e.g., change ``` to ```text) to satisfy the linter and consistency; locate the blocks by the visible headings/labels "Operator Reconciliation" and the sequence diagram blocks around the existing diagram text and update their fences accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/adr/002-operator-managed-migrations.md`:
- Around line 76-94: The fenced diagrams lack a language identifier; update the
three fenced code blocks that render the "Operator Reconciliation" diagram and
the two sequence diagrams so their opening backticks include the language
identifier "text" (e.g., change ``` to ```text) to satisfy the linter and
consistency; locate the blocks by the visible headings/labels "Operator
Reconciliation" and the sequence diagram blocks around the existing diagram text
and update their fences accordingly.
In `@docs/adr/003-declarative-store-lifecycle-crds.md`:
- Around line 54-75: Add the YAML language identifier to the code fences for all
CRD examples to enable proper syntax highlighting: update the FGAStore example
(the block showing apiVersion: openfga.dev/v1alpha1 / kind: FGAStore), the
FGAModel example (the block describing model schema), and the FGATuples example
(the block showing tuples/state) so each opening triple-backtick uses ```yaml;
ensure any other fenced blocks in this file that contain YAML are similarly
updated.
In `@docs/adr/004-operator-deployment-model.md`:
- Around line 69-90: Update the fenced code block showing the helm-charts
directory diagram in docs/adr/004-operator-deployment-model.md by adding the
language identifier "text" to the opening fence (i.e., change the ``` block that
precedes the helm-charts/ tree to ```text) so the diagram is lint-friendly;
leave the content of the tree (helm-charts/, charts/, openfga/,
openfga-operator/, crds/, etc.) unchanged.
- Around line 140-146: The operator values example is ambiguous about mutually
exclusive settings; update the operator config examples so they clearly show
three separate, mutually exclusive modes: (1) all-in-one mode using
watchNamespace: "" (or omitted) to watch only the operator's namespace, (2)
single-namespace mode using watchNamespace: "<namespace>" to watch a specific
namespace, and (3) standalone/global mode using watchAllNamespaces: true to
watch all namespaces. Reference the operator block and the keys watchNamespace
and watchAllNamespaces (operator -> watchNamespace / watchAllNamespaces) and
present each mode as its own distinct commented YAML snippet so readers
understand they should use only one of these options.
In `@docs/adr/README.md`:
- Around line 33-39: The fenced code block containing the lifecycle diagram (the
triple-backtick block whose content starts with "Proposed → Accepted →
(optionally) Superseded or Deprecated") should include a language identifier for
linter compliance; change the opening fence from ``` to ```text so the block
becomes ```text ... ``` to satisfy linters without altering the diagram content
or readability.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 361cf8ee-3c79-4208-8d78-66b0501e5e11
📒 Files selected for processing (6)
docs/adr/000-template.mddocs/adr/001-adopt-openfga-operator.mddocs/adr/002-operator-managed-migrations.mddocs/adr/003-declarative-store-lifecycle-crds.mddocs/adr/004-operator-deployment-model.mddocs/adr/README.md
|
|
||
| ## Decision | ||
|
|
||
| We will build an **OpenFGA Kubernetes Operator** that handles: |
Propose adopting a Kubernetes operator for OpenFGA lifecycle management, covering migration handling, declarative CRDs for stores/models/tuples, and the operator deployment model as a Helm subchart.
Also adds the ADR process documentation, template, and chart analysis.
ADR-001: Adopt a Kubernetes Operator
ADR-002: Operator-Managed Migrations
ADR-003: Declarative Store Lifecycle CRDs
ADR-004: Operator Deployment as Helm Subchart
Description
What problem is being solved?
How is it being solved?
What changes are made to solve it?
References
Review Checklist
mainSummary by CodeRabbit