Skip to content

Commit 9678903

Browse files
lebaucetbavelier
andauthored
Fix AppArmor annotations for absent containers in annotation overrides (#2897)
* Fix AppArmor annotations for absent containers in direct annotation overrides The fix in a0dc8c0 added a container existence check to overrideAppArmorProfile(), preventing invalid AppArmor annotations when a container (e.g. security-agent with directSendFromSystemProbe=true) is absent from the pod spec. However, the same guard was missing from the direct annotation loop in PodTemplateSpec(), which blindly copies spec.override.nodeAgent.annotations to the pod template. Any AppArmor annotation set via that path would bypass the existing fix and still produce an invalid DaemonSet. Apply the same container existence check when iterating override.Annotations: skip AppArmor annotations (container.apparmor.security.beta.kubernetes.io/<name>) if <name> does not match any container in the pod spec. * Use slices shared helper instead of duplicating same logic * Add unit tests --------- Co-authored-by: Timothée Bavelier <timothee.bavelier@datadoghq.com>
1 parent ed1b311 commit 9678903

3 files changed

Lines changed: 70 additions & 9 deletions

File tree

internal/controller/datadogagent/override/container.go

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -242,15 +242,7 @@ func overrideAppArmorProfile(containerName apicommon.AgentContainerName, manager
242242
// Only add the AppArmor annotation if the container actually exists in the pod spec.
243243
// This avoids invalid DaemonSet configurations when a container is not present
244244
// (e.g. security-agent is absent when directSendFromSystemProbe is enabled).
245-
containerExists := false
246-
allContainers := append(manager.PodTemplateSpec().Spec.Containers, manager.PodTemplateSpec().Spec.InitContainers...)
247-
for _, c := range allContainers {
248-
if c.Name == effectiveName {
249-
containerExists = true
250-
break
251-
}
252-
}
253-
if !containerExists {
245+
if !podSpecHasContainer(&manager.PodTemplateSpec().Spec, effectiveName) {
254246
return
255247
}
256248

internal/controller/datadogagent/override/podtemplatespec.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,14 @@ func PodTemplateSpec(logger logr.Logger, manager feature.PodTemplateManagers, ov
193193
manager.PodTemplateSpec().Spec.Tolerations = append(manager.PodTemplateSpec().Spec.Tolerations, override.Tolerations...)
194194

195195
for annotationName, annotationVal := range override.Annotations {
196+
// For AppArmor annotations, skip if the referenced container doesn't exist.
197+
// This mirrors the check in overrideAppArmorProfile() and prevents invalid DaemonSet
198+
// configurations when a container is absent (e.g. security-agent with directSendFromSystemProbe).
199+
if containerName, ok := strings.CutPrefix(annotationName, common.AppArmorAnnotationKey+"/"); ok {
200+
if !podSpecHasContainer(&manager.PodTemplateSpec().Spec, containerName) {
201+
continue
202+
}
203+
}
196204
manager.Annotation().AddAnnotation(annotationName, annotationVal)
197205
}
198206

@@ -262,6 +270,14 @@ func overrideCustomConfigVolumes(logger logr.Logger, manager feature.PodTemplate
262270
}
263271
}
264272

273+
// podSpecHasContainer reports whether the pod spec contains a (init)container with the given name.
274+
func podSpecHasContainer(podSpec *corev1.PodSpec, name string) bool {
275+
allContainers := append(podSpec.Containers, podSpec.InitContainers...)
276+
return slices.ContainsFunc(allContainers, func(c corev1.Container) bool {
277+
return c.Name == name
278+
})
279+
}
280+
265281
func sortKeys(keysMap map[v2alpha1.AgentConfigFileName]v2alpha1.CustomConfig) []v2alpha1.AgentConfigFileName {
266282
sortedKeys := make([]v2alpha1.AgentConfigFileName, 0, len(keysMap))
267283
for key := range keysMap {

internal/controller/datadogagent/override/podtemplatespec_test.go

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1036,6 +1036,59 @@ func TestPodTemplateSpec(t *testing.T) {
10361036
assert.Equal(t, "gcr.io/datadoghq/ddot-collector:7.72.1", otelAgentImage, "OtelAgent should not have JMX suffix")
10371037
},
10381038
},
1039+
{
1040+
name: "AppArmor annotation in override.Annotations for existing container is added",
1041+
existingManager: func() *fake.PodTemplateManagers {
1042+
manager := fake.NewPodTemplateManagers(t, v1.PodTemplateSpec{})
1043+
manager.PodTemplateSpec().Spec.Containers = []v1.Container{
1044+
{Name: string(apicommon.CoreAgentContainerName)},
1045+
}
1046+
return manager
1047+
},
1048+
override: v2alpha1.DatadogAgentComponentOverride{
1049+
Annotations: map[string]string{
1050+
fmt.Sprintf("%s/%s", common.AppArmorAnnotationKey, apicommon.CoreAgentContainerName): "runtime/default",
1051+
},
1052+
},
1053+
validateManager: func(t *testing.T, manager *fake.PodTemplateManagers) {
1054+
annotation := fmt.Sprintf("%s/%s", common.AppArmorAnnotationKey, apicommon.CoreAgentContainerName)
1055+
assert.Equal(t, "runtime/default", manager.AnnotationMgr.Annotations[annotation])
1056+
},
1057+
},
1058+
{
1059+
name: "AppArmor annotation in override.Annotations for absent container is skipped",
1060+
existingManager: func() *fake.PodTemplateManagers {
1061+
manager := fake.NewPodTemplateManagers(t, v1.PodTemplateSpec{})
1062+
manager.PodTemplateSpec().Spec.Containers = []v1.Container{
1063+
{Name: string(apicommon.CoreAgentContainerName)},
1064+
}
1065+
return manager
1066+
},
1067+
override: v2alpha1.DatadogAgentComponentOverride{
1068+
Annotations: map[string]string{
1069+
fmt.Sprintf("%s/%s", common.AppArmorAnnotationKey, apicommon.SecurityAgentContainerName): "runtime/default",
1070+
},
1071+
},
1072+
validateManager: func(t *testing.T, manager *fake.PodTemplateManagers) {
1073+
annotation := fmt.Sprintf("%s/%s", common.AppArmorAnnotationKey, apicommon.SecurityAgentContainerName)
1074+
_, found := manager.AnnotationMgr.Annotations[annotation]
1075+
assert.False(t, found, "AppArmor annotation for absent container should not be added")
1076+
},
1077+
},
1078+
{
1079+
name: "non-AppArmor annotation in override.Annotations is always added",
1080+
existingManager: func() *fake.PodTemplateManagers {
1081+
return fake.NewPodTemplateManagers(t, v1.PodTemplateSpec{})
1082+
},
1083+
override: v2alpha1.DatadogAgentComponentOverride{
1084+
Annotations: map[string]string{
1085+
"some-other-annotation": "value",
1086+
},
1087+
},
1088+
validateManager: func(t *testing.T, manager *fake.PodTemplateManagers) {
1089+
assert.Equal(t, "value", manager.AnnotationMgr.Annotations["some-other-annotation"])
1090+
},
1091+
},
10391092
{
10401093
name: "Add CEL Workload Exclude",
10411094
existingManager: func() *fake.PodTemplateManagers {

0 commit comments

Comments
 (0)