diff --git a/api/datadoghq/common/types.go b/api/datadoghq/common/types.go index 378f02b77d..07ff02402f 100644 --- a/api/datadoghq/common/types.go +++ b/api/datadoghq/common/types.go @@ -44,6 +44,8 @@ const ( InitConfigContainerName AgentContainerName = "init-config" // SeccompSetupContainerName is the name of the Seccomp Setup init container SeccompSetupContainerName AgentContainerName = "seccomp-setup" + // HostProfilerSeccompSetupContainerName is the name of the host-profiler seccomp setup init container + HostProfilerSeccompSetupContainerName AgentContainerName = "host-profiler-seccomp-setup" // UnprivilegedSingleAgentContainerName is the name of a container which may run // any combination of Core, Trace and Process Agent processes in a single container. diff --git a/internal/controller/datadogagent/experimental/image.go b/internal/controller/datadogagent/experimental/image.go index 46682a533c..189a368696 100644 --- a/internal/controller/datadogagent/experimental/image.go +++ b/internal/controller/datadogagent/experimental/image.go @@ -59,6 +59,25 @@ func overrideImage(currentImg string, overrideImg imageOverride) string { return images.OverrideAgentImage(currentImg, overrideImgConfig) } +// ResolveImageOverride returns the effective image for a container after applying the experimental image override +// annotation. If no override is configured, or if the annotation is malformed, it returns currentImg. +func ResolveImageOverride(dda metav1.Object, containerName, currentImg string) (string, error) { + if dda == nil { + return currentImg, nil + } + + imageOverrides, err := getImageOverrideConfig(dda) + if err != nil { + return currentImg, err + } + + if imageOverride, ok := imageOverrides[containerName]; ok { + return overrideImage(currentImg, imageOverride), nil + } + + return currentImg, nil +} + func applyExperimentalImageOverrides(logger logr.Logger, dda metav1.Object, manager feature.PodTemplateManagers) { // We support overriding the image used for any non-init container in the Agent's pod spec. // diff --git a/internal/controller/datadogagent/feature/hostprofiler/feature.go b/internal/controller/datadogagent/feature/hostprofiler/feature.go index 92b1643b42..571526e441 100644 --- a/internal/controller/datadogagent/feature/hostprofiler/feature.go +++ b/internal/controller/datadogagent/feature/hostprofiler/feature.go @@ -1,10 +1,8 @@ package hostprofiler import ( - "encoding/json" "errors" "fmt" - "strings" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" @@ -14,6 +12,7 @@ import ( apicommon "github.com/DataDog/datadog-operator/api/datadoghq/common" "github.com/DataDog/datadog-operator/api/datadoghq/v2alpha1" "github.com/DataDog/datadog-operator/internal/controller/datadogagent/common" + "github.com/DataDog/datadog-operator/internal/controller/datadogagent/experimental" "github.com/DataDog/datadog-operator/internal/controller/datadogagent/feature" featureutils "github.com/DataDog/datadog-operator/internal/controller/datadogagent/feature/utils" ) @@ -23,7 +22,6 @@ var errHostPIDDisabledManually = errors.New("Host PID is required for host profi type hostProfilerFeature struct { owner metav1.Object hostPIDDisabledManually bool - hostProfilerImage string logger logr.Logger } @@ -49,19 +47,13 @@ func (o *hostProfilerFeature) ID() feature.IDType { return feature.HostProfilerIDType } -func (o *hostProfilerFeature) Configure(dda metav1.Object, ddaSpec *v2alpha1.DatadogAgentSpec, _ *v2alpha1.RemoteConfigConfiguration) feature.RequiredComponents { +func (o *hostProfilerFeature) Configure(dda metav1.Object, _ *v2alpha1.DatadogAgentSpec, _ *v2alpha1.RemoteConfigConfiguration) feature.RequiredComponents { o.owner = dda if !featureutils.HasFeatureEnableAnnotation(dda, featureutils.EnableHostProfilerAnnotation) { return feature.RequiredComponents{} } - // Resolve the host-profiler image now, during Configure, so ManageNodeAgent can use - // the correct image for the seccomp init container and profile name. Both the - // experimental annotation and spec.override.nodeAgent.image are applied after - // ManageNodeAgent runs, so we must read them here. - o.hostProfilerImage = resolveHostProfilerImage(dda, ddaSpec) - return feature.RequiredComponents{ Agent: feature.RequiredComponent{ IsRequired: ptr.To(true), @@ -111,13 +103,10 @@ func (o *hostProfilerFeature) ManageNodeAgent(managers feature.PodTemplateManage hostProfilerContainer.SecurityContext = &corev1.SecurityContext{} } - // Use the pre-resolved image (set in Configure) so that experimental image overrides, - // which are applied after ManageNodeAgent, are correctly reflected in the seccomp profile - // name and the init container image. - hostProfilerImage := o.hostProfilerImage - if hostProfilerImage == "" { - hostProfilerImage = hostProfilerContainer.Image - } + // Experimental image overrides are applied after ManageNodeAgent, so mirror their image + // resolution here to keep the seccomp profile and init container aligned with the final + // host-profiler container image. + hostProfilerImage := resolveHostProfilerImage(o.owner, hostProfilerContainer.Image) sc := hostProfilerContainer.SecurityContext sc.AllowPrivilegeEscalation = ptr.To(false) @@ -192,37 +181,12 @@ func (o *hostProfilerFeature) ManageOtelAgentGateway(managers feature.PodTemplat } // resolveHostProfilerImage returns the host-profiler image to use for the seccomp init container -// and profile name. Both override sources are applied after ManageNodeAgent runs, so we read them -// during Configure instead. Priority: experimental annotation > spec.override.nodeAgent.image > "". -func resolveHostProfilerImage(dda metav1.Object, ddaSpec *v2alpha1.DatadogAgentSpec) string { - // 1. Experimental per-container image override annotation. - if annotations := dda.GetAnnotations(); annotations != nil { - if raw := annotations["experimental.agent.datadoghq.com/image-override-config"]; raw != "" { - var overrides map[string]struct { - Name string `json:"name,omitempty"` - Tag string `json:"tag,omitempty"` - } - if err := json.Unmarshal([]byte(raw), &overrides); err == nil { - if o, ok := overrides[string(apicommon.HostProfiler)]; ok && o.Name != "" { - if o.Tag != "" && !strings.Contains(o.Name, ":") { - return o.Name + ":" + o.Tag - } - return o.Name - } - } - } - } - - // 2. spec.override.nodeAgent.image — applied to all agent containers including host-profiler. - if ddaSpec != nil { - if componentOverride, ok := ddaSpec.Override[v2alpha1.NodeAgentComponentName]; ok && componentOverride.Image != nil { - img := componentOverride.Image - if img.Name != "" && img.Tag != "" && !strings.Contains(img.Name, ":") { - return img.Name + ":" + img.Tag - } - return img.Name - } +// and profile name. It uses the same experimental image override semantics as the final pod +// mutation so the seccomp init image cannot drift from the host-profiler container image. +func resolveHostProfilerImage(dda metav1.Object, baseImage string) string { + hostProfilerImage, err := experimental.ResolveImageOverride(dda, string(apicommon.HostProfiler), baseImage) + if err != nil { + return baseImage } - - return "" + return hostProfilerImage } diff --git a/internal/controller/datadogagent/feature/hostprofiler/feature_test.go b/internal/controller/datadogagent/feature/hostprofiler/feature_test.go index 517dbf5933..f9e6e88c75 100644 --- a/internal/controller/datadogagent/feature/hostprofiler/feature_test.go +++ b/internal/controller/datadogagent/feature/hostprofiler/feature_test.go @@ -7,13 +7,16 @@ import ( "github.com/DataDog/datadog-operator/api/datadoghq/v2alpha1" apiutils "github.com/DataDog/datadog-operator/api/utils" "github.com/DataDog/datadog-operator/internal/controller/datadogagent/common" + "github.com/DataDog/datadog-operator/internal/controller/datadogagent/experimental" "github.com/DataDog/datadog-operator/internal/controller/datadogagent/feature" "github.com/DataDog/datadog-operator/internal/controller/datadogagent/feature/fake" "github.com/DataDog/datadog-operator/internal/controller/datadogagent/feature/test" + "github.com/DataDog/datadog-operator/internal/controller/datadogagent/override" "github.com/DataDog/datadog-operator/pkg/images" "github.com/DataDog/datadog-operator/pkg/kubernetes" "github.com/DataDog/datadog-operator/pkg/testutils" + "github.com/go-logr/logr" "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" @@ -156,20 +159,20 @@ func testExpectedAgent(agentContainerName apicommon.AgentContainerName, expected } func TestResolveHostProfilerImage(t *testing.T) { + baseImage := "docker.io/datadog/agent:7.63.0" tests := []struct { name string annotations map[string]string - ddaSpec *v2alpha1.DatadogAgentSpec want string }{ { - name: "no annotations, no spec override", - want: "", + name: "no annotations", + want: baseImage, }, { name: "annotation absent", annotations: map[string]string{"some.other/annotation": "value"}, - want: "", + want: baseImage, }, { name: "experimental annotation — full ref in name", @@ -179,59 +182,142 @@ func TestResolveHostProfilerImage(t *testing.T) { want: "gcr.io/x/host-profiler:v2", }, { - name: "experimental annotation — name and tag fields", + name: "experimental annotation — tagged name preserves registry", annotations: map[string]string{ - "experimental.agent.datadoghq.com/image-override-config": `{"host-profiler":{"name":"gcr.io/x/host-profiler","tag":"v2"}}`, + "experimental.agent.datadoghq.com/image-override-config": `{"host-profiler":{"name":"host-profiler:v2"}}`, }, - want: "gcr.io/x/host-profiler:v2", + want: "docker.io/datadog/host-profiler:v2", + }, + { + name: "experimental annotation — name and tag fields preserve registry", + annotations: map[string]string{ + "experimental.agent.datadoghq.com/image-override-config": `{"host-profiler":{"name":"host-profiler","tag":"v2"}}`, + }, + want: "docker.io/datadog/host-profiler:v2", }, { name: "experimental annotation — name with tag takes precedence over tag field", annotations: map[string]string{ - "experimental.agent.datadoghq.com/image-override-config": `{"host-profiler":{"name":"gcr.io/x/host-profiler:v1","tag":"v2"}}`, + "experimental.agent.datadoghq.com/image-override-config": `{"host-profiler":{"name":"host-profiler:v1","tag":"v2"}}`, }, - want: "gcr.io/x/host-profiler:v1", + want: "docker.io/datadog/host-profiler:v1", }, { name: "experimental annotation — different container, ignored", annotations: map[string]string{ "experimental.agent.datadoghq.com/image-override-config": `{"agent":{"name":"gcr.io/x/agent:v2"}}`, }, - want: "", + want: baseImage, }, { name: "experimental annotation — malformed json", annotations: map[string]string{ "experimental.agent.datadoghq.com/image-override-config": `not-json`, }, - want: "", + want: baseImage, }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dda := &metav1.ObjectMeta{Annotations: tt.annotations} + assert.Equal(t, tt.want, resolveHostProfilerImage(dda, baseImage)) + }) + } +} + +func TestHostProfilerSeccompImageStaysAlignedThroughOverrides(t *testing.T) { + ifNotPresent := corev1.PullIfNotPresent + tests := []struct { + name string + annotations map[string]string + baseImage string + componentOverride *v2alpha1.DatadogAgentComponentOverride + wantImage string + }{ { - name: "spec.override.nodeAgent.image", - ddaSpec: &v2alpha1.DatadogAgentSpec{ - Override: map[v2alpha1.ComponentName]*v2alpha1.DatadogAgentComponentOverride{ - v2alpha1.NodeAgentComponentName: {Image: &v2alpha1.AgentImageConfig{Name: "gcr.io/x/agent", Tag: "7.99.0"}}, - }, + name: "node agent image override does not rewrite host-profiler seccomp image", + baseImage: "gcr.io/datadoghq/agent:7.80.1", + componentOverride: &v2alpha1.DatadogAgentComponentOverride{ + Image: &v2alpha1.AgentImageConfig{Name: "agent", Tag: "nightly", PullPolicy: &ifNotPresent}, }, - want: "gcr.io/x/agent:7.99.0", + wantImage: "gcr.io/datadoghq/agent:7.80.1", }, + // Tag-only experimental overrides are intentionally omitted here: host-profiler must select + // an image name that contains the profiler, while tag-only would keep the agent image name. { - name: "experimental annotation takes precedence over spec.override.nodeAgent.image", + name: "experimental name and tag preserves registry after node agent override", annotations: map[string]string{ - "experimental.agent.datadoghq.com/image-override-config": `{"host-profiler":{"name":"gcr.io/x/host-profiler:v2"}}`, + "experimental.agent.datadoghq.com/image-override-config": `{"host-profiler":{"name":"host-profiler","tag":"v2"}}`, }, - ddaSpec: &v2alpha1.DatadogAgentSpec{ - Override: map[v2alpha1.ComponentName]*v2alpha1.DatadogAgentComponentOverride{ - v2alpha1.NodeAgentComponentName: {Image: &v2alpha1.AgentImageConfig{Tag: "7.99.0"}}, - }, + baseImage: "custom.registry/agent:7.80.1-fips", + componentOverride: &v2alpha1.DatadogAgentComponentOverride{ + Image: &v2alpha1.AgentImageConfig{Name: "agent", Tag: "nightly", PullPolicy: &ifNotPresent}, }, - want: "gcr.io/x/host-profiler:v2", + wantImage: "custom.registry/host-profiler:v2", }, } + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - dda := &metav1.ObjectMeta{Annotations: tt.annotations} - assert.Equal(t, tt.want, resolveHostProfilerImage(dda, tt.ddaSpec)) + annotations := map[string]string{"agent.datadoghq.com/host-profiler-enabled": "true"} + for k, v := range tt.annotations { + annotations[k] = v + } + dda := testutils.NewDatadogAgentBuilder(). + WithName("datadog-agent"). + WithAnnotations(annotations). + Build() + dda.Spec.Override[v2alpha1.NodeAgentComponentName] = tt.componentOverride + + manager := fake.NewPodTemplateManagers(t, corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: string(apicommon.CoreAgentContainerName), Image: "gcr.io/datadoghq/agent:7.80.1"}, + { + Name: string(apicommon.HostProfiler), + Image: tt.baseImage, + SecurityContext: &corev1.SecurityContext{ + ReadOnlyRootFilesystem: ptr.To(true), + }, + }, + }, + }, + }) + + hostProfilerFeat := buildHostProfilerFeature(nil).(*hostProfilerFeature) + hostProfilerFeat.Configure(dda, &dda.Spec, nil) + assert.NoError(t, hostProfilerFeat.ManageNodeAgent(manager)) + + override.PodTemplateSpec(logr.Discard(), manager, tt.componentOverride, v2alpha1.NodeAgentComponentName, dda.Name) + experimental.ApplyExperimentalOverrides(logr.Discard(), dda, manager) + + var hostProfilerContainer *corev1.Container + for i := range manager.PodTemplateSpec().Spec.Containers { + if manager.PodTemplateSpec().Spec.Containers[i].Name == string(apicommon.HostProfiler) { + hostProfilerContainer = &manager.PodTemplateSpec().Spec.Containers[i] + break + } + } + assert.NotNil(t, hostProfilerContainer) + if hostProfilerContainer != nil { + assert.Equal(t, tt.wantImage, hostProfilerContainer.Image) + assert.Equal(t, ifNotPresent, hostProfilerContainer.ImagePullPolicy) + assert.Equal(t, seccompProfileName(tt.wantImage), *hostProfilerContainer.SecurityContext.SeccompProfile.LocalhostProfile) + } + + var setupContainer *corev1.Container + for i := range manager.PodTemplateSpec().Spec.InitContainers { + if manager.PodTemplateSpec().Spec.InitContainers[i].Name == string(apicommon.HostProfilerSeccompSetupContainerName) { + setupContainer = &manager.PodTemplateSpec().Spec.InitContainers[i] + break + } + } + assert.NotNil(t, setupContainer) + if setupContainer != nil { + assert.Equal(t, tt.wantImage, setupContainer.Image) + assert.Equal(t, ifNotPresent, setupContainer.ImagePullPolicy) + assert.Contains(t, setupContainer.Command, common.SeccompRootVolumePath+"/"+seccompProfileName(tt.wantImage)) + } }) } } diff --git a/internal/controller/datadogagent/feature/hostprofiler/seccomp.go b/internal/controller/datadogagent/feature/hostprofiler/seccomp.go index 6260371282..28a80202ce 100644 --- a/internal/controller/datadogagent/feature/hostprofiler/seccomp.go +++ b/internal/controller/datadogagent/feature/hostprofiler/seccomp.go @@ -11,6 +11,7 @@ import ( corev1 "k8s.io/api/core/v1" + apicommon "github.com/DataDog/datadog-operator/api/datadoghq/common" "github.com/DataDog/datadog-operator/internal/controller/datadogagent/common" ) @@ -41,7 +42,7 @@ func defaultCapabilities() []corev1.Capability { func buildSeccompSetupInitContainer(image string) corev1.Container { return corev1.Container{ - Name: "host-profiler-seccomp-setup", + Name: string(apicommon.HostProfilerSeccompSetupContainerName), Image: image, Command: []string{ "cp", diff --git a/internal/controller/datadogagent/override/podtemplatespec.go b/internal/controller/datadogagent/override/podtemplatespec.go index 2cfaa7f088..3c9f9dbc80 100644 --- a/internal/controller/datadogagent/override/podtemplatespec.go +++ b/internal/controller/datadogagent/override/podtemplatespec.go @@ -73,7 +73,7 @@ func PodTemplateSpec(logger logr.Logger, manager feature.PodTemplateManagers, ov JMXEnabled: false, } manager.PodTemplateSpec().Spec.Containers[i].Image = images.OverrideAgentImage(container.Image, otelOverride) - } else { + } else if containerName != apicommon.HostProfiler { manager.PodTemplateSpec().Spec.Containers[i].Image = images.OverrideAgentImage(container.Image, override.Image) } if override.Image.PullPolicy != nil { @@ -83,7 +83,10 @@ func PodTemplateSpec(logger logr.Logger, manager feature.PodTemplateManagers, ov } for i, initContainer := range manager.PodTemplateSpec().Spec.InitContainers { - manager.PodTemplateSpec().Spec.InitContainers[i].Image = images.OverrideAgentImage(initContainer.Image, override.Image) + // host-profiler-seccomp-setup copies a seccomp profile JSON baked into the profiler image, not the agent image. + if apicommon.AgentContainerName(initContainer.Name) != apicommon.HostProfilerSeccompSetupContainerName { + manager.PodTemplateSpec().Spec.InitContainers[i].Image = images.OverrideAgentImage(initContainer.Image, override.Image) + } if override.Image.PullPolicy != nil { manager.PodTemplateSpec().Spec.InitContainers[i].ImagePullPolicy = *override.Image.PullPolicy } diff --git a/internal/controller/datadogagent/override/podtemplatespec_test.go b/internal/controller/datadogagent/override/podtemplatespec_test.go index 9c74f95a5f..205a444c3d 100644 --- a/internal/controller/datadogagent/override/podtemplatespec_test.go +++ b/internal/controller/datadogagent/override/podtemplatespec_test.go @@ -343,6 +343,45 @@ func TestPodTemplateSpec(t *testing.T) { }, t) }, }, + { + name: "image override does not rewrite host-profiler images but applies pull policy", + existingManager: func() *fake.PodTemplateManagers { + manager := fake.NewPodTemplateManagers(t, v1.PodTemplateSpec{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + {Name: string(apicommon.CoreAgentContainerName), Image: "agent:old"}, + {Name: string(apicommon.HostProfiler), Image: "profiler:old"}, + }, + InitContainers: []v1.Container{ + {Name: string(apicommon.InitVolumeContainerName), Image: "agent:old"}, + {Name: string(apicommon.HostProfilerSeccompSetupContainerName), Image: "profiler:old"}, + }, + }, + }) + return manager + }, + override: v2alpha1.DatadogAgentComponentOverride{ + Image: &v2alpha1.AgentImageConfig{Name: "agent", Tag: "new", PullPolicy: &ifNotPresent}, + }, + validateManager: func(t *testing.T, manager *fake.PodTemplateManagers) { + for _, c := range manager.PodTemplateSpec().Spec.Containers { + if c.Name == string(apicommon.HostProfiler) { + assert.Equal(t, "profiler:old", c.Image, "host-profiler image must not be overridden") + } else { + assert.NotEqual(t, "profiler:old", c.Image, "container %s image should have been overridden", c.Name) + } + assert.Equal(t, v1.PullIfNotPresent, c.ImagePullPolicy, "container %s pull policy should have been overridden", c.Name) + } + for _, c := range manager.PodTemplateSpec().Spec.InitContainers { + if c.Name == string(apicommon.HostProfilerSeccompSetupContainerName) { + assert.Equal(t, "profiler:old", c.Image, "host-profiler-seccomp-setup image must not be overridden") + } else { + assert.NotEqual(t, "profiler:old", c.Image, "init container %s image should have been overridden", c.Name) + } + assert.Equal(t, v1.PullIfNotPresent, c.ImagePullPolicy, "init container %s pull policy should have been overridden", c.Name) + } + }, + }, { name: "add envs", existingManager: func() *fake.PodTemplateManagers {