Merge pull request #2400 from miheer/add-router-field-for-ocpRouteSource

Fix CNAME record when multiple router canonical name are defined in Route Status.

docs/tutorials/openshift.md

@@ -2,6 +2,60 @@
 This tutorial describes how to configure ExternalDNS to use the OpenShift Route source.
 It is meant to supplement the other provider-specific setup tutorials.
 
+### For OCP 4.x
+
+In OCP 4.x, if you have multiple ingress controllers then you must specify an ingress controller name or a router name(you can get it from the route's Status.Ingress.RouterName field).
+If you don't specify an ingress controller's or router name when you have multiple ingresscontrollers in your environment then the route gets populated with multiple entries of router canonical hostnames which causes external dns to create a CNAME record with multiple router canonical hostnames pointing to the route host which is a violation of RFC 1912 and is not allowed by Cloud Providers which leads to failure of record creation.
+Once you specify the ingresscontroller or router name then that will be matched by the external-dns and the router canonical hostname corresponding to this routerName(which is present in route's Status.Ingress.RouterName field) is selected and a CNAME record of this route host pointing to this router canonical hostname is created.
+
+Your externaldns CR shall be created as per the following example.
+Replace names in the domain section and zone ID as per your environment.
+This is example is for AWS environment.
+
+```yaml
+
+  apiVersion: externaldns.olm.openshift.io/v1alpha1
+  kind: ExternalDNS
+  metadata:
+    name: sample1
+  spec:
+    domains:
+      - filterType: Include
+        matchType: Exact
+        names: apps.miheer.externaldns
+    provider:
+      type: AWS
+    source:
+      hostnameAnnotation: Allow
+      openshiftRouteOptions:
+        routerName: default
+      type: OpenShiftRoute
+    zones:
+      - Z05387772BD5723IZFRX3
+
+```
+
+This will create an externaldns pod with the following container args under spec in the external-dns namespace where `- --source=openshift-route` and `- --openshift-router-name=default` is added by the external-dns-operator.
+
+```
+spec:
+  containers:
+  - args:
+    - --domain-filter=apps.misalunk.externaldns
+    - --metrics-address=127.0.0.1:7979
+    - --txt-owner-id=external-dns-sample1
+    - --provider=aws
+    - --source=openshift-route
+    - --policy=sync
+    - --registry=txt
+    - --log-level=debug
+    - --zone-id-filter=Z05387772BD5723IZFRX3
+    - --openshift-router-name=default
+    - --txt-prefix=external-dns-
+
+```
+
+### For OCP 3.11 environment
 ### Prepare ROUTER_CANONICAL_HOSTNAME in default/router deployment
 Read and go through [Finding the Host Name of the Router](https://docs.openshift.com/container-platform/3.11/install_config/router/default_haproxy_router.html#finding-router-hostname).
 If no ROUTER_CANONICAL_HOSTNAME is set, you must annotate each route with external-dns.alpha.kubernetes.io/target!

main.go

@@ -131,6 +131,7 @@ func main() {
 		SkipperRouteGroupVersion:       cfg.SkipperRouteGroupVersion,
 		RequestTimeout:                 cfg.RequestTimeout,
 		DefaultTargets:                 cfg.DefaultTargets,
+		OCPRouterName:                  cfg.OCPRouterName,
 	}
 
 	// Lookup all the selected sources by names and pass them the desired configuration.

pkg/apis/externaldns/types.go

@@ -175,6 +175,7 @@ type Config struct {
 	GoDaddySecretKey                  string `secure:"yes"`
 	GoDaddyTTL                        int64
 	GoDaddyOTE                        bool
+	OCPRouterName                     string
 }
 
 var defaultConfig = &Config{
@@ -361,8 +362,9 @@ func (cfg *Config) ParseFlags(args []string) error {
 	// Flags related to Skipper RouteGroup
 	app.Flag("skipper-routegroup-groupversion", "The resource version for skipper routegroup").Default(source.DefaultRoutegroupVersion).StringVar(&cfg.SkipperRouteGroupVersion)
 
-	// Flags related to processing sources
+	// Flags related to processing source
 	app.Flag("source", "The resource types that are queried for endpoints; specify multiple times for multiple sources (required, options: service, ingress, node, fake, connector, istio-gateway, istio-virtualservice, cloudfoundry, contour-ingressroute, contour-httpproxy, gloo-proxy, crd, empty, skipper-routegroup, openshift-route, ambassador-host, kong-tcpingress)").Required().PlaceHolder("source").EnumsVar(&cfg.Sources, "service", "ingress", "node", "pod", "istio-gateway", "istio-virtualservice", "cloudfoundry", "contour-ingressroute", "contour-httpproxy", "gloo-proxy", "fake", "connector", "crd", "empty", "skipper-routegroup", "openshift-route", "ambassador-host", "kong-tcpingress")
+	app.Flag("openshift-router-name", "if source is openshift-route then you can pass the ingress controller name. Based on this name external-dns will select the respective router from the route status and map that routerCanonicalHostname to the route host while creating a CNAME record.").StringVar(&cfg.OCPRouterName)
 	app.Flag("namespace", "Limit sources of endpoints to a specific namespace (default: all namespaces)").Default(defaultConfig.Namespace).StringVar(&cfg.Namespace)
 	app.Flag("annotation-filter", "Filter sources managed by external-dns via annotation using label selector semantics (default: all sources)").Default(defaultConfig.AnnotationFilter).StringVar(&cfg.AnnotationFilter)
 	app.Flag("label-filter", "Filter sources managed by external-dns via label selector when listing all resources; currently supported by source types CRD, ingress, service and openshift-route").Default(defaultConfig.LabelFilter).StringVar(&cfg.LabelFilter)

pkg/apis/externaldns/types_test.go

@@ -115,6 +115,7 @@ var (
 		DigitalOceanAPIPageSize:     50,
 		ManagedDNSRecordTypes:       []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME},
 		RFC2136BatchChangeSize:      50,
+		OCPRouterName:               "default",
 	}
 
 	overriddenConfig = &Config{
@@ -225,6 +226,7 @@ func TestParseFlags(t *testing.T) {
 			args: []string{
 				"--source=service",
 				"--provider=google",
+				"--openshift-router-name=default",
 			},
 			envVars:  map[string]string{},
 			expected: minimalConfig,

source/openshift_route.go

@@ -49,6 +49,7 @@ type ocpRouteSource struct {
 	ignoreHostnameAnnotation bool
 	routeInformer            routeInformer.RouteInformer
 	labelSelector            labels.Selector
+	ocpRouterName            string
 }
 
 // NewOcpRouteSource creates a new ocpRouteSource with the given config.
@@ -60,6 +61,7 @@ func NewOcpRouteSource(
 	combineFQDNAnnotation bool,
 	ignoreHostnameAnnotation bool,
 	labelSelector labels.Selector,
+	ocpRouterName string,
 ) (Source, error) {
 	tmpl, err := parseTemplate(fqdnTemplate)
 	if err != nil {
@@ -96,6 +98,7 @@ func NewOcpRouteSource(
 		ignoreHostnameAnnotation: ignoreHostnameAnnotation,
 		routeInformer:            informer,
 		labelSelector:            labelSelector,
+		ocpRouterName:            ocpRouterName,
 	}, nil
 }
 
@@ -128,7 +131,7 @@ func (ors *ocpRouteSource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint,
 			continue
 		}
 
-		orEndpoints := endpointsFromOcpRoute(ocpRoute, ors.ignoreHostnameAnnotation)
+		orEndpoints := ors.endpointsFromOcpRoute(ocpRoute, ors.ignoreHostnameAnnotation)
 
 		// apply template if host is missing on OpenShift Route
 		if (ors.combineFQDNAnnotation || len(orEndpoints) == 0) && ors.fqdnTemplate != nil {
@@ -174,7 +177,7 @@ func (ors *ocpRouteSource) endpointsFromTemplate(ocpRoute *routev1.Route) ([]*en
 
 	targets := getTargetsFromTargetAnnotation(ocpRoute.Annotations)
 	if len(targets) == 0 {
-		targets = targetsFromOcpRouteStatus(ocpRoute.Status)
+		targets = ors.targetsFromOcpRouteStatus(ocpRoute.Status)
 	}
 
 	providerSpecific, setIdentifier := getProviderSpecificAnnotations(ocpRoute.Annotations)
@@ -223,7 +226,7 @@ func (ors *ocpRouteSource) setResourceLabel(ocpRoute *routev1.Route, endpoints [
 }
 
 // endpointsFromOcpRoute extracts the endpoints from a OpenShift Route object
-func endpointsFromOcpRoute(ocpRoute *routev1.Route, ignoreHostnameAnnotation bool) []*endpoint.Endpoint {
+func (ors *ocpRouteSource) endpointsFromOcpRoute(ocpRoute *routev1.Route, ignoreHostnameAnnotation bool) []*endpoint.Endpoint {
 	var endpoints []*endpoint.Endpoint
 
 	ttl, err := getTTLFromAnnotations(ocpRoute.Annotations)
@@ -234,7 +237,7 @@ func endpointsFromOcpRoute(ocpRoute *routev1.Route, ignoreHostnameAnnotation boo
 	targets := getTargetsFromTargetAnnotation(ocpRoute.Annotations)
 
 	if len(targets) == 0 {
-		targets = targetsFromOcpRouteStatus(ocpRoute.Status)
+		targets = ors.targetsFromOcpRouteStatus(ocpRoute.Status)
 	}
 
 	providerSpecific, setIdentifier := getProviderSpecificAnnotations(ocpRoute.Annotations)
@@ -253,14 +256,18 @@ func endpointsFromOcpRoute(ocpRoute *routev1.Route, ignoreHostnameAnnotation boo
 	return endpoints
 }
 
-func targetsFromOcpRouteStatus(status routev1.RouteStatus) endpoint.Targets {
+func (ors *ocpRouteSource) targetsFromOcpRouteStatus(status routev1.RouteStatus) endpoint.Targets {
 	var targets endpoint.Targets
-
 	for _, ing := range status.Ingress {
-		if ing.RouterCanonicalHostname != "" {
+		if len(ors.ocpRouterName) != 0 {
+			if ing.RouterName == ors.ocpRouterName {
+				targets = append(targets, ing.RouterCanonicalHostname)
+				return targets
+			}
+		} else if ing.RouterCanonicalHostname != "" {
 			targets = append(targets, ing.RouterCanonicalHostname)
+			return targets
 		}
 	}
-
 	return targets
 }

source/openshift_route_test.go

@@ -50,6 +50,7 @@ func (suite *OCPRouteSuite) SetupTest() {
 		false,
 		false,
 		labels.Everything(),
+		"",
 	)
 
 	suite.routeWithTargets = &routev1.Route{
@@ -147,6 +148,7 @@ func testOcpRouteSourceNewOcpRouteSource(t *testing.T) {
 				false,
 				false,
 				labelSelector,
+				"",
 			)
 
 			if ti.expectError {
@@ -160,8 +162,6 @@ func testOcpRouteSourceNewOcpRouteSource(t *testing.T) {
 
 // testOcpRouteSourceEndpoints tests that various OCP routes generate the correct endpoints.
 func testOcpRouteSourceEndpoints(t *testing.T) {
-	t.Parallel()
-
 	for _, tc := range []struct {
 		title                    string
 		targetNamespace          string
@@ -172,6 +172,7 @@ func testOcpRouteSourceEndpoints(t *testing.T) {
 		expected                 []*endpoint.Endpoint
 		expectError              bool
 		labelFilter              string
+		ocpRouterName            string
 	}{
 		{
 			title:                    "route with basic hostname and route status target",
@@ -196,6 +197,7 @@ func testOcpRouteSourceEndpoints(t *testing.T) {
 					},
 				},
 			},
+			ocpRouterName: "",
 			expected: []*endpoint.Endpoint{
 				{
 					DNSName: "my-domain.com",
@@ -206,6 +208,119 @@ func testOcpRouteSourceEndpoints(t *testing.T) {
 			},
 			expectError: false,
 		},
+		{
+			title:                    "route with basic hostname and route status target with one RouterCanonicalHostname and one ocpRouterNames defined",
+			targetNamespace:          "",
+			annotationFilter:         "",
+			fqdnTemplate:             "",
+			ignoreHostnameAnnotation: false,
+			ocpRoute: &routev1.Route{
+				Spec: routev1.RouteSpec{
+					Host: "my-domain.com",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace:   "default",
+					Name:        "route-with-target",
+					Annotations: map[string]string{},
+				},
+				Status: routev1.RouteStatus{
+					Ingress: []routev1.RouteIngress{
+						{
+							RouterName:              "default",
+							RouterCanonicalHostname: "router-default.my-domain.com",
+						},
+					},
+				},
+			},
+			ocpRouterName: "default",
+			expected: []*endpoint.Endpoint{
+				{
+					DNSName: "my-domain.com",
+					Targets: []string{
+						"router-default.my-domain.com",
+					},
+				},
+			},
+			expectError: false,
+		},
+		{
+			title:                    "route with basic hostname and route status target with one RouterCanonicalHostname and one ocpRouterNames defined and two router canonical names",
+			targetNamespace:          "",
+			annotationFilter:         "",
+			fqdnTemplate:             "",
+			ignoreHostnameAnnotation: false,
+			ocpRoute: &routev1.Route{
+				Spec: routev1.RouteSpec{
+					Host: "my-domain.com",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace:   "default",
+					Name:        "route-with-target",
+					Annotations: map[string]string{},
+				},
+				Status: routev1.RouteStatus{
+					Ingress: []routev1.RouteIngress{
+						{
+							RouterName:              "default",
+							RouterCanonicalHostname: "router-default.my-domain.com",
+						},
+						{
+							RouterName:              "test",
+							RouterCanonicalHostname: "router-test.my-domain.com",
+						},
+					},
+				},
+			},
+			ocpRouterName: "default",
+			expected: []*endpoint.Endpoint{
+				{
+					DNSName: "my-domain.com",
+					Targets: []string{
+						"router-default.my-domain.com",
+					},
+				},
+			},
+			expectError: false,
+		},
+		{
+			title:                    "route with basic hostname and route status target with one RouterCanonicalHostname and one ocpRouterName defined and two router canonical names",
+			targetNamespace:          "",
+			annotationFilter:         "",
+			fqdnTemplate:             "",
+			ignoreHostnameAnnotation: false,
+			ocpRoute: &routev1.Route{
+				Spec: routev1.RouteSpec{
+					Host: "my-domain.com",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace:   "default",
+					Name:        "route-with-target",
+					Annotations: map[string]string{},
+				},
+				Status: routev1.RouteStatus{
+					Ingress: []routev1.RouteIngress{
+						{
+							RouterName:              "default",
+							RouterCanonicalHostname: "router-default.my-domain.com",
+						},
+						{
+							RouterName:              "test",
+							RouterCanonicalHostname: "router-test.my-domain.com",
+						},
+					},
+				},
+			},
+			ocpRouterName: "default",
+			expected: []*endpoint.Endpoint{
+				{
+					DNSName: "my-domain.com",
+					Targets: []string{
+						"router-default.my-domain.com",
+					},
+				},
+			},
+			expectError: false,
+		},
 		{
 			title:                    "route with incorrect externalDNS controller annotation",
 			targetNamespace:          "",
@@ -221,8 +336,9 @@ func testOcpRouteSourceEndpoints(t *testing.T) {
 					},
 				},
 			},
-			expected:    []*endpoint.Endpoint{},
-			expectError: false,
+			ocpRouterName: "",
+			expected:      []*endpoint.Endpoint{},
+			expectError:   false,
 		},
 		{
 			title:                    "route with basic hostname and annotation target",
@@ -242,6 +358,7 @@ func testOcpRouteSourceEndpoints(t *testing.T) {
 					},
 				},
 			},
+			ocpRouterName: "",
 			expected: []*endpoint.Endpoint{
 				{
 					DNSName: "my-annotation-domain.com",
@@ -273,6 +390,7 @@ func testOcpRouteSourceEndpoints(t *testing.T) {
 					},
 				},
 			},
+			ocpRouterName: "",
 			expected: []*endpoint.Endpoint{
 				{
 					DNSName: "my-annotation-domain.com",
@@ -304,17 +422,16 @@ func testOcpRouteSourceEndpoints(t *testing.T) {
 					},
 				},
 			},
-			expected:    []*endpoint.Endpoint{},
-			expectError: false,
+			ocpRouterName: "",
+			expected:      []*endpoint.Endpoint{},
+			expectError:   false,
 		},
 	} {
 		tc := tc
 		t.Run(tc.title, func(t *testing.T) {
 			t.Parallel()
-
 			// Create a Kubernetes testing client
 			fakeClient := fake.NewSimpleClientset()
-
 			_, err := fakeClient.RouteV1().Routes(tc.ocpRoute.Namespace).Create(context.Background(), tc.ocpRoute, metav1.CreateOptions{})
 			require.NoError(t, err)
 
@@ -329,7 +446,9 @@ func testOcpRouteSourceEndpoints(t *testing.T) {
 				false,
 				false,
 				labelSelector,
+				tc.ocpRouterName,
 			)
+
 			require.NoError(t, err)
 
 			res, err := source.Endpoints(context.Background())

source/store.go

@@ -67,6 +67,7 @@ type Config struct {
 	SkipperRouteGroupVersion       string
 	RequestTimeout                 time.Duration
 	DefaultTargets                 []string
+	OCPRouterName                  string
 }
 
 // ClientGenerator provides clients
@@ -254,7 +255,7 @@ func BuildWithConfig(source string, p ClientGenerator, cfg *Config) (Source, err
 		if err != nil {
 			return nil, err
 		}
-		return NewOcpRouteSource(ocpClient, cfg.Namespace, cfg.AnnotationFilter, cfg.FQDNTemplate, cfg.CombineFQDNAndAnnotation, cfg.IgnoreHostnameAnnotation, cfg.LabelFilter)
+		return NewOcpRouteSource(ocpClient, cfg.Namespace, cfg.AnnotationFilter, cfg.FQDNTemplate, cfg.CombineFQDNAndAnnotation, cfg.IgnoreHostnameAnnotation, cfg.LabelFilter, cfg.OCPRouterName)
 	case "fake":
 		return NewFakeSource(cfg.FQDNTemplate)
 	case "connector":