Simplify Opaque Port Manifests

[deusxanima]

What problem are you trying to solve?

Currently our protocol detection opaque port markings are exclusive (i.e., we mark which ports we want to exclude from protocol detection), which creates issues for users who have to exclude massive port-ranges for integration with other infrastructure components such as syslog, for example. Adding massive ranges expands the manifest and we've seen issues where it also creates problems for the k8s api, not to mention the additional memory pressure on the proxies themselves which has to be accounted for.

[dwilliams782]

Just incase I'm misunderstanding, are you aware that the opaque-port annotation accepts a range, so config.linkerd.io/opaque-ports: 1-65535 is a valid config?

[deusxanima]

@dwilliams782 good question. The issue presents specifically when port-ranges are used. With massive ranges each port in the range is added to the manifest which is then expanded to the point where it can 1. make the config and annotations a nightmare to manage, 2. increase proxy mem usage, and 3. overwhelm the k8s api due to the massive character count that is added to some events/requests.

[hawkw]

Proxy PR linkerd/linkerd2-proxy#2395 changes the proxy to accept port ranges in the LINKERD2_PROXY_INBOUND_PORTS_DISABLE_PROTOCOL_DETECTION environment variable. Once this PR merges, we'll also need to change the proxy-injector to pass the ranges through when populating this environment variable, rather than populating it with each individual port in the range. That should fix this issue!

[hawkw]

We've now merged the proxy release that modifies the proxy to accept ranges in the environment variable. I'm working on modifying the proxy-injector to actually emit ranges when patching a manifest.

This is somewhat more complex than simply removing code that turns a list of ranges into a list of individual port numbers, though, because the proxy-injector also performs filtering of the opaque port list so that it includes only those ports actually exposed by the Service:

linkerd2/pkg/inject/inject.go

Lines 376 to 427 in 5f7bbba

	func (conf *ResourceConfig) CreateOpaquePortsPatch() ([]byte, error) {
	if conf.HasWorkloadAnnotation(k8s.ProxyOpaquePortsAnnotation) {
	// The workload already has the opaque ports annotation so a patch
	// does not need to be created.
	return nil, nil
	}
	opaquePorts, ok := conf.GetConfigAnnotation(k8s.ProxyOpaquePortsAnnotation)
	if ok {
	// The workload's namespace has the opaque ports annotation, so it
	// should inherit that value. A patch is created which adds that
	// list.
	return conf.CreateAnnotationPatch(opaquePorts)
	}
	// Both the workload and the namespace do not have the annotation so a
	// patch is created which adds the default list.
	defaultPorts := strings.Split(conf.GetValues().Proxy.OpaquePorts, ",")
	var filteredPorts []string
	if conf.IsPod() {
	// The workload is a pod so only add the default opaque ports that it
	// exposes as container ports.
	filteredPorts = conf.FilterPodOpaquePorts(defaultPorts)
	} else if conf.IsService() {
	// The workload is a service so only add the default opaque ports that
	// are exposed as a service port, or targeted as a targetPort.
	service := conf.workload.obj.(*corev1.Service)
	for _, p := range service.Spec.Ports {
	port := strconv.Itoa(int(p.Port))
	if p.TargetPort.Type == 0 && p.TargetPort.IntVal == 0 {
	// The port's targetPort is not set, so add the port if is
	// opaque by default. Checking that targetPort is not set
	// avoids marking a port as opaque if it targets a port that
	// not opaque (e.g. port=3306 and targetPort=80; 3306 should
	// not be opaque)
	if util.ContainsString(port, defaultPorts) {
	filteredPorts = append(filteredPorts, port)
	}
	} else if util.ContainsString(strconv.Itoa(int(p.TargetPort.IntVal)), defaultPorts) {
	// The port's targetPort is set; if it is opaque then port
	// should also be opaque.
	filteredPorts = append(filteredPorts, port)
	}
	}
	}
	if len(filteredPorts) == 0 {
	// There are no default opaque ports to add so a patch does not need
	// to be created.
	return nil, nil
	}
	ports := strings.Join(filteredPorts, ",")
	return conf.CreateAnnotationPatch(ports)
	}