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