v2: various fixes to make K8s tproxy multiport acceptance tests and m…

…anual explicit upstreams (single port) tests pass (#18874) Adding coauthors who mobbed/paired at various points throughout last week. Co-authored-by: Dan Stough <[email protected]> Co-authored-by: Iryna Shustava <[email protected]> Co-authored-by: John Murret <[email protected]> Co-authored-by: Michael Zalimeni <[email protected]> Co-authored-by: Ashwin Venkatesh <[email protected]> Co-authored-by: Michael Wilkerson <[email protected]>
hashicorp · Sep 20, 2023 · 3a2e620 · 3a2e620
1 parent 1a3081a
commit 3a2e620
Show file tree

Hide file tree

Showing 30 changed files with 187 additions and 54 deletions.
diff --git a/agent/grpc-external/services/dataplane/get_envoy_bootstrap_params.go b/agent/grpc-external/services/dataplane/get_envoy_bootstrap_params.go
@@ -59,7 +59,6 @@ func (s *Server) GetEnvoyBootstrapParams(ctx context.Context, req *pbdataplane.G
 			Tenancy: &pbresource.Tenancy{
 				Namespace: req.Namespace,
 				Partition: req.Partition,
-				PeerName:  "local",
 			},
 			Type: catalog.WorkloadType,
 		}
@@ -69,6 +68,7 @@ func (s *Server) GetEnvoyBootstrapParams(ctx context.Context, req *pbdataplane.G
 		if err != nil {
 			// This error should already include the gRPC status code and so we don't need to wrap it
 			// in status.Error.
+			logger.Error("Error looking up workload", "error", err)
 			return nil, err
 		}
 		var workload pbcatalog.Workload
@@ -93,6 +93,7 @@ func (s *Server) GetEnvoyBootstrapParams(ctx context.Context, req *pbdataplane.G
 			Type:    mesh.ProxyConfigurationType,
 		})
 		if err != nil {
+			logger.Error("Error looking up proxyConfiguration", "error", err)
 			return nil, err
 		}
 

diff --git a/agent/xds/delta.go b/agent/xds/delta.go
@@ -116,7 +116,13 @@ func getEnvoyConfiguration(proxySnapshot proxysnapshot.ProxySnapshot, logger hcl
 		)
 		c := proxySnapshot.(*proxytracker.ProxyState)
 		logger.Trace("ProxyState", c)
-		return generator.AllResourcesFromIR(c)
+		resources, err := generator.AllResourcesFromIR(c)
+		if err != nil {
+			logger.Error("error generating resources from proxy state template", "err", err)
+			return nil, err
+		}
+		logger.Trace("generated resources from proxy state template", "resources", resources)
+		return resources, nil
 	default:
 		return nil, errors.New("proxysnapshot must be of type ProxyState or ConfigSnapshot")
 	}
@@ -428,9 +434,8 @@ func newResourceIDFromEnvoyNode(node *envoy_config_core_v3.Node) *pbresource.ID
 		Tenancy: &pbresource.Tenancy{
 			Namespace: entMeta.NamespaceOrDefault(),
 			Partition: entMeta.PartitionOrDefault(),
-			PeerName:  "local",
 		},
-		Type: mesh.ProxyStateTemplateV1AlphaType,
+		Type: mesh.ProxyStateTemplateType,
 	}
 }
 

diff --git a/agent/xdsv2/cluster_resources.go b/agent/xdsv2/cluster_resources.go
@@ -376,6 +376,8 @@ func addEnvoyLBToCluster(dynamicConfig *pbproxystate.DynamicEndpointGroupConfig,
 }
 
 // TODO(proxystate): In a future PR this will create clusters and add it to ProxyResources.proxyState
+// Currently, we do not traverse the listener -> endpoint paths and instead just generate each resource by iterating
+// through its top level map. In the future we want to traverse these paths to ensure each listener has a cluster, etc.
 func (pr *ProxyResources) makeEnvoyClusterFromL4Destination(l4 *pbproxystate.L4Destination) error {
 	return nil
 }
