[management] Add tests for networks managers (#3049)

management/server/networks/resources/manager.go

@@ -93,6 +93,11 @@ func (m *managerImpl) CreateResource(ctx context.Context, userID string, resourc
 		return nil, fmt.Errorf("failed to create new network resource: %w", err)
 	}
 
+	_, err = m.store.GetNetworkResourceByName(ctx, store.LockingStrengthShare, resource.AccountID, resource.Name)
+	if err == nil {
+		return nil, errors.New("resource already exists")
+	}
+
 	return resource, m.store.SaveNetworkResource(ctx, store.LockingStrengthUpdate, resource)
 }
 
@@ -126,12 +131,23 @@ func (m *managerImpl) UpdateResource(ctx context.Context, userID string, resourc
 		return nil, status.NewPermissionDeniedError()
 	}
 
-	resourceType, err := types.GetResourceType(resource.Address)
+	resourceType, addr, err := types.GetResourceType(resource.Address)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get resource type: %w", err)
 	}
 
 	resource.Type = resourceType
+	resource.Address = addr
+
+	_, err = m.store.GetNetworkResourceByID(ctx, store.LockingStrengthShare, resource.AccountID, resource.ID)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get network resource: %w", err)
+	}
+
+	oldResource, err := m.store.GetNetworkResourceByName(ctx, store.LockingStrengthShare, resource.AccountID, resource.Name)
+	if err == nil && oldResource.ID != resource.ID {
+		return nil, errors.New("new resource name already exists")
+	}
 
 	return resource, m.store.SaveNetworkResource(ctx, store.LockingStrengthUpdate, resource)
 }

management/server/networks/resources/manager_test.go

