diff --git a/lib/services/local/access_monitoring_rules.go b/lib/services/local/access_monitoring_rules.go index 7ab5f196f976f..60200a835afea 100644 --- a/lib/services/local/access_monitoring_rules.go +++ b/lib/services/local/access_monitoring_rules.go @@ -85,7 +85,7 @@ func (s *AccessMonitoringRulesService) CreateAccessMonitoringRule(ctx context.Co // UpdateAccessMonitoringRule updates an existing AccessMonitoringRule resource. func (s *AccessMonitoringRulesService) UpdateAccessMonitoringRule(ctx context.Context, amr *accessmonitoringrulesv1.AccessMonitoringRule) (*accessmonitoringrulesv1.AccessMonitoringRule, error) { - updated, err := s.svc.UpdateResource(ctx, amr) + updated, err := s.svc.UnconditionalUpdateResource(ctx, amr) return updated, trace.Wrap(err) } diff --git a/lib/services/local/databaseobject.go b/lib/services/local/databaseobject.go index 4d422ef6d9351..ab5d874e5f388 100644 --- a/lib/services/local/databaseobject.go +++ b/lib/services/local/databaseobject.go @@ -45,7 +45,7 @@ func (s *DatabaseObjectService) UpsertDatabaseObject(ctx context.Context, object } func (s *DatabaseObjectService) UpdateDatabaseObject(ctx context.Context, object *dbobjectv1.DatabaseObject) (*dbobjectv1.DatabaseObject, error) { - out, err := s.service.UpdateResource(ctx, object) + out, err := s.service.UnconditionalUpdateResource(ctx, object) return out, trace.Wrap(err) } diff --git a/lib/services/local/databaseobjectimportrule.go b/lib/services/local/databaseobjectimportrule.go index 87fdafdfae87c..8ab3c1f310929 100644 --- a/lib/services/local/databaseobjectimportrule.go +++ b/lib/services/local/databaseobjectimportrule.go @@ -43,7 +43,7 @@ func (s *databaseObjectImportRuleService) UpsertDatabaseObjectImportRule(ctx con } func (s *databaseObjectImportRuleService) UpdateDatabaseObjectImportRule(ctx context.Context, rule *databaseobjectimportrulev1.DatabaseObjectImportRule) (*databaseobjectimportrulev1.DatabaseObjectImportRule, error) { - out, err := s.service.UpdateResource(ctx, rule) + out, err := s.service.UnconditionalUpdateResource(ctx, rule) return out, trace.Wrap(err) } diff --git a/lib/services/local/generic/generic_wrapper.go b/lib/services/local/generic/generic_wrapper.go index 4bb3a1673b427..076b05caace3f 100644 --- a/lib/services/local/generic/generic_wrapper.go +++ b/lib/services/local/generic/generic_wrapper.go @@ -125,14 +125,20 @@ func (s ServiceWrapper[T]) UpsertResource(ctx context.Context, resource T) (T, e return adapter.resource, trace.Wrap(err) } -// UpdateResource updates an existing resource. -func (s ServiceWrapper[T]) UpdateResource(ctx context.Context, resource T) (T, error) { +// UnconditionalUpdateResource updates an existing resource without checking the provided resource revision. +// Because UnconditionalUpdateResource can blindly overwrite an existing item, ConditionalUpdateResource should +// be preferred. +// See https://github.com/gravitational/teleport/blob/master/rfd/0153-resource-guidelines.md#update-1 for more details +// about the Update operation. +func (s ServiceWrapper[T]) UnconditionalUpdateResource(ctx context.Context, resource T) (T, error) { adapter, err := s.service.UpdateResource(ctx, newResourceMetadataAdapter(resource)) return adapter.resource, trace.Wrap(err) } // ConditionalUpdateResource updates an existing resource if the provided // resource and the existing resource have matching revisions. +// See https://github.com/gravitational/teleport/blob/master/rfd/0126-backend-migrations.md#optimistic-locking for more +// details about the conditional update. func (s ServiceWrapper[T]) ConditionalUpdateResource(ctx context.Context, resource T) (T, error) { adapter, err := s.service.ConditionalUpdateResource(ctx, newResourceMetadataAdapter(resource)) return adapter.resource, trace.Wrap(err) diff --git a/lib/services/local/generic/generic_wrapper_test.go b/lib/services/local/generic/generic_wrapper_test.go index 197bbdb5ecd77..672bb2a88dee8 100644 --- a/lib/services/local/generic/generic_wrapper_test.go +++ b/lib/services/local/generic/generic_wrapper_test.go @@ -180,7 +180,7 @@ func TestGenericWrapperCRUD(t *testing.T) { // Update a resource. r1.Metadata.Labels = map[string]string{"newlabel": "newvalue"} - r1, err = service.UpdateResource(ctx, r1) + r1, err = service.UnconditionalUpdateResource(ctx, r1) require.NoError(t, err) r, err = service.GetResource(ctx, r1.GetMetadata().GetName()) require.NoError(t, err) @@ -198,7 +198,7 @@ func TestGenericWrapperCRUD(t *testing.T) { // Update a resource that doesn't exist. doesNotExist := newTestResource153("doesnotexist") - _, err = service.UpdateResource(ctx, doesNotExist) + _, err = service.UnconditionalUpdateResource(ctx, doesNotExist) require.True(t, trace.IsNotFound(err)) // Delete a resource.