Convert CLI inject proxy options to annotations (#2547)

* Include the DisableExternalProfile option even if it's 'false'. The override logic depends on this option to assign different profile suffix.
* Check for proxy and init image overrides even when registry option is empty
* Append the config annotations to the pod's meta before creating the patch. This ensures that any configs provided via the CLI options are persisted as annotations before the configs override.
* Persist linkerd version CLI option

Signed-off-by: Ivan Sim <ivan@buoyant.io>

cli/cmd/inject.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"strconv"
 	"strings"
 
 	jsonpatch "github.com/evanphx/json-patch"
@@ -34,15 +35,13 @@ type injectOptions struct {
 }
 
 type resourceTransformerInject struct {
-	configs *config.All
+	configs               *config.All
-
+	overrideAnnotations   map[string]string
 	proxyOutboundCapacity map[string]uint
 }
 
-func runInjectCmd(inputs []io.Reader, errWriter, outWriter io.Writer, conf *config.All) int {
+func runInjectCmd(inputs []io.Reader, errWriter, outWriter io.Writer, transformer *resourceTransformerInject) int {
-	return transformInput(inputs, errWriter, outWriter, resourceTransformerInject{
+	return transformInput(inputs, errWriter, outWriter, transformer)
-		configs: conf,
-	})
 }
 
 func newInjectOptions() *injectOptions {
@@ -89,9 +88,14 @@ sub-folders, or coming from stdin.`,
 			if err != nil {
 				return err
 			}
+			overrideAnnotations := map[string]string{}
+			options.overrideConfigs(configs, overrideAnnotations)
 
-			options.overrideConfigs(configs)
+			transformer := &resourceTransformerInject{
-			exitCode := uninjectAndInject(in, stderr, stdout, configs)
+				configs:             configs,
+				overrideAnnotations: overrideAnnotations,
+			}
+			exitCode := uninjectAndInject(in, stderr, stdout, transformer)
 			os.Exit(exitCode)
 			return nil
 		},
@@ -106,12 +110,12 @@ sub-folders, or coming from stdin.`,
 	return cmd
 }
 
-func uninjectAndInject(inputs []io.Reader, errWriter, outWriter io.Writer, conf *config.All) int {
+func uninjectAndInject(inputs []io.Reader, errWriter, outWriter io.Writer, transformer *resourceTransformerInject) int {
 	var out bytes.Buffer
-	if exitCode := runUninjectSilentCmd(inputs, errWriter, &out, conf); exitCode != 0 {
+	if exitCode := runUninjectSilentCmd(inputs, errWriter, &out, transformer.configs); exitCode != 0 {
 		return exitCode
 	}
-	return runInjectCmd([]io.Reader{&out}, errWriter, outWriter, conf)
+	return runInjectCmd([]io.Reader{&out}, errWriter, outWriter, transformer)
 }
 
 func (rt resourceTransformerInject) transform(bytes []byte) ([]byte, []inject.Report, error) {
@@ -127,6 +131,14 @@ func (rt resourceTransformerInject) transform(bytes []byte) ([]byte, []inject.Re
 		r := inject.Report{UnsupportedResource: true}
 		return bytes, []inject.Report{r}, nil
 	}
+
+	conf.AppendPodAnnotations(map[string]string{
+		k8s.CreatedByAnnotation: k8s.CreatedByAnnotationValue(),
+	})
+	if len(rt.overrideAnnotations) > 0 {
+		conf.AppendPodAnnotations(rt.overrideAnnotations)
+	}
+
 	p, reports, err := conf.GetPatch(bytes, inject.ShouldInjectCLI)
 	if err != nil {
 		return nil, nil, err
@@ -134,7 +146,7 @@ func (rt resourceTransformerInject) transform(bytes []byte) ([]byte, []inject.Re
 	if p.IsEmpty() {
 		return bytes, reports, nil
 	}
-	p.AddCreatedByPodAnnotation(k8s.CreatedByAnnotationValue())
+
 	patchJSON, err := p.Marshal()
 	if err != nil {
 		return nil, nil, err
@@ -281,61 +293,95 @@ func (options *injectOptions) fetchConfigsOrDefault() (*config.All, error) {
 	return api.Config(context.Background(), &public.Empty{})
 }
 
-// overrideConfigs uses command-line overrides to update the provided configs
+// overrideConfigs uses command-line overrides to update the provided configs.
-func (options *injectOptions) overrideConfigs(configs *config.All) {
+// the overrideAnnotations map keeps track of which configs are overridden, by
+// storing the corresponding annotations and values.
+func (options *injectOptions) overrideConfigs(configs *config.All, overrideAnnotations map[string]string) {
+	if options.linkerdVersion != "" {
+		configs.Global.Version = options.linkerdVersion
+	}
+
 	if len(options.ignoreInboundPorts) > 0 {
 		configs.Proxy.IgnoreInboundPorts = toPorts(options.ignoreInboundPorts)
+		overrideAnnotations[k8s.ProxyIgnoreInboundPortsAnnotation] = parsePorts(configs.Proxy.IgnoreInboundPorts)
 	}
 	if len(options.ignoreOutboundPorts) > 0 {
 		configs.Proxy.IgnoreOutboundPorts = toPorts(options.ignoreOutboundPorts)
+		overrideAnnotations[k8s.ProxyIgnoreOutboundPortsAnnotation] = parsePorts(configs.Proxy.IgnoreOutboundPorts)
 	}
 
 	if options.proxyAdminPort != 0 {
 		configs.Proxy.AdminPort = toPort(options.proxyAdminPort)
+		overrideAnnotations[k8s.ProxyAdminPortAnnotation] = parsePort(configs.Proxy.AdminPort)
 	}
 	if options.proxyControlPort != 0 {
 		configs.Proxy.ControlPort = toPort(options.proxyControlPort)
+		overrideAnnotations[k8s.ProxyControlPortAnnotation] = parsePort(configs.Proxy.ControlPort)
 	}
 	if options.proxyInboundPort != 0 {
 		configs.Proxy.InboundPort = toPort(options.proxyInboundPort)
+		overrideAnnotations[k8s.ProxyInboundPortAnnotation] = parsePort(configs.Proxy.InboundPort)
 	}
 	if options.proxyOutboundPort != 0 {
 		configs.Proxy.OutboundPort = toPort(options.proxyOutboundPort)
+		overrideAnnotations[k8s.ProxyOutboundPortAnnotation] = parsePort(configs.Proxy.OutboundPort)
 	}
 
-	if options.dockerRegistry != "" {
+	if options.proxyImage != "" {
-		configs.Proxy.ProxyImage.ImageName = registryOverride(configs.Proxy.ProxyImage.ImageName, options.dockerRegistry)
+		configs.Proxy.ProxyImage.ImageName = options.proxyImage
-		configs.Proxy.ProxyInitImage.ImageName = registryOverride(configs.Proxy.ProxyInitImage.ImageName, options.dockerRegistry)
+		if options.dockerRegistry != "" {
+			configs.Proxy.ProxyImage.ImageName = registryOverride(configs.Proxy.ProxyImage.ImageName, options.dockerRegistry)
+		}
+		overrideAnnotations[k8s.ProxyImageAnnotation] = configs.Proxy.ProxyImage.ImageName
+	}
+
+	if options.initImage != "" {
+		configs.Proxy.ProxyInitImage.ImageName = options.initImage
+		if options.dockerRegistry != "" {
+			configs.Proxy.ProxyInitImage.ImageName = registryOverride(configs.Proxy.ProxyInitImage.ImageName, options.dockerRegistry)
+		}
+		overrideAnnotations[k8s.ProxyInitImageAnnotation] = configs.Proxy.ProxyInitImage.ImageName
 	}
 
 	if options.imagePullPolicy != "" {
 		configs.Proxy.ProxyImage.PullPolicy = options.imagePullPolicy
 		configs.Proxy.ProxyInitImage.PullPolicy = options.imagePullPolicy
+		overrideAnnotations[k8s.ProxyImagePullPolicyAnnotation] = options.imagePullPolicy
 	}
 
 	if options.proxyUID != 0 {
 		configs.Proxy.ProxyUid = options.proxyUID
+		overrideAnnotations[k8s.ProxyUIDAnnotation] = strconv.FormatInt(options.proxyUID, 10)
 	}
 
 	if options.proxyLogLevel != "" {
 		configs.Proxy.LogLevel = &config.LogLevel{Level: options.proxyLogLevel}
+		overrideAnnotations[k8s.ProxyLogLevelAnnotation] = options.proxyLogLevel
 	}
 
+	// keep track of this option because its true/false value results in different
+	// values being assigned to the LINKERD2_PROXY_DESTINATION_PROFILE_SUFFIXES
+	// env var. Its annotation is added only if its value is true.
+	configs.Proxy.DisableExternalProfiles = options.disableExternalProfiles
 	if options.disableExternalProfiles {
-		configs.Proxy.DisableExternalProfiles = true
+		overrideAnnotations[k8s.ProxyDisableExternalProfilesAnnotation] = "true"
 	}
 
 	if options.proxyCPURequest != "" {
 		configs.Proxy.Resource.RequestCpu = options.proxyCPURequest
+		overrideAnnotations[k8s.ProxyCPURequestAnnotation] = options.proxyCPURequest
 	}
 	if options.proxyCPULimit != "" {
 		configs.Proxy.Resource.LimitCpu = options.proxyCPULimit
+		overrideAnnotations[k8s.ProxyCPULimitAnnotation] = options.proxyCPULimit
 	}
 	if options.proxyMemoryRequest != "" {
 		configs.Proxy.Resource.RequestMemory = options.proxyMemoryRequest
+		overrideAnnotations[k8s.ProxyMemoryRequestAnnotation] = options.proxyMemoryRequest
 	}
 	if options.proxyMemoryLimit != "" {
 		configs.Proxy.Resource.LimitMemory = options.proxyMemoryLimit
+		overrideAnnotations[k8s.ProxyMemoryLimitAnnotation] = options.proxyMemoryLimit
 	}
 }
 
@@ -343,6 +389,10 @@ func toPort(p uint) *config.Port {
 	return &config.Port{Port: uint32(p)}
 }
 
+func parsePort(port *config.Port) string {
+	return strconv.FormatUint(uint64(port.GetPort()), 10)
+}
+
 func toPorts(ints []uint) []*config.Port {
 	ports := make([]*config.Port, len(ints))
 	for i, p := range ints {
@@ -350,3 +400,12 @@ func toPorts(ints []uint) []*config.Port {
 	}
 	return ports
 }
+
+func parsePorts(ports []*config.Port) string {
+	var str string
+	for _, port := range ports {
+		str += parsePort(port) + ","
+	}
+
+	return strings.TrimSuffix(str, ",")
+}

cli/cmd/inject_test.go

@@ -39,8 +39,12 @@ func testUninjectAndInject(t *testing.T, tc testCase) {
 
 	output := new(bytes.Buffer)
 	report := new(bytes.Buffer)
+	transformer := &resourceTransformerInject{
+		configs:             tc.testInjectConfig,
+		overrideAnnotations: map[string]string{},
+	}
 
-	if exitCode := uninjectAndInject([]io.Reader{read}, report, output, tc.testInjectConfig); exitCode != 0 {
+	if exitCode := uninjectAndInject([]io.Reader{read}, report, output, transformer); exitCode != 0 {
 		t.Errorf("Unexpected error injecting YAML: %v\n", report)
 	}
 	diffTestdata(t, tc.goldenFileName, output.String())
@@ -211,7 +215,8 @@ func testInjectCmd(t *testing.T, tc injectCmd) {
 		t.Fatalf("Unexpected error: %v", err)
 	}
 
-	exitCode := runInjectCmd([]io.Reader{in}, errBuffer, outBuffer, testConfig)
+	transformer := &resourceTransformerInject{configs: testConfig}
+	exitCode := runInjectCmd([]io.Reader{in}, errBuffer, outBuffer, transformer)
 	if exitCode != tc.exitCode {
 		t.Fatalf("Expected exit code to be %d but got: %d", tc.exitCode, exitCode)
 	}
@@ -268,8 +273,8 @@ func testInjectFilePath(t *testing.T, tc injectFilePath) {
 
 	errBuf := &bytes.Buffer{}
 	actual := &bytes.Buffer{}
-	configs := testInstallConfig()
+	transformer := &resourceTransformerInject{configs: testInstallConfig()}
-	if exitCode := runInjectCmd(in, errBuf, actual, configs); exitCode != 0 {
+	if exitCode := runInjectCmd(in, errBuf, actual, transformer); exitCode != 0 {
 		t.Fatal("Unexpected error. Exit code from runInjectCmd: ", exitCode)
 	}
 	diffTestdata(t, tc.expectedFile, actual.String())
@@ -286,8 +291,8 @@ func testReadFromFolder(t *testing.T, resourceFolder string, expectedFolder stri
 
 	errBuf := &bytes.Buffer{}
 	actual := &bytes.Buffer{}
-	configs := testInstallConfig()
+	transformer := &resourceTransformerInject{configs: testInstallConfig()}
-	if exitCode := runInjectCmd(in, errBuf, actual, configs); exitCode != 0 {
+	if exitCode := runInjectCmd(in, errBuf, actual, transformer); exitCode != 0 {
 		t.Fatal("Unexpected error. Exit code from runInjectCmd: ", exitCode)
 	}
 

controller/proxy-injector/webhook.go

@@ -122,6 +122,9 @@ func (w *Webhook) inject(request *admissionv1beta1.AdmissionRequest) (*admission
 		return admissionResponse, nil
 	}
 
+	conf.AppendPodAnnotations(map[string]string{
+		pkgK8s.CreatedByAnnotation: fmt.Sprintf("linkerd/proxy-injector %s", version.Version),
+	})
 	p, reports, err := conf.GetPatch(request.Object.Raw, inject.ShouldInjectWebhook)
 	if err != nil {
 		return nil, err
@@ -131,8 +134,6 @@ func (w *Webhook) inject(request *admissionv1beta1.AdmissionRequest) (*admission
 		return admissionResponse, nil
 	}
 
-	p.AddCreatedByPodAnnotation(fmt.Sprintf("linkerd/proxy-injector %s", version.Version))
-
 	patchJSON, err := p.Marshal()
 	if err != nil {
 		return nil, err

pkg/inject/inject.go

@@ -95,9 +95,10 @@ type ResourceConfig struct {
 	pod struct {
 		// Meta is the pod's metadata. It's exported so that the YAML marshaling
 		// will work in the ParseMeta() function.
-		Meta   *metav1.ObjectMeta `json:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"`
+		Meta        *metav1.ObjectMeta `json:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"`
-		labels map[string]string
+		labels      map[string]string
-		spec   *v1.PodSpec
+		annotations map[string]string
+		spec        *v1.PodSpec
 	}
 }
 
@@ -110,6 +111,7 @@ func NewResourceConfig(configs *config.All) *ResourceConfig {
 
 	config.pod.Meta = &metav1.ObjectMeta{}
 	config.pod.labels = map[string]string{k8s.ControllerNSLabel: configs.GetGlobal().GetLinkerdNamespace()}
+	config.pod.annotations = map[string]string{}
 	return config
 }
 
@@ -141,6 +143,13 @@ func (conf *ResourceConfig) WithOwnerRetriever(f OwnerRetrieverFunc) *ResourceCo
 	return conf
 }
 
+// AppendPodAnnotations appends the given annotations to the pod spec in conf
+func (conf *ResourceConfig) AppendPodAnnotations(annotations map[string]string) {
+	for annotation, value := range annotations {
+		conf.pod.annotations[annotation] = value
+	}
+}
+
 // YamlMarshalObj returns the yaml for the workload in conf
 func (conf *ResourceConfig) YamlMarshalObj() ([]byte, error) {
 	return yaml.Marshal(conf.workload.obj)
@@ -187,8 +196,8 @@ func (conf *ResourceConfig) GetPatch(
 	if conf.pod.spec != nil {
 		report.update(conf)
 		if shouldInject(conf, report) {
-			conf.injectPodSpec(patch)
 			conf.injectObjectMeta(patch)
+			conf.injectPodSpec(patch)
 		}
 	} else {
 		report.UnsupportedResource = true
@@ -367,6 +376,10 @@ func (conf *ResourceConfig) parse(bytes []byte) error {
 		}
 	}
 
+	if conf.pod.Meta.Annotations == nil {
+		conf.pod.Meta.Annotations = map[string]string{}
+	}
+
 	return nil
 }
 
@@ -607,6 +620,14 @@ func (conf *ResourceConfig) injectObjectMeta(patch *Patch) {
 			patch.addPodLabel(k, v)
 		}
 	}
+
+	for k, v := range conf.pod.annotations {
+		patch.addPodAnnotation(k, v)
+
+		// append any additional pod annotations to the pod's meta.
+		// for e.g., annotations that were converted from CLI inject options.
+		conf.pod.Meta.Annotations[k] = v
+	}
 }
 
 func (conf *ResourceConfig) getOverride(annotation string) string {
@@ -820,13 +841,18 @@ func (conf *ResourceConfig) proxyLivenessProbe() *v1.Probe {
 }
 
 func (conf *ResourceConfig) proxyDestinationProfileSuffixes() string {
-	if overrides := conf.getOverride(k8s.ProxyDisableExternalProfilesAnnotation); overrides != "" {
+	disableExternalProfiles := conf.configs.GetProxy().GetDisableExternalProfiles()
-		disableExternalProfiles, err := strconv.ParseBool(overrides)
+	if override := conf.getOverride(k8s.ProxyDisableExternalProfilesAnnotation); override != "" {
-		if err == nil && disableExternalProfiles {
+		value, err := strconv.ParseBool(override)
-			return internalProfileSuffix
+		if err == nil {
+			disableExternalProfiles = value
 		}
 	}
 
+	if disableExternalProfiles {
+		return internalProfileSuffix
+	}
+
 	return defaultProfileSuffix
 }
 

pkg/inject/patch.go

@@ -129,12 +129,6 @@ func (p *Patch) addPodAnnotation(key, value string) {
 	})
 }
 
-// AddCreatedByPodAnnotation tags the pod so that we can tell apart injections
-// from the CLI and the webhook
-func (p *Patch) AddCreatedByPodAnnotation(s string) {
-	p.addPodAnnotation(k8s.CreatedByAnnotation, s)
-}
-
 // IsEmpty returns true if the patch doesn't contain any operations
 func (p *Patch) IsEmpty() bool {
 	return len(p.patchOps) == 0