From 5a79a7ad3b7618ae9d7ae440a6f15f8de7bae058 Mon Sep 17 00:00:00 2001 From: khewonc <39867936+khewonc@users.noreply.github.com> Date: Thu, 11 Jun 2026 17:44:14 -0400 Subject: [PATCH] Use allowlist for dap validation --- .../datadogagentprofile_validation.go | 237 +++++++----------- .../datadogagentprofile_validation_test.go | 138 ++++++++++ 2 files changed, 229 insertions(+), 146 deletions(-) diff --git a/api/datadoghq/v1alpha1/datadogagentprofile_validation.go b/api/datadoghq/v1alpha1/datadogagentprofile_validation.go index 7001bab241..12e394fe64 100644 --- a/api/datadoghq/v1alpha1/datadogagentprofile_validation.go +++ b/api/datadoghq/v1alpha1/datadogagentprofile_validation.go @@ -8,11 +8,31 @@ package v1alpha1 import ( "fmt" "reflect" + "strings" + "unicode" "github.com/DataDog/datadog-operator/api/datadoghq/common" "github.com/DataDog/datadog-operator/api/datadoghq/v2alpha1" ) +var datadogAgentProfileFeatureAllowlist = map[string]struct{}{ + "gpu": {}, + "apm": {}, +} + +var datadogAgentProfileComponentOverrideAllowlist = map[string]struct{}{ + "containers": {}, + "priorityClassName": {}, + "runtimeClassName": {}, + "updateStrategy": {}, + "labels": {}, +} + +var datadogAgentProfileContainerOverrideAllowlist = map[string]struct{}{ + "resources": {}, + "env": {}, +} + // ValidateDatadogAgentProfileSpec is used to check if a DatadogAgentProfileSpec is valid func ValidateDatadogAgentProfileSpec(spec *DatadogAgentProfileSpec) error { if err := validateProfileAffinity(spec.ProfileAffinity); err != nil { @@ -64,134 +84,25 @@ func validateFeatures(features *v2alpha1.DatadogFeatures) error { return nil } - // Only GPU feature is currently supported in DatadogAgentProfile context. - // Remove supported features from the `unsupportedFeatures` array. - unsupportedFeatures := []struct { - value any - name string - }{ - {features.OtelCollector, "otelCollector"}, - {features.LogCollection, "logCollection"}, - {features.LiveProcessCollection, "liveProcessCollection"}, - {features.LiveContainerCollection, "liveContainerCollection"}, - {features.ProcessDiscovery, "processDiscovery"}, - {features.OOMKill, "oomKill"}, - {features.TCPQueueLength, "tcpQueueLength"}, - {features.EBPFCheck, "ebpfCheck"}, - {features.APM, "apm"}, - {features.ASM, "asm"}, - {features.CSPM, "cspm"}, - {features.CWS, "cws"}, - {features.NPM, "npm"}, - {features.USM, "usm"}, - {features.Dogstatsd, "dogstatsd"}, - {features.OTLP, "otlp"}, - {features.RemoteConfiguration, "remoteConfiguration"}, - {features.SBOM, "sbom"}, - {features.ServiceDiscovery, "serviceDiscovery"}, - {features.EventCollection, "eventCollection"}, - {features.OrchestratorExplorer, "orchestratorExplorer"}, - {features.KubeStateMetricsCore, "kubeStateMetricsCore"}, - {features.AdmissionController, "admissionController"}, - {features.ExternalMetricsServer, "externalMetricsServer"}, - {features.Autoscaling, "autoscaling"}, - {features.ClusterChecks, "clusterChecks"}, - {features.PrometheusScrape, "prometheusScrape"}, - {features.HelmCheck, "helmCheck"}, - {features.ControlPlaneMonitoring, "controlPlaneMonitoring"}, - } - - for _, feature := range unsupportedFeatures { - // Use reflection to check if the underlying value is actually nil - // because any can hold a typed nil pointer - if feature.value != nil && !reflect.ValueOf(feature.value).IsNil() { - return unsupportedError(feature.name) - } - } - - // GPU is allowed, no error returned - return nil + return validateAllowlistedFields(features, datadogAgentProfileFeatureAllowlist, jsonFieldName) } func validateOverride(component v2alpha1.ComponentName, override *v2alpha1.DatadogAgentComponentOverride) error { if component != v2alpha1.NodeAgentComponentName { return fmt.Errorf("only node agent componentoverrides are supported") } - - if override.Name != nil { - return unsupportedError("component name") - } - if override.Replicas != nil { - return unsupportedError("component replicas") - } - if override.CreatePodDisruptionBudget != nil { - return unsupportedError("component create pod disruption budget") - } - if override.CreateRbac != nil { - return unsupportedError("component create rbac") + if override == nil { + return undefinedError("component override") } - if override.ServiceAccountName != nil { - return unsupportedError("component service account name") - } - if override.ServiceAccountAnnotations != nil { - return unsupportedError("component service account annotations") - } - if override.Image != nil { - return unsupportedError("component image") - } - if override.Env != nil { - return unsupportedError("component env") - } - if override.EnvFrom != nil { - return unsupportedError("component env from") - } - if override.CustomConfigurations != nil { - return unsupportedError("component custom configurations") - } - if override.ExtraConfd != nil { - return unsupportedError("component extra confd") - } - if override.ExtraChecksd != nil { - return unsupportedError("component extra checksd") + + if err := validateAllowlistedFields(override, datadogAgentProfileComponentOverrideAllowlist, prefixedJSONFieldName("component")); err != nil { + return err } for name, override := range override.Containers { if err := validateContainerOverride(name, override); err != nil { return err } } - if override.Volumes != nil { - return unsupportedError("component volumes") - } - if override.SecurityContext != nil { - return unsupportedError("component security context") - } - if override.Affinity != nil { - return unsupportedError("component affinity") - } - if override.DNSPolicy != nil { - return unsupportedError("component dns policy") - } - if override.DNSConfig != nil { - return unsupportedError("component dns config") - } - if override.NodeSelector != nil { - return unsupportedError("component node selector") - } - if override.Tolerations != nil { - return unsupportedError("component tolerations") - } - if override.Annotations != nil { - return unsupportedError("component annotations") - } - if override.HostNetwork != nil { - return unsupportedError("component host network") - } - if override.HostPID != nil { - return unsupportedError("component host pid") - } - if override.Disabled != nil { - return unsupportedError("component disabled") - } return nil } @@ -209,45 +120,79 @@ func validateContainerOverride(name common.AgentContainerName, override *v2alpha if _, ok := supportedContainers[name]; !ok { return unsupportedError(fmt.Sprintf("container %s", name)) } - - if override.Name != nil { - return unsupportedError("container name") - } - if override.LogLevel != nil { - return unsupportedError("container log level") - } - if override.VolumeMounts != nil { - return unsupportedError("container volume mounts") - } - if override.Command != nil { - return unsupportedError("container command") + if override == nil { + return undefinedError(fmt.Sprintf("container %s", name)) } - if override.Args != nil { - return unsupportedError("container args") - } - if override.HealthPort != nil { - return unsupportedError("container health port") - } - if override.ReadinessProbe != nil { - return unsupportedError("container readiness probe") + + return validateAllowlistedFields(override, datadogAgentProfileContainerOverrideAllowlist, prefixedJSONFieldName("container")) +} + +// For every set field in a struct/pointer, read the JSON name +// (nodeSelector from json:"nodeSelector,omitempty") and check +// the name is in the allowlist. If not in the list, error +func validateAllowlistedFields(value any, allowlist map[string]struct{}, unsupportedFieldName func(reflect.StructField) string) error { + structValue := reflect.ValueOf(value) + if structValue.Kind() == reflect.Ptr { + if structValue.IsNil() { + return nil + } + structValue = structValue.Elem() } - if override.LivenessProbe != nil { - return unsupportedError("container liveness probe") + + structType := structValue.Type() + for i := 0; i < structValue.NumField(); i++ { + if isEmptyOverrideValue(structValue.Field(i)) { + continue + } + + field := structType.Field(i) + if _, ok := allowlist[jsonFieldName(field)]; !ok { + return unsupportedError(unsupportedFieldName(field)) + } } - if override.StartupProbe != nil { - return unsupportedError("container startup probe") + + return nil +} + +func isEmptyOverrideValue(value reflect.Value) bool { + switch value.Kind() { + case reflect.Chan, reflect.Func, reflect.Interface, reflect.Map, reflect.Ptr, reflect.Slice: + return value.IsNil() + default: + return value.IsZero() } - if override.SecurityContext != nil { - return unsupportedError("container security context") +} + +func jsonFieldName(field reflect.StructField) string { + name, _, _ := strings.Cut(field.Tag.Get("json"), ",") + if name == "" || name == "-" { + return field.Name } - if override.SeccompConfig != nil { - return unsupportedError("container seccomp config") + + return name +} + +func prefixedJSONFieldName(prefix string) func(reflect.StructField) string { + return func(field reflect.StructField) string { + return fmt.Sprintf("%s %s", prefix, splitJSONFieldName(field)) } - if override.AppArmorProfileName != nil { - return unsupportedError("container app armor profile name") +} + +func splitJSONFieldName(field reflect.StructField) string { + name := []rune(jsonFieldName(field)) + var words []rune + for i, r := range name { + if i > 0 && unicode.IsUpper(r) { + previous := name[i-1] + hasNext := i+1 < len(name) + if unicode.IsLower(previous) || hasNext && unicode.IsLower(name[i+1]) { + words = append(words, ' ') + } + } + words = append(words, unicode.ToLower(r)) } - return nil + return string(words) } func unsupportedError(config string) error { diff --git a/api/datadoghq/v1alpha1/datadogagentprofile_validation_test.go b/api/datadoghq/v1alpha1/datadogagentprofile_validation_test.go index d2af2a8fa0..a7e64ca943 100644 --- a/api/datadoghq/v1alpha1/datadogagentprofile_validation_test.go +++ b/api/datadoghq/v1alpha1/datadogagentprofile_validation_test.go @@ -6,6 +6,7 @@ package v1alpha1 import ( + "reflect" "testing" "k8s.io/utils/ptr" @@ -227,3 +228,140 @@ func TestIsValidDatadogAgentProfile(t *testing.T) { }) } } + +func TestValidateDatadogAgentProfileFeaturesAllowlist(t *testing.T) { + allowedFeatureFields := map[string]struct{}{ + "GPU": {}, + } + + featuresType := reflect.TypeOf(v2alpha1.DatadogFeatures{}) + for i := 0; i < featuresType.NumField(); i++ { + field := featuresType.Field(i) + t.Run(field.Name, func(t *testing.T) { + if !assert.Equal(t, reflect.Ptr, field.Type.Kind(), "DatadogFeatures fields should be pointer types") { + return + } + + features := &v2alpha1.DatadogFeatures{} + reflect.ValueOf(features).Elem().FieldByName(field.Name).Set(reflect.New(field.Type.Elem())) + + spec := &DatadogAgentProfileSpec{ + ProfileAffinity: validProfileAffinity(), + Config: &v2alpha1.DatadogAgentSpec{ + Features: features, + }, + } + + result := ValidateDatadogAgentProfileSpec(spec) + if _, ok := allowedFeatureFields[field.Name]; ok { + assert.NoError(t, result) + } else { + if assert.Error(t, result) { + assert.Contains(t, result.Error(), "override is not supported") + } + } + }) + } +} + +func TestValidateDatadogAgentProfileComponentOverrideAllowlist(t *testing.T) { + allowedComponentOverrideFields := map[string]struct{}{ + "Containers": {}, + "PriorityClassName": {}, + "RuntimeClassName": {}, + "UpdateStrategy": {}, + "Labels": {}, + } + + overrideType := reflect.TypeOf(v2alpha1.DatadogAgentComponentOverride{}) + for i := 0; i < overrideType.NumField(); i++ { + field := overrideType.Field(i) + t.Run(field.Name, func(t *testing.T) { + override := &v2alpha1.DatadogAgentComponentOverride{} + setConfiguredField(t, reflect.ValueOf(override).Elem().FieldByName(field.Name)) + + spec := &DatadogAgentProfileSpec{ + ProfileAffinity: validProfileAffinity(), + Config: &v2alpha1.DatadogAgentSpec{ + Override: map[v2alpha1.ComponentName]*v2alpha1.DatadogAgentComponentOverride{ + v2alpha1.NodeAgentComponentName: override, + }, + }, + } + + result := ValidateDatadogAgentProfileSpec(spec) + if _, ok := allowedComponentOverrideFields[field.Name]; ok { + assert.NoError(t, result) + } else { + if assert.Error(t, result) { + assert.Contains(t, result.Error(), "override is not supported") + } + } + }) + } +} + +func TestValidateDatadogAgentProfileContainerOverrideAllowlist(t *testing.T) { + allowedContainerOverrideFields := map[string]struct{}{ + "Resources": {}, + "Env": {}, + } + + containerType := reflect.TypeOf(v2alpha1.DatadogAgentGenericContainer{}) + for i := 0; i < containerType.NumField(); i++ { + field := containerType.Field(i) + t.Run(field.Name, func(t *testing.T) { + containerOverride := &v2alpha1.DatadogAgentGenericContainer{} + setConfiguredField(t, reflect.ValueOf(containerOverride).Elem().FieldByName(field.Name)) + + spec := &DatadogAgentProfileSpec{ + ProfileAffinity: validProfileAffinity(), + Config: &v2alpha1.DatadogAgentSpec{ + Override: map[v2alpha1.ComponentName]*v2alpha1.DatadogAgentComponentOverride{ + v2alpha1.NodeAgentComponentName: { + Containers: map[common.AgentContainerName]*v2alpha1.DatadogAgentGenericContainer{ + common.CoreAgentContainerName: containerOverride, + }, + }, + }, + }, + } + + result := ValidateDatadogAgentProfileSpec(spec) + if _, ok := allowedContainerOverrideFields[field.Name]; ok { + assert.NoError(t, result) + } else { + if assert.Error(t, result) { + assert.Contains(t, result.Error(), "override is not supported") + } + } + }) + } +} + +func validProfileAffinity() *ProfileAffinity { + return &ProfileAffinity{ + ProfileNodeAffinity: []corev1.NodeSelectorRequirement{ + { + Key: "foo", + Operator: corev1.NodeSelectorOpIn, + Values: []string{"bar"}, + }, + }, + } +} + +func setConfiguredField(t *testing.T, fieldValue reflect.Value) { + t.Helper() + + switch fieldValue.Kind() { + case reflect.Map: + fieldValue.Set(reflect.MakeMap(fieldValue.Type())) + case reflect.Ptr: + fieldValue.Set(reflect.New(fieldValue.Type().Elem())) + case reflect.Slice: + fieldValue.Set(reflect.MakeSlice(fieldValue.Type(), 0, 0)) + default: + t.Fatalf("unsupported field kind %q", fieldValue.Kind()) + } +}