fix(tags): remove a data race EE-4310 (#7862)

pull/7865/head
andres-portainer 2022-10-13 11:12:12 -03:00 committed by GitHub
parent 8f1ac38963
commit 367f3dd6d4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 90 additions and 66 deletions

View File

@ -24,6 +24,7 @@ type Connection interface {
SetServiceName(bucketName string) error
GetObject(bucketName string, key []byte, object interface{}) error
UpdateObject(bucketName string, key []byte, object interface{}) error
UpdateObjectFunc(bucketName string, key []byte, object any, updateFn func()) error
DeleteObject(bucketName string, key []byte) error
DeleteAllObjects(bucketName string, matching func(o interface{}) (id int, ok bool)) error
GetNextIdentifier(bucketName string) int

View File

@ -179,7 +179,7 @@ func (connection *DbConnection) ConvertToKey(v int) []byte {
return b
}
// CreateBucket is a generic function used to create a bucket inside a database database.
// CreateBucket is a generic function used to create a bucket inside a database.
func (connection *DbConnection) SetServiceName(bucketName string) error {
return connection.Batch(func(tx *bolt.Tx) error {
_, err := tx.CreateBucketIfNotExists([]byte(bucketName))
@ -187,7 +187,7 @@ func (connection *DbConnection) SetServiceName(bucketName string) error {
})
}
// GetObject is a generic function used to retrieve an unmarshalled object from a database database.
// GetObject is a generic function used to retrieve an unmarshalled object from a database.
func (connection *DbConnection) GetObject(bucketName string, key []byte, object interface{}) error {
var data []byte
@ -219,7 +219,7 @@ func (connection *DbConnection) getEncryptionKey() []byte {
return connection.EncryptionKey
}
// UpdateObject is a generic function used to update an object inside a database database.
// UpdateObject is a generic function used to update an object inside a database.
func (connection *DbConnection) UpdateObject(bucketName string, key []byte, object interface{}) error {
data, err := connection.MarshalObject(object)
if err != nil {
@ -232,7 +232,33 @@ func (connection *DbConnection) UpdateObject(bucketName string, key []byte, obje
})
}
// DeleteObject is a generic function used to delete an object inside a database database.
// UpdateObjectFunc is a generic function used to update an object safely without race conditions.
func (connection *DbConnection) UpdateObjectFunc(bucketName string, key []byte, object any, updateFn func()) error {
return connection.Batch(func(tx *bolt.Tx) error {
bucket := tx.Bucket([]byte(bucketName))
data := bucket.Get(key)
if data == nil {
return dserrors.ErrObjectNotFound
}
err := connection.UnmarshalObjectWithJsoniter(data, object)
if err != nil {
return err
}
updateFn()
data, err = connection.MarshalObject(object)
if err != nil {
return err
}
return bucket.Put(key, data)
})
}
// DeleteObject is a generic function used to delete an object inside a database.
func (connection *DbConnection) DeleteObject(bucketName string, key []byte) error {
return connection.Batch(func(tx *bolt.Tx) error {
bucket := tx.Bucket([]byte(bucketName))

View File

@ -251,6 +251,7 @@ type (
Tag(ID portainer.TagID) (*portainer.Tag, error)
Create(tag *portainer.Tag) error
UpdateTag(ID portainer.TagID, tag *portainer.Tag) error
UpdateTagFunc(ID portainer.TagID, updateFunc func(tag *portainer.Tag)) error
DeleteTag(ID portainer.TagID) error
BucketName() string
}

View File

@ -80,12 +80,24 @@ func (service *Service) Create(tag *portainer.Tag) error {
)
}
// UpdateTag updates a tag.
// Deprecated: Use UpdateTagFunc instead.
func (service *Service) UpdateTag(ID portainer.TagID, tag *portainer.Tag) error {
identifier := service.connection.ConvertToKey(int(ID))
return service.connection.UpdateObject(BucketName, identifier, tag)
}
// UpdateTagFunc updates a tag inside a transaction avoiding data races.
func (service *Service) UpdateTagFunc(ID portainer.TagID, updateFunc func(tag *portainer.Tag)) error {
id := service.connection.ConvertToKey(int(ID))
tag := &portainer.Tag{}
service.connection.UpdateObjectFunc(BucketName, id, tag, func() {
updateFunc(tag)
})
return nil
}
// DeleteTag deletes a tag.
func (service *Service) DeleteTag(ID portainer.TagID) error {
identifier := service.connection.ConvertToKey(int(ID))

View File

@ -91,15 +91,13 @@ func (handler *Handler) endpointGroupCreate(w http.ResponseWriter, r *http.Reque
}
for _, tagID := range endpointGroup.TagIDs {
tag, err := handler.DataStore.Tag().Tag(tagID)
if err != nil {
return httperror.InternalServerError("Unable to retrieve tag from the database", err)
}
handler.DataStore.Tag().UpdateTagFunc(tagID, func(tag *portainer.Tag) {
tag.EndpointGroups[endpointGroup.ID] = true
})
err = handler.DataStore.Tag().UpdateTag(tagID, tag)
if err != nil {
if handler.DataStore.IsErrObjectNotFound(err) {
return httperror.InternalServerError("Unable to find a tag inside the database", err)
} else if err != nil {
return httperror.InternalServerError("Unable to persist tag changes inside the database", err)
}
}

View File

@ -66,15 +66,13 @@ func (handler *Handler) endpointGroupDelete(w http.ResponseWriter, r *http.Reque
}
for _, tagID := range endpointGroup.TagIDs {
tag, err := handler.DataStore.Tag().Tag(tagID)
if err != nil {
return httperror.InternalServerError("Unable to retrieve tag from the database", err)
}
handler.DataStore.Tag().UpdateTagFunc(tagID, func(tag *portainer.Tag) {
delete(tag.EndpointGroups, endpointGroup.ID)
})
err = handler.DataStore.Tag().UpdateTag(tagID, tag)
if err != nil {
if handler.DataStore.IsErrObjectNotFound(err) {
return httperror.InternalServerError("Unable to find a tag inside the database", err)
} else if err != nil {
return httperror.InternalServerError("Unable to persist tag changes inside the database", err)
}
}

View File

@ -81,28 +81,26 @@ func (handler *Handler) endpointGroupUpdate(w http.ResponseWriter, r *http.Reque
removeTags := tag.Difference(endpointGroupTagSet, payloadTagSet)
for tagID := range removeTags {
tag, err := handler.DataStore.Tag().Tag(tagID)
if err != nil {
return httperror.InternalServerError("Unable to find a tag inside the database", err)
}
handler.DataStore.Tag().UpdateTagFunc(tagID, func(tag *portainer.Tag) {
delete(tag.EndpointGroups, endpointGroup.ID)
err = handler.DataStore.Tag().UpdateTag(tag.ID, tag)
if err != nil {
})
if handler.DataStore.IsErrObjectNotFound(err) {
return httperror.InternalServerError("Unable to find a tag inside the database", err)
} else if err != nil {
return httperror.InternalServerError("Unable to persist tag changes inside the database", err)
}
}
endpointGroup.TagIDs = payload.TagIDs
for _, tagID := range payload.TagIDs {
tag, err := handler.DataStore.Tag().Tag(tagID)
if err != nil {
return httperror.InternalServerError("Unable to find a tag inside the database", err)
}
handler.DataStore.Tag().UpdateTagFunc(tagID, func(tag *portainer.Tag) {
tag.EndpointGroups[endpointGroup.ID] = true
})
err = handler.DataStore.Tag().UpdateTag(tag.ID, tag)
if err != nil {
if handler.DataStore.IsErrObjectNotFound(err) {
return httperror.InternalServerError("Unable to find a tag inside the database", err)
} else if err != nil {
return httperror.InternalServerError("Unable to persist tag changes inside the database", err)
}
}

View File

@ -530,14 +530,9 @@ func (handler *Handler) saveEndpointAndUpdateAuthorizations(endpoint *portainer.
}
for _, tagID := range endpoint.TagIDs {
tag, err := handler.DataStore.Tag().Tag(tagID)
if err != nil {
return err
}
err = handler.DataStore.Tag().UpdateTagFunc(tagID, func(tag *portainer.Tag) {
tag.Endpoints[endpoint.ID] = true
err = handler.DataStore.Tag().UpdateTag(tagID, tag)
})
if err != nil {
return err
}

View File

@ -62,15 +62,13 @@ func (handler *Handler) endpointDelete(w http.ResponseWriter, r *http.Request) *
}
for _, tagID := range endpoint.TagIDs {
tag, err := handler.DataStore.Tag().Tag(tagID)
if err != nil {
return httperror.NotFound("Unable to find tag inside the database", err)
}
err = handler.DataStore.Tag().UpdateTagFunc(tagID, func(tag *portainer.Tag) {
delete(tag.Endpoints, endpoint.ID)
})
err = handler.DataStore.Tag().UpdateTag(tagID, tag)
if err != nil {
if handler.DataStore.IsErrObjectNotFound(err) {
return httperror.NotFound("Unable to find tag inside the database", err)
} else if err != nil {
return httperror.InternalServerError("Unable to persist tag relation inside the database", err)
}
}

View File

@ -139,29 +139,26 @@ func (handler *Handler) endpointUpdate(w http.ResponseWriter, r *http.Request) *
removeTags := tag.Difference(endpointTagSet, payloadTagSet)
for tagID := range removeTags {
tag, err := handler.DataStore.Tag().Tag(tagID)
if err != nil {
return httperror.InternalServerError("Unable to find a tag inside the database", err)
}
err = handler.DataStore.Tag().UpdateTagFunc(tagID, func(tag *portainer.Tag) {
delete(tag.Endpoints, endpoint.ID)
err = handler.DataStore.Tag().UpdateTag(tag.ID, tag)
if err != nil {
})
if handler.DataStore.IsErrObjectNotFound(err) {
return httperror.InternalServerError("Unable to find a tag inside the database", err)
} else if err != nil {
return httperror.InternalServerError("Unable to persist tag changes inside the database", err)
}
}
endpoint.TagIDs = payload.TagIDs
for _, tagID := range payload.TagIDs {
tag, err := handler.DataStore.Tag().Tag(tagID)
if err != nil {
return httperror.InternalServerError("Unable to find a tag inside the database", err)
}
err = handler.DataStore.Tag().UpdateTagFunc(tagID, func(tag *portainer.Tag) {
tag.Endpoints[endpoint.ID] = true
})
err = handler.DataStore.Tag().UpdateTag(tag.ID, tag)
if err != nil {
if handler.DataStore.IsErrObjectNotFound(err) {
return httperror.InternalServerError("Unable to find a tag inside the database", err)
} else if err != nil {
return httperror.InternalServerError("Unable to persist tag changes inside the database", err)
}
}