diff --git a/agent/xdsv2/listener_resources.go b/agent/xdsv2/listener_resources.go
@@ -540,31 +540,30 @@ func (pr *ProxyResources) makeEnvoyTLSParameters(defaultParams *pbproxystate.TLS
 }
 
 func (pr *ProxyResources) makeEnvoyTransportSocket(ts *pbproxystate.TransportSocket) (*envoy_core_v3.TransportSocket, error) {
-	// TODO(JM): did this just make tests pass.  Figure out whether proxyState.Tls will always be available.
-	if pr.proxyState.Tls == nil {
-		return nil, nil
-	}
 	if ts == nil {
 		return nil, nil
 	}
 	commonTLSContext := &envoy_tls_v3.CommonTlsContext{}
+	if ts.AlpnProtocols != nil {
+		commonTLSContext.AlpnProtocols = ts.AlpnProtocols
+	}
 
 	// Create connection TLS. Listeners should only look at inbound TLS.
 	switch ts.ConnectionTls.(type) {
 	case *pbproxystate.TransportSocket_InboundMesh:
 		downstreamContext := &envoy_tls_v3.DownstreamTlsContext{}
 		downstreamContext.CommonTlsContext = commonTLSContext
 		// Set TLS Parameters.
-		tlsParams := pr.makeEnvoyTLSParameters(pr.proxyState.Tls.InboundTlsParameters, ts.TlsParameters)
-		commonTLSContext.TlsParams = tlsParams
+		if pr.proxyState.Tls != nil {
+			tlsParams := pr.makeEnvoyTLSParameters(pr.proxyState.Tls.InboundTlsParameters, ts.TlsParameters)
+			commonTLSContext.TlsParams = tlsParams
+		} else {
+			commonTLSContext.TlsParams = &envoy_tls_v3.TlsParameters{}
+		}
 
 		// Set the certificate config on the tls context.
 		// For inbound mesh, we need to add the identity certificate
 		// and the validation context for the mesh depending on the provided trust bundle names.
-		if pr.proxyState.Tls == nil {
-			// if tls is nil but connection tls is provided, then the proxy state is misconfigured
-			return nil, fmt.Errorf("proxyState.Tls is required to generate router's transport socket")
-		}
 		im := ts.ConnectionTls.(*pbproxystate.TransportSocket_InboundMesh).InboundMesh
 		leaf, ok := pr.proxyState.LeafCertificates[im.IdentityKey]
 		if !ok {
@@ -640,9 +639,13 @@ func (pr *ProxyResources) makeEnvoyTransportSocket(ts *pbproxystate.TransportSoc
 	case *pbproxystate.TransportSocket_InboundNonMesh:
 		downstreamContext := &envoy_tls_v3.DownstreamTlsContext{}
 		downstreamContext.CommonTlsContext = commonTLSContext
-		// Set TLS Parameters
-		tlsParams := pr.makeEnvoyTLSParameters(pr.proxyState.Tls.InboundTlsParameters, ts.TlsParameters)
-		commonTLSContext.TlsParams = tlsParams
+		// Set TLS Parameters.
+		if pr.proxyState.Tls != nil {
+			tlsParams := pr.makeEnvoyTLSParameters(pr.proxyState.Tls.InboundTlsParameters, ts.TlsParameters)
+			commonTLSContext.TlsParams = tlsParams
+		} else {
+			commonTLSContext.TlsParams = &envoy_tls_v3.TlsParameters{}
+		}
 		// For non-mesh, we don't care about validation context as currently we don't support mTLS for non-mesh connections.
 		nonMeshTLS := ts.ConnectionTls.(*pbproxystate.TransportSocket_InboundNonMesh).InboundNonMesh
 		err := pr.addNonMeshCertConfig(commonTLSContext, nonMeshTLS)
@@ -657,15 +660,15 @@ func (pr *ProxyResources) makeEnvoyTransportSocket(ts *pbproxystate.TransportSoc
 	case *pbproxystate.TransportSocket_OutboundMesh:
 		upstreamContext := &envoy_tls_v3.UpstreamTlsContext{}
 		upstreamContext.CommonTlsContext = commonTLSContext
-		// Set TLS Parameters
-		tlsParams := pr.makeEnvoyTLSParameters(pr.proxyState.Tls.OutboundTlsParameters, ts.TlsParameters)
-		commonTLSContext.TlsParams = tlsParams
+		// Set TLS Parameters.
+		if pr.proxyState.Tls != nil {
+			tlsParams := pr.makeEnvoyTLSParameters(pr.proxyState.Tls.OutboundTlsParameters, ts.TlsParameters)
+			commonTLSContext.TlsParams = tlsParams
+		} else {
+			commonTLSContext.TlsParams = &envoy_tls_v3.TlsParameters{}
+		}
 		// For outbound mesh, we need to insert the mesh identity certificate
 		// and the validation context for the mesh depending on the provided trust bundle names.
-		if pr.proxyState.Tls == nil {
-			// if tls is nil but connection tls is provided, then the proxy state is misconfigured
-			return nil, fmt.Errorf("proxyState.Tls is required to generate router's transport socket")
-		}
 		om := ts.GetOutboundMesh()
 		leaf, ok := pr.proxyState.LeafCertificates[om.IdentityKey]
 		if !ok {

diff --git a/agent/xdsv2/resources_test.go b/agent/xdsv2/resources_test.go
@@ -4,17 +4,19 @@
 package xdsv2
 
 import (
+	"os"
+	"path/filepath"
+	"sort"
+	"testing"
+
 	envoy_cluster_v3 "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3"
 	envoy_listener_v3 "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3"
+
 	"github.com/hashicorp/consul/agent/xds/response"
 	"github.com/hashicorp/consul/envoyextensions/xdscommon"
 	proxytracker "github.com/hashicorp/consul/internal/mesh/proxy-tracker"
 	meshv1alpha1 "github.com/hashicorp/consul/proto-public/pbmesh/v1alpha1"
 	"github.com/hashicorp/consul/sdk/testutil"
-	"os"
-	"path/filepath"
-	"sort"
-	"testing"
 
 	"github.com/stretchr/testify/require"
 	"google.golang.org/protobuf/encoding/protojson"

diff --git a/agent/xdsv2/testdata/input/l4-single-implicit-destination-tproxy.golden b/agent/xdsv2/testdata/input/l4-single-implicit-destination-tproxy.golden
@@ -53,7 +53,8 @@
                 "validationContext": {
                   "spiffeIds": [
                     "spiffe://foo.consul/ap/default/ns/default/identity/api1-identity"
-                  ]
+                  ],
+                  "trustBundlePeerNameKey": "local"
                 },
                 "sni": "api-1.default.dc1.internal.foo.consul"
               },
@@ -64,7 +65,19 @@
           }
         }
       }
+    },
+  "leafCertificates": {
+    "test-identity": {
+        "cert": "cert1",
+        "key": "key1"
+    }
+  },
+  "trustBundles": {
+    "local": {
+        "trustDomain": "foo.consul",
+        "roots": ["root1"]
     }
+  }
   },
   "requiredEndpoints": {
     "api-1.default.dc1.internal.foo.consul": {

diff --git a/agent/xdsv2/testdata/output/clusters/l4-single-implicit-destination-tproxy.golden b/agent/xdsv2/testdata/output/clusters/l4-single-implicit-destination-tproxy.golden
@@ -10,7 +10,42 @@
           "ads":  {},
           "resourceApiVersion":  "V3"
         }
-      }
+      },
+      "name": "tcp.api-1.default.dc1.internal.foo.consul",
+      "transportSocket": {
+        "name": "tls",
+        "typedConfig": {
+          "@type": "type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.UpstreamTlsContext",
+          "commonTlsContext": {
+            "alpnProtocols": [
+              "consul~tcp"
+            ],
+            "tlsCertificates": [
+              {
+                "certificateChain": {
+                  "inlineString": "cert1\n"
+                },
+                "privateKey": {
+                  "inlineString": "key1\n"
+                }
+              }
+            ],
+            "tlsParams": {},
+            "validationContext": {
+              "matchSubjectAltNames": [
+                {
+                  "exact": "spiffe://foo.consul/ap/default/ns/default/identity/api1-identity"
+                }
+              ],
+              "trustedCa": {
+                "inlineString": "root1\n"
+              }
+            }
+          },
+          "sni": "api-1.default.dc1.internal.foo.consul"
+        }
+      },
+      "type": "EDS"
     }
   ],
   "typeUrl":  "type.googleapis.com/envoy.config.cluster.v3.Cluster",

