feature(inject): introduce PatchProducer abstraction (#14175)

This PR introduces the `PatchProducer` abstraction. This allows to abstract away the creation of proxy injection patches and to potentially layer multiple patches on top of each other.

Signed-off-by: Zahari Dichev <[email protected]>

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 := conf.GetPodPatch(rt.injectProxy, rt.overrider)
+	patchJSON, err := inject.ProduceMergedPatch(inject.PatchProducers, conf, 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),
+		injector.Inject(*linkerdNamespace, inject.GetOverriddenValues, inject.PatchProducers),
 		"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) webhook.Handler {
+func Inject(linkerdNamespace string, overrider inject.ValueOverrider, patchProducers []inject.PatchProducer) webhook.Handler {
 	return func(
 		ctx context.Context,
 		api *k8s.MetadataAPI,
@@ -116,10 +116,11 @@ func Inject(linkerdNamespace string, overrider inject.ValueOverrider) webhook.Ha
 				}
 			}
 
-			patchJSON, err := resourceConfig.GetPodPatch(true, overrider)
+			patchJSON, err := inject.ProduceMergedPatch(patchProducers, resourceConfig, true, overrider)
 			if err != nil {
 				return nil, err
 			}
+
 			if parent != nil {
 				recorder.Event(parent, v1.EventTypeNormal, eventTypeInjected, "Linkerd sidecar proxy injected")
 			}

controller/proxy-injector/webhook_test.go

@@ -119,7 +119,7 @@ func TestGetPodPatch(t *testing.T) {
 					t.Fatal(err)
 				}
 
-				patchJSON, err := fullConf.GetPodPatch(true, inject.GetOverriddenValues)
+				patchJSON, err := inject.ProduceMergedPatch(inject.PatchProducers, fullConf, 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 := conf.GetPodPatch(true, inject.GetOverriddenValues)
+		patchJSON, err := inject.ProduceMergedPatch(inject.PatchProducers, conf, 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 := conf.GetPodPatch(true, inject.GetOverriddenValues)
+		patchJSON, err := inject.ProduceMergedPatch(inject.PatchProducers, conf, 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 := conf.GetPodPatch(true, inject.GetOverriddenValues)
+		patchJSON, err := inject.ProduceMergedPatch(inject.PatchProducers, conf, 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 := conf.GetPodPatch(true, inject.GetOverriddenValues)
+		patchJSON, err := inject.ProduceMergedPatch(inject.PatchProducers, conf, 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 := conf.GetPodPatch(true, inject.GetOverriddenValues)
+		patchJSON, err := inject.ProduceMergedPatch(inject.PatchProducers, conf, 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 := conf.GetPodPatch(true, inject.GetOverriddenValues)
+		patchJSON, err := inject.ProduceMergedPatch(inject.PatchProducers, conf, true, inject.GetOverriddenValues)
 		if err != nil {
 			t.Fatalf("Unexpected PatchForAdmissionRequest error: %s", err)
 		}

pkg/inject/inject.go

@@ -8,7 +8,6 @@ import (
 	"errors"
 	"fmt"
 	"html/template"
-	"net"
 	"reflect"
 	"regexp"
 	"sort"
@@ -90,7 +89,19 @@ var (
 	}
 )
 
-type ValueOverrider func(values *l5dcharts.Values, overrides map[string]string, namedPorts map[string]int32) (*l5dcharts.Values, error)
+// 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(values *l5dcharts.Values, overrides map[string]string, namedPorts map[string]int32) (*OverriddenValues, error)
 
 // Origin defines where the input YAML comes from. Refer the ResourceConfig's
 // 'origin' field
@@ -188,15 +199,15 @@ 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(values *l5dcharts.Values, overrides map[string]string, namedPorts map[string]int32) (*l5dcharts.Values, error) {
+func GetOverriddenValues(values *l5dcharts.Values, overrides map[string]string, namedPorts map[string]int32) (*OverriddenValues, error) {
 	// Make a copy of Values and mutate that
 	copyValues, err := values.DeepCopy()
 	if err != nil {
 		return nil, err
 	}
 
 	applyAnnotationOverrides(copyValues, overrides, namedPorts)
-	return copyValues, nil
+	return &OverriddenValues{Values: copyValues}, nil
 }
 
 func applyAnnotationOverrides(values *l5dcharts.Values, annotations map[string]string, namedPorts map[string]int32) {
@@ -663,37 +674,12 @@ 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 (conf *ResourceConfig) GetPodPatch(injectProxy bool, overrider ValueOverrider) ([]byte, error) {
-	namedPorts := make(map[string]int32)
-	if conf.HasPodTemplate() {
-		namedPorts = util.GetNamedPorts(conf.pod.spec.Containers)
-	}
-
-	values, err := overrider(conf.values, conf.getAnnotationOverrides(), namedPorts)
-	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)
-			}
-		}
-	}
-
+func GetPodPatch(conf *ResourceConfig, injectProxy bool, values *OverriddenValues, patchPathPrefix string) ([]JSONPatch, error) {
 	patch := &podPatch{
-		Values:      *values,
+		Values:      *values.Values,
 		Annotations: map[string]string{},
 		Labels:      map[string]string{},
-	}
-	switch strings.ToLower(conf.workload.metaType.Kind) {
-	case k8s.Pod:
-	case k8s.CronJob:
-		patch.PathPrefix = "/spec/jobTemplate/spec/template"
-	default:
-		patch.PathPrefix = "/spec/template"
+		PathPrefix:  patchPathPrefix,
 	}
 
 	if conf.pod.spec != nil {
@@ -734,7 +720,12 @@ func (conf *ResourceConfig) GetPodPatch(injectProxy bool, overrider ValueOverrid
 	// Get rid of invalid trailing commas
 	res := rTrail.ReplaceAll(buf.Bytes(), []byte("}\n]"))
 
-	return res, nil
+	patchResult := []JSONPatch{}
+	if err := json.Unmarshal(res, &patchResult); err != nil {
+		return nil, err
+	}
+
+	return patchResult, nil
 }
 
 // GetConfigAnnotation returns two values. The first value is the annotation

pkg/inject/inject_fuzzer.go

@@ -2,6 +2,7 @@ package inject
 
 import (
 	l5dcharts "github.com/linkerd/linkerd2/pkg/charts/linkerd2"
+	"github.com/linkerd/linkerd2/pkg/util"
 
 	fuzz "github.com/AdaLogics/go-fuzz-headers"
 )
@@ -25,7 +26,18 @@ func FuzzInject(data []byte) int {
 	if err != nil {
 		return 0
 	}
-	_, _ = conf.GetPodPatch(injectProxy, GetOverriddenValues)
+
+	namedPorts := make(map[string]int32)
+	if conf.HasPodTemplate() {
+		namedPorts = util.GetNamedPorts(conf.pod.spec.Containers)
+	}
+
+	values, err := GetOverriddenValues(conf.values, conf.getAnnotationOverrides(), namedPorts)
+	if err != nil {
+		return 0
+	}
+
+	_, _ = GetPodPatch(conf, injectProxy, values, getPatchPathPrefix(conf))
 	_, _ = conf.CreateOpaquePortsPatch()
 
 	report := &Report{}

pkg/inject/inject_test.go

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

pkg/inject/patch.go

@@ -0,0 +1,80 @@
+package inject
+
+import (
+	"encoding/json"
+	"fmt"
+	"net"
+	"strings"
+
+	"github.com/linkerd/linkerd2/pkg/k8s"
+	"github.com/linkerd/linkerd2/pkg/util"
+)
+
+// PatchProducers is the default set of patch producers used to generate Linkerd injection patches.
+var PatchProducers = []PatchProducer{GetPodPatch}
+
+// PatchProducer is a function that generates a patch for a given resource configuration
+// and OverriddenValues.
+type PatchProducer func(conf *ResourceConfig, injectProxy bool, values *OverriddenValues, patchPathPrefix string) ([]JSONPatch, error)
+
+// JSONPatch format is specified in RFC 6902
+type JSONPatch struct {
+	Operation string      `json:"op"`
+	Path      string      `json:"path"`
+	Value     interface{} `json:"value,omitempty"`
+}
+
+func getPatchPathPrefix(conf *ResourceConfig) string {
+	switch strings.ToLower(conf.workload.metaType.Kind) {
+	case k8s.Pod:
+		return ""
+	case k8s.CronJob:
+		return "/spec/jobTemplate/spec/template"
+	default:
+		return "/spec/template"
+	}
+}
+
+// ProduceMergedPatch executes the provided PatchProducers to generate a merged JSON patch that combines
+// all generated patches.
+func ProduceMergedPatch(producers []PatchProducer, conf *ResourceConfig, injectProxy bool, overrider ValueOverrider) ([]byte, error) {
+	namedPorts := make(map[string]int32)
+	if conf.HasPodTemplate() {
+		namedPorts = util.GetNamedPorts(conf.pod.spec.Containers)
+	}
+
+	values, err := overrider(conf.values, conf.getAnnotationOverrides(), namedPorts)
+	if err != nil {
+		return nil, fmt.Errorf("could not generate Overridden Values: %w", err)
+	}
+
+	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)
+			}
+		}
+	}
+
+	merged := []JSONPatch{}
+	for _, producer := range producers {
+		patch, err := producer(conf, injectProxy, values, getPatchPathPrefix(conf))
+		if err != nil {
+			return nil, err
+		}
+
+		// If the patch is empty, skip it
+		if len(patch) == 0 {
+			continue
+		}
+
+		merged = append(merged, patch...)
+	}
+
+	return json.Marshal(merged)
+}