fix(permission): make the response key lossless and migrate its readers

cmd/permission.go

@@ -5,6 +5,7 @@ import (
 	"os"
 
 	"github.com/MakeNowJust/heredoc"
+	"github.com/raystack/frontier/internal/bootstrap/schema"
 	"github.com/raystack/frontier/pkg/file"
 	frontierv1beta1 "github.com/raystack/frontier/proto/v1beta1"
 	"github.com/raystack/salt/cli/printer"
@@ -186,12 +187,13 @@ func viewPermissionCommand(cliConfig *Config) *cli.Command {
 
 			spinner.Stop()
 
+			permNamespace, permName := schema.PermissionNamespaceAndNameFromKey(action.GetKey())
+
 			report = append(report, []string{"ID", "NAME", "NAMESPACE"})
-			//nolint:staticcheck
 			report = append(report, []string{
 				action.GetId(),
-				action.GetName(),
-				action.GetNamespace(),
+				permName,
+				permNamespace,
 			})
 			printer.Table(os.Stdout, report)
 
@@ -248,12 +250,12 @@ func listPermissionCommand(cliConfig *Config) *cli.Command {
 			fmt.Printf(" \nShowing %d permission(s)\n \n", len(permissions))
 
 			report = append(report, []string{"ID", "NAME", "NAMESPACE"})
-			//nolint:staticcheck
 			for _, a := range permissions {
+				permNamespace, permName := schema.PermissionNamespaceAndNameFromKey(a.GetKey())
 				report = append(report, []string{
 					a.GetId(),
-					a.GetName(),
-					a.GetNamespace(),
+					permName,
+					permNamespace,
 				})
 			}
 			printer.Table(os.Stdout, report)

internal/api/v1beta1connect/permission.go

@@ -94,14 +94,23 @@ func transformPermissionToPB(perm permission.Permission) (*frontierv1beta1.Permi
 		}
 	}
 
+	// key is the replacement for the deprecated namespace/name fields, so it
+	// must read back to the exact stored pair. A row that cannot round-trip
+	// (namespace without a slash, or with a dot in a part) fails loudly here
+	// instead of returning a key that reads back as a different permission.
+	key := schema.PermissionKeyFromNamespaceAndName(perm.NamespaceID, perm.Name)
+	if ns, name := schema.PermissionNamespaceAndNameFromKey(key); ns != perm.NamespaceID || name != perm.Name {
+		return nil, fmt.Errorf("permission namespace %q and name %q do not round-trip through key %q", perm.NamespaceID, perm.Name, key)
+	}
+
 	return &frontierv1beta1.Permission{
 		Id:        perm.ID,
 		Name:      perm.Name,
 		CreatedAt: timestamppb.New(perm.CreatedAt),
 		UpdatedAt: timestamppb.New(perm.UpdatedAt),
 		Namespace: perm.NamespaceID,
 		Metadata:  metadata,
-		Key:       schema.PermissionKeyFromNamespaceAndName(perm.NamespaceID, perm.Name),
+		Key:       key,
 	}, nil
 }
 

internal/api/v1beta1connect/permission_test.go

@@ -522,3 +522,24 @@ func TestHandler_GetPermission(t *testing.T) {
 		})
 	}
 }
+
+func TestTransformPermissionToPB_KeyRoundTrip(t *testing.T) {
+	t.Run("a name with dots survives the key round trip", func(t *testing.T) {
+		got, err := transformPermissionToPB(permission.Permission{
+			ID:          "p1",
+			Name:        "soft.delete",
+			NamespaceID: "database/instance",
+		})
+		assert.NoError(t, err)
+		assert.Equal(t, "database.instance.soft.delete", got.GetKey())
+	})
+
+	t.Run("a namespace without a slash fails instead of emitting a wrong key", func(t *testing.T) {
+		_, err := transformPermissionToPB(permission.Permission{
+			ID:          "p2",
+			Name:        "get",
+			NamespaceID: "foo",
+		})
+		assert.ErrorContains(t, err, "round-trip")
+	})
+}

internal/bootstrap/schema/schema.go

@@ -275,9 +275,13 @@ func FQPermissionNameFromNamespace(namespace, verb string) string {
 	return fmt.Sprintf("%s_%s_%s", service, resource, verb)
 }
 
