From 401f8867e526aaa4a8cda785d6abb51a2c94e4a0 Mon Sep 17 00:00:00 2001 From: Steve Sloka Date: Thu, 18 Jun 2020 00:01:21 -0400 Subject: [PATCH] cmd/contour: replace current xds server with go-control-plane impl Fixes #2134 by implementing the Envoy go-control-plane to replace Contour's current custom xDS gRPC server. The change utilizes snapshots as a way to represent a versioned point in time representation of the xDS resources (RDS, CDS, EDS, LDS, SDS). When the dag is rebuilt or an endpoint changes, a new snapshot is created with the updated caches for each xDS resource type. Signed-off-by: Steve Sloka --- cmd/contour/cli.go | 3 + cmd/contour/serve.go | 51 +- examples/contour/03-envoy.yaml | 2 +- examples/render/contour.yaml | 2 +- go.mod | 2 + go.sum | 28 + internal/assert/assert.go | 22 - internal/contour/cachehandler.go | 7 + internal/contour/cluster.go | 28 +- internal/contour/cluster_test.go | 86 +-- internal/contour/cond.go | 94 --- internal/contour/cond_test.go | 108 --- internal/contour/endpointstranslator.go | 35 +- internal/contour/endpointstranslator_test.go | 151 ++-- internal/contour/example_test.go | 47 -- internal/contour/listener.go | 34 +- internal/contour/listener_test.go | 68 +- internal/contour/route.go | 37 +- internal/contour/route_test.go | 66 +- internal/contour/secret.go | 27 +- internal/contour/secret_test.go | 50 +- internal/contour/snapshot.go | 90 +++ internal/e2e/cds_test.go | 340 ++++----- internal/e2e/e2e.go | 38 +- internal/e2e/eds_test.go | 115 ++- internal/e2e/lds_test.go | 573 ++++++--------- internal/e2e/rds_test.go | 676 ++++++++---------- internal/envoy/accesslog_test.go | 2 +- internal/envoy/auth_test.go | 7 +- internal/envoy/endpoint_test.go | 22 +- internal/envoy/healthcheck_test.go | 7 +- internal/envoy/listener_test.go | 5 +- internal/envoy/secret_test.go | 6 +- internal/envoy/stats_test.go | 7 +- .../featuretests/backendcavalidation_test.go | 1 - internal/featuretests/featuretests.go | 38 +- internal/grpc/server.go | 114 +-- internal/grpc/server_test.go | 275 ------- internal/grpc/xds.go | 165 ----- internal/grpc/xds_test.go | 209 ------ internal/protobuf/helpers.go | 8 +- 41 files changed, 1043 insertions(+), 2603 deletions(-) delete mode 100644 internal/contour/cond.go delete mode 100644 internal/contour/cond_test.go delete mode 100644 internal/contour/example_test.go create mode 100644 internal/contour/snapshot.go delete mode 100644 internal/grpc/server_test.go delete mode 100644 internal/grpc/xds.go delete mode 100644 internal/grpc/xds_test.go diff --git a/cmd/contour/cli.go b/cmd/contour/cli.go index 7ae7ae3190a..82535d30ccc 100644 --- a/cmd/contour/cli.go +++ b/cmd/contour/cli.go @@ -22,6 +22,8 @@ import ( "log" "os" + envoy_api_v2_core "github.com/envoyproxy/go-control-plane/envoy/api/v2/core" + v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2" "github.com/golang/protobuf/proto" "google.golang.org/grpc" @@ -124,6 +126,7 @@ func watchstream(st stream, typeURL string, resources []string) { req := &v2.DiscoveryRequest{ TypeUrl: typeURL, ResourceNames: resources, + Node: &envoy_api_v2_core.Node{Id: "contour"}, } err := st.Send(req) check(err) diff --git a/cmd/contour/serve.go b/cmd/contour/serve.go index f60e876286f..2605750a1fe 100644 --- a/cmd/contour/serve.go +++ b/cmd/contour/serve.go @@ -23,6 +23,7 @@ import ( "syscall" "time" + "github.com/envoyproxy/go-control-plane/pkg/cache/v2" projectcontourv1alpha1 "github.com/projectcontour/contour/apis/projectcontour/v1alpha1" "github.com/projectcontour/contour/internal/annotation" "github.com/projectcontour/contour/internal/contour" @@ -199,14 +200,16 @@ func doServe(log logrus.FieldLogger, ctx *serveContext) error { listenerConfig.DefaultHTTPVersions = defaultHTTPVersions + ch := &contour.CacheHandler{ + ListenerVisitorConfig: listenerConfig, + ListenerCache: contour.NewListenerCache(ctx.statsAddr, ctx.statsPort), + FieldLogger: log.WithField("context", "CacheHandler"), + Metrics: metrics.NewMetrics(registry), + } + // step 3. build our mammoth Kubernetes event handler. eventHandler := &contour.EventHandler{ - CacheHandler: &contour.CacheHandler{ - ListenerVisitorConfig: listenerConfig, - ListenerCache: contour.NewListenerCache(ctx.statsAddr, ctx.statsPort), - FieldLogger: log.WithField("context", "CacheHandler"), - Metrics: metrics.NewMetrics(registry), - }, + CacheHandler: ch, HoldoffDelay: 100 * time.Millisecond, HoldoffMaxDelay: 500 * time.Millisecond, Builder: dag.Builder{ @@ -275,6 +278,28 @@ func doServe(log logrus.FieldLogger, ctx *serveContext) error { FieldLogger: log.WithField("context", "endpointstranslator"), } + // snapshotCache is used to store the state of what all xDS services should + // contain at any given point in time. + snapshotCache := cache.NewSnapshotCache(false, contour.DefaultHash, + log.WithField("context", "xDS")) + + // snapshotHandler is used to produce new snapshots when the internal state changes + // for any xDS resource. + snapshotHandler := &contour.SnapshotHandler{ + CacheHandler: ch, + EndpointsTranslator: et, + SnapshotCache: snapshotCache, + } + + // Pass the snapshotHandler off to the CacheHandler & EndpointsTranslator + // so they are able to inform the snapshotHandler that the xDS resources + // have changed and should be updated in Envoy. + ch.SnapshotHandler = snapshotHandler + et.SnapshotHandler = snapshotHandler + + // Trigger an initial snapshot for static resources + snapshotHandler.UpdateSnapshot() + informerSyncList.InformOnResources(clusterInformerFactory, &k8s.DynamicClientHandler{ Next: &contour.EventRecorder{ @@ -401,15 +426,9 @@ func doServe(log logrus.FieldLogger, ctx *serveContext) error { } log.Printf("informer caches synced") - resources := map[string]cgrpc.Resource{ - eventHandler.CacheHandler.ClusterCache.TypeURL(): &eventHandler.CacheHandler.ClusterCache, - eventHandler.CacheHandler.RouteCache.TypeURL(): &eventHandler.CacheHandler.RouteCache, - eventHandler.CacheHandler.ListenerCache.TypeURL(): &eventHandler.CacheHandler.ListenerCache, - eventHandler.CacheHandler.SecretCache.TypeURL(): &eventHandler.CacheHandler.SecretCache, - et.TypeURL(): et, - } opts := ctx.grpcOptions() - s := cgrpc.NewAPI(log, resources, registry, opts...) + grpcServer := cgrpc.NewAPI(registry, snapshotCache, opts...) + addr := net.JoinHostPort(ctx.xdsAddr, strconv.Itoa(ctx.xdsPort)) l, err := net.Listen("tcp", addr) if err != nil { @@ -426,10 +445,10 @@ func doServe(log logrus.FieldLogger, ctx *serveContext) error { go func() { <-stop - s.Stop() + grpcServer.Stop() }() - return s.Serve(l) + return grpcServer.Serve(l) }) // step 14. Setup SIGTERM handler diff --git a/examples/contour/03-envoy.yaml b/examples/contour/03-envoy.yaml index 91a31cba04f..28b6c53daaa 100644 --- a/examples/contour/03-envoy.yaml +++ b/examples/contour/03-envoy.yaml @@ -48,7 +48,7 @@ spec: - -c - /config/envoy.json - --service-cluster $(CONTOUR_NAMESPACE) - - --service-node $(ENVOY_POD_NAME) + - --service-node contour - --log-level info command: - envoy diff --git a/examples/render/contour.yaml b/examples/render/contour.yaml index 1f48923edee..c42ad739a0b 100644 --- a/examples/render/contour.yaml +++ b/examples/render/contour.yaml @@ -1788,7 +1788,7 @@ spec: - -c - /config/envoy.json - --service-cluster $(CONTOUR_NAMESPACE) - - --service-node $(ENVOY_POD_NAME) + - --service-node contour - --log-level info command: - envoy diff --git a/go.mod b/go.mod index 143c71cadcf..034cf604a79 100644 --- a/go.mod +++ b/go.mod @@ -19,6 +19,7 @@ require ( github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4 github.com/prometheus/common v0.6.0 github.com/sirupsen/logrus v1.4.2 + github.com/stretchr/testify v1.4.0 golang.org/x/tools v0.0.0-20190929041059-e7abfedfabcf // indirect google.golang.org/grpc v1.25.1 gopkg.in/alecthomas/kingpin.v2 v2.2.6 @@ -28,6 +29,7 @@ require ( k8s.io/client-go v0.18.2 k8s.io/gengo v0.0.0-20191120174120-e74f70b9b27e // indirect k8s.io/klog v1.0.0 + k8s.io/utils v0.0.0-20200324210504-a9aa75ae1b89 sigs.k8s.io/controller-tools v0.2.9 sigs.k8s.io/kustomize/kyaml v0.1.1 sigs.k8s.io/service-apis v0.0.0-20200213014236-51691dd89266 diff --git a/go.sum b/go.sum index e2b8f4d1c12..ed04f17c998 100644 --- a/go.sum +++ b/go.sum @@ -77,6 +77,7 @@ github.com/elazarl/goproxy v0.0.0-20180725130230-947c36da3153/go.mod h1:/Zj4wYkg github.com/emicklei/go-restful v0.0.0-20170410110728-ff4f55a20633/go.mod h1:otzb+WCGbkyDHkqmQmT5YD2WR4BBwUdeQoFo8l/7tVs= github.com/emicklei/go-restful v2.9.5+incompatible/go.mod h1:otzb+WCGbkyDHkqmQmT5YD2WR4BBwUdeQoFo8l/7tVs= github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= +github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= github.com/envoyproxy/go-control-plane v0.9.5 h1:lRJIqDD8yjV1YyPRqecMdytjDLs2fTXq363aCib5xPU= github.com/envoyproxy/go-control-plane v0.9.5/go.mod h1:OXl5to++W0ctG+EHWTFUjiypVxC/Y4VLc/KFU+al13s= github.com/envoyproxy/protoc-gen-validate v0.1.0 h1:EQciDnbrYxy13PgWoY8AqoxGiPrpgBZ1R8UNe3ddc+A= @@ -174,12 +175,22 @@ github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5y github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.2 h1:6nsPYzhq5kReh6QImI3k5qWzO4PEbvbIW2cwSfR/6xs= github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= +github.com/golang/protobuf v1.4.0-rc.1/go.mod h1:ceaxUfeHdC40wWswd/P6IGgMaK3YpKi5j83Wpe3EHw8= +github.com/golang/protobuf v1.4.0-rc.1.0.20200221234624-67d41d38c208/go.mod h1:xKAWHe0F5eneWXFV3EuXVDTCmh+JuBKY0li0aMyXATA= +github.com/golang/protobuf v1.4.0-rc.2/go.mod h1:LlEzMj4AhA7rCAGe4KMBDvJI+AwstrUpVNzEA03Pprs= +github.com/golang/protobuf v1.4.0-rc.4.0.20200313231945-b860323f09d0/go.mod h1:WU3c8KckQ9AFe+yFwt9sWVRKCVIyN9cPHBJSNnbL67w= +github.com/golang/protobuf v1.4.0/go.mod h1:jodUvKwWbYaEsadDk5Fwe5c77LiNKVO9IDvqG2KuDX0= +github.com/golang/protobuf v1.4.1 h1:ZFgWrT+bLgsYPirOnRfKLYJLvssAegOj/hgyMFdJZe0= +github.com/golang/protobuf v1.4.1/go.mod h1:U8fpvMrcmy5pZrNK1lt4xCsGvpyWQ/VVv6QDs8UjoX8= github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ= github.com/google/btree v1.0.0/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ= github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M= github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= +github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= github.com/google/go-cmp v0.4.0 h1:xsAVV57WRhGj6kEIi8ReJzQlHHqcBYCElAvkovg3B/4= github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/google/go-cmp v0.5.0 h1:/QaMHBdZ26BB3SSst0Iwl10Epc+xhTquomWX0oZEB6w= +github.com/google/go-cmp v0.5.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/gofuzz v0.0.0-20161122191042-44d81051d367/go.mod h1:HP5RmnzzSNb993RKQDq4+1A4ia9nllfqcQFTQJedwGI= github.com/google/gofuzz v1.0.0 h1:A8PeW59pxE9IoFRqBp37U+mSNaQoZ46F1f0f863XSXw= github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= @@ -506,12 +517,29 @@ google.golang.org/genproto v0.0.0-20190418145605-e7d98fc518a7/go.mod h1:VzzqZJRn google.golang.org/genproto v0.0.0-20190502173448-54afdca5d873/go.mod h1:VzzqZJRnGkLBvHegQrXjBqPurQTc5/KpmUdxsrq26oE= google.golang.org/genproto v0.0.0-20190819201941-24fa4b261c55 h1:gSJIx1SDwno+2ElGhA4+qG2zF97qiUzTM+rQ0klBOcE= google.golang.org/genproto v0.0.0-20190819201941-24fa4b261c55/go.mod h1:DMBHOl98Agz4BDEuKkezgsaosCRResVns1a3J2ZsMNc= +google.golang.org/genproto v0.0.0-20200526211855-cb27e3aa2013 h1:+kGHl1aib/qcwaRi1CbqBZ1rk19r85MNUf8HaBghugY= +google.golang.org/genproto v0.0.0-20200526211855-cb27e3aa2013/go.mod h1:NbSheEEYHJ7i3ixzK3sjbqSGDJWnxyFXZblF3eUsNvo= google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c= google.golang.org/grpc v1.23.0 h1:AzbTB6ux+okLTzP8Ru1Xs41C303zdcfEht7MQnYJt5A= google.golang.org/grpc v1.23.0/go.mod h1:Y5yQAOtifL1yxbo5wqy6BxZv8vAUGQwXBOALyacEbxg= google.golang.org/grpc v1.23.1/go.mod h1:Y5yQAOtifL1yxbo5wqy6BxZv8vAUGQwXBOALyacEbxg= google.golang.org/grpc v1.25.1 h1:wdKvqQk7IttEw92GoRyKG2IDrUIpgpj6H6m81yfeMW0= google.golang.org/grpc v1.25.1/go.mod h1:c3i+UQWmh7LiEpx4sFZnkU36qjEYZ0imhYfXVyQciAY= +google.golang.org/grpc v1.27.0 h1:rRYRFMVgRv6E0D70Skyfsr28tDXIuuPZyWGMPdMcnXg= +google.golang.org/grpc v1.27.0/go.mod h1:qbnxyOmOxrQa7FizSgH+ReBfzJrCY1pSN7KXBS8abTk= +google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8= +google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0= +google.golang.org/protobuf v0.0.0-20200228230310-ab0ca4ff8a60/go.mod h1:cfTl7dwQJ+fmap5saPgwCLgHXTUD7jkjRqWcaiX5VyM= +google.golang.org/protobuf v1.20.0/go.mod h1:A+miEFZTKqfCUM6K7xSMQL9OKL/b6hQv+e19PK+JZNE= +google.golang.org/protobuf v1.20.1-0.20200309200217-e05f789c0967/go.mod h1:A+miEFZTKqfCUM6K7xSMQL9OKL/b6hQv+e19PK+JZNE= +google.golang.org/protobuf v1.20.1/go.mod h1:KqelGeouBkcbcuB3HCk4/YH2tmNLk6YSWA5LIWeI/lY= +google.golang.org/protobuf v1.21.0/go.mod h1:47Nbq4nVaFHyn7ilMalzfO3qCViNmqZ2kzikPIcrTAo= +google.golang.org/protobuf v1.22.0/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2l/sGQquU= +google.golang.org/protobuf v1.23.0/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2l/sGQquU= +google.golang.org/protobuf v1.23.1-0.20200526195155-81db48ad09cc/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2l/sGQquU= +google.golang.org/protobuf v1.24.0/go.mod h1:r/3tXBNzIEhYS9I1OUVjXDlt8tc493IdKGjtUeSXeh4= +google.golang.org/protobuf v1.25.0 h1:Ejskq+SyPohKW+1uil0JJMtmHCgJPJ/qWTxr8qp+R4c= +google.golang.org/protobuf v1.25.0/go.mod h1:9JNX74DMeImyA3h4bdi1ymwjUzf21/xIlbajtzgsN7c= gopkg.in/alecthomas/kingpin.v2 v2.2.6 h1:jMFz6MfLP0/4fUyZle81rXUoxOBFi19VUFKVDOQfozc= gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/internal/assert/assert.go b/internal/assert/assert.go index 51c0036968a..7237463295b 100644 --- a/internal/assert/assert.go +++ b/internal/assert/assert.go @@ -17,12 +17,7 @@ package assert import ( "testing" - v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2" - "github.com/golang/protobuf/proto" - "github.com/golang/protobuf/ptypes" - "github.com/golang/protobuf/ptypes/any" "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" ) type Assert struct { @@ -41,8 +36,6 @@ func Equal(t *testing.T, want, got interface{}, opts ...cmp.Option) { func (a Assert) Equal(want, got interface{}, opts ...cmp.Option) { a.t.Helper() opts = append(opts, - cmpopts.IgnoreFields(v2.DiscoveryResponse{}, "VersionInfo", "Nonce"), - cmpopts.AcyclicTransformer("UnmarshalAny", unmarshalAny), // errors to be equal only if both are nil or both are non-nil. cmp.Comparer(func(x, y error) bool { return (x == nil) == (y == nil) @@ -53,18 +46,3 @@ func (a Assert) Equal(want, got interface{}, opts ...cmp.Option) { a.t.Fatal(diff) } } - -func unmarshalAny(a *any.Any) proto.Message { - if a == nil { - return nil - } - pb, err := ptypes.Empty(a) - if err != nil { - panic(err.Error()) - } - err = ptypes.UnmarshalAny(a, pb) - if err != nil { - panic(err.Error()) - } - return pb -} diff --git a/internal/contour/cachehandler.go b/internal/contour/cachehandler.go index b3029d69906..4f57eff2389 100644 --- a/internal/contour/cachehandler.go +++ b/internal/contour/cachehandler.go @@ -33,6 +33,11 @@ type CacheHandler struct { ClusterCache SecretCache + // SnapshotHandler is used to generate new snapshots + // when any caches change so that Envoy can be updated + // with the new configuration. + SnapshotHandler *SnapshotHandler + *metrics.Metrics logrus.FieldLogger @@ -47,6 +52,8 @@ func (ch *CacheHandler) OnChange(dag *dag.DAG) { ch.updateRoutes(dag) ch.updateClusters(dag) + ch.SnapshotHandler.UpdateSnapshot() + ch.SetDAGLastRebuilt(time.Now()) } diff --git a/internal/contour/cluster.go b/internal/contour/cluster.go index 5c3c5fd5450..930356264d2 100644 --- a/internal/contour/cluster.go +++ b/internal/contour/cluster.go @@ -17,10 +17,8 @@ import ( "sort" "sync" - resource "github.com/envoyproxy/go-control-plane/pkg/resource/v2" - v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2" - "github.com/golang/protobuf/proto" + "github.com/envoyproxy/go-control-plane/pkg/cache/types" "github.com/projectcontour/contour/internal/dag" "github.com/projectcontour/contour/internal/envoy" "github.com/projectcontour/contour/internal/protobuf" @@ -31,7 +29,6 @@ import ( type ClusterCache struct { mu sync.Mutex values map[string]*v2.Cluster - Cond } // Update replaces the contents of the cache with the supplied map. @@ -40,11 +37,10 @@ func (c *ClusterCache) Update(v map[string]*v2.Cluster) { defer c.mu.Unlock() c.values = v - c.Cond.Notify() } // Contents returns a copy of the cache's contents. -func (c *ClusterCache) Contents() []proto.Message { +func (c *ClusterCache) Contents() []types.Resource { c.mu.Lock() defer c.mu.Unlock() var values []*v2.Cluster @@ -55,26 +51,6 @@ func (c *ClusterCache) Contents() []proto.Message { return protobuf.AsMessages(values) } -func (c *ClusterCache) Query(names []string) []proto.Message { - c.mu.Lock() - defer c.mu.Unlock() - var values []*v2.Cluster - for _, n := range names { - // if the cluster is not registered we cannot return - // a blank cluster because each cluster has a required - // discovery type; DNS, EDS, etc. We cannot determine the - // correct value for this property from the cluster's name - // provided by the query so we must not return a blank cluster. - if v, ok := c.values[n]; ok { - values = append(values, v) - } - } - sort.Stable(sorter.For(values)) - return protobuf.AsMessages(values) -} - -func (*ClusterCache) TypeURL() string { return resource.ClusterType } - type clusterVisitor struct { clusters map[string]*v2.Cluster } diff --git a/internal/contour/cluster_test.go b/internal/contour/cluster_test.go index 8dc106b9681..708e9376294 100644 --- a/internal/contour/cluster_test.go +++ b/internal/contour/cluster_test.go @@ -17,6 +17,8 @@ import ( "testing" "time" + "github.com/envoyproxy/go-control-plane/pkg/cache/types" + v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2" envoy_api_v2_cluster "github.com/envoyproxy/go-control-plane/envoy/api/v2/cluster" envoy_api_v2_core "github.com/envoyproxy/go-control-plane/envoy/api/v2/core" @@ -35,7 +37,7 @@ import ( func TestClusterCacheContents(t *testing.T) { tests := map[string]struct { contents map[string]*v2.Cluster - want []proto.Message + want []types.Resource }{ "empty": { contents: nil, @@ -52,7 +54,7 @@ func TestClusterCacheContents(t *testing.T) { ServiceName: "default/kuard", }, }), - want: []proto.Message{ + want: []types.Resource{ cluster(&v2.Cluster{ Name: "default/kuard/443/da39a3ee5e", AltStatName: "default_kuard_443", @@ -76,86 +78,6 @@ func TestClusterCacheContents(t *testing.T) { } } -func TestClusterCacheQuery(t *testing.T) { - tests := map[string]struct { - contents map[string]*v2.Cluster - query []string - want []proto.Message - }{ - "exact match": { - contents: clustermap( - &v2.Cluster{ - Name: "default/kuard/443/da39a3ee5e", - AltStatName: "default_kuard_443", - ClusterDiscoveryType: envoy.ClusterDiscoveryType(v2.Cluster_EDS), - EdsClusterConfig: &v2.Cluster_EdsClusterConfig{ - EdsConfig: envoy.ConfigSource("contour"), - ServiceName: "default/kuard", - }, - }), - query: []string{"default/kuard/443/da39a3ee5e"}, - want: []proto.Message{ - cluster(&v2.Cluster{ - Name: "default/kuard/443/da39a3ee5e", - AltStatName: "default_kuard_443", - ClusterDiscoveryType: envoy.ClusterDiscoveryType(v2.Cluster_EDS), - EdsClusterConfig: &v2.Cluster_EdsClusterConfig{ - EdsConfig: envoy.ConfigSource("contour"), - ServiceName: "default/kuard", - }, - }), - }, - }, - "partial match": { - contents: clustermap( - &v2.Cluster{ - Name: "default/kuard/443/da39a3ee5e", - AltStatName: "default_kuard_443", - ClusterDiscoveryType: envoy.ClusterDiscoveryType(v2.Cluster_EDS), - EdsClusterConfig: &v2.Cluster_EdsClusterConfig{ - EdsConfig: envoy.ConfigSource("contour"), - ServiceName: "default/kuard", - }, - }), - query: []string{"default/kuard/443/da39a3ee5e", "foo/bar/baz"}, - want: []proto.Message{ - cluster(&v2.Cluster{ - Name: "default/kuard/443/da39a3ee5e", - AltStatName: "default_kuard_443", - ClusterDiscoveryType: envoy.ClusterDiscoveryType(v2.Cluster_EDS), - EdsClusterConfig: &v2.Cluster_EdsClusterConfig{ - EdsConfig: envoy.ConfigSource("contour"), - ServiceName: "default/kuard", - }, - }), - }, - }, - "no match": { - contents: clustermap( - &v2.Cluster{ - Name: "default/kuard/443/da39a3ee5e", - AltStatName: "default_kuard_443", - ClusterDiscoveryType: envoy.ClusterDiscoveryType(v2.Cluster_EDS), - EdsClusterConfig: &v2.Cluster_EdsClusterConfig{ - EdsConfig: envoy.ConfigSource("contour"), - ServiceName: "default/kuard", - }, - }), - query: []string{"foo/bar/baz"}, - want: nil, - }, - } - - for name, tc := range tests { - t.Run(name, func(t *testing.T) { - var cc ClusterCache - cc.Update(tc.contents) - got := cc.Query(tc.query) - assert.Equal(t, tc.want, got) - }) - } -} - func TestClusterVisit(t *testing.T) { tests := map[string]struct { objs []interface{} diff --git a/internal/contour/cond.go b/internal/contour/cond.go deleted file mode 100644 index 031c57ed4cf..00000000000 --- a/internal/contour/cond.go +++ /dev/null @@ -1,94 +0,0 @@ -// Copyright Project Contour Authors -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package contour - -import "sync" - -// Cond implements a condition variable, a rendezvous point for goroutines -// waiting for or announcing the ocurence of an event. -// -// Unlike sync.Cond, Cond communciates with waiters via channels registered by -// the waiters. This permits goroutines to wait on Cond events using select. -type Cond struct { - mu sync.Mutex - waiters []waiter - last int -} - -type waiter struct { - ch chan int - hints []string -} - -// Register registers ch to receive a value when Notify is called. -// The value of last is the count of the times Notify has been called on this Cond. -// It functions of a sequence counter, if the value of last supplied to Register -// is less than the Conds internal counter, then the caller has missed at least -// one notification and will fire immediately. -// -// Sends by the broadcaster to ch must not block, therefore ch must have a capacity -// of at least 1. -func (c *Cond) Register(ch chan int, last int, hints ...string) { - c.mu.Lock() - defer c.mu.Unlock() - - if last < c.last { - // notify this channel immediately - ch <- c.last - return - } - c.waiters = append(c.waiters, waiter{ - ch: ch, - hints: hints, - }) -} - -// Notify notifies all interested waiters that an event has ocured. -func (c *Cond) Notify(hints ...string) { - c.mu.Lock() - defer c.mu.Unlock() - c.last++ - - notify := c.waiters - c.waiters = nil - - for _, waiter := range notify { - if len(hints) == 0 { - // notify unconditionally - waiter.ch <- c.last - continue - } - if intersection(hints, waiter.hints) { - // one of the hints registered has been notified - waiter.ch <- c.last - continue - } - - // not notified this time, put back on the list - c.waiters = append(c.waiters, waiter) - } -} - -// intersection returns true if the set of elements in left -// intersects with the set in right. -func intersection(left, right []string) bool { - for _, l := range left { - for _, r := range right { - if l == r { - return true - } - } - } - return false -} diff --git a/internal/contour/cond_test.go b/internal/contour/cond_test.go deleted file mode 100644 index 22a61269422..00000000000 --- a/internal/contour/cond_test.go +++ /dev/null @@ -1,108 +0,0 @@ -// Copyright Project Contour Authors -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package contour - -import "testing" - -func TestCondRegisterBeforeNotifyShouldNotBroadcast(t *testing.T) { - var c Cond - ch := make(chan int, 1) - c.Register(ch, 0) - select { - case <-ch: - t.Fatal("ch was notified before broadcast") - default: - } -} - -func TestCondRegisterAfterNotifyShouldBroadcast(t *testing.T) { - var c Cond - ch := make(chan int, 1) - c.Notify() - c.Register(ch, 0) - select { - case v := <-ch: - if v != 1 { - t.Fatal("ch was notified with the wrong sequence number", v) - } - default: - t.Fatal("ch was not notified on registration") - } -} - -func TestCondRegisterAfterNotifyWithCorrectSequenceShouldNotBroadcast(t *testing.T) { - var c Cond - ch := make(chan int, 1) - c.Notify() - c.Register(ch, 0) - seq := <-ch - - c.Register(ch, seq) - select { - case v := <-ch: - t.Fatal("ch was notified immediately with seq", v) - default: - } -} - -func TestCondRegisterWithHintShouldNotifyWithoutHint(t *testing.T) { - var c Cond - ch := make(chan int, 1) - c.Register(ch, 1, "ingress_https") - c.Notify() - select { - case v := <-ch: - if v != 1 { - t.Fatal("ch was notified with the wrong sequence number", v) - } - default: - t.Fatal("ch was not notified") - } -} - -func TestCondRegisterWithHintShouldNotifyWithHint(t *testing.T) { - var c Cond - ch := make(chan int, 1) - c.Register(ch, 1, "ingress_https") - c.Notify("ingress_https") - select { - case v := <-ch: - if v != 1 { - t.Fatal("ch was notified with the wrong sequence number", v) - } - default: - t.Fatal("ch was not notified") - } -} - -func TestCondRegisterWithHintShouldNotNotifyWithWrongHint(t *testing.T) { - var c Cond - ch := make(chan int, 1) - c.Register(ch, 1, "ingress_https") - c.Notify("banana") - select { - case v := <-ch: - t.Fatal("ch was notified when it should not be", v) - default: - } - c.Notify("ingress_https") - select { - case v := <-ch: - if v != 2 { - t.Fatal("ch was notified with the wrong sequence number", v) - } - default: - t.Fatal("ch was not notified") - } -} diff --git a/internal/contour/endpointstranslator.go b/internal/contour/endpointstranslator.go index 68de1bb3fe7..05378a1ed66 100644 --- a/internal/contour/endpointstranslator.go +++ b/internal/contour/endpointstranslator.go @@ -18,10 +18,10 @@ import ( "strings" "sync" + "github.com/envoyproxy/go-control-plane/pkg/cache/types" + v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2" envoy_api_v2_endpoint "github.com/envoyproxy/go-control-plane/envoy/api/v2/endpoint" - resource "github.com/envoyproxy/go-control-plane/pkg/resource/v2" - "github.com/golang/protobuf/proto" "github.com/projectcontour/contour/internal/envoy" "github.com/projectcontour/contour/internal/protobuf" "github.com/projectcontour/contour/internal/sorter" @@ -36,6 +36,11 @@ import ( type EndpointsTranslator struct { logrus.FieldLogger clusterLoadAssignmentCache + + // SnapshotHandler is used to generate new snapshots + // when the clusterLoadAssignmentCache changes Envoy + // can be updated with the new configuration. + SnapshotHandler *SnapshotHandler } func (e *EndpointsTranslator) OnAdd(obj interface{}) { @@ -72,32 +77,12 @@ func (e *EndpointsTranslator) OnDelete(obj interface{}) { } } -func (e *EndpointsTranslator) Contents() []proto.Message { +func (e *EndpointsTranslator) Contents() []types.Resource { values := e.clusterLoadAssignmentCache.Contents() sort.Stable(sorter.For(values)) return protobuf.AsMessages(values) } -func (e *EndpointsTranslator) Query(names []string) []proto.Message { - e.clusterLoadAssignmentCache.mu.Lock() - defer e.clusterLoadAssignmentCache.mu.Unlock() - values := make([]*v2.ClusterLoadAssignment, 0, len(names)) - for _, n := range names { - v, ok := e.entries[n] - if !ok { - v = &v2.ClusterLoadAssignment{ - ClusterName: n, - } - } - values = append(values, v) - } - - sort.Stable(sorter.For(values)) - return protobuf.AsMessages(values) -} - -func (*EndpointsTranslator) TypeURL() string { return resource.EndpointType } - func (e *EndpointsTranslator) addEndpoints(ep *v1.Endpoints) { e.recomputeClusterLoadAssignment(nil, ep) } @@ -182,12 +167,12 @@ func (e *EndpointsTranslator) recomputeClusterLoadAssignment(oldep, newep *v1.En } } + e.SnapshotHandler.UpdateSnapshot() } type clusterLoadAssignmentCache struct { mu sync.Mutex entries map[string]*v2.ClusterLoadAssignment - Cond } // Add adds an entry to the cache. If a ClusterLoadAssignment with the same @@ -199,7 +184,6 @@ func (c *clusterLoadAssignmentCache) Add(a *v2.ClusterLoadAssignment) { c.entries = make(map[string]*v2.ClusterLoadAssignment) } c.entries[a.ClusterName] = a - c.Notify(a.ClusterName) } // Remove removes the named entry from the cache. If the entry @@ -208,7 +192,6 @@ func (c *clusterLoadAssignmentCache) Remove(name string) { c.mu.Lock() defer c.mu.Unlock() delete(c.entries, name) - c.Notify(name) } // Contents returns a copy of the contents of the cache. diff --git a/internal/contour/endpointstranslator_test.go b/internal/contour/endpointstranslator_test.go index 5fce24d63e0..daa08b2b0d1 100644 --- a/internal/contour/endpointstranslator_test.go +++ b/internal/contour/endpointstranslator_test.go @@ -16,8 +16,10 @@ package contour import ( "testing" + "github.com/envoyproxy/go-control-plane/pkg/cache/v2" + v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2" - "github.com/golang/protobuf/proto" + "github.com/envoyproxy/go-control-plane/pkg/cache/types" "github.com/projectcontour/contour/internal/assert" "github.com/projectcontour/contour/internal/envoy" v1 "k8s.io/api/core/v1" @@ -26,7 +28,7 @@ import ( func TestEndpointsTranslatorContents(t *testing.T) { tests := map[string]struct { contents map[string]*v2.ClusterLoadAssignment - want []proto.Message + want []types.Resource }{ "empty": { contents: nil, @@ -38,7 +40,7 @@ func TestEndpointsTranslatorContents(t *testing.T) { envoy.SocketAddress("10.10.10.10", 80), ), ), - want: []proto.Message{ + want: []types.Resource{ envoy.ClusterLoadAssignment("default/httpbin-org", envoy.SocketAddress("10.10.10.10", 80), ), @@ -56,66 +58,10 @@ func TestEndpointsTranslatorContents(t *testing.T) { } } -func TestEndpointCacheQuery(t *testing.T) { - tests := map[string]struct { - contents map[string]*v2.ClusterLoadAssignment - query []string - want []proto.Message - }{ - "exact match": { - contents: clusterloadassignments( - envoy.ClusterLoadAssignment("default/httpbin-org", - envoy.SocketAddress("10.10.10.10", 80), - ), - ), - query: []string{"default/httpbin-org"}, - want: []proto.Message{ - envoy.ClusterLoadAssignment("default/httpbin-org", - envoy.SocketAddress("10.10.10.10", 80), - ), - }, - }, - "partial match": { - contents: clusterloadassignments( - envoy.ClusterLoadAssignment("default/httpbin-org", - envoy.SocketAddress("10.10.10.10", 80), - ), - ), - query: []string{"default/kuard/8080", "default/httpbin-org"}, - want: []proto.Message{ - envoy.ClusterLoadAssignment("default/httpbin-org", - envoy.SocketAddress("10.10.10.10", 80), - ), - envoy.ClusterLoadAssignment("default/kuard/8080"), - }, - }, - "no match": { - contents: clusterloadassignments( - envoy.ClusterLoadAssignment("default/httpbin-org", - envoy.SocketAddress("10.10.10.10", 80), - ), - ), - query: []string{"default/kuard/8080"}, - want: []proto.Message{ - envoy.ClusterLoadAssignment("default/kuard/8080"), - }, - }, - } - - for name, tc := range tests { - t.Run(name, func(t *testing.T) { - var et EndpointsTranslator - et.entries = tc.contents - got := et.Query(tc.query) - assert.Equal(t, tc.want, got) - }) - } -} - func TestEndpointsTranslatorAddEndpoints(t *testing.T) { tests := map[string]struct { ep *v1.Endpoints - want []proto.Message + want []types.Resource }{ "simple": { ep: endpoints("default", "simple", v1.EndpointSubset{ @@ -124,7 +70,7 @@ func TestEndpointsTranslatorAddEndpoints(t *testing.T) { port("", 8080), ), }), - want: []proto.Message{ + want: []types.Resource{ envoy.ClusterLoadAssignment("default/simple", envoy.SocketAddress("192.168.183.24", 8080)), }, }, @@ -140,7 +86,7 @@ func TestEndpointsTranslatorAddEndpoints(t *testing.T) { port("", 80), ), }), - want: []proto.Message{ + want: []types.Resource{ envoy.ClusterLoadAssignment("default/httpbin-org", envoy.SocketAddress("23.23.247.89", 80), // addresses should be sorted envoy.SocketAddress("50.17.192.147", 80), @@ -159,7 +105,7 @@ func TestEndpointsTranslatorAddEndpoints(t *testing.T) { port("a", 8675), ), }), - want: []proto.Message{ + want: []types.Resource{ envoy.ClusterLoadAssignment("default/httpbin-org/a", // cluster names should be sorted envoy.SocketAddress("10.10.1.1", 8675), ), @@ -179,7 +125,7 @@ func TestEndpointsTranslatorAddEndpoints(t *testing.T) { port("a", 8675), ), }), - want: []proto.Message{ + want: []types.Resource{ envoy.ClusterLoadAssignment("default/httpbin-org/a", envoy.SocketAddress("10.10.1.1", 8675), // addresses should be sorted envoy.SocketAddress("10.10.2.2", 8675), @@ -210,7 +156,7 @@ func TestEndpointsTranslatorAddEndpoints(t *testing.T) { port("b", 309), ), }), - want: []proto.Message{ + want: []types.Resource{ envoy.ClusterLoadAssignment("default/httpbin-org/a", envoy.SocketAddress("10.10.1.1", 8675), ), @@ -228,6 +174,20 @@ func TestEndpointsTranslatorAddEndpoints(t *testing.T) { et := &EndpointsTranslator{ FieldLogger: log, } + ch := &CacheHandler{ + ListenerCache: ListenerCache{}, + RouteCache: RouteCache{}, + ClusterCache: ClusterCache{}, + SecretCache: SecretCache{}, + } + snapshotCache := cache.NewSnapshotCache(false, cache.IDHash{}, nil) + sh := &SnapshotHandler{ + CacheHandler: ch, + EndpointsTranslator: et, + SnapshotCache: snapshotCache, + } + + et.SnapshotHandler = sh et.OnAdd(tc.ep) got := et.Contents() assert.Equal(t, tc.want, got) @@ -239,7 +199,7 @@ func TestEndpointsTranslatorRemoveEndpoints(t *testing.T) { tests := map[string]struct { setup func(*EndpointsTranslator) ep *v1.Endpoints - want []proto.Message + want []types.Resource }{ "remove existing": { setup: func(et *EndpointsTranslator) { @@ -273,7 +233,7 @@ func TestEndpointsTranslatorRemoveEndpoints(t *testing.T) { port("", 8080), ), }), - want: []proto.Message{ + want: []types.Resource{ envoy.ClusterLoadAssignment("default/simple", envoy.SocketAddress("192.168.183.24", 8080)), }, }, @@ -329,6 +289,20 @@ func TestEndpointsTranslatorRemoveEndpoints(t *testing.T) { et := &EndpointsTranslator{ FieldLogger: log, } + ch := &CacheHandler{ + ListenerCache: ListenerCache{}, + RouteCache: RouteCache{}, + ClusterCache: ClusterCache{}, + SecretCache: SecretCache{}, + } + snapshotCache := cache.NewSnapshotCache(false, cache.IDHash{}, nil) + sh := &SnapshotHandler{ + CacheHandler: ch, + EndpointsTranslator: et, + SnapshotCache: snapshotCache, + } + et.SnapshotHandler = sh + tc.setup(et) et.OnDelete(tc.ep) got := et.Contents() @@ -340,7 +314,7 @@ func TestEndpointsTranslatorRemoveEndpoints(t *testing.T) { func TestEndpointsTranslatorRecomputeClusterLoadAssignment(t *testing.T) { tests := map[string]struct { oldep, newep *v1.Endpoints - want []proto.Message + want []types.Resource }{ "simple": { newep: endpoints("default", "simple", v1.EndpointSubset{ @@ -349,7 +323,7 @@ func TestEndpointsTranslatorRecomputeClusterLoadAssignment(t *testing.T) { port("", 8080), ), }), - want: []proto.Message{ + want: []types.Resource{ envoy.ClusterLoadAssignment("default/simple", envoy.SocketAddress("192.168.183.24", 8080)), }, }, @@ -365,7 +339,7 @@ func TestEndpointsTranslatorRecomputeClusterLoadAssignment(t *testing.T) { port("", 80), ), }), - want: []proto.Message{ + want: []types.Resource{ envoy.ClusterLoadAssignment("default/httpbin-org", envoy.SocketAddress("23.23.247.89", 80), envoy.SocketAddress("50.17.192.147", 80), @@ -381,7 +355,7 @@ func TestEndpointsTranslatorRecomputeClusterLoadAssignment(t *testing.T) { port("https", 8443), ), }), - want: []proto.Message{ + want: []types.Resource{ envoy.ClusterLoadAssignment("default/secure/https", envoy.SocketAddress("192.168.183.24", 8443)), }, }, @@ -398,7 +372,21 @@ func TestEndpointsTranslatorRecomputeClusterLoadAssignment(t *testing.T) { for name, tc := range tests { t.Run(name, func(t *testing.T) { - var et EndpointsTranslator + et := &EndpointsTranslator{} + ch := &CacheHandler{ + ListenerCache: ListenerCache{}, + RouteCache: RouteCache{}, + ClusterCache: ClusterCache{}, + SecretCache: SecretCache{}, + } + snapshotCache := cache.NewSnapshotCache(false, cache.IDHash{}, nil) + sh := &SnapshotHandler{ + CacheHandler: ch, + EndpointsTranslator: et, + SnapshotCache: snapshotCache, + } + + et.SnapshotHandler = sh et.recomputeClusterLoadAssignment(tc.oldep, tc.newep) got := et.Contents() assert.Equal(t, tc.want, got) @@ -408,7 +396,22 @@ func TestEndpointsTranslatorRecomputeClusterLoadAssignment(t *testing.T) { // See #602 func TestEndpointsTranslatorScaleToZeroEndpoints(t *testing.T) { - var et EndpointsTranslator + et := &EndpointsTranslator{} + ch := &CacheHandler{ + ListenerCache: ListenerCache{}, + RouteCache: RouteCache{}, + ClusterCache: ClusterCache{}, + SecretCache: SecretCache{}, + } + snapshotCache := cache.NewSnapshotCache(false, cache.IDHash{}, nil) + sh := &SnapshotHandler{ + CacheHandler: ch, + EndpointsTranslator: et, + SnapshotCache: snapshotCache, + } + + et.SnapshotHandler = sh + e1 := endpoints("default", "simple", v1.EndpointSubset{ Addresses: addresses("192.168.183.24"), Ports: ports( @@ -418,7 +421,7 @@ func TestEndpointsTranslatorScaleToZeroEndpoints(t *testing.T) { et.OnAdd(e1) // Assert endpoint was added - want := []proto.Message{ + want := []types.Resource{ envoy.ClusterLoadAssignment("default/simple", envoy.SocketAddress("192.168.183.24", 8080)), } got := et.Contents() diff --git a/internal/contour/example_test.go b/internal/contour/example_test.go deleted file mode 100644 index 34ca20c40d9..00000000000 --- a/internal/contour/example_test.go +++ /dev/null @@ -1,47 +0,0 @@ -// Copyright Project Contour Authors -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package contour_test - -import ( - "context" - "fmt" - "time" - - "github.com/projectcontour/contour/internal/contour" -) - -func ExampleCond() { - ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) - defer cancel() - ch := make(chan int, 1) - last := 0 - var c contour.Cond - go func() { - for { - time.Sleep(100 * time.Millisecond) - c.Notify() - } - }() - - for { - c.Register(ch, last) - select { - case last = <-ch: - fmt.Println("notification received:", last) - case <-ctx.Done(): - fmt.Println("timeout") - return - } - } -} diff --git a/internal/contour/listener.go b/internal/contour/listener.go index 7c56e2ddc96..e1bc6cb10a9 100644 --- a/internal/contour/listener.go +++ b/internal/contour/listener.go @@ -18,12 +18,12 @@ import ( "sort" "sync" + "github.com/envoyproxy/go-control-plane/pkg/cache/types" + v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2" envoy_api_v2_auth "github.com/envoyproxy/go-control-plane/envoy/api/v2/auth" envoy_api_v2_listener "github.com/envoyproxy/go-control-plane/envoy/api/v2/listener" envoy_api_v2_accesslog "github.com/envoyproxy/go-control-plane/envoy/config/filter/accesslog/v2" - resource "github.com/envoyproxy/go-control-plane/pkg/resource/v2" - "github.com/golang/protobuf/proto" "github.com/projectcontour/contour/internal/dag" "github.com/projectcontour/contour/internal/envoy" "github.com/projectcontour/contour/internal/protobuf" @@ -217,7 +217,6 @@ type ListenerCache struct { mu sync.Mutex values map[string]*v2.Listener staticValues map[string]*v2.Listener - Cond } // NewListenerCache returns an instance of a ListenerCache @@ -236,11 +235,10 @@ func (c *ListenerCache) Update(v map[string]*v2.Listener) { defer c.mu.Unlock() c.values = v - c.Cond.Notify() } // Contents returns a copy of the cache's contents. -func (c *ListenerCache) Contents() []proto.Message { +func (c *ListenerCache) Contents() []types.Resource { c.mu.Lock() defer c.mu.Unlock() var values []*v2.Listener @@ -250,37 +248,11 @@ func (c *ListenerCache) Contents() []proto.Message { for _, v := range c.staticValues { values = append(values, v) } - sort.Stable(sorter.For(values)) - return protobuf.AsMessages(values) -} -// Query returns the proto.Messages in the ListenerCache that match -// a slice of strings -func (c *ListenerCache) Query(names []string) []proto.Message { - c.mu.Lock() - defer c.mu.Unlock() - var values []*v2.Listener - for _, n := range names { - v, ok := c.values[n] - if !ok { - v, ok = c.staticValues[n] - if !ok { - // if the listener is not registered in - // dynamic or static values then skip it - // as there is no way to return a blank - // listener because the listener address - // field is required. - continue - } - } - values = append(values, v) - } sort.Stable(sorter.For(values)) return protobuf.AsMessages(values) } -func (*ListenerCache) TypeURL() string { return resource.ListenerType } - type listenerVisitor struct { *ListenerVisitorConfig diff --git a/internal/contour/listener_test.go b/internal/contour/listener_test.go index 3e7fd5aaca7..30cec088a1d 100644 --- a/internal/contour/listener_test.go +++ b/internal/contour/listener_test.go @@ -22,7 +22,7 @@ import ( envoy_api_v2_auth "github.com/envoyproxy/go-control-plane/envoy/api/v2/auth" envoy_api_v2_core "github.com/envoyproxy/go-control-plane/envoy/api/v2/core" envoy_api_v2_listener "github.com/envoyproxy/go-control-plane/envoy/api/v2/listener" - "github.com/golang/protobuf/proto" + xds "github.com/envoyproxy/go-control-plane/pkg/cache/types" projcontour "github.com/projectcontour/contour/apis/projectcontour/v1" "github.com/projectcontour/contour/internal/assert" "github.com/projectcontour/contour/internal/dag" @@ -37,7 +37,7 @@ import ( func TestListenerCacheContents(t *testing.T) { tests := map[string]struct { contents map[string]*v2.Listener - want []proto.Message + want []xds.Resource }{ "empty": { contents: nil, @@ -50,7 +50,7 @@ func TestListenerCacheContents(t *testing.T) { FilterChains: envoy.FilterChains(envoy.HTTPConnectionManager(ENVOY_HTTP_LISTENER, envoy.FileAccessLogEnvoy(DEFAULT_HTTP_ACCESS_LOG), 0)), SocketOptions: envoy.TCPKeepaliveSocketOptions(), }), - want: []proto.Message{ + want: []xds.Resource{ &v2.Listener{ Name: ENVOY_HTTP_LISTENER, Address: envoy.SocketAddress("0.0.0.0", 8080), @@ -71,68 +71,6 @@ func TestListenerCacheContents(t *testing.T) { } } -func TestListenerCacheQuery(t *testing.T) { - tests := map[string]struct { - contents map[string]*v2.Listener - query []string - want []proto.Message - }{ - "exact match": { - contents: listenermap(&v2.Listener{ - Name: ENVOY_HTTP_LISTENER, - Address: envoy.SocketAddress("0.0.0.0", 8080), - FilterChains: envoy.FilterChains(envoy.HTTPConnectionManager(ENVOY_HTTP_LISTENER, envoy.FileAccessLogEnvoy(DEFAULT_HTTP_ACCESS_LOG), 0)), - SocketOptions: envoy.TCPKeepaliveSocketOptions(), - }), - query: []string{ENVOY_HTTP_LISTENER}, - want: []proto.Message{ - &v2.Listener{ - Name: ENVOY_HTTP_LISTENER, - Address: envoy.SocketAddress("0.0.0.0", 8080), - FilterChains: envoy.FilterChains(envoy.HTTPConnectionManager(ENVOY_HTTP_LISTENER, envoy.FileAccessLogEnvoy(DEFAULT_HTTP_ACCESS_LOG), 0)), - SocketOptions: envoy.TCPKeepaliveSocketOptions(), - }, - }, - }, - "partial match": { - contents: listenermap(&v2.Listener{ - Name: ENVOY_HTTP_LISTENER, - Address: envoy.SocketAddress("0.0.0.0", 8080), - FilterChains: envoy.FilterChains(envoy.HTTPConnectionManager(ENVOY_HTTP_LISTENER, envoy.FileAccessLogEnvoy(DEFAULT_HTTP_ACCESS_LOG), 0)), - SocketOptions: envoy.TCPKeepaliveSocketOptions(), - }), - query: []string{ENVOY_HTTP_LISTENER, "stats-listener"}, - want: []proto.Message{ - &v2.Listener{ - Name: ENVOY_HTTP_LISTENER, - Address: envoy.SocketAddress("0.0.0.0", 8080), - FilterChains: envoy.FilterChains(envoy.HTTPConnectionManager(ENVOY_HTTP_LISTENER, envoy.FileAccessLogEnvoy(DEFAULT_HTTP_ACCESS_LOG), 0)), - SocketOptions: envoy.TCPKeepaliveSocketOptions(), - }, - }, - }, - "no match": { - contents: listenermap(&v2.Listener{ - Name: ENVOY_HTTP_LISTENER, - Address: envoy.SocketAddress("0.0.0.0", 8080), - FilterChains: envoy.FilterChains(envoy.HTTPConnectionManager(ENVOY_HTTP_LISTENER, envoy.FileAccessLogEnvoy(DEFAULT_HTTP_ACCESS_LOG), 0)), - SocketOptions: envoy.TCPKeepaliveSocketOptions(), - }), - query: []string{"stats-listener"}, - want: nil, - }, - } - - for name, tc := range tests { - t.Run(name, func(t *testing.T) { - var lc ListenerCache - lc.Update(tc.contents) - got := lc.Query(tc.query) - assert.Equal(t, tc.want, got) - }) - } -} - func TestListenerVisit(t *testing.T) { httpsFilterFor := func(vhost string) *envoy_api_v2_listener.Filter { return envoy.HTTPConnectionManagerBuilder(). diff --git a/internal/contour/route.go b/internal/contour/route.go index ad5bd5d48f5..306f84f7fed 100644 --- a/internal/contour/route.go +++ b/internal/contour/route.go @@ -18,10 +18,10 @@ import ( "sort" "sync" + "github.com/envoyproxy/go-control-plane/pkg/cache/types" + v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2" envoy_api_v2_route "github.com/envoyproxy/go-control-plane/envoy/api/v2/route" - resource "github.com/envoyproxy/go-control-plane/pkg/resource/v2" - "github.com/golang/protobuf/proto" "github.com/projectcontour/contour/internal/dag" "github.com/projectcontour/contour/internal/envoy" "github.com/projectcontour/contour/internal/protobuf" @@ -32,7 +32,6 @@ import ( type RouteCache struct { mu sync.Mutex values map[string]*v2.RouteConfiguration - Cond } // Update replaces the contents of the cache with the supplied map. @@ -41,11 +40,10 @@ func (c *RouteCache) Update(v map[string]*v2.RouteConfiguration) { defer c.mu.Unlock() c.values = v - c.Cond.Notify() } // Contents returns a copy of the cache's contents. -func (c *RouteCache) Contents() []proto.Message { +func (c *RouteCache) Contents() []types.Resource { c.mu.Lock() defer c.mu.Unlock() @@ -58,35 +56,6 @@ func (c *RouteCache) Contents() []proto.Message { return protobuf.AsMessages(values) } -// Query searches the RouteCache for the named RouteConfiguration entries. -func (c *RouteCache) Query(names []string) []proto.Message { - c.mu.Lock() - defer c.mu.Unlock() - - var values []*v2.RouteConfiguration - for _, n := range names { - v, ok := c.values[n] - if !ok { - // if there is no route registered with the cache - // we return a blank route configuration. This is - // not the same as returning nil, we're choosing to - // say "the configuration you asked for _does exists_, - // but it contains no useful information. - v = &v2.RouteConfiguration{ - Name: n, - } - } - values = append(values, v) - } - - //sort.RouteConfigurations(values) - sort.Stable(sorter.For(values)) - return protobuf.AsMessages(values) -} - -// TypeURL returns the string type of RouteCache Resource. -func (*RouteCache) TypeURL() string { return resource.RouteType } - type routeVisitor struct { routes map[string]*v2.RouteConfiguration } diff --git a/internal/contour/route_test.go b/internal/contour/route_test.go index 87066c6b581..ddafba664cd 100644 --- a/internal/contour/route_test.go +++ b/internal/contour/route_test.go @@ -20,7 +20,7 @@ import ( v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2" envoy_api_v2_core "github.com/envoyproxy/go-control-plane/envoy/api/v2/core" envoy_api_v2_route "github.com/envoyproxy/go-control-plane/envoy/api/v2/route" - "github.com/golang/protobuf/proto" + cachetypes "github.com/envoyproxy/go-control-plane/pkg/cache/types" "github.com/golang/protobuf/ptypes/wrappers" projcontour "github.com/projectcontour/contour/apis/projectcontour/v1" "github.com/projectcontour/contour/internal/assert" @@ -37,7 +37,7 @@ import ( func TestRouteCacheContents(t *testing.T) { tests := map[string]struct { contents map[string]*v2.RouteConfiguration - want []proto.Message + want []cachetypes.Resource }{ "empty": { contents: nil, @@ -52,7 +52,7 @@ func TestRouteCacheContents(t *testing.T) { Name: "ingress_https", }, }, - want: []proto.Message{ + want: []cachetypes.Resource{ &v2.RouteConfiguration{ Name: "ingress_http", }, @@ -73,66 +73,6 @@ func TestRouteCacheContents(t *testing.T) { } } -func TestRouteCacheQuery(t *testing.T) { - tests := map[string]struct { - contents map[string]*v2.RouteConfiguration - query []string - want []proto.Message - }{ - "exact match": { - contents: map[string]*v2.RouteConfiguration{ - "ingress_http": { - Name: "ingress_http", - }, - }, - query: []string{"ingress_http"}, - want: []proto.Message{ - &v2.RouteConfiguration{ - Name: "ingress_http", - }, - }, - }, - "partial match": { - contents: map[string]*v2.RouteConfiguration{ - "ingress_http": { - Name: "ingress_http", - }, - }, - query: []string{"stats-handler", "ingress_http"}, - want: []proto.Message{ - &v2.RouteConfiguration{ - Name: "ingress_http", - }, - &v2.RouteConfiguration{ - Name: "stats-handler", - }, - }, - }, - "no match": { - contents: map[string]*v2.RouteConfiguration{ - "ingress_http": { - Name: "ingress_http", - }, - }, - query: []string{"stats-handler"}, - want: []proto.Message{ - &v2.RouteConfiguration{ - Name: "stats-handler", - }, - }, - }, - } - - for name, tc := range tests { - t.Run(name, func(t *testing.T) { - var rc RouteCache - rc.Update(tc.contents) - got := rc.Query(tc.query) - assert.Equal(t, tc.want, got) - }) - } -} - func TestRouteVisit(t *testing.T) { tests := map[string]struct { objs []interface{} diff --git a/internal/contour/secret.go b/internal/contour/secret.go index b3ef96f92d8..99f7f1ac942 100644 --- a/internal/contour/secret.go +++ b/internal/contour/secret.go @@ -17,9 +17,9 @@ import ( "sort" "sync" + "github.com/envoyproxy/go-control-plane/pkg/cache/types" + envoy_api_v2_auth "github.com/envoyproxy/go-control-plane/envoy/api/v2/auth" - resource "github.com/envoyproxy/go-control-plane/pkg/resource/v2" - "github.com/golang/protobuf/proto" "github.com/projectcontour/contour/internal/dag" "github.com/projectcontour/contour/internal/envoy" "github.com/projectcontour/contour/internal/protobuf" @@ -30,20 +30,17 @@ import ( type SecretCache struct { mu sync.Mutex values map[string]*envoy_api_v2_auth.Secret - Cond } // Update replaces the contents of the cache with the supplied map. func (c *SecretCache) Update(v map[string]*envoy_api_v2_auth.Secret) { c.mu.Lock() defer c.mu.Unlock() - c.values = v - c.Cond.Notify() } // Contents returns a copy of the cache's contents. -func (c *SecretCache) Contents() []proto.Message { +func (c *SecretCache) Contents() []types.Resource { c.mu.Lock() defer c.mu.Unlock() var values []*envoy_api_v2_auth.Secret @@ -54,24 +51,6 @@ func (c *SecretCache) Contents() []proto.Message { return protobuf.AsMessages(values) } -func (c *SecretCache) Query(names []string) []proto.Message { - c.mu.Lock() - defer c.mu.Unlock() - var values []*envoy_api_v2_auth.Secret - for _, n := range names { - // we can only return secrets where their value is - // known. if the secret is not registered in the cache - // we return nothing. - if v, ok := c.values[n]; ok { - values = append(values, v) - } - } - sort.Stable(sorter.For(values)) - return protobuf.AsMessages(values) -} - -func (*SecretCache) TypeURL() string { return resource.SecretType } - type secretVisitor struct { secrets map[string]*envoy_api_v2_auth.Secret } diff --git a/internal/contour/secret_test.go b/internal/contour/secret_test.go index 563fda92fe5..65be7d3a6aa 100644 --- a/internal/contour/secret_test.go +++ b/internal/contour/secret_test.go @@ -18,7 +18,7 @@ import ( envoy_api_v2_auth "github.com/envoyproxy/go-control-plane/envoy/api/v2/auth" envoy_api_v2_core "github.com/envoyproxy/go-control-plane/envoy/api/v2/core" - "github.com/golang/protobuf/proto" + cachetypes "github.com/envoyproxy/go-control-plane/pkg/cache/types" projcontour "github.com/projectcontour/contour/apis/projectcontour/v1" "github.com/projectcontour/contour/internal/assert" "github.com/projectcontour/contour/internal/dag" @@ -32,7 +32,7 @@ import ( func TestSecretCacheContents(t *testing.T) { tests := map[string]struct { contents map[string]*envoy_api_v2_auth.Secret - want []proto.Message + want []cachetypes.Resource }{ "empty": { contents: nil, @@ -42,7 +42,7 @@ func TestSecretCacheContents(t *testing.T) { contents: secretmap( secret("default/secret/68621186db", secretdata(CERTIFICATE, RSA_PRIVATE_KEY)), ), - want: []proto.Message{ + want: []cachetypes.Resource{ secret("default/secret/68621186db", secretdata(CERTIFICATE, RSA_PRIVATE_KEY)), }, }, @@ -58,50 +58,6 @@ func TestSecretCacheContents(t *testing.T) { } } -func TestSecretCacheQuery(t *testing.T) { - tests := map[string]struct { - contents map[string]*envoy_api_v2_auth.Secret - query []string - want []proto.Message - }{ - "exact match": { - contents: secretmap( - secret("default/secret/68621186db", secretdata(CERTIFICATE, RSA_PRIVATE_KEY)), - ), - query: []string{"default/secret/68621186db"}, - want: []proto.Message{ - secret("default/secret/68621186db", secretdata(CERTIFICATE, RSA_PRIVATE_KEY)), - }, - }, - "partial match": { - contents: secretmap( - secret("default/secret-a/68621186db", secretdata(CERTIFICATE, RSA_PRIVATE_KEY)), - secret("default/secret-b/5397c67313", secretdata(CERTIFICATE_2, RSA_PRIVATE_KEY_2)), - ), - query: []string{"default/secret/68621186db", "default/secret-b/5397c67313"}, - want: []proto.Message{ - secret("default/secret-b/5397c67313", secretdata(CERTIFICATE_2, RSA_PRIVATE_KEY_2)), - }, - }, - "no match": { - contents: secretmap( - secret("default/secret/68621186db", secretdata(CERTIFICATE, RSA_PRIVATE_KEY)), - ), - query: []string{"default/secret-b/5397c67313"}, - want: nil, - }, - } - - for name, tc := range tests { - t.Run(name, func(t *testing.T) { - var sc SecretCache - sc.Update(tc.contents) - got := sc.Query(tc.query) - assert.Equal(t, tc.want, got) - }) - } -} - func TestSecretVisit(t *testing.T) { tests := map[string]struct { objs []interface{} diff --git a/internal/contour/snapshot.go b/internal/contour/snapshot.go new file mode 100644 index 00000000000..3a0ab069afb --- /dev/null +++ b/internal/contour/snapshot.go @@ -0,0 +1,90 @@ +// Copyright Project Contour Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package contour + +import ( + "fmt" + + envoy_api_v2_core "github.com/envoyproxy/go-control-plane/envoy/api/v2/core" + "github.com/envoyproxy/go-control-plane/pkg/cache/types" + "github.com/envoyproxy/go-control-plane/pkg/cache/v2" + "github.com/sirupsen/logrus" +) + +// SnapshotHandler implements the xDS snapshot cache +type SnapshotHandler struct { + CacheHandler *CacheHandler + EndpointsTranslator *EndpointsTranslator + + // SnapshotVersion holds the current version of the snapshot + snapshotVersion int + + // snapshotCache is a snapshot-based cache that maintains a single versioned + // snapshot of responses for xDS resources that Contour manages + SnapshotCache cache.SnapshotCache + + logrus.FieldLogger +} + +// ConstantHash is a specialized node ID hasher used to allow +// any instance of Envoy to connect to Contour regardless of the +// service-node flag configured on Envoy. +type ConstantHash string + +func (c ConstantHash) ID(*envoy_api_v2_core.Node) string { + return string(c) +} + +func (c ConstantHash) String() string { + return string(c) +} + +var _ cache.NodeHash = ConstantHash("") +var DefaultHash = ConstantHash("contour") + +// UpdateSnapshot is called when any cache changes and +// Envoy should be updated with a new configuration. +// +// It does not take into account a specific cache changing. +// When called, all xDS resources are updated with a new version. +// Envoy sees this as a noop, but could be improved in future refactorings. +func (s *SnapshotHandler) UpdateSnapshot() { + + // Generate new snapshot version. + snapshotVersion := s.getNewSnapshotVersion() + + // Create an snapshot with all xDS resources. + snapshot := cache.NewSnapshot(snapshotVersion, + s.EndpointsTranslator.Contents(), + s.CacheHandler.ClusterCache.Contents(), + s.CacheHandler.RouteCache.Contents(), + s.CacheHandler.ListenerCache.Contents(), + nil) + + // Update the Secrets xDS resource manually until a new version of go-control-plane is released. + // ref: https://github.com/envoyproxy/go-control-plane/pull/314 + snapshot.Resources[types.Secret] = cache.NewResources( + snapshotVersion, + s.CacheHandler.SecretCache.Contents()) + + if err := s.SnapshotCache.SetSnapshot(DefaultHash.String(), snapshot); err != nil { + s.Errorf("UpdateSnapshot: Error setting snapshot: %q", err) + } +} + +func (s *SnapshotHandler) getNewSnapshotVersion() string { + // Increment the snapshot version & return as string. + s.snapshotVersion++ + return fmt.Sprintf("%d", s.snapshotVersion) +} diff --git a/internal/e2e/cds_test.go b/internal/e2e/cds_test.go index a108d9478df..94cec29ecf6 100644 --- a/internal/e2e/cds_test.go +++ b/internal/e2e/cds_test.go @@ -18,11 +18,14 @@ import ( "testing" "time" + "github.com/golang/protobuf/ptypes/any" + + tassert "github.com/stretchr/testify/assert" + v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2" envoy_cluster "github.com/envoyproxy/go-control-plane/envoy/api/v2/cluster" envoy_api_v2_core "github.com/envoyproxy/go-control-plane/envoy/api/v2/core" projcontour "github.com/projectcontour/contour/apis/projectcontour/v1" - "github.com/projectcontour/contour/internal/assert" "github.com/projectcontour/contour/internal/envoy" "github.com/projectcontour/contour/internal/featuretests" "github.com/projectcontour/contour/internal/protobuf" @@ -64,18 +67,13 @@ func TestClusterLongServiceName(t *testing.T) { )) // check that it's been translated correctly. - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "2", - Resources: resources(t, - cluster("default/kbujbkuh-c83ceb/8080/da39a3ee5e", "default/kbujbkuhdod66gjdmwmijz8xzgsx1nkfbrloezdjiulquzk4x3p0nnvpzi8r", "default_kbujbkuhdod66gjdmwmijz8xzgsx1nkfbrloezdjiulquzk4x3p0nnvpzi8r_8080"), - ), - TypeUrl: clusterType, - Nonce: "2", - }, streamCDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + cluster("default/kbujbkuh-c83ceb/8080/da39a3ee5e", "default/kbujbkuhdod66gjdmwmijz8xzgsx1nkfbrloezdjiulquzk4x3p0nnvpzi8r", "default_kbujbkuhdod66gjdmwmijz8xzgsx1nkfbrloezdjiulquzk4x3p0nnvpzi8r_8080"), + ), streamCDS(t, cc)) } // Test adding, updating, and removing a service -// doesn't leave turds in the CDS cache. +// doesn't leave objects in the CDS cache. func TestClusterAddUpdateDelete(t *testing.T) { rh, cc, done := setup(t) defer done() @@ -126,14 +124,9 @@ func TestClusterAddUpdateDelete(t *testing.T) { }) rh.OnAdd(s1) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "3", - Resources: resources(t, - cluster("default/kuard/80/da39a3ee5e", "default/kuard", "default_kuard_80"), - ), - TypeUrl: clusterType, - Nonce: "3", - }, streamCDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + cluster("default/kuard/80/da39a3ee5e", "default/kuard", "default_kuard_80"), + ), streamCDS(t, cc)) // s2 is the same as s2, but the service port has a name s2 := service("default", "kuard", v1.ServicePort{ @@ -147,14 +140,9 @@ func TestClusterAddUpdateDelete(t *testing.T) { rh.OnUpdate(s1, s2) // check that we get two CDS records because the port is now named. - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "4", - Resources: resources(t, - cluster("default/kuard/80/da39a3ee5e", "default/kuard/http", "default_kuard_80"), - ), - TypeUrl: clusterType, - Nonce: "4", - }, streamCDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + cluster("default/kuard/80/da39a3ee5e", "default/kuard/http", "default_kuard_80"), + ), streamCDS(t, cc)) // s3 is like s2, but has a second named port. The k8s spec // requires all ports to be named if there is more than one of them. @@ -178,15 +166,10 @@ func TestClusterAddUpdateDelete(t *testing.T) { // check that we get four CDS records. Order is important // because the CDS cache is sorted. - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "5", - Resources: resources(t, - cluster("default/kuard/443/da39a3ee5e", "default/kuard/https", "default_kuard_443"), - cluster("default/kuard/80/da39a3ee5e", "default/kuard/http", "default_kuard_80"), - ), - TypeUrl: clusterType, - Nonce: "5", - }, streamCDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + cluster("default/kuard/80/da39a3ee5e", "default/kuard/http", "default_kuard_80"), + cluster("default/kuard/443/da39a3ee5e", "default/kuard/https", "default_kuard_443"), + ), streamCDS(t, cc)) // s4 is s3 with the http port removed. s4 := service("default", "kuard", @@ -203,14 +186,9 @@ func TestClusterAddUpdateDelete(t *testing.T) { // check that we get two CDS records only, and that the 80 and http // records have been removed even though the service object remains. - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "6", - Resources: resources(t, - cluster("default/kuard/443/da39a3ee5e", "default/kuard/https", "default_kuard_443"), - ), - TypeUrl: clusterType, - Nonce: "6", - }, streamCDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + cluster("default/kuard/443/da39a3ee5e", "default/kuard/https", "default_kuard_443"), + ), streamCDS(t, cc)) } // pathological hard case, one service is removed, the other is moved to a different port, and its name removed. @@ -263,15 +241,10 @@ func TestClusterRenameUpdateDelete(t *testing.T) { ) rh.OnAdd(s1) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "2", - Resources: resources(t, - cluster("default/kuard/443/da39a3ee5e", "default/kuard/https", "default_kuard_443"), - cluster("default/kuard/80/da39a3ee5e", "default/kuard/http", "default_kuard_80"), - ), - TypeUrl: clusterType, - Nonce: "2", - }, streamCDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + cluster("default/kuard/443/da39a3ee5e", "default/kuard/https", "default_kuard_443"), + cluster("default/kuard/80/da39a3ee5e", "default/kuard/http", "default_kuard_80")), + streamCDS(t, cc)) // s2 removes the name on port 80, moves it to port 443 and deletes the https port s2 := service("default", "kuard", @@ -283,35 +256,20 @@ func TestClusterRenameUpdateDelete(t *testing.T) { ) rh.OnUpdate(s1, s2) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "3", - Resources: resources(t, - cluster("default/kuard/443/da39a3ee5e", "default/kuard", "default_kuard_443"), - ), - TypeUrl: clusterType, - Nonce: "3", - }, streamCDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + cluster("default/kuard/443/da39a3ee5e", "default/kuard", "default_kuard_443"), + ), streamCDS(t, cc)) // now replace s2 with s1 to check it works in the other direction. rh.OnUpdate(s2, s1) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "4", - Resources: resources(t, - cluster("default/kuard/443/da39a3ee5e", "default/kuard/https", "default_kuard_443"), - cluster("default/kuard/80/da39a3ee5e", "default/kuard/http", "default_kuard_80"), - ), - TypeUrl: clusterType, - Nonce: "4", - }, streamCDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + cluster("default/kuard/443/da39a3ee5e", "default/kuard/https", "default_kuard_443"), + cluster("default/kuard/80/da39a3ee5e", "default/kuard/http", "default_kuard_80"), + ), streamCDS(t, cc)) // cleanup and check rh.OnDelete(s1) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "5", - Resources: resources(t), - TypeUrl: clusterType, - Nonce: "5", - }, streamCDS(t, cc)) + tassert.ElementsMatch(t, resources(t), streamCDS(t, cc)) } // issue#243. A single unnamed service with a different numeric target port @@ -342,14 +300,9 @@ func TestIssue243(t *testing.T) { }, ) rh.OnAdd(s1) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "2", - Resources: resources(t, - cluster("default/kuard/80/da39a3ee5e", "default/kuard", "default_kuard_80"), - ), - TypeUrl: clusterType, - Nonce: "2", - }, streamCDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + cluster("default/kuard/80/da39a3ee5e", "default/kuard", "default_kuard_80"), + ), streamCDS(t, cc)) }) } @@ -385,14 +338,9 @@ func TestIssue247(t *testing.T) { }, ) rh.OnAdd(s1) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "2", - Resources: resources(t, - cluster("default/kuard/80/da39a3ee5e", "default/kuard", "default_kuard_80"), - ), - TypeUrl: clusterType, - Nonce: "2", - }, streamCDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + cluster("default/kuard/80/da39a3ee5e", "default/kuard", "default_kuard_80"), + ), streamCDS(t, cc)) } func TestCDSResourceFiltering(t *testing.T) { rh, cc, done := setup(t) @@ -444,33 +392,19 @@ func TestCDSResourceFiltering(t *testing.T) { }, ) rh.OnAdd(s2) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "3", - Resources: resources(t, - // note, resources are sorted by Cluster.Name - cluster("default/httpbin/8080/da39a3ee5e", "default/httpbin", "default_httpbin_8080"), - cluster("default/kuard/80/da39a3ee5e", "default/kuard", "default_kuard_80"), - ), - TypeUrl: clusterType, - Nonce: "3", - }, streamCDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + // note, resources are sorted by Cluster.Name + cluster("default/httpbin/8080/da39a3ee5e", "default/httpbin", "default_httpbin_8080"), + cluster("default/kuard/80/da39a3ee5e", "default/kuard", "default_kuard_80"), + ), streamCDS(t, cc)) // assert we can filter on one resource - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "3", - Resources: resources(t, - cluster("default/kuard/80/da39a3ee5e", "default/kuard", "default_kuard_80"), - ), - TypeUrl: clusterType, - Nonce: "3", - }, streamCDS(t, cc, "default/kuard/80/da39a3ee5e")) + tassert.ElementsMatch(t, resources(t, + cluster("default/kuard/80/da39a3ee5e", "default/kuard", "default_kuard_80"), + ), streamCDS(t, cc, "default/kuard/80/da39a3ee5e")) // assert a non matching filter returns a response with no entries. - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "3", - TypeUrl: clusterType, - Nonce: "3", - }, streamCDS(t, cc, "default/httpbin/9000")) + tassert.ElementsMatch(t, []*v2.Cluster{}, streamCDS(t, cc, "default/httpbin/9000")) } func TestClusterCircuitbreakerAnnotations(t *testing.T) { @@ -509,30 +443,25 @@ func TestClusterCircuitbreakerAnnotations(t *testing.T) { rh.OnAdd(s1) // check that it's been translated correctly. - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "2", - Resources: resources(t, - featuretests.DefaultCluster(&v2.Cluster{ - Name: "default/kuard/8080/da39a3ee5e", - AltStatName: "default_kuard_8080", - ClusterDiscoveryType: envoy.ClusterDiscoveryType(v2.Cluster_EDS), - EdsClusterConfig: &v2.Cluster_EdsClusterConfig{ - EdsConfig: envoy.ConfigSource("contour"), - ServiceName: "default/kuard", - }, - CircuitBreakers: &envoy_cluster.CircuitBreakers{ - Thresholds: []*envoy_cluster.CircuitBreakers_Thresholds{{ - MaxConnections: protobuf.UInt32(9000), - MaxPendingRequests: protobuf.UInt32(4096), - MaxRequests: protobuf.UInt32(404), - MaxRetries: protobuf.UInt32(7), - }}, - }, - }), - ), - TypeUrl: clusterType, - Nonce: "2", - }, streamCDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + featuretests.DefaultCluster(&v2.Cluster{ + Name: "default/kuard/8080/da39a3ee5e", + AltStatName: "default_kuard_8080", + ClusterDiscoveryType: envoy.ClusterDiscoveryType(v2.Cluster_EDS), + EdsClusterConfig: &v2.Cluster_EdsClusterConfig{ + EdsConfig: envoy.ConfigSource("contour"), + ServiceName: "default/kuard", + }, + CircuitBreakers: &envoy_cluster.CircuitBreakers{ + Thresholds: []*envoy_cluster.CircuitBreakers_Thresholds{{ + MaxConnections: protobuf.UInt32(9000), + MaxPendingRequests: protobuf.UInt32(4096), + MaxRequests: protobuf.UInt32(404), + MaxRetries: protobuf.UInt32(7), + }}, + }, + }), + ), streamCDS(t, cc)) // update s1 with slightly weird values s2 := serviceWithAnnotations( @@ -552,27 +481,22 @@ func TestClusterCircuitbreakerAnnotations(t *testing.T) { rh.OnUpdate(s1, s2) // check that it's been translated correctly. - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "3", - Resources: resources(t, - featuretests.DefaultCluster(&v2.Cluster{ - Name: "default/kuard/8080/da39a3ee5e", - AltStatName: "default_kuard_8080", - ClusterDiscoveryType: envoy.ClusterDiscoveryType(v2.Cluster_EDS), - EdsClusterConfig: &v2.Cluster_EdsClusterConfig{ - EdsConfig: envoy.ConfigSource("contour"), - ServiceName: "default/kuard", - }, - CircuitBreakers: &envoy_cluster.CircuitBreakers{ - Thresholds: []*envoy_cluster.CircuitBreakers_Thresholds{{ - MaxPendingRequests: protobuf.UInt32(9999), - }}, - }, - }), - ), - TypeUrl: clusterType, - Nonce: "3", - }, streamCDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + featuretests.DefaultCluster(&v2.Cluster{ + Name: "default/kuard/8080/da39a3ee5e", + AltStatName: "default_kuard_8080", + ClusterDiscoveryType: envoy.ClusterDiscoveryType(v2.Cluster_EDS), + EdsClusterConfig: &v2.Cluster_EdsClusterConfig{ + EdsConfig: envoy.ConfigSource("contour"), + ServiceName: "default/kuard", + }, + CircuitBreakers: &envoy_cluster.CircuitBreakers{ + Thresholds: []*envoy_cluster.CircuitBreakers_Thresholds{{ + MaxPendingRequests: protobuf.UInt32(9999), + }}, + }, + }), + ), streamCDS(t, cc)) } // issue 581, different service parameters should generate @@ -624,14 +548,9 @@ func TestClusterPerServiceParameters(t *testing.T) { }, }) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "1", - Resources: resources(t, - cluster("default/kuard/80/da39a3ee5e", "default/kuard", "default_kuard_80"), - ), - TypeUrl: clusterType, - Nonce: "1", - }, streamCDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + cluster("default/kuard/80/da39a3ee5e", "default/kuard", "default_kuard_80"), + ), streamCDS(t, cc)) } // issue 581, different load balancer parameters should @@ -687,33 +606,28 @@ func TestClusterLoadBalancerStrategyPerRoute(t *testing.T) { }, }) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "1", - Resources: resources(t, - featuretests.DefaultCluster(&v2.Cluster{ - Name: "default/kuard/80/58d888c08a", - AltStatName: "default_kuard_80", - ClusterDiscoveryType: envoy.ClusterDiscoveryType(v2.Cluster_EDS), - EdsClusterConfig: &v2.Cluster_EdsClusterConfig{ - EdsConfig: envoy.ConfigSource("contour"), - ServiceName: "default/kuard", - }, - LbPolicy: v2.Cluster_RANDOM, - }), - featuretests.DefaultCluster(&v2.Cluster{ - Name: "default/kuard/80/8bf87fefba", - AltStatName: "default_kuard_80", - ClusterDiscoveryType: envoy.ClusterDiscoveryType(v2.Cluster_EDS), - EdsClusterConfig: &v2.Cluster_EdsClusterConfig{ - EdsConfig: envoy.ConfigSource("contour"), - ServiceName: "default/kuard", - }, - LbPolicy: v2.Cluster_LEAST_REQUEST, - }), - ), - TypeUrl: clusterType, - Nonce: "1", - }, streamCDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + featuretests.DefaultCluster(&v2.Cluster{ + Name: "default/kuard/80/58d888c08a", + AltStatName: "default_kuard_80", + ClusterDiscoveryType: envoy.ClusterDiscoveryType(v2.Cluster_EDS), + EdsClusterConfig: &v2.Cluster_EdsClusterConfig{ + EdsConfig: envoy.ConfigSource("contour"), + ServiceName: "default/kuard", + }, + LbPolicy: v2.Cluster_RANDOM, + }), + featuretests.DefaultCluster(&v2.Cluster{ + Name: "default/kuard/80/8bf87fefba", + AltStatName: "default_kuard_80", + ClusterDiscoveryType: envoy.ClusterDiscoveryType(v2.Cluster_EDS), + EdsClusterConfig: &v2.Cluster_EdsClusterConfig{ + EdsConfig: envoy.ConfigSource("contour"), + ServiceName: "default/kuard", + }, + LbPolicy: v2.Cluster_LEAST_REQUEST, + }), + ), streamCDS(t, cc)) } func TestClusterWithHealthChecks(t *testing.T) { @@ -757,14 +671,9 @@ func TestClusterWithHealthChecks(t *testing.T) { }, }) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "1", - Resources: resources(t, - clusterWithHealthCheck("default/kuard/80/bc862a33ca", "default/kuard", "default_kuard_80", "/healthz", true), - ), - TypeUrl: clusterType, - Nonce: "1", - }, streamCDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + clusterWithHealthCheck("default/kuard/80/bc862a33ca", "default/kuard", "default_kuard_80", "/healthz", true), + ), streamCDS(t, cc)) } // Test processing a service that exists but is not referenced @@ -815,14 +724,9 @@ func TestUnreferencedService(t *testing.T) { }, }) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "1", - Resources: resources(t, - cluster("default/kuard/80/da39a3ee5e", "default/kuard", "default_kuard_80"), - ), - TypeUrl: clusterType, - Nonce: "1", - }, streamCDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + cluster("default/kuard/80/da39a3ee5e", "default/kuard", "default_kuard_80"), + ), streamCDS(t, cc)) // This service which is added should not cause a DAG rebuild rh.OnAdd(&v1.Service{ @@ -839,14 +743,9 @@ func TestUnreferencedService(t *testing.T) { }, }) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "1", - Resources: resources(t, - cluster("default/kuard/80/da39a3ee5e", "default/kuard", "default_kuard_80"), - ), - TypeUrl: clusterType, - Nonce: "1", - }, streamCDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + cluster("default/kuard/80/da39a3ee5e", "default/kuard", "default_kuard_80"), + ), streamCDS(t, cc)) } func serviceWithAnnotations(ns, name string, annotations map[string]string, ports ...v1.ServicePort) *v1.Service { @@ -862,7 +761,7 @@ func serviceWithAnnotations(ns, name string, annotations map[string]string, port } } -func streamCDS(t *testing.T, cc *grpc.ClientConn, rn ...string) *v2.DiscoveryResponse { +func streamCDS(t *testing.T, cc *grpc.ClientConn, rn ...string) []*any.Any { t.Helper() rds := v2.NewClusterDiscoveryServiceClient(cc) st, err := rds.StreamClusters(context.TODO()) @@ -870,7 +769,8 @@ func streamCDS(t *testing.T, cc *grpc.ClientConn, rn ...string) *v2.DiscoveryRes return stream(t, st, &v2.DiscoveryRequest{ TypeUrl: clusterType, ResourceNames: rn, - }) + Node: &envoy_api_v2_core.Node{Id: "contour"}, + }).Resources } func cluster(name, servicename, statName string) *v2.Cluster { diff --git a/internal/e2e/e2e.go b/internal/e2e/e2e.go index 655f7ad7703..d192a5888f1 100644 --- a/internal/e2e/e2e.go +++ b/internal/e2e/e2e.go @@ -22,11 +22,12 @@ import ( "time" v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2" + envoy_api_v2_core "github.com/envoyproxy/go-control-plane/envoy/api/v2/core" discovery "github.com/envoyproxy/go-control-plane/envoy/service/discovery/v2" + "github.com/envoyproxy/go-control-plane/pkg/cache/types" + v2cache "github.com/envoyproxy/go-control-plane/pkg/cache/v2" resource "github.com/envoyproxy/go-control-plane/pkg/resource/v2" - "github.com/golang/protobuf/proto" "github.com/golang/protobuf/ptypes/any" - "github.com/projectcontour/contour/internal/assert" "github.com/projectcontour/contour/internal/contour" "github.com/projectcontour/contour/internal/dag" cgrpc "github.com/projectcontour/contour/internal/grpc" @@ -36,6 +37,7 @@ import ( "github.com/projectcontour/contour/internal/workgroup" "github.com/prometheus/client_golang/prometheus" "github.com/sirupsen/logrus" + tassert "github.com/stretchr/testify/assert" "google.golang.org/grpc" v1 "k8s.io/api/core/v1" "k8s.io/client-go/tools/cache" @@ -78,12 +80,22 @@ func setup(t *testing.T, opts ...func(*contour.EventHandler)) (cache.ResourceEve } r := prometheus.NewRegistry() + ch := &contour.CacheHandler{ Metrics: metrics.NewMetrics(r), ListenerCache: contour.NewListenerCache(statsAddress, statsPort), FieldLogger: log, } + // Setup snapshot cache + snapshotCache := v2cache.NewSnapshotCache(false, v2cache.IDHash{}, nil) + + snapshotHandler := &contour.SnapshotHandler{ + CacheHandler: ch, + EndpointsTranslator: et, + SnapshotCache: snapshotCache, + } + rand.Seed(time.Now().Unix()) eh := &contour.EventHandler{ @@ -100,6 +112,12 @@ func setup(t *testing.T, opts ...func(*contour.EventHandler)) (cache.ResourceEve HoldoffMaxDelay: time.Duration(rand.Intn(500)) * time.Millisecond, } + ch.SnapshotHandler = snapshotHandler + et.SnapshotHandler = snapshotHandler + + // Trigger an initial snapshot for static resources + snapshotHandler.UpdateSnapshot() + for _, opt := range opts { opt(eh) } @@ -108,14 +126,8 @@ func setup(t *testing.T, opts ...func(*contour.EventHandler)) (cache.ResourceEve check(t, err) discard := logrus.New() discard.Out = new(discardWriter) - // Resource types in xDS v2. - srv := cgrpc.NewAPI(discard, map[string]cgrpc.Resource{ - ch.ClusterCache.TypeURL(): &ch.ClusterCache, - ch.RouteCache.TypeURL(): &ch.RouteCache, - ch.ListenerCache.TypeURL(): &ch.ListenerCache, - ch.SecretCache.TypeURL(): &ch.SecretCache, - et.TypeURL(): et, - }, r) + + srv := cgrpc.NewAPI(r, snapshotCache) var g workgroup.Group @@ -204,7 +216,7 @@ func check(t *testing.T, err error) { } } -func resources(t *testing.T, protos ...proto.Message) []*any.Any { +func resources(t *testing.T, protos ...types.Resource) []*any.Any { t.Helper() var anys []*any.Any for _, a := range protos { @@ -248,6 +260,7 @@ func (c *Contour) Request(typeurl string, names ...string) *Response { resp := c.sendRequest(st, &v2.DiscoveryRequest{ TypeUrl: typeurl, ResourceNames: names, + Node: &envoy_api_v2_core.Node{Id: "contour"}, }) return &Response{ Contour: c, @@ -276,5 +289,6 @@ type Response struct { func (r *Response) Equals(want *v2.DiscoveryResponse) { r.Helper() - assert.Equal(r.T, want, r.DiscoveryResponse) + tassert.ElementsMatch(r.T, want.Resources, r.DiscoveryResponse.Resources) + //assert.Equal(r.T, want, r.DiscoveryResponse) } diff --git a/internal/e2e/eds_test.go b/internal/e2e/eds_test.go index f1a23acbf82..bcbe2777f54 100644 --- a/internal/e2e/eds_test.go +++ b/internal/e2e/eds_test.go @@ -17,8 +17,12 @@ import ( "context" "testing" + "github.com/golang/protobuf/ptypes/any" + tassert "github.com/stretchr/testify/assert" + + envoy_api_v2_core "github.com/envoyproxy/go-control-plane/envoy/api/v2/core" + v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2" - "github.com/projectcontour/contour/internal/assert" "github.com/projectcontour/contour/internal/envoy" "google.golang.org/grpc" v1 "k8s.io/api/core/v1" @@ -52,33 +56,23 @@ func TestAddRemoveEndpoints(t *testing.T) { rh.OnAdd(e1) // check that it's been translated correctly. - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "2", - Resources: resources(t, - envoy.ClusterLoadAssignment( - "super-long-namespace-name-oh-boy/what-a-descriptive-service-name-you-must-be-so-proud/http", - envoy.SocketAddress("172.16.0.1", 8000), // endpoints and cluster names should be sorted - envoy.SocketAddress("172.16.0.2", 8000), - ), - envoy.ClusterLoadAssignment( - "super-long-namespace-name-oh-boy/what-a-descriptive-service-name-you-must-be-so-proud/https", - envoy.SocketAddress("172.16.0.1", 8443), - envoy.SocketAddress("172.16.0.2", 8443), - ), + tassert.ElementsMatch(t, resources(t, + envoy.ClusterLoadAssignment( + "super-long-namespace-name-oh-boy/what-a-descriptive-service-name-you-must-be-so-proud/http", + envoy.SocketAddress("172.16.0.1", 8000), // endpoints and cluster names should be sorted + envoy.SocketAddress("172.16.0.2", 8000), ), - TypeUrl: endpointType, - Nonce: "2", - }, streamEDS(t, cc)) + envoy.ClusterLoadAssignment( + "super-long-namespace-name-oh-boy/what-a-descriptive-service-name-you-must-be-so-proud/https", + envoy.SocketAddress("172.16.0.1", 8443), + envoy.SocketAddress("172.16.0.2", 8443), + ), + ), streamEDS(t, cc)) // remove e1 and check that the EDS cache is now empty. rh.OnDelete(e1) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "4", - Resources: resources(t), - TypeUrl: endpointType, - Nonce: "4", - }, streamEDS(t, cc)) + tassert.ElementsMatch(t, resources(t), streamEDS(t, cc)) } func TestAddEndpointComplicated(t *testing.T) { @@ -112,22 +106,17 @@ func TestAddEndpointComplicated(t *testing.T) { rh.OnAdd(e1) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "2", - Resources: resources(t, - envoy.ClusterLoadAssignment( - "default/kuard/admin", - envoy.SocketAddress("10.48.1.77", 9000), - envoy.SocketAddress("10.48.1.78", 9000), - ), - envoy.ClusterLoadAssignment( - "default/kuard/foo", - envoy.SocketAddress("10.48.1.78", 8080), - ), + tassert.ElementsMatch(t, resources(t, + envoy.ClusterLoadAssignment( + "default/kuard/admin", + envoy.SocketAddress("10.48.1.77", 9000), + envoy.SocketAddress("10.48.1.78", 9000), ), - TypeUrl: endpointType, - Nonce: "2", - }, streamEDS(t, cc)) + envoy.ClusterLoadAssignment( + "default/kuard/foo", + envoy.SocketAddress("10.48.1.78", 8080), + ), + ), streamEDS(t, cc)) } func TestEndpointFilter(t *testing.T) { @@ -163,27 +152,16 @@ func TestEndpointFilter(t *testing.T) { rh.OnAdd(e1) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "2", - Resources: resources(t, - envoy.ClusterLoadAssignment( - "default/kuard/foo", - envoy.SocketAddress("10.48.1.78", 8080), - ), - ), - TypeUrl: endpointType, - Nonce: "2", - }, streamEDS(t, cc, "default/kuard/foo")) - - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "2", - TypeUrl: endpointType, - Resources: resources(t, - envoy.ClusterLoadAssignment("default/kuard/bar"), + tassert.ElementsMatch(t, resources(t, + envoy.ClusterLoadAssignment("default/kuard/foo", + envoy.SocketAddress("10.48.1.78", 8080), ), - Nonce: "2", - }, streamEDS(t, cc, "default/kuard/bar")) + ), streamEDS(t, cc, "default/kuard/foo")) + tassert.ElementsMatch(t, resources(t, envoy.ClusterLoadAssignment("default/kuard/admin", + envoy.SocketAddress("10.48.1.77", 9000), + envoy.SocketAddress("10.48.1.78", 9000))), + streamEDS(t, cc, "default/kuard/admin")) } // issue 602, test that an update from N endpoints @@ -201,28 +179,18 @@ func TestIssue602(t *testing.T) { rh.OnAdd(e1) // Assert endpoint was added - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "1", - Resources: resources(t, - envoy.ClusterLoadAssignment("default/simple", envoy.SocketAddress("192.168.183.24", 8080)), - ), - TypeUrl: endpointType, - Nonce: "1", - }, streamEDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + envoy.ClusterLoadAssignment("default/simple", envoy.SocketAddress("192.168.183.24", 8080)), + ), streamEDS(t, cc)) // e2 is the same as e1, but without endpoint subsets e2 := endpoints("default", "simple") rh.OnUpdate(e1, e2) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "2", - Resources: resources(t), - TypeUrl: endpointType, - Nonce: "2", - }, streamEDS(t, cc)) + tassert.ElementsMatch(t, resources(t), streamEDS(t, cc)) } -func streamEDS(t *testing.T, cc *grpc.ClientConn, rn ...string) *v2.DiscoveryResponse { +func streamEDS(t *testing.T, cc *grpc.ClientConn, rn ...string) []*any.Any { t.Helper() rds := v2.NewEndpointDiscoveryServiceClient(cc) st, err := rds.StreamEndpoints(context.TODO()) @@ -230,7 +198,8 @@ func streamEDS(t *testing.T, cc *grpc.ClientConn, rn ...string) *v2.DiscoveryRes return stream(t, st, &v2.DiscoveryRequest{ TypeUrl: endpointType, ResourceNames: rn, - }) + Node: &envoy_api_v2_core.Node{Id: "contour"}, + }).Resources } func endpoints(ns, name string, subsets ...v1.EndpointSubset) *v1.Endpoints { diff --git a/internal/e2e/lds_test.go b/internal/e2e/lds_test.go index f4ea1364c9b..868a7213fa3 100644 --- a/internal/e2e/lds_test.go +++ b/internal/e2e/lds_test.go @@ -18,11 +18,15 @@ import ( "path" "testing" + "github.com/golang/protobuf/ptypes/any" + tassert "github.com/stretchr/testify/assert" + + envoy_api_v2_core "github.com/envoyproxy/go-control-plane/envoy/api/v2/core" + v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2" envoy_api_v2_auth "github.com/envoyproxy/go-control-plane/envoy/api/v2/auth" envoy_api_v2_listener "github.com/envoyproxy/go-control-plane/envoy/api/v2/listener" projcontour "github.com/projectcontour/contour/apis/projectcontour/v1" - "github.com/projectcontour/contour/internal/assert" "github.com/projectcontour/contour/internal/contour" "github.com/projectcontour/contour/internal/dag" "github.com/projectcontour/contour/internal/envoy" @@ -49,14 +53,9 @@ func TestNonTLSListener(t *testing.T) { // assert that without any ingress objects registered // there are no active listeners - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "0", - Resources: resources(t, - staticListener(), - ), - TypeUrl: listenerType, - Nonce: "0", - }, streamLDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + staticListener(), + ), streamLDS(t, cc)) // i1 is a simple ingress, no hostname, no tls. i1 := &v1beta1.Ingress{ @@ -85,20 +84,16 @@ func TestNonTLSListener(t *testing.T) { // add it and assert that we now have a ingress_http listener rh.OnAdd(i1) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "1", - Resources: resources(t, - &v2.Listener{ - Name: "ingress_http", - Address: envoy.SocketAddress("0.0.0.0", 8080), - FilterChains: envoy.FilterChains(envoy.HTTPConnectionManager("ingress_http", envoy.FileAccessLogEnvoy("/dev/stdout"), 0)), - SocketOptions: envoy.TCPKeepaliveSocketOptions(), - }, - staticListener(), - ), - TypeUrl: listenerType, - Nonce: "1", - }, streamLDS(t, cc)) + + tassert.ElementsMatch(t, resources(t, + &v2.Listener{ + Name: "ingress_http", + Address: envoy.SocketAddress("0.0.0.0", 8080), + FilterChains: envoy.FilterChains(envoy.HTTPConnectionManager("ingress_http", envoy.FileAccessLogEnvoy("/dev/stdout"), 0)), + SocketOptions: envoy.TCPKeepaliveSocketOptions(), + }, + staticListener(), + ), streamLDS(t, cc)) // i2 is the same as i1 but has the kubernetes.io/ingress.allow-http: "false" annotation i2 := &v1beta1.Ingress{ @@ -116,14 +111,10 @@ func TestNonTLSListener(t *testing.T) { // update i1 to i2 and verify that ingress_http has gone. rh.OnUpdate(i1, i2) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "2", - Resources: resources(t, - staticListener(), - ), - TypeUrl: listenerType, - Nonce: "2", - }, streamLDS(t, cc)) + + tassert.ElementsMatch(t, resources(t, + staticListener(), + ), streamLDS(t, cc)) // i3 is similar to i2, but uses the ingress.kubernetes.io/force-ssl-redirect: "true" annotation // to force 80 -> 443 upgrade @@ -142,20 +133,16 @@ func TestNonTLSListener(t *testing.T) { // update i2 to i3 and check that ingress_http has returned rh.OnUpdate(i2, i3) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "3", - Resources: resources(t, - &v2.Listener{ - Name: "ingress_http", - Address: envoy.SocketAddress("0.0.0.0", 8080), - FilterChains: envoy.FilterChains(envoy.HTTPConnectionManager("ingress_http", envoy.FileAccessLogEnvoy("/dev/stdout"), 0)), - SocketOptions: envoy.TCPKeepaliveSocketOptions(), - }, - staticListener(), - ), - TypeUrl: listenerType, - Nonce: "3", - }, streamLDS(t, cc)) + + tassert.ElementsMatch(t, resources(t, + &v2.Listener{ + Name: "ingress_http", + Address: envoy.SocketAddress("0.0.0.0", 8080), + FilterChains: envoy.FilterChains(envoy.HTTPConnectionManager("ingress_http", envoy.FileAccessLogEnvoy("/dev/stdout"), 0)), + SocketOptions: envoy.TCPKeepaliveSocketOptions(), + }, + staticListener(), + ), streamLDS(t, cc)) } func TestTLSListener(t *testing.T) { @@ -214,42 +201,33 @@ func TestTLSListener(t *testing.T) { rh.OnAdd(s1) // assert that there is only a static listener - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "0", - Resources: resources(t, - staticListener(), - ), - TypeUrl: listenerType, - Nonce: "0", - }, streamLDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + staticListener(), + ), streamLDS(t, cc)) // add ingress and assert the existence of ingress_http and ingres_https rh.OnAdd(i1) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "1", - Resources: resources(t, - &v2.Listener{ - Name: "ingress_http", - Address: envoy.SocketAddress("0.0.0.0", 8080), - FilterChains: envoy.FilterChains(envoy.HTTPConnectionManager("ingress_http", envoy.FileAccessLogEnvoy("/dev/stdout"), 0)), - SocketOptions: envoy.TCPKeepaliveSocketOptions(), - }, - &v2.Listener{ - Name: "ingress_https", - Address: envoy.SocketAddress("0.0.0.0", 8443), - ListenerFilters: envoy.ListenerFilters( - envoy.TLSInspector(), - ), - FilterChains: filterchaintls("kuard.example.com", s1, - httpsFilterFor("kuard.example.com"), - "h2", "http/1.1"), - SocketOptions: envoy.TCPKeepaliveSocketOptions(), - }, - staticListener(), - ), - TypeUrl: listenerType, - Nonce: "1", - }, streamLDS(t, cc)) + + tassert.ElementsMatch(t, resources(t, + &v2.Listener{ + Name: "ingress_http", + Address: envoy.SocketAddress("0.0.0.0", 8080), + FilterChains: envoy.FilterChains(envoy.HTTPConnectionManager("ingress_http", envoy.FileAccessLogEnvoy("/dev/stdout"), 0)), + SocketOptions: envoy.TCPKeepaliveSocketOptions(), + }, + &v2.Listener{ + Name: "ingress_https", + Address: envoy.SocketAddress("0.0.0.0", 8443), + ListenerFilters: envoy.ListenerFilters( + envoy.TLSInspector(), + ), + FilterChains: filterchaintls("kuard.example.com", s1, + httpsFilterFor("kuard.example.com"), + "h2", "http/1.1"), + SocketOptions: envoy.TCPKeepaliveSocketOptions(), + }, + staticListener(), + ), streamLDS(t, cc)) // i2 is the same as i1 but has the kubernetes.io/ingress.allow-http: "false" annotation i2 := &v1beta1.Ingress{ @@ -280,36 +258,24 @@ func TestTLSListener(t *testing.T) { // update i1 to i2 and verify that ingress_http has gone. rh.OnUpdate(i1, i2) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "2", - Resources: resources(t, - &v2.Listener{ - Name: "ingress_https", - Address: envoy.SocketAddress("0.0.0.0", 8443), - ListenerFilters: envoy.ListenerFilters( - envoy.TLSInspector(), - ), - FilterChains: filterchaintls("kuard.example.com", s1, - httpsFilterFor("kuard.example.com"), - "h2", "http/1.1"), - SocketOptions: envoy.TCPKeepaliveSocketOptions(), - }, - staticListener(), - ), - TypeUrl: listenerType, - Nonce: "2", - }, streamLDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + &v2.Listener{ + Name: "ingress_https", + Address: envoy.SocketAddress("0.0.0.0", 8443), + ListenerFilters: envoy.ListenerFilters( + envoy.TLSInspector(), + ), + FilterChains: filterchaintls("kuard.example.com", s1, + httpsFilterFor("kuard.example.com"), + "h2", "http/1.1"), + SocketOptions: envoy.TCPKeepaliveSocketOptions(), + }, + staticListener(), + ), streamLDS(t, cc)) // delete secret and assert that ingress_https is removed rh.OnDelete(s1) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "3", - Resources: resources(t, - staticListener(), - ), - TypeUrl: listenerType, - Nonce: "3", - }, streamLDS(t, cc)) + tassert.ElementsMatch(t, resources(t, staticListener()), streamLDS(t, cc)) } func TestHTTPProxyTLSListener(t *testing.T) { @@ -396,14 +362,9 @@ func TestHTTPProxyTLSListener(t *testing.T) { rh.OnAdd(secret1) // assert that there is only a static listener - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "0", - Resources: resources(t, - staticListener(), - ), - TypeUrl: listenerType, - Nonce: "0", - }, streamLDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + staticListener(), + ), streamLDS(t, cc)) l1 := &v2.Listener{ Name: "ingress_https", @@ -423,35 +384,26 @@ func TestHTTPProxyTLSListener(t *testing.T) { // add ingress and assert the existence of ingress_http and ingres_https rh.OnAdd(p1) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "1", - Resources: resources(t, - &v2.Listener{ - Name: "ingress_http", - Address: envoy.SocketAddress("0.0.0.0", 8080), - FilterChains: envoy.FilterChains( - envoy.HTTPConnectionManager("ingress_http", envoy.FileAccessLogEnvoy("/dev/stdout"), 0), - ), - SocketOptions: envoy.TCPKeepaliveSocketOptions(), - }, - l1, - staticListener(), - ), - TypeUrl: listenerType, - Nonce: "1", - }, streamLDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + &v2.Listener{ + Name: "ingress_http", + Address: envoy.SocketAddress("0.0.0.0", 8080), + FilterChains: envoy.FilterChains( + envoy.HTTPConnectionManager("ingress_http", envoy.FileAccessLogEnvoy("/dev/stdout"), 0), + ), + SocketOptions: envoy.TCPKeepaliveSocketOptions(), + }, + l1, + staticListener(), + ), streamLDS(t, cc)) // delete secret and assert both listeners are removed because the // httpproxy is no longer valid. rh.OnDelete(secret1) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "2", - Resources: resources(t, - staticListener(), - ), - TypeUrl: listenerType, - Nonce: "2", - }, streamLDS(t, cc)) + + tassert.ElementsMatch(t, resources(t, + staticListener(), + ), streamLDS(t, cc)) rh.OnDelete(p1) // add secret @@ -478,23 +430,18 @@ func TestHTTPProxyTLSListener(t *testing.T) { // add ingress and assert the existence of ingress_http and ingres_https rh.OnAdd(p2) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "4", - Resources: resources(t, - &v2.Listener{ - Name: "ingress_http", - Address: envoy.SocketAddress("0.0.0.0", 8080), - FilterChains: envoy.FilterChains( - envoy.HTTPConnectionManager("ingress_http", envoy.FileAccessLogEnvoy("/dev/stdout"), 0), - ), - SocketOptions: envoy.TCPKeepaliveSocketOptions(), - }, - l2, - staticListener(), - ), - TypeUrl: listenerType, - Nonce: "4", - }, streamLDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + &v2.Listener{ + Name: "ingress_http", + Address: envoy.SocketAddress("0.0.0.0", 8080), + FilterChains: envoy.FilterChains( + envoy.HTTPConnectionManager("ingress_http", envoy.FileAccessLogEnvoy("/dev/stdout"), 0), + ), + SocketOptions: envoy.TCPKeepaliveSocketOptions(), + }, + l2, + staticListener(), + ), streamLDS(t, cc)) } func TestLDSFilter(t *testing.T) { @@ -554,48 +501,34 @@ func TestLDSFilter(t *testing.T) { // add ingress and fetch ingress_https rh.OnAdd(i1) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "1", - Resources: resources(t, - &v2.Listener{ - Name: "ingress_https", - Address: envoy.SocketAddress("0.0.0.0", 8443), - ListenerFilters: envoy.ListenerFilters( - envoy.TLSInspector(), - ), - FilterChains: filterchaintls("kuard.example.com", s1, - httpsFilterFor("kuard.example.com"), - "h2", "http/1.1"), - SocketOptions: envoy.TCPKeepaliveSocketOptions(), - }, - ), - TypeUrl: listenerType, - Nonce: "1", - }, streamLDS(t, cc, "ingress_https")) + tassert.ElementsMatch(t, resources(t, + &v2.Listener{ + Name: "ingress_https", + Address: envoy.SocketAddress("0.0.0.0", 8443), + ListenerFilters: envoy.ListenerFilters( + envoy.TLSInspector(), + ), + FilterChains: filterchaintls("kuard.example.com", s1, + httpsFilterFor("kuard.example.com"), + "h2", "http/1.1"), + SocketOptions: envoy.TCPKeepaliveSocketOptions(), + }, + ), streamLDS(t, cc, "ingress_https")) // fetch ingress_http - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "1", - Resources: resources(t, - &v2.Listener{ - Name: "ingress_http", - Address: envoy.SocketAddress("0.0.0.0", 8080), - FilterChains: envoy.FilterChains( - envoy.HTTPConnectionManager("ingress_http", envoy.FileAccessLogEnvoy("/dev/stdout"), 0), - ), - SocketOptions: envoy.TCPKeepaliveSocketOptions(), - }, - ), - TypeUrl: listenerType, - Nonce: "1", - }, streamLDS(t, cc, "ingress_http")) + tassert.ElementsMatch(t, resources(t, + &v2.Listener{ + Name: "ingress_http", + Address: envoy.SocketAddress("0.0.0.0", 8080), + FilterChains: envoy.FilterChains( + envoy.HTTPConnectionManager("ingress_http", envoy.FileAccessLogEnvoy("/dev/stdout"), 0), + ), + SocketOptions: envoy.TCPKeepaliveSocketOptions(), + }, + ), streamLDS(t, cc, "ingress_http")) // fetch something non existent. - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "1", - TypeUrl: listenerType, - Nonce: "1", - }, streamLDS(t, cc, "HTTP")) + tassert.ElementsMatch(t, []*any.Any{}, streamLDS(t, cc, "HTTP")) } func TestLDSStreamEmpty(t *testing.T) { @@ -603,11 +536,7 @@ func TestLDSStreamEmpty(t *testing.T) { defer done() // assert that streaming LDS with no ingresses does not stall. - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "0", - TypeUrl: listenerType, - Nonce: "0", - }, streamLDS(t, cc, "HTTP")) + tassert.ElementsMatch(t, []*any.Any{}, streamLDS(t, cc, "HTTP")) } func TestLDSIngressHTTPUseProxyProtocol(t *testing.T) { @@ -618,14 +547,9 @@ func TestLDSIngressHTTPUseProxyProtocol(t *testing.T) { // assert that without any ingress objects registered // there is only a static listener - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "0", - Resources: resources(t, - staticListener(), - ), - TypeUrl: listenerType, - Nonce: "0", - }, streamLDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + staticListener(), + ), streamLDS(t, cc)) // i1 is a simple ingress, no hostname, no tls. i1 := &v1beta1.Ingress{ @@ -654,23 +578,18 @@ func TestLDSIngressHTTPUseProxyProtocol(t *testing.T) { // add it and assert that we now have a ingress_http listener using // the proxy protocol (the true param to filterchain) rh.OnAdd(i1) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "1", - Resources: resources(t, - &v2.Listener{ - Name: "ingress_http", - Address: envoy.SocketAddress("0.0.0.0", 8080), - ListenerFilters: envoy.ListenerFilters( - envoy.ProxyProtocol(), - ), - FilterChains: envoy.FilterChains(envoy.HTTPConnectionManager("ingress_http", envoy.FileAccessLogEnvoy("/dev/stdout"), 0)), - SocketOptions: envoy.TCPKeepaliveSocketOptions(), - }, - staticListener(), - ), - TypeUrl: listenerType, - Nonce: "1", - }, streamLDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + &v2.Listener{ + Name: "ingress_http", + Address: envoy.SocketAddress("0.0.0.0", 8080), + ListenerFilters: envoy.ListenerFilters( + envoy.ProxyProtocol(), + ), + FilterChains: envoy.FilterChains(envoy.HTTPConnectionManager("ingress_http", envoy.FileAccessLogEnvoy("/dev/stdout"), 0)), + SocketOptions: envoy.TCPKeepaliveSocketOptions(), + }, + staticListener(), + ), streamLDS(t, cc)) } func TestLDSIngressHTTPSUseProxyProtocol(t *testing.T) { @@ -717,14 +636,7 @@ func TestLDSIngressHTTPSUseProxyProtocol(t *testing.T) { rh.OnAdd(s1) // assert that there is only a static listener - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "0", - Resources: resources(t, - staticListener(), - ), - TypeUrl: listenerType, - Nonce: "0", - }, streamLDS(t, cc)) + tassert.ElementsMatch(t, resources(t, staticListener()), streamLDS(t, cc)) rh.OnAdd(&v1.Service{ ObjectMeta: metav1.ObjectMeta{ @@ -756,24 +668,19 @@ func TestLDSIngressHTTPSUseProxyProtocol(t *testing.T) { "h2", "http/1.1"), SocketOptions: envoy.TCPKeepaliveSocketOptions(), } - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "1", - Resources: resources(t, - &v2.Listener{ - Name: "ingress_http", - Address: envoy.SocketAddress("0.0.0.0", 8080), - ListenerFilters: envoy.ListenerFilters( - envoy.ProxyProtocol(), - ), - FilterChains: envoy.FilterChains(envoy.HTTPConnectionManager("ingress_http", envoy.FileAccessLogEnvoy("/dev/stdout"), 0)), - SocketOptions: envoy.TCPKeepaliveSocketOptions(), - }, - ingress_https, - staticListener(), - ), - TypeUrl: listenerType, - Nonce: "1", - }, streamLDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + &v2.Listener{ + Name: "ingress_http", + Address: envoy.SocketAddress("0.0.0.0", 8080), + ListenerFilters: envoy.ListenerFilters( + envoy.ProxyProtocol(), + ), + FilterChains: envoy.FilterChains(envoy.HTTPConnectionManager("ingress_http", envoy.FileAccessLogEnvoy("/dev/stdout"), 0)), + SocketOptions: envoy.TCPKeepaliveSocketOptions(), + }, + ingress_https, + staticListener(), + ), streamLDS(t, cc)) } func TestLDSCustomAddressAndPort(t *testing.T) { @@ -823,14 +730,9 @@ func TestLDSCustomAddressAndPort(t *testing.T) { rh.OnAdd(s1) // assert that there is only a static listener - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "0", - Resources: resources(t, - staticListener(), - ), - TypeUrl: listenerType, - Nonce: "0", - }, streamLDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + staticListener(), + ), streamLDS(t, cc)) rh.OnAdd(&v1.Service{ ObjectMeta: metav1.ObjectMeta{ @@ -869,16 +771,11 @@ func TestLDSCustomAddressAndPort(t *testing.T) { "h2", "http/1.1"), SocketOptions: envoy.TCPKeepaliveSocketOptions(), } - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "1", - Resources: resources(t, - ingress_http, - ingress_https, - staticListener(), - ), - TypeUrl: listenerType, - Nonce: "1", - }, streamLDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + ingress_http, + ingress_https, + staticListener(), + ), streamLDS(t, cc)) } func TestLDSCustomAccessLogPaths(t *testing.T) { @@ -940,14 +837,9 @@ func TestLDSCustomAccessLogPaths(t *testing.T) { rh.OnAdd(s1) // assert that there is only a static listener - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "0", - Resources: resources(t, - staticListener(), - ), - TypeUrl: listenerType, - Nonce: "0", - }, streamLDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + staticListener(), + ), streamLDS(t, cc)) rh.OnAdd(i1) @@ -976,16 +868,11 @@ func TestLDSCustomAccessLogPaths(t *testing.T) { "h2", "http/1.1"), SocketOptions: envoy.TCPKeepaliveSocketOptions(), } - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "1", - Resources: resources(t, - ingress_http, - ingress_https, - staticListener(), - ), - TypeUrl: listenerType, - Nonce: "1", - }, streamLDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + ingress_http, + ingress_https, + staticListener(), + ), streamLDS(t, cc)) } func TestHTTPProxyHTTPS(t *testing.T) { @@ -993,14 +880,9 @@ func TestHTTPProxyHTTPS(t *testing.T) { defer done() // assert that there is only a static listener - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "0", - Resources: resources(t, - staticListener(), - ), - TypeUrl: listenerType, - Nonce: "0", - }, streamLDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + staticListener(), + ), streamLDS(t, cc)) // s1 is a tls secret s1 := &v1.Secret{ @@ -1080,16 +962,11 @@ func TestHTTPProxyHTTPS(t *testing.T) { "h2", "http/1.1"), SocketOptions: envoy.TCPKeepaliveSocketOptions(), } - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "1", - Resources: resources(t, - ingressHTTP, - ingressHTTPS, - staticListener(), - ), - TypeUrl: listenerType, - Nonce: "1", - }, streamLDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + ingressHTTP, + ingressHTTPS, + staticListener(), + ), streamLDS(t, cc)) } func TestHTTPProxyMinimumTLSVersion(t *testing.T) { @@ -1173,23 +1050,18 @@ func TestHTTPProxyMinimumTLSVersion(t *testing.T) { } // verify that p1's TLS 1.1 minimum has been upgraded to 1.2 - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "1", - Resources: resources(t, - &v2.Listener{ - Name: "ingress_http", - Address: envoy.SocketAddress("0.0.0.0", 8080), - FilterChains: envoy.FilterChains( - envoy.HTTPConnectionManager("ingress_http", envoy.FileAccessLogEnvoy("/dev/stdout"), 0), - ), - SocketOptions: envoy.TCPKeepaliveSocketOptions(), - }, - l1, - staticListener(), - ), - TypeUrl: listenerType, - Nonce: "1", - }, streamLDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + &v2.Listener{ + Name: "ingress_http", + Address: envoy.SocketAddress("0.0.0.0", 8080), + FilterChains: envoy.FilterChains( + envoy.HTTPConnectionManager("ingress_http", envoy.FileAccessLogEnvoy("/dev/stdout"), 0), + ), + SocketOptions: envoy.TCPKeepaliveSocketOptions(), + }, + l1, + staticListener(), + ), streamLDS(t, cc)) // p2 is a tls httpproxy p2 := &projcontour.HTTPProxy{ @@ -1240,23 +1112,18 @@ func TestHTTPProxyMinimumTLSVersion(t *testing.T) { } // verify that p2's TLS 1.3 minimum has NOT been downgraded to 1.2 - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "2", - Resources: resources(t, - &v2.Listener{ - Name: "ingress_http", - Address: envoy.SocketAddress("0.0.0.0", 8080), - FilterChains: envoy.FilterChains( - envoy.HTTPConnectionManager("ingress_http", envoy.FileAccessLogEnvoy("/dev/stdout"), 0), - ), - SocketOptions: envoy.TCPKeepaliveSocketOptions(), - }, - l2, - staticListener(), - ), - TypeUrl: listenerType, - Nonce: "2", - }, streamLDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + &v2.Listener{ + Name: "ingress_http", + Address: envoy.SocketAddress("0.0.0.0", 8080), + FilterChains: envoy.FilterChains( + envoy.HTTPConnectionManager("ingress_http", envoy.FileAccessLogEnvoy("/dev/stdout"), 0), + ), + SocketOptions: envoy.TCPKeepaliveSocketOptions(), + }, + l2, + staticListener(), + ), streamLDS(t, cc)) } func TestLDSHTTPProxyRootCannotDelegateToAnotherRoot(t *testing.T) { @@ -1322,25 +1189,20 @@ func TestLDSHTTPProxyRootCannotDelegateToAnotherRoot(t *testing.T) { // verify that port 80 is present because while it is not possible to // delegate to it, child can host a vhost which opens port 80. - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "2", - Resources: resources(t, - &v2.Listener{ - Name: "ingress_http", - Address: envoy.SocketAddress("0.0.0.0", 8080), - FilterChains: envoy.FilterChains( - envoy.HTTPConnectionManager("ingress_http", envoy.FileAccessLogEnvoy("/dev/stdout"), 0), - ), - SocketOptions: envoy.TCPKeepaliveSocketOptions(), - }, - staticListener(), - ), - TypeUrl: listenerType, - Nonce: "2", - }, streamLDS(t, cc)) + tassert.ElementsMatch(t, resources(t, + &v2.Listener{ + Name: "ingress_http", + Address: envoy.SocketAddress("0.0.0.0", 8080), + FilterChains: envoy.FilterChains( + envoy.HTTPConnectionManager("ingress_http", envoy.FileAccessLogEnvoy("/dev/stdout"), 0), + ), + SocketOptions: envoy.TCPKeepaliveSocketOptions(), + }, + staticListener(), + ), streamLDS(t, cc)) } -func streamLDS(t *testing.T, cc *grpc.ClientConn, rn ...string) *v2.DiscoveryResponse { +func streamLDS(t *testing.T, cc *grpc.ClientConn, rn ...string) []*any.Any { t.Helper() rds := v2.NewListenerDiscoveryServiceClient(cc) st, err := rds.StreamListeners(context.TODO()) @@ -1348,7 +1210,8 @@ func streamLDS(t *testing.T, cc *grpc.ClientConn, rn ...string) *v2.DiscoveryRes return stream(t, st, &v2.DiscoveryRequest{ TypeUrl: listenerType, ResourceNames: rn, - }) + Node: &envoy_api_v2_core.Node{Id: "contour"}, + }).Resources } func backend(name string, port intstr.IntOrString) *v1beta1.IngressBackend { diff --git a/internal/e2e/rds_test.go b/internal/e2e/rds_test.go index 98d846c0ceb..d2187ec1b91 100644 --- a/internal/e2e/rds_test.go +++ b/internal/e2e/rds_test.go @@ -21,11 +21,14 @@ import ( "sort" "testing" + tassert "github.com/stretchr/testify/assert" + + envoy_api_v2_core "github.com/envoyproxy/go-control-plane/envoy/api/v2/core" + v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2" envoy_api_v2_route "github.com/envoyproxy/go-control-plane/envoy/api/v2/route" "github.com/golang/protobuf/ptypes/any" projcontour "github.com/projectcontour/contour/apis/projectcontour/v1" - "github.com/projectcontour/contour/internal/assert" "github.com/projectcontour/contour/internal/contour" "github.com/projectcontour/contour/internal/dag" "github.com/projectcontour/contour/internal/envoy" @@ -106,19 +109,14 @@ func TestEditIngress(t *testing.T) { rh.OnAdd(old) // check that it's been translated correctly. - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "1", - Resources: routeResources(t, - envoy.RouteConfiguration("ingress_http", - envoy.VirtualHost("*", &envoy_api_v2_route.Route{ - Match: routePrefix("/"), - Action: routecluster("default/kuard/80/da39a3ee5e"), - }), - ), + tassert.ElementsMatch(t, routeResources(t, + envoy.RouteConfiguration("ingress_http", + envoy.VirtualHost("*", &envoy_api_v2_route.Route{ + Match: routePrefix("/"), + Action: routecluster("default/kuard/80/da39a3ee5e"), + }), ), - TypeUrl: routeType, - Nonce: "1", - }, streamRDS(t, cc)) + ), streamRDS(t, cc)) // update old to new rh.OnUpdate(old, &v1beta1.Ingress{ @@ -141,19 +139,14 @@ func TestEditIngress(t *testing.T) { }) // check that ingress_http has been updated. - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "2", - Resources: routeResources(t, - envoy.RouteConfiguration("ingress_http", - envoy.VirtualHost("*", &envoy_api_v2_route.Route{ - Match: routePrefix("/testing"), - Action: routecluster("default/kuard/80/da39a3ee5e"), - }), - ), + tassert.ElementsMatch(t, routeResources(t, + envoy.RouteConfiguration("ingress_http", + envoy.VirtualHost("*", &envoy_api_v2_route.Route{ + Match: routePrefix("/testing"), + Action: routecluster("default/kuard/80/da39a3ee5e"), + }), ), - TypeUrl: routeType, - Nonce: "2", - }, streamRDS(t, cc)) + ), streamRDS(t, cc)) } // heptio/contour#101 @@ -212,21 +205,16 @@ func TestIngressPathRouteWithoutHost(t *testing.T) { rh.OnAdd(s1) // check that it's been translated correctly. - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "2", - Resources: routeResources(t, - envoy.RouteConfiguration("ingress_http", - envoy.VirtualHost("*", - &envoy_api_v2_route.Route{ - Match: routePrefix("/hello"), - Action: routecluster("default/hello/80/da39a3ee5e"), - }, - ), + tassert.ElementsMatch(t, routeResources(t, + envoy.RouteConfiguration("ingress_http", + envoy.VirtualHost("*", + &envoy_api_v2_route.Route{ + Match: routePrefix("/hello"), + Action: routecluster("default/hello/80/da39a3ee5e"), + }, ), ), - TypeUrl: routeType, - Nonce: "2", - }, streamRDS(t, cc)) + ), streamRDS(t, cc)) } func TestEditIngressInPlace(t *testing.T) { @@ -286,21 +274,16 @@ func TestEditIngressInPlace(t *testing.T) { } rh.OnAdd(s2) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "2", - Resources: routeResources(t, - envoy.RouteConfiguration("ingress_http", - envoy.VirtualHost("hello.example.com", - &envoy_api_v2_route.Route{ - Match: routePrefix("/"), - Action: routecluster("default/wowie/80/da39a3ee5e"), - }, - ), + tassert.ElementsMatch(t, routeResources(t, + envoy.RouteConfiguration("ingress_http", + envoy.VirtualHost("hello.example.com", + &envoy_api_v2_route.Route{ + Match: routePrefix("/"), + Action: routecluster("default/wowie/80/da39a3ee5e"), + }, ), ), - TypeUrl: routeType, - Nonce: "2", - }, streamRDS(t, cc)) + ), streamRDS(t, cc)) // i2 is like i1 but adds a second route i2 := &v1beta1.Ingress{ @@ -329,25 +312,20 @@ func TestEditIngressInPlace(t *testing.T) { }, } rh.OnUpdate(i1, i2) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "3", - Resources: routeResources(t, - envoy.RouteConfiguration("ingress_http", - envoy.VirtualHost("hello.example.com", - &envoy_api_v2_route.Route{ - Match: routePrefix("/whoop"), - Action: routecluster("default/kerpow/9000/da39a3ee5e"), - }, - &envoy_api_v2_route.Route{ - Match: routePrefix("/"), - Action: routecluster("default/wowie/80/da39a3ee5e"), - }, - ), + tassert.ElementsMatch(t, routeResources(t, + envoy.RouteConfiguration("ingress_http", + envoy.VirtualHost("hello.example.com", + &envoy_api_v2_route.Route{ + Match: routePrefix("/whoop"), + Action: routecluster("default/kerpow/9000/da39a3ee5e"), + }, + &envoy_api_v2_route.Route{ + Match: routePrefix("/"), + Action: routecluster("default/wowie/80/da39a3ee5e"), + }, ), ), - TypeUrl: routeType, - Nonce: "3", - }, streamRDS(t, cc)) + ), streamRDS(t, cc)) // i3 is like i2, but adds the ingress.kubernetes.io/force-ssl-redirect: "true" annotation i3 := &v1beta1.Ingress{ @@ -381,25 +359,20 @@ func TestEditIngressInPlace(t *testing.T) { }, } rh.OnUpdate(i2, i3) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "4", - Resources: routeResources(t, - envoy.RouteConfiguration("ingress_http", - envoy.VirtualHost("hello.example.com", - &envoy_api_v2_route.Route{ - Match: routePrefix("/whoop"), - Action: envoy.UpgradeHTTPS(), - }, - &envoy_api_v2_route.Route{ - Match: routePrefix("/"), - Action: envoy.UpgradeHTTPS(), - }, - ), + tassert.ElementsMatch(t, routeResources(t, + envoy.RouteConfiguration("ingress_http", + envoy.VirtualHost("hello.example.com", + &envoy_api_v2_route.Route{ + Match: routePrefix("/whoop"), + Action: envoy.UpgradeHTTPS(), + }, + &envoy_api_v2_route.Route{ + Match: routePrefix("/"), + Action: envoy.UpgradeHTTPS(), + }, ), ), - TypeUrl: routeType, - Nonce: "4", - }, streamRDS(t, cc)) + ), streamRDS(t, cc)) rh.OnAdd(&v1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -447,37 +420,32 @@ func TestEditIngressInPlace(t *testing.T) { }, } rh.OnUpdate(i3, i4) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "5", - Resources: routeResources(t, - envoy.RouteConfiguration("ingress_http", - envoy.VirtualHost("hello.example.com", - &envoy_api_v2_route.Route{ - Match: routePrefix("/whoop"), - Action: envoy.UpgradeHTTPS(), - }, - &envoy_api_v2_route.Route{ - Match: routePrefix("/"), - Action: envoy.UpgradeHTTPS(), - }, - ), + tassert.ElementsMatch(t, routeResources(t, + envoy.RouteConfiguration("ingress_http", + envoy.VirtualHost("hello.example.com", + &envoy_api_v2_route.Route{ + Match: routePrefix("/whoop"), + Action: envoy.UpgradeHTTPS(), + }, + &envoy_api_v2_route.Route{ + Match: routePrefix("/"), + Action: envoy.UpgradeHTTPS(), + }, ), - envoy.RouteConfiguration("https/hello.example.com", - envoy.VirtualHost("hello.example.com", - &envoy_api_v2_route.Route{ - Match: routePrefix("/whoop"), - Action: routecluster("default/kerpow/9000/da39a3ee5e"), - }, - &envoy_api_v2_route.Route{ - Match: routePrefix("/"), - Action: routecluster("default/wowie/80/da39a3ee5e"), - }, - ), + ), + envoy.RouteConfiguration("https/hello.example.com", + envoy.VirtualHost("hello.example.com", + &envoy_api_v2_route.Route{ + Match: routePrefix("/whoop"), + Action: routecluster("default/kerpow/9000/da39a3ee5e"), + }, + &envoy_api_v2_route.Route{ + Match: routePrefix("/"), + Action: routecluster("default/wowie/80/da39a3ee5e"), + }, ), ), - TypeUrl: routeType, - Nonce: "5", - }, streamRDS(t, cc)) + ), streamRDS(t, cc)) } // contour#250 ingress.kubernetes.io/force-ssl-redirect: "true" should apply @@ -579,7 +547,7 @@ func TestSSLRedirectOverlay(t *testing.T) { }, }) - assertRDS(t, cc, "5", virtualhosts( + assertRDS(t, cc, virtualhosts( envoy.VirtualHost("example.com", &envoy_api_v2_route.Route{ Match: routePrefix("/.well-known/acme-challenge/gVJl5NWL2owUqZekjHkt_bo3OHYC2XNDURRRgLI5JTk"), @@ -658,7 +626,7 @@ func TestInvalidCertInIngress(t *testing.T) { }, }) - assertRDS(t, cc, "1", virtualhosts( + assertRDS(t, cc, virtualhosts( envoy.VirtualHost("kuard.io", &envoy_api_v2_route.Route{ Match: routePrefix("/"), @@ -677,7 +645,7 @@ func TestInvalidCertInIngress(t *testing.T) { Data: secretdata(CERTIFICATE, RSA_PRIVATE_KEY), }) - assertRDS(t, cc, "2", virtualhosts( + assertRDS(t, cc, virtualhosts( envoy.VirtualHost("kuard.io", &envoy_api_v2_route.Route{ Match: routePrefix("/"), @@ -744,7 +712,7 @@ func TestIssue257(t *testing.T) { } rh.OnAdd(s1) - assertRDS(t, cc, "2", virtualhosts( + assertRDS(t, cc, virtualhosts( envoy.VirtualHost("*", &envoy_api_v2_route.Route{ Match: routePrefix("/"), @@ -797,7 +765,7 @@ func TestIssue257(t *testing.T) { } rh.OnUpdate(i1, i2) - assertRDS(t, cc, "3", virtualhosts( + assertRDS(t, cc, virtualhosts( envoy.VirtualHost("kuard.db.gd-ms.com", &envoy_api_v2_route.Route{ Match: routePrefix("/"), @@ -906,45 +874,35 @@ func TestRDSFilter(t *testing.T) { } rh.OnAdd(s2) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "5", - Resources: routeResources(t, - envoy.RouteConfiguration("ingress_http", - envoy.VirtualHost("example.com", - &envoy_api_v2_route.Route{ - Match: routePrefix("/.well-known/acme-challenge/gVJl5NWL2owUqZekjHkt_bo3OHYC2XNDURRRgLI5JTk"), - Action: routecluster("nginx-ingress/challenge-service/8009/da39a3ee5e"), - }, - &envoy_api_v2_route.Route{ - Match: routePrefix("/"), // match all - Action: envoy.UpgradeHTTPS(), - }, - ), + tassert.ElementsMatch(t, routeResources(t, + envoy.RouteConfiguration("ingress_http", + envoy.VirtualHost("example.com", + &envoy_api_v2_route.Route{ + Match: routePrefix("/.well-known/acme-challenge/gVJl5NWL2owUqZekjHkt_bo3OHYC2XNDURRRgLI5JTk"), + Action: routecluster("nginx-ingress/challenge-service/8009/da39a3ee5e"), + }, + &envoy_api_v2_route.Route{ + Match: routePrefix("/"), // match all + Action: envoy.UpgradeHTTPS(), + }, ), ), - TypeUrl: routeType, - Nonce: "5", - }, streamRDS(t, cc, "ingress_http")) - - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "5", - Resources: routeResources(t, - envoy.RouteConfiguration("https/example.com", - envoy.VirtualHost("example.com", - &envoy_api_v2_route.Route{ - Match: routePrefix("/.well-known/acme-challenge/gVJl5NWL2owUqZekjHkt_bo3OHYC2XNDURRRgLI5JTk"), - Action: routecluster("nginx-ingress/challenge-service/8009/da39a3ee5e"), - }, - &envoy_api_v2_route.Route{ - Match: routePrefix("/"), - Action: routecluster("default/app-service/8080/da39a3ee5e"), - }, - ), + ), streamRDS(t, cc, "ingress_http")) + + tassert.ElementsMatch(t, routeResources(t, + envoy.RouteConfiguration("https/example.com", + envoy.VirtualHost("example.com", + &envoy_api_v2_route.Route{ + Match: routePrefix("/.well-known/acme-challenge/gVJl5NWL2owUqZekjHkt_bo3OHYC2XNDURRRgLI5JTk"), + Action: routecluster("nginx-ingress/challenge-service/8009/da39a3ee5e"), + }, + &envoy_api_v2_route.Route{ + Match: routePrefix("/"), + Action: routecluster("default/app-service/8080/da39a3ee5e"), + }, ), ), - TypeUrl: routeType, - Nonce: "5", - }, streamRDS(t, cc, "https/example.com")) + ), streamRDS(t, cc, "https/example.com")) } // issue 404 @@ -1027,31 +985,26 @@ func TestDefaultBackendDoesNotOverwriteNamedHost(t *testing.T) { }, }) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "1", - Resources: routeResources(t, - envoy.RouteConfiguration("ingress_http", - envoy.VirtualHost("*", - &envoy_api_v2_route.Route{ - Match: routePrefix("/kuard"), - Action: routecluster("default/kuard/8080/da39a3ee5e"), - }, - &envoy_api_v2_route.Route{ - Match: routePrefix("/"), - Action: routecluster("default/kuard/80/da39a3ee5e"), - }, - ), - envoy.VirtualHost("test-gui", - &envoy_api_v2_route.Route{ - Match: routePrefix("/"), - Action: routecluster("default/test-gui/80/da39a3ee5e"), - }, - ), + tassert.ElementsMatch(t, routeResources(t, + envoy.RouteConfiguration("ingress_http", + envoy.VirtualHost("*", + &envoy_api_v2_route.Route{ + Match: routePrefix("/kuard"), + Action: routecluster("default/kuard/8080/da39a3ee5e"), + }, + &envoy_api_v2_route.Route{ + Match: routePrefix("/"), + Action: routecluster("default/kuard/80/da39a3ee5e"), + }, + ), + envoy.VirtualHost("test-gui", + &envoy_api_v2_route.Route{ + Match: routePrefix("/"), + Action: routecluster("default/test-gui/80/da39a3ee5e"), + }, ), ), - TypeUrl: routeType, - Nonce: "1", - }, streamRDS(t, cc, "ingress_http")) + ), streamRDS(t, cc, "ingress_http")) } // Test DAGAdapter.IngressClass setting works, this could be done @@ -1093,7 +1046,7 @@ func TestRDSIngressClassAnnotation(t *testing.T) { }, } rh.OnAdd(i1) - assertRDS(t, cc, "1", virtualhosts( + assertRDS(t, cc, virtualhosts( envoy.VirtualHost("*", &envoy_api_v2_route.Route{ Match: routePrefix("/"), @@ -1118,7 +1071,7 @@ func TestRDSIngressClassAnnotation(t *testing.T) { }, } rh.OnUpdate(i1, i2) - assertRDS(t, cc, "2", nil, nil) + assertRDS(t, cc, nil, nil) i3 := &v1beta1.Ingress{ ObjectMeta: metav1.ObjectMeta{ @@ -1136,7 +1089,7 @@ func TestRDSIngressClassAnnotation(t *testing.T) { }, } rh.OnUpdate(i2, i3) - assertRDS(t, cc, "2", nil, nil) + assertRDS(t, cc, nil, nil) i4 := &v1beta1.Ingress{ ObjectMeta: metav1.ObjectMeta{ @@ -1154,7 +1107,7 @@ func TestRDSIngressClassAnnotation(t *testing.T) { }, } rh.OnUpdate(i3, i4) - assertRDS(t, cc, "3", virtualhosts( + assertRDS(t, cc, virtualhosts( envoy.VirtualHost("*", &envoy_api_v2_route.Route{ Match: routePrefix("/"), @@ -1179,7 +1132,7 @@ func TestRDSIngressClassAnnotation(t *testing.T) { }, } rh.OnUpdate(i4, i5) - assertRDS(t, cc, "4", virtualhosts( + assertRDS(t, cc, virtualhosts( envoy.VirtualHost("*", &envoy_api_v2_route.Route{ Match: routePrefix("/"), @@ -1189,7 +1142,7 @@ func TestRDSIngressClassAnnotation(t *testing.T) { ), nil) rh.OnUpdate(i5, i3) - assertRDS(t, cc, "5", nil, nil) + assertRDS(t, cc, nil, nil) } // issue 523, check for data races caused by accidentally @@ -1314,7 +1267,7 @@ func TestRDSIngressSpecMissingHTTPKey(t *testing.T) { } rh.OnAdd(s1) - assertRDS(t, cc, "2", virtualhosts( + assertRDS(t, cc, virtualhosts( envoy.VirtualHost("test2.test.com", &envoy_api_v2_route.Route{ Match: routePrefix("/"), @@ -1363,7 +1316,7 @@ func TestRouteWithAServiceWeight(t *testing.T) { } rh.OnAdd(p1) - assertRDS(t, cc, "1", virtualhosts( + assertRDS(t, cc, virtualhosts( envoy.VirtualHost("test2.test.com", &envoy_api_v2_route.Route{ Match: routePrefix("/a"), @@ -1397,7 +1350,7 @@ func TestRouteWithAServiceWeight(t *testing.T) { } rh.OnUpdate(p1, p2) - assertRDS(t, cc, "2", virtualhosts( + assertRDS(t, cc, virtualhosts( envoy.VirtualHost("test2.test.com", &envoy_api_v2_route.Route{ Match: routePrefix("/a"), @@ -1463,29 +1416,24 @@ func TestRouteWithTLS(t *testing.T) { rh.OnAdd(p1) // check that ingress_http has been updated. - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "1", - Resources: routeResources(t, - envoy.RouteConfiguration("ingress_http", - envoy.VirtualHost("test2.test.com", - &envoy_api_v2_route.Route{ - Action: envoy.UpgradeHTTPS(), - Match: routePrefix("/a"), - }, - ), + tassert.ElementsMatch(t, routeResources(t, + envoy.RouteConfiguration("ingress_http", + envoy.VirtualHost("test2.test.com", + &envoy_api_v2_route.Route{ + Action: envoy.UpgradeHTTPS(), + Match: routePrefix("/a"), + }, ), - envoy.RouteConfiguration("https/test2.test.com", - envoy.VirtualHost("test2.test.com", - &envoy_api_v2_route.Route{ - Match: routePrefix("/a"), - Action: routecluster("default/kuard/80/da39a3ee5e"), - }, - ), + ), + envoy.RouteConfiguration("https/test2.test.com", + envoy.VirtualHost("test2.test.com", + &envoy_api_v2_route.Route{ + Match: routePrefix("/a"), + Action: routecluster("default/kuard/80/da39a3ee5e"), + }, ), ), - TypeUrl: routeType, - Nonce: "1", - }, streamRDS(t, cc)) + ), streamRDS(t, cc)) } func TestRouteWithTLS_InsecurePaths(t *testing.T) { @@ -1564,37 +1512,32 @@ func TestRouteWithTLS_InsecurePaths(t *testing.T) { rh.OnAdd(p1) // check that ingress_http has been updated. - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "1", - Resources: routeResources(t, - envoy.RouteConfiguration("ingress_http", - envoy.VirtualHost("test2.test.com", - &envoy_api_v2_route.Route{ - Match: routePrefix("/secure"), - Action: envoy.UpgradeHTTPS(), - }, - &envoy_api_v2_route.Route{ - Match: routePrefix("/insecure"), - Action: routecluster("default/kuard/80/da39a3ee5e"), - }, - ), + tassert.ElementsMatch(t, routeResources(t, + envoy.RouteConfiguration("ingress_http", + envoy.VirtualHost("test2.test.com", + &envoy_api_v2_route.Route{ + Match: routePrefix("/secure"), + Action: envoy.UpgradeHTTPS(), + }, + &envoy_api_v2_route.Route{ + Match: routePrefix("/insecure"), + Action: routecluster("default/kuard/80/da39a3ee5e"), + }, ), - envoy.RouteConfiguration("https/test2.test.com", - envoy.VirtualHost("test2.test.com", - &envoy_api_v2_route.Route{ - Match: routePrefix("/secure"), - Action: routecluster("default/svc2/80/da39a3ee5e"), - }, - &envoy_api_v2_route.Route{ - Match: routePrefix("/insecure"), - Action: routecluster("default/kuard/80/da39a3ee5e"), - }, - ), + ), + envoy.RouteConfiguration("https/test2.test.com", + envoy.VirtualHost("test2.test.com", + &envoy_api_v2_route.Route{ + Match: routePrefix("/secure"), + Action: routecluster("default/svc2/80/da39a3ee5e"), + }, + &envoy_api_v2_route.Route{ + Match: routePrefix("/insecure"), + Action: routecluster("default/kuard/80/da39a3ee5e"), + }, ), ), - TypeUrl: routeType, - Nonce: "1", - }, streamRDS(t, cc)) + ), streamRDS(t, cc)) } func TestRouteWithTLS_InsecurePaths_DisablePermitInsecureTrue(t *testing.T) { @@ -1677,37 +1620,32 @@ func TestRouteWithTLS_InsecurePaths_DisablePermitInsecureTrue(t *testing.T) { rh.OnAdd(p1) // check that ingress_http has been updated. - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "1", - Resources: routeResources(t, - envoy.RouteConfiguration("ingress_http", - envoy.VirtualHost("test2.test.com", - &envoy_api_v2_route.Route{ - Match: routePrefix("/secure"), - Action: envoy.UpgradeHTTPS(), - }, - &envoy_api_v2_route.Route{ - Match: routePrefix("/insecure"), - Action: envoy.UpgradeHTTPS(), - }, - ), + tassert.ElementsMatch(t, routeResources(t, + envoy.RouteConfiguration("ingress_http", + envoy.VirtualHost("test2.test.com", + &envoy_api_v2_route.Route{ + Match: routePrefix("/secure"), + Action: envoy.UpgradeHTTPS(), + }, + &envoy_api_v2_route.Route{ + Match: routePrefix("/insecure"), + Action: envoy.UpgradeHTTPS(), + }, ), - envoy.RouteConfiguration("https/test2.test.com", - envoy.VirtualHost("test2.test.com", - &envoy_api_v2_route.Route{ - Match: routePrefix("/secure"), - Action: routecluster("default/svc2/80/da39a3ee5e"), - }, - &envoy_api_v2_route.Route{ - Match: routePrefix("/insecure"), - Action: routecluster("default/kuard/80/da39a3ee5e"), - }, - ), + ), + envoy.RouteConfiguration("https/test2.test.com", + envoy.VirtualHost("test2.test.com", + &envoy_api_v2_route.Route{ + Match: routePrefix("/secure"), + Action: routecluster("default/svc2/80/da39a3ee5e"), + }, + &envoy_api_v2_route.Route{ + Match: routePrefix("/insecure"), + Action: routecluster("default/kuard/80/da39a3ee5e"), + }, ), ), - TypeUrl: routeType, - Nonce: "1", - }, streamRDS(t, cc)) + ), streamRDS(t, cc)) } // issue 1234, assert that RoutePrefix and RouteRegex work as expected @@ -1759,28 +1697,23 @@ func TestRoutePrefixRouteRegex(t *testing.T) { rh.OnAdd(old) // check that it's been translated correctly. - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "1", - Resources: routeResources(t, - envoy.RouteConfiguration("ingress_http", - envoy.VirtualHost("*", - &envoy_api_v2_route.Route{ - Match: routeRegex("/[^/]+/invoices(/.*|/?)"), - Action: routecluster("default/kuard/80/da39a3ee5e"), - }, - &envoy_api_v2_route.Route{ - Match: routePrefix("/"), - Action: routecluster("default/kuard/80/da39a3ee5e"), - }, - ), + tassert.ElementsMatch(t, routeResources(t, + envoy.RouteConfiguration("ingress_http", + envoy.VirtualHost("*", + &envoy_api_v2_route.Route{ + Match: routeRegex("/[^/]+/invoices(/.*|/?)"), + Action: routecluster("default/kuard/80/da39a3ee5e"), + }, + &envoy_api_v2_route.Route{ + Match: routePrefix("/"), + Action: routecluster("default/kuard/80/da39a3ee5e"), + }, ), ), - TypeUrl: routeType, - Nonce: "1", - }, streamRDS(t, cc)) + ), streamRDS(t, cc)) } -func assertRDS(t *testing.T, cc *grpc.ClientConn, versioninfo string, ingress_http, ingress_https []*envoy_api_v2_route.VirtualHost) { +func assertRDS(t *testing.T, cc *grpc.ClientConn, ingress_http, ingress_https []*envoy_api_v2_route.VirtualHost) { t.Helper() routes := []*v2.RouteConfiguration{ @@ -1792,15 +1725,10 @@ func assertRDS(t *testing.T, cc *grpc.ClientConn, versioninfo string, ingress_ht envoy.RouteConfiguration(path.Join("https", vh.Name), vh)) } - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: versioninfo, - Resources: routeResources(t, routes...), - TypeUrl: routeType, - Nonce: versioninfo, - }, streamRDS(t, cc)) + tassert.ElementsMatch(t, routeResources(t, routes...), streamRDS(t, cc)) } -func streamRDS(t *testing.T, cc *grpc.ClientConn, rn ...string) *v2.DiscoveryResponse { +func streamRDS(t *testing.T, cc *grpc.ClientConn, rn ...string) []*any.Any { t.Helper() rds := v2.NewRouteDiscoveryServiceClient(cc) st, err := rds.StreamRoutes(context.TODO()) @@ -1808,7 +1736,8 @@ func streamRDS(t *testing.T, cc *grpc.ClientConn, rn ...string) *v2.DiscoveryRes return stream(t, st, &v2.DiscoveryRequest{ TypeUrl: routeType, ResourceNames: rn, - }) + Node: &envoy_api_v2_core.Node{Id: "contour"}, + }).Resources } type weightedcluster struct { @@ -1917,7 +1846,7 @@ func TestHTTPProxyRouteWithAServiceWeight(t *testing.T) { } rh.OnAdd(proxy1) - assertRDS(t, cc, "1", virtualhosts( + assertRDS(t, cc, virtualhosts( envoy.VirtualHost("test2.test.com", &envoy_api_v2_route.Route{ Match: routePrefix("/a"), @@ -1949,7 +1878,7 @@ func TestHTTPProxyRouteWithAServiceWeight(t *testing.T) { } rh.OnUpdate(proxy1, proxy2) - assertRDS(t, cc, "2", virtualhosts( + assertRDS(t, cc, virtualhosts( envoy.VirtualHost("test2.test.com", &envoy_api_v2_route.Route{ Match: routePrefix("/a"), @@ -2013,29 +1942,24 @@ func TestHTTPProxyRouteWithTLS(t *testing.T) { rh.OnAdd(proxy1) // check that ingress_http has been updated. - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "1", - Resources: routeResources(t, - envoy.RouteConfiguration("ingress_http", - envoy.VirtualHost("test2.test.com", - &envoy_api_v2_route.Route{ - Match: routePrefix("/a"), - Action: envoy.UpgradeHTTPS(), - }, - ), + tassert.ElementsMatch(t, routeResources(t, + envoy.RouteConfiguration("ingress_http", + envoy.VirtualHost("test2.test.com", + &envoy_api_v2_route.Route{ + Match: routePrefix("/a"), + Action: envoy.UpgradeHTTPS(), + }, ), - envoy.RouteConfiguration("https/test2.test.com", - envoy.VirtualHost("test2.test.com", - &envoy_api_v2_route.Route{ - Match: routePrefix("/a"), - Action: routecluster("default/kuard/80/da39a3ee5e"), - }, - ), + ), + envoy.RouteConfiguration("https/test2.test.com", + envoy.VirtualHost("test2.test.com", + &envoy_api_v2_route.Route{ + Match: routePrefix("/a"), + Action: routecluster("default/kuard/80/da39a3ee5e"), + }, ), ), - TypeUrl: routeType, - Nonce: "1", - }, streamRDS(t, cc)) + ), streamRDS(t, cc)) } func TestHTTPProxyRouteWithTLS_InsecurePaths(t *testing.T) { @@ -2110,37 +2034,32 @@ func TestHTTPProxyRouteWithTLS_InsecurePaths(t *testing.T) { rh.OnAdd(proxy1) // check that ingress_http has been updated. - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "1", - Resources: routeResources(t, - envoy.RouteConfiguration("ingress_http", - envoy.VirtualHost("test2.test.com", - &envoy_api_v2_route.Route{ - Match: routePrefix("/secure"), - Action: envoy.UpgradeHTTPS(), - }, - &envoy_api_v2_route.Route{ - Match: routePrefix("/insecure"), - Action: routecluster("default/kuard/80/da39a3ee5e"), - }, - ), + tassert.ElementsMatch(t, routeResources(t, + envoy.RouteConfiguration("ingress_http", + envoy.VirtualHost("test2.test.com", + &envoy_api_v2_route.Route{ + Match: routePrefix("/secure"), + Action: envoy.UpgradeHTTPS(), + }, + &envoy_api_v2_route.Route{ + Match: routePrefix("/insecure"), + Action: routecluster("default/kuard/80/da39a3ee5e"), + }, ), - envoy.RouteConfiguration("https/test2.test.com", - envoy.VirtualHost("test2.test.com", - &envoy_api_v2_route.Route{ - Match: routePrefix("/secure"), - Action: routecluster("default/svc2/80/da39a3ee5e"), - }, - &envoy_api_v2_route.Route{ - Match: routePrefix("/insecure"), - Action: routecluster("default/kuard/80/da39a3ee5e"), - }, - ), + ), + envoy.RouteConfiguration("https/test2.test.com", + envoy.VirtualHost("test2.test.com", + &envoy_api_v2_route.Route{ + Match: routePrefix("/secure"), + Action: routecluster("default/svc2/80/da39a3ee5e"), + }, + &envoy_api_v2_route.Route{ + Match: routePrefix("/insecure"), + Action: routecluster("default/kuard/80/da39a3ee5e"), + }, ), ), - TypeUrl: routeType, - Nonce: "1", - }, streamRDS(t, cc)) + ), streamRDS(t, cc)) } func TestHTTPProxyRouteWithTLS_InsecurePaths_DisablePermitInsecureTrue(t *testing.T) { @@ -2219,37 +2138,32 @@ func TestHTTPProxyRouteWithTLS_InsecurePaths_DisablePermitInsecureTrue(t *testin rh.OnAdd(proxy1) // check that ingress_http has been updated. - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "1", - Resources: routeResources(t, - envoy.RouteConfiguration("ingress_http", - envoy.VirtualHost("test2.test.com", - &envoy_api_v2_route.Route{ - Match: routePrefix("/secure"), - Action: envoy.UpgradeHTTPS(), - }, - &envoy_api_v2_route.Route{ - Match: routePrefix("/insecure"), - Action: envoy.UpgradeHTTPS(), - }, - ), + tassert.ElementsMatch(t, routeResources(t, + envoy.RouteConfiguration("ingress_http", + envoy.VirtualHost("test2.test.com", + &envoy_api_v2_route.Route{ + Match: routePrefix("/secure"), + Action: envoy.UpgradeHTTPS(), + }, + &envoy_api_v2_route.Route{ + Match: routePrefix("/insecure"), + Action: envoy.UpgradeHTTPS(), + }, ), - envoy.RouteConfiguration("https/test2.test.com", - envoy.VirtualHost("test2.test.com", - &envoy_api_v2_route.Route{ - Match: routePrefix("/secure"), - Action: routecluster("default/svc2/80/da39a3ee5e"), - }, - &envoy_api_v2_route.Route{ - Match: routePrefix("/insecure"), - Action: routecluster("default/kuard/80/da39a3ee5e"), - }, - ), + ), + envoy.RouteConfiguration("https/test2.test.com", + envoy.VirtualHost("test2.test.com", + &envoy_api_v2_route.Route{ + Match: routePrefix("/secure"), + Action: routecluster("default/svc2/80/da39a3ee5e"), + }, + &envoy_api_v2_route.Route{ + Match: routePrefix("/insecure"), + Action: routecluster("default/kuard/80/da39a3ee5e"), + }, ), ), - TypeUrl: routeType, - Nonce: "1", - }, streamRDS(t, cc)) + ), streamRDS(t, cc)) } func TestRDSHTTPProxyRootCannotDelegateToAnotherRoot(t *testing.T) { @@ -2309,21 +2223,16 @@ func TestRDSHTTPProxyRootCannotDelegateToAnotherRoot(t *testing.T) { // verify that child's route is present because while it is not possible to // delegate to it, it can host www.containersteve.com. - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "2", - Resources: routeResources(t, - envoy.RouteConfiguration("ingress_http", - envoy.VirtualHost("www.containersteve.com", - &envoy_api_v2_route.Route{ - Match: routePrefix("/"), - Action: routecluster("marketing/green/80/da39a3ee5e"), - }, - ), + tassert.ElementsMatch(t, routeResources(t, + envoy.RouteConfiguration("ingress_http", + envoy.VirtualHost("www.containersteve.com", + &envoy_api_v2_route.Route{ + Match: routePrefix("/"), + Action: routecluster("marketing/green/80/da39a3ee5e"), + }, ), ), - TypeUrl: routeType, - Nonce: "2", - }, streamRDS(t, cc)) + ), streamRDS(t, cc)) } func TestRDSHTTPProxyDuplicateIncludeConditions(t *testing.T) { @@ -2453,14 +2362,9 @@ func TestRDSHTTPProxyDuplicateIncludeConditions(t *testing.T) { rh.OnAdd(proxyChildA) rh.OnAdd(proxyChildB) - assert.Equal(t, &v2.DiscoveryResponse{ - VersionInfo: "2", - Resources: routeResources(t, - envoy.RouteConfiguration("ingress_http"), - ), - TypeUrl: routeType, - Nonce: "2", - }, streamRDS(t, cc)) + tassert.ElementsMatch(t, routeResources(t, + envoy.RouteConfiguration("ingress_http"), + ), streamRDS(t, cc)) } func virtualhosts(v ...*envoy_api_v2_route.VirtualHost) []*envoy_api_v2_route.VirtualHost { return v } diff --git a/internal/envoy/accesslog_test.go b/internal/envoy/accesslog_test.go index be359e92451..ff785f82b94 100644 --- a/internal/envoy/accesslog_test.go +++ b/internal/envoy/accesslog_test.go @@ -20,8 +20,8 @@ import ( envoy_accesslog "github.com/envoyproxy/go-control-plane/envoy/config/filter/accesslog/v2" "github.com/envoyproxy/go-control-plane/pkg/wellknown" _struct "github.com/golang/protobuf/ptypes/struct" - "github.com/projectcontour/contour/internal/assert" "github.com/projectcontour/contour/internal/protobuf" + "github.com/stretchr/testify/assert" ) func TestFileAccessLog(t *testing.T) { diff --git a/internal/envoy/auth_test.go b/internal/envoy/auth_test.go index f94ec125daa..3854f136c8e 100644 --- a/internal/envoy/auth_test.go +++ b/internal/envoy/auth_test.go @@ -16,10 +16,11 @@ package envoy import ( "testing" + "github.com/projectcontour/contour/internal/assert" + envoy_api_v2_auth "github.com/envoyproxy/go-control-plane/envoy/api/v2/auth" envoy_api_v2_core "github.com/envoyproxy/go-control-plane/envoy/api/v2/core" matcher "github.com/envoyproxy/go-control-plane/envoy/type/matcher" - "github.com/google/go-cmp/cmp" "github.com/projectcontour/contour/internal/dag" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -108,9 +109,7 @@ func TestUpstreamTLSContext(t *testing.T) { for name, tc := range tests { t.Run(name, func(t *testing.T) { got := UpstreamTLSContext(tc.validation, tc.externalName, tc.alpnProtocols...) - if diff := cmp.Diff(tc.want, got); diff != "" { - t.Fatal(diff) - } + assert.Equal(t, tc.want, got) }) } } diff --git a/internal/envoy/endpoint_test.go b/internal/envoy/endpoint_test.go index 6ab3711832e..1d600d82542 100644 --- a/internal/envoy/endpoint_test.go +++ b/internal/envoy/endpoint_test.go @@ -18,7 +18,7 @@ import ( v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2" envoy_api_v2_endpoint "github.com/envoyproxy/go-control-plane/envoy/api/v2/endpoint" - "github.com/google/go-cmp/cmp" + "github.com/projectcontour/contour/internal/assert" ) func TestLBEndpoint(t *testing.T) { @@ -30,9 +30,7 @@ func TestLBEndpoint(t *testing.T) { }, }, } - if diff := cmp.Diff(want, got); diff != "" { - t.Fatal(diff) - } + assert.Equal(t, want, got) } func TestEndpoints(t *testing.T) { @@ -55,9 +53,7 @@ func TestEndpoints(t *testing.T) { }, }}, }} - if diff := cmp.Diff(want, got); diff != "" { - t.Fatal(diff) - } + assert.Equal(t, want, got) } func TestClusterLoadAssignment(t *testing.T) { @@ -66,9 +62,7 @@ func TestClusterLoadAssignment(t *testing.T) { ClusterName: "empty", } - if diff := cmp.Diff(want, got); diff != "" { - t.Fatal(diff) - } + assert.Equal(t, want, got) got = ClusterLoadAssignment("one addr", SocketAddress("microsoft.com", 81)) want = &v2.ClusterLoadAssignment{ @@ -76,9 +70,7 @@ func TestClusterLoadAssignment(t *testing.T) { Endpoints: Endpoints(SocketAddress("microsoft.com", 81)), } - if diff := cmp.Diff(want, got); diff != "" { - t.Fatal(diff) - } + assert.Equal(t, want, got) got = ClusterLoadAssignment("two addrs", SocketAddress("microsoft.com", 81), @@ -92,7 +84,5 @@ func TestClusterLoadAssignment(t *testing.T) { ), } - if diff := cmp.Diff(want, got); diff != "" { - t.Fatal(diff) - } + assert.Equal(t, want, got) } diff --git a/internal/envoy/healthcheck_test.go b/internal/envoy/healthcheck_test.go index 7a84bdc959f..e713eaba5dc 100644 --- a/internal/envoy/healthcheck_test.go +++ b/internal/envoy/healthcheck_test.go @@ -18,7 +18,7 @@ import ( "time" envoy_api_v2_core "github.com/envoyproxy/go-control-plane/envoy/api/v2/core" - "github.com/google/go-cmp/cmp" + "github.com/projectcontour/contour/internal/assert" "github.com/projectcontour/contour/internal/dag" "github.com/projectcontour/contour/internal/protobuf" ) @@ -95,10 +95,7 @@ func TestHealthCheck(t *testing.T) { for name, tc := range tests { t.Run(name, func(t *testing.T) { got := httpHealthCheck(tc.cluster) - if diff := cmp.Diff(tc.want, got); diff != "" { - t.Fatal(diff) - } - + assert.Equal(t, tc.want, got) }) } } diff --git a/internal/envoy/listener_test.go b/internal/envoy/listener_test.go index 9cbf0e43cd5..32ee9e53c54 100644 --- a/internal/envoy/listener_test.go +++ b/internal/envoy/listener_test.go @@ -25,7 +25,6 @@ import ( http "github.com/envoyproxy/go-control-plane/envoy/config/filter/network/http_connection_manager/v2" envoy_config_v2_tcpproxy "github.com/envoyproxy/go-control-plane/envoy/config/filter/network/tcp_proxy/v2" "github.com/envoyproxy/go-control-plane/pkg/wellknown" - "github.com/google/go-cmp/cmp" "github.com/projectcontour/contour/internal/assert" "github.com/projectcontour/contour/internal/dag" "github.com/projectcontour/contour/internal/protobuf" @@ -143,9 +142,7 @@ func TestSocketAddress(t *testing.T) { }, }, } - if diff := cmp.Diff(want, got); diff != "" { - t.Fatal(diff) - } + assert.Equal(t, want, got) got = SocketAddress("::", port) want = &envoy_api_v2_core.Address{ diff --git a/internal/envoy/secret_test.go b/internal/envoy/secret_test.go index 9863f685971..c6d1a91e7ff 100644 --- a/internal/envoy/secret_test.go +++ b/internal/envoy/secret_test.go @@ -16,6 +16,8 @@ package envoy import ( "testing" + "github.com/projectcontour/contour/internal/assert" + envoy_api_v2_auth "github.com/envoyproxy/go-control-plane/envoy/api/v2/auth" envoy_api_v2_core "github.com/envoyproxy/go-control-plane/envoy/api/v2/core" "github.com/google/go-cmp/cmp" @@ -65,9 +67,7 @@ func TestSecret(t *testing.T) { for name, tc := range tests { t.Run(name, func(t *testing.T) { got := Secret(tc.secret) - if diff := cmp.Diff(tc.want, got); diff != "" { - t.Fatal(diff) - } + assert.Equal(t, tc.want, got) }) } } diff --git a/internal/envoy/stats_test.go b/internal/envoy/stats_test.go index e8b10820f88..5769f7a82f0 100644 --- a/internal/envoy/stats_test.go +++ b/internal/envoy/stats_test.go @@ -16,12 +16,13 @@ package envoy import ( "testing" + "github.com/projectcontour/contour/internal/assert" + v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2" envoy_api_v2_listener "github.com/envoyproxy/go-control-plane/envoy/api/v2/listener" envoy_api_v2_route "github.com/envoyproxy/go-control-plane/envoy/api/v2/route" http "github.com/envoyproxy/go-control-plane/envoy/config/filter/network/http_connection_manager/v2" "github.com/envoyproxy/go-control-plane/pkg/wellknown" - "github.com/google/go-cmp/cmp" "github.com/projectcontour/contour/internal/protobuf" ) @@ -94,9 +95,7 @@ func TestStatsListener(t *testing.T) { for name, tc := range tests { t.Run(name, func(t *testing.T) { got := StatsListener(tc.address, tc.port) - if diff := cmp.Diff(tc.want, got); diff != "" { - t.Fatal(diff) - } + assert.Equal(t, tc.want, got) }) } } diff --git a/internal/featuretests/backendcavalidation_test.go b/internal/featuretests/backendcavalidation_test.go index 437f5ea1b81..d424b647e8b 100644 --- a/internal/featuretests/backendcavalidation_test.go +++ b/internal/featuretests/backendcavalidation_test.go @@ -216,5 +216,4 @@ func TestClusterServiceTLSBackendCAValidation(t *testing.T) { Resources: nil, TypeUrl: secretType, }) - } diff --git a/internal/featuretests/featuretests.go b/internal/featuretests/featuretests.go index 2730c780145..8b8b5f13766 100644 --- a/internal/featuretests/featuretests.go +++ b/internal/featuretests/featuretests.go @@ -23,9 +23,11 @@ import ( "time" v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2" + envoy_api_v2_core "github.com/envoyproxy/go-control-plane/envoy/api/v2/core" discovery "github.com/envoyproxy/go-control-plane/envoy/service/discovery/v2" + cachetypes "github.com/envoyproxy/go-control-plane/pkg/cache/types" + v2cache "github.com/envoyproxy/go-control-plane/pkg/cache/v2" resource "github.com/envoyproxy/go-control-plane/pkg/resource/v2" - "github.com/golang/protobuf/proto" "github.com/golang/protobuf/ptypes/any" projcontour "github.com/projectcontour/contour/apis/projectcontour/v1" "github.com/projectcontour/contour/internal/assert" @@ -39,6 +41,7 @@ import ( "github.com/projectcontour/contour/internal/workgroup" "github.com/prometheus/client_golang/prometheus" "github.com/sirupsen/logrus" + tassert "github.com/stretchr/testify/assert" "google.golang.org/grpc" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" @@ -105,6 +108,21 @@ func setupWithFallbackCert(t *testing.T, fallbackCertName, fallbackCertNamespace }, } + // Setup snapshot cache + snapshotCache := v2cache.NewSnapshotCache(false, v2cache.IDHash{}, log) + + snapshotHandler := &contour.SnapshotHandler{ + CacheHandler: ch, + EndpointsTranslator: et, + SnapshotCache: snapshotCache, + } + + ch.SnapshotHandler = snapshotHandler + et.SnapshotHandler = snapshotHandler + + // Trigger an initial snapshot for static resources + snapshotHandler.UpdateSnapshot() + for _, opt := range opts { opt(eh) } @@ -116,14 +134,8 @@ func setupWithFallbackCert(t *testing.T, fallbackCertName, fallbackCertNamespace check(t, err) discard := logrus.New() discard.Out = new(discardWriter) - // Resource types in xDS v2. - srv := cgrpc.NewAPI(discard, map[string]cgrpc.Resource{ - ch.ClusterCache.TypeURL(): &ch.ClusterCache, - ch.RouteCache.TypeURL(): &ch.RouteCache, - ch.ListenerCache.TypeURL(): &ch.ListenerCache, - ch.SecretCache.TypeURL(): &ch.SecretCache, - et.TypeURL(): et, - }, r) + + srv := cgrpc.NewAPI(prometheus.NewRegistry(), snapshotCache) var g workgroup.Group @@ -241,7 +253,7 @@ func routeResources(t *testing.T, routes ...*v2.RouteConfiguration) []*any.Any { return resources(t, protobuf.AsMessages(routes)...) } -func resources(t *testing.T, protos ...proto.Message) []*any.Any { +func resources(t *testing.T, protos ...cachetypes.Resource) []*any.Any { t.Helper() anys := make([]*any.Any, 0, len(protos)) for _, pb := range protos { @@ -362,10 +374,13 @@ func (c *Contour) Request(typeurl string, names ...string) *Response { default: c.Fatal("unknown typeURL:", typeurl) } + resp := c.sendRequest(st, &v2.DiscoveryRequest{ TypeUrl: typeurl, ResourceNames: names, + Node: &envoy_api_v2_core.Node{Id: "contour"}, }) + return &Response{ Contour: c, DiscoveryResponse: resp, @@ -393,7 +408,6 @@ type Response struct { func (r *Response) Equals(want *v2.DiscoveryResponse) *Contour { r.Helper() - assert.Equal(r.T, want, r.DiscoveryResponse) - + tassert.ElementsMatch(r.T, want.Resources, r.DiscoveryResponse.Resources) return r.Contour } diff --git a/internal/grpc/server.go b/internal/grpc/server.go index 4b068585655..862897ad272 100644 --- a/internal/grpc/server.go +++ b/internal/grpc/server.go @@ -17,107 +17,33 @@ package grpc import ( "context" - "google.golang.org/grpc" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" - + api "github.com/envoyproxy/go-control-plane/envoy/api/v2" + discovery "github.com/envoyproxy/go-control-plane/envoy/service/discovery/v2" + "github.com/envoyproxy/go-control-plane/pkg/cache/v2" + xds "github.com/envoyproxy/go-control-plane/pkg/server/v2" grpc_prometheus "github.com/grpc-ecosystem/go-grpc-prometheus" "github.com/prometheus/client_golang/prometheus" - - v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2" - discovery "github.com/envoyproxy/go-control-plane/envoy/service/discovery/v2" - loadstats "github.com/envoyproxy/go-control-plane/envoy/service/load_stats/v2" - "github.com/sirupsen/logrus" + "google.golang.org/grpc" ) // NewAPI returns a *grpc.Server which responds to the Envoy v2 xDS gRPC API. -func NewAPI(log logrus.FieldLogger, resources map[string]Resource, registry *prometheus.Registry, opts ...grpc.ServerOption) *grpc.Server { - s := &grpcServer{ - xdsHandler{ - FieldLogger: log, - resources: resources, - }, - grpc_prometheus.NewServerMetrics(), - } - registry.MustRegister(s.metrics) - opts = append(opts, grpc.StreamInterceptor(s.metrics.StreamServerInterceptor()), - grpc.UnaryInterceptor(s.metrics.UnaryServerInterceptor())) - g := grpc.NewServer(opts...) - v2.RegisterClusterDiscoveryServiceServer(g, s) - v2.RegisterEndpointDiscoveryServiceServer(g, s) - v2.RegisterListenerDiscoveryServiceServer(g, s) - v2.RegisterRouteDiscoveryServiceServer(g, s) - discovery.RegisterSecretDiscoveryServiceServer(g, s) - s.metrics.InitializeMetrics(g) - return g -} - -// grpcServer implements the LDS, RDS, CDS, and EDS, gRPC endpoints. -type grpcServer struct { - xdsHandler - metrics *grpc_prometheus.ServerMetrics -} - -func (s *grpcServer) FetchClusters(_ context.Context, req *v2.DiscoveryRequest) (*v2.DiscoveryResponse, error) { - return nil, status.Errorf(codes.Unimplemented, "FetchClusters unimplemented") -} - -func (s *grpcServer) FetchEndpoints(_ context.Context, req *v2.DiscoveryRequest) (*v2.DiscoveryResponse, error) { - return nil, status.Errorf(codes.Unimplemented, "FetchEndpoints unimplemented") -} - -func (s *grpcServer) DeltaEndpoints(v2.EndpointDiscoveryService_DeltaEndpointsServer) error { - return status.Errorf(codes.Unimplemented, "DeltaEndpoints unimplemented") -} - -func (s *grpcServer) FetchListeners(_ context.Context, req *v2.DiscoveryRequest) (*v2.DiscoveryResponse, error) { - return nil, status.Errorf(codes.Unimplemented, "FetchListeners unimplemented") -} - -func (s *grpcServer) DeltaListeners(v2.ListenerDiscoveryService_DeltaListenersServer) error { - return status.Errorf(codes.Unimplemented, "DeltaListeners unimplemented") -} - -func (s *grpcServer) FetchRoutes(_ context.Context, req *v2.DiscoveryRequest) (*v2.DiscoveryResponse, error) { - return nil, status.Errorf(codes.Unimplemented, "FetchRoutes unimplemented") -} - -func (s *grpcServer) FetchSecrets(_ context.Context, req *v2.DiscoveryRequest) (*v2.DiscoveryResponse, error) { - return nil, status.Errorf(codes.Unimplemented, "FetchSecrets unimplemented") -} - -func (s *grpcServer) DeltaSecrets(discovery.SecretDiscoveryService_DeltaSecretsServer) error { - return status.Errorf(codes.Unimplemented, "DeltaSecrets unimplemented") -} - -func (s *grpcServer) StreamClusters(srv v2.ClusterDiscoveryService_StreamClustersServer) error { - return s.stream(srv) -} - -func (s *grpcServer) StreamEndpoints(srv v2.EndpointDiscoveryService_StreamEndpointsServer) error { - return s.stream(srv) -} - -func (s *grpcServer) StreamLoadStats(srv loadstats.LoadReportingService_StreamLoadStatsServer) error { - return status.Errorf(codes.Unimplemented, "StreamLoadStats unimplemented") -} - -func (s *grpcServer) DeltaClusters(v2.ClusterDiscoveryService_DeltaClustersServer) error { - return status.Errorf(codes.Unimplemented, "IncrementalClusters unimplemented") -} +func NewAPI(registry *prometheus.Registry, snapshotCache cache.SnapshotCache, opts ...grpc.ServerOption) *grpc.Server { + metrics := grpc_prometheus.NewServerMetrics() -func (s *grpcServer) DeltaRoutes(v2.RouteDiscoveryService_DeltaRoutesServer) error { - return status.Errorf(codes.Unimplemented, "IncrementalRoutes unimplemented") -} + registry.MustRegister(metrics) + opts = append(opts, grpc.StreamInterceptor(metrics.StreamServerInterceptor()), + grpc.UnaryInterceptor(metrics.UnaryServerInterceptor())) + g := grpc.NewServer(opts...) -func (s *grpcServer) StreamListeners(srv v2.ListenerDiscoveryService_StreamListenersServer) error { - return s.stream(srv) -} + xdsServer := xds.NewServer(context.Background(), snapshotCache, nil) -func (s *grpcServer) StreamRoutes(srv v2.RouteDiscoveryService_StreamRoutesServer) error { - return s.stream(srv) -} + discovery.RegisterAggregatedDiscoveryServiceServer(g, xdsServer) + discovery.RegisterSecretDiscoveryServiceServer(g, xdsServer) + api.RegisterEndpointDiscoveryServiceServer(g, xdsServer) + api.RegisterClusterDiscoveryServiceServer(g, xdsServer) + api.RegisterRouteDiscoveryServiceServer(g, xdsServer) + api.RegisterListenerDiscoveryServiceServer(g, xdsServer) -func (s *grpcServer) StreamSecrets(srv discovery.SecretDiscoveryService_StreamSecretsServer) error { - return s.stream(srv) + metrics.InitializeMetrics(g) + return g } diff --git a/internal/grpc/server_test.go b/internal/grpc/server_test.go deleted file mode 100644 index 784e7a3814f..00000000000 --- a/internal/grpc/server_test.go +++ /dev/null @@ -1,275 +0,0 @@ -// Copyright Project Contour Authors -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package grpc - -import ( - "context" - "io/ioutil" - "net" - "testing" - "time" - - v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2" - discovery "github.com/envoyproxy/go-control-plane/envoy/service/discovery/v2" - resource "github.com/envoyproxy/go-control-plane/pkg/resource/v2" - "github.com/projectcontour/contour/internal/contour" - "github.com/projectcontour/contour/internal/metrics" - "github.com/prometheus/client_golang/prometheus" - "github.com/sirupsen/logrus" - "google.golang.org/grpc" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" - v1 "k8s.io/api/core/v1" - "k8s.io/api/networking/v1beta1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/intstr" -) - -func TestGRPC(t *testing.T) { - // tr and et is recreated before the start of each test. - var et *contour.EndpointsTranslator - var eh *contour.EventHandler - - tests := map[string]func(*testing.T, *grpc.ClientConn){ - "StreamClusters": func(t *testing.T, cc *grpc.ClientConn) { - eh.OnAdd(&v1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: "simple", - Namespace: "default", - }, - Spec: v1.ServiceSpec{ - Selector: map[string]string{ - "app": "simple", - }, - Ports: []v1.ServicePort{{ - Protocol: "TCP", - Port: 80, - TargetPort: intstr.FromInt(6502), - }}, - }, - }) - - sds := v2.NewClusterDiscoveryServiceClient(cc) - ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) - defer cancel() - stream, err := sds.StreamClusters(ctx) - check(t, err) - sendreq(t, stream, resource.ClusterType) // send initial notification - checkrecv(t, stream) // check we receive one notification - checktimeout(t, stream) // check that the second receive times out - }, - "StreamEndpoints": func(t *testing.T, cc *grpc.ClientConn) { - et.OnAdd(&v1.Endpoints{ - ObjectMeta: metav1.ObjectMeta{ - Name: "kube-scheduler", - Namespace: "kube-system", - }, - Subsets: []v1.EndpointSubset{{ - Addresses: []v1.EndpointAddress{{ - IP: "130.211.139.167", - }}, - Ports: []v1.EndpointPort{{ - Port: 80, - }, { - Port: 443, - }}, - }}, - }) - - eds := v2.NewEndpointDiscoveryServiceClient(cc) - ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) - defer cancel() - stream, err := eds.StreamEndpoints(ctx) - check(t, err) - sendreq(t, stream, resource.EndpointType) // send initial notification - checkrecv(t, stream) // check we receive one notification - checktimeout(t, stream) // check that the second receive times out - }, - "StreamListeners": func(t *testing.T, cc *grpc.ClientConn) { - // add an ingress, which will create a non tls listener - eh.OnAdd(&v1beta1.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Name: "httpbin-org", - Namespace: "default", - }, - Spec: v1beta1.IngressSpec{ - Rules: []v1beta1.IngressRule{{ - Host: "httpbin.org", - IngressRuleValue: v1beta1.IngressRuleValue{ - HTTP: &v1beta1.HTTPIngressRuleValue{ - Paths: []v1beta1.HTTPIngressPath{{ - Backend: v1beta1.IngressBackend{ - ServiceName: "httpbin-org", - ServicePort: intstr.FromInt(80), - }, - }}, - }, - }, - }}, - }, - }) - - lds := v2.NewListenerDiscoveryServiceClient(cc) - ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) - defer cancel() - stream, err := lds.StreamListeners(ctx) - check(t, err) - sendreq(t, stream, resource.ListenerType) // send initial notification - checkrecv(t, stream) // check we receive one notification - checktimeout(t, stream) // check that the second receive times out - }, - "StreamRoutes": func(t *testing.T, cc *grpc.ClientConn) { - eh.OnAdd(&v1beta1.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Name: "httpbin-org", - Namespace: "default", - }, - Spec: v1beta1.IngressSpec{ - Rules: []v1beta1.IngressRule{{ - Host: "httpbin.org", - IngressRuleValue: v1beta1.IngressRuleValue{ - HTTP: &v1beta1.HTTPIngressRuleValue{ - Paths: []v1beta1.HTTPIngressPath{{ - Backend: v1beta1.IngressBackend{ - ServiceName: "httpbin-org", - ServicePort: intstr.FromInt(80), - }, - }}, - }, - }, - }}, - }, - }) - - rds := v2.NewRouteDiscoveryServiceClient(cc) - ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) - defer cancel() - stream, err := rds.StreamRoutes(ctx) - check(t, err) - sendreq(t, stream, resource.RouteType) // send initial notification - checkrecv(t, stream) // check we receive one notification - checktimeout(t, stream) // check that the second receive times out - }, - "StreamSecrets": func(t *testing.T, cc *grpc.ClientConn) { - eh.OnAdd(&v1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "secret", - Namespace: "default", - }, - Data: map[string][]byte{ - v1.TLSCertKey: []byte("certificate"), - v1.TLSPrivateKeyKey: []byte("key"), - }, - }) - - sds := discovery.NewSecretDiscoveryServiceClient(cc) - ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) - defer cancel() - stream, err := sds.StreamSecrets(ctx) - check(t, err) - sendreq(t, stream, resource.SecretType) // send initial notification - checkrecv(t, stream) // check we receive one notification - checktimeout(t, stream) // check that the second receive times out - }, - } - - log := logrus.New() - log.SetOutput(ioutil.Discard) - for name, fn := range tests { - t.Run(name, func(t *testing.T) { - et = &contour.EndpointsTranslator{ - FieldLogger: log, - } - ch := contour.CacheHandler{ - Metrics: metrics.NewMetrics(prometheus.NewRegistry()), - } - - eh = &contour.EventHandler{ - CacheHandler: &ch, - FieldLogger: log, - } - r := prometheus.NewRegistry() - srv := NewAPI(log, map[string]Resource{ - ch.ClusterCache.TypeURL(): &ch.ClusterCache, - ch.RouteCache.TypeURL(): &ch.RouteCache, - ch.ListenerCache.TypeURL(): &ch.ListenerCache, - ch.SecretCache.TypeURL(): &ch.SecretCache, - et.TypeURL(): et, - }, r) - l, err := net.Listen("tcp", "127.0.0.1:0") - check(t, err) - done := make(chan error, 1) - stop := make(chan struct{}) - run := eh.Start() - go run(stop) // nolint:errcheck - go func() { - done <- srv.Serve(l) - }() - defer func() { - srv.Stop() - close(stop) - <-done - }() - cc, err := grpc.Dial(l.Addr().String(), grpc.WithInsecure()) - check(t, err) - defer cc.Close() - fn(t, cc) - }) - } -} - -func check(t *testing.T, err error) { - t.Helper() - if err != nil { - t.Fatal(err) - } -} - -func sendreq(t *testing.T, stream interface { - Send(*v2.DiscoveryRequest) error -}, typeurl string) { - t.Helper() - err := stream.Send(&v2.DiscoveryRequest{ - TypeUrl: typeurl, - }) - check(t, err) -} - -func checkrecv(t *testing.T, stream interface { - Recv() (*v2.DiscoveryResponse, error) -}) { - t.Helper() - _, err := stream.Recv() - check(t, err) -} - -func checktimeout(t *testing.T, stream interface { - Recv() (*v2.DiscoveryResponse, error) -}) { - t.Helper() - _, err := stream.Recv() - if err == nil { - t.Fatal("expected timeout") - } - s, ok := status.FromError(err) - if !ok { - t.Fatalf("%T %v", err, err) - } - - // Work around grpc/grpc-go#1645 which sometimes seems to - // set the status code to Unknown, even when the message is derived from context.DeadlineExceeded. - if s.Code() != codes.DeadlineExceeded && s.Message() != context.DeadlineExceeded.Error() { - t.Fatalf("expected %q, got %q %T %v", codes.DeadlineExceeded, s.Code(), err, err) - } -} diff --git a/internal/grpc/xds.go b/internal/grpc/xds.go deleted file mode 100644 index 632e5fe51a0..00000000000 --- a/internal/grpc/xds.go +++ /dev/null @@ -1,165 +0,0 @@ -// Copyright Project Contour Authors -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package grpc - -import ( - "context" - "fmt" - "strconv" - "sync/atomic" - - envoy_api_v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2" - "github.com/golang/protobuf/proto" - "github.com/golang/protobuf/ptypes" - "github.com/golang/protobuf/ptypes/any" - "github.com/sirupsen/logrus" -) - -// Resource represents a source of proto.Messages that can be registered -// for interest. -type Resource interface { - // Contents returns the contents of this resource. - Contents() []proto.Message - - // Query returns an entry for each resource name supplied. - Query(names []string) []proto.Message - - // Register registers ch to receive a value when Notify is called. - Register(chan int, int, ...string) - - // TypeURL returns the typeURL of messages returned from Values. - TypeURL() string -} - -// xdsHandler implements the Envoy xDS gRPC protocol. -type xdsHandler struct { - logrus.FieldLogger - connections counter - resources map[string]Resource // registered resource types -} - -type grpcStream interface { - Context() context.Context - Send(*envoy_api_v2.DiscoveryResponse) error - Recv() (*envoy_api_v2.DiscoveryRequest, error) -} - -// stream processes a stream of DiscoveryRequests. -func (xh *xdsHandler) stream(st grpcStream) error { - // bump connection counter and set it as a field on the logger - log := xh.WithField("connection", xh.connections.next()) - - // Notify whether the stream terminated on error. - done := func(log *logrus.Entry, err error) error { - if err != nil { - log.WithError(err).Error("stream terminated") - } else { - log.Info("stream terminated") - } - - return err - } - - ch := make(chan int, 1) - - // internally all registration values start at zero so sending - // a last that is less than zero will guarantee that each stream - // will generate a response immediately, then wait. - last := -1 - ctx := st.Context() - - // now stick in this loop until the client disconnects. - for { - // first we wait for the request from Envoy, this is part of - // the xDS protocol. - req, err := st.Recv() - if err != nil { - return done(log, err) - } - - // note: redeclare log in this scope so the next time around the loop all is forgotten. - log := log.WithField("version_info", req.VersionInfo).WithField("response_nonce", req.ResponseNonce) - if req.Node != nil { - log = log.WithField("node_id", req.Node.Id).WithField("node_version", req.Node.BuildVersion) - } - - if status := req.ErrorDetail; status != nil { - // if Envoy rejected the last update log the details here. - // TODO(dfc) issue 1176: handle xDS ACK/NACK - log.WithField("code", status.Code).Error(status.Message) - } - - // from the request we derive the resource to stream which have - // been registered according to the typeURL. - r, ok := xh.resources[req.TypeUrl] - if !ok { - return done(log, fmt.Errorf("no resource registered for typeURL %q", req.TypeUrl)) - } - - log = log.WithField("resource_names", req.ResourceNames).WithField("type_url", req.TypeUrl) - log.Info("stream_wait") - - // now we wait for a notification, if this is the first request received on this - // connection last will be less than zero and that will trigger a response immediately. - r.Register(ch, last, req.ResourceNames...) - select { - case last = <-ch: - // boom, something in the cache has changed. - // TODO(dfc) the thing that has changed may not be in the scope of the filter - // so we're going to be sending an update that is a no-op. See #426 - - var resources []proto.Message - switch len(req.ResourceNames) { - case 0: - // no resource hints supplied, return the full - // contents of the resource - resources = r.Contents() - default: - // resource hints supplied, return exactly those - resources = r.Query(req.ResourceNames) - } - - any := make([]*any.Any, 0, len(resources)) - for _, r := range resources { - a, err := ptypes.MarshalAny(r) - if err != nil { - return done(log, err) - } - - any = append(any, a) - } - - resp := &envoy_api_v2.DiscoveryResponse{ - VersionInfo: strconv.Itoa(last), - Resources: any, - TypeUrl: r.TypeURL(), - Nonce: strconv.Itoa(last), - } - - if err := st.Send(resp); err != nil { - return done(log, err) - } - - case <-ctx.Done(): - return done(log, ctx.Err()) - } - } -} - -// counter holds an atomically incrementing counter. -type counter uint64 - -func (c *counter) next() uint64 { - return atomic.AddUint64((*uint64)(c), 1) -} diff --git a/internal/grpc/xds_test.go b/internal/grpc/xds_test.go deleted file mode 100644 index d0cfb662335..00000000000 --- a/internal/grpc/xds_test.go +++ /dev/null @@ -1,209 +0,0 @@ -// Copyright Project Contour Authors -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package grpc - -import ( - "context" - "fmt" - "io" - "io/ioutil" - "testing" - - v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2" - "github.com/golang/protobuf/proto" - "github.com/sirupsen/logrus" -) - -func TestXDSHandlerStream(t *testing.T) { - log := logrus.New() - log.SetOutput(ioutil.Discard) - tests := map[string]struct { - xh xdsHandler - stream grpcStream - want error - }{ - "recv returns error immediately": { - xh: xdsHandler{FieldLogger: log}, - stream: &mockStream{ - context: context.Background, - recv: func() (*v2.DiscoveryRequest, error) { - return nil, io.EOF - }, - }, - want: io.EOF, - }, - "no registered typeURL": { - xh: xdsHandler{FieldLogger: log}, - stream: &mockStream{ - context: context.Background, - recv: func() (*v2.DiscoveryRequest, error) { - return &v2.DiscoveryRequest{ - TypeUrl: "com.heptio.potato", - }, nil - }, - }, - want: fmt.Errorf("no resource registered for typeURL %q", "com.heptio.potato"), - }, - "failed to convert values to any": { - xh: xdsHandler{ - FieldLogger: log, - resources: map[string]Resource{ - "com.heptio.potato": &mockResource{ - register: func(ch chan int, i int) { - ch <- i + 1 - }, - contents: func() []proto.Message { - return []proto.Message{nil} - }, - typeurl: func() string { return "com.heptio.potato" }, - }, - }, - }, - stream: &mockStream{ - context: context.Background, - recv: func() (*v2.DiscoveryRequest, error) { - return &v2.DiscoveryRequest{ - TypeUrl: "com.heptio.potato", - }, nil - }, - }, - want: fmt.Errorf("proto: Marshal called with nil"), - }, - "failed to send": { - xh: xdsHandler{ - FieldLogger: log, - resources: map[string]Resource{ - "com.heptio.potato": &mockResource{ - register: func(ch chan int, i int) { - ch <- i + 1 - }, - contents: func() []proto.Message { - return []proto.Message{new(v2.ClusterLoadAssignment)} - }, - typeurl: func() string { return "com.heptio.potato" }, - }, - }, - }, - stream: &mockStream{ - context: context.Background, - recv: func() (*v2.DiscoveryRequest, error) { - return &v2.DiscoveryRequest{ - TypeUrl: "com.heptio.potato", - }, nil - }, - send: func(resp *v2.DiscoveryResponse) error { - return io.EOF - }, - }, - want: io.EOF, - }, - "context canceled": { - xh: xdsHandler{ - FieldLogger: log, - resources: map[string]Resource{ - "com.heptio.potato": &mockResource{ - register: func(ch chan int, i int) { - // do nothing - }, - typeurl: func() string { return "com.heptio.potato" }, - }, - }, - }, - stream: &mockStream{ - context: func() context.Context { - ctx := context.Background() - ctx, cancel := context.WithCancel(ctx) - cancel() - return ctx - }, - recv: func() (*v2.DiscoveryRequest, error) { - return &v2.DiscoveryRequest{ - TypeUrl: "com.heptio.potato", - }, nil - }, - send: func(resp *v2.DiscoveryResponse) error { - return io.EOF - }, - }, - want: context.Canceled, - }, - } - - for name, tc := range tests { - t.Run(name, func(t *testing.T) { - got := tc.xh.stream(tc.stream) - if !equalError(tc.want, got) { - t.Fatalf("expected: %v, got: %v", tc.want, got) - } - }) - } -} - -type mockStream struct { - context func() context.Context - send func(*v2.DiscoveryResponse) error - recv func() (*v2.DiscoveryRequest, error) -} - -func (m *mockStream) Context() context.Context { return m.context() } -func (m *mockStream) Send(resp *v2.DiscoveryResponse) error { return m.send(resp) } -func (m *mockStream) Recv() (*v2.DiscoveryRequest, error) { return m.recv() } - -type mockResource struct { - contents func() []proto.Message - query func([]string) []proto.Message - register func(chan int, int) - typeurl func() string -} - -func (m *mockResource) Contents() []proto.Message { return m.contents() } -func (m *mockResource) Query(names []string) []proto.Message { return m.query(names) } -func (m *mockResource) Register(ch chan int, last int, hints ...string) { m.register(ch, last) } -func (m *mockResource) TypeURL() string { return m.typeurl() } - -func TestCounterNext(t *testing.T) { - var c counter - // not a map this time as we want tests to execute - // in sequence. - tests := []struct { - fn func() uint64 - want uint64 - }{{ - fn: c.next, - want: 1, - }, { - fn: c.next, - want: 2, - }, { - fn: c.next, - want: 3, - }} - - for _, tc := range tests { - got := tc.fn() - if tc.want != got { - t.Fatalf("expected %d, got %d", tc.want, got) - } - } -} - -func equalError(a, b error) bool { - if a == nil { - return b == nil - } - if b == nil { - return a == nil - } - return a.Error() == b.Error() -} diff --git a/internal/protobuf/helpers.go b/internal/protobuf/helpers.go index 904bc372b5a..90e77219315 100644 --- a/internal/protobuf/helpers.go +++ b/internal/protobuf/helpers.go @@ -18,6 +18,8 @@ import ( "reflect" "time" + "github.com/envoyproxy/go-control-plane/pkg/cache/types" + "github.com/golang/protobuf/proto" "github.com/golang/protobuf/ptypes" "github.com/golang/protobuf/ptypes/any" @@ -50,16 +52,16 @@ func Bool(val bool) *wrappers.BoolValue { // AsMessages casts the given slice of values (that implement the proto.Message // interface) to a slice of proto.Message. If the length of the slice is 0, it // returns nil. -func AsMessages(messages interface{}) []proto.Message { +func AsMessages(messages interface{}) []types.Resource { v := reflect.ValueOf(messages) if v.Len() == 0 { return nil } - protos := make([]proto.Message, v.Len()) + protos := make([]types.Resource, v.Len()) for i := range protos { - protos[i] = v.Index(i).Interface().(proto.Message) + protos[i] = v.Index(i).Interface().(types.Resource) } return protos