[management] Add support for disabling resources and routing peers in…

… networks (#3154) * sync openapi changes Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * add option to disable network resource(s) Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * add network resource enabled state from api Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * fix tests Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * add option to disable network router(s) Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * fix tests Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * Add tests Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> * migrate old network resources and routers Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com> --------- Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com>
netbirdio · Jan 8, 2025 · 409003b · 409003b
1 parent 9e6e34b
commit 409003b
Show file tree

Hide file tree

Showing 13 changed files with 188 additions and 14 deletions.
diff --git a/management/server/http/api/openapi.yml b/management/server/http/api/openapi.yml
@@ -800,15 +800,18 @@ components:
               items:
                 type: string
                 example: "ch8i4ug6lnn4g9hqv797"
+            sourceResource:
+              description: Policy rule source resource that the rule is applied to
+              $ref: '#/components/schemas/Resource'
             destinations:
               description: Policy rule destination group IDs
               type: array
               items:
                 type: string
                 example: "ch8i4ug6lnn4g9h7v7m0"
-          required:
-            - sources
-            - destinations
+            destinationResource:
+              description: Policy rule destination resource that the rule is applied to
+              $ref: '#/components/schemas/Resource'
 
     PolicyRuleCreate:
       allOf:
@@ -1325,9 +1328,14 @@ components:
           description: Network resource address (either a direct host like 1.1.1.1 or 1.1.1.1/32, or a subnet like 192.168.178.0/24, or domains like example.com and *.example.com)
           type: string
           example: "1.1.1.1"
+        enabled:
+          description: Network resource status
+          type: boolean
+          example: true
       required:
         - name
         - address
+        - enabled
     NetworkResourceRequest:
       allOf:
         - $ref: '#/components/schemas/NetworkResourceMinimum'
@@ -1390,12 +1398,17 @@ components:
           description: Indicate if peer should masquerade traffic to this route's prefix
           type: boolean
           example: true
+        enabled:
+          description: Network router status
+          type: boolean
+          example: true
       required:
         # Only one property has to be set
         #- peer
         #- peer_groups
         - metric
         - masquerade
+        - enabled
     NetworkRouter:
       allOf:
         - type: object

diff --git a/management/server/http/api/types.gen.go b/management/server/http/api/types.gen.go
diff --git a/management/server/migration/migration.go b/management/server/migration/migration.go
@@ -305,3 +305,53 @@ func hiddenKey(key string, length int) string {
 	}
 	return prefix + strings.Repeat("*", length)
 }
+
+func MigrateNewField[T any](ctx context.Context, db *gorm.DB, columnName string, defaultValue any) error {
+	var model T
+
+	if !db.Migrator().HasTable(&model) {
+		log.WithContext(ctx).Debugf("Table for %T does not exist, no migration needed", model)
+		return nil
+	}
+
+	stmt := &gorm.Statement{DB: db}
+	err := stmt.Parse(&model)
+	if err != nil {
+		return fmt.Errorf("parse model: %w", err)
+	}
+	tableName := stmt.Schema.Table
+
+	if err := db.Transaction(func(tx *gorm.DB) error {
+		if !tx.Migrator().HasColumn(&model, columnName) {
+			log.WithContext(ctx).Infof("Column %s does not exist in table %s, adding it", columnName, tableName)
+			if err := tx.Migrator().AddColumn(&model, columnName); err != nil {
+				return fmt.Errorf("add column %s: %w", columnName, err)
+			}
+		}
+
+		var rows []map[string]any
+		if err := tx.Table(tableName).
+			Select("id", columnName).
+			Where(columnName + " IS NULL OR " + columnName + " = ''").
+			Find(&rows).Error; err != nil {
+			return fmt.Errorf("failed to find rows with empty %s: %w", columnName, err)
+		}
+
+		if len(rows) == 0 {
+			log.WithContext(ctx).Infof("No rows with empty %s found in table %s, no migration needed", columnName, tableName)
+			return nil
+		}
+
+		for _, row := range rows {
+			if err := tx.Table(tableName).Where("id = ?", row["id"]).Update(columnName, defaultValue).Error; err != nil {
+				return fmt.Errorf("failed to update row with id %v: %w", row["id"], err)
+			}
+		}
+		return nil
+	}); err != nil {
+		return err
+	}
+
+	log.WithContext(ctx).Infof("Migration of empty %s to default value in table %s completed", columnName, tableName)
+	return nil
+}
diff --git a/management/server/networks/resources/manager.go b/management/server/networks/resources/manager.go
@@ -101,7 +101,7 @@ func (m *managerImpl) CreateResource(ctx context.Context, userID string, resourc
 		return nil, status.NewPermissionDeniedError()
 	}
 