diff --git a/internal/mesh/internal/controllers/sidecarproxy/builder/destination_builder.go b/internal/mesh/internal/controllers/sidecarproxy/builder/destination_builder.go
@@ -508,7 +508,8 @@ func (b *Builder) addCluster(clusterName, sni, portName string, destinationIdent
 								OutboundMesh: &pbproxystate.OutboundMeshMTLS{
 									IdentityKey: b.proxyStateTemplate.ProxyState.Identity.Name,
 									ValidationContext: &pbproxystate.MeshOutboundValidationContext{
-										SpiffeIds: spiffeIDs,
+										SpiffeIds:              spiffeIDs,
+										TrustBundlePeerNameKey: b.id.Tenancy.PeerName,
 									},
 									Sni: sni,
 								},

diff --git a/internal/mesh/internal/controllers/sidecarproxy/builder/local_app.go b/internal/mesh/internal/controllers/sidecarproxy/builder/local_app.go
@@ -243,6 +243,9 @@ func (b *Builder) addInboundListener(name string, workload *pbcatalog.Workload)
 		},
 	}
 
+	// Add TLS inspection capability to be able to parse ALPN and/or SNI information from inbound connections.
+	listener.Capabilities = append(listener.Capabilities, pbproxystate.Capability_CAPABILITY_L4_TLS_INSPECTION)
+
 	return b.NewListenerBuilder(listener)
 }
 

