feat(ssi): last-TRUE-wins SSI matching

pkg/clusteragent/admission/mutate/autoinstrumentation/auto_instrumentation.go

@@ -27,7 +27,7 @@ import (
 // NewAutoInstrumentation is a helper function to create a fully initialized webhook for SSI. Our webhook is made up of
 // several components, but consumers of this webhook should not need to care about how the webhook is wired together.
 // When on-demand instrumentation is enabled and rcClient is non-nil, the mutator also subscribes to remote-config SSI
-// policies (APM_POLICIES), which are layered on top of the configuration baseline at runtime.
+// policies (APM_POLICIES), evaluated after static targets with last-TRUE-wins among RC policies.
 func NewAutoInstrumentation(datadogConfig config.Component, wmeta workloadmeta.Component, serverVersion *version.Info, csiDriverWatcher libraryinjection.CSIDriverWatcher, rcClient *rcclient.Client) (*Webhook, error) {
 	config, err := NewConfig(datadogConfig)
 	if err != nil {

pkg/clusteragent/admission/mutate/autoinstrumentation/policy_matcher.go

@@ -16,8 +16,8 @@ import (
 )
 
 // policyMatcher evaluates SSI policies against pods using the pure Go policy
-// engine. It holds the effective ordered policy set (configuration policies,
-// optionally augmented with remote-config ones) and resolves the first match.
+// engine. The last policy that evaluates to TRUE wins. FALSE and ABSTAIN do
+// not match.
 type policyMatcher struct {
 	policies []policies.Policy
 	wmeta    workloadmeta.Component
@@ -31,8 +31,7 @@ func newPolicyMatcher(ps []policies.Policy, wmeta workloadmeta.Component) *polic
 	}
 }
 
-// Match returns the outcome of the first policy that matches the pod, mirroring
-// the "first match wins" semantics of the target mutator.
+// Match returns the outcome of the last policy that matches the pod.
 func (m *policyMatcher) Match(pod *corev1.Pod) (policies.Outcome, bool) {
 	idx := m.matchIndex(pod)
 	if idx < 0 {
@@ -41,13 +40,13 @@ func (m *policyMatcher) Match(pod *corev1.Pod) (policies.Outcome, bool) {
 	return m.policies[idx].Outcome, true
 }
 
-// matchIndex returns the index of the first policy that matches the pod, or -1
-// if none match. Policies are evaluated in order (first match wins).
+// matchIndex returns the index of the last policy that evaluates to TRUE, or
+// -1 if none match.
 //
 // Namespace labels are fetched lazily when the first policy that needs them is
 // reached. If they cannot be resolved, policies that need namespace labels are
 // skipped while policies using the available pod and namespace-name facts keep
-// their relative first-match ordering.
+// their relative last-match ordering.
 func (m *policyMatcher) matchIndex(pod *corev1.Pod) int {
 	if m == nil || pod == nil {
 		return -1
@@ -59,6 +58,7 @@ func (m *policyMatcher) matchIndex(pod *corev1.Pod) int {
 	}
 	namespaceLabelsLoaded := false
 	namespaceLabelsUnavailable := false
+	matched := -1
 
 	for i := range m.policies {
 		if nodeUsesNamespaceLabels(m.policies[i].Rules) && !namespaceLabelsLoaded {
@@ -78,10 +78,10 @@ func (m *policyMatcher) matchIndex(pod *corev1.Pod) int {
 		}
 
 		if policies.Evaluate(m.policies[i].Rules, ctx) == policies.ResultTrue {
-			return i
+			matched = i
 		}
 	}
-	return -1
+	return matched
 }
 
 func nodeUsesNamespaceLabels(n *policies.Node) bool {

pkg/clusteragent/admission/mutate/autoinstrumentation/policy_matcher_test.go

@@ -44,8 +44,8 @@ func TestPolicyMatcherPodLabels(t *testing.T) {
 	}
 
 	out, ok := m.Match(podWith("any", map[string]string{"app": "db"}))
-	if !ok || out.TracerVersions["java"] != "latest" {
-		t.Fatalf("db pod: got %+v ok=%v", out, ok)
+	if !ok || out.TracerVersions["php"] != "latest" {
+		t.Fatalf("db pod should hit last TRUE (catch-all): got %+v ok=%v", out, ok)
 	}
 
 	out, ok = m.Match(podWith("any", map[string]string{"app": "web"}))

pkg/clusteragent/admission/mutate/autoinstrumentation/rc_policies.go

@@ -56,11 +56,11 @@ func remotePolicyPathOrder(path string) int {
 }
 
 // subscribeRemoteConfig wires the remote-config client to the mutator so that
-// SSI policies delivered over remote config are layered on top of the
-// configuration baseline. It is a no-op when remote config is not available,
-// in which case the mutator keeps matching against its configuration baseline
-// only. The wire format is the dd-wls policies document; targets do not appear
-// on this path.
+// SSI policies delivered over remote config are evaluated after static targets.
+// RC policies are last-TRUE-wins on the wire order (default first, exceptions
+// after). It is a no-op when remote config is not available, in which case the
+// mutator keeps matching against its configuration baseline only. The wire
+// format is the dd-wls policies document; targets do not appear on this path.
 func (m *TargetMutator) subscribeRemoteConfig(client *rcclient.Client) {
 	if client == nil {
 		return

pkg/clusteragent/admission/mutate/autoinstrumentation/rc_policies_test.go

@@ -24,6 +24,12 @@ apm_config:
     enabled: false
 `
 
+const rcSSIOnNoTargets = `
+apm_config:
+  instrumentation:
+    enabled: true
+`
+
 const rcCatchAllCfg = `
 apm_config:
   instrumentation:
@@ -78,74 +84,51 @@ func TestRemotePolicies_AppliedOnEmptyBaseline(t *testing.T) {
 	require.Nil(t, m.getMatchingTarget(rcPod("ns", map[string]string{"app": "other"})))
 }
 
-// TestRemotePolicies_PrecedenceOverConfig verifies that remote policies are
-// evaluated before the configuration baseline (first match wins), while
-// non-matching pods still fall through to the configuration.
-func TestRemotePolicies_PrecedenceOverConfig(t *testing.T) {
+// TestRemotePolicies_HelmCatchAllWinsOverRemote verifies that an explicit static
+// catch-all matches in the static phase, so remote policies never apply.
+func TestRemotePolicies_HelmCatchAllWinsOverRemote(t *testing.T) {
 	wmeta := newMatchTestWmeta(t)
 	m := newMatchMutator(t, rcCatchAllCfg, wmeta)
 
-	// Baseline: the catch-all config target matches everything.
-	name, fromPolicy := matchedTarget(t, m, rcPod("ns", map[string]string{"app": "db"}))
-	require.Equal(t, "config-default", name)
-	require.False(t, fromPolicy)
-
 	require.NoError(t, m.SetRemotePolicies([]policies.Policy{
 		podLabelPolicy("remote", "app", "db", true, map[string]string{"python": "default"}),
+		podLabelPolicy("remote-deny", "app", "legacy", false, nil),
 	}))
 
-	// Remote wins for the matching pod...
-	name, fromPolicy = matchedTarget(t, m, rcPod("ns", map[string]string{"app": "db"}))
-	require.Equal(t, "remote", name)
-	require.True(t, fromPolicy)
-
-	// ...but unrelated pods still fall through to the config baseline.
-	name, fromPolicy = matchedTarget(t, m, rcPod("ns", map[string]string{"app": "other"}))
+	name, fromPolicy := matchedTarget(t, m, rcPod("ns", map[string]string{"app": "db"}))
 	require.Equal(t, "config-default", name)
 	require.False(t, fromPolicy)
-}
 
-// TestRemotePolicies_DenyStopsInjection verifies that a matched deny policy
-// prevents injection even when a later policy (or the config baseline) would
-// otherwise match.
-func TestRemotePolicies_DenyStopsInjection(t *testing.T) {
-	wmeta := newMatchTestWmeta(t)
-	m := newMatchMutator(t, rcCatchAllCfg, wmeta)
-
-	require.NoError(t, m.SetRemotePolicies([]policies.Policy{
-		podLabelPolicy("remote-deny", "app", "legacy", false, nil),
-	}))
-
-	// The deny policy matches first, so no target is returned even though the
-	// config catch-all would otherwise apply.
-	require.Nil(t, m.getMatchingTarget(rcPod("ns", map[string]string{"app": "legacy"})))
-
-	// A non-matching pod still hits the config baseline.
-	name, fromPolicy := matchedTarget(t, m, rcPod("ns", map[string]string{"app": "ok"}))
+	name, fromPolicy = matchedTarget(t, m, rcPod("ns", map[string]string{"app": "legacy"}))
 	require.Equal(t, "config-default", name)
 	require.False(t, fromPolicy)
 }
 
-// TestRemotePolicies_ClearRevertsToBaseline verifies that clearing the remote
-// policies reverts the mutator to its configuration baseline.
+// TestRemotePolicies_ClearRevertsToBaseline verifies that clearing remote
+// policies restores the synthetic inject-all default.
 func TestRemotePolicies_ClearRevertsToBaseline(t *testing.T) {
 	wmeta := newMatchTestWmeta(t)
-	m := newMatchMutator(t, rcCatchAllCfg, wmeta)
+	m := newMatchMutator(t, rcSSIOnNoTargets, wmeta)
+
+	name, fromPolicy := matchedTarget(t, m, rcPod("ns", map[string]string{"app": "db"}))
+	require.Equal(t, "default", name)
+	require.False(t, fromPolicy)
 
 	require.NoError(t, m.SetRemotePolicies([]policies.Policy{
 		podLabelPolicy("remote", "app", "db", true, map[string]string{"python": "default"}),
 	}))
-	name, _ := matchedTarget(t, m, rcPod("ns", map[string]string{"app": "db"}))
+	name, fromPolicy = matchedTarget(t, m, rcPod("ns", map[string]string{"app": "db"}))
 	require.Equal(t, "remote", name)
+	require.True(t, fromPolicy)
 
 	m.ClearRemotePolicies()
-	name, fromPolicy := matchedTarget(t, m, rcPod("ns", map[string]string{"app": "db"}))
-	require.Equal(t, "config-default", name)
+	name, fromPolicy = matchedTarget(t, m, rcPod("ns", map[string]string{"app": "db"}))
+	require.Equal(t, "default", name)
 	require.False(t, fromPolicy)
 }
 
-// TestRemotePolicies_FirstMatchWins verifies the ordering among remote policies.
-func TestRemotePolicies_FirstMatchWins(t *testing.T) {
+// TestRemotePolicies_LastMatchWins verifies last-TRUE-wins among remote policies.
+func TestRemotePolicies_LastMatchWins(t *testing.T) {
 	wmeta := newMatchTestWmeta(t)
 	m := newMatchMutator(t, rcDisabledCfg, wmeta)
 
@@ -155,15 +138,15 @@ func TestRemotePolicies_FirstMatchWins(t *testing.T) {
 	}))
 
 	name, _ := matchedTarget(t, m, rcPod("ns", map[string]string{"app": "db"}))
-	require.Equal(t, "first", name)
+	require.Equal(t, "second", name)
 }
 
 // TestOnRemoteConfigUpdate_ParsesAndApplies exercises the remote-config callback
 // end to end with a dd-wls policies document, then verifies that an empty update
-// reverts the mutator to the configuration baseline.
+// clears remote policies.
 func TestOnRemoteConfigUpdate_ParsesAndApplies(t *testing.T) {
 	wmeta := newMatchTestWmeta(t)
-	m := newMatchMutator(t, rcCatchAllCfg, wmeta)
+	m := newMatchMutator(t, rcDisabledCfg, wmeta)
 
 	const raw = `{
       "policies": [{
@@ -196,11 +179,9 @@ func TestOnRemoteConfigUpdate_ParsesAndApplies(t *testing.T) {
 	require.Equal(t, "java for db-user", name)
 	require.True(t, fromPolicy)
 
-	// An empty update reverts to the config baseline.
+	// An empty update clears remote policies (SSI off → nothing).
 	m.onRemoteConfigUpdate(map[string]state.RawConfig{}, apply)
-	name, fromPolicy = matchedTarget(t, m, rcPod("ns", map[string]string{"app": "db-user"}))
-	require.Equal(t, "config-default", name)
-	require.False(t, fromPolicy)
+	require.Nil(t, m.getMatchingTarget(rcPod("ns", map[string]string{"app": "db-user"})))
 }
 
 func TestOnRemoteConfigUpdate_OrdersPolicyIDsByNumericPrefix(t *testing.T) {
@@ -242,14 +223,15 @@ func TestOnRemoteConfigUpdate_OrdersPolicyIDsByNumericPrefix(t *testing.T) {
 		"datadog/2/APM_POLICIES/2.allow/config": {Config: []byte(allow)},
 	}, func(string, state.ApplyStatus) {})
 
-	set := m.activeSet()
-	require.Len(t, set.matcher.policies, 2)
-	require.Equal(t, "allow", set.matcher.policies[0].Name)
-	require.Equal(t, "deny", set.matcher.policies[1].Name)
+	remotePolicies := m.remotePolicies.Load()
+	require.NotNil(t, remotePolicies)
+	require.Len(t, remotePolicies.matcher.policies, 2)
+	// Numeric prefix sorts 2.allow before 10.deny; last-TRUE-wins then picks deny.
+	require.Equal(t, "allow", remotePolicies.matcher.policies[0].Name)
+	require.Equal(t, "deny", remotePolicies.matcher.policies[1].Name)
 
-	name, fromPolicy := matchedTarget(t, m, rcPod("ns", map[string]string{"app": "db"}))
-	require.Equal(t, "allow", name)
-	require.True(t, fromPolicy)
+	// Last-TRUE-wins: deny is after allow, both match app=db.
+	require.Nil(t, m.getMatchingTarget(rcPod("ns", map[string]string{"app": "db"})))
 }
 
 // TestOnRemoteConfigUpdate_InvalidPayloadKeepsBaseline verifies that one malformed