Skip to content

Commit 76e7d0c

Browse files
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/<ns>:<id>#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) <noreply@anthropic.com>
1 parent 3430fcd commit 76e7d0c

4 files changed

Lines changed: 188 additions & 9 deletions

File tree

core/resource/service.go

Lines changed: 76 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,8 +170,82 @@ func (s Service) List(ctx context.Context, flt Filter) ([]Resource, error) {
170170
return s.repository.List(ctx, flt)
171171
}
172172

173-
func (s Service) Update(ctx context.Context, resource Resource) (Resource, error) {
174-
return s.repository.Update(ctx, resource)
173+
// Update persists a resource change and reconciles its #project / #owner
174+
// SpiceDB relations. Previously Update only touched title/metadata in the DB
175+
// and never reconciled relations, so moving a resource to a new project or
176+
// reassigning its owner silently left the old #project/#owner tuples in place
177+
// (and never wrote the new ones). Name and namespace stay immutable here; the
178+
// URN tracks the (possibly new) project name.
179+
func (s Service) Update(ctx context.Context, res Resource) (Resource, error) {
180+
existing, err := s.repository.GetByID(ctx, res.ID)
181+
if err != nil {
182+
return Resource{}, err
183+
}
184+
185+
principalID := res.PrincipalID
186+
principalType := res.PrincipalType
187+
// PAT → resolve to underlying user, mirroring Create
188+
if principalType == schema.PATPrincipal {
189+
sub, err := s.resolvePATUser(ctx, principalID)
190+
if err != nil {
191+
return Resource{}, fmt.Errorf("resolving PAT principal: %w", err)
192+
}
193+
principalID = sub.ID
194+
principalType = sub.Namespace
195+
}
196+
// no owner supplied → keep the current one
197+
if strings.TrimSpace(principalID) == "" {
198+
principalID = existing.PrincipalID
199+
principalType = existing.PrincipalType
200+
}
201+
202+
resourceProject, err := s.projectService.Get(ctx, res.ProjectID)
203+
if err != nil {
204+
return Resource{}, fmt.Errorf("failed to get project: %w", err)
205+
}
206+
207+
updated, err := s.repository.Update(ctx, Resource{
208+
ID: existing.ID,
209+
URN: existing.CreateURN(resourceProject.Name),
210+
Name: existing.Name,
211+
Title: res.Title,
212+
ProjectID: resourceProject.ID,
213+
NamespaceID: existing.NamespaceID,
214+
PrincipalID: principalID,
215+
PrincipalType: principalType,
216+
Metadata: res.Metadata,
217+
})
218+
if err != nil {
219+
return Resource{}, err
220+
}
221+
222+
// reconcile the project grant if the resource moved projects
223+
if existing.ProjectID != updated.ProjectID {
224+
if err = s.relationService.Delete(ctx, relation.Relation{
225+
Object: relation.Object{ID: updated.ID, Namespace: updated.NamespaceID},
226+
RelationName: schema.ProjectRelationName,
227+
}); err != nil && !errors.Is(err, relation.ErrNotExist) {
228+
return Resource{}, err
229+
}
230+
if err = s.AddProjectToResource(ctx, updated.ProjectID, updated); err != nil {
231+
return Resource{}, err
232+
}
233+
}
234+
235+
// reconcile the owner grant if the owner changed
236+
if existing.PrincipalID != updated.PrincipalID || existing.PrincipalType != updated.PrincipalType {
237+
if err = s.relationService.Delete(ctx, relation.Relation{
238+
Object: relation.Object{ID: updated.ID, Namespace: updated.NamespaceID},
239+
RelationName: schema.OwnerRelationName,
240+
}); err != nil && !errors.Is(err, relation.ErrNotExist) {
241+
return Resource{}, err
242+
}
243+
if err = s.AddResourceOwner(ctx, updated); err != nil {
244+
return Resource{}, err
245+
}
246+
}
247+
248+
return updated, nil
175249
}
176250

177251
func (s Service) AddProjectToResource(ctx context.Context, projectID string, res Resource) error {

core/resource/service_test.go

Lines changed: 44 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -468,15 +468,54 @@ func TestList(t *testing.T) {
468468
}
469469

470470
func TestUpdate(t *testing.T) {
471-
t.Run("delegates to repository", func(t *testing.T) {
472-
repo, _, _, _, _, _, _, _, svc := newTestService(t)
473-
res := resource.Resource{ID: "r1", Title: "updated"}
474-
repo.EXPECT().Update(mock.Anything, res).Return(res, nil)
471+
proj := project.Project{ID: uuid.New().String(), Name: "proj"}
475472

476-
got, err := svc.Update(context.Background(), res)
473+
t.Run("metadata-only update does not touch relations", func(t *testing.T) {
474+
repo, _, _, _, projectSvc, _, _, _, svc := newTestService(t)
475+
existing := resource.Resource{
476+
ID: "r1", Name: "res", NamespaceID: "resource/item",
477+
ProjectID: proj.ID, PrincipalID: "u1", PrincipalType: schema.UserPrincipal,
478+
}
479+
repo.EXPECT().GetByID(mock.Anything, "r1").Return(existing, nil)
480+
projectSvc.EXPECT().Get(mock.Anything, proj.ID).Return(proj, nil)
481+
// owner unchanged, project unchanged -> repository update only, no relation calls
482+
updated := existing
483+
updated.Title = "updated"
484+
repo.EXPECT().Update(mock.Anything, mock.Anything).Return(updated, nil)
485+
486+
got, err := svc.Update(context.Background(), resource.Resource{
487+
ID: "r1", Title: "updated", ProjectID: proj.ID,
488+
PrincipalID: "u1", PrincipalType: schema.UserPrincipal,
489+
})
477490
assert.NoError(t, err)
478491
assert.Equal(t, "updated", got.Title)
479492
})
493+
494+
t.Run("moving project reconciles the #project relation", func(t *testing.T) {
495+
repo, _, relationSvc, _, projectSvc, _, _, _, svc := newTestService(t)
496+
newProj := project.Project{ID: uuid.New().String(), Name: "new-proj"}
497+
existing := resource.Resource{
498+
ID: "r1", Name: "res", NamespaceID: "resource/item",
499+
ProjectID: proj.ID, PrincipalID: "u1", PrincipalType: schema.UserPrincipal,
500+
}
501+
updated := existing
502+
updated.ProjectID = newProj.ID
503+
repo.EXPECT().GetByID(mock.Anything, "r1").Return(existing, nil)
504+
projectSvc.EXPECT().Get(mock.Anything, newProj.ID).Return(newProj, nil)
505+
repo.EXPECT().Update(mock.Anything, mock.Anything).Return(updated, nil)
506+
// old #project tuple removed, new one written
507+
relationSvc.EXPECT().Delete(mock.Anything, relation.Relation{
508+
Object: relation.Object{ID: "r1", Namespace: "resource/item"},
509+
RelationName: schema.ProjectRelationName,
510+
}).Return(nil)
511+
relationSvc.EXPECT().Create(mock.Anything, mock.Anything).Return(relation.Relation{}, nil)
512+
513+
_, err := svc.Update(context.Background(), resource.Resource{
514+
ID: "r1", ProjectID: newProj.ID,
515+
PrincipalID: "u1", PrincipalType: schema.UserPrincipal,
516+
})
517+
assert.NoError(t, err)
518+
})
480519
}
481520

482521
func TestDelete(t *testing.T) {

internal/store/postgres/resource_repository.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,8 +186,12 @@ func (r ResourceRepository) Update(ctx context.Context, res resource.Resource) (
186186
}
187187
query, params, err := dialect.Update(TABLE_RESOURCES).Set(
188188
goqu.Record{
189-
"title": res.Title,
190-
"metadata": marshaledMetadata,
189+
"title": res.Title,
190+
"metadata": marshaledMetadata,
191+
"urn": res.URN,
192+
"project_id": res.ProjectID,
193+
"principal_id": res.PrincipalID,
194+
"principal_type": res.PrincipalType,
191195
},
192196
).Where(goqu.Ex{"id": res.ID}).Returning(&ResourceCols{}).ToSQL()
193197
if err != nil {

test/e2e/regression/api_test.go

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2788,6 +2788,68 @@ func (s *APIRegressionTestSuite) TestWebhookAPI() {
27882788
})
27892789
}
27902790

2791+
// TestProjectResourceUpdateReconcile asserts that moving a resource to another
2792+
// project reconciles its #project SpiceDB relation: the new project tuple is
2793+
// written and the old one is removed (gap #1661.7).
2794+
func (s *APIRegressionTestSuite) TestProjectResourceUpdateReconcile() {
2795+
ctxOrgAdminAuth := testbench.ContextWithAuth(context.Background(), s.adminCookie)
2796+
2797+
createOrgResp, err := s.testBench.Client.CreateOrganization(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateOrganizationRequest{
2798+
Body: &frontierv1beta1.OrganizationRequestBody{Name: "org-resource-reconcile"},
2799+
}))
2800+
s.Require().NoError(err)
2801+
orgID := createOrgResp.Msg.GetOrganization().GetId()
2802+
2803+
projA, err := s.testBench.Client.CreateProject(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateProjectRequest{
2804+
Body: &frontierv1beta1.ProjectRequestBody{Name: "resource-reconcile-proj-a", OrgId: orgID},
2805+
}))
2806+
s.Require().NoError(err)
2807+
projB, err := s.testBench.Client.CreateProject(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateProjectRequest{
2808+
Body: &frontierv1beta1.ProjectRequestBody{Name: "resource-reconcile-proj-b", OrgId: orgID},
2809+
}))
2810+
s.Require().NoError(err)
2811+
2812+
createResourceResp, err := s.testBench.Client.CreateProjectResource(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateProjectResourceRequest{
2813+
ProjectId: projA.Msg.GetProject().GetId(),
2814+
Body: &frontierv1beta1.ResourceRequestBody{
2815+
Name: "reconcile-res",
2816+
Namespace: computeOrderNamespace,
2817+
},
2818+
}))
2819+
s.Require().NoError(err)
2820+
resourceID := createResourceResp.Msg.GetResource().GetId()
2821+
2822+
projectSubjectOf := func() string {
2823+
resp, err := s.testBench.AdminClient.ListRelations(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.ListRelationsRequest{
2824+
Object: schema.JoinNamespaceAndResourceID(computeOrderNamespace, resourceID),
2825+
}))
2826+
s.Require().NoError(err)
2827+
for _, rel := range resp.Msg.GetRelations() {
2828+
if rel.GetRelation() == schema.ProjectRelationName {
2829+
return rel.GetSubject()
2830+
}
2831+
}
2832+
return ""
2833+
}
2834+
2835+
// before the move the resource points at project A
2836+
s.Assert().Equal(schema.JoinNamespaceAndResourceID(schema.ProjectNamespace, projA.Msg.GetProject().GetId()), projectSubjectOf())
2837+
2838+
// move the resource to project B
2839+
_, err = s.testBench.Client.UpdateProjectResource(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.UpdateProjectResourceRequest{
2840+
Id: resourceID,
2841+
ProjectId: projB.Msg.GetProject().GetId(),
2842+
Body: &frontierv1beta1.ResourceRequestBody{
2843+
Name: "reconcile-res",
2844+
Namespace: computeOrderNamespace,
2845+
},
2846+
}))
2847+
s.Require().NoError(err)
2848+
2849+
// the #project relation now points at project B, and only B (old tuple gone)
2850+
s.Assert().Equal(schema.JoinNamespaceAndResourceID(schema.ProjectNamespace, projB.Msg.GetProject().GetId()), projectSubjectOf())
2851+
}
2852+
27912853
func TestEndToEndAPIRegressionTestSuite(t *testing.T) {
27922854
suite.Run(t, new(APIRegressionTestSuite))
27932855
}

0 commit comments

Comments
 (0)