From e0ce3671e8d4ba4e26b881320ad3f36a97aa8605 Mon Sep 17 00:00:00 2001 From: andres-portainer <91705312+andres-portainer@users.noreply.github.com> Date: Mon, 10 Apr 2023 19:03:51 -0300 Subject: [PATCH] fix(tags): migrate to transactional code EE-5330 (#8755) --- api/http/handler/tags/handler.go | 16 +++++- api/http/handler/tags/tag_create.go | 33 ++++++++---- api/http/handler/tags/tag_delete.go | 78 ++++++++++++++++++++--------- 3 files changed, 94 insertions(+), 33 deletions(-) diff --git a/api/http/handler/tags/handler.go b/api/http/handler/tags/handler.go index f79f70e6f..e1dd9423c 100644 --- a/api/http/handler/tags/handler.go +++ b/api/http/handler/tags/handler.go @@ -3,10 +3,12 @@ package tags import ( "net/http" - "github.com/gorilla/mux" httperror "github.com/portainer/libhttp/error" + "github.com/portainer/libhttp/response" "github.com/portainer/portainer/api/dataservices" "github.com/portainer/portainer/api/http/security" + + "github.com/gorilla/mux" ) // Handler is the HTTP handler used to handle tag operations. @@ -29,3 +31,15 @@ func NewHandler(bouncer *security.RequestBouncer) *Handler { return h } + +func txResponse(w http.ResponseWriter, r any, err error) *httperror.HandlerError { + if err != nil { + if httpErr, ok := err.(*httperror.HandlerError); ok { + return httpErr + } + + return httperror.InternalServerError("Unexpected error", err) + } + + return response.JSON(w, r) +} diff --git a/api/http/handler/tags/tag_create.go b/api/http/handler/tags/tag_create.go index 06ee233e6..a4ece0e49 100644 --- a/api/http/handler/tags/tag_create.go +++ b/api/http/handler/tags/tag_create.go @@ -7,19 +7,20 @@ import ( "github.com/asaskevich/govalidator" httperror "github.com/portainer/libhttp/error" "github.com/portainer/libhttp/request" - "github.com/portainer/libhttp/response" portainer "github.com/portainer/portainer/api" + "github.com/portainer/portainer/api/dataservices" + "github.com/portainer/portainer/pkg/featureflags" ) type tagCreatePayload struct { - // Name Name string `validate:"required" example:"org/acme"` } func (payload *tagCreatePayload) Validate(r *http.Request) error { if govalidator.IsNull(payload.Name) { - return errors.New("Invalid tag name") + return errors.New("invalid tag name") } + return nil } @@ -44,14 +45,28 @@ func (handler *Handler) tagCreate(w http.ResponseWriter, r *http.Request) *httpe return httperror.BadRequest("Invalid request payload", err) } - tags, err := handler.DataStore.Tag().Tags() + var tag *portainer.Tag + if featureflags.IsEnabled(portainer.FeatureNoTx) { + tag, err = createTag(handler.DataStore, payload) + } else { + err = handler.DataStore.UpdateTx(func(tx dataservices.DataStoreTx) error { + tag, err = createTag(tx, payload) + return err + }) + } + + return txResponse(w, tag, err) +} + +func createTag(tx dataservices.DataStoreTx, payload tagCreatePayload) (*portainer.Tag, error) { + tags, err := tx.Tag().Tags() if err != nil { - return httperror.InternalServerError("Unable to retrieve tags from the database", err) + return nil, httperror.InternalServerError("Unable to retrieve tags from the database", err) } for _, tag := range tags { if tag.Name == payload.Name { - return &httperror.HandlerError{StatusCode: http.StatusConflict, Message: "This name is already associated to a tag", Err: errors.New("A tag already exists with this name")} + return nil, &httperror.HandlerError{StatusCode: http.StatusConflict, Message: "This name is already associated to a tag", Err: errors.New("a tag already exists with this name")} } } @@ -61,10 +76,10 @@ func (handler *Handler) tagCreate(w http.ResponseWriter, r *http.Request) *httpe Endpoints: map[portainer.EndpointID]bool{}, } - err = handler.DataStore.Tag().Create(tag) + err = tx.Tag().Create(tag) if err != nil { - return httperror.InternalServerError("Unable to persist the tag inside the database", err) + return nil, httperror.InternalServerError("Unable to persist the tag inside the database", err) } - return response.JSON(w, tag) + return tag, nil } diff --git a/api/http/handler/tags/tag_delete.go b/api/http/handler/tags/tag_delete.go index 40a0efd9f..2e8107956 100644 --- a/api/http/handler/tags/tag_delete.go +++ b/api/http/handler/tags/tag_delete.go @@ -7,7 +7,9 @@ import ( "github.com/portainer/libhttp/request" "github.com/portainer/libhttp/response" portainer "github.com/portainer/portainer/api" + "github.com/portainer/portainer/api/dataservices" "github.com/portainer/portainer/api/internal/edge" + "github.com/portainer/portainer/pkg/featureflags" ) // @id TagDelete @@ -29,89 +31,119 @@ func (handler *Handler) tagDelete(w http.ResponseWriter, r *http.Request) *httpe if err != nil { return httperror.BadRequest("Invalid tag identifier route variable", err) } - tagID := portainer.TagID(id) - tag, err := handler.DataStore.Tag().Tag(tagID) - if handler.DataStore.IsErrObjectNotFound(err) { + if featureflags.IsEnabled(portainer.FeatureNoTx) { + err = deleteTag(handler.DataStore, portainer.TagID(id)) + } else { + err = handler.DataStore.UpdateTx(func(tx dataservices.DataStoreTx) error { + return deleteTag(tx, portainer.TagID(id)) + }) + } + + if err != nil { + if httpErr, ok := err.(*httperror.HandlerError); ok { + return httpErr + } + + return httperror.InternalServerError("Unexpected error", err) + } + + return response.Empty(w) +} + +func deleteTag(tx dataservices.DataStoreTx, tagID portainer.TagID) error { + tag, err := tx.Tag().Tag(tagID) + if tx.IsErrObjectNotFound(err) { return httperror.NotFound("Unable to find a tag with the specified identifier inside the database", err) } else if err != nil { return httperror.InternalServerError("Unable to find a tag with the specified identifier inside the database", err) } for endpointID := range tag.Endpoints { - endpoint, err := handler.DataStore.Endpoint().Endpoint(endpointID) + endpoint, err := tx.Endpoint().Endpoint(endpointID) if err != nil { return httperror.InternalServerError("Unable to retrieve environment from the database", err) } endpoint.TagIDs = removeElement(endpoint.TagIDs, tagID) - err = handler.DataStore.Endpoint().UpdateEndpoint(endpoint.ID, endpoint) + err = tx.Endpoint().UpdateEndpoint(endpoint.ID, endpoint) if err != nil { return httperror.InternalServerError("Unable to update environment", err) } } for endpointGroupID := range tag.EndpointGroups { - endpointGroup, err := handler.DataStore.EndpointGroup().EndpointGroup(endpointGroupID) + endpointGroup, err := tx.EndpointGroup().EndpointGroup(endpointGroupID) if err != nil { return httperror.InternalServerError("Unable to retrieve environment group from the database", err) } endpointGroup.TagIDs = removeElement(endpointGroup.TagIDs, tagID) - err = handler.DataStore.EndpointGroup().UpdateEndpointGroup(endpointGroup.ID, endpointGroup) + err = tx.EndpointGroup().UpdateEndpointGroup(endpointGroup.ID, endpointGroup) if err != nil { return httperror.InternalServerError("Unable to update environment group", err) } } - endpoints, err := handler.DataStore.Endpoint().Endpoints() + endpoints, err := tx.Endpoint().Endpoints() if err != nil { return httperror.InternalServerError("Unable to retrieve environments from the database", err) } - edgeGroups, err := handler.DataStore.EdgeGroup().EdgeGroups() + edgeGroups, err := tx.EdgeGroup().EdgeGroups() if err != nil { return httperror.InternalServerError("Unable to retrieve edge groups from the database", err) } - edgeStacks, err := handler.DataStore.EdgeStack().EdgeStacks() + edgeStacks, err := tx.EdgeStack().EdgeStacks() if err != nil { return httperror.InternalServerError("Unable to retrieve edge stacks from the database", err) } for _, endpoint := range endpoints { if (tag.Endpoints[endpoint.ID] || tag.EndpointGroups[endpoint.GroupID]) && (endpoint.Type == portainer.EdgeAgentOnDockerEnvironment || endpoint.Type == portainer.EdgeAgentOnKubernetesEnvironment) { - err = handler.updateEndpointRelations(endpoint, edgeGroups, edgeStacks) + err = updateEndpointRelations(tx, endpoint, edgeGroups, edgeStacks) if err != nil { return httperror.InternalServerError("Unable to update environment relations in the database", err) } } } - for _, edgeGroup := range edgeGroups { - err = handler.DataStore.EdgeGroup().UpdateEdgeGroupFunc(edgeGroup.ID, func(g *portainer.EdgeGroup) { - g.TagIDs = removeElement(g.TagIDs, tagID) - }) - if err != nil { - return httperror.InternalServerError("Unable to update edge group", err) + if featureflags.IsEnabled(portainer.FeatureNoTx) { + for _, edgeGroup := range edgeGroups { + err = tx.EdgeGroup().UpdateEdgeGroupFunc(edgeGroup.ID, func(g *portainer.EdgeGroup) { + g.TagIDs = removeElement(g.TagIDs, tagID) + }) + if err != nil { + return httperror.InternalServerError("Unable to update edge group", err) + } + } + } else { + for _, edgeGroup := range edgeGroups { + edgeGroup.TagIDs = removeElement(edgeGroup.TagIDs, tagID) + + err = tx.EdgeGroup().UpdateEdgeGroup(edgeGroup.ID, &edgeGroup) + if err != nil { + return httperror.InternalServerError("Unable to update edge group", err) + } } } - err = handler.DataStore.Tag().DeleteTag(tagID) + err = tx.Tag().DeleteTag(tagID) if err != nil { return httperror.InternalServerError("Unable to remove the tag from the database", err) } - return response.Empty(w) + return nil } -func (handler *Handler) updateEndpointRelations(endpoint portainer.Endpoint, edgeGroups []portainer.EdgeGroup, edgeStacks []portainer.EdgeStack) error { - endpointRelation, err := handler.DataStore.EndpointRelation().EndpointRelation(endpoint.ID) +func updateEndpointRelations(tx dataservices.DataStoreTx, endpoint portainer.Endpoint, edgeGroups []portainer.EdgeGroup, edgeStacks []portainer.EdgeStack) error { + endpointRelation, err := tx.EndpointRelation().EndpointRelation(endpoint.ID) if err != nil { return err } - endpointGroup, err := handler.DataStore.EndpointGroup().EndpointGroup(endpoint.GroupID) + endpointGroup, err := tx.EndpointGroup().EndpointGroup(endpoint.GroupID) if err != nil { return err } @@ -123,7 +155,7 @@ func (handler *Handler) updateEndpointRelations(endpoint portainer.Endpoint, edg } endpointRelation.EdgeStacks = stacksSet - return handler.DataStore.EndpointRelation().UpdateEndpointRelation(endpoint.ID, endpointRelation) + return tx.EndpointRelation().UpdateEndpointRelation(endpoint.ID, endpointRelation) } func removeElement(slice []portainer.TagID, elem portainer.TagID) []portainer.TagID {