From 01ed773f6a7ebdd0f641ad845121f80af39b4eea Mon Sep 17 00:00:00 2001 From: Muhammad Abduh Date: Tue, 14 Jul 2026 11:26:21 +0700 Subject: [PATCH] feat: add soft-delete tombstone (deleted_at/deleted_by) for resources Delete API now stamps deleted_at/deleted_by immediately on the resources row, and all read paths (GetByURN, List) exclude tombstoned resources by default via partial indexes, while the background sync worker still bypasses the filter so in-flight teardown can finish. Co-Authored-By: Claude Sonnet 5 --- core/mocks/resource_store.go | 50 +++++++++++++++++++- core/resource/resource.go | 3 ++ core/write.go | 11 +++-- core/write_test.go | 7 ++- internal/server/v1/mocks/resource_service.go | 23 ++++----- internal/server/v1/resources/server.go | 8 +++- internal/server/v1/resources/server_test.go | 10 ++-- internal/store/postgres/resource_model.go | 35 +++++++++++--- internal/store/postgres/resource_store.go | 49 ++++++++++++++++++- internal/store/postgres/schema.sql | 9 +++- 10 files changed, 174 insertions(+), 31 deletions(-) diff --git a/core/mocks/resource_store.go b/core/mocks/resource_store.go index df3aebe5..e9e78e80 100644 --- a/core/mocks/resource_store.go +++ b/core/mocks/resource_store.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.42.1. DO NOT EDIT. +// Code generated by mockery v2.53.3. DO NOT EDIT. package mocks @@ -324,6 +324,54 @@ func (_c *ResourceStore_Revisions_Call) RunAndReturn(run func(context.Context, r return _c } +// SoftDelete provides a mock function with given fields: ctx, urn, deletedBy +func (_m *ResourceStore) SoftDelete(ctx context.Context, urn string, deletedBy string) error { + ret := _m.Called(ctx, urn, deletedBy) + + if len(ret) == 0 { + panic("no return value specified for SoftDelete") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string, string) error); ok { + r0 = rf(ctx, urn, deletedBy) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// ResourceStore_SoftDelete_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'SoftDelete' +type ResourceStore_SoftDelete_Call struct { + *mock.Call +} + +// SoftDelete is a helper method to define mock.On call +// - ctx context.Context +// - urn string +// - deletedBy string +func (_e *ResourceStore_Expecter) SoftDelete(ctx interface{}, urn interface{}, deletedBy interface{}) *ResourceStore_SoftDelete_Call { + return &ResourceStore_SoftDelete_Call{Call: _e.mock.On("SoftDelete", ctx, urn, deletedBy)} +} + +func (_c *ResourceStore_SoftDelete_Call) Run(run func(ctx context.Context, urn string, deletedBy string)) *ResourceStore_SoftDelete_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(string), args[2].(string)) + }) + return _c +} + +func (_c *ResourceStore_SoftDelete_Call) Return(_a0 error) *ResourceStore_SoftDelete_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *ResourceStore_SoftDelete_Call) RunAndReturn(run func(context.Context, string, string) error) *ResourceStore_SoftDelete_Call { + _c.Call.Return(run) + return _c +} + // SyncOne provides a mock function with given fields: ctx, scope, syncFn func (_m *ResourceStore) SyncOne(ctx context.Context, scope map[string][]string, syncFn resource.SyncFn) error { ret := _m.Called(ctx, scope, syncFn) diff --git a/core/resource/resource.go b/core/resource/resource.go index 32879d48..c4d794c7 100644 --- a/core/resource/resource.go +++ b/core/resource/resource.go @@ -24,6 +24,7 @@ type Store interface { Create(ctx context.Context, r Resource, hooks ...MutationHook) error Update(ctx context.Context, r Resource, saveRevision bool, reason string, hooks ...MutationHook) error Delete(ctx context.Context, urn string, hooks ...MutationHook) error + SoftDelete(ctx context.Context, urn string, deletedBy string) error Revisions(ctx context.Context, selector RevisionsSelector) ([]Revision, error) @@ -48,6 +49,8 @@ type Resource struct { UpdatedAt time.Time `json:"updated_at"` UpdatedBy string `json:"updated_by"` CreatedBy string `json:"created_by"` + DeletedAt *time.Time `json:"deleted_at,omitempty"` + DeletedBy string `json:"deleted_by,omitempty"` Spec Spec `json:"spec"` State State `json:"state"` } diff --git a/core/write.go b/core/write.go index 55ba5d0f..348ab7e2 100644 --- a/core/write.go +++ b/core/write.go @@ -57,11 +57,16 @@ func (svc *Service) UpdateResource(ctx context.Context, urn string, req resource }, resourceOpts...) } -func (svc *Service) DeleteResource(ctx context.Context, urn string) error { +func (svc *Service) DeleteResource(ctx context.Context, urn string, deletedBy string) error { _, actionErr := svc.ApplyAction(ctx, urn, module.ActionRequest{ - Name: module.DeleteAction, + Name: module.DeleteAction, + UserID: deletedBy, }, WithDryRun(false)) - return actionErr + if actionErr != nil { + return actionErr + } + + return svc.store.SoftDelete(ctx, urn, deletedBy) } func (svc *Service) ApplyAction(ctx context.Context, urn string, act module.ActionRequest, resourceOpts ...Options) (*resource.Resource, error) { diff --git a/core/write_test.go b/core/write_test.go index fd419bb4..3bbfae3f 100644 --- a/core/write_test.go +++ b/core/write_test.go @@ -703,6 +703,11 @@ func TestService_DeleteResource(t *testing.T) { Return(nil). Once() + resourceRepo.EXPECT(). + SoftDelete(mock.Anything, "orn:entropy:mock:foo:bar", "test-user"). + Return(nil). + Once() + return core.New(resourceRepo, mod, deadClock, defaultSyncBackoff, defaultMaxRetries, serviceName) }, urn: "orn:entropy:mock:foo:bar", @@ -716,7 +721,7 @@ func TestService_DeleteResource(t *testing.T) { t.Parallel() svc := tt.setup(t) - err := svc.DeleteResource(context.Background(), tt.urn) + err := svc.DeleteResource(context.Background(), tt.urn, "test-user") if tt.wantErr != nil { assert.Error(t, err) assert.True(t, errors.Is(err, tt.wantErr)) diff --git a/internal/server/v1/mocks/resource_service.go b/internal/server/v1/mocks/resource_service.go index 404a1ef4..d3a77a1b 100644 --- a/internal/server/v1/mocks/resource_service.go +++ b/internal/server/v1/mocks/resource_service.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.43.2. DO NOT EDIT. +// Code generated by mockery v2.53.3. DO NOT EDIT. package mocks @@ -175,17 +175,17 @@ func (_c *ResourceService_CreateResource_Call) RunAndReturn(run func(context.Con return _c } -// DeleteResource provides a mock function with given fields: ctx, urn -func (_m *ResourceService) DeleteResource(ctx context.Context, urn string) error { - ret := _m.Called(ctx, urn) +// DeleteResource provides a mock function with given fields: ctx, urn, deletedBy +func (_m *ResourceService) DeleteResource(ctx context.Context, urn string, deletedBy string) error { + ret := _m.Called(ctx, urn, deletedBy) if len(ret) == 0 { panic("no return value specified for DeleteResource") } var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string) error); ok { - r0 = rf(ctx, urn) + if rf, ok := ret.Get(0).(func(context.Context, string, string) error); ok { + r0 = rf(ctx, urn, deletedBy) } else { r0 = ret.Error(0) } @@ -201,13 +201,14 @@ type ResourceService_DeleteResource_Call struct { // DeleteResource is a helper method to define mock.On call // - ctx context.Context // - urn string -func (_e *ResourceService_Expecter) DeleteResource(ctx interface{}, urn interface{}) *ResourceService_DeleteResource_Call { - return &ResourceService_DeleteResource_Call{Call: _e.mock.On("DeleteResource", ctx, urn)} +// - deletedBy string +func (_e *ResourceService_Expecter) DeleteResource(ctx interface{}, urn interface{}, deletedBy interface{}) *ResourceService_DeleteResource_Call { + return &ResourceService_DeleteResource_Call{Call: _e.mock.On("DeleteResource", ctx, urn, deletedBy)} } -func (_c *ResourceService_DeleteResource_Call) Run(run func(ctx context.Context, urn string)) *ResourceService_DeleteResource_Call { +func (_c *ResourceService_DeleteResource_Call) Run(run func(ctx context.Context, urn string, deletedBy string)) *ResourceService_DeleteResource_Call { _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(string)) + run(args[0].(context.Context), args[1].(string), args[2].(string)) }) return _c } @@ -217,7 +218,7 @@ func (_c *ResourceService_DeleteResource_Call) Return(_a0 error) *ResourceServic return _c } -func (_c *ResourceService_DeleteResource_Call) RunAndReturn(run func(context.Context, string) error) *ResourceService_DeleteResource_Call { +func (_c *ResourceService_DeleteResource_Call) RunAndReturn(run func(context.Context, string, string) error) *ResourceService_DeleteResource_Call { _c.Call.Return(run) return _c } diff --git a/internal/server/v1/resources/server.go b/internal/server/v1/resources/server.go index 35a66efb..b091ced8 100644 --- a/internal/server/v1/resources/server.go +++ b/internal/server/v1/resources/server.go @@ -17,7 +17,7 @@ type ResourceService interface { ListResources(ctx context.Context, filter resource.Filter, withSpecConfigs bool) (resource.PagedResource, error) CreateResource(ctx context.Context, res resource.Resource, resourceOpts ...core.Options) (*resource.Resource, error) UpdateResource(ctx context.Context, urn string, req resource.UpdateRequest, resourceOpts ...core.Options) (*resource.Resource, error) - DeleteResource(ctx context.Context, urn string) error + DeleteResource(ctx context.Context, urn string, deletedBy string) error ApplyAction(ctx context.Context, urn string, action module.ActionRequest, resourceOpts ...core.Options) (*resource.Resource, error) GetLog(ctx context.Context, urn string, filter map[string]string) (<-chan module.LogChunk, error) @@ -144,11 +144,15 @@ func (server APIServer) ListResources(ctx context.Context, request *entropyv1bet } func (server APIServer) DeleteResource(ctx context.Context, request *entropyv1beta1.DeleteResourceRequest) (*entropyv1beta1.DeleteResourceResponse, error) { - err := server.resourceSvc.DeleteResource(ctx, request.GetUrn()) + userIdentifier, err := serverutils.GetUserIdentifier(ctx) if err != nil { return nil, serverutils.ToRPCError(err) } + if err := server.resourceSvc.DeleteResource(ctx, request.GetUrn(), userIdentifier); err != nil { + return nil, serverutils.ToRPCError(err) + } + return &entropyv1beta1.DeleteResourceResponse{}, nil } diff --git a/internal/server/v1/resources/server_test.go b/internal/server/v1/resources/server_test.go index 92f7ef46..72df896a 100644 --- a/internal/server/v1/resources/server_test.go +++ b/internal/server/v1/resources/server_test.go @@ -524,7 +524,7 @@ func TestAPIServer_DeleteResource(t *testing.T) { t.Helper() resourceService := &mocks.ResourceService{} resourceService.EXPECT(). - DeleteResource(mock.Anything, "p-testdata-gl-testname-log"). + DeleteResource(mock.Anything, "p-testdata-gl-testname-log", "john.doe@goto.com"). Return(errors.ErrNotFound).Once() return NewAPIServer(resourceService) }, @@ -540,7 +540,7 @@ func TestAPIServer_DeleteResource(t *testing.T) { t.Helper() resourceService := &mocks.ResourceService{} resourceService.EXPECT(). - DeleteResource(mock.Anything, "p-testdata-gl-testname-log"). + DeleteResource(mock.Anything, "p-testdata-gl-testname-log", "john.doe@goto.com"). Return(nil).Once() return NewAPIServer(resourceService) @@ -558,7 +558,11 @@ func TestAPIServer_DeleteResource(t *testing.T) { t.Parallel() srv := tt.setup(t) - got, err := srv.DeleteResource(context.Background(), tt.request) + ctx := context.Background() + md := metadata.New(map[string]string{"user-id": "john.doe@goto.com"}) + ctx = metadata.NewIncomingContext(ctx, md) + + got, err := srv.DeleteResource(ctx, tt.request) if tt.wantErr != nil { assert.Error(t, err) assert.Truef(t, errors.Is(err, tt.wantErr), "'%s' != '%s'", tt.wantErr, err) diff --git a/internal/store/postgres/resource_model.go b/internal/store/postgres/resource_model.go index 18876394..b38c1bfc 100644 --- a/internal/store/postgres/resource_model.go +++ b/internal/store/postgres/resource_model.go @@ -13,7 +13,7 @@ import ( "github.com/goto/entropy/pkg/errors" ) -const listResourceByFilterQuery = `SELECT r.id, r.urn, r.kind, r.name, r.project, r.created_at, r.updated_at, r.state_status, r.state_output, r.state_module_data, r.state_next_sync, r.state_sync_result, r.created_by, r.updated_by, +const listResourceByFilterQuery = `SELECT r.id, r.urn, r.kind, r.name, r.project, r.created_at, r.updated_at, r.state_status, r.state_output, r.state_module_data, r.state_next_sync, r.state_sync_result, r.created_by, r.updated_by, r.deleted_at, r.deleted_by, COALESCE(NULLIF(array_agg(rt.tag), '{NULL}'), '{}')::text[] AS tags, jsonb_object_agg(COALESCE(rd.dependency_key, ''), d.urn) AS dependencies FROM resources r @@ -22,12 +22,13 @@ FROM resources r LEFT JOIN resource_tags rt ON r.id = rt.resource_id WHERE ($1 = '' OR r.project = $1) AND ($2 = '' OR r.kind = $2) + AND r.deleted_at IS NULL GROUP BY r.id LIMIT $3 OFFSET $4 ` -const listResourceWithSpecConfigsByFilterQuery = `SELECT r.id, r.urn, r.kind, r.name, r.project, r.created_at, r.updated_at, r.spec_configs, r.state_status, r.state_output, r.state_module_data, r.state_next_sync, r.state_sync_result, r.created_by, r.updated_by, +const listResourceWithSpecConfigsByFilterQuery = `SELECT r.id, r.urn, r.kind, r.name, r.project, r.created_at, r.updated_at, r.spec_configs, r.state_status, r.state_output, r.state_module_data, r.state_next_sync, r.state_sync_result, r.created_by, r.updated_by, r.deleted_at, r.deleted_by, COALESCE(NULLIF(array_agg(rt.tag), '{NULL}'), '{}')::text[] AS tags, jsonb_object_agg(COALESCE(rd.dependency_key, ''), d.urn) AS dependencies FROM resources r @@ -36,6 +37,7 @@ FROM resources r LEFT JOIN resource_tags rt ON r.id = rt.resource_id WHERE ($1 = '' OR r.project = $1) AND ($2 = '' OR r.kind = $2) + AND r.deleted_at IS NULL GROUP BY r.id LIMIT $3 OFFSET $4 @@ -57,6 +59,8 @@ type resourceModel struct { StateModuleData []byte `db:"state_module_data"` StateNextSync *time.Time `db:"state_next_sync"` StateSyncResult json.RawMessage `db:"state_sync_result"` + DeletedAt *time.Time `db:"deleted_at"` + DeletedBy sql.NullString `db:"deleted_by"` } type ListResourceByFilterRow struct { @@ -75,6 +79,8 @@ type ListResourceByFilterRow struct { StateSyncResult []byte CreatedBy string UpdatedBy string + DeletedAt *time.Time + DeletedBy sql.NullString Tags pq.StringArray Dependencies []byte } @@ -110,6 +116,8 @@ func listResourceWithSpecConfigsByFilter(ctx context.Context, db *sqlx.DB, proje &i.StateSyncResult, &i.CreatedBy, &i.UpdatedBy, + &i.DeletedAt, + &i.DeletedBy, &i.Tags, &i.Dependencies, ); err != nil { @@ -153,6 +161,8 @@ func listResourceByFilter(ctx context.Context, db *sqlx.DB, project, kind string &i.StateSyncResult, &i.CreatedBy, &i.UpdatedBy, + &i.DeletedAt, + &i.DeletedBy, &i.Tags, &i.Dependencies, ); err != nil { @@ -166,13 +176,16 @@ func listResourceByFilter(ctx context.Context, db *sqlx.DB, project, kind string return items, nil } -func readResourceRecord(ctx context.Context, r sqlx.QueryerContext, urn string, into *resourceModel) error { +func readResourceRecord(ctx context.Context, r sqlx.QueryerContext, urn string, includeDeleted bool, into *resourceModel) error { cols := []string{ "id", "urn", "kind", "project", "name", "created_at", "updated_at", "created_by", "updated_by", "spec_configs", "state_status", "state_output", "state_module_data", - "state_next_sync", "state_sync_result", + "state_next_sync", "state_sync_result", "deleted_at", "deleted_by", } builder := sq.Select(cols...).From(tableResources).Where(sq.Eq{"urn": urn}) + if !includeDeleted { + builder = builder.Where(sq.Expr("deleted_at IS NULL")) + } query, args, err := builder.PlaceholderFormat(sq.Dollar).ToSql() if err != nil { @@ -218,10 +231,18 @@ func readResourceDeps(ctx context.Context, r sq.BaseRunner, id int64, into map[s } func translateURNToID(ctx context.Context, r sq.BaseRunner, urn string) (int64, error) { - row := sq.Select("id"). + return translateURNToIDInternal(ctx, r, urn, false) +} + +func translateURNToIDInternal(ctx context.Context, r sq.BaseRunner, urn string, includeDeleted bool) (int64, error) { + q := sq.Select("id"). From(tableResources). - Where(sq.Eq{"urn": urn}). - PlaceholderFormat(sq.Dollar). + Where(sq.Eq{"urn": urn}) + if !includeDeleted { + q = q.Where(sq.Expr("deleted_at IS NULL")) + } + + row := q.PlaceholderFormat(sq.Dollar). RunWith(r). QueryRowContext(ctx) diff --git a/internal/store/postgres/resource_store.go b/internal/store/postgres/resource_store.go index 51751450..a277a51a 100644 --- a/internal/store/postgres/resource_store.go +++ b/internal/store/postgres/resource_store.go @@ -17,12 +17,23 @@ import ( ) func (st *Store) GetByURN(ctx context.Context, urn string) (*resource.Resource, error) { + return st.getByURN(ctx, urn, false) +} + +// getByURNIncludingDeleted is used internally by the sync worker, which must +// keep being able to fetch resources that have already been tombstoned so +// in-flight teardown work can finish. +func (st *Store) getByURNIncludingDeleted(ctx context.Context, urn string) (*resource.Resource, error) { + return st.getByURN(ctx, urn, true) +} + +func (st *Store) getByURN(ctx context.Context, urn string, includeDeleted bool) (*resource.Resource, error) { var rec resourceModel var tags []string deps := map[string]string{} readResourceParts := func(ctx context.Context, tx *sqlx.Tx) error { - if err := readResourceRecord(ctx, tx, urn, &rec); err != nil { + if err := readResourceRecord(ctx, tx, urn, includeDeleted, &rec); err != nil { return err } @@ -68,6 +79,8 @@ func (st *Store) GetByURN(ctx context.Context, urn string) (*resource.Resource, UpdatedAt: rec.UpdatedAt, CreatedBy: rec.CreatedBy, UpdatedBy: rec.UpdatedBy, + DeletedAt: rec.DeletedAt, + DeletedBy: rec.DeletedBy.String, Spec: resource.Spec{ Configs: rec.SpecConfigs, Dependencies: deps, @@ -141,6 +154,8 @@ func (st *Store) List(ctx context.Context, filter resource.Filter, withSpecConfi UpdatedAt: *res.UpdatedAt, UpdatedBy: res.UpdatedBy, CreatedBy: res.CreatedBy, + DeletedAt: res.DeletedAt, + DeletedBy: res.DeletedBy.String, Spec: resource.Spec{ Configs: res.SpecConfigs, Dependencies: deps, @@ -268,6 +283,36 @@ func (st *Store) Update(ctx context.Context, r resource.Resource, saveRevision b return nil } +func (st *Store) SoftDelete(ctx context.Context, urn string, deletedBy string) error { + ctx = otelsql.WithCustomAttributes( + ctx, + []attribute.KeyValue{ + attribute.String("db.repository.method", "SoftDelete"), + attribute.String(string(semconv.DBSQLTableKey), tableResources), + }..., + ) + + res, err := sq.Update(tableResources). + Set("deleted_at", sq.Expr("current_timestamp")). + Set("deleted_by", deletedBy). + Where(sq.Eq{"urn": urn}). + Where(sq.Expr("deleted_at IS NULL")). + PlaceholderFormat(sq.Dollar). + RunWith(st.db). + ExecContext(ctx) + if err != nil { + return err + } + + affected, err := res.RowsAffected() + if err != nil { + return err + } else if affected == 0 { + return errors.ErrNotFound.WithMsgf("resource with urn '%s' does not exist", urn) + } + return nil +} + func (st *Store) Delete(ctx context.Context, urn string, hooks ...resource.MutationHook) error { deleteFn := func(ctx context.Context, tx *sqlx.Tx) error { id, err := translateURNToID(ctx, tx, urn) @@ -325,7 +370,7 @@ func (st *Store) SyncOne(ctx context.Context, scope map[string][]string, syncFn return err } - cur, err := st.GetByURN(ctx, urn) + cur, err := st.getByURNIncludingDeleted(ctx, urn) if err != nil { return err } diff --git a/internal/store/postgres/schema.sql b/internal/store/postgres/schema.sql index baa5ba7d..d0608e7c 100644 --- a/internal/store/postgres/schema.sql +++ b/internal/store/postgres/schema.sql @@ -79,4 +79,11 @@ ALTER TABLE revisions ADD COLUMN IF NOT EXISTS created_by TEXT NOT NULL DEFAULT ALTER TABLE revision_tags DROP CONSTRAINT revision_tags_revision_id_fkey, ADD CONSTRAINT revision_tags_revision_id_fkey FOREIGN KEY (revision_id) - REFERENCES revisions (id) ON DELETE CASCADE; \ No newline at end of file + REFERENCES revisions (id) ON DELETE CASCADE; + +ALTER TABLE resources + ADD COLUMN IF NOT EXISTS deleted_at timestamptz, + ADD COLUMN IF NOT EXISTS deleted_by TEXT; + +CREATE INDEX IF NOT EXISTS idx_resources_project_kind_active ON resources (project, kind) WHERE deleted_at IS NULL; +CREATE INDEX IF NOT EXISTS idx_resources_next_sync_active ON resources (state_next_sync) WHERE deleted_at IS NULL; \ No newline at end of file