Repository: open-telemetry/opentelemetry-operator

Issue:
Ports are being duplicated for containers when user provides them
### Component(s)

collector

### What happened?

When getting the ports that are needed for the different components, we ensure they are not duplicated for `Services` and `Ingress` but not for `Containers`.

We even have a test that asserts it should be duplicated (`TestContainerPorts/ports in spec Config and ContainerPorts`).

Also, the way the ports are renamed if there is a collision, is different in the case of containers.

### Kubernetes Version

.

### Operator version

main

### Collector version

.

### Environment information

_No response_

### Log output

```shell

```

### Additional context

_No response_

Discussion:

Solution patch:
diff --git a/.chloggen/3950.yaml b/.chloggen/3950.yaml
new file mode 100755
index 00000000..54f55b2f
--- /dev/null
+++ b/.chloggen/3950.yaml
@@ -0,0 +1,21 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: bug_fix
+
+# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action)
+component: collector
+
+# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Fixes container port duplication for user-provided and operator inferred ports
+
+# One or more tracking issues related to the change
+issues: [3950]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext: |
+  This patch improves container port collisions when the user defines additional ports in the CR.
+
+  When ports are specified in the CR:
+  - If a port number from the CR matches a port inferred from the config, the inferred port is removed
+  - If a port name from the CR matches an inferred port name, the inferred port is renamed to "port-{number}"
diff --git a/.github/workflows/changelog.yaml b/.github/workflows/changelog.yaml
index a81483a2..8d035da6 100644
--- a/.github/workflows/changelog.yaml
+++ b/.github/workflows/changelog.yaml
@@ -36,7 +36,7 @@ jobs:
       - name: Setup Go
         uses: actions/setup-go@0aaccfd150d50ccaeb58ebd88d36e91967a5f35b # v5.4.0
         with:
-          go-version: "~1.24.2"
+          go-version: "~1.24.3"
 
       - name: Ensure no changes to the CHANGELOG
         run: |
diff --git a/.github/workflows/continuous-integration.yaml b/.github/workflows/continuous-integration.yaml
index 29f30293..e15a0026 100644
--- a/.github/workflows/continuous-integration.yaml
+++ b/.github/workflows/continuous-integration.yaml
@@ -22,7 +22,7 @@ jobs:
       uses: actions/setup-go@0aaccfd150d50ccaeb58ebd88d36e91967a5f35b # v5.4.0
       id: setup-go
       with:
-        go-version: "~1.24.2"
+        go-version: "~1.24.3"
 
     - name: Cache tools
       uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 # v4.2.3
@@ -47,7 +47,7 @@ jobs:
       uses: actions/setup-go@0aaccfd150d50ccaeb58ebd88d36e91967a5f35b # v5.4.0
       id: setup-go
       with:
-        go-version: "~1.24.2"
+        go-version: "~1.24.3"
 
     - name: Cache tools
       uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 # v4.2.3
@@ -79,7 +79,7 @@ jobs:
     - name: Set up Go
       uses: actions/setup-go@0aaccfd150d50ccaeb58ebd88d36e91967a5f35b # v5.4.0
       with:
-        go-version: "~1.24.2"
+        go-version: "~1.24.3"
 
     - name: Initialize CodeQL
       uses: github/codeql-action/init@60168efe1c415ce0f5521ea06d5c2062adbeed1b # v3.28.17
diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml
index 4d6e2785..b429bfa8 100644
--- a/.github/workflows/e2e.yaml
+++ b/.github/workflows/e2e.yaml
@@ -27,7 +27,7 @@ jobs:
         uses: actions/setup-go@0aaccfd150d50ccaeb58ebd88d36e91967a5f35b # v5.4.0
         id: setup-go
         with:
-          go-version: "~1.24.2"
+          go-version: "~1.24.3"
 
       - name: Cache tools
         uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 # v4.2.3
@@ -100,7 +100,7 @@ jobs:
       uses: actions/setup-go@0aaccfd150d50ccaeb58ebd88d36e91967a5f35b # v5.4.0
       id: setup-go
       with:
-        go-version: "~1.24.2"
+        go-version: "~1.24.3"
 
     - name: Cache tools
       uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 # v4.2.3
diff --git a/.github/workflows/publish-images.yaml b/.github/workflows/publish-images.yaml
index 85a47dc0..3ccdac8b 100644
--- a/.github/workflows/publish-images.yaml
+++ b/.github/workflows/publish-images.yaml
@@ -25,7 +25,7 @@ jobs:
 
       - uses: actions/setup-go@0aaccfd150d50ccaeb58ebd88d36e91967a5f35b # v5.4.0
         with:
-          go-version: "~1.24.2"
+          go-version: "~1.24.3"
 
       - name: Unshallow
         run: git fetch --prune --unshallow
diff --git a/.github/workflows/publish-must-gather.yaml b/.github/workflows/publish-must-gather.yaml
index 3c08dbd0..7af7efc8 100644
--- a/.github/workflows/publish-must-gather.yaml
+++ b/.github/workflows/publish-must-gather.yaml
@@ -25,7 +25,7 @@ jobs:
 
       - uses: actions/setup-go@0aaccfd150d50ccaeb58ebd88d36e91967a5f35b # v5.4.0
         with:
-          go-version: "~1.24.2"
+          go-version: "~1.24.3"
 
       - name: Unshallow
         run: git fetch --prune --unshallow
diff --git a/.github/workflows/publish-operator-opamp-bridge.yaml b/.github/workflows/publish-operator-opamp-bridge.yaml
index c61f1a1f..49f5adb2 100644
--- a/.github/workflows/publish-operator-opamp-bridge.yaml
+++ b/.github/workflows/publish-operator-opamp-bridge.yaml
@@ -29,7 +29,7 @@ jobs:
 
       - uses: actions/setup-go@0aaccfd150d50ccaeb58ebd88d36e91967a5f35b # v5.4.0
         with:
-          go-version: "~1.24.2"
+          go-version: "~1.24.3"
 
       # TODO: We're currently not using this. Should we?
       - name: Read version
diff --git a/.github/workflows/publish-target-allocator.yaml b/.github/workflows/publish-target-allocator.yaml
index 2cc1aea5..06f085d6 100644
--- a/.github/workflows/publish-target-allocator.yaml
+++ b/.github/workflows/publish-target-allocator.yaml
@@ -29,7 +29,7 @@ jobs:
 
       - uses: actions/setup-go@0aaccfd150d50ccaeb58ebd88d36e91967a5f35b # v5.4.0
         with:
-          go-version: "~1.24.2"
+          go-version: "~1.24.3"
 
       # TODO: We're currently not using this. Should we?
       - name: Read version
diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml
index a078510a..3a79032c 100644
--- a/.github/workflows/release.yaml
+++ b/.github/workflows/release.yaml
@@ -36,7 +36,7 @@ jobs:
     - name: Set up Go
       uses: actions/setup-go@0aaccfd150d50ccaeb58ebd88d36e91967a5f35b # v5.4.0
       with:
-        go-version: "~1.24.2"
+        go-version: "~1.24.3"
 
     - name: "generate release resources"
       run: make release-artifacts IMG_PREFIX="ghcr.io/open-telemetry/opentelemetry-operator" VERSION=${DESIRED_VERSION}
diff --git a/.github/workflows/scorecard.yaml b/.github/workflows/scorecard.yaml
index a61e7ae3..546379c4 100644
--- a/.github/workflows/scorecard.yaml
+++ b/.github/workflows/scorecard.yaml
@@ -29,7 +29,7 @@ jobs:
         uses: actions/setup-go@0aaccfd150d50ccaeb58ebd88d36e91967a5f35b # v5.4.0
         id: setup-go
         with:
-          go-version: "~1.24.2"
+          go-version: "~1.24.3"
 
       - name: Check out code into the Go module directory
         uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
diff --git a/internal/manifests/collector/container.go b/internal/manifests/collector/container.go
index feb4f2b3..676918e9 100644
--- a/internal/manifests/collector/container.go
+++ b/internal/manifests/collector/container.go
@@ -32,20 +32,7 @@ func Container(cfg config.Config, logger logr.Logger, otelcol v1beta1.OpenTeleme
 		image = cfg.CollectorImage()
 	}
 
-	// build container ports from service ports
-	ports, err := getConfigContainerPorts(logger, otelcol.Spec.Config)
-	if err != nil {
-		logger.Error(err, "container ports config")
-	}
-
-	for _, p := range otelcol.Spec.Ports {
-		ports[p.Name] = corev1.ContainerPort{
-			Name:          p.Name,
-			ContainerPort: p.Port,
-			Protocol:      p.Protocol,
-			HostPort:      p.HostPort,
-		}
-	}
+	ports := getContainerPorts(logger, otelcol)
 
 	var volumeMounts []corev1.VolumeMount
 	argsMap := otelcol.Spec.Args
