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

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],
@@ -31,19 +33,20 @@ 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)
+ val compiledPath: Option[Regex] = path.map(_.r)
  def uriMatches(uri: String, p: String): Boolean = p.isEmpty || (compiledPath exists { cp =>
  uri match {
  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)
@@ -53,12 +56,29 @@ case class IngressPath(
  }
  }
 
+ def maybeCaptureGroupsToUri(requestUri: String, useCapturingGroupsGlobal: Option[Boolean]): String = {
+ (useCapturingGroupsGlobal, useCapturingGroups) match {
+ case (Some(true), _) | (_, Some(true)) => captureGroupsToUri(requestUri)
+ case (_, _) => requestUri
+ }
+ }
+
+ def captureGroupsToUri(requestUri: String): String = {
+ def captureGroups(pathRegex: Regex, uri: String) =
+ pathRegex.findAllIn(uri).matchData.flatMap(_.subgroups)
+
+ compiledPath match {
+ case Some(p) => captureGroups(p, requestUri).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 +147,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 +158,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,75 @@ 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")
+ val path = IngressPath(host, Some("/prefix/.*"), None, ns.get, "svc1", "80")
  assert(path.matches(host, "/prefix/and-other-stuff"))
  }
 
  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("use path capture groups only when configured") {
+ case class C(uri: String, ingressPath: IngressPath, useCapturingGroupsGlobal: Option[Boolean])
+
+ def ingressPath(path: Option[String], useCapturingGroups: Option[Boolean]): IngressPath =
+ IngressPath(None, path, useCapturingGroups, "namespace", "service", "6666")
+
+ val testCases = Map(
+ "/a/b/0" -> C("/a/b/0", ingressPath(None, None), None),
+ "/a/b/1" -> C("/a/b/1", ingressPath(Some(""), None), None),
+ "/a/b/2" -> C("/a/b/2", ingressPath(Some("/a/b/2"), None), None),
+ "/a/b/3" -> C("/a/b/3", ingressPath(Some("/a/b/3"), None), Some(false)),
+ "/a/b/4" -> C("/a/b/4", ingressPath(Some("/a/b/4"), Some(false)), None),
+ "/a/b/5" -> C("/a/b/5", ingressPath(Some("/a/b/5"), Some(false)), Some(false)),
+
+ "/a/b/uri" -> C("/uri", ingressPath(Some("/a/b/(.*)"), Some(true)), None),
+ "/a/b/uri" -> C("/uri", ingressPath(Some("/a/b/(.*)"), Some(true)), Some(false)),
+ "/a/b/uri" -> C("/uri", ingressPath(Some("/a/b/(.*)"), Some(true)), Some(true)),
+ "/a/b/1234/uri" -> C("/1234/uri", ingressPath(Some("/a/b/([0-9]*)/(.*)"), Some(true)), None),
+ "/a/b/1234/uri" -> C("/1234/uri", ingressPath(Some("/a/b/([0-9]*)/(.*)"), Some(true)), Some(false)),
+ "/a/b/1234/uri" -> C("/1234/uri", ingressPath(Some("/a/b/([0-9]*)/(.*)"), Some(true)), Some(true)),
+ "/a/b/1234/c/uri" -> C("/1234/uri", ingressPath(Some("/a/b/([0-9]*)/c/(.*)"), Some(true)), None),
+ "/a/b/1234/c/uri" -> C("/1234/uri", ingressPath(Some("/a/b/([0-9]*)/c/(.*)"), Some(true)), Some(false)),
+ "/a/b/1234/c/uri" -> C("/1234/uri", ingressPath(Some("/a/b/([0-9]*)/c/(.*)"), Some(true)), Some(true)),
+
+ "/nonexistent/" -> C("/", ingressPath(Some("/a/b/c"), Some(true)), Some(true))
+ )
+
+ testCases foreach {
+ case (requestUri, C(residualUri, ingressPath, globalFlag)) =>
+ val result = ingressPath.maybeCaptureGroupsToUri(requestUri, globalFlag)
+ assert(result == residualUri, s", test case: $requestUri -> $residualUri, $ingressPath, $globalFlag")
+ }
+ }
 }

linkerd/protocol/http/src/main/scala/io/buoyant/linkerd/protocol/http/IngressIdentifier.scala

@@ -17,6 +17,7 @@ class IngressIdentifier(
  pfx: Path,
  baseDtab: () => Dtab,
  namespace: Option[String],
+ useCapturingGroups: Option[Boolean],
  apiClient: Service[http.Request, http.Response]
 ) extends Identifier[Request] {
 
@@ -32,6 +33,7 @@ class IngressIdentifier(
  case Some(a) =>
  val path = pfx ++ Path.Utf8(a.namespace, a.port, a.svc)
  val dst = Dst.Path(path, baseDtab(), Dtab.local)
+ req.uri = a.maybeCaptureGroupsToUri(req.uri, useCapturingGroups)
  Future.value(new IdentifiedRequest(dst, req))
  }
  }
@@ -40,7 +42,8 @@ class IngressIdentifier(
 case class IngressIdentifierConfig(
  host: Option[String],
  port: Option[Port],
- namespace: Option[String]
+ namespace: Option[String],
+ useCapturingGroups: Option[Boolean]
 ) extends HttpIdentifierConfig with ClientConfig {
  @JsonIgnore
  override def portNum: Option[Int] = port.map(_.port)
@@ -50,7 +53,7 @@ case class IngressIdentifierConfig(
  baseDtab: () => Dtab = () => Dtab.base
  ): Identifier[Request] = {
  val client = mkClient(Params.empty).configured(Label("ingress-identifier"))
- new IngressIdentifier(prefix, baseDtab, namespace, client.newService(dst))
+ new IngressIdentifier(prefix, baseDtab, namespace, useCapturingGroups, client.newService(dst))
  }
 }
 

linkerd/protocol/http/src/test/scala/io/buoyant/linkerd/protocol/http/IngressIdentifierConfigTest.scala

@@ -0,0 +1,45 @@
+package io.buoyant.linkerd.protocol.http
+
+import com.twitter.finagle.Path
+import com.twitter.finagle.util.LoadService
+import io.buoyant.config.Parser
+import io.buoyant.linkerd.IdentifierInitializer
+import org.scalatest.FunSuite
+
+class IngressIdentifierConfigTest extends FunSuite {
+
+ test("sanity") {
+ // ensure it doesn't totally blowup
+ val _ = new IngressIdentifierConfig(None, None, None, None).newIdentifier(Path.empty)
+ }
+
+ test("service registration") {
+ assert(LoadService[IdentifierInitializer].exists(_.isInstanceOf[IngressIdentifierInitializer]))
+ }
+
+ test("parse config") {
+ val yaml = s"""
+ |kind: io.l5d.ingress
+ |useCapturingGroups: true
+ |namespace: myNameSpace
+ |host: 127.0.0.1
+ |port: 6666
+ """.stripMargin
+
+ val mapper = Parser.objectMapper(yaml, Iterable(Seq(IngressIdentifierInitializer)))
+ val config = mapper.readValue[IngressIdentifierConfig](yaml)
+
+ assert(config.namespace.isDefined)
+ assert(config.namespace.get == "myNameSpace")
+
+ assert(config.host.isDefined)
+ assert(config.host.get == "127.0.0.1")
+
+ assert(config.port.isDefined)
+ assert(config.port.get.port == 6666)
+
+ assert(config.useCapturingGroups.isDefined)
+ assert(config.useCapturingGroups.get)
+ }
+
+}