Rewrite path using regex capture groups (linkerd#1144)

Problem
Missing feature for ingress identifier path rewriting

Solution
Use regexp capture groups as the rewritten path segments

Validation
Unit testing

Fixes linkerd#1144

finagle/h2/src/main/scala/com/twitter/finagle/buoyant/h2/Message.scala

@@ -90,6 +90,12 @@ trait Request extends Message {
  def authority: String
  def path: String
  override def dup(): Request
+ def transform(
+ scheme: String = scheme,
+ method: Method = method,
+ authority: String = authority,
+ path: String = path
+ ): Request
 }
 
 object Request {
@@ -122,6 +128,15 @@ object Request {
  }
  override def authority = headers.get(Headers.Authority).getOrElse("")
  override def path = headers.get(Headers.Path).getOrElse("/")
+ override def transform(scheme: String, method: Method, authority: String, path: String) = copy(
+ Headers(
+ Headers.Scheme -> scheme,
+ Headers.Method -> method.toString,
+ Headers.Authority -> authority,
+ Headers.Path -> path
+ ),
+ stream
+ )
  }
 }
 

k8s/src/main/scala/io/buoyant/k8s/IngressCache.scala

@@ -6,6 +6,8 @@ import com.twitter.util._
 import java.util.concurrent.atomic.AtomicReference
 import io.buoyant.namer.RichActivity
 
