Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: dynamodb registry when statements over 25 #4021

Merged
merged 5 commits into from
Nov 10, 2023
Merged

fix: dynamodb registry when statements over 25 #4021

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4a8dbc5
fix: dynamodb registry when statements over 25
danie1sullivan Nov 7, 2023
b83935b
Apply suggestions from code review
danie1sullivan Nov 9, 2023
5469ab6
Use variable for DynomoDB batch size
danie1sullivan Nov 9, 2023
f8807ac
Shorten batch size test
danie1sullivan Nov 9, 2023
d828e51
Fix batch size override in test
danie1sullivan Nov 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 6 additions & 3 deletions registry/dynamodb.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ type DynamoDBRegistry struct {

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(provider provider.Provider, ownerID string, dynamodbAPI DynamoDBAPI, table string, txtPrefix, txtSuffix, txtWildcardReplacement string, managedRecordTypes, excludeRecordTypes []string, txtEncryptAESKey []byte, cacheInterval time.Duration) (*DynamoDBRegistry, error) {
if ownerID == "" {
Expand Down Expand Up @@ -477,9 +480,9 @@ func (im *DynamoDBRegistry) appendDelete(statements []*dynamodb.BatchStatementRe
func (im *DynamoDBRegistry) executeStatements(ctx context.Context, statements []*dynamodb.BatchStatementRequest, handleErr func(request *dynamodb.BatchStatementRequest, response *dynamodb.BatchStatementResponse) error) error {
for len(statements) > 0 {
var chunk []*dynamodb.BatchStatementRequest
if len(statements) > 25 {
chunk = chunk[:25]
statements = statements[25:]
if len(statements) > int(dynamodbMaxBatchSize) {
chunk = statements[:dynamodbMaxBatchSize]
statements = statements[dynamodbMaxBatchSize:]
} else {
chunk = statements
statements = nil
Expand Down
120 changes: 120 additions & 0 deletions registry/dynamodb_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ func TestDynamoDBRegistryRecords(t *testing.T) {
func TestDynamoDBRegistryApplyChanges(t *testing.T) {
for _, tc := range []struct {
name string
maxBatchSize uint8
stubConfig DynamoDBStubConfig
addRecords []*endpoint.Endpoint
changes plan.Changes
Expand Down Expand Up @@ -294,6 +295,118 @@ func TestDynamoDBRegistryApplyChanges(t *testing.T) {
},
},
},
{
name: "create more entries than DynamoDB batch size limit",
maxBatchSize: 2,
changes: plan.Changes{
Create: []*endpoint.Endpoint{
{
DNSName: "new1.test-zone.example.org",
Targets: endpoint.Targets{"new1.loadbalancer.com"},
RecordType: endpoint.RecordTypeCNAME,
SetIdentifier: "set-new",
Labels: map[string]string{
endpoint.ResourceLabelKey: "ingress/default/new1-ingress",
},
},
{
DNSName: "new2.test-zone.example.org",
Targets: endpoint.Targets{"new2.loadbalancer.com"},
RecordType: endpoint.RecordTypeCNAME,
SetIdentifier: "set-new",
Labels: map[string]string{
endpoint.ResourceLabelKey: "ingress/default/new2-ingress",
},
},
{
DNSName: "new3.test-zone.example.org",
Targets: endpoint.Targets{"new3.loadbalancer.com"},
RecordType: endpoint.RecordTypeCNAME,
SetIdentifier: "set-new",
Labels: map[string]string{
endpoint.ResourceLabelKey: "ingress/default/new3-ingress",
},
},
},
},
stubConfig: DynamoDBStubConfig{
ExpectInsert: map[string]map[string]string{
"new1.test-zone.example.org#CNAME#set-new": {endpoint.ResourceLabelKey: "ingress/default/new1-ingress"},
"new2.test-zone.example.org#CNAME#set-new": {endpoint.ResourceLabelKey: "ingress/default/new2-ingress"},
"new3.test-zone.example.org#CNAME#set-new": {endpoint.ResourceLabelKey: "ingress/default/new3-ingress"},
},
ExpectDelete: sets.New("quux.test-zone.example.org#A#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: "",
},
},
{
DNSName: "bar.test-zone.example.org",
Targets: endpoint.Targets{"my-domain.com"},
RecordType: endpoint.RecordTypeCNAME,
Labels: map[string]string{
endpoint.OwnerLabelKey: "test-owner",
endpoint.ResourceLabelKey: "ingress/default/my-ingress",
},
},
{
DNSName: "baz.test-zone.example.org",
Targets: endpoint.Targets{"1.1.1.1"},
RecordType: endpoint.RecordTypeA,
SetIdentifier: "set-1",
Labels: map[string]string{
endpoint.OwnerLabelKey: "test-owner",
endpoint.ResourceLabelKey: "ingress/default/my-ingress",
},
},
{
DNSName: "baz.test-zone.example.org",
Targets: endpoint.Targets{"2.2.2.2"},
RecordType: endpoint.RecordTypeA,
SetIdentifier: "set-2",
Labels: map[string]string{
endpoint.OwnerLabelKey: "test-owner",
endpoint.ResourceLabelKey: "ingress/default/other-ingress",
},
},
{
DNSName: "new1.test-zone.example.org",
Targets: endpoint.Targets{"new1.loadbalancer.com"},
RecordType: endpoint.RecordTypeCNAME,
SetIdentifier: "set-new",
Labels: map[string]string{
endpoint.OwnerLabelKey: "test-owner",
endpoint.ResourceLabelKey: "ingress/default/new1-ingress",
},
},
{
DNSName: "new2.test-zone.example.org",
Targets: endpoint.Targets{"new2.loadbalancer.com"},
RecordType: endpoint.RecordTypeCNAME,
SetIdentifier: "set-new",
Labels: map[string]string{
endpoint.OwnerLabelKey: "test-owner",
endpoint.ResourceLabelKey: "ingress/default/new2-ingress",
},
},
{
DNSName: "new3.test-zone.example.org",
Targets: endpoint.Targets{"new3.loadbalancer.com"},
RecordType: endpoint.RecordTypeCNAME,
SetIdentifier: "set-new",
Labels: map[string]string{
endpoint.OwnerLabelKey: "test-owner",
endpoint.ResourceLabelKey: "ingress/default/new3-ingress",
},
},
},
},
{
name: "create orphaned",
changes: plan.Changes{
Expand Down Expand Up @@ -911,6 +1024,11 @@ func TestDynamoDBRegistryApplyChanges(t *testing.T) {
},
} {
t.Run(tc.name, func(t *testing.T) {
originalMaxBatchSize := dynamodbMaxBatchSize
if tc.maxBatchSize > 0 {
dynamodbMaxBatchSize = tc.maxBatchSize
}

api, p := newDynamoDBAPIStub(t, &tc.stubConfig)
if len(tc.addRecords) > 0 {
_ = p.(*wrappedProvider).Provider.ApplyChanges(context.Background(), &plan.Changes{
Expand Down Expand Up @@ -945,6 +1063,8 @@ func TestDynamoDBRegistryApplyChanges(t *testing.T) {
if tc.expectedError == "" {
assert.Empty(t, r.orphanedLabels)
}

dynamodbMaxBatchSize = originalMaxBatchSize
})
}
}
Expand Down