refactor(registry): introduce factory with uniform New constructors

controller/controller_test.go

@@ -33,7 +33,7 @@ import (
 	"sigs.k8s.io/external-dns/plan"
 	"sigs.k8s.io/external-dns/provider"
 	"sigs.k8s.io/external-dns/provider/fakes"
-	"sigs.k8s.io/external-dns/registry"
+	registryfactory "sigs.k8s.io/external-dns/registry/factory"
 	"sigs.k8s.io/external-dns/registry/noop"
 
 	"github.com/stretchr/testify/assert"
@@ -157,7 +157,7 @@ func getTestSource() *testutils.MockSource {
 
 func getTestConfig() *externaldns.Config {
 	cfg := externaldns.NewConfig()
-	cfg.Registry = registry.NOOP
+	cfg.Registry = externaldns.RegistryNoop
 	cfg.ManagedDNSRecordTypes = []string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME}
 	return cfg
 }
@@ -216,7 +216,7 @@ func TestRunOnce(t *testing.T) {
 
 	emitter := fake.NewFakeEventEmitter()
 
-	r, err := registry.SelectRegistry(cfg, provider)
+	r, err := registryfactory.Select(cfg, provider)
 	require.NoError(t, err)
 
 	// Run our controller once to trigger the validation.
@@ -246,7 +246,7 @@ func TestRun(t *testing.T) {
 	cfg := getTestConfig()
 	provider := getTestProvider()
 
-	r, err := registry.SelectRegistry(cfg, provider)
+	r, err := registryfactory.Select(cfg, provider)
 	require.NoError(t, err)
 
 	// Run our controller once to trigger the validation.
@@ -334,7 +334,7 @@ func TestShouldRunOnce(t *testing.T) {
 func testControllerFiltersDomains(t *testing.T, configuredEndpoints []*endpoint.Endpoint, domainFilter *endpoint.DomainFilter, providerEndpoints []*endpoint.Endpoint, expectedChanges []*plan.Changes) {
 	t.Helper()
 	cfg := externaldns.NewConfig()
-	cfg.Registry = registry.NOOP
+	cfg.Registry = externaldns.RegistryNoop
 	cfg.ManagedDNSRecordTypes = []string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME}
 
 	source := new(testutils.MockSource)
@@ -344,7 +344,7 @@ func testControllerFiltersDomains(t *testing.T, configuredEndpoints []*endpoint.
 	provider := &filteredMockProvider{
 		RecordsStore: providerEndpoints,
 	}
-	r, err := registry.SelectRegistry(cfg, provider)
+	r, err := registryfactory.Select(cfg, provider)
 	require.NoError(t, err)
 
 	ctrl := &Controller{
@@ -584,7 +584,7 @@ func TestRunOnce_EmitChangeEvent(t *testing.T) {
 					WithRefObject(&events.ObjectReference{}),
 			}, nil)
 
-			r, err := registry.SelectRegistry(getTestConfig(), &fakes.MockProvider{ApplyChangesErr: tt.applyErr})
+			r, err := registryfactory.Select(getTestConfig(), &fakes.MockProvider{ApplyChangesErr: tt.applyErr})
 			require.NoError(t, err)
 
 			emitter := fake.NewFakeEventEmitter()

controller/execute.go

@@ -39,7 +39,7 @@ import (
 	"sigs.k8s.io/external-dns/provider"
 	providerfactory "sigs.k8s.io/external-dns/provider/factory"
 	webhookapi "sigs.k8s.io/external-dns/provider/webhook/api"
-	"sigs.k8s.io/external-dns/registry"
+	registryfactory "sigs.k8s.io/external-dns/registry/factory"
 	"sigs.k8s.io/external-dns/source"
 	"sigs.k8s.io/external-dns/source/annotations"
 	"sigs.k8s.io/external-dns/source/wrappers"
@@ -141,7 +141,7 @@ func buildController(
 	if !ok {
 		return nil, fmt.Errorf("unknown policy: %s", cfg.Policy)
 	}
-	reg, err := registry.SelectRegistry(cfg, p)
+	reg, err := registryfactory.Select(cfg, p)
 	if err != nil {
 		return nil, err
 	}

controller/metrics_test.go

@@ -25,7 +25,7 @@ import (
 	"sigs.k8s.io/external-dns/internal/testutils"
 	"sigs.k8s.io/external-dns/pkg/apis/externaldns"
 	"sigs.k8s.io/external-dns/plan"
-	"sigs.k8s.io/external-dns/registry"
+	registryfactory "sigs.k8s.io/external-dns/registry/factory"
 )
 
 func TestVerifyARecords(t *testing.T) {
@@ -323,7 +323,7 @@ func newMixedRecordsFixture() *Controller {
 	})
 
 	cfg := externaldns.NewConfig()
-	cfg.Registry = registry.NOOP
+	cfg.Registry = externaldns.RegistryNoop
 	cfg.ManagedDNSRecordTypes = endpoint.KnownRecordTypes
 
 	source := new(testutils.MockSource)
@@ -332,7 +332,7 @@ func newMixedRecordsFixture() *Controller {
 	provider := &filteredMockProvider{
 		RecordsStore: providerEndpoints,
 	}
-	r, _ := registry.SelectRegistry(cfg, provider)
+	r, _ := registryfactory.Select(cfg, provider)
 
 	return &Controller{
 		Source:             source,

pkg/apis/externaldns/constants.go

@@ -17,6 +17,11 @@ limitations under the License.
 package externaldns
 
 const (
+	RegistryTXT      = "txt"
+	RegistryNoop     = "noop"
+	RegistryDynamoDB = "dynamodb"
+	RegistryAWSSD    = "aws-sd"
+
 	ProviderAkamai       = "akamai"
 	ProviderAlibabaCloud = "alibabacloud"
 	ProviderAWS          = "aws"

pkg/apis/externaldns/types.go

@@ -347,7 +347,7 @@ var defaultConfig = &Config{
 	PublishInternal:              false,
 	RegexDomainExclude:           regexp.MustCompile(""),
 	RegexDomainFilter:            regexp.MustCompile(""),
-	Registry:                     "txt",
+	Registry:                     RegistryTXT,
 	RequestTimeout:               time.Second * 30,
 	RFC2136BatchChangeSize:       50,
 	RFC2136GSSTSIG:               false,
@@ -682,7 +682,7 @@ func bindFlags(b flags.FlagBinder, cfg *Config) {
 	b.EnumVar("policy", "Modify how DNS records are synchronized between sources and providers (default: sync, options: sync, upsert-only, create-only)", defaultConfig.Policy, &cfg.Policy, "sync", "upsert-only", "create-only")
 
 	// Flags related to the registry
-	b.EnumVar("registry", "The registry implementation to use to keep track of DNS record ownership (default: txt, options: txt, noop, dynamodb, aws-sd)", defaultConfig.Registry, &cfg.Registry, "txt", "noop", "dynamodb", "aws-sd")
+	b.EnumVar("registry", "The registry implementation to use to keep track of DNS record ownership (default: txt, options: txt, noop, dynamodb, aws-sd)", defaultConfig.Registry, &cfg.Registry, RegistryTXT, RegistryNoop, RegistryDynamoDB, RegistryAWSSD)
 	b.StringVar("txt-owner-id", "When using the TXT or DynamoDB registry, a name that identifies this instance of ExternalDNS (default: default)", defaultConfig.TXTOwnerID, &cfg.TXTOwnerID)
 	b.StringVar("txt-prefix", "When using the TXT registry, a custom string that's prefixed to each ownership DNS record (optional). Could contain record type template like '%{record_type}-prefix-'. Mutual exclusive with txt-suffix!", defaultConfig.TXTPrefix, &cfg.TXTPrefix)
 	b.StringVar("txt-suffix", "When using the TXT registry, a custom string that's suffixed to the host portion of each ownership DNS record (optional). Could contain record type template like '-%{record_type}-suffix'. Mutual exclusive with txt-prefix!", defaultConfig.TXTSuffix, &cfg.TXTSuffix)

registry/awssd/registry.go

@@ -21,8 +21,10 @@ import (
 	"errors"
 
 	"sigs.k8s.io/external-dns/endpoint"
+	"sigs.k8s.io/external-dns/pkg/apis/externaldns"
 	"sigs.k8s.io/external-dns/plan"
 	"sigs.k8s.io/external-dns/provider"
+	"sigs.k8s.io/external-dns/registry"
 )
 
 // AWSSDRegistry implements registry interface with ownership information associated via the Description field of SD Service
@@ -31,8 +33,13 @@ type AWSSDRegistry struct {
 	ownerID  string
 }
 
-// NewAWSSDRegistry returns implementation of registry for AWS SD
-func NewAWSSDRegistry(provider provider.Provider, ownerID string) (*AWSSDRegistry, error) {
+// New creates an AWSSDRegistry from the given configuration.
+func New(cfg *externaldns.Config, p provider.Provider) (registry.Registry, error) {
+	return newRegistry(p, cfg.TXTOwnerID)
+}
+
+// newRegistry returns implementation of registry for AWS SD
+func newRegistry(provider provider.Provider, ownerID string) (*AWSSDRegistry, error) {
 	if ownerID == "" {
 		return nil, errors.New("owner id cannot be empty")
 	}

registry/awssd/registry_test.go

@@ -51,12 +51,12 @@ func newInMemoryProvider(endpoints []*endpoint.Endpoint, onApplyChanges func(cha
 	}
 }
 
-func TestAWSSDRegistry_NewAWSSDRegistry(t *testing.T) {
+func TestAWSSDRegistry_newRegistry(t *testing.T) {
 	p := newInMemoryProvider(nil, nil)
-	_, err := NewAWSSDRegistry(p, "")
+	_, err := newRegistry(p, "")
 	require.Error(t, err)
 
-	_, err = NewAWSSDRegistry(p, "owner")
+	_, err = newRegistry(p, "owner")
 	require.NoError(t, err)
 }
 
@@ -102,7 +102,7 @@ func TestAWSSDRegistryTest_Records(t *testing.T) {
 		},
 	}
 
-	r, _ := NewAWSSDRegistry(p, "records-owner")
+	r, _ := newRegistry(p, "records-owner")
 	records, _ := r.Records(t.Context())
 
 	assert.True(t, testutils.SameEndpoints(records, expectedRecords))
@@ -155,7 +155,7 @@ func TestAWSSDRegistry_Records_ApplyChanges(t *testing.T) {
 		}
 		assert.True(t, testutils.SamePlanChanges(mGot, mExpected))
 	})
-	r, err := NewAWSSDRegistry(p, "owner")
+	r, err := newRegistry(p, "owner")
 	require.NoError(t, err)
 
 	err = r.ApplyChanges(t.Context(), changes)

registry/dynamodb/registry.go

@@ -27,22 +27,25 @@ import (
 
 	"github.com/aws/aws-sdk-go-v2/aws"
 	"github.com/aws/aws-sdk-go-v2/feature/dynamodb/attributevalue"
-	"github.com/aws/aws-sdk-go-v2/service/dynamodb"
+	awsdynamodb "github.com/aws/aws-sdk-go-v2/service/dynamodb"
 	dynamodbtypes "github.com/aws/aws-sdk-go-v2/service/dynamodb/types"
 	log "github.com/sirupsen/logrus"
 	"k8s.io/apimachinery/pkg/util/sets"
 
 	"sigs.k8s.io/external-dns/endpoint"
+	"sigs.k8s.io/external-dns/pkg/apis/externaldns"
 	"sigs.k8s.io/external-dns/plan"
 	"sigs.k8s.io/external-dns/provider"
+	provideraws "sigs.k8s.io/external-dns/provider/aws"
+	"sigs.k8s.io/external-dns/registry"
 	"sigs.k8s.io/external-dns/registry/mapper"
 )
 
 // DynamoDBAPI is the subset of the AWS DynamoDB API that we actually use.  Add methods as required. Signatures must match exactly.
 type DynamoDBAPI interface {
-	DescribeTable(context.Context, *dynamodb.DescribeTableInput, ...func(*dynamodb.Options)) (*dynamodb.DescribeTableOutput, error)
-	Scan(context.Context, *dynamodb.ScanInput, ...func(*dynamodb.Options)) (*dynamodb.ScanOutput, error)
-	BatchExecuteStatement(context.Context, *dynamodb.BatchExecuteStatementInput, ...func(*dynamodb.Options)) (*dynamodb.BatchExecuteStatementOutput, error)
+	DescribeTable(context.Context, *awsdynamodb.DescribeTableInput, ...func(*awsdynamodb.Options)) (*awsdynamodb.DescribeTableOutput, error)
+	Scan(context.Context, *awsdynamodb.ScanInput, ...func(*awsdynamodb.Options)) (*awsdynamodb.ScanOutput, error)
+	BatchExecuteStatement(context.Context, *awsdynamodb.BatchExecuteStatementInput, ...func(*awsdynamodb.Options)) (*awsdynamodb.BatchExecuteStatementOutput, error)
 }
 
 // DynamoDBRegistry implements registry interface with ownership implemented via an AWS DynamoDB table.
@@ -75,8 +78,16 @@ const dynamodbAttributeMigrate = "dynamodb/needs-migration"
 // DynamoDB allows a maximum batch size of 25 items.
 var dynamodbMaxBatchSize uint8 = 25
 
-// NewDynamoDBRegistry returns a new DynamoDBRegistry object.
-func NewDynamoDBRegistry(
+// New creates a DynamoDBRegistry from the given configuration.
+func New(cfg *externaldns.Config, p provider.Provider) (registry.Registry, error) {
+	client := awsdynamodb.NewFromConfig(provideraws.CreateDefaultV2Config(cfg), WithRegion(cfg.AWSDynamoDBRegion))
+	return newRegistry(p, cfg.TXTOwnerID, client,
+		cfg.AWSDynamoDBTable, cfg.TXTPrefix, cfg.TXTSuffix, cfg.TXTWildcardReplacement,
+		cfg.ManagedDNSRecordTypes, cfg.ExcludeDNSRecordTypes, []byte(cfg.TXTEncryptAESKey), cfg.TXTCacheInterval)
+}
+
+// newRegistry returns a new DynamoDBRegistry object.
+func newRegistry(
 	provider provider.Provider,
 	ownerID string, dynamodbAPI DynamoDBAPI,
 	table, txtPrefix, txtSuffix, txtWildcardReplacement string,
@@ -371,7 +382,7 @@ func (im *DynamoDBRegistry) AdjustEndpoints(endpoints []*endpoint.Endpoint) ([]*
 }
 
 func (im *DynamoDBRegistry) readLabels(ctx context.Context) error {
-	table, err := im.dynamodbAPI.DescribeTable(ctx, &dynamodb.DescribeTableInput{
+	table, err := im.dynamodbAPI.DescribeTable(ctx, &awsdynamodb.DescribeTableInput{
 		TableName: aws.String(im.table),
 	})
 	if err != nil {
@@ -399,7 +410,7 @@ func (im *DynamoDBRegistry) readLabels(ctx context.Context) error {
 	}
 
 	labels := map[endpoint.EndpointKey]endpoint.Labels{}
-	scanPaginator := dynamodb.NewScanPaginator(im.dynamodbAPI, &dynamodb.ScanInput{
+	scanPaginator := awsdynamodb.NewScanPaginator(im.dynamodbAPI, &awsdynamodb.ScanInput{
 		TableName:        aws.String(im.table),
 		FilterExpression: aws.String("o = :ownerval"),
 		ExpressionAttributeValues: map[string]dynamodbtypes.AttributeValue{
@@ -528,7 +539,7 @@ func (im *DynamoDBRegistry) executeStatements(ctx context.Context, statements []
 			statements = nil
 		}
 
-		output, err := im.dynamodbAPI.BatchExecuteStatement(ctx, &dynamodb.BatchExecuteStatementInput{
+		output, err := im.dynamodbAPI.BatchExecuteStatement(ctx, &awsdynamodb.BatchExecuteStatementInput{
 			Statements: chunk,
 		})
 		if err != nil {
@@ -580,11 +591,11 @@ func (im *DynamoDBRegistry) removeFromCache(ep *endpoint.Endpoint) {
 	}
 }
 
-func WithRegion(region string) func(*dynamodb.Options) {
+func WithRegion(region string) func(*awsdynamodb.Options) {
 	if region == "" {
 		return nil
 	}
-	return func(opts *dynamodb.Options) {
+	return func(opts *awsdynamodb.Options) {
 		opts.Region = region
 	}
 }

registry/dynamodb/registry_test.go

@@ -44,31 +44,31 @@ const (
 func TestDynamoDBRegistryNew(t *testing.T) {
 	api, p := newDynamoDBAPIStub(t, nil)
 
-	_, err := NewDynamoDBRegistry(p, "test-owner", api, "test-table", "", "", "", []string{}, []string{}, []byte(""), time.Hour)
+	_, err := newRegistry(p, "test-owner", api, "test-table", "", "", "", []string{}, []string{}, []byte(""), time.Hour)
 	require.NoError(t, err)
 
-	_, err = NewDynamoDBRegistry(p, "test-owner", api, "test-table", "testPrefix", "", "", []string{}, []string{}, []byte(""), time.Hour)
+	_, err = newRegistry(p, "test-owner", api, "test-table", "testPrefix", "", "", []string{}, []string{}, []byte(""), time.Hour)
 	require.NoError(t, err)
 
-	_, err = NewDynamoDBRegistry(p, "test-owner", api, "test-table", "", "testSuffix", "", []string{}, []string{}, []byte(""), time.Hour)
+	_, err = newRegistry(p, "test-owner", api, "test-table", "", "testSuffix", "", []string{}, []string{}, []byte(""), time.Hour)
 	require.NoError(t, err)
 
-	_, err = NewDynamoDBRegistry(p, "test-owner", api, "test-table", "", "", "testWildcard", []string{}, []string{}, []byte(""), time.Hour)
+	_, err = newRegistry(p, "test-owner", api, "test-table", "", "", "testWildcard", []string{}, []string{}, []byte(""), time.Hour)
 	require.NoError(t, err)
 
-	_, err = NewDynamoDBRegistry(p, "test-owner", api, "test-table", "", "", "testWildcard", []string{}, []string{}, []byte(";k&l)nUC/33:{?d{3)54+,AD?]SX%yh^"), time.Hour)
+	_, err = newRegistry(p, "test-owner", api, "test-table", "", "", "testWildcard", []string{}, []string{}, []byte(";k&l)nUC/33:{?d{3)54+,AD?]SX%yh^"), time.Hour)
 	require.NoError(t, err)
 
-	_, err = NewDynamoDBRegistry(p, "", api, "test-table", "", "", "", []string{}, []string{}, []byte(""), time.Hour)
+	_, err = newRegistry(p, "", api, "test-table", "", "", "", []string{}, []string{}, []byte(""), time.Hour)
 	require.EqualError(t, err, "owner id cannot be empty")
 
-	_, err = NewDynamoDBRegistry(p, "test-owner", api, "", "", "", "", []string{}, []string{}, []byte(""), time.Hour)
+	_, err = newRegistry(p, "test-owner", api, "", "", "", "", []string{}, []string{}, []byte(""), time.Hour)
 	require.EqualError(t, err, "table cannot be empty")
 
-	_, err = NewDynamoDBRegistry(p, "test-owner", api, "test-table", "", "", "", []string{}, []string{}, []byte(";k&l)nUC/33:{?d{3)54+,AD?]SX%yh^x"), time.Hour)
+	_, err = newRegistry(p, "test-owner", api, "test-table", "", "", "", []string{}, []string{}, []byte(";k&l)nUC/33:{?d{3)54+,AD?]SX%yh^x"), time.Hour)
 	require.EqualError(t, err, "the AES Encryption key must be 32 bytes long, in either plain text or base64-encoded format")
 
-	_, err = NewDynamoDBRegistry(p, "test-owner", api, "test-table", "testPrefix", "testSuffix", "", []string{}, []string{}, []byte(""), time.Hour)
+	_, err = newRegistry(p, "test-owner", api, "test-table", "testPrefix", "testSuffix", "", []string{}, []string{}, []byte(""), time.Hour)
 	require.EqualError(t, err, "txt-prefix and txt-suffix are mutually exclusive")
 }
 
@@ -101,7 +101,7 @@ func TestDynamoDBRegistryNew_EncryptionConfig(t *testing.T) {
 		},
 	}
 	for _, test := range tests {
-		actual, err := NewDynamoDBRegistry(p, "test-owner", api, "test-table", "", "", "", []string{}, []string{}, test.aesKeyRaw, time.Hour)
+		actual, err := newRegistry(p, "test-owner", api, "test-table", "", "", "", []string{}, []string{}, test.aesKeyRaw, time.Hour)
 		if test.errorExpected {
 			require.Error(t, err)
 		} else {
@@ -157,7 +157,7 @@ func TestDynamoDBRegistryRecordsBadTable(t *testing.T) {
 			api, p := newDynamoDBAPIStub(t, nil)
 			tc.setup(&api.tableDescription)
 
-			r, _ := NewDynamoDBRegistry(p, "test-owner", api, "test-table", "", "", "", []string{}, []string{}, nil, time.Hour)
+			r, _ := newRegistry(p, "test-owner", api, "test-table", "", "", "", []string{}, []string{}, nil, time.Hour)
 
 			_, err := r.Records(t.Context())
 			assert.EqualError(t, err, tc.expected)
@@ -243,7 +243,7 @@ func TestDynamoDBRegistryRecords(t *testing.T) {
 		},
 	}
 
-	r, _ := NewDynamoDBRegistry(p, "test-owner", api, "test-table", "txt.", "", "", []string{}, []string{}, nil, time.Hour)
+	r, _ := newRegistry(p, "test-owner", api, "test-table", "txt.", "", "", []string{}, []string{}, nil, time.Hour)
 	_ = p.(*wrappedProvider).Provider.ApplyChanges(t.Context(), &plan.Changes{
 		Create: []*endpoint.Endpoint{
 			endpoint.NewEndpoint("migrate.test-zone.example.org", endpoint.RecordTypeA, "3.3.3.3").WithSetIdentifier("set-3"),
@@ -1083,7 +1083,7 @@ func TestDynamoDBRegistryApplyChanges(t *testing.T) {
 
 			ctx := t.Context()
 
-			r, _ := NewDynamoDBRegistry(p, "test-owner", api, "test-table", "txt.", "", "", []string{}, []string{}, nil, time.Hour)
+			r, _ := newRegistry(p, "test-owner", api, "test-table", "txt.", "", "", []string{}, []string{}, nil, time.Hour)
 			_, err := r.Records(ctx)
 			require.NoError(t, err)