mirrors/external-dns
1
0
Fork 0
mirror of https://github.com/kubernetes-sigs/external-dns.git synced 2025-08-05 17:16:59 +02:00
Code Issues Packages Projects Releases Wiki Activity

Remove PropertyValuesEqual method from Provider interface

Browse Source
This commit is contained in:
John Gardiner Myers 2023-06-16 20:49:27 -07:00
parent 4fb2c7470a
commit 5affab0561
13 changed files with 48 additions and 165 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
controller/controller.go
View File

@ -217,12 +217,11 @@ func (c *Controller) RunOnce(ctx context.Context) error {
endpoints = c.Registry.AdjustEndpoints(endpoints)
plan := &plan.Plan{
Policies: []plan.Policy{c.Policy},
Current: records,
Desired: endpoints,
DomainFilter: endpoint.MatchAllDomainFilters{c.DomainFilter, c.Registry.GetDomainFilter()},
PropertyComparator: c.Registry.PropertyValuesEqual,
ManagedRecords: c.ManagedRecordTypes,
Policies: []plan.Policy{c.Policy},
Current: records,
Desired: endpoints,
DomainFilter: endpoint.MatchAllDomainFilters{c.DomainFilter, c.Registry.GetDomainFilter()},
ManagedRecords: c.ManagedRecordTypes,
}
plan = plan.Calculate()

18
plan/plan.go
View File

@ -44,8 +44,6 @@ type Plan struct {
Changes *Changes
// DomainFilter matches DNS names
DomainFilter endpoint.DomainFilterInterface
// Property comparator compares custom properties of providers
PropertyComparator PropertyComparator
// DNS record types that will be considered for management
ManagedRecords []string
}
@ -217,21 +215,9 @@ func (p *Plan) shouldUpdateProviderSpecific(desired, current *endpoint.Endpoint)
if current.ProviderSpecific != nil {
for _, c := range current.ProviderSpecific {
if d, ok := desiredProperties[c.Name]; ok {
if p.PropertyComparator != nil {
if !p.PropertyComparator(c.Name, c.Value, d.Value) {
return true
}
} else if c.Value != d.Value {
return true
}
return c.Value != d.Value
} else {
if p.PropertyComparator != nil {
if !p.PropertyComparator(c.Name, c.Value, "") {
return true
}
} else if c.Value != "" {
return true
}
return c.Value != ""
}
}
}

131
plan/plan_test.go
View File

@ -342,21 +342,18 @@ func (suite *PlanTestSuite) TestSyncSecondRoundWithProviderSpecificChange() {
validateEntries(suite.T(), changes.Delete, expectedDelete)
}
func (suite *PlanTestSuite) TestSyncSecondRoundWithProviderSpecificDefaultFalse() {
func (suite *PlanTestSuite) TestSyncSecondRoundWithProviderSpecificRemoval() {
current := []*endpoint.Endpoint{suite.bar127AWithProviderSpecificFalse}
desired := []*endpoint.Endpoint{suite.bar127AWithProviderSpecificUnset}
expectedCreate := []*endpoint.Endpoint{}
expectedUpdateOld := []*endpoint.Endpoint{}
expectedUpdateNew := []*endpoint.Endpoint{}
expectedUpdateOld := []*endpoint.Endpoint{suite.bar127AWithProviderSpecificFalse}
expectedUpdateNew := []*endpoint.Endpoint{suite.bar127AWithProviderSpecificUnset}
expectedDelete := []*endpoint.Endpoint{}
p := &Plan{
Policies: []Policy{&SyncPolicy{}},
Current: current,
Desired: desired,
PropertyComparator: func(name, previous, current string) bool {
return CompareBoolean(false, name, previous, current)
},
Policies: []Policy{&SyncPolicy{}},
Current: current,
Desired: desired,
ManagedRecords: []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME},
}
@ -367,29 +364,28 @@ func (suite *PlanTestSuite) TestSyncSecondRoundWithProviderSpecificDefaultFalse(
validateEntries(suite.T(), changes.Delete, expectedDelete)
}
func (suite *PlanTestSuite) TestSyncSecondRoundWithProviderSpecificDefualtTrue() {
current := []*endpoint.Endpoint{suite.bar127AWithProviderSpecificTrue}
desired := []*endpoint.Endpoint{suite.bar127AWithProviderSpecificUnset}
expectedCreate := []*endpoint.Endpoint{}
expectedUpdateOld := []*endpoint.Endpoint{}
expectedUpdateNew := []*endpoint.Endpoint{}
expectedDelete := []*endpoint.Endpoint{}
p := &Plan{
Policies: []Policy{&SyncPolicy{}},
Current: current,
Desired: desired,
PropertyComparator: func(name, previous, current string) bool {
return CompareBoolean(true, name, previous, current)
},
}
changes := p.Calculate().Changes
validateEntries(suite.T(), changes.Create, expectedCreate)
validateEntries(suite.T(), changes.UpdateNew, expectedUpdateNew)
validateEntries(suite.T(), changes.UpdateOld, expectedUpdateOld)
validateEntries(suite.T(), changes.Delete, expectedDelete)
}
// todo: this is currently an expect-fail
//func (suite *PlanTestSuite) TestSyncSecondRoundWithProviderSpecificAddition() {
// current := []*endpoint.Endpoint{suite.bar127AWithProviderSpecificUnset}
// desired := []*endpoint.Endpoint{suite.bar127AWithProviderSpecificTrue}
// expectedCreate := []*endpoint.Endpoint{}
// expectedUpdateOld := []*endpoint.Endpoint{suite.bar127AWithProviderSpecificUnset}
// expectedUpdateNew := []*endpoint.Endpoint{suite.bar127AWithProviderSpecificTrue}
// expectedDelete := []*endpoint.Endpoint{}
//
// p := &Plan{
// Policies: []Policy{&SyncPolicy{}},
// Current: current,
// Desired: desired,
// ManagedRecords: []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME},
// }
//
// changes := p.Calculate().Changes
// validateEntries(suite.T(), changes.Create, expectedCreate)
// validateEntries(suite.T(), changes.UpdateNew, expectedUpdateNew)
// validateEntries(suite.T(), changes.UpdateOld, expectedUpdateOld)
// validateEntries(suite.T(), changes.Delete, expectedDelete)
//}
func (suite *PlanTestSuite) TestSyncSecondRoundWithOwnerInherited() {
current := []*endpoint.Endpoint{suite.fooV1Cname}
@ -744,15 +740,11 @@ func TestNormalizeDNSName(t *testing.T) {
}
func TestShouldUpdateProviderSpecific(tt *testing.T) {
comparator := func(name, previous, current string) bool {
return previous == current
}
for _, test := range []struct {
name string
current *endpoint.Endpoint
desired *endpoint.Endpoint
propertyComparator func(name, previous, current string) bool
shouldUpdate bool
name string
current *endpoint.Endpoint
desired *endpoint.Endpoint
shouldUpdate bool
}{
{
name: "skip AWS target health",
@ -768,8 +760,7 @@ func TestShouldUpdateProviderSpecific(tt *testing.T) {
{Name: "aws/evaluate-target-health", Value: "true"},
},
},
propertyComparator: comparator,
shouldUpdate: false,
shouldUpdate: false,
},
{
name: "custom property unchanged",
@ -783,8 +774,7 @@ func TestShouldUpdateProviderSpecific(tt *testing.T) {
{Name: "custom/property", Value: "true"},
},
},
propertyComparator: comparator,
shouldUpdate: false,
shouldUpdate: false,
},
{
name: "custom property value changed",
@ -798,54 +788,10 @@ func TestShouldUpdateProviderSpecific(tt *testing.T) {
{Name: "custom/property", Value: "false"},
},
},
propertyComparator: comparator,
shouldUpdate: true,
},
{
name: "custom property key changed",
current: &endpoint.Endpoint{
ProviderSpecific: []endpoint.ProviderSpecificProperty{
{Name: "custom/property", Value: "true"},
},
},
desired: &endpoint.Endpoint{
ProviderSpecific: []endpoint.ProviderSpecificProperty{
{Name: "new/property", Value: "true"},
},
},
propertyComparator: comparator,
shouldUpdate: true,
},
{
name: "desired has same key and value as current but not comparator is set",
current: &endpoint.Endpoint{
ProviderSpecific: []endpoint.ProviderSpecificProperty{
{Name: "custom/property", Value: "true"},
},
},
desired: &endpoint.Endpoint{
ProviderSpecific: []endpoint.ProviderSpecificProperty{
{Name: "custom/property", Value: "true"},
},
},
shouldUpdate: false,
},
{
name: "desired has same key and different value as current but not comparator is set",
current: &endpoint.Endpoint{
ProviderSpecific: []endpoint.ProviderSpecificProperty{
{Name: "custom/property", Value: "true"},
},
},
desired: &endpoint.Endpoint{
ProviderSpecific: []endpoint.ProviderSpecificProperty{
{Name: "custom/property", Value: "false"},
},
},
shouldUpdate: true,
},
{
name: "desired has different key from current but not comparator is set",
name: "custom property key changed",
current: &endpoint.Endpoint{
ProviderSpecific: []endpoint.ProviderSpecificProperty{
{Name: "custom/property", Value: "true"},
@ -861,10 +807,9 @@ func TestShouldUpdateProviderSpecific(tt *testing.T) {
} {
tt.Run(test.name, func(t *testing.T) {
plan := &Plan{
Current: []*endpoint.Endpoint{test.current},
Desired: []*endpoint.Endpoint{test.desired},
PropertyComparator: test.propertyComparator,
ManagedRecords: []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME},
Current: []*endpoint.Endpoint{test.current},
Desired: []*endpoint.Endpoint{test.desired},
ManagedRecords: []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME},
}
b := plan.shouldUpdateProviderSpecific(test.desired, test.current)
assert.Equal(t, test.shouldUpdate, b)

7
provider/cloudflare/cloudflare_test.go
View File

@ -1150,10 +1150,9 @@ func TestProviderPropertiesIdempotency(t *testing.T) {
desired = provider.AdjustEndpoints(desired)
plan := plan.Plan{
Current: current,
Desired: desired,
PropertyComparator: provider.PropertyValuesEqual,
ManagedRecords: []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME},
Current: current,
Desired: desired,
ManagedRecords: []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME},
}
plan = *plan.Calculate()

8
provider/designate/designate.go
View File

@ -340,14 +340,6 @@ func (p designateProvider) Records(ctx context.Context) ([]*endpoint.Endpoint, e
return result, nil
}
func (p designateProvider) PropertyValuesEqual(name, previous, current string) bool {
if name == designateRecordSetID || name == designateZoneID || name == designateOriginalRecords {
return true
}
return previous == current
}
// temporary structure to hold recordset parameters so that we could aggregate endpoints into recordsets
type recordSet struct {
dnsName string

4
provider/plural/plural.go
View File

@ -83,10 +83,6 @@ func (p *PluralProvider) Records(_ context.Context) (endpoints []*endpoint.Endpo
return
}
func (p *PluralProvider) PropertyValuesEqual(name, previous, current string) bool {
return p.BaseProvider.PropertyValuesEqual(name, previous, current)
}
func (p *PluralProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint {
return endpoints
}

5
provider/provider.go
View File

@ -29,7 +29,6 @@ import (
type Provider interface {
Records(ctx context.Context) ([]*endpoint.Endpoint, error)
ApplyChanges(ctx context.Context, changes *plan.Changes) error
PropertyValuesEqual(name string, previous string, current string) bool
// AdjustEndpoints canonicalizes a set of candidate endpoints.
// It is called with a set of candidate endpoints obtained from the various sources.
// It returns a set modified as required by the provider. The provider is responsible for
@ -47,10 +46,6 @@ func (b BaseProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoin
return endpoints
}
func (b BaseProvider) PropertyValuesEqual(name, previous, current string) bool {
return previous == current
}
func (b BaseProvider) GetDomainFilter() endpoint.DomainFilter {
return endpoint.DomainFilter{}
}

9
provider/provider_test.go
View File

@ -54,12 +54,3 @@ func TestDifference(t *testing.T) {
assert.Equal(t, remove, []string{"foo"})
assert.Equal(t, leave, []string{"bar"})
}
func TestBaseProviderPropertyEquality(t *testing.T) {
p := BaseProvider{}
assert.True(t, p.PropertyValuesEqual("some.property", "", ""), "Both properties not present")
assert.False(t, p.PropertyValuesEqual("some.property", "", "Foo"), "First property missing")
assert.False(t, p.PropertyValuesEqual("some.property", "Foo", ""), "Second property missing")
assert.True(t, p.PropertyValuesEqual("some.property", "Foo", "Foo"), "Properties the same")
assert.False(t, p.PropertyValuesEqual("some.property", "Foo", "Bar"), "Attributes differ")
}

4
registry/aws_sd_registry.go
View File

@ -95,10 +95,6 @@ func (sdr *AWSSDRegistry) updateLabels(endpoints []*endpoint.Endpoint) {
}
}
func (sdr *AWSSDRegistry) PropertyValuesEqual(name string, previous string, current string) bool {
return sdr.provider.PropertyValuesEqual(name, previous, current)
}
// AdjustEndpoints modifies the endpoints as needed by the specific provider
func (sdr *AWSSDRegistry) AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint {
return sdr.provider.AdjustEndpoints(endpoints)

5
registry/dynamodb.go
View File

@ -248,11 +248,6 @@ func (im *DynamoDBRegistry) ApplyChanges(ctx context.Context, changes *plan.Chan
})
}
// PropertyValuesEqual compares two attribute values for equality.
func (im *DynamoDBRegistry) PropertyValuesEqual(name string, previous string, current string) bool {
return im.provider.PropertyValuesEqual(name, previous, current)
}
// AdjustEndpoints modifies the endpoints as needed by the specific provider.
func (im *DynamoDBRegistry) AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint {
return im.provider.AdjustEndpoints(endpoints)

5
registry/noop.go
View File

@ -50,11 +50,6 @@ func (im *NoopRegistry) ApplyChanges(ctx context.Context, changes *plan.Changes)
return im.provider.ApplyChanges(ctx, changes)
}
// PropertyValuesEqual compares two property values for equality
func (im *NoopRegistry) PropertyValuesEqual(attribute string, previous string, current string) bool {
return im.provider.PropertyValuesEqual(attribute, previous, current)
}
// AdjustEndpoints modifies the endpoints as needed by the specific provider
func (im *NoopRegistry) AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint {
return im.provider.AdjustEndpoints(endpoints)

1
registry/registry.go
View File

@ -32,7 +32,6 @@ import (
type Registry interface {
Records(ctx context.Context) ([]*endpoint.Endpoint, error)
ApplyChanges(ctx context.Context, changes *plan.Changes) error
PropertyValuesEqual(attribute string, previous string, current string) bool
AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint
GetDomainFilter() endpoint.DomainFilter
}

5
registry/txt.go
View File

@ -284,11 +284,6 @@ func (im *TXTRegistry) ApplyChanges(ctx context.Context, changes *plan.Changes)
return im.provider.ApplyChanges(ctx, filteredChanges)
}
// PropertyValuesEqual compares two attribute values for equality
func (im *TXTRegistry) PropertyValuesEqual(name string, previous string, current string) bool {
return im.provider.PropertyValuesEqual(name, previous, current)
}
// AdjustEndpoints modifies the endpoints as needed by the specific provider
func (im *TXTRegistry) AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint {
return im.provider.AdjustEndpoints(endpoints)