From 3e08168d55214f19d07bca47c122e5c8c0091cc3 Mon Sep 17 00:00:00 2001 From: Abhishek Sah Date: Mon, 8 Jun 2026 09:56:20 +0530 Subject: [PATCH 1/2] fix(resource): reconcile #project/#owner relations on update resource.Update was DB-only and persisted just title/metadata, so moving a resource to a new project or reassigning its owner was a silent no-op: the change was dropped from the row and the old #project/#owner SpiceDB tuples stayed put while the new ones were never written. Update now: - persists project_id, principal_id, principal_type, and a recomputed URN (name and namespace remain immutable on update); - when the project changes, deletes the old app/:#project tuple and writes the new one; - when the owner changes, deletes the old #owner tuple and writes the new one. PAT owners are resolved to the underlying user, mirroring Create. Adds an e2e regression test: create a resource in project A, move it to project B, assert the #project relation now points at B (old tuple gone). Verified it fails without the reconcile (relation stays on A) and passes with it. Refs #1661 Co-Authored-By: Claude Opus 4.8 (1M context) --- core/resource/service.go | 78 ++++++++++++++++++- core/resource/service_test.go | 49 ++++++++++-- .../store/postgres/resource_repository.go | 8 +- test/e2e/regression/api_test.go | 62 +++++++++++++++ 4 files changed, 188 insertions(+), 9 deletions(-) diff --git a/core/resource/service.go b/core/resource/service.go index 4671e2f55..38a764c0a 100644 --- a/core/resource/service.go +++ b/core/resource/service.go @@ -170,8 +170,82 @@ func (s Service) List(ctx context.Context, flt Filter) ([]Resource, error) { return s.repository.List(ctx, flt) } -func (s Service) Update(ctx context.Context, resource Resource) (Resource, error) { - return s.repository.Update(ctx, resource) +// Update persists a resource change and reconciles its #project / #owner +// SpiceDB relations. Previously Update only touched title/metadata in the DB +// and never reconciled relations, so moving a resource to a new project or +// reassigning its owner silently left the old #project/#owner tuples in place +// (and never wrote the new ones). Name and namespace stay immutable here; the +// URN tracks the (possibly new) project name. +func (s Service) Update(ctx context.Context, res Resource) (Resource, error) { + existing, err := s.repository.GetByID(ctx, res.ID) + if err != nil { + return Resource{}, err + } + + principalID := res.PrincipalID + principalType := res.PrincipalType + // PAT → resolve to underlying user, mirroring Create + if principalType == schema.PATPrincipal { + sub, err := s.resolvePATUser(ctx, principalID) + if err != nil { + return Resource{}, fmt.Errorf("resolving PAT principal: %w", err) + } + principalID = sub.ID + principalType = sub.Namespace + } + // no owner supplied → keep the current one + if strings.TrimSpace(principalID) == "" { + principalID = existing.PrincipalID + principalType = existing.PrincipalType + } + + resourceProject, err := s.projectService.Get(ctx, res.ProjectID) + if err != nil { + return Resource{}, fmt.Errorf("failed to get project: %w", err) + } + + updated, err := s.repository.Update(ctx, Resource{ + ID: existing.ID, + URN: existing.CreateURN(resourceProject.Name), + Name: existing.Name, + Title: res.Title, + ProjectID: resourceProject.ID, + NamespaceID: existing.NamespaceID, + PrincipalID: principalID, + PrincipalType: principalType, + Metadata: res.Metadata, + }) + if err != nil { + return Resource{}, err + } + + // reconcile the project grant if the resource moved projects + if existing.ProjectID != updated.ProjectID { + if err = s.relationService.Delete(ctx, relation.Relation{ + Object: relation.Object{ID: updated.ID, Namespace: updated.NamespaceID}, + RelationName: schema.ProjectRelationName, + }); err != nil && !errors.Is(err, relation.ErrNotExist) { + return Resource{}, err + } + if err = s.AddProjectToResource(ctx, updated.ProjectID, updated); err != nil { + return Resource{}, err + } + } + + // reconcile the owner grant if the owner changed + if existing.PrincipalID != updated.PrincipalID || existing.PrincipalType != updated.PrincipalType { + if err = s.relationService.Delete(ctx, relation.Relation{ + Object: relation.Object{ID: updated.ID, Namespace: updated.NamespaceID}, + RelationName: schema.OwnerRelationName, + }); err != nil && !errors.Is(err, relation.ErrNotExist) { + return Resource{}, err + } + if err = s.AddResourceOwner(ctx, updated); err != nil { + return Resource{}, err + } + } + + return updated, nil } func (s Service) AddProjectToResource(ctx context.Context, projectID string, res Resource) error { diff --git a/core/resource/service_test.go b/core/resource/service_test.go index bdad57694..2f1945350 100644 --- a/core/resource/service_test.go +++ b/core/resource/service_test.go @@ -468,15 +468,54 @@ func TestList(t *testing.T) { } func TestUpdate(t *testing.T) { - t.Run("delegates to repository", func(t *testing.T) { - repo, _, _, _, _, _, _, _, svc := newTestService(t) - res := resource.Resource{ID: "r1", Title: "updated"} - repo.EXPECT().Update(mock.Anything, res).Return(res, nil) + proj := project.Project{ID: uuid.New().String(), Name: "proj"} - got, err := svc.Update(context.Background(), res) + t.Run("metadata-only update does not touch relations", func(t *testing.T) { + repo, _, _, _, projectSvc, _, _, _, svc := newTestService(t) + existing := resource.Resource{ + ID: "r1", Name: "res", NamespaceID: "resource/item", + ProjectID: proj.ID, PrincipalID: "u1", PrincipalType: schema.UserPrincipal, + } + repo.EXPECT().GetByID(mock.Anything, "r1").Return(existing, nil) + projectSvc.EXPECT().Get(mock.Anything, proj.ID).Return(proj, nil) + // owner unchanged, project unchanged -> repository update only, no relation calls + updated := existing + updated.Title = "updated" + repo.EXPECT().Update(mock.Anything, mock.Anything).Return(updated, nil) + + got, err := svc.Update(context.Background(), resource.Resource{ + ID: "r1", Title: "updated", ProjectID: proj.ID, + PrincipalID: "u1", PrincipalType: schema.UserPrincipal, + }) assert.NoError(t, err) assert.Equal(t, "updated", got.Title) }) + + t.Run("moving project reconciles the #project relation", func(t *testing.T) { + repo, _, relationSvc, _, projectSvc, _, _, _, svc := newTestService(t) + newProj := project.Project{ID: uuid.New().String(), Name: "new-proj"} + existing := resource.Resource{ + ID: "r1", Name: "res", NamespaceID: "resource/item", + ProjectID: proj.ID, PrincipalID: "u1", PrincipalType: schema.UserPrincipal, + } + updated := existing + updated.ProjectID = newProj.ID + repo.EXPECT().GetByID(mock.Anything, "r1").Return(existing, nil) + projectSvc.EXPECT().Get(mock.Anything, newProj.ID).Return(newProj, nil) + repo.EXPECT().Update(mock.Anything, mock.Anything).Return(updated, nil) + // old #project tuple removed, new one written + relationSvc.EXPECT().Delete(mock.Anything, relation.Relation{ + Object: relation.Object{ID: "r1", Namespace: "resource/item"}, + RelationName: schema.ProjectRelationName, + }).Return(nil) + relationSvc.EXPECT().Create(mock.Anything, mock.Anything).Return(relation.Relation{}, nil) + + _, err := svc.Update(context.Background(), resource.Resource{ + ID: "r1", ProjectID: newProj.ID, + PrincipalID: "u1", PrincipalType: schema.UserPrincipal, + }) + assert.NoError(t, err) + }) } func TestDelete(t *testing.T) { diff --git a/internal/store/postgres/resource_repository.go b/internal/store/postgres/resource_repository.go index f9112ef2b..aa228d485 100644 --- a/internal/store/postgres/resource_repository.go +++ b/internal/store/postgres/resource_repository.go @@ -186,8 +186,12 @@ func (r ResourceRepository) Update(ctx context.Context, res resource.Resource) ( } query, params, err := dialect.Update(TABLE_RESOURCES).Set( goqu.Record{ - "title": res.Title, - "metadata": marshaledMetadata, + "title": res.Title, + "metadata": marshaledMetadata, + "urn": res.URN, + "project_id": res.ProjectID, + "principal_id": res.PrincipalID, + "principal_type": res.PrincipalType, }, ).Where(goqu.Ex{"id": res.ID}).Returning(&ResourceCols{}).ToSQL() if err != nil { diff --git a/test/e2e/regression/api_test.go b/test/e2e/regression/api_test.go index c5aa5d4c9..a29e76923 100644 --- a/test/e2e/regression/api_test.go +++ b/test/e2e/regression/api_test.go @@ -2894,6 +2894,68 @@ func (s *APIRegressionTestSuite) TestOrganizationRoleDeleteInUse() { s.Assert().False(roleHasPermTuples()) } +// TestProjectResourceUpdateReconcile asserts that moving a resource to another +// project reconciles its #project SpiceDB relation: the new project tuple is +// written and the old one is removed (gap #1661.7). +func (s *APIRegressionTestSuite) TestProjectResourceUpdateReconcile() { + ctxOrgAdminAuth := testbench.ContextWithAuth(context.Background(), s.adminCookie) + + createOrgResp, err := s.testBench.Client.CreateOrganization(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateOrganizationRequest{ + Body: &frontierv1beta1.OrganizationRequestBody{Name: "org-resource-reconcile"}, + })) + s.Require().NoError(err) + orgID := createOrgResp.Msg.GetOrganization().GetId() + + projA, err := s.testBench.Client.CreateProject(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateProjectRequest{ + Body: &frontierv1beta1.ProjectRequestBody{Name: "resource-reconcile-proj-a", OrgId: orgID}, + })) + s.Require().NoError(err) + projB, err := s.testBench.Client.CreateProject(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateProjectRequest{ + Body: &frontierv1beta1.ProjectRequestBody{Name: "resource-reconcile-proj-b", OrgId: orgID}, + })) + s.Require().NoError(err) + + createResourceResp, err := s.testBench.Client.CreateProjectResource(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateProjectResourceRequest{ + ProjectId: projA.Msg.GetProject().GetId(), + Body: &frontierv1beta1.ResourceRequestBody{ + Name: "reconcile-res", + Namespace: computeOrderNamespace, + }, + })) + s.Require().NoError(err) + resourceID := createResourceResp.Msg.GetResource().GetId() + + projectSubjectOf := func() string { + resp, err := s.testBench.AdminClient.ListRelations(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.ListRelationsRequest{ + Object: schema.JoinNamespaceAndResourceID(computeOrderNamespace, resourceID), + })) + s.Require().NoError(err) + for _, rel := range resp.Msg.GetRelations() { + if rel.GetRelation() == schema.ProjectRelationName { + return rel.GetSubject() + } + } + return "" + } + + // before the move the resource points at project A + s.Assert().Equal(schema.JoinNamespaceAndResourceID(schema.ProjectNamespace, projA.Msg.GetProject().GetId()), projectSubjectOf()) + + // move the resource to project B + _, err = s.testBench.Client.UpdateProjectResource(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.UpdateProjectResourceRequest{ + Id: resourceID, + ProjectId: projB.Msg.GetProject().GetId(), + Body: &frontierv1beta1.ResourceRequestBody{ + Name: "reconcile-res", + Namespace: computeOrderNamespace, + }, + })) + s.Require().NoError(err) + + // the #project relation now points at project B, and only B (old tuple gone) + s.Assert().Equal(schema.JoinNamespaceAndResourceID(schema.ProjectNamespace, projB.Msg.GetProject().GetId()), projectSubjectOf()) +} + func TestEndToEndAPIRegressionTestSuite(t *testing.T) { suite.Run(t, new(APIRegressionTestSuite)) } From d6a4f382ac557a92e2cd87d62c84ceef2d200445 Mon Sep 17 00:00:00 2001 From: Abhishek Sah Date: Thu, 11 Jun 2026 14:12:18 +0530 Subject: [PATCH 2/2] test(resource): update repo Update fixture for full-resource write The resource repository Update now persists urn, project_id, principal_id, and principal_type (not just title/metadata), so the test must pass a complete resource. Passing an empty principal_id wrote an invalid value into the uuid column and failed. Refs #1661. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/store/postgres/resource_repository_test.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/internal/store/postgres/resource_repository_test.go b/internal/store/postgres/resource_repository_test.go index 9e651eaca..ead7739c5 100644 --- a/internal/store/postgres/resource_repository_test.go +++ b/internal/store/postgres/resource_repository_test.go @@ -356,10 +356,13 @@ func (s *ResourceRepositoryTestSuite) TestUpdate() { Description: "should update a resource", ResourceID: s.resources[0].ID, ResourceToUpdate: resource.Resource{ - ID: s.resources[0].ID, - Name: "resource-1", - ProjectID: s.resources[0].ProjectID, - NamespaceID: s.resources[0].NamespaceID, + ID: s.resources[0].ID, + URN: "resource-1-urn", + Name: "resource-1", + ProjectID: s.resources[0].ProjectID, + NamespaceID: s.resources[0].NamespaceID, + PrincipalID: s.resources[0].PrincipalID, + PrincipalType: s.resources[0].PrincipalType, }, ExpectedResource: resource.Resource{ ID: s.resources[0].ID,