diff --git a/...carproxy/builder/testdata/destination/l4-implicit-and-explicit-destinations-tproxy.golden b/...carproxy/builder/testdata/destination/l4-implicit-and-explicit-destinations-tproxy.golden
@@ -18,7 +18,8 @@
                 "validationContext": {
                   "spiffeIds": [
                     "spiffe://foo.consul/ap/default/ns/default/identity/api1-identity"
-                  ]
+                  ],
+                  "trustBundlePeerNameKey": "local"
                 }
               }
             }
@@ -43,7 +44,8 @@
                 "validationContext": {
                   "spiffeIds": [
                     "spiffe://foo.consul/ap/default/ns/default/identity/api2-identity"
-                  ]
+                  ],
+                  "trustBundlePeerNameKey": "local"
                 }
               }
             }

diff --git a/...nternal/controllers/sidecarproxy/builder/testdata/destination/l4-multi-destination.golden b/...nternal/controllers/sidecarproxy/builder/testdata/destination/l4-multi-destination.golden
@@ -28,7 +28,8 @@
                 "validationContext": {
                   "spiffeIds": [
                     "spiffe://foo.consul/ap/default/ns/default/identity/api1-identity"
-                  ]
+                  ],
+                  "trustBundlePeerNameKey": "local"
                 }
               }
             }
@@ -53,7 +54,8 @@
                 "validationContext": {
                   "spiffeIds": [
                     "spiffe://foo.consul/ap/default/ns/default/identity/api2-identity"
-                  ]
+                  ],
+                  "trustBundlePeerNameKey": "local"
                 }
               }
             }

diff --git a/...sidecarproxy/builder/testdata/destination/l4-multiple-implicit-destinations-tproxy.golden b/...sidecarproxy/builder/testdata/destination/l4-multiple-implicit-destinations-tproxy.golden
@@ -18,7 +18,8 @@
                 "validationContext": {
                   "spiffeIds": [
                     "spiffe://foo.consul/ap/default/ns/default/identity/api1-identity"
-                  ]
+                  ],
+                  "trustBundlePeerNameKey": "local"
                 }
               }
             }
