Repository: linkerd/linkerd2
Pull request #5851 (2021-03-02 00:05:34)

CLI: add `--opaque-ports` flag to `inject`
Continuation of https://github.com/linkerd/linkerd2/pull/5721/

The `config.linkerd.io/opaque-ports` annotation can now be set using the `--opaque-ports` flag on `inject`

Example

```bash
$ linkerd inject /path/to/manifest.yaml --opaque-ports 3000,5000-6000,mysql
```

This annotation is the only one which is applied to services.

<!--  Thanks for sending a pull request!

If you already have a well-structured git commit message, chances are GitHub
set the title and description of this PR to the git commit message subject and
body, respectively. If so, you may delete these instructions and submit your PR.

If this is your first time, please read our contributor guide:
https://github.com/linkerd/linkerd2/blob/main/CONTRIBUTING.md

The title and description of your Pull Request should match the git commit
subject and body, respectively. Git commit messages are structured as follows:

```
Subject

Problem

Solution

Validation

Fixes #[GitHub issue ID]

DCO Sign off
```

Example git commit message:

```
Introduce Pull Request Template

GitHub's community guidelines recommend a pull request template, the repo was
lacking one.

Introduce a `PULL_REQUEST_TEMPLATE.md` file.

Once merged, the
[Community profile checklist](https://github.com/linkerd/linkerd2/community)
should indicate the repo now provides a pull request template.

Fixes #3321

Signed-off-by: Jane Smith <jane.smith@example.com>
```

Note the git commit message subject becomes the pull request title.

For more details around git commits, see the section on Committing in our
contributor guide:
https://github.com/linkerd/linkerd2/blob/main/CONTRIBUTING.md#committing
-->



Patch:
diff --git a/.proxy-version b/.proxy-version
index ce6305098..436c2699d 100644
--- a/.proxy-version
+++ b/.proxy-version
@@ -1,1 +1,1 @@
-v2.135.0
+v2.136.0
diff --git a/cli/cmd/inject.go b/cli/cmd/inject.go
index ec7254688..42112bc19 100644
--- a/cli/cmd/inject.go
+++ b/cli/cmd/inject.go
@@ -169,9 +169,14 @@ func (rt resourceTransformerInject) transform(bytes []byte) ([]byte, []inject.Re
 	}
 
 	reports := []inject.Report{*report}
-	// Injection of services depends on being able to retrieve the namespace
-	// annotations which can only occur in the proxy injector webhook.
+
 	if conf.IsService() {
+		opaquePortsAnnotations := map[string]string{}
+		if opaquePorts, ok := rt.overrideAnnotations[k8s.ProxyOpaquePortsAnnotation]; ok {
+			opaquePortsAnnotations[k8s.ProxyOpaquePortsAnnotation] = opaquePorts
+			b, err := conf.AnnotateService(opaquePortsAnnotations)
+			return b, reports, err
+		}
 		return bytes, reports, nil
 	}
 
