[1.17] fix: Handle errors in ggv2 plugins when resource is not in a watched namespace (#10242)

Co-authored-by: soloio-bulldozer[bot] <48420018+soloio-bulldozer[bot]@users.noreply.github.com>

Author: davidjumani

changelog/v1.17.15/fix-unwatched-resource-npe.yaml

@@ -0,0 +1,6 @@
+changelog:
+- type: FIX
+  issueLink: https://github.com/solo-io/solo-projects/issues/7082
+  resolvesIssue: false
+  description: Fixes a bug where gloo segfaults if resources are applied to a unwatched namespace.
+

projects/gateway2/translator/plugins/routeoptions/route_options_plugin.go

@@ -5,6 +5,8 @@ import (
 	"errors"
 	"fmt"
 
+	"github.com/hashicorp/go-multierror"
+	"github.com/rotisserie/eris"
 	"github.com/solo-io/gloo/projects/gloo/pkg/xds"
 	"github.com/solo-io/go-utils/contextutils"
 	"github.com/solo-io/solo-kit/pkg/api/v1/clients"
@@ -31,6 +33,8 @@ import (
 var (
 	_ plugins.RoutePlugin  = &plugin{}
 	_ plugins.StatusPlugin = &plugin{}
+
+	ReadingRouteOptionErrStr = "error reading RouteOption"
 )
 
 // holds the data structures needed to derive and report a classic GE status
@@ -133,9 +137,14 @@ func (p *plugin) ApplyStatusPlugin(ctx context.Context, statusCtx *plugins.Statu
 		}
 	}
 	routeOptionReport := make(reporter.ResourceReports)
+	var multierr *multierror.Error
 	for roKey, status := range p.legacyStatusCache {
 		// get the obj by namespacedName
-		roObj, _ := p.routeOptionClient.Read(roKey.Namespace, roKey.Name, clients.ReadOpts{Ctx: ctx})
+		roObj, err := p.routeOptionClient.Read(roKey.Namespace, roKey.Name, clients.ReadOpts{Ctx: ctx})
+		if err != nil {
+			multierr = multierror.Append(multierr, eris.Wrapf(err, "%s %s in namespace %s", ReadingRouteOptionErrStr, roKey.Name, roKey.Namespace))
+			continue
+		}
 
 		// mark this object to be processed
 		routeOptionReport.Accept(roObj)
@@ -146,12 +155,13 @@ func (p *plugin) ApplyStatusPlugin(ctx context.Context, statusCtx *plugins.Statu
 		}
 
 		// actually write out the reports!
-		err := p.statusReporter.WriteReports(ctx, routeOptionReport, status.subresourceStatus)
+		err = p.statusReporter.WriteReports(ctx, routeOptionReport, status.subresourceStatus)
 		if err != nil {
-			return fmt.Errorf("error writing status report from RouteOptionPlugin: %w", err)
+			multierr = multierror.Append(multierr, fmt.Errorf("error writing status report from RouteOptionPlugin: %w", err))
+			continue
 		}
 	}
-	return nil
+	return multierr.ErrorOrNil()
 }
 
 // tracks the attachment of a RouteOption so we know which RouteOptions to report status for

projects/gateway2/translator/plugins/routeoptions/route_options_plugin_test.go

@@ -42,6 +42,7 @@ var _ = Describe("RouteOptionsPlugin", func() {
 		cancel            context.CancelFunc
 		routeOptionClient sologatewayv1.RouteOptionClient
 		statusReporter    reporter.StatusReporter
+		statusCtx         *plugins.StatusContext
 	)
 
 	BeforeEach(func() {
@@ -259,6 +260,81 @@ var _ = Describe("RouteOptionsPlugin", func() {
 			})
 		})
 
+		Context("There is an error reading the RouteOptions", func() {
+			BeforeEach(func() {
+				statusCtx = &plugins.StatusContext{
+					ProxiesWithReports: []translatorutils.ProxyWithReports{
+						{
+							Proxy: &v1.Proxy{},
+							Reports: translatorutils.TranslationReports{
+								ProxyReport:     &validation.ProxyReport{},
+								ResourceReports: reporter.ResourceReports{},
+							},
+						},
+					},
+				}
+			})
+
+			When("The RouteOption has a TargetRef", func() {
+				It("errors out", func() {
+					deps := []client.Object{attachedRouteOption()}
+					fakeClient := testutils.BuildIndexedFakeClient(deps, gwquery.IterateIndices, rtoptquery.IterateIndices)
+					gwQueries := testutils.BuildGatewayQueriesWithClient(fakeClient)
+					plugin := NewPlugin(gwQueries, fakeClient, routeOptionClient, statusReporter)
+
+					ctx := context.Background()
+					routeCtx := &plugins.RouteContext{
+						Route: &gwv1.HTTPRoute{
+							ObjectMeta: metav1.ObjectMeta{
+								Name:      "route",
+								Namespace: "default",
+							},
+						},
+					}
+
+					outputRoute := &v1.Route{
+						Options: &v1.RouteOptions{},
+					}
+					plugin.ApplyRoutePlugin(ctx, routeCtx, outputRoute)
+
+					err := plugin.ApplyStatusPlugin(ctx, statusCtx)
+					Expect(err).To(MatchError(ContainSubstring(ReadingRouteOptionErrStr)))
+				})
+			})
+
+			When("The HTTPRoute has an ExtensionRef", func() {
+				It("errors out", func() {
+					deps := []client.Object{routeOption()}
+					fakeClient := testutils.BuildIndexedFakeClient(deps, gwquery.IterateIndices, rtoptquery.IterateIndices)
+					gwQueries := testutils.BuildGatewayQueriesWithClient(fakeClient)
+					plugin := NewPlugin(gwQueries, fakeClient, routeOptionClient, statusReporter)
+
+					rtCtx := &plugins.RouteContext{
+						Route: &gwv1.HTTPRoute{},
+						Rule: &gwv1.HTTPRouteRule{
+							Filters: []gwv1.HTTPRouteFilter{{
+								Type: gwv1.HTTPRouteFilterExtensionRef,
+								ExtensionRef: &gwv1.LocalObjectReference{
+									Group: gwv1.Group(sologatewayv1.RouteOptionGVK.Group),
+									Kind:  gwv1.Kind(sologatewayv1.RouteOptionGVK.Kind),
+									Name:  "filter-policy",
+								},
+							}},
+						},
+					}
+
+					outputRoute := &v1.Route{
+						Options: &v1.RouteOptions{},
+					}
+					plugin.ApplyRoutePlugin(context.Background(), rtCtx, outputRoute)
+
+					err := plugin.ApplyStatusPlugin(ctx, statusCtx)
+					Expect(err).To(MatchError(ContainSubstring(ReadingRouteOptionErrStr)))
+				})
+
+			})
+		})
+
 		When("Two RouteOptions are attached correctly with different creation timestamps", func() {
 			It("correctly adds faultinjection from the earliest created object", func() {
 				routeOptionClient.Write(attachedInternal(), clients.WriteOpts{})

projects/gateway2/translator/plugins/virtualhostoptions/virtualhost_options_plugin.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/hashicorp/go-multierror"
 	"github.com/rotisserie/eris"
 	"github.com/solo-io/go-utils/contextutils"
 	"github.com/solo-io/solo-kit/pkg/api/v1/clients"
@@ -30,6 +31,8 @@ import (
 var (
 	_ plugins.ListenerPlugin = &plugin{}
 	_ plugins.StatusPlugin   = &plugin{}
+
+	ReadingVirtualHostOptionErrStr = "error reading VirtualHostOption"
 )
 
 type plugin struct {
@@ -162,6 +165,7 @@ func optionsToStr(opts []*solokubev1.VirtualHostOption) string {
 
 // Add all statuses for processed VirtualHostOptions. These could come from the VHO itself or
 // or any VH to which it is attached.
+// It returns the aggregated errors reading the VirtualHostOptions if any.
 func (p *plugin) ApplyStatusPlugin(ctx context.Context, statusCtx *plugins.StatusContext) error {
 	logger := contextutils.LoggerFrom(ctx)
 	// gather all VirtualHostOptions we need to report status for
@@ -199,9 +203,14 @@ func (p *plugin) ApplyStatusPlugin(ctx context.Context, statusCtx *plugins.Statu
 	}
 	virtualHostOptionReport := make(reporter.ResourceReports)
 	// Loop through vhostopts we processed and have a status for
+	var multierr *multierror.Error
 	for vhOptKey, status := range p.classicStatusCache {
 		// get the obj by namespacedName
-		vhOptObj, _ := p.vhOptionClient.Read(vhOptKey.Namespace, vhOptKey.Name, clients.ReadOpts{Ctx: ctx})
+		vhOptObj, err := p.vhOptionClient.Read(vhOptKey.Namespace, vhOptKey.Name, clients.ReadOpts{Ctx: ctx})
+		if err != nil {
+			multierr = multierror.Append(multierr, eris.Wrapf(err, "%s %s in namespace %s", ReadingVirtualHostOptionErrStr, vhOptKey.Name, vhOptKey.Namespace))
+			continue
+		}
 
 		// mark this object to be processed
 		virtualHostOptionReport.Accept(vhOptObj)
@@ -214,13 +223,14 @@ func (p *plugin) ApplyStatusPlugin(ctx context.Context, statusCtx *plugins.Statu
 		virtualHostOptionReport.AddWarnings(vhOptObj, status.warnings...)
 
 		// actually write out the reports!
-		err := p.statusReporter.WriteReports(ctx, virtualHostOptionReport, status.subresourceStatus)
+		err = p.statusReporter.WriteReports(ctx, virtualHostOptionReport, status.subresourceStatus)
 		if err != nil {
-			return eris.Wrap(err, "writing status report from VirtualHostOptionPlugin")
+			multierr = multierror.Append(multierr, eris.Wrap(err, "writing status report from VirtualHostOptionPlugin"))
+			continue
 		}
 
 	}
-	return nil
+	return multierr.ErrorOrNil()
 }
 
 // given a ProxyReport, extract and aggregate all VirtualHost errors that have VirtualHostOption source metadata

projects/gateway2/translator/plugins/virtualhostoptions/virtualhost_options_plugin_test.go

@@ -17,7 +17,9 @@ import (
 	"github.com/solo-io/gloo/projects/gateway2/translator/plugins/utils"
 	vhoptquery "github.com/solo-io/gloo/projects/gateway2/translator/plugins/virtualhostoptions/query"
 	"github.com/solo-io/gloo/projects/gateway2/translator/testutils"
+	"github.com/solo-io/gloo/projects/gateway2/translator/translatorutils"
 	"github.com/solo-io/gloo/projects/gateway2/wellknown"
+	"github.com/solo-io/gloo/projects/gloo/pkg/api/grpc/validation"
 	v1 "github.com/solo-io/gloo/projects/gloo/pkg/api/v1"
 	"github.com/solo-io/gloo/projects/gloo/pkg/api/v1/options/headers"
 	"github.com/solo-io/gloo/projects/gloo/pkg/api/v1/options/retries"
@@ -194,6 +196,27 @@ var _ = Describe("VirtualHostOptions Plugin", func() {
 				}
 			})
 		})
+
+		When("There is an error reading the VirtualHostOptions", func() {
+			It("errors out", func() {
+				plugin.ApplyListenerPlugin(ctx, listenerCtx, outputListener)
+
+				statusCtx := &plugins.StatusContext{
+					ProxiesWithReports: []translatorutils.ProxyWithReports{
+						{
+							Proxy: &v1.Proxy{},
+							Reports: translatorutils.TranslationReports{
+								ProxyReport:     &validation.ProxyReport{},
+								ResourceReports: reporter.ResourceReports{},
+							},
+						},
+					},
+				}
+
+				err := plugin.ApplyStatusPlugin(ctx, statusCtx)
+				Expect(err).To(MatchError(ContainSubstring(ReadingVirtualHostOptionErrStr)))
+			})
+		})
 	})
 })