From 8db6563a9cd49ce43fc86f44fd27f03452770435 Mon Sep 17 00:00:00 2001 From: Jacob Bohanon Date: Fri, 4 Oct 2024 15:01:41 -0400 Subject: [PATCH] validation: enhance translation with full envoy validation (#10090) Co-authored-by: soloio-bulldozer[bot] <48420018+soloio-bulldozer[bot]@users.noreply.github.com> Co-authored-by: changelog-bot --- .github/workflows/pr-kubernetes-tests.yaml | 4 +- ...expand-envoy-validation-functionality.yaml | 14 + pkg/utils/envoyutils/bootstrap/bootstrap.go | 236 ++++++++++ .../bootstrap/bootstrap_suite_test.go | 2 +- .../envoyutils/bootstrap/bootstrap_test.go | 407 +++++++++++++++++- pkg/utils/envoyutils/validation/validation.go | 28 ++ .../validation/validation_suite_test.go | 13 - projects/gloo/pkg/translator/translator.go | 11 + .../validation/full_envoy_validation/suite.go | 51 +++ .../e2e/tests/full_envoy_validation_test.go | 54 +++ .../e2e/tests/full_envoy_validation_tests.go | 20 + .../manifests/full-envoy-validation-helm.yaml | 7 + 12 files changed, 816 insertions(+), 31 deletions(-) create mode 100644 changelog/v1.18.0-beta25/expand-envoy-validation-functionality.yaml delete mode 100644 pkg/utils/envoyutils/validation/validation_suite_test.go create mode 100644 test/kubernetes/e2e/features/validation/full_envoy_validation/suite.go create mode 100644 test/kubernetes/e2e/tests/full_envoy_validation_test.go create mode 100644 test/kubernetes/e2e/tests/full_envoy_validation_tests.go create mode 100644 test/kubernetes/e2e/tests/manifests/full-envoy-validation-helm.yaml diff --git a/.github/workflows/pr-kubernetes-tests.yaml b/.github/workflows/pr-kubernetes-tests.yaml index f79bffd368c..2927e512d84 100644 --- a/.github/workflows/pr-kubernetes-tests.yaml +++ b/.github/workflows/pr-kubernetes-tests.yaml @@ -75,10 +75,10 @@ jobs: go-test-args: '-v -timeout=30m' go-test-run-regex: '(^TestK8sGatewayIstio$$|^TestGlooGatewayEdgeGateway$$|^TestGlooctlIstioInjectEdgeApiGateway$$|^TestK8sGatewayNoValidation$$)' - # September 24, 2024: 22 minutes + # October 1, 2024: 27 minutes - cluster-name: 'cluster-five' go-test-args: '-v -timeout=25m' - go-test-run-regex: '^TestValidationStrict$$|^TestValidationAlwaysAccept$$|^TestTransformationValidationDisabled$$|^TestGloomtlsGatewayEdgeGateway$$|^TestHelm$$|^TestHelmSettings$$|^TestWatchNamespaceSelector$$' + go-test-run-regex: '^TestFullEnvoyValidation$$|^TestValidationStrict$$|^TestValidationAlwaysAccept$$|^TestTransformationValidationDisabled$$|^TestGloomtlsGatewayEdgeGateway$$|^TestHelm$$|^TestHelmSettings$$|^TestWatchNamespaceSelector$$' # In our PR tests, we run the suite of tests using the upper ends of versions that we claim to support # The versions should mirror: https://docs.solo.io/gloo-edge/latest/reference/support/ diff --git a/changelog/v1.18.0-beta25/expand-envoy-validation-functionality.yaml b/changelog/v1.18.0-beta25/expand-envoy-validation-functionality.yaml new file mode 100644 index 00000000000..75783f68c1f --- /dev/null +++ b/changelog/v1.18.0-beta25/expand-envoy-validation-functionality.yaml @@ -0,0 +1,14 @@ +changelog: + - type: NEW_FEATURE + issueLink: https://github.com/solo-io/gloo/issues/5720 + resolvesIssue: false + description: >- + Adds feature to utilize Envoy's validate mode to validate all config during translation. + This will be helpful in catching issues which Gloo translation cannot or otherwise does + not view as errors before the config gets served to Envoy. + - type: HELM + issueLink: https://github.com/solo-io/gloo/issues/5720 + resolvesIssue: false + description: >- + Add value to enable full Envoy validation after translation. This functionality + is disabled by default but can be enabled with gateway.validation.fullEnvoyValidation=true. diff --git a/pkg/utils/envoyutils/bootstrap/bootstrap.go b/pkg/utils/envoyutils/bootstrap/bootstrap.go index 9be57e6cf12..02d6454ee1d 100644 --- a/pkg/utils/envoyutils/bootstrap/bootstrap.go +++ b/pkg/utils/envoyutils/bootstrap/bootstrap.go @@ -2,9 +2,13 @@ package bootstrap import ( "bytes" + "context" + "errors" envoy_config_bootstrap_v3 "github.com/envoyproxy/go-control-plane/envoy/config/bootstrap/v3" + envoy_config_cluster_v3 "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3" envoy_config_core_v3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" + envoy_config_endpoint_v3 "github.com/envoyproxy/go-control-plane/envoy/config/endpoint/v3" envoy_config_listener_v3 "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3" envoy_config_route_v3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" envoy_extensions_filters_network_http_connection_manager_v3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/http_connection_manager/v3" @@ -12,7 +16,16 @@ import ( "github.com/golang/protobuf/jsonpb" "github.com/golang/protobuf/proto" anypb "github.com/golang/protobuf/ptypes/any" + "github.com/rotisserie/eris" "github.com/solo-io/gloo/projects/gloo/pkg/utils" + envoycache "github.com/solo-io/solo-kit/pkg/api/v1/control-plane/cache" +) + +var ( + // errNoHcm represents a situation where a filter chain does not have an HttpConnectionManager. + // Because this could occur for valid reasons, such as a TCP proxy, we use + // this as a sentinel error to inform us it's ok to ignore it and continue. + errNoHcm = eris.New("no HttpConnectionManager found") ) func FromEnvoyResources(resources *EnvoyResources) (string, error) { @@ -102,3 +115,226 @@ func FromFilter(filterName string, msg proto.Message) (string, error) { return FromEnvoyResources(&EnvoyResources{Listeners: []*envoy_config_listener_v3.Listener{listener}}) } + +// FromSnapshot accepts an xds Snapshot and converts it into valid bootstrap json. +func FromSnapshot( + ctx context.Context, + snap envoycache.Snapshot, +) (string, error) { + + // Get the resources we're going to need as concrete types. + resources, err := resourcesFromSnapshot(snap) + if err != nil { + return "", err + } + + // This map will hold the aggregate of all cluster names that are routed to + // by a FilterChain. + routedCluster := map[string]struct{}{} + + // Gather up all of the clusters that we target with RouteConfigs that are associated with a FilterChain. + if err := extractRoutedClustersFromListeners(routedCluster, resources.Listeners, resources.routes); err != nil { + return "", err + } + + // Next, we will look through our Snapshot's clusters and delete the ones which are + // already routed to. + convertToStaticClusters(routedCluster, resources.Clusters, resources.endpoints) + + // We now need to find clusters which do not exist, even though they are targeted by + // a route. In static mode, envoy won't start without these. At this point in the + // processing, routedClusters holds this list, so we use it to create blackhole + // clusters for these routes to target. It is important to have unique clusters + // for the targets since some envoy functionality relies on such setup, like + // weighted destinations. + resources.Clusters = addBlackholeClusters(routedCluster, resources.Clusters) + + return FromEnvoyResources(resources) +} + +// extractRoutedClustersFromListeners accepts a hash set of strings containing the names of clusters +// to which routes point, a slice of pointers to Listener structs, +// and a slice of pointers to RouteConfiguration structs from the snapshot. It looks +// through the FilterChains on each Listener for an HttpConnectionManager, gets the +// routes on that hcm, and gets all of the clusters targeted by those routes. It then +// converts the hcm config to use static RouteConfiguration. routedCluster and elements +// of listeners are mutated in this function. +func extractRoutedClustersFromListeners( + routedCluster map[string]struct{}, + listeners []*envoy_config_listener_v3.Listener, + routes []*envoy_config_route_v3.RouteConfiguration, +) error { + for _, l := range listeners { + for _, fc := range l.GetFilterChains() { + // Get the HttpConnectionManager for this FilterChain if it exists. + hcm, f, err := getHcmForFilterChain(fc) + if err != nil { + // If we just don't have an hcm on this filter chain, skip to the next one. + if errors.Is(err, errNoHcm) { + continue + } + // If we encountered any other error, fail loudly. + return err + } + + // We use Route Discovery Service (RDS) in lieu of static route table config, so we + // need to get the RouteConfiguration name to lookup in our Snapshot-provided routes, + // which contain what we serve over RDS. + routeConfigName := hcm.GetRds().GetRouteConfigName() + if routeConfigName == "" { + continue + } + // Find matching route config from snapshot. + for _, r := range routes { + if r.GetName() != routeConfigName { + // These aren't the routes you're looking for. + continue + } + + // Add clusters targeted by routes on this config to our aggregate list of all targeted clusters + findTargetedClusters(r, routedCluster) + + // We need to add our route table as a static config to this hcm instead of + // relying on RDS, the we pack it back up and set it back on the filter chain. + if err = setStaticRouteConfig(f, hcm, r); err != nil { + return err + } + } + } + } + + return nil +} + +// convertToStaticClusters accepts a hash set of strings containing the names of clusters +// to which routes point, a slice of pointers to Cluster structs, +// and a slice of pointers to ClusterLoadAssignment structs from the snapshot. It +// deletes all clusters that exist from the routedCluster hash set, then converts +// the cluster's EDS config to static config using the endpoints from the snapshot. +// clusters is mutated in this function. +func convertToStaticClusters( + routedCluster map[string]struct{}, + clusters []*envoy_config_cluster_v3.Cluster, + endpoints []*envoy_config_endpoint_v3.ClusterLoadAssignment, +) { + for _, c := range clusters { + delete(routedCluster, c.GetName()) + + // We use Endpoint Discovery Service (EDS) in lieu of static endpoint config, so we + // need to get the EDS ServiceName name to lookup in our Snapshot-provided endpoints, + // which contain what we serve over EDS. + if c.GetEdsClusterConfig() != nil { + clusterName := c.GetName() + if edsServiceName := c.GetEdsClusterConfig().GetServiceName(); edsServiceName != "" { + clusterName = edsServiceName + } + + // Find endpoints matching our EDS config and convert the cluster to use + // static endpoint config matching that which would have been served over EDS. + for _, e := range endpoints { + if e.GetClusterName() == clusterName { + c.LoadAssignment = e + c.EdsClusterConfig = nil + c.ClusterDiscoveryType = &envoy_config_cluster_v3.Cluster_Type{ + Type: envoy_config_cluster_v3.Cluster_STRICT_DNS, + } + } + } + } + } +} + +// addBlackholeClusters accepts a hash set of strings containing the names of clusters +// to which routes point and a slice of pointers to Cluster structs from the snapshot. It +// adds an cluster to clusters for each entry in the routedCluster set. clusters is mutated +// by this function. +func addBlackholeClusters( + routedCluster map[string]struct{}, + clusters []*envoy_config_cluster_v3.Cluster, +) []*envoy_config_cluster_v3.Cluster { + for c := range routedCluster { + clusters = append(clusters, &envoy_config_cluster_v3.Cluster{ + Name: c, + ClusterDiscoveryType: &envoy_config_cluster_v3.Cluster_Type{ + Type: envoy_config_cluster_v3.Cluster_STATIC, + }, + LoadAssignment: &envoy_config_endpoint_v3.ClusterLoadAssignment{ + ClusterName: c, + Endpoints: []*envoy_config_endpoint_v3.LocalityLbEndpoints{}, + }, + }) + } + return clusters +} + +// getHcmForFilterChain accepts a pointer to a FilterChain and looks for the HttpConnectionManager +// network filter if one exists. It returns a pointer to the HttpConnectionManager struct and +// a pointer to the filter that actually contained it. This function has no side effects. +func getHcmForFilterChain(fc *envoy_config_listener_v3.FilterChain) ( + *envoy_extensions_filters_network_http_connection_manager_v3.HttpConnectionManager, + *envoy_config_listener_v3.Filter, + error, +) { + for _, f := range fc.GetFilters() { + if f.GetTypedConfig().GetTypeUrl() == "type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager" { + hcmAny, err := utils.AnyToMessage(f.GetTypedConfig()) + if err != nil { + return nil, nil, err + } + // This check can be unreliable if the proto *Any format can be successfully unmarshalled to this concrete type, + // which is surprisingly easy to do. This codepath is not tested as I was unable to force a failure, but we're + // leaving the check in to guard against NPE from the concrete cast. + if hcm, ok := hcmAny.(*envoy_extensions_filters_network_http_connection_manager_v3.HttpConnectionManager); ok { + return hcm, f, nil + } else { + return nil, nil, eris.Errorf("filter %v has hcm type url but casting to concrete failed", f.GetName()) + } + } + } + return nil, nil, errNoHcm +} + +// findTargetedClusters accepts a pointer to a RouteConfiguration and a hash set of strings. It +// finds all clusters and weighted clusters targeted by routes on the virtual hosts in the RouteConfiguration +// and adds their names to the routedCluster hash set. routedCluster is mutated in this function. +func findTargetedClusters(r *envoy_config_route_v3.RouteConfiguration, routedCluster map[string]struct{}) { + for _, v := range r.GetVirtualHosts() { + for _, r := range v.GetRoutes() { + if r.GetRoute() == nil { + continue + } + + if c := r.GetRoute().GetCluster(); c != "" { + routedCluster[c] = struct{}{} + } + if wc := r.GetRoute().GetWeightedClusters().GetClusters(); len(wc) != 0 { + for _, c := range wc { + routedCluster[c.GetName()] = struct{}{} + } + } + } + } +} + +// setStaticRouteConfig accepts pointers to each of a Filter, HttpConnectionManager, and RouteConfiguration. +// It adds the RouteConfiguration to the HttpConnectionManager as static, marshals the hcm, and sets the filter's +// TypedConfig. f and hcm are mutated in this function. +func setStaticRouteConfig( + f *envoy_config_listener_v3.Filter, + hcm *envoy_extensions_filters_network_http_connection_manager_v3.HttpConnectionManager, + r *envoy_config_route_v3.RouteConfiguration, +) error { + hcm.RouteSpecifier = &envoy_extensions_filters_network_http_connection_manager_v3.HttpConnectionManager_RouteConfig{ + RouteConfig: r, + } + + hcmAny, err := utils.MessageToAny(hcm) + if err != nil { + return err + } + + f.ConfigType = &envoy_config_listener_v3.Filter_TypedConfig{ + TypedConfig: hcmAny, + } + return nil +} diff --git a/pkg/utils/envoyutils/bootstrap/bootstrap_suite_test.go b/pkg/utils/envoyutils/bootstrap/bootstrap_suite_test.go index 6670d071753..d2ca4cf4b07 100644 --- a/pkg/utils/envoyutils/bootstrap/bootstrap_suite_test.go +++ b/pkg/utils/envoyutils/bootstrap/bootstrap_suite_test.go @@ -1,4 +1,4 @@ -package bootstrap_test +package bootstrap import ( "testing" diff --git a/pkg/utils/envoyutils/bootstrap/bootstrap_test.go b/pkg/utils/envoyutils/bootstrap/bootstrap_test.go index b9737a27ee0..bb2791b58fc 100644 --- a/pkg/utils/envoyutils/bootstrap/bootstrap_test.go +++ b/pkg/utils/envoyutils/bootstrap/bootstrap_test.go @@ -1,29 +1,224 @@ -package bootstrap_test +package bootstrap import ( - "log" + "context" + + envoytransformation "github.com/solo-io/gloo/projects/gloo/pkg/api/external/envoy/extensions/transformation" + "github.com/solo-io/gloo/projects/gloo/pkg/utils" + + envoycache "github.com/solo-io/solo-kit/pkg/api/v1/control-plane/cache" + "github.com/solo-io/solo-kit/pkg/api/v1/control-plane/resource" + "github.com/solo-io/solo-kit/pkg/api/v1/control-plane/types" envoy_config_bootstrap_v3 "github.com/envoyproxy/go-control-plane/envoy/config/bootstrap/v3" + envoy_config_cluster_v3 "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3" envoy_config_core_v3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" + envoy_config_endpoint_v3 "github.com/envoyproxy/go-control-plane/envoy/config/endpoint/v3" envoy_config_listener_v3 "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3" envoy_config_route_v3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" envoy_extensions_filters_network_http_connection_manager_v3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/http_connection_manager/v3" "github.com/envoyproxy/go-control-plane/pkg/wellknown" anypb "github.com/golang/protobuf/ptypes/any" - . "github.com/solo-io/gloo/pkg/utils/envoyutils/bootstrap" - "github.com/solo-io/gloo/pkg/utils/protoutils" - envoytransformation "github.com/solo-io/gloo/projects/gloo/pkg/api/external/envoy/extensions/transformation" - "github.com/solo-io/gloo/projects/gloo/pkg/utils" + "google.golang.org/protobuf/encoding/protojson" "google.golang.org/protobuf/proto" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) +const ( + hcmTypeUrl = "type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager" +) + var _ = Describe("Static bootstrap generation", func() { + var ( + listeners []*envoy_config_listener_v3.Listener + clusters []*envoy_config_cluster_v3.Cluster + routes []*envoy_config_route_v3.RouteConfiguration + endpoints []*envoy_config_endpoint_v3.ClusterLoadAssignment + ) + BeforeEach(func() { + listeners = []*envoy_config_listener_v3.Listener{} + clusters = []*envoy_config_cluster_v3.Cluster{{ + Name: "foo", + EdsClusterConfig: &envoy_config_cluster_v3.Cluster_EdsClusterConfig{ + ServiceName: "foo-eds", + }, + }} + routes = []*envoy_config_route_v3.RouteConfiguration{{ + Name: "foo-routes", + VirtualHosts: []*envoy_config_route_v3.VirtualHost{ + { + Name: "placeholder_host", + Domains: []string{"*"}, + Routes: []*envoy_config_route_v3.Route{ + { + Action: &envoy_config_route_v3.Route_Route{ + Route: &envoy_config_route_v3.RouteAction{ + ClusterSpecifier: &envoy_config_route_v3.RouteAction_Cluster{ + Cluster: "foo", + }, + }, + }, + Name: "foo-route", + }, + { + Action: &envoy_config_route_v3.Route_Route{ + Route: &envoy_config_route_v3.RouteAction{ + ClusterSpecifier: &envoy_config_route_v3.RouteAction_Cluster{ + Cluster: "bar", + }, + }, + }, + Name: "bar-route", + }, + }, + }, + }, + }, + } + endpoints = []*envoy_config_endpoint_v3.ClusterLoadAssignment{{ + ClusterName: "foo-eds", + }} + }) + Context("Util functions", func() { + var ( + routedCluster map[string]struct{} + ) + BeforeEach(func() { + routedCluster = make(map[string]struct{}) + }) + Context("extractRoutedClustersFromListeners", func() { + It("does not error if no hcm", func() { + l := &envoy_config_listener_v3.Listener{ + Name: "fake-listener", + Address: &envoy_config_core_v3.Address{}, + FilterChains: []*envoy_config_listener_v3.FilterChain{{ + FilterChainMatch: &envoy_config_listener_v3.FilterChainMatch{}, + Filters: []*envoy_config_listener_v3.Filter{}, + }}, + } + listeners = append(listeners, l) + Expect(extractRoutedClustersFromListeners(routedCluster, listeners, routes)).NotTo(HaveOccurred()) + Expect(routedCluster).To(BeEmpty()) + }) + It("extracts a single happy cluster", func() { + hcmAny, err := utils.MessageToAny(&envoy_extensions_filters_network_http_connection_manager_v3.HttpConnectionManager{ + StatPrefix: "placeholder", + RouteSpecifier: &envoy_extensions_filters_network_http_connection_manager_v3.HttpConnectionManager_Rds{ + Rds: &envoy_extensions_filters_network_http_connection_manager_v3.Rds{ + RouteConfigName: "foo-routes", + }, + }, + }) + Expect(err).NotTo(HaveOccurred()) + l := &envoy_config_listener_v3.Listener{ + Name: "fake-listener", + Address: &envoy_config_core_v3.Address{}, + FilterChains: []*envoy_config_listener_v3.FilterChain{{ + FilterChainMatch: &envoy_config_listener_v3.FilterChainMatch{}, + Filters: []*envoy_config_listener_v3.Filter{{ + Name: wellknown.HTTPConnectionManager, + ConfigType: &envoy_config_listener_v3.Filter_TypedConfig{ + TypedConfig: hcmAny, + }, + }}, + }}, + } + listeners = append(listeners, l) + Expect(extractRoutedClustersFromListeners(routedCluster, listeners, routes)).NotTo(HaveOccurred()) + Expect(routedCluster).To(HaveKey("foo")) + }) + }) + Context("convertToStaticClusters", func() { + BeforeEach(func() { + routedCluster = map[string]struct{}{"foo": struct{}{}, "bar": struct{}{}} + clusters = []*envoy_config_cluster_v3.Cluster{{ + Name: "foo", + EdsClusterConfig: &envoy_config_cluster_v3.Cluster_EdsClusterConfig{ + ServiceName: "foo-eds", + }, + }} + endpoints = []*envoy_config_endpoint_v3.ClusterLoadAssignment{{ + ClusterName: "foo-eds", + }} + }) + It("converts and removes from routedCluster", func() { + Expect(clusters[0].GetLoadAssignment()).To(BeNil()) + convertToStaticClusters(routedCluster, clusters, endpoints) + Expect(routedCluster).To(HaveKey("bar")) + Expect(routedCluster).NotTo(HaveKey("foo")) + Expect(clusters).To(HaveLen(1)) + Expect(clusters[0].GetLoadAssignment()).NotTo(BeNil()) + Expect(clusters[0].GetLoadAssignment().GetClusterName()).To(Equal("foo-eds")) + }) + }) + Context("addBlackholeClusters", func() { + BeforeEach(func() { + routedCluster = map[string]struct{}{"bar": struct{}{}} + clusters = []*envoy_config_cluster_v3.Cluster{{ + Name: "foo", + EdsClusterConfig: &envoy_config_cluster_v3.Cluster_EdsClusterConfig{ + ServiceName: "foo-eds", + }, + }} + endpoints = []*envoy_config_endpoint_v3.ClusterLoadAssignment{{ + ClusterName: "foo-eds", + }} + }) + It("adds blackhole clusters for missing values", func() { + clusters = addBlackholeClusters(routedCluster, clusters) + Expect(clusters).To(HaveLen(2)) + Expect(clusters[1].GetName()).To(Equal("bar")) + Expect(clusters[1].GetLoadAssignment().GetClusterName()).To(Equal("bar")) + }) + }) + Context("getHcmForFilterChain", func() { + It("gets the HCM", func() { + hcmAny, err := utils.MessageToAny(&envoy_extensions_filters_network_http_connection_manager_v3.HttpConnectionManager{ + StatPrefix: "placeholder", + RouteSpecifier: &envoy_extensions_filters_network_http_connection_manager_v3.HttpConnectionManager_Rds{ + Rds: &envoy_extensions_filters_network_http_connection_manager_v3.Rds{ + RouteConfigName: "foo-routes", + }, + }, + }) + Expect(err).NotTo(HaveOccurred()) + fc := &envoy_config_listener_v3.FilterChain{ + FilterChainMatch: &envoy_config_listener_v3.FilterChainMatch{}, + Filters: []*envoy_config_listener_v3.Filter{{ + Name: wellknown.HTTPConnectionManager, + ConfigType: &envoy_config_listener_v3.Filter_TypedConfig{ + TypedConfig: hcmAny, + }, + }}, + } + hcm, filter, err := getHcmForFilterChain(fc) + Expect(err).NotTo(HaveOccurred()) + Expect(hcm.StatPrefix).To(Equal("placeholder")) + Expect(filter.GetTypedConfig().GetTypeUrl()).To(Equal(hcmTypeUrl)) + }) + }) + Context("findTargetedClusters", func() { + It("finds clusters targeted by routes", func() { + findTargetedClusters(routes[0], routedCluster) + Expect(routedCluster).To(HaveLen(2)) + Expect(routedCluster).To(HaveKey("foo")) + Expect(routedCluster).To(HaveKey("bar")) + }) + }) + Context("setStaticRouteConfig", func() { + It("sets the route config as static and mutates the filter", func() { + f := &envoy_config_listener_v3.Filter{} + hcm := &envoy_extensions_filters_network_http_connection_manager_v3.HttpConnectionManager{} + Expect(setStaticRouteConfig(f, hcm, routes[0])).NotTo(HaveOccurred()) + Expect(hcm.GetRouteConfig().GetName()).To(Equal(routes[0].GetName())) + Expect(f.GetTypedConfig().GetTypeUrl()).To(Equal(hcmTypeUrl)) + }) + }) + }) Context("From Filter", func() { It("produces correct bootstrap", func() { - Skip("TODO") inTransformation := &envoytransformation.RouteTransformations{ ClearRouteCache: true, Transformations: []*envoytransformation.RouteTransformations_RouteTransformation{ @@ -96,18 +291,200 @@ var _ = Describe("Static bootstrap generation", func() { }, } - var actualBootstrap *envoy_config_bootstrap_v3.Bootstrap + actualBootstrap := &envoy_config_bootstrap_v3.Bootstrap{} - log.Println(actual) - err = protoutils.UnmarshalBytesAllowUnknown([]byte(actual), actualBootstrap) - // err = (&jsonpb.Unmarshaler{ - // AllowUnknownFields: true, - // AnyResolver: nil, - // }).UnmarshalString(actual, actualBootstrap) - // err = jsonpb.Unmarshal(bytes.NewBuffer([]byte(actual)), actualBootstrap) + err = protojson.Unmarshal([]byte(actual), actualBootstrap) + Expect(err).NotTo(HaveOccurred()) + + Expect(proto.Equal(expectedBootstrap, actualBootstrap)).To(BeTrue()) + }) + }) + Context("From Snapshot", func() { + var ( + snap *fakeSnapshot + ) + BeforeEach(func() { + snap = &fakeSnapshot{ + m: map[string]envoycache.Resources{ + types.ListenerTypeV3: envoycache.NewResources("", []envoycache.Resource{}), + types.ClusterTypeV3: envoycache.NewResources("", []envoycache.Resource{}), + types.RouteTypeV3: envoycache.NewResources("", []envoycache.Resource{}), + types.EndpointTypeV3: envoycache.NewResources("", []envoycache.Resource{}), + }, + } + hcmAny, err := utils.MessageToAny(&envoy_extensions_filters_network_http_connection_manager_v3.HttpConnectionManager{ + StatPrefix: "placeholder", + RouteSpecifier: &envoy_extensions_filters_network_http_connection_manager_v3.HttpConnectionManager_Rds{ + Rds: &envoy_extensions_filters_network_http_connection_manager_v3.Rds{ + RouteConfigName: "foo-routes", + }, + }, + }) + Expect(err).NotTo(HaveOccurred()) + listeners = append(listeners, &envoy_config_listener_v3.Listener{ + Name: "placeholder_listener", + Address: &envoy_config_core_v3.Address{ + Address: &envoy_config_core_v3.Address_SocketAddress{SocketAddress: &envoy_config_core_v3.SocketAddress{ + Address: "0.0.0.0", + PortSpecifier: &envoy_config_core_v3.SocketAddress_PortValue{PortValue: 8081}, + }}, + }, + FilterChains: []*envoy_config_listener_v3.FilterChain{{ + Name: "placeholder_filter_chain", + Filters: []*envoy_config_listener_v3.Filter{{ + Name: wellknown.HTTPConnectionManager, + ConfigType: &envoy_config_listener_v3.Filter_TypedConfig{ + TypedConfig: hcmAny, + }, + }}, + }}, + }) + }) + + // types.SecretTypeV3 are omitted due to not being converted from snapshot into bootstrap. + JustBeforeEach(func() { + for _, l := range listeners { + snap.m[types.ListenerTypeV3].Items[l.GetName()] = resource.NewEnvoyResource(l) + } + for _, c := range clusters { + snap.m[types.ClusterTypeV3].Items[c.GetName()] = resource.NewEnvoyResource(c) + } + for _, r := range routes { + snap.m[types.RouteTypeV3].Items[r.GetName()] = resource.NewEnvoyResource(r) + } + for _, e := range endpoints { + snap.m[types.EndpointTypeV3].Items[e.GetClusterName()] = resource.NewEnvoyResource(e) + } + }) + It("produces correct bootstrap", func() { + + actual, err := FromSnapshot(context.Background(), snap) + Expect(err).NotTo(HaveOccurred()) + + expectedBootstrap := &envoy_config_bootstrap_v3.Bootstrap{ + Node: &envoy_config_core_v3.Node{ + Id: "validation-node-id", + Cluster: "validation-cluster", + }, + StaticResources: &envoy_config_bootstrap_v3.Bootstrap_StaticResources{ + Clusters: []*envoy_config_cluster_v3.Cluster{ + { + Name: "foo", + ClusterDiscoveryType: &envoy_config_cluster_v3.Cluster_Type{ + Type: envoy_config_cluster_v3.Cluster_STRICT_DNS, + }, + LoadAssignment: &envoy_config_endpoint_v3.ClusterLoadAssignment{ + ClusterName: "foo-eds", + }, + }, + { + Name: "bar", + ClusterDiscoveryType: &envoy_config_cluster_v3.Cluster_Type{ + Type: envoy_config_cluster_v3.Cluster_STATIC, + }, + LoadAssignment: &envoy_config_endpoint_v3.ClusterLoadAssignment{ + ClusterName: "bar", + }, + }, + }, + Listeners: []*envoy_config_listener_v3.Listener{{ + Name: "placeholder_listener", + Address: &envoy_config_core_v3.Address{ + Address: &envoy_config_core_v3.Address_SocketAddress{SocketAddress: &envoy_config_core_v3.SocketAddress{ + Address: "0.0.0.0", + PortSpecifier: &envoy_config_core_v3.SocketAddress_PortValue{PortValue: 8081}, + }}, + }, + FilterChains: []*envoy_config_listener_v3.FilterChain{ + { + Name: "placeholder_filter_chain", + Filters: []*envoy_config_listener_v3.Filter{ + { + Name: wellknown.HTTPConnectionManager, + ConfigType: &envoy_config_listener_v3.Filter_TypedConfig{ + TypedConfig: func() *anypb.Any { + hcmAny, err := utils.MessageToAny(&envoy_extensions_filters_network_http_connection_manager_v3.HttpConnectionManager{ + StatPrefix: "placeholder", + RouteSpecifier: &envoy_extensions_filters_network_http_connection_manager_v3.HttpConnectionManager_RouteConfig{ + RouteConfig: &envoy_config_route_v3.RouteConfiguration{ + Name: "foo-routes", + VirtualHosts: []*envoy_config_route_v3.VirtualHost{ + { + Name: "placeholder_host", + Domains: []string{"*"}, + Routes: []*envoy_config_route_v3.Route{ + { + Name: "foo-route", + Action: &envoy_config_route_v3.Route_Route{ + Route: &envoy_config_route_v3.RouteAction{ + ClusterSpecifier: &envoy_config_route_v3.RouteAction_Cluster{ + Cluster: "foo", + }, + }, + }, + }, + { + Name: "bar-route", + Action: &envoy_config_route_v3.Route_Route{ + Route: &envoy_config_route_v3.RouteAction{ + ClusterSpecifier: &envoy_config_route_v3.RouteAction_Cluster{ + Cluster: "bar", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }) + Expect(err).NotTo(HaveOccurred()) + return hcmAny + }(), + }, + }, + }, + }, + }, + }}, + }, + } + + actualBootstrap := &envoy_config_bootstrap_v3.Bootstrap{} + + err = protojson.Unmarshal([]byte(actual), actualBootstrap) Expect(err).NotTo(HaveOccurred()) Expect(proto.Equal(expectedBootstrap, actualBootstrap)).To(BeTrue()) }) }) }) + +type fakeSnapshot struct { + m map[string]envoycache.Resources +} + +func (f *fakeSnapshot) GetResources(typ string) envoycache.Resources { + if res, ok := f.m[typ]; ok { + return res + } + panic("unknown resources type" + typ) + +} + +// Clone shouldn't be called on a generic snapshot until https://github.com/solo-io/solo-kit/issues/461 is resolved. +func (f *fakeSnapshot) Clone() envoycache.Snapshot { + // don't need to worry about cloning for testing purposes. + return f +} + +// Unused +func (f *fakeSnapshot) Consistent() error { + panic("not implemented") +} + +// Unused +func (f *fakeSnapshot) MakeConsistent() { + panic("not implemented") +} diff --git a/pkg/utils/envoyutils/validation/validation.go b/pkg/utils/envoyutils/validation/validation.go index 5a433b21610..be766a83c0d 100644 --- a/pkg/utils/envoyutils/validation/validation.go +++ b/pkg/utils/envoyutils/validation/validation.go @@ -3,9 +3,13 @@ package validation import ( "context" + "github.com/rotisserie/eris" + "github.com/solo-io/gloo/pkg/utils/envoyutils/bootstrap" "github.com/solo-io/gloo/pkg/utils/envutils" "github.com/solo-io/gloo/projects/envoyinit/pkg/runner" "github.com/solo-io/gloo/projects/gloo/constants" + "github.com/solo-io/go-utils/contextutils" + envoycache "github.com/solo-io/solo-kit/pkg/api/v1/control-plane/cache" ) const ( @@ -21,3 +25,27 @@ const ( func ValidateBootstrap(ctx context.Context, bootstrap string) error { return runner.RunEnvoyValidate(ctx, envutils.GetOrDefault(constants.EnvoyBinaryEnv, defaultEnvoyPath, false), bootstrap) } + +// ValidateSnapshot accepts an xDS snapshot, clones it, and does the necessary +// conversions to imitate the same config being provided as static bootsrap config to +// Envoy, then executes Envoy in validate mode to ensure the config is valid. +// This is necessary as some configurations that work in dynamic do not work in static +// and some configurations may require the context of the destination such as mounted files. +func ValidateSnapshot( + ctx context.Context, + snap envoycache.Snapshot, +) error { + // THIS IS CRITICAL SO WE DO NOT INTERFERE WITH THE CONTROL PLANE. + // The logic for converting xDS to static bootstrap mutates some of + // the inputs, which is unacceptable when calling from translation. + snap = snap.Clone() + + bootstrapJson, err := bootstrap.FromSnapshot(ctx, snap) + if err != nil { + err = eris.Wrap(err, "error converting xDS snapshot to static bootstrap") + contextutils.LoggerFrom(ctx).Error(err) + return err + } + + return ValidateBootstrap(ctx, bootstrapJson) +} diff --git a/pkg/utils/envoyutils/validation/validation_suite_test.go b/pkg/utils/envoyutils/validation/validation_suite_test.go deleted file mode 100644 index ac1bf349970..00000000000 --- a/pkg/utils/envoyutils/validation/validation_suite_test.go +++ /dev/null @@ -1,13 +0,0 @@ -package validation_test - -import ( - "testing" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" -) - -func TestValidation(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecs(t, "Validation Suite") -} diff --git a/projects/gloo/pkg/translator/translator.go b/projects/gloo/pkg/translator/translator.go index f952a9d0710..5da7cea5882 100644 --- a/projects/gloo/pkg/translator/translator.go +++ b/projects/gloo/pkg/translator/translator.go @@ -16,6 +16,7 @@ import ( envoy_config_route_v3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" "github.com/golang/protobuf/proto" errors "github.com/rotisserie/eris" + envoyvalidation "github.com/solo-io/gloo/pkg/utils/envoyutils/validation" validationapi "github.com/solo-io/gloo/projects/gloo/pkg/api/grpc/validation" v1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1" "github.com/solo-io/gloo/projects/gloo/pkg/plugins" @@ -128,6 +129,16 @@ func (t *translatorInstance) Translate( } } + // If we're not validating the full proxy with envoy validate mode, we can + // return here. + if !t.settings.GetGateway().GetValidation().GetFullEnvoyValidation().GetValue() { + return xdsSnapshot, reports, proxyReport + } + + if err := envoyvalidation.ValidateSnapshot(ctx, xdsSnapshot); err != nil { + reports.AddError(proxy, err) + } + return xdsSnapshot, reports, proxyReport } diff --git a/test/kubernetes/e2e/features/validation/full_envoy_validation/suite.go b/test/kubernetes/e2e/features/validation/full_envoy_validation/suite.go new file mode 100644 index 00000000000..d730bed3b6a --- /dev/null +++ b/test/kubernetes/e2e/features/validation/full_envoy_validation/suite.go @@ -0,0 +1,51 @@ +package full_envoy_validation + +import ( + "context" + + gloo_defaults "github.com/solo-io/gloo/projects/gloo/pkg/defaults" + "github.com/solo-io/gloo/test/kubernetes/e2e" + "github.com/solo-io/gloo/test/kubernetes/e2e/features/validation" + "github.com/solo-io/solo-kit/pkg/api/v1/clients" + "github.com/solo-io/solo-kit/pkg/api/v1/resources" + "github.com/solo-io/solo-kit/pkg/api/v1/resources/core" + "github.com/stretchr/testify/suite" +) + +var _ e2e.NewSuiteFunc = NewTestingSuite + +// testingSuite is the entire Suite of tests for the webhook validation fullEnvoyValidation=true feature +type testingSuite struct { + suite.Suite + + ctx context.Context + + // testInstallation contains all the metadata/utilities necessary to execute a series of tests + // against an installation of Gloo Gateway + testInstallation *e2e.TestInstallation +} + +func NewTestingSuite(ctx context.Context, testInst *e2e.TestInstallation) suite.TestingSuite { + return &testingSuite{ + ctx: ctx, + testInstallation: testInst, + } +} + +// TestRejectInvalidTransformation checks webhook rejects invalid transformation when fullEnvoyValidation=true +func (s *testingSuite) TestRejectInvalidTransformation() { + err := s.testInstallation.Actions.Kubectl().ApplyFile(s.ctx, validation.ExampleUpstream, "-n", s.testInstallation.Metadata.InstallNamespace) + s.Assert().NoError(err) + s.testInstallation.Assertions.EventuallyResourceStatusMatchesState( + func() (resources.InputResource, error) { + return s.testInstallation.ResourceClients.UpstreamClient().Read(s.testInstallation.Metadata.InstallNamespace, "nginx-upstream", clients.ReadOpts{Ctx: s.ctx}) + }, + core.Status_Accepted, + gloo_defaults.GlooReporter, + ) + // rejects invalid inja template in transformation + output, err := s.testInstallation.Actions.Kubectl().ApplyFileWithOutput(s.ctx, validation.VSTransformationHeaderText, "-n", s.testInstallation.Metadata.InstallNamespace) + s.Assert().Error(err) + s.Assert().Contains(output, "Failed to parse response template: Failed to parse "+ + "header template ':status': [inja.exception.parser_error] (at 1:92) expected statement close, got '%'") +} diff --git a/test/kubernetes/e2e/tests/full_envoy_validation_test.go b/test/kubernetes/e2e/tests/full_envoy_validation_test.go new file mode 100644 index 00000000000..ee54573981e --- /dev/null +++ b/test/kubernetes/e2e/tests/full_envoy_validation_test.go @@ -0,0 +1,54 @@ +package tests_test + +import ( + "context" + "os" + "testing" + "time" + + "github.com/solo-io/gloo/pkg/utils/envutils" + "github.com/solo-io/gloo/test/kubernetes/e2e" + . "github.com/solo-io/gloo/test/kubernetes/e2e/tests" + "github.com/solo-io/gloo/test/kubernetes/testutils/gloogateway" + "github.com/solo-io/gloo/test/testutils" +) + +// TestFullEnvoyValidation is the function which executes a series of tests against a given +// installation where full Envoy validation is enabled (fullEnvoyValidation=true) +func TestFullEnvoyValidation(t *testing.T) { + ctx := context.Background() + installNs, nsEnvPredefined := envutils.LookupOrDefault(testutils.InstallNamespace, "full-envoy-validation-test") + testInstallation := e2e.CreateTestInstallation( + t, + &gloogateway.Context{ + InstallNamespace: installNs, + ProfileValuesManifestFile: e2e.FullGatewayProfilePath, + ValuesManifestFile: e2e.ManifestPath("full-envoy-validation-helm.yaml"), + }, + ) + + testHelper := e2e.MustTestHelper(ctx, testInstallation) + + // Set the env to the install namespace if it is not already set + if !nsEnvPredefined { + os.Setenv(testutils.InstallNamespace, installNs) + } + + // We register the cleanup function _before_ we actually perform the installation. + // This allows us to uninstall Gloo Gateway, in case the original installation only completed partially + t.Cleanup(func() { + if !nsEnvPredefined { + os.Unsetenv(testutils.InstallNamespace) + } + if t.Failed() { + testInstallation.PreFailHandler(ctx) + } + + testInstallation.UninstallGlooGatewayWithTestHelper(ctx, testHelper) + }) + + // Install Gloo Gateway with correct validation settings + testInstallation.InstallGlooGatewayWithTestHelper(ctx, testHelper, 5*time.Minute) + + FullEnvoyValidationSuiteRunner().Run(ctx, t, testInstallation) +} diff --git a/test/kubernetes/e2e/tests/full_envoy_validation_tests.go b/test/kubernetes/e2e/tests/full_envoy_validation_tests.go new file mode 100644 index 00000000000..322d21c4693 --- /dev/null +++ b/test/kubernetes/e2e/tests/full_envoy_validation_tests.go @@ -0,0 +1,20 @@ +package tests + +import ( + "github.com/solo-io/gloo/test/kubernetes/e2e" + "github.com/solo-io/gloo/test/kubernetes/e2e/features/basicrouting" + "github.com/solo-io/gloo/test/kubernetes/e2e/features/validation/full_envoy_validation" +) + +func FullEnvoyValidationSuiteRunner() e2e.SuiteRunner { + validationSuiteRunner := e2e.NewSuiteRunner(false) + + // These tests verify the efficacy of the full envoy validate mode with known-bad config + // that is not caught during Gloo translation. + validationSuiteRunner.Register("FullEnvoyValidation", full_envoy_validation.NewTestingSuite) + // These tests verify that our happypath config is not adversely affected by the + // addition of the full envoy validate mode after translation. + validationSuiteRunner.Register("BasicRoutingHappyPath", basicrouting.NewBasicEdgeRoutingSuite) + + return validationSuiteRunner +} diff --git a/test/kubernetes/e2e/tests/manifests/full-envoy-validation-helm.yaml b/test/kubernetes/e2e/tests/manifests/full-envoy-validation-helm.yaml new file mode 100644 index 00000000000..2c13d97feb5 --- /dev/null +++ b/test/kubernetes/e2e/tests/manifests/full-envoy-validation-helm.yaml @@ -0,0 +1,7 @@ +gateway: + validation: + failurePolicy: Fail # For "strict" validation mode, fail the validation if webhook server is not available + allowWarnings: false # For "strict" validation mode, webhook will also reject warnings + # transformation validation is disabled because full envoy validation is enabled. + disableTransformationValidation: true + fullEnvoyValidation: true