+import scala.util.matching.Regex
+
 case class IngressSpec(
  name: Option[String],
  namespace: Option[String],
@@ -16,13 +18,13 @@ case class IngressSpec(
  val matchingPath = rules.find(_.matches(hostHeader, requestPath))
  (matchingPath, fallbackBackend) match {
  case (Some(path), _) =>
- log.info("k8s found rule matching %s %s: %s", hostHeader.getOrElse(""), requestPath, path)
+ log.info("k8s found rule matching '%s' '%s': '%s'", hostHeader.getOrElse(""), requestPath, path)
  Some(path)
  case (None, Some(default)) =>
- log.info("k8s using default service %s for request %s %s", default, hostHeader.getOrElse(""), requestPath)
+ log.info("k8s using default service '%s' for request '%s' '%s'", default, hostHeader.getOrElse(""), requestPath)
  Some(default)
  case _ =>
- log.info("k8s no suitable rule found in %s for request %s %s", name.getOrElse(""), hostHeader.getOrElse(""), requestPath)
+ log.info("k8s no suitable rule found in '%s' for request '%s' '%s'", name.getOrElse(""), hostHeader.getOrElse(""), requestPath)
  None
  }
  }
@@ -31,34 +33,67 @@ case class IngressSpec(
 case class IngressPath(
  host: Option[String] = None,
  path: Option[String] = None,
+ useCapturingGroups: Option[Boolean],
  namespace: String,
  svc: String,
  port: String
 ) {
- val compiledPath = path.map(_.r)
- def uriMatches(uri: String, p: String): Boolean = p.isEmpty || (compiledPath exists { cp =>
+ val compiledPath: Option[Regex] = path.map(_.r)
+
+ def uriMatches(uri: String, p: String): Boolean = p.isEmpty || (compiledPath exists { (cp: Regex) =>
  uri match {
- case cp() => true
+ case cp(_*) => true
  case _ => false
  }
  })
 
- def matches(hostHeader: Option[String], requestPath: String) = {
+ def matches(hostHeader: Option[String], requestPath: String): Boolean = {
  (host, path) match {
- case (Some(host), Some(p)) =>
- uriMatches(requestPath, p) && hostHeader.contains(host)
- case (Some(host), None) => hostHeader.contains(host)
+ case (Some(h), Some(p)) =>
+ uriMatches(requestPath, p) && hostHeader.contains(h)
+ case (Some(h), None) => hostHeader.contains(h)
  case (None, Some(p)) => uriMatches(requestPath, p)
  case (None, None) => true
  }
  }
 
+ def rewrite(requestUri: String, useCapturingGroupsGlobal: Boolean): String = {
+ (useCapturingGroupsGlobal, useCapturingGroups) match {
+ case (_, Some(true)) => captureGroupsToUri(requestUri)
+ case (true, None) => captureGroupsToUri(requestUri)
+ case (_, _) => requestUri
+ }
+ }
+
+ def hasNoCapturingGroups: Boolean = !hasCapturingGroups
+
+ def hasCapturingGroups: Boolean = compiledPath match {
+ case Some(r) if r.pattern.matcher("").groupCount() > 0 => true
+ case _ => false
+ }
+
+ private def captureGroupsToUri(requestUri: String): String = {
+ def captureGroups(pathRegex: Regex, uri: String) = {
+ if (hasNoCapturingGroups)
+ log.warning("Attempting to match capturing groups, but the pattern has none: '%s'", compiledPath.getOrElse(""))
+ for { m <- pathRegex findAllMatchIn uri; sub <- m.subgroups } yield sub
+ }
+
+ compiledPath match {
+ case Some(p) => captureGroups(p, requestUri)
+ .map(_.stripPrefix("/"))
+ .map(_.stripSuffix("/"))
+ .mkString("/", "/", "")
+ case None => requestUri
+ }
+ }
 }
 
 object IngressCache {
  type IngressState = Activity.State[Seq[IngressSpec]]
  val annotationKey = "kubernetes.io/ingress.class"
  val annotationValue = "linkerd"
+ val useCapturingGroupsKey = "linkerd.io/use-capturing-groups"
 
  private[k8s] def getMatchingPath(hostHeader: Option[String], requestPath: String, ingresses: Seq[IngressSpec]): Option[IngressPath] =
  ingresses
@@ -127,6 +162,8 @@ class IngressCache(namespace: Option[String], apiClient: Service[Request, Respon
  case _ =>
  }
 
+ val useCapturingGroups = annotations.get(useCapturingGroupsKey).map(_.toBoolean)
+
  val namespace = ingress.metadata.flatMap(meta => meta.namespace)
  ingress.spec.map { spec =>
  val paths = for (
@@ -136,10 +173,10 @@ class IngressCache(namespace: Option[String], apiClient: Service[Request, Respon
  http <- rule.http.toSeq;
  path <- http.paths
  ) yield {
- IngressPath(rule.host, path.path, namespace.getOrElse("default"), path.backend.serviceName, path.backend.servicePort)
+ IngressPath(rule.host, path.path, useCapturingGroups, namespace.getOrElse("default"), path.backend.serviceName, path.backend.servicePort)
  }
 
- val fallback = spec.backend.map(b => IngressPath(None, None, namespace.getOrElse("default"), b.serviceName, b.servicePort))
+ val fallback = spec.backend.map(b => IngressPath(None, None, useCapturingGroups, namespace.getOrElse("default"), b.serviceName, b.servicePort))
  IngressSpec(ingress.metadata.flatMap(_.name), namespace, fallback, paths)
  }
  }

k8s/src/test/scala/io/buoyant/k8s/IngressCacheTest.scala

@@ -9,41 +9,138 @@ class IngressCacheTest extends FunSuite with Awaits {
 
  test("on multiple path matches, return first match") {
  val paths = Seq(
- IngressPath(host, Some("/path"), ns.get, "primary-svc", "80"),
- IngressPath(host, Some("/path"), ns.get, "secondary-svc", "80")
+ IngressPath(host, Some("/path"), None, ns.get, "primary-svc", "80"),
+ IngressPath(host, Some("/path"), None, ns.get, "secondary-svc", "80")
  )
  val spec = IngressSpec(Some("my-ingress"), ns, None, paths)
  val matchingPath = IngressCache.getMatchingPath(host, "/path", Seq(spec))
  assert(matchingPath.get.svc == "primary-svc")
  }
 
  test("on multiple host matches, return first match") {
- val resource1 = IngressSpec(Some("polar-bear1"), ns, None, Seq(IngressPath(host, None, ns.get, "svc1", "80")))
- val resource2 = IngressSpec(Some("polar-bear2"), ns, None, Seq(IngressPath(host, None, ns.get, "svc2", "80")))
+ val resource1 = IngressSpec(Some("polar-bear1"), ns, None, Seq(IngressPath(host, None, None, ns.get, "svc1", "80")))
+ val resource2 = IngressSpec(Some("polar-bear2"), ns, None, Seq(IngressPath(host, None, None, ns.get, "svc2", "80")))
  val matchingPath = IngressCache.getMatchingPath(host, "/path", Seq(resource1, resource2))
  assert(matchingPath.get.svc == "svc1")
  }
 
  test("match on path regex") {
- val path = IngressPath(host, Some("/prefix/.*"), ns.get, "svc1", "80")
- assert(path.matches(host, "/prefix/and-other-stuff"))
+ val testCases = Map(
+ "/prefix" -> ingressPath(Some("/prefix.*")),
+ "/prefix" -> ingressPath(Some("/prefix/?.*")),
+ "/prefix/" -> ingressPath(Some("/prefix/.*")),
+ "/prefix/and-other-stuff" -> ingressPath(Some("/prefix/.*")),
+ "/path1/?foo=bar" -> ingressPath(Some("/path1/?.*")),
+ "/path1?foo=bar/" -> ingressPath(Some("/path1/?.*")),
+ "/path1/more/?foo=bar" -> ingressPath(Some("/path1/?.*"))
+ )
+ testCases foreach {
+ case (requestUri, ingressPath) =>
+ assert(ingressPath.matches(host, requestUri), s", '$requestUri' -> '${ingressPath.path.get}'")
+ }
+ }
+
+ test("match on path regex with capturing group") {
+ val testCases = Map(
+ "/prefix" -> ingressPath(Some("/prefix(.*)")),
+ "/prefix" -> ingressPath(Some("/prefix(/?.*)")),
+ "/prefix/" -> ingressPath(Some("/prefix(/?.*)")),
+ "/prefix/and-other-stuff" -> ingressPath(Some("/prefix/(.*)")),
+ "/path1/?foo=bar" -> ingressPath(Some("/path1(/?.*)")),
+ "/path1?foo=bar/" -> ingressPath(Some("/path1(/?.*)")),
+ "/path1/more/?foo=bar" -> ingressPath(Some("/path1(/?.*)"))
+ )
+ testCases foreach {
+ case (requestUri, ingressPath) =>
+ assert(ingressPath.matches(host, requestUri), s", '$requestUri' -> '${ingressPath.path.get}'")
+ }
  }
 
  test("match / with reqs that have empty paths only") {
- val path = IngressPath(host, Some("/"), ns.get, "svc1", "80")
+ val path = IngressPath(host, Some("/"), None, ns.get, "svc1", "80")
  assert(path.matches(host, "/"))
  assert(!path.matches(host, "/foo"))
  }
 
  test("match empty string with all reqs") {
- val path = IngressPath(host, Some(""), ns.get, "svc1", "80")
+ val path = IngressPath(host, Some(""), None, ns.get, "svc1", "80")
  assert(path.matches(host, "/"))
  assert(path.matches(host, "/foo"))
  }
 
  test("match omitted path with all reqs") {
- val path = IngressPath(host, None, ns.get, "svc1", "80")
+ val path = IngressPath(host, None, None, ns.get, "svc1", "80")
  assert(path.matches(host, "/"))
  assert(path.matches(host, "/foo"))
  }
+
+ test("don't rewrite path when not enabled") {
+ val testCases = Map(
+ "/a/b/0" -> C("/a/b/0", ingressPath(None, None), false),
+ "/a/b/1" -> C("/a/b/1", ingressPath(Some(""), None), false),
+ "/a/b/2" -> C("/a/b/2", ingressPath(Some("/a/b/2"), None), false),
+ "/a/b/3" -> C("/a/b/3", ingressPath(Some("/a/b/3"), Some(false)), false),
+ "/a/b/4" -> C("/a/b/4", ingressPath(Some("/a/b/4"), Some(false)), true)
+ )
+
+ testCases foreach {
+ case (requestUri, C(residualUri, ingressPath, globalFlag)) =>
+ val result = ingressPath.rewrite(requestUri, globalFlag)
+ assert(result == residualUri, s", test case: $requestUri -> $residualUri, $ingressPath, $globalFlag")
+ }
+ }
+
+ test("rewrite path when enabled") {
+ val testCases = Map(
+ "/a/b/uri" -> C("/uri", ingressPath(Some("/a/b/(.*)"), Some(true)), false),
+ "/a/b/uri" -> C("/uri", ingressPath(Some("/a/b/(.*)"), Some(true)), true),
+ "/a/b/uri" -> C("/uri", ingressPath(Some("/a/b/(.*)"), None), true)
+ )
+
+ testCases foreach {
+ case (requestUri, C(residualUri, ingressPath, globalFlag)) =>
+ val result = ingressPath.rewrite(requestUri, globalFlag)
+ assert(result == residualUri, s", test case: $requestUri -> $residualUri, $ingressPath, $globalFlag")
+ }
+ }
+
+ test("rewrite path using capture groups") {
+ val testCases = Map(
+ "/nonexistent/" -> C("/", ingressPath(Some("/a/b/c"))),
+ "/a/b/c/" -> C("/", ingressPath(Some("/a/b/c"))),
+ "/a/b/c/" -> C("/a/b/c", ingressPath(Some("(/a/b/c)"))),
+
+ "/a/b/c" -> C("/c", ingressPath(Some("/a/b/(.*)"))),
+ "/a/b/1234/c" -> C("/1234/c", ingressPath(Some("/a/b/([0-9]*)/(.*)"))),
+ "/a/b/1234/c/c" -> C("/1234/c", ingressPath(Some("/a/b/([0-9]*)/c/(.*)"))),
+ "/a/b/1234/c/c/d" -> C("/1234/c", ingressPath(Some("/a/b/([0-9]*)/c/(.*)/d"))),
+
+ "/a/b/c/" -> C("/c", ingressPath(Some("/a/b(.*)"))),
+ "/a/b/c/" -> C("/c", ingressPath(Some("/a/b/(.*)"))),
+ "/a/b/c/1" -> C("/c/1", ingressPath(Some("/a/b(/.*)"))),
+ "/a/b/c/2" -> C("/c", ingressPath(Some("/a/b/(.*/)"))),
+ "/a/b/c/3" -> C("/c", ingressPath(Some("/a/b(/.*/)"))),
+ "/a/b/c/4" -> C("/", ingressPath(Some("/a/b/(/.*/)"))),
+ "/a" -> C("/", ingressPath(Some("/a(/?.*)"))),
+ "/a/" -> C("/", ingressPath(Some("/a(/?.*)"))),
+ "/a/b" -> C("/b", ingressPath(Some("/a(/?.*)"))),
+ "/a/b/" -> C("/b", ingressPath(Some("/a(/?.*)"))),
+ "/a/b/c" -> C("/b/c", ingressPath(Some("/a(/?.*)"))),
+ "/a/b/c/" -> C("/b/c", ingressPath(Some("/a(/?.*)"))),
+ "/a/b/c/d" -> C("/b/c/d", ingressPath(Some("/a(/?.*)"))),
+ "/a/b/c/d/" -> C("/b/c/d", ingressPath(Some("/a(/?.*)"))),
+ "/a/b/?foo=bar#tag" -> C("/b/?foo=bar#tag", ingressPath(Some("/a(/?.*)")))
+ )
+
+ testCases foreach {
+ case (requestUri, C(residualUri, ingressPath, globalFlag)) =>
+ val result = ingressPath.rewrite(requestUri, globalFlag)
+ assert(result == residualUri, s", test case: $requestUri -> $residualUri, $ingressPath, $globalFlag")
+ }
+ }
+
+ case class C(uri: String, ingressPath: IngressPath, useCapturingGroupsGlobal: Boolean = true)
+
+ def ingressPath(path: Option[String], useCapturingGroups: Option[Boolean] = Some(true)): IngressPath =
+ IngressPath(host, path, useCapturingGroups, ns.get, "svc1", "80")
 }

linkerd/docs/protocol-h2.md

@@ -251,6 +251,56 @@ Key | Default Value | Description
 namespace | (all) | The Kubernetes namespace where the ingress resources are deployed. If not specified, linkerd will watch all namespaces.
 host | `localhost` | The Kubernetes master host.
 port | `8001` | The Kubernetes master port.
+useCapturingGroups | `false` | Enable path regular expression capturing groups feature.
+
+#### Capturing groups
+
+> This example watches all ingress resources in all namespaces and uses capturing group feature (using configuration):
+
+```yaml
+routers:
+- protocol: http
+ identifier:
+ kind: io.l5d.ingress
+ useCapturingGroups: true
+ servers:
+ - port: 4140
+ dtab: /svc => /#/io.l5d.k8s
+
+namers:
+- kind: io.l5d.k8s
+```
+
+> An example ingress resource watched by the linkerd ingress controller (using annotation):
+
+```yaml
+apiVersion: extensions/v1beta1
+kind: Ingress
+metadata:
+ name: my-first-ingress
+ namespace: default
+annotations:
+ kubernetes.io/ingress.class: "linkerd"
+ linkerd.io/use-capturing-groups: "true"
+spec:
+ rules:
+ - http:
+ paths:
+ - path: /testpath(/?.*)
+ backend:
+ serviceName: test
+ servicePort: 80
+```
+
+> So an HTTP/2 request like `https://localhost:4140/testpath/1/abcd` would have path rewritten to `/1/abcd`
+
+Ingress path | Request path | Request path to kubernetes service
+------------ | ------------ | ----------------------------------
+`/testpath/(.+)` | `/testpath/a/b/1234` | `/a/b/1234`
+`/testpath/(.+)/b/([0-9]+)` | `/testpath/a/b/1234` | `/a/1234`
+`/testpath/(.+)/(b)/(.+)` | `/testpath/a/b/1234` | `/a/b/1234`
+`/.+/(.+)/(b)/(.+)` | `/testpath/a/b/1234` | `/a/b/1234`
+`/testpath/(.+)` | `/wrong/path` | `/`
 
 #### Identifier Path Parameters