@@ -0,0 +1,379 @@
+package resources
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/netbirdio/netbird/management/server/networks/resources/types"
+	"github.com/netbirdio/netbird/management/server/permissions"
+	"github.com/netbirdio/netbird/management/server/status"
+	"github.com/netbirdio/netbird/management/server/store"
+)
+
+func Test_GetAllResourcesInNetworkReturnsResources(t *testing.T) {
+	ctx := context.Background()
+	accountID := "testAccountId"
+	userID := "allowedUser"
+	networkID := "testNetworkId"
+
+	s, cleanUp, err := store.NewTestStoreFromSQL(context.Background(), "../../testdata/networks.sql", t.TempDir())
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(cleanUp)
+	permissionsManager := permissions.NewManagerMock()
+	manager := NewManager(s, permissionsManager)
+
+	resources, err := manager.GetAllResourcesInNetwork(ctx, accountID, userID, networkID)
+	require.NoError(t, err)
+	require.Len(t, resources, 2)
+}
+
+func Test_GetAllResourcesInNetworkReturnsPermissionDenied(t *testing.T) {
+	ctx := context.Background()
+	accountID := "testAccountId"
+	userID := "invalidUser"
+	networkID := "testNetworkId"
+
+	s, cleanUp, err := store.NewTestStoreFromSQL(context.Background(), "../../testdata/networks.sql", t.TempDir())
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(cleanUp)
+	permissionsManager := permissions.NewManagerMock()
+	manager := NewManager(s, permissionsManager)
+
+	resources, err := manager.GetAllResourcesInNetwork(ctx, accountID, userID, networkID)
+	require.Error(t, err)
+	require.Equal(t, status.NewPermissionDeniedError(), err)
+	require.Nil(t, resources)
+}
+func Test_GetAllResourcesInAccountReturnsResources(t *testing.T) {
+	ctx := context.Background()
+	accountID := "testAccountId"
+	userID := "allowedUser"
+
+	s, cleanUp, err := store.NewTestStoreFromSQL(context.Background(), "../../testdata/networks.sql", t.TempDir())
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(cleanUp)
+	permissionsManager := permissions.NewManagerMock()
+	manager := NewManager(s, permissionsManager)
+
+	resources, err := manager.GetAllResourcesInAccount(ctx, accountID, userID)
+	require.NoError(t, err)
+	require.Len(t, resources, 2)
+}
+
+func Test_GetAllResourcesInAccountReturnsPermissionDenied(t *testing.T) {
+	ctx := context.Background()
+	accountID := "testAccountId"
+	userID := "invalidUser"
+
+	s, cleanUp, err := store.NewTestStoreFromSQL(context.Background(), "../../testdata/networks.sql", t.TempDir())
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(cleanUp)
+	permissionsManager := permissions.NewManagerMock()
+	manager := NewManager(s, permissionsManager)
+
+	resources, err := manager.GetAllResourcesInAccount(ctx, accountID, userID)
+	require.Error(t, err)
+	require.Equal(t, status.NewPermissionDeniedError(), err)
+	require.Nil(t, resources)
+}
+
+func Test_GetResourceInNetworkReturnsResources(t *testing.T) {
+	ctx := context.Background()
+	accountID := "testAccountId"
+	userID := "allowedUser"
+	networkID := "testNetworkId"
+	resourceID := "testResourceId"
+
+	s, cleanUp, err := store.NewTestStoreFromSQL(context.Background(), "../../testdata/networks.sql", t.TempDir())
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(cleanUp)
+	permissionsManager := permissions.NewManagerMock()
+	manager := NewManager(s, permissionsManager)
+
+	resource, err := manager.GetResource(ctx, accountID, userID, networkID, resourceID)
+	require.NoError(t, err)
+	require.Equal(t, resourceID, resource.ID)
+}
+
+func Test_GetResourceInNetworkReturnsPermissionDenied(t *testing.T) {
+	ctx := context.Background()
+	accountID := "testAccountId"
+	userID := "invalidUser"
+	networkID := "testNetworkId"
+	resourceID := "testResourceId"
+
+	s, cleanUp, err := store.NewTestStoreFromSQL(context.Background(), "../../testdata/networks.sql", t.TempDir())
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(cleanUp)
+	permissionsManager := permissions.NewManagerMock()
+	manager := NewManager(s, permissionsManager)
+
+	resources, err := manager.GetResource(ctx, accountID, userID, networkID, resourceID)
+	require.Error(t, err)
+	require.Equal(t, status.NewPermissionDeniedError(), err)
+	require.Nil(t, resources)
+}
+
+func Test_CreateResourceSuccessfully(t *testing.T) {
+	ctx := context.Background()
+	userID := "allowedUser"
+	resource := &types.NetworkResource{
+		AccountID:   "testAccountId",
+		NetworkID:   "testNetworkId",
+		Name:        "newResourceId",
+		Description: "description",
+		Address:     "192.168.1.1",
+	}
+
+	store, cleanUp, err := store.NewTestStoreFromSQL(context.Background(), "../../testdata/networks.sql", t.TempDir())
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(cleanUp)
+	permissionsManager := permissions.NewManagerMock()
+	manager := NewManager(store, permissionsManager)
+
+	createdResource, err := manager.CreateResource(ctx, userID, resource)
+	require.NoError(t, err)
+	require.Equal(t, resource.Name, createdResource.Name)
+}
+
+func Test_CreateResourceFailsWithPermissionDenied(t *testing.T) {
+	ctx := context.Background()
+	userID := "invalidUser"
+	resource := &types.NetworkResource{
+		AccountID:   "testAccountId",
+		NetworkID:   "testNetworkId",
+		Name:        "testResourceId",
+		Description: "description",
+		Address:     "192.168.1.1",
+	}
+
+	store, cleanUp, err := store.NewTestStoreFromSQL(context.Background(), "../../testdata/networks.sql", t.TempDir())
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(cleanUp)
+	permissionsManager := permissions.NewManagerMock()
+	manager := NewManager(store, permissionsManager)
+
+	createdResource, err := manager.CreateResource(ctx, userID, resource)
+	require.Error(t, err)
+	require.Equal(t, status.NewPermissionDeniedError(), err)
+	require.Nil(t, createdResource)
+}
+
+func Test_CreateResourceFailsWithInvalidAddress(t *testing.T) {
+	ctx := context.Background()
+	userID := "allowedUser"
+	resource := &types.NetworkResource{
+		AccountID:   "testAccountId",
+		NetworkID:   "testNetworkId",
+		Name:        "testResourceId",
+		Description: "description",
+		Address:     "invalid-address",
+	}
+
+	store, cleanUp, err := store.NewTestStoreFromSQL(context.Background(), "../../testdata/networks.sql", t.TempDir())
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(cleanUp)
+	permissionsManager := permissions.NewManagerMock()
+	manager := NewManager(store, permissionsManager)
+
+	createdResource, err := manager.CreateResource(ctx, userID, resource)
+	require.Error(t, err)
+	require.Nil(t, createdResource)
+}
+
+func Test_CreateResourceFailsWithUsedName(t *testing.T) {
+	ctx := context.Background()
+	userID := "allowedUser"
+	resource := &types.NetworkResource{
+		AccountID:   "testAccountId",
+		NetworkID:   "testNetworkId",
+		Name:        "testResourceId",
+		Description: "description",
+		Address:     "invalid-address",
+	}
+
+	store, cleanUp, err := store.NewTestStoreFromSQL(context.Background(), "../../testdata/networks.sql", t.TempDir())
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(cleanUp)
+	permissionsManager := permissions.NewManagerMock()
+	manager := NewManager(store, permissionsManager)
+
+	createdResource, err := manager.CreateResource(ctx, userID, resource)
+	require.Error(t, err)
+	require.Nil(t, createdResource)
+}
+
+func Test_UpdateResourceSuccessfully(t *testing.T) {
+	ctx := context.Background()
+	accountID := "testAccountId"
+	userID := "allowedUser"
+	networkID := "testNetworkId"
+	resourceID := "testResourceId"
+	resource := &types.NetworkResource{
+		AccountID:   accountID,
+		NetworkID:   networkID,
+		Name:        "someNewName",
+		ID:          resourceID,
+		Description: "new-description",
+		Address:     "1.2.3.0/24",
+	}
+
+	s, cleanUp, err := store.NewTestStoreFromSQL(context.Background(), "../../testdata/networks.sql", t.TempDir())
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(cleanUp)
+	permissionsManager := permissions.NewManagerMock()
+	manager := NewManager(s, permissionsManager)
+
+	updatedResource, err := manager.UpdateResource(ctx, userID, resource)
+	require.NoError(t, err)
+	require.NotNil(t, updatedResource)
+	require.Equal(t, "new-description", updatedResource.Description)
+	require.Equal(t, "1.2.3.0/24", updatedResource.Address)
+	require.Equal(t, types.NetworkResourceType("subnet"), updatedResource.Type)
+}
+
+func Test_UpdateResourceFailsWithResourceNotFound(t *testing.T) {
+	ctx := context.Background()
+	accountID := "testAccountId"
+	userID := "allowedUser"
+	networkID := "testNetworkId"
+	resourceID := "otherResourceId"
+	resource := &types.NetworkResource{
+		AccountID:   accountID,
+		NetworkID:   networkID,
+		Name:        resourceID,
+		Description: "new-description",
+		Address:     "1.2.3.0/24",
+	}
+
+	s, cleanUp, err := store.NewTestStoreFromSQL(context.Background(), "../../testdata/networks.sql", t.TempDir())
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(cleanUp)
+	permissionsManager := permissions.NewManagerMock()
+	manager := NewManager(s, permissionsManager)
+
+	updatedResource, err := manager.UpdateResource(ctx, userID, resource)
+	require.Error(t, err)
+	require.Nil(t, updatedResource)
+}
+
+func Test_UpdateResourceFailsWithNameInUse(t *testing.T) {
+	ctx := context.Background()
+	accountID := "testAccountId"
+	userID := "allowedUser"
+	networkID := "testNetworkId"
+	resourceID := "testResourceId"
+	resource := &types.NetworkResource{
+		AccountID:   accountID,
+		NetworkID:   networkID,
+		ID:          resourceID,
+		Name:        "used-name",
+		Description: "new-description",
+		Address:     "1.2.3.0/24",
+	}
+
+	s, cleanUp, err := store.NewTestStoreFromSQL(context.Background(), "../../testdata/networks.sql", t.TempDir())
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(cleanUp)
+	permissionsManager := permissions.NewManagerMock()
+	manager := NewManager(s, permissionsManager)
+
+	updatedResource, err := manager.UpdateResource(ctx, userID, resource)
+	require.Error(t, err)
+	require.Equal(t, errors.New("new resource name already exists"), err)
+	require.Nil(t, updatedResource)
+}
+
+func Test_UpdateResourceFailsWithPermissionDenied(t *testing.T) {
+	ctx := context.Background()
+	accountID := "testAccountId"
+	userID := "invalidUser"
+	networkID := "testNetworkId"
+	resourceID := "testResourceId"
+	resource := &types.NetworkResource{
+		AccountID:   accountID,
+		NetworkID:   networkID,
+		Name:        resourceID,
+		Description: "new-description",
+		Address:     "1.2.3.0/24",
+	}
+
+	s, cleanUp, err := store.NewTestStoreFromSQL(context.Background(), "../../testdata/networks.sql", t.TempDir())
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(cleanUp)
+	permissionsManager := permissions.NewManagerMock()
+	manager := NewManager(s, permissionsManager)
+
+	updatedResource, err := manager.UpdateResource(ctx, userID, resource)
+	require.Error(t, err)
+	require.Nil(t, updatedResource)
+}
+
+func Test_DeleteResourceSuccessfully(t *testing.T) {
+	ctx := context.Background()
+	accountID := "testAccountId"
+	userID := "allowedUser"
+	networkID := "testNetworkId"
+	resourceID := "testResourceId"
+
+	s, cleanUp, err := store.NewTestStoreFromSQL(context.Background(), "../../testdata/networks.sql", t.TempDir())
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(cleanUp)
+	permissionsManager := permissions.NewManagerMock()
+	manager := NewManager(s, permissionsManager)
+
+	err = manager.DeleteResource(ctx, accountID, userID, networkID, resourceID)
+	require.NoError(t, err)
+}
+
+func Test_DeleteResourceFailsWithPermissionDenied(t *testing.T) {
+	ctx := context.Background()
+	accountID := "testAccountId"
+	userID := "invalidUser"
+	networkID := "testNetworkId"
+	resourceID := "testResourceId"
+
+	store, cleanUp, err := store.NewTestStoreFromSQL(context.Background(), "../../testdata/networks.sql", t.TempDir())
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(cleanUp)
+	permissionsManager := permissions.NewManagerMock()
+	manager := NewManager(store, permissionsManager)
+
+	err = manager.DeleteResource(ctx, accountID, userID, networkID, resourceID)
+	require.Error(t, err)
+}

