catalog: validating Protocol and Health enums on Service, Workload, a…

…nd ServiceEndpoints (#18554)

internal/catalog/internal/types/service.go

@@ -6,10 +6,11 @@ package types
 import (
 	"math"
 
+	"github.com/hashicorp/go-multierror"
+
 	"github.com/hashicorp/consul/internal/resource"
 	pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v1alpha1"
 	"github.com/hashicorp/consul/proto-public/pbresource"
-	"github.com/hashicorp/go-multierror"
 )
 
 const (
@@ -87,6 +88,17 @@ func ValidateService(res *pbresource.Resource) error {
 			})
 		}
 
+		if protoErr := validateProtocol(port.Protocol); protoErr != nil {
+			err = multierror.Append(err, resource.ErrInvalidListElement{
+				Name:  "ports",
+				Index: idx,
+				Wrapped: resource.ErrInvalidField{
+					Name:    "protocol",
+					Wrapped: protoErr,
+				},
+			})
+		}
+
 		// validate the virtual port is within the allowed range - 0 is allowed
 		// to signify that no virtual port should be used and the port will not
 		// be available for transparent proxying within the mesh.

internal/catalog/internal/types/service_endpoints.go

@@ -6,10 +6,11 @@ package types
 import (
 	"math"
 
+	"github.com/hashicorp/go-multierror"
+
 	"github.com/hashicorp/consul/internal/resource"
 	pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v1alpha1"
 	"github.com/hashicorp/consul/proto-public/pbresource"
-	"github.com/hashicorp/go-multierror"
 )
 
 const (
@@ -128,6 +129,13 @@ func validateEndpoint(endpoint *pbcatalog.Endpoint, res *pbresource.Resource) er
 		})
 	}
 
+	if healthErr := validateHealth(endpoint.HealthStatus); healthErr != nil {
+		err = multierror.Append(err, resource.ErrInvalidField{
+			Name:    "health_status",
+			Wrapped: healthErr,
+		})
+	}
+
 	// Validate the endpoints ports
 	for portName, port := range endpoint.Ports {
 		// Port names must be DNS labels
@@ -139,14 +147,25 @@ func validateEndpoint(endpoint *pbcatalog.Endpoint, res *pbresource.Resource) er
 			})
 		}
 
