Inject warns on UDP ports (linkerd#1617)

linkerd only routes TCP data, but `linkerd inject` does not warn when it injects into pods with ports set to `protocol: UDP`. Modify `linkerd inject` to warn when injected into a pod with `protocol: UDP`. The Linkerd sidecar will still be injected, but the stderr output will include a warning. Also add stderr checking on all inject unit tests. Part of linkerd#1516. Signed-off-by: Andrew Seigner <[email protected]>
Tim-Brooks · Sep 11, 2018 · 7eec5f1 · 7eec5f1
1 parent f330159
commit 7eec5f1
Show file tree

Hide file tree

Showing 17 changed files with 331 additions and 20 deletions.
diff --git a/cli/cmd/inject.go b/cli/cmd/inject.go
@@ -36,6 +36,7 @@ const (
  // for inject reports
  hostNetworkDesc = "hostNetwork: pods do not use host networking"
  unsupportedDesc = "supported: at least one resource injected"
+ udpDesc = "udp: pod specs do not include UDP ports"
 )
 
 type injectOptions struct {
@@ -49,6 +50,7 @@ type injectOptions struct {
 type injectReport struct {
  name string
  hostNetwork bool
+ udp bool // true if any port in any container has `protocol: UDP`
  unsupportedResource bool
 }
 
@@ -177,7 +179,9 @@ func injectObjectMeta(t *metaV1.ObjectMeta, k8sLabels map[string]string, options
  * injected, return false.
  */
 func injectPodSpec(t *v1.PodSpec, identity k8s.TLSIdentity, controlPlaneDNSNameOverride string, options *injectOptions, report *injectReport) bool {
- // Pods with `hostNetwork=true` share a network namespace with the host. The
+ report.udp = checkUDPPorts(t)
+
+ // Pods with `hostNetwork: true` share a network namespace with the host. The
  // init-container would destroy the iptables configuration on the host, so
  // skip the injection in this case.
  if t.HostNetwork {
@@ -218,7 +222,7 @@ func injectPodSpec(t *v1.PodSpec, identity k8s.TLSIdentity, controlPlaneDNSNameO
  Image: options.taggedProxyInitImage(),
  ImagePullPolicy: v1.PullPolicy(options.imagePullPolicy),
  TerminationMessagePolicy: v1.TerminationMessageFallbackToLogsOnError,
- Args: initArgs,
+ Args:  initArgs,
  SecurityContext: &v1.SecurityContext{
  Capabilities: &v1.Capabilities{
  Add: []v1.Capability{v1.Capability("NET_ADMIN")},
@@ -633,15 +637,26 @@ func generateReport(injectReports []injectReport, output io.Writer) {
 
  injected := []string{}
  hostNetwork := []string{}
+ udp := []string{}
 
  for _, r := range injectReports {
  if !r.hostNetwork && !r.unsupportedResource {
  injected = append(injected, r.name)
- } else if r.hostNetwork {
+ }
+
+ if r.hostNetwork {
  hostNetwork = append(hostNetwork, r.name)
  }
+
+ if r.udp {
+ udp = append(udp, r.name)
+ }
  }
 
+ //
+ // Warnings
+ //
+
  // leading newline to separate from yaml output on stdout
  output.Write([]byte("\n"))
 
@@ -663,6 +678,21 @@ func generateReport(injectReports []injectReport, output io.Writer) {
  output.Write([]byte(fmt.Sprintf("%s%s -- no supported objects found\n", unsupportedPrefix, warnStatus)))
  }
 
+ udpPrefix := fmt.Sprintf("%s%s", udpDesc, getFiller(udpDesc))
+ if len(udp) == 0 {
+ output.Write([]byte(fmt.Sprintf("%s%s\n", udpPrefix, okStatus)))
+ } else {
+ verb := "uses"
+ if len(udp) > 1 {
+ verb = "use"
+ }
+ output.Write([]byte(fmt.Sprintf("%s%s -- %s %s \"protocol: UDP\"\n", udpPrefix, warnStatus, strings.Join(udp, ", "), verb)))
+ }
+
+ //
+ // Summary
+ //
+
  summary := fmt.Sprintf("Summary: %d of %d YAML document(s) injected", len(injected), len(injectReports))
  output.Write([]byte(fmt.Sprintf("\n%s\n", summary)))
 
@@ -682,3 +712,15 @@ func getFiller(text string) string {
 
  return filler
 }
+
+func checkUDPPorts(t *v1.PodSpec) bool {
+ // check for ports with `protocol: UDP`, which will not be routed by Linkerd
+ for _, container := range t.Containers {
+ for _, port := range container.Ports {
+ if port.Protocol == v1.ProtocolUDP {
+ return true
+ }
+ }
+ }
+ return false
+}
diff --git a/cli/cmd/inject_test.go b/cli/cmd/inject_test.go
@@ -22,17 +22,69 @@ func TestInjectYAML(t *testing.T) {
  testCases := []struct {
  inputFileName string
  goldenFileName string
+ reportFileName string
  testInjectOptions *injectOptions
  }{
- {"inject_emojivoto_deployment.input.yml", "inject_emojivoto_deployment.golden.yml", defaultOptions},
- {"inject_emojivoto_list.input.yml", "inject_emojivoto_list.golden.yml", defaultOptions},
- {"inject_emojivoto_deployment_hostNetwork_false.input.yml", "inject_emojivoto_deployment_hostNetwork_false.golden.yml", defaultOptions},
- {"inject_emojivoto_deployment_hostNetwork_true.input.yml", "inject_emojivoto_deployment_hostNetwork_true.golden.yml", defaultOptions},
- {"inject_emojivoto_deployment_controller_name.input.yml", "inject_emojivoto_deployment_controller_name.golden.yml", defaultOptions},
- {"inject_emojivoto_statefulset.input.yml", "inject_emojivoto_statefulset.golden.yml", defaultOptions},
- {"inject_emojivoto_pod.input.yml", "inject_emojivoto_pod.golden.yml", defaultOptions},
- {"inject_emojivoto_deployment.input.yml", "inject_emojivoto_deployment_tls.golden.yml", tlsOptions},
- {"inject_emojivoto_pod.input.yml", "inject_emojivoto_pod_tls.golden.yml", tlsOptions},
+ {
+ inputFileName: "inject_emojivoto_deployment.input.yml",
+ goldenFileName: "inject_emojivoto_deployment.golden.yml",
+ reportFileName: "inject_emojivoto_deployment.report",
+ testInjectOptions: defaultOptions,
+ },
+ {
+ inputFileName: "inject_emojivoto_list.input.yml",
+ goldenFileName: "inject_emojivoto_list.golden.yml",
+ reportFileName: "inject_emojivoto_list.report",
+ testInjectOptions: defaultOptions,
+ },
+ {
+ inputFileName: "inject_emojivoto_deployment_hostNetwork_false.input.yml",
+ goldenFileName: "inject_emojivoto_deployment_hostNetwork_false.golden.yml",
+ reportFileName: "inject_emojivoto_deployment_hostNetwork_false.report",
+ testInjectOptions: defaultOptions,
+ },
+ {
+ inputFileName: "inject_emojivoto_deployment_hostNetwork_true.input.yml",
+ goldenFileName: "inject_emojivoto_deployment_hostNetwork_true.golden.yml",
+ reportFileName: "inject_emojivoto_deployment_hostNetwork_true.report",
+ testInjectOptions: defaultOptions,
+ },
+ {
+ inputFileName: "inject_emojivoto_deployment_controller_name.input.yml",
+ goldenFileName: "inject_emojivoto_deployment_controller_name.golden.yml",
+ reportFileName: "inject_emojivoto_deployment_controller_name.report",
+ testInjectOptions: defaultOptions,
+ },
+ {
+ inputFileName: "inject_emojivoto_statefulset.input.yml",
+ goldenFileName: "inject_emojivoto_statefulset.golden.yml",
+ reportFileName: "inject_emojivoto_statefulset.report",
+ testInjectOptions: defaultOptions,
+ },
+ {
+ inputFileName: "inject_emojivoto_pod.input.yml",
+ goldenFileName: "inject_emojivoto_pod.golden.yml",
+ reportFileName: "inject_emojivoto_pod.report",
+ testInjectOptions: defaultOptions,
+ },
+ {
+ inputFileName: "inject_emojivoto_deployment.input.yml",
+ goldenFileName: "inject_emojivoto_deployment_tls.golden.yml",
+ reportFileName: "inject_emojivoto_deployment.report",
+ testInjectOptions: tlsOptions,
+ },
+ {
+ inputFileName: "inject_emojivoto_pod.input.yml",
+ goldenFileName: "inject_emojivoto_pod_tls.golden.yml",
+ reportFileName: "inject_emojivoto_pod.report",
+ testInjectOptions: tlsOptions,
+ },
+ {
+ inputFileName: "inject_emojivoto_deployment_udp.input.yml",
+ goldenFileName: "inject_emojivoto_deployment_udp.golden.yml",
+ reportFileName: "inject_emojivoto_deployment_udp.report",
+ testInjectOptions: defaultOptions,
+ },
  }
 
  for i, tc := range testCases {
@@ -53,13 +105,16 @@ func TestInjectYAML(t *testing.T) {
  }
 
  actualOutput := output.String()
+ expectedOutput := readOptionalTestFile(t, tc.goldenFileName)
+ if expectedOutput != actualOutput {
+ t.Errorf("Result mismatch.\nExpected: %s\nActual: %s", expectedOutput, actualOutput)
+ }
 
- goldenFileBytes, err := ioutil.ReadFile("testdata/" + tc.goldenFileName)
- if err != nil {
- t.Fatalf("Unexpected error: %v", err)
+ actualReport := report.String()
+ expectedReport := readOptionalTestFile(t, tc.reportFileName)
+ if expectedReport != actualReport {
+ t.Errorf("Result mismatch.\nExpected: %s\nActual: %s", expectedReport, actualReport)
  }
- expectedOutput := string(goldenFileBytes)
- diffCompare(t, actualOutput, expectedOutput)
  })
  }
 }
@@ -103,12 +158,15 @@ func TestRunInjectCmd(t *testing.T) {
 
  actualStdOutResult := outBuffer.String()
  expectedStdOutResult := readOptionalTestFile(t, tc.stdOutGoldenFileName)
-
- diffCompare(t, actualStdOutResult, expectedStdOutResult)
+ if expectedStdOutResult != actualStdOutResult {
+ t.Errorf("Result mismatch.\nExpected: %s\nActual: %s", expectedStdOutResult, actualStdOutResult)
+ }
 
  actualStdErrResult := errBuffer.String()
  expectedStdErrResult := readOptionalTestFile(t, tc.stdErrGoldenFileName)
- diffCompare(t, actualStdErrResult, expectedStdErrResult)
+ if expectedStdErrResult != actualStdErrResult {
+ t.Errorf("Result mismatch.\nExpected: %s\nActual: %s", expectedStdErrResult, actualStdErrResult)
+ }
  })
  }
 }

diff --git a/cli/cmd/testdata/inject-filepath/expected/injected_nginx.stderr b/cli/cmd/testdata/inject-filepath/expected/injected_nginx.stderr
@@ -1,6 +1,7 @@
 
 hostNetwork: pods do not use host networking...............................[ok]
 supported: at least one resource injected..................................[ok]
+udp: pod specs do not include UDP ports....................................[ok]
 
 Summary: 1 of 1 YAML document(s) injected
  deployment/nginx

diff --git a/cli/cmd/testdata/inject-filepath/expected/injected_nginx_redis.stderr b/cli/cmd/testdata/inject-filepath/expected/injected_nginx_redis.stderr
@@ -1,13 +1,15 @@
 
 hostNetwork: pods do not use host networking...............................[ok]
 supported: at least one resource injected..................................[ok]
+udp: pod specs do not include UDP ports....................................[ok]
 
 Summary: 1 of 1 YAML document(s) injected
  deployment/redis
 
 
 hostNetwork: pods do not use host networking...............................[ok]
 supported: at least one resource injected..................................[ok]
+udp: pod specs do not include UDP ports....................................[ok]
 
 Summary: 1 of 1 YAML document(s) injected
  deployment/nginx

diff --git a/cli/cmd/testdata/inject-filepath/expected/injected_redis.stderr b/cli/cmd/testdata/inject-filepath/expected/injected_redis.stderr
@@ -1,6 +1,7 @@
 
 hostNetwork: pods do not use host networking...............................[ok]
 supported: at least one resource injected..................................[ok]
+udp: pod specs do not include UDP ports....................................[ok]
 
 Summary: 1 of 1 YAML document(s) injected
  deployment/redis

diff --git a/cli/cmd/testdata/inject_emojivoto_deployment.report b/cli/cmd/testdata/inject_emojivoto_deployment.report
@@ -0,0 +1,8 @@
+
+hostNetwork: pods do not use host networking...............................[ok]
+supported: at least one resource injected..................................[ok]
+udp: pod specs do not include UDP ports....................................[ok]
+
+Summary: 1 of 1 YAML document(s) injected
+ deployment/web
+
diff --git a/cli/cmd/testdata/inject_emojivoto_deployment_controller_name.report b/cli/cmd/testdata/inject_emojivoto_deployment_controller_name.report
@@ -0,0 +1,9 @@
+
+hostNetwork: pods do not use host networking...............................[ok]
+supported: at least one resource injected..................................[ok]
+udp: pod specs do not include UDP ports....................................[ok]
+
+Summary: 2 of 2 YAML document(s) injected
+ deployment/controller
+ deployment/not-controller
+
diff --git a/cli/cmd/testdata/inject_emojivoto_deployment_hostNetwork_false.report b/cli/cmd/testdata/inject_emojivoto_deployment_hostNetwork_false.report
@@ -0,0 +1,8 @@
+
+hostNetwork: pods do not use host networking...............................[ok]
+supported: at least one resource injected..................................[ok]
+udp: pod specs do not include UDP ports....................................[ok]
+
+Summary: 1 of 1 YAML document(s) injected
+ deployment/web
+
diff --git a/cli/cmd/testdata/inject_emojivoto_deployment_hostNetwork_true.report b/cli/cmd/testdata/inject_emojivoto_deployment_hostNetwork_true.report
@@ -0,0 +1,7 @@
+
+hostNetwork: pods do not use host networking...............................[warn] -- deployment/web uses "hostNetwork: true"
+supported: at least one resource injected..................................[warn] -- no supported objects found
+udp: pod specs do not include UDP ports....................................[ok]
+
+Summary: 0 of 1 YAML document(s) injected
+
diff --git a/cli/cmd/testdata/inject_emojivoto_deployment_udp.golden.yml b/cli/cmd/testdata/inject_emojivoto_deployment_udp.golden.yml
@@ -0,0 +1,104 @@
+apiVersion: apps/v1beta1
+kind: Deployment
+metadata:
+ creationTimestamp: null
+ name: web
+ namespace: emojivoto
+spec:
+ replicas: 1
+ selector:
+ matchLabels:
+ app: web-svc
+ strategy: {}
+ template:
+ metadata:
+ annotations:
+ linkerd.io/created-by: linkerd/cli undefined
+ linkerd.io/proxy-version: testinjectversion
+ creationTimestamp: null
+ labels:
+ app: web-svc
+ linkerd.io/control-plane-ns: linkerd
+ linkerd.io/proxy-deployment: web
+ 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:v3
+ name: web-svc
+ ports:
+ - containerPort: 9100
+ hostPort: 9100
+ name: http
+ protocol: UDP
+ resources: {}
+ - env:
+ - name: LINKERD2_PROXY_LOG
+ value: warn,linkerd2_proxy=info
+ - name: LINKERD2_PROXY_BIND_TIMEOUT
+ value: 10s
+ - name: LINKERD2_PROXY_CONTROL_URL
+ value: tcp:https://proxy-api.linkerd.svc.cluster.local:8086
+ - name: LINKERD2_PROXY_CONTROL_LISTENER
+ value: tcp:https://0.0.0.0:4190
+ - name: LINKERD2_PROXY_METRICS_LISTENER
+ value: tcp:https://0.0.0.0:4191
+ - name: LINKERD2_PROXY_PRIVATE_LISTENER
+ value: tcp:https://127.0.0.1:4140
+ - name: LINKERD2_PROXY_PUBLIC_LISTENER
+ value: tcp:https://0.0.0.0:4143
+ - name: LINKERD2_PROXY_POD_NAMESPACE
+ valueFrom:
+ fieldRef:
+ fieldPath: metadata.namespace
+ image: gcr.io/linkerd-io/proxy:testinjectversion
+ imagePullPolicy: IfNotPresent
+ livenessProbe:
+ httpGet:
+ path: /metrics
+ port: 4191
+ initialDelaySeconds: 10
+ name: linkerd-proxy
+ ports:
+ - containerPort: 4143
+ name: linkerd-proxy
+ - containerPort: 4191
+ name: linkerd-metrics
+ readinessProbe:
+ httpGet:
+ path: /metrics
+ port: 4191
+ initialDelaySeconds: 10
+ resources: {}
+ securityContext:
+ runAsUser: 2102
+ terminationMessagePolicy: FallbackToLogsOnError
+ initContainers:
+ - args:
+ - --incoming-proxy-port
+ - "4143"
+ - --outgoing-proxy-port
+ - "4140"
+ - --proxy-uid
+ - "2102"
+ - --inbound-ports-to-ignore
+ - 4190,4191
+ image: gcr.io/linkerd-io/proxy-init:testinjectversion
+ imagePullPolicy: IfNotPresent
+ name: linkerd-init
+ resources: {}
+ securityContext:
+ capabilities:
+ add:
+ - NET_ADMIN
+ privileged: false
+ terminationMessagePolicy: FallbackToLogsOnError
+status: {}
+---