Feat/migrate fga to seco

[simontesar]

• Configures existing initializer to act for LogicalClusters belonging to Accounts of type org only and create FGA tuples via its Store
• Introduces an separate initalizer those of non-org Accounts to create tuples via its Store
• Exposes tuples to be managed via an importable package to be used for finalization in the account-operator

Once this PR is merged, platform-mesh/account-operator#141 in the account-operator needs to be updated to import the new security-operator version.

[nexus49]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[coderabbitai]

Walkthrough

The pull request introduces multicluster support and refactors reconciliation logic for logical clusters. The single LogicalClusterReconciler is split into OrgLogicalClusterReconciler (for organization-type accounts) and AccountLogicalClusterReconciler (for other account types), each with distinct processing workflows. New client factories enable querying resources across clusters via virtual workspaces. FGA tuple generation logic is added to support fine-grained authorization for accounts and organizations. Configuration extends to include FGA-related fields (ObjectType, ParentRelation, CreatorRelation). A new predicate filters logical clusters by account type. Subroutines are enhanced to handle multicluster operations and FGA Store management.

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 10

🤖 Fix all issues with AI agents

In `@cmd/initializer.go`:
- Around line 106-110: In initializerCmd.RunE the call to getKubeconfigFromPath
uses the wrong config variable (operatorCfg.KCP.Kubeconfig); replace this with
initializerCfg.KCP.Kubeconfig so the initializer command reads its own
configuration. Locate the block calling getKubeconfigFromPath and update the
argument to initializerCfg.KCP.Kubeconfig, keeping the existing error handling
(log.Error().Err(err).Msg(...) and return err) and leaving getKubeconfigFromPath
unchanged.

In `@internal/client/all_platformmesh.go`:
- Around line 37-40: The code accesses
apiExportEndpointSlice.Status.APIExportEndpoints[0] without checking length;
update the logic in the function that parses the virtual workspace URL (where
virtualWorkspaceUrl is assigned) to first verify
len(apiExportEndpointSlice.Status.APIExportEndpoints) > 0 and return a clear
error (or handle the empty case) if it's zero, before calling url.Parse on
APIExportEndpoints[0].URL; reference apiExportEndpointSlice, APIExportEndpoints,
virtualWorkspaceUrl and ensure the error path uses fmt.Errorf with a descriptive
message so callers can detect the absent endpoints.

In `@internal/controller/accountlogicalcluster_controller.go`:
- Line 23: Fix the typo in the package comment for
AccountLogicalClusterReconciler: change the word "intializer" to "initializer"
in the comment that currently reads "AccountLogicalClusterReconciler acts as an
intializer for account workspaces." to read "AccountLogicalClusterReconciler
acts as an initializer for account workspaces."

In `@internal/predicates/accounttype.go`:
- Around line 17-25: The predicate LogicalClusterIsAccountTypeOrg currently
indexes parts[0] and parts[1] before verifying the slice length which can cause
an index-out-of-bounds panic; update the predicate (in
LogicalClusterIsAccountTypeOrg and the inner func where you split p and build
parts) to first check len(parts) == 3 (and that p/non-empty) and only then
compare parts[0] == "root" && parts[1] == "orgs" so the length check precedes
any element access; reference the symbols LogicalClusterIsAccountTypeOrg,
kcpPathAnnotation, and the local variable parts when making this change.

In `@internal/subroutine/account_tuples.go`:
- Around line 131-148: The Finalize method on AccountTuplesSubroutine currently
only logs and never removes the account's FGA tuples despite Finalizers()
returning "core.platform-mesh.io/account-fga-tuples"; update
AccountTuplesSubroutine.Finalize to locate the parent organization Store using
the LogicalCluster (lc) path annotation (kcpcore.LogicalClusterPathAnnotationKey
/ variable p), remove all tuples associated with this account/workspace from the
organization's Store (via whatever Store client/cleanup method your codebase
exposes), handle and log errors (returning an OperatorError if cleanup fails),
and only then allow deletion by removing the finalizer (i.e., return success
once tuples are deleted); if you intend to defer cleanup instead, replace the
finalizer registration in AccountTuplesSubroutine.Finalizers with documentation
in code/comments explaining who/gc is responsible.
- Around line 49-51: The call to mccontext.ClusterFrom(ctx) discards the boolean
result so a missing cluster name yields a zero-value lcID and later invalid
client construction; change the assignment to capture the ok (e.g., lcID, ok :=
mccontext.ClusterFrom(ctx)), check ok and if false log an error via the existing
log (use log.ChildLogger("ID", lcID).ChildLogger("path", p).Error() or similar)
and return/abort the operation before proceeding to client construction (the
code around the subsequent client creation), mirroring the handling used in
workspace_initializer.go.