-	resource, err = types.NewNetworkResource(resource.AccountID, resource.NetworkID, resource.Name, resource.Description, resource.Address, resource.GroupIDs)
+	resource, err = types.NewNetworkResource(resource.AccountID, resource.NetworkID, resource.Name, resource.Description, resource.Address, resource.GroupIDs, resource.Enabled)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create new network resource: %w", err)
 	}

diff --git a/management/server/networks/resources/types/resource.go b/management/server/networks/resources/types/resource.go
@@ -40,9 +40,10 @@ type NetworkResource struct {
 	GroupIDs    []string `gorm:"-"`
 	Domain      string
 	Prefix      netip.Prefix `gorm:"serializer:json"`
+	Enabled     bool
 }
 
-func NewNetworkResource(accountID, networkID, name, description, address string, groupIDs []string) (*NetworkResource, error) {
+func NewNetworkResource(accountID, networkID, name, description, address string, groupIDs []string, enabled bool) (*NetworkResource, error) {
 	resourceType, domain, prefix, err := GetResourceType(address)
 	if err != nil {
 		return nil, fmt.Errorf("invalid address: %w", err)
@@ -59,6 +60,7 @@ func NewNetworkResource(accountID, networkID, name, description, address string,
 		Domain:      domain,
 		Prefix:      prefix,
 		GroupIDs:    groupIDs,
+		Enabled:     enabled,
 	}, nil
 }
 
@@ -75,6 +77,7 @@ func (n *NetworkResource) ToAPIResponse(groups []api.GroupMinimum) *api.NetworkR
 		Type:        api.NetworkResourceType(n.Type.String()),
 		Address:     addr,
 		Groups:      groups,
+		Enabled:     n.Enabled,
 	}
 }
 
@@ -86,6 +89,7 @@ func (n *NetworkResource) FromAPIRequest(req *api.NetworkResourceRequest) {
 	}
 	n.Address = req.Address
 	n.GroupIDs = req.Groups
+	n.Enabled = req.Enabled
 }
 
 func (n *NetworkResource) Copy() *NetworkResource {
@@ -100,6 +104,7 @@ func (n *NetworkResource) Copy() *NetworkResource {
 		Domain:      n.Domain,
 		Prefix:      n.Prefix,
 		GroupIDs:    n.GroupIDs,
+		Enabled:     n.Enabled,
 	}
 }
 
@@ -115,7 +120,7 @@ func (n *NetworkResource) ToRoute(peer *nbpeer.Peer, router *routerTypes.Network
 		PeerGroups:          nil,
 		Masquerade:          router.Masquerade,
 		Metric:              router.Metric,
-		Enabled:             true,
+		Enabled:             n.Enabled,
 		Groups:              nil,
 		AccessControlGroups: nil,
 	}

