From f64445bb3a26b3aae322cf5455472c7a6c5b45bc Mon Sep 17 00:00:00 2001 From: Yang Song Date: Fri, 15 May 2026 11:01:09 -0400 Subject: [PATCH 1/3] [GKE Autopilot] Reference v1.0.5 WorkloadAllowlist exemption when OTel collector is enabled When the OTel collector is enabled on GKE Autopilot, the otel-agent (ddot-collector image) container's hostPath mounts (e.g. /var/run/containerd) are rejected by Warden because no current partner exemption permits them. Add the v1.0.5 exemption path to the AllowlistSynchronizer that the operator provisions whenever the DatadogAgent renders an otel-agent container. The matching v1.0.5 exemption YAML is published separately in the partner allowlist repo. Mirrors the helm-charts change in OTAGENT-980 (follow-up to OTAGENT-448). Changes: - ComputeAllowlistPaths returns the set of exemption paths for the synchronizer, appending v1.0.5 when otelCollectorEnabled is true. - CreateAllowlistSynchronizer takes an otelCollectorEnabled flag. - applyExperimentalAutopilotOverrides detects the otel-agent container in the rendered pod template and passes the flag through. - New unit tests cover ComputeAllowlistPaths and hasOtelAgentContainer. Refs: OTAGENT-980 Co-Authored-By: Claude Opus 4.7 (1M context) --- .../datadogagent/experimental/autopilot.go | 14 ++++- .../experimental/autopilot_test.go | 57 +++++++++++++++++++ .../allowlistsynchronizer.go | 42 +++++++++++--- .../allowlistsynchronizer_test.go | 52 +++++++++++++++++ 4 files changed, 156 insertions(+), 9 deletions(-) create mode 100644 internal/controller/datadogagent/experimental/autopilot_test.go create mode 100644 pkg/allowlistsynchronizer/allowlistsynchronizer_test.go diff --git a/internal/controller/datadogagent/experimental/autopilot.go b/internal/controller/datadogagent/experimental/autopilot.go index 8d62dd4a40..5be2f69179 100644 --- a/internal/controller/datadogagent/experimental/autopilot.go +++ b/internal/controller/datadogagent/experimental/autopilot.go @@ -66,7 +66,7 @@ func IsAutopilotEnabled(obj metav1.Object) bool { func applyExperimentalAutopilotOverrides(dda metav1.Object, manager feature.PodTemplateManagers) { if IsAutopilotEnabled(dda) { - allowlistsynchronizer.CreateAllowlistSynchronizer() + allowlistsynchronizer.CreateAllowlistSynchronizer(hasOtelAgentContainer(manager)) if manager.PodTemplateSpec().Labels == nil { manager.PodTemplateSpec().Labels = map[string]string{} @@ -151,3 +151,15 @@ func applyExperimentalAutopilotOverrides(dda metav1.Object, manager feature.PodT } } } + +// hasOtelAgentContainer reports whether the otel-agent (ddot-collector) container is +// present in the PodTemplateSpec, which signals that the OTel collector feature is +// enabled on the DatadogAgent and the v1.0.5 WorkloadAllowlist exemption is required. +func hasOtelAgentContainer(manager feature.PodTemplateManagers) bool { + for _, c := range manager.PodTemplateSpec().Spec.Containers { + if c.Name == string(apicommon.OtelAgent) { + return true + } + } + return false +} diff --git a/internal/controller/datadogagent/experimental/autopilot_test.go b/internal/controller/datadogagent/experimental/autopilot_test.go new file mode 100644 index 0000000000..23a98bd575 --- /dev/null +++ b/internal/controller/datadogagent/experimental/autopilot_test.go @@ -0,0 +1,57 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2016-present Datadog, Inc. + +package experimental + +import ( + "testing" + + apicommon "github.com/DataDog/datadog-operator/api/datadoghq/common" + "github.com/DataDog/datadog-operator/internal/controller/datadogagent/feature/fake" + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" +) + +// TestHasOtelAgentContainer verifies that the otel-agent container detection used to +// gate the v1.0.5 WorkloadAllowlist exemption inclusion works for both the present +// and absent cases. See OTAGENT-980. +func TestHasOtelAgentContainer(t *testing.T) { + tests := []struct { + name string + containers []corev1.Container + expected bool + }{ + { + name: "no otel-agent container", + containers: []corev1.Container{ + {Name: string(apicommon.CoreAgentContainerName)}, + {Name: string(apicommon.TraceAgentContainerName)}, + }, + expected: false, + }, + { + name: "otel-agent container present", + containers: []corev1.Container{ + {Name: string(apicommon.CoreAgentContainerName)}, + {Name: string(apicommon.OtelAgent)}, + }, + expected: true, + }, + { + name: "empty containers", + containers: nil, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + manager := fake.NewPodTemplateManagers(t, corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{Containers: tt.containers}, + }) + assert.Equal(t, tt.expected, hasOtelAgentContainer(manager)) + }) + } +} diff --git a/pkg/allowlistsynchronizer/allowlistsynchronizer.go b/pkg/allowlistsynchronizer/allowlistsynchronizer.go index 564c0eb16a..9bef4c77b0 100644 --- a/pkg/allowlistsynchronizer/allowlistsynchronizer.go +++ b/pkg/allowlistsynchronizer/allowlistsynchronizer.go @@ -29,6 +29,16 @@ var ( var logger = logf.Log.WithName("AllowlistSynchronizer") +// Allowlist paths kept in sync with the Datadog partner exemption YAMLs published in +// the datadog-gke-workload-allowlist repo. +const ( + // allowlistPathV101 exempts the base agent / process-agent / trace-agent containers. + allowlistPathV101 = "Datadog/datadog/datadog-datadog-daemonset-exemption-v1.0.1.yaml" + // allowlistPathV105 exempts the otel-agent (ddot-collector) container when OTel + // feature gates are configured. See OTAGENT-980. + allowlistPathV105 = "Datadog/datadog/datadog-datadog-daemonset-exemption-v1.0.5.yaml" +) + type AllowlistSynchronizer struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata"` @@ -46,8 +56,21 @@ type AllowlistSynchronizerSpec struct { AllowlistPaths []string `json:"allowlistPaths,omitempty"` } -func createAllowlistSynchronizerResource(k8sClient client.Client) error { - obj := &AllowlistSynchronizer{ +// ComputeAllowlistPaths returns the set of Datadog WorkloadAllowlist exemption paths +// that the AllowlistSynchronizer should reference for the given DatadogAgent. +// +// v1.0.5 is included when the OTel collector is enabled so that the otel-agent +// (ddot-collector image) container is permitted by GKE Autopilot Warden. See OTAGENT-980. +func ComputeAllowlistPaths(otelCollectorEnabled bool) []string { + paths := []string{allowlistPathV101} + if otelCollectorEnabled { + paths = append(paths, allowlistPathV105) + } + return paths +} + +func newAllowlistSynchronizer(paths []string) *AllowlistSynchronizer { + return &AllowlistSynchronizer{ TypeMeta: metav1.TypeMeta{ APIVersion: "allowlistsynchronizers.auto.gke.io", Kind: "AllowlistSynchronizer", @@ -60,19 +83,22 @@ func createAllowlistSynchronizerResource(k8sClient client.Client) error { }, }, Spec: AllowlistSynchronizerSpec{ - AllowlistPaths: []string{ - "Datadog/datadog/datadog-datadog-daemonset-exemption-v1.0.1.yaml", - }, + AllowlistPaths: paths, }, } +} - return k8sClient.Create(context.TODO(), obj) +func createAllowlistSynchronizerResource(k8sClient client.Client, paths []string) error { + return k8sClient.Create(context.TODO(), newAllowlistSynchronizer(paths)) } // CreateAllowlistSynchronizer creates a GKE AllowlistSynchronizer Custom Resource (auto.gke.io/v1) for the Datadog WorkloadAllowlist if it doesn't exist. // The AllowlistSynchronizer is needed so that GKE Autopilot can sync the Datadog WorkloadAllowlist to the cluster. See the CRD reference: // https://cloud.google.com/kubernetes-engine/docs/reference/crds/allowlistsynchronizer -func CreateAllowlistSynchronizer() { +// +// otelCollectorEnabled controls whether the v1.0.5 exemption path (which permits the +// otel-agent / ddot-collector container) is included. +func CreateAllowlistSynchronizer(otelCollectorEnabled bool) { cfg, configErr := config.GetConfig() if configErr != nil { logger.Error(configErr, "failed to load kubeconfig") @@ -99,7 +125,7 @@ func CreateAllowlistSynchronizer() { return } - if err := createAllowlistSynchronizerResource(k8sClient); err != nil { + if err := createAllowlistSynchronizerResource(k8sClient, ComputeAllowlistPaths(otelCollectorEnabled)); err != nil { if apierrors.IsAlreadyExists(err) { return } diff --git a/pkg/allowlistsynchronizer/allowlistsynchronizer_test.go b/pkg/allowlistsynchronizer/allowlistsynchronizer_test.go new file mode 100644 index 0000000000..6b19cb2ed2 --- /dev/null +++ b/pkg/allowlistsynchronizer/allowlistsynchronizer_test.go @@ -0,0 +1,52 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2016-present Datadog, Inc. + +package allowlistsynchronizer + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestComputeAllowlistPaths(t *testing.T) { + tests := []struct { + name string + otelCollectorEnabled bool + expected []string + }{ + { + name: "OTel collector disabled", + otelCollectorEnabled: false, + expected: []string{ + "Datadog/datadog/datadog-datadog-daemonset-exemption-v1.0.1.yaml", + }, + }, + { + name: "OTel collector enabled adds v1.0.5", + otelCollectorEnabled: true, + expected: []string{ + "Datadog/datadog/datadog-datadog-daemonset-exemption-v1.0.1.yaml", + "Datadog/datadog/datadog-datadog-daemonset-exemption-v1.0.5.yaml", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, ComputeAllowlistPaths(tt.otelCollectorEnabled)) + }) + } +} + +func TestNewAllowlistSynchronizer(t *testing.T) { + paths := ComputeAllowlistPaths(true) + s := newAllowlistSynchronizer(paths) + + assert.Equal(t, "datadog-synchronizer", s.Name) + assert.Equal(t, "AllowlistSynchronizer", s.Kind) + assert.Equal(t, "pre-install,pre-upgrade", s.Annotations["helm.sh/hook"]) + assert.Equal(t, paths, s.Spec.AllowlistPaths) +} From 7f3b538ac170ecef8740751c0f7536a71c5a12a9 Mon Sep 17 00:00:00 2001 From: Yang Song Date: Fri, 15 May 2026 11:42:24 -0400 Subject: [PATCH 2/3] Improve patch coverage for allowlistsynchronizer and autopilot helpers Extract reconcileAllowlistSynchronizer from CreateAllowlistSynchronizer so the Get/Create reconciliation can be exercised against a fake client (the k8s plumbing wrapper that loads kubeconfig and builds the rest client remains out of scope for unit tests). In the experimental autopilot package, expose a package-level createAllowlistSynchronizer var so tests can stub the synchronizer creator and cover applyExperimentalAutopilotOverrides without hitting kubeconfig. New tests: - TestReconcileAllowlistSynchronizer: covers the create-when-absent, no-op-when-exists, Get-error, AlreadyExists, and other-Create-error branches (100% line coverage). - TestCreateAllowlistSynchronizerResource: verifies the rendered resource for both the OTel-enabled and disabled cases. - TestApplyExperimentalAutopilotOverrides_SynchronizerInvocation: asserts the autopilot override path invokes the synchronizer creator with the correct otelCollectorEnabled flag derived from the pod template. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../datadogagent/experimental/autopilot.go | 5 +- .../experimental/autopilot_test.go | 67 ++++++++++ .../allowlistsynchronizer.go | 7 ++ .../allowlistsynchronizer_test.go | 114 ++++++++++++++++++ 4 files changed, 192 insertions(+), 1 deletion(-) diff --git a/internal/controller/datadogagent/experimental/autopilot.go b/internal/controller/datadogagent/experimental/autopilot.go index 5be2f69179..e5b149e48d 100644 --- a/internal/controller/datadogagent/experimental/autopilot.go +++ b/internal/controller/datadogagent/experimental/autopilot.go @@ -64,9 +64,12 @@ func IsAutopilotEnabled(obj metav1.Object) bool { return strings.EqualFold(ann[getExperimentalAnnotationKey(ExperimentalAutopilotSubkey)], "true") } +// createAllowlistSynchronizer is overridden in tests to avoid hitting kubeconfig loading. +var createAllowlistSynchronizer = allowlistsynchronizer.CreateAllowlistSynchronizer + func applyExperimentalAutopilotOverrides(dda metav1.Object, manager feature.PodTemplateManagers) { if IsAutopilotEnabled(dda) { - allowlistsynchronizer.CreateAllowlistSynchronizer(hasOtelAgentContainer(manager)) + createAllowlistSynchronizer(hasOtelAgentContainer(manager)) if manager.PodTemplateSpec().Labels == nil { manager.PodTemplateSpec().Labels = map[string]string{} diff --git a/internal/controller/datadogagent/experimental/autopilot_test.go b/internal/controller/datadogagent/experimental/autopilot_test.go index 23a98bd575..1bbb64ea60 100644 --- a/internal/controller/datadogagent/experimental/autopilot_test.go +++ b/internal/controller/datadogagent/experimental/autopilot_test.go @@ -12,6 +12,7 @@ import ( "github.com/DataDog/datadog-operator/internal/controller/datadogagent/feature/fake" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // TestHasOtelAgentContainer verifies that the otel-agent container detection used to @@ -55,3 +56,69 @@ func TestHasOtelAgentContainer(t *testing.T) { }) } } + +// TestApplyExperimentalAutopilotOverrides_SynchronizerInvocation verifies that the +// autopilot override branch invokes the synchronizer creator with the correct flag +// derived from the rendered pod template (OTAGENT-980). +func TestApplyExperimentalAutopilotOverrides_SynchronizerInvocation(t *testing.T) { + // Stub out the synchronizer creator so tests don't hit kubeconfig loading. + origCreator := createAllowlistSynchronizer + defer func() { createAllowlistSynchronizer = origCreator }() + + autopilotAnnotation := map[string]string{ + ExperimentalAnnotationPrefix + "/" + ExperimentalAutopilotSubkey: "true", + } + + tests := []struct { + name string + annotations map[string]string + containers []corev1.Container + expectInvoked bool + expectOtelFlagPassed bool + }{ + { + name: "autopilot disabled does not invoke synchronizer", + annotations: nil, + containers: []corev1.Container{{Name: string(apicommon.CoreAgentContainerName)}}, + expectInvoked: false, + expectOtelFlagPassed: false, + }, + { + name: "autopilot enabled without otel-agent invokes synchronizer with false", + annotations: autopilotAnnotation, + containers: []corev1.Container{{Name: string(apicommon.CoreAgentContainerName)}}, + expectInvoked: true, + expectOtelFlagPassed: false, + }, + { + name: "autopilot enabled with otel-agent invokes synchronizer with true", + annotations: autopilotAnnotation, + containers: []corev1.Container{{Name: string(apicommon.CoreAgentContainerName)}, {Name: string(apicommon.OtelAgent)}}, + expectInvoked: true, + expectOtelFlagPassed: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var invoked bool + var passedFlag bool + createAllowlistSynchronizer = func(otelCollectorEnabled bool) { + invoked = true + passedFlag = otelCollectorEnabled + } + + dda := &metav1.ObjectMeta{Annotations: tt.annotations} + manager := fake.NewPodTemplateManagers(t, corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{Containers: tt.containers}, + }) + + applyExperimentalAutopilotOverrides(dda, manager) + + assert.Equal(t, tt.expectInvoked, invoked, "synchronizer creator invocation") + if tt.expectInvoked { + assert.Equal(t, tt.expectOtelFlagPassed, passedFlag, "otelCollectorEnabled flag") + } + }) + } +} diff --git a/pkg/allowlistsynchronizer/allowlistsynchronizer.go b/pkg/allowlistsynchronizer/allowlistsynchronizer.go index 9bef4c77b0..4526b30319 100644 --- a/pkg/allowlistsynchronizer/allowlistsynchronizer.go +++ b/pkg/allowlistsynchronizer/allowlistsynchronizer.go @@ -117,6 +117,13 @@ func CreateAllowlistSynchronizer(otelCollectorEnabled bool) { return } + reconcileAllowlistSynchronizer(k8sClient, otelCollectorEnabled) +} + +// reconcileAllowlistSynchronizer creates the AllowlistSynchronizer resource if it does +// not already exist, logging any failure. Extracted from CreateAllowlistSynchronizer so +// it can be exercised with a fake client. +func reconcileAllowlistSynchronizer(k8sClient client.Client, otelCollectorEnabled bool) { existing := &AllowlistSynchronizer{} if existingErr := k8sClient.Get(context.TODO(), client.ObjectKey{Name: "datadog-synchronizer"}, existing); existingErr == nil { return diff --git a/pkg/allowlistsynchronizer/allowlistsynchronizer_test.go b/pkg/allowlistsynchronizer/allowlistsynchronizer_test.go index 6b19cb2ed2..54add265ce 100644 --- a/pkg/allowlistsynchronizer/allowlistsynchronizer_test.go +++ b/pkg/allowlistsynchronizer/allowlistsynchronizer_test.go @@ -6,9 +6,18 @@ package allowlistsynchronizer import ( + "context" + "errors" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" ) func TestComputeAllowlistPaths(t *testing.T) { @@ -49,4 +58,109 @@ func TestNewAllowlistSynchronizer(t *testing.T) { assert.Equal(t, "AllowlistSynchronizer", s.Kind) assert.Equal(t, "pre-install,pre-upgrade", s.Annotations["helm.sh/hook"]) assert.Equal(t, paths, s.Spec.AllowlistPaths) + + // DeepCopyObject returns a distinct value with the same spec, satisfying runtime.Object. + cp, ok := s.DeepCopyObject().(*AllowlistSynchronizer) + require.True(t, ok) + assert.Equal(t, s.Spec.AllowlistPaths, cp.Spec.AllowlistPaths) +} + +func TestReconcileAllowlistSynchronizer(t *testing.T) { + scheme := runtime.NewScheme() + require.NoError(t, SchemeBuilder.AddToScheme(scheme)) + + t.Run("creates resource when absent", func(t *testing.T) { + k8sClient := fake.NewClientBuilder().WithScheme(scheme).Build() + reconcileAllowlistSynchronizer(k8sClient, true) + + got := &AllowlistSynchronizer{} + require.NoError(t, k8sClient.Get(context.TODO(), client.ObjectKey{Name: "datadog-synchronizer"}, got)) + assert.Equal(t, []string{allowlistPathV101, allowlistPathV105}, got.Spec.AllowlistPaths) + }) + + t.Run("is a no-op when synchronizer already exists", func(t *testing.T) { + existing := newAllowlistSynchronizer([]string{allowlistPathV101}) + k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(existing).Build() + + // Call with otelCollectorEnabled=true; existing resource should NOT be overwritten. + reconcileAllowlistSynchronizer(k8sClient, true) + + got := &AllowlistSynchronizer{} + require.NoError(t, k8sClient.Get(context.TODO(), client.ObjectKey{Name: "datadog-synchronizer"}, got)) + assert.Equal(t, []string{allowlistPathV101}, got.Spec.AllowlistPaths, "existing synchronizer should be preserved") + }) + + t.Run("returns silently on Get error other than NotFound", func(t *testing.T) { + k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithInterceptorFuncs(interceptor.Funcs{ + Get: func(ctx context.Context, c client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + return errors.New("transient API error") + }, + }).Build() + + // Should not panic; should not have created anything. + reconcileAllowlistSynchronizer(k8sClient, true) + + got := &AllowlistSynchronizer{} + err := k8sClient.Get(context.TODO(), client.ObjectKey{Name: "datadog-synchronizer"}, got) + // The interceptor is still installed, so Get returns our injected error. + assert.Error(t, err) + }) + + t.Run("returns silently when Create reports AlreadyExists", func(t *testing.T) { + k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithInterceptorFuncs(interceptor.Funcs{ + Create: func(ctx context.Context, c client.WithWatch, obj client.Object, opts ...client.CreateOption) error { + return apierrors.NewAlreadyExists(schema.GroupResource{Group: "auto.gke.io", Resource: "allowlistsynchronizers"}, "datadog-synchronizer") + }, + }).Build() + + // Should not panic; the AlreadyExists branch is the early return inside the create error handler. + reconcileAllowlistSynchronizer(k8sClient, true) + }) + + t.Run("returns silently on other Create errors", func(t *testing.T) { + k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithInterceptorFuncs(interceptor.Funcs{ + Create: func(ctx context.Context, c client.WithWatch, obj client.Object, opts ...client.CreateOption) error { + return errors.New("unexpected create error") + }, + }).Build() + + reconcileAllowlistSynchronizer(k8sClient, false) + }) +} + +func TestCreateAllowlistSynchronizerResource(t *testing.T) { + scheme := runtime.NewScheme() + require.NoError(t, SchemeBuilder.AddToScheme(scheme)) + + tests := []struct { + name string + otelCollectorEnabled bool + expectedPaths []string + }{ + { + name: "OTel disabled creates synchronizer with v1.0.1 only", + otelCollectorEnabled: false, + expectedPaths: []string{allowlistPathV101}, + }, + { + name: "OTel enabled creates synchronizer with v1.0.1 and v1.0.5", + otelCollectorEnabled: true, + expectedPaths: []string{allowlistPathV101, allowlistPathV105}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + k8sClient := fake.NewClientBuilder().WithScheme(scheme).Build() + + err := createAllowlistSynchronizerResource(k8sClient, ComputeAllowlistPaths(tt.otelCollectorEnabled)) + require.NoError(t, err) + + got := &AllowlistSynchronizer{} + require.NoError(t, k8sClient.Get(context.TODO(), client.ObjectKey{Name: "datadog-synchronizer"}, got)) + assert.Equal(t, tt.expectedPaths, got.Spec.AllowlistPaths) + assert.Equal(t, "pre-install,pre-upgrade", got.Annotations["helm.sh/hook"]) + assert.Equal(t, "-1", got.Annotations["helm.sh/hook-weight"]) + }) + } } From 9437b3cd512e4ef35a6ce65e8f817eafa2ea2eda Mon Sep 17 00:00:00 2001 From: Yang Song Date: Fri, 15 May 2026 14:26:21 -0400 Subject: [PATCH 3/3] Update existing AllowlistSynchronizer when allowlistPaths drift Address PR review (#3022): the previous early-return on Get success meant that an AllowlistSynchronizer left over from an older operator version (or from a prior install with OTel disabled) would never get the v1.0.5 path added when OTel is enabled later. Warden would then keep rejecting the otel-agent pod even though the operator had reconciled. reconcileAllowlistSynchronizer now compares the existing spec.allowlistPaths against ComputeAllowlistPaths(otelCollectorEnabled) and issues an Update when they differ. Same direction in reverse: if OTel is disabled later, v1.0.5 is dropped from the spec. New test cases: - no-op when existing paths match desired - update when stale paths are missing v1.0.5 - update when stale paths still have v1.0.5 after OTel is disabled - silent return when Update fails Refs: OTAGENT-980 Co-Authored-By: Claude Opus 4.7 (1M context) --- .../allowlistsynchronizer.go | 28 ++++++++--- .../allowlistsynchronizer_test.go | 46 +++++++++++++++++-- 2 files changed, 65 insertions(+), 9 deletions(-) diff --git a/pkg/allowlistsynchronizer/allowlistsynchronizer.go b/pkg/allowlistsynchronizer/allowlistsynchronizer.go index 4526b30319..35d92e2951 100644 --- a/pkg/allowlistsynchronizer/allowlistsynchronizer.go +++ b/pkg/allowlistsynchronizer/allowlistsynchronizer.go @@ -4,6 +4,7 @@ package allowlistsynchronizer import ( "context" + "slices" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -121,18 +122,33 @@ func CreateAllowlistSynchronizer(otelCollectorEnabled bool) { } // reconcileAllowlistSynchronizer creates the AllowlistSynchronizer resource if it does -// not already exist, logging any failure. Extracted from CreateAllowlistSynchronizer so -// it can be exercised with a fake client. +// not already exist; if it exists with stale allowlistPaths (e.g. installed by an older +// operator version that didn't reference v1.0.5), the spec is updated in place so OTel +// collector workloads are admitted by Warden after enabling the feature. Extracted from +// CreateAllowlistSynchronizer so it can be exercised with a fake client. func reconcileAllowlistSynchronizer(k8sClient client.Client, otelCollectorEnabled bool) { + desired := ComputeAllowlistPaths(otelCollectorEnabled) + existing := &AllowlistSynchronizer{} - if existingErr := k8sClient.Get(context.TODO(), client.ObjectKey{Name: "datadog-synchronizer"}, existing); existingErr == nil { + getErr := k8sClient.Get(context.TODO(), client.ObjectKey{Name: "datadog-synchronizer"}, existing) + switch { + case getErr == nil: + if slices.Equal(existing.Spec.AllowlistPaths, desired) { + return + } + existing.Spec.AllowlistPaths = desired + if updateErr := k8sClient.Update(context.TODO(), existing); updateErr != nil { + logger.Error(updateErr, "failed to update AllowlistSynchronizer resource") + return + } + logger.Info("Successfully updated AllowlistSynchronizer allowlistPaths") return - } else if !apierrors.IsNotFound(existingErr) { - logger.Error(existingErr, "failed to check existing AllowlistSynchronizer resource") + case !apierrors.IsNotFound(getErr): + logger.Error(getErr, "failed to check existing AllowlistSynchronizer resource") return } - if err := createAllowlistSynchronizerResource(k8sClient, ComputeAllowlistPaths(otelCollectorEnabled)); err != nil { + if err := createAllowlistSynchronizerResource(k8sClient, desired); err != nil { if apierrors.IsAlreadyExists(err) { return } diff --git a/pkg/allowlistsynchronizer/allowlistsynchronizer_test.go b/pkg/allowlistsynchronizer/allowlistsynchronizer_test.go index 54add265ce..d3eaf8937f 100644 --- a/pkg/allowlistsynchronizer/allowlistsynchronizer_test.go +++ b/pkg/allowlistsynchronizer/allowlistsynchronizer_test.go @@ -78,16 +78,56 @@ func TestReconcileAllowlistSynchronizer(t *testing.T) { assert.Equal(t, []string{allowlistPathV101, allowlistPathV105}, got.Spec.AllowlistPaths) }) - t.Run("is a no-op when synchronizer already exists", func(t *testing.T) { + t.Run("is a no-op when existing synchronizer already matches desired paths", func(t *testing.T) { + existing := newAllowlistSynchronizer([]string{allowlistPathV101, allowlistPathV105}) + k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(existing).Build() + originalRV := existing.ResourceVersion + + reconcileAllowlistSynchronizer(k8sClient, true) + + got := &AllowlistSynchronizer{} + require.NoError(t, k8sClient.Get(context.TODO(), client.ObjectKey{Name: "datadog-synchronizer"}, got)) + assert.Equal(t, []string{allowlistPathV101, allowlistPathV105}, got.Spec.AllowlistPaths) + assert.Equal(t, originalRV, got.ResourceVersion, "no update should be issued when paths match") + }) + + t.Run("updates existing synchronizer when paths are stale", func(t *testing.T) { + // Simulates a synchronizer installed by an older operator version that only + // referenced v1.0.1; OTel is now enabled, so v1.0.5 must be added. existing := newAllowlistSynchronizer([]string{allowlistPathV101}) k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(existing).Build() + originalRV := existing.ResourceVersion - // Call with otelCollectorEnabled=true; existing resource should NOT be overwritten. reconcileAllowlistSynchronizer(k8sClient, true) got := &AllowlistSynchronizer{} require.NoError(t, k8sClient.Get(context.TODO(), client.ObjectKey{Name: "datadog-synchronizer"}, got)) - assert.Equal(t, []string{allowlistPathV101}, got.Spec.AllowlistPaths, "existing synchronizer should be preserved") + assert.Equal(t, []string{allowlistPathV101, allowlistPathV105}, got.Spec.AllowlistPaths) + assert.NotEqual(t, originalRV, got.ResourceVersion, "update should have bumped the resourceVersion") + }) + + t.Run("removes v1.0.5 from existing synchronizer when OTel is disabled", func(t *testing.T) { + // Covers the reverse direction: an existing resource has v1.0.5 but OTel is + // no longer enabled, so the path should be dropped. + existing := newAllowlistSynchronizer([]string{allowlistPathV101, allowlistPathV105}) + k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(existing).Build() + + reconcileAllowlistSynchronizer(k8sClient, false) + + got := &AllowlistSynchronizer{} + require.NoError(t, k8sClient.Get(context.TODO(), client.ObjectKey{Name: "datadog-synchronizer"}, got)) + assert.Equal(t, []string{allowlistPathV101}, got.Spec.AllowlistPaths) + }) + + t.Run("returns silently when Update fails", func(t *testing.T) { + existing := newAllowlistSynchronizer([]string{allowlistPathV101}) + k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(existing).WithInterceptorFuncs(interceptor.Funcs{ + Update: func(ctx context.Context, c client.WithWatch, obj client.Object, opts ...client.UpdateOption) error { + return errors.New("update failed") + }, + }).Build() + + reconcileAllowlistSynchronizer(k8sClient, true) }) t.Run("returns silently on Get error other than NotFound", func(t *testing.T) {