Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: improve extension server unit tests #3503

Merged
merged 3 commits into from
May 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions internal/extension/registry/extension_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@
"crypto/x509"
"errors"
"fmt"
"net"

"google.golang.org/grpc"
"google.golang.org/grpc/credentials"
"google.golang.org/grpc/credentials/insecure"
"google.golang.org/grpc/test/bufconn"
corev1 "k8s.io/api/core/v1"
k8scli "sigs.k8s.io/controller-runtime/pkg/client"
k8sclicfg "sigs.k8s.io/controller-runtime/pkg/client/config"
Expand Down Expand Up @@ -73,6 +75,37 @@
}, nil
}

func NewInMemoryManager(cfg v1alpha1.ExtensionManager, server extension.EnvoyGatewayExtensionServer) (extTypes.Manager, func(), error) {
if server == nil {
return nil, nil, fmt.Errorf("in-memory manager must be passed a server")

Check warning on line 80 in internal/extension/registry/extension_manager.go

View check run for this annotation

Codecov / codecov/patch

internal/extension/registry/extension_manager.go#L80

Added line #L80 was not covered by tests
}

buffer := 101024 * 1024
lis := bufconn.Listen(buffer)

baseServer := grpc.NewServer()
extension.RegisterEnvoyGatewayExtensionServer(baseServer, server)
go func() {
_ = baseServer.Serve(lis)
}()
conn, err := grpc.DialContext(context.Background(), "",
grpc.WithContextDialer(func(context.Context, string) (net.Conn, error) {
return lis.Dial()
}), grpc.WithTransportCredentials(insecure.NewCredentials()))
if err != nil {
return nil, nil, err

Check warning on line 96 in internal/extension/registry/extension_manager.go

View check run for this annotation

Codecov / codecov/patch

internal/extension/registry/extension_manager.go#L96

Added line #L96 was not covered by tests
}
c := func() {
lis.Close()
baseServer.Stop()
}

return &Manager{
extensionConnCache: conn,
extension: cfg,
}, c, nil
}

