From ce63f90f6d0b0e1ab7d7c481a81764b98a36f586 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Mon, 17 Aug 2026 11:52:59 +0530 Subject: [PATCH 1/8] fix(reconcile): identify permissions by key instead of namespace and name --- internal/reconcile/permission.go | 81 ++++++++-------- internal/reconcile/permission_reconciler.go | 32 ++----- .../reconcile/permission_reconciler_test.go | 47 ++++----- internal/reconcile/permission_test.go | 95 ++++++++----------- 4 files changed, 115 insertions(+), 140 deletions(-) diff --git a/internal/reconcile/permission.go b/internal/reconcile/permission.go index 1c4543961..12443cbdd 100644 --- a/internal/reconcile/permission.go +++ b/internal/reconcile/permission.go @@ -11,22 +11,21 @@ import ( // KindPermission is the desired-state document kind for custom permissions. const KindPermission = "Permission" -// PermissionSpec is one desired permission. A permission is identity only -// (namespace + name): it is added or deleted, never updated. Deleting needs -// the explicit flag; a permission that just disappears from the file fails -// the plan instead. +// PermissionSpec is one desired permission, identified by its key in +// service.resource.verb form (for example compute.order.get). A permission is +// identity only: it is added or deleted, never updated. Deleting needs the +// explicit flag; a permission that just disappears from the file fails the plan +// instead. type PermissionSpec struct { - Namespace string `yaml:"namespace"` - Name string `yaml:"name"` - Delete bool `yaml:"delete,omitempty"` + Key string `yaml:"key"` + Delete bool `yaml:"delete,omitempty"` } -func (s PermissionSpec) String() string { - return s.Namespace + ":" + s.Name -} +func (s PermissionSpec) String() string { return s.Key } -func (s PermissionSpec) slug() string { - return schema.FQPermissionNameFromNamespace(s.Namespace, s.Name) +// namespaceAndName splits the key into its service/resource namespace and verb. +func (s PermissionSpec) namespaceAndName() (string, string) { + return schema.PermissionNamespaceAndNameFromKey(s.Key) } // isBaseNamespace reports whether a namespace belongs to the base schema, @@ -36,24 +35,31 @@ func isBaseNamespace(ns string) bool { } func validatePermissionSpec(s PermissionSpec) error { - if strings.TrimSpace(s.Namespace) == "" || strings.TrimSpace(s.Name) == "" { - return fmt.Errorf("namespace and name are required") + if strings.TrimSpace(s.Key) == "" { + return fmt.Errorf("key is required") + } + ns, name := s.namespaceAndName() + if ns == "" || name == "" { + return fmt.Errorf("invalid key %q (must be in service.resource.verb form)", s.Key) } - if isBaseNamespace(s.Namespace) { - return fmt.Errorf("namespace %q is part of the base schema, which the server manages", s.Namespace) + if isBaseNamespace(ns) { + return fmt.Errorf("key %q is part of the base schema, which the server manages", s.Key) } // One shared check so the reconcile plan and the CreatePermission API agree on // what a valid custom permission is: SpiceDB grammar, no reserved verb, and a // slug that fits SpiceDB's relation-name limit. This stops a plan that passes and // then fails when the schema compiles. - return schema.ValidateCustomPermission(s.Namespace, s.Name) + if err := schema.ValidateCustomPermission(ns, name); err != nil { + return fmt.Errorf("invalid key %q: %w", s.Key, err) + } + return nil } -// currentPermission is one custom permission as returned by ListPermissions. +// currentPermission is one custom permission as returned by ListPermissions, +// identified by its key. type currentPermission struct { - ID string - Namespace string - Name string + ID string + Key string } type permissionOp struct { @@ -69,39 +75,32 @@ func (o permissionOp) String() string { return fmt.Sprintf("add permission %s", o.spec) } -// diffPermissions returns the ops that make the current custom permissions -// match the desired spec. Every custom permission on the server must appear in -// the spec — kept, or marked delete — so nothing is ever removed by omission. +// diffPermissions returns the ops that make the current custom permissions match +// the desired spec. Every custom permission on the server must appear in the +// spec, kept or marked delete, so nothing is ever removed by omission. The key +// is the identity: it is one-to-one with the slug the server enforces, because a +// valid key's namespace parts hold no underscores. func diffPermissions(desired []PermissionSpec, current []currentPermission) ([]permissionOp, error) { - bySlug := make(map[string]currentPermission, len(current)) + byKey := make(map[string]currentPermission, len(current)) for _, c := range current { - bySlug[schema.FQPermissionNameFromNamespace(c.Namespace, c.Name)] = c + byKey[c.Key] = c } - // The slug is the identity the server enforces (unique in the database), so - // the diff is keyed by it. Distinct namespace+name pairs can flatten to the - // same slug when a namespace part contains underscores; that is a conflict. seen := map[string]PermissionSpec{} - accounted := map[string]struct{}{} var adds, removes []permissionOp for _, s := range desired { if err := validatePermissionSpec(s); err != nil { return nil, fmt.Errorf("invalid permission spec %s: %w", s, err) } - slug := s.slug() - if prev, dup := seen[slug]; dup { - if prev.Namespace != s.Namespace || prev.Name != s.Name { - return nil, fmt.Errorf("permissions %s and %s collide on the same slug %q", prev, s, slug) - } + if prev, dup := seen[s.Key]; dup { if prev.Delete != s.Delete { return nil, fmt.Errorf("permission %s is listed both with and without delete", s) } continue } - seen[slug] = s - accounted[slug] = struct{}{} + seen[s.Key] = s - cur, exists := bySlug[slug] + cur, exists := byKey[s.Key] switch { case s.Delete && exists: removes = append(removes, permissionOp{action: opRemove, spec: s, id: cur.ID}) @@ -111,9 +110,9 @@ func diffPermissions(desired []PermissionSpec, current []currentPermission) ([]p } var unaccounted []string - for slug, c := range bySlug { - if _, ok := accounted[slug]; !ok { - unaccounted = append(unaccounted, c.Namespace+":"+c.Name) + for key := range byKey { + if _, ok := seen[key]; !ok { + unaccounted = append(unaccounted, key) } } if len(unaccounted) > 0 { diff --git a/internal/reconcile/permission_reconciler.go b/internal/reconcile/permission_reconciler.go index 7fd28fe59..38db4bb79 100644 --- a/internal/reconcile/permission_reconciler.go +++ b/internal/reconcile/permission_reconciler.go @@ -32,8 +32,8 @@ func NewPermissionReconciler(client PermissionAPI, header string) *PermissionRec func (r *PermissionReconciler) Kind() string { return KindPermission } -// Validate checks every entry, and the in-file slug conflicts, without touching -// the server, so a bad entry stops the whole file before anything applies. +// Validate checks every entry without touching the server, so a bad entry stops +// the whole file before anything applies. func (r *PermissionReconciler) Validate(spec []byte) error { var specs []PermissionSpec if err := decodeSpec(spec, &specs); err != nil { @@ -44,16 +44,12 @@ func (r *PermissionReconciler) Validate(spec []byte) error { if err := validatePermissionSpec(s); err != nil { return fmt.Errorf("invalid permission spec %s: %w", s, err) } - slug := s.slug() - if prev, dup := seen[slug]; dup { - if prev.Namespace != s.Namespace || prev.Name != s.Name { - return fmt.Errorf("permissions %s and %s collide on the same slug %q", prev, s, slug) - } + if prev, dup := seen[s.Key]; dup { if prev.Delete != s.Delete { return fmt.Errorf("permission %s is listed both with and without delete", s) } } - seen[slug] = s + seen[s.Key] = s } return nil } @@ -99,15 +95,9 @@ func (r *PermissionReconciler) Export(ctx context.Context) (any, error) { } specs := make([]PermissionSpec, 0, len(current)) for _, c := range current { - specs = append(specs, PermissionSpec{Namespace: c.Namespace, Name: c.Name}) + specs = append(specs, PermissionSpec{Key: c.Key}) } - sort.Slice(specs, func(i, j int) bool { - a, b := specs[i], specs[j] - if a.Namespace != b.Namespace { - return a.Namespace < b.Namespace - } - return a.Name < b.Name - }) + sort.Slice(specs, func(i, j int) bool { return specs[i].Key < specs[j].Key }) return specs, nil } @@ -120,16 +110,12 @@ func (r *PermissionReconciler) fetchCurrent(ctx context.Context) ([]currentPermi for _, p := range resp.Msg.GetPermissions() { ns, name := schema.PermissionNamespaceAndNameFromKey(p.GetKey()) if ns == "" || name == "" { - return nil, fmt.Errorf("permission %s: cannot split key %q into namespace and name", p.GetId(), p.GetKey()) + return nil, fmt.Errorf("permission %s: cannot parse key %q into service.resource.verb", p.GetId(), p.GetKey()) } if isBaseNamespace(ns) { continue } - current = append(current, currentPermission{ - ID: p.GetId(), - Namespace: ns, - Name: name, - }) + current = append(current, currentPermission{ID: p.GetId(), Key: p.GetKey()}) } return current, nil } @@ -139,7 +125,7 @@ func (r *PermissionReconciler) apply(ctx context.Context, op permissionOp) error case opAdd: _, err := r.client.CreatePermission(ctx, authReq(&frontierv1beta1.CreatePermissionRequest{ Bodies: []*frontierv1beta1.PermissionRequestBody{{ - Key: schema.PermissionKeyFromNamespaceAndName(op.spec.Namespace, op.spec.Name), + Key: op.spec.Key, }}, }, r.header)) return err diff --git a/internal/reconcile/permission_reconciler_test.go b/internal/reconcile/permission_reconciler_test.go index 625bae92d..f9443ce5c 100644 --- a/internal/reconcile/permission_reconciler_test.go +++ b/internal/reconcile/permission_reconciler_test.go @@ -30,6 +30,8 @@ func (f *fakePermissionAPI) DeletePermission(_ context.Context, req *connect.Req return connect.NewResponse(&frontierv1beta1.DeletePermissionResponse{}), nil } +// permissionPB builds a listed permission the way the server returns it: the +// response carries the key, which is the reconciler's identity. func permissionPB(id, namespace, name string) *frontierv1beta1.Permission { return &frontierv1beta1.Permission{Id: id, Key: schema.PermissionKeyFromNamespaceAndName(namespace, name)} } @@ -40,7 +42,7 @@ func TestPermissionReconciler(t *testing.T) { permissionPB("b1", "app/organization", "administer"), // base: must be ignored permissionPB("p1", "compute/order", "legacy"), }} - spec := []byte("- {namespace: compute/order, name: legacy, delete: true}\n- {namespace: compute/order, name: get}\n") + spec := []byte("- {key: compute.order.legacy, delete: true}\n- {key: compute.order.get}\n") rep, err := NewPermissionReconciler(api, "").Reconcile(context.Background(), spec, false) @@ -55,27 +57,14 @@ func TestPermissionReconciler(t *testing.T) { assert.Equal(t, []string{"p1"}, api.deleted) }) - t.Run("a permission whose key does not split fails instead of being misread", func(t *testing.T) { - api := &fakePermissionAPI{perms: []*frontierv1beta1.Permission{ - {Id: "p1", Key: "notakey"}, - }} - spec := []byte("- {namespace: compute/order, name: get}\n") - - _, err := NewPermissionReconciler(api, "").Reconcile(context.Background(), spec, true) - - assert.ErrorContains(t, err, "notakey") - assert.Empty(t, api.created) - assert.Empty(t, api.deleted) - }) - t.Run("dry-run plans without applying", func(t *testing.T) { api := &fakePermissionAPI{} - spec := []byte("- {namespace: compute/order, name: get}\n") + spec := []byte("- {key: compute.order.get}\n") rep, err := NewPermissionReconciler(api, "").Reconcile(context.Background(), spec, true) assert.NoError(t, err) - assert.Equal(t, []string{"add permission compute/order:get"}, rep.Planned) + assert.Equal(t, []string{"add permission compute.order.get"}, rep.Planned) assert.Zero(t, rep.Applied) assert.Empty(t, api.created) }) @@ -83,7 +72,19 @@ func TestPermissionReconciler(t *testing.T) { t.Run("an unknown field in the spec fails the plan", func(t *testing.T) { api := &fakePermissionAPI{} // `delet` instead of `delete`: must fail, not silently ignore the delete - spec := []byte("- {namespace: compute/order, name: get, delet: true}\n") + spec := []byte("- {key: compute.order.get, delet: true}\n") + + _, err := NewPermissionReconciler(api, "").Reconcile(context.Background(), spec, true) + + assert.ErrorContains(t, err, "parse Permission spec") + assert.Empty(t, api.created) + }) + + t.Run("the old namespace/name format is rejected", func(t *testing.T) { + // The kind now takes a key. A file still using namespace/name must fail + // loudly (unknown fields) rather than silently reconcile nothing. + api := &fakePermissionAPI{} + spec := []byte("- {namespace: compute/order, name: get}\n") _, err := NewPermissionReconciler(api, "").Reconcile(context.Background(), spec, true) @@ -91,15 +92,15 @@ func TestPermissionReconciler(t *testing.T) { assert.Empty(t, api.created) }) - t.Run("an ambiguous namespace fails Validate before any server call", func(t *testing.T) { - // The charset check is server-free, so a namespace that would collide on the - // slug must fail the whole file up front (rule 3), not at apply. + t.Run("an ambiguous key fails Validate before any server call", func(t *testing.T) { + // The charset check is server-free, so a key whose namespace would collide on + // the slug must fail the whole file up front (rule 3), not at apply. api := &fakePermissionAPI{} - spec := []byte("- {namespace: resource_order/item, name: get}\n") + spec := []byte("- {key: resource_order.item.get}\n") err := NewPermissionReconciler(api, "").Validate(spec) - assert.ErrorContains(t, err, "resource_order/item") + assert.ErrorContains(t, err, "resource_order.item.get") assert.Empty(t, api.created) }) @@ -113,7 +114,7 @@ func TestPermissionReconciler(t *testing.T) { out, err := Export(context.Background(), registry, KindPermission) assert.NoError(t, err) - assert.NotContains(t, string(out), "app/project") + assert.NotContains(t, string(out), "app.project") reports, err := Run(context.Background(), registry, out, true) assert.NoError(t, err) diff --git a/internal/reconcile/permission_test.go b/internal/reconcile/permission_test.go index a8d358e96..e120aaa70 100644 --- a/internal/reconcile/permission_test.go +++ b/internal/reconcile/permission_test.go @@ -9,29 +9,29 @@ import ( func TestDiffPermissions(t *testing.T) { current := []currentPermission{ - {ID: "p1", Namespace: "compute/order", Name: "get"}, - {ID: "p2", Namespace: "compute/order", Name: "legacy"}, + {ID: "p1", Key: "compute.order.get"}, + {ID: "p2", Key: "compute.order.legacy"}, } t.Run("adds missing and deletes flagged, adds first", func(t *testing.T) { ops, err := diffPermissions([]PermissionSpec{ - {Namespace: "compute/order", Name: "get"}, - {Namespace: "compute/order", Name: "legacy", Delete: true}, - {Namespace: "compute/disk", Name: "mount"}, + {Key: "compute.order.get"}, + {Key: "compute.order.legacy", Delete: true}, + {Key: "compute.disk.mount"}, }, current) assert.NoError(t, err) if assert.Len(t, ops, 2) { - assert.Equal(t, "add permission compute/disk:mount", ops[0].String()) - assert.Equal(t, "delete permission compute/order:legacy", ops[1].String()) + assert.Equal(t, "add permission compute.disk.mount", ops[0].String()) + assert.Equal(t, "delete permission compute.order.legacy", ops[1].String()) assert.Equal(t, "p2", ops[1].id) } }) t.Run("no changes when converged", func(t *testing.T) { ops, err := diffPermissions([]PermissionSpec{ - {Namespace: "compute/order", Name: "get"}, - {Namespace: "compute/order", Name: "legacy"}, + {Key: "compute.order.get"}, + {Key: "compute.order.legacy"}, }, current) assert.NoError(t, err) assert.Empty(t, ops) @@ -39,9 +39,9 @@ func TestDiffPermissions(t *testing.T) { t.Run("delete of an absent permission is a no-op", func(t *testing.T) { ops, err := diffPermissions([]PermissionSpec{ - {Namespace: "compute/order", Name: "get"}, - {Namespace: "compute/order", Name: "legacy"}, - {Namespace: "compute/order", Name: "gone", Delete: true}, + {Key: "compute.order.get"}, + {Key: "compute.order.legacy"}, + {Key: "compute.order.gone", Delete: true}, }, current) assert.NoError(t, err) assert.Empty(t, ops) @@ -49,57 +49,47 @@ func TestDiffPermissions(t *testing.T) { t.Run("a server permission missing from the file fails the plan", func(t *testing.T) { _, err := diffPermissions([]PermissionSpec{ - {Namespace: "compute/order", Name: "get"}, + {Key: "compute.order.get"}, }, current) - assert.ErrorContains(t, err, "compute/order:legacy") + assert.ErrorContains(t, err, "compute.order.legacy") assert.ErrorContains(t, err, "delete: true") }) t.Run("conflicting delete flags for the same permission fail", func(t *testing.T) { _, err := diffPermissions([]PermissionSpec{ - {Namespace: "compute/order", Name: "get"}, - {Namespace: "compute/order", Name: "get", Delete: true}, - {Namespace: "compute/order", Name: "legacy"}, + {Key: "compute.order.get"}, + {Key: "compute.order.get", Delete: true}, + {Key: "compute.order.legacy"}, }, current) assert.ErrorContains(t, err, "listed both with and without delete") }) - t.Run("a file namespace that would collide with a server slug is rejected, not absorbed", func(t *testing.T) { - // The server stores resource/order_item; the file lists a genuinely different - // namespace resource_order/item that flattens to the same slug. The diff used - // to treat it as already present and plan zero ops (the rule 2 gap). The - // ambiguous namespace is now rejected at validation, so it cannot be absorbed. - server := []currentPermission{{ID: "p1", Namespace: "resource/order_item", Name: "get"}} - _, err := diffPermissions([]PermissionSpec{{Namespace: "resource_order/item", Name: "get"}}, server) - assert.ErrorContains(t, err, "resource_order/item") - }) - - t.Run("rejects a namespace with an underscore or uppercase in a part", func(t *testing.T) { + t.Run("rejects a key whose namespace part has an underscore or uppercase", func(t *testing.T) { // The slug joins service, resource, and verb with "_", so an underscore in a - // part makes two namespaces flatten to one slug; uppercase cannot be a - // SpiceDB object type. Both are rejected so the slug stays one-to-one. - for _, ns := range []string{"resource_order/item", "resource/order_item", "Compute/order", "compute/Order"} { - _, err := diffPermissions([]PermissionSpec{{Namespace: ns, Name: "get"}}, nil) - if assert.Error(t, err, ns) { - assert.ErrorContains(t, err, "namespace") + // part makes two keys flatten to one slug; uppercase cannot be a SpiceDB + // object type. Both are rejected so the key stays one-to-one with the slug. + for _, key := range []string{"resource_order.item.get", "resource.order_item.get", "Compute.order.get", "compute.Order.get"} { + _, err := diffPermissions([]PermissionSpec{{Key: key}}, nil) + if assert.Error(t, err, key) { + assert.ErrorContains(t, err, "key") } } }) - t.Run("accepts valid custom namespaces", func(t *testing.T) { - for _, ns := range []string{"resource/aoi", "user/project", "org/user", "compute/disk"} { - ops, err := diffPermissions([]PermissionSpec{{Namespace: ns, Name: "get"}}, nil) - assert.NoError(t, err, ns) + t.Run("accepts valid custom keys", func(t *testing.T) { + for _, key := range []string{"resource.aoi.get", "user.project.get", "org.user.get", "compute.disk.get"} { + ops, err := diffPermissions([]PermissionSpec{{Key: key}}, nil) + assert.NoError(t, err, key) assert.Len(t, ops, 1) // a valid new permission plans an add } }) - t.Run("rejects a permission whose flattened slug is too long", func(t *testing.T) { + t.Run("rejects a key whose flattened slug is too long", func(t *testing.T) { // Each part is valid on its own, but the slug service_resource_verb overflows // SpiceDB's sixty-four character relation limit, so it must fail the plan // rather than pass and then fail when the schema compiles. - ns := strings.Repeat("a", 30) + "/" + strings.Repeat("b", 30) - _, err := diffPermissions([]PermissionSpec{{Namespace: ns, Name: "get"}}, nil) + key := strings.Repeat("a", 30) + "." + strings.Repeat("b", 30) + ".get" + _, err := diffPermissions([]PermissionSpec{{Key: key}}, nil) if assert.Error(t, err) { assert.ErrorContains(t, err, "too long") } @@ -109,24 +99,23 @@ func TestDiffPermissions(t *testing.T) { // The generator adds owner, project, and granted to every custom resource, so // a verb equal to one of them would declare the same relation twice and the // schema would fail to compile. The plan must reject it up front. - for _, name := range []string{"owner", "project", "granted"} { - _, err := diffPermissions([]PermissionSpec{{Namespace: "compute/order", Name: name}}, nil) - if assert.Error(t, err, name) { + for _, key := range []string{"compute.order.owner", "compute.order.project", "compute.order.granted"} { + _, err := diffPermissions([]PermissionSpec{{Key: key}}, nil) + if assert.Error(t, err, key) { assert.ErrorContains(t, err, "reserved") } } }) - t.Run("rejects base-schema namespaces and bad shapes", func(t *testing.T) { - for _, s := range []PermissionSpec{ - {Namespace: "app/organization", Name: "hack"}, - {Namespace: "app", Name: "hack"}, - {Namespace: "compute", Name: "get"}, - {Namespace: "compute/order", Name: "not-alnum"}, - {Namespace: "compute/order", Name: ""}, + t.Run("rejects base-schema keys and bad shapes", func(t *testing.T) { + for _, key := range []string{ + "app.organization.hack", // base schema + "compute.get", // only two parts + "compute.order.", // empty verb + "", // empty } { - _, err := diffPermissions([]PermissionSpec{s}, nil) - assert.Error(t, err, "%+v", s) + _, err := diffPermissions([]PermissionSpec{{Key: key}}, nil) + assert.Error(t, err, key) } }) } From 38af4652c4236db89b33bab0657fa40f3d676186 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Mon, 17 Aug 2026 11:52:59 +0530 Subject: [PATCH 2/8] docs(reconcile): describe the permission key format --- docs/content/docs/reconcile.mdx | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/docs/content/docs/reconcile.mdx b/docs/content/docs/reconcile.mdx index 49a060477..30aafd18a 100644 --- a/docs/content/docs/reconcile.mdx +++ b/docs/content/docs/reconcile.mdx @@ -93,22 +93,21 @@ concerns users. Service users cannot be disabled today, so they are always liste ## The Permission kind -A permission is an identity: a `service/resource` namespace plus a verb. There is nothing +A permission is an identity: a key in `service.resource.verb` form. There is nothing else to manage on it, so it is either created or deleted. ```yaml apiVersion: v1 kind: Permission spec: - - namespace: compute/order - name: get - - namespace: compute/order - name: legacy + - key: compute.order.get + - key: compute.order.legacy delete: true ``` -- The namespace has two parts, `service/resource`. The name is alphanumeric. -- Frontier's own permissions (the `app` namespaces) belong to the base schema. The +- The key has three parts joined by dots, `service.resource.verb`. Each part is lowercase + alphanumeric. +- Frontier's own permissions (the `app` service) belong to the base schema. The reconciler ignores them and rejects a file entry that names one. - A custom permission on the server that the file does not list fails the plan. Nothing is deleted just because it is missing; deleting needs `delete: true` on the entry. From f2d5021708ded28d5f5315f64e459ea5ab86a768 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Mon, 17 Aug 2026 13:10:17 +0530 Subject: [PATCH 3/8] fix(reconcile): skip a permission with an unparseable key instead of failing the list --- internal/reconcile/permission_reconciler.go | 10 ++++++---- internal/reconcile/permission_reconciler_test.go | 14 ++++++++++++++ 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/internal/reconcile/permission_reconciler.go b/internal/reconcile/permission_reconciler.go index 38db4bb79..b4f971963 100644 --- a/internal/reconcile/permission_reconciler.go +++ b/internal/reconcile/permission_reconciler.go @@ -109,10 +109,12 @@ func (r *PermissionReconciler) fetchCurrent(ctx context.Context) ([]currentPermi var current []currentPermission for _, p := range resp.Msg.GetPermissions() { ns, name := schema.PermissionNamespaceAndNameFromKey(p.GetKey()) - if ns == "" || name == "" { - return nil, fmt.Errorf("permission %s: cannot parse key %q into service.resource.verb", p.GetId(), p.GetKey()) - } - if isBaseNamespace(ns) { + // A key that does not split into service.resource.verb is not a custom + // permission this reconciler manages: it is a base or system permission, or + // comes from an older server that does not set the key. Skip it instead of + // failing the whole list. Export only emits keys it kept here, so a skipped + // permission never shows up as missing from the file either. + if ns == "" || name == "" || isBaseNamespace(ns) { continue } current = append(current, currentPermission{ID: p.GetId(), Key: p.GetKey()}) diff --git a/internal/reconcile/permission_reconciler_test.go b/internal/reconcile/permission_reconciler_test.go index f9443ce5c..e62aba65b 100644 --- a/internal/reconcile/permission_reconciler_test.go +++ b/internal/reconcile/permission_reconciler_test.go @@ -104,6 +104,20 @@ func TestPermissionReconciler(t *testing.T) { assert.Empty(t, api.created) }) + t.Run("a server permission with an unparseable key is skipped, not fatal", func(t *testing.T) { + // An older server, or a base permission, may return a key that does not split + // into service.resource.verb. It must be ignored, not fail the whole plan. + api := &fakePermissionAPI{perms: []*frontierv1beta1.Permission{ + {Id: "weird", Key: "not-a-key"}, + permissionPB("p1", "compute/order", "get"), + }} + spec := []byte("- {key: compute.order.get}\n") + + rep, err := NewPermissionReconciler(api, "").Reconcile(context.Background(), spec, true) + assert.NoError(t, err) + assert.Empty(t, rep.Planned) // the good permission converges; the weird one is ignored + }) + t.Run("reconciling an exported document plans no changes", func(t *testing.T) { api := &fakePermissionAPI{perms: []*frontierv1beta1.Permission{ permissionPB("b1", "app/project", "get"), From 0625881cc78d3dfbec811dbe7c66e0cf66b2b940 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Mon, 17 Aug 2026 13:10:17 +0530 Subject: [PATCH 4/8] docs(authz): use the permission key form in custom-resources examples --- docs/content/docs/authz/custom-resources.mdx | 35 ++++++++------------ 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/docs/content/docs/authz/custom-resources.mdx b/docs/content/docs/authz/custom-resources.mdx index 242fcdad1..4735ebbee 100644 --- a/docs/content/docs/authz/custom-resources.mdx +++ b/docs/content/docs/authz/custom-resources.mdx @@ -31,21 +31,17 @@ You register the actions in one of two ways: - **The admin API** (`CreatePermission`, `CreateRole`). This makes the same change one call at a time, useful for one-off changes or scripting. -Here is the `compute/machine` example as a reconcile file. Each entry is a namespace and an -action name: +Here is the `compute/machine` example as a reconcile file. Each entry is a key in +`service.resource.verb` form: ```yaml apiVersion: v1 kind: Permission spec: - - namespace: compute/machine - name: get - - namespace: compute/machine - name: create - - namespace: compute/machine - name: update - - namespace: compute/machine - name: delete + - key: compute.machine.get + - key: compute.machine.create + - key: compute.machine.update + - key: compute.machine.delete ``` Apply it with a superuser credential (for example the bootstrap service account): @@ -276,21 +272,16 @@ apiVersion: v1 kind: Permission spec: # Per-item actions live on the resource itself, checked against compute/machine:. - - namespace: compute/machine - name: get - - namespace: compute/machine - name: update - - namespace: compute/machine - name: delete + - key: compute.machine.get + - key: compute.machine.update + - key: compute.machine.delete # Project-level capabilities live on user/project, a proxy for app/project. # Checked against app/project:, because there is no single - # machine to check against. Do NOT use namespace app/project here: the - # app namespaces are reserved for the base schema and are rejected. - - namespace: user/project - name: createcomputemachine - - namespace: user/project - name: listcomputemachines + # machine to check against. Do NOT use the app service here: the app + # namespaces are reserved for the base schema and are rejected. + - key: user.project.createcomputemachine + - key: user.project.listcomputemachines --- apiVersion: v1 kind: Role From f1db2aefbae16c213a99e04c337dc50a263d63db Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Mon, 17 Aug 2026 14:00:07 +0530 Subject: [PATCH 5/8] docs(authz): describe the permission rules in key form --- docs/content/docs/authz/custom-resources.mdx | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/docs/content/docs/authz/custom-resources.mdx b/docs/content/docs/authz/custom-resources.mdx index 4735ebbee..856cae11d 100644 --- a/docs/content/docs/authz/custom-resources.mdx +++ b/docs/content/docs/authz/custom-resources.mdx @@ -59,15 +59,14 @@ permissions in this same format, so you can capture the live state into a file. A few rules the reconcile flow enforces for permissions: -- A permission is **identity only** (namespace plus name). It is added or deleted, never - updated. +- A permission is **identity only** (its key). It is added or deleted, never updated. - Nothing is deleted by leaving it out. A permission that is in the server but missing from the file fails the plan. To remove one, mark its entry with `delete: true`. -- A namespace must be in `service/resource` form: two non-empty parts, each lowercase - alphanumeric. So `compute/machine` works, but `compute/machine-v2` does not (the hyphen is - not alphanumeric). The action name must be alphanumeric too. -- A namespace under `app` or `app/...` is rejected, because the base schema owns those types - (see [Why a separate `user/project` namespace](#why-a-separate-userproject-namespace)). +- A key has three parts joined by dots, `service.resource.verb`. Each part is lowercase, + starts with a letter, and is at least three characters. So `compute.machine.get` works, but + `compute.machine.get-all` does not (the hyphen is not alphanumeric). +- A key whose service is `app` (an `app.*` key) is rejected, because the base schema owns + those types (see [Why a separate `user/project` namespace](#why-a-separate-userproject-namespace)). ### What happens when a permission is registered From b621861123e47a7a9fe215dd9afc32c7c832aa9c Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Tue, 18 Aug 2026 11:45:31 +0530 Subject: [PATCH 6/8] docs(rfc): describe the Permission entry in key form --- docs/rfcs/0001-declarative-reconcile.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/rfcs/0001-declarative-reconcile.md b/docs/rfcs/0001-declarative-reconcile.md index 4cbba0ddd..8afe0746e 100644 --- a/docs/rfcs/0001-declarative-reconcile.md +++ b/docs/rfcs/0001-declarative-reconcile.md @@ -51,7 +51,7 @@ role is a value. | Kind | Object or value | Identity | A missing entry | How to remove | |---|---|---|---|---| | PlatformUser | value | principal + relation | access removed | leave the entry out | -| Permission | object | namespace + name | plan fails | set `delete: true` | +| Permission | object | key | plan fails | set `delete: true` | | Role, custom | object | name | plan fails | set `delete: true` | | Role, predefined | value | name | reset to the shipped definition | cannot be removed | | Preference | value | trait name | reset to the trait default | leave the entry out, it resets | @@ -207,8 +207,8 @@ entry removes that access. A user with both relations has two entries. Adding a email that does not exist creates the user. The bootstrap service account is server-managed: the flow skips it on the server side and rejects it in the file. -**Permission.** An entry is `{namespace, name}`, for example `compute/order` + `get`. A -permission is an identity only, so it is created or deleted, never updated. Creating one also +**Permission.** An entry is `{key}` in `service.resource.verb` form, for example +`compute.order.get`. A permission is an identity only, so it is created or deleted, never updated. Creating one also updates the authorization schema, so it is usable in roles right away, on every pod at once. The base `app` namespaces are server-managed. From 32735d13152339ea9989603865ef41fb4766bba7 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Wed, 19 Aug 2026 13:48:34 +0530 Subject: [PATCH 7/8] fix(reconcile): skip a current permission that fails the key grammar so a legacy row cannot wedge the plan --- internal/reconcile/permission_reconciler.go | 11 +++++++++++ .../reconcile/permission_reconciler_test.go | 17 +++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/internal/reconcile/permission_reconciler.go b/internal/reconcile/permission_reconciler.go index b4f971963..f881033f3 100644 --- a/internal/reconcile/permission_reconciler.go +++ b/internal/reconcile/permission_reconciler.go @@ -117,6 +117,17 @@ func (r *PermissionReconciler) fetchCurrent(ctx context.Context) ([]currentPermi if ns == "" || name == "" || isBaseNamespace(ns) { continue } + // Also skip a legacy row whose parsed parts break the grammar the reconciler + // enforces: an underscore or uppercase in a part, a reserved verb, or an + // over-long slug. No file entry could name such a row, because + // validatePermissionSpec rejects the same key, so keeping it would wedge every + // plan: it can be neither kept (it counts as unaccounted) nor deleted (a delete + // spec fails validation first). Treat it as out of scope, like a base + // permission. New rows cannot reach this state now that CreatePermission runs + // the same check, but rows created before that tightening can. + if schema.ValidateCustomPermission(ns, name) != nil { + continue + } current = append(current, currentPermission{ID: p.GetId(), Key: p.GetKey()}) } return current, nil diff --git a/internal/reconcile/permission_reconciler_test.go b/internal/reconcile/permission_reconciler_test.go index e62aba65b..757280bc5 100644 --- a/internal/reconcile/permission_reconciler_test.go +++ b/internal/reconcile/permission_reconciler_test.go @@ -118,6 +118,23 @@ func TestPermissionReconciler(t *testing.T) { assert.Empty(t, rep.Planned) // the good permission converges; the weird one is ignored }) + t.Run("a legacy server permission that breaks the grammar is skipped, not fatal", func(t *testing.T) { + // A row created before CreatePermission validated namespaces can carry an + // underscore in a part. Its key parses, but validatePermissionSpec would reject + // that key, so no file entry can name it. It must be out of scope, or every plan + // would wedge: it could be neither kept (unaccounted) nor deleted (the delete + // spec fails validation first). + api := &fakePermissionAPI{perms: []*frontierv1beta1.Permission{ + {Id: "legacy", Key: "resource.order_item.get"}, // underscore in the resource part + permissionPB("p1", "compute/order", "get"), + }} + spec := []byte("- {key: compute.order.get}\n") + + rep, err := NewPermissionReconciler(api, "").Reconcile(context.Background(), spec, true) + assert.NoError(t, err) + assert.Empty(t, rep.Planned) // the good permission converges; the legacy one is ignored + }) + t.Run("reconciling an exported document plans no changes", func(t *testing.T) { api := &fakePermissionAPI{perms: []*frontierv1beta1.Permission{ permissionPB("b1", "app/project", "get"), From de5da442c77092eab659ac50b1c550d16e16e698 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Wed, 19 Aug 2026 13:48:34 +0530 Subject: [PATCH 8/8] docs(reconcile): list the reserved-verb and slug-length key rules and tighten the grammar wording --- docs/content/docs/authz/custom-resources.mdx | 6 ++++++ docs/content/docs/reconcile.mdx | 8 ++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/docs/content/docs/authz/custom-resources.mdx b/docs/content/docs/authz/custom-resources.mdx index 856cae11d..98690dc84 100644 --- a/docs/content/docs/authz/custom-resources.mdx +++ b/docs/content/docs/authz/custom-resources.mdx @@ -65,6 +65,12 @@ A few rules the reconcile flow enforces for permissions: - A key has three parts joined by dots, `service.resource.verb`. Each part is lowercase, starts with a letter, and is at least three characters. So `compute.machine.get` works, but `compute.machine.get-all` does not (the hyphen is not alphanumeric). +- The verb cannot be `owner`, `project`, or `granted`. The generated schema already defines + relations with those names on every resource, so a key like `compute.machine.owner` is + rejected even though it meets the rule above. +- The whole flattened name `service_resource_verb` must be at most 64 characters, the limit + SpiceDB puts on a relation name. Three parts that are each fine on their own can still be + rejected together as too long. - A key whose service is `app` (an `app.*` key) is rejected, because the base schema owns those types (see [Why a separate `user/project` namespace](#why-a-separate-userproject-namespace)). diff --git a/docs/content/docs/reconcile.mdx b/docs/content/docs/reconcile.mdx index 30aafd18a..05ac8fac4 100644 --- a/docs/content/docs/reconcile.mdx +++ b/docs/content/docs/reconcile.mdx @@ -105,8 +105,12 @@ spec: delete: true ``` -- The key has three parts joined by dots, `service.resource.verb`. Each part is lowercase - alphanumeric. +- The key has three parts joined by dots, `service.resource.verb`. Each part is lowercase, + starts with a letter, and is at least three characters. +- The verb cannot be `owner`, `project`, or `granted`. The generated schema already defines + relations with those names on every resource. +- The whole flattened name `service_resource_verb` must be at most 64 characters, SpiceDB's + limit on a relation name. - Frontier's own permissions (the `app` service) belong to the base schema. The reconciler ignores them and rejects a file entry that names one. - A custom permission on the server that the file does not list fails the plan. Nothing