From 7ee69c59d6727bddf8b1af71da544eac7df08a9b Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Tue, 8 Oct 2024 14:30:39 -0400 Subject: [PATCH] Fixes bug where we tried to default config for the wrong components (#3337) * fixes bug where we tried to default config for the wrong components * switch to featuregate * fix kusomtize --- .chloggen/add_all_receiver_defaults.yaml | 4 +++- apis/v1beta1/collector_webhook.go | 4 ++++ internal/components/generic_parser.go | 4 ++-- .../single_endpoint_receiver_test.go | 24 +++++++++++++++++++ pkg/featuregate/featuregate.go | 7 ++++++ 5 files changed, 40 insertions(+), 3 deletions(-) diff --git a/.chloggen/add_all_receiver_defaults.yaml b/.chloggen/add_all_receiver_defaults.yaml index 4784ede779..e4bb2b6c2b 100755 --- a/.chloggen/add_all_receiver_defaults.yaml +++ b/.chloggen/add_all_receiver_defaults.yaml @@ -13,4 +13,6 @@ issues: [3126] # (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: +subtext: | + This feature is enabled by default. It can be disabled by specifying + `--feature-gates=-operator.collector.default.config`. diff --git a/apis/v1beta1/collector_webhook.go b/apis/v1beta1/collector_webhook.go index e1a53a543d..e79754b4bd 100644 --- a/apis/v1beta1/collector_webhook.go +++ b/apis/v1beta1/collector_webhook.go @@ -30,6 +30,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/internal/fips" ta "github.com/open-telemetry/opentelemetry-operator/internal/manifests/targetallocator/adapters" "github.com/open-telemetry/opentelemetry-operator/internal/rbac" + "github.com/open-telemetry/opentelemetry-operator/pkg/featuregate" ) var ( @@ -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 !featuregate.EnableConfigDefaulting.IsEnabled() { + return nil + } return otelcol.Spec.Config.ApplyDefaults(c.logger) } diff --git a/internal/components/generic_parser.go b/internal/components/generic_parser.go index c27521a9ce..02887a5892 100644 --- a/internal/components/generic_parser.go +++ b/internal/components/generic_parser.go @@ -44,7 +44,7 @@ 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 } @@ -52,7 +52,7 @@ func (g *GenericParser[T]) GetDefaultConfig(logger logr.Logger, config interface 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) { diff --git a/internal/components/receivers/single_endpoint_receiver_test.go b/internal/components/receivers/single_endpoint_receiver_test.go index 899d24f25d..faaae6dbd7 100644 --- a/internal/components/receivers/single_endpoint_receiver_test.go +++ b/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)) + } + }) }) } } diff --git a/pkg/featuregate/featuregate.go b/pkg/featuregate/featuregate.go index f50095e874..bf83d666ce 100644 --- a/pkg/featuregate/featuregate.go +++ b/pkg/featuregate/featuregate.go @@ -40,6 +40,13 @@ var ( featuregate.WithRegisterDescription("enables feature to set GOMEMLIMIT and GOMAXPROCS automatically"), featuregate.WithRegisterFromVersion("v0.100.0"), ) + // EnableConfigDefaulting is the feature gate that enables the operator to default the endpoint for known components. + EnableConfigDefaulting = featuregate.GlobalRegistry().MustRegister( + "operator.collector.default.config", + featuregate.StageBeta, + featuregate.WithRegisterDescription("enables the operator to default the endpoint for known components"), + featuregate.WithRegisterFromVersion("v0.110.0"), + ) ) // Flags creates a new FlagSet that represents the available featuregate flags using the supplied featuregate registry.