diff --git a/management/server/networks/routers/manager_test.go b/management/server/networks/routers/manager_test.go
@@ -101,7 +101,7 @@ func Test_GetRouterReturnsPermissionDenied(t *testing.T) {
 func Test_CreateRouterSuccessfully(t *testing.T) {
 	ctx := context.Background()
 	userID := "allowedUser"
-	router, err := types.NewNetworkRouter("testAccountId", "testNetworkId", "testPeerId", []string{}, false, 9999)
+	router, err := types.NewNetworkRouter("testAccountId", "testNetworkId", "testPeerId", []string{}, false, 9999, true)
 	if err != nil {
 		require.NoError(t, err)
 	}
@@ -127,7 +127,7 @@ func Test_CreateRouterSuccessfully(t *testing.T) {
 func Test_CreateRouterFailsWithPermissionDenied(t *testing.T) {
 	ctx := context.Background()
 	userID := "invalidUser"
-	router, err := types.NewNetworkRouter("testAccountId", "testNetworkId", "testPeerId", []string{}, false, 9999)
+	router, err := types.NewNetworkRouter("testAccountId", "testNetworkId", "testPeerId", []string{}, false, 9999, true)
 	if err != nil {
 		require.NoError(t, err)
 	}
@@ -191,7 +191,7 @@ func Test_DeleteRouterFailsWithPermissionDenied(t *testing.T) {
 func Test_UpdateRouterSuccessfully(t *testing.T) {
 	ctx := context.Background()
 	userID := "allowedUser"
-	router, err := types.NewNetworkRouter("testAccountId", "testNetworkId", "testPeerId", []string{}, false, 1)
+	router, err := types.NewNetworkRouter("testAccountId", "testNetworkId", "testPeerId", []string{}, false, 1, true)
 	if err != nil {
 		require.NoError(t, err)
 	}
@@ -213,7 +213,7 @@ func Test_UpdateRouterSuccessfully(t *testing.T) {
 func Test_UpdateRouterFailsWithPermissionDenied(t *testing.T) {
 	ctx := context.Background()
 	userID := "invalidUser"
-	router, err := types.NewNetworkRouter("testAccountId", "testNetworkId", "testPeerId", []string{}, false, 1)
+	router, err := types.NewNetworkRouter("testAccountId", "testNetworkId", "testPeerId", []string{}, false, 1, true)
 	if err != nil {
 		require.NoError(t, err)
 	}

diff --git a/management/server/networks/routers/types/router.go b/management/server/networks/routers/types/router.go
@@ -17,9 +17,10 @@ type NetworkRouter struct {
 	PeerGroups []string `gorm:"serializer:json"`
 	Masquerade bool
 	Metric     int
+	Enabled    bool
 }
 
-func NewNetworkRouter(accountID string, networkID string, peer string, peerGroups []string, masquerade bool, metric int) (*NetworkRouter, error) {
+func NewNetworkRouter(accountID string, networkID string, peer string, peerGroups []string, masquerade bool, metric int, enabled bool) (*NetworkRouter, error) {
 	if peer != "" && len(peerGroups) > 0 {
 		return nil, errors.New("peer and peerGroups cannot be set at the same time")
 	}
@@ -32,6 +33,7 @@ func NewNetworkRouter(accountID string, networkID string, peer string, peerGroup
 		PeerGroups: peerGroups,
 		Masquerade: masquerade,
 		Metric:     metric,
+		Enabled:    enabled,
 	}, nil
 }
 
@@ -42,6 +44,7 @@ func (n *NetworkRouter) ToAPIResponse() *api.NetworkRouter {
 		PeerGroups: &n.PeerGroups,
 		Masquerade: n.Masquerade,
 		Metric:     n.Metric,
+		Enabled:    n.Enabled,
 	}
 }
 
@@ -56,6 +59,7 @@ func (n *NetworkRouter) FromAPIRequest(req *api.NetworkRouterRequest) {
 
 	n.Masquerade = req.Masquerade
 	n.Metric = req.Metric
+	n.Enabled = req.Enabled
 }
 
 func (n *NetworkRouter) Copy() *NetworkRouter {
@@ -67,6 +71,7 @@ func (n *NetworkRouter) Copy() *NetworkRouter {
 		PeerGroups: n.PeerGroups,
 		Masquerade: n.Masquerade,
 		Metric:     n.Metric,
+		Enabled:    n.Enabled,
 	}
 }
 

diff --git a/management/server/networks/routers/types/router_test.go b/management/server/networks/routers/types/router_test.go
@@ -11,6 +11,7 @@ func TestNewNetworkRouter(t *testing.T) {
 		peerGroups    []string
 		masquerade    bool
 		metric        int
+		enabled       bool
 		expectedError bool
 	}{
 		// Valid cases
@@ -22,6 +23,7 @@ func TestNewNetworkRouter(t *testing.T) {
 			peerGroups:    nil,
 			masquerade:    true,
 			metric:        100,
+			enabled:       true,
 			expectedError: false,
 		},
 		{
@@ -32,6 +34,7 @@ func TestNewNetworkRouter(t *testing.T) {
 			peerGroups:    []string{"group-1", "group-2"},
 			masquerade:    false,
 			metric:        200,
+			enabled:       false,
 			expectedError: false,
 		},
 		{
@@ -42,6 +45,7 @@ func TestNewNetworkRouter(t *testing.T) {
 			peerGroups:    nil,
 			masquerade:    true,
 			metric:        300,
+			enabled:       true,
 			expectedError: false,
 		},
 
@@ -54,13 +58,14 @@ func TestNewNetworkRouter(t *testing.T) {
 			peerGroups:    []string{"group-3"},
 			masquerade:    false,
 			metric:        400,
+			enabled:       false,
 			expectedError: true,
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			router, err := NewNetworkRouter(tt.accountID, tt.networkID, tt.peer, tt.peerGroups, tt.masquerade, tt.metric)
+			router, err := NewNetworkRouter(tt.accountID, tt.networkID, tt.peer, tt.peerGroups, tt.masquerade, tt.metric, tt.enabled)
 
 			if tt.expectedError && err == nil {
 				t.Fatalf("Expected an error, got nil")
@@ -94,6 +99,10 @@ func TestNewNetworkRouter(t *testing.T) {
 				if router.Metric != tt.metric {
 					t.Errorf("Expected Metric %d, got %d", tt.metric, router.Metric)
 				}
+
+				if router.Enabled != tt.enabled {
+					t.Errorf("Expected Enabled %v, got %v", tt.enabled, router.Enabled)
+				}
 			}
 		})
 	}