// HasExtension checks to see whether a given Group and Kind has an
// associated extension registered for it.
func (m *Manager) HasExtension(g v1.Group, k v1.Kind) bool {
Expand Down
70 changes: 0 additions & 70 deletions internal/extension/testutils/manager.go

This file was deleted.

18 changes: 14 additions & 4 deletions internal/gatewayapi/runner/runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ import (

egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
"github.com/envoyproxy/gateway/internal/envoygateway/config"
"github.com/envoyproxy/gateway/internal/extension/testutils"
"github.com/envoyproxy/gateway/internal/extension/registry"
"github.com/envoyproxy/gateway/internal/ir"
"github.com/envoyproxy/gateway/internal/message"
pb "github.com/envoyproxy/gateway/proto/extension"
)

func TestRunner(t *testing.T) {
Expand All @@ -31,12 +32,15 @@ func TestRunner(t *testing.T) {
infraIR := new(message.InfraIR)
cfg, err := config.New()
require.NoError(t, err)
extMgr, closeFunc, err := registry.NewInMemoryManager(egv1a1.ExtensionManager{}, &pb.UnimplementedEnvoyGatewayExtensionServer{})
require.NoError(t, err)
defer closeFunc()
r := New(&Config{
Server: *cfg,
ProviderResources: pResources,
XdsIR: xdsIR,
InfraIR: infraIR,
ExtensionManager: testutils.NewManager(egv1a1.ExtensionManager{}),
ExtensionManager: extMgr,
})
ctx := context.Background()
// Start
Expand Down Expand Up @@ -115,12 +119,15 @@ func TestDeleteStatusKeys(t *testing.T) {
infraIR := new(message.InfraIR)
cfg, err := config.New()
require.NoError(t, err)
extMgr, closeFunc, err := registry.NewInMemoryManager(egv1a1.ExtensionManager{}, &pb.UnimplementedEnvoyGatewayExtensionServer{})
require.NoError(t, err)
defer closeFunc()
r := New(&Config{
Server: *cfg,
ProviderResources: pResources,
XdsIR: xdsIR,
InfraIR: infraIR,
ExtensionManager: testutils.NewManager(egv1a1.ExtensionManager{}),
ExtensionManager: extMgr,
})
ctx := context.Background()

Expand Down Expand Up @@ -205,12 +212,15 @@ func TestDeleteAllStatusKeys(t *testing.T) {
infraIR := new(message.InfraIR)
cfg, err := config.New()
require.NoError(t, err)
extMgr, closeFunc, err := registry.NewInMemoryManager(egv1a1.ExtensionManager{}, &pb.UnimplementedEnvoyGatewayExtensionServer{})
require.NoError(t, err)
defer closeFunc()
r := New(&Config{
Server: *cfg,
ProviderResources: pResources,
XdsIR: xdsIR,
InfraIR: infraIR,
ExtensionManager: testutils.NewManager(egv1a1.ExtensionManager{}),
ExtensionManager: extMgr,
})
ctx := context.Background()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@
// The full text of the Apache license is available in the LICENSE file at
// the root of the repo.

package testutils
package translator

import (
"context"
"errors"
"fmt"
"strings"
Expand All @@ -16,24 +17,36 @@ import (
listenerV3 "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3"
routeV3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3"
tlsV3 "github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/tls/v3"
"github.com/golang/protobuf/proto"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/types/known/durationpb"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"

pb "github.com/envoyproxy/gateway/proto/extension"
)

type XDSHookClient struct{}
type testingExtensionServer struct {
pb.UnimplementedEnvoyGatewayExtensionServer
}

// PostRouteModifyHook returns a modified version of the route using context info and the passed in extensionResources
func (c *XDSHookClient) PostRouteModifyHook(route *routeV3.Route, routeHostnames []string, extensionResources []*unstructured.Unstructured) (*routeV3.Route, error) {
func (t *testingExtensionServer) PostRouteModify(_ context.Context, req *pb.PostRouteModifyRequest) (*pb.PostRouteModifyResponse, error) {
// Simulate an error an extension may return
if route.Name == "extension-post-xdsroute-hook-error" {
return nil, errors.New("route hook resource error")
if req.Route.Name == "extension-post-xdsroute-hook-error" {
return &pb.PostRouteModifyResponse{
Route: req.Route,
}, errors.New("route hook resource error")
}

// Setup a new route to avoid operating directly on the passed in pointer for better test coverage that the
// route we are returning gets used properly
modifiedRoute := proto.Clone(route).(*routeV3.Route)
for _, extensionResource := range extensionResources {
modifiedRoute := proto.Clone(req.Route).(*routeV3.Route)
for _, extensionResourceBytes := range req.PostRouteContext.ExtensionResources {
extensionResource := unstructured.Unstructured{}
if err := extensionResource.UnmarshalJSON(extensionResourceBytes.UnstructuredBytes); err != nil {
return &pb.PostRouteModifyResponse{
Route: req.Route,
}, err
}
modifiedRoute.ResponseHeadersToAdd = append(modifiedRoute.ResponseHeadersToAdd,
&coreV3.HeaderValueOption{
Header: &coreV3.HeaderValue{
Expand All @@ -44,7 +57,7 @@ func (c *XDSHookClient) PostRouteModifyHook(route *routeV3.Route, routeHostnames
&coreV3.HeaderValueOption{
Header: &coreV3.HeaderValue{
Key: "mock-extension-was-here-route-hostnames",
Value: strings.Join(routeHostnames, ", "),
Value: strings.Join(req.PostRouteContext.Hostnames, ", "),
},
},
&coreV3.HeaderValueOption{
Expand Down Expand Up @@ -73,19 +86,23 @@ func (c *XDSHookClient) PostRouteModifyHook(route *routeV3.Route, routeHostnames
},
)
}
return modifiedRoute, nil
return &pb.PostRouteModifyResponse{
Route: modifiedRoute,
}, nil
}

// PostVirtualHostModifyHook returns a modified version of the virtualhost with a new route injected
func (c *XDSHookClient) PostVirtualHostModifyHook(vh *routeV3.VirtualHost) (*routeV3.VirtualHost, error) {
func (t *testingExtensionServer) PostVirtualHostModify(_ context.Context, req *pb.PostVirtualHostModifyRequest) (*pb.PostVirtualHostModifyResponse, error) {
// Only make the change when the VirtualHost's name matches the expected testdata
// This prevents us from having to update every single testfile.out
if vh.Name == "extension-post-xdsvirtualhost-hook-error/*" {
return nil, fmt.Errorf("extension post xds virtual host hook error")
} else if vh.Name == "extension-listener" {
if req.VirtualHost.Name == "extension-post-xdsvirtualhost-hook-error/*" {
return &pb.PostVirtualHostModifyResponse{
VirtualHost: req.VirtualHost,
}, fmt.Errorf("extension post xds virtual host hook error")
} else if req.VirtualHost.Name == "extension-listener" {
// Setup a new VirtualHost to avoid operating directly on the passed in pointer for better test coverage that the
// VirtualHost we are returning gets used properly
modifiedVH := proto.Clone(vh).(*routeV3.VirtualHost)
modifiedVH := proto.Clone(req.VirtualHost).(*routeV3.VirtualHost)
modifiedVH.Routes = append(modifiedVH.Routes, &routeV3.Route{
Name: "mock-extension-inserted-route",
Action: &routeV3.Route_DirectResponse{
Expand All @@ -94,31 +111,41 @@ func (c *XDSHookClient) PostVirtualHostModifyHook(vh *routeV3.VirtualHost) (*rou
},
},
})
return modifiedVH, nil
return &pb.PostVirtualHostModifyResponse{
VirtualHost: modifiedVH,
}, nil
}
return vh, nil
return &pb.PostVirtualHostModifyResponse{
VirtualHost: req.VirtualHost,
}, nil
}

// PostHTTPListenerModifyHook returns a modified version of the listener with a changed statprefix of the listener
// A more useful use-case for an extension would be looping through the FilterChains to find the
// HTTPConnectionManager(s) and inject a custom HTTPFilter, but that for testing purposes we don't need to make a complex change
func (c *XDSHookClient) PostHTTPListenerModifyHook(l *listenerV3.Listener) (*listenerV3.Listener, error) {
func (t *testingExtensionServer) PostHTTPListenerModify(_ context.Context, req *pb.PostHTTPListenerModifyRequest) (*pb.PostHTTPListenerModifyResponse, error) {
// Only make the change when the listener's name matches the expected testdata
// This prevents us from having to update every single testfile.out
if l.Name == "extension-post-xdslistener-hook-error" {
return nil, fmt.Errorf("extension post xds listener hook error")
} else if l.Name == "extension-listener" {
if req.Listener.Name == "extension-post-xdslistener-hook-error" {
return &pb.PostHTTPListenerModifyResponse{
Listener: req.Listener,
}, fmt.Errorf("extension post xds listener hook error")
} else if req.Listener.Name == "extension-listener" {
// Setup a new Listener to avoid operating directly on the passed in pointer for better test coverage that the
// Listener we are returning gets used properly
modifiedListener := proto.Clone(l).(*listenerV3.Listener)
modifiedListener := proto.Clone(req.Listener).(*listenerV3.Listener)
modifiedListener.StatPrefix = "mock-extension-inserted-prefix"
return modifiedListener, nil
return &pb.PostHTTPListenerModifyResponse{
Listener: modifiedListener,
}, nil
}
return l, nil
return &pb.PostHTTPListenerModifyResponse{
Listener: req.Listener,
}, nil
}

// PostTranslateModifyHook inserts and overrides some clusters/secrets
func (c *XDSHookClient) PostTranslateModifyHook(clusters []*clusterV3.Cluster, secrets []*tlsV3.Secret) ([]*clusterV3.Cluster, []*tlsV3.Secret, error) {
func (t *testingExtensionServer) PostTranslateModify(_ context.Context, req *pb.PostTranslateModifyRequest) (*pb.PostTranslateModifyResponse, error) {
extensionSvcEndpoint := &endpointV3.LbEndpoint_Endpoint{
Endpoint: &endpointV3.Endpoint{
Address: &coreV3.Address{
Expand All @@ -135,13 +162,18 @@ func (c *XDSHookClient) PostTranslateModifyHook(clusters []*clusterV3.Cluster, s
},
}

for idx, cluster := range clusters {
response := &pb.PostTranslateModifyResponse{
Clusters: make([]*clusterV3.Cluster, len(req.Clusters)),
Secrets: make([]*tlsV3.Secret, len(req.Secrets)),
}
for idx, cluster := range req.Clusters {
response.Clusters[idx] = proto.Clone(cluster).(*clusterV3.Cluster)
if cluster.Name == "first-route" {
clusters[idx].ConnectTimeout = &durationpb.Duration{Seconds: 30}
response.Clusters[idx].ConnectTimeout = &durationpb.Duration{Seconds: 30}
}
}

clusters = append(clusters, &clusterV3.Cluster{
response.Clusters = append(response.Clusters, &clusterV3.Cluster{
Name: "mock-extension-injected-cluster",
LoadAssignment: &endpointV3.ClusterLoadAssignment{
ClusterName: "mock-extension-injected-cluster",
Expand All @@ -157,7 +189,10 @@ func (c *XDSHookClient) PostTranslateModifyHook(clusters []*clusterV3.Cluster, s
},
})

secrets = append(secrets, &tlsV3.Secret{
for idx, secret := range req.Secrets {
response.Secrets[idx] = proto.Clone(secret).(*tlsV3.Secret)
}
response.Secrets = append(response.Secrets, &tlsV3.Secret{
Name: "mock-extension-injected-secret",
Type: &tlsV3.Secret_GenericSecret{
GenericSecret: &tlsV3.GenericSecret{
Expand All @@ -170,5 +205,5 @@ func (c *XDSHookClient) PostTranslateModifyHook(clusters []*clusterV3.Cluster, s
},
})

return clusters, secrets, nil
return response, nil
}
Loading