diff --git a/management/server/account.go b/management/server/account.go index 4c707af3a..a0d4568ec 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -80,7 +80,6 @@ type AccountManager interface { GetUsersFromAccount(accountID, userID string) ([]*UserInfo, error) GetGroup(accountId, groupID string) (*Group, error) SaveGroup(accountID, userID string, group *Group) error - UpdateGroup(accountID string, groupID string, operations []GroupUpdateOperation) (*Group, error) DeleteGroup(accountId, userId, groupID string) error ListGroups(accountId string) ([]*Group, error) GroupAddPeer(accountId, groupID, peerID string) error @@ -93,13 +92,11 @@ type AccountManager interface { GetRoute(accountID, routeID, userID string) (*route.Route, error) CreateRoute(accountID string, prefix, peerID, description, netID string, masquerade bool, metric int, groups []string, enabled bool, userID string) (*route.Route, error) SaveRoute(accountID, userID string, route *route.Route) error - UpdateRoute(accountID, routeID string, operations []RouteUpdateOperation) (*route.Route, error) DeleteRoute(accountID, routeID, userID string) error ListRoutes(accountID, userID string) ([]*route.Route, error) GetNameServerGroup(accountID, nsGroupID string) (*nbdns.NameServerGroup, error) CreateNameServerGroup(accountID string, name, description string, nameServerList []nbdns.NameServer, groups []string, primary bool, domains []string, enabled bool, userID string) (*nbdns.NameServerGroup, error) SaveNameServerGroup(accountID, userID string, nsGroupToSave *nbdns.NameServerGroup) error - UpdateNameServerGroup(accountID, nsGroupID, userID string, operations []NameServerGroupUpdateOperation) (*nbdns.NameServerGroup, error) DeleteNameServerGroup(accountID, nsGroupID, userID string) error ListNameServerGroups(accountID string) ([]*nbdns.NameServerGroup, error) GetDNSDomain() string @@ -1605,19 +1602,3 @@ func newAccountWithId(accountID, userID, domain string) *Account { } return acc } - -func removeFromList(inputList []string, toRemove []string) []string { - toRemoveMap := make(map[string]struct{}) - for _, item := range toRemove { - toRemoveMap[item] = struct{}{} - } - - var resultList []string - for _, item := range inputList { - _, ok := toRemoveMap[item] - if !ok { - resultList = append(resultList, item) - } - } - return resultList -} diff --git a/management/server/group.go b/management/server/group.go index 5b1d2ac9f..697fe5d70 100644 --- a/management/server/group.go +++ b/management/server/group.go @@ -33,26 +33,6 @@ type Group struct { Peers []string } -const ( - // UpdateGroupName indicates a name update operation - UpdateGroupName GroupUpdateOperationType = iota - // InsertPeersToGroup indicates insert peers to group operation - InsertPeersToGroup - // RemovePeersFromGroup indicates a remove peers from group operation - RemovePeersFromGroup - // UpdateGroupPeers indicates a replacement of group peers list - UpdateGroupPeers -) - -// GroupUpdateOperationType operation type -type GroupUpdateOperationType int - -// GroupUpdateOperation operation object with type and values to be applied -type GroupUpdateOperation struct { - Type GroupUpdateOperationType - Values []string -} - // EventMeta returns activity event meta related to the group func (g *Group) EventMeta() map[string]any { return map[string]any{"name": g.Name} @@ -165,57 +145,6 @@ func difference(a, b []string) []string { return diff } -// UpdateGroup updates a group using a list of operations -func (am *DefaultAccountManager) UpdateGroup(accountID string, - groupID string, operations []GroupUpdateOperation, -) (*Group, error) { - unlock := am.Store.AcquireAccountLock(accountID) - defer unlock() - - account, err := am.Store.GetAccount(accountID) - if err != nil { - return nil, err - } - - groupToUpdate, ok := account.Groups[groupID] - if !ok { - return nil, status.Errorf(status.NotFound, "group with ID %s no longer exists", groupID) - } - - group := groupToUpdate.Copy() - - for _, operation := range operations { - switch operation.Type { - case UpdateGroupName: - group.Name = operation.Values[0] - case UpdateGroupPeers: - group.Peers = operation.Values - case InsertPeersToGroup: - sourceList := group.Peers - resultList := removeFromList(sourceList, operation.Values) - group.Peers = append(resultList, operation.Values...) - case RemovePeersFromGroup: - sourceList := group.Peers - resultList := removeFromList(sourceList, operation.Values) - group.Peers = resultList - } - } - - account.Groups[groupID] = group - - account.Network.IncSerial() - if err = am.Store.SaveAccount(account); err != nil { - return nil, err - } - - err = am.updateAccountPeers(account) - if err != nil { - return nil, err - } - - return group, nil -} - // DeleteGroup object of the peers func (am *DefaultAccountManager) DeleteGroup(accountId, userId, groupID string) error { unlock := am.Store.AcquireAccountLock(accountId) diff --git a/management/server/http/groups_handler_test.go b/management/server/http/groups_handler_test.go index 44603059a..ddb1233bf 100644 --- a/management/server/http/groups_handler_test.go +++ b/management/server/http/groups_handler_test.go @@ -53,22 +53,6 @@ func initGroupTestData(user *server.User, groups ...*server.Group) *GroupsHandle Issued: server.GroupIssuedAPI, }, nil }, - UpdateGroupFunc: func(_ string, groupID string, operations []server.GroupUpdateOperation) (*server.Group, error) { - var group server.Group - group.ID = groupID - for _, operation := range operations { - switch operation.Type { - case server.UpdateGroupName: - group.Name = operation.Values[0] - case server.UpdateGroupPeers, server.InsertPeersToGroup: - group.Peers = operation.Values - case server.RemovePeersFromGroup: - default: - return nil, fmt.Errorf("no operation") - } - } - return &group, nil - }, GetPeerByIPFunc: func(_ string, peerIP string) (*server.Peer, error) { for _, peer := range TestPeers { if peer.IP.String() == peerIP { diff --git a/management/server/http/nameservers_handler_test.go b/management/server/http/nameservers_handler_test.go index 75fcb4c1c..100f4b87a 100644 --- a/management/server/http/nameservers_handler_test.go +++ b/management/server/http/nameservers_handler_test.go @@ -88,31 +88,6 @@ func initNameserversTestData() *NameserversHandler { } return status.Errorf(status.NotFound, "nameserver group with ID %s was not found", nsGroupToSave.ID) }, - UpdateNameServerGroupFunc: func(accountID, nsGroupID, _ string, operations []server.NameServerGroupUpdateOperation) (*nbdns.NameServerGroup, error) { - nsGroupToUpdate := baseExistingNSGroup.Copy() - if nsGroupID != nsGroupToUpdate.ID { - return nil, status.Errorf(status.NotFound, "nameserver group ID %s no longer exists", nsGroupID) - } - for _, operation := range operations { - switch operation.Type { - case server.UpdateNameServerGroupName: - nsGroupToUpdate.Name = operation.Values[0] - case server.UpdateNameServerGroupDescription: - nsGroupToUpdate.Description = operation.Values[0] - case server.UpdateNameServerGroupNameServers: - var parsedNSList []nbdns.NameServer - for _, nsURL := range operation.Values { - parsed, err := nbdns.ParseNameServerURL(nsURL) - if err != nil { - return nil, err - } - parsedNSList = append(parsedNSList, parsed) - } - nsGroupToUpdate.NameServers = parsedNSList - } - } - return nsGroupToUpdate, nil - }, GetAccountFromTokenFunc: func(_ jwtclaims.AuthorizationClaims) (*server.Account, *server.User, error) { return testingNSAccount, testingAccount.Users["test_user"], nil }, diff --git a/management/server/http/routes_handler_test.go b/management/server/http/routes_handler_test.go index c4270284c..3f2b7b910 100644 --- a/management/server/http/routes_handler_test.go +++ b/management/server/http/routes_handler_test.go @@ -8,7 +8,6 @@ import ( "net/http" "net/http/httptest" "net/netip" - "strconv" "testing" "github.com/netbirdio/netbird/management/server/http/api" @@ -108,38 +107,6 @@ func initRoutesTestData() *RoutesHandler { IP: netip.MustParseAddr(existingPeerID).AsSlice(), }, nil }, - UpdateRouteFunc: func(_ string, routeID string, operations []server.RouteUpdateOperation) (*route.Route, error) { - routeToUpdate := baseExistingRoute - if routeID != routeToUpdate.ID { - return nil, status.Errorf(status.NotFound, "route %s no longer exists", routeID) - } - for _, operation := range operations { - switch operation.Type { - case server.UpdateRouteNetwork: - routeToUpdate.NetworkType, routeToUpdate.Network, _ = route.ParseNetwork(operation.Values[0]) - case server.UpdateRouteDescription: - routeToUpdate.Description = operation.Values[0] - case server.UpdateRouteNetworkIdentifier: - routeToUpdate.NetID = operation.Values[0] - case server.UpdateRoutePeer: - routeToUpdate.Peer = operation.Values[0] - if routeToUpdate.Peer == notFoundPeerID { - return nil, status.Errorf(status.InvalidArgument, "peer with ID %s not found", routeToUpdate.Peer) - } - case server.UpdateRouteMetric: - routeToUpdate.Metric, _ = strconv.Atoi(operation.Values[0]) - case server.UpdateRouteMasquerade: - routeToUpdate.Masquerade, _ = strconv.ParseBool(operation.Values[0]) - case server.UpdateRouteEnabled: - routeToUpdate.Enabled, _ = strconv.ParseBool(operation.Values[0]) - case server.UpdateRouteGroups: - routeToUpdate.Groups = operation.Values - default: - return nil, fmt.Errorf("no operation") - } - } - return routeToUpdate, nil - }, GetAccountFromTokenFunc: func(_ jwtclaims.AuthorizationClaims) (*server.Account, *server.User, error) { return testingAccount, testingAccount.Users["test_user"], nil }, diff --git a/management/server/mock_server/account_mock.go b/management/server/mock_server/account_mock.go index 4bfa922c7..24bf9f3c9 100644 --- a/management/server/mock_server/account_mock.go +++ b/management/server/mock_server/account_mock.go @@ -31,7 +31,6 @@ type MockAccountManager struct { AddPeerFunc func(setupKey string, userId string, peer *server.Peer) (*server.Peer, *server.NetworkMap, error) GetGroupFunc func(accountID, groupID string) (*server.Group, error) SaveGroupFunc func(accountID, userID string, group *server.Group) error - UpdateGroupFunc func(accountID string, groupID string, operations []server.GroupUpdateOperation) (*server.Group, error) DeleteGroupFunc func(accountID, userId, groupID string) error ListGroupsFunc func(accountID string) ([]*server.Group, error) GroupAddPeerFunc func(accountID, groupID, peerKey string) error @@ -54,7 +53,6 @@ type MockAccountManager struct { CreateRouteFunc func(accountID string, prefix, peer, description, netID string, masquerade bool, metric int, groups []string, enabled bool, userID string) (*route.Route, error) GetRouteFunc func(accountID, routeID, userID string) (*route.Route, error) SaveRouteFunc func(accountID, userID string, route *route.Route) error - UpdateRouteFunc func(accountID string, routeID string, operations []server.RouteUpdateOperation) (*route.Route, error) DeleteRouteFunc func(accountID, routeID, userID string) error ListRoutesFunc func(accountID, userID string) ([]*route.Route, error) SaveSetupKeyFunc func(accountID string, key *server.SetupKey, userID string) (*server.SetupKey, error) @@ -68,7 +66,6 @@ type MockAccountManager struct { GetNameServerGroupFunc func(accountID, nsGroupID string) (*nbdns.NameServerGroup, error) CreateNameServerGroupFunc func(accountID string, name, description string, nameServerList []nbdns.NameServer, groups []string, primary bool, domains []string, enabled bool, userID string) (*nbdns.NameServerGroup, error) SaveNameServerGroupFunc func(accountID, userID string, nsGroupToSave *nbdns.NameServerGroup) error - UpdateNameServerGroupFunc func(accountID, nsGroupID, userID string, operations []server.NameServerGroupUpdateOperation) (*nbdns.NameServerGroup, error) DeleteNameServerGroupFunc func(accountID, nsGroupID, userID string) error ListNameServerGroupsFunc func(accountID string) ([]*nbdns.NameServerGroup, error) CreateUserFunc func(accountID, userID string, key *server.UserInfo) (*server.UserInfo, error) @@ -267,14 +264,6 @@ func (am *MockAccountManager) SaveGroup(accountID, userID string, group *server. return status.Errorf(codes.Unimplemented, "method SaveGroup is not implemented") } -// UpdateGroup mock implementation of UpdateGroup from server.AccountManager interface -func (am *MockAccountManager) UpdateGroup(accountID string, groupID string, operations []server.GroupUpdateOperation) (*server.Group, error) { - if am.UpdateGroupFunc != nil { - return am.UpdateGroupFunc(accountID, groupID, operations) - } - return nil, status.Errorf(codes.Unimplemented, "method UpdateGroup not implemented") -} - // DeleteGroup mock implementation of DeleteGroup from server.AccountManager interface func (am *MockAccountManager) DeleteGroup(accountId, userId, groupID string) error { if am.DeleteGroupFunc != nil { @@ -435,14 +424,6 @@ func (am *MockAccountManager) SaveRoute(accountID, userID string, route *route.R return status.Errorf(codes.Unimplemented, "method SaveRoute is not implemented") } -// UpdateRoute mock implementation of UpdateRoute from server.AccountManager interface -func (am *MockAccountManager) UpdateRoute(accountID, ruleID string, operations []server.RouteUpdateOperation) (*route.Route, error) { - if am.UpdateRouteFunc != nil { - return am.UpdateRouteFunc(accountID, ruleID, operations) - } - return nil, status.Errorf(codes.Unimplemented, "method UpdateRoute not implemented") -} - // DeleteRoute mock implementation of DeleteRoute from server.AccountManager interface func (am *MockAccountManager) DeleteRoute(accountID, routeID, userID string) error { if am.DeleteRouteFunc != nil { @@ -533,14 +514,6 @@ func (am *MockAccountManager) SaveNameServerGroup(accountID, userID string, nsGr return nil } -// UpdateNameServerGroup mocks UpdateNameServerGroup of the AccountManager interface -func (am *MockAccountManager) UpdateNameServerGroup(accountID, nsGroupID, userID string, operations []server.NameServerGroupUpdateOperation) (*nbdns.NameServerGroup, error) { - if am.UpdateNameServerGroupFunc != nil { - return am.UpdateNameServerGroupFunc(accountID, nsGroupID, userID, operations) - } - return nil, nil -} - // DeleteNameServerGroup mocks DeleteNameServerGroup of the AccountManager interface func (am *MockAccountManager) DeleteNameServerGroup(accountID, nsGroupID, userID string) error { if am.DeleteNameServerGroupFunc != nil { diff --git a/management/server/nameserver.go b/management/server/nameserver.go index eb2127945..7025388ba 100644 --- a/management/server/nameserver.go +++ b/management/server/nameserver.go @@ -3,7 +3,6 @@ package server import ( "errors" "regexp" - "strconv" "unicode/utf8" "github.com/miekg/dns" @@ -15,54 +14,7 @@ import ( "github.com/netbirdio/netbird/management/server/status" ) -const ( - // UpdateNameServerGroupName indicates a nameserver group name update operation - UpdateNameServerGroupName NameServerGroupUpdateOperationType = iota - // UpdateNameServerGroupDescription indicates a nameserver group description update operation - UpdateNameServerGroupDescription - // UpdateNameServerGroupNameServers indicates a nameserver group nameservers list update operation - UpdateNameServerGroupNameServers - // UpdateNameServerGroupGroups indicates a nameserver group' groups update operation - UpdateNameServerGroupGroups - // UpdateNameServerGroupEnabled indicates a nameserver group status update operation - UpdateNameServerGroupEnabled - // UpdateNameServerGroupPrimary indicates a nameserver group primary status update operation - UpdateNameServerGroupPrimary - // UpdateNameServerGroupDomains indicates a nameserver group' domains update operation - UpdateNameServerGroupDomains - - domainPattern = `^(?i)[a-z0-9]+([\-\.]{1}[a-z0-9]+)*\.[a-z]{2,}$` -) - -// NameServerGroupUpdateOperationType operation type -type NameServerGroupUpdateOperationType int - -func (t NameServerGroupUpdateOperationType) String() string { - switch t { - case UpdateNameServerGroupDescription: - return "UpdateNameServerGroupDescription" - case UpdateNameServerGroupName: - return "UpdateNameServerGroupName" - case UpdateNameServerGroupNameServers: - return "UpdateNameServerGroupNameServers" - case UpdateNameServerGroupGroups: - return "UpdateNameServerGroupGroups" - case UpdateNameServerGroupEnabled: - return "UpdateNameServerGroupEnabled" - case UpdateNameServerGroupPrimary: - return "UpdateNameServerGroupPrimary" - case UpdateNameServerGroupDomains: - return "UpdateNameServerGroupDomains" - default: - return "InvalidOperation" - } -} - -// NameServerGroupUpdateOperation operation object with type and values to be applied -type NameServerGroupUpdateOperation struct { - Type NameServerGroupUpdateOperationType - Values []string -} +const domainPattern = `^(?i)[a-z0-9]+([\-\.]{1}[a-z0-9]+)*\.[a-z]{2,}$` // GetNameServerGroup gets a nameserver group object from account and nameserver group IDs func (am *DefaultAccountManager) GetNameServerGroup(accountID, nsGroupID string) (*nbdns.NameServerGroup, error) { @@ -172,109 +124,6 @@ func (am *DefaultAccountManager) SaveNameServerGroup(accountID, userID string, n return nil } -// UpdateNameServerGroup updates existing nameserver group with set of operations -func (am *DefaultAccountManager) UpdateNameServerGroup(accountID, nsGroupID, userID string, operations []NameServerGroupUpdateOperation) (*nbdns.NameServerGroup, error) { - - unlock := am.Store.AcquireAccountLock(accountID) - defer unlock() - - account, err := am.Store.GetAccount(accountID) - if err != nil { - return nil, err - } - - if len(operations) == 0 { - return nil, status.Errorf(status.InvalidArgument, "operations shouldn't be empty") - } - - nsGroupToUpdate, ok := account.NameServerGroups[nsGroupID] - if !ok { - return nil, status.Errorf(status.NotFound, "nameserver group ID %s no longer exists", nsGroupID) - } - - newNSGroup := nsGroupToUpdate.Copy() - - for _, operation := range operations { - valuesCount := len(operation.Values) - if valuesCount < 1 { - return nil, status.Errorf(status.InvalidArgument, "operation %s contains invalid number of values, it should be at least 1", operation.Type.String()) - } - - for _, value := range operation.Values { - if value == "" { - return nil, status.Errorf(status.InvalidArgument, "operation %s contains invalid empty string value", operation.Type.String()) - } - } - switch operation.Type { - case UpdateNameServerGroupDescription: - newNSGroup.Description = operation.Values[0] - case UpdateNameServerGroupName: - if valuesCount > 1 { - return nil, status.Errorf(status.InvalidArgument, "failed to parse name values, expected 1 value got %d", valuesCount) - } - err = validateNSGroupName(operation.Values[0], nsGroupID, account.NameServerGroups) - if err != nil { - return nil, err - } - newNSGroup.Name = operation.Values[0] - case UpdateNameServerGroupNameServers: - var nsList []nbdns.NameServer - for _, url := range operation.Values { - ns, err := nbdns.ParseNameServerURL(url) - if err != nil { - return nil, err - } - nsList = append(nsList, ns) - } - err = validateNSList(nsList) - if err != nil { - return nil, err - } - newNSGroup.NameServers = nsList - case UpdateNameServerGroupGroups: - err = validateGroups(operation.Values, account.Groups) - if err != nil { - return nil, err - } - newNSGroup.Groups = operation.Values - case UpdateNameServerGroupEnabled: - enabled, err := strconv.ParseBool(operation.Values[0]) - if err != nil { - return nil, status.Errorf(status.InvalidArgument, "failed to parse enabled %s, not boolean", operation.Values[0]) - } - newNSGroup.Enabled = enabled - case UpdateNameServerGroupPrimary: - primary, err := strconv.ParseBool(operation.Values[0]) - if err != nil { - return nil, status.Errorf(status.InvalidArgument, "failed to parse primary status %s, not boolean", operation.Values[0]) - } - newNSGroup.Primary = primary - case UpdateNameServerGroupDomains: - err = validateDomainInput(false, operation.Values) - if err != nil { - return nil, err - } - newNSGroup.Domains = operation.Values - } - } - - account.NameServerGroups[nsGroupID] = newNSGroup - - account.Network.IncSerial() - err = am.Store.SaveAccount(account) - if err != nil { - return nil, err - } - - err = am.updateAccountPeers(account) - if err != nil { - log.Error(err) - return newNSGroup.Copy(), status.Errorf(status.Internal, "failed to update peers after update nameserver %s", newNSGroup.Name) - } - - return newNSGroup.Copy(), nil -} - // DeleteNameServerGroup deletes nameserver group with nsGroupID func (am *DefaultAccountManager) DeleteNameServerGroup(accountID, nsGroupID, userID string) error { diff --git a/management/server/nameserver_test.go b/management/server/nameserver_test.go index 9d4425056..ab3edaed4 100644 --- a/management/server/nameserver_test.go +++ b/management/server/nameserver_test.go @@ -655,323 +655,6 @@ func TestSaveNameServerGroup(t *testing.T) { } } -func TestUpdateNameServerGroup(t *testing.T) { - nsGroupID := "testingNSGroup" - - existingNSGroup := &nbdns.NameServerGroup{ - ID: nsGroupID, - Name: "super", - Description: "super", - Primary: true, - NameServers: []nbdns.NameServer{ - { - IP: netip.MustParseAddr("1.1.1.1"), - NSType: nbdns.UDPNameServerType, - Port: nbdns.DefaultDNSPort, - }, - { - IP: netip.MustParseAddr("1.1.2.2"), - NSType: nbdns.UDPNameServerType, - Port: nbdns.DefaultDNSPort, - }, - }, - Groups: []string{group1ID}, - Enabled: true, - } - - testCases := []struct { - name string - existingNSGroup *nbdns.NameServerGroup - nsGroupID string - operations []NameServerGroupUpdateOperation - shouldCreate bool - errFunc require.ErrorAssertionFunc - expectedNSGroup *nbdns.NameServerGroup - }{ - { - name: "Should Config Single Property", - existingNSGroup: existingNSGroup, - nsGroupID: existingNSGroup.ID, - operations: []NameServerGroupUpdateOperation{ - NameServerGroupUpdateOperation{ - Type: UpdateNameServerGroupName, - Values: []string{"superNew"}, - }, - }, - errFunc: require.NoError, - shouldCreate: true, - expectedNSGroup: &nbdns.NameServerGroup{ - ID: nsGroupID, - Name: "superNew", - Description: "super", - Primary: true, - NameServers: []nbdns.NameServer{ - { - IP: netip.MustParseAddr("1.1.1.1"), - NSType: nbdns.UDPNameServerType, - Port: nbdns.DefaultDNSPort, - }, - { - IP: netip.MustParseAddr("1.1.2.2"), - NSType: nbdns.UDPNameServerType, - Port: nbdns.DefaultDNSPort, - }, - }, - Groups: []string{group1ID}, - Enabled: true, - }, - }, - { - name: "Should Config Multiple Properties", - existingNSGroup: existingNSGroup, - nsGroupID: existingNSGroup.ID, - operations: []NameServerGroupUpdateOperation{ - NameServerGroupUpdateOperation{ - Type: UpdateNameServerGroupName, - Values: []string{"superNew"}, - }, - NameServerGroupUpdateOperation{ - Type: UpdateNameServerGroupDescription, - Values: []string{"superDescription"}, - }, - NameServerGroupUpdateOperation{ - Type: UpdateNameServerGroupNameServers, - Values: []string{"udp://127.0.0.1:53", "udp://8.8.8.8:53"}, - }, - NameServerGroupUpdateOperation{ - Type: UpdateNameServerGroupGroups, - Values: []string{group1ID, group2ID}, - }, - NameServerGroupUpdateOperation{ - Type: UpdateNameServerGroupEnabled, - Values: []string{"false"}, - }, - NameServerGroupUpdateOperation{ - Type: UpdateNameServerGroupPrimary, - Values: []string{"false"}, - }, - NameServerGroupUpdateOperation{ - Type: UpdateNameServerGroupDomains, - Values: []string{validDomain}, - }, - }, - errFunc: require.NoError, - shouldCreate: true, - expectedNSGroup: &nbdns.NameServerGroup{ - ID: nsGroupID, - Name: "superNew", - Description: "superDescription", - Primary: false, - Domains: []string{validDomain}, - NameServers: []nbdns.NameServer{ - { - IP: netip.MustParseAddr("127.0.0.1"), - NSType: nbdns.UDPNameServerType, - Port: nbdns.DefaultDNSPort, - }, - { - IP: netip.MustParseAddr("8.8.8.8"), - NSType: nbdns.UDPNameServerType, - Port: nbdns.DefaultDNSPort, - }, - }, - Groups: []string{group1ID, group2ID}, - Enabled: false, - }, - }, - { - name: "Should Not Config On Invalid ID", - existingNSGroup: existingNSGroup, - nsGroupID: "nonExistingNSGroup", - errFunc: require.Error, - }, - { - name: "Should Not Config On Empty Operations", - existingNSGroup: existingNSGroup, - nsGroupID: existingNSGroup.ID, - operations: []NameServerGroupUpdateOperation{}, - errFunc: require.Error, - }, - { - name: "Should Not Config On Empty Values", - existingNSGroup: existingNSGroup, - nsGroupID: existingNSGroup.ID, - operations: []NameServerGroupUpdateOperation{ - NameServerGroupUpdateOperation{ - Type: UpdateNameServerGroupName, - }, - }, - errFunc: require.Error, - }, - { - name: "Should Not Config On Empty String", - existingNSGroup: existingNSGroup, - nsGroupID: existingNSGroup.ID, - operations: []NameServerGroupUpdateOperation{ - NameServerGroupUpdateOperation{ - Type: UpdateNameServerGroupName, - Values: []string{""}, - }, - }, - errFunc: require.Error, - }, - { - name: "Should Not Config On Invalid Name Large String", - existingNSGroup: existingNSGroup, - nsGroupID: existingNSGroup.ID, - operations: []NameServerGroupUpdateOperation{ - NameServerGroupUpdateOperation{ - Type: UpdateNameServerGroupName, - Values: []string{"12345678901234567890qwertyuiopqwertyuiop1"}, - }, - }, - errFunc: require.Error, - }, - { - name: "Should Not Config On Invalid On Existing Name", - existingNSGroup: existingNSGroup, - nsGroupID: existingNSGroup.ID, - operations: []NameServerGroupUpdateOperation{ - NameServerGroupUpdateOperation{ - Type: UpdateNameServerGroupName, - Values: []string{existingNSGroupName}, - }, - }, - errFunc: require.Error, - }, - { - name: "Should Not Config On Invalid On Multiple Name Values", - existingNSGroup: existingNSGroup, - nsGroupID: existingNSGroup.ID, - operations: []NameServerGroupUpdateOperation{ - NameServerGroupUpdateOperation{ - Type: UpdateNameServerGroupName, - Values: []string{"nameOne", "nameTwo"}, - }, - }, - errFunc: require.Error, - }, - { - name: "Should Not Config On Invalid Boolean", - existingNSGroup: existingNSGroup, - nsGroupID: existingNSGroup.ID, - operations: []NameServerGroupUpdateOperation{ - NameServerGroupUpdateOperation{ - Type: UpdateNameServerGroupEnabled, - Values: []string{"yes"}, - }, - }, - errFunc: require.Error, - }, - { - name: "Should Not Config On Invalid Nameservers Wrong Schema", - existingNSGroup: existingNSGroup, - nsGroupID: existingNSGroup.ID, - operations: []NameServerGroupUpdateOperation{ - NameServerGroupUpdateOperation{ - Type: UpdateNameServerGroupNameServers, - Values: []string{"https://127.0.0.1:53"}, - }, - }, - errFunc: require.Error, - }, - { - name: "Should Not Config On Invalid Nameservers Wrong IP", - existingNSGroup: existingNSGroup, - nsGroupID: existingNSGroup.ID, - operations: []NameServerGroupUpdateOperation{ - NameServerGroupUpdateOperation{ - Type: UpdateNameServerGroupNameServers, - Values: []string{"udp://8.8.8.300:53"}, - }, - }, - errFunc: require.Error, - }, - { - name: "Should Not Config On Large Number Of Nameservers", - existingNSGroup: existingNSGroup, - nsGroupID: existingNSGroup.ID, - operations: []NameServerGroupUpdateOperation{ - NameServerGroupUpdateOperation{ - Type: UpdateNameServerGroupNameServers, - Values: []string{"udp://127.0.0.1:53", "udp://8.8.8.8:53", "udp://8.8.4.4:53"}, - }, - }, - errFunc: require.Error, - }, - { - name: "Should Not Config On Invalid GroupID", - existingNSGroup: existingNSGroup, - nsGroupID: existingNSGroup.ID, - operations: []NameServerGroupUpdateOperation{ - NameServerGroupUpdateOperation{ - Type: UpdateNameServerGroupGroups, - Values: []string{"nonExistingGroupID"}, - }, - }, - errFunc: require.Error, - }, - { - name: "Should Not Config On Invalid Domains", - existingNSGroup: existingNSGroup, - nsGroupID: existingNSGroup.ID, - operations: []NameServerGroupUpdateOperation{ - NameServerGroupUpdateOperation{ - Type: UpdateNameServerGroupDomains, - Values: []string{invalidDomain}, - }, - }, - errFunc: require.Error, - }, - { - name: "Should Not Config On Invalid Primary Status", - existingNSGroup: existingNSGroup, - nsGroupID: existingNSGroup.ID, - operations: []NameServerGroupUpdateOperation{ - NameServerGroupUpdateOperation{ - Type: UpdateNameServerGroupPrimary, - Values: []string{"yes"}, - }, - }, - errFunc: require.Error, - }, - } - for _, testCase := range testCases { - t.Run(testCase.name, func(t *testing.T) { - am, err := createNSManager(t) - if err != nil { - t.Error("failed to create account manager") - } - - account, err := initTestNSAccount(t, am) - if err != nil { - t.Error("failed to init testing account") - } - - account.NameServerGroups[testCase.existingNSGroup.ID] = testCase.existingNSGroup - - err = am.Store.SaveAccount(account) - if err != nil { - t.Error("account should be saved") - } - - updatedRoute, err := am.UpdateNameServerGroup(account.Id, testCase.nsGroupID, userID, testCase.operations) - testCase.errFunc(t, err) - - if !testCase.shouldCreate { - return - } - - testCase.expectedNSGroup.ID = updatedRoute.ID - - if !testCase.expectedNSGroup.IsEqual(updatedRoute) { - t.Errorf("new nameserver group didn't match expected group:\nGot %#v\nExpected:%#v\n", updatedRoute, testCase.expectedNSGroup) - } - - }) - } -} - func TestDeleteNameServerGroup(t *testing.T) { nsGroupID := "testingNSGroup" diff --git a/management/server/route.go b/management/server/route.go index f51b7c2db..b232c2bb6 100644 --- a/management/server/route.go +++ b/management/server/route.go @@ -2,7 +2,6 @@ package server import ( "net/netip" - "strconv" "unicode/utf8" "github.com/netbirdio/netbird/management/proto" @@ -13,57 +12,6 @@ import ( log "github.com/sirupsen/logrus" ) -const ( - // UpdateRouteDescription indicates a route description update operation - UpdateRouteDescription RouteUpdateOperationType = iota - // UpdateRouteNetwork indicates a route IP update operation - UpdateRouteNetwork - // UpdateRoutePeer indicates a route peer update operation - UpdateRoutePeer - // UpdateRouteMetric indicates a route metric update operation - UpdateRouteMetric - // UpdateRouteMasquerade indicates a route masquerade update operation - UpdateRouteMasquerade - // UpdateRouteEnabled indicates a route enabled update operation - UpdateRouteEnabled - // UpdateRouteNetworkIdentifier indicates a route net ID update operation - UpdateRouteNetworkIdentifier - // UpdateRouteGroups indicates a group list update operation - UpdateRouteGroups -) - -// RouteUpdateOperationType operation type -type RouteUpdateOperationType int - -func (t RouteUpdateOperationType) String() string { - switch t { - case UpdateRouteDescription: - return "UpdateRouteDescription" - case UpdateRouteNetwork: - return "UpdateRouteNetwork" - case UpdateRoutePeer: - return "UpdateRoutePeer" - case UpdateRouteMetric: - return "UpdateRouteMetric" - case UpdateRouteMasquerade: - return "UpdateRouteMasquerade" - case UpdateRouteEnabled: - return "UpdateRouteEnabled" - case UpdateRouteNetworkIdentifier: - return "UpdateRouteNetworkIdentifier" - case UpdateRouteGroups: - return "UpdateRouteGroups" - default: - return "InvalidOperation" - } -} - -// RouteUpdateOperation operation object with type and values to be applied -type RouteUpdateOperation struct { - Type RouteUpdateOperationType - Values []string -} - // GetRoute gets a route object from account and route IDs func (am *DefaultAccountManager) GetRoute(accountID, routeID, userID string) (*route.Route, error) { unlock := am.Store.AcquireAccountLock(accountID) @@ -241,109 +189,6 @@ func (am *DefaultAccountManager) SaveRoute(accountID, userID string, routeToSave return nil } -// UpdateRoute updates existing route with set of operations -func (am *DefaultAccountManager) UpdateRoute(accountID, routeID string, operations []RouteUpdateOperation) (*route.Route, error) { - unlock := am.Store.AcquireAccountLock(accountID) - defer unlock() - - account, err := am.Store.GetAccount(accountID) - if err != nil { - return nil, err - } - - routeToUpdate, ok := account.Routes[routeID] - if !ok { - return nil, status.Errorf(status.NotFound, "route %s no longer exists", routeID) - } - - newRoute := routeToUpdate.Copy() - - for _, operation := range operations { - - if len(operation.Values) != 1 { - return nil, status.Errorf(status.InvalidArgument, "operation %s contains invalid number of values, it should be 1", operation.Type.String()) - } - - switch operation.Type { - case UpdateRouteDescription: - newRoute.Description = operation.Values[0] - case UpdateRouteNetworkIdentifier: - if utf8.RuneCountInString(operation.Values[0]) > route.MaxNetIDChar || operation.Values[0] == "" { - return nil, status.Errorf(status.InvalidArgument, "identifier should be between 1 and %d", route.MaxNetIDChar) - } - newRoute.NetID = operation.Values[0] - case UpdateRouteNetwork: - prefixType, prefix, err := route.ParseNetwork(operation.Values[0]) - if err != nil { - return nil, status.Errorf(status.InvalidArgument, "failed to parse IP %s", operation.Values[0]) - } - err = am.checkPrefixPeerExists(accountID, routeToUpdate.Peer, prefix) - if err != nil { - return nil, err - } - newRoute.Network = prefix - newRoute.NetworkType = prefixType - case UpdateRoutePeer: - if operation.Values[0] != "" { - peer := account.GetPeer(operation.Values[0]) - if peer == nil { - return nil, status.Errorf(status.InvalidArgument, "peer with ID %s not found", operation.Values[0]) - } - } - - err = am.checkPrefixPeerExists(accountID, operation.Values[0], routeToUpdate.Network) - if err != nil { - return nil, err - } - newRoute.Peer = operation.Values[0] - case UpdateRouteMetric: - metric, err := strconv.Atoi(operation.Values[0]) - if err != nil { - return nil, status.Errorf(status.InvalidArgument, "failed to parse metric %s, not int", operation.Values[0]) - } - if metric < route.MinMetric || metric > route.MaxMetric { - return nil, status.Errorf(status.InvalidArgument, "failed to parse metric %s, value should be %d > N < %d", - operation.Values[0], - route.MinMetric, - route.MaxMetric, - ) - } - newRoute.Metric = metric - case UpdateRouteMasquerade: - masquerade, err := strconv.ParseBool(operation.Values[0]) - if err != nil { - return nil, status.Errorf(status.InvalidArgument, "failed to parse masquerade %s, not boolean", operation.Values[0]) - } - newRoute.Masquerade = masquerade - case UpdateRouteEnabled: - enabled, err := strconv.ParseBool(operation.Values[0]) - if err != nil { - return nil, status.Errorf(status.InvalidArgument, "failed to parse enabled %s, not boolean", operation.Values[0]) - } - newRoute.Enabled = enabled - case UpdateRouteGroups: - err = validateGroups(operation.Values, account.Groups) - if err != nil { - return nil, err - } - newRoute.Groups = operation.Values - } - } - - account.Routes[routeID] = newRoute - - account.Network.IncSerial() - if err = am.Store.SaveAccount(account); err != nil { - return nil, err - } - - err = am.updateAccountPeers(account) - if err != nil { - return nil, status.Errorf(status.Internal, "failed to update account peers") - } - return newRoute, nil -} - // DeleteRoute deletes route with routeID func (am *DefaultAccountManager) DeleteRoute(accountID, routeID, userID string) error { unlock := am.Store.AcquireAccountLock(accountID) diff --git a/management/server/route_test.go b/management/server/route_test.go index c943aee0b..69241da31 100644 --- a/management/server/route_test.go +++ b/management/server/route_test.go @@ -524,265 +524,6 @@ func TestSaveRoute(t *testing.T) { } } -func TestUpdateRoute(t *testing.T) { - routeID := "testingRouteID" - - existingRoute := &route.Route{ - ID: routeID, - Network: netip.MustParsePrefix("192.168.0.0/16"), - NetID: "superRoute", - NetworkType: route.IPv4Network, - Peer: peer1ID, - Description: "super", - Masquerade: false, - Metric: 9999, - Enabled: true, - Groups: []string{routeGroup1}, - } - - testCases := []struct { - name string - existingRoute *route.Route - operations []RouteUpdateOperation - shouldCreate bool - errFunc require.ErrorAssertionFunc - expectedRoute *route.Route - }{ - { - name: "Happy Path Single OPS", - existingRoute: existingRoute, - operations: []RouteUpdateOperation{ - { - Type: UpdateRoutePeer, - Values: []string{peer2ID}, - }, - }, - errFunc: require.NoError, - shouldCreate: true, - expectedRoute: &route.Route{ - ID: routeID, - Network: netip.MustParsePrefix("192.168.0.0/16"), - NetID: "superRoute", - NetworkType: route.IPv4Network, - Peer: peer2ID, - Description: "super", - Masquerade: false, - Metric: 9999, - Enabled: true, - Groups: []string{routeGroup1}, - }, - }, - { - name: "Happy Path Multiple OPS", - existingRoute: existingRoute, - operations: []RouteUpdateOperation{ - { - Type: UpdateRouteDescription, - Values: []string{"great"}, - }, - { - Type: UpdateRouteNetwork, - Values: []string{"192.168.0.0/24"}, - }, - { - Type: UpdateRoutePeer, - Values: []string{peer2ID}, - }, - { - Type: UpdateRouteMetric, - Values: []string{"3030"}, - }, - { - Type: UpdateRouteMasquerade, - Values: []string{"true"}, - }, - { - Type: UpdateRouteEnabled, - Values: []string{"false"}, - }, - { - Type: UpdateRouteNetworkIdentifier, - Values: []string{"megaRoute"}, - }, - { - Type: UpdateRouteGroups, - Values: []string{routeGroup2}, - }, - }, - errFunc: require.NoError, - shouldCreate: true, - expectedRoute: &route.Route{ - ID: routeID, - Network: netip.MustParsePrefix("192.168.0.0/24"), - NetID: "megaRoute", - NetworkType: route.IPv4Network, - Peer: peer2ID, - Description: "great", - Masquerade: true, - Metric: 3030, - Enabled: false, - Groups: []string{routeGroup2}, - }, - }, - { - name: "Empty Values Should Fail", - existingRoute: existingRoute, - operations: []RouteUpdateOperation{ - { - Type: UpdateRoutePeer, - }, - }, - errFunc: require.Error, - }, - { - name: "Multiple Values Should Fail", - existingRoute: existingRoute, - operations: []RouteUpdateOperation{ - { - Type: UpdateRoutePeer, - Values: []string{peer2ID, peer1ID}, - }, - }, - errFunc: require.Error, - }, - { - name: "Bad Prefix Should Fail", - existingRoute: existingRoute, - operations: []RouteUpdateOperation{ - { - Type: UpdateRouteNetwork, - Values: []string{"192.168.0.0/34"}, - }, - }, - errFunc: require.Error, - }, - { - name: "Bad Peer Should Fail", - existingRoute: existingRoute, - operations: []RouteUpdateOperation{ - { - Type: UpdateRoutePeer, - Values: []string{"non existing Peer"}, - }, - }, - errFunc: require.Error, - }, - { - name: "Empty Peer", - existingRoute: existingRoute, - operations: []RouteUpdateOperation{ - { - Type: UpdateRoutePeer, - Values: []string{""}, - }, - }, - errFunc: require.NoError, - shouldCreate: true, - expectedRoute: &route.Route{ - ID: routeID, - Network: netip.MustParsePrefix("192.168.0.0/16"), - NetID: "superRoute", - NetworkType: route.IPv4Network, - Peer: "", - Description: "super", - Masquerade: false, - Metric: 9999, - Enabled: true, - Groups: []string{routeGroup1}, - }, - }, - { - name: "Large Network ID Should Fail", - existingRoute: existingRoute, - operations: []RouteUpdateOperation{ - { - Type: UpdateRouteNetworkIdentifier, - Values: []string{"12345678901234567890qwertyuiopqwertyuiop1"}, - }, - }, - errFunc: require.Error, - }, - { - name: "Empty Network ID Should Fail", - existingRoute: existingRoute, - operations: []RouteUpdateOperation{ - { - Type: UpdateRouteNetworkIdentifier, - Values: []string{""}, - }, - }, - errFunc: require.Error, - }, - { - name: "Invalid Metric Should Fail", - existingRoute: existingRoute, - operations: []RouteUpdateOperation{ - { - Type: UpdateRouteMetric, - Values: []string{"999999"}, - }, - }, - errFunc: require.Error, - }, - { - name: "Invalid Boolean Should Fail", - existingRoute: existingRoute, - operations: []RouteUpdateOperation{ - { - Type: UpdateRouteMasquerade, - Values: []string{"yes"}, - }, - }, - errFunc: require.Error, - }, - { - name: "Invalid Group Should Fail", - existingRoute: existingRoute, - operations: []RouteUpdateOperation{ - { - Type: UpdateRouteGroups, - Values: []string{routeInvalidGroup1}, - }, - }, - errFunc: require.Error, - }, - } - for _, testCase := range testCases { - t.Run(testCase.name, func(t *testing.T) { - am, err := createRouterManager(t) - if err != nil { - t.Error("failed to create account manager") - } - - account, err := initTestRouteAccount(t, am) - if err != nil { - t.Error("failed to init testing account") - } - - account.Routes[testCase.existingRoute.ID] = testCase.existingRoute - - err = am.Store.SaveAccount(account) - if err != nil { - t.Error("account should be saved") - } - - updatedRoute, err := am.UpdateRoute(account.Id, testCase.existingRoute.ID, testCase.operations) - - testCase.errFunc(t, err) - - if !testCase.shouldCreate { - return - } - - testCase.expectedRoute.ID = updatedRoute.ID - - if !testCase.expectedRoute.IsEqual(updatedRoute) { - t.Errorf("new route didn't match expected route:\nGot %#v\nExpected:%#v\n", updatedRoute, testCase.expectedRoute) - } - }) - } -} - func TestDeleteRoute(t *testing.T) { testingRoute := &route.Route{ ID: "testingRoute",