From 5e06f8144434b643e5e4d8275d2586889b31a7a8 Mon Sep 17 00:00:00 2001 From: Michael Nairn Date: Wed, 1 May 2024 19:42:59 +0100 Subject: [PATCH 1/4] provider: Copy external-dns inmemory provider Copies the inmemory provider from https://github.com/Kuadrant/external-dns/tree/master/provider/inmemory We need to make a few modifications so will copy it inyo this repo for now. This commit has no modifications so we can compare easily at a later date. --- .../provider/inmemory/inmemory.go | 344 ++++++++++ .../provider/inmemory/inmemory_test.go | 589 ++++++++++++++++++ 2 files changed, 933 insertions(+) create mode 100644 internal/external-dns/provider/inmemory/inmemory.go create mode 100644 internal/external-dns/provider/inmemory/inmemory_test.go diff --git a/internal/external-dns/provider/inmemory/inmemory.go b/internal/external-dns/provider/inmemory/inmemory.go new file mode 100644 index 00000000..eab515a6 --- /dev/null +++ b/internal/external-dns/provider/inmemory/inmemory.go @@ -0,0 +1,344 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package inmemory + +import ( + "context" + "errors" + "strings" + + log "github.com/sirupsen/logrus" + "k8s.io/apimachinery/pkg/util/sets" + + "sigs.k8s.io/external-dns/endpoint" + "sigs.k8s.io/external-dns/plan" + "sigs.k8s.io/external-dns/provider" +) + +var ( + // ErrZoneAlreadyExists error returned when zone cannot be created when it already exists + ErrZoneAlreadyExists = errors.New("specified zone already exists") + // ErrZoneNotFound error returned when specified zone does not exists + ErrZoneNotFound = errors.New("specified zone not found") + // ErrRecordAlreadyExists when create request is sent but record already exists + ErrRecordAlreadyExists = errors.New("record already exists") + // ErrRecordNotFound when update/delete request is sent but record not found + ErrRecordNotFound = errors.New("record not found") + // ErrDuplicateRecordFound when record is repeated in create/update/delete + ErrDuplicateRecordFound = errors.New("invalid batch request") +) + +// InMemoryProvider - dns provider only used for testing purposes +// initialized as dns provider with no records +type InMemoryProvider struct { + provider.BaseProvider + domain endpoint.DomainFilter + client *inMemoryClient + filter *filter + OnApplyChanges func(ctx context.Context, changes *plan.Changes) + OnRecords func() +} + +// InMemoryOption allows to extend in-memory provider +type InMemoryOption func(*InMemoryProvider) + +// InMemoryWithLogging injects logging when ApplyChanges is called +func InMemoryWithLogging() InMemoryOption { + return func(p *InMemoryProvider) { + p.OnApplyChanges = func(ctx context.Context, changes *plan.Changes) { + for _, v := range changes.Create { + log.Infof("CREATE: %v", v) + } + for _, v := range changes.UpdateOld { + log.Infof("UPDATE (old): %v", v) + } + for _, v := range changes.UpdateNew { + log.Infof("UPDATE (new): %v", v) + } + for _, v := range changes.Delete { + log.Infof("DELETE: %v", v) + } + } + } +} + +// InMemoryWithDomain modifies the domain on which dns zones are filtered +func InMemoryWithDomain(domainFilter endpoint.DomainFilter) InMemoryOption { + return func(p *InMemoryProvider) { + p.domain = domainFilter + } +} + +// InMemoryInitZones pre-seeds the InMemoryProvider with given zones +func InMemoryInitZones(zones []string) InMemoryOption { + return func(p *InMemoryProvider) { + for _, z := range zones { + if err := p.CreateZone(z); err != nil { + log.Warnf("Unable to initialize zones for inmemory provider") + } + } + } +} + +// NewInMemoryProvider returns InMemoryProvider DNS provider interface implementation +func NewInMemoryProvider(opts ...InMemoryOption) *InMemoryProvider { + im := &InMemoryProvider{ + filter: &filter{}, + OnApplyChanges: func(ctx context.Context, changes *plan.Changes) {}, + OnRecords: func() {}, + domain: endpoint.NewDomainFilter([]string{""}), + client: newInMemoryClient(), + } + + for _, opt := range opts { + opt(im) + } + + return im +} + +// CreateZone adds new zone if not present +func (im *InMemoryProvider) CreateZone(newZone string) error { + return im.client.CreateZone(newZone) +} + +// Zones returns filtered zones as specified by domain +func (im *InMemoryProvider) Zones() map[string]string { + return im.filter.Zones(im.client.Zones()) +} + +// Records returns the list of endpoints +func (im *InMemoryProvider) Records(ctx context.Context) ([]*endpoint.Endpoint, error) { + defer im.OnRecords() + + endpoints := make([]*endpoint.Endpoint, 0) + + for zoneID := range im.Zones() { + records, err := im.client.Records(zoneID) + if err != nil { + return nil, err + } + + endpoints = append(endpoints, copyEndpoints(records)...) + } + + return endpoints, nil +} + +// ApplyChanges simply modifies records in memory +// error checking occurs before any modifications are made, i.e. batch processing +// create record - record should not exist +// update/delete record - record should exist +// create/update/delete lists should not have overlapping records +func (im *InMemoryProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) error { + defer im.OnApplyChanges(ctx, changes) + + perZoneChanges := map[string]*plan.Changes{} + + zones := im.Zones() + for zoneID := range zones { + perZoneChanges[zoneID] = &plan.Changes{} + } + + for _, ep := range changes.Create { + zoneID := im.filter.EndpointZoneID(ep, zones) + if zoneID == "" { + continue + } + perZoneChanges[zoneID].Create = append(perZoneChanges[zoneID].Create, ep) + } + for _, ep := range changes.UpdateNew { + zoneID := im.filter.EndpointZoneID(ep, zones) + if zoneID == "" { + continue + } + perZoneChanges[zoneID].UpdateNew = append(perZoneChanges[zoneID].UpdateNew, ep) + } + for _, ep := range changes.UpdateOld { + zoneID := im.filter.EndpointZoneID(ep, zones) + if zoneID == "" { + continue + } + perZoneChanges[zoneID].UpdateOld = append(perZoneChanges[zoneID].UpdateOld, ep) + } + for _, ep := range changes.Delete { + zoneID := im.filter.EndpointZoneID(ep, zones) + if zoneID == "" { + continue + } + perZoneChanges[zoneID].Delete = append(perZoneChanges[zoneID].Delete, ep) + } + + for zoneID := range perZoneChanges { + change := &plan.Changes{ + Create: perZoneChanges[zoneID].Create, + UpdateNew: perZoneChanges[zoneID].UpdateNew, + UpdateOld: perZoneChanges[zoneID].UpdateOld, + Delete: perZoneChanges[zoneID].Delete, + } + err := im.client.ApplyChanges(ctx, zoneID, change) + if err != nil { + return err + } + } + + return nil +} + +func copyEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint { + records := make([]*endpoint.Endpoint, 0, len(endpoints)) + for _, ep := range endpoints { + newEp := endpoint.NewEndpointWithTTL(ep.DNSName, ep.RecordType, ep.RecordTTL, ep.Targets...).WithSetIdentifier(ep.SetIdentifier) + newEp.Labels = endpoint.NewLabels() + for k, v := range ep.Labels { + newEp.Labels[k] = v + } + newEp.ProviderSpecific = append(endpoint.ProviderSpecific(nil), ep.ProviderSpecific...) + records = append(records, newEp) + } + return records +} + +type filter struct { + domain string +} + +// Zones filters map[zoneID]zoneName for names having f.domain as suffix +func (f *filter) Zones(zones map[string]string) map[string]string { + result := map[string]string{} + for zoneID, zoneName := range zones { + if strings.HasSuffix(zoneName, f.domain) { + result[zoneID] = zoneName + } + } + return result +} + +// EndpointZoneID determines zoneID for endpoint from map[zoneID]zoneName by taking longest suffix zoneName match in endpoint DNSName +// returns empty string if no match found +func (f *filter) EndpointZoneID(endpoint *endpoint.Endpoint, zones map[string]string) (zoneID string) { + var matchZoneID, matchZoneName string + for zoneID, zoneName := range zones { + if strings.HasSuffix(endpoint.DNSName, zoneName) && len(zoneName) > len(matchZoneName) { + matchZoneName = zoneName + matchZoneID = zoneID + } + } + return matchZoneID +} + +type zone map[endpoint.EndpointKey]*endpoint.Endpoint + +type inMemoryClient struct { + zones map[string]zone +} + +func newInMemoryClient() *inMemoryClient { + return &inMemoryClient{map[string]zone{}} +} + +func (c *inMemoryClient) Records(zone string) ([]*endpoint.Endpoint, error) { + if _, ok := c.zones[zone]; !ok { + return nil, ErrZoneNotFound + } + + var records []*endpoint.Endpoint + for _, rec := range c.zones[zone] { + records = append(records, rec) + } + return records, nil +} + +func (c *inMemoryClient) Zones() map[string]string { + zones := map[string]string{} + for zone := range c.zones { + zones[zone] = zone + } + return zones +} + +func (c *inMemoryClient) CreateZone(zone string) error { + if _, ok := c.zones[zone]; ok { + return ErrZoneAlreadyExists + } + c.zones[zone] = map[endpoint.EndpointKey]*endpoint.Endpoint{} + + return nil +} + +func (c *inMemoryClient) ApplyChanges(ctx context.Context, zoneID string, changes *plan.Changes) error { + if err := c.validateChangeBatch(zoneID, changes); err != nil { + return err + } + for _, newEndpoint := range changes.Create { + c.zones[zoneID][newEndpoint.Key()] = newEndpoint + } + for _, updateEndpoint := range changes.UpdateNew { + c.zones[zoneID][updateEndpoint.Key()] = updateEndpoint + } + for _, deleteEndpoint := range changes.Delete { + delete(c.zones[zoneID], deleteEndpoint.Key()) + } + return nil +} + +func (c *inMemoryClient) updateMesh(mesh sets.Set[endpoint.EndpointKey], record *endpoint.Endpoint) error { + if mesh.Has(record.Key()) { + return ErrDuplicateRecordFound + } + mesh.Insert(record.Key()) + return nil +} + +// validateChangeBatch validates that the changes passed to InMemory DNS provider is valid +func (c *inMemoryClient) validateChangeBatch(zone string, changes *plan.Changes) error { + curZone, ok := c.zones[zone] + if !ok { + return ErrZoneNotFound + } + mesh := sets.New[endpoint.EndpointKey]() + for _, newEndpoint := range changes.Create { + if _, exists := curZone[newEndpoint.Key()]; exists { + return ErrRecordAlreadyExists + } + if err := c.updateMesh(mesh, newEndpoint); err != nil { + return err + } + } + for _, updateEndpoint := range changes.UpdateNew { + if _, exists := curZone[updateEndpoint.Key()]; !exists { + return ErrRecordNotFound + } + if err := c.updateMesh(mesh, updateEndpoint); err != nil { + return err + } + } + for _, updateOldEndpoint := range changes.UpdateOld { + if rec, exists := curZone[updateOldEndpoint.Key()]; !exists || rec.Targets[0] != updateOldEndpoint.Targets[0] { + return ErrRecordNotFound + } + } + for _, deleteEndpoint := range changes.Delete { + if rec, exists := curZone[deleteEndpoint.Key()]; !exists || rec.Targets[0] != deleteEndpoint.Targets[0] { + return ErrRecordNotFound + } + if err := c.updateMesh(mesh, deleteEndpoint); err != nil { + return err + } + } + return nil +} diff --git a/internal/external-dns/provider/inmemory/inmemory_test.go b/internal/external-dns/provider/inmemory/inmemory_test.go new file mode 100644 index 00000000..7028744b --- /dev/null +++ b/internal/external-dns/provider/inmemory/inmemory_test.go @@ -0,0 +1,589 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package inmemory + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "sigs.k8s.io/external-dns/endpoint" + "sigs.k8s.io/external-dns/plan" + "sigs.k8s.io/external-dns/provider" + + "github.com/kuadrant/dns-operator/internal/external-dns/testutils" +) + +var _ provider.Provider = &InMemoryProvider{} + +func TestInMemoryProvider(t *testing.T) { + t.Run("Records", testInMemoryRecords) + t.Run("validateChangeBatch", testInMemoryValidateChangeBatch) + t.Run("ApplyChanges", testInMemoryApplyChanges) + t.Run("NewInMemoryProvider", testNewInMemoryProvider) + t.Run("CreateZone", testInMemoryCreateZone) +} + +func testInMemoryRecords(t *testing.T) { + for _, ti := range []struct { + title string + zone string + expectError bool + init map[string]zone + expected []*endpoint.Endpoint + }{ + { + title: "no records, no zone", + zone: "", + init: map[string]zone{}, + expectError: false, + }, + { + title: "records, wrong zone", + zone: "net", + init: map[string]zone{ + "org": {}, + "com": {}, + }, + expectError: false, + }, + { + title: "records, zone with records", + zone: "org", + init: map[string]zone{ + "org": makeZone( + "example.org", "8.8.8.8", endpoint.RecordTypeA, + "example.org", "", endpoint.RecordTypeTXT, + "foo.org", "4.4.4.4", endpoint.RecordTypeCNAME, + ), + "com": makeZone("example.com", "4.4.4.4", endpoint.RecordTypeCNAME), + }, + expectError: false, + expected: []*endpoint.Endpoint{ + { + DNSName: "example.org", + Targets: endpoint.Targets{"8.8.8.8"}, + RecordType: endpoint.RecordTypeA, + }, + { + DNSName: "example.org", + RecordType: endpoint.RecordTypeTXT, + Targets: endpoint.Targets{""}, + }, + { + DNSName: "foo.org", + Targets: endpoint.Targets{"4.4.4.4"}, + RecordType: endpoint.RecordTypeCNAME, + }, + }, + }, + } { + t.Run(ti.title, func(t *testing.T) { + c := newInMemoryClient() + c.zones = ti.init + im := NewInMemoryProvider() + im.client = c + f := filter{domain: ti.zone} + im.filter = &f + records, err := im.Records(context.Background()) + if ti.expectError { + assert.Nil(t, records) + assert.EqualError(t, err, ErrZoneNotFound.Error()) + } else { + require.NoError(t, err) + assert.True(t, testutils.SameEndpoints(ti.expected, records), "Endpoints not the same: Expected: %+v Records: %+v", ti.expected, records) + } + }) + } +} + +func testInMemoryValidateChangeBatch(t *testing.T) { + init := map[string]zone{ + "org": makeZone( + "example.org", "8.8.8.8", endpoint.RecordTypeA, + "example.org", "", endpoint.RecordTypeTXT, + "foo.org", "bar.org", endpoint.RecordTypeCNAME, + "foo.bar.org", "5.5.5.5", endpoint.RecordTypeA, + ), + "com": makeZone("example.com", "another-example.com", endpoint.RecordTypeCNAME), + } + for _, ti := range []struct { + title string + expectError bool + errorType error + init map[string]zone + changes *plan.Changes + zone string + }{ + { + title: "no zones, no update", + expectError: true, + zone: "", + init: map[string]zone{}, + changes: &plan.Changes{ + Create: []*endpoint.Endpoint{}, + UpdateNew: []*endpoint.Endpoint{}, + UpdateOld: []*endpoint.Endpoint{}, + Delete: []*endpoint.Endpoint{}, + }, + errorType: ErrZoneNotFound, + }, + { + title: "zones, no update", + expectError: true, + zone: "", + init: init, + changes: &plan.Changes{ + Create: []*endpoint.Endpoint{}, + UpdateNew: []*endpoint.Endpoint{}, + UpdateOld: []*endpoint.Endpoint{}, + Delete: []*endpoint.Endpoint{}, + }, + errorType: ErrZoneNotFound, + }, + { + title: "zones, update, wrong zone", + expectError: true, + zone: "test", + init: init, + changes: &plan.Changes{ + Create: []*endpoint.Endpoint{}, + UpdateNew: []*endpoint.Endpoint{}, + UpdateOld: []*endpoint.Endpoint{}, + Delete: []*endpoint.Endpoint{}, + }, + errorType: ErrZoneNotFound, + }, + { + title: "zones, update, right zone, invalid batch - already exists", + expectError: true, + zone: "org", + init: init, + changes: &plan.Changes{ + Create: []*endpoint.Endpoint{ + { + DNSName: "example.org", + Targets: endpoint.Targets{"8.8.8.8"}, + RecordType: endpoint.RecordTypeA, + }, + }, + UpdateNew: []*endpoint.Endpoint{}, + UpdateOld: []*endpoint.Endpoint{}, + Delete: []*endpoint.Endpoint{}, + }, + errorType: ErrRecordAlreadyExists, + }, + { + title: "zones, update, right zone, invalid batch - record not found for update", + expectError: true, + zone: "org", + init: init, + changes: &plan.Changes{ + Create: []*endpoint.Endpoint{ + { + DNSName: "foo.org", + Targets: endpoint.Targets{"4.4.4.4"}, + RecordType: endpoint.RecordTypeA, + }, + }, + UpdateNew: []*endpoint.Endpoint{ + { + DNSName: "foo.org", + Targets: endpoint.Targets{"4.4.4.4"}, + RecordType: endpoint.RecordTypeA, + }, + }, + UpdateOld: []*endpoint.Endpoint{}, + Delete: []*endpoint.Endpoint{}, + }, + errorType: ErrRecordNotFound, + }, + { + title: "zones, update, right zone, invalid batch - record not found for update", + expectError: true, + zone: "org", + init: init, + changes: &plan.Changes{ + Create: []*endpoint.Endpoint{ + { + DNSName: "foo.org", + Targets: endpoint.Targets{"4.4.4.4"}, + RecordType: endpoint.RecordTypeA, + }, + }, + UpdateNew: []*endpoint.Endpoint{ + { + DNSName: "foo.org", + Targets: endpoint.Targets{"4.4.4.4"}, + RecordType: endpoint.RecordTypeA, + }, + }, + UpdateOld: []*endpoint.Endpoint{}, + Delete: []*endpoint.Endpoint{}, + }, + errorType: ErrRecordNotFound, + }, + { + title: "zones, update, right zone, invalid batch - duplicated create", + expectError: true, + zone: "org", + init: init, + changes: &plan.Changes{ + Create: []*endpoint.Endpoint{ + { + DNSName: "foo.org", + Targets: endpoint.Targets{"4.4.4.4"}, + RecordType: endpoint.RecordTypeA, + }, + { + DNSName: "foo.org", + Targets: endpoint.Targets{"4.4.4.4"}, + RecordType: endpoint.RecordTypeA, + }, + }, + UpdateNew: []*endpoint.Endpoint{}, + UpdateOld: []*endpoint.Endpoint{}, + Delete: []*endpoint.Endpoint{}, + }, + errorType: ErrDuplicateRecordFound, + }, + { + title: "zones, update, right zone, invalid batch - duplicated update/delete", + expectError: true, + zone: "org", + init: init, + changes: &plan.Changes{ + Create: []*endpoint.Endpoint{}, + UpdateNew: []*endpoint.Endpoint{ + { + DNSName: "example.org", + Targets: endpoint.Targets{"8.8.8.8"}, + RecordType: endpoint.RecordTypeA, + }, + }, + UpdateOld: []*endpoint.Endpoint{}, + Delete: []*endpoint.Endpoint{ + { + DNSName: "example.org", + Targets: endpoint.Targets{"8.8.8.8"}, + RecordType: endpoint.RecordTypeA, + }, + }, + }, + errorType: ErrDuplicateRecordFound, + }, + { + title: "zones, update, right zone, invalid batch - duplicated update", + expectError: true, + zone: "org", + init: init, + changes: &plan.Changes{ + Create: []*endpoint.Endpoint{}, + UpdateNew: []*endpoint.Endpoint{ + { + DNSName: "example.org", + Targets: endpoint.Targets{"8.8.8.8"}, + RecordType: endpoint.RecordTypeA, + }, + { + DNSName: "example.org", + Targets: endpoint.Targets{"8.8.8.8"}, + RecordType: endpoint.RecordTypeA, + }, + }, + UpdateOld: []*endpoint.Endpoint{}, + Delete: []*endpoint.Endpoint{}, + }, + errorType: ErrDuplicateRecordFound, + }, + { + title: "zones, update, right zone, invalid batch - wrong update old", + expectError: true, + zone: "org", + init: init, + changes: &plan.Changes{ + Create: []*endpoint.Endpoint{}, + UpdateNew: []*endpoint.Endpoint{}, + UpdateOld: []*endpoint.Endpoint{ + { + DNSName: "new.org", + Targets: endpoint.Targets{"8.8.8.8"}, + RecordType: endpoint.RecordTypeA, + }, + }, + Delete: []*endpoint.Endpoint{}, + }, + errorType: ErrRecordNotFound, + }, + { + title: "zones, update, right zone, invalid batch - wrong delete", + expectError: true, + zone: "org", + init: init, + changes: &plan.Changes{ + Create: []*endpoint.Endpoint{}, + UpdateNew: []*endpoint.Endpoint{}, + UpdateOld: []*endpoint.Endpoint{}, + Delete: []*endpoint.Endpoint{ + { + DNSName: "new.org", + Targets: endpoint.Targets{"8.8.8.8"}, + RecordType: endpoint.RecordTypeA, + }, + }, + }, + errorType: ErrRecordNotFound, + }, + { + title: "zones, update, right zone, valid batch - delete", + expectError: false, + zone: "org", + init: init, + changes: &plan.Changes{ + Create: []*endpoint.Endpoint{}, + UpdateNew: []*endpoint.Endpoint{}, + UpdateOld: []*endpoint.Endpoint{}, + Delete: []*endpoint.Endpoint{ + { + DNSName: "foo.bar.org", + Targets: endpoint.Targets{"5.5.5.5"}, + RecordType: endpoint.RecordTypeA, + }, + }, + }, + }, + { + title: "zones, update, right zone, valid batch - update and create", + expectError: false, + zone: "org", + init: init, + changes: &plan.Changes{ + Create: []*endpoint.Endpoint{ + { + DNSName: "foo.bar.new.org", + Targets: endpoint.Targets{"4.8.8.9"}, + RecordType: endpoint.RecordTypeA, + }, + }, + UpdateNew: []*endpoint.Endpoint{ + { + DNSName: "foo.bar.org", + Targets: endpoint.Targets{"4.8.8.4"}, + RecordType: endpoint.RecordTypeA, + }, + }, + UpdateOld: []*endpoint.Endpoint{ + { + DNSName: "foo.bar.org", + Targets: endpoint.Targets{"5.5.5.5"}, + RecordType: endpoint.RecordTypeA, + }, + }, + Delete: []*endpoint.Endpoint{}, + }, + }, + } { + t.Run(ti.title, func(t *testing.T) { + c := &inMemoryClient{} + c.zones = ti.init + ichanges := &plan.Changes{ + Create: ti.changes.Create, + UpdateNew: ti.changes.UpdateNew, + UpdateOld: ti.changes.UpdateOld, + Delete: ti.changes.Delete, + } + err := c.validateChangeBatch(ti.zone, ichanges) + if ti.expectError { + assert.EqualError(t, err, ti.errorType.Error()) + } else { + assert.NoError(t, err) + } + }) + } +} + +func getInitData() map[string]zone { + return map[string]zone{ + "org": makeZone("example.org", "8.8.8.8", endpoint.RecordTypeA, + "example.org", "", endpoint.RecordTypeTXT, + "foo.org", "4.4.4.4", endpoint.RecordTypeCNAME, + "foo.bar.org", "5.5.5.5", endpoint.RecordTypeA, + ), + "com": makeZone("example.com", "4.4.4.4", endpoint.RecordTypeCNAME), + } +} + +func testInMemoryApplyChanges(t *testing.T) { + for _, ti := range []struct { + title string + expectError bool + init map[string]zone + changes *plan.Changes + expectedZonesState map[string]zone + }{ + { + title: "unmatched zone, should be ignored in the apply step", + expectError: false, + changes: &plan.Changes{ + Create: []*endpoint.Endpoint{{ + DNSName: "example.de", + Targets: endpoint.Targets{"8.8.8.8"}, + RecordType: endpoint.RecordTypeA, + }}, + UpdateNew: []*endpoint.Endpoint{}, + UpdateOld: []*endpoint.Endpoint{}, + Delete: []*endpoint.Endpoint{}, + }, + expectedZonesState: getInitData(), + }, + { + title: "expect error", + expectError: true, + changes: &plan.Changes{ + Create: []*endpoint.Endpoint{}, + UpdateNew: []*endpoint.Endpoint{ + { + DNSName: "example.org", + Targets: endpoint.Targets{"8.8.8.8"}, + RecordType: endpoint.RecordTypeA, + }, + }, + UpdateOld: []*endpoint.Endpoint{}, + Delete: []*endpoint.Endpoint{ + { + DNSName: "example.org", + Targets: endpoint.Targets{"8.8.8.8"}, + RecordType: endpoint.RecordTypeA, + }, + }, + }, + }, + { + title: "zones, update, right zone, valid batch - delete", + expectError: false, + changes: &plan.Changes{ + Create: []*endpoint.Endpoint{}, + UpdateNew: []*endpoint.Endpoint{}, + UpdateOld: []*endpoint.Endpoint{}, + Delete: []*endpoint.Endpoint{ + { + DNSName: "foo.bar.org", + Targets: endpoint.Targets{"5.5.5.5"}, + RecordType: endpoint.RecordTypeA, + }, + }, + }, + expectedZonesState: map[string]zone{ + "org": makeZone("example.org", "8.8.8.8", endpoint.RecordTypeA, + "example.org", "", endpoint.RecordTypeTXT, + "foo.org", "4.4.4.4", endpoint.RecordTypeCNAME, + ), + "com": makeZone("example.com", "4.4.4.4", endpoint.RecordTypeCNAME), + }, + }, + { + title: "zones, update, right zone, valid batch - update, create, delete", + expectError: false, + changes: &plan.Changes{ + Create: []*endpoint.Endpoint{ + { + DNSName: "foo.bar.new.org", + Targets: endpoint.Targets{"4.8.8.9"}, + RecordType: endpoint.RecordTypeA, + Labels: endpoint.NewLabels(), + }, + }, + UpdateNew: []*endpoint.Endpoint{ + { + DNSName: "foo.bar.org", + Targets: endpoint.Targets{"4.8.8.4"}, + RecordType: endpoint.RecordTypeA, + Labels: endpoint.NewLabels(), + }, + }, + UpdateOld: []*endpoint.Endpoint{ + { + DNSName: "foo.bar.org", + Targets: endpoint.Targets{"5.5.5.5"}, + RecordType: endpoint.RecordTypeA, + Labels: endpoint.NewLabels(), + }, + }, + Delete: []*endpoint.Endpoint{ + { + DNSName: "example.org", + Targets: endpoint.Targets{"8.8.8.8"}, + RecordType: endpoint.RecordTypeA, + Labels: endpoint.NewLabels(), + }, + }, + }, + expectedZonesState: map[string]zone{ + "org": makeZone( + "example.org", "", endpoint.RecordTypeTXT, + "foo.org", "4.4.4.4", endpoint.RecordTypeCNAME, + "foo.bar.org", "4.8.8.4", endpoint.RecordTypeA, + "foo.bar.new.org", "4.8.8.9", endpoint.RecordTypeA, + ), + "com": makeZone("example.com", "4.4.4.4", endpoint.RecordTypeCNAME), + }, + }, + } { + t.Run(ti.title, func(t *testing.T) { + im := NewInMemoryProvider() + c := &inMemoryClient{} + c.zones = getInitData() + im.client = c + + err := im.ApplyChanges(context.Background(), ti.changes) + if ti.expectError { + assert.Error(t, err) + } else { + require.NoError(t, err) + assert.Equal(t, ti.expectedZonesState, c.zones) + } + }) + } +} + +func testNewInMemoryProvider(t *testing.T) { + cfg := NewInMemoryProvider() + assert.NotNil(t, cfg.client) +} + +func testInMemoryCreateZone(t *testing.T) { + im := NewInMemoryProvider() + err := im.CreateZone("zone") + assert.NoError(t, err) + err = im.CreateZone("zone") + assert.EqualError(t, err, ErrZoneAlreadyExists.Error()) +} + +func makeZone(s ...string) map[endpoint.EndpointKey]*endpoint.Endpoint { + if len(s)%3 != 0 { + panic("makeZone arguments must be multiple of 3") + } + + output := map[endpoint.EndpointKey]*endpoint.Endpoint{} + for i := 0; i < len(s); i += 3 { + ep := endpoint.NewEndpoint(s[i], s[i+2], s[i+1]) + output[ep.Key()] = ep + } + + return output +} From ecc38e22476407aebded6625a0de884375ef7335 Mon Sep 17 00:00:00 2001 From: Michael Nairn Date: Wed, 1 May 2024 10:28:16 +0100 Subject: [PATCH 2/4] provider: Add inmemory provider Adds a new provider `inmemory` for testing purposes which wraps the external dns `inmemory` provider https://github.com/kubernetes-sigs/external-dns/tree/master/provider/inmemory Also adds the inmemory provider to the provider factory allowing it to be used via the creation of a ManagedZone. ``` apiVersion: v1 kind: Secret metadata: name: inmemory-credentials type: kuadrant.io/inmemory data: {} --- apiVersion: kuadrant.io/v1alpha1 kind: ManagedZone metadata: name: example.com spec: domainName: example.com description: "example.com managed domain" dnsProviderSecretRef: name: inmemory-credentials ``` The external-dns provider has been modified slightly to work for some of our testing scenarios. * Add delete zone (Removes the key from the zones map) * Exposes the client so we can change it when loading a new provider instance. Allows us to share the same zone map for all providers loaded via the factory. --- cmd/main.go | 1 + .../provider/inmemory/inmemory.go | 58 ++++++++++----- .../provider/inmemory/inmemory_test.go | 71 +++++++++--------- internal/provider/factory.go | 2 + internal/provider/inmemory/inmemory.go | 72 +++++++++++++++++++ 5 files changed, 150 insertions(+), 54 deletions(-) create mode 100644 internal/provider/inmemory/inmemory.go diff --git a/cmd/main.go b/cmd/main.go index c5e1fcf3..f47b2d32 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -36,6 +36,7 @@ import ( "github.com/kuadrant/dns-operator/internal/provider" _ "github.com/kuadrant/dns-operator/internal/provider/aws" _ "github.com/kuadrant/dns-operator/internal/provider/google" + _ "github.com/kuadrant/dns-operator/internal/provider/inmemory" //+kubebuilder:scaffold:imports ) diff --git a/internal/external-dns/provider/inmemory/inmemory.go b/internal/external-dns/provider/inmemory/inmemory.go index eab515a6..935c971f 100644 --- a/internal/external-dns/provider/inmemory/inmemory.go +++ b/internal/external-dns/provider/inmemory/inmemory.go @@ -22,18 +22,18 @@ import ( "strings" log "github.com/sirupsen/logrus" - "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/util/sets" "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/plan" "sigs.k8s.io/external-dns/provider" ) var ( - // ErrZoneAlreadyExists error returned when zone cannot be created when it already exists - ErrZoneAlreadyExists = errors.New("specified zone already exists") - // ErrZoneNotFound error returned when specified zone does not exists - ErrZoneNotFound = errors.New("specified zone not found") + // ErrZoneAlreadyExists error returned when Zone cannot be created when it already exists + ErrZoneAlreadyExists = errors.New("specified Zone already exists") + // ErrZoneNotFound error returned when specified Zone does not exists + ErrZoneNotFound = errors.New("specified Zone not found") // ErrRecordAlreadyExists when create request is sent but record already exists ErrRecordAlreadyExists = errors.New("record already exists") // ErrRecordNotFound when update/delete request is sent but record not found @@ -47,7 +47,7 @@ var ( type InMemoryProvider struct { provider.BaseProvider domain endpoint.DomainFilter - client *inMemoryClient + client *InMemoryClient filter *filter OnApplyChanges func(ctx context.Context, changes *plan.Changes) OnRecords func() @@ -94,6 +94,13 @@ func InMemoryInitZones(zones []string) InMemoryOption { } } +// InMemoryWithClient modifies the client which the provider will use +func InMemoryWithClient(memoryClient *InMemoryClient) InMemoryOption { + return func(p *InMemoryProvider) { + p.client = memoryClient + } +} + // NewInMemoryProvider returns InMemoryProvider DNS provider interface implementation func NewInMemoryProvider(opts ...InMemoryOption) *InMemoryProvider { im := &InMemoryProvider{ @@ -101,7 +108,7 @@ func NewInMemoryProvider(opts ...InMemoryOption) *InMemoryProvider { OnApplyChanges: func(ctx context.Context, changes *plan.Changes) {}, OnRecords: func() {}, domain: endpoint.NewDomainFilter([]string{""}), - client: newInMemoryClient(), + client: NewInMemoryClient(), } for _, opt := range opts { @@ -111,11 +118,16 @@ func NewInMemoryProvider(opts ...InMemoryOption) *InMemoryProvider { return im } -// CreateZone adds new zone if not present +// CreateZone adds new Zone if not present func (im *InMemoryProvider) CreateZone(newZone string) error { return im.client.CreateZone(newZone) } +// DeleteZone deletes a Zone if present +func (im *InMemoryProvider) DeleteZone(zone string) error { + return im.client.DeleteZone(zone) +} + // Zones returns filtered zones as specified by domain func (im *InMemoryProvider) Zones() map[string]string { return im.filter.Zones(im.client.Zones()) @@ -241,17 +253,17 @@ func (f *filter) EndpointZoneID(endpoint *endpoint.Endpoint, zones map[string]st return matchZoneID } -type zone map[endpoint.EndpointKey]*endpoint.Endpoint +type Zone map[endpoint.EndpointKey]*endpoint.Endpoint -type inMemoryClient struct { - zones map[string]zone +type InMemoryClient struct { + zones map[string]Zone } -func newInMemoryClient() *inMemoryClient { - return &inMemoryClient{map[string]zone{}} +func NewInMemoryClient() *InMemoryClient { + return &InMemoryClient{map[string]Zone{}} } -func (c *inMemoryClient) Records(zone string) ([]*endpoint.Endpoint, error) { +func (c *InMemoryClient) Records(zone string) ([]*endpoint.Endpoint, error) { if _, ok := c.zones[zone]; !ok { return nil, ErrZoneNotFound } @@ -263,7 +275,7 @@ func (c *inMemoryClient) Records(zone string) ([]*endpoint.Endpoint, error) { return records, nil } -func (c *inMemoryClient) Zones() map[string]string { +func (c *InMemoryClient) Zones() map[string]string { zones := map[string]string{} for zone := range c.zones { zones[zone] = zone @@ -271,7 +283,7 @@ func (c *inMemoryClient) Zones() map[string]string { return zones } -func (c *inMemoryClient) CreateZone(zone string) error { +func (c *InMemoryClient) CreateZone(zone string) error { if _, ok := c.zones[zone]; ok { return ErrZoneAlreadyExists } @@ -280,7 +292,15 @@ func (c *inMemoryClient) CreateZone(zone string) error { return nil } -func (c *inMemoryClient) ApplyChanges(ctx context.Context, zoneID string, changes *plan.Changes) error { +func (c *InMemoryClient) DeleteZone(zone string) error { + if _, ok := c.zones[zone]; ok { + delete(c.zones, zone) + return nil + } + return ErrZoneNotFound +} + +func (c *InMemoryClient) ApplyChanges(ctx context.Context, zoneID string, changes *plan.Changes) error { if err := c.validateChangeBatch(zoneID, changes); err != nil { return err } @@ -296,7 +316,7 @@ func (c *inMemoryClient) ApplyChanges(ctx context.Context, zoneID string, change return nil } -func (c *inMemoryClient) updateMesh(mesh sets.Set[endpoint.EndpointKey], record *endpoint.Endpoint) error { +func (c *InMemoryClient) updateMesh(mesh sets.Set[endpoint.EndpointKey], record *endpoint.Endpoint) error { if mesh.Has(record.Key()) { return ErrDuplicateRecordFound } @@ -305,7 +325,7 @@ func (c *inMemoryClient) updateMesh(mesh sets.Set[endpoint.EndpointKey], record } // validateChangeBatch validates that the changes passed to InMemory DNS provider is valid -func (c *inMemoryClient) validateChangeBatch(zone string, changes *plan.Changes) error { +func (c *InMemoryClient) validateChangeBatch(zone string, changes *plan.Changes) error { curZone, ok := c.zones[zone] if !ok { return ErrZoneNotFound diff --git a/internal/external-dns/provider/inmemory/inmemory_test.go b/internal/external-dns/provider/inmemory/inmemory_test.go index 7028744b..fb1abcde 100644 --- a/internal/external-dns/provider/inmemory/inmemory_test.go +++ b/internal/external-dns/provider/inmemory/inmemory_test.go @@ -22,6 +22,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/plan" "sigs.k8s.io/external-dns/provider" @@ -44,28 +45,28 @@ func testInMemoryRecords(t *testing.T) { title string zone string expectError bool - init map[string]zone + init map[string]Zone expected []*endpoint.Endpoint }{ { - title: "no records, no zone", + title: "no records, no Zone", zone: "", - init: map[string]zone{}, + init: map[string]Zone{}, expectError: false, }, { - title: "records, wrong zone", + title: "records, wrong Zone", zone: "net", - init: map[string]zone{ + init: map[string]Zone{ "org": {}, "com": {}, }, expectError: false, }, { - title: "records, zone with records", + title: "records, Zone with records", zone: "org", - init: map[string]zone{ + init: map[string]Zone{ "org": makeZone( "example.org", "8.8.8.8", endpoint.RecordTypeA, "example.org", "", endpoint.RecordTypeTXT, @@ -94,7 +95,7 @@ func testInMemoryRecords(t *testing.T) { }, } { t.Run(ti.title, func(t *testing.T) { - c := newInMemoryClient() + c := NewInMemoryClient() c.zones = ti.init im := NewInMemoryProvider() im.client = c @@ -113,7 +114,7 @@ func testInMemoryRecords(t *testing.T) { } func testInMemoryValidateChangeBatch(t *testing.T) { - init := map[string]zone{ + init := map[string]Zone{ "org": makeZone( "example.org", "8.8.8.8", endpoint.RecordTypeA, "example.org", "", endpoint.RecordTypeTXT, @@ -126,7 +127,7 @@ func testInMemoryValidateChangeBatch(t *testing.T) { title string expectError bool errorType error - init map[string]zone + init map[string]Zone changes *plan.Changes zone string }{ @@ -134,7 +135,7 @@ func testInMemoryValidateChangeBatch(t *testing.T) { title: "no zones, no update", expectError: true, zone: "", - init: map[string]zone{}, + init: map[string]Zone{}, changes: &plan.Changes{ Create: []*endpoint.Endpoint{}, UpdateNew: []*endpoint.Endpoint{}, @@ -157,7 +158,7 @@ func testInMemoryValidateChangeBatch(t *testing.T) { errorType: ErrZoneNotFound, }, { - title: "zones, update, wrong zone", + title: "zones, update, wrong Zone", expectError: true, zone: "test", init: init, @@ -170,7 +171,7 @@ func testInMemoryValidateChangeBatch(t *testing.T) { errorType: ErrZoneNotFound, }, { - title: "zones, update, right zone, invalid batch - already exists", + title: "zones, update, right Zone, invalid batch - already exists", expectError: true, zone: "org", init: init, @@ -189,7 +190,7 @@ func testInMemoryValidateChangeBatch(t *testing.T) { errorType: ErrRecordAlreadyExists, }, { - title: "zones, update, right zone, invalid batch - record not found for update", + title: "zones, update, right Zone, invalid batch - record not found for update", expectError: true, zone: "org", init: init, @@ -214,7 +215,7 @@ func testInMemoryValidateChangeBatch(t *testing.T) { errorType: ErrRecordNotFound, }, { - title: "zones, update, right zone, invalid batch - record not found for update", + title: "zones, update, right Zone, invalid batch - record not found for update", expectError: true, zone: "org", init: init, @@ -239,7 +240,7 @@ func testInMemoryValidateChangeBatch(t *testing.T) { errorType: ErrRecordNotFound, }, { - title: "zones, update, right zone, invalid batch - duplicated create", + title: "zones, update, right Zone, invalid batch - duplicated create", expectError: true, zone: "org", init: init, @@ -263,7 +264,7 @@ func testInMemoryValidateChangeBatch(t *testing.T) { errorType: ErrDuplicateRecordFound, }, { - title: "zones, update, right zone, invalid batch - duplicated update/delete", + title: "zones, update, right Zone, invalid batch - duplicated update/delete", expectError: true, zone: "org", init: init, @@ -288,7 +289,7 @@ func testInMemoryValidateChangeBatch(t *testing.T) { errorType: ErrDuplicateRecordFound, }, { - title: "zones, update, right zone, invalid batch - duplicated update", + title: "zones, update, right Zone, invalid batch - duplicated update", expectError: true, zone: "org", init: init, @@ -312,7 +313,7 @@ func testInMemoryValidateChangeBatch(t *testing.T) { errorType: ErrDuplicateRecordFound, }, { - title: "zones, update, right zone, invalid batch - wrong update old", + title: "zones, update, right Zone, invalid batch - wrong update old", expectError: true, zone: "org", init: init, @@ -331,7 +332,7 @@ func testInMemoryValidateChangeBatch(t *testing.T) { errorType: ErrRecordNotFound, }, { - title: "zones, update, right zone, invalid batch - wrong delete", + title: "zones, update, right Zone, invalid batch - wrong delete", expectError: true, zone: "org", init: init, @@ -350,7 +351,7 @@ func testInMemoryValidateChangeBatch(t *testing.T) { errorType: ErrRecordNotFound, }, { - title: "zones, update, right zone, valid batch - delete", + title: "zones, update, right Zone, valid batch - delete", expectError: false, zone: "org", init: init, @@ -368,7 +369,7 @@ func testInMemoryValidateChangeBatch(t *testing.T) { }, }, { - title: "zones, update, right zone, valid batch - update and create", + title: "zones, update, right Zone, valid batch - update and create", expectError: false, zone: "org", init: init, @@ -399,7 +400,7 @@ func testInMemoryValidateChangeBatch(t *testing.T) { }, } { t.Run(ti.title, func(t *testing.T) { - c := &inMemoryClient{} + c := &InMemoryClient{} c.zones = ti.init ichanges := &plan.Changes{ Create: ti.changes.Create, @@ -417,8 +418,8 @@ func testInMemoryValidateChangeBatch(t *testing.T) { } } -func getInitData() map[string]zone { - return map[string]zone{ +func getInitData() map[string]Zone { + return map[string]Zone{ "org": makeZone("example.org", "8.8.8.8", endpoint.RecordTypeA, "example.org", "", endpoint.RecordTypeTXT, "foo.org", "4.4.4.4", endpoint.RecordTypeCNAME, @@ -432,12 +433,12 @@ func testInMemoryApplyChanges(t *testing.T) { for _, ti := range []struct { title string expectError bool - init map[string]zone + init map[string]Zone changes *plan.Changes - expectedZonesState map[string]zone + expectedZonesState map[string]Zone }{ { - title: "unmatched zone, should be ignored in the apply step", + title: "unmatched Zone, should be ignored in the apply step", expectError: false, changes: &plan.Changes{ Create: []*endpoint.Endpoint{{ @@ -474,7 +475,7 @@ func testInMemoryApplyChanges(t *testing.T) { }, }, { - title: "zones, update, right zone, valid batch - delete", + title: "zones, update, right Zone, valid batch - delete", expectError: false, changes: &plan.Changes{ Create: []*endpoint.Endpoint{}, @@ -488,7 +489,7 @@ func testInMemoryApplyChanges(t *testing.T) { }, }, }, - expectedZonesState: map[string]zone{ + expectedZonesState: map[string]Zone{ "org": makeZone("example.org", "8.8.8.8", endpoint.RecordTypeA, "example.org", "", endpoint.RecordTypeTXT, "foo.org", "4.4.4.4", endpoint.RecordTypeCNAME, @@ -497,7 +498,7 @@ func testInMemoryApplyChanges(t *testing.T) { }, }, { - title: "zones, update, right zone, valid batch - update, create, delete", + title: "zones, update, right Zone, valid batch - update, create, delete", expectError: false, changes: &plan.Changes{ Create: []*endpoint.Endpoint{ @@ -533,7 +534,7 @@ func testInMemoryApplyChanges(t *testing.T) { }, }, }, - expectedZonesState: map[string]zone{ + expectedZonesState: map[string]Zone{ "org": makeZone( "example.org", "", endpoint.RecordTypeTXT, "foo.org", "4.4.4.4", endpoint.RecordTypeCNAME, @@ -546,7 +547,7 @@ func testInMemoryApplyChanges(t *testing.T) { } { t.Run(ti.title, func(t *testing.T) { im := NewInMemoryProvider() - c := &inMemoryClient{} + c := &InMemoryClient{} c.zones = getInitData() im.client = c @@ -568,9 +569,9 @@ func testNewInMemoryProvider(t *testing.T) { func testInMemoryCreateZone(t *testing.T) { im := NewInMemoryProvider() - err := im.CreateZone("zone") + err := im.CreateZone("Zone") assert.NoError(t, err) - err = im.CreateZone("zone") + err = im.CreateZone("Zone") assert.EqualError(t, err, ErrZoneAlreadyExists.Error()) } diff --git a/internal/provider/factory.go b/internal/provider/factory.go index 1597fcd1..acda562b 100644 --- a/internal/provider/factory.go +++ b/internal/provider/factory.go @@ -82,6 +82,8 @@ func nameForProviderSecret(secret *v1.Secret) (string, error) { return "aws", nil case "kuadrant.io/gcp": return "google", nil + case "kuadrant.io/inmemory": + return "inmemory", nil } return "", errUnsupportedProvider } diff --git a/internal/provider/inmemory/inmemory.go b/internal/provider/inmemory/inmemory.go new file mode 100644 index 00000000..58e3eb38 --- /dev/null +++ b/internal/provider/inmemory/inmemory.go @@ -0,0 +1,72 @@ +/* +Copyright 2024. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package inmemory + +import ( + "context" + + v1 "k8s.io/api/core/v1" + + "github.com/kuadrant/dns-operator/api/v1alpha1" + "github.com/kuadrant/dns-operator/internal/external-dns/provider/inmemory" + "github.com/kuadrant/dns-operator/internal/provider" +) + +type InMemoryDNSProvider struct { + *inmemory.InMemoryProvider + ctx context.Context +} + +var client *inmemory.InMemoryClient + +var _ provider.Provider = &InMemoryDNSProvider{} + +func NewProviderFromSecret(ctx context.Context, _ *v1.Secret, c provider.Config) (provider.Provider, error) { + inmemoryProvider := inmemory.NewInMemoryProvider(inmemory.InMemoryWithClient(client), inmemory.InMemoryWithDomain(c.DomainFilter), inmemory.InMemoryWithLogging()) + p := &InMemoryDNSProvider{ + InMemoryProvider: inmemoryProvider, + ctx: ctx, + } + return p, nil +} + +func (i InMemoryDNSProvider) EnsureManagedZone(managedZone *v1alpha1.ManagedZone) (provider.ManagedZoneOutput, error) { + _ = i.CreateZone(managedZone.Spec.DomainName) + return provider.ManagedZoneOutput{ + ID: managedZone.Spec.DomainName, + NameServers: nil, + RecordCount: 0, + }, nil +} + +func (i InMemoryDNSProvider) DeleteManagedZone(managedZone *v1alpha1.ManagedZone) error { + return i.DeleteZone(managedZone.Spec.DomainName) +} + +func (i InMemoryDNSProvider) HealthCheckReconciler() provider.HealthCheckReconciler { + return &provider.FakeHealthCheckReconciler{} +} + +func (i InMemoryDNSProvider) ProviderSpecific() provider.ProviderSpecificLabels { + return provider.ProviderSpecificLabels{} +} + +// Register this Provider with the provider factory +func init() { + client = inmemory.NewInMemoryClient() + provider.RegisterProvider("inmemory", NewProviderFromSecret) +} From 0fc1ff55c4b1c438c6c0bc610e6289349e65e651 Mon Sep 17 00:00:00 2001 From: Michael Nairn Date: Wed, 1 May 2024 10:29:36 +0100 Subject: [PATCH 3/4] plan: update owner check Checks the owner is not set to an empty string when checking ownership of endpoints. --- internal/external-dns/plan/plan.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/external-dns/plan/plan.go b/internal/external-dns/plan/plan.go index ef96c044..2f332f8f 100644 --- a/internal/external-dns/plan/plan.go +++ b/internal/external-dns/plan/plan.go @@ -286,7 +286,7 @@ func (p *Plan) Calculate() *Plan { candidate := t.resolver.ResolveUpdate(records.current, records.candidates) current := records.current.DeepCopy() owners := []string{} - if endpointOwner, hasOwner := current.Labels[endpoint.OwnerLabelKey]; hasOwner { + if endpointOwner, hasOwner := current.Labels[endpoint.OwnerLabelKey]; hasOwner && endpointOwner != "" { if p.OwnerID == "" { // Only allow owned records to be updated by other owned records errs = append(errs, fmt.Errorf("%w, cannot update endpoint '%s' with no owner when existing endpoint is already owned", ErrOwnerConflict, candidate.DNSName)) From 57d0cab6d44c030aad8e21cd71c3503417d9db51 Mon Sep 17 00:00:00 2001 From: Michael Nairn Date: Wed, 1 May 2024 10:32:48 +0100 Subject: [PATCH 4/4] tests: Update integration test suite Update integration test suite to use the `inmemory` provider. Each test case must now create and cleanup all dnsrecords it requires for testing. Use ginkgo cli for executing integration tests in Makefile (make test-integration). --- Makefile | 4 +- .../controller/dnsrecord_controller_test.go | 150 ++++++++++++++++-- internal/controller/helper_test.go | 2 +- internal/controller/suite_test.go | 31 ++-- 4 files changed, 153 insertions(+), 34 deletions(-) diff --git a/Makefile b/Makefile index 47718113..d0289a0b 100644 --- a/Makefile +++ b/Makefile @@ -140,8 +140,8 @@ test-unit: manifests generate fmt vet ## Run unit tests. KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test ./... -tags=unit -coverprofile cover-unit.out .PHONY: test-integration -test-integration: manifests generate fmt vet envtest ## Run integration tests. - KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test ./internal/controller... -tags=integration -coverprofile cover-integration.out +test-integration: manifests generate fmt vet envtest ginkgo ## Run integration tests. + KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" $(GINKGO) -tags=integration ./internal/controller -coverprofile cover-integration.out .PHONY: test-e2e test-e2e: ginkgo diff --git a/internal/controller/dnsrecord_controller_test.go b/internal/controller/dnsrecord_controller_test.go index e88d2e6b..28885e7d 100644 --- a/internal/controller/dnsrecord_controller_test.go +++ b/internal/controller/dnsrecord_controller_test.go @@ -45,6 +45,17 @@ var _ = Describe("DNSRecordReconciler", func() { managedZone = testBuildManagedZone("mz-example-com", testNamespace, "example.com") Expect(k8sClient.Create(ctx, managedZone)).To(Succeed()) + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(managedZone), managedZone) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(managedZone.Status.Conditions).To( + ContainElement(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(string(v1alpha1.ConditionTypeReady)), + "Status": Equal(metav1.ConditionTrue), + "ObservedGeneration": Equal(managedZone.Generation), + })), + ) + }, TestTimeoutMedium, time.Second).Should(Succeed()) dnsRecord = &v1alpha1.DNSRecord{ ObjectMeta: metav1.ObjectMeta{ @@ -135,8 +146,7 @@ var _ = Describe("DNSRecordReconciler", func() { }) It("should not allow ownerID to be updated once set", func() { - Expect(k8sClient.Create(ctx, dnsRecord)).To(BeNil()) - + Expect(k8sClient.Create(ctx, dnsRecord)).To(Succeed()) Eventually(func(g Gomega) { err := k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsRecord), dnsRecord) g.Expect(err).NotTo(HaveOccurred()) @@ -180,33 +190,123 @@ var _ = Describe("DNSRecordReconciler", func() { }) It("should increase write counter if fail to publish record or record is overridden", func() { - dnsRecord.Spec.Endpoints = getTestNonExistingEndpoints() - Expect(k8sClient.Create(ctx, dnsRecord)).To(Succeed()) + dnsRecord = &v1alpha1.DNSRecord{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-record-1", + Namespace: testNamespace, + }, + Spec: v1alpha1.DNSRecordSpec{ + ManagedZoneRef: &v1alpha1.ManagedZoneReference{ + Name: managedZone.Name, + }, + Endpoints: []*externaldnsendpoint.Endpoint{ + { + DNSName: "foo.example.com", + Targets: []string{ + "127.0.0.1", + }, + RecordType: "A", + SetIdentifier: "", + RecordTTL: 60, + Labels: nil, + ProviderSpecific: nil, + }, + }, + }, + } + dnsRecord2 = &v1alpha1.DNSRecord{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-record-2", + Namespace: testNamespace, + }, + Spec: v1alpha1.DNSRecordSpec{ + ManagedZoneRef: &v1alpha1.ManagedZoneReference{ + Name: managedZone.Name, + }, + Endpoints: []*externaldnsendpoint.Endpoint{ + { + DNSName: "foo.example.com", + Targets: []string{ + "127.0.0.2", + }, + RecordType: "A", + SetIdentifier: "", + RecordTTL: 60, + Labels: nil, + ProviderSpecific: nil, + }, + }, + }, + } - // should be requeue record for validation after the write attempt + By("creating dnsrecord " + dnsRecord.Name + " with endpoint dnsName: `foo.example.com` and target: `127.0.0.1`") + Expect(k8sClient.Create(ctx, dnsRecord)).To(Succeed()) + By("checking dnsrecord " + dnsRecord.Name + " becomes ready") Eventually(func(g Gomega) { err := k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsRecord), dnsRecord) g.Expect(err).NotTo(HaveOccurred()) g.Expect(dnsRecord.Status.Conditions).To( ContainElement(MatchFields(IgnoreExtras, Fields{ "Type": Equal(string(v1alpha1.ConditionTypeReady)), - "Status": Equal(metav1.ConditionFalse), - "Reason": Equal("AwaitingValidation"), - "Message": Equal("Awaiting validation"), + "Status": Equal(metav1.ConditionTrue), + "Reason": Equal("ProviderSuccess"), + "Message": Equal("Provider ensured the dns record"), "ObservedGeneration": Equal(dnsRecord.Generation), })), ) + g.Expect(dnsRecord.Finalizers).To(ContainElement(DNSRecordFinalizer)) }, TestTimeoutMedium, time.Second).Should(Succeed()) - // should be increasing write counter + By("creating dnsrecord " + dnsRecord2.Name + " with endpoint dnsName: `foo.example.com` and target: `127.0.0.2`") + Expect(k8sClient.Create(ctx, dnsRecord2)).To(Succeed()) + + By("checking dnsrecord " + dnsRecord.Name + " and " + dnsRecord2.Name + " conflict") Eventually(func(g Gomega) { err := k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsRecord), dnsRecord) g.Expect(err).NotTo(HaveOccurred()) + g.Expect(dnsRecord.Status.Conditions).To( + ContainElement(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(string(v1alpha1.ConditionTypeReady)), + "Status": Equal(metav1.ConditionFalse), + "Reason": Equal("AwaitingValidation"), + "Message": Equal("Awaiting validation"), + "ObservedGeneration": Equal(dnsRecord.Generation), + })), + ) g.Expect(dnsRecord.Status.WriteCounter).To(BeNumerically(">", int64(0))) + + err = k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsRecord2), dnsRecord2) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(dnsRecord2.Status.Conditions).To( + ContainElement(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(string(v1alpha1.ConditionTypeReady)), + "Status": Equal(metav1.ConditionFalse), + "Reason": Equal("AwaitingValidation"), + "Message": Equal("Awaiting validation"), + "ObservedGeneration": Equal(dnsRecord2.Generation), + })), + ) + g.Expect(dnsRecord2.Status.WriteCounter).To(BeNumerically(">", int64(0))) }, TestTimeoutLong, time.Second).Should(Succeed()) }) It("should not allow second record to change the type", func() { + Expect(k8sClient.Create(ctx, dnsRecord)).To(Succeed()) + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsRecord), dnsRecord) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(dnsRecord.Status.Conditions).To( + ContainElement(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(string(v1alpha1.ConditionTypeReady)), + "Status": Equal(metav1.ConditionTrue), + "Reason": Equal("ProviderSuccess"), + "Message": Equal("Provider ensured the dns record"), + "ObservedGeneration": Equal(dnsRecord.Generation), + })), + ) + g.Expect(dnsRecord.Finalizers).To(ContainElement(DNSRecordFinalizer)) + }, TestTimeoutMedium, time.Second).Should(Succeed()) + dnsRecord2 = &v1alpha1.DNSRecord{ ObjectMeta: metav1.ObjectMeta{ Name: "foo2.example.com", @@ -248,6 +348,22 @@ var _ = Describe("DNSRecordReconciler", func() { }) It("should not allow owned record to update it", func() { + Expect(k8sClient.Create(ctx, dnsRecord)).To(Succeed()) + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsRecord), dnsRecord) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(dnsRecord.Status.Conditions).To( + ContainElement(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(string(v1alpha1.ConditionTypeReady)), + "Status": Equal(metav1.ConditionTrue), + "Reason": Equal("ProviderSuccess"), + "Message": Equal("Provider ensured the dns record"), + "ObservedGeneration": Equal(dnsRecord.Generation), + })), + ) + g.Expect(dnsRecord.Finalizers).To(ContainElement(DNSRecordFinalizer)) + }, TestTimeoutMedium, time.Second).Should(Succeed()) + dnsRecord2 = &v1alpha1.DNSRecord{ ObjectMeta: metav1.ObjectMeta{ Name: "foo2.example.com", @@ -278,6 +394,22 @@ var _ = Describe("DNSRecordReconciler", func() { }) It("should not allow a record to have a target that matches the root host if an endpoint doesn't exist for the target dns name", func() { + Expect(k8sClient.Create(ctx, dnsRecord)).To(Succeed()) + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsRecord), dnsRecord) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(dnsRecord.Status.Conditions).To( + ContainElement(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(string(v1alpha1.ConditionTypeReady)), + "Status": Equal(metav1.ConditionTrue), + "Reason": Equal("ProviderSuccess"), + "Message": Equal("Provider ensured the dns record"), + "ObservedGeneration": Equal(dnsRecord.Generation), + })), + ) + g.Expect(dnsRecord.Finalizers).To(ContainElement(DNSRecordFinalizer)) + }, TestTimeoutMedium, time.Second).Should(Succeed()) + dnsRecord2 = &v1alpha1.DNSRecord{ ObjectMeta: metav1.ObjectMeta{ Name: "bar.example.com", diff --git a/internal/controller/helper_test.go b/internal/controller/helper_test.go index 1754ef24..c479ed41 100644 --- a/internal/controller/helper_test.go +++ b/internal/controller/helper_test.go @@ -12,7 +12,7 @@ import ( ) const ( - TestTimeoutMedium = time.Second * 10 + TestTimeoutMedium = time.Second * 15 TestTimeoutLong = time.Second * 30 TestRetryIntervalMedium = time.Millisecond * 250 RequeueDuration = time.Second * 6 diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 61e4cbd3..cb5269e1 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -27,6 +27,7 @@ import ( "github.com/google/uuid" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -39,20 +40,20 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" - externaldnsendpoint "sigs.k8s.io/external-dns/endpoint" - externaldnsplan "sigs.k8s.io/external-dns/plan" "github.com/kuadrant/dns-operator/api/v1alpha1" "github.com/kuadrant/dns-operator/internal/provider" _ "github.com/kuadrant/dns-operator/internal/provider/aws" providerFake "github.com/kuadrant/dns-operator/internal/provider/fake" _ "github.com/kuadrant/dns-operator/internal/provider/google" + "github.com/kuadrant/dns-operator/internal/provider/inmemory" //+kubebuilder:scaffold:imports ) // These tests use Ginkgo (BDD-style Go testing framework). Refer to // http://onsi.github.io/ginkgo/ to learn more about Ginkgo. +var dnsProvider provider.Provider var cfg *rest.Config var k8sClient client.Client var testEnv *envtest.Environment @@ -60,26 +61,7 @@ var ctx context.Context var cancel context.CancelFunc var dnsProviderFactory = &providerFake.Factory{ ProviderForFunc: func(ctx context.Context, pa v1alpha1.ProviderAccessor, c provider.Config) (provider.Provider, error) { - return &providerFake.Provider{ - RecordsFunc: func(context.Context) ([]*externaldnsendpoint.Endpoint, error) { - return getTestEndpoints(), nil - }, - ApplyChangesFunc: func(context.Context, *externaldnsplan.Changes) error { - return nil - }, - AdjustEndpointsFunc: func(eps []*externaldnsendpoint.Endpoint) ([]*externaldnsendpoint.Endpoint, error) { - return eps, nil - }, - GetDomainFilterFunc: func() externaldnsendpoint.DomainFilter { - return externaldnsendpoint.DomainFilter{} - }, - EnsureManagedZoneFunc: func(zone *v1alpha1.ManagedZone) (provider.ManagedZoneOutput, error) { - return provider.ManagedZoneOutput{}, nil - }, - DeleteManagedZoneFunc: func(zone *v1alpha1.ManagedZone) error { - return nil - }, - }, nil + return dnsProvider, nil }, } @@ -92,6 +74,8 @@ func TestControllers(t *testing.T) { var _ = BeforeSuite(func() { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) + logrus.SetLevel(logrus.ErrorLevel) + ctx, cancel = context.WithCancel(ctrl.SetupSignalHandler()) By("bootstrapping test environment") testEnv = &envtest.Environment{ @@ -135,6 +119,9 @@ var _ = BeforeSuite(func() { }).SetupWithManager(mgr, RequeueDuration, ValidityDuration) Expect(err).ToNot(HaveOccurred()) + dnsProvider, err = inmemory.NewProviderFromSecret(ctx, &v1.Secret{}, provider.Config{}) + Expect(err).ToNot(HaveOccurred()) + go func() { defer GinkgoRecover() err = mgr.Start(ctx)