From c80909f0e72ea29d6f127013289eb19c5438b508 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20Grumb=C3=B6ck?= Date: Wed, 1 Apr 2020 19:05:16 +0000 Subject: [PATCH 1/6] Added txt-suffix feature (rebased) --- main.go | 2 +- pkg/apis/externaldns/types.go | 5 +- pkg/apis/externaldns/validation/validation.go | 5 + registry/txt.go | 49 +++- registry/txt_test.go | 225 +++++++++++++++++- 5 files changed, 265 insertions(+), 21 deletions(-) diff --git a/main.go b/main.go index 871ec1c97a..44f8af2049 100644 --- a/main.go +++ b/main.go @@ -299,7 +299,7 @@ func main() { case "noop": r, err = registry.NewNoopRegistry(p) case "txt": - r, err = registry.NewTXTRegistry(p, cfg.TXTPrefix, cfg.TXTOwnerID, cfg.TXTCacheInterval) + r, err = registry.NewTXTRegistry(p, cfg.TXTPrefix, cfg.TXTSuffix, cfg.TXTOwnerID, cfg.TXTCacheInterval) case "aws-sd": r, err = registry.NewAWSSDRegistry(p.(*awssd.AWSSDProvider), cfg.TXTOwnerID) default: diff --git a/pkg/apis/externaldns/types.go b/pkg/apis/externaldns/types.go index 385427d9d6..53a6e8219b 100644 --- a/pkg/apis/externaldns/types.go +++ b/pkg/apis/externaldns/types.go @@ -109,6 +109,7 @@ type Config struct { Registry string TXTOwnerID string TXTPrefix string + TXTSuffix string Interval time.Duration Once bool DryRun bool @@ -205,6 +206,7 @@ var defaultConfig = &Config{ Registry: "txt", TXTOwnerID: "default", TXTPrefix: "", + TXTSuffix: "", TXTCacheInterval: 0, Interval: time.Minute, Once: false, @@ -392,7 +394,8 @@ func (cfg *Config) ParseFlags(args []string) error { // Flags related to the registry app.Flag("registry", "The registry implementation to use to keep track of DNS record ownership (default: txt, options: txt, noop, aws-sd)").Default(defaultConfig.Registry).EnumVar(&cfg.Registry, "txt", "noop", "aws-sd") app.Flag("txt-owner-id", "When using the TXT registry, a name that identifies this instance of ExternalDNS (default: default)").Default(defaultConfig.TXTOwnerID).StringVar(&cfg.TXTOwnerID) - app.Flag("txt-prefix", "When using the TXT registry, a custom string that's prefixed to each ownership DNS record (optional)").Default(defaultConfig.TXTPrefix).StringVar(&cfg.TXTPrefix) + app.Flag("txt-prefix", "When using the TXT registry, a custom string that's prefixed to each ownership DNS record (optional). Mutual exclusive with txt-suffix!").Default(defaultConfig.TXTPrefix).StringVar(&cfg.TXTPrefix) + app.Flag("txt-suffix", "When using the TXT registry, a custom string that's suffixed to the host portion of each ownership DNS record (optional). Mutual exclusive with txt-prefix!").Default(defaultConfig.TXTSuffix).StringVar(&cfg.TXTSuffix) // Flags related to the main control loop app.Flag("txt-cache-interval", "The interval between cache synchronizations in duration format (default: disabled)").Default(defaultConfig.TXTCacheInterval.String()).DurationVar(&cfg.TXTCacheInterval) diff --git a/pkg/apis/externaldns/validation/validation.go b/pkg/apis/externaldns/validation/validation.go index 292c8d869e..58371ed6a9 100644 --- a/pkg/apis/externaldns/validation/validation.go +++ b/pkg/apis/externaldns/validation/validation.go @@ -91,5 +91,10 @@ func ValidateConfig(cfg *externaldns.Config) error { if cfg.IgnoreHostnameAnnotation && cfg.FQDNTemplate == "" { return errors.New("FQDN Template must be set if ignoring annotations") } + + if len(cfg.TXTPrefix) > 0 && len(cfg.TXTSuffix) > 0 { + return errors.New("txt-prefix and txt-suffix are mutual exclusive") + } + return nil } diff --git a/registry/txt.go b/registry/txt.go index 2d99b0660a..ca417f10f7 100644 --- a/registry/txt.go +++ b/registry/txt.go @@ -22,6 +22,7 @@ import ( "fmt" "strings" "time" + "regexp" log "github.com/sirupsen/logrus" @@ -43,12 +44,16 @@ type TXTRegistry struct { } // NewTXTRegistry returns new TXTRegistry object -func NewTXTRegistry(provider provider.Provider, txtPrefix, ownerID string, cacheInterval time.Duration) (*TXTRegistry, error) { +func NewTXTRegistry(provider provider.Provider, txtPrefix, txtSuffix, ownerID string, cacheInterval time.Duration) (*TXTRegistry, error) { if ownerID == "" { return nil, errors.New("owner id cannot be empty") } - mapper := newPrefixNameMapper(txtPrefix) + if len(txtPrefix) > 0 && len(txtSuffix) > 0 { + return nil, errors.New("txt-prefix and txt-suffix are mutual exclusive") + } + + mapper := newaffixNameMapper(txtPrefix, txtSuffix) return &TXTRegistry{ provider: provider, @@ -201,26 +206,50 @@ type nameMapper interface { toTXTName(string) string } -type prefixNameMapper struct { +type affixNameMapper struct { prefix string + suffix string } -var _ nameMapper = prefixNameMapper{} +var _ nameMapper = affixNameMapper{} -func newPrefixNameMapper(prefix string) prefixNameMapper { - return prefixNameMapper{prefix: strings.ToLower(prefix)} +func newaffixNameMapper(prefix string, suffix string) affixNameMapper { + return affixNameMapper{prefix: strings.ToLower(prefix), suffix: strings.ToLower(suffix)} } -func (pr prefixNameMapper) toEndpointName(txtDNSName string) string { +/* func (pr affixNameMapper) toEndpointName(txtDNSName string) string { lowerDNSName := strings.ToLower(txtDNSName) - if strings.HasPrefix(lowerDNSName, pr.prefix) { + regex := regexp.MustCompile(`\.`) + DNSName := regex.Split(lowerDNSName, 2) + if (strings.HasPrefix(DNSName[0], pr.prefix) && strings.HasSuffix(DNSName[0], pr.suffix)) { + DNSName[0] = strings.TrimPrefix(DNSName[0], pr.prefix) + DNSName[0] = strings.TrimSuffix(DNSName[0], pr.suffix) + return DNSName[0] + "." + DNSName[1] + } + return "" +}*/ + +func (pr affixNameMapper) toEndpointName(txtDNSName string) string { + lowerDNSName := strings.ToLower(txtDNSName) + if strings.HasPrefix(lowerDNSName, pr.prefix) && len(pr.suffix) == 0 { return strings.TrimPrefix(lowerDNSName, pr.prefix) } + + if len(pr.suffix) > 0 { + regex := regexp.MustCompile(`\.`) + DNSName := regex.Split(lowerDNSName, 2) + if strings.HasSuffix(DNSName[0], pr.suffix) { + return strings.TrimSuffix(DNSName[0], pr.suffix) + "." + DNSName[1] + } + } return "" } -func (pr prefixNameMapper) toTXTName(endpointDNSName string) string { - return pr.prefix + endpointDNSName + +func (pr affixNameMapper) toTXTName(endpointDNSName string) string { + regex := regexp.MustCompile(`\.`) + DNSName := regex.Split(endpointDNSName, 2) + return pr.prefix + DNSName[0] + pr.suffix + "." + DNSName[1] } func (im *TXTRegistry) addToCache(ep *endpoint.Endpoint) { diff --git a/registry/txt_test.go b/registry/txt_test.go index 7347077914..60580b664d 100644 --- a/registry/txt_test.go +++ b/registry/txt_test.go @@ -47,23 +47,33 @@ func testTXTRegistryNew(t *testing.T) { _, err := NewTXTRegistry(p, "txt", "", time.Hour) require.Error(t, err) - r, err := NewTXTRegistry(p, "txt", "owner", time.Hour) + _, err = NewTXTRegistry(p, "", "txt", "", time.Hour) + require.Error(t, err) + + r, err := NewTXTRegistry(p, "txt", "", "owner", time.Hour) + require.NoError(t, err) + + r, err = NewTXTRegistry(p, "", "txt", "owner", time.Hour) require.NoError(t, err) - _, ok := r.mapper.(prefixNameMapper) + _, err = NewTXTRegistry(p, "txt", "txt", "owner", time.Hour) + require.Error(t, err) + + _, ok := r.mapper.(affixNameMapper) require.True(t, ok) assert.Equal(t, "owner", r.ownerID) assert.Equal(t, p, r.provider) - r, err = NewTXTRegistry(p, "", "owner", time.Hour) + r, err = NewTXTRegistry(p, "", "", "owner", time.Hour) require.NoError(t, err) - _, ok = r.mapper.(prefixNameMapper) + _, ok = r.mapper.(affixNameMapper) assert.True(t, ok) } func testTXTRegistryRecords(t *testing.T) { t.Run("With prefix", testTXTRegistryRecordsPrefixed) + t.Run("With suffix", testTXTRegistryRecordsSuffixed) t.Run("No prefix", testTXTRegistryRecordsNoPrefix) } @@ -160,13 +170,118 @@ func testTXTRegistryRecordsPrefixed(t *testing.T) { }, } - r, _ := NewTXTRegistry(p, "txt.", "owner", time.Hour) + r, _ := NewTXTRegistry(p, "txt.", "", "owner", time.Hour) records, _ := r.Records(ctx) assert.True(t, testutils.SameEndpoints(records, expectedRecords)) // Ensure prefix is case-insensitive - r, _ = NewTXTRegistry(p, "TxT.", "owner", time.Hour) + r, _ = NewTXTRegistry(p, "TxT.", "", "owner", time.Hour) + records, _ = r.Records(ctx) + + assert.True(t, testutils.SameEndpointLabels(records, expectedRecords)) +} + +func testTXTRegistryRecordsSuffixed(t *testing.T) { + ctx := context.Background() + p := inmemory.NewInMemoryProvider() + p.CreateZone(testZone) + p.ApplyChanges(ctx, &plan.Changes{ + Create: []*endpoint.Endpoint{ + newEndpointWithOwnerAndLabels("foo.test-zone.example.org", "foo.loadbalancer.com", endpoint.RecordTypeCNAME, "", endpoint.Labels{"foo": "somefoo"}), + newEndpointWithOwnerAndLabels("bar.test-zone.example.org", "my-domain.com", endpoint.RecordTypeCNAME, "", endpoint.Labels{"bar": "somebar"}), + newEndpointWithOwner("bar-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("bar-txt.test-zone.example.org", "baz.test-zone.example.org", endpoint.RecordTypeCNAME, ""), + newEndpointWithOwner("qux.test-zone.example.org", "random", endpoint.RecordTypeTXT, ""), + newEndpointWithOwnerAndLabels("tar.test-zone.example.org", "tar.loadbalancer.com", endpoint.RecordTypeCNAME, "", endpoint.Labels{"tar": "sometar"}), + newEndpointWithOwner("tar-TxT.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner-2\"", endpoint.RecordTypeTXT, ""), // case-insensitive TXT prefix + newEndpointWithOwner("foobar.test-zone.example.org", "foobar.loadbalancer.com", endpoint.RecordTypeCNAME, ""), + newEndpointWithOwner("foobar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("multiple.test-zone.example.org", "lb1.loadbalancer.com", endpoint.RecordTypeCNAME, "").WithSetIdentifier("test-set-1"), + newEndpointWithOwner("multiple.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-1"), + newEndpointWithOwner("multiple.test-zone.example.org", "lb2.loadbalancer.com", endpoint.RecordTypeCNAME, "").WithSetIdentifier("test-set-2"), + newEndpointWithOwner("multiple.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-2"), + }, + }) + expectedRecords := []*endpoint.Endpoint{ + { + DNSName: "foo.test-zone.example.org", + Targets: endpoint.Targets{"foo.loadbalancer.com"}, + RecordType: endpoint.RecordTypeCNAME, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "", + "foo": "somefoo", + }, + }, + { + DNSName: "bar.test-zone.example.org", + Targets: endpoint.Targets{"my-domain.com"}, + RecordType: endpoint.RecordTypeCNAME, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "owner", + "bar": "somebar", + }, + }, + { + DNSName: "bar-txt.test-zone.example.org", + Targets: endpoint.Targets{"baz.test-zone.example.org"}, + RecordType: endpoint.RecordTypeCNAME, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "", + }, + }, + { + DNSName: "qux.test-zone.example.org", + Targets: endpoint.Targets{"random"}, + RecordType: endpoint.RecordTypeTXT, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "", + }, + }, + { + DNSName: "tar.test-zone.example.org", + Targets: endpoint.Targets{"tar.loadbalancer.com"}, + RecordType: endpoint.RecordTypeCNAME, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "owner-2", + "tar": "sometar", + }, + }, + { + DNSName: "foobar.test-zone.example.org", + Targets: endpoint.Targets{"foobar.loadbalancer.com"}, + RecordType: endpoint.RecordTypeCNAME, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "", + }, + }, + { + DNSName: "multiple.test-zone.example.org", + Targets: endpoint.Targets{"lb1.loadbalancer.com"}, + SetIdentifier: "test-set-1", + RecordType: endpoint.RecordTypeCNAME, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "", + }, + }, + { + DNSName: "multiple.test-zone.example.org", + Targets: endpoint.Targets{"lb2.loadbalancer.com"}, + SetIdentifier: "test-set-2", + RecordType: endpoint.RecordTypeCNAME, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "", + }, + }, + } + + r, _ := NewTXTRegistry(p, "", "-txt", "owner", time.Hour) + records, _ := r.Records(ctx) + + assert.True(t, testutils.SameEndpoints(records, expectedRecords)) + + // Ensure prefix is case-insensitive + r, _ = NewTXTRegistry(p, "", "-TxT", "owner", time.Hour) records, _ = r.Records(ctx) assert.True(t, testutils.SameEndpointLabels(records, expectedRecords)) @@ -241,7 +356,7 @@ func testTXTRegistryRecordsNoPrefix(t *testing.T) { }, } - r, _ := NewTXTRegistry(p, "", "owner", time.Hour) + r, _ := NewTXTRegistry(p, "", "", "owner", time.Hour) records, _ := r.Records(ctx) assert.True(t, testutils.SameEndpoints(records, expectedRecords)) @@ -249,6 +364,7 @@ func testTXTRegistryRecordsNoPrefix(t *testing.T) { func testTXTRegistryApplyChanges(t *testing.T) { t.Run("With Prefix", testTXTRegistryApplyChangesWithPrefix) + t.Run("With Suffix", testTXTRegistryApplyChangesWithSuffix) t.Run("No prefix", testTXTRegistryApplyChangesNoPrefix) } @@ -277,7 +393,7 @@ func testTXTRegistryApplyChangesWithPrefix(t *testing.T) { newEndpointWithOwner("txt.multiple.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-2"), }, }) - r, _ := NewTXTRegistry(p, "txt.", "owner", time.Hour) + r, _ := NewTXTRegistry(p, "txt.", "", "owner", time.Hour) changes := &plan.Changes{ Create: []*endpoint.Endpoint{ @@ -343,6 +459,97 @@ func testTXTRegistryApplyChangesWithPrefix(t *testing.T) { require.NoError(t, err) } +func testTXTRegistryApplyChangesWithSuffix(t *testing.T) { + p := inmemory.NewInMemoryProvider() + p.CreateZone(testZone) + ctxEndpoints := []*endpoint.Endpoint{} + ctx := context.WithValue(context.Background(), provider.RecordsContextKey, ctxEndpoints) + p.OnApplyChanges = func(ctx context.Context, got *plan.Changes) { + assert.Equal(t, ctxEndpoints, ctx.Value(provider.RecordsContextKey)) + } + p.ApplyChanges(ctx, &plan.Changes{ + Create: []*endpoint.Endpoint{ + newEndpointWithOwner("foo.test-zone.example.org", "foo.loadbalancer.com", endpoint.RecordTypeCNAME, ""), + newEndpointWithOwner("bar.test-zone.example.org", "my-domain.com", endpoint.RecordTypeCNAME, ""), + newEndpointWithOwner("bar-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("bar-txt.test-zone.example.org", "baz.test-zone.example.org", endpoint.RecordTypeCNAME, ""), + newEndpointWithOwner("qux.test-zone.example.org", "random", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("tar.test-zone.example.org", "tar.loadbalancer.com", endpoint.RecordTypeCNAME, ""), + newEndpointWithOwner("tar-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("foobar.test-zone.example.org", "foobar.loadbalancer.com", endpoint.RecordTypeCNAME, ""), + newEndpointWithOwner("foobar-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("multiple.test-zone.example.org", "lb1.loadbalancer.com", endpoint.RecordTypeCNAME, "").WithSetIdentifier("test-set-1"), + newEndpointWithOwner("multiple-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-1"), + newEndpointWithOwner("multiple.test-zone.example.org", "lb2.loadbalancer.com", endpoint.RecordTypeCNAME, "").WithSetIdentifier("test-set-2"), + newEndpointWithOwner("multiple-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-2"), + }, + }) + r, _ := NewTXTRegistry(p, "", "-txt", "owner", time.Hour) + + changes := &plan.Changes{ + Create: []*endpoint.Endpoint{ + newEndpointWithOwnerResource("new-record-1.test-zone.example.org", "new-loadbalancer-1.lb.com", "", "", "ingress/default/my-ingress"), + newEndpointWithOwnerResource("multiple.test-zone.example.org", "lb3.loadbalancer.com", "", "", "ingress/default/my-ingress").WithSetIdentifier("test-set-3"), + }, + Delete: []*endpoint.Endpoint{ + newEndpointWithOwner("foobar.test-zone.example.org", "foobar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner"), + newEndpointWithOwner("multiple.test-zone.example.org", "lb1.loadbalancer.com", endpoint.RecordTypeCNAME, "owner").WithSetIdentifier("test-set-1"), + }, + UpdateNew: []*endpoint.Endpoint{ + newEndpointWithOwnerResource("tar.test-zone.example.org", "new-tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress-2"), + newEndpointWithOwnerResource("multiple.test-zone.example.org", "new.loadbalancer.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress-2").WithSetIdentifier("test-set-2"), + }, + UpdateOld: []*endpoint.Endpoint{ + newEndpointWithOwner("tar.test-zone.example.org", "tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner"), + newEndpointWithOwner("multiple.test-zone.example.org", "lb2.loadbalancer.com", endpoint.RecordTypeCNAME, "owner").WithSetIdentifier("test-set-2"), + }, + } + expected := &plan.Changes{ + Create: []*endpoint.Endpoint{ + newEndpointWithOwnerResource("new-record-1.test-zone.example.org", "new-loadbalancer-1.lb.com", "", "owner", "ingress/default/my-ingress"), + newEndpointWithOwner("new-record-1-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwnerResource("multiple.test-zone.example.org", "lb3.loadbalancer.com", "", "owner", "ingress/default/my-ingress").WithSetIdentifier("test-set-3"), + newEndpointWithOwner("multiple-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-3"), + }, + Delete: []*endpoint.Endpoint{ + newEndpointWithOwner("foobar.test-zone.example.org", "foobar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner"), + newEndpointWithOwner("foobar-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("multiple.test-zone.example.org", "lb1.loadbalancer.com", endpoint.RecordTypeCNAME, "owner").WithSetIdentifier("test-set-1"), + newEndpointWithOwner("multiple-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-1"), + }, + UpdateNew: []*endpoint.Endpoint{ + newEndpointWithOwnerResource("tar.test-zone.example.org", "new-tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress-2"), + newEndpointWithOwner("tar-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress-2\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwnerResource("multiple.test-zone.example.org", "new.loadbalancer.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress-2").WithSetIdentifier("test-set-2"), + newEndpointWithOwner("multiple-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress-2\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-2"), + }, + UpdateOld: []*endpoint.Endpoint{ + newEndpointWithOwner("tar.test-zone.example.org", "tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner"), + newEndpointWithOwner("tar-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("multiple.test-zone.example.org", "lb2.loadbalancer.com", endpoint.RecordTypeCNAME, "owner").WithSetIdentifier("test-set-2"), + newEndpointWithOwner("multiple-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-2"), + }, + } + p.OnApplyChanges = func(ctx context.Context, got *plan.Changes) { + mExpected := map[string][]*endpoint.Endpoint{ + "Create": expected.Create, + "UpdateNew": expected.UpdateNew, + "UpdateOld": expected.UpdateOld, + "Delete": expected.Delete, + } + mGot := map[string][]*endpoint.Endpoint{ + "Create": got.Create, + "UpdateNew": got.UpdateNew, + "UpdateOld": got.UpdateOld, + "Delete": got.Delete, + } + assert.True(t, testutils.SamePlanChanges(mGot, mExpected)) + assert.Equal(t, nil, ctx.Value(provider.RecordsContextKey)) + } + err := r.ApplyChanges(ctx, changes) + require.NoError(t, err) +} + func testTXTRegistryApplyChangesNoPrefix(t *testing.T) { p := inmemory.NewInMemoryProvider() p.CreateZone(testZone) @@ -364,7 +571,7 @@ func testTXTRegistryApplyChangesNoPrefix(t *testing.T) { newEndpointWithOwner("foobar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), }, }) - r, _ := NewTXTRegistry(p, "", "owner", time.Hour) + r, _ := NewTXTRegistry(p, "", "", "owner", time.Hour) changes := &plan.Changes{ Create: []*endpoint.Endpoint{ From aab98ab4cdcf4a8c07b70fcaf27b972893f19830 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20Grumb=C3=B6ck?= Date: Tue, 7 Apr 2020 08:02:45 +0000 Subject: [PATCH 2/6] remediation of gofmt and gosimple linting errors --- registry/txt.go | 7 +++---- registry/txt_test.go | 6 ++---- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/registry/txt.go b/registry/txt.go index ca417f10f7..54451a2609 100644 --- a/registry/txt.go +++ b/registry/txt.go @@ -20,9 +20,9 @@ import ( "context" "errors" "fmt" + "regexp" "strings" "time" - "regexp" log "github.com/sirupsen/logrus" @@ -240,15 +240,14 @@ func (pr affixNameMapper) toEndpointName(txtDNSName string) string { DNSName := regex.Split(lowerDNSName, 2) if strings.HasSuffix(DNSName[0], pr.suffix) { return strings.TrimSuffix(DNSName[0], pr.suffix) + "." + DNSName[1] - } + } } return "" } - func (pr affixNameMapper) toTXTName(endpointDNSName string) string { regex := regexp.MustCompile(`\.`) - DNSName := regex.Split(endpointDNSName, 2) + DNSName := regex.Split(endpointDNSName, 2) return pr.prefix + DNSName[0] + pr.suffix + "." + DNSName[1] } diff --git a/registry/txt_test.go b/registry/txt_test.go index 60580b664d..8f6cd77edb 100644 --- a/registry/txt_test.go +++ b/registry/txt_test.go @@ -693,10 +693,8 @@ func newEndpointWithOwner(dnsName, target, recordType, ownerID string) *endpoint func newEndpointWithOwnerAndLabels(dnsName, target, recordType, ownerID string, labels endpoint.Labels) *endpoint.Endpoint { e := endpoint.NewEndpoint(dnsName, recordType, target) e.Labels[endpoint.OwnerLabelKey] = ownerID - if labels != nil { - for k, v := range labels { - e.Labels[k] = v - } + for k, v := range labels { + e.Labels[k] = v } return e } From 72d49c125c73501522f2d846d553c9daf63197a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20Grumb=C3=B6ck?= Date: Tue, 7 Apr 2020 08:45:49 +0000 Subject: [PATCH 3/6] remediate lint staticcheck error --- registry/txt_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/registry/txt_test.go b/registry/txt_test.go index 8f6cd77edb..fd50b4b10a 100644 --- a/registry/txt_test.go +++ b/registry/txt_test.go @@ -52,6 +52,7 @@ func testTXTRegistryNew(t *testing.T) { r, err := NewTXTRegistry(p, "txt", "", "owner", time.Hour) require.NoError(t, err) + assert.Equal(t, p, r.provider) r, err = NewTXTRegistry(p, "", "txt", "owner", time.Hour) require.NoError(t, err) From d2ee19505b7088aaa686b110f6f74d46f733f464 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20Grumb=C3=B6ck?= Date: Fri, 10 Apr 2020 15:57:06 +0200 Subject: [PATCH 4/6] Removed commented testfunction --- registry/txt.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/registry/txt.go b/registry/txt.go index 54451a2609..34ee93a0d0 100644 --- a/registry/txt.go +++ b/registry/txt.go @@ -217,18 +217,6 @@ func newaffixNameMapper(prefix string, suffix string) affixNameMapper { return affixNameMapper{prefix: strings.ToLower(prefix), suffix: strings.ToLower(suffix)} } -/* func (pr affixNameMapper) toEndpointName(txtDNSName string) string { - lowerDNSName := strings.ToLower(txtDNSName) - regex := regexp.MustCompile(`\.`) - DNSName := regex.Split(lowerDNSName, 2) - if (strings.HasPrefix(DNSName[0], pr.prefix) && strings.HasSuffix(DNSName[0], pr.suffix)) { - DNSName[0] = strings.TrimPrefix(DNSName[0], pr.prefix) - DNSName[0] = strings.TrimSuffix(DNSName[0], pr.suffix) - return DNSName[0] + "." + DNSName[1] - } - return "" -}*/ - func (pr affixNameMapper) toEndpointName(txtDNSName string) string { lowerDNSName := strings.ToLower(txtDNSName) if strings.HasPrefix(lowerDNSName, pr.prefix) && len(pr.suffix) == 0 { From 03db178dc79ecb3b5144465e41f8290cca2acd65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20Grumb=C3=B6ck?= Date: Tue, 12 May 2020 23:10:14 +0200 Subject: [PATCH 5/6] Fix tests --- registry/txt_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/registry/txt_test.go b/registry/txt_test.go index fd50b4b10a..5157dd560d 100644 --- a/registry/txt_test.go +++ b/registry/txt_test.go @@ -44,7 +44,7 @@ func TestTXTRegistry(t *testing.T) { func testTXTRegistryNew(t *testing.T) { p := inmemory.NewInMemoryProvider() - _, err := NewTXTRegistry(p, "txt", "", time.Hour) + _, err := NewTXTRegistry(p, "txt", "", "", time.Hour) require.Error(t, err) _, err = NewTXTRegistry(p, "", "txt", "", time.Hour) From 6cb94432cab606577a276ade7e27d61d5ea85ca1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20Grumb=C3=B6ck?= Date: Mon, 18 May 2020 07:15:49 +0000 Subject: [PATCH 6/6] Use strings instead of regex for splitting --- registry/txt.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/registry/txt.go b/registry/txt.go index 34ee93a0d0..0dd1bc2e3c 100644 --- a/registry/txt.go +++ b/registry/txt.go @@ -20,7 +20,6 @@ import ( "context" "errors" "fmt" - "regexp" "strings" "time" @@ -224,8 +223,7 @@ func (pr affixNameMapper) toEndpointName(txtDNSName string) string { } if len(pr.suffix) > 0 { - regex := regexp.MustCompile(`\.`) - DNSName := regex.Split(lowerDNSName, 2) + DNSName := strings.SplitN(lowerDNSName, ".", 2) if strings.HasSuffix(DNSName[0], pr.suffix) { return strings.TrimSuffix(DNSName[0], pr.suffix) + "." + DNSName[1] } @@ -234,8 +232,7 @@ func (pr affixNameMapper) toEndpointName(txtDNSName string) string { } func (pr affixNameMapper) toTXTName(endpointDNSName string) string { - regex := regexp.MustCompile(`\.`) - DNSName := regex.Split(endpointDNSName, 2) + DNSName := strings.SplitN(endpointDNSName, ".", 2) return pr.prefix + DNSName[0] + pr.suffix + "." + DNSName[1] }