merge main

management/internals/modules/reverseproxy/service/manager/manager_test.go

@@ -19,6 +19,7 @@ import (
 	"github.com/netbirdio/netbird/management/server/account"
 	"github.com/netbirdio/netbird/management/server/activity"
 	"github.com/netbirdio/netbird/management/server/mock_server"
+	resourcetypes "github.com/netbirdio/netbird/management/server/networks/resources/types"
 	nbpeer "github.com/netbirdio/netbird/management/server/peer"
 	"github.com/netbirdio/netbird/management/server/permissions"
 	"github.com/netbirdio/netbird/management/server/permissions/modules"
@@ -803,8 +804,8 @@ func TestCreateServiceFromPeer(t *testing.T) {
 		mgr, testStore := setupIntegrationTest(t)
 
 		req := &rpservice.ExposeServiceRequest{
-			Port:     8080,
-			Protocol: "http",
+			Port: 8080,
+			Mode: "http",
 		}
 
 		resp, err := mgr.CreateServiceFromPeer(ctx, testAccountID, testPeerID, req)
@@ -826,9 +827,9 @@ func TestCreateServiceFromPeer(t *testing.T) {
 		mgr, _ := setupIntegrationTest(t)
 
 		req := &rpservice.ExposeServiceRequest{
-			Port:     80,
-			Protocol: "http",
-			Domain:   "example.com",
+			Port:   80,
+			Mode:   "http",
+			Domain: "example.com",
 		}
 
 		resp, err := mgr.CreateServiceFromPeer(ctx, testAccountID, testPeerID, req)
@@ -847,8 +848,8 @@ func TestCreateServiceFromPeer(t *testing.T) {
 		require.NoError(t, err)
 
 		req := &rpservice.ExposeServiceRequest{
-			Port:     8080,
-			Protocol: "http",
+			Port: 8080,
+			Mode: "http",
 		}
 
 		_, err = mgr.CreateServiceFromPeer(ctx, testAccountID, testPeerID, req)
@@ -860,8 +861,8 @@ func TestCreateServiceFromPeer(t *testing.T) {
 		mgr, _ := setupIntegrationTest(t)
 
 		req := &rpservice.ExposeServiceRequest{
-			Port:     0,
-			Protocol: "http",
+			Port: 0,
+			Mode: "http",
 		}
 
 		_, err := mgr.CreateServiceFromPeer(ctx, testAccountID, testPeerID, req)
@@ -878,62 +879,52 @@ func TestExposeServiceRequestValidate(t *testing.T) {
 	}{
 		{
 			name:    "valid http request",
-			req:     rpservice.ExposeServiceRequest{Port: 8080, Protocol: "http"},
+			req:     rpservice.ExposeServiceRequest{Port: 8080, Mode: "http"},
 			wantErr: "",
 		},
 		{
-			name:    "valid https request with pin",
-			req:     rpservice.ExposeServiceRequest{Port: 443, Protocol: "https", Pin: "123456"},
-			wantErr: "",
+			name:    "https mode rejected",
+			req:     rpservice.ExposeServiceRequest{Port: 443, Mode: "https", Pin: "123456"},
+			wantErr: "unsupported mode",
 		},
 		{
 			name:    "port zero rejected",
-			req:     rpservice.ExposeServiceRequest{Port: 0, Protocol: "http"},
+			req:     rpservice.ExposeServiceRequest{Port: 0, Mode: "http"},
 			wantErr: "port must be between 1 and 65535",
 		},
 		{
-			name:    "negative port rejected",
-			req:     rpservice.ExposeServiceRequest{Port: -1, Protocol: "http"},
-			wantErr: "port must be between 1 and 65535",
-		},
-		{
-			name:    "port above 65535 rejected",
-			req:     rpservice.ExposeServiceRequest{Port: 65536, Protocol: "http"},
-			wantErr: "port must be between 1 and 65535",
-		},
-		{
-			name:    "unsupported protocol",
-			req:     rpservice.ExposeServiceRequest{Port: 80, Protocol: "tcp"},
-			wantErr: "unsupported protocol",
+			name:    "unsupported mode",
+			req:     rpservice.ExposeServiceRequest{Port: 80, Mode: "ftp"},
+			wantErr: "unsupported mode",
 		},
 		{
 			name:    "invalid pin format",
-			req:     rpservice.ExposeServiceRequest{Port: 80, Protocol: "http", Pin: "abc"},
+			req:     rpservice.ExposeServiceRequest{Port: 80, Mode: "http", Pin: "abc"},
 			wantErr: "invalid pin",
 		},
 		{
 			name:    "pin too short",
-			req:     rpservice.ExposeServiceRequest{Port: 80, Protocol: "http", Pin: "12345"},
+			req:     rpservice.ExposeServiceRequest{Port: 80, Mode: "http", Pin: "12345"},
 			wantErr: "invalid pin",
 		},
 		{
 			name:    "valid 6-digit pin",
-			req:     rpservice.ExposeServiceRequest{Port: 80, Protocol: "http", Pin: "000000"},
+			req:     rpservice.ExposeServiceRequest{Port: 80, Mode: "http", Pin: "000000"},
 			wantErr: "",
 		},
 		{
 			name:    "empty user group name",
-			req:     rpservice.ExposeServiceRequest{Port: 80, Protocol: "http", UserGroups: []string{"valid", ""}},
+			req:     rpservice.ExposeServiceRequest{Port: 80, Mode: "http", UserGroups: []string{"valid", ""}},
 			wantErr: "user group name cannot be empty",
 		},
 		{
 			name:    "invalid name prefix",
-			req:     rpservice.ExposeServiceRequest{Port: 80, Protocol: "http", NamePrefix: "INVALID"},
+			req:     rpservice.ExposeServiceRequest{Port: 80, Mode: "http", NamePrefix: "INVALID"},
 			wantErr: "invalid name prefix",
 		},
 		{
 			name:    "valid name prefix",
-			req:     rpservice.ExposeServiceRequest{Port: 80, Protocol: "http", NamePrefix: "my-service"},
+			req:     rpservice.ExposeServiceRequest{Port: 80, Mode: "http", NamePrefix: "my-service"},
 			wantErr: "",
 		},
 	}
@@ -966,14 +957,14 @@ func TestDeleteServiceFromPeer_ByDomain(t *testing.T) {
 
 		// First create a service
 		req := &rpservice.ExposeServiceRequest{
-			Port:     8080,
-			Protocol: "http",
+			Port: 8080,
+			Mode: "http",
 		}
 		resp, err := mgr.CreateServiceFromPeer(ctx, testAccountID, testPeerID, req)
 		require.NoError(t, err)
 
-		// Delete by domain using unexported method
-		err = mgr.deleteServiceFromPeer(ctx, testAccountID, testPeerID, resp.Domain, false)
+		svcID := resolveServiceIDByDomain(t, testStore, resp.Domain)
+		err = mgr.deleteServiceFromPeer(ctx, testAccountID, testPeerID, svcID, false)
 		require.NoError(t, err)
 
 		// Verify service is deleted
@@ -982,16 +973,17 @@ func TestDeleteServiceFromPeer_ByDomain(t *testing.T) {
 	})
 
 	t.Run("expire uses correct activity", func(t *testing.T) {
-		mgr, _ := setupIntegrationTest(t)
+		mgr, testStore := setupIntegrationTest(t)
 
 		req := &rpservice.ExposeServiceRequest{
-			Port:     8080,
-			Protocol: "http",
+			Port: 8080,
+			Mode: "http",
 		}
 		resp, err := mgr.CreateServiceFromPeer(ctx, testAccountID, testPeerID, req)
 		require.NoError(t, err)
 
-		err = mgr.deleteServiceFromPeer(ctx, testAccountID, testPeerID, resp.Domain, true)
+		svcID := resolveServiceIDByDomain(t, testStore, resp.Domain)
+		err = mgr.deleteServiceFromPeer(ctx, testAccountID, testPeerID, svcID, true)
 		require.NoError(t, err)
 	})
 }
@@ -1003,13 +995,14 @@ func TestStopServiceFromPeer(t *testing.T) {
 		mgr, testStore := setupIntegrationTest(t)
 
 		req := &rpservice.ExposeServiceRequest{
-			Port:     8080,
-			Protocol: "http",
+			Port: 8080,
+			Mode: "http",
 		}
 		resp, err := mgr.CreateServiceFromPeer(ctx, testAccountID, testPeerID, req)
 		require.NoError(t, err)
 
-		err = mgr.StopServiceFromPeer(ctx, testAccountID, testPeerID, resp.Domain)
+		svcID := resolveServiceIDByDomain(t, testStore, resp.Domain)
+		err = mgr.StopServiceFromPeer(ctx, testAccountID, testPeerID, svcID)
 		require.NoError(t, err)
 
 		_, err = testStore.GetServiceByDomain(ctx, resp.Domain)
@@ -1022,8 +1015,8 @@ func TestDeleteService_DeletesEphemeralExpose(t *testing.T) {
 	mgr, testStore := setupIntegrationTest(t)
 
 	resp, err := mgr.CreateServiceFromPeer(ctx, testAccountID, testPeerID, &rpservice.ExposeServiceRequest{
-		Port:     8080,
-		Protocol: "http",
+		Port: 8080,
+		Mode: "http",
 	})
 	require.NoError(t, err)
 
@@ -1042,8 +1035,8 @@ func TestDeleteService_DeletesEphemeralExpose(t *testing.T) {
 	assert.Equal(t, int64(0), count, "ephemeral service should be deleted after API delete")
 
 	_, err = mgr.CreateServiceFromPeer(ctx, testAccountID, testPeerID, &rpservice.ExposeServiceRequest{
-		Port:     9090,
-		Protocol: "http",
+		Port: 9090,
+		Mode: "http",
 	})
 	assert.NoError(t, err, "new expose should succeed after API delete")
 }
@@ -1054,8 +1047,8 @@ func TestDeleteAllServices_DeletesEphemeralExposes(t *testing.T) {
 
 	for i := range 3 {
 		_, err := mgr.CreateServiceFromPeer(ctx, testAccountID, testPeerID, &rpservice.ExposeServiceRequest{
-			Port:     8080 + i,
-			Protocol: "http",
+			Port: uint16(8080 + i),
+			Mode: "http",
 		})
 		require.NoError(t, err)
 	}
@@ -1076,21 +1069,22 @@ func TestRenewServiceFromPeer(t *testing.T) {
 	ctx := context.Background()
 
 	t.Run("renews tracked expose", func(t *testing.T) {
-		mgr, _ := setupIntegrationTest(t)
+		mgr, testStore := setupIntegrationTest(t)
 
 		resp, err := mgr.CreateServiceFromPeer(ctx, testAccountID, testPeerID, &rpservice.ExposeServiceRequest{
-			Port:     8080,
-			Protocol: "http",
+			Port: 8080,
+			Mode: "http",
 		})
 		require.NoError(t, err)
 
-		err = mgr.RenewServiceFromPeer(ctx, testAccountID, testPeerID, resp.Domain)
+		svcID := resolveServiceIDByDomain(t, testStore, resp.Domain)
+		err = mgr.RenewServiceFromPeer(ctx, testAccountID, testPeerID, svcID)
 		require.NoError(t, err)
 	})
 
 	t.Run("fails for untracked domain", func(t *testing.T) {
 		mgr, _ := setupIntegrationTest(t)
-		err := mgr.RenewServiceFromPeer(ctx, testAccountID, testPeerID, "nonexistent.com")
+		err := mgr.RenewServiceFromPeer(ctx, testAccountID, testPeerID, "nonexistent-service-id")
 		require.Error(t, err)
 	})
 }
@@ -1191,3 +1185,156 @@ func TestDeleteService_DeletesTargets(t *testing.T) {
 	require.NoError(t, err)
 	assert.Len(t, targets, 0, "All targets should be deleted when service is deleted")
 }
+
+func TestValidateProtocolChange(t *testing.T) {
+	tests := []struct {
+		name    string
+		oldP    string
+		newP    string
+		wantErr bool
+	}{
+		{"empty to http", "", "http", false},
+		{"http to http", "http", "http", false},
+		{"same protocol", "tcp", "tcp", false},
+		{"empty new proto", "tcp", "", false},
+		{"http to tcp", "http", "tcp", true},
+		{"tcp to udp", "tcp", "udp", true},
+		{"tls to http", "tls", "http", true},
+		{"udp to tls", "udp", "tls", true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := validateProtocolChange(tt.oldP, tt.newP)
+			if tt.wantErr {
+				require.Error(t, err)
+				assert.Contains(t, err.Error(), "cannot change mode")
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
+}
+
+func TestValidateTargetReferences_ResourceTypeMismatch(t *testing.T) {
+	ctx := context.Background()
+	ctrl := gomock.NewController(t)
+	mockStore := store.NewMockStore(ctrl)
+	accountID := "test-account"
+
+	tests := []struct {
+		name         string
+		targetType   rpservice.TargetType
+		resourceType resourcetypes.NetworkResourceType
+		wantErr      bool
+	}{
+		{"host matches host", rpservice.TargetTypeHost, resourcetypes.Host, false},
+		{"domain matches domain", rpservice.TargetTypeDomain, resourcetypes.Domain, false},
+		{"subnet matches subnet", rpservice.TargetTypeSubnet, resourcetypes.Subnet, false},
+		{"host but resource is domain", rpservice.TargetTypeHost, resourcetypes.Domain, true},
+		{"domain but resource is host", rpservice.TargetTypeDomain, resourcetypes.Host, true},
+		{"host but resource is subnet", rpservice.TargetTypeHost, resourcetypes.Subnet, true},
+		{"subnet but resource is domain", rpservice.TargetTypeSubnet, resourcetypes.Domain, true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			mockStore.EXPECT().
+				GetNetworkResourceByID(gomock.Any(), store.LockingStrengthShare, accountID, "resource-1").
+				Return(&resourcetypes.NetworkResource{Type: tt.resourceType}, nil)
+
+			targets := []*rpservice.Target{
+				{TargetId: "resource-1", TargetType: tt.targetType, Host: "10.0.0.1"},
+			}
+			err := validateTargetReferences(ctx, mockStore, accountID, targets)
+			if tt.wantErr {
+				require.Error(t, err)
+				assert.Contains(t, err.Error(), "target_type")
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
+}
+
+func TestValidateTargetReferences_PeerValid(t *testing.T) {
+	ctx := context.Background()
+	ctrl := gomock.NewController(t)
+	mockStore := store.NewMockStore(ctrl)
+	accountID := "test-account"
+
+	mockStore.EXPECT().
+		GetPeerByID(gomock.Any(), store.LockingStrengthShare, accountID, "peer-1").
+		Return(&nbpeer.Peer{}, nil)
+
+	targets := []*rpservice.Target{
+		{TargetId: "peer-1", TargetType: rpservice.TargetTypePeer},
+	}
+	require.NoError(t, validateTargetReferences(ctx, mockStore, accountID, targets))
+}
+
+func TestValidateSubdomainRequirement(t *testing.T) {
+	ptrBool := func(b bool) *bool { return &b }
+
+	tests := []struct {
+		name             string
+		domain           string
+		cluster          string
+		requireSubdomain *bool
+		wantErr          bool
+	}{
+		{
+			name:             "subdomain present, require_subdomain true",
+			domain:           "app.eu1.proxy.netbird.io",
+			cluster:          "eu1.proxy.netbird.io",
+			requireSubdomain: ptrBool(true),
+			wantErr:          false,
+		},
+		{
+			name:             "bare cluster domain, require_subdomain true",
+			domain:           "eu1.proxy.netbird.io",
+			cluster:          "eu1.proxy.netbird.io",
+			requireSubdomain: ptrBool(true),
+			wantErr:          true,
+		},
+		{
+			name:             "bare cluster domain, require_subdomain false",
+			domain:           "eu1.proxy.netbird.io",
+			cluster:          "eu1.proxy.netbird.io",
+			requireSubdomain: ptrBool(false),
+			wantErr:          false,
+		},
+		{
+			name:             "bare cluster domain, require_subdomain nil (default)",
+			domain:           "eu1.proxy.netbird.io",
+			cluster:          "eu1.proxy.netbird.io",
+			requireSubdomain: nil,
+			wantErr:          false,
+		},
+		{
+			name:             "custom domain apex is not the cluster",
+			domain:           "example.com",
+			cluster:          "eu1.proxy.netbird.io",
+			requireSubdomain: ptrBool(true),
+			wantErr:          false,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			ctrl := gomock.NewController(t)
+
+			mockCtrl := proxy.NewMockController(ctrl)
+			mockCtrl.EXPECT().ClusterRequireSubdomain(tc.cluster).Return(tc.requireSubdomain).AnyTimes()
+
+			mgr := &Manager{proxyController: mockCtrl}
+			err := mgr.validateSubdomainRequirement(tc.domain, tc.cluster)
+			if tc.wantErr {
+				require.Error(t, err)
+				assert.Contains(t, err.Error(), "requires a subdomain label")
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
+}