From 707bdc4d9575eae66bbdb57eda40171b89e91b57 Mon Sep 17 00:00:00 2001 From: Martin Linkhorst Date: Mon, 22 May 2017 15:38:27 +0200 Subject: [PATCH] ref(*): remove superflous zone parameter (#212) --- controller/controller.go | 6 ++---- controller/controller_test.go | 14 +++----------- provider/aws.go | 4 ++-- provider/aws_test.go | 20 ++++++++++---------- provider/google.go | 4 ++-- provider/google_test.go | 22 +++++++++++----------- provider/inmemory.go | 4 ++-- provider/inmemory_test.go | 4 ++-- provider/provider.go | 4 ++-- registry/noop.go | 8 ++++---- registry/noop_test.go | 12 ++++++------ registry/registry.go | 8 ++++---- registry/txt.go | 8 ++++---- registry/txt_test.go | 16 ++++++++-------- 14 files changed, 62 insertions(+), 72 deletions(-) diff --git a/controller/controller.go b/controller/controller.go index ed709d320..251c236ea 100644 --- a/controller/controller.go +++ b/controller/controller.go @@ -33,8 +33,6 @@ import ( // * Take both lists and calculate a Plan to move current towards desired state. // * Tell the DNS provider to apply the changes calucated by the Plan. type Controller struct { - Zone string - Source source.Source Registry registry.Registry // The policy that defines which changes to DNS records are allowed @@ -45,7 +43,7 @@ type Controller struct { // RunOnce runs a single iteration of a reconciliation loop. func (c *Controller) RunOnce() error { - records, err := c.Registry.Records(c.Zone) + records, err := c.Registry.Records() if err != nil { return err } @@ -63,7 +61,7 @@ func (c *Controller) RunOnce() error { plan = plan.Calculate() - return c.Registry.ApplyChanges(c.Zone, plan.Changes) + return c.Registry.ApplyChanges(plan.Changes) } // Run runs RunOnce in a loop with a delay until stopChan receives a value. diff --git a/controller/controller_test.go b/controller/controller_test.go index 630b33dd5..b3d359085 100644 --- a/controller/controller_test.go +++ b/controller/controller_test.go @@ -30,21 +30,16 @@ import ( // mockProvider returns mock endpoints and validates changes. type mockProvider struct { RecordsStore []*endpoint.Endpoint - ExpectZone string ExpectChanges *plan.Changes } // Records returns the desired mock endpoints. -func (p *mockProvider) Records(zone string) ([]*endpoint.Endpoint, error) { +func (p *mockProvider) Records() ([]*endpoint.Endpoint, error) { return p.RecordsStore, nil } // ApplyChanges validates that the passed in changes satisfy the assumtions. -func (p *mockProvider) ApplyChanges(zone string, changes *plan.Changes) error { - if zone != p.ExpectZone { - return errors.New("zone is incorrect") - } - +func (p *mockProvider) ApplyChanges(changes *plan.Changes) error { if len(changes.Create) != len(p.ExpectChanges.Create) { return errors.New("number of created records is wrong") } @@ -77,10 +72,9 @@ func (p *mockProvider) ApplyChanges(zone string, changes *plan.Changes) error { } // newMockProvider creates a new mockProvider returning the given endpoints and validating the desired changes. -func newMockProvider(endpoints []*endpoint.Endpoint, zone string, changes *plan.Changes) provider.Provider { +func newMockProvider(endpoints []*endpoint.Endpoint, changes *plan.Changes) provider.Provider { dnsProvider := &mockProvider{ RecordsStore: endpoints, - ExpectZone: zone, ExpectChanges: changes, } @@ -115,7 +109,6 @@ func TestRunOnce(t *testing.T) { Target: "4.3.2.1", }, }, - "test-zone", &plan.Changes{ Create: []*endpoint.Endpoint{ {DNSName: "create-record", Target: "1.2.3.4"}, @@ -136,7 +129,6 @@ func TestRunOnce(t *testing.T) { // Run our controller once to trigger the validation. ctrl := &Controller{ - Zone: "test-zone", Source: source, Registry: r, Policy: &plan.SyncPolicy{}, diff --git a/provider/aws.go b/provider/aws.go index a9b69da62..5cd8ec8cb 100644 --- a/provider/aws.go +++ b/provider/aws.go @@ -116,7 +116,7 @@ func (p *AWSProvider) Zones() (map[string]*route53.HostedZone, error) { } // Records returns the list of records in a given hosted zone. -func (p *AWSProvider) Records(_ string) (endpoints []*endpoint.Endpoint, _ error) { +func (p *AWSProvider) Records() (endpoints []*endpoint.Endpoint, _ error) { zones, err := p.Zones() if err != nil { return nil, err @@ -174,7 +174,7 @@ func (p *AWSProvider) DeleteRecords(endpoints []*endpoint.Endpoint) error { } // ApplyChanges applies a given set of changes in a given zone. -func (p *AWSProvider) ApplyChanges(_ string, changes *plan.Changes) error { +func (p *AWSProvider) ApplyChanges(changes *plan.Changes) error { combinedChanges := make([]*route53.Change, 0, len(changes.Create)+len(changes.UpdateNew)+len(changes.Delete)) combinedChanges = append(combinedChanges, newChanges(route53.ChangeActionCreate, changes.Create)...) diff --git a/provider/aws_test.go b/provider/aws_test.go index 9eef18f76..1193aa0e2 100644 --- a/provider/aws_test.go +++ b/provider/aws_test.go @@ -174,7 +174,7 @@ func TestAWSRecords(t *testing.T) { endpoint.NewEndpoint("list-test-alias.zone-1.ext-dns-test-2.teapot.zalan.do", "foo.eu-central-1.elb.amazonaws.com", "ALIAS"), }) - records, err := provider.Records("_") + records, err := provider.Records() if err != nil { t.Fatal(err) } @@ -199,7 +199,7 @@ func TestAWSCreateRecords(t *testing.T) { t.Fatal(err) } - records, err := provider.Records("_") + records, err := provider.Records() if err != nil { t.Fatal(err) } @@ -233,7 +233,7 @@ func TestAWSUpdateRecords(t *testing.T) { t.Fatal(err) } - records, err := provider.Records("_") + records, err := provider.Records() if err != nil { t.Fatal(err) } @@ -260,7 +260,7 @@ func TestAWSDeleteRecords(t *testing.T) { t.Fatal(err) } - records, err := provider.Records("_") + records, err := provider.Records() if err != nil { t.Fatal(err) } @@ -314,11 +314,11 @@ func TestAWSApplyChanges(t *testing.T) { Delete: deleteRecords, } - if err := provider.ApplyChanges("_", changes); err != nil { + if err := provider.ApplyChanges(changes); err != nil { t.Fatal(err) } - records, err := provider.Records("_") + records, err := provider.Records() if err != nil { t.Fatal(err) } @@ -383,11 +383,11 @@ func TestAWSApplyChangesDryRun(t *testing.T) { Delete: deleteRecords, } - if err := provider.ApplyChanges("_", changes); err != nil { + if err := provider.ApplyChanges(changes); err != nil { t.Fatal(err) } - records, err := provider.Records("_") + records, err := provider.Records() if err != nil { t.Fatal(err) } @@ -669,7 +669,7 @@ func setupAWSRecords(t *testing.T, provider *AWSProvider, endpoints []*endpoint. clearAWSRecords(t, provider, "/hostedzone/zone-2.ext-dns-test-2.teapot.zalan.do.") clearAWSRecords(t, provider, "/hostedzone/zone-3.ext-dns-test-2.teapot.zalan.do.") - records, err := provider.Records("_") + records, err := provider.Records() if err != nil { t.Fatal(err) } @@ -680,7 +680,7 @@ func setupAWSRecords(t *testing.T, provider *AWSProvider, endpoints []*endpoint. t.Fatal(err) } - records, err = provider.Records("_") + records, err = provider.Records() if err != nil { t.Fatal(err) } diff --git a/provider/google.go b/provider/google.go index 4573b90c1..897ce6252 100644 --- a/provider/google.go +++ b/provider/google.go @@ -150,7 +150,7 @@ func (p *googleProvider) Zones() (map[string]*dns.ManagedZone, error) { } // Records returns the list of records in all relevant zones. -func (p *googleProvider) Records(_ string) (endpoints []*endpoint.Endpoint, _ error) { +func (p *googleProvider) Records() (endpoints []*endpoint.Endpoint, _ error) { zones, err := p.Zones() if err != nil { return nil, err @@ -214,7 +214,7 @@ func (p *googleProvider) DeleteRecords(endpoints []*endpoint.Endpoint) error { } // ApplyChanges applies a given set of changes in a given zone. -func (p *googleProvider) ApplyChanges(_ string, changes *plan.Changes) error { +func (p *googleProvider) ApplyChanges(changes *plan.Changes) error { change := &dns.Change{} change.Additions = append(change.Additions, newRecords(changes.Create)...) diff --git a/provider/google_test.go b/provider/google_test.go index 9eb943bc1..33ec6bf3e 100644 --- a/provider/google_test.go +++ b/provider/google_test.go @@ -212,7 +212,7 @@ func TestGoogleRecords(t *testing.T) { provider := newGoogleProvider(t, "ext-dns-test-2.gcp.zalan.do.", false, originalEndpoints) - records, err := provider.Records("_") + records, err := provider.Records() if err != nil { t.Fatal(err) } @@ -233,7 +233,7 @@ func TestGoogleCreateRecords(t *testing.T) { t.Fatal(err) } - records, err := provider.Records("_") + records, err := provider.Records() if err != nil { t.Fatal(err) } @@ -267,7 +267,7 @@ func TestGoogleUpdateRecords(t *testing.T) { t.Fatal(err) } - records, err := provider.Records("_") + records, err := provider.Records() if err != nil { t.Fatal(err) } @@ -292,7 +292,7 @@ func TestGoogleDeleteRecords(t *testing.T) { t.Fatal(err) } - records, err := provider.Records("_") + records, err := provider.Records() if err != nil { t.Fatal(err) } @@ -340,11 +340,11 @@ func TestGoogleApplyChanges(t *testing.T) { Delete: deleteRecords, } - if err := provider.ApplyChanges("_", changes); err != nil { + if err := provider.ApplyChanges(changes); err != nil { t.Fatal(err) } - records, err := provider.Records("_") + records, err := provider.Records() if err != nil { t.Fatal(err) } @@ -401,11 +401,11 @@ func TestGoogleApplyChangesDryRun(t *testing.T) { Delete: deleteRecords, } - if err := provider.ApplyChanges("_", changes); err != nil { + if err := provider.ApplyChanges(changes); err != nil { t.Fatal(err) } - records, err := provider.Records("_") + records, err := provider.Records() if err != nil { t.Fatal(err) } @@ -416,7 +416,7 @@ func TestGoogleApplyChangesDryRun(t *testing.T) { func TestGoogleApplyChangesEmpty(t *testing.T) { provider := newGoogleProvider(t, "ext-dns-test-2.gcp.zalan.do.", false, []*endpoint.Endpoint{}) - if err := provider.ApplyChanges("_", &plan.Changes{}); err != nil { + if err := provider.ApplyChanges(&plan.Changes{}); err != nil { t.Error(err) } } @@ -586,7 +586,7 @@ func setupGoogleRecords(t *testing.T, provider *googleProvider, endpoints []*end clearGoogleRecords(t, provider, "zone-1-ext-dns-test-2-gcp-zalan-do") clearGoogleRecords(t, provider, "zone-2-ext-dns-test-2-gcp-zalan-do") - records, err := provider.Records("_") + records, err := provider.Records() if err != nil { t.Fatal(err) } @@ -597,7 +597,7 @@ func setupGoogleRecords(t *testing.T, provider *googleProvider, endpoints []*end t.Fatal(err) } - records, err = provider.Records("_") + records, err = provider.Records() if err != nil { t.Fatal(err) } diff --git a/provider/inmemory.go b/provider/inmemory.go index dc05f96db..b6acae430 100644 --- a/provider/inmemory.go +++ b/provider/inmemory.go @@ -69,7 +69,7 @@ func (im *InMemoryProvider) Zones() map[string]string { } // Records returns the list of endpoints -func (im *InMemoryProvider) Records(_ string) ([]*endpoint.Endpoint, error) { +func (im *InMemoryProvider) Records() ([]*endpoint.Endpoint, error) { defer im.OnRecords() endpoints := make([]*endpoint.Endpoint, 0) @@ -93,7 +93,7 @@ func (im *InMemoryProvider) Records(_ 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(_ string, changes *plan.Changes) error { +func (im *InMemoryProvider) ApplyChanges(changes *plan.Changes) error { defer im.OnApplyChanges(changes) perZoneChanges := map[string]*plan.Changes{} diff --git a/provider/inmemory_test.go b/provider/inmemory_test.go index 9aa97cbbe..881844ace 100644 --- a/provider/inmemory_test.go +++ b/provider/inmemory_test.go @@ -209,7 +209,7 @@ func testInMemoryRecords(t *testing.T) { im.client = c f := filter{domain: ti.zone} im.filter = &f - records, err := im.Records("_") + records, err := im.Records() if ti.expectError && records != nil { t.Errorf("wrong zone should not return records") } @@ -740,7 +740,7 @@ func testInMemoryApplyChanges(t *testing.T) { c.zones = init im.client = c - err := im.ApplyChanges("_", ti.changes) + err := im.ApplyChanges(ti.changes) if ti.expectError && err == nil { t.Errorf("should return an error") } diff --git a/provider/provider.go b/provider/provider.go index 799ff4929..3f3b8cfcc 100644 --- a/provider/provider.go +++ b/provider/provider.go @@ -26,8 +26,8 @@ import ( // Provider defines the interface DNS providers should implement. type Provider interface { - Records(zone string) ([]*endpoint.Endpoint, error) - ApplyChanges(zone string, changes *plan.Changes) error + Records() ([]*endpoint.Endpoint, error) + ApplyChanges(changes *plan.Changes) error } // suitableType returns the DNS resource record type suitable for the target. diff --git a/registry/noop.go b/registry/noop.go index e33700f8c..aadc801a5 100644 --- a/registry/noop.go +++ b/registry/noop.go @@ -35,11 +35,11 @@ func NewNoopRegistry(provider provider.Provider) (*NoopRegistry, error) { } // Records returns the current records from the dns provider -func (im *NoopRegistry) Records(zone string) ([]*endpoint.Endpoint, error) { - return im.provider.Records(zone) +func (im *NoopRegistry) Records() ([]*endpoint.Endpoint, error) { + return im.provider.Records() } // ApplyChanges propagates changes to the dns provider -func (im *NoopRegistry) ApplyChanges(zone string, changes *plan.Changes) error { - return im.provider.ApplyChanges(zone, changes) +func (im *NoopRegistry) ApplyChanges(changes *plan.Changes) error { + return im.provider.ApplyChanges(changes) } diff --git a/registry/noop_test.go b/registry/noop_test.go index 0dd93980a..4e6fd27a4 100644 --- a/registry/noop_test.go +++ b/registry/noop_test.go @@ -54,13 +54,13 @@ func testNoopRecords(t *testing.T) { RecordType: "CNAME", }, } - p.ApplyChanges("_", &plan.Changes{ + p.ApplyChanges(&plan.Changes{ Create: providerRecords, }) r, _ := NewNoopRegistry(p) - eps, err := r.Records("_") + eps, err := r.Records() if err != nil { t.Error(err) } @@ -92,13 +92,13 @@ func testNoopApplyChanges(t *testing.T) { }, } - p.ApplyChanges("_", &plan.Changes{ + p.ApplyChanges(&plan.Changes{ Create: providerRecords, }) // wrong changes r, _ := NewNoopRegistry(p) - err := r.ApplyChanges("_", &plan.Changes{ + err := r.ApplyChanges(&plan.Changes{ Create: []*endpoint.Endpoint{ { DNSName: "example.org", @@ -111,7 +111,7 @@ func testNoopApplyChanges(t *testing.T) { } //correct changes - err = r.ApplyChanges("_", &plan.Changes{ + err = r.ApplyChanges(&plan.Changes{ Create: []*endpoint.Endpoint{ { DNSName: "new-record.org", @@ -134,7 +134,7 @@ func testNoopApplyChanges(t *testing.T) { if err != nil { t.Fatal(err) } - res, _ := p.Records("_") + res, _ := p.Records() if !testutils.SameEndpoints(res, expectedUpdate) { t.Error("incorrectly updated dns provider") } diff --git a/registry/registry.go b/registry/registry.go index 589e0121d..9b7cf5802 100644 --- a/registry/registry.go +++ b/registry/registry.go @@ -23,12 +23,12 @@ import ( ) // Registry is an interface which should enables ownership concept in external-dns -// Record(zone string) returns ALL records registered with DNS provider (TODO: for multi-zone support return all records) +// Records() returns ALL records registered with DNS provider (TODO: for multi-zone support return all records) // each entry includes owner information -// ApplyChanges(zone string, changes *plan.Changes) propagates the changes to the DNS Provider API and correspondingly updates ownership depending on type of registry being used +// ApplyChanges(changes *plan.Changes) propagates the changes to the DNS Provider API and correspondingly updates ownership depending on type of registry being used type Registry interface { - Records(zone string) ([]*endpoint.Endpoint, error) - ApplyChanges(zone string, changes *plan.Changes) error + Records() ([]*endpoint.Endpoint, error) + ApplyChanges(changes *plan.Changes) error } //TODO(ideahitme): consider moving this to Plan diff --git a/registry/txt.go b/registry/txt.go index 9ef4e1957..bded60bab 100644 --- a/registry/txt.go +++ b/registry/txt.go @@ -58,8 +58,8 @@ func NewTXTRegistry(provider provider.Provider, txtPrefix, ownerID string) (*TXT // Records returns the current records from the registry excluding TXT Records // If TXT records was created previously to indicate ownership its corresponding value // will be added to the endpoints Labels map -func (im *TXTRegistry) Records(zone string) ([]*endpoint.Endpoint, error) { - records, err := im.provider.Records(zone) +func (im *TXTRegistry) Records() ([]*endpoint.Endpoint, error) { + records, err := im.provider.Records() if err != nil { return nil, err } @@ -93,7 +93,7 @@ func (im *TXTRegistry) Records(zone string) ([]*endpoint.Endpoint, error) { // ApplyChanges updates dns provider with the changes // for each created/deleted record it will also take into account TXT records for creation/deletion -func (im *TXTRegistry) ApplyChanges(zone string, changes *plan.Changes) error { +func (im *TXTRegistry) ApplyChanges(changes *plan.Changes) error { filteredChanges := &plan.Changes{ Create: changes.Create, UpdateNew: filterOwnedRecords(im.ownerID, changes.UpdateNew), @@ -109,7 +109,7 @@ func (im *TXTRegistry) ApplyChanges(zone string, changes *plan.Changes) error { txt := endpoint.NewEndpoint(im.mapper.toTXTName(r.DNSName), im.getTXTLabel(), "TXT") filteredChanges.Delete = append(filteredChanges.Delete, txt) } - return im.provider.ApplyChanges(zone, filteredChanges) + return im.provider.ApplyChanges(filteredChanges) } /** diff --git a/registry/txt_test.go b/registry/txt_test.go index 37eab177b..f9618dc4a 100644 --- a/registry/txt_test.go +++ b/registry/txt_test.go @@ -70,7 +70,7 @@ func testTXTRegistryRecords(t *testing.T) { func testTXTRegistryRecordsPrefixed(t *testing.T) { p := provider.NewInMemoryProvider() p.CreateZone(testZone) - p.ApplyChanges("_", &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", ""), @@ -135,7 +135,7 @@ func testTXTRegistryRecordsPrefixed(t *testing.T) { } r, _ := NewTXTRegistry(p, "txt.", "owner") - records, _ := r.Records(testZone) + records, _ := r.Records() if !testutils.SameEndpoints(records, expectedRecords) { t.Error("incorrect result returned from txt registry") } @@ -144,7 +144,7 @@ func testTXTRegistryRecordsPrefixed(t *testing.T) { func testTXTRegistryRecordsNoPrefix(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", ""), @@ -209,7 +209,7 @@ func testTXTRegistryRecordsNoPrefix(t *testing.T) { } r, _ := NewTXTRegistry(p, "", "owner") - records, _ := r.Records(testZone) + records, _ := r.Records() if !testutils.SameEndpoints(records, expectedRecords) { t.Error("incorrect result returned from txt registry") @@ -224,7 +224,7 @@ func testTXTRegistryApplyChanges(t *testing.T) { func testTXTRegistryApplyChangesWithPrefix(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", ""), @@ -286,7 +286,7 @@ func testTXTRegistryApplyChangesWithPrefix(t *testing.T) { t.Error("incorrect plan changes are passed to provider") } } - err := r.ApplyChanges(testZone, changes) + err := r.ApplyChanges(changes) if err != nil { t.Fatal(err) } @@ -295,7 +295,7 @@ func testTXTRegistryApplyChangesWithPrefix(t *testing.T) { func testTXTRegistryApplyChangesNoPrefix(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", ""), @@ -353,7 +353,7 @@ func testTXTRegistryApplyChangesNoPrefix(t *testing.T) { t.Error("incorrect plan changes are passed to provider") } } - err := r.ApplyChanges(testZone, changes) + err := r.ApplyChanges(changes) if err != nil { t.Fatal(err) }