extension: fix pointer error

previously I was under the false impression that despite the linter
warning about an innefectual assignment that things were working as
intended. This was because the test cases directly operated on passed in
pointers instead of ensuring that the returned pointer was being used.

Signed-off-by: AliceProxy <[email protected]>

internal/extension/testutils/hooks.go

@@ -8,6 +8,7 @@ package testutils
 import (
 	"errors"
 	"fmt"
+	"reflect"
 	"strings"
 
 	clusterV3 "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3"
@@ -24,16 +25,21 @@ type XDSHookClient struct{}
 
 // 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) {
+	// Simulate an error an extension may return
+	if route.Name == "extension-post-xdsroute-hook-error" {
+		return nil, 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
+	var modifiedRoute *routeV3.Route
+	copyPtr(route, modifiedRoute)
 	for _, extensionResource := range extensionResources {
-		// Simulate an error an extension may return
-		if route.Name == "extension-post-xdsroute-hook-error" {
-			return nil, errors.New("route hook resource error")
-		}
-		route.ResponseHeadersToAdd = append(route.ResponseHeadersToAdd,
+		modifiedRoute.ResponseHeadersToAdd = append(modifiedRoute.ResponseHeadersToAdd,
 			&coreV3.HeaderValueOption{
 				Header: &coreV3.HeaderValue{
 					Key:   "mock-extension-was-here-route-name",
-					Value: route.Name,
+					Value: modifiedRoute.Name,
 				},
 			},
 			&coreV3.HeaderValueOption{
@@ -68,7 +74,7 @@ func (c *XDSHookClient) PostRouteModifyHook(route *routeV3.Route, routeHostnames
 			},
 		)
 	}
-	return route, nil
+	return modifiedRoute, nil
 }
 
 // PostVirtualHostModifyHook returns a modified version of the virtualhost with a new route injected
@@ -78,14 +84,19 @@ func (c *XDSHookClient) PostVirtualHostModifyHook(vh *routeV3.VirtualHost) (*rou
 	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" {
-		vh.Routes = append(vh.Routes, &routeV3.Route{
+		// 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
+		var modifiedVH *routeV3.VirtualHost
+		copyPtr(vh, modifiedVH)
+		modifiedVH.Routes = append(modifiedVH.Routes, &routeV3.Route{
 			Name: "mock-extension-inserted-route",
 			Action: &routeV3.Route_DirectResponse{
 				DirectResponse: &routeV3.DirectResponseAction{
 					Status: uint32(200),
 				},
 			},
 		})
+		return modifiedVH, nil
 	}
 	return vh, nil
 }
@@ -100,7 +111,12 @@ func (c *XDSHookClient) PostHTTPListenerModifyHook(l *listenerV3.Listener) (*lis
 	if l.Name == "extension-post-xdslistener-hook-error" {
 		return nil, fmt.Errorf("extension post xds listener hook error")
 	} else if l.Name == "extension-listener" {
-		l.StatPrefix = "mock-extension-inserted-prefix"
+		// 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
+		var modifiedListener *listenerV3.Listener
+		copyPtr(l, modifiedListener)
+		modifiedListener.StatPrefix = "mock-extension-inserted-prefix"
+		return modifiedListener, nil
 	}
 	return l, nil
 }
@@ -161,3 +177,14 @@ func (c *XDSHookClient) PostTranslateModifyHook(clusters []*clusterV3.Cluster, s
 
 	return clusters, secrets, nil
 }
+
+func copyPtr(src interface{}, dest interface{}) {
+	srcVal := reflect.ValueOf(src)
+	if srcVal.Kind() == reflect.Ptr {
+		srcElem := srcVal.Elem()
+		destVal := reflect.New(srcElem.Type())
+		destElem := destVal.Elem()
+		destElem.Set(srcElem)
+		reflect.ValueOf(dest).Elem().Set(destVal.Elem())
+	}
+}

internal/xds/translator/extension.go

@@ -9,6 +9,8 @@
 package translator
 
 import (
+	"reflect"
+
 	clusterv3 "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3"
 	listenerv3 "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3"
 	routev3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3"
@@ -53,9 +55,7 @@ func processExtensionPostRouteHook(route *routev3.Route, vHost *routev3.VirtualH
 
 	// If the extension returned a modified Route, then copy its to the one that was passed in as a reference
 	if modifiedRoute != nil {
-		// Overwrite the pointer for the original route.
-		// Uses nolint because of the ineffectual assignment check
-		route = modifiedRoute //nolint
+		copyPtr(modifiedRoute, route)
 	}
 	return nil
 }
@@ -81,9 +81,7 @@ func processExtensionPostVHostHook(vHost *routev3.VirtualHost, em *extensionType
 
 	// If the extension returned a modified Virtual Host, then copy its to the one that was passed in as a reference
 	if modifiedVH != nil {
-		// Overwrite the pointer for the original virtual host.
-		// Uses nolint because of the ineffectual assignment check
-		vHost = modifiedVH //nolint
+		copyPtr(modifiedVH, vHost)
 	}
 
 	return nil
@@ -168,3 +166,14 @@ func processExtensionPostTranslationHook(tCtx *types.ResourceVersionTable, em *e
 
 	return nil
 }
+
+func copyPtr(src interface{}, dest interface{}) {
+	srcVal := reflect.ValueOf(src)
+	if srcVal.Kind() == reflect.Ptr {
+		srcElem := srcVal.Elem()
+		destVal := reflect.New(srcElem.Type())
+		destElem := destVal.Elem()
+		destElem.Set(srcElem)
+		reflect.ValueOf(dest).Elem().Set(destVal.Elem())
+	}
+}