disable component.UseLocalHostAsDefaultHost by default

Signed-off-by: Benedikt Bongartz <bongartz@klimlive.de>
open-telemetry · Jul 17, 2024 · 7adcff0 · 7adcff0
1 parent 1daff44
commit 7adcff0
Show file tree

Hide file tree

Showing 16 changed files with 59 additions and 205 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,7 +6,6 @@
 
 ### 🛑 Breaking changes 🛑
 
-- `collector`: The new collector release enables `component.UseLocalHostAsDefaultHost` by default. This mainly affects the `otlpreceiver`. But may has side effects on components like the `healthcheck extension`. The `operator` will change IPs not explicitly configured in the `otlpreceiver` configuration such as `:4317` or empty fields to `0.0.0.0` during the upgrade routine and when creating a CR. More details in the [OpenTelemetry Collector Changelog](https://github.com/open-telemetry/opentelemetry-collector/blob/main/CHANGELOG.md#v1110v01040).
 - `opamp`: Adds support for v1beta1 OpenTelemetry Collector API in the OpAMP Bridge (#2985)
   This change adds support for the OpAMP Bridge to manage and apply OpenTelemetry Collectors using the v1beta1 API in 
   the OpAMP Bridge. This change removes support for applying OpenTelemetry Collectors using the v1alpha1 API version.
@@ -15,7 +14,7 @@
 
 ### 💡 Enhancements 💡
 
-- `collector`: set correct target port in services created based on the given otel config. (#3139)
+- `operator`: Disables `component.UseLocalHostAsDefaultHost` by default (#3139)
 - `collector`: Changes the default parser to silently fail. (#3133)
 - `collector, target allocator`: If the target allocator is enabled, the collector featuregate `confmap.unifyEnvVarExpansion' is disabled. (#3119) 
 - `operator`: Release leader election lease on exit (#3058)

diff --git a/apis/v1beta1/collector_webhook.go b/apis/v1beta1/collector_webhook.go
@@ -79,13 +79,8 @@ func (c CollectorWebhook) Default(_ context.Context, obj runtime.Object) error {
 		otelcol.Spec.TargetAllocator.Replicas = &one
 	}
 
-	if otelcol.Spec.TargetAllocator.Enabled {
-		TAUnifyEnvVarExpansion(otelcol)
-	}
-
-	if err := DefaultOTLPAddress(otelcol); err != nil {
-		return err
-	}
+	TAUnifyEnvVarExpansion(otelcol)
+	ComponentUseLocalHostAsDefaultHost(otelcol)
 
 	if otelcol.Spec.Autoscaler != nil && otelcol.Spec.Autoscaler.MaxReplicas != nil {
 		if otelcol.Spec.Autoscaler.MinReplicas == nil {
@@ -476,64 +471,35 @@ func TAUnifyEnvVarExpansion(otelcol *OpenTelemetryCollector) {
 
 	const (
 		baseFlag = "feature-gates"
-		fgFlag   = "-confmap.unifyEnvVarExpansion"
+		fgFlag   = "confmap.unifyEnvVarExpansion"
 	)
 	if otelcol.Spec.Args == nil {
 		otelcol.Spec.Args = make(map[string]string)
 	}
 	args, ok := otelcol.Spec.Args[baseFlag]
 	if !ok || len(args) == 0 {
-		otelcol.Spec.Args[baseFlag] = fgFlag
+		otelcol.Spec.Args[baseFlag] = "-" + fgFlag
 	} else if !strings.Contains(otelcol.Spec.Args[baseFlag], fgFlag) {
-		otelcol.Spec.Args[baseFlag] += "," + fgFlag
+		otelcol.Spec.Args[baseFlag] += ",-" + fgFlag
 	}
-
 }
 
-// DefaultOTLPAddress binds configured otlp receivers to 0.0.0.0 if nothing else
-// is explicitly defined.
-func DefaultOTLPAddress(otelcol *OpenTelemetryCollector) error {
-	for key, rc := range otelcol.Spec.Config.Receivers.Object {
-		// check if otel is configured
-		if !strings.HasPrefix(key, "otlp") {
-			continue
-		}
-
-		cfg, ok := rc.(map[string]interface{})
-		if !ok {
-			continue
-		}
-
-		protocols, ok := cfg["protocols"].(map[string]interface{})
-		if !ok {
-			continue
-		}
-
-		if _, exists := protocols["grpc"]; exists {
-			grpcConfig, ok := protocols["grpc"].(map[string]interface{})
-			if !ok {
-				grpcConfig = make(map[string]interface{})
-				protocols["grpc"] = grpcConfig
-			}
-
-			gEndpoint, ok := grpcConfig["endpoint"].(string)
-			if !ok || !strings.Contains(gEndpoint, ":") {
-				grpcConfig["endpoint"] = "0.0.0.0:4317"
-			}
-		}
-
-		if _, exists := protocols["http"]; exists {
-			httpConfig, ok := protocols["http"].(map[string]interface{})
-			if !ok {
-				httpConfig = make(map[string]interface{})
-				protocols["http"] = httpConfig
-			}
-
-			hEndpoint, ok := httpConfig["endpoint"].(string)
-			if !ok || !strings.Contains(hEndpoint, ":") {
-				httpConfig["endpoint"] = "0.0.0.0:4318"
-			}
-		}
+// ComponentUseLocalHostAsDefaultHost enables component.UseLocalHostAsDefaultHost
+// featuregate on the given collector instance.
+// NOTE: For more details, visit:
+// https://github.com/open-telemetry/opentelemetry-collector/issues/8510
+func ComponentUseLocalHostAsDefaultHost(otelcol *OpenTelemetryCollector) {
+	const (
+		baseFlag = "feature-gates"
+		fgFlag   = "component.UseLocalHostAsDefaultHost"
+	)
+	if otelcol.Spec.Args == nil {
+		otelcol.Spec.Args = make(map[string]string)
+	}
+	args, ok := otelcol.Spec.Args[baseFlag]
+	if !ok || len(args) == 0 {
+		otelcol.Spec.Args[baseFlag] = "-" + fgFlag
+	} else if !strings.Contains(otelcol.Spec.Args[baseFlag], fgFlag) {
+		otelcol.Spec.Args[baseFlag] += ",-" + fgFlag
 	}
-	return nil
 }
diff --git a/apis/v1beta1/collector_webhook_test.go b/apis/v1beta1/collector_webhook_test.go
@@ -117,6 +117,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
 				},
 				Spec: OpenTelemetryCollectorSpec{
 					OpenTelemetryCommonFields: OpenTelemetryCommonFields{
+						Args:            map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"},
 						ManagementState: ManagementStateManaged,
 						Replicas:        &one,
 						PodDisruptionBudget: &PodDisruptionBudgetSpec{
@@ -152,6 +153,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
 					Mode:            ModeSidecar,
 					UpgradeStrategy: "adhoc",
 					OpenTelemetryCommonFields: OpenTelemetryCommonFields{
+						Args:            map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"},
 						Replicas:        &five,
 						ManagementState: ManagementStateManaged,
 						PodDisruptionBudget: &PodDisruptionBudgetSpec{
@@ -186,6 +188,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
 					Mode:            ModeSidecar,
 					UpgradeStrategy: "adhoc",
 					OpenTelemetryCommonFields: OpenTelemetryCommonFields{
+						Args:            map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"},
 						Replicas:        &five,
 						ManagementState: ManagementStateUnmanaged,
 						PodDisruptionBudget: &PodDisruptionBudgetSpec{
@@ -218,6 +221,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
 					Mode:            ModeDeployment,
 					UpgradeStrategy: UpgradeStrategyAutomatic,
 					OpenTelemetryCommonFields: OpenTelemetryCommonFields{
+						Args:            map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"},
 						Replicas:        &one,
 						ManagementState: ManagementStateManaged,
 						PodDisruptionBudget: &PodDisruptionBudgetSpec{
@@ -254,6 +258,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
 				Spec: OpenTelemetryCollectorSpec{
 					Mode: ModeDeployment,
 					OpenTelemetryCommonFields: OpenTelemetryCommonFields{
+						Args:            map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"},
 						ManagementState: ManagementStateManaged,
 						Replicas:        &one,
 						PodDisruptionBudget: &PodDisruptionBudgetSpec{
@@ -297,6 +302,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
 				Spec: OpenTelemetryCollectorSpec{
 					Mode: ModeDeployment,
 					OpenTelemetryCommonFields: OpenTelemetryCommonFields{
+						Args:            map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"},
 						Replicas:        &one,
 						ManagementState: ManagementStateManaged,
 						PodDisruptionBudget: &PodDisruptionBudgetSpec{
@@ -336,6 +342,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
 				Spec: OpenTelemetryCollectorSpec{
 					Mode: ModeDeployment,
 					OpenTelemetryCommonFields: OpenTelemetryCommonFields{
+						Args:            map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"},
 						Replicas:        &one,
 						ManagementState: ManagementStateManaged,
 						PodDisruptionBudget: &PodDisruptionBudgetSpec{
@@ -386,6 +393,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
 				Spec: OpenTelemetryCollectorSpec{
 					Mode: ModeDeployment,
 					OpenTelemetryCommonFields: OpenTelemetryCommonFields{
+						Args:            map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"},
 						Replicas:        &one,
 						ManagementState: ManagementStateManaged,
 						PodDisruptionBudget: &PodDisruptionBudgetSpec{
@@ -431,6 +439,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
 				Spec: OpenTelemetryCollectorSpec{
 					Mode: ModeDeployment,
 					OpenTelemetryCommonFields: OpenTelemetryCommonFields{
+						Args:            map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"},
 						Replicas:        &one,
 						ManagementState: ManagementStateManaged,
 						PodDisruptionBudget: &PodDisruptionBudgetSpec{
@@ -475,6 +484,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
 				Spec: OpenTelemetryCollectorSpec{
 					Mode: ModeDeployment,
 					OpenTelemetryCommonFields: OpenTelemetryCommonFields{
+						Args:            map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"},
 						Replicas:        &one,
 						ManagementState: ManagementStateManaged,
 						PodDisruptionBudget: &PodDisruptionBudgetSpec{
@@ -493,58 +503,6 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
 				},
 			},
 		},
-		{
-			name: "default address otlp receiver",
-			otelcol: OpenTelemetryCollector{
-				Spec: OpenTelemetryCollectorSpec{
-					Config: Config{
-						Receivers: AnyConfig{
-							Object: map[string]interface{}{
-								"otlp/something": map[string]interface{}{
-									"protocols": map[string]interface{}{
-										"http": nil,
-									},
-								},
-							},
-						},
-					},
-				},
-			},
-			expected: OpenTelemetryCollector{
-				ObjectMeta: metav1.ObjectMeta{
-					Labels: map[string]string{
-						"app.kubernetes.io/managed-by": "opentelemetry-operator",
-					},
-				},
-				Spec: OpenTelemetryCollectorSpec{
-					OpenTelemetryCommonFields: OpenTelemetryCommonFields{
-						ManagementState: ManagementStateManaged,
-						Replicas:        &one,
-						PodDisruptionBudget: &PodDisruptionBudgetSpec{
-							MaxUnavailable: &intstr.IntOrString{
-								Type:   intstr.Int,
-								IntVal: 1,
-							},
-						},
-					},
-					Config: Config{
-						Receivers: AnyConfig{
-							Object: map[string]interface{}{
-								"otlp/something": map[string]interface{}{
-									"protocols": map[string]interface{}{
-										"http": map[string]interface{}{
-											"endpoint": "0.0.0.0:4318",
-										},
-									},
-								},
-							},
-						},
-					},
-					Mode:            ModeDeployment,
-					UpgradeStrategy: UpgradeStrategyAutomatic,
-				},
-			},
-		},
 	}
 
 	for _, test := range tests {

diff --git a/pkg/collector/upgrade/v0_104_0.go b/pkg/collector/upgrade/v0_104_0.go
@@ -26,9 +26,7 @@ func upgrade0_104_0_TA(_ VersionUpgrade, otelcol *v1beta1.OpenTelemetryCollector
 }
 
 func upgrade0_104_0(u VersionUpgrade, otelcol *v1beta1.OpenTelemetryCollector) (*v1beta1.OpenTelemetryCollector, error) {
-	if err := v1beta1.DefaultOTLPAddress(otelcol); err != nil {
-		return nil, err
-	}
+	v1beta1.ComponentUseLocalHostAsDefaultHost(otelcol)
 
 	const issueID = "https://github.com/open-telemetry/opentelemetry-collector/issues/8510"
 	warnStr := fmt.Sprintf(

diff --git a/pkg/collector/upgrade/v0_104_0_test.go b/pkg/collector/upgrade/v0_104_0_test.go
@@ -29,7 +29,6 @@ import (
 )
 
 func Test0_104_0Upgrade(t *testing.T) {
-
 	collectorInstance := v1beta1.OpenTelemetryCollector{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "OpenTelemetryCollector",
@@ -42,45 +41,7 @@ func Test0_104_0Upgrade(t *testing.T) {
 		Status: v1beta1.OpenTelemetryCollectorStatus{
 			Version: "0.103.0",
 		},
-		Spec: v1beta1.OpenTelemetryCollectorSpec{
-			Config: v1beta1.Config{
-				Receivers: v1beta1.AnyConfig{
-					Object: map[string]interface{}{
-						"otlp": map[string]interface{}{
-							"protocols": map[string]interface{}{
-								"grpc": nil,
-								"http": nil,
-							},
-						},
-						"otlp/nothing": &v1beta1.AnyConfig{
-							Object: map[string]interface{}{
-								"protocols": nil,
-							},
-						},
-						"otlp/empty": map[string]interface{}{
-							"protocols": map[string]interface{}{
-								"grpc": map[string]interface{}{
-									"endpoint": "",
-								},
-								"http": map[string]interface{}{
-									"endpoint": "",
-								},
-							},
-						},
-						"otlp/something": map[string]interface{}{
-							"protocols": map[string]interface{}{
-								"grpc": map[string]interface{}{
-									"endpoint": "123.123.123.123:8642",
-								},
-								"http": map[string]interface{}{
-									"endpoint": "123.123.123.123:8642",
-								},
-							},
-						},
-					},
-				},
-			},
-		},
+		Spec: v1beta1.OpenTelemetryCollectorSpec{},
 	}
 
 	versionUpgrade := &upgrade.VersionUpgrade{
@@ -90,48 +51,11 @@ func Test0_104_0Upgrade(t *testing.T) {
 		Recorder: record.NewFakeRecorder(upgrade.RecordBufferSize),
 	}
 
-	_, err := versionUpgrade.ManagedInstance(context.Background(), collectorInstance)
+	col, err := versionUpgrade.ManagedInstance(context.Background(), collectorInstance)
 	if err != nil {
 		t.Errorf("expect err: nil but got: %v", err)
 	}
-
-	assert.EqualValues(t, map[string]interface{}{
-		"protocols": map[string]interface{}{
-			"grpc": map[string]interface{}{
-				"endpoint": "0.0.0.0:4317",
-			},
-			"http": map[string]interface{}{
-				"endpoint": "0.0.0.0:4318",
-			},
-		},
-	}, collectorInstance.Spec.Config.Receivers.Object["otlp"], "normal entry is not up-to-date")
-
-	assert.EqualValues(t, &v1beta1.AnyConfig{
-		Object: map[string]interface{}{
-			"protocols": nil,
-		},
-	}, collectorInstance.Spec.Config.Receivers.Object["otlp/nothing"], "no updated expected")
-
-	assert.EqualValues(t, map[string]interface{}{
-		"protocols": map[string]interface{}{
-			"grpc": map[string]interface{}{
-				"endpoint": "0.0.0.0:4317",
-			},
-			"http": map[string]interface{}{
-				"endpoint": "0.0.0.0:4318",
-			},
-		},
-	}, collectorInstance.Spec.Config.Receivers.Object["otlp/empty"], "empty entry is not up-to-date")
-
-	assert.EqualValues(t, map[string]interface{}{
-		"protocols": map[string]interface{}{
-			"grpc": map[string]interface{}{
-				"endpoint": "123.123.123.123:8642",
-			},
-			"http": map[string]interface{}{
-				"endpoint": "123.123.123.123:8642",
-			},
-		},
-	}, collectorInstance.Spec.Config.Receivers.Object["otlp/something"], "endpoints exist, did not expect an update")
-
+	assert.EqualValues(t,
+		map[string]string{"feature-gates": "-component.UseLocalHostAsDefaultHost"},
+		col.Spec.Args, "missing featuregate")
 }
diff --git a/pkg/collector/upgrade/v0_38_0_test.go b/pkg/collector/upgrade/v0_38_0_test.go
@@ -82,8 +82,9 @@ service:
 
 	// verify
 	assert.Equal(t, map[string]string{
-		"--hii":  "hello",
-		"--arg1": "",
+		"--hii":         "hello",
+		"--arg1":        "",
+		"feature-gates": "-component.UseLocalHostAsDefaultHost",
 	}, res.Spec.Args)
 
 	// verify
@@ -148,7 +149,8 @@ service:
 	// verify
 	assert.YAMLEq(t, configWithLogging, res.Spec.Config)
 	assert.Equal(t, map[string]string{
-		"--hii":  "hello",
-		"--arg1": "",
+		"--hii":         "hello",
+		"--arg1":        "",
+		"feature-gates": "-component.UseLocalHostAsDefaultHost",
 	}, res.Spec.Args)
 }