From f11c37c2eefe0139115ce90eb49c33f1d70aea0f Mon Sep 17 00:00:00 2001 From: Yerken Date: Tue, 9 May 2017 13:19:47 +0200 Subject: [PATCH] refactor inmemory provider (#199) * reactor inmemoy provider * add inmemoryclient to logically split functions * implement apply changes * fix all tests * chore: use bogus value for zone to ensure it's ignored * chore: use bogus value for zone to ensure it's ignored (2) --- provider/inmemory.go | 289 ++++++++++++++++++-------- provider/inmemory_test.go | 413 ++++++++++++-------------------------- registry/noop_test.go | 35 ++-- registry/txt_test.go | 16 +- 4 files changed, 351 insertions(+), 402 deletions(-) diff --git a/provider/inmemory.go b/provider/inmemory.go index 08c218613..dc05f96db 100644 --- a/provider/inmemory.go +++ b/provider/inmemory.go @@ -18,6 +18,7 @@ package provider import ( "errors" + "strings" "github.com/kubernetes-incubator/external-dns/endpoint" "github.com/kubernetes-incubator/external-dns/plan" @@ -36,12 +37,12 @@ var ( ErrInvalidBatchRequest = errors.New("invalid batch request") ) -type zone map[string][]*InMemoryRecord - // InMemoryProvider - dns provider only used for testing purposes // initialized as dns provider with no records type InMemoryProvider struct { - zones map[string]zone + domain string + client *inMemoryClient + filter *filter OnApplyChanges func(changes *plan.Changes) OnRecords func() } @@ -49,39 +50,42 @@ type InMemoryProvider struct { // NewInMemoryProvider returns InMemoryProvider DNS provider interface implementation func NewInMemoryProvider() *InMemoryProvider { return &InMemoryProvider{ - zones: map[string]zone{}, + filter: &filter{}, OnApplyChanges: func(changes *plan.Changes) {}, OnRecords: func() {}, + domain: "", + client: newInMemoryClient(), } } -// InMemoryRecord - record stored in memory -// has additional fields: -// Type - type of string (TODO: Type should probably be part of endpoint struct) -// Payload - string - additional information stored -type InMemoryRecord struct { - Type string - Payload string - *endpoint.Endpoint -} - // CreateZone adds new zone if not present func (im *InMemoryProvider) CreateZone(newZone string) error { - if _, exist := im.zones[newZone]; exist { - return ErrZoneAlreadyExists - } - im.zones[newZone] = zone{} - return nil + 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(zone string) ([]*endpoint.Endpoint, error) { +func (im *InMemoryProvider) Records(_ string) ([]*endpoint.Endpoint, error) { defer im.OnRecords() - if _, exists := im.zones[zone]; !exists { - return nil, ErrZoneNotFound + endpoints := make([]*endpoint.Endpoint, 0) + + for zoneID := range im.Zones() { + records, err := im.client.Records(zoneID) + if err != nil { + return nil, err + } + + for _, record := range records { + endpoints = append(endpoints, endpoint.NewEndpoint(record.Name, record.Target, record.Type)) + } } - return im.endpoints(zone), nil + + return endpoints, nil } // ApplyChanges simply modifies records in memory @@ -89,97 +93,229 @@ func (im *InMemoryProvider) Records(zone string) ([]*endpoint.Endpoint, error) { // 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(zone string, changes *plan.Changes) error { +func (im *InMemoryProvider) ApplyChanges(_ string, changes *plan.Changes) error { defer im.OnApplyChanges(changes) - if err := im.validateChangeBatch(zone, changes); err != nil { - return err + perZoneChanges := map[string]*plan.Changes{} + + zones := im.Zones() + for zoneID := range zones { + perZoneChanges[zoneID] = &plan.Changes{} } - for _, newEndpoint := range changes.Create { - im.zones[zone][newEndpoint.DNSName] = append(im.zones[zone][newEndpoint.DNSName], &InMemoryRecord{ - Type: suitableType(newEndpoint), - Endpoint: newEndpoint, + for _, ep := range changes.Create { + zoneID := im.filter.EndpointZoneID(ep, zones) + perZoneChanges[zoneID].Create = append(perZoneChanges[zoneID].Create, ep) + } + for _, ep := range changes.UpdateNew { + zoneID := im.filter.EndpointZoneID(ep, zones) + perZoneChanges[zoneID].UpdateNew = append(perZoneChanges[zoneID].UpdateNew, ep) + } + for _, ep := range changes.UpdateOld { + zoneID := im.filter.EndpointZoneID(ep, zones) + perZoneChanges[zoneID].UpdateOld = append(perZoneChanges[zoneID].UpdateOld, ep) + } + for _, ep := range changes.Delete { + zoneID := im.filter.EndpointZoneID(ep, zones) + perZoneChanges[zoneID].Delete = append(perZoneChanges[zoneID].Delete, ep) + } + + for zoneID := range perZoneChanges { + change := &inMemoryChange{ + Create: convertToInMemoryRecord(perZoneChanges[zoneID].Create), + UpdateNew: convertToInMemoryRecord(perZoneChanges[zoneID].UpdateNew), + UpdateOld: convertToInMemoryRecord(perZoneChanges[zoneID].UpdateOld), + Delete: convertToInMemoryRecord(perZoneChanges[zoneID].Delete), + } + err := im.client.ApplyChanges(zoneID, change) + if err != nil { + return err + } + } + + return nil +} + +func convertToInMemoryRecord(endpoints []*endpoint.Endpoint) []*inMemoryRecord { + records := []*inMemoryRecord{} + for _, ep := range endpoints { + records = append(records, &inMemoryRecord{ + Type: suitableType(ep), + Name: ep.DNSName, + Target: ep.Target, }) } + 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 +} + +// inMemoryRecord - record stored in memory +// Type - type of string +// Name - DNS name assigned to the record +// Target - target of the record +// Payload - string - additional information stored +type inMemoryRecord struct { + Type string + Payload string + Name string + Target string +} + +type zone map[string][]*inMemoryRecord + +type inMemoryChange struct { + Create []*inMemoryRecord + UpdateNew []*inMemoryRecord + UpdateOld []*inMemoryRecord + Delete []*inMemoryRecord +} + +type inMemoryClient struct { + zones map[string]zone +} + +func newInMemoryClient() *inMemoryClient { + return &inMemoryClient{map[string]zone{}} +} + +func (c *inMemoryClient) Records(zone string) ([]*inMemoryRecord, error) { + if _, ok := c.zones[zone]; !ok { + return nil, ErrZoneNotFound + } + + records := []*inMemoryRecord{} + 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[string][]*inMemoryRecord{} + + return nil +} + +func (c *inMemoryClient) ApplyChanges(zoneID string, changes *inMemoryChange) error { + if err := c.validateChangeBatch(zoneID, changes); err != nil { + return err + } + for _, newEndpoint := range changes.Create { + if _, ok := c.zones[zoneID][newEndpoint.Name]; !ok { + c.zones[zoneID][newEndpoint.Name] = make([]*inMemoryRecord, 0) + } + c.zones[zoneID][newEndpoint.Name] = append(c.zones[zoneID][newEndpoint.Name], newEndpoint) + } for _, updateEndpoint := range changes.UpdateNew { - for _, curEndpoint := range changes.UpdateOld { - if curEndpoint.DNSName == updateEndpoint.DNSName && curEndpoint.RecordType == updateEndpoint.RecordType { - for _, recordToUpdate := range im.zones[zone][updateEndpoint.DNSName] { - if recordToUpdate.Target == curEndpoint.Target { - recordToUpdate.Target = updateEndpoint.Target - } - } + for _, rec := range c.zones[zoneID][updateEndpoint.Name] { + if rec.Type == updateEndpoint.Type { + rec.Target = updateEndpoint.Target + break } } } for _, deleteEndpoint := range changes.Delete { - newRecordSet := make([]*InMemoryRecord, 0) - for _, record := range im.zones[zone][deleteEndpoint.DNSName] { - if record.Type != suitableType(deleteEndpoint) { - newRecordSet = append(newRecordSet, record) + newSet := make([]*inMemoryRecord, 0) + for _, rec := range c.zones[zoneID][deleteEndpoint.Name] { + if rec.Type != deleteEndpoint.Type { + newSet = append(newSet, rec) } } - im.zones[zone][deleteEndpoint.DNSName] = newRecordSet + c.zones[zoneID][deleteEndpoint.Name] = newSet } return nil } +func (c *inMemoryClient) updateMesh(mesh map[string]map[string]bool, endpoint *inMemoryRecord) error { + if _, exists := mesh[endpoint.Name]; exists { + if mesh[endpoint.Name][endpoint.Type] { + return ErrInvalidBatchRequest + } + mesh[endpoint.Name][endpoint.Type] = true + return nil + } + mesh[endpoint.Name] = map[string]bool{endpoint.Type: true} + return nil +} + // validateChangeBatch validates that the changes passed to InMemory DNS provider is valid -func (im *InMemoryProvider) validateChangeBatch(zone string, changes *plan.Changes) error { - existing, ok := im.zones[zone] +func (c *inMemoryClient) validateChangeBatch(zone string, changes *inMemoryChange) error { + curZone, ok := c.zones[zone] if !ok { return ErrZoneNotFound } mesh := map[string]map[string]bool{} for _, newEndpoint := range changes.Create { - if im.findByType(suitableType(newEndpoint), existing[newEndpoint.DNSName]) != nil { + if c.findByType(newEndpoint.Type, curZone[newEndpoint.Name]) != nil { return ErrRecordAlreadyExists } - if _, exists := mesh[newEndpoint.DNSName]; exists { - if mesh[newEndpoint.DNSName][suitableType(newEndpoint)] { - return ErrInvalidBatchRequest - } - mesh[newEndpoint.DNSName][suitableType(newEndpoint)] = true - continue + if err := c.updateMesh(mesh, newEndpoint); err != nil { + return err } - mesh[newEndpoint.DNSName] = map[string]bool{suitableType(newEndpoint): true} } for _, updateEndpoint := range changes.UpdateNew { - if im.findByType(suitableType(updateEndpoint), existing[updateEndpoint.DNSName]) == nil { + if c.findByType(updateEndpoint.Type, curZone[updateEndpoint.Name]) == nil { return ErrRecordNotFound } - if _, exists := mesh[updateEndpoint.DNSName]; exists { - if mesh[updateEndpoint.DNSName][suitableType(updateEndpoint)] { - return ErrInvalidBatchRequest - } - mesh[updateEndpoint.DNSName][suitableType(updateEndpoint)] = true - continue + if err := c.updateMesh(mesh, updateEndpoint); err != nil { + return err } - mesh[updateEndpoint.DNSName] = map[string]bool{suitableType(updateEndpoint): true} } for _, updateOldEndpoint := range changes.UpdateOld { - if rec := im.findByType(suitableType(updateOldEndpoint), existing[updateOldEndpoint.DNSName]); rec == nil || rec.Target != updateOldEndpoint.Target { + if rec := c.findByType(updateOldEndpoint.Type, curZone[updateOldEndpoint.Name]); rec == nil || rec.Target != updateOldEndpoint.Target { return ErrRecordNotFound } } for _, deleteEndpoint := range changes.Delete { - if rec := im.findByType(suitableType(deleteEndpoint), existing[deleteEndpoint.DNSName]); rec == nil || rec.Target != deleteEndpoint.Target { + if rec := c.findByType(deleteEndpoint.Type, curZone[deleteEndpoint.Name]); rec == nil || rec.Target != deleteEndpoint.Target { return ErrRecordNotFound } - if _, exists := mesh[deleteEndpoint.DNSName]; exists { - if mesh[deleteEndpoint.DNSName][suitableType(deleteEndpoint)] { - return ErrInvalidBatchRequest - } - mesh[deleteEndpoint.DNSName][suitableType(deleteEndpoint)] = true - continue + if err := c.updateMesh(mesh, deleteEndpoint); err != nil { + return err } - mesh[deleteEndpoint.DNSName] = map[string]bool{suitableType(deleteEndpoint): true} } return nil } -func (im *InMemoryProvider) findByType(recordType string, records []*InMemoryRecord) *InMemoryRecord { +func (c *inMemoryClient) findByType(recordType string, records []*inMemoryRecord) *inMemoryRecord { for _, record := range records { if record.Type == recordType { return record @@ -187,16 +323,3 @@ func (im *InMemoryProvider) findByType(recordType string, records []*InMemoryRec } return nil } - -func (im *InMemoryProvider) endpoints(zone string) []*endpoint.Endpoint { - endpoints := make([]*endpoint.Endpoint, 0) - if zoneRecords, exists := im.zones[zone]; exists { - for _, recordsPerName := range zoneRecords { - for _, record := range recordsPerName { - record.Endpoint.RecordType = record.Type - endpoints = append(endpoints, record.Endpoint) - } - } - } - return endpoints -} diff --git a/provider/inmemory_test.go b/provider/inmemory_test.go index 11a89934a..9aa97cbbe 100644 --- a/provider/inmemory_test.go +++ b/provider/inmemory_test.go @@ -30,9 +30,8 @@ var ( ) func TestInMemoryProvider(t *testing.T) { - t.Run("Records", testInMemoryRecords) - t.Run("endpoints", testInMemoryEndpoints) t.Run("findByType", testInMemoryFindByType) + t.Run("Records", testInMemoryRecords) t.Run("validateChangeBatch", testInMemoryValidateChangeBatch) t.Run("ApplyChanges", testInMemoryApplyChanges) t.Run("NewInMemoryProvider", testNewInMemoryProvider) @@ -43,8 +42,8 @@ func testInMemoryFindByType(t *testing.T) { for _, ti := range []struct { title string findType string - records []*InMemoryRecord - expected *InMemoryRecord + records []*inMemoryRecord + expected *inMemoryRecord expectedEmpty bool }{ { @@ -64,7 +63,7 @@ func testInMemoryFindByType(t *testing.T) { { title: "one record, empty type", findType: "", - records: []*InMemoryRecord{ + records: []*inMemoryRecord{ { Type: "A", }, @@ -75,7 +74,7 @@ func testInMemoryFindByType(t *testing.T) { { title: "one record, wrong type", findType: "CNAME", - records: []*InMemoryRecord{ + records: []*inMemoryRecord{ { Type: "A", }, @@ -86,19 +85,19 @@ func testInMemoryFindByType(t *testing.T) { { title: "one record, right type", findType: "A", - records: []*InMemoryRecord{ + records: []*inMemoryRecord{ { Type: "A", }, }, - expected: &InMemoryRecord{ + expected: &inMemoryRecord{ Type: "A", }, }, { title: "multiple records, right type", findType: "A", - records: []*InMemoryRecord{ + records: []*inMemoryRecord{ { Type: "A", }, @@ -106,14 +105,14 @@ func testInMemoryFindByType(t *testing.T) { Type: "TXT", }, }, - expected: &InMemoryRecord{ + expected: &inMemoryRecord{ Type: "A", }, }, } { t.Run(ti.title, func(t *testing.T) { - im := NewInMemoryProvider() - record := im.findByType(ti.findType, ti.records) + c := newInMemoryClient() + record := c.findByType(ti.findType, ti.records) if ti.expectedEmpty && record != nil { t.Errorf("should return nil") } @@ -127,113 +126,64 @@ func testInMemoryFindByType(t *testing.T) { } } -func testInMemoryEndpoints(t *testing.T) { +func testInMemoryRecords(t *testing.T) { for _, ti := range []struct { - title string - zone string - init map[string]zone - expected []*endpoint.Endpoint + title string + zone string + expectError bool + init map[string]zone + expected []*endpoint.Endpoint }{ { - title: "no records, no zone", - zone: "", - init: map[string]zone{}, - expected: []*endpoint.Endpoint{}, + title: "no records, no zone", + zone: "", + init: map[string]zone{}, + expectError: false, }, { - title: "no records, zone", - zone: "central", - init: map[string]zone{}, - expected: []*endpoint.Endpoint{}, - }, - { - title: "records, no zone", - zone: "", + title: "records, wrong zone", + zone: "net", init: map[string]zone{ - "org": { - "example.org": []*InMemoryRecord{ - {}, - }, - "foo.org": []*InMemoryRecord{ - {}, - }, - }, - "com": { - "example.com": []*InMemoryRecord{ - {}, - }, - "foo.com": []*InMemoryRecord{ - {}, - }, - }, + "org": {}, + "com": {}, }, - expected: []*endpoint.Endpoint{}, - }, - { - title: "records, zone with no records", - zone: "", - init: map[string]zone{ - "org": { - "example.org": []*InMemoryRecord{ - {}, - }, - "foo.org": []*InMemoryRecord{ - {}, - }, - }, - "com": { - "example.com": []*InMemoryRecord{ - {}, - }, - "foo.com": []*InMemoryRecord{ - {}, - }, - }, - }, - expected: []*endpoint.Endpoint{}, + expectError: false, }, { title: "records, zone with records", zone: "org", init: map[string]zone{ "org": { - "example.org": []*InMemoryRecord{ + "example.org": []*inMemoryRecord{ { - Endpoint: &endpoint.Endpoint{ - DNSName: "example.org", - Target: "8.8.8.8", - }, - Type: "A", + Name: "example.org", + Target: "8.8.8.8", + Type: "A", }, { - Endpoint: &endpoint.Endpoint{ - DNSName: "example.org", - }, + Name: "example.org", Type: "TXT", }, }, - "foo.org": []*InMemoryRecord{ + "foo.org": []*inMemoryRecord{ { - Endpoint: &endpoint.Endpoint{ - DNSName: "foo.org", - Target: "bar.org", - }, - Type: "CNAME", + Name: "foo.org", + Target: "4.4.4.4", + Type: "CNAME", }, }, }, "com": { - "example.com": []*InMemoryRecord{ + "example.com": []*inMemoryRecord{ { - Endpoint: &endpoint.Endpoint{ - DNSName: "example.com", - Target: "4.4.4.4", - }, - Type: "A", + Name: "example.com", + Target: "4.4.4.4", + Type: "CNAME", }, }, }, }, + expectError: false, expected: []*endpoint.Endpoint{ { DNSName: "example.org", @@ -246,92 +196,20 @@ func testInMemoryEndpoints(t *testing.T) { }, { DNSName: "foo.org", - Target: "bar.org", + Target: "4.4.4.4", RecordType: "CNAME", }, }, }, } { t.Run(ti.title, func(t *testing.T) { - im := &InMemoryProvider{zones: ti.init} - if !testutils.SameEndpoints(im.endpoints(ti.zone), ti.expected) { - t.Errorf("endpoints returned wrong set") - } - }) - } -} - -func testInMemoryRecords(t *testing.T) { - for _, ti := range []struct { - title string - zone string - expectError bool - init map[string]zone - }{ - { - title: "no records, no zone", - zone: "", - init: map[string]zone{}, - expectError: true, - }, - { - title: "records, wrong zone", - zone: "net", - init: map[string]zone{ - "org": {}, - "com": {}, - }, - expectError: true, - }, - { - title: "records, zone with records", - zone: "org", - init: map[string]zone{ - "org": { - "example.org": []*InMemoryRecord{ - { - Endpoint: &endpoint.Endpoint{ - DNSName: "example.org", - Target: "8.8.8.8", - }, - Type: "A", - }, - { - Endpoint: &endpoint.Endpoint{ - DNSName: "example.org", - }, - Type: "TXT", - }, - }, - "foo.org": []*InMemoryRecord{ - { - Endpoint: &endpoint.Endpoint{ - DNSName: "foo.org", - Target: "4.4.4.4", - }, - Type: "CNAME", - }, - }, - }, - "com": { - "example.com": []*InMemoryRecord{ - { - Endpoint: &endpoint.Endpoint{ - DNSName: "example.com", - Target: "4.4.4.4", - }, - Type: "CNAME", - }, - }, - }, - }, - expectError: false, - }, - } { - t.Run(ti.title, func(t *testing.T) { + c := newInMemoryClient() + c.zones = ti.init im := NewInMemoryProvider() - im.zones = ti.init - records, err := im.Records(ti.zone) + im.client = c + f := filter{domain: ti.zone} + im.filter = &f + records, err := im.Records("_") if ti.expectError && records != nil { t.Errorf("wrong zone should not return records") } @@ -341,7 +219,7 @@ func testInMemoryRecords(t *testing.T) { if !ti.expectError && err != nil { t.Errorf("unexpected error") } - if !ti.expectError && !testutils.SameEndpoints(im.endpoints(ti.zone), records) { + if !ti.expectError && !testutils.SameEndpoints(ti.expected, records) { t.Errorf("endpoints returned wrong set") } }) @@ -351,48 +229,37 @@ func testInMemoryRecords(t *testing.T) { func testInMemoryValidateChangeBatch(t *testing.T) { init := map[string]zone{ "org": { - "example.org": []*InMemoryRecord{ + "example.org": []*inMemoryRecord{ { - Endpoint: &endpoint.Endpoint{ - DNSName: "example.org", - Target: "8.8.8.8", - }, - Type: "A", + Name: "example.org", + Target: "8.8.8.8", + Type: "A", }, { - Endpoint: &endpoint.Endpoint{ - DNSName: "example.org", - }, - Type: "TXT", + Name: "example.org", }, }, - "foo.org": []*InMemoryRecord{ + "foo.org": []*inMemoryRecord{ { - Endpoint: &endpoint.Endpoint{ - DNSName: "foo.org", - Target: "bar.org", - }, - Type: "CNAME", + Name: "foo.org", + Target: "bar.org", + Type: "CNAME", }, }, - "foo.bar.org": []*InMemoryRecord{ + "foo.bar.org": []*inMemoryRecord{ { - Endpoint: &endpoint.Endpoint{ - DNSName: "foo.bar.org", - Target: "5.5.5.5", - }, - Type: "A", + Name: "foo.bar.org", + Target: "5.5.5.5", + Type: "A", }, }, }, "com": { - "example.com": []*InMemoryRecord{ + "example.com": []*inMemoryRecord{ { - Endpoint: &endpoint.Endpoint{ - DNSName: "example.com", - Target: "another-example.com", - }, - Type: "CNAME", + Name: "example.com", + Target: "another-example.com", + Type: "CNAME", }, }, }, @@ -657,10 +524,15 @@ func testInMemoryValidateChangeBatch(t *testing.T) { }, } { t.Run(ti.title, func(t *testing.T) { - im := &InMemoryProvider{ - zones: ti.init, + c := &inMemoryClient{} + c.zones = ti.init + ichanges := &inMemoryChange{ + Create: convertToInMemoryRecord(ti.changes.Create), + UpdateNew: convertToInMemoryRecord(ti.changes.UpdateNew), + UpdateOld: convertToInMemoryRecord(ti.changes.UpdateOld), + Delete: convertToInMemoryRecord(ti.changes.Delete), } - err := im.validateChangeBatch(ti.zone, ti.changes) + err := c.validateChangeBatch(ti.zone, ichanges) if ti.expectError && err != ti.errorType { t.Errorf("returns wrong type of error: %v, expected: %v", err, ti.errorType) } @@ -718,40 +590,35 @@ func testInMemoryApplyChanges(t *testing.T) { }, expectedZonesState: map[string]zone{ "org": { - "example.org": []*InMemoryRecord{ + "example.org": []*inMemoryRecord{ { - Endpoint: &endpoint.Endpoint{ - DNSName: "example.org", - Target: "8.8.8.8", - }, - Type: "A", + + Name: "example.org", + Target: "8.8.8.8", + Type: "A", }, { - Endpoint: &endpoint.Endpoint{ - DNSName: "example.org", - }, + + Name: "example.org", Type: "TXT", }, }, - "foo.org": []*InMemoryRecord{ + "foo.org": []*inMemoryRecord{ { - Endpoint: &endpoint.Endpoint{ - DNSName: "foo.org", - Target: "4.4.4.4", - }, - Type: "CNAME", + + Name: "foo.org", + Target: "4.4.4.4", + Type: "CNAME", }, }, - "foo.bar.org": []*InMemoryRecord{}, + "foo.bar.org": []*inMemoryRecord{}, }, "com": { - "example.com": []*InMemoryRecord{ + "example.com": []*inMemoryRecord{ { - Endpoint: &endpoint.Endpoint{ - DNSName: "example.com", - Target: "4.4.4.4", - }, - Type: "CNAME", + Name: "example.com", + Target: "4.4.4.4", + Type: "CNAME", }, }, }, @@ -789,50 +656,40 @@ func testInMemoryApplyChanges(t *testing.T) { }, expectedZonesState: map[string]zone{ "org": { - "example.org": []*InMemoryRecord{ + "example.org": []*inMemoryRecord{ { - Endpoint: &endpoint.Endpoint{ - DNSName: "example.org", - }, + Name: "example.org", Type: "TXT", }, }, - "foo.org": []*InMemoryRecord{ + "foo.org": []*inMemoryRecord{ { - Endpoint: &endpoint.Endpoint{ - DNSName: "foo.org", - Target: "4.4.4.4", - }, - Type: "CNAME", + Name: "foo.org", + Target: "4.4.4.4", + Type: "CNAME", }, }, - "foo.bar.org": []*InMemoryRecord{ + "foo.bar.org": []*inMemoryRecord{ { - Endpoint: &endpoint.Endpoint{ - DNSName: "foo.bar.org", - Target: "4.8.8.4", - }, - Type: "A", + Name: "foo.bar.org", + Target: "4.8.8.4", + Type: "A", }, }, - "foo.bar.new.org": []*InMemoryRecord{ + "foo.bar.new.org": []*inMemoryRecord{ { - Endpoint: &endpoint.Endpoint{ - DNSName: "foo.bar.new.org", - Target: "4.8.8.9", - }, - Type: "A", + Name: "foo.bar.new.org", + Target: "4.8.8.9", + Type: "A", }, }, }, "com": { - "example.com": []*InMemoryRecord{ + "example.com": []*inMemoryRecord{ { - Endpoint: &endpoint.Endpoint{ - DNSName: "example.com", - Target: "4.4.4.4", - }, - Type: "CNAME", + Name: "example.com", + Target: "4.4.4.4", + Type: "CNAME", }, }, }, @@ -842,56 +699,48 @@ func testInMemoryApplyChanges(t *testing.T) { t.Run(ti.title, func(t *testing.T) { init := map[string]zone{ "org": { - "example.org": []*InMemoryRecord{ + "example.org": []*inMemoryRecord{ { - Endpoint: &endpoint.Endpoint{ - DNSName: "example.org", - Target: "8.8.8.8", - }, - Type: "A", + Name: "example.org", + Target: "8.8.8.8", + Type: "A", }, { - Endpoint: &endpoint.Endpoint{ - DNSName: "example.org", - }, + Name: "example.org", Type: "TXT", }, }, - "foo.org": []*InMemoryRecord{ + "foo.org": []*inMemoryRecord{ { - Endpoint: &endpoint.Endpoint{ - DNSName: "foo.org", - Target: "4.4.4.4", - }, - Type: "CNAME", + Name: "foo.org", + Target: "4.4.4.4", + Type: "CNAME", }, }, - "foo.bar.org": []*InMemoryRecord{ + "foo.bar.org": []*inMemoryRecord{ { - Endpoint: &endpoint.Endpoint{ - DNSName: "foo.bar.org", - Target: "5.5.5.5", - }, - Type: "A", + Name: "foo.bar.org", + Target: "5.5.5.5", + Type: "A", }, }, }, "com": { - "example.com": []*InMemoryRecord{ + "example.com": []*inMemoryRecord{ { - Endpoint: &endpoint.Endpoint{ - DNSName: "example.com", - Target: "4.4.4.4", - }, - Type: "CNAME", + Name: "example.com", + Target: "4.4.4.4", + Type: "CNAME", }, }, }, } im := NewInMemoryProvider() - im.zones = init + c := &inMemoryClient{} + c.zones = init + im.client = c - err := im.ApplyChanges(ti.zone, ti.changes) + err := im.ApplyChanges("_", ti.changes) if ti.expectError && err == nil { t.Errorf("should return an error") } @@ -899,7 +748,7 @@ func testInMemoryApplyChanges(t *testing.T) { t.Error(err) } if !ti.expectError { - if !reflect.DeepEqual(im.zones, ti.expectedZonesState) { + if !reflect.DeepEqual(c.zones, ti.expectedZonesState) { t.Errorf("invalid update") } } @@ -909,7 +758,7 @@ func testInMemoryApplyChanges(t *testing.T) { func testNewInMemoryProvider(t *testing.T) { cfg := NewInMemoryProvider() - if cfg.zones == nil { + if cfg.client == nil { t.Error("nil map") } } diff --git a/registry/noop_test.go b/registry/noop_test.go index 2dacff9eb..0dd93980a 100644 --- a/registry/noop_test.go +++ b/registry/noop_test.go @@ -46,25 +46,21 @@ func testNoopInit(t *testing.T) { func testNoopRecords(t *testing.T) { p := provider.NewInMemoryProvider() - p.CreateZone("zone") + p.CreateZone("org") providerRecords := []*endpoint.Endpoint{ { - DNSName: "example.org", - Target: "example-lb.com", + DNSName: "example.org", + Target: "example-lb.com", + RecordType: "CNAME", }, } - p.ApplyChanges("zone", &plan.Changes{ + p.ApplyChanges("_", &plan.Changes{ Create: providerRecords, }) r, _ := NewNoopRegistry(p) - _, err := r.Records("wrong-zone") - if err == nil { - t.Error("Should fail for wrong zone: wrong-zone") - } - r, _ = NewNoopRegistry(p) - eps, err := r.Records("zone") + eps, err := r.Records("_") if err != nil { t.Error(err) } @@ -76,7 +72,7 @@ func testNoopRecords(t *testing.T) { func testNoopApplyChanges(t *testing.T) { // do some prep p := provider.NewInMemoryProvider() - p.CreateZone("zone") + p.CreateZone("org") providerRecords := []*endpoint.Endpoint{ { DNSName: "example.org", @@ -96,20 +92,13 @@ func testNoopApplyChanges(t *testing.T) { }, } - p.ApplyChanges("zone", &plan.Changes{ + p.ApplyChanges("_", &plan.Changes{ Create: providerRecords, }) - // wrong zone - r, _ := NewNoopRegistry(p) - err := r.ApplyChanges("wrong-zone", &plan.Changes{}) - if err != provider.ErrZoneNotFound { - t.Error("should return zone not found for apply changes on wrong zone") - } - // wrong changes - r, _ = NewNoopRegistry(p) - err = r.ApplyChanges("zone", &plan.Changes{ + r, _ := NewNoopRegistry(p) + err := r.ApplyChanges("_", &plan.Changes{ Create: []*endpoint.Endpoint{ { DNSName: "example.org", @@ -122,7 +111,7 @@ func testNoopApplyChanges(t *testing.T) { } //correct changes - err = r.ApplyChanges("zone", &plan.Changes{ + err = r.ApplyChanges("_", &plan.Changes{ Create: []*endpoint.Endpoint{ { DNSName: "new-record.org", @@ -145,7 +134,7 @@ func testNoopApplyChanges(t *testing.T) { if err != nil { t.Fatal(err) } - res, _ := p.Records("zone") + res, _ := p.Records("_") if !testutils.SameEndpoints(res, expectedUpdate) { t.Error("incorrectly updated dns provider") } diff --git a/registry/txt_test.go b/registry/txt_test.go index 70fa6f44c..37eab177b 100644 --- a/registry/txt_test.go +++ b/registry/txt_test.go @@ -26,7 +26,7 @@ import ( ) const ( - testZone = "test-zone.example.com." + testZone = "test-zone.example.org" ) func TestTXTRegistry(t *testing.T) { @@ -60,11 +60,6 @@ func testTXTRegistryNew(t *testing.T) { if _, ok := r.mapper.(prefixNameMapper); !ok { t.Error("Incorrect type of prefix name mapper") } - - rs, err := r.Records("random-zone") - if err == nil || rs != nil { - t.Error("incorrect zone should trigger error") - } } func testTXTRegistryRecords(t *testing.T) { @@ -75,7 +70,7 @@ func testTXTRegistryRecords(t *testing.T) { func testTXTRegistryRecordsPrefixed(t *testing.T) { p := provider.NewInMemoryProvider() p.CreateZone(testZone) - p.ApplyChanges(testZone, &plan.Changes{ + p.ApplyChanges("_", &plan.Changes{ Create: []*endpoint.Endpoint{ newEndpointWithOwner("foo.test-zone.example.org", "foo.loadbalancer.com", "CNAME", ""), newEndpointWithOwner("bar.test-zone.example.org", "my-domain.com", "CNAME", ""), @@ -295,13 +290,6 @@ func testTXTRegistryApplyChangesWithPrefix(t *testing.T) { if err != nil { t.Fatal(err) } - - changes = &plan.Changes{} - p.OnApplyChanges = func(c *plan.Changes) {} - err = r.ApplyChanges("new-zone", changes) - if err == nil { - t.Error("expected error") - } } func testTXTRegistryApplyChangesNoPrefix(t *testing.T) {