Expose container ports on the collector pod

apis/v1alpha1/opentelemetrycollector_types.go

@@ -83,7 +83,7 @@ type OpenTelemetryCollectorSpec struct {
 	VolumeMounts []v1.VolumeMount `json:"volumeMounts,omitempty"`
 	// Ports allows a set of ports to be exposed by the underlying v1.Service. By default, the operator
 	// will attempt to infer the required ports by parsing the .Spec.Config property but this property can be
-	// used to open aditional ports that can't be inferred by the operator, like for custom receivers.
+	// used to open additional ports that can't be inferred by the operator, like for custom receivers.
 	// +optional
 	// +listType=atomic
 	Ports []v1.ServicePort `json:"ports,omitempty"`

apis/v1alpha1/opentelemetrycollector_webhook.go

@@ -18,6 +18,7 @@ import (
 	"fmt"
 
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/validation"
 	ctrl "sigs.k8s.io/controller-runtime"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
@@ -120,6 +121,16 @@ func (r *OpenTelemetryCollector) validateCRDSpec() error {
 		}
 	}
 
+	// validator port config
+	for _, p := range r.Spec.Ports {
+		nameErrs := validation.IsValidPortName(p.Name)
+		numErrs := validation.IsValidPortNum(int(p.Port))
+		if len(nameErrs) > 0 || len(numErrs) > 0 {
+			return fmt.Errorf("the OpenTelemetry Spec Ports configuration is incorrect, port name '%s' errors: %s, num '%d' errors: %s",
+				p.Name, nameErrs, p.Port, numErrs)
+		}
+	}
+
 	// validate autoscale with horizontal pod autoscaler
 	if r.Spec.MaxReplicas != nil {
 		if *r.Spec.MaxReplicas < int32(1) {

apis/v1alpha1/opentelemetrycollector_webhook_test.go

@@ -18,6 +18,8 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	autoscalingv2 "k8s.io/api/autoscaling/v2"
+	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -76,3 +78,255 @@ func TestOTELColDefaultingWebhook(t *testing.T) {
 		})
 	}
 }
