diff --git a/source/istio_gateway.go b/source/istio_gateway.go index 99eb084d02..dda1dbcc4d 100644 --- a/source/istio_gateway.go +++ b/source/istio_gateway.go @@ -41,10 +41,11 @@ import ( "sigs.k8s.io/external-dns/source/template" ) -// IstioGatewayIngressSource is the annotation used to determine if the gateway is implemented by an Ingress object -// instead of a standard LoadBalancer service type -// Using var instead of const because annotation keys can be customized -var IstioGatewayIngressSource = annotations.Ingress +// IstioGatewayIngressSource returns the annotation key used to determine if the gateway +// is implemented by an Ingress object instead of a standard LoadBalancer service type. +// This must be a function (not a package-level var) because the annotation prefix can +// be customized at runtime via --annotation-prefix / SetAnnotationPrefix. +func IstioGatewayIngressSource() string { return annotations.Ingress } // gatewaySource is an implementation of Source for Istio Gateway objects. // The gateway implementation uses the spec.servers.hosts values for the hostnames. @@ -152,7 +153,8 @@ func (sc *gatewaySource) Endpoints(_ context.Context) ([]*endpoint.Endpoint, err gwEndpoints, err := sc.endpointsFromGateway(gwHostnames, gateway) if err != nil { - return nil, err + log.Warnf("Could not generate endpoints for gateway '%s/%s': %v", gateway.Namespace, gateway.Name, err) + continue } // apply template if host is missing on gateway @@ -167,7 +169,8 @@ func (sc *gatewaySource) Endpoints(_ context.Context) ([]*endpoint.Endpoint, err }, ) if err != nil { - return nil, err + log.Warnf("Could not apply template for gateway '%s/%s': %v", gateway.Namespace, gateway.Name, err) + continue } if endpoint.HasNoEmptyEndpoints(gwEndpoints, types.IstioGateway, gateway) { @@ -220,7 +223,7 @@ func (sc *gatewaySource) targetsFromGateway(gateway *networkingv1.Gateway) (endp return targets, nil } - ingressStr, ok := gateway.Annotations[IstioGatewayIngressSource] + ingressStr, ok := gateway.Annotations[IstioGatewayIngressSource()] if ok && ingressStr != "" { return sc.targetsFromIngress(ingressStr, gateway) } diff --git a/source/istio_gateway_test.go b/source/istio_gateway_test.go index d4f99dd5ee..65c603ce43 100644 --- a/source/istio_gateway_test.go +++ b/source/istio_gateway_test.go @@ -46,6 +46,27 @@ import ( // This is a compile-time validation that gatewaySource is a Source. var _ Source = &gatewaySource{} +// TestAnnotationPrefixPropagation verifies that IstioGatewayIngressSource() reflects +// changes made by SetAnnotationPrefix at runtime. This is a regression test for a bug +// where it was a package-level var evaluated at import time — before SetAnnotationPrefix +// was called — causing annotation lookups to use the wrong prefix and silently produce +// no endpoints, leading to record deletion under sync policy. +func TestAnnotationPrefixPropagation(t *testing.T) { + t.Cleanup(func() { annotations.SetAnnotationPrefix(annotations.DefaultAnnotationPrefix) }) + + assert.Equal(t, annotations.DefaultAnnotationPrefix+"ingress", IstioGatewayIngressSource()) + + // Simulate --annotation-prefix=custom.io/ + annotations.SetAnnotationPrefix("custom.io/") + assert.Equal(t, "custom.io/ingress", IstioGatewayIngressSource(), + "IstioGatewayIngressSource() must reflect updated annotation prefix") + + // Simulate changing it again — must not be cached from first call + annotations.SetAnnotationPrefix("another.io/") + assert.Equal(t, "another.io/ingress", IstioGatewayIngressSource(), + "IstioGatewayIngressSource() must track subsequent prefix changes") +} + type GatewaySuite struct { suite.Suite source Source @@ -206,7 +227,7 @@ func testEndpointsFromGatewayConfig(t *testing.T) { }, config: fakeGatewayConfig{ annotations: map[string]string{ - IstioGatewayIngressSource: "ingress1", + IstioGatewayIngressSource(): "ingress1", }, dnsnames: [][]string{ {"foo.bar"}, @@ -258,7 +279,7 @@ func testEndpointsFromGatewayConfig(t *testing.T) { }, config: fakeGatewayConfig{ annotations: map[string]string{ - IstioGatewayIngressSource: "ingress1", + IstioGatewayIngressSource(): "ingress1", }, dnsnames: [][]string{ {"foo.bar"}, @@ -319,7 +340,7 @@ func testEndpointsFromGatewayConfig(t *testing.T) { }, config: fakeGatewayConfig{ annotations: map[string]string{ - IstioGatewayIngressSource: "ingress1", + IstioGatewayIngressSource(): "ingress1", }, dnsnames: [][]string{ {""}, @@ -380,7 +401,7 @@ func testEndpointsFromGatewayConfig(t *testing.T) { }, config: fakeGatewayConfig{ annotations: map[string]string{ - IstioGatewayIngressSource: "istio-other2/ingress1", + IstioGatewayIngressSource(): "istio-other2/ingress1", }, dnsnames: [][]string{ {"foo.bar"}, // Kubernetes requires removal of trailing dot @@ -634,7 +655,7 @@ func testGatewayEndpoints(t *testing.T) { namespace: "testing1", dnsnames: [][]string{{"example.org"}}, annotations: map[string]string{ - IstioGatewayIngressSource: "testing2/ingress1", + IstioGatewayIngressSource(): "testing2/ingress1", }, }, }, @@ -1000,7 +1021,7 @@ func testGatewayEndpoints(t *testing.T) { name: "fake3", namespace: "", annotations: map[string]string{ - IstioGatewayIngressSource: "not-real/ingress1", + IstioGatewayIngressSource(): "not-real/ingress1", annotations.TargetKey: "1.2.3.4", }, dnsnames: [][]string{{"example3.org"}}, @@ -1142,7 +1163,7 @@ func testGatewayEndpoints(t *testing.T) { name: "fake1", namespace: "", annotations: map[string]string{ - IstioGatewayIngressSource: "ingress1", + IstioGatewayIngressSource(): "ingress1", annotations.HostnameKey: "dns-through-hostname.com", annotations.TargetKey: "gateway-target.com", }, @@ -1317,7 +1338,7 @@ func testGatewayEndpoints(t *testing.T) { name: "fake1", namespace: "", annotations: map[string]string{ - IstioGatewayIngressSource: "", + IstioGatewayIngressSource(): "", }, dnsnames: [][]string{}, }, @@ -1451,13 +1472,13 @@ func testGatewayEndpoints(t *testing.T) { name: "fake1", namespace: "", annotations: map[string]string{ - IstioGatewayIngressSource: "ingress2", + IstioGatewayIngressSource(): "ingress2", }, dnsnames: [][]string{{"new.org"}}, }, }, expected: []*endpoint.Endpoint{}, - expectError: true, + expectError: false, // bad annotation should be skipped gracefully, not crash the controller }, } { @@ -1775,7 +1796,7 @@ func TestSingleGatewayMultipleServicesPointingToSameLoadBalancer(t *testing.T) { { Hosts: []string{"example.org"}, Tls: &istionetworking.ServerTLSSettings{ - ServerCertificate: IstioGatewayIngressSource, + ServerCertificate: IstioGatewayIngressSource(), Mode: istionetworking.ServerTLSSettings_SIMPLE, }, }, diff --git a/source/istio_virtualservice.go b/source/istio_virtualservice.go index 1128e7aa6d..01a8c9b352 100644 --- a/source/istio_virtualservice.go +++ b/source/istio_virtualservice.go @@ -157,7 +157,8 @@ func (sc *virtualServiceSource) Endpoints(ctx context.Context) ([]*endpoint.Endp gwEndpoints, err := sc.endpointsFromVirtualService(ctx, vService) if err != nil { - return nil, err + log.Warnf("Could not generate endpoints for VirtualService '%s/%s': %v", vService.Namespace, vService.Name, err) + continue } // apply template if host is missing on VirtualService @@ -166,7 +167,8 @@ func (sc *virtualServiceSource) Endpoints(ctx context.Context) ([]*endpoint.Endp func() ([]*endpoint.Endpoint, error) { return sc.endpointsFromTemplate(ctx, vService) }, ) if err != nil { - return nil, err + log.Warnf("Could not apply template for VirtualService '%s/%s': %v", vService.Namespace, vService.Name, err) + continue } if endpoint.HasNoEmptyEndpoints(gwEndpoints, types.IstioVirtualService, vService) { @@ -403,7 +405,7 @@ func (sc *virtualServiceSource) targetsFromGateway(gateway *networkingv1.Gateway return targets, nil } - ingressStr, ok := gateway.Annotations[IstioGatewayIngressSource] + ingressStr, ok := gateway.Annotations[IstioGatewayIngressSource()] if ok && ingressStr != "" { return sc.targetsFromIngress(ingressStr, gateway) } diff --git a/source/istio_virtualservice_test.go b/source/istio_virtualservice_test.go index fb540c7ad7..d9a555adae 100644 --- a/source/istio_virtualservice_test.go +++ b/source/istio_virtualservice_test.go @@ -583,7 +583,7 @@ func testEndpointsFromVirtualServiceConfig(t *testing.T) { name: "mygw", dnsnames: [][]string{{"*"}}, annotations: map[string]string{ - IstioGatewayIngressSource: "ingress1", + IstioGatewayIngressSource(): "ingress1", }, }, vsconfig: fakeVirtualServiceConfig{ @@ -616,7 +616,7 @@ func testEndpointsFromVirtualServiceConfig(t *testing.T) { name: "mygw", dnsnames: [][]string{{"*"}}, annotations: map[string]string{ - IstioGatewayIngressSource: "ingress/ingress2", + IstioGatewayIngressSource(): "ingress/ingress2", }, }, vsconfig: fakeVirtualServiceConfig{ @@ -790,7 +790,7 @@ func testVirtualServiceEndpoints(t *testing.T) { namespace: namespace, dnsnames: [][]string{{"example.org"}}, annotations: map[string]string{ - IstioGatewayIngressSource: "ingress1", + IstioGatewayIngressSource(): "ingress1", }, }, { @@ -798,7 +798,7 @@ func testVirtualServiceEndpoints(t *testing.T) { namespace: namespace, dnsnames: [][]string{{"new.org"}}, annotations: map[string]string{ - IstioGatewayIngressSource: "ingress1", + IstioGatewayIngressSource(): "ingress1", }, }, }, @@ -1063,7 +1063,7 @@ func testVirtualServiceEndpoints(t *testing.T) { namespace: "testing1", dnsnames: [][]string{{"*"}}, annotations: map[string]string{ - IstioGatewayIngressSource: "ingress1", + IstioGatewayIngressSource(): "ingress1", }, }, }, @@ -1297,7 +1297,7 @@ func testVirtualServiceEndpoints(t *testing.T) { namespace: namespace, dnsnames: [][]string{{"*"}}, annotations: map[string]string{ - IstioGatewayIngressSource: "ingress2", + IstioGatewayIngressSource(): "ingress2", }, }, }, @@ -1310,7 +1310,7 @@ func testVirtualServiceEndpoints(t *testing.T) { }, }, expected: []*endpoint.Endpoint{}, - expectError: true, + expectError: false, // bad annotation should be skipped gracefully, not crash the controller }, { title: "our controller type is dns-controller", @@ -1603,7 +1603,7 @@ func testVirtualServiceEndpoints(t *testing.T) { dnsnames: [][]string{{"*"}}, annotations: map[string]string{ annotations.TargetKey: "gateway-target.com", - IstioGatewayIngressSource: "ingress1", + IstioGatewayIngressSource(): "ingress1", }, }, }, @@ -1957,7 +1957,7 @@ func testVirtualServiceEndpoints(t *testing.T) { "app": "igw4", }, annotations: map[string]string{ - IstioGatewayIngressSource: "testing1/ingress1", + IstioGatewayIngressSource(): "testing1/ingress1", }, }, },