In `@internal/subroutine/invite/subroutine.go`:
- Line 222: Fix the error message returned when Keycloak responds with a non-201
status: correct the misspelling "keylcloak" to "keycloak" and change the text to
reference 201/Created instead of "non-200"; update the return that constructs
the error (the call using errors.NewOperatorError(fmt.Errorf(...), true, true)
that mentions res.Status) so the fmt.Errorf message says something like
"keycloak returned non-201 status code: %s" or "keycloak returned unexpected
status code (expected 201): %s", using res.Status to include the actual status.

In `@internal/subroutine/workspace_initializer.go`:
- Around line 82-86: The code discards the boolean from
mccontext.ClusterFrom(ctx), so lcID may be zero-value and clients will target an
invalid cluster; update the block that calls mccontext.ClusterFrom(ctx) to
capture the second return value (e.g., ok), check if ok is false and return an
appropriate operator error instead of proceeding, and then use lcID to build the
child logger (replace the existing lcID-only usage). Ensure the error returned
is consistent with the surrounding pattern (ctrl.Result{},
errors.NewOperatorError(...)) so the controller reconciler stops when cluster
context is missing.
- Around line 60-63: Remove the dead parentRelation field from the struct in
workspace_initializer.go: delete the parentRelation declaration and any
assignments to it in the constructor (where the struct is built), leaving
objectType and creatorRelation intact; verify usages of TuplesForOrganization
still only pass creatorRelation and objectType and run tests to ensure nothing
else referenced parentRelation.

In `@pkg/fga/tuples.go`:
- Around line 27-40: baseTuples currently dereferences acc.Spec.Creator without
checking for nil which can panic; add a nil guard at the top of baseTuples to
avoid dereferencing a nil pointer: compute a local user string (e.g., if
acc.Spec.Creator == nil set user to a safe fallback like "user:unknown" or an
empty user string, otherwise set user = fmt.Sprintf("user:%s",
formatUser(*acc.Spec.Creator))) and then use that local user variable in the
returned tuples instead of fmt.Sprintf(... formatUser(*acc.Spec.Creator)). This
change touches the baseTuples function and ensures callers TuplesForAccount and
TuplesForOrganization no longer risk a runtime panic.

🧹 Nitpick comments (5)

internal/client/logicalcluster.go (1)

24-36: Variable copy shadows the Go builtin.

Line 25 uses copy as a variable name, which shadows the built-in copy function. While there is no functional impact here, it can be confusing. Consider renaming to cfgCopy or clonedCfg.

internal/subroutine/workspace_initializer.go (2)

103-113: Locally created orgsClient vs. constructor-injected w.orgsClient — potential confusion.

Line 103 creates a fresh client targeting "root:orgs" to fetch the Account, while line 134 uses w.orgsClient (injected at construction) for the Store CreateOrUpdate. If both are intended to target the same workspace, consider using one consistently. If they differ intentionally, a comment clarifying why would help future maintainers.

---

178-183: generateStoreName silently returns an empty string when the annotation is missing.

While Process validates the annotation earlier (line 82), this function is exported by name convention to the package and could be called from other contexts where the annotation hasn't been validated, resulting in a Store with an empty name.

Consider returning an error

-func generateStoreName(lc *kcpcorev1alpha1.LogicalCluster) string {
+func generateStoreName(lc *kcpcorev1alpha1.LogicalCluster) (string, error) {
 	if path, ok := lc.Annotations["kcp.io/path"]; ok {
 		pathElements := strings.Split(path, ":")
-		return pathElements[len(pathElements)-1]
+		return pathElements[len(pathElements)-1], nil
 	}
-	return ""
+	return "", fmt.Errorf("annotation kcp.io/path not found on LogicalCluster")
 }

internal/subroutine/account_tuples.go (1)

53-58: Three new REST clients created per reconciliation cycle.

Each call to iclient.NewForLogicalCluster constructs a new client with its own HTTP transport. On a busy cluster with many accounts, this could produce significant overhead (connections, TLS handshakes). Consider caching or reusing clients per logical cluster, or at minimum document that this is acceptable given expected scale.

Also applies to: 68-73, 81-86

internal/controller/accountlogicalcluster_controller.go (1)

30-41: Remove unused mcc field from AccountTuplesSubroutine.

The mcc field is assigned in the constructor but never referenced in Process, Finalize, or any other method. This dead code should be removed and reintroduced only when needed.