From a4274346fc206ab46a5f5b72134d30b354693bc4 Mon Sep 17 00:00:00 2001 From: Kyle Martin Date: Wed, 9 Mar 2022 22:33:10 +1100 Subject: [PATCH] Add annotation filter to Ambassador Host Source This change makes the Ambassador Host source respect the External-DNS annotationFilter allowing for an Ambassador Host resource to specify what External-DNS deployment to use when there are multiple External-DNS deployments within the same cluster. Before this change if you had two External-DNS deployments within the cluster and used the Ambassador Host source the first External-DNS to process the resource will create the record and not the one that was specified in the filter annotation. I added the `filterByAnnotations` function so that it matched the same way the other sources have implemented annotation filtering. I didn't add the controller check only because I wanted to keep this change to implementing the annotationFilter. I added Endpoint tests to validate that the filterByAnnotations function works as expected. Again these tests were based of the Endpoint tests that other sources use. To keep the tests simpler I only allow for a single load balancer to be used. Example: Create two External-DNS deployments 1 public and 1 private and set the Ambassador Host to use the public External-DNS using the annotation filter. ``` --- apiVersion: apps/v1 kind: Deployment metadata: name: external-dns-private spec: strategy: type: Recreate selector: matchLabels: app: external-dns-private template: metadata: labels: app: external-dns-private annotations: iam.amazonaws.com/role: {ARN} # AWS ARN role spec: serviceAccountName: external-dns containers: - name: external-dns image: k8s.gcr.io/external-dns/external-dns:latest args: - --source=ambassador-host - --domain-filter=example.net # will make ExternalDNS see only the hosted zones matching provided domain, omit to process all available hosted zones - --provider=aws - --policy=upsert-only # would prevent ExternalDNS from deleting any records, omit to enable full synchronization - --aws-zone-type=private # only look at public hosted zones (valid values are public, private or no value for both) - --registry=txt - --txt-owner-id= {Hosted Zone ID} # Insert Route53 Hosted Zone ID here - --annotation-filter=kubernetes.io/ingress.class in (private) --- apiVersion: apps/v1 kind: Deployment metadata: name: external-dns-public spec: strategy: type: Recreate selector: matchLabels: app: external-dns-public template: metadata: labels: app: external-dns-public annotations: iam.amazonaws.com/role: {ARN} # AWS ARN role spec: serviceAccountName: external-dns containers: - name: external-dns image: k8s.gcr.io/external-dns/external-dns:latest args: - --source=ambassador-host - --domain-filter=example.net # will make ExternalDNS see only the hosted zones matching provided domain, omit to process all available hosted zones - --provider=aws - --policy=upsert-only # would prevent ExternalDNS from deleting any records, omit to enable full synchronization - --aws-zone-type= # only look at public hosted zones (valid values are public, private or no value for both) - --registry=txt - --txt-owner-id= {Hosted Zone ID} # Insert Route53 Hosted Zone ID here - --annotation-filter=kubernetes.io/ingress.class in (public) --- apiVersion: getambassador.io/v3alpha1 kind: Host metadata: name: your-hostname annotations: external-dns.ambassador-service: emissary-ingress/emissary kubernetes.io/ingress.class: public spec: acmeProvider: authority: none hostname: your-hostname.example.com ``` Fixes kubernetes-sigs/external-dns#2632 --- source/ambassador_host.go | 53 ++++- source/ambassador_host_test.go | 395 +++++++++++++++++++++++++++++++++ source/store.go | 2 +- 3 files changed, 447 insertions(+), 3 deletions(-) diff --git a/source/ambassador_host.go b/source/ambassador_host.go index a2214dcd3a..52c08b5a65 100644 --- a/source/ambassador_host.go +++ b/source/ambassador_host.go @@ -57,6 +57,7 @@ type ambassadorHostSource struct { dynamicKubeClient dynamic.Interface kubeClient kubernetes.Interface namespace string + annotationFilter string ambassadorHostInformer informers.GenericInformer unstructuredConverter *unstructuredConverter } @@ -66,7 +67,9 @@ func NewAmbassadorHostSource( ctx context.Context, dynamicKubeClient dynamic.Interface, kubeClient kubernetes.Interface, - namespace string) (Source, error) { + namespace string, + annotationFilter string, +) (Source, error) { var err error // Use shared informer to listen for add/update/delete of Host in the specified namespace. @@ -84,6 +87,7 @@ func NewAmbassadorHostSource( informerFactory.Start(ctx.Done()) + // wait for the local cache to be populated. if err := waitForDynamicCacheSync(context.Background(), informerFactory); err != nil { return nil, err } @@ -97,6 +101,7 @@ func NewAmbassadorHostSource( dynamicKubeClient: dynamicKubeClient, kubeClient: kubeClient, namespace: namespace, + annotationFilter: annotationFilter, ambassadorHostInformer: ambassadorHostInformer, unstructuredConverter: uc, }, nil @@ -110,7 +115,8 @@ func (sc *ambassadorHostSource) Endpoints(ctx context.Context) ([]*endpoint.Endp return nil, err } - endpoints := []*endpoint.Endpoint{} + // Get a list of Ambassador Host resources + var ambassadorHosts []*ambassador.Host for _, hostObj := range hosts { unstructuredHost, ok := hostObj.(*unstructured.Unstructured) if !ok { @@ -122,7 +128,18 @@ func (sc *ambassadorHostSource) Endpoints(ctx context.Context) ([]*endpoint.Endp if err != nil { return nil, err } + ambassadorHosts = append(ambassadorHosts, host) + } + + // Filter Ambassador Hosts + ambassadorHosts, err = sc.filterByAnnotations(ambassadorHosts) + if err != nil { + return nil, errors.Wrap(err, "failed to filter Ambassador Hosts") + } + endpoints := []*endpoint.Endpoint{} + + for _, host := range ambassadorHosts { fullname := fmt.Sprintf("%s/%s", host.Namespace, host.Name) // look for the "exernal-dns.ambassador-service" annotation. If it is not there then just ignore this `Host` @@ -274,3 +291,35 @@ func newUnstructuredConverter() (*unstructuredConverter, error) { return uc, nil } + +// Filter a list of Ambassador Host Resources to only return the ones that +// contain the required External-DNS annotation filter +func (sc *ambassadorHostSource) filterByAnnotations(ambassadorHosts []*ambassador.Host) ([]*ambassador.Host, error) { + // External-DNS Annotation Filter + labelSelector, err := metav1.ParseToLabelSelector(sc.annotationFilter) + if err != nil { + return nil, err + } + + selector, err := metav1.LabelSelectorAsSelector(labelSelector) + if err != nil { + return nil, err + } + + // empty filter returns original list of Ambassador Hosts + if selector.Empty() { + return ambassadorHosts, nil + } + + // Return a filtered list of Ambassador Hosts + filteredList := []*ambassador.Host{} + for _, host := range ambassadorHosts { + annotations := labels.Set(host.Annotations) + // include Ambassador Host if its annotations match the annotation filter + if selector.Matches(annotations) { + filteredList = append(filteredList, host) + } + } + + return filteredList, nil +} diff --git a/source/ambassador_host_test.go b/source/ambassador_host_test.go index bb960600e0..dbfa282918 100644 --- a/source/ambassador_host_test.go +++ b/source/ambassador_host_test.go @@ -17,12 +17,25 @@ limitations under the License. package source import ( + "context" "testing" + ambassador "github.com/datawire/ambassador/pkg/api/getambassador.io/v2" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + fakeDynamic "k8s.io/client-go/dynamic/fake" + "k8s.io/client-go/kubernetes/fake" + "sigs.k8s.io/external-dns/endpoint" ) +// This is a compile-time validation that ambassadorHostSource is a Source. +var _ Source = &ambassadorHostSource{} + type AmbassadorSuite struct { suite.Suite } @@ -30,6 +43,7 @@ type AmbassadorSuite struct { func TestAmbassadorSource(t *testing.T) { suite.Run(t, new(AmbassadorSuite)) t.Run("Interface", testAmbassadorSourceImplementsSource) + t.Run("Endpoints", testAmbassadorSourceEndpoints) } // testAmbassadorSourceImplementsSource tests that ambassadorHostSource is a valid Source. @@ -37,6 +51,315 @@ func testAmbassadorSourceImplementsSource(t *testing.T) { require.Implements(t, (*Source)(nil), new(ambassadorHostSource)) } +// testAmbassadorSourceEndpoints tests that various Ambassador Hosts generate the correct endpoints. +func testAmbassadorSourceEndpoints(t *testing.T) { + for _, ti := range []struct { + title string + targetNamespace string + annotationFilter string + loadBalancer fakeAmbassadorLoadBalancerService + ambassadorHostItems []fakeAmbassadorHost + expected []*endpoint.Endpoint + expectError bool + }{ + { + title: "no host", + targetNamespace: "", + }, + { + title: "two simple hosts", + targetNamespace: "", + loadBalancer: fakeAmbassadorLoadBalancerService{ + ips: []string{"8.8.8.8"}, + hostnames: []string{"lb.com"}, + name: "emissary", + namespace: "emissary-ingress", + }, + ambassadorHostItems: []fakeAmbassadorHost{ + { + name: "fake1", + namespace: "", + hostname: "fake1.org", + annotations: map[string]string{ + "external-dns.ambassador-service": "emissary-ingress/emissary", + }, + }, + { + name: "fake2", + namespace: "", + hostname: "fake2.org", + annotations: map[string]string{ + "external-dns.ambassador-service": "emissary-ingress/emissary", + }, + }, + }, + expected: []*endpoint.Endpoint{ + { + DNSName: "fake1.org", + Targets: endpoint.Targets{"8.8.8.8"}, + }, + { + DNSName: "fake1.org", + Targets: endpoint.Targets{"lb.com"}, + }, + { + DNSName: "fake2.org", + Targets: endpoint.Targets{"8.8.8.8"}, + }, + { + DNSName: "fake2.org", + Targets: endpoint.Targets{"lb.com"}, + }, + }, + }, + { + title: "two simple hosts on different namespaces", + targetNamespace: "", + loadBalancer: fakeAmbassadorLoadBalancerService{ + ips: []string{"8.8.8.8"}, + hostnames: []string{"lb.com"}, + name: "emissary", + namespace: "emissary-ingress", + }, + ambassadorHostItems: []fakeAmbassadorHost{ + { + name: "fake1", + namespace: "testing1", + hostname: "fake1.org", + annotations: map[string]string{ + "external-dns.ambassador-service": "emissary-ingress/emissary", + }, + }, + { + name: "fake2", + namespace: "testing2", + hostname: "fake2.org", + annotations: map[string]string{ + "external-dns.ambassador-service": "emissary-ingress/emissary", + }, + }, + }, + expected: []*endpoint.Endpoint{ + { + DNSName: "fake1.org", + Targets: endpoint.Targets{"8.8.8.8"}, + }, + { + DNSName: "fake1.org", + Targets: endpoint.Targets{"lb.com"}, + }, + { + DNSName: "fake2.org", + Targets: endpoint.Targets{"8.8.8.8"}, + }, + { + DNSName: "fake2.org", + Targets: endpoint.Targets{"lb.com"}, + }, + }, + }, + { + title: "invalid non matching host ambassador service annotation", + targetNamespace: "", + loadBalancer: fakeAmbassadorLoadBalancerService{ + ips: []string{"8.8.8.8"}, + hostnames: []string{"lb.com"}, + name: "emissary", + namespace: "emissary-ingress", + }, + ambassadorHostItems: []fakeAmbassadorHost{ + { + name: "fake1", + namespace: "testing1", + hostname: "fake1.org", + annotations: map[string]string{ + "external-dns.ambassador-service": "emissary-ingress/invalid", + }, + }, + }, + expected: []*endpoint.Endpoint{}, + expectError: true, + }, + { + title: "valid matching annotation filter expression", + targetNamespace: "", + annotationFilter: "kubernetes.io/ingress.class in (external-ingress)", + loadBalancer: fakeAmbassadorLoadBalancerService{ + ips: []string{"8.8.8.8"}, + hostnames: []string{"lb.com"}, + name: "emissary", + namespace: "emissary-ingress", + }, + ambassadorHostItems: []fakeAmbassadorHost{ + { + name: "fake1", + namespace: "testing1", + hostname: "fake1.org", + annotations: map[string]string{ + "external-dns.ambassador-service": "emissary-ingress/emissary", + "kubernetes.io/ingress.class": "external-ingress", + }, + }, + }, + expected: []*endpoint.Endpoint{ + { + DNSName: "fake1.org", + Targets: endpoint.Targets{"8.8.8.8"}, + }, + { + DNSName: "fake1.org", + Targets: endpoint.Targets{"lb.com"}, + }, + }, + }, + { + title: "valid non-matching annotation filter expression", + targetNamespace: "", + annotationFilter: "kubernetes.io/ingress.class in (external-ingress)", + loadBalancer: fakeAmbassadorLoadBalancerService{ + ips: []string{"8.8.8.8"}, + hostnames: []string{"lb.com"}, + name: "emissary", + namespace: "emissary-ingress", + }, + ambassadorHostItems: []fakeAmbassadorHost{ + { + name: "fake1", + namespace: "testing1", + hostname: "fake1.org", + annotations: map[string]string{ + "external-dns.ambassador-service": "emissary-ingress/emissary", + "kubernetes.io/ingress.class": "internal-ingress", + }, + }, + }, + expected: []*endpoint.Endpoint{}, + }, + { + title: "invalid annotation filter expression", + targetNamespace: "", + annotationFilter: "kubernetes.io/ingress.class in (external ingress)", + loadBalancer: fakeAmbassadorLoadBalancerService{ + ips: []string{"8.8.8.8"}, + hostnames: []string{"lb.com"}, + name: "emissary", + namespace: "emissary-ingress", + }, + ambassadorHostItems: []fakeAmbassadorHost{ + { + name: "fake1", + namespace: "testing1", + hostname: "fake1.org", + annotations: map[string]string{ + "external-dns.ambassador-service": "emissary-ingress/emissary", + "kubernetes.io/ingress.class": "internal-ingress", + }, + }, + }, + expected: []*endpoint.Endpoint{}, + expectError: true, + }, + { + title: "valid matching annotation filter label", + targetNamespace: "", + annotationFilter: "kubernetes.io/ingress.class=external-ingress", + loadBalancer: fakeAmbassadorLoadBalancerService{ + ips: []string{"8.8.8.8"}, + hostnames: []string{"lb.com"}, + name: "emissary", + namespace: "emissary-ingress", + }, + ambassadorHostItems: []fakeAmbassadorHost{ + { + name: "fake1", + namespace: "testing1", + hostname: "fake1.org", + annotations: map[string]string{ + "external-dns.ambassador-service": "emissary-ingress/emissary", + "kubernetes.io/ingress.class": "external-ingress", + }, + }, + }, + expected: []*endpoint.Endpoint{ + { + DNSName: "fake1.org", + Targets: endpoint.Targets{"8.8.8.8"}, + }, + { + DNSName: "fake1.org", + Targets: endpoint.Targets{"lb.com"}, + }, + }, + }, + { + title: "valid non-matching annotation filter label", + targetNamespace: "", + annotationFilter: "kubernetes.io/ingress.class=external-ingress", + loadBalancer: fakeAmbassadorLoadBalancerService{ + ips: []string{"8.8.8.8"}, + hostnames: []string{"lb.com"}, + name: "emissary", + namespace: "emissary-ingress", + }, + ambassadorHostItems: []fakeAmbassadorHost{ + { + name: "fake1", + namespace: "testing1", + hostname: "fake1.org", + annotations: map[string]string{ + "external-dns.ambassador-service": "emissary-ingress/emissary", + "kubernetes.io/ingress.class": "internal-ingress", + }, + }, + }, + expected: []*endpoint.Endpoint{}, + }, + } { + ti := ti + t.Run(ti.title, func(t *testing.T) { + // Create a slice of Ambassador Hosts + ambassadorHosts := make([]*ambassador.Host, 0) + + // Convert our test data into Ambassador Hosts + for _, host := range ti.ambassadorHostItems { + ambassadorHosts = append(ambassadorHosts, host.Host()) + } + + fakeKubernetesClient := fake.NewSimpleClientset() + service := ti.loadBalancer.Service() + _, err := fakeKubernetesClient.CoreV1().Services(service.Namespace).Create(context.Background(), service, metav1.CreateOptions{}) + require.NoError(t, err) + + fakeDynamicClient, scheme := newAmbassadorDynamicKubernetesClient() + + for _, host := range ambassadorHosts { + converted, err := convertAmbassadorHostToUnstructured(host, scheme) + require.NoError(t, err) + _, err = fakeDynamicClient.Resource(ambHostGVR).Namespace(host.Namespace).Create(context.Background(), converted, metav1.CreateOptions{}) + require.NoError(t, err) + } + + ambassadorHostSource, err := NewAmbassadorHostSource( + context.TODO(), + fakeDynamicClient, + fakeKubernetesClient, + ti.targetNamespace, + ti.annotationFilter, + ) + require.NoError(t, err) + + res, err := ambassadorHostSource.Endpoints(context.Background()) + if ti.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + + validateEndpoints(t, res, ti.expected) + }) + } +} + // TestParseAmbLoadBalancerService tests our parsing of Ambassador service info. func TestParseAmbLoadBalancerService(t *testing.T) { vectors := []struct { @@ -76,3 +399,75 @@ func TestParseAmbLoadBalancerService(t *testing.T) { } } } + +func convertAmbassadorHostToUnstructured(hp *ambassador.Host, s *runtime.Scheme) (*unstructured.Unstructured, error) { + unstructuredAmbassadorHost := &unstructured.Unstructured{} + if err := s.Convert(hp, unstructuredAmbassadorHost, context.Background()); err != nil { + return nil, err + } + return unstructuredAmbassadorHost, nil +} + +func newAmbassadorDynamicKubernetesClient() (*fakeDynamic.FakeDynamicClient, *runtime.Scheme) { + s := runtime.NewScheme() + _ = ambassador.AddToScheme(s) + return fakeDynamic.NewSimpleDynamicClient(s), s +} + +type fakeAmbassadorHost struct { + namespace string + name string + annotations map[string]string + hostname string +} + +func (ir fakeAmbassadorHost) Host() *ambassador.Host { + spec := ambassador.HostSpec{ + Hostname: ir.hostname, + } + + host := &ambassador.Host{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ir.namespace, + Name: ir.name, + Annotations: ir.annotations, + }, + Spec: &spec, + } + + return host +} + +type fakeAmbassadorLoadBalancerService struct { + ips []string + hostnames []string + namespace string + name string +} + +func (ig fakeAmbassadorLoadBalancerService) Service() *v1.Service { + svc := &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ig.namespace, + Name: ig.name, + }, + Status: v1.ServiceStatus{ + LoadBalancer: v1.LoadBalancerStatus{ + Ingress: []v1.LoadBalancerIngress{}, + }, + }, + } + + for _, ip := range ig.ips { + svc.Status.LoadBalancer.Ingress = append(svc.Status.LoadBalancer.Ingress, v1.LoadBalancerIngress{ + IP: ip, + }) + } + for _, hostname := range ig.hostnames { + svc.Status.LoadBalancer.Ingress = append(svc.Status.LoadBalancer.Ingress, v1.LoadBalancerIngress{ + Hostname: hostname, + }) + } + + return svc +} diff --git a/source/store.go b/source/store.go index 942400aef5..265a60c4e0 100644 --- a/source/store.go +++ b/source/store.go @@ -234,7 +234,7 @@ func BuildWithConfig(ctx context.Context, source string, p ClientGenerator, cfg if err != nil { return nil, err } - return NewAmbassadorHostSource(ctx, dynamicClient, kubernetesClient, cfg.Namespace) + return NewAmbassadorHostSource(ctx, dynamicClient, kubernetesClient, cfg.Namespace, cfg.AnnotationFilter) case "contour-httpproxy": dynamicClient, err := p.DynamicKubernetesClient() if err != nil {