feat(appsec): add ingress-nginx proxy injection support#3053
Conversation
Exposes the cluster-agent's new AppSec ingress-nginx admission-controller
injection knobs (DD_ADMISSION_CONTROLLER_APPSEC_NGINX_INIT_IMAGE and
DD_ADMISSION_CONTROLLER_APPSEC_NGINX_MODULE_MOUNT_PATH) via annotations,
and adds the required ClusterRole permissions (IngressClass watch +
ConfigMap manage) gated behind the existing AppSec injector feature flag.
Constraint: Annotation-based to match existing AppSec proxy injection
pattern (Envoy/Istio sidecar config also uses annotations)
Rejected: CRD field approach | breaks symmetry with existing AppSec config
Rejected: Expanding existing [get, update] configmaps rule | separate-rule
approach preserves backwards-compat
Confidence: high
Scope-risk: narrow
Directive: Security comment on configmaps rule is verbatim per
briefing -- DO NOT paraphrase if refactoring
|
@codex review |
|
🎯 Code Coverage (details) 🔗 Commit SHA: 5023ee2 | Docs | Datadog PR Page | Give us feedback! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d7b9bb246
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3053 +/- ##
==========================================
+ Coverage 42.43% 43.31% +0.87%
==========================================
Files 337 340 +3
Lines 29020 30057 +1037
==========================================
+ Hits 12315 13018 +703
- Misses 15895 16201 +306
- Partials 810 838 +28
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 27 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
The cluster-agent appends the ingress-nginx controller version to the
init image reference (initImage + ":" + version in buildInitContainer),
so forwarding a user-supplied image would produce an invalid reference
like "repo:user-tag:v1.15.1". Descope the init image annotation and
env var from the operator until the cluster-agent side is fixed.
NginxModuleMountPath is unaffected and remains.
Constraint: cluster-agent buildInitContainer design makes init image
override unsafe to expose
Confidence: high
Scope-risk: narrow
|
Good catch. The cluster-agent's |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 41e958e925
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 41e958e925
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ) | ||
|
|
||
| var allowedProxyValues = []string{"envoy-gateway", "istio", "istio-gateway"} | ||
| var allowedProxyValues = []string{"envoy-gateway", "istio", "istio-gateway", "ingress-nginx"} |
There was a problem hiding this comment.
Gate ingress-nginx on a supporting Cluster Agent
For clusters pinning the Cluster Agent below 7.79.0, the current feature gate still admits 7.76.x via ClusterAgentMinVersion, so this newly whitelisted value is accepted and forwarded in DD_APPSEC_PROXY_PROXIES even though ingress-nginx injection was added to the Cluster Agent in 7.79.0. Those users get a valid DatadogAgent that cannot inject ingress-nginx (or may fail DCA config validation) instead of being rejected/gated; please require a 7.79+ DCA when this proxy/env is used or leave the value disallowed for older images.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@eliottness My agent triggered on the same finding, and classify it as critical
There was a problem hiding this comment.
Suggestion:
Add a separate ingress-nginx minimum version gate, likely 7.79.0, at least when appsec.injector.proxies contains ingress-nginx or appsec.nginx.module_mount_path is set. Alternatively, confirm/backport evidence that the feature is available in 7.76.0 and update the upstream/version notes accordingly.
There was a problem hiding this comment.
This can't really even be bulletproof because of runtime detection and I don't want previous users to be forced to upgrade but I guess I can at least make sure when it is set manually to force a higher version yes
There was a problem hiding this comment.
Addressed in 6f90199. When ingress-nginx is explicitly in the proxies annotation or module_mount_path is set, Configure() now checks ClusterAgentNginxMinVersion = "7.79.0" and returns empty RequiredComponents if the cluster-agent image is below that version. Runtime auto-detection cannot be gated here since it is handled internally by the cluster-agent. Existing Envoy/Istio users on 7.76.0+ are unaffected.
When ingress-nginx is explicitly configured (proxies annotation contains
"ingress-nginx" or module_mount_path annotation is set), require
cluster-agent >= 7.79.0 which is the first release with ingress-nginx
injection support. Older cluster-agents silently ignore the config but
users get no feedback; now they get an early version check.
The base AppSec feature still only requires 7.76.0 — existing Envoy/Istio
users are unaffected.
Constraint: runtime auto-detection cannot be version-gated since the
cluster-agent handles that path internally
Confidence: high
Scope-risk: narrow
tbavelier
left a comment
There was a problem hiding this comment.
Same RBAC comment than on 2874, apart from that, LGTM
The operator cannot grant RBAC permissions it does not hold itself. Add +kubebuilder:rbac: markers on the controller for the AppSec rules that were missing from the operator ClusterRole: - gateway.envoyproxy.io: envoypatchpolicies, backend (pre-existing gap) - networking.istio.io: gateways (pre-existing gap) - networking.k8s.io: ingressclasses (new in this PR) - "": configmaps wider verbs (new in this PR) Run make generate to regenerate config/rbac/role.yaml with the new permissions so the reconciler can create the DCA ClusterRole. Constraint: Operator must hold every permission it delegates to the DCA Confidence: high Scope-risk: narrow
What does this PR do?
Adds operator support for the new AppSec ingress-nginx admission-controller injection feature introduced in cluster-agent PR #49318 (merged 2026-04-17).
The cluster-agent now supports a third proxy type for AppSec injection (ingress-nginx, alongside Envoy Gateway and Istio). This PR surfaces the new configuration knob and the required RBAC permissions so users deploying the cluster-agent via the datadog-operator can use the feature.
Changes:
agent.datadoghq.com/appsec.nginx.module_mount_path— overrides the mount path inside the controller pod where thenginx-datadogmodule is exposedDD_ADMISSION_CONTROLLER_APPSEC_NGINX_MODULE_MOUNT_PATHenv var (only when non-empty; cluster-agent owns the default)networking.k8s.io/ingressclasses [get, list, watch]— for discovering ingress-nginx controllers""/configmaps [get, list, watch, create, update, delete]— for managing the DD-owned ConfigMaps that prepend the WAF config (cluster-wide, unscoped; see security note inrbac.go)"ingress-nginx"added to theallowedProxyValueswhitelistMotivation
Cluster-agent PR #49318 extended AppSec proxy injection to ingress-nginx. Without this operator change, users deploying via the datadog-operator cannot use the feature because:
IngressClassresources and manage the DD-owned ConfigMapsAdditional Notes
agent.datadoghq.com/appsec.*configmaps [get, update]rule is preserved unchanged; the new wider rule is appended as a separate entrymake generate && make manifestsproduces zero diffappsec.datadoghq.com/watched-configmap=trueandapp.kubernetes.io/component=datadog-appsec-injectorat runtimeappsec.nginx.init_image) was descoped: the cluster-agent appends the ingress-nginx controller version to the init image reference, making a user-supplied tagged image unsafe to forward. This will be addressed on the cluster-agent side separately.briefing-datadog-helm-chart-ingress-nginx-appsec.mdtracks the corresponding Helm chart update (separate PR)Minimum Agent Versions
ClusterAgentMinVersionconstant; cluster-agent ignores unknown env vars on older versions)Describe your test plan
Unit tests
Tests added (TDD) covering:
TestConfigValidate:"ingress-nginx"accepted as valid proxy valueTestFromAnnotations: happy path, unset annotation, empty-string annotationTestAppsecFeature: env var emitted when annotation set; absent when feature disabled, annotation empty, or annotation unsetTestAppsecRBACPolicyRules: both new rules present; existing[get, update]rule preservedgo test ./internal/controller/datadogagent/feature/appsec/... -v -count=1QA on kind
Prerequisites:
kind create clustermake docker-build IMG=datadog/operator:devkind load docker-image datadog/operator:devStep 1 — Deploy the operator with a DatadogAgent that has AppSec injection enabled:
Step 2 — Verify the ClusterRole includes the new RBAC rules:
Step 3 — Verify the cluster-agent container has the env var:
Step 4 — Verify env var is absent when annotation is not set:
Remove the
appsec.nginx.module_mount_pathannotation from the DatadogAgent, wait for reconciliation, and verify:Checklist
bug,enhancement,refactoring,documentation,tooling, and/ordependenciesqa/skip-qalabel