chore(inject): remove PatchProducer abstraction (#14346)

In a recent change, we introduced the `PatchProducer` abstraction in order to provide a way to add customizable injection logic to the injector. After some further experimentation, it has became clear that there might be better ways to customize the injection logic. 

For now, we remove this abstraction as we are not going to be using it in the near future.

Signed-off-by: Zahari Dichev <zaharidichev@gmail.com>

Author: zaharidichev

cli/cmd/inject.go

@@ -207,7 +207,7 @@ func (rt resourceTransformerInject) transform(bytes []byte) ([]byte, []inject.Re
 		conf.AppendPodAnnotation(k8s.ProxyInjectAnnotation, k8s.ProxyInjectEnabled)
 	}
 
-	patchJSON, err := inject.ProduceMergedPatch(inject.PatchProducers, conf, rt.injectProxy, rt.overrider)
+	patchJSON, err := conf.GetPodPatch(rt.injectProxy, rt.overrider)
 	if err != nil {
 		return nil, nil, err
 	}

controller/cmd/proxy-injector/main.go

@@ -25,7 +25,7 @@ func Main(args []string) {
 	webhook.Launch(
 		context.Background(),
 		[]k8s.APIResource{k8s.NS, k8s.Deploy, k8s.RC, k8s.RS, k8s.Job, k8s.DS, k8s.SS, k8s.Pod, k8s.CJ},
-		injector.Inject(*linkerdNamespace, inject.GetOverriddenValues, inject.PatchProducers),
+		injector.Inject(*linkerdNamespace, inject.GetOverriddenValues),
 		"linkerd-proxy-injector",
 		*metricsAddr,
 		*addr,

controller/proxy-injector/webhook.go

@@ -28,7 +28,7 @@ const (
 // Inject returns the function that produces an AdmissionResponse containing
 // the patch, if any, to apply to the pod (proxy sidecar and eventually the
 // init container to set it up)
-func Inject(linkerdNamespace string, overrider inject.ValueOverrider, patchProducers []inject.PatchProducer) webhook.Handler {
+func Inject(linkerdNamespace string, overrider inject.ValueOverrider) webhook.Handler {
 	return func(
 		ctx context.Context,
 		api *k8s.MetadataAPI,
@@ -116,7 +116,7 @@ func Inject(linkerdNamespace string, overrider inject.ValueOverrider, patchProdu
 				}
 			}
 
-			patchJSON, err := inject.ProduceMergedPatch(patchProducers, resourceConfig, true, overrider)
+			patchJSON, err := resourceConfig.GetPodPatch(true, overrider)
 			if err != nil {
 				return nil, err
 			}

controller/proxy-injector/webhook_test.go

@@ -119,7 +119,7 @@ func TestGetPodPatch(t *testing.T) {
 					t.Fatal(err)
 				}
 
-				patchJSON, err := inject.ProduceMergedPatch(inject.PatchProducers, fullConf, true, inject.GetOverriddenValues)
+				patchJSON, err := fullConf.GetPodPatch(true, inject.GetOverriddenValues)
 				if err != nil {
 					t.Fatalf("Unexpected PatchForAdmissionRequest error: %s", err)
 				}
@@ -142,7 +142,7 @@ func TestGetPodPatch(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		patchJSON, err := inject.ProduceMergedPatch(inject.PatchProducers, conf, true, inject.GetOverriddenValues)
+		patchJSON, err := conf.GetPodPatch(true, inject.GetOverriddenValues)
 		if err != nil {
 			t.Fatalf("Unexpected PatchForAdmissionRequest error: %s", err)
 		}
@@ -163,7 +163,7 @@ func TestGetPodPatch(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		patchJSON, err := inject.ProduceMergedPatch(inject.PatchProducers, conf, true, inject.GetOverriddenValues)
+		patchJSON, err := conf.GetPodPatch(true, inject.GetOverriddenValues)
 		if err != nil {
 			t.Fatalf("Unexpected PatchForAdmissionRequest error: %s", err)
 		}
@@ -186,7 +186,7 @@ func TestGetPodPatch(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		patchJSON, err := inject.ProduceMergedPatch(inject.PatchProducers, conf, true, inject.GetOverriddenValues)
+		patchJSON, err := conf.GetPodPatch(true, inject.GetOverriddenValues)
 		if err != nil {
 			t.Fatalf("Unexpected PatchForAdmissionRequest error: %s", err)
 		}
@@ -210,7 +210,7 @@ func TestGetPodPatch(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		patchJSON, err := inject.ProduceMergedPatch(inject.PatchProducers, conf, true, inject.GetOverriddenValues)
+		patchJSON, err := conf.GetPodPatch(true, inject.GetOverriddenValues)
 		if err != nil {
 			t.Fatalf("Unexpected PatchForAdmissionRequest error: %s", err)
 		}
@@ -240,7 +240,7 @@ func TestGetPodPatch(t *testing.T) {
 		// The namespace has two config annotations: one valid and one invalid
 		// the pod patch should only contain the valid annotation.
 		inject.AppendNamespaceAnnotations(conf.GetOverrideAnnotations(), conf.GetNsAnnotations(), conf.GetWorkloadAnnotations())
-		patchJSON, err := inject.ProduceMergedPatch(inject.PatchProducers, conf, true, inject.GetOverriddenValues)
+		patchJSON, err := conf.GetPodPatch(true, inject.GetOverriddenValues)
 		if err != nil {
 			t.Fatalf("Unexpected PatchForAdmissionRequest error: %s", err)
 		}
@@ -254,7 +254,7 @@ func TestGetPodPatch(t *testing.T) {
 		deployment := fileContents(factory, t, "deployment-with-injected-proxy.yaml")
 		fakeReq := getFakePodReq(deployment)
 		conf := confNsDisabled().WithKind(fakeReq.Kind.Kind)
-		patchJSON, err := inject.ProduceMergedPatch(inject.PatchProducers, conf, true, inject.GetOverriddenValues)
+		patchJSON, err := conf.GetPodPatch(true, inject.GetOverriddenValues)
 		if err != nil {
 			t.Fatalf("Unexpected PatchForAdmissionRequest error: %s", err)
 		}

pkg/inject/inject.go

@@ -8,6 +8,7 @@ import (
 	"errors"
 	"fmt"
 	"html/template"
+	"net"
 	"reflect"
 	"regexp"
 	"sort"
@@ -89,19 +90,9 @@ var (
 	}
 )
 
-// OverriddenValues contains the result of executing an instance of ValueOverrider
-type OverriddenValues struct {
-	*l5dcharts.Values
-
-	// may contain additional values that are not part of the l5dcharts.Values
-	// in order to allow custom ValueOverrider implementations to add their own
-	// values to the rendering logic.
-	Additional map[string]interface{}
-}
-
 // ValueOverrider is used to override the default values that are used in chart rendering based
 // on the annotations provided in overrides.
-type ValueOverrider func(rc *ResourceConfig) (*OverriddenValues, error)
+type ValueOverrider func(rc *ResourceConfig) (*l5dcharts.Values, error)
 
 // Origin defines where the input YAML comes from. Refer the ResourceConfig's
 // 'origin' field
@@ -199,7 +190,7 @@ func AppendNamespaceAnnotations(base map[string]string, nsAnn map[string]string,
 
 // GetOverriddenValues returns the final Values struct which is created
 // by overriding annotated configuration on top of default Values
-func GetOverriddenValues(rc *ResourceConfig) (*OverriddenValues, error) {
+func GetOverriddenValues(rc *ResourceConfig) (*l5dcharts.Values, error) {
 	// Make a copy of Values and mutate that
 	copyValues, err := rc.GetValues().DeepCopy()
 	if err != nil {
@@ -212,7 +203,7 @@ func GetOverriddenValues(rc *ResourceConfig) (*OverriddenValues, error) {
 	}
 
 	ApplyAnnotationOverrides(copyValues, rc.GetAnnotationOverrides(), namedPorts)
-	return &OverriddenValues{Values: copyValues}, nil
+	return copyValues, nil
 }
 
 func ApplyAnnotationOverrides(values *l5dcharts.Values, annotations map[string]string, namedPorts map[string]int32) {
@@ -686,12 +677,32 @@ func (conf *ResourceConfig) GetAnnotationOverrides() map[string]string {
 
 // GetPodPatch returns the JSON patch containing the proxy and init containers specs, if any.
 // If injectProxy is false, only the config.linkerd.io annotations are set.
-func GetPodPatch(conf *ResourceConfig, injectProxy bool, values *OverriddenValues, patchPathPrefix string) ([]JSONPatch, error) {
+func (conf *ResourceConfig) GetPodPatch(injectProxy bool, overrider ValueOverrider) ([]byte, error) {
+	values, err := overrider(conf)
+	values.Proxy.PodInboundPorts = getPodInboundPorts(conf.pod.spec)
+	if err != nil {
+		return nil, fmt.Errorf("could not generate Overridden Values: %w", err)
+	}
+
+	if values.ClusterNetworks != "" {
+		for _, network := range strings.Split(strings.Trim(values.ClusterNetworks, ","), ",") {
+			if _, _, err := net.ParseCIDR(network); err != nil {
+				return nil, fmt.Errorf("cannot parse destination get networks: %w", err)
+			}
+		}
+	}
+
 	patch := &podPatch{
-		Values:      *values.Values,
+		Values:      *values,
 		Annotations: map[string]string{},
 		Labels:      map[string]string{},
-		PathPrefix:  patchPathPrefix,
+	}
+	switch strings.ToLower(conf.workload.metaType.Kind) {
+	case k8s.Pod:
+	case k8s.CronJob:
+		patch.PathPrefix = "/spec/jobTemplate/spec/template"
+	default:
+		patch.PathPrefix = "/spec/template"
 	}
 
 	if conf.pod.spec != nil {
@@ -732,12 +743,7 @@ func GetPodPatch(conf *ResourceConfig, injectProxy bool, values *OverriddenValue
 	// Get rid of invalid trailing commas
 	res := rTrail.ReplaceAll(buf.Bytes(), []byte("}\n]"))
 
-	patchResult := []JSONPatch{}
-	if err := json.Unmarshal(res, &patchResult); err != nil {
-		return nil, err
-	}
-
-	return patchResult, nil
+	return res, nil
 }
 
 // GetConfigAnnotation returns two values. The first value is the annotation

pkg/inject/inject_fuzzer.go

@@ -26,12 +26,7 @@ func FuzzInject(data []byte) int {
 		return 0
 	}
 
-	values, err := GetOverriddenValues(conf)
-	if err != nil {
-		return 0
-	}
-
-	_, _ = GetPodPatch(conf, injectProxy, values, getPatchPathPrefix(conf))
+	_, _ = conf.GetPodPatch(injectProxy, GetOverriddenValues)
 	_, _ = conf.CreateOpaquePortsPatch()
 
 	report := &Report{}

pkg/inject/inject_test.go

@@ -330,7 +330,7 @@ func TestGetOverriddenValues(t *testing.T) {
 				t.Fatal(err)
 			}
 			expected := testCase.expected()
-			if diff := deep.Equal(actual.Values, expected); diff != nil {
+			if diff := deep.Equal(actual, expected); diff != nil {
 				t.Errorf("%+v", diff)
 			}
 		})