fix(ssi): merge target and namespace implementations (#42068)

### What does this PR do?
When we introduced targets, we created a separate code path to handle the target based configuration. However, the namespace based configuration can also be represented and supported by the target implementation. This commit merges the two implementations.

When I merged these in code, I found bugs in the target implementation surrounding [Local SDK Injection](https://docs.datadoghq.com/tracing/guide/local_sdk_injection/) and how it was handled with targets defined. This change fixes those bugs to match the expectations around enabledNamespaces.

### Motivation
We want to reduce the complexity of the auto instrumentation webhook. There is no need to have two implementations to support our customer facing configuration options and as shown with this change, two implementations open up more chances for bugs.

See [SSI Kubernetes | Platform Stability](https://docs.google.com/document/d/1NqrPEUn3RfcdS_hQUQB-pJFJN9N-x5I202CB7s7CnwQ/edit?usp=sharing) for more details on all changes related to cleanup.

### Describe how you validated your changes
I tested this change heavily using [injector-dev](https://github.com/DataDog/injector-dev). For each scenario listed, I ran with and without the `--build` flag to use the latest agent release vs this branch to ensure there was a seamless migration path.

<details>
<summary>Broken annotation based injection with target list</summary>

This should have injection but it does not currently:

```yaml
helm:
  apps:
    - name: annotation-example
      namespace: application
      values:
        env:
          - name: DD_TRACE_DEBUG
            value: "true"
          - name: DD_APM_INSTRUMENTATION_DEBUG
            value: "true"
        image:
          repository: registry.ddbuild.io/ci/injector-dev/python
          tag: 2cd78ded
        podLabels:
          admission.datadoghq.com/enabled: "true"
          tags.datadoghq.com/env: local
        podAnnotations:
          admission.datadoghq.com/python-lib.version: "v3"
        service:
          port: "8080"
  versions:
    agent: 7.71.1
    cluster_agent:
      version: 7.71.1
      build: {}
    injector: 0.48.0
  config:
    datadog:
      apm:
        instrumentation:
          enabled: false
          targets:
            - name: python
              podSelector:
                matchLabels:
                  language: python
              ddTraceVersions:
                python: "3"
```

 </details>

<details>
<summary>Annotation based injection</summary>

We expect injection:
```yaml
helm:
  apps:
    - name: annotation-example
      namespace: application
      values:
        env:
          - name: DD_TRACE_DEBUG
            value: "true"
          - name: DD_APM_INSTRUMENTATION_DEBUG
            value: "true"
        image:
          repository: registry.ddbuild.io/ci/injector-dev/python
          tag: 2cd78ded
        podLabels:
          admission.datadoghq.com/enabled: "true"
          tags.datadoghq.com/env: local
        podAnnotations:
          admission.datadoghq.com/python-lib.version: "v3"
        service:
          port: "8080"
  versions:
    agent: 7.71.1
    cluster_agent:
      version: 7.71.1
      build: {}
    injector: 0.48.0
  config:
    datadog:
      apm:
        instrumentation:
          enabled: false
```

 </details>

<details>
<summary>Namespace based selection</summary>

We expect injection:
```yaml
helm:
  apps:
    - name: namespace-selection-example
      namespace: application
      values:
        env:
          - name: DD_TRACE_DEBUG
            value: "true"
          - name: DD_APM_INSTRUMENTATION_DEBUG
            value: "true"
        image:
          repository: registry.ddbuild.io/ci/injector-dev/python
          tag: 2cd78ded
        podLabels:
          tags.datadoghq.com/env: local
        service:
          port: "8080"
  versions:
    agent: 7.71.1
    cluster_agent:
      version: 7.71.1
      build: {}
    injector: 0.48.0
  config:
    datadog:
      apm:
        instrumentation:
          enabled: true
          enabledNamespaces:
            - "application"
          libVersions:
            python: "3"
```

 </details>

<details>
<summary>Workload selection</summary>

We expect injection:
```yaml
helm:
  apps:
    - name: workload-selection-example
      namespace: application
      values:
        env:
          - name: DD_TRACE_DEBUG
            value: "true"
          - name: DD_APM_INSTRUMENTATION_DEBUG
            value: "true"
        image:
          repository: registry.ddbuild.io/ci/injector-dev/python
          tag: 2cd78ded
        podLabels:
          language: python
          tags.datadoghq.com/env: local
        service:
          port: "8080"
  versions:
    agent: 7.71.1
    cluster_agent:
      version: 7.71.1
      build: {}
    injector: 0.48.0
  config:
    datadog:
      apm:
        instrumentation:
          enabled: true
          targets:
            - name: python
              podSelector:
                matchLabels:
                  language: python
              ddTraceVersions:
                python: "3"
```
 </details>

<details>
<summary>Instrumentation disabled</summary>

We expect no injection to occur:
```yaml
helm:
  apps:
    - name: disabled-example
      namespace: application
      values:
        env:
          - name: DD_TRACE_DEBUG
            value: "true"
          - name: DD_APM_INSTRUMENTATION_DEBUG
            value: "true"
        image:
          repository: registry.ddbuild.io/ci/injector-dev/python
          tag: 2cd78ded
        podLabels:
          tags.datadoghq.com/env: local
        service:
          port: "8080"
  versions:
    agent: 7.71.1
    cluster_agent:
      version: 7.71.1
      build: {}
    injector: 0.48.0
  config:
    datadog:
      apm:
        instrumentation:
          enabled: false
```
 </details>


### Additional Notes
This change opted to not change the behavior of any tests and to keep test changes minimal. A follow up PR will rewrite some of these tests to test the webhook behavior more directly.

Co-authored-by: stanistan <[email protected]>
Co-authored-by: mark.spicer <[email protected]>

Author: betterengineering

pkg/clusteragent/admission/controllers/webhook/controller_base.go

@@ -189,7 +189,7 @@ func generateAutoInstrumentationWebhook(wmeta workloadmeta.Component, datadogCon
 		return nil, fmt.Errorf("failed to create auto instrumentation config: %v", err)
 	}
 
-	apm, err := autoinstrumentation.NewMutatorWithFilter(config, wmeta, imageResolver)
+	apm, err := autoinstrumentation.NewTargetMutator(config, wmeta, imageResolver)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create auto instrumentation namespace mutator: %v", err)
 	}

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

@@ -218,10 +218,6 @@ func (s libInfoSource) injectionType() string {
 	}
 }
 
-func (s libInfoSource) isSingleStep() bool {
-	return s.injectionType() == singleStepInstrumentationInstallType
-}
-
 // isFromLanguageDetection tells us whether this source comes from
 // the language detection reporting and annotation.
 func (s libInfoSource) isFromLanguageDetection() bool {

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

@@ -447,7 +447,7 @@ func TestInjectAutoInstruConfigV2(t *testing.T) {
 				tt.expectedInstallType = "k8s_single_step"
 			}
 
-			mutator, err := NewNamespaceMutator(config, wmeta, imageResolver)
+			mutator, err := NewTargetMutator(config, wmeta, imageResolver)
 			require.NoError(t, err)
 
 			err = mutator.core.injectTracers(tt.pod, tt.libInfo)
@@ -581,7 +581,7 @@ func TestMutatorCoreNewInjector(t *testing.T) {
 	)
 	config, err := NewConfig(mockConfig)
 	require.NoError(t, err)
-	m, err := NewNamespaceMutator(config, wmeta, imageResolver)
+	m, err := NewTargetMutator(config, wmeta, imageResolver)
 	require.NoError(t, err)
 	core := m.core
 
@@ -618,14 +618,12 @@ func TestExtractLibInfo(t *testing.T) {
 
 	var mockConfig model.Config
 	tests := []struct {
-		name                   string
-		pod                    *corev1.Pod
-		deployments            []common.MockDeployment
-		assertExtractedLibInfo func(*testing.T, extractedPodLibInfo)
-		containerRegistry      string
-		expectedLibsToInject   []libInfo
-		expectedPodEligible    *bool
-		setupConfig            func()
+		name                 string
+		pod                  *corev1.Pod
+		containerRegistry    string
+		expectedLibsToInject []libInfo
+		expectedPodEligible  *bool
+		setupConfig          func()
 	}{
 		{
 			name:              "java",
@@ -669,13 +667,11 @@ func TestExtractLibInfo(t *testing.T) {
 			},
 		},
 		{
-			name:                "python with unlabelled injection off",
-			pod:                 common.FakePodWithAnnotation("admission.datadoghq.com/python-lib.version", "v1"),
-			containerRegistry:   "registry",
-			expectedPodEligible: pointer.Ptr(false),
-			expectedLibsToInject: []libInfo{
-				defaultLibInfoWithVersion(python, "v1"),
-			},
+			name:                 "python with unlabelled injection off",
+			pod:                  common.FakePodWithAnnotation("admission.datadoghq.com/python-lib.version", "v1"),
+			containerRegistry:    "registry",
+			expectedPodEligible:  pointer.Ptr(false),
+			expectedLibsToInject: []libInfo{},
 			setupConfig: func() {
 				mockConfig.SetWithoutSource("admission_controller.mutate_unlabelled", false)
 			},
@@ -756,7 +752,7 @@ func TestExtractLibInfo(t *testing.T) {
 			pod:                  common.FakePodWithAnnotation("admission.datadoghq.com/all-lib.version", "latest"),
 			containerRegistry:    "registry",
 			expectedPodEligible:  pointer.Ptr(false),
-			expectedLibsToInject: allLatestDefaultLibs,
+			expectedLibsToInject: []libInfo{},
 			setupConfig: func() {
 				mockConfig.SetWithoutSource("admission_controller.mutate_unlabelled", false)
 			},
@@ -780,16 +776,6 @@ func TestExtractLibInfo(t *testing.T) {
 				mockConfig.SetWithoutSource("admission_controller.mutate_unlabelled", false)
 			},
 		},
-		{
-			name:                 "all with mutate_unlabelled off",
-			pod:                  common.FakePodWithAnnotation("admission.datadoghq.com/all-lib.version", "latest"),
-			containerRegistry:    "registry",
-			expectedPodEligible:  pointer.Ptr(false),
-			expectedLibsToInject: allLatestDefaultLibs,
-			setupConfig: func() {
-				mockConfig.SetWithoutSource("admission_controller.mutate_unlabelled", false)
-			},
-		},
 		{
 			name: "all with mutate_unlabelled off, but labelled admission enabled",
 			pod: &corev1.Pod{
@@ -810,32 +796,41 @@ func TestExtractLibInfo(t *testing.T) {
 			},
 		},
 		{
-			name:                 "all with mutate_unlabelled off",
-			pod:                  common.FakePodWithAnnotation("admission.datadoghq.com/all-lib.version", "latest"),
+			name: "all with mutate_unlabelled off, but labelled admission enabled",
+			pod: &corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						"admission.datadoghq.com/all-lib.version": "latest",
+					},
+					Labels: map[string]string{
+						"admission.datadoghq.com/enabled": "true",
+					},
+				},
+			},
 			containerRegistry:    "registry",
-			expectedPodEligible:  pointer.Ptr(false),
+			expectedPodEligible:  pointer.Ptr(true),
 			expectedLibsToInject: allLatestDefaultLibs,
 			setupConfig: func() {
 				mockConfig.SetWithoutSource("admission_controller.mutate_unlabelled", false)
 			},
 		},
 		{
-			name: "all with mutate_unlabelled off, but labelled admission enabled",
+			name: "java with mutate_unlabelled on and labelled admission disabled",
 			pod: &corev1.Pod{
 				ObjectMeta: metav1.ObjectMeta{
 					Annotations: map[string]string{
-						"admission.datadoghq.com/all-lib.version": "latest",
+						"admission.datadoghq.com/java-lib.version": "v1",
 					},
 					Labels: map[string]string{
-						"admission.datadoghq.com/enabled": "true",
+						"admission.datadoghq.com/enabled": "false",
 					},
 				},
 			},
 			containerRegistry:    "registry",
-			expectedPodEligible:  pointer.Ptr(true),
-			expectedLibsToInject: allLatestDefaultLibs,
+			expectedPodEligible:  pointer.Ptr(false),
+			expectedLibsToInject: []libInfo{},
 			setupConfig: func() {
-				mockConfig.SetWithoutSource("admission_controller.mutate_unlabelled", false)
+				mockConfig.SetWithoutSource("admission_controller.mutate_unlabelled", true)
 			},
 		},
 		{
@@ -917,74 +912,6 @@ func TestExtractLibInfo(t *testing.T) {
 				mockConfig.SetWithoutSource("admission_controller.mutate_unlabelled", true)
 			},
 		},
-		{
-			name: "pod with lang-detection deployment and default libs",
-			pod: common.FakePodSpec{
-				ParentKind: "replicaset",
-				ParentName: "deployment-123",
-			}.Create(),
-			deployments: []common.MockDeployment{
-				{
-					ContainerName:  "pod",
-					DeploymentName: "deployment",
-					Namespace:      "ns",
-					Languages:      languageSetOf("python"),
-				},
-			},
-			containerRegistry: "registry",
-			assertExtractedLibInfo: func(t *testing.T, i extractedPodLibInfo) {
-				t.Helper()
-				require.Equal(t, &libInfoLanguageDetection{
-					libs: []libInfo{
-						python.defaultLibInfo("registry", "pod"),
-					},
-					injectionEnabled: true,
-				}, i.languageDetection)
-			},
-			expectedLibsToInject: []libInfo{
-				python.defaultLibInfo("registry", "pod"),
-			},
-			setupConfig: func() {
-				mockConfig.SetWithoutSource("apm_config.instrumentation.enabled", true)
-				mockConfig.SetWithoutSource("apm_config.instrumentation.lib_versions", defaultLibraries)
-				mockConfig.SetWithoutSource("language_detection.enabled", true)
-				mockConfig.SetWithoutSource("language_detection.reporting.enabled", true)
-				mockConfig.SetWithoutSource("admission_controller.auto_instrumentation.inject_auto_detected_libraries", true)
-			},
-		},
-		{
-			name: "pod with lang-detection deployment and libs set",
-			pod: common.FakePodSpec{
-				ParentKind: "replicaset",
-				ParentName: "deployment-123",
-			}.Create(),
-			deployments: []common.MockDeployment{
-				{
-					ContainerName:  "pod",
-					DeploymentName: "deployment",
-					Namespace:      "ns",
-					Languages:      languageSetOf("python"),
-				},
-			},
-			containerRegistry: "registry",
-			assertExtractedLibInfo: func(t *testing.T, i extractedPodLibInfo) {
-				t.Helper()
-				require.Equal(t, &libInfoLanguageDetection{
-					libs:             []libInfo{python.defaultLibInfo("registry", "pod")},
-					injectionEnabled: true,
-				}, i.languageDetection)
-			},
-			expectedLibsToInject: []libInfo{
-				java.defaultLibInfo("registry", ""),
-			},
-			setupConfig: func() {
-				mockConfig.SetWithoutSource("apm_config.instrumentation.enabled", true)
-				mockConfig.SetWithoutSource("apm_config.instrumentation.lib_versions", defaultLibrariesFor("java"))
-				mockConfig.SetWithoutSource("language_detection.enabled", true)
-				mockConfig.SetWithoutSource("language_detection.reporting.enabled", true)
-				mockConfig.SetWithoutSource("admission_controller.auto_instrumentation.inject_auto_detected_libraries", true)
-			},
-		},
 		{
 			name:              "php",
 			pod:               common.FakePodWithAnnotation("admission.datadoghq.com/php-lib.version", "v1"),
@@ -1008,7 +935,7 @@ func TestExtractLibInfo(t *testing.T) {
 				mockConfig.SetWithoutSource(k, v)
 			}
 
-			wmeta := common.FakeStoreWithDeployment(t, tt.deployments)
+			wmeta := common.FakeStoreWithDeployment(t, nil)
 			mockConfig = configmock.New(t)
 			for k, v := range overrides {
 				mockConfig.SetWithoutSource(k, v)
@@ -1020,18 +947,19 @@ func TestExtractLibInfo(t *testing.T) {
 
 			config, err := NewConfig(mockConfig)
 			require.NoError(t, err)
-			mutator, err := NewNamespaceMutator(config, wmeta, imageResolver)
+			mutator, err := NewTargetMutator(config, wmeta, imageResolver)
 			require.NoError(t, err)
 
 			if tt.expectedPodEligible != nil {
-				require.Equal(t, *tt.expectedPodEligible, mutator.isPodEligible(tt.pod))
+				require.Equal(t, *tt.expectedPodEligible, mutator.ShouldMutatePod(tt.pod))
 			}
 
-			extracted := mutator.extractLibInfo(tt.pod)
-			if tt.assertExtractedLibInfo != nil {
-				tt.assertExtractedLibInfo(t, extracted)
+			libVersions := []libInfo{}
+			internalTarget := mutator.getTarget(tt.pod)
+			if internalTarget != nil {
+				libVersions = internalTarget.libVersions
 			}
-			require.ElementsMatch(t, tt.expectedLibsToInject, extracted.libs)
+			require.ElementsMatch(t, tt.expectedLibsToInject, libVersions)
 		})
 	}
 }
@@ -1743,7 +1671,7 @@ func TestInjectLibInitContainer(t *testing.T) {
 			// N.B. this is a bit hacky but consistent.
 			config.initSecurityContext = tt.secCtx
 
-			mutator, err := NewNamespaceMutator(config, wmeta, imageResolver)
+			mutator, err := NewTargetMutator(config, wmeta, imageResolver)
 			require.NoError(t, err)
 
 			c := tt.lang.libInfo("", tt.image).initContainers(imageResolver)[0]
@@ -2017,14 +1945,20 @@ func TestShouldInject(t *testing.T) {
 				workloadmetafxmock.MockModule(workloadmeta.NewParams()),
 			)
 
+			// Explicitly adding an annotation because we are testing the enabled label, mutateUnlabelled, and the
+			// interactions with apm enabled. Users need an explicit annotation for local library injection to work.
+			tt.pod.Annotations = map[string]string{
+				fmt.Sprintf(customLibAnnotationKeyFormat, "java"): "v1",
+			}
+
 			mockConfig = configmock.New(t)
 			tt.setupConfig()
 
 			config, err := NewConfig(mockConfig)
 			require.NoError(t, err)
-			mutator, err := NewNamespaceMutator(config, wmeta, imageResolver)
+			mutator, err := NewTargetMutator(config, wmeta, imageResolver)
 			require.NoError(t, err)
-			require.Equal(t, tt.want, mutator.isPodEligible(tt.pod), "expected webhook.isPodEligible() to be %t", tt.want)
+			require.Equal(t, tt.want, mutator.ShouldMutatePod(tt.pod), "expected webhook.isPodEligible() to be %t", tt.want)
 		})
 	}
 }

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

@@ -38,6 +38,9 @@ type Config struct {
 	// containerRegistry is the container registry to use for the autoinstrumentation logic
 	containerRegistry string
 
+	// mutateUnlabelled is used to control if we require workloads to have a label when using Local Lib Injection.
+	mutateUnlabelled bool
+
 	// precomputed containerMutators for the security and profiling products
 	securityClientLibraryMutator  containerMutator
 	profilingClientLibraryMutator containerMutator
@@ -106,11 +109,14 @@ func NewConfig(datadogConfig config.Component) (*Config, error) {
 	}
 
 	containerRegistry := mutatecommon.ContainerRegistry(datadogConfig, "admission_controller.auto_instrumentation.container_registry")
+	mutateUnlabelled := datadogConfig.GetBool("admission_controller.mutate_unlabelled")
+
 	return &Config{
 		Webhook:                       NewWebhookConfig(datadogConfig),
 		LanguageDetection:             NewLanguageDetectionConfig(datadogConfig),
 		Instrumentation:               instrumentationConfig,
 		containerRegistry:             containerRegistry,
+		mutateUnlabelled:              mutateUnlabelled,
 		initResources:                 initResources,
 		initSecurityContext:           initSecurityContext,
 		defaultResourceRequirements:   defaultResourceRequirements,

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

@@ -30,10 +30,10 @@ func TestNewInstrumentationConfig(t *testing.T) {
 			shouldErr:  false,
 			expected: &InstrumentationConfig{
 				Enabled:            true,
-				EnabledNamespaces:  []string{"default"},
+				EnabledNamespaces:  []string{"application"},
 				DisabledNamespaces: []string{},
 				LibVersions: map[string]string{
-					"python": "default",
+					"python": "v3",
 				},
 				InjectorImageTag: "foo",
 			},