@@ -122,7 +109,7 @@ func Container(cfg config.Config, logger logr.Logger, otelcol v1beta1.OpenTeleme
 		Name:            naming.Container(),
 		Image:           image,
 		ImagePullPolicy: otelcol.Spec.ImagePullPolicy,
-		Ports:           portMapToList(ports),
+		Ports:           ports,
 		VolumeMounts:    volumeMounts,
 		Args:            args,
 		Env:             getContainerEnvVars(otelcol, logger),
@@ -135,8 +122,8 @@ func Container(cfg config.Config, logger logr.Logger, otelcol v1beta1.OpenTeleme
 	}
 }
 
-func getConfigContainerPorts(logger logr.Logger, conf v1beta1.Config) (map[string]corev1.ContainerPort, error) {
-	ports := map[string]corev1.ContainerPort{}
+func getConfigContainerPorts(logger logr.Logger, conf v1beta1.Config) ([]corev1.ContainerPort, error) {
+	ports := []corev1.ContainerPort{}
 	ps, err := conf.GetAllPorts(logger)
 	if err != nil {
 		return ports, err
@@ -155,11 +142,11 @@ func getConfigContainerPorts(logger logr.Logger, conf v1beta1.Config) (map[strin
 					"port.name.errs", nameErrs, "num.errs", numErrs)
 				continue
 			}
-			ports[truncName] = corev1.ContainerPort{
+			ports = append(ports, corev1.ContainerPort{
 				Name:          truncName,
 				ContainerPort: p.Port,
 				Protocol:      p.Protocol,
-			}
+			})
 		}
 	}
 
@@ -169,26 +156,15 @@ func getConfigContainerPorts(logger logr.Logger, conf v1beta1.Config) (map[strin
 		metricsPort = 8888
 	}
 
-	ports["metrics"] = corev1.ContainerPort{
+	ports = append(ports, corev1.ContainerPort{
 		Name:          "metrics",
 		ContainerPort: metricsPort,
 		Protocol:      corev1.ProtocolTCP,
-	}
+	})
 
 	return ports, nil
 }
 
-func portMapToList(portMap map[string]corev1.ContainerPort) []corev1.ContainerPort {
-	ports := make([]corev1.ContainerPort, 0, len(portMap))
-	for _, p := range portMap {
-		ports = append(ports, p)
-	}
-	sort.Slice(ports, func(i, j int) bool {
-		return ports[i].Name < ports[j].Name
-	})
-	return ports
-}
-
 func defaultProbeSettings(probe *corev1.Probe, probeConfig *v1beta1.Probe) {
 	if probe != nil && probeConfig != nil {
 		if probeConfig.InitialDelaySeconds != nil {
@@ -210,6 +186,108 @@ func defaultProbeSettings(probe *corev1.Probe, probeConfig *v1beta1.Probe) {
 	}
 }
 
+func getContainerPorts(logger logr.Logger, otelcol v1beta1.OpenTelemetryCollector) []corev1.ContainerPort {
+	// build container ports from service ports
+	ports, err := getConfigContainerPorts(logger, otelcol.Spec.Config)
+	if err != nil {
+		logger.Error(err, "container ports config")
+	}
+
+	if len(otelcol.Spec.Ports) > 0 {
+		// we should add all the ports from the CR
+		// there are two cases where problems might occur:
+		// 1) when the port number is already being used by a receiver
+		// 2) same, but for the port name
+		//
+		// in the first case, we remove the port we inferred from the list
+		// in the second case, we rename our inferred port to something like "port-%d"
+		portNumbers, portNames := extractPortNumbersAndNames(otelcol.Spec.Ports)
+		var resultingInferredPorts []corev1.ContainerPort
+		for _, inferred := range ports {
+			if filtered := filterContainerPort(logger, inferred, portNumbers, portNames); filtered != nil {
+				resultingInferredPorts = append(resultingInferredPorts, *filtered)
+			}
+		}
+		specPorts := getSpecPorts(otelcol.Spec.Ports, logger)
+		ports = append(specPorts, resultingInferredPorts...)
+	}
+
+	sort.Slice(ports, func(i, j int) bool {
+		return ports[i].Name < ports[j].Name
+	})
+	return ports
+}
+
+// filterContainerPort filters container ports to avoid conflicts with user-specified ports.
+// If the candidate port number is already in use, returns nil.
+// If the candidate port name conflicts with an existing name, attempts to use a fallback name of format "port-{number}".
+// If both the original name and fallback name are taken, returns nil with a warning log.
+// Otherwise returns the (potentially renamed) candidate port.
+func filterContainerPort(logger logr.Logger, candidate corev1.ContainerPort, portNumbers map[PortNumberKey]bool, portNames map[string]bool) *corev1.ContainerPort {
+	if portNumbers[newPortNumberKey(candidate.ContainerPort, candidate.Protocol)] {
+		return nil
+	}
+
+	// do we have the port name there already?
+	if portNames[candidate.Name] {
+		// there's already a port with the same name! do we have a 'port-%d' already?
+		fallbackName := fmt.Sprintf("port-%d", candidate.ContainerPort)
+		if portNames[fallbackName] {
+			// that wasn't expected, better skip this port
+			logger.V(2).Info("a port name specified in the CR clashes with an inferred port name, and the fallback port name clashes with another port name! Skipping this port.",
+				"inferred-port-name", candidate.Name,
+				"fallback-port-name", fallbackName,
+			)
+			return nil
+		}
+
+		candidate.Name = fallbackName
+		return &candidate
+	}
+
+	// this port is unique, return as is
+	return &candidate
+}
+
+func toContainerPorts(ports []v1beta1.PortsSpec) []corev1.ContainerPort {
+	var containerPorts []corev1.ContainerPort
+	for _, p := range ports {
+		containerPorts = append(containerPorts, corev1.ContainerPort{
+			Name:          p.Name,
+			ContainerPort: p.Port,
+			Protocol:      p.Protocol,
+			HostPort:      p.HostPort,
+		})
+	}
+	return containerPorts
+}
+
+// getSpecPorts takes a slice of PortsSpec and returns container ports, handling duplicate port names by generating unique fallback names.
+// If a port name is duplicated and its fallback name is also taken, that port is skipped with a log message.
+func getSpecPorts(ports []v1beta1.PortsSpec, logger logr.Logger) []corev1.ContainerPort {
+	// Handle duplicate port names in otelcol.Spec.Ports
+	seenNames := make(map[string]bool)
+	var specPorts []corev1.ContainerPort
+	for _, port := range toContainerPorts(ports) {
+		if seenNames[port.Name] {
+			// If we've seen this name before, generate a unique name
+			fallbackName := fmt.Sprintf("port-%d", port.ContainerPort)
+			if seenNames[fallbackName] {
+				// If the fallback name is also taken, skip this port
+				logger.V(2).Info("skipping port due to duplicate name and fallback name conflict",
+					"port-name", port.Name,
+					"fallback-name", fallbackName,
+				)
+				continue
+			}
+			port.Name = fallbackName
+		}
+		seenNames[port.Name] = true
+		specPorts = append(specPorts, port)
+	}
+	return specPorts
+}
+
 // getContainerEnvVars returns the environment variables for the collector container.
 // It combines user-defined environment variables from the OpenTelemetryCollector spec
 // with automatically inferred environment variables, giving precedence to user-defined ones.
diff --git a/internal/manifests/collector/service.go b/internal/manifests/collector/service.go
index 25096e25..52bbab18 100644
--- a/internal/manifests/collector/service.go
+++ b/internal/manifests/collector/service.go
@@ -220,6 +220,11 @@ func newPortNumberKey(port int32, protocol corev1.Protocol) PortNumberKey {
 	return PortNumberKey{Port: port, Protocol: protocol}
 }
 
+// filterPort filters service ports to avoid conflicts with user-specified ports.
+// If the candidate port number is already in use, returns nil.
+// If the candidate port name conflicts with an existing name, attempts to use a fallback name of format "port-{number}".
+// If both the original name and fallback name are taken, returns nil with a warning log.
+// Otherwise returns the (potentially renamed) candidate port.
 func filterPort(logger logr.Logger, candidate corev1.ServicePort, portNumbers map[PortNumberKey]bool, portNames map[string]bool) *corev1.ServicePort {
 	if portNumbers[newPortNumberKey(candidate.Port, candidate.Protocol)] {
 		return nil
@@ -231,9 +236,10 @@ func filterPort(logger logr.Logger, candidate corev1.ServicePort, portNumbers ma
 		fallbackName := fmt.Sprintf("port-%d", candidate.Port)
 		if portNames[fallbackName] {
 			// that wasn't expected, better skip this port
-			logger.V(2).Info("a port name specified in the CR clashes with an inferred port name, and the fallback port name clashes with another port name! Skipping this port.",
+			logger.V(0).Info("a port name specified in the CR clashes with an inferred port name, and the fallback port name clashes with another port name! Skipping this port.",
 				"inferred-port-name", candidate.Name,
 				"fallback-port-name", fallbackName,
+				"type", "warning",
 			)
 			return nil
 		}



Test patch:
diff --git a/internal/manifests/collector/container_test.go b/internal/manifests/collector/container_test.go
index d67ee3be..8ac24952 100644
--- a/internal/manifests/collector/container_test.go
+++ b/internal/manifests/collector/container_test.go
@@ -174,10 +174,6 @@ service:
 				},
 			},
 			expectedPorts: []corev1.ContainerPort{
-				{
-					Name:          "examplereceiver",
-					ContainerPort: 12345,
-				},
 				metricContainerPort,
 				{
 					Name:          "testport1",
@@ -209,13 +205,13 @@ service:
 			},
 			expectedPorts: []corev1.ContainerPort{
 				{
-					Name:          "examplereceiver",
-					ContainerPort: 12345,
+					Name:          "port-11111",
+					ContainerPort: 11111,
 				},
 				metricContainerPort,
 				{
 					Name:          "testport1",
-					ContainerPort: 11111,
+					ContainerPort: 12345,
 				},
 			},
 		},
diff --git a/tests/e2e/smoke-ports/00-install.yaml b/tests/e2e/smoke-ports/00-install.yaml
index cf47b6a1..66cf0426 100644
--- a/tests/e2e/smoke-ports/00-install.yaml
+++ b/tests/e2e/smoke-ports/00-install.yaml
@@ -1,4 +1,4 @@
-apiVersion: opentelemetry.io/v1alpha1
+apiVersion: opentelemetry.io/v1beta1
 kind: OpenTelemetryCollector
 metadata:
   name: smoke-ports
@@ -11,7 +11,7 @@ spec:
     protocol: TCP
     targetPort: 4317
     hostPort: 4317
-  config: |
+  config:
     receivers:
       jaeger:
         protocols:
diff --git a/tests/e2e/smoke-ports/01-assert.yaml b/tests/e2e/smoke-ports/01-assert.yaml
new file mode 100644
index 00000000..3ec0b0f8
--- /dev/null
+++ b/tests/e2e/smoke-ports/01-assert.yaml
@@ -0,0 +1,29 @@
+apiVersion: apps/v1
+kind: DaemonSet
+metadata:
+  name: smoke-ports-collector
+spec:
+  template:
+    spec:
+       containers:
+       - name: otc-container
+         ports:
+         - containerPort: 9090
+           hostPort: 9090
+           name: custom-port
+           protocol: TCP
+         - containerPort: 8888
+           name: metrics
+           protocol: TCP
+---
+apiVersion: v1
+kind: Service
+metadata:
+  name: smoke-ports-collector
+spec:
+  ports:
+  - appProtocol: http
+    name: custom-port
+    port: 9090
+    protocol: TCP
+    targetPort: 9090
diff --git a/tests/e2e/smoke-ports/01-install.yaml b/tests/e2e/smoke-ports/01-install.yaml
new file mode 100644
index 00000000..fc153636
--- /dev/null
+++ b/tests/e2e/smoke-ports/01-install.yaml
@@ -0,0 +1,25 @@
+apiVersion: opentelemetry.io/v1beta1
+kind: OpenTelemetryCollector
+metadata:
+  name: smoke-ports
+spec:
+  mode: daemonset
+  ports:
+  - appProtocol: http
+    name: custom-port
+    port: 9090
+    protocol: TCP
+    targetPort: 9090
+    hostPort: 9090
+  config:
+    receivers:
+      nop: {}
+    processors:
+    exporters:
+      prometheus:
+        endpoint: "0.0.0.0:9090"
+    service:
+      pipelines:
+        traces:
+          receivers: [nop]
+          exporters: [prometheus]
diff --git a/tests/e2e/smoke-ports/chainsaw-test.yaml b/tests/e2e/smoke-ports/chainsaw-test.yaml
index 3d9816dd..ffd5f5e0 100755
--- a/tests/e2e/smoke-ports/chainsaw-test.yaml
+++ b/tests/e2e/smoke-ports/chainsaw-test.yaml
@@ -12,3 +12,9 @@ spec:
         file: 00-install.yaml
     - assert:
         file: 00-assert.yaml
+  - name: step-01
+    try:
+    - apply:
+        file: 01-install.yaml
+    - assert:
+        file: 01-assert.yaml