+
+func TestOTELColValidatingWebhook(t *testing.T) {
+	zero := int32(0)
+	one := int32(1)
+	three := int32(3)
+	five := int32(5)
+
+	tests := []struct { //nolint:govet
+		name        string
+		otelcol     OpenTelemetryCollector
+		expectedErr string
+	}{
+		{
+			name:    "valid empty spec",
+			otelcol: OpenTelemetryCollector{},
+		},
+		{
+			name: "valid full spec",
+			otelcol: OpenTelemetryCollector{
+				Spec: OpenTelemetryCollectorSpec{
+					Mode:            ModeStatefulSet,
+					MinReplicas:     &one,
+					Replicas:        &three,
+					MaxReplicas:     &five,
+					UpgradeStrategy: "adhoc",
+					TargetAllocator: OpenTelemetryTargetAllocator{
+						Enabled: true,
+					},
+					Config: `receivers:
+  examplereceiver:
+    endpoint: "0.0.0.0:12345"
+  examplereceiver/settings:
+    endpoint: "0.0.0.0:12346"
+  prometheus:
+    config:
+      scrape_config:
+        job_name: otel-collector
+        scrape_interval: 10s
+  jaeger/custom:
+    protocols:
+      thrift_http:
+        endpoint: 0.0.0.0:15268
+`,
+					Ports: []v1.ServicePort{
+						{
+							Name: "port1",
+							Port: 5555,
+						},
+						{
+							Name:     "port2",
+							Port:     5554,
+							Protocol: v1.ProtocolUDP,
+						},
+					},
+					Autoscaler: &AutoscalerSpec{
+						Behavior: &autoscalingv2.HorizontalPodAutoscalerBehavior{
+							ScaleDown: &autoscalingv2.HPAScalingRules{
+								StabilizationWindowSeconds: &three,
+							},
+							ScaleUp: &autoscalingv2.HPAScalingRules{
+								StabilizationWindowSeconds: &five,
+							},
+						},
+						TargetCPUUtilization: &five,
+					},
+				},
+			},
+		},
+		{
+			name: "invalid mode with volume claim templates",
+			otelcol: OpenTelemetryCollector{
+				Spec: OpenTelemetryCollectorSpec{
+					Mode:                 ModeSidecar,
+					VolumeClaimTemplates: []v1.PersistentVolumeClaim{{}, {}},
+				},
+			},
+			expectedErr: "does not support the attribute 'volumeClaimTemplates'",
+		},
+		{
+			name: "invalid mode with tolerations",
+			otelcol: OpenTelemetryCollector{
+				Spec: OpenTelemetryCollectorSpec{
+					Mode:        ModeSidecar,
+					Tolerations: []v1.Toleration{{}, {}},
+				},
+			},
+			expectedErr: "does not support the attribute 'tolerations'",
+		},
+		{
+			name: "invalid mode with target allocator",
+			otelcol: OpenTelemetryCollector{
+				Spec: OpenTelemetryCollectorSpec{
+					Mode: ModeDeployment,
+					TargetAllocator: OpenTelemetryTargetAllocator{
+						Enabled: true,
+					},
+				},
+			},
+			expectedErr: "does not support the target allocation deployment",
+		},
+		{
+			name: "invalid target allocator config",
+			otelcol: OpenTelemetryCollector{
+				Spec: OpenTelemetryCollectorSpec{
+					Mode: ModeStatefulSet,
+					TargetAllocator: OpenTelemetryTargetAllocator{
+						Enabled: true,
+					},
+				},
+			},
+			expectedErr: "the OpenTelemetry Spec Prometheus configuration is incorrect",
+		},
+		{
+			name: "invalid port name",
+			otelcol: OpenTelemetryCollector{
+				Spec: OpenTelemetryCollectorSpec{
+					Ports: []v1.ServicePort{
+						{
+							// this port name contains a non alphanumeric character, which is invalid.
+							Name:     "-test🦄port",
+							Port:     12345,
+							Protocol: v1.ProtocolTCP,
+						},
+					},
+				},
+			},
+			expectedErr: "the OpenTelemetry Spec Ports configuration is incorrect",
+		},
+		{
+			name: "invalid port name, too long",
+			otelcol: OpenTelemetryCollector{
+				Spec: OpenTelemetryCollectorSpec{
+					Ports: []v1.ServicePort{
+						{
+							Name: "aaaabbbbccccdddd", // len: 16, too long
+							Port: 5555,
+						},
+					},
+				},
+			},
+			expectedErr: "the OpenTelemetry Spec Ports configuration is incorrect",
+		},
+		{
+			name: "invalid port num",
+			otelcol: OpenTelemetryCollector{
+				Spec: OpenTelemetryCollectorSpec{
+					Ports: []v1.ServicePort{
+						{
+							Name: "aaaabbbbccccddd", // len: 15
+							// no port set means it's 0, which is invalid
+						},
+					},
+				},
+			},
+			expectedErr: "the OpenTelemetry Spec Ports configuration is incorrect",
+		},
+		{
+			name: "invalid max replicas",
+			otelcol: OpenTelemetryCollector{
+				Spec: OpenTelemetryCollectorSpec{
+					MaxReplicas: &zero,
+				},
+			},
+			expectedErr: "maxReplicas should be defined and more than one",
+		},
+		{
+			name: "invalid replicas, greater than max",
+			otelcol: OpenTelemetryCollector{
+				Spec: OpenTelemetryCollectorSpec{
+					MaxReplicas: &three,
+					Replicas:    &five,
+				},
+			},
+			expectedErr: "replicas must not be greater than maxReplicas",
+		},
+		{
+			name: "invalid min replicas, greater than max",
+			otelcol: OpenTelemetryCollector{
+				Spec: OpenTelemetryCollectorSpec{
+					MaxReplicas: &three,
+					MinReplicas: &five,
+				},
+			},
+			expectedErr: "minReplicas must not be greater than maxReplicas",
+		},
+		{
+			name: "invalid min replicas, lesser than 1",
+			otelcol: OpenTelemetryCollector{
+				Spec: OpenTelemetryCollectorSpec{
+					MaxReplicas: &three,
+					MinReplicas: &zero,
+				},
+			},
+			expectedErr: "minReplicas should be one or more",
+		},
+		{
+			name: "invalid autoscaler scale down",
+			otelcol: OpenTelemetryCollector{
+				Spec: OpenTelemetryCollectorSpec{
+					MaxReplicas: &three,
+					Autoscaler: &AutoscalerSpec{
+						Behavior: &autoscalingv2.HorizontalPodAutoscalerBehavior{
+							ScaleDown: &autoscalingv2.HPAScalingRules{
+								StabilizationWindowSeconds: &zero,
+							},
+						},
+					},
+				},
+			},
+			expectedErr: "scaleDown should be one or more",
+		},
+		{
+			name: "invalid autoscaler scale up",
+			otelcol: OpenTelemetryCollector{
+				Spec: OpenTelemetryCollectorSpec{
+					MaxReplicas: &three,
+					Autoscaler: &AutoscalerSpec{
+						Behavior: &autoscalingv2.HorizontalPodAutoscalerBehavior{
+							ScaleUp: &autoscalingv2.HPAScalingRules{
+								StabilizationWindowSeconds: &zero,
+							},
+						},
+					},
+				},
+			},
+			expectedErr: "scaleUp should be one or more",
+		},
+		{
+			name: "invalid autoscaler target cpu utilization",
+			otelcol: OpenTelemetryCollector{
+				Spec: OpenTelemetryCollectorSpec{
+					MaxReplicas: &three,
+					Autoscaler: &AutoscalerSpec{
+						TargetCPUUtilization: &zero,
+					},
+				},
+			},
+			expectedErr: "targetCPUUtilization should be greater than 0 and less than 100",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			err := test.otelcol.validateCRDSpec()
+			if test.expectedErr == "" {
+				assert.NoError(t, err)
+				return
+			}
+			assert.ErrorContains(t, err, test.expectedErr)
+		})
+	}
+}

bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml

@@ -556,7 +556,7 @@ spec:
                 description: Ports allows a set of ports to be exposed by the underlying
                   v1.Service. By default, the operator will attempt to infer the required
                   ports by parsing the .Spec.Config property but this property can
-                  be used to open aditional ports that can't be inferred by the operator,
+                  be used to open additional ports that can't be inferred by the operator,
                   like for custom receivers.
                 items:
                   description: ServicePort contains information on service's port.

config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml

@@ -554,7 +554,7 @@ spec:
                 description: Ports allows a set of ports to be exposed by the underlying
                   v1.Service. By default, the operator will attempt to infer the required
                   ports by parsing the .Spec.Config property but this property can
-                  be used to open aditional ports that can't be inferred by the operator,
+                  be used to open additional ports that can't be inferred by the operator,
                   like for custom receivers.
                 items:
                   description: ServicePort contains information on service's port.

docs/api.md

@@ -1792,7 +1792,7 @@ OpenTelemetryCollectorSpec defines the desired state of OpenTelemetryCollector.
         <td><b><a href="#opentelemetrycollectorspecportsindex">ports</a></b></td>
         <td>[]object</td>
         <td>
-          Ports allows a set of ports to be exposed by the underlying v1.Service. By default, the operator will attempt to infer the required ports by parsing the .Spec.Config property but this property can be used to open aditional ports that can't be inferred by the operator, like for custom receivers.<br/>
+          Ports allows a set of ports to be exposed by the underlying v1.Service. By default, the operator will attempt to infer the required ports by parsing the .Spec.Config property but this property can be used to open additional ports that can't be inferred by the operator, like for custom receivers.<br/>
         </td>
         <td>false</td>
       </tr><tr>

pkg/collector/adapters/config_to_ports_test.go