+// PermissionNamespaceAndNameFromKey splits a "service.resource.verb" key into
+// its namespace ("service/resource") and name ("verb"). Namespace parts can
+// never contain a dot, so the first two segments are always the namespace and
+// everything after them is the name — a name with dots survives the split.
 func PermissionNamespaceAndNameFromKey(key string) (string, string) {
-	parts := strings.Split(key, ".")
-	if len(parts) != 3 {
+	parts := strings.SplitN(key, ".", 3)
+	if len(parts) != 3 || parts[0] == "" || parts[1] == "" || parts[2] == "" {
 		return "", ""
 	}
 	return fmt.Sprintf("%s/%s", parts[0], parts[1]), parts[2]

internal/bootstrap/schema/schema_test.go

@@ -50,6 +50,53 @@ func TestFQPermissionNameFromNamespace(t *testing.T) {
 	}
 }
 
+func TestPermissionNamespaceAndNameFromKey(t *testing.T) {
+	tests := []struct {
+		key           string
+		wantNamespace string
+		wantName      string
+	}{
+		{"compute.instance.delete", "compute/instance", "delete"},
+		{"app.organization.get", "app/organization", "get"},
+		// dots after the namespace belong to the name
+		{"compute.instance.soft.delete", "compute/instance", "soft.delete"},
+		{"compute.instance", "", ""}, // too few parts
+		{"compute", "", ""},
+		{"", "", ""},
+		{".instance.delete", "", ""},  // empty service
+		{"compute..delete", "", ""},   // empty resource
+		{"compute.instance.", "", ""}, // empty name
+	}
+	for _, tt := range tests {
+		t.Run(tt.key, func(t *testing.T) {
+			ns, name := schema.PermissionNamespaceAndNameFromKey(tt.key)
+			if ns != tt.wantNamespace || name != tt.wantName {
+				t.Errorf("PermissionNamespaceAndNameFromKey(%q) = (%q, %q), want (%q, %q)", tt.key, ns, name, tt.wantNamespace, tt.wantName)
+			}
+		})
+	}
+}
+
+func TestPermissionKeyRoundTrip(t *testing.T) {
+	tests := []struct {
+		namespace string
+		name      string
+	}{
+		{"compute/instance", "delete"},
+		{"app/organization", "get"},
+		{"database/instance", "soft.delete"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.namespace+":"+tt.name, func(t *testing.T) {
+			key := schema.PermissionKeyFromNamespaceAndName(tt.namespace, tt.name)
+			ns, name := schema.PermissionNamespaceAndNameFromKey(key)
+			if ns != tt.namespace || name != tt.name {
+				t.Errorf("round trip through key %q = (%q, %q), want (%q, %q)", key, ns, name, tt.namespace, tt.name)
+			}
+		})
+	}
+}
+
 func TestIsValidPermissionNamespace(t *testing.T) {
 	tests := []struct {
 		ns   string

internal/reconcile/permission_reconciler.go

@@ -117,15 +117,18 @@ func (r *PermissionReconciler) fetchCurrent(ctx context.Context) ([]currentPermi
 		return nil, fmt.Errorf("list permissions: %w", err)
 	}
 	var current []currentPermission
-	//nolint:staticcheck
 	for _, p := range resp.Msg.GetPermissions() {
-		if isBaseNamespace(p.GetNamespace()) {
+		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())
+		}
+		if isBaseNamespace(ns) {
 			continue
 		}
 		current = append(current, currentPermission{
 			ID:        p.GetId(),
-			Namespace: p.GetNamespace(),
-			Name:      p.GetName(),
+			Namespace: ns,
+			Name:      name,
 		})
 	}
 	return current, nil

internal/reconcile/permission_reconciler_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"connectrpc.com/connect"
+	"github.com/raystack/frontier/internal/bootstrap/schema"
 	frontierv1beta1 "github.com/raystack/frontier/proto/v1beta1"
 	"github.com/stretchr/testify/assert"
 )
@@ -30,7 +31,7 @@ func (f *fakePermissionAPI) DeletePermission(_ context.Context, req *connect.Req
 }
 
 func permissionPB(id, namespace, name string) *frontierv1beta1.Permission {
-	return &frontierv1beta1.Permission{Id: id, Namespace: namespace, Name: name}
+	return &frontierv1beta1.Permission{Id: id, Key: schema.PermissionKeyFromNamespaceAndName(namespace, name)}
 }
 
 func TestPermissionReconciler(t *testing.T) {
@@ -54,6 +55,19 @@ 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")

test/e2e/regression/service_registration_test.go

@@ -146,8 +146,7 @@ func (s *ServiceRegistrationRegressionTestSuite) TestServiceRegistration() {
 		var lastPermCount int
 		for _, perm := range []string{"get", "update", "delete"} {
 			for _, listPerm := range listPermResp.Msg.GetPermissions() {
-				//nolint:staticcheck
-				if listPerm.GetName() == perm && listPerm.GetNamespace() == "database/instance" {
+				if listPerm.GetKey() == "database.instance."+perm {
 					lastPermCount++
 				}
 			}
@@ -229,8 +228,7 @@ func (s *ServiceRegistrationRegressionTestSuite) TestPermissionDeleteCascade() {
 	s.Require().NoError(err)
 	var builtinID string
 	for _, p := range listResp.Msg.GetPermissions() {
-		//nolint:staticcheck
-		if p.GetNamespace() == "app/organization" && p.GetName() == "get" {
+		if p.GetKey() == "app.organization.get" {
 			builtinID = p.GetId()
 			break
 		}