Skip to content

feat(appsec): add ingress-nginx proxy injection support#3053

Merged
eliottness merged 4 commits into
mainfrom
eliottness/appsec-ingress-nginx-injection
Jun 3, 2026
Merged

feat(appsec): add ingress-nginx proxy injection support#3053
eliottness merged 4 commits into
mainfrom
eliottness/appsec-ingress-nginx-injection

Conversation

@eliottness

@eliottness eliottness commented May 28, 2026

Copy link
Copy Markdown
Contributor

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:

  • New annotation agent.datadoghq.com/appsec.nginx.module_mount_path — overrides the mount path inside the controller pod where the nginx-datadog module is exposed
  • The annotation forwards to the cluster-agent as DD_ADMISSION_CONTROLLER_APPSEC_NGINX_MODULE_MOUNT_PATH env var (only when non-empty; cluster-agent owns the default)
  • Two new ClusterRole rules added to the AppSec feature RBAC:
    • 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 in rbac.go)
  • "ingress-nginx" added to the allowedProxyValues whitelist

Motivation

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:

  1. The cluster-agent lacks the RBAC permissions to watch IngressClass resources and manage the DD-owned ConfigMaps
  2. There is no way to override the module mount path

Additional Notes

  • Annotation-based (not CRD fields): matches the existing AppSec proxy injection pattern — Envoy/Istio sidecar config is already annotation-based via agent.datadoghq.com/appsec.*
  • Additive-only RBAC: the existing configmaps [get, update] rule is preserved unchanged; the new wider rule is appended as a separate entry
  • No CRD schema changes: make generate && make manifests produces zero diff
  • Security note on the new configmaps rule is verbatim from the cluster-agent briefing — the reconciler filters by labels appsec.datadoghq.com/watched-configmap=true and app.kubernetes.io/component=datadog-appsec-injector at runtime
  • The init image override (appsec.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.
  • Sister change: briefing-datadog-helm-chart-ingress-nginx-appsec.md tracks the corresponding Helm chart update (separate PR)

Minimum Agent Versions

  • Agent: N/A (operator-side change only)
  • Cluster Agent: v7.76.0 (minimum already enforced by ClusterAgentMinVersion constant; 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 value
  • TestFromAnnotations: happy path, unset annotation, empty-string annotation
  • TestAppsecFeature: env var emitted when annotation set; absent when feature disabled, annotation empty, or annotation unset
  • TestAppsecRBACPolicyRules: both new rules present; existing [get, update] rule preserved
go test ./internal/controller/datadogagent/feature/appsec/... -v -count=1

QA on kind

Prerequisites:

  • A kind cluster: kind create cluster
  • The operator built from this branch: make docker-build IMG=datadog/operator:dev
  • Load it into kind: kind load docker-image datadog/operator:dev

Step 1 — Deploy the operator with a DatadogAgent that has AppSec injection enabled:

apiVersion: datadoghq.com/v2alpha1
kind: DatadogAgent
metadata:
  name: datadog
  annotations:
    agent.datadoghq.com/appsec.injector.enabled: "true"
    agent.datadoghq.com/appsec.injector.autoDetect: "true"
    agent.datadoghq.com/appsec.nginx.module_mount_path: "/custom/modules"
spec:
  global:
    credentials:
      apiKey: <your-api-key>
  features:
    clusterAgent:
      replicas: 1

Step 2 — Verify the ClusterRole includes the new RBAC rules:

# Get the generated ClusterRole name
CR_NAME=$(kubectl get clusterrole -l app.kubernetes.io/managed-by=datadog-operator \
  -o jsonpath='{.items[*].metadata.name}' | tr " " "\n" | grep appsec)

# Verify ingressclasses rule
kubectl get clusterrole "$CR_NAME" -o json \
  | jq '.rules[] | select(.resources[] == "ingressclasses")'
# Expected: apiGroups=["networking.k8s.io"], verbs=["get","list","watch"]

# Verify new configmaps rule (the one with 6 verbs, not the existing 2-verb rule)
kubectl get clusterrole "$CR_NAME" -o json \
  | jq '[.rules[] | select(.resources[] == "configmaps")] | .[] | select(.verbs | length > 2)'
# Expected: apiGroups=[""], verbs=["get","list","watch","create","update","delete"]

# Verify existing configmaps rule still present
kubectl get clusterrole "$CR_NAME" -o json \
  | jq '[.rules[] | select(.resources[] == "configmaps")] | .[] | select(.verbs | length == 2)'
# Expected: apiGroups=[""], verbs=["get","update"]

Step 3 — Verify the cluster-agent container has the env var:

DCA_POD=$(kubectl get pods -l app.kubernetes.io/name=datadog-cluster-agent \
  -o jsonpath='{.items[0].metadata.name}')

# Verify module mount path env var is set
kubectl exec "$DCA_POD" -- env | grep DD_ADMISSION_CONTROLLER_APPSEC_NGINX_MODULE_MOUNT_PATH
# Expected: DD_ADMISSION_CONTROLLER_APPSEC_NGINX_MODULE_MOUNT_PATH=/custom/modules

Step 4 — Verify env var is absent when annotation is not set:

Remove the appsec.nginx.module_mount_path annotation from the DatadogAgent, wait for reconciliation, and verify:

kubectl exec "$DCA_POD" -- env | grep DD_ADMISSION_CONTROLLER_APPSEC_NGINX
# Expected: no output (env var not present — cluster-agent uses its own default)

Checklist

  • PR has at least one valid label: bug, enhancement, refactoring, documentation, tooling, and/or dependencies
  • PR has a milestone or the qa/skip-qa label
  • All commits are signed (see: signing commits)

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
@eliottness

Copy link
Copy Markdown
Contributor Author

@codex review

@datadog-prod-us1-3

datadog-prod-us1-3 Bot commented May 28, 2026

Copy link
Copy Markdown

Code Coverage

🎯 Code Coverage (details)
Patch Coverage: 96.77%
Overall Coverage: 43.62% (+0.26%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 5023ee2 | Docs | Datadog PR Page | Give us feedback!

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread internal/controller/datadogagent/feature/appsec/config.go Outdated
@codecov-commenter

codecov-commenter commented May 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.05882% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 43.31%. Comparing base (c85321f) to head (5023ee2).
⚠️ Report is 12 commits behind head on main.

Files with missing lines Patch % Lines
.../controller/datadogagent/feature/appsec/feature.go 90.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            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     
Flag Coverage Δ
unittests 43.31% <97.05%> (+0.87%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...l/controller/datadogagent/feature/appsec/config.go 100.00% <100.00%> (ø)
...al/controller/datadogagent/feature/appsec/const.go 100.00% <ø> (ø)
...nal/controller/datadogagent/feature/appsec/rbac.go 100.00% <100.00%> (ø)
internal/controller/datadogagent_controller.go 62.12% <ø> (ø)
.../controller/datadogagent/feature/appsec/feature.go 69.60% <90.00%> (+1.56%) ⬆️

... and 27 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c85321f...5023ee2. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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
@eliottness

Copy link
Copy Markdown
Contributor Author

Good catch. The cluster-agent's buildInitContainer appends the controller version to the init image (initImage + ":" + version), so forwarding a user-supplied tagged reference would produce an invalid image like repo:user-tag:v1.15.1. Rather than adding validation here, we've decided to descope the init image override from the operator entirely until the cluster-agent side is addressed. Removed AnnotationNginxInitImage / DDAdmission_CONTROLLER_APPSEC_NGINX_INIT_IMAGE in 41e958e. NginxModuleMountPath is unaffected.

@eliottness eliottness added enhancement New feature or request qa/skip-qa labels May 28, 2026
@eliottness

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread internal/controller/datadogagent/feature/appsec/feature.go
@eliottness
eliottness marked this pull request as ready for review May 28, 2026 10:51
@eliottness
eliottness requested a review from a team May 28, 2026 10:51
@eliottness
eliottness requested a review from a team as a code owner May 28, 2026 10:51

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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"}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eliottness My agent triggered on the same finding, and classify it as critical

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@eliottness eliottness added this to the v1.28.0 milestone May 28, 2026
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 tbavelier left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same RBAC comment than on 2874, apart from that, LGTM

Comment thread internal/controller/datadogagent/feature/appsec/rbac.go
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
@eliottness
eliottness merged commit 7ef174d into main Jun 3, 2026
38 checks passed
@eliottness
eliottness deleted the eliottness/appsec-ingress-nginx-injection branch June 3, 2026 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants