chore: add validation for port ranges

internal/api/handlers/schedulers_handler.go

@@ -30,8 +30,6 @@ import (
 
 	"github.com/topfreegames/maestro/internal/core/ports"
 
-	"github.com/go-playground/validator/v10"
-
 	"github.com/topfreegames/maestro/internal/api/handlers/requestadapters"
 	"github.com/topfreegames/maestro/internal/core/logs"
 	"go.uber.org/zap"
@@ -138,7 +136,7 @@ func (h *SchedulersHandler) CreateScheduler(ctx context.Context, request *api.Cr
 	handlerLogger.Info("handling create scheduler request")
 	scheduler, err := requestadapters.FromApiCreateSchedulerRequestToEntity(request)
 	if err != nil {
-		apiValidationError := parseValidationError(err.(validator.ValidationErrors))
+		apiValidationError := parseValidationError(err)
 		handlerLogger.Error("error parsing scheduler", zap.Error(apiValidationError))
 		return nil, status.Error(codes.InvalidArgument, apiValidationError.Error())
 	}
@@ -166,7 +164,7 @@ func (h *SchedulersHandler) NewSchedulerVersion(ctx context.Context, request *ap
 	handlerLogger.Info("handling new scheduler version request")
 	scheduler, err := requestadapters.FromApiNewSchedulerVersionRequestToEntity(request)
 	if err != nil {
-		apiValidationError := parseValidationError(err.(validator.ValidationErrors))
+		apiValidationError := parseValidationError(err)
 		handlerLogger.Error("error parsing scheduler version", zap.Error(apiValidationError))
 		return nil, status.Error(codes.InvalidArgument, apiValidationError.Error())
 	}

internal/core/entities/scheduler.go

@@ -23,6 +23,7 @@
 package entities
 
 import (
+	"errors"
 	"time"
 
 	"github.com/topfreegames/maestro/internal/core/entities/autoscaling"
@@ -50,6 +51,12 @@ const (
 	StateOnError = "on-error"
 )
 
+var (
+	ErrNoPortRangeConfigured     = errors.New("must configure scheduler.PortRange or scheduler.Spec.Container.Ports.TargetPortRange")
+	ErrBothPortRangesConfigured  = errors.New("cannot configure both scheduler.PortRange and scheduler.Spec.Container.Ports.TargetPortRange")
+	ErrMissingContainerPortRange = errors.New("must configure TargetPortRange for all scheduler.Spec.Container.Ports")
+)
+
 // Scheduler represents one of the basic maestro structs.
 // It holds GameRooms specifications, as well as optional events forwarders.
 type Scheduler struct {
@@ -108,7 +115,17 @@ func (s *Scheduler) SetSchedulerRollbackVersion(version string) {
 }
 
 func (s *Scheduler) Validate() error {
-	return validations.Validate.Struct(s)
+	err := validations.Validate.Struct(s)
+	if err != nil {
+		return err
+	}
+
+	err = s.HasValidPortRangeConfiguration()
+	if err != nil {
+		return err
+	}
+
+	return nil
 }
 
 // IsMajorVersion checks if the scheduler changes are major or not.
@@ -145,3 +162,39 @@ func (s *Scheduler) IsMajorVersion(newScheduler *Scheduler) bool {
 		),
 	)
 }
+
+// HasValidPortRangeConfiguration checks if the scheduler's port range configuration is valid.
+// It is possible to configure PortRange in the scheduler, but also TargetPortRange in each Spec.Container.Ports,
+// both port ranges were made optional in the API, so we need to validate them here.
+// The scheduler.PortRange was kept to avoid a breaking change to schedulers in older versions.
+func (s *Scheduler) HasValidPortRangeConfiguration() error {
+	hasSchedulerPortRange := false
+	if s.PortRange != nil {
+		hasSchedulerPortRange = true
+	}
+
+	hasContainerPortRange := false
+	for _, c := range s.Spec.Containers {
+		amountOfTargetPortRanges := 0
+		for _, p := range c.Ports {
+			if p.TargetPortRange != nil {
+				amountOfTargetPortRanges++
+				hasContainerPortRange = true
+			}
+		}
+
+		if !hasSchedulerPortRange && len(c.Ports) != amountOfTargetPortRanges {
+			return ErrMissingContainerPortRange
+		}
+	}
+
+	if hasSchedulerPortRange && hasContainerPortRange {
+		return ErrBothPortRangesConfigured
+	}
+
+	if !hasSchedulerPortRange && !hasContainerPortRange {
+		return ErrNoPortRangeConfigured
+	}
+
+	return nil
+}

internal/core/entities/scheduler_test.go

@@ -311,3 +311,79 @@ func TestIsMajorVersion(t *testing.T) {
 		})
 	}
 }
+
+func TestHasValidPortRangeConfiguration(t *testing.T) {
+	tests := map[string]struct {
+		scheduler *entities.Scheduler
+		expected  error
+	}{
+		"should succeed if only scheduler.portrange is configured": {
+			scheduler: &entities.Scheduler{PortRange: &port.PortRange{}},
+			expected:  nil,
+		},
+		"should succeed if only scheduler.spec.container.ports.targetportrange is configured": {
+			scheduler: &entities.Scheduler{
+				Spec: game_room.Spec{
+					Containers: []game_room.Container{
+						{
+							Ports: []game_room.ContainerPort{
+								{
+									TargetPortRange: &port.PortRange{},
+								},
+							},
+						},
+					},
+				},
+			},
+			expected: nil,
+		},
+		"should fail if neither scheduler.portrange nor container.ports.targetportrange are configured": {
+			scheduler: &entities.Scheduler{},
+			expected:  entities.ErrNoPortRangeConfigured,
+		},
+		"should fail if both scheduler.portrange and container.ports.targetportrange are configured": {
+			scheduler: &entities.Scheduler{
+				PortRange: &port.PortRange{},
+				Spec: game_room.Spec{
+					Containers: []game_room.Container{
+						{
+							Ports: []game_room.ContainerPort{
+								{
+									TargetPortRange: &port.PortRange{},
+								},
+							},
+						},
+					},
+				},
+			},
+			expected: entities.ErrBothPortRangesConfigured,
+		},
+		"should fail if not all container.ports have targetportrange configured": {
+			scheduler: &entities.Scheduler{
+				PortRange: &port.PortRange{},
+				Spec: game_room.Spec{
+					Containers: []game_room.Container{
+						{
+							Ports: []game_room.ContainerPort{
+								{
+									TargetPortRange: &port.PortRange{},
+								},
+								{
+									TargetPortRange: nil,
+								},
+							},
+						},
+					},
+				},
+			},
+			expected: entities.ErrBothPortRangesConfigured,
+		},
+	}
+
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			err := test.scheduler.HasValidPortRangeConfiguration()
+			require.ErrorIs(t, test.expected, err)
+		})
+	}
+}