Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 37 additions & 8 deletions provider/akamai/akamai.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,12 @@ func (p AkamaiProvider) Records(context.Context) ([]*endpoint.Endpoint, error) {

// ApplyChanges applies a given set of changes in a given zone.
func (p AkamaiProvider) ApplyChanges(_ context.Context, changes *plan.Changes) error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
func (p AkamaiProvider) ApplyChanges(_ context.Context, changes *plan.Changes) error {
func (p AkamaiProvider) ApplyChanges(_ context.Context, changes *plan.Changes) error {
// return early if there is nothing to change
if len(changes.Create) == 0 && len(changes.Delete) == 0 && len(changes.UpdateNew) == 0 {
log.Info("All records are already up to date")
return nil
}
zoneNameIDMapper := provider.ZoneIDName{}
zones, err := p.fetchZones()
if err != nil {
log.Errorf("Failed to fetch zones from Akamai")
return err
}
for _, z := range zones.Zones {
zoneNameIDMapper[z.Zone] = z.Zone
}
log.Debugf("Processing zones: [%v]", zoneNameIDMapper)
// Create recordsets
log.Debugf("Create Changes requested [%v]", changes.Create)
if err := p.createRecordsets(zoneNameIDMapper, changes.Create); err != nil {
return err
}
// Delete recordsets
log.Debugf("Delete Changes requested [%v]", changes.Delete)
if err := p.deleteRecordsets(zoneNameIDMapper, changes.Delete); err != nil {
return err
}
// Update recordsets
log.Debugf("Update Changes requested [%v]", changes.UpdateNew)
if err := p.updateNewRecordsets(zoneNameIDMapper, changes.UpdateNew); err != nil {
return err
}
// Check that all old endpoints were accounted for
revRecs := changes.Delete
revRecs = append(revRecs, changes.UpdateNew...)
for _, rec := range changes.UpdateOld {
found := false
for _, r := range revRecs {
if rec.DNSName == r.DNSName {
found = true
break
}
}
if !found {
log.Warnf("UpdateOld endpoint '%s' is not accounted for in UpdateNew|Delete endpoint list", rec.DNSName)
}
}
return nil
}

// return early if there is nothing to change
if len(changes.Create) == 0 && len(changes.Delete) == 0 && len(changes.UpdateNew) == 0 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if len(changes.Create) == 0 && len(changes.Delete) == 0 && len(changes.UpdateNew) == 0 {
if !changes.HasChanges() {

and most likely log.Debug, not Info

log.Info("All records are already up to date")
return nil
}

zoneNameIDMapper := provider.ZoneIDName{}
zones, err := p.fetchZones()
if err != nil {
Expand All @@ -268,24 +274,35 @@ func (p AkamaiProvider) ApplyChanges(_ context.Context, changes *plan.Changes) e
}
log.Debugf("Processing zones: [%v]", zoneNameIDMapper)

// Filter changes to only those matching managed zones
filteredCreate := filterEndpointsByZone(zoneNameIDMapper, changes.Create)
filteredDelete := filterEndpointsByZone(zoneNameIDMapper, changes.Delete)
filteredUpdateNew := filterEndpointsByZone(zoneNameIDMapper, changes.UpdateNew)

// Return early if no changes match any managed zones
if len(filteredCreate) == 0 && len(filteredDelete) == 0 && len(filteredUpdateNew) == 0 {
log.Info("All records are already up to date, there are no changes for the matching hosted zones")
return nil
}

// Create recordsets
log.Debugf("Create Changes requested [%v]", changes.Create)
if err := p.createRecordsets(zoneNameIDMapper, changes.Create); err != nil {
log.Debugf("Create Changes requested [%v]", filteredCreate)
if err := p.createRecordsets(zoneNameIDMapper, filteredCreate); err != nil {
return err
}
// Delete recordsets
log.Debugf("Delete Changes requested [%v]", changes.Delete)
if err := p.deleteRecordsets(zoneNameIDMapper, changes.Delete); err != nil {
log.Debugf("Delete Changes requested [%v]", filteredDelete)
if err := p.deleteRecordsets(zoneNameIDMapper, filteredDelete); err != nil {
return err
}
// Update recordsets
log.Debugf("Update Changes requested [%v]", changes.UpdateNew)
if err := p.updateNewRecordsets(zoneNameIDMapper, changes.UpdateNew); err != nil {
log.Debugf("Update Changes requested [%v]", filteredUpdateNew)
if err := p.updateNewRecordsets(zoneNameIDMapper, filteredUpdateNew); err != nil {
return err
}
// Check that all old endpoints were accounted for
revRecs := changes.Delete
revRecs = append(revRecs, changes.UpdateNew...)
revRecs := filteredDelete
revRecs = append(revRecs, filteredUpdateNew...)
for _, rec := range changes.UpdateOld {
found := false
for _, r := range revRecs {
Expand Down Expand Up @@ -484,3 +501,15 @@ func edgeChangesByZone(zoneMap provider.ZoneIDName, endpoints []*endpoint.Endpoi

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// remove empty zones to avoid no-op API calls
for zone, eps := range createsByZone {
if len(eps) == 0 {
delete(createsByZone, zone)
}
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That eliminates the empty API calls at the source. Then filterEndpointsByZone and the second early return become unnecessary and can be removed. Still needs testing, could be that the unit tests are not right or smth.

return createsByZone
}

// filterEndpointsByZone returns only endpoints that match a zone in the mapper
func filterEndpointsByZone(zoneNameIDMapper provider.ZoneIDName, endpoints []*endpoint.Endpoint) []*endpoint.Endpoint {
filtered := make([]*endpoint.Endpoint, 0, len(endpoints))
for _, ep := range endpoints {
zoneName, _ := zoneNameIDMapper.FindZone(ep.DNSName)
if zoneName != "" {
filtered = append(filtered, ep)
}
}
return filtered
}
51 changes: 51 additions & 0 deletions provider/akamai/akamai_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -369,3 +369,54 @@ func TestAkamaiApplyChanges(t *testing.T) {
apply := c.ApplyChanges(context.Background(), changes)
assert.NoError(t, apply)
}

func TestAkamaiApplyChangesEmpty(t *testing.T) {
stub := newStub()
domfilter := endpoint.NewDomainFilter([]string{"example.com"})
idfilter := provider.ZoneIDFilter{}
c, err := createAkamaiStubProvider(stub, domfilter, idfilter)
require.NoError(t, err)

// Don't set up any zone output - if fetchZones is called, it would return empty
// but we're testing that it's not called at all
changes := &plan.Changes{
Create: []*endpoint.Endpoint{},
Delete: []*endpoint.Endpoint{},
UpdateOld: []*endpoint.Endpoint{},
UpdateNew: []*endpoint.Endpoint{},
}

err = c.ApplyChanges(context.Background(), changes)
assert.NoError(t, err)
}

func TestAkamaiApplyChangesFilteredByDomain(t *testing.T) {
stub := newStub()
domfilter := endpoint.NewDomainFilter([]string{"example.com"})
idfilter := provider.ZoneIDFilter{}
c, err := createAkamaiStubProvider(stub, domfilter, idfilter)
require.NoError(t, err)

// Set up zones for example.com only
stub.setOutput("zone", []any{"example.com"})

// All changes are for other.com which doesn't match the domain filter
// These should be filtered out and no API calls should be made
changes := &plan.Changes{
Create: []*endpoint.Endpoint{
{DNSName: "www.other.com", RecordType: "A", Targets: endpoint.Targets{"10.0.0.1"}, RecordTTL: 300},
},
Delete: []*endpoint.Endpoint{
{DNSName: "delete.other.com", RecordType: "A", Targets: endpoint.Targets{"10.0.0.2"}, RecordTTL: 300},
},
UpdateOld: []*endpoint.Endpoint{
{DNSName: "update.other.com", RecordType: "A", Targets: endpoint.Targets{"10.0.0.3"}, RecordTTL: 300},
},
UpdateNew: []*endpoint.Endpoint{
{DNSName: "update.other.com", RecordType: "A", Targets: endpoint.Targets{"10.0.0.4"}, RecordTTL: 300},
},
}

err = c.ApplyChanges(context.Background(), changes)
assert.NoError(t, err)
}
Loading