management/server/networks/resources/types/resource.go

@@ -35,7 +35,7 @@ type NetworkResource struct {
 }
 
 func NewNetworkResource(accountID, networkID, name, description, address string) (*NetworkResource, error) {
-	resourceType, err := GetResourceType(address)
+	resourceType, address, err := GetResourceType(address)
 	if err != nil {
 		return nil, fmt.Errorf("invalid address: %w", err)
 	}
@@ -84,23 +84,26 @@ func (n *NetworkResource) Copy() *NetworkResource {
 }
 
 // GetResourceType returns the type of the resource based on the address
-func GetResourceType(address string) (NetworkResourceType, error) {
+func GetResourceType(address string) (NetworkResourceType, string, error) {
 	if ip, cidr, err := net.ParseCIDR(address); err == nil {
 		ones, _ := cidr.Mask.Size()
-		if strings.HasSuffix(address, "/32") || (ip != nil && ones == 32) {
-			return host, nil
+		if strings.HasSuffix(address, "/32") {
+			return host, address, nil
 		}
-		return subnet, nil
+		if ip != nil && ones == 32 {
+			return host, address + "/32", nil
+		}
+		return subnet, address, nil
 	}
 
 	if net.ParseIP(address) != nil {
-		return host, nil
+		return host, address + "/32", nil
 	}
 
 	domainRegex := regexp.MustCompile(`^(\*\.)?([a-zA-Z0-9-]+\.)+[a-zA-Z]{2,}$`)
 	if domainRegex.MatchString(address) {
-		return domain, nil
+		return domain, address, nil
 	}
 
-	return "", errors.New("not a host, subnet, or domain")
+	return "", "", errors.New("not a host, subnet, or domain")
 }

management/server/networks/resources/types/resource_test.go

@@ -9,26 +9,27 @@ func TestGetResourceType(t *testing.T) {
 		input        string
 		expectedType NetworkResourceType
 		expectedErr  bool
+		expectedAddr string
 	}{
 		// Valid host IPs
-		{"1.1.1.1", host, false},
-		{"1.1.1.1/32", host, false},
+		{"1.1.1.1", host, false, "1.1.1.1/32"},
+		{"1.1.1.1/32", host, false, "1.1.1.1/32"},
 		// Valid subnets
-		{"192.168.1.0/24", subnet, false},
-		{"10.0.0.0/16", subnet, false},
+		{"192.168.1.0/24", subnet, false, "192.168.1.0/24"},
+		{"10.0.0.0/16", subnet, false, "10.0.0.0/16"},
 		// Valid domains
-		{"example.com", domain, false},
-		{"*.example.com", domain, false},
-		{"sub.example.com", domain, false},
+		{"example.com", domain, false, "example.com"},
+		{"*.example.com", domain, false, "*.example.com"},
+		{"sub.example.com", domain, false, "sub.example.com"},
 		// Invalid inputs
-		{"invalid", "", true},
-		{"1.1.1.1/abc", "", true},
-		{"1234", "", true},
+		{"invalid", "", true, ""},
+		{"1.1.1.1/abc", "", true, ""},
+		{"1234", "", true, ""},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.input, func(t *testing.T) {
-			result, err := GetResourceType(tt.input)
+			result, addr, err := GetResourceType(tt.input)
 			if result != tt.expectedType {
 				t.Errorf("Expected type %v, got %v", tt.expectedType, result)
 			}
@@ -36,6 +37,10 @@ func TestGetResourceType(t *testing.T) {
 			if tt.expectedErr && err == nil {
 				t.Errorf("Expected error, got nil")
 			}
+
+			if addr != tt.expectedAddr {
+				t.Errorf("Expected address %v, got %v", tt.expectedAddr, addr)
+			}
 		})
 	}
 }