Skip to content

Commit

Permalink
[management] fix groups delete and resource create and update error r…
Browse files Browse the repository at this point in the history
…esponse (#3189)
  • Loading branch information
pascal-fischer authored Jan 16, 2025
1 parent 992a6c7 commit c6f7a29
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 8 deletions.
6 changes: 3 additions & 3 deletions management/server/group.go
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ func validateDeleteGroup(ctx context.Context, transaction store.Store, group *ty
if group.Issued == types.GroupIssuedIntegration {
executingUser, err := transaction.GetUserByUserID(ctx, store.LockingStrengthShare, userID)
if err != nil {
return err
return status.Errorf(status.Internal, "failed to get user")
}
if executingUser.Role != types.UserRoleAdmin || !executingUser.IsServiceUser {
return status.Errorf(status.PermissionDenied, "only service users with admin power can delete integration group")
Expand Down Expand Up @@ -505,7 +505,7 @@ func validateDeleteGroup(ctx context.Context, transaction store.Store, group *ty
func checkGroupLinkedToSettings(ctx context.Context, transaction store.Store, group *types.Group) error {
dnsSettings, err := transaction.GetAccountDNSSettings(ctx, store.LockingStrengthShare, group.AccountID)
if err != nil {
return err
return status.Errorf(status.Internal, "failed to get DNS settings")
}

if slices.Contains(dnsSettings.DisabledManagementGroups, group.ID) {
Expand All @@ -514,7 +514,7 @@ func checkGroupLinkedToSettings(ctx context.Context, transaction store.Store, gr

settings, err := transaction.GetAccountSettings(ctx, store.LockingStrengthShare, group.AccountID)
if err != nil {
return err
return status.Errorf(status.Internal, "failed to get account settings")
}

if settings.Extra != nil && slices.Contains(settings.Extra.IntegratedValidatorGroups, group.ID) {
Expand Down
5 changes: 3 additions & 2 deletions management/server/http/handlers/groups/groups_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,8 +239,9 @@ func (h *handler) deleteGroup(w http.ResponseWriter, r *http.Request) {

err = h.accountManager.DeleteGroup(r.Context(), accountID, userID, groupID)
if err != nil {
_, ok := err.(*server.GroupLinkError)
if ok {
wrappedErr, ok := err.(interface{ Unwrap() []error })
if ok && len(wrappedErr.Unwrap()) > 0 {
err = wrappedErr.Unwrap()[0]
util.WriteErrorResponse(err.Error(), http.StatusBadRequest, w)
return
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"io"
"net"
Expand Down Expand Up @@ -73,10 +74,12 @@ func initGroupTestData(initGroups ...*types.Group) *handler {
},
DeleteGroupFunc: func(_ context.Context, accountID, userId, groupID string) error {
if groupID == "linked-grp" {
return &server.GroupLinkError{
err := &server.GroupLinkError{
Resource: "something",
Name: "linked-grp",
}
var allErrors error
return errors.Join(allErrors, err)
}
if groupID == "invalid-grp" {
return fmt.Errorf("internal error")
Expand Down
4 changes: 2 additions & 2 deletions management/server/networks/resources/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func (m *managerImpl) CreateResource(ctx context.Context, userID string, resourc
err = m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error {
_, err = transaction.GetNetworkResourceByName(ctx, store.LockingStrengthShare, resource.AccountID, resource.Name)
if err == nil {
return errors.New("resource already exists")
return status.Errorf(status.InvalidArgument, "resource with name %s already exists", resource.Name)
}

network, err := transaction.GetNetworkByID(ctx, store.LockingStrengthUpdate, resource.AccountID, resource.NetworkID)
Expand Down Expand Up @@ -223,7 +223,7 @@ func (m *managerImpl) UpdateResource(ctx context.Context, userID string, resourc

oldResource, err := transaction.GetNetworkResourceByName(ctx, store.LockingStrengthShare, resource.AccountID, resource.Name)
if err == nil && oldResource.ID != resource.ID {
return errors.New("new resource name already exists")
return status.Errorf(status.InvalidArgument, "new resource name already exists")
}

oldResource, err = transaction.GetNetworkResourceByID(ctx, store.LockingStrengthShare, resource.AccountID, resource.ID)
Expand Down

0 comments on commit c6f7a29

Please sign in to comment.