fixes bug where we tried to default config for the wrong components

Author: jaronoff97

apis/v1beta1/collector_webhook.go

@@ -43,13 +43,14 @@ var (
 // +kubebuilder:object:generate=false
 
 type CollectorWebhook struct {
-	logger   logr.Logger
-	cfg      config.Config
-	scheme   *runtime.Scheme
-	reviewer *rbac.Reviewer
-	metrics  *Metrics
-	bv       BuildValidator
-	fips     fips.FIPSCheck
+	logger            logr.Logger
+	cfg               config.Config
+	scheme            *runtime.Scheme
+	reviewer          *rbac.Reviewer
+	metrics           *Metrics
+	bv                BuildValidator
+	fips              fips.FIPSCheck
+	disableDefaulting bool
 }
 
 func (c CollectorWebhook) Default(_ context.Context, obj runtime.Object) error {
@@ -100,6 +101,9 @@ func (c CollectorWebhook) Default(_ context.Context, obj runtime.Object) error {
 	if len(otelcol.Spec.ManagementState) == 0 {
 		otelcol.Spec.ManagementState = ManagementStateManaged
 	}
+	if c.disableDefaulting {
+		return nil
+	}
 	return otelcol.Spec.Config.ApplyDefaults(c.logger)
 }
 
@@ -431,20 +435,30 @@ func NewCollectorWebhook(
 	metrics *Metrics,
 	bv BuildValidator,
 	fips fips.FIPSCheck,
-) *CollectorWebhook {
+	disableDefaulting bool) *CollectorWebhook {
 	return &CollectorWebhook{
-		logger:   logger,
-		scheme:   scheme,
-		cfg:      cfg,
-		reviewer: reviewer,
-		metrics:  metrics,
-		bv:       bv,
-		fips:     fips,
+		logger:            logger,
+		scheme:            scheme,
+		cfg:               cfg,
+		reviewer:          reviewer,
+		metrics:           metrics,
+		bv:                bv,
+		fips:              fips,
+		disableDefaulting: disableDefaulting,
 	}
 }
 
-func SetupCollectorWebhook(mgr ctrl.Manager, cfg config.Config, reviewer *rbac.Reviewer, metrics *Metrics, bv BuildValidator, fipsCheck fips.FIPSCheck) error {
-	cvw := NewCollectorWebhook(mgr.GetLogger().WithValues("handler", "CollectorWebhook", "version", "v1beta1"), mgr.GetScheme(), cfg, reviewer, metrics, bv, fipsCheck)
+func SetupCollectorWebhook(mgr ctrl.Manager, cfg config.Config, reviewer *rbac.Reviewer, metrics *Metrics, bv BuildValidator, fipsCheck fips.FIPSCheck, disableDefaulting bool) error {
+	cvw := NewCollectorWebhook(
+		mgr.GetLogger().WithValues("handler", "CollectorWebhook", "version", "v1beta1"),
+		mgr.GetScheme(),
+		cfg,
+		reviewer,
+		metrics,
+		bv,
+		fipsCheck,
+		disableDefaulting,
+	)
 	return ctrl.NewWebhookManagedBy(mgr).
 		For(&OpenTelemetryCollector{}).
 		WithValidator(cvw).

apis/v1beta1/collector_webhook_test.go

@@ -103,18 +103,10 @@ func TestValidate(t *testing.T) {
 
 	for _, tt := range tests {
 		test := tt
-		webhook := v1beta1.NewCollectorWebhook(
-			logr.Discard(),
-			testScheme,
-			config.New(
-				config.WithCollectorImage("collector:v0.0.0"),
-				config.WithTargetAllocatorImage("ta:v0.0.0"),
-			),
-			getReviewer(test.shouldFailSar),
-			nil,
-			bv,
-			nil,
-		)
+		webhook := v1beta1.NewCollectorWebhook(logr.Discard(), testScheme, config.New(
+			config.WithCollectorImage("collector:v0.0.0"),
+			config.WithTargetAllocatorImage("ta:v0.0.0"),
+		), getReviewer(test.shouldFailSar), nil, bv, nil, false)
 		t.Run(tt.name, func(t *testing.T) {
 			tt := tt
 			warnings, err := webhook.Validate(context.Background(), &tt.collector)
@@ -539,18 +531,10 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
 	for _, test := range tests {
 		test := test
 		t.Run(test.name, func(t *testing.T) {
-			cvw := v1beta1.NewCollectorWebhook(
-				logr.Discard(),
-				testScheme,
-				config.New(
-					config.WithCollectorImage("collector:v0.0.0"),
-					config.WithTargetAllocatorImage("ta:v0.0.0"),
-				),
-				getReviewer(test.shouldFailSar),
-				nil,
-				bv,
-				nil,
-			)
+			cvw := v1beta1.NewCollectorWebhook(logr.Discard(), testScheme, config.New(
+				config.WithCollectorImage("collector:v0.0.0"),
+				config.WithTargetAllocatorImage("ta:v0.0.0"),
+			), getReviewer(test.shouldFailSar), nil, bv, nil, false)
 			ctx := context.Background()
 			err := cvw.Default(ctx, &test.otelcol)
 			assert.NoError(t, err)
@@ -1331,18 +1315,10 @@ func TestOTELColValidatingWebhook(t *testing.T) {
 	for _, test := range tests {
 		test := test
 		t.Run(test.name, func(t *testing.T) {
-			cvw := v1beta1.NewCollectorWebhook(
-				logr.Discard(),
-				testScheme,
-				config.New(
-					config.WithCollectorImage("collector:v0.0.0"),
-					config.WithTargetAllocatorImage("ta:v0.0.0"),
-				),
-				getReviewer(test.shouldFailSar),
-				nil,
-				bv,
-				nil,
-			)
+			cvw := v1beta1.NewCollectorWebhook(logr.Discard(), testScheme, config.New(
+				config.WithCollectorImage("collector:v0.0.0"),
+				config.WithTargetAllocatorImage("ta:v0.0.0"),
+			), getReviewer(test.shouldFailSar), nil, bv, nil, false)
 			ctx := context.Background()
 			warnings, err := cvw.ValidateCreate(ctx, &test.otelcol)
 			if test.expectedErr == "" {
@@ -1399,18 +1375,10 @@ func TestOTELColValidateUpdateWebhook(t *testing.T) {
 	for _, test := range tests {
 		test := test
 		t.Run(test.name, func(t *testing.T) {
-			cvw := v1beta1.NewCollectorWebhook(
-				logr.Discard(),
-				testScheme,
-				config.New(
-					config.WithCollectorImage("collector:v0.0.0"),
-					config.WithTargetAllocatorImage("ta:v0.0.0"),
-				),
-				getReviewer(test.shouldFailSar),
-				nil,
-				bv,
-				nil,
-			)
+			cvw := v1beta1.NewCollectorWebhook(logr.Discard(), testScheme, config.New(
+				config.WithCollectorImage("collector:v0.0.0"),
+				config.WithTargetAllocatorImage("ta:v0.0.0"),
+			), getReviewer(test.shouldFailSar), nil, bv, nil, false)
 			ctx := context.Background()
 			warnings, err := cvw.ValidateUpdate(ctx, &test.otelcolOld, &test.otelcolNew)
 			if test.expectedErr == "" {

config/manager/kustomization.yaml

@@ -1,2 +1,8 @@
 resources:
 - manager.yaml
+apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+images:
+- name: controller
+  newName: jaronoff/opentelemetry-operator
+  newTag: op-v177

controllers/suite_test.go

@@ -182,7 +182,7 @@ func TestMain(m *testing.M) {
 	}
 	reviewer := rbac.NewReviewer(clientset)
 
-	if err = v1beta1.SetupCollectorWebhook(mgr, config.New(), reviewer, nil, nil, nil); err != nil {
+	if err = v1beta1.SetupCollectorWebhook(mgr, config.New(), reviewer, nil, nil, nil, false); err != nil {
 		fmt.Printf("failed to SetupWebhookWithManager: %v", err)
 		os.Exit(1)
 	}

internal/components/generic_parser.go

@@ -44,15 +44,15 @@ func (g *GenericParser[T]) GetDefaultConfig(logger logr.Logger, config interface
 		return config, nil
 	}
 
-	if g.settings.defaultRecAddr == "" {
+	if g.settings.defaultRecAddr == "" || g.settings.port == 0 {
 		return config, nil
 	}
 
 	var parsed T
 	if err := mapstructure.Decode(config, &parsed); err != nil {
 		return nil, err
 	}
-	return g.defaultsApplier(logger, g.settings.defaultRecAddr, g.settings.GetServicePort().Port, parsed)
+	return g.defaultsApplier(logger, g.settings.defaultRecAddr, g.settings.port, parsed)
 }
 
 func (g *GenericParser[T]) GetLivenessProbe(logger logr.Logger, config interface{}) (*corev1.Probe, error) {

internal/components/receivers/single_endpoint_receiver_test.go

@@ -15,6 +15,7 @@
 package receivers_test
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -82,6 +83,7 @@ func TestDownstreamParsers(t *testing.T) {
 		{"awsxray", "awsxray", "__awsxray", 2000, false},
 		{"tcplog", "tcplog", "__tcplog", 0, true},
 		{"udplog", "udplog", "__udplog", 0, true},
+		{"k8s_cluster", "k8s_cluster", "__k8s_cluster", 0, false},
 	} {
 		t.Run(tt.receiverName, func(t *testing.T) {
 			t.Run("builds successfully", func(t *testing.T) {
@@ -143,6 +145,28 @@ func TestDownstreamParsers(t *testing.T) {
 				assert.EqualValues(t, 65535, ports[0].Port)
 				assert.Equal(t, naming.PortName(tt.receiverName, int32(tt.defaultPort)), ports[0].Name)
 			})
+
+			t.Run("returns a default config", func(t *testing.T) {
+				// prepare
+				parser := receivers.ReceiverFor(tt.receiverName)
+
+				// test
+				config, err := parser.GetDefaultConfig(logger, map[string]interface{}{})
+
+				// verify
+				assert.NoError(t, err)
+				configMap, ok := config.(map[string]interface{})
+				assert.True(t, ok)
+				if tt.defaultPort == 0 {
+					assert.Empty(t, configMap, 0)
+					return
+				}
+				if tt.listenAddrParser {
+					assert.Equal(t, configMap["listen_address"], fmt.Sprintf("0.0.0.0:%d", tt.defaultPort))
+				} else {
+					assert.Equal(t, configMap["endpoint"], fmt.Sprintf("0.0.0.0:%d", tt.defaultPort))
+				}
+			})
 		})
 	}
 }

internal/webhook/podmutation/webhookhandler_suite_test.go

@@ -105,7 +105,7 @@ func TestMain(m *testing.M) {
 	}
 	reviewer := rbac.NewReviewer(clientset)
 
-	if err = v1beta1.SetupCollectorWebhook(mgr, config.New(), reviewer, nil, nil, nil); err != nil {
+	if err = v1beta1.SetupCollectorWebhook(mgr, config.New(), reviewer, nil, nil, nil, false); err != nil {
 		fmt.Printf("failed to SetupWebhookWithManager: %v", err)
 		os.Exit(1)
 	}

main.go

@@ -123,6 +123,7 @@ func main() {
 		enableNodeJSInstrumentation      bool
 		enableJavaInstrumentation        bool
 		enableCRMetrics                  bool
+		disableEndpointDefaulting        bool
 		collectorImage                   string
 		targetAllocatorImage             string
 		operatorOpAMPBridgeImage         string
@@ -162,6 +163,7 @@ func main() {
 	pflag.BoolVar(&enableNodeJSInstrumentation, constants.FlagNodeJS, true, "Controls whether the operator supports nodejs auto-instrumentation")
 	pflag.BoolVar(&enableJavaInstrumentation, constants.FlagJava, true, "Controls whether the operator supports java auto-instrumentation")
 	pflag.BoolVar(&enableCRMetrics, constants.FlagCRMetrics, false, "Controls whether exposing the CR metrics is enabled")
+	pflag.BoolVar(&disableEndpointDefaulting, "disable-event-defaulting", false, "Controls whether the operator should attempt to default the configuration for components")
 
 	stringFlagOrEnv(&collectorImage, "collector-image", "RELATED_IMAGE_COLLECTOR", fmt.Sprintf("ghcr.io/open-telemetry/opentelemetry-collector-releases/opentelemetry-collector:%s", v.OpenTelemetryCollector), "The default OpenTelemetry collector image. This image is used when no image is specified in the CustomResource.")
 	stringFlagOrEnv(&targetAllocatorImage, "target-allocator-image", "RELATED_IMAGE_TARGET_ALLOCATOR", fmt.Sprintf("ghcr.io/open-telemetry/opentelemetry-operator/target-allocator:%s", v.TargetAllocator), "The default OpenTelemetry target allocator image. This image is used when no image is specified in the CustomResource.")
@@ -216,6 +218,7 @@ func main() {
 		"go-version", v.Go,
 		"go-arch", runtime.GOARCH,
 		"go-os", runtime.GOOS,
+		"disable-event-defaulting", disableEndpointDefaulting,
 		"labels-filter", labelsFilter,
 		"annotations-filter", annotationsFilter,
 		"enable-multi-instrumentation", enableMultiInstrumentation,
@@ -447,7 +450,7 @@ func main() {
 			logger.Info("Fips disabled components", "receivers", receivers, "exporters", exporters, "processors", processors, "extensions", extensions)
 			fipsCheck = fips.NewFipsCheck(receivers, exporters, processors, extensions)
 		}
-		if err = otelv1beta1.SetupCollectorWebhook(mgr, cfg, reviewer, crdMetrics, bv, fipsCheck); err != nil {
+		if err = otelv1beta1.SetupCollectorWebhook(mgr, cfg, reviewer, crdMetrics, bv, fipsCheck, disableEndpointDefaulting); err != nil {
 			setupLog.Error(err, "unable to create webhook", "webhook", "OpenTelemetryCollector")
 			os.Exit(1)
 		}

pkg/collector/upgrade/suite_test.go

@@ -105,7 +105,7 @@ func TestMain(m *testing.M) {
 	}
 	reviewer := rbac.NewReviewer(clientset)
 
-	if err = v1beta1.SetupCollectorWebhook(mgr, config.New(), reviewer, nil, nil, nil); err != nil {
+	if err = v1beta1.SetupCollectorWebhook(mgr, config.New(), reviewer, nil, nil, nil, false); err != nil {
 		fmt.Printf("failed to SetupWebhookWithManager: %v", err)
 		os.Exit(1)
 	}