+		if protoErr := validateProtocol(port.Protocol); protoErr != nil {
+			err = multierror.Append(err, resource.ErrInvalidMapValue{
+				Map: "ports",
+				Key: portName,
+				Wrapped: resource.ErrInvalidField{
+					Name:    "protocol",
+					Wrapped: protoErr,
+				},
+			})
+		}
+
 		// As the physical port is the real port the endpoint will be bound to
 		// it must be in the standard 1-65535 range.
 		if port.Port < 1 || port.Port > math.MaxUint16 {
 			err = multierror.Append(err, resource.ErrInvalidMapValue{
 				Map: "ports",
 				Key: portName,
 				Wrapped: resource.ErrInvalidField{
-					Name:    "phsical_port",
+					Name:    "physical_port",
 					Wrapped: errInvalidPhysicalPort,
 				},
 			})

internal/catalog/internal/types/service_endpoints_test.go

@@ -6,11 +6,12 @@ package types
 import (
 	"testing"
 
+	"github.com/stretchr/testify/require"
+
 	"github.com/hashicorp/consul/internal/resource"
 	rtest "github.com/hashicorp/consul/internal/resource/resourcetest"
 	pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v1alpha1"
 	"github.com/hashicorp/consul/proto-public/pbresource"
-	"github.com/stretchr/testify/require"
 )
 
 var (
@@ -130,6 +131,20 @@ func TestValidateServiceEndpoints_EndpointInvalid(t *testing.T) {
 				require.ErrorIs(t, err, resource.ErrEmpty)
 			},
 		},
+		"invalid-health-status": {
+			modify: func(endpoint *pbcatalog.Endpoint) {
+				endpoint.Ports["foo"] = &pbcatalog.WorkloadPort{
+					Port: 42,
+				}
+				endpoint.HealthStatus = 99
+			},
+			validateErr: func(t *testing.T, err error) {
+				rtest.RequireError(t, err, resource.ErrInvalidField{
+					Name:    "health_status",
+					Wrapped: resource.NewConstError("not a supported enum value: 99"),
+				})
+			},
+		},
 		"invalid-port-name": {
 			modify: func(endpoint *pbcatalog.Endpoint) {
 				endpoint.Ports[""] = &pbcatalog.WorkloadPort{
@@ -144,6 +159,24 @@ func TestValidateServiceEndpoints_EndpointInvalid(t *testing.T) {
 				})
 			},
 		},
+		"invalid-port-protocol": {
+			modify: func(endpoint *pbcatalog.Endpoint) {
+				endpoint.Ports["foo"] = &pbcatalog.WorkloadPort{
+					Port:     42,
+					Protocol: 99,
+				}
+			},
+			validateErr: func(t *testing.T, err error) {
+				rtest.RequireError(t, err, resource.ErrInvalidMapValue{
+					Map: "ports",
+					Key: "foo",
+					Wrapped: resource.ErrInvalidField{
+						Name:    "protocol",
+						Wrapped: resource.NewConstError("not a supported enum value: 99"),
+					},
+				})
+			},
+		},
 		"port-0": {
 			modify: func(endpoint *pbcatalog.Endpoint) {
 				endpoint.Ports["foo"].Port = 0

internal/catalog/internal/types/service_test.go

@@ -6,12 +6,13 @@ package types
 import (
 	"testing"
 
-	"github.com/hashicorp/consul/internal/resource"
-	pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v1alpha1"
-	"github.com/hashicorp/consul/proto-public/pbresource"
 	"github.com/stretchr/testify/require"
 	"google.golang.org/protobuf/reflect/protoreflect"
 	"google.golang.org/protobuf/types/known/anypb"
+
+	"github.com/hashicorp/consul/internal/resource"
+	pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v1alpha1"
+	"github.com/hashicorp/consul/proto-public/pbresource"
 )
 
 func createServiceResource(t *testing.T, data protoreflect.ProtoMessage) *pbresource.Resource {
@@ -171,6 +172,37 @@ func TestValidateService_VirtualPortReused(t *testing.T) {
 	require.EqualValues(t, 42, actual.Value)
 }
 
+func TestValidateService_InvalidPortProtocol(t *testing.T) {
+	data := &pbcatalog.Service{
+		Workloads: &pbcatalog.WorkloadSelector{
+			Prefixes: []string{""},
+		},
+		Ports: []*pbcatalog.ServicePort{
+			{
+				TargetPort: "foo",
+				Protocol:   99,
+			},
+		},
+	}
+
+	res := createServiceResource(t, data)
+
+	err := ValidateService(res)
+
+	expected := resource.ErrInvalidListElement{
+		Name:  "ports",
+		Index: 0,
+		Wrapped: resource.ErrInvalidField{
+			Name:    "protocol",
+			Wrapped: resource.NewConstError("not a supported enum value: 99"),
+		},
+	}
+
+	var actual resource.ErrInvalidListElement
+	require.ErrorAs(t, err, &actual)
+	require.Equal(t, expected, actual)
+}
+
 func TestValidateService_VirtualPortInvalid(t *testing.T) {
 	data := &pbcatalog.Service{
 		Workloads: &pbcatalog.WorkloadSelector{

internal/catalog/internal/types/validators.go

@@ -4,15 +4,17 @@
 package types
 
 import (
+	"fmt"
 	"net"
 	"regexp"
 	"strings"
 
+	"github.com/hashicorp/go-multierror"
+	"google.golang.org/protobuf/proto"
+
 	"github.com/hashicorp/consul/internal/resource"
 	pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v1alpha1"
 	"github.com/hashicorp/consul/proto-public/pbresource"
-	"github.com/hashicorp/go-multierror"
-	"google.golang.org/protobuf/proto"
 )
 
 const (
@@ -135,6 +137,19 @@ func validatePortName(name string) error {
 	return nil
 }
 
+func validateProtocol(protocol pbcatalog.Protocol) error {
+	switch protocol {
+	case pbcatalog.Protocol_PROTOCOL_TCP,
+		pbcatalog.Protocol_PROTOCOL_HTTP,
+		pbcatalog.Protocol_PROTOCOL_HTTP2,
+		pbcatalog.Protocol_PROTOCOL_GRPC,
+		pbcatalog.Protocol_PROTOCOL_MESH:
+		return nil
+	default:
+		return resource.NewConstError(fmt.Sprintf("not a supported enum value: %v", protocol))
+	}
+}
+
 // validateWorkloadAddress will validate the WorkloadAddress type. This involves validating
 // the Host within the workload address and the ports references. For ports references we
 // ensure that values in the addresses ports array are present in the set of map keys.
@@ -207,3 +222,16 @@ func validateReference(allowedType *pbresource.Type, allowedTenancy *pbresource.
 
 	return err
 }
+
+func validateHealth(health pbcatalog.Health) error {
+	switch health {
+	case pbcatalog.Health_HEALTH_ANY,
+		pbcatalog.Health_HEALTH_PASSING,
+		pbcatalog.Health_HEALTH_WARNING,
+		pbcatalog.Health_HEALTH_CRITICAL,
+		pbcatalog.Health_HEALTH_MAINTENANCE:
+		return nil
+	default:
+		return resource.NewConstError(fmt.Sprintf("not a supported enum value: %v", health))
+	}
+}

internal/catalog/internal/types/validators_test.go

@@ -8,11 +8,12 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/hashicorp/go-multierror"
+	"github.com/stretchr/testify/require"
+
 	"github.com/hashicorp/consul/internal/resource"
 	pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v1alpha1"
 	"github.com/hashicorp/consul/proto-public/pbresource"
-	"github.com/hashicorp/go-multierror"
-	"github.com/stretchr/testify/require"
 )
 
 func TestIsValidDNSLabel(t *testing.T) {

internal/catalog/internal/types/workload.go

@@ -7,10 +7,11 @@ import (
 	"math"
 	"sort"
 
+	"github.com/hashicorp/go-multierror"
+
 	"github.com/hashicorp/consul/internal/resource"
 	pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v1alpha1"
 	"github.com/hashicorp/consul/proto-public/pbresource"
-	"github.com/hashicorp/go-multierror"
 )
 
 const (
@@ -76,6 +77,17 @@ func ValidateWorkload(res *pbresource.Resource) error {
 			})
 		}
 
+		if protoErr := validateProtocol(port.Protocol); protoErr != nil {
+			err = multierror.Append(err, resource.ErrInvalidMapValue{
+				Map: "ports",
+				Key: portName,
+				Wrapped: resource.ErrInvalidField{
+					Name:    "protocol",
+					Wrapped: protoErr,
+				},
+			})
+		}
+
 		// Collect the list of mesh ports
 		if port.Protocol == pbcatalog.Protocol_PROTOCOL_MESH {
 			meshPorts = append(meshPorts, portName)

internal/catalog/internal/types/workload_test.go

@@ -6,12 +6,13 @@ package types
 import (
 	"testing"
 
-	"github.com/hashicorp/consul/internal/resource"
-	pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v1alpha1"
-	"github.com/hashicorp/consul/proto-public/pbresource"
 	"github.com/stretchr/testify/require"
 	"google.golang.org/protobuf/reflect/protoreflect"
 	"google.golang.org/protobuf/types/known/anypb"
+
+	"github.com/hashicorp/consul/internal/resource"
+	pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v1alpha1"
+	"github.com/hashicorp/consul/proto-public/pbresource"
 )
 
 func createWorkloadResource(t *testing.T, data protoreflect.ProtoMessage) *pbresource.Resource {
@@ -227,6 +228,30 @@ func TestValidateWorkload_InvalidPortName(t *testing.T) {
 	require.Equal(t, expected, actual)
 }
 
+func TestValidateWorkload_InvalidPortProtocol(t *testing.T) {
+	data := validWorkload()
+	data.Ports["foo"] = &pbcatalog.WorkloadPort{
+		Port:     42,
+		Protocol: 99,
+	}
+
+	res := createWorkloadResource(t, data)
+
+	err := ValidateWorkload(res)
+	require.Error(t, err)
+	expected := resource.ErrInvalidMapValue{
+		Map: "ports",
+		Key: "foo",
+		Wrapped: resource.ErrInvalidField{
+			Name:    "protocol",
+			Wrapped: resource.NewConstError("not a supported enum value: 99"),
+		},
+	}
+	var actual resource.ErrInvalidMapValue
+	require.ErrorAs(t, err, &actual)
+	require.Equal(t, expected, actual)
+}
+
 func TestValidateWorkload_Port0(t *testing.T) {
 	data := validWorkload()
 	data.Ports["bar"] = &pbcatalog.WorkloadPort{Port: 0}