CLI: Remove the `--disable-tap` flag from inject (#5671)

Fixes https://github.com/linkerd/linkerd2/issues/5664

- Remove `--disable-flag` from `inject`
-  Move `config.linkerd.io/disable-tap` to `viz.linkerd.io/disable-tap`

Signed-off-by: Mayank Shah <mayankshah1614@gmail.com>

charts/partials/templates/_validate.tpl

@@ -2,8 +2,4 @@
 {{- if .disableIdentity -}}
 {{- fail (printf "Can't disable identity mTLS for %s. Set '.Values.global.proxy.disableIdentity' to 'false'" .component) -}}
 {{- end -}}
-
-{{- if .disableTap -}}
-{{- fail (printf "Can't disable tap for %s. Set '.Values.global.proxy.disableTap' to 'false'" .component) -}}
-{{- end -}}
 {{- end -}}

cli/cmd/doc.go

@@ -207,10 +207,6 @@ func generateAnnotationsDocs() []annotationDoc {
 			Name:        k8s.ProxyDisableIdentityAnnotation,
 			Description: "Disables resources from participating in TLS identity",
 		},
-		{
-			Name:        k8s.ProxyDisableTapAnnotation,
-			Description: "Disables resources from being tapped",
-		},
 		{
 			Name:        k8s.ProxyEnableDebugAnnotation,
 			Description: "Inject a debug sidecar for data plane debugging",

cli/cmd/inject.go

@@ -427,10 +427,6 @@ func getOverrideAnnotations(values *charts.Values, base *charts.Values) map[stri
 		overrideAnnotations[k8s.ProxyRequireIdentityOnInboundPortsAnnotation] = proxy.RequireIdentityOnInboundPorts
 	}
 
-	if proxy.DisableTap != baseProxy.DisableTap {
-		overrideAnnotations[k8s.ProxyDisableTapAnnotation] = strconv.FormatBool(proxy.DisableTap)
-	}
-
 	if proxy.EnableExternalProfiles != baseProxy.EnableExternalProfiles {
 		overrideAnnotations[k8s.ProxyEnableExternalProfilesAnnotation] = strconv.FormatBool(proxy.EnableExternalProfiles)
 	}

cli/cmd/inject_test.go

@@ -594,7 +594,6 @@ func TestProxyConfigurationAnnotations(t *testing.T) {
 	values.GetGlobal().Proxy.LogLevel = "debug"
 	values.GetGlobal().Proxy.LogFormat = "cool"
 	values.GetGlobal().Proxy.DisableIdentity = true
-	values.GetGlobal().Proxy.DisableTap = true
 	values.GetGlobal().Proxy.EnableExternalProfiles = true
 	values.GetGlobal().Proxy.Resources.CPU.Request = "10m"
 	values.GetGlobal().Proxy.Resources.CPU.Limit = "100m"
@@ -603,17 +602,17 @@ func TestProxyConfigurationAnnotations(t *testing.T) {
 	values.GetGlobal().Proxy.WaitBeforeExitSeconds = 10
 
 	expectedOverrides := map[string]string{
-		k8s.ProxyIgnoreInboundPortsAnnotation:     "8500-8505",
+		k8s.ProxyIgnoreInboundPortsAnnotation:  "8500-8505",
-		k8s.ProxyIgnoreOutboundPortsAnnotation:    "3306",
+		k8s.ProxyIgnoreOutboundPortsAnnotation: "3306",
-		k8s.ProxyAdminPortAnnotation:              "1234",
+		k8s.ProxyAdminPortAnnotation:           "1234",
-		k8s.ProxyControlPortAnnotation:            "4191",
+		k8s.ProxyControlPortAnnotation:         "4191",
-		k8s.ProxyInboundPortAnnotation:            "4144",
+		k8s.ProxyInboundPortAnnotation:         "4144",
-		k8s.ProxyOutboundPortAnnotation:           "4141",
+		k8s.ProxyOutboundPortAnnotation:        "4141",
-		k8s.ProxyUIDAnnotation:                    "999",
+		k8s.ProxyUIDAnnotation:                 "999",
-		k8s.ProxyLogLevelAnnotation:               "debug",
+		k8s.ProxyLogLevelAnnotation:            "debug",
-		k8s.ProxyLogFormatAnnotation:              "cool",
+		k8s.ProxyLogFormatAnnotation:           "cool",
-		k8s.ProxyDisableIdentityAnnotation:        "true",
+		k8s.ProxyDisableIdentityAnnotation:     "true",
-		k8s.ProxyDisableTapAnnotation:             "true",
+
 		k8s.ProxyEnableExternalProfilesAnnotation: "true",
 		k8s.ProxyCPURequestAnnotation:             "10m",
 		k8s.ProxyCPULimitAnnotation:               "100m",

cli/cmd/options.go

@@ -460,12 +460,6 @@ func makeInjectFlags(defaults *l5dcharts.Values) ([]flag.Flag, *pflag.FlagSet) {
 				return nil
 			}),
 
-		flag.NewBoolFlag(injectFlags, "disable-tap", defaults.GetGlobal().Proxy.DisableTap,
-			"Disables resources from being tapped", func(values *l5dcharts.Values, value bool) error {
-				values.GetGlobal().Proxy.DisableTap = value
-				return nil
-			}),
-
 		flag.NewStringSliceFlag(injectFlags, "require-identity-on-inbound-ports", strings.Split(defaults.GetGlobal().Proxy.RequireIdentityOnInboundPorts, ","),
 			"Inbound ports on which the proxy should require identity", func(values *l5dcharts.Values, value []string) error {
 				values.GetGlobal().Proxy.RequireIdentityOnInboundPorts = strings.Join(value, ",")

cli/cmd/testdata/inject_emojivoto_deployment_config_overrides.golden.yml

@@ -12,7 +12,6 @@ spec:
     metadata:
       annotations:
         config.linkerd.io/admin-port: "9998"
-        config.linkerd.io/disable-tap: "true"
         config.linkerd.io/proxy-cpu-limit: "1"
         config.linkerd.io/proxy-cpu-request: "0.5"
         config.linkerd.io/proxy-memory-limit: 256Mi

cli/cmd/testdata/inject_emojivoto_deployment_config_overrides.input.yml

@@ -12,7 +12,6 @@ spec:
     metadata:
       annotations:
         config.linkerd.io/admin-port: "9998"
-        config.linkerd.io/disable-tap: "true"
         config.linkerd.io/proxy-cpu-limit: "1"
         config.linkerd.io/proxy-cpu-request: "0.5"
         config.linkerd.io/proxy-memory-limit: 256Mi

cli/cmd/testdata/install_controlplane_tracing_output.golden

@@ -894,7 +894,6 @@ data:
       proxy:
         capabilities: null
         disableIdentity: false
-        disableTap: false
         enableExternalProfiles: false
         image:
           name: ghcr.io/linkerd/proxy

cli/cmd/testdata/install_custom_domain.golden

@@ -894,7 +894,6 @@ data:
       proxy:
         capabilities: null
         disableIdentity: false
-        disableTap: false
         enableExternalProfiles: false
         image:
           name: ghcr.io/linkerd/proxy

cli/cmd/testdata/install_custom_registry.golden

@@ -894,7 +894,6 @@ data:
       proxy:
         capabilities: null
         disableIdentity: false
-        disableTap: false
         enableExternalProfiles: false
         image:
           name: my.custom.registry/linkerd-io/proxy

cli/cmd/testdata/install_default.golden

@@ -894,7 +894,6 @@ data:
       proxy:
         capabilities: null
         disableIdentity: false
-        disableTap: false
         enableExternalProfiles: false
         image:
           name: ghcr.io/linkerd/proxy

cli/cmd/testdata/install_default_override_dst_get_nets.golden

@@ -894,7 +894,6 @@ data:
       proxy:
         capabilities: null
         disableIdentity: false
-        disableTap: false
         enableExternalProfiles: false
         image:
           name: ghcr.io/linkerd/proxy

cli/cmd/testdata/install_ha_output.golden

@@ -900,7 +900,6 @@ data:
       proxy:
         capabilities: null
         disableIdentity: false
-        disableTap: false
         enableExternalProfiles: false
         image:
           name: ghcr.io/linkerd/proxy

cli/cmd/testdata/install_ha_with_overrides_output.golden

@@ -900,7 +900,6 @@ data:
       proxy:
         capabilities: null
         disableIdentity: false
-        disableTap: false
         enableExternalProfiles: false
         image:
           name: ghcr.io/linkerd/proxy

cli/cmd/testdata/install_heartbeat_disabled_output.golden

@@ -851,7 +851,6 @@ data:
       proxy:
         capabilities: null
         disableIdentity: false
-        disableTap: false
         enableExternalProfiles: false
         image:
           name: ghcr.io/linkerd/proxy

cli/cmd/testdata/install_helm_output.golden

@@ -904,7 +904,6 @@ data:
       proxy:
         capabilities: null
         disableIdentity: false
-        disableTap: false
         enableExternalProfiles: false
         image:
           name: ghcr.io/linkerd/proxy

cli/cmd/testdata/install_helm_output_ha.golden

@@ -910,7 +910,6 @@ data:
       proxy:
         capabilities: null
         disableIdentity: false
-        disableTap: false
         enableExternalProfiles: false
         image:
           name: ghcr.io/linkerd/proxy

cli/cmd/testdata/install_helm_output_ha_labels.golden

@@ -914,7 +914,6 @@ data:
       proxy:
         capabilities: null
         disableIdentity: false
-        disableTap: false
         enableExternalProfiles: false
         image:
           name: ghcr.io/linkerd/proxy

cli/cmd/testdata/install_helm_output_ha_namespace_selector.golden

@@ -910,7 +910,6 @@ data:
       proxy:
         capabilities: null
         disableIdentity: false
-        disableTap: false
         enableExternalProfiles: false
         image:
           name: ghcr.io/linkerd/proxy

cli/cmd/testdata/install_no_init_container.golden

@@ -891,7 +891,6 @@ data:
       proxy:
         capabilities: null
         disableIdentity: false
-        disableTap: false
         enableExternalProfiles: false
         image:
           name: ghcr.io/linkerd/proxy

cli/cmd/testdata/install_output.golden

@@ -894,7 +894,6 @@ data:
       proxy:
         capabilities: null
         disableIdentity: false
-        disableTap: false
         enableExternalProfiles: false
         image:
           name: ProxyImageName

cli/cmd/testdata/install_proxy_ignores.golden

@@ -894,7 +894,6 @@ data:
       proxy:
         capabilities: null
         disableIdentity: false
-        disableTap: false
         enableExternalProfiles: false
         image:
           name: ghcr.io/linkerd/proxy

cli/cmd/testdata/install_values_file.golden

@@ -880,7 +880,6 @@ data:
       proxy:
         capabilities: null
         disableIdentity: false
-        disableTap: false
         enableExternalProfiles: false
         image:
           name: ghcr.io/linkerd/proxy

pkg/charts/linkerd2/values.go

@@ -107,7 +107,6 @@ type (
 		// This should match .Resources.CPU.Limit, but must be a whole number
 		Cores                         int64            `json:"cores,omitempty"`
 		DisableIdentity               bool             `json:"disableIdentity"`
-		DisableTap                    bool             `json:"disableTap"`
 		EnableExternalProfiles        bool             `json:"enableExternalProfiles"`
 		Image                         *Image           `json:"image"`
 		LogLevel                      string           `json:"logLevel"`

pkg/inject/inject.go

@@ -40,7 +40,6 @@ var (
 		k8s.ProxyAdminPortAnnotation,
 		k8s.ProxyControlPortAnnotation,
 		k8s.ProxyDisableIdentityAnnotation,
-		k8s.ProxyDisableTapAnnotation,
 		k8s.ProxyEnableDebugAnnotation,
 		k8s.ProxyEnableExternalProfilesAnnotation,
 		k8s.ProxyImagePullPolicyAnnotation,
@@ -707,13 +706,6 @@ func (conf *ResourceConfig) applyAnnotationOverrides(values *l5dcharts.Values) {
 		}
 	}
 
-	if override, ok := annotations[k8s.ProxyDisableTapAnnotation]; ok {
-		value, err := strconv.ParseBool(override)
-		if err == nil {
-			values.GetGlobal().Proxy.DisableTap = value
-		}
-	}
-
 	if override, ok := annotations[k8s.ProxyRequireIdentityOnInboundPortsAnnotation]; ok {
 		values.GetGlobal().Proxy.RequireIdentityOnInboundPorts = override
 	}

pkg/k8s/labels.go

@@ -7,7 +7,6 @@ package k8s
 
 import (
 	"fmt"
-	"strconv"
 
 	"github.com/linkerd/linkerd2/pkg/version"
 	appsv1 "k8s.io/api/apps/v1"
@@ -225,9 +224,6 @@ const (
 	// ProxyDisableIdentityAnnotation can be used to disable identity on the injected proxy.
 	ProxyDisableIdentityAnnotation = ProxyConfigAnnotationsPrefix + "/disable-identity"
 
-	// ProxyDisableTapAnnotation can be used to disable tap on the injected proxy.
-	ProxyDisableTapAnnotation = ProxyConfigAnnotationsPrefix + "/disable-tap"
-
 	// ProxyEnableDebugAnnotation is set to true if the debug container is
 	// injected.
 	ProxyEnableDebugAnnotation = ProxyConfigAnnotationsPrefix + "/enable-debug-sidecar"
@@ -477,22 +473,3 @@ func GetPodLabels(ownerKind, ownerName string, pod *corev1.Pod) map[string]strin
 func IsMeshed(pod *corev1.Pod, controllerNS string) bool {
 	return pod.Labels[ControllerNSLabel] == controllerNS
 }
-
-// IsTapDisabled returns true if a namespace or pod has an annotation for
-// explicitly disabling tap
-func IsTapDisabled(obj interface{}) bool {
-	var valStr string
-	switch resource := obj.(type) {
-	case *corev1.Pod:
-		valStr = resource.GetAnnotations()[ProxyDisableTapAnnotation]
-	case *corev1.Namespace:
-		valStr = resource.GetAnnotations()[ProxyDisableTapAnnotation]
-	}
-	if valStr != "" {
-		valBool, err := strconv.ParseBool(valStr)
-		if err == nil && valBool {
-			return true
-		}
-	}
-	return false
-}

test/integration/inject/inject_test.go

@@ -50,7 +50,6 @@ func TestInjectManualParams(t *testing.T) {
 
 	injectionValidator := testutil.InjectValidator{
 		DisableIdentity:        true,
-		DisableTap:             true,
 		Version:                "proxy-version",
 		Image:                  "proxy-image",
 		InitImage:              "init-image",
@@ -113,7 +112,6 @@ func TestInjectAutoParams(t *testing.T) {
 			AutoInject:             true,
 			AdminPort:              8888,
 			ControlPort:            8881,
-			DisableTap:             true,
 			EnableExternalProfiles: true,
 			EnableDebug:            true,
 			ImagePullPolicy:        "Never",

test/integration/tap/tap_test.go

@@ -145,7 +145,7 @@ func TestCliTap(t *testing.T) {
 				testutil.Fatal(t, "expected an error, got none")
 			}
 			expectedErr := `Error: no pods to tap for deployment/t4
-pods found with tap disabled via the config.linkerd.io/disable-tap annotation`
+pods found with tap disabled via the viz.linkerd.io/disable-tap annotation`
 			split := strings.Split(stderr, "\n")
 			actualErr := strings.Join(split[:2], "\n")
 			if actualErr != expectedErr {

test/integration/tap/testdata/tap_application.yaml

@@ -133,7 +133,7 @@ spec:
   template:
     metadata:
       annotations:
-        config.linkerd.io/disable-tap: "true"
+        viz.linkerd.io/disable-tap: "true"
       labels:
         app: t4
     spec:

testutil/inject_validator.go

@@ -25,7 +25,6 @@ type InjectValidator struct {
 	AutoInject             bool
 	AdminPort              int
 	ControlPort            int
-	DisableTap             bool
 	EnableDebug            bool
 	EnableExternalProfiles bool
 	ImagePullPolicy        string
@@ -77,15 +76,6 @@ func (iv *InjectValidator) validateEnvVar(container *v1.Container, envName, expe
 	return fmt.Errorf("cannot find env: %s", envName)
 }
 
-func (iv *InjectValidator) validateNoEnvVar(container *v1.Container, envName string) error {
-	for _, env := range container.Env {
-		if env.Name == envName {
-			return fmt.Errorf("env: %s, expected to not be set, actual %s", envName, env.Value)
-		}
-	}
-	return nil
-}
-
 func (iv *InjectValidator) validatePort(container *v1.Container, portName string, expectedValue int) error {
 	for _, port := range container.Ports {
 		if port.Name == portName {
@@ -137,12 +127,6 @@ func (iv *InjectValidator) validateProxyContainer(pod *v1.PodSpec) error {
 		}
 	}
 
-	if iv.DisableTap {
-		if err := iv.validateNoEnvVar(proxyContainer, "LINKERD2_PROXY_TAP_SVC"); err != nil {
-			return err
-		}
-	}
-
 	if iv.EnableExternalProfiles {
 		if err := iv.validateEnvVar(proxyContainer, "LINKERD2_PROXY_DESTINATION_PROFILE_SUFFIXES", "."); err != nil {
 			return err
@@ -422,11 +406,6 @@ func (iv *InjectValidator) GetFlagsAndAnnotations() ([]string, map[string]string
 		flags = append(flags, "--disable-identity")
 	}
 
-	if iv.DisableTap {
-		annotations[k8s.ProxyDisableTapAnnotation] = enabled
-		flags = append(flags, "--disable-tap")
-	}
-
 	if iv.EnableDebug {
 		annotations[k8s.ProxyEnableDebugAnnotation] = enabled
 		flags = append(flags, "--enable-debug-sidecar")

viz/pkg/healthcheck/healthcheck.go

@@ -339,7 +339,7 @@ func (hc *HealthChecker) checkForTapConfiguration(ctx context.Context, pods []co
 			return err
 		}
 		// Check if Tap is disabled
-		if !k8s.IsTapDisabled(pod) && !k8s.IsTapDisabled(ns) {
+		if !vizLabels.IsTapDisabled(pod) && !vizLabels.IsTapDisabled(ns) {
 			// Check for tap-injector annotation
 			if !vizLabels.IsTapEnabled(&pod) {
 				podsWithoutTap = append(podsWithoutTap, fmt.Sprintf("* %s", pod.Name))

viz/pkg/labels/labels.go

@@ -13,6 +13,9 @@ const (
 	// VizTapEnabled is set by the tap-injector component when tap has been
 	// enabled on a pod.
 	VizTapEnabled = VizAnnotationsPrefix + "/tap-enabled"
+
+	// VizTapDisabled can be used to disable tap on the injected proxy.
+	VizTapDisabled = VizAnnotationsPrefix + "/disable-tap"
 )
 
 // IsTapEnabled returns true if a pod has an annotation indicating that tap
@@ -27,3 +30,22 @@ func IsTapEnabled(pod *corev1.Pod) bool {
 	}
 	return false
 }
+
+// IsTapDisabled returns true if a namespace or pod has an annotation for
+// explicitly disabling tap
+func IsTapDisabled(obj interface{}) bool {
+	var valStr string
+	switch resource := obj.(type) {
+	case *corev1.Pod:
+		valStr = resource.GetAnnotations()[VizTapDisabled]
+	case *corev1.Namespace:
+		valStr = resource.GetAnnotations()[VizTapDisabled]
+	}
+	if valStr != "" {
+		valBool, err := strconv.ParseBool(valStr)
+		if err == nil && valBool {
+			return true
+		}
+	}
+	return false
+}

viz/tap/api/grpc_server.go

@@ -88,7 +88,7 @@ func (s *GRPCTapServer) TapByResource(req *tapPb.TapByResourceRequest, stream ta
 
 		for _, pod := range podsFor {
 			if pkgK8s.IsMeshed(pod, s.controllerNamespace) {
-				if pkgK8s.IsTapDisabled(pod) {
+				if vizLabels.IsTapDisabled(pod) {
 					tapDisabled = true
 				} else if !vizLabels.IsTapEnabled(pod) {
 					tapNotEnabled = true
@@ -104,7 +104,7 @@ func (s *GRPCTapServer) TapByResource(req *tapPb.TapByResourceRequest, stream ta
 		errStr := fmt.Errorf("no pods to tap for %s/%s", res.GetType(), res.GetName())
 		errStrings = append(errStrings, errStr.Error())
 		if tapDisabled {
-			errStr = fmt.Errorf("pods found with tap disabled via the %s annotation", pkgK8s.ProxyDisableTapAnnotation)
+			errStr = fmt.Errorf("pods found with tap disabled via the %s annotation", vizLabels.VizTapDisabled)
 			errStrings = append(errStrings, errStr.Error())
 		}
 		if tapNotEnabled {

viz/tap/api/grpc_server_test.go

@@ -186,7 +186,7 @@ status:
 		},
 		{
 			err: status.Errorf(codes.NotFound, `no pods to tap for pod/emojivoto-meshed-tap-disabled
-pods found with tap disabled via the config.linkerd.io/disable-tap annotation`),
+pods found with tap disabled via the viz.linkerd.io/disable-tap annotation`),
 			k8sRes: []string{`
 apiVersion: v1
 kind: Pod
@@ -197,7 +197,7 @@ metadata:
     app: emoji-svc
     linkerd.io/control-plane-ns: controller-ns
   annotations:
-    config.linkerd.io/disable-tap: "true"
+    viz.linkerd.io/disable-tap: "true"
     linkerd.io/proxy-version: testinjectversion
 status:
   phase: Running

viz/tap/injector/webhook.go

@@ -8,7 +8,6 @@ import (
 	"github.com/ghodss/yaml"
 	"github.com/linkerd/linkerd2/controller/k8s"
 	"github.com/linkerd/linkerd2/controller/webhook"
-	labels "github.com/linkerd/linkerd2/pkg/k8s"
 	vizLabels "github.com/linkerd/linkerd2/viz/pkg/labels"
 	"github.com/prometheus/common/log"
 	admissionv1beta1 "k8s.io/api/admission/v1beta1"
@@ -53,7 +52,7 @@ func Mutate(tapSvcName string) webhook.Handler {
 			return nil, err
 		}
 		var t *template.Template
-		if labels.IsTapDisabled(namespace) || labels.IsTapDisabled(pod) {
+		if vizLabels.IsTapDisabled(namespace) || vizLabels.IsTapDisabled(pod) {
 			return admissionResponse, nil
 		}
 		t, err = template.New("tpl").Parse(tpl)