@@ -204,6 +209,7 @@ func (rt resourceTransformerInject) transform(bytes []byte) ([]byte, []inject.Re
 	if err != nil {
 		return nil, nil, err
 	}
+
 	if len(patchJSON) == 0 {
 		return bytes, reports, nil
 	}
@@ -323,9 +329,7 @@ func (resourceTransformerInject) generateReport(reports []inject.Report, output
 	}
 
 	for _, r := range reports {
-		if r.Kind == k8s.Service {
-			output.Write([]byte(fmt.Sprintf("service \"%s\" skipped\n", r.Name)))
-		} else if b, _ := r.Injectable(); b {
+		if b, _ := r.Injectable(); b {
 			output.Write([]byte(fmt.Sprintf("%s \"%s\" injected\n", r.Kind, r.Name)))
 		} else {
 			if r.Kind != "" {
@@ -393,6 +397,9 @@ func getOverrideAnnotations(values *charts.Values, base *charts.Values) map[stri
 	if proxy.Ports.Outbound != baseProxy.Ports.Outbound {
 		overrideAnnotations[k8s.ProxyOutboundPortAnnotation] = fmt.Sprintf("%d", proxy.Ports.Outbound)
 	}
+	if proxy.OpaquePorts != baseProxy.OpaquePorts {
+		overrideAnnotations[k8s.ProxyOpaquePortsAnnotation] = proxy.OpaquePorts
+	}
 
 	if proxy.Image.Name != baseProxy.Image.Name {
 		overrideAnnotations[k8s.ProxyImageAnnotation] = proxy.Image.Name
diff --git a/cli/cmd/options.go b/cli/cmd/options.go
index 0db8b3b4c..cf820754d 100644
--- a/cli/cmd/options.go
+++ b/cli/cmd/options.go
@@ -471,6 +471,12 @@ func makeInjectFlags(defaults *l5dcharts.Values) ([]flag.Flag, *pflag.FlagSet) {
 				values.Proxy.IsIngress = value
 				return nil
 			}),
+
+		flag.NewStringSliceFlag(injectFlags, "opaque-ports", strings.Split(defaults.Proxy.OpaquePorts, ","),
+			"Set opaque ports on the proxy", func(values *l5dcharts.Values, value []string) error {
+				values.Proxy.OpaquePorts = strings.Join(value, ",")
+				return nil
+			}),
 	}
 
 	return flags, injectFlags
diff --git a/cli/cmd/uninject.go b/cli/cmd/uninject.go
index 07b49464c..7be001540 100644
--- a/cli/cmd/uninject.go
+++ b/cli/cmd/uninject.go
@@ -78,6 +78,7 @@ func (rt resourceTransformerUninject) transform(bytes []byte) ([]byte, []inject.
 		output = bytes
 		report.UnsupportedResource = true
 	}
+
 	return output, []inject.Report{*report}, nil
 }
 
diff --git a/jaeger/cmd/check.go b/jaeger/cmd/check.go
index 0083c9167..261e4cb26 100644
--- a/jaeger/cmd/check.go
+++ b/jaeger/cmd/check.go
@@ -85,48 +85,24 @@ func jaegerCategory(hc *healthcheck.HealthChecker) *healthcheck.Category {
 			}))
 
 	checkers = append(checkers,
-		*healthcheck.NewChecker("collector pod is running").
-			WithHintAnchor("l5d-jaeger-collector-running").
+		*healthcheck.NewChecker("jaeger extension pods are running").
+			WithHintAnchor("l5d-jaeger-pods-running").
 			Fatal().
 			WithRetryDeadline(hc.RetryDeadline).
 			SurfaceErrorOnRetry().
 			WithCheck(func(ctx context.Context) error {
-				// Check for Collector pod
-				podList, err := hc.KubeAPIClient().CoreV1().Pods(jaegerNamespace).List(ctx, metav1.ListOptions{LabelSelector: "component=collector"})
+				pods, err := hc.KubeAPIClient().GetPodsByNamespace(ctx, jaegerNamespace)
 				if err != nil {
 					return err
 				}
-				return healthcheck.CheckPodsRunning(podList.Items, fmt.Sprintf("No collector pods found in the %s namespace", jaegerNamespace))
-			}))
 
-	checkers = append(checkers,
-		*healthcheck.NewChecker("jaeger pod is running").
-			WithHintAnchor("l5d-jaeger-jaeger-running").
-			Fatal().
-			WithRetryDeadline(hc.RetryDeadline).
-			SurfaceErrorOnRetry().
-			WithCheck(func(ctx context.Context) error {
-				// Check for Jaeger pod
-				podList, err := hc.KubeAPIClient().CoreV1().Pods(jaegerNamespace).List(ctx, metav1.ListOptions{LabelSelector: "component=jaeger"})
+				// Check for relevant pods to be present
+				err = healthcheck.CheckForPods(pods, []string{"collector", "jaeger", "jaeger-injector"})
 				if err != nil {
 					return err
 				}
-				return healthcheck.CheckPodsRunning(podList.Items, fmt.Sprintf("No jaeger pods found in the %s namespace", jaegerNamespace))
-			}))
 
-	checkers = append(checkers,
-		*healthcheck.NewChecker("jaeger injector pod is running").
-			WithHintAnchor("l5d-jaeger-jaeger-running").
-			Fatal().
-			WithRetryDeadline(hc.RetryDeadline).
-			SurfaceErrorOnRetry().
-			WithCheck(func(ctx context.Context) error {
-				// Check for Jaeger Injector pod
-				podList, err := hc.KubeAPIClient().CoreV1().Pods(jaegerNamespace).List(ctx, metav1.ListOptions{LabelSelector: "component=jaeger-injector"})
-				if err != nil {
-					return err
-				}
-				return healthcheck.CheckPodsRunning(podList.Items, fmt.Sprintf("No jaeger injector pods found in the %s namespace", jaegerNamespace))
+				return healthcheck.CheckPodsRunning(pods, "")
 			}))
 
 	return healthcheck.NewCategory(linkerdJaegerExtensionCheck, checkers, true)
diff --git a/pkg/inject/inject.go b/pkg/inject/inject.go
index 7eeb68e87..471d8ab6c 100644
--- a/pkg/inject/inject.go
+++ b/pkg/inject/inject.go
@@ -916,7 +916,7 @@ func (conf *ResourceConfig) IsNamespace() bool {
 	return strings.ToLower(conf.workload.metaType.Kind) == k8s.Namespace
 }
 
-//IsService checks if a given config is a workload of Kind service
+// IsService checks if a given config is a workload of Kind service
 func (conf *ResourceConfig) IsService() bool {
 	return strings.ToLower(conf.workload.metaType.Kind) == k8s.Service
 }
@@ -942,6 +942,29 @@ func (conf *ResourceConfig) InjectNamespace(annotations map[string]string) ([]by
 	return yaml.JSONToYAML(j)
 }
 
+// AnnotateService returns a Service with the appropriate annotations
+// Currently, a `Service` may only need the `config.linkerd.io/opaque-ports` annotation via `inject`
+// See - https://github.com/linkerd/linkerd2/pull/5721
+func (conf *ResourceConfig) AnnotateService(annotations map[string]string) ([]byte, error) {
+	ns, ok := conf.workload.obj.(*corev1.Service)
+	if !ok {
+		return nil, errors.New("can't inject service. Type assertion failed")
+	}
+
+	//For overriding annotations
+	if len(annotations) > 0 {
+		for annotation, value := range annotations {
+			ns.Annotations[annotation] = value
+		}
+	}
+
+	j, err := getFilteredJSON(ns)
+	if err != nil {
+		return nil, err
+	}
+	return yaml.JSONToYAML(j)
+}
+
 //getFilteredJSON method performs JSON marshaling such that zero values of
 //empty structs are respected by `omitempty` tags. We make use of a drop-in
 //replacement of the standard json/encoding library, without which empty struct values
diff --git a/pkg/inject/uninject.go b/pkg/inject/uninject.go
index d2525efe0..afe7c431f 100644
--- a/pkg/inject/uninject.go
+++ b/pkg/inject/uninject.go
@@ -11,7 +11,7 @@ import (
 // Uninject removes from the workload in conf the init and proxy containers,
 // the TLS volumes and the extra annotations/labels that were added
 func (conf *ResourceConfig) Uninject(report *Report) ([]byte, error) {
-	if conf.IsNamespace() {
+	if conf.IsNamespace() || conf.IsService() {
 		uninjectObjectMeta(conf.workload.Meta, report)
 		return conf.YamlMarshalObj()
 	}


Test patch:
diff --git a/cli/cmd/inject_test.go b/cli/cmd/inject_test.go
index a3609359c..dc77c19c3 100644
--- a/cli/cmd/inject_test.go
+++ b/cli/cmd/inject_test.go
@@ -92,6 +92,9 @@ func TestUninjectAndInject(t *testing.T) {
 	cniEnabledConfig := defaultConfig()
 	cniEnabledConfig.CNIEnabled = true
 
+	opaquePortsConfig := defaultConfig()
+	opaquePortsConfig.Proxy.OpaquePorts = "3000,5000-6000,mysql"
+
 	proxyIgnorePortsConfig := defaultConfig()
 	proxyIgnorePortsConfig.ProxyInit.IgnoreInboundPorts = "22,8100-8102"
 	proxyIgnorePortsConfig.ProxyInit.IgnoreOutboundPorts = "5432"
@@ -289,6 +292,13 @@ func TestUninjectAndInject(t *testing.T) {
 			injectProxy:      true,
 			testInjectConfig: proxyIgnorePortsConfig,
 		},
+		{
+			inputFileName:    "inject_emojivoto_deployment.input.yml",
+			goldenFileName:   "inject_emojivoto_deployment_opaque_ports.golden.yml",
+			reportFileName:   "inject_emojivoto_deployment_opaque_ports.report",
+			injectProxy:      true,
+			testInjectConfig: opaquePortsConfig,
+		},
 	}
 
 	for i, tc := range testCases {
diff --git a/cli/cmd/testdata/inject_emojivoto_deployment_opaque_ports.golden.yml b/cli/cmd/testdata/inject_emojivoto_deployment_opaque_ports.golden.yml
new file mode 100644
index 000000000..affacff64
--- /dev/null
+++ b/cli/cmd/testdata/inject_emojivoto_deployment_opaque_ports.golden.yml
@@ -0,0 +1,179 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: web
+  namespace: emojivoto
+spec:
+  replicas: 1
+  selector:
+    matchLabels:
+      app: web-svc
+  template:
+    metadata:
+      annotations:
+        config.linkerd.io/opaque-ports: 3000,5000-6000,mysql
+        linkerd.io/created-by: linkerd/cli dev-undefined
+        linkerd.io/identity-mode: default
+        linkerd.io/proxy-version: test-inject-proxy-version
+      labels:
+        app: web-svc
+        linkerd.io/control-plane-ns: linkerd
+        linkerd.io/proxy-deployment: web
+        linkerd.io/workload-ns: emojivoto
+    spec:
+      containers:
+      - env:
+        - name: WEB_PORT
+          value: "80"
+        - name: EMOJISVC_HOST
+          value: emoji-svc.emojivoto:8080
+        - name: VOTINGSVC_HOST
+          value: voting-svc.emojivoto:8080
+        - name: INDEX_BUNDLE
+          value: dist/index_bundle.js
+        image: buoyantio/emojivoto-web:v10
+        name: web-svc
+        ports:
+        - containerPort: 80
+          name: http
+      - env:
+        - name: LINKERD2_PROXY_LOG
+          value: warn,linkerd=info
+        - name: LINKERD2_PROXY_LOG_FORMAT
+          value: plain
+        - name: LINKERD2_PROXY_DESTINATION_SVC_ADDR
+          value: linkerd-dst-headless.linkerd.svc.cluster.local.:8086
+        - name: LINKERD2_PROXY_DESTINATION_PROFILE_NETWORKS
+          value: 10.0.0.0/8,100.64.0.0/10,172.16.0.0/12,192.168.0.0/16
+        - name: LINKERD2_PROXY_INBOUND_CONNECT_TIMEOUT
+          value: 100ms
+        - name: LINKERD2_PROXY_OUTBOUND_CONNECT_TIMEOUT
+          value: 1000ms
+        - name: LINKERD2_PROXY_CONTROL_LISTEN_ADDR
+          value: 0.0.0.0:4190
+        - name: LINKERD2_PROXY_ADMIN_LISTEN_ADDR
+          value: 0.0.0.0:4191
+        - name: LINKERD2_PROXY_OUTBOUND_LISTEN_ADDR
+          value: 127.0.0.1:4140
+        - name: LINKERD2_PROXY_INBOUND_LISTEN_ADDR
+          value: 0.0.0.0:4143
+        - name: LINKERD2_PROXY_DESTINATION_PROFILE_SUFFIXES
+          value: svc.cluster.local.
+        - name: LINKERD2_PROXY_INBOUND_ACCEPT_KEEPALIVE
+          value: 10000ms
+        - name: LINKERD2_PROXY_OUTBOUND_CONNECT_KEEPALIVE
+          value: 10000ms
+        - name: LINKERD2_PROXY_INBOUND_PORTS_DISABLE_PROTOCOL_DETECTION
+          value: 3000,5000-6000,mysql
+        - name: _pod_ns
+          valueFrom:
+            fieldRef:
+              fieldPath: metadata.namespace
+        - name: _pod_nodeName
+          valueFrom:
+            fieldRef:
+              fieldPath: spec.nodeName
+        - name: LINKERD2_PROXY_DESTINATION_CONTEXT
+          value: |
+            {"ns":"$(_pod_ns)", "nodeName":"$(_pod_nodeName)"}
+        - name: LINKERD2_PROXY_IDENTITY_DIR
+          value: /var/run/linkerd/identity/end-entity
+        - name: LINKERD2_PROXY_IDENTITY_TRUST_ANCHORS
+          value: |
+            -----BEGIN CERTIFICATE-----
+            MIIBwTCCAWagAwIBAgIQeDZp5lDaIygQ5UfMKZrFATAKBggqhkjOPQQDAjApMScw
+            JQYDVQQDEx5pZGVudGl0eS5saW5rZXJkLmNsdXN0ZXIubG9jYWwwHhcNMjAwODI4
+            MDcxMjQ3WhcNMzAwODI2MDcxMjQ3WjApMScwJQYDVQQDEx5pZGVudGl0eS5saW5r
+            ZXJkLmNsdXN0ZXIubG9jYWwwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAARqc70Z
+            l1vgw79rjB5uSITICUA6GyfvSFfcuIis7B/XFSkkwAHU5S/s1AAP+R0TX7HBWUC4
+            uaG4WWsiwJKNn7mgo3AwbjAOBgNVHQ8BAf8EBAMCAQYwEgYDVR0TAQH/BAgwBgEB
+            /wIBATAdBgNVHQ4EFgQU5YtjVVPfd7I7NLHsn2C26EByGV0wKQYDVR0RBCIwIIIe
+            aWRlbnRpdHkubGlua2VyZC5jbHVzdGVyLmxvY2FsMAoGCCqGSM49BAMCA0kAMEYC
+            IQCN7lBFLDDvjx6V0+XkjpKERRsJYf5adMvnloFl48ilJgIhANtxhndcr+QJPuC8
+            vgUC0d2/9FMueIVMb+46WTCOjsqr
+            -----END CERTIFICATE-----
+        - name: LINKERD2_PROXY_IDENTITY_TOKEN_FILE
+          value: /var/run/secrets/kubernetes.io/serviceaccount/token
+        - name: LINKERD2_PROXY_IDENTITY_SVC_ADDR
+          value: linkerd-identity-headless.linkerd.svc.cluster.local.:8080
+        - name: _pod_sa
+          valueFrom:
+            fieldRef:
+              fieldPath: spec.serviceAccountName
+        - name: _l5d_ns
+          value: linkerd
+        - name: _l5d_trustdomain
+          value: cluster.local
+        - name: LINKERD2_PROXY_IDENTITY_LOCAL_NAME
+          value: $(_pod_sa).$(_pod_ns).serviceaccount.identity.$(_l5d_ns).$(_l5d_trustdomain)
+        - name: LINKERD2_PROXY_IDENTITY_SVC_NAME
+          value: linkerd-identity.$(_l5d_ns).serviceaccount.identity.$(_l5d_ns).$(_l5d_trustdomain)
+        - name: LINKERD2_PROXY_DESTINATION_SVC_NAME
+          value: linkerd-destination.$(_l5d_ns).serviceaccount.identity.$(_l5d_ns).$(_l5d_trustdomain)
+        image: cr.l5d.io/linkerd/proxy:test-inject-proxy-version
+        imagePullPolicy: IfNotPresent
+        livenessProbe:
+          httpGet:
+            path: /live
+            port: 4191
+          initialDelaySeconds: 10
+        name: linkerd-proxy
+        ports:
+        - containerPort: 4143
+          name: linkerd-proxy
+        - containerPort: 4191
+          name: linkerd-admin
+        readinessProbe:
+          httpGet:
+            path: /ready
+            port: 4191
+          initialDelaySeconds: 2
+        securityContext:
+          allowPrivilegeEscalation: false
+          readOnlyRootFilesystem: true
+          runAsUser: 2102
+        terminationMessagePolicy: FallbackToLogsOnError
+        volumeMounts:
+        - mountPath: /var/run/linkerd/identity/end-entity
+          name: linkerd-identity-end-entity
+      initContainers:
+      - args:
+        - --incoming-proxy-port
+        - "4143"
+        - --outgoing-proxy-port
+        - "4140"
+        - --proxy-uid
+        - "2102"
+        - --inbound-ports-to-ignore
+        - 4190,4191
+        image: cr.l5d.io/linkerd/proxy-init:v1.3.9
+        imagePullPolicy: IfNotPresent
+        name: linkerd-init
+        resources:
+          limits:
+            cpu: 100m
+            memory: 50Mi
+          requests:
+            cpu: 10m
+            memory: 10Mi
+        securityContext:
+          allowPrivilegeEscalation: false
+          capabilities:
+            add:
+            - NET_ADMIN
+            - NET_RAW
+          privileged: false
+          readOnlyRootFilesystem: true
+          runAsNonRoot: false
+          runAsUser: 0
+        terminationMessagePolicy: FallbackToLogsOnError
+        volumeMounts:
+        - mountPath: /run
+          name: linkerd-proxy-init-xtables-lock
+      volumes:
+      - emptyDir: {}
+        name: linkerd-proxy-init-xtables-lock
+      - emptyDir:
+          medium: Memory
+        name: linkerd-identity-end-entity
+---
diff --git a/cli/cmd/testdata/inject_emojivoto_deployment_opaque_ports.report b/cli/cmd/testdata/inject_emojivoto_deployment_opaque_ports.report
new file mode 100644
index 000000000..99851e468
--- /dev/null
+++ b/cli/cmd/testdata/inject_emojivoto_deployment_opaque_ports.report
@@ -0,0 +1,3 @@
+
+deployment "web" injected
+
diff --git a/cli/cmd/testdata/inject_emojivoto_deployment_opaque_ports.report.verbose b/cli/cmd/testdata/inject_emojivoto_deployment_opaque_ports.report.verbose
new file mode 100644
index 000000000..59734f772
--- /dev/null
+++ b/cli/cmd/testdata/inject_emojivoto_deployment_opaque_ports.report.verbose
@@ -0,0 +1,10 @@
+
+√ pods do not use host networking
+√ pods do not have a 3rd party proxy or initContainer already injected
+√ pods are not annotated to disable injection
+√ at least one resource injected
+√ pod specs do not include UDP ports
+√ pods do not have automountServiceAccountToken set to "false"
+
+deployment "web" injected
+
diff --git a/test/integration/testdata/inject.report.golden b/test/integration/testdata/inject.report.golden
index c7c5928fd..7031afeea 100644
--- a/test/integration/testdata/inject.report.golden
+++ b/test/integration/testdata/inject.report.golden
@@ -1,6 +1,6 @@
 
 deployment "smoke-test-terminus" injected
-service "smoke-test-terminus-svc" skipped
+service "smoke-test-terminus-svc" injected
 deployment "smoke-test-gateway" injected
-service "smoke-test-gateway-svc" skipped
+service "smoke-test-gateway-svc" injected
 
diff --git a/test/integration/tracing/testdata/check.jaeger.golden b/test/integration/tracing/testdata/check.jaeger.golden
index d422b8935..a9d18b29a 100644
--- a/test/integration/tracing/testdata/check.jaeger.golden
+++ b/test/integration/tracing/testdata/check.jaeger.golden
@@ -4,8 +4,6 @@ linkerd-jaeger
 √ collector and jaeger service account exists
 √ collector config map exists
 √ jaeger extension pods are injected
-√ collector pod is running
-√ jaeger pod is running
-√ jaeger injector pod is running
+√ jaeger extension pods are running
 
 Status check results are √