fix expose paths (#19257)

When testing adding http probes to apps, I ran into some issues which I fixed here: - The listener should be listening on the exposed listener port, updated that. - The listener and route names were pointing to the path of the exposed path. In my test, the path was "/" resulting in an empty string path. Also, the path may not be unique across exposed path listeners, so I decided to use the path+exposed port as the unique identifier.
hashicorp · Oct 17, 2023 · 51b58cd · 51b58cd
1 parent 9b719e6
commit 51b58cd
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 32 deletions.
diff --git a/internal/mesh/internal/controllers/sidecarproxy/builder/expose_paths.go b/internal/mesh/internal/controllers/sidecarproxy/builder/expose_paths.go
@@ -29,7 +29,7 @@ func (b *Builder) buildExposePaths(workload *pbcatalog.Workload) {
 }
 
 func (b *Builder) addExposePathsListener(workload *pbcatalog.Workload, exposePath *pbmesh.ExposePath) *ListenerBuilder {
-	listenerName := fmt.Sprintf("exposed_path_%s", exposePathName(exposePath))
+	listenerName := exposePathListenerName(exposePath)
 
 	listener := &pbproxystate.Listener{
 		Name:      listenerName,
@@ -44,7 +44,7 @@ func (b *Builder) addExposePathsListener(workload *pbcatalog.Workload, exposePat
 	listener.BindAddress = &pbproxystate.Listener_HostPort{
 		HostPort: &pbproxystate.HostPortAddress{
 			Host: meshAddress.Host,
-			Port: exposePath.LocalPathPort,
+			Port: exposePath.ListenerPort,
 		},
 	}
 
@@ -55,7 +55,7 @@ func (b *ListenerBuilder) addExposePathsRouter(exposePath *pbmesh.ExposePath) *L
 	if b.listener == nil {
 		return b
 	}
-	destinationName := exposePathDestinationName(exposePath)
+	destinationName := exposePathRouteName(exposePath)
 
 	var l7Protocol pbproxystate.L7Protocol
 
@@ -88,7 +88,7 @@ func (b *ListenerBuilder) addExposePathsRouter(exposePath *pbmesh.ExposePath) *L
 }
 
 func (b *Builder) addExposePathsRoute(exposePath *pbmesh.ExposePath, clusterName string) *Builder {
-	routeName := exposePathDestinationName(exposePath)
+	routeName := exposePathRouteName(exposePath)
 	routeRule := &pbproxystate.RouteRule{
 		Match: &pbproxystate.RouteMatch{
 			PathMatch: &pbproxystate.PathMatch{
@@ -120,12 +120,22 @@ func (b *Builder) addExposePathsRoute(exposePath *pbmesh.ExposePath, clusterName
 
 func exposePathName(exposePath *pbmesh.ExposePath) string {
 	r := regexp.MustCompile(`[^a-zA-Z0-9]+`)
-	return r.ReplaceAllString(exposePath.Path, "")
+	// The regex removes anything not a letter or number from the path.
+	path := r.ReplaceAllString(exposePath.Path, "")
+	return path
 }
 
-func exposePathDestinationName(exposePath *pbmesh.ExposePath) string {
-	path := exposePathName(exposePath)
-	return fmt.Sprintf("exposed_path_filter_%s_%d", path, exposePath.ListenerPort)
+func exposePathListenerName(exposePath *pbmesh.ExposePath) string {
+	// The path could be empty, so the unique name for this exposed path is the path and listener port.
+	pathPort := fmt.Sprintf("%s%d", exposePathName(exposePath), exposePath.ListenerPort)
+	listenerName := fmt.Sprintf("exposed_path_%s", pathPort)
+	return listenerName
+}
+
+func exposePathRouteName(exposePath *pbmesh.ExposePath) string {
+	// The path could be empty, so the unique name for this exposed path is the path and listener port.
+	pathPort := fmt.Sprintf("%s%d", exposePathName(exposePath), exposePath.ListenerPort)
+	return fmt.Sprintf("exposed_path_route_%s", pathPort)
 }
 
 func exposePathClusterName(exposePath *pbmesh.ExposePath) string {

diff --git a/...nal/mesh/internal/controllers/sidecarproxy/builder/testdata/source/l7-expose-paths.golden b/...nal/mesh/internal/controllers/sidecarproxy/builder/testdata/source/l7-expose-paths.golden
@@ -107,16 +107,16 @@
         "direction": "DIRECTION_INBOUND",
         "hostPort": {
           "host": "10.0.0.1",
-          "port": 9090
+          "port": 1234
         },
-        "name": "exposed_path_health",
+        "name": "exposed_path_health1234",
         "routers": [
           {
             "l7": {
               "route": {
-                "name": "exposed_path_filter_health_1234"
+                "name": "exposed_path_route_health1234"
               },
-              "statPrefix": "exposed_path_filter_health_1234",
+              "statPrefix": "exposed_path_route_health1234",
               "staticRoute": true
             }
           }
@@ -126,31 +126,31 @@
         "direction": "DIRECTION_INBOUND",
         "hostPort": {
           "host": "10.0.0.1",
-          "port": 9091
+          "port": 1235
         },
-        "name": "exposed_path_GetHealth",
+        "name": "exposed_path_GetHealth1235",
         "routers": [
           {
             "l7": {
               "protocol": "L7_PROTOCOL_HTTP2",
               "route": {
-                "name": "exposed_path_filter_GetHealth_1235"
+                "name": "exposed_path_route_GetHealth1235"
               },
-              "statPrefix": "exposed_path_filter_GetHealth_1235",
+              "statPrefix": "exposed_path_route_GetHealth1235",
               "staticRoute": true
             }
           }
         ]
       }
     ],
     "routes": {
-      "exposed_path_filter_GetHealth_1235": {
+      "exposed_path_route_GetHealth1235": {
         "virtualHosts": [
           {
             "domains": [
               "*"
             ],
-            "name": "exposed_path_filter_GetHealth_1235",
+            "name": "exposed_path_route_GetHealth1235",
             "routeRules": [
               {
                 "destination": {
@@ -168,13 +168,13 @@
           }
         ]
       },
-      "exposed_path_filter_health_1234": {
+      "exposed_path_route_health1234": {
         "virtualHosts": [
           {
             "domains": [
               "*"
             ],
-            "name": "exposed_path_filter_health_1234",
+            "name": "exposed_path_route_health1234",
             "routeRules": [
               {
                 "destination": {

diff --git a/...nal/controllers/sidecarproxy/builder/testdata/source/local-and-inbound-connections.golden b/...nal/controllers/sidecarproxy/builder/testdata/source/local-and-inbound-connections.golden
@@ -169,16 +169,16 @@
         "direction": "DIRECTION_INBOUND",
         "hostPort": {
           "host": "10.0.0.1",
-          "port": 9090
+          "port": 1234
         },
-        "name": "exposed_path_health",
+        "name": "exposed_path_health1234",
         "routers": [
           {
             "l7": {
               "route": {
-                "name": "exposed_path_filter_health_1234"
+                "name": "exposed_path_route_health1234"
               },
-              "statPrefix": "exposed_path_filter_health_1234",
+              "statPrefix": "exposed_path_route_health1234",
               "staticRoute": true
             }
           }
@@ -188,31 +188,31 @@
         "direction": "DIRECTION_INBOUND",
         "hostPort": {
           "host": "10.0.0.1",
-          "port": 9091
+          "port": 1235
         },
-        "name": "exposed_path_GetHealth",
+        "name": "exposed_path_GetHealth1235",
         "routers": [
           {
             "l7": {
               "protocol": "L7_PROTOCOL_HTTP2",
               "route": {
-                "name": "exposed_path_filter_GetHealth_1235"
+                "name": "exposed_path_route_GetHealth1235"
               },
-              "statPrefix": "exposed_path_filter_GetHealth_1235",
+              "statPrefix": "exposed_path_route_GetHealth1235",
               "staticRoute": true
             }
           }
         ]
       }
     ],
     "routes": {
-      "exposed_path_filter_GetHealth_1235": {
+      "exposed_path_route_GetHealth1235": {
         "virtualHosts": [
           {
             "domains": [
               "*"
             ],
-            "name": "exposed_path_filter_GetHealth_1235",
+            "name": "exposed_path_route_GetHealth1235",
             "routeRules": [
               {
                 "destination": {
@@ -230,13 +230,13 @@
           }
         ]
       },
-      "exposed_path_filter_health_1234": {
+      "exposed_path_route_health1234": {
         "virtualHosts": [
           {
             "domains": [
               "*"
             ],
-            "name": "exposed_path_filter_health_1234",
+            "name": "exposed_path_route_health1234",
             "routeRules": [
               {
                 "destination": {