@@ -31,8 +31,7 @@ import (
 
 var logger = logf.Log.WithName("unit-tests")
 
-func TestExtractPortsFromConfig(t *testing.T) {
-	configStr := `receivers:
+var portConfigStr = `receivers:
   examplereceiver:
     endpoint: "0.0.0.0:12345"
   examplereceiver/settings:
@@ -77,8 +76,9 @@ service:
       exporters: [logging]
 `
 
+func TestExtractPortsFromConfig(t *testing.T) {
 	// prepare
-	config, err := adapters.ConfigFromString(configStr)
+	config, err := adapters.ConfigFromString(portConfigStr)
 	require.NoError(t, err)
 	require.NotEmpty(t, config)
 
@@ -94,18 +94,20 @@ service:
 	targetPort4317 := intstr.IntOrString{Type: 0, IntVal: 4317, StrVal: ""}
 	targetPort4318 := intstr.IntOrString{Type: 0, IntVal: 4318, StrVal: ""}
 
-	assert.Len(t, ports, 11)
-	assert.Equal(t, corev1.ServicePort{Name: "examplereceiver", Port: int32(12345)}, ports[0])
-	assert.Equal(t, corev1.ServicePort{Name: "examplereceiver-settings", Port: int32(12346)}, ports[1])
-	assert.Equal(t, corev1.ServicePort{Name: "jaeger-custom-thrift-http", AppProtocol: &httpAppProtocol, Protocol: "TCP", Port: int32(15268), TargetPort: targetPortZero}, ports[2])
-	assert.Equal(t, corev1.ServicePort{Name: "jaeger-grpc", AppProtocol: &grpcAppProtocol, Protocol: "TCP", Port: int32(14250)}, ports[3])
-	assert.Equal(t, corev1.ServicePort{Name: "jaeger-thrift-binary", Protocol: "UDP", Port: int32(6833)}, ports[4])
-	assert.Equal(t, corev1.ServicePort{Name: "jaeger-thrift-compact", Protocol: "UDP", Port: int32(6831)}, ports[5])
-	assert.Equal(t, corev1.ServicePort{Name: "otlp-2-grpc", AppProtocol: &grpcAppProtocol, Protocol: "TCP", Port: int32(55555)}, ports[6])
-	assert.Equal(t, corev1.ServicePort{Name: "otlp-grpc", AppProtocol: &grpcAppProtocol, Port: int32(4317), TargetPort: targetPort4317}, ports[7])
-	assert.Equal(t, corev1.ServicePort{Name: "otlp-http", AppProtocol: &httpAppProtocol, Port: int32(4318), TargetPort: targetPort4318}, ports[8])
-	assert.Equal(t, corev1.ServicePort{Name: "otlp-http-legacy", AppProtocol: &httpAppProtocol, Port: int32(55681), TargetPort: targetPort4318}, ports[9])
-	assert.Equal(t, corev1.ServicePort{Name: "zipkin", AppProtocol: &httpAppProtocol, Protocol: "TCP", Port: int32(9411)}, ports[10])
+	expectedPorts := []corev1.ServicePort{
+		{Name: "examplereceiver", Port: 12345},
+		{Name: "examplereceiver-settings", Port: 12346},
+		{Name: "jaeger-custom-thrift-http", AppProtocol: &httpAppProtocol, Protocol: "TCP", Port: 15268, TargetPort: targetPortZero},
+		{Name: "jaeger-grpc", AppProtocol: &grpcAppProtocol, Protocol: "TCP", Port: 14250},
+		{Name: "jaeger-thrift-binary", Protocol: "UDP", Port: 6833},
+		{Name: "jaeger-thrift-compact", Protocol: "UDP", Port: 6831},
+		{Name: "otlp-2-grpc", AppProtocol: &grpcAppProtocol, Protocol: "TCP", Port: 55555},
+		{Name: "otlp-grpc", AppProtocol: &grpcAppProtocol, Port: 4317, TargetPort: targetPort4317},
+		{Name: "otlp-http", AppProtocol: &httpAppProtocol, Port: 4318, TargetPort: targetPort4318},
+		{Name: "otlp-http-legacy", AppProtocol: &httpAppProtocol, Port: 55681, TargetPort: targetPort4318},
+		{Name: "zipkin", AppProtocol: &httpAppProtocol, Protocol: "TCP", Port: 9411},
+	}
+	assert.ElementsMatch(t, expectedPorts, ports)
 }
 
 func TestNoPortsParsed(t *testing.T) {