@@ -43,7 +44,8 @@
                 "validationContext": {
                   "spiffeIds": [
                     "spiffe://foo.consul/ap/default/ns/default/identity/api2-identity"
-                  ]
+                  ],
+                  "trustBundlePeerNameKey": "local"
                 }
               }
             }

diff --git a/...decarproxy/builder/testdata/destination/l4-single-destination-ip-port-bind-address.golden b/...decarproxy/builder/testdata/destination/l4-single-destination-ip-port-bind-address.golden
@@ -28,7 +28,8 @@
                 "validationContext": {
                   "spiffeIds": [
                     "spiffe://foo.consul/ap/default/ns/default/identity/api1-identity"
-                  ]
+                  ],
+                  "trustBundlePeerNameKey": "local"
                 }
               }
             }
@@ -50,7 +51,9 @@
               "outboundMesh": {
                 "identityKey": "test-identity",
                 "sni": "api-2.default.dc1.internal.foo.consul",
-                "validationContext": {}
+                "validationContext": {
+                  "trustBundlePeerNameKey": "local"
+                }
               }
             }
           }

diff --git a/...rproxy/builder/testdata/destination/l4-single-destination-unix-socket-bind-address.golden b/...rproxy/builder/testdata/destination/l4-single-destination-unix-socket-bind-address.golden
@@ -18,7 +18,8 @@
                 "validationContext": {
                   "spiffeIds": [
                     "spiffe://foo.consul/ap/default/ns/default/identity/api2-identity"
-                  ]
+                  ],
+                  "trustBundlePeerNameKey": "local"
                 }
               }
             }

diff --git a/...rs/sidecarproxy/builder/testdata/destination/l4-single-implicit-destination-tproxy.golden b/...rs/sidecarproxy/builder/testdata/destination/l4-single-implicit-destination-tproxy.golden
@@ -18,7 +18,8 @@
                 "validationContext": {
                   "spiffeIds": [
                     "spiffe://foo.consul/ap/default/ns/default/identity/api1-identity"
-                  ]
+                  ],
+                  "trustBundlePeerNameKey": "local"
                 }
               }
             }

diff --git a/...rnal/controllers/sidecarproxy/builder/testdata/destination/mixed-multi-destination.golden b/...rnal/controllers/sidecarproxy/builder/testdata/destination/mixed-multi-destination.golden
@@ -18,7 +18,8 @@
                 "validationContext": {
                   "spiffeIds": [
                     "spiffe://foo.consul/ap/default/ns/default/identity/api1-identity"
-                  ]
+                  ],
+                  "trustBundlePeerNameKey": "local"
                 }
               }
             }
@@ -40,7 +41,9 @@
               "outboundMesh": {
                 "identityKey": "test-identity",
                 "sni": "api-2.default.dc1.internal.foo.consul",
-                "validationContext": {}
+                "validationContext": {
+                  "trustBundlePeerNameKey": "local"
+                }
               }
             }
           }
@@ -74,7 +77,8 @@
                 "validationContext": {
                   "spiffeIds": [
                     "spiffe://foo.consul/ap/default/ns/default/identity/api1-identity"
-                  ]
+                  ],
+                  "trustBundlePeerNameKey": "local"
                 }
               }
             }
@@ -99,7 +103,8 @@
                 "validationContext": {
                   "spiffeIds": [
                     "spiffe://foo.consul/ap/default/ns/default/identity/api2-identity"
-                  ]
+                  ],
+                  "trustBundlePeerNameKey": "local"
                 }
               }
             }
-Original file line number
+Diff line change
@@ Expand Up @@
     		},
     	}
+    	// Add TLS inspection capability to be able to parse ALPN and/or SNI information from inbound connections.
+    	listener.Capabilities = append(listener.Capabilities, pbproxystate.Capability_CAPABILITY_L4_TLS_INSPECTION)
     	return b.NewListenerBuilder(listener)
     }
@@ Expand Down @@