Merge pull request #3686 from rumstead/fix/crashback-virtual-service-no-gateway

fix: extdns crashes when virtual service points to nonexistent …

source/istio_virtualservice.go

@@ -23,12 +23,12 @@ import (
 	"strings"
 	"text/template"
 
-	networkingv1alpha3 "istio.io/client-go/pkg/apis/networking/v1alpha3"
-
 	log "github.com/sirupsen/logrus"
+	networkingv1alpha3 "istio.io/client-go/pkg/apis/networking/v1alpha3"
 	istioclient "istio.io/client-go/pkg/clientset/versioned"
 	istioinformers "istio.io/client-go/pkg/informers/externalversions"
 	networkingv1alpha3informer "istio.io/client-go/pkg/informers/externalversions/networking/v1alpha3"
+	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	kubeinformers "k8s.io/client-go/informers"
@@ -203,7 +203,10 @@ func (sc *virtualServiceSource) getGateway(ctx context.Context, gatewayStr strin
 	}
 
 	gateway, err := sc.istioClient.NetworkingV1alpha3().Gateways(namespace).Get(ctx, name, metav1.GetOptions{})
-	if err != nil {
+	if errors.IsNotFound(err) {
+		log.Warnf("VirtualService (%s/%s) references non-existent gateway: %s ", virtualService.Namespace, virtualService.Name, gatewayStr)
+		return nil, nil
+	} else if err != nil {
 		log.Errorf("Failed retrieving gateway %s referenced by VirtualService %s/%s: %v", gatewayStr, virtualService.Namespace, virtualService.Name, err)
 		return nil, err
 	}

source/istio_virtualservice_test.go

@@ -18,19 +18,23 @@ package source
 
 import (
 	"context"
+	"fmt"
 	"testing"
 
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-
 	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
+	"istio.io/api/meta/v1alpha1"
 	istionetworking "istio.io/api/networking/v1alpha3"
 	networkingv1alpha3 "istio.io/client-go/pkg/apis/networking/v1alpha3"
 	istiofake "istio.io/client-go/pkg/clientset/versioned/fake"
+	fakenetworking3 "istio.io/client-go/pkg/clientset/versioned/typed/networking/v1alpha3/fake"
 	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/kubernetes/fake"
+	k8sclienttesting "k8s.io/client-go/testing"
 
 	"sigs.k8s.io/external-dns/endpoint"
 )
@@ -1579,6 +1583,8 @@ func newTestVirtualServiceSource(loadBalancerList []fakeIngressGatewayService, g
 
 	for _, gw := range gwList {
 		gwObj := gw.Config()
+		// use create instead of add
+		// https://github.com/kubernetes/client-go/blob/92512ee2b8cf6696e9909245624175b7f0c971d9/testing/fixture.go#LL336C3-L336C52
 		_, err := fakeIstioClient.NetworkingV1alpha3().Gateways(gw.namespace).Create(context.Background(), &gwObj, metav1.CreateOptions{})
 		if err != nil {
 			return nil, err
@@ -1634,3 +1640,118 @@ func (c fakeVirtualServiceConfig) Config() *networkingv1alpha3.VirtualService {
 		Spec: vs,
 	}
 }
+
+func TestVirtualServiceSourceGetGateway(t *testing.T) {
+	type fields struct {
+		virtualServiceSource *virtualServiceSource
+	}
+	type args struct {
+		ctx            context.Context
+		gatewayStr     string
+		virtualService *networkingv1alpha3.VirtualService
+	}
+	tests := []struct {
+		name           string
+		fields         fields
+		args           args
+		want           *networkingv1alpha3.Gateway
+		expectedErrStr string
+	}{
+		{name: "EmptyGateway", fields: fields{
+			virtualServiceSource: func() *virtualServiceSource { vs, _ := newTestVirtualServiceSource(nil, nil); return vs }(),
+		}, args: args{
+			ctx:            context.TODO(),
+			gatewayStr:     "",
+			virtualService: nil,
+		}, want: nil, expectedErrStr: ""},
+		{name: "MeshGateway", fields: fields{
+			virtualServiceSource: func() *virtualServiceSource { vs, _ := newTestVirtualServiceSource(nil, nil); return vs }(),
+		}, args: args{
+			ctx:            context.TODO(),
+			gatewayStr:     IstioMeshGateway,
+			virtualService: nil,
+		}, want: nil, expectedErrStr: ""},
+		{name: "MissingGateway", fields: fields{
+			virtualServiceSource: func() *virtualServiceSource { vs, _ := newTestVirtualServiceSource(nil, nil); return vs }(),
+		}, args: args{
+			ctx:        context.TODO(),
+			gatewayStr: "doesnt/exist",
+			virtualService: &networkingv1alpha3.VirtualService{
+				TypeMeta:   metav1.TypeMeta{},
+				ObjectMeta: metav1.ObjectMeta{Name: "exist", Namespace: "doesnt"},
+				Spec:       istionetworking.VirtualService{},
+				Status:     v1alpha1.IstioStatus{},
+			},
+		}, want: nil, expectedErrStr: ""},
+		{name: "InvalidGatewayStr", fields: fields{
+			virtualServiceSource: func() *virtualServiceSource { vs, _ := newTestVirtualServiceSource(nil, nil); return vs }(),
+		}, args: args{
+			ctx:            context.TODO(),
+			gatewayStr:     "1/2/3/",
+			virtualService: &networkingv1alpha3.VirtualService{},
+		}, want: nil, expectedErrStr: "invalid gateway name (name or namespace/name) found '1/2/3/'"},
+		{name: "ExistingGateway", fields: fields{
+			virtualServiceSource: func() *virtualServiceSource {
+				vs, _ := newTestVirtualServiceSource(nil, []fakeGatewayConfig{{
+					namespace: "bar",
+					name:      "foo",
+				}})
+				return vs
+			}(),
+		}, args: args{
+			ctx:        context.TODO(),
+			gatewayStr: "bar/foo",
+			virtualService: &networkingv1alpha3.VirtualService{
+				TypeMeta:   metav1.TypeMeta{},
+				ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "bar"},
+				Spec:       istionetworking.VirtualService{},
+				Status:     v1alpha1.IstioStatus{},
+			},
+		}, want: &networkingv1alpha3.Gateway{
+			TypeMeta:   metav1.TypeMeta{},
+			ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "bar"},
+			Spec:       istionetworking.Gateway{},
+			Status:     v1alpha1.IstioStatus{},
+		}, expectedErrStr: ""},
+		{name: "ErrorGettingGateway", fields: fields{
+			virtualServiceSource: func() *virtualServiceSource {
+				istioFake := istiofake.NewSimpleClientset()
+				istioFake.NetworkingV1alpha3().(*fakenetworking3.FakeNetworkingV1alpha3).PrependReactor("get", "gateways", func(action k8sclienttesting.Action) (handled bool, ret runtime.Object, err error) {
+					return true, &networkingv1alpha3.Gateway{}, fmt.Errorf("error getting gateway")
+				})
+				vs, _ := NewIstioVirtualServiceSource(
+					context.TODO(),
+					fake.NewSimpleClientset(),
+					istioFake,
+					"",
+					"",
+					"{{.Name}}",
+					false,
+					false,
+				)
+				return vs.(*virtualServiceSource)
+			}(),
+		}, args: args{
+			ctx:        context.TODO(),
+			gatewayStr: "foo/bar",
+			virtualService: &networkingv1alpha3.VirtualService{
+				TypeMeta:   metav1.TypeMeta{},
+				ObjectMeta: metav1.ObjectMeta{Name: "gateway", Namespace: "error"},
+				Spec:       istionetworking.VirtualService{},
+				Status:     v1alpha1.IstioStatus{},
+			},
+		}, want: nil, expectedErrStr: "error getting gateway"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := tt.fields.virtualServiceSource.getGateway(tt.args.ctx, tt.args.gatewayStr, tt.args.virtualService)
+			if tt.expectedErrStr != "" {
+				assert.EqualError(t, err, tt.expectedErrStr, fmt.Sprintf("getGateway(%v, %v, %v)", tt.args.ctx, tt.args.gatewayStr, tt.args.virtualService))
+				return
+			} else {
+				require.NoError(t, err)
+			}
+			assert.Equalf(t, tt.want, got, "getGateway(%v, %v, %v)", tt.args.ctx, tt.args.gatewayStr, tt.args.virtualService)
+		})
+	}
+}