Conversation
WalkthroughThis pull request introduces a Kubernetes-native operator to orchestrate OpenFGA database migrations, replacing the existing Helm hook-based approach. The implementation includes a controller-runtime operator in Go, a Helm subchart for operator deployment, integration into the main OpenFGA chart, comprehensive testing, and architectural documentation via ADRs. Changes
Sequence DiagramsequenceDiagram
participant User as GitOps/User
participant Helm as Helm Install
participant K8s as Kubernetes
participant Op as Migration Operator
participant DB as Database
User->>Helm: helm install (operator.enabled=true)
Helm->>K8s: Create Deployment (replicas=0, annotations)
Helm->>K8s: Create MigrationReconciler
Helm->>K8s: Return immediately
Note over K8s: Operator starts watching...
K8s->>Op: Detect Deployment with migration enabled
Op->>Op: Read current migration version from ConfigMap
Op->>Op: Extract desired version from image tag
Note over Op: Versions differ → Migration needed
Op->>K8s: Scale Deployment replicas → 0
Op->>K8s: Create migration Job (runs openfga migrate)
K8s->>DB: Job executes migration SQL
DB-->>K8s: Migration succeeds
K8s->>Op: Job marked as Complete
Op->>K8s: Update migration-status ConfigMap (new version)
Op->>K8s: Scale Deployment replicas → desired count
K8s->>K8s: Deployment pods start (migration complete)
K8s-->>User: OpenFGA ready
Note over Op,User: On migration failure:
Note over Op: Set MigrationFailed condition
Note over Op: Delete failed Job after backoff
Note over Op: Requeue with 60s cooldown
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR introduces an OpenFGA migration operator and integrates it into the existing Helm chart to replace Helm hook/initContainer-based migration orchestration (improving compatibility with GitOps tools and helm --wait).
Changes:
- Added a new Go-based operator (controller-runtime) that orchestrates migrations via Kubernetes Jobs and a migration-status ConfigMap.
- Added an
openfga-operatorHelm subchart and integrated it as an optional dependency of the parentopenfgachart (operator.enabled). - Added ADR documentation and local integration test values/instructions for validating operator-driven migration behavior.
Reviewed changes
Copilot reviewed 37 out of 39 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| operator/tests/values-no-db.yaml | Values for permanent DB failure integration scenario |
| operator/tests/values-happy-path.yaml | Values + embedded Postgres manifests for happy-path scenario |
| operator/tests/values-db-outage.yaml | Values + Postgres scaled-to-0 outage/recovery scenario |
| operator/tests/README.md | Step-by-step local integration test instructions |
| operator/README.md | Operator overview, dev commands, and configuration docs |
| operator/Makefile | Basic build/test/vet/lint and docker targets for the operator |
| operator/internal/controller/migration_controller.go | Core reconciliation logic for scaling, Job orchestration, ConfigMap status |
| operator/internal/controller/migration_controller_test.go | Unit tests for reconcile paths and image tag parsing |
| operator/internal/controller/helpers.go | Job builder + scaling + ConfigMap helpers |
| operator/go.mod | Operator module definition and dependencies |
| operator/go.sum | Dependency lockfile for operator module |
| operator/Dockerfile | Multi-stage build producing distroless runtime image |
| operator/cmd/main.go | Operator entrypoint, flags, manager setup, health endpoints |
| operator/.dockerignore | Docker context exclusions for faster builds |
| docs/adr/README.md | ADR index and ADR process documentation |
| docs/adr/004-operator-deployment-model.md | ADR for deploying operator as Helm subchart dependency |
| docs/adr/003-declarative-store-lifecycle-crds.md | ADR for future CRD-based store/model/tuples management |
| docs/adr/002-operator-managed-migrations.md | ADR describing migration flow moved from Helm hooks to operator |
| docs/adr/001-adopt-openfga-operator.md | ADR establishing the operator approach and staged roadmap |
| docs/adr/000-template.md | ADR template for future architectural decisions |
| charts/openfga/values.yaml | Added operator + migration values surface |
| charts/openfga/values.schema.json | Schema updates for new operator/migration values and subchart passthrough |
| charts/openfga/templates/serviceaccount.yaml | Adds optional dedicated migration ServiceAccount in operator mode |
| charts/openfga/templates/rbac.yaml | Skips legacy hook/init-container RBAC when operator is enabled |
| charts/openfga/templates/job.yaml | Skips legacy Helm hook migration Job when operator is enabled |
| charts/openfga/templates/deployment.yaml | Operator mode annotations, replicas=0 behavior, disables hook init-container path |
| charts/openfga/templates/_helpers.tpl | Helper for migration ServiceAccount naming |
| charts/openfga/Chart.yaml | Adds openfga-operator dependency gated by operator.enabled |
| charts/openfga/Chart.lock | Lockfile updated to include the new subchart dependency |
| charts/openfga-operator/values.yaml | Operator chart configuration (image, leader election, watch scope, resources) |
| charts/openfga-operator/templates/serviceaccount.yaml | Operator ServiceAccount manifest |
| charts/openfga-operator/templates/NOTES.txt | Post-install notes for operator chart |
| charts/openfga-operator/templates/deployment.yaml | Operator Deployment manifest and CLI args wiring |
| charts/openfga-operator/templates/clusterrolebinding.yaml | ClusterRoleBinding for operator permissions |
| charts/openfga-operator/templates/clusterrole.yaml | ClusterRole for operator (deployments/jobs/configmaps/etc.) |
| charts/openfga-operator/templates/_helpers.tpl | Operator chart naming/labels helpers |
| charts/openfga-operator/crds/README.md | Placeholder note for future CRDs (Stage 2+) |
| charts/openfga-operator/Chart.yaml | Operator chart metadata/versioning |
| .github/workflows/operator.yml | CI workflow for operator tests + multi-arch build/push |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (1)
operator/tests/values-no-db.yaml (1)
18-18: Avoid inline credentials in test fixture URIs.Line 18 embeds basic-auth credentials, which triggers secret scanners and normalizes plaintext credential patterns. Prefer a redacted placeholder for this failure-path fixture.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@operator/tests/values-no-db.yaml` at line 18, Replace the inline basic-auth credentials embedded in the YAML "uri" value (the failing PostgreSQL URI) with a redacted placeholder or environment-variable reference (e.g., remove "openfga:changeme@" and replace with a token like "<REDACTED_CREDENTIALS>" or use an env var reference) so the test fixture no longer contains plaintext credentials; update the "uri" key's value accordingly and ensure any test code reading this fixture can handle the placeholder or env var.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/operator.yml:
- Around line 7-11: The workflow path filters are too narrow; update the
operator workflow (operator.yml) to also trigger on changes that affect image
publishing and self-validation by adding the chart and workflow files to the
paths filter — include at minimum "charts/openfga-operator/Chart.yaml" and
".github/workflows/operator.yml" (or broader "charts/**" and
".github/workflows/**") so that functions relying on Chart.yaml-derived tags and
workflow self-changes will cause the workflow to run.
In `@charts/openfga-operator/templates/clusterrole.yaml`:
- Around line 20-25: Remove the unused Secret and ServiceAccount permissions
from the operator ClusterRole template: delete the rules that list resources
"secrets" with verb "get" and "serviceaccounts" with verbs "get","list","create"
in the charts/openfga-operator/templates/clusterrole.yaml so the ClusterRole
only grants permissions actually used by the controller (referenced by
migration_controller.go and helpers.go which do not access Secrets or create
ServiceAccounts); ensure no other ClusterRole rules depend on those resource
entries and run a quick helm template to validate the manifest.
In `@charts/openfga-operator/templates/deployment.yaml`:
- Around line 27-36: The deployment template leaves podSecurityContext and
container securityContext empty (podSecurityContext and securityContext blocks)
which allows runtime-default privileges; update the chart defaults in
values.yaml and the deployment template to enable hardened defaults: set
podSecurityContext with runAsNonRoot: true and runAsUser (non-root uid), and set
container securityContext to runAsNonRoot: true, runAsUser, seccompProfile: {
type: RuntimeDefault }, capabilities: { drop: ["ALL"] }, and
readOnlyRootFilesystem: true; ensure these keys are present and not commented
out so the operator container and pod inherit the hardened defaults (refer to
podSecurityContext and the container securityContext entries in the deployment
template and the commented hardening directives in values.yaml).
In `@charts/openfga-operator/templates/NOTES.txt`:
- Around line 3-5: Replace the hard assertion in the Helm NOTES template that
the operator image "({{ .Values.image.repository }}:{{ .Values.image.tag |
default .Chart.AppVersion }}) does not exist yet" with a neutral, conditional
message: detect if a developer flag or a value (e.g., .Values.image.isDev or by
checking if .Values.image.tag equals .Chart.AppVersion) indicates a development
build and only then warn that the image may not be pushed; otherwise emit a
neutral info line that the operator uses the specified image without asserting
non-existence. Update the template text in NOTES.txt around the referenced
symbols (.Values.image.repository, .Values.image.tag, .Chart.AppVersion) to
reflect this conditional/neutral wording.
In `@charts/openfga/templates/_helpers.tpl`:
- Around line 80-85: The helper openfga.migrationServiceAccountName currently
always returns a generated name; change it to first check
.Values.migration.serviceAccount.create and
.Values.migration.serviceAccount.name, and if create is false and name is not
provided, return an empty string (so operator-created migration Jobs won't
reference a non-existent SA). Update the logic in the
openfga.migrationServiceAccountName function to: if
.Values.migration.serviceAccount.name use that (trunc/trimSuffix), else if
.Values.migration.serviceAccount.create is false return "" immediately,
otherwise generate and return the fallback printf "%s-migration" (include
"openfga.fullname" .) with trunc/trimSuffix.
In `@charts/openfga/templates/deployment.yaml`:
- Around line 8-10: The template is honoring only .Values.operator.enabled but
must also respect .Values.migration.enabled; update the conditional logic
wherever migration-related annotations/fields are set (e.g., the blocks that set
openfga.dev/desired-replicas, openfga.dev/migration-service-account, and any
migration init-container toggles) to require both .Values.operator.enabled and
.Values.migration.enabled (e.g., change conditionals to check both flags before
emitting those annotations/fields), and apply the same fix to the other
occurrences noted (lines ~16-23 and ~49) so operator-mode branches do not inject
migration annotations or set replicas to 0 when migration.enabled is false.
In `@docs/adr/002-operator-managed-migrations.md`:
- Around line 119-125: The ADR currently states "exponential backoff" for
database connectivity retries but the controller implements a fixed retry
cadence; update the ADR text in the "Database unreachable" row to document the
actual retry policy used by the controller (replace "exponential backoff" with
the fixed 60s retry), and mention how this interacts with Job reschedules and
operator restart behavior (keep references to MigrationFailed condition,
activeDeadlineSeconds, ConfigMap re-read and Job status resume semantics to
preserve context).
In `@docs/adr/004-operator-deployment-model.md`:
- Around line 36-39: Update the ADR text and all examples to reflect that the
operator is disabled by default: change the description of the "Operator as a
conditional subchart dependency (selected)" option from "Enabled by default" to
"Disabled by default", update the examples to show how to enable it by setting
operator.enabled: true, and adjust the other occurrences that claim it is
enabled by default (mentions of openfga-operator and any numbered sections
referenced around lines 74-75 and 127-128) so they align with the actual default
in values.schema.json where operator.enabled is false.
In `@docs/adr/README.md`:
- Around line 33-39: The fenced code block in docs/adr/README.md is missing a
language label which triggers markdownlint; edit that block (the diagram block
containing "Proposed → Accepted → (optionally) Superseded or Deprecated" and the
feedback loop arrows) to add a language specifier (e.g., use "text") immediately
after the opening backticks so the block becomes a labeled fenced code block.
In `@operator/cmd/main.go`:
- Around line 40-70: Default flags leave the operator watching all namespaces;
ensure it falls back to the pod's namespace by reading the POD_NAMESPACE env var
when watchNamespace is empty and watchAllNamespaces is false, and set
cacheOpts.DefaultNamespaces accordingly before calling ctrl.NewManager. In
practice, add an os.LookupEnv("POD_NAMESPACE") check after flag.Parse() and, if
watchNamespace == "" && !watchAllNamespaces && POD_NAMESPACE is present, set
cacheOpts.DefaultNamespaces = map[string]cache.Config{podNamespace: {}}
(referencing watchNamespace, watchAllNamespaces, cacheOpts.DefaultNamespaces,
and ctrl.NewManager), and also ensure the chart/deployment injects POD_NAMESPACE
into the operator pod env.
In `@operator/go.mod`:
- Line 3: The Docker builder image is using a floating patch tag (FROM
golang:1.25) while operator/go.mod pins go 1.25.6; update the Dockerfile's base
image to the exact patch version to match go.mod by changing the FROM
golang:1.25 line to FROM golang:1.25.6 so builds are reproducible and aligned
with the declared go 1.25.6 in operator/go.mod.
In `@operator/internal/controller/helpers.go`:
- Around line 120-142: The Job pod template currently copies
NodeSelector/Tolerations/Affinity but omits imagePullSecrets and security
contexts; update the PodTemplateSpec in the migration Job (where Template:
corev1.PodTemplateSpec is built) to also set Spec.ImagePullSecrets =
deployment.Spec.Template.Spec.ImagePullSecrets, Spec.SecurityContext =
deployment.Spec.Template.Spec.SecurityContext, and copy the container-level
SecurityContext into the "migrate-database" container
(Containers[0].SecurityContext =
deployment.Spec.Template.Spec.Containers[i].SecurityContext for the matching
mainContainer.Image/name), ensuring the migration container keeps the same
Image/Args/Env while inheriting the source Deployment's imagePullSecrets and
security settings.
In `@operator/internal/controller/migration_controller.go`:
- Around line 49-55: The code incorrectly assumes the main OpenFGA container is
Containers[0]; update the logic to locate the OpenFGA container by name (e.g.,
look for container.Name == "openfga" or the configured main container name)
instead of indexing 0, retrieve its Image and env once (store the resolved core
container object or its index) and pass or reuse that reference when calling
extractImageTag and buildMigrationJob so both use the same resolved container;
update any checks (previous len/Containers[0] usages) to handle missing named
container and return/skip with a clear log if not found.
- Around line 108-121: The success branch handling job.Status.Succeeded >= 1
never clears a prior MigrationFailed condition; update the logic so that after
logging success and before returning it clears/removes the MigrationFailed
condition in the migration status ConfigMap. Specifically, in the
job.Status.Succeeded block (around updateMigrationStatus and
ensureDeploymentScaled), modify or extend updateMigrationStatus (or call a new
helper) to set the MigrationFailed condition to false/remove it for
desiredVersion/jobName so kubectl describe no longer shows a stale failure; keep
the existing replica scaling flow (ensureDeploymentScaled) and return only after
the status has been updated.
- Around line 129-149: The 60s backoff is being bypassed because deleting the
failed Job immediately requeues the owning Deployment; fix by persisting a
retry-until timestamp on the Deployment before deleting the Job and checking
that timestamp during job-creation logic. Specifically: in the failure branch
where you call setMigrationFailedCondition(deployment, desiredVersion) and
r.Status().Update(...) (and before r.Delete(...)), set an annotation or a field
on deployment.Status (e.g., "migration.retryUntil" =
time.Now().Add(60*time.Second).UTC().Format(time.RFC3339)) and persist it
(handle update conflicts); then in the reconcile path that decides to create a
new migration Job (the code that currently creates the Job when IsNotFound),
read and parse that "migration.retryUntil" value and skip Job creation until the
timestamp has passed (clear the annotation/status field when creating the Job).
This ensures the controller honors the cooldown even though the owned Job watch
will requeue the Deployment.
In `@operator/Makefile`:
- Around line 5-6: The build target fails when bin/ is missing; update the
Makefile's build target (the "build:" target) to ensure the output directory
exists by creating bin before running go build (use mkdir -p bin) so that go
build -o bin/operator ./cmd/ never errors on fresh clones or after make clean.
In `@operator/README.md`:
- Around line 9-15: Update the README text that currently references the
hard-coded ConfigMap name "openfga-migration-status" to use the
deployment-scoped ConfigMap name pattern used elsewhere in the PR (e.g.,
"<release>-migration-status"); specifically, edit the line describing version
comparison so it no longer mentions "openfga-migration-status" but instead
references the deployment-scoped ConfigMap name pattern
("<release>-migration-status" or equivalent placeholder) so docs and tests point
to the correct resource.
---
Nitpick comments:
In `@operator/tests/values-no-db.yaml`:
- Line 18: Replace the inline basic-auth credentials embedded in the YAML "uri"
value (the failing PostgreSQL URI) with a redacted placeholder or
environment-variable reference (e.g., remove "openfga:changeme@" and replace
with a token like "<REDACTED_CREDENTIALS>" or use an env var reference) so the
test fixture no longer contains plaintext credentials; update the "uri" key's
value accordingly and ensure any test code reading this fixture can handle the
placeholder or env var.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8f9a57a5-18a8-4395-92d9-b4eae0a8affc
⛔ Files ignored due to path filters (2)
charts/openfga/Chart.lockis excluded by!**/*.lockoperator/go.sumis excluded by!**/*.sum
📒 Files selected for processing (37)
.github/workflows/operator.ymlcharts/openfga-operator/Chart.yamlcharts/openfga-operator/crds/README.mdcharts/openfga-operator/templates/NOTES.txtcharts/openfga-operator/templates/_helpers.tplcharts/openfga-operator/templates/clusterrole.yamlcharts/openfga-operator/templates/clusterrolebinding.yamlcharts/openfga-operator/templates/deployment.yamlcharts/openfga-operator/templates/serviceaccount.yamlcharts/openfga-operator/values.yamlcharts/openfga/Chart.yamlcharts/openfga/templates/_helpers.tplcharts/openfga/templates/deployment.yamlcharts/openfga/templates/job.yamlcharts/openfga/templates/rbac.yamlcharts/openfga/templates/serviceaccount.yamlcharts/openfga/values.schema.jsoncharts/openfga/values.yamldocs/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.mdoperator/.dockerignoreoperator/Dockerfileoperator/Makefileoperator/README.mdoperator/cmd/main.gooperator/go.modoperator/internal/controller/helpers.gooperator/internal/controller/migration_controller.gooperator/internal/controller/migration_controller_test.gooperator/tests/README.mdoperator/tests/values-db-outage.yamloperator/tests/values-happy-path.yamloperator/tests/values-no-db.yaml
emilic
marked this pull request as draft
April 10, 2026 17:45
emilic
added a commit
that referenced
this pull request
Apr 11, 2026
- Harden pod security (runAsNonRoot, seccompProfile, drop ALL caps) - Find container by name instead of index to handle sidecars - Skip migration for memory datastore - Persist retry-after annotation before Job deletion to survive re-enqueue - Clear MigrationFailed condition on success - Propagate imagePullSecrets and securityContext to migration Jobs - Remove unused RBAC rules (secrets, serviceaccounts) - Add POD_NAMESPACE downward API for namespace-scoped watch default - Remove no-op migration values (timeout, backoffLimit, resources) - Fix migration SA helper to require name when create=false - Guard operator logic on both operator.enabled and migration.enabled - Build and load operator image into kind for chart-testing CI - Add path filters to operator workflow - Fix ADR inaccuracies (retry strategy, default-enabled wording) - Pin Dockerfile base image to golang:1.26.2
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
emilic
added a commit
that referenced
this pull request
Apr 11, 2026
- Harden pod security (runAsNonRoot, seccompProfile, drop ALL caps) - Find container by name instead of index to handle sidecars - Skip migration for memory datastore - Persist retry-after annotation before Job deletion to survive re-enqueue - Clear MigrationFailed condition on success - Propagate imagePullSecrets and securityContext to migration Jobs - Remove unused RBAC rules (secrets, serviceaccounts) - Add POD_NAMESPACE downward API for namespace-scoped watch default - Remove no-op migration values (timeout, backoffLimit, resources) - Fix migration SA helper to require name when create=false - Guard operator logic on both operator.enabled and migration.enabled - Build and load operator image into kind for chart-testing CI - Add path filters to operator workflow - Fix ADR inaccuracies (retry strategy, default-enabled wording) - Pin Dockerfile base image to golang:1.26.2
emilic
force-pushed
the
feat/operator-migration
branch
from
April 11, 2026 12:37
24897df to
f149c60
Compare
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Contributor
Author
|
@copilot review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 38 out of 40 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
charts/openfga/templates/deployment.yaml:65
- When
operator.enabled=truebutmigration.enabled=false, the chart disables the hook-based migration Job (templates/job.yaml) and RBAC (templates/rbac.yaml), but the Deployment can still render the legacywait-for-migrationinitContainer because$operatorMigration := and operator.enabled migration.enabledevaluates to false. With the defaultdatastore.migrationType: job+waitForMigrations: true, this will deadlock (initContainer waits for a Job that the chart no longer creates). Consider changing the initContainer gating to disable legacy migration orchestration wheneveroperator.enabledis true (or explicitly fail fast for the unsupported combination).
{{- $operatorMigration := and .Values.operator.enabled .Values.migration.enabled }}
{{ if or (and (not $operatorMigration) (or (and (has .Values.datastore.engine (list "postgres" "mysql")) .Values.datastore.applyMigrations .Values.datastore.waitForMigrations))) .Values.extraInitContainers }}
initContainers:
{{- if not $operatorMigration }}
{{- if and (has .Values.datastore.engine (list "postgres" "mysql")) .Values.datastore.applyMigrations .Values.datastore.waitForMigrations (eq .Values.datastore.migrationType "job") }}
- name: wait-for-migration
securityContext:
{{- toYaml .Values.securityContext | nindent 12 }}
image: "{{ .Values.initContainer.repository }}:{{ .Values.initContainer.tag }}"
imagePullPolicy: {{ .Values.initContainer.pullPolicy }}
args: ["job-wr", '{{ include "openfga.fullname" . }}-migrate']
resources:
{{- toYaml .Values.datastore.migrations.resources | nindent 12 }}
{{- end }}
{{- if and (has .Values.datastore.engine (list "postgres" "mysql")) (eq .Values.datastore.migrationType "initContainer") }}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
🧹 Nitpick comments (7)
operator/README.md (1)
90-102: Add language specifier to the project structure block.The directory tree is missing a language identifier. Add
textafter the opening backticks to resolve the markdownlint warning.📝 Proposed fix
-``` +```text operator/ ├── cmd/ │ └── main.go # Entry point, manager setup🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@operator/README.md` around lines 90 - 102, Update the fenced code block in operator/README.md that contains the project tree (the block starting with "operator/" and listing cmd/, internal/, Dockerfile, etc.) to include a language specifier by changing the opening ``` to ```text so the block becomes a text-formatted code fence; this will resolve the markdownlint warning without altering the block contents.docs/adr/004-operator-deployment-model.md (1)
69-90: Add language specifier to this fenced code block.The directory structure block is missing a language identifier, which triggers the markdownlint warning. Add
textafter the opening backticks.📝 Proposed fix
-``` +```text helm-charts/ ├── charts/ │ ├── openfga/ # Main chart (existing)🤖 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, The fenced code block showing the Helm chart directory structure lacks a language specifier; update that block by adding "text" immediately after the opening triple backticks (i.e., change ``` to ```text) so the directory tree is treated as plain text and markdownlint warning is resolved (refer to the directory structure block containing "helm-charts/" and the nested "openfga" / "openfga-operator" entries).operator/cmd/main.go (1)
31-43: Consider validating that Job configuration flags are non-negative.The flags
--backoff-limit,--active-deadline-seconds, and--ttl-seconds-after-finishedare parsed asintbut later cast toint32/int64. Negative values would produce unexpected behavior. The Kubernetes API would reject invalid values, but an early validation with a clear error message would improve the operator experience.🛡️ Suggested validation after flag.Parse()
flag.Parse() + if backoffLimit < 0 || activeDeadline < 0 || ttlAfterFinished < 0 { + fmt.Fprintln(os.Stderr, "error: job configuration flags must be non-negative") + os.Exit(1) + } + ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts)))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@operator/cmd/main.go` around lines 31 - 43, After parsing flags, validate the three Job-related flag variables backoffLimit, activeDeadline, and ttlAfterFinished are non-negative; in main.go (after flag.Parse()) add checks that return/log a clear fatal error if any value < 0 (e.g., using log.Fatalf or klog.Fatalf) so the operator exits with a descriptive message instead of proceeding to cast to int32/int64 and creating invalid Kubernetes Job specs; reference the flag variables backoffLimit, activeDeadline, ttlAfterFinished and the controller.Default* constants when adding the validation.operator/internal/controller/migration_controller.go (1)
79-90: Status update error handling could be improved.When clearing the
MigrationFailedcondition, errors fromr.Status().Update()are logged but execution continues. If this update fails due to a conflict, the condition remains stale. Consider returning a requeue to ensure the condition is eventually cleared.♻️ Suggested improvement
if currentVersion == desiredVersion { logger.V(1).Info("migration up to date", "version", desiredVersion) clearMigrationFailedCondition(deployment) if patchErr := r.Status().Update(ctx, deployment); patchErr != nil { - logger.Error(patchErr, "failed to clear MigrationFailed condition") + if apierrors.IsConflict(patchErr) { + return ctrl.Result{Requeue: true}, nil + } + logger.Error(patchErr, "failed to clear MigrationFailed condition") } if _, scaleErr := ensureDeploymentScaled(ctx, r.Client, deployment); scaleErr != nil {Apply the same pattern at lines 146-148.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@operator/internal/controller/migration_controller.go` around lines 79 - 90, The status update that clears MigrationFailed via clearMigrationFailedCondition(deployment) calls r.Status().Update(ctx, deployment) but only logs errors and proceeds; change this so that if r.Status().Update returns an error (e.g., a conflict) you return that error (or return ctrl.Result{Requeue: true} with the error) instead of continuing, mirroring the pattern used at lines 146-148; locate the block that checks currentVersion == desiredVersion, handle patchErr from r.Status().Update(ctx, deployment) by returning the error or a requeue result so the condition will be retried before calling ensureDeploymentScaled and returning success.operator/internal/controller/migration_controller_test.go (1)
166-231: Test lacks verification of MigrationFailed condition clearing.The
TestReconcile_JobSucceeded_UpdatesConfigMapAndScalesUptest verifies ConfigMap creation and Deployment scaling, but doesn't verify that a priorMigrationFailedcondition gets cleared. Consider adding a test case where the Deployment starts with aMigrationFailedcondition and verifying it's cleared after success.🧪 Suggested additional test case
func TestReconcile_JobSucceeded_ClearsMigrationFailedCondition(t *testing.T) { // Given: a Deployment with a MigrationFailed condition and a succeeded Job. dep := newTestDeployment("openfga", "default", "openfga/openfga:v1.14.0", 0) dep.Annotations[AnnotationDesiredReplicas] = "3" dep.Status.Conditions = []appsv1.DeploymentCondition{ { Type: "MigrationFailed", Status: corev1.ConditionTrue, Reason: "MigrationJobFailed", }, } job := &batchv1.Job{ // ... same as existing test ... Status: batchv1.JobStatus{Succeeded: 1}, } r := newReconciler(dep, job) _, err := r.Reconcile(context.Background(), ctrl.Request{ NamespacedName: types.NamespacedName{Name: "openfga", Namespace: "default"}, }) if err != nil { t.Fatalf("unexpected error: %v", err) } // Verify MigrationFailed condition is cleared. updated := &appsv1.Deployment{} _ = r.Get(context.Background(), types.NamespacedName{Name: "openfga", Namespace: "default"}, updated) for _, c := range updated.Status.Conditions { if c.Type == "MigrationFailed" && c.Status == corev1.ConditionTrue { t.Error("expected MigrationFailed condition to be cleared") } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@operator/internal/controller/migration_controller_test.go` around lines 166 - 231, Add a test that ensures a pre-existing MigrationFailed condition on the Deployment is removed after a successful Job: create a Deployment via newTestDeployment, set dep.Annotations[AnnotationDesiredReplicas] and seed dep.Status.Conditions with a DeploymentCondition having Type "MigrationFailed" and Status corev1.ConditionTrue, create a succeeded batchv1.Job as in the existing TestReconcile_JobSucceeded_UpdatesConfigMapAndScalesUp, call newReconciler(...).Reconcile(...), then Get the updated Deployment and assert that no Status.Condition remains with Type "MigrationFailed" && Status corev1.ConditionTrue; name the new test TestReconcile_JobSucceeded_ClearsMigrationFailedCondition and reuse the same Job/Reconiler setup symbols.operator/internal/controller/helpers.go (1)
258-285: Scaling logic has a subtle edge case when replicas annotation already exists.If the
AnnotationDesiredReplicasannotation already exists (e.g., from a previous scale-down), the function correctly avoids overwriting it (line 272). However, if the annotation contains "0" from a previous interrupted reconciliation where the Deployment was manually set to 0 replicas before the operator took over,ensureDeploymentScaledwould restore to 0 replicas.This is documented behavior per the ADR ("Deployment starts at replicas: 0"), but consider logging a warning when the stored desired replicas is 0 to aid debugging.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@operator/internal/controller/helpers.go` around lines 258 - 285, In scaleDeploymentToZero, detect when deployment.Annotations[AnnotationDesiredReplicas] already exists and equals "0" and emit a warning with the Deployment identity to aid debugging; update the function (around the annotation-check block) to check the stored value string == "0" and call your controller logger (e.g., klog.Warningf or ctrl.Log.WithValues("deployment", deployment.Name, "namespace", deployment.Namespace).V(0).Info/Warning) to record that the saved desired replicas is zero before returning/continuing, leaving the existing behavior of not overwriting the annotation intact.docs/adr/002-operator-managed-migrations.md (1)
76-94: Consider adding language specifiers to code fences.The ASCII diagrams and sequence blocks lack language specifiers. Adding
textorplaintextas the language would satisfy markdown linters and improve rendering consistency across platforms.📝 Suggested fix
-``` +```text ┌────────────────────────────────────────────────────────┐ │ Operator Reconciliation │Apply similarly to the code fences at lines 130 and 147.
🤖 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 ASCII diagrams and sequence blocks (for example the block starting with "Operator Reconciliation" and the other two visual blocks later in the file) are missing Markdown language specifiers; update each triple-backtick fence that contains ASCII art/sequence steps to use a plain text specifier (e.g., ```text or ```plaintext) so markdown linters/renderers handle them consistently.
🤖 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 ASCII diagrams and sequence blocks (for example the
block starting with "Operator Reconciliation" and the other two visual blocks
later in the file) are missing Markdown language specifiers; update each
triple-backtick fence that contains ASCII art/sequence steps to use a plain text
specifier (e.g., ```text or ```plaintext) so markdown linters/renderers handle
them consistently.
In `@docs/adr/004-operator-deployment-model.md`:
- Around line 69-90: The fenced code block showing the Helm chart directory
structure lacks a language specifier; update that block by adding "text"
immediately after the opening triple backticks (i.e., change ``` to ```text) so
the directory tree is treated as plain text and markdownlint warning is resolved
(refer to the directory structure block containing "helm-charts/" and the nested
"openfga" / "openfga-operator" entries).
In `@operator/cmd/main.go`:
- Around line 31-43: After parsing flags, validate the three Job-related flag
variables backoffLimit, activeDeadline, and ttlAfterFinished are non-negative;
in main.go (after flag.Parse()) add checks that return/log a clear fatal error
if any value < 0 (e.g., using log.Fatalf or klog.Fatalf) so the operator exits
with a descriptive message instead of proceeding to cast to int32/int64 and
creating invalid Kubernetes Job specs; reference the flag variables
backoffLimit, activeDeadline, ttlAfterFinished and the controller.Default*
constants when adding the validation.
In `@operator/internal/controller/helpers.go`:
- Around line 258-285: In scaleDeploymentToZero, detect when
deployment.Annotations[AnnotationDesiredReplicas] already exists and equals "0"
and emit a warning with the Deployment identity to aid debugging; update the
function (around the annotation-check block) to check the stored value string ==
"0" and call your controller logger (e.g., klog.Warningf or
ctrl.Log.WithValues("deployment", deployment.Name, "namespace",
deployment.Namespace).V(0).Info/Warning) to record that the saved desired
replicas is zero before returning/continuing, leaving the existing behavior of
not overwriting the annotation intact.
In `@operator/internal/controller/migration_controller_test.go`:
- Around line 166-231: Add a test that ensures a pre-existing MigrationFailed
condition on the Deployment is removed after a successful Job: create a
Deployment via newTestDeployment, set dep.Annotations[AnnotationDesiredReplicas]
and seed dep.Status.Conditions with a DeploymentCondition having Type
"MigrationFailed" and Status corev1.ConditionTrue, create a succeeded
batchv1.Job as in the existing
TestReconcile_JobSucceeded_UpdatesConfigMapAndScalesUp, call
newReconciler(...).Reconcile(...), then Get the updated Deployment and assert
that no Status.Condition remains with Type "MigrationFailed" && Status
corev1.ConditionTrue; name the new test
TestReconcile_JobSucceeded_ClearsMigrationFailedCondition and reuse the same
Job/Reconiler setup symbols.
In `@operator/internal/controller/migration_controller.go`:
- Around line 79-90: The status update that clears MigrationFailed via
clearMigrationFailedCondition(deployment) calls r.Status().Update(ctx,
deployment) but only logs errors and proceeds; change this so that if
r.Status().Update returns an error (e.g., a conflict) you return that error (or
return ctrl.Result{Requeue: true} with the error) instead of continuing,
mirroring the pattern used at lines 146-148; locate the block that checks
currentVersion == desiredVersion, handle patchErr from r.Status().Update(ctx,
deployment) by returning the error or a requeue result so the condition will be
retried before calling ensureDeploymentScaled and returning success.
In `@operator/README.md`:
- Around line 90-102: Update the fenced code block in operator/README.md that
contains the project tree (the block starting with "operator/" and listing cmd/,
internal/, Dockerfile, etc.) to include a language specifier by changing the
opening ``` to ```text so the block becomes a text-formatted code fence; this
will resolve the markdownlint warning without altering the block contents.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5c414505-2633-4683-95be-cd73ce829f78
⛔ Files ignored due to path filters (2)
charts/openfga/Chart.lockis excluded by!**/*.lockoperator/go.sumis excluded by!**/*.sum
📒 Files selected for processing (38)
.github/workflows/operator.yml.github/workflows/test.ymlcharts/openfga-operator/Chart.yamlcharts/openfga-operator/crds/README.mdcharts/openfga-operator/templates/NOTES.txtcharts/openfga-operator/templates/_helpers.tplcharts/openfga-operator/templates/clusterrole.yamlcharts/openfga-operator/templates/clusterrolebinding.yamlcharts/openfga-operator/templates/deployment.yamlcharts/openfga-operator/templates/serviceaccount.yamlcharts/openfga-operator/values.yamlcharts/openfga/Chart.yamlcharts/openfga/templates/_helpers.tplcharts/openfga/templates/deployment.yamlcharts/openfga/templates/job.yamlcharts/openfga/templates/rbac.yamlcharts/openfga/templates/serviceaccount.yamlcharts/openfga/values.schema.jsoncharts/openfga/values.yamldocs/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.mdoperator/.dockerignoreoperator/Dockerfileoperator/Makefileoperator/README.mdoperator/cmd/main.gooperator/go.modoperator/internal/controller/helpers.gooperator/internal/controller/migration_controller.gooperator/internal/controller/migration_controller_test.gooperator/tests/README.mdoperator/tests/values-db-outage.yamloperator/tests/values-happy-path.yamloperator/tests/values-no-db.yaml
✅ Files skipped from review due to trivial changes (11)
- operator/.dockerignore
- charts/openfga-operator/crds/README.md
- charts/openfga/Chart.yaml
- operator/go.mod
- operator/Dockerfile
- charts/openfga-operator/templates/NOTES.txt
- docs/adr/001-adopt-openfga-operator.md
- charts/openfga-operator/Chart.yaml
- charts/openfga-operator/templates/_helpers.tpl
- charts/openfga/templates/_helpers.tpl
- charts/openfga-operator/values.yaml
🚧 Files skipped from review as they are similar to previous changes (4)
- charts/openfga/templates/serviceaccount.yaml
- charts/openfga/values.schema.json
- .github/workflows/operator.yml
- charts/openfga/templates/deployment.yaml
govulncheck reported 12 vulnerabilities reachable from the operator: standard library issues in net/http, net/url, crypto/tls, crypto/x509 and encoding/asn1 fixed by Go 1.26.6, plus golang.org/x/net (fixed in v0.55.0) and golang.org/x/text (fixed in v0.39.0). It reports none after this change.
Pin the actions in operator.yml to commit SHAs like the other workflows (#344), reusing the checkout and login-action versions already pinned there. Add gomod and docker entries for operator/ so the module and base images get updates.
The operator scaled the Deployment to an openfga.dev/desired-replicas annotation and the chart omitted spec.replicas, which broke in practice: - Switching an existing release to operator mode removed spec.replicas, so Helm's three-way merge and server-side apply both reset the Deployment to one replica until the migration finished. - kubectl scale was reverted immediately and HPAs could not be used. - The readiness gate it relied on only holds pods back below schema revision 4, so on upgrades new pods already served before the migration ran. The chart now renders replicas as in legacy mode and the operator only runs migration Jobs. Along with that: - Trust a migration Job only by its openfga.dev/desired-version annotation. The label fallback accepted the legacy Helm hook Job, whose version label is the chart appVersion, and skipped migration 006 on an upgrade from v1.9.5. - Keep a failed Job for 60s and then replace it, instead of tracking the delay in a retry-after annotation on the Deployment. - Leave activeDeadlineSeconds unset by default, as the hook Job does, so long index builds and table rebuilds are not killed halfway. A Job whose pod cannot start is rebuilt instead once the Deployment's pod template changes, tracked through an openfga.dev/pod-template-hash annotation. - Only opt the Deployment in, and create the migration service account, for Postgres and MySQL, which lets the operator drop its memory engine check.
github.com/openfga/openfga-operator does not exist; the module is at github.com/openfga/helm-charts/operator, so make the path match.
Read the event name and ref from the environment instead of inlining expressions in the script, and drop an unused loop variable.
…ion.enabled The operator was spread over three top-level values: operator.enabled, openfga-operator (subchart values) and migration.enabled. Use openfga-operator.enabled as the dependency condition, which is Helm's convention and how this chart already toggles postgresql and mysql, so the operator's toggle and configuration live under one key. migration.enabled duplicated datastore.applyMigrations: both meant "do not run migrations from this release", and the helper required both. Keep only applyMigrations. migration.serviceAccount stays, since the parent chart creates that service account.
Add a section on operator-run migrations to the openfga chart README and list the openfga-operator chart in the repository README, since chart-releaser publishes it. Note that the migration pod does not carry the OpenFGA pod labels, which matters for NetworkPolicies. Make the ADR index match ADR-001's status, drop the claim that the operator was scaffolded with kubebuilder, and remove the empty crds/ placeholder directory from the operator chart.
- Run the kind, ct install and E2E steps when operator/ changes, not only when a chart changes; operator code was otherwise never tested against a cluster on its own. - Fail operator PRs that change the image inputs without bumping the operator chart's appVersion and version, since CI publishes each version tag once. Document the full release chain in the operator README. - Upgrade to the chart's appVersion in the E2E instead of a pinned v1.14.1, so every OpenFGA version bump exercises migrating to it. - Run the operator's unit tests with -race.
A version change replaced the migration Job at once, even with its pod running. Aborting a non-transactional step such as the concurrent index build in Postgres migration 006 leaves an invalid index, and the rerun's IF NOT EXISTS then skips it while goose records the version as applied. Wait for a running Job to finish and replace it afterwards, as already done for pod template changes.
Track the complete migration Job identity, preserve migration-specific pod configuration, and prevent unsafe resource replacement. Harden retry cleanup and require parent chart release version propagation.
…e datastore too Review findings on the migration Job: - Sidecars and init containers: the Job now runs the Deployment's other containers as native sidecars (restartPolicy: Always, so they stop when migrate exits) and copies its init containers, so a database proxy on localhost works. Needs Kubernetes 1.29 when sidecars are present. - migrate.labels and migrate.annotations are forwarded through the openfga.dev/migration-labels and -annotations Deployment annotations, with helm.sh/* keys dropped, so Istio/Linkerd injection can be disabled for the migration pod. The operator's identity labels and openfga.dev/* annotations cannot be overridden. - The migration identity is the image tag plus an openfga.dev/migration-trigger annotation the chart derives from the datastore settings and the new migration.trigger value, so pointing the release at another database with the same image runs the migration, and a rotated Secret can be handled declaratively instead of by deleting the status ConfigMap. - Only Jobs the operator created or the legacy hook Job are replaced; a same-name Job from elsewhere is left alone and reported with a MigrationJobConflict event. A status ConfigMap not owned by the Deployment is not trusted. Deletion uses foreground propagation with UID and resourceVersion preconditions so two migrations never overlap. - ttlSecondsAfterFinished is applied after success only. Set at creation, a TTL below the retry delay removed failed Jobs before the retry, and a TTL of 0 could remove a completed Job before its outcome was recorded. - Events on the Deployment for started, succeeded, failed and conflicting migrations. The release guard moves to .github/scripts/check-operator-release.sh and also requires the openfga chart version and its operator dependency to move.
This comment was marked as resolved.
This comment was marked as resolved.
…tity Three regressions from ca25836 and 3bc8ab5, each reproduced on a cluster: - A Job whose pod exists but cannot start (CreateContainerConfigError, ImagePullBackOff, unschedulable) counted as started, so it was never rebuilt after the Deployment was fixed. Only a Ready or already succeeded pod counts as started. - The datastore URI was dropped from the migration trigger, so pointing a release at another database with the same image ran no migration and left the new pods NotReady. The URI is back in the trigger with the credentials removed, so a password change does not count and no secret goes into the hash. - The pod template hash was part of the recorded identity, so every pod change such as a log level ran a migration. The identity is the image and the trigger again; the hash only decides whether a Job that has not started is rebuilt.
Member
|
Three regressions in the latest pushes, fixed in a7e467c:
|
release.yml waited up to 20 minutes for ghcr.io/openfga/openfga-operator:<appVersion> before running chart-releaser. If the operator build failed on main, every chart in that push stayed unreleased until another merge, since workflow_dispatch never publishes the :<appVersion> tag. The chart already treats the openfga image this way: no registry check, the PR E2E installs it. test.yml builds and loads the operator image into kind, so a PR is proven installable before merge; the remaining window is the few minutes between chart publish and image push on the same commit, which kubelet retries through on its own. release.yml is back to its state on main.
…app service account by default The Helm hook Job ran as the OpenFGA service account. Creating a separate <release>-migration service account by default meant a release on IRSA or Workload Identity lost its IAM role on the migration Job when switching to the operator. The operator already falls back to the pod's service account when the annotation is absent, so migration.serviceAccount.create now defaults to false and the dedicated account is opt-in. rbac.create gates the operator's Role and RoleBinding, as the Helm RBAC guidelines recommend. The operator chart notes no longer warn about an unpublished image.
…ty scope The image build now attaches an SBOM and build provenance and signs the digest with cosign keyless, then verifies the signature in the same job, the way openfga/openfga releases do. controller-runtime served /metrics on 8080 to any pod in the cluster with no declared port and no authentication. The chart now passes --metrics-bind-address=0 unless metrics.enabled is set, which also declares a named container port. The operator README gets a Security section: namespace scope, the Role's verbs, ports, how to verify the image signature and where to report issues. The operator chart declares the Artifact Hub operator, capability and signing key annotations that the openfga chart already carries.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
What problem is being solved?
The OpenFGA Helm chart uses Helm hooks (
post-install,post-upgrade) and ak8s-wait-forinit container to orchestrate database migrations. This approach breaks in multiple ways:helm install --waitdeadlocks — the init container waits for the hook Job, but the hook only runs after the Deployment is ready ("hen egg" problem when used with helm deploy #120)hook-delete-policyconflict — the Job is cleaned up before FluxCD can confirm the Deployment is healthy (Hooked job deletion incompatible with Helm--waitand FluxCD by default #100)k8s-wait-foris unmaintained — 2+ years stale, known CVEs, pinned by tag not digest (Reuse k8s-wait-for image repo and tag across migrate job and initContainer #126,k8s-wait-forimage is inactively maintained and contains many vulnerabilities #132, An external dependency to groundnuty/k8s-wait-for is pinned using tag. #144)These represent the single biggest pain point for users of the chart.
How is it being solved?
A lightweight Kubernetes operator moves migration orchestration from deploy-time Helm hooks into the runtime control plane. The operator watches OpenFGA Deployments, detects version changes, and runs migrations as regular Kubernetes Jobs — no hooks, no init containers.
This is Stage 1 of the operator, focused solely on migration orchestration. It does not introduce any CRDs and never changes the Deployment's replica count or pod template. The operator scopes its watch to Deployments matching the labels
app.kubernetes.io/part-of: openfgaandapp.kubernetes.io/component: authorization-controller, and only reconciles those that opt in via theopenfga.dev/migration-enabled: "true"annotation emitted by the chart for Postgres and MySQL.What changes are made to solve it?
New: Go operator (
operator/)openfga.dev/migration-triggerannotation the chart derives from the datastore connection settings (engine, host and database, Secret names and keys; never credentials). When it differs from the{name}-migration-statusConfigMap the operator creates a Job runningopenfga migrate, waits for completion via Job conditions, and records the identity. So a new image, or the same image pointed at another database, both migrate; a password change or a pod template change does notrestartPolicy: Always, so a database proxy is available and stops when the migration exits); the init containers.migrate.*values (labels, non-hook annotations such assidecar.istio.io/inject: "false", extra volumes and mounts, init containers, sidecars, timeout) anddatastore.migrations.resourcesare forwarded through Deployment annotations and applied on top. Least privilege is enforced at the RBAC layer via a dedicated migration ServiceAccount, not by filtering env varsNotReadyuntil the schema reachesMinimumSupportedDatastoreSchemaRevision(4 since v1.3.x). On upgrades the existing schema already meets it, so new pods serve on the previous schema while the Job applies the newer migrations, the same as the Helm hook flow. See ADR-002MigrationJobConflictevent until it is removedMigrationFailedcondition on the Deployment, keeps the failed Job for 60s so its logs can be read, then replaces it. The delay is measured from the Job's own condition, so it survives operator restarts, and the operator self-heals when the database comes back.ttlSecondsAfterFinishedis applied only after success, so failed Jobs are never garbage-collected before the retry and a completed Job is never removed before its outcome is recordedMigrationStarted,MigrationSucceeded,MigrationFailed,MigrationJobConflictopenfga.dev/container-nameannotation (returns an error for misconfigured Deployments instead of silently picking the first container)backoffLimit,activeDeadlineSeconds,ttlSecondsAfterFinished) are configurable via Helm values and validated at operator startup. There is no deadline by default, like the hook Job, so long index builds and MySQL table rebuilds are not cut off halfwayNew: Operator Helm subchart (
charts/openfga-operator/)allowPrivilegeEscalation: false, read-only root FS, non-root, dropped caps)values.schema.jsonvalidation,.helmignoreserviceAccount.create: falserequires an explicitserviceAccount.name(prevents silently binding operator RBAC todefault)values.yaml: leader election, watch namespace, namespace override, resource limits, Job backoff/deadline/TTL, PDB,rbac.create,metrics.enabled(controller-runtime metrics are off unless enabled, since the endpoint is unauthenticated)operator,operatorCapabilitiesandsignKeyannotations, same GPG key as the openfga chartModified: Parent chart (
charts/openfga/)openfga-operator.enabled(defaults tofalse), following the same convention aspostgresql.enabledandmysql.enabled, so its toggle and configuration live under one key. With it disabled the rendered output is identical tomaindatastore.applyMigrations: true:openfga.dev/migration-enabled,openfga.dev/container-name,openfga.dev/migration-triggerannotations, optionallyopenfga.dev/migration-service-account, and theopenfga.dev/migration-*annotations carrying themigrate.*valuesspec.replicasis rendered as before (omitted whenautoscaling.enabled), sokubectl scale, HPAs, and GitOps tools behave the same with or without the operatormigration.serviceAccount.creategives it a dedicated one for cloud IAM annotationsmigration.trigger: set to any new value to run the migration again when neither the image nor the datastore settings changed, e.g. after rotating a Secret under the same name to point at another databasevalues.schema.jsonupdated for theopenfga-operator.enabled,migration.trigger,migration.serviceAccountandmigrate.labelspropertiesCI
.github/workflows/operator.yml: Go tests with-race, gofmt, and vet; multi-platform (amd64 + arm64) image build for PRs touching the operator, pushed toghcr.io/{owner}/openfga-operatorfrom main, the:<appVersion>tag published once per version. Pushed images carry an SBOM and build provenance and are signed with cosign keyless, verified in the same job; the operator README has thecosign verifycommand. Actions are pinned to commit SHAs.github/scripts/check-operator-release.sh: fails a PR that changes the operator image or chart without bumping the operator chart'sappVersion/version, the openfga chart version, its dependency pin andChart.lock, so every change is actually released. Comes with a test matrix (check-operator-release_test.sh)release.ymlis unchanged: chart publishing does not depend on the operator image build, same as for theopenfga/openfgaimagetest.yml: helm-unittest for the operator chart, the operator image loaded into kind, chart-testing, and an operator + Postgres E2E that installs v1.9.5 and upgrades to the chart's appVersion across the v1.10.0 migration. These also run for operator-only changesNew: Integration test values (
operator/tests/)New: Architecture Decision Records (
docs/adr/)How each issue is resolved:
--waitdeadlock)helm install --waitreturns once the operator has migrated and the pods are ready; upgrades roll out as usualhook-delete-policy— operator manages Job lifecycle with TTL and explicit cleanupk8s-wait-for)Testing
Verified on kind (Kubernetes 1.37) with Postgres 17 and MySQL 8.4, using a small probe app that seeds a store (groups, nested folders, conditions, wildcard access, ~4k tuples), checks 62 expected authorization results, and keeps ~5k requests/s running during upgrades:
helm install --waiton Helm 3.12.1, 3.22, and 4.3helm rollbackto an older version, database outage with retries (including an operator restart), operator down during a migration, leader election failover, two releases in one namespace, operator in a separate namespace, memory engine, HPA, deleting the status ConfigMap, GitOps-style server-side apply with no drifthelm upgradefixed the key; a database proxy sidecar (socat) with the URI on localhost;ttlSecondsAfterFinished: 0; a foreign same-name Job left untouched with aMigrationJobConflictevent; repointingdatastore.uriat an empty database ran the migration, a password-only change and alog.levelchange did nottest.ymloperator E2E step replayed locally with Helm 3.12.1References
--waitand FluxCD by default #100k8s-wait-forimage is inactively maintained and contains many vulnerabilities #132, An external dependency to groundnuty/k8s-wait-for is pinned using tag. #144Review Checklist
main