[management] Enforce peer or peer groups requirement for network routers (#5894)

management/server/http/handlers/networks/routers_handler.go

@@ -105,6 +105,12 @@ func (h *routersHandler) createRouter(w http.ResponseWriter, r *http.Request) {
 	router.NetworkID = networkID
 	router.AccountID = accountID
 	router.Enabled = true
+
+	if err := router.Validate(); err != nil {
+		util.WriteErrorResponse(err.Error(), http.StatusBadRequest, w)
+		return
+	}
+
 	router, err = h.routersManager.CreateRouter(r.Context(), userID, router)
 	if err != nil {
 		util.WriteError(r.Context(), err, w)
@@ -157,6 +163,11 @@ func (h *routersHandler) updateRouter(w http.ResponseWriter, r *http.Request) {
 	router.ID = mux.Vars(r)["routerId"]
 	router.AccountID = accountID
 
+	if err := router.Validate(); err != nil {
+		util.WriteErrorResponse(err.Error(), http.StatusBadRequest, w)
+		return
+	}
+
 	router, err = h.routersManager.UpdateRouter(r.Context(), userID, router)
 	if err != nil {
 		util.WriteError(r.Context(), err, w)

management/server/http/testing/integration/networks_handler_integration_test.go

@@ -1170,13 +1170,17 @@ func Test_NetworkRouters_Create(t *testing.T) {
 				Metric:     100,
 				Enabled:    true,
 			},
-			expectedStatus: http.StatusOK,
+			expectedStatus: http.StatusBadRequest,
-			verifyResponse: func(t *testing.T, router *api.NetworkRouter) {
+		},
-				t.Helper()
+		{
-				assert.NotEmpty(t, router.Id)
+			name:      "Create router without peer and peer_groups",
-				assert.Equal(t, peerID, *router.Peer)
+			networkId: "testNetworkId",
-				assert.Equal(t, 1, len(*router.PeerGroups))
+			requestBody: &api.NetworkRouterRequest{
+				Masquerade: true,
+				Metric:     100,
+				Enabled:    true,
 			},
+			expectedStatus: http.StatusBadRequest,
 		},
 		{
 			name:      "Create router in non-existing network",
@@ -1341,13 +1345,18 @@ func Test_NetworkRouters_Update(t *testing.T) {
 				Metric:     100,
 				Enabled:    true,
 			},
-			expectedStatus: http.StatusOK,
+			expectedStatus: http.StatusBadRequest,
-			verifyResponse: func(t *testing.T, router *api.NetworkRouter) {
+		},
-				t.Helper()
+		{
-				assert.Equal(t, "testRouterId", router.Id)
+			name:      "Update router without peer and peer_groups",
-				assert.Equal(t, peerID, *router.Peer)
+			networkId: "testNetworkId",
-				assert.Equal(t, 1, len(*router.PeerGroups))
+			routerId:  "testRouterId",
+			requestBody: &api.NetworkRouterRequest{
+				Masquerade: true,
+				Metric:     100,
+				Enabled:    true,
 			},
+			expectedStatus: http.StatusBadRequest,
 		},
 	}
 

management/server/networks/routers/types/router.go

@@ -21,11 +21,7 @@ type NetworkRouter struct {
 }
 
 func NewNetworkRouter(accountID string, networkID string, peer string, peerGroups []string, masquerade bool, metric int, enabled bool) (*NetworkRouter, error) {
-	if peer != "" && len(peerGroups) > 0 {
+	r := &NetworkRouter{
-		return nil, errors.New("peer and peerGroups cannot be set at the same time")
-	}
-
-	return &NetworkRouter{
 		ID:         xid.New().String(),
 		AccountID:  accountID,
 		NetworkID:  networkID,
@@ -34,7 +30,25 @@ func NewNetworkRouter(accountID string, networkID string, peer string, peerGroup
 		Masquerade: masquerade,
 		Metric:     metric,
 		Enabled:    enabled,
-	}, nil
+	}
+
+	if err := r.Validate(); err != nil {
+		return nil, err
+	}
+
+	return r, nil
+}
+
+func (n *NetworkRouter) Validate() error {
+	if n.Peer != "" && len(n.PeerGroups) > 0 {
+		return errors.New("peer and peer_groups cannot be set at the same time")
+	}
+
+	if n.Peer == "" && len(n.PeerGroups) == 0 {
+		return errors.New("either peer or peer_groups must be provided")
+	}
+
+	return nil
 }
 
 func (n *NetworkRouter) ToAPIResponse() *api.NetworkRouter {

management/server/networks/routers/types/router_test.go

@@ -38,7 +38,7 @@ func TestNewNetworkRouter(t *testing.T) {
 			expectedError: false,
 		},
 		{
-			name:          "Valid with no peer or peerGroups",
+			name:          "Invalid with no peer or peerGroups",
 			networkID:     "network-3",
 			accountID:     "account-3",
 			peer:          "",
@@ -46,7 +46,18 @@ func TestNewNetworkRouter(t *testing.T) {
 			masquerade:    true,
 			metric:        300,
 			enabled:       true,
-			expectedError: false,
+			expectedError: true,
+		},
+		{
+			name:          "Invalid with empty peerGroups slice",
+			networkID:     "network-5",
+			accountID:     "account-5",
+			peer:          "",
+			peerGroups:    []string{},
+			masquerade:    true,
+			metric:        500,
+			enabled:       true,
+			expectedError: true,
 		},
 
 		// Invalid cases