From 39436eab344802a57e864ffef4d8ea35244ea40e Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Sat, 6 Sep 2025 19:35:16 +0900 Subject: [PATCH 01/15] refactor: decouple CRD types from internal endpoint package --- apis/v1alpha1/dnsendpoint.go | 33 ++++++++++-- apis/v1alpha1/zz_generated.deepcopy.go | 71 ++++++++++++++++++++++++-- source/crd.go | 24 ++++++++- source/crd_test.go | 44 ++++++++-------- 4 files changed, 145 insertions(+), 27 deletions(-) diff --git a/apis/v1alpha1/dnsendpoint.go b/apis/v1alpha1/dnsendpoint.go index 522bd1beeb..786eff29c0 100644 --- a/apis/v1alpha1/dnsendpoint.go +++ b/apis/v1alpha1/dnsendpoint.go @@ -18,8 +18,6 @@ package v1alpha1 import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "sigs.k8s.io/external-dns/endpoint" ) // +genclient @@ -51,7 +49,36 @@ type DNSEndpointList struct { // DNSEndpointSpec defines the desired state of DNSEndpoint type DNSEndpointSpec struct { - Endpoints []*endpoint.Endpoint `json:"endpoints,omitempty"` + Endpoints []*Endpoint `json:"endpoints"` +} + +type ProviderSpecificProperty struct { + Name string `json:"name,omitempty"` + Value string `json:"value,omitempty"` +} + +// ProviderSpecific holds configuration which is specific to individual DNS providers +type ProviderSpecific []ProviderSpecificProperty + +// Endpoint is a high-level way of a connection between a service and an IP +// +kubebuilder:object:generate=true +type Endpoint struct { + // The hostname of the DNS record + DNSName string `json:"dnsName,omitempty"` + // The targets the DNS record points to + Targets []string `json:"targets,omitempty"` + // RecordType type of record, e.g. CNAME, A, AAAA, SRV, TXT etc + RecordType string `json:"recordType,omitempty"` + // Identifier to distinguish multiple records with the same name and type (e.g. Route53 records with routing policies other than 'simple') + SetIdentifier string `json:"setIdentifier,omitempty"` + // TTL for the record + RecordTTL int64 `json:"recordTTL,omitempty"` + // Labels stores labels defined for the Endpoint + // +optional + Labels map[string]string `json:"labels,omitempty"` + // ProviderSpecific stores provider specific config + // +optional + ProviderSpecific ProviderSpecific `json:"providerSpecific,omitempty"` } // DNSEndpointStatus defines the observed state of DNSEndpoint diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go index e2f5bf1b85..f9fa4852dc 100644 --- a/apis/v1alpha1/zz_generated.deepcopy.go +++ b/apis/v1alpha1/zz_generated.deepcopy.go @@ -6,7 +6,6 @@ package v1alpha1 import ( runtime "k8s.io/apimachinery/pkg/runtime" - "sigs.k8s.io/external-dns/endpoint" ) // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. @@ -73,11 +72,11 @@ func (in *DNSEndpointSpec) DeepCopyInto(out *DNSEndpointSpec) { *out = *in if in.Endpoints != nil { in, out := &in.Endpoints, &out.Endpoints - *out = make([]*endpoint.Endpoint, len(*in)) + *out = make([]*Endpoint, len(*in)) for i := range *in { if (*in)[i] != nil { in, out := &(*in)[i], &(*out)[i] - *out = new(endpoint.Endpoint) + *out = new(Endpoint) (*in).DeepCopyInto(*out) } } @@ -108,3 +107,69 @@ func (in *DNSEndpointStatus) DeepCopy() *DNSEndpointStatus { in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Endpoint) DeepCopyInto(out *Endpoint) { + *out = *in + if in.Targets != nil { + in, out := &in.Targets, &out.Targets + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.Labels != nil { + in, out := &in.Labels, &out.Labels + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } + if in.ProviderSpecific != nil { + in, out := &in.ProviderSpecific, &out.ProviderSpecific + *out = make(ProviderSpecific, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Endpoint. +func (in *Endpoint) DeepCopy() *Endpoint { + if in == nil { + return nil + } + out := new(Endpoint) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in ProviderSpecific) DeepCopyInto(out *ProviderSpecific) { + { + in := &in + *out = make(ProviderSpecific, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ProviderSpecific. +func (in ProviderSpecific) DeepCopy() ProviderSpecific { + if in == nil { + return nil + } + out := new(ProviderSpecific) + in.DeepCopyInto(out) + return *out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ProviderSpecificProperty) DeepCopyInto(out *ProviderSpecificProperty) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ProviderSpecificProperty. +func (in *ProviderSpecificProperty) DeepCopy() *ProviderSpecificProperty { + if in == nil { + return nil + } + out := new(ProviderSpecificProperty) + in.DeepCopyInto(out) + return out +} diff --git a/source/crd.go b/source/crd.go index 2911f1f6d7..f259f074ae 100644 --- a/source/crd.go +++ b/source/crd.go @@ -160,6 +160,27 @@ func (cs *crdSource) AddEventHandler(_ context.Context, handler func()) { } } +func fromCRDEndpoint(crdEp *apiv1alpha1.Endpoint) *endpoint.Endpoint { + //TODO: + p := endpoint.ProviderSpecific{} + for _, ps := range crdEp.ProviderSpecific { + p = append(p, endpoint.ProviderSpecificProperty{ + Name: ps.Name, + Value: ps.Value, + }) + } + ep := &endpoint.Endpoint{ + DNSName: crdEp.DNSName, + Targets: crdEp.Targets, + RecordType: crdEp.RecordType, + SetIdentifier: crdEp.SetIdentifier, + RecordTTL: endpoint.TTL(crdEp.RecordTTL), + Labels: crdEp.Labels, + ProviderSpecific: p, + } + return ep +} + // Endpoints returns endpoint objects. func (cs *crdSource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, error) { endpoints := []*endpoint.Endpoint{} @@ -181,7 +202,8 @@ func (cs *crdSource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, error for _, dnsEndpoint := range result.Items { var crdEndpoints []*endpoint.Endpoint - for _, ep := range dnsEndpoint.Spec.Endpoints { + for _, crdEp := range dnsEndpoint.Spec.Endpoints { + ep := fromCRDEndpoint(crdEp) if (ep.RecordType == endpoint.RecordTypeCNAME || ep.RecordType == endpoint.RecordTypeA || ep.RecordType == endpoint.RecordTypeAAAA) && len(ep.Targets) < 1 { log.Debugf("Endpoint %s with DNSName %s has an empty list of targets, allowing it to pass through for default-targets processing", dnsEndpoint.Name, ep.DNSName) } diff --git a/source/crd_test.go b/source/crd_test.go index ff6cec416d..b189c66577 100644 --- a/source/crd_test.go +++ b/source/crd_test.go @@ -62,7 +62,7 @@ func objBody(codec runtime.Encoder, obj runtime.Object) io.ReadCloser { return io.NopCloser(bytes.NewReader([]byte(runtime.EncodeOrDie(codec, obj)))) } -func fakeRESTClient(endpoints []*endpoint.Endpoint, apiVersion, kind, namespace, name string, annotations map[string]string, labels map[string]string, _ *testing.T) rest.Interface { +func fakeRESTClient(endpoints []*apiv1alpha1.Endpoint, apiVersion, kind, namespace, name string, annotations map[string]string, labels map[string]string, _ *testing.T) rest.Interface { groupVersion, _ := schema.ParseGroupVersion(apiVersion) scheme := runtime.NewScheme() _ = addKnownTypes(scheme, groupVersion) @@ -143,7 +143,7 @@ func testCRDSourceEndpoints(t *testing.T) { apiVersion string registeredKind string kind string - endpoints []*endpoint.Endpoint + endpoints []*apiv1alpha1.Endpoint expectEndpoints bool expectError bool annotationFilter string @@ -157,7 +157,7 @@ func testCRDSourceEndpoints(t *testing.T) { apiVersion: "blah.k8s.io/v1alpha1", registeredKind: "DNSEndpoint", kind: "DNSEndpoint", - endpoints: []*endpoint.Endpoint{ + endpoints: []*apiv1alpha1.Endpoint{ { DNSName: "abc.example.org", Targets: endpoint.Targets{"1.2.3.4"}, @@ -174,7 +174,7 @@ func testCRDSourceEndpoints(t *testing.T) { apiVersion: "test.k8s.io/v1alpha1", registeredKind: "DNSEndpoint", kind: "JustEndpoint", - endpoints: []*endpoint.Endpoint{ + endpoints: []*apiv1alpha1.Endpoint{ { DNSName: "abc.example.org", Targets: endpoint.Targets{"1.2.3.4"}, @@ -193,7 +193,7 @@ func testCRDSourceEndpoints(t *testing.T) { kind: "DNSEndpoint", namespace: "foo", registeredNamespace: "foo", - endpoints: []*endpoint.Endpoint{ + endpoints: []*apiv1alpha1.Endpoint{ { DNSName: "abc.example.org", Targets: endpoint.Targets{"1.2.3.4"}, @@ -212,7 +212,7 @@ func testCRDSourceEndpoints(t *testing.T) { kind: "DNSEndpoint", namespace: "foo", registeredNamespace: "bar", - endpoints: []*endpoint.Endpoint{ + endpoints: []*apiv1alpha1.Endpoint{ { DNSName: "abc.example.org", Targets: endpoint.Targets{"1.2.3.4"}, @@ -231,7 +231,7 @@ func testCRDSourceEndpoints(t *testing.T) { kind: "DNSEndpoint", namespace: "foo", registeredNamespace: "foo", - endpoints: []*endpoint.Endpoint{ + endpoints: []*apiv1alpha1.Endpoint{ { DNSName: "no-targets.example.org", Targets: endpoint.Targets{}, @@ -250,7 +250,7 @@ func testCRDSourceEndpoints(t *testing.T) { kind: "DNSEndpoint", namespace: "foo", registeredNamespace: "foo", - endpoints: []*endpoint.Endpoint{ + endpoints: []*apiv1alpha1.Endpoint{ { DNSName: "abc.example.org", Targets: endpoint.Targets{"1.2.3.4"}, @@ -269,7 +269,7 @@ func testCRDSourceEndpoints(t *testing.T) { kind: "DNSEndpoint", namespace: "foo", registeredNamespace: "foo", - endpoints: []*endpoint.Endpoint{ + endpoints: []*apiv1alpha1.Endpoint{ { DNSName: "abc.example.org", Targets: endpoint.Targets{"1.2.3.4"}, @@ -296,7 +296,7 @@ func testCRDSourceEndpoints(t *testing.T) { registeredNamespace: "foo", annotations: map[string]string{"test": "that"}, annotationFilter: "test=filter_something_else", - endpoints: []*endpoint.Endpoint{ + endpoints: []*apiv1alpha1.Endpoint{ { DNSName: "abc.example.org", Targets: endpoint.Targets{"1.2.3.4"}, @@ -317,7 +317,7 @@ func testCRDSourceEndpoints(t *testing.T) { registeredNamespace: "foo", annotations: map[string]string{"test": "that"}, annotationFilter: "test=that", - endpoints: []*endpoint.Endpoint{ + endpoints: []*apiv1alpha1.Endpoint{ { DNSName: "abc.example.org", Targets: endpoint.Targets{"1.2.3.4"}, @@ -338,7 +338,7 @@ func testCRDSourceEndpoints(t *testing.T) { registeredNamespace: "foo", labels: map[string]string{"test": "that"}, labelFilter: "test=filter_something_else", - endpoints: []*endpoint.Endpoint{ + endpoints: []*apiv1alpha1.Endpoint{ { DNSName: "abc.example.org", Targets: endpoint.Targets{"1.2.3.4"}, @@ -359,7 +359,7 @@ func testCRDSourceEndpoints(t *testing.T) { registeredNamespace: "foo", labels: map[string]string{"test": "that"}, labelFilter: "test=that", - endpoints: []*endpoint.Endpoint{ + endpoints: []*apiv1alpha1.Endpoint{ { DNSName: "abc.example.org", Targets: endpoint.Targets{"1.2.3.4"}, @@ -380,7 +380,7 @@ func testCRDSourceEndpoints(t *testing.T) { registeredNamespace: "foo", labels: map[string]string{"test": "that"}, labelFilter: "test=that", - endpoints: []*endpoint.Endpoint{ + endpoints: []*apiv1alpha1.Endpoint{ { DNSName: "abc.example.org", Targets: endpoint.Targets{"ns1.k8s.io", "ns2.k8s.io"}, @@ -401,7 +401,7 @@ func testCRDSourceEndpoints(t *testing.T) { registeredNamespace: "foo", labels: map[string]string{"test": "that"}, labelFilter: "test=that", - endpoints: []*endpoint.Endpoint{ + endpoints: []*apiv1alpha1.Endpoint{ { DNSName: "_svc._tcp.example.org", Targets: endpoint.Targets{"0 0 80 abc.example.org", "0 0 80 def.example.org"}, @@ -422,7 +422,7 @@ func testCRDSourceEndpoints(t *testing.T) { registeredNamespace: "foo", labels: map[string]string{"test": "that"}, labelFilter: "test=that", - endpoints: []*endpoint.Endpoint{ + endpoints: []*apiv1alpha1.Endpoint{ { DNSName: "example.org", Targets: endpoint.Targets{`100 10 "S" "SIP+D2U" "!^.*$!sip:customer-service@example.org!" _sip._udp.example.org.`, `102 10 "S" "SIP+D2T" "!^.*$!sip:customer-service@example.org!" _sip._tcp.example.org.`}, @@ -443,7 +443,7 @@ func testCRDSourceEndpoints(t *testing.T) { registeredNamespace: "foo", labels: map[string]string{"test": "that"}, labelFilter: "test=that", - endpoints: []*endpoint.Endpoint{ + endpoints: []*apiv1alpha1.Endpoint{ { DNSName: "example.org", Targets: endpoint.Targets{"foo.example.org."}, @@ -464,7 +464,7 @@ func testCRDSourceEndpoints(t *testing.T) { registeredNamespace: "foo", labels: map[string]string{"test": "that"}, labelFilter: "test=that", - endpoints: []*endpoint.Endpoint{ + endpoints: []*apiv1alpha1.Endpoint{ { DNSName: "example.org", Targets: endpoint.Targets{`100 10 "S" "SIP+D2U" "!^.*$!sip:customer-service@example.org!" _sip._udp.example.org`, `102 10 "S" "SIP+D2T" "!^.*$!sip:customer-service@example.org!" _sip._tcp.example.org`}, @@ -513,7 +513,11 @@ func testCRDSourceEndpoints(t *testing.T) { } // Validate received endpoints against expected endpoints. - validateEndpoints(t, receivedEndpoints, ti.endpoints) + endpoints := make([]*endpoint.Endpoint, 0, len(ti.endpoints)) + for _, ep := range ti.endpoints { + endpoints = append(endpoints, fromCRDEndpoint(ep)) + } + validateEndpoints(t, receivedEndpoints, endpoints) for _, e := range receivedEndpoints { // TODO: at the moment not all sources apply ResourceLabelKey @@ -754,7 +758,7 @@ func generateTestFixtureDNSEndpointsByType(namespace string, typeCounts map[stri Namespace: namespace, }, Spec: apiv1alpha1.DNSEndpointSpec{ - Endpoints: []*endpoint.Endpoint{ + Endpoints: []*apiv1alpha1.Endpoint{ { DNSName: strings.ToLower(fmt.Sprintf("%s-%d.example.com", rt, idx)), RecordType: rt, From 85203ba19f03b296eab04e8f77cd6f36f7a286c6 Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Sat, 6 Sep 2025 23:50:43 +0900 Subject: [PATCH 02/15] perf(endpoint): optimize ProviderSpecific to use map for O(1) access --- apis/v1alpha1/dnsendpoint.go | 1 + endpoint/endpoint.go | 82 ++++--- endpoint/endpoint_test.go | 202 +++++++++--------- endpoint/zz_generated.deepcopy.go | 37 ---- internal/testutils/endpoint.go | 20 +- internal/testutils/endpoint_test.go | 12 +- plan/plan.go | 18 +- plan/plan_test.go | 57 ++--- provider/aws/aws_test.go | 20 +- provider/cloudflare/cloudflare.go | 23 +- .../cloudflare/cloudflare_regional_test.go | 58 ++--- provider/cloudflare/cloudflare_test.go | 150 +++---------- provider/inmemory/inmemory.go | 7 +- provider/scaleway/scaleway_test.go | 60 ++---- provider/webhook/webhook_test.go | 10 +- registry/dynamodb_test.go | 10 +- registry/txt_test.go | 42 ++-- source/ambassador_host_test.go | 7 +- source/annotations/provider_specific.go | 47 +--- source/annotations/provider_specific_test.go | 58 ++--- source/crd.go | 8 +- source/endpoints_test.go | 12 +- source/gloo_proxy_test.go | 20 +- source/ingress_test.go | 6 +- source/wrappers/multisource_test.go | 22 +- source/wrappers/nat64source.go | 24 ++- 26 files changed, 376 insertions(+), 637 deletions(-) delete mode 100644 endpoint/zz_generated.deepcopy.go diff --git a/apis/v1alpha1/dnsendpoint.go b/apis/v1alpha1/dnsendpoint.go index 786eff29c0..c580ba5475 100644 --- a/apis/v1alpha1/dnsendpoint.go +++ b/apis/v1alpha1/dnsendpoint.go @@ -52,6 +52,7 @@ type DNSEndpointSpec struct { Endpoints []*Endpoint `json:"endpoints"` } +// ProviderSpecificProperty holds the name and value of a configuration which is specific to individual DNS providers type ProviderSpecificProperty struct { Name string `json:"name,omitempty"` Value string `json:"value,omitempty"` diff --git a/endpoint/endpoint.go b/endpoint/endpoint.go index b322369c77..60b7ac0bc2 100644 --- a/endpoint/endpoint.go +++ b/endpoint/endpoint.go @@ -206,14 +206,54 @@ func (t Targets) IsLess(o Targets) bool { return false } -// ProviderSpecificProperty holds the name and value of a configuration which is specific to individual DNS providers -type ProviderSpecificProperty struct { - Name string `json:"name,omitempty"` - Value string `json:"value,omitempty"` +// ProviderSpecific holds configuration which is specific to individual DNS providers +type ProviderSpecific map[string]string + +func (ps ProviderSpecific) Set(key, value string) { + ps[key] = value } -// ProviderSpecific holds configuration which is specific to individual DNS providers -type ProviderSpecific []ProviderSpecificProperty +func (ps ProviderSpecific) Get(key string) (string, bool) { + value, ok := ps[key] + return value, ok +} + +func (ps ProviderSpecific) Delete(key string) { + delete(ps, key) +} + +// String returns a stable, backwards-compatible string form. +// For compatibility with older branches where ProviderSpecific was +// []ProviderSpecificProperty, we render: +// - empty or nil as "[]" +// - non-empty as slice-like entries sorted by key, e.g. "[{k1 v1} {k2 v2}]" +// This preserves previous log expectations and keeps output deterministic. +func (ps ProviderSpecific) String() string { + if len(ps) == 0 { + return "[]" + } + // Collect and sort keys for stable output. + keys := make([]string, 0, len(ps)) + for k := range ps { + keys = append(keys, k) + } + sort.Strings(keys) + // Build entries like "{key value}" preserving stable order. + b := strings.Builder{} + b.WriteByte('[') + for i, k := range keys { + if i > 0 { + b.WriteByte(' ') + } + b.WriteByte('{') + b.WriteString(k) + b.WriteByte(' ') + b.WriteString(ps[k]) + b.WriteByte('}') + } + b.WriteByte(']') + return b.String() +} // EndpointKey is the type of a map key for separating endpoints or targets. type EndpointKey struct { @@ -224,7 +264,6 @@ type EndpointKey struct { } // Endpoint is a high-level way of a connection between a service and an IP -// +kubebuilder:object:generate=true type Endpoint struct { // The hostname of the DNS record DNSName string `json:"dnsName,omitempty"` @@ -293,37 +332,26 @@ func (e *Endpoint) WithProviderSpecific(key, value string) *Endpoint { // GetProviderSpecificProperty returns the value of a ProviderSpecificProperty if the property exists. func (e *Endpoint) GetProviderSpecificProperty(key string) (string, bool) { - for _, providerSpecific := range e.ProviderSpecific { - if providerSpecific.Name == key { - return providerSpecific.Value, true - } + if e.ProviderSpecific == nil { + return "", false } - return "", false + return e.ProviderSpecific.Get(key) } // SetProviderSpecificProperty sets the value of a ProviderSpecificProperty. func (e *Endpoint) SetProviderSpecificProperty(key string, value string) { - for i, providerSpecific := range e.ProviderSpecific { - if providerSpecific.Name == key { - e.ProviderSpecific[i] = ProviderSpecificProperty{ - Name: key, - Value: value, - } - return - } + if e.ProviderSpecific == nil { + e.ProviderSpecific = make(ProviderSpecific) } - - e.ProviderSpecific = append(e.ProviderSpecific, ProviderSpecificProperty{Name: key, Value: value}) + e.ProviderSpecific.Set(key, value) } // DeleteProviderSpecificProperty deletes any ProviderSpecificProperty of the specified name. func (e *Endpoint) DeleteProviderSpecificProperty(key string) { - for i, providerSpecific := range e.ProviderSpecific { - if providerSpecific.Name == key { - e.ProviderSpecific = append(e.ProviderSpecific[:i], e.ProviderSpecific[i+1:]...) - return - } + if e.ProviderSpecific == nil { + return } + e.ProviderSpecific.Delete(key) } // WithLabel adds or updates a label for the Endpoint. diff --git a/endpoint/endpoint_test.go b/endpoint/endpoint_test.go index 73017319a4..ebe41d55ae 100644 --- a/endpoint/endpoint_test.go +++ b/endpoint/endpoint_test.go @@ -191,11 +191,8 @@ func TestIsLess(t *testing.T) { func TestGetProviderSpecificProperty(t *testing.T) { e := &Endpoint{ - ProviderSpecific: []ProviderSpecificProperty{ - { - Name: "name", - Value: "value", - }, + ProviderSpecific: ProviderSpecific{ + "name": "value", }, } @@ -220,18 +217,15 @@ func TestSetProviderSpecficProperty(t *testing.T) { key string value string expectedIdentifier string - expected []ProviderSpecificProperty + expected ProviderSpecific }{ { name: "endpoint is empty", endpoint: Endpoint{}, key: "key1", value: "value1", - expected: []ProviderSpecificProperty{ - { - Name: "key1", - Value: "value1", - }, + expected: ProviderSpecific{ + "key1": "value1", }, }, { @@ -244,26 +238,17 @@ func TestSetProviderSpecficProperty(t *testing.T) { Targets: Targets{ "example.org", "example.com", "1.2.4.5", }, - ProviderSpecific: []ProviderSpecificProperty{ - { - Name: "name1", - Value: "value1", - }, + ProviderSpecific: ProviderSpecific{ + "name1": "value1", }, }, expectedIdentifier: "newIdentifier", key: "name2", value: "value2", - expected: []ProviderSpecificProperty{ - { - Name: "name1", - Value: "value1", - }, - { - Name: "name2", - Value: "value2", - }, + expected: ProviderSpecific{ + "name1": "value1", + "name2": "value2", }, }, { @@ -276,37 +261,19 @@ func TestSetProviderSpecficProperty(t *testing.T) { Targets: Targets{ "example.org", "example.com", "1.2.4.5", }, - ProviderSpecific: []ProviderSpecificProperty{ - { - Name: "name1", - Value: "value1", - }, - { - Name: "name2", - Value: "value2", - }, - { - Name: "name3", - Value: "value3", - }, + ProviderSpecific: ProviderSpecific{ + "name1": "value1", + "name2": "value2", + "name3": "value3", }, }, key: "name2", value: "value2", expectedIdentifier: "newIdentifier", - expected: []ProviderSpecificProperty{ - { - Name: "name1", - Value: "value1", - }, - { - Name: "name2", - Value: "value2", - }, - { - Name: "name3", - Value: "value3", - }, + expected: ProviderSpecific{ + "name1": "value1", + "name2": "value2", + "name3": "value3", }, }, { @@ -319,21 +286,15 @@ func TestSetProviderSpecficProperty(t *testing.T) { Targets: Targets{ "example.org", "example.com", "1.2.4.5", }, - ProviderSpecific: []ProviderSpecificProperty{ - { - Name: "name1", - Value: "value1", - }, + ProviderSpecific: ProviderSpecific{ + "name1": "value1", }, }, key: "name1", value: "value2", expectedIdentifier: "identifier", - expected: []ProviderSpecificProperty{ - { - Name: "name1", - Value: "value2", - }, + expected: ProviderSpecific{ + "name1": "value2", }, }, } @@ -345,7 +306,7 @@ func TestSetProviderSpecficProperty(t *testing.T) { identifier := c.endpoint.WithSetIdentifier(c.endpoint.SetIdentifier) assert.Equal(t, c.expectedIdentifier, identifier.SetIdentifier) assert.Equal(t, expectedString, c.endpoint.String()) - if !reflect.DeepEqual([]ProviderSpecificProperty(c.endpoint.ProviderSpecific), c.expected) { + if !reflect.DeepEqual(c.endpoint.ProviderSpecific, c.expected) { t.Errorf("unexpected ProviderSpecific:\nGot: %#v\nExpected: %#v", c.endpoint.ProviderSpecific, c.expected) } }) @@ -357,75 +318,51 @@ func TestDeleteProviderSpecificProperty(t *testing.T) { name string endpoint Endpoint key string - expected []ProviderSpecificProperty + expected ProviderSpecific }{ { name: "name and key are not matching", endpoint: Endpoint{ - ProviderSpecific: []ProviderSpecificProperty{ - { - Name: "name1", - Value: "value1", - }, + ProviderSpecific: ProviderSpecific{ + "name1": "value1", }, }, key: "name2", - expected: []ProviderSpecificProperty{ - { - Name: "name1", - Value: "value1", - }, + expected: ProviderSpecific{ + "name1": "value1", }, }, { name: "some keys are matching and some keys are not matching", endpoint: Endpoint{ - ProviderSpecific: []ProviderSpecificProperty{ - { - Name: "name1", - Value: "value1", - }, - { - Name: "name2", - Value: "value2", - }, - { - Name: "name3", - Value: "value3", - }, + ProviderSpecific: ProviderSpecific{ + "name1": "value1", + "name2": "value2", + "name3": "value3", }, }, key: "name2", - expected: []ProviderSpecificProperty{ - { - Name: "name1", - Value: "value1", - }, - { - Name: "name3", - Value: "value3", - }, + expected: ProviderSpecific{ + "name1": "value1", + "name3": "value3", }, }, { name: "name and key are matching", endpoint: Endpoint{ - ProviderSpecific: []ProviderSpecificProperty{ - { - Name: "name1", - Value: "value1", - }, + ProviderSpecific: ProviderSpecific{ + "name1": "value1", }, }, key: "name1", - expected: []ProviderSpecificProperty{}, + expected: ProviderSpecific{}, }, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { c.endpoint.DeleteProviderSpecificProperty(c.key) - if !reflect.DeepEqual([]ProviderSpecificProperty(c.endpoint.ProviderSpecific), c.expected) { + if !reflect.DeepEqual(c.endpoint.ProviderSpecific, c.expected) { t.Errorf("unexpected ProviderSpecific:\nGot: %#v\nExpected: %#v", c.endpoint.ProviderSpecific, c.expected) } }) @@ -1036,3 +973,64 @@ func TestEndpoint_WithMinTTL(t *testing.T) { }) } } + +func TestProviderSpecificString(t *testing.T) { + tests := []struct { + name string + ps ProviderSpecific + want string + }{ + {name: "nil", ps: nil, want: "[]"}, + {name: "empty", ps: ProviderSpecific{}, want: "[]"}, + {name: "single", ps: ProviderSpecific{"k": "v"}, want: "[{k v}]"}, + {name: "multi_sorted", ps: ProviderSpecific{"b": "2", "a": "1"}, want: "[{a 1} {b 2}]"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.ps.String(); got != tt.want { + t.Fatalf("ProviderSpecific.String() = %q, want %q", got, tt.want) + } + }) + } +} + +func TestEndpointString(t *testing.T) { + tests := []struct { + name string + ep *Endpoint + want string + }{ + { + name: "no set id, nil ps", + ep: &Endpoint{DNSName: "x.example.com", RecordType: RecordTypeA, Targets: Targets{"1.2.3.4"}}, + want: "x.example.com 0 IN A 1.2.3.4 []", + }, + { + name: "no set id, empty ps", + ep: &Endpoint{DNSName: "x.example.com", RecordType: RecordTypeA, Targets: Targets{"1.2.3.4"}, ProviderSpecific: ProviderSpecific{}}, + want: "x.example.com 0 IN A 1.2.3.4 []", + }, + { + name: "with set id and ps", + ep: &Endpoint{DNSName: "x.example.com", RecordType: RecordTypeA, SetIdentifier: "id", Targets: Targets{"1.2.3.4"}, ProviderSpecific: ProviderSpecific{"k": "v"}}, + want: "x.example.com 0 IN A id 1.2.3.4 [{k v}]", + }, + { + name: "multiple targets, nil ps", + ep: &Endpoint{DNSName: "x.example.com", RecordType: RecordTypeA, Targets: Targets{"1.2.3.4", "2.3.4.5"}}, + want: "x.example.com 0 IN A 1.2.3.4;2.3.4.5 []", + }, + { + name: "multiple targets, with set id and ps", + ep: &Endpoint{DNSName: "x.example.com", RecordType: RecordTypeA, SetIdentifier: "id", Targets: Targets{"1.2.3.4", "2.3.4.5"}, ProviderSpecific: ProviderSpecific{"a": "1", "b": "2"}}, + want: "x.example.com 0 IN A id 1.2.3.4;2.3.4.5 [{a 1} {b 2}]", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.ep.String(); got != tt.want { + t.Fatalf("Endpoint.String() = %q, want %q", got, tt.want) + } + }) + } +} diff --git a/endpoint/zz_generated.deepcopy.go b/endpoint/zz_generated.deepcopy.go deleted file mode 100644 index e86498d37d..0000000000 --- a/endpoint/zz_generated.deepcopy.go +++ /dev/null @@ -1,37 +0,0 @@ -//go:build !ignore_autogenerated - -// Code generated by controller-gen. DO NOT EDIT. - -package endpoint - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *Endpoint) DeepCopyInto(out *Endpoint) { - *out = *in - if in.Targets != nil { - in, out := &in.Targets, &out.Targets - *out = make(Targets, len(*in)) - copy(*out, *in) - } - if in.Labels != nil { - in, out := &in.Labels, &out.Labels - *out = make(Labels, len(*in)) - for key, val := range *in { - (*out)[key] = val - } - } - if in.ProviderSpecific != nil { - in, out := &in.ProviderSpecific, &out.ProviderSpecific - *out = make(ProviderSpecific, len(*in)) - copy(*out, *in) - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Endpoint. -func (in *Endpoint) DeepCopy() *Endpoint { - if in == nil { - return nil - } - out := new(Endpoint) - in.DeepCopyInto(out) - return out -} diff --git a/internal/testutils/endpoint.go b/internal/testutils/endpoint.go index 0a33d3a218..d6b3b99b62 100644 --- a/internal/testutils/endpoint.go +++ b/internal/testutils/endpoint.go @@ -29,12 +29,6 @@ import ( /** test utility functions for endpoints verifications */ -type byNames endpoint.ProviderSpecific - -func (p byNames) Len() int { return len(p) } -func (p byNames) Swap(i, j int) { p[i], p[j] = p[j], p[i] } -func (p byNames) Less(i, j int) bool { return p[i].Name < p[j].Name } - type byAllFields []*endpoint.Endpoint func (b byAllFields) Len() int { return len(b) } @@ -47,11 +41,7 @@ func (b byAllFields) Less(i, j int) bool { // This rather bad, we need a more complex comparison for Targets, which considers all elements if b[i].Targets.Same(b[j].Targets) { if b[i].RecordType == (b[j].RecordType) { - sa := b[i].ProviderSpecific - sb := b[j].ProviderSpecific - sort.Sort(byNames(sa)) - sort.Sort(byNames(sb)) - return reflect.DeepEqual(sa, sb) + return SameProviderSpecific(b[i].ProviderSpecific, b[j].ProviderSpecific) } return b[i].RecordType <= b[j].RecordType } @@ -118,13 +108,9 @@ func SamePlanChanges(a, b map[string][]*endpoint.Endpoint) bool { SameEndpoints(a["UpdateOld"], b["UpdateOld"]) && SameEndpoints(a["UpdateNew"], b["UpdateNew"]) } -// SameProviderSpecific verifies that two maps contain the same string/string key/value pairs +// SameProviderSpecific verifies that two maps contain the same string/[]string key/value pairs func SameProviderSpecific(a, b endpoint.ProviderSpecific) bool { - sa := a - sb := b - sort.Sort(byNames(sa)) - sort.Sort(byNames(sb)) - return reflect.DeepEqual(sa, sb) + return reflect.DeepEqual(a, b) } // NewTargetsFromAddr convert an array of netip.Addr to Targets (array of string) diff --git a/internal/testutils/endpoint_test.go b/internal/testutils/endpoint_test.go index 8daff40423..3b64a3472b 100644 --- a/internal/testutils/endpoint_test.go +++ b/internal/testutils/endpoint_test.go @@ -66,7 +66,7 @@ func TestExampleSameEndpoints(t *testing.T) { DNSName: "example.org", Targets: endpoint.Targets{"load-balancer.org"}, ProviderSpecific: endpoint.ProviderSpecific{ - endpoint.ProviderSpecificProperty{Name: "foo", Value: "bar"}, + "foo": "bar", }, }, } @@ -97,7 +97,7 @@ func makeEndpoint(DNSName string) *endpoint.Endpoint { endpoint.OwnedRecordLabelKey: "owned", }, ProviderSpecific: endpoint.ProviderSpecific{ - {Name: "key", Value: "val"}, + "key": "val", }, } } @@ -129,7 +129,7 @@ func TestSameEndpoint(t *testing.T) { endpoint.OwnedRecordLabelKey: "owned-true", }, ProviderSpecific: endpoint.ProviderSpecific{ - {Name: "key1", Value: "val1"}, + "key1": "val1", }, }, b: &endpoint.Endpoint{ @@ -144,7 +144,7 @@ func TestSameEndpoint(t *testing.T) { endpoint.OwnedRecordLabelKey: "owned-true", }, ProviderSpecific: endpoint.ProviderSpecific{ - {Name: "key1", Value: "val1"}, + "key1": "val1", }, }, isSameEndpoint: true, @@ -194,13 +194,13 @@ func TestSameEndpoint(t *testing.T) { a: &endpoint.Endpoint{ DNSName: "example.org", ProviderSpecific: endpoint.ProviderSpecific{ - {Name: "key1", Value: "val1"}, + "key1": "val1", }, }, b: &endpoint.Endpoint{ DNSName: "example.org", ProviderSpecific: endpoint.ProviderSpecific{ - {Name: "key1", Value: "val2"}, + "key1": "val2", }, }, isSameEndpoint: false, diff --git a/plan/plan.go b/plan/plan.go index 299623776b..9f4bfd245f 100644 --- a/plan/plan.go +++ b/plan/plan.go @@ -298,23 +298,17 @@ func shouldUpdateTTL(desired, current *endpoint.Endpoint) bool { } func (p *Plan) shouldUpdateProviderSpecific(desired, current *endpoint.Endpoint) bool { - desiredProperties := map[string]endpoint.ProviderSpecificProperty{} - - for _, d := range desired.ProviderSpecific { - desiredProperties[d.Name] = d + if len(desired.ProviderSpecific) != len(current.ProviderSpecific) { + return true } - for _, c := range current.ProviderSpecific { - if d, ok := desiredProperties[c.Name]; ok { - if c.Value != d.Value { - return true - } - delete(desiredProperties, c.Name) - } else { + + for key, desiredValue := range desired.ProviderSpecific { + if currentValue, exists := current.ProviderSpecific[key]; !exists || currentValue != desiredValue { return true } } - return len(desiredProperties) > 0 + return false } // filterRecordsForPlan removes records that are not relevant to the planner. diff --git a/plan/plan_test.go b/plan/plan_test.go index 10ce5a992b..f788bbaa59 100644 --- a/plan/plan_test.go +++ b/plan/plan_test.go @@ -159,14 +159,8 @@ func (suite *PlanTestSuite) SetupTest() { endpoint.ResourceLabelKey: "ingress/default/bar-127", }, ProviderSpecific: endpoint.ProviderSpecific{ - endpoint.ProviderSpecificProperty{ - Name: "alias", - Value: "false", - }, - endpoint.ProviderSpecificProperty{ - Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied", - Value: "true", - }, + "alias": "false", + "external-dns.alpha.kubernetes.io/cloudflare-proxied": "true", }, } suite.bar127AWithProviderSpecificFalse = &endpoint.Endpoint{ @@ -177,14 +171,8 @@ func (suite *PlanTestSuite) SetupTest() { endpoint.ResourceLabelKey: "ingress/default/bar-127", }, ProviderSpecific: endpoint.ProviderSpecific{ - endpoint.ProviderSpecificProperty{ - Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied", - Value: "false", - }, - endpoint.ProviderSpecificProperty{ - Name: "alias", - Value: "false", - }, + "external-dns.alpha.kubernetes.io/cloudflare-proxied": "false", + "alias": "false", }, } suite.bar127AWithProviderSpecificUnset = &endpoint.Endpoint{ @@ -195,10 +183,7 @@ func (suite *PlanTestSuite) SetupTest() { endpoint.ResourceLabelKey: "ingress/default/bar-127", }, ProviderSpecific: endpoint.ProviderSpecific{ - endpoint.ProviderSpecificProperty{ - Name: "alias", - Value: "false", - }, + "alias": "false", }, } suite.bar192A = &endpoint.Endpoint{ @@ -1137,14 +1122,14 @@ func TestShouldUpdateProviderSpecific(tt *testing.T) { name: "skip AWS target health", current: &endpoint.Endpoint{ DNSName: "foo.com", - ProviderSpecific: []endpoint.ProviderSpecificProperty{ - {Name: "aws/evaluate-target-health", Value: "true"}, + ProviderSpecific: endpoint.ProviderSpecific{ + "aws/evaluate-target-health": "true", }, }, desired: &endpoint.Endpoint{ DNSName: "bar.com", - ProviderSpecific: []endpoint.ProviderSpecificProperty{ - {Name: "aws/evaluate-target-health", Value: "true"}, + ProviderSpecific: endpoint.ProviderSpecific{ + "aws/evaluate-target-health": "true", }, }, shouldUpdate: false, @@ -1152,13 +1137,13 @@ func TestShouldUpdateProviderSpecific(tt *testing.T) { { name: "custom property unchanged", current: &endpoint.Endpoint{ - ProviderSpecific: []endpoint.ProviderSpecificProperty{ - {Name: "custom/property", Value: "true"}, + ProviderSpecific: endpoint.ProviderSpecific{ + "custom/property": "true", }, }, desired: &endpoint.Endpoint{ - ProviderSpecific: []endpoint.ProviderSpecificProperty{ - {Name: "custom/property", Value: "true"}, + ProviderSpecific: endpoint.ProviderSpecific{ + "custom/property": "true", }, }, shouldUpdate: false, @@ -1166,13 +1151,13 @@ func TestShouldUpdateProviderSpecific(tt *testing.T) { { name: "custom property value changed", current: &endpoint.Endpoint{ - ProviderSpecific: []endpoint.ProviderSpecificProperty{ - {Name: "custom/property", Value: "true"}, + ProviderSpecific: endpoint.ProviderSpecific{ + "custom/property": "true", }, }, desired: &endpoint.Endpoint{ - ProviderSpecific: []endpoint.ProviderSpecificProperty{ - {Name: "custom/property", Value: "false"}, + ProviderSpecific: endpoint.ProviderSpecific{ + "custom/property": "false", }, }, shouldUpdate: true, @@ -1180,13 +1165,13 @@ func TestShouldUpdateProviderSpecific(tt *testing.T) { { name: "custom property key changed", current: &endpoint.Endpoint{ - ProviderSpecific: []endpoint.ProviderSpecificProperty{ - {Name: "custom/property", Value: "true"}, + ProviderSpecific: endpoint.ProviderSpecific{ + "custom/property": "true", }, }, desired: &endpoint.Endpoint{ - ProviderSpecific: []endpoint.ProviderSpecificProperty{ - {Name: "new/property", Value: "true"}, + ProviderSpecific: endpoint.ProviderSpecific{ + "new/property": "true", }, }, shouldUpdate: true, diff --git a/provider/aws/aws_test.go b/provider/aws/aws_test.go index a269c3ede8..7d5d3f220b 100644 --- a/provider/aws/aws_test.go +++ b/provider/aws/aws_test.go @@ -2084,14 +2084,8 @@ func TestAWSCreateRecordsWithALIAS(t *testing.T) { Targets: endpoint.Targets{"foo.eu-central-1.elb.amazonaws.com"}, RecordType: endpoint.RecordTypeA, ProviderSpecific: endpoint.ProviderSpecific{ - endpoint.ProviderSpecificProperty{ - Name: providerSpecificAlias, - Value: "true", - }, - endpoint.ProviderSpecificProperty{ - Name: providerSpecificEvaluateTargetHealth, - Value: key, - }, + providerSpecificAlias: "true", + providerSpecificEvaluateTargetHealth: key, }, }, { @@ -2099,14 +2093,8 @@ func TestAWSCreateRecordsWithALIAS(t *testing.T) { Targets: endpoint.Targets{"foo.eu-central-1.elb.amazonaws.com"}, RecordType: endpoint.RecordTypeAAAA, ProviderSpecific: endpoint.ProviderSpecific{ - endpoint.ProviderSpecificProperty{ - Name: providerSpecificAlias, - Value: "true", - }, - endpoint.ProviderSpecificProperty{ - Name: providerSpecificEvaluateTargetHealth, - Value: key, - }, + providerSpecificAlias: "true", + providerSpecificEvaluateTargetHealth: key, }, }, } diff --git a/provider/cloudflare/cloudflare.go b/provider/cloudflare/cloudflare.go index c701d8b925..57a036d5a1 100644 --- a/provider/cloudflare/cloudflare.go +++ b/provider/cloudflare/cloudflare.go @@ -924,15 +924,12 @@ func getCustomHostnamesSSLOptions(customHostnamesConfig CustomHostnamesConfig) * func shouldBeProxied(ep *endpoint.Endpoint, proxiedByDefault bool) bool { proxied := proxiedByDefault - for _, v := range ep.ProviderSpecific { - if v.Name == annotations.CloudflareProxiedKey { - b, err := strconv.ParseBool(v.Value) - if err != nil { - log.Errorf("Failed to parse annotation [%q]: %v", annotations.CloudflareProxiedKey, err) - } else { - proxied = b - } - break + if value, exists := ep.ProviderSpecific.Get(annotations.CloudflareProxiedKey); exists { + b, err := strconv.ParseBool(value) + if err != nil { + log.Errorf("Failed to parse annotation [%q]: %v", annotations.CloudflareProxiedKey, err) + } else { + proxied = b } } @@ -943,11 +940,9 @@ func shouldBeProxied(ep *endpoint.Endpoint, proxiedByDefault bool) bool { } func getEndpointCustomHostnames(ep *endpoint.Endpoint) []string { - for _, v := range ep.ProviderSpecific { - if v.Name == annotations.CloudflareCustomHostnameKey { - customHostnames := strings.Split(v.Value, ",") - return customHostnames - } + if value, exists := ep.ProviderSpecific.Get(annotations.CloudflareCustomHostnameKey); exists { + customHostnames := strings.Split(value, ",") + return customHostnames } return []string{} } diff --git a/provider/cloudflare/cloudflare_regional_test.go b/provider/cloudflare/cloudflare_regional_test.go index b114adafcc..74d3ebb13f 100644 --- a/provider/cloudflare/cloudflare_regional_test.go +++ b/provider/cloudflare/cloudflare_regional_test.go @@ -119,10 +119,7 @@ func TestCloudflareRegionalHostnameActions(t *testing.T) { DNSName: "create.bar.com", Targets: endpoint.Targets{"127.0.0.1"}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", - Value: "eu", - }, + "external-dns.alpha.kubernetes.io/cloudflare-region-key": "eu", }, }, }, @@ -174,10 +171,7 @@ func TestCloudflareRegionalHostnameActions(t *testing.T) { DNSName: "update.bar.com", Targets: endpoint.Targets{"127.0.0.1"}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", - Value: "eu", - }, + "external-dns.alpha.kubernetes.io/cloudflare-region-key": "eu", }, }, }, @@ -264,10 +258,7 @@ func TestCloudflareRegionalHostnameActions(t *testing.T) { DNSName: "nochange.bar.com", Targets: endpoint.Targets{"127.0.0.1"}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", - Value: "eu", - }, + "external-dns.alpha.kubernetes.io/cloudflare-region-key": "eu", }, }, }, @@ -396,10 +387,7 @@ func Test_regionalHostname(t *testing.T) { RecordType: "A", DNSName: "example.com", ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", - Value: "eu", - }, + "external-dns.alpha.kubernetes.io/cloudflare-region-key": "eu", }, }, config: RegionalServicesConfig{ @@ -419,10 +407,7 @@ func Test_regionalHostname(t *testing.T) { RecordType: "A", DNSName: "example.com", ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", - Value: "", - }, + "external-dns.alpha.kubernetes.io/cloudflare-region-key": "", }, }, config: RegionalServicesConfig{ @@ -442,10 +427,7 @@ func Test_regionalHostname(t *testing.T) { RecordType: "TXT", DNSName: "example.com", ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", - Value: "eu", - }, + "external-dns.alpha.kubernetes.io/cloudflare-region-key": "eu", }, }, config: RegionalServicesConfig{ @@ -462,10 +444,7 @@ func Test_regionalHostname(t *testing.T) { RecordType: "A", DNSName: "example.com", ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", - Value: "us", - }, + "external-dns.alpha.kubernetes.io/cloudflare-region-key": "us", }, }, config: RegionalServicesConfig{ @@ -869,7 +848,7 @@ func TestApplyChangesWithRegionalHostnamesFaillures(t *testing.T) { DNSName: "foo.error.com", Targets: endpoint.Targets{"127.0.0.1"}, ProviderSpecific: endpoint.ProviderSpecific{ - {Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", Value: "eu"}, + "external-dns.alpha.kubernetes.io/cloudflare-region-key": "eu", }, }, }, @@ -892,7 +871,7 @@ func TestApplyChangesWithRegionalHostnamesFaillures(t *testing.T) { DNSName: "rherror.bar.com", Targets: endpoint.Targets{"127.0.0.1"}, ProviderSpecific: endpoint.ProviderSpecific{ - {Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", Value: "eu"}, + "external-dns.alpha.kubernetes.io/cloudflare-region-key": "eu", }, }, }, @@ -924,7 +903,7 @@ func TestApplyChangesWithRegionalHostnamesFaillures(t *testing.T) { DNSName: "rherror.bar.com", Targets: endpoint.Targets{"127.0.0.1"}, ProviderSpecific: endpoint.ProviderSpecific{ - {Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", Value: "eu"}, + "external-dns.alpha.kubernetes.io/cloudflare-region-key": "eu", }, }, }, @@ -934,7 +913,7 @@ func TestApplyChangesWithRegionalHostnamesFaillures(t *testing.T) { DNSName: "rherror.bar.com", Targets: endpoint.Targets{"127.0.0.2"}, ProviderSpecific: endpoint.ProviderSpecific{ - {Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", Value: "eu"}, + "external-dns.alpha.kubernetes.io/cloudflare-region-key": "eu", }, }, }, @@ -987,7 +966,7 @@ func TestApplyChangesWithRegionalHostnamesFaillures(t *testing.T) { DNSName: "foo.bar.com", Targets: endpoint.Targets{"127.0.0.1"}, ProviderSpecific: endpoint.ProviderSpecific{ - {Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", Value: "eu"}, + "external-dns.alpha.kubernetes.io/cloudflare-region-key": "eu", }, }, { @@ -995,7 +974,7 @@ func TestApplyChangesWithRegionalHostnamesFaillures(t *testing.T) { DNSName: "foo.bar.com", Targets: endpoint.Targets{"127.0.0.1"}, ProviderSpecific: endpoint.ProviderSpecific{ - {Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", Value: "us"}, + "external-dns.alpha.kubernetes.io/cloudflare-region-key": "us", }, }, }, @@ -1073,7 +1052,7 @@ func TestApplyChangesWithRegionalHostnamesDryRun(t *testing.T) { DNSName: "foo.bar.com", Targets: endpoint.Targets{"127.0.0.1"}, ProviderSpecific: endpoint.ProviderSpecific{ - {Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", Value: "eu"}, + "external-dns.alpha.kubernetes.io/cloudflare-region-key": "eu", }, }, }, @@ -1105,7 +1084,7 @@ func TestApplyChangesWithRegionalHostnamesDryRun(t *testing.T) { DNSName: "foo.bar.com", Targets: endpoint.Targets{"127.0.0.1"}, ProviderSpecific: endpoint.ProviderSpecific{ - {Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", Value: "eu"}, + "external-dns.alpha.kubernetes.io/cloudflare-region-key": "eu", }, }, }, @@ -1115,7 +1094,7 @@ func TestApplyChangesWithRegionalHostnamesDryRun(t *testing.T) { DNSName: "foo.bar.com", Targets: endpoint.Targets{"127.0.0.2"}, ProviderSpecific: endpoint.ProviderSpecific{ - {Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", Value: "eu"}, + "external-dns.alpha.kubernetes.io/cloudflare-region-key": "eu", }, }, }, @@ -1265,10 +1244,7 @@ func TestCloudflareAdjustEndpointsRegionalServices(t *testing.T) { if tc.initialRegionKey != "" { testEndpoint.ProviderSpecific = endpoint.ProviderSpecific{ - endpoint.ProviderSpecificProperty{ - Name: annotations.CloudflareRegionKey, - Value: tc.initialRegionKey, - }, + annotations.CloudflareRegionKey: tc.initialRegionKey, } } diff --git a/provider/cloudflare/cloudflare_test.go b/provider/cloudflare/cloudflare_test.go index 6bb7a6bc1a..2c3bf5ff03 100644 --- a/provider/cloudflare/cloudflare_test.go +++ b/provider/cloudflare/cloudflare_test.go @@ -622,10 +622,7 @@ func TestCloudflareProxiedOverrideTrue(t *testing.T) { DNSName: "bar.com", Targets: endpoint.Targets{"127.0.0.1"}, ProviderSpecific: endpoint.ProviderSpecific{ - endpoint.ProviderSpecificProperty{ - Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied", - Value: "true", - }, + "external-dns.alpha.kubernetes.io/cloudflare-proxied": "true", }, }, } @@ -656,10 +653,7 @@ func TestCloudflareProxiedOverrideFalse(t *testing.T) { DNSName: "bar.com", Targets: endpoint.Targets{"127.0.0.1"}, ProviderSpecific: endpoint.ProviderSpecific{ - endpoint.ProviderSpecificProperty{ - Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied", - Value: "false", - }, + "external-dns.alpha.kubernetes.io/cloudflare-proxied": "false", }, }, } @@ -690,10 +684,7 @@ func TestCloudflareProxiedOverrideIllegal(t *testing.T) { DNSName: "bar.com", Targets: endpoint.Targets{"127.0.0.1"}, ProviderSpecific: endpoint.ProviderSpecific{ - endpoint.ProviderSpecificProperty{ - Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied", - Value: "asfasdfa", - }, + "external-dns.alpha.kubernetes.io/cloudflare-proxied": "asfasdfa", }, }, } @@ -754,10 +745,7 @@ func TestCloudflareSetProxied(t *testing.T) { DNSName: testCase.domain, Targets: endpoint.Targets{targets[0]}, ProviderSpecific: endpoint.ProviderSpecific{ - endpoint.ProviderSpecificProperty{ - Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied", - Value: "true", - }, + "external-dns.alpha.kubernetes.io/cloudflare-proxied": "true", }, }, } @@ -1223,10 +1211,7 @@ func TestCloudflareGroupByNameAndType(t *testing.T) { RecordTTL: endpoint.TTL(defaultTTL), Labels: endpoint.Labels{}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied", - Value: "false", - }, + "external-dns.alpha.kubernetes.io/cloudflare-proxied": "false", }, }, }, @@ -1257,10 +1242,7 @@ func TestCloudflareGroupByNameAndType(t *testing.T) { RecordTTL: endpoint.TTL(defaultTTL), Labels: endpoint.Labels{}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied", - Value: "false", - }, + "external-dns.alpha.kubernetes.io/cloudflare-proxied": "false", }, }, }, @@ -1305,10 +1287,7 @@ func TestCloudflareGroupByNameAndType(t *testing.T) { RecordTTL: endpoint.TTL(defaultTTL), Labels: endpoint.Labels{}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied", - Value: "false", - }, + "external-dns.alpha.kubernetes.io/cloudflare-proxied": "false", }, }, { @@ -1318,10 +1297,7 @@ func TestCloudflareGroupByNameAndType(t *testing.T) { RecordTTL: endpoint.TTL(defaultTTL), Labels: endpoint.Labels{}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied", - Value: "false", - }, + "external-dns.alpha.kubernetes.io/cloudflare-proxied": "false", }, }, }, @@ -1359,10 +1335,7 @@ func TestCloudflareGroupByNameAndType(t *testing.T) { RecordTTL: endpoint.TTL(defaultTTL), Labels: endpoint.Labels{}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied", - Value: "false", - }, + "external-dns.alpha.kubernetes.io/cloudflare-proxied": "false", }, }, { @@ -1372,10 +1345,7 @@ func TestCloudflareGroupByNameAndType(t *testing.T) { RecordTTL: endpoint.TTL(defaultTTL), Labels: endpoint.Labels{}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied", - Value: "false", - }, + "external-dns.alpha.kubernetes.io/cloudflare-proxied": "false", }, }, }, @@ -1413,10 +1383,7 @@ func TestCloudflareGroupByNameAndType(t *testing.T) { RecordTTL: endpoint.TTL(defaultTTL), Labels: endpoint.Labels{}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied", - Value: "false", - }, + "external-dns.alpha.kubernetes.io/cloudflare-proxied": "false", }, }, }, @@ -1694,10 +1661,7 @@ func TestCloudflareComplexUpdate(t *testing.T) { RecordTTL: endpoint.TTL(defaultTTL), Labels: endpoint.Labels{}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied", - Value: "true", - }, + "external-dns.alpha.kubernetes.io/cloudflare-proxied": "true", }, }, }) @@ -1782,10 +1746,7 @@ func TestCustomTTLWithEnabledProxyNotChanged(t *testing.T) { RecordTTL: 300, Labels: endpoint.Labels{}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied", - Value: "true", - }, + "external-dns.alpha.kubernetes.io/cloudflare-proxied": "true", }, }, } @@ -1900,10 +1861,7 @@ func TestCloudFlareProvider_newCloudFlareChange(t *testing.T) { RecordType: "A", Targets: []string{"192.0.2.1"}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: annotations.CloudflareRecordCommentKey, - Value: freeValidCommentBuilder.String(), - }, + annotations.CloudflareRecordCommentKey: freeInvalidCommentBuilder.String(), }, }, expected: len(freeValidCommentBuilder.String()), @@ -1916,10 +1874,7 @@ func TestCloudFlareProvider_newCloudFlareChange(t *testing.T) { RecordType: "A", Targets: []string{"192.0.2.1"}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: annotations.CloudflareRecordCommentKey, - Value: freeInvalidCommentBuilder.String(), - }, + annotations.CloudflareRecordCommentKey: freeInvalidCommentBuilder.String(), }, }, expected: freeZoneMaxCommentLength, @@ -1932,10 +1887,7 @@ func TestCloudFlareProvider_newCloudFlareChange(t *testing.T) { RecordType: "A", Targets: []string{"192.0.2.1"}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: annotations.CloudflareRecordCommentKey, - Value: paidValidCommentBuilder.String(), - }, + annotations.CloudflareRecordCommentKey: paidValidCommentBuilder.String(), }, }, expected: len(paidValidCommentBuilder.String()), @@ -1948,10 +1900,7 @@ func TestCloudFlareProvider_newCloudFlareChange(t *testing.T) { RecordType: "A", Targets: []string{"192.0.2.1"}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: annotations.CloudflareRecordCommentKey, - Value: paidInvalidCommentBuilder.String(), - }, + annotations.CloudflareRecordCommentKey: paidInvalidCommentBuilder.String(), }, }, expected: paidZoneMaxCommentLength, @@ -2344,10 +2293,7 @@ func TestCloudflareDisabledCustomHostnameOperations(t *testing.T) { RecordTTL: endpoint.TTL(defaultTTL), Labels: endpoint.Labels{}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "a.foo.fancybar.com", - }, + "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname": "a.foo.fancybar.com", }, }, { @@ -2364,10 +2310,7 @@ func TestCloudflareDisabledCustomHostnameOperations(t *testing.T) { RecordTTL: endpoint.TTL(defaultTTL), Labels: endpoint.Labels{}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "c1.foo.fancybar.com", - }, + "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname": "c1.foo.fancybar.com", }, }, }, @@ -2390,10 +2333,7 @@ func TestCloudflareDisabledCustomHostnameOperations(t *testing.T) { RecordTTL: endpoint.TTL(defaultTTL), Labels: endpoint.Labels{}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "b.foo.fancybar.com", - }, + "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname": "b.foo.fancybar.com", }, }, { @@ -2403,10 +2343,7 @@ func TestCloudflareDisabledCustomHostnameOperations(t *testing.T) { RecordTTL: endpoint.TTL(defaultTTL), Labels: endpoint.Labels{}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "c2.foo.fancybar.com", - }, + "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname": "c2.foo.fancybar.com", }, }, }, @@ -2467,10 +2404,7 @@ func TestCloudflareCustomHostnameNotFoundOnRecordDeletion(t *testing.T) { RecordTTL: endpoint.TTL(defaultTTL), Labels: endpoint.Labels{}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "newerror-getCustomHostnameOrigin.foo.fancybar.com", - }, + "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname": "newerror-getCustomHostnameOrigin.foo.fancybar.com", }, }, }, @@ -2499,10 +2433,7 @@ func TestCloudflareCustomHostnameNotFoundOnRecordDeletion(t *testing.T) { RecordTTL: endpoint.TTL(defaultTTL), Labels: endpoint.Labels{}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "a.foo.fancybar.com", - }, + "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname": "a.foo.fancybar.com", }, }, }, @@ -2587,10 +2518,7 @@ func TestCloudflareListCustomHostnamesWithPagionation(t *testing.T) { RecordTTL: endpoint.TTL(defaultTTL), Labels: endpoint.Labels{}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: fmt.Sprintf("host-%d.foo.fancybar.com", i), - }, + "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname": fmt.Sprintf("host-%d.foo.fancybar.com", i), }, }, } @@ -2669,10 +2597,7 @@ func TestCloudflareApplyChanges_AllErrorLogPaths(t *testing.T) { RecordType: "MX", Targets: endpoint.Targets{"not-a-valid-mx"}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "bad-create-custom.bar.com", - }, + "extenal-dns.alpha.kubernetes.io/cloudflare-custom-hostname": "bad-create-custom.bar.com", }, }}, }, @@ -2687,10 +2612,7 @@ func TestCloudflareApplyChanges_AllErrorLogPaths(t *testing.T) { RecordType: "MX", Targets: endpoint.Targets{"not-a-valid-mx"}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "bad-delete-custom.bar.com", - }, + "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname": "bad-delete-custom.bar.com", }, }}, }, @@ -2705,10 +2627,7 @@ func TestCloudflareApplyChanges_AllErrorLogPaths(t *testing.T) { RecordType: "MX", Targets: endpoint.Targets{"not-a-valid-mx"}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "bad-update-add-custom.bar.com", - }, + "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname": "bad-update-add-custom.bar.com", }, }}, UpdateOld: []*endpoint.Endpoint{{ @@ -2716,10 +2635,7 @@ func TestCloudflareApplyChanges_AllErrorLogPaths(t *testing.T) { RecordType: "MX", Targets: endpoint.Targets{"not-a-valid-mx-but-still-updated"}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "bad-update-add-custom.bar.com", - }, + "exntelnal-dns.alpha.kubernetes.io/cloudflare-custom-hostname": "bad-update-add-custom.bar.com", }, }}, }, @@ -2734,10 +2650,7 @@ func TestCloudflareApplyChanges_AllErrorLogPaths(t *testing.T) { RecordType: "MX", Targets: endpoint.Targets{"not-a-valid-mx"}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "bad-update-leave-custom.bar.com", - }, + "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname": "bad-update-leave-custom.bar.com", }, }}, UpdateNew: []*endpoint.Endpoint{{ @@ -2745,10 +2658,7 @@ func TestCloudflareApplyChanges_AllErrorLogPaths(t *testing.T) { RecordType: "MX", Targets: endpoint.Targets{"not-a-valid-mx"}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "bad-update-leave-custom.bar.com", - }, + "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname": "bad", }, }}, }, diff --git a/provider/inmemory/inmemory.go b/provider/inmemory/inmemory.go index b17ef900e7..01e80b4967 100644 --- a/provider/inmemory/inmemory.go +++ b/provider/inmemory/inmemory.go @@ -207,7 +207,12 @@ func copyEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint { for k, v := range ep.Labels { newEp.Labels[k] = v } - newEp.ProviderSpecific = append(endpoint.ProviderSpecific(nil), ep.ProviderSpecific...) + if ep.ProviderSpecific != nil { + newEp.ProviderSpecific = make(endpoint.ProviderSpecific) + for k, v := range ep.ProviderSpecific { + newEp.ProviderSpecific.Set(k, v) + } + } records = append(records, newEp) } return records diff --git a/provider/scaleway/scaleway_test.go b/provider/scaleway/scaleway_test.go index eec95c6953..b1d3b54e6f 100644 --- a/provider/scaleway/scaleway_test.go +++ b/provider/scaleway/scaleway_test.go @@ -180,10 +180,7 @@ func TestScalewayProvider_AdjustEndpoints(t *testing.T) { RecordType: "A", Targets: []string{"1.1.1.1"}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: scalewayPriorityKey, - Value: "0", - }, + scalewayPriorityKey: "0", }, }, { @@ -192,10 +189,7 @@ func TestScalewayProvider_AdjustEndpoints(t *testing.T) { RecordType: "A", Targets: []string{"1.1.1.1"}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: scalewayPriorityKey, - Value: "10", - }, + scalewayPriorityKey: "10", }, }, { @@ -214,10 +208,7 @@ func TestScalewayProvider_AdjustEndpoints(t *testing.T) { RecordType: "A", Targets: []string{"1.1.1.1"}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: scalewayPriorityKey, - Value: "0", - }, + scalewayPriorityKey: "0", }, }, { @@ -226,10 +217,7 @@ func TestScalewayProvider_AdjustEndpoints(t *testing.T) { RecordType: "A", Targets: []string{"1.1.1.1"}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: scalewayPriorityKey, - Value: "10", - }, + scalewayPriorityKey: "10", }, }, { @@ -238,10 +226,7 @@ func TestScalewayProvider_AdjustEndpoints(t *testing.T) { RecordType: "A", Targets: []string{"1.1.1.1"}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: scalewayPriorityKey, - Value: "0", - }, + scalewayPriorityKey: "0", }, }, } @@ -296,10 +281,7 @@ func TestScalewayProvider_Records(t *testing.T) { RecordType: "A", Targets: []string{"1.1.1.1"}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: scalewayPriorityKey, - Value: "0", - }, + scalewayPriorityKey: "0", }, }, { @@ -308,10 +290,7 @@ func TestScalewayProvider_Records(t *testing.T) { RecordType: "A", Targets: []string{"1.1.1.2", "1.1.1.3"}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: scalewayPriorityKey, - Value: "0", - }, + scalewayPriorityKey: "0", }, }, { @@ -320,10 +299,7 @@ func TestScalewayProvider_Records(t *testing.T) { RecordType: "A", Targets: []string{"1.1.1.1"}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: scalewayPriorityKey, - Value: "0", - }, + scalewayPriorityKey: "0", }, }, { @@ -332,10 +308,7 @@ func TestScalewayProvider_Records(t *testing.T) { RecordType: "CNAME", Targets: []string{"test.example.com"}, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: scalewayPriorityKey, - Value: "30", - }, + scalewayPriorityKey: "30", }, }, } @@ -498,10 +471,7 @@ func TestScalewayProvider_generateApplyRequests(t *testing.T) { DNSName: "test.example.com", RecordType: "CNAME", ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: scalewayPriorityKey, - Value: "20", - }, + scalewayPriorityKey: "20", }, RecordTTL: 600, Targets: []string{"example.com"}, @@ -523,10 +493,7 @@ func TestScalewayProvider_generateApplyRequests(t *testing.T) { { DNSName: "me.example.com", ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: scalewayPriorityKey, - Value: "30", - }, + scalewayPriorityKey: "30", }, RecordType: "A", RecordTTL: 600, @@ -542,10 +509,7 @@ func TestScalewayProvider_generateApplyRequests(t *testing.T) { { DNSName: "me.example.com", ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: scalewayPriorityKey, - Value: "1234", - }, + scalewayPriorityKey: "1234", }, RecordType: "A", Targets: []string{"3.3.3.3"}, diff --git a/provider/webhook/webhook_test.go b/provider/webhook/webhook_test.go index 85c2a3b55c..f9697c6109 100644 --- a/provider/webhook/webhook_test.go +++ b/provider/webhook/webhook_test.go @@ -395,8 +395,9 @@ func TestApplyChangesWithProviderSpecificProperty(t *testing.T) { assert.NoError(t, err) assert.Len(t, changes.Create, 1) assert.Len(t, changes.Create[0].ProviderSpecific, 1) - assert.Equal(t, "prop1", changes.Create[0].ProviderSpecific[0].Name) - assert.Equal(t, "value1", changes.Create[0].ProviderSpecific[0].Value) + v, ok := changes.Create[0].ProviderSpecific.Get("prop1") + assert.True(t, ok) + assert.Equal(t, "value1", v) w.WriteHeader(http.StatusNoContent) return } @@ -413,10 +414,7 @@ func TestApplyChangesWithProviderSpecificProperty(t *testing.T) { "", }, ProviderSpecific: endpoint.ProviderSpecific{ - endpoint.ProviderSpecificProperty{ - Name: "prop1", - Value: "value1", - }, + "prop1": "value1", }, } err = p.ApplyChanges(context.TODO(), &plan.Changes{ diff --git a/registry/dynamodb_test.go b/registry/dynamodb_test.go index 091927fabd..12b5f3a9a8 100644 --- a/registry/dynamodb_test.go +++ b/registry/dynamodb_test.go @@ -213,10 +213,7 @@ func TestDynamoDBRegistryRecords(t *testing.T) { endpoint.ResourceLabelKey: "ingress/default/other-ingress", }, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: dynamodbAttributeMigrate, - Value: "true", - }, + dynamodbAttributeMigrate: "true", }, }, { @@ -868,10 +865,7 @@ func TestDynamoDBRegistryApplyChanges(t *testing.T) { endpoint.ResourceLabelKey: "ingress/default/my-ingress", }, ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: dynamodbAttributeMigrate, - Value: "true", - }, + dynamodbAttributeMigrate: "true", }, }, }, diff --git a/registry/txt_test.go b/registry/txt_test.go index 05cdd030d2..235cbe72ab 100644 --- a/registry/txt_test.go +++ b/registry/txt_test.go @@ -424,11 +424,8 @@ func testTXTRegistryRecordsNoPrefix(t *testing.T) { Labels: map[string]string{ endpoint.OwnerLabelKey: "owner", }, - ProviderSpecific: []endpoint.ProviderSpecificProperty{ - { - Name: "alias", - Value: "true", - }, + ProviderSpecific: endpoint.ProviderSpecific{ + "alias": "true", }, }, { @@ -990,11 +987,8 @@ func testTXTRegistryMissingRecordsNoPrefix(t *testing.T) { // owner was added from the TXT record's target endpoint.OwnerLabelKey: "owner", }, - ProviderSpecific: []endpoint.ProviderSpecificProperty{ - { - Name: "txt/force-update", - Value: "true", - }, + ProviderSpecific: endpoint.ProviderSpecific{ + "txt/force-update": "true", }, }, { @@ -1004,11 +998,8 @@ func testTXTRegistryMissingRecordsNoPrefix(t *testing.T) { Labels: map[string]string{ endpoint.OwnerLabelKey: "owner", }, - ProviderSpecific: []endpoint.ProviderSpecificProperty{ - { - Name: "txt/force-update", - Value: "true", - }, + ProviderSpecific: endpoint.ProviderSpecific{ + "txt/force-update": "true", }, }, { @@ -1095,11 +1086,8 @@ func testTXTRegistryMissingRecordsWithPrefix(t *testing.T) { // owner was added from the TXT record's target endpoint.OwnerLabelKey: "owner", }, - ProviderSpecific: []endpoint.ProviderSpecificProperty{ - { - Name: "txt/force-update", - Value: "true", - }, + ProviderSpecific: endpoint.ProviderSpecific{ + "txt/force-update": "true", }, }, { @@ -1109,11 +1097,8 @@ func testTXTRegistryMissingRecordsWithPrefix(t *testing.T) { Labels: map[string]string{ endpoint.OwnerLabelKey: "owner", }, - ProviderSpecific: []endpoint.ProviderSpecificProperty{ - { - Name: "txt/force-update", - Value: "true", - }, + ProviderSpecific: endpoint.ProviderSpecific{ + "txt/force-update": "true", }, }, { @@ -1123,11 +1108,8 @@ func testTXTRegistryMissingRecordsWithPrefix(t *testing.T) { Labels: map[string]string{ endpoint.OwnerLabelKey: "owner", }, - ProviderSpecific: []endpoint.ProviderSpecificProperty{ - { - Name: "txt/force-update", - Value: "true", - }, + ProviderSpecific: endpoint.ProviderSpecific{ + "txt/force-update": "true", }, }, { diff --git a/source/ambassador_host_test.go b/source/ambassador_host_test.go index 561bc1e02a..3b18875047 100644 --- a/source/ambassador_host_test.go +++ b/source/ambassador_host_test.go @@ -273,10 +273,9 @@ func TestAmbassadorHostSource(t *testing.T) { DNSName: "www.example.org", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"1.1.1.1"}, - ProviderSpecific: endpoint.ProviderSpecific{{ - Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied", - Value: "true", - }}, + ProviderSpecific: endpoint.ProviderSpecific{ + "external-dns.alpha.kubernetes.io/cloudflare-proxied": "true", + }, }, }, }, { diff --git a/source/annotations/provider_specific.go b/source/annotations/provider_specific.go index 0518effe5c..f7bd9335e0 100644 --- a/source/annotations/provider_specific.go +++ b/source/annotations/provider_specific.go @@ -21,60 +21,33 @@ import ( ) func ProviderSpecificAnnotations(annotations map[string]string) (endpoint.ProviderSpecific, string) { - providerSpecificAnnotations := endpoint.ProviderSpecific{} + providerSpecificAnnotations := make(endpoint.ProviderSpecific) if hasAliasFromAnnotations(annotations) { - providerSpecificAnnotations = append(providerSpecificAnnotations, endpoint.ProviderSpecificProperty{ - Name: "alias", - Value: "true", - }) + providerSpecificAnnotations.Set("alias", "true") } setIdentifier := "" for k, v := range annotations { if k == SetIdentifierKey { setIdentifier = v } else if attr, ok := strings.CutPrefix(k, AWSPrefix); ok { - providerSpecificAnnotations = append(providerSpecificAnnotations, endpoint.ProviderSpecificProperty{ - Name: fmt.Sprintf("aws/%s", attr), - Value: v, - }) + providerSpecificAnnotations.Set(fmt.Sprintf("aws/%s", attr), v) } else if attr, ok := strings.CutPrefix(k, SCWPrefix); ok { - providerSpecificAnnotations = append(providerSpecificAnnotations, endpoint.ProviderSpecificProperty{ - Name: fmt.Sprintf("scw/%s", attr), - Value: v, - }) + providerSpecificAnnotations.Set(fmt.Sprintf("scw/%s", attr), v) } else if attr, ok := strings.CutPrefix(k, WebhookPrefix); ok { // Support for wildcard annotations for webhook providers - providerSpecificAnnotations = append(providerSpecificAnnotations, endpoint.ProviderSpecificProperty{ - Name: fmt.Sprintf("webhook/%s", attr), - Value: v, - }) + providerSpecificAnnotations.Set(fmt.Sprintf("webhook/%s", attr), v) } else if attr, ok := strings.CutPrefix(k, CoreDNSPrefix); ok { - providerSpecificAnnotations = append(providerSpecificAnnotations, endpoint.ProviderSpecificProperty{ - Name: fmt.Sprintf("coredns/%s", attr), - Value: v, - }) + providerSpecificAnnotations.Set(fmt.Sprintf("coredns/%s", attr), v) } else if strings.HasPrefix(k, CloudflarePrefix) { if strings.Contains(k, CloudflareCustomHostnameKey) { - providerSpecificAnnotations = append(providerSpecificAnnotations, endpoint.ProviderSpecificProperty{ - Name: CloudflareCustomHostnameKey, - Value: v, - }) + providerSpecificAnnotations.Set(CloudflareCustomHostnameKey, v) } else if strings.Contains(k, CloudflareProxiedKey) { - providerSpecificAnnotations = append(providerSpecificAnnotations, endpoint.ProviderSpecificProperty{ - Name: CloudflareProxiedKey, - Value: v, - }) + providerSpecificAnnotations.Set(CloudflareProxiedKey, v) } else if strings.Contains(k, CloudflareRegionKey) { - providerSpecificAnnotations = append(providerSpecificAnnotations, endpoint.ProviderSpecificProperty{ - Name: CloudflareRegionKey, - Value: v, - }) + providerSpecificAnnotations.Set(CloudflareRegionKey, v) } else if strings.Contains(k, CloudflareRecordCommentKey) { - providerSpecificAnnotations = append(providerSpecificAnnotations, endpoint.ProviderSpecificProperty{ - Name: CloudflareRecordCommentKey, - Value: v, - }) + providerSpecificAnnotations.Set(CloudflareRecordCommentKey, v) } } } diff --git a/source/annotations/provider_specific_test.go b/source/annotations/provider_specific_test.go index eb56e1d138..4821f84ed2 100644 --- a/source/annotations/provider_specific_test.go +++ b/source/annotations/provider_specific_test.go @@ -40,7 +40,7 @@ func TestProviderSpecificAnnotations(t *testing.T) { CloudflareProxiedKey: "true", }, expected: endpoint.ProviderSpecific{ - {Name: CloudflareProxiedKey, Value: "true"}, + CloudflareProxiedKey: "true", }, setIdentifier: "", }, @@ -50,7 +50,7 @@ func TestProviderSpecificAnnotations(t *testing.T) { CloudflareCustomHostnameKey: "custom.example.com", }, expected: endpoint.ProviderSpecific{ - {Name: CloudflareCustomHostnameKey, Value: "custom.example.com"}, + CloudflareCustomHostnameKey: "custom.example.com", }, setIdentifier: "", }, @@ -60,7 +60,7 @@ func TestProviderSpecificAnnotations(t *testing.T) { "external-dns.alpha.kubernetes.io/aws-weight": "100", }, expected: endpoint.ProviderSpecific{ - {Name: "aws/weight", Value: "100"}, + "aws/weight": "100", }, setIdentifier: "", }, @@ -70,7 +70,7 @@ func TestProviderSpecificAnnotations(t *testing.T) { "external-dns.alpha.kubernetes.io/coredns-group": "g1", }, expected: endpoint.ProviderSpecific{ - {Name: "coredns/group", Value: "g1"}, + "coredns/group": "g1", }, setIdentifier: "", }, @@ -125,11 +125,9 @@ func TestGetProviderSpecificCloudflareAnnotations(t *testing.T) { } { t.Run(tc.title, func(t *testing.T) { providerSpecificAnnotations, _ := ProviderSpecificAnnotations(tc.annotations) - for _, providerSpecificAnnotation := range providerSpecificAnnotations { - if providerSpecificAnnotation.Name == tc.expectedKey { - assert.Equal(t, strconv.FormatBool(tc.expectedValue), providerSpecificAnnotation.Value) - return - } + if value, exists := providerSpecificAnnotations.Get(tc.expectedKey); exists { + assert.Equal(t, strconv.FormatBool(tc.expectedValue), value) + return } t.Errorf("Cloudflare provider specific annotation %s is not set correctly to %v", tc.expectedKey, tc.expectedValue) }) @@ -168,11 +166,9 @@ func TestGetProviderSpecificCloudflareAnnotations(t *testing.T) { } { t.Run(tc.title, func(t *testing.T) { providerSpecificAnnotations, _ := ProviderSpecificAnnotations(tc.annotations) - for _, providerSpecificAnnotation := range providerSpecificAnnotations { - if providerSpecificAnnotation.Name == tc.expectedKey { - assert.Equal(t, tc.expectedValue, providerSpecificAnnotation.Value) - return - } + if value, exists := providerSpecificAnnotations.Get(tc.expectedKey); exists { + assert.Equal(t, tc.expectedValue, value) + return } t.Errorf("Cloudflare provider specific annotation %s is not set correctly to %v", tc.expectedKey, tc.expectedValue) }) @@ -202,11 +198,9 @@ func TestGetProviderSpecificCloudflareAnnotations(t *testing.T) { } { t.Run(tc.title, func(t *testing.T) { providerSpecificAnnotations, _ := ProviderSpecificAnnotations(tc.annotations) - for _, providerSpecificAnnotation := range providerSpecificAnnotations { - if providerSpecificAnnotation.Name == tc.expectedKey { - assert.Equal(t, tc.expectedValue, providerSpecificAnnotation.Value) - return - } + if value, exists := providerSpecificAnnotations.Get(tc.expectedKey); exists { + assert.Equal(t, tc.expectedValue, value) + return } t.Errorf("Cloudflare provider specific annotation %s is not set correctly to %s", tc.expectedKey, tc.expectedValue) }) @@ -239,11 +233,9 @@ func TestGetProviderSpecificAliasAnnotations(t *testing.T) { } { t.Run(tc.title, func(t *testing.T) { providerSpecificAnnotations, _ := ProviderSpecificAnnotations(tc.annotations) - for _, providerSpecificAnnotation := range providerSpecificAnnotations { - if providerSpecificAnnotation.Name == "alias" { - assert.Equal(t, strconv.FormatBool(tc.expectedValue), providerSpecificAnnotation.Value) - return - } + if value, exists := providerSpecificAnnotations.Get("alias"); exists { + assert.Equal(t, strconv.FormatBool(tc.expectedValue), value) + return } t.Errorf("provider specific annotation alias is not set correctly to %v", tc.expectedValue) }) @@ -267,10 +259,8 @@ func TestGetProviderSpecificAliasAnnotations(t *testing.T) { } { t.Run(tc.title, func(t *testing.T) { providerSpecificAnnotations, _ := ProviderSpecificAnnotations(tc.annotations) - for _, providerSpecificAnnotation := range providerSpecificAnnotations { - if providerSpecificAnnotation.Name == "alias" { - t.Error("provider specific annotation alias is not expected to be set") - } + if _, exists := providerSpecificAnnotations.Get("alias"); exists { + t.Error("provider specific annotation alias is not expected to be set") } }) @@ -328,15 +318,9 @@ func TestGetProviderSpecificIdentifierAnnotations(t *testing.T) { providerSpecificAnnotations, identifier := ProviderSpecificAnnotations(tc.annotations) assert.Equal(t, tc.expectedIdentifier, identifier) for expectedAnnotationKey, expectedAnnotationValue := range tc.expectedResult { - expectedResultFound := false - for _, providerSpecificAnnotation := range providerSpecificAnnotations { - if providerSpecificAnnotation.Name == expectedAnnotationKey { - assert.Equal(t, expectedAnnotationValue, providerSpecificAnnotation.Value) - expectedResultFound = true - break - } - } - if !expectedResultFound { + if value, exists := providerSpecificAnnotations.Get(expectedAnnotationKey); exists { + assert.Equal(t, expectedAnnotationValue, value) + } else { t.Errorf("provider specific annotation %s has not been set", expectedAnnotationKey) } } diff --git a/source/crd.go b/source/crd.go index f259f074ae..ac47064426 100644 --- a/source/crd.go +++ b/source/crd.go @@ -161,13 +161,9 @@ func (cs *crdSource) AddEventHandler(_ context.Context, handler func()) { } func fromCRDEndpoint(crdEp *apiv1alpha1.Endpoint) *endpoint.Endpoint { - //TODO: - p := endpoint.ProviderSpecific{} + p := make(endpoint.ProviderSpecific) for _, ps := range crdEp.ProviderSpecific { - p = append(p, endpoint.ProviderSpecificProperty{ - Name: ps.Name, - Value: ps.Value, - }) + p.Set(ps.Name, ps.Value) } ep := &endpoint.Endpoint{ DNSName: crdEp.DNSName, diff --git a/source/endpoints_test.go b/source/endpoints_test.go index e64f903e18..9caa1e4749 100644 --- a/source/endpoints_test.go +++ b/source/endpoints_test.go @@ -43,7 +43,7 @@ func TestEndpointsForHostname(t *testing.T) { targets: endpoint.Targets{"192.0.2.1", "192.0.2.2"}, ttl: endpoint.TTL(300), providerSpecific: endpoint.ProviderSpecific{ - {Name: "provider", Value: "value"}, + "provider": "value", }, setIdentifier: "identifier", resource: "resource", @@ -53,7 +53,7 @@ func TestEndpointsForHostname(t *testing.T) { Targets: endpoint.Targets{"192.0.2.1", "192.0.2.2"}, RecordType: endpoint.RecordTypeA, RecordTTL: endpoint.TTL(300), - ProviderSpecific: endpoint.ProviderSpecific{{Name: "provider", Value: "value"}}, + ProviderSpecific: endpoint.ProviderSpecific{"provider": "value"}, SetIdentifier: "identifier", Labels: map[string]string{endpoint.ResourceLabelKey: "resource"}, }, @@ -65,7 +65,7 @@ func TestEndpointsForHostname(t *testing.T) { targets: endpoint.Targets{"2001:db8::1", "2001:db8::2"}, ttl: endpoint.TTL(300), providerSpecific: endpoint.ProviderSpecific{ - {Name: "provider", Value: "value"}, + "provider": "value", }, setIdentifier: "identifier", resource: "resource", @@ -75,7 +75,7 @@ func TestEndpointsForHostname(t *testing.T) { Targets: endpoint.Targets{"2001:db8::1", "2001:db8::2"}, RecordType: endpoint.RecordTypeAAAA, RecordTTL: endpoint.TTL(300), - ProviderSpecific: endpoint.ProviderSpecific{{Name: "provider", Value: "value"}}, + ProviderSpecific: endpoint.ProviderSpecific{"provider": "value"}, SetIdentifier: "identifier", Labels: map[string]string{endpoint.ResourceLabelKey: "resource"}, }, @@ -87,7 +87,7 @@ func TestEndpointsForHostname(t *testing.T) { targets: endpoint.Targets{"cname.example.com"}, ttl: endpoint.TTL(300), providerSpecific: endpoint.ProviderSpecific{ - {Name: "provider", Value: "value"}, + "provider": "value", }, setIdentifier: "identifier", resource: "resource", @@ -97,7 +97,7 @@ func TestEndpointsForHostname(t *testing.T) { Targets: endpoint.Targets{"cname.example.com"}, RecordType: endpoint.RecordTypeCNAME, RecordTTL: endpoint.TTL(300), - ProviderSpecific: endpoint.ProviderSpecific{{Name: "provider", Value: "value"}}, + ProviderSpecific: endpoint.ProviderSpecific{"provider": "value"}, SetIdentifier: "identifier", Labels: map[string]string{endpoint.ResourceLabelKey: "resource"}, }, diff --git a/source/gloo_proxy_test.go b/source/gloo_proxy_test.go index 03eb12b364..23e69af3ef 100644 --- a/source/gloo_proxy_test.go +++ b/source/gloo_proxy_test.go @@ -507,10 +507,7 @@ func TestGlooSource(t *testing.T) { RecordTTL: 42, Labels: endpoint.Labels{}, ProviderSpecific: endpoint.ProviderSpecific{ - endpoint.ProviderSpecificProperty{ - Name: "aws/geolocation-country-code", - Value: "LU", - }, + "aws/geolocation-country-code": "LU", }, }, { @@ -529,10 +526,7 @@ func TestGlooSource(t *testing.T) { RecordTTL: 24, Labels: endpoint.Labels{}, ProviderSpecific: endpoint.ProviderSpecific{ - endpoint.ProviderSpecificProperty{ - Name: "aws/geolocation-country-code", - Value: "JP", - }, + "aws/geolocation-country-code": "JP", }, }, { @@ -559,10 +553,7 @@ func TestGlooSource(t *testing.T) { RecordTTL: 420, Labels: endpoint.Labels{}, ProviderSpecific: endpoint.ProviderSpecific{ - endpoint.ProviderSpecificProperty{ - Name: "aws/geolocation-country-code", - Value: "ES", - }, + "aws/geolocation-country-code": "ES", }, }, { @@ -580,10 +571,7 @@ func TestGlooSource(t *testing.T) { RecordTTL: 460, Labels: endpoint.Labels{}, ProviderSpecific: endpoint.ProviderSpecific{ - endpoint.ProviderSpecificProperty{ - Name: "aws/geolocation-country-code", - Value: "IT", - }, + "aws/geolocation-country-code": "IT", }, }, }) diff --git a/source/ingress_test.go b/source/ingress_test.go index 6452383e20..08de7dfeb7 100644 --- a/source/ingress_test.go +++ b/source/ingress_test.go @@ -1084,9 +1084,9 @@ func testIngressEndpoints(t *testing.T) { DNSName: "example.org", Targets: endpoint.Targets{"ingress-target.com"}, RecordType: endpoint.RecordTypeCNAME, - ProviderSpecific: endpoint.ProviderSpecific{{ - Name: "alias", Value: "true", - }}, + ProviderSpecific: endpoint.ProviderSpecific{ + "alias": "true", + }, }, }, }, diff --git a/source/wrappers/multisource_test.go b/source/wrappers/multisource_test.go index 082ed1c7cf..61e5c44735 100644 --- a/source/wrappers/multisource_test.go +++ b/source/wrappers/multisource_test.go @@ -47,6 +47,17 @@ func testMultiSourceImplementsSource(t *testing.T) { func testMultiSourceEndpoints(t *testing.T) { foo := &endpoint.Endpoint{DNSName: "foo", Targets: endpoint.Targets{"8.8.8.8"}} bar := &endpoint.Endpoint{DNSName: "bar", Targets: endpoint.Targets{"8.8.4.4"}} + // Create thread-safe copies of endpoints for concurrent test execution + cloneEndpointsForTesting := func(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint { + copies := make([]*endpoint.Endpoint, 0, len(endpoints)) + for _, ep := range endpoints { + copies = append(copies, &endpoint.Endpoint{ + DNSName: ep.DNSName, + Targets: ep.Targets, + }) + } + return copies + } for _, tc := range []struct { title string @@ -65,13 +76,16 @@ func testMultiSourceEndpoints(t *testing.T) { }, { "single non-empty child source returns child's endpoints", - [][]*endpoint.Endpoint{{foo.DeepCopy()}}, - []*endpoint.Endpoint{foo.DeepCopy()}, + [][]*endpoint.Endpoint{cloneEndpointsForTesting([]*endpoint.Endpoint{foo})}, + cloneEndpointsForTesting([]*endpoint.Endpoint{foo}), }, { "multiple non-empty child sources returns merged children's endpoints", - [][]*endpoint.Endpoint{{foo.DeepCopy()}, {bar.DeepCopy()}}, - []*endpoint.Endpoint{foo.DeepCopy(), bar.DeepCopy()}, + [][]*endpoint.Endpoint{ + cloneEndpointsForTesting([]*endpoint.Endpoint{foo}), + cloneEndpointsForTesting([]*endpoint.Endpoint{bar}), + }, + cloneEndpointsForTesting([]*endpoint.Endpoint{foo, bar}), }, } { diff --git a/source/wrappers/nat64source.go b/source/wrappers/nat64source.go index 2c2527db4d..f36eabb43d 100644 --- a/source/wrappers/nat64source.go +++ b/source/wrappers/nat64source.go @@ -102,9 +102,27 @@ func (s *nat64Source) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, erro continue } - v4EP := ep.DeepCopy() - v4EP.Targets = v4Targets - v4EP.RecordType = endpoint.RecordTypeA + v4EP := &endpoint.Endpoint{ + DNSName: ep.DNSName, + Targets: v4Targets, + RecordType: endpoint.RecordTypeA, + SetIdentifier: ep.SetIdentifier, + RecordTTL: ep.RecordTTL, + } + + if ep.Labels != nil { + v4EP.Labels = make(endpoint.Labels, len(ep.Labels)) + for k, v := range ep.Labels { + v4EP.Labels[k] = v + } + } + + if ep.ProviderSpecific != nil { + v4EP.ProviderSpecific = make(endpoint.ProviderSpecific, len(ep.ProviderSpecific)) + for k, v := range ep.ProviderSpecific { + v4EP.ProviderSpecific.Set(k, v) + } + } additionalEndpoints = append(additionalEndpoints, v4EP) } From 5fdeeba32c9c48ab036ba31711962c587e97b712 Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Wed, 10 Sep 2025 07:54:22 +0900 Subject: [PATCH 03/15] refactor: use maps package for ProviderSpecific operations --- internal/testutils/endpoint.go | 5 +++-- plan/plan.go | 13 ++----------- provider/inmemory/inmemory.go | 5 ++--- source/wrappers/nat64source.go | 9 +++------ 4 files changed, 10 insertions(+), 22 deletions(-) diff --git a/internal/testutils/endpoint.go b/internal/testutils/endpoint.go index d6b3b99b62..74b4fe43a5 100644 --- a/internal/testutils/endpoint.go +++ b/internal/testutils/endpoint.go @@ -18,6 +18,7 @@ package testutils import ( "fmt" + "maps" "math/rand" "net/netip" "reflect" @@ -108,9 +109,9 @@ func SamePlanChanges(a, b map[string][]*endpoint.Endpoint) bool { SameEndpoints(a["UpdateOld"], b["UpdateOld"]) && SameEndpoints(a["UpdateNew"], b["UpdateNew"]) } -// SameProviderSpecific verifies that two maps contain the same string/[]string key/value pairs +// SameProviderSpecific verifies that two maps contain the same key/value pairs func SameProviderSpecific(a, b endpoint.ProviderSpecific) bool { - return reflect.DeepEqual(a, b) + return maps.Equal(a, b) } // NewTargetsFromAddr convert an array of netip.Addr to Targets (array of string) diff --git a/plan/plan.go b/plan/plan.go index 9f4bfd245f..879d88b6db 100644 --- a/plan/plan.go +++ b/plan/plan.go @@ -18,6 +18,7 @@ package plan import ( "fmt" + "maps" "slices" "strings" @@ -298,17 +299,7 @@ func shouldUpdateTTL(desired, current *endpoint.Endpoint) bool { } func (p *Plan) shouldUpdateProviderSpecific(desired, current *endpoint.Endpoint) bool { - if len(desired.ProviderSpecific) != len(current.ProviderSpecific) { - return true - } - - for key, desiredValue := range desired.ProviderSpecific { - if currentValue, exists := current.ProviderSpecific[key]; !exists || currentValue != desiredValue { - return true - } - } - - return false + return !maps.Equal(desired.ProviderSpecific, current.ProviderSpecific) } // filterRecordsForPlan removes records that are not relevant to the planner. diff --git a/provider/inmemory/inmemory.go b/provider/inmemory/inmemory.go index 01e80b4967..142793fac2 100644 --- a/provider/inmemory/inmemory.go +++ b/provider/inmemory/inmemory.go @@ -19,6 +19,7 @@ package inmemory import ( "context" "errors" + "maps" "strings" log "github.com/sirupsen/logrus" @@ -209,9 +210,7 @@ func copyEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint { } if ep.ProviderSpecific != nil { newEp.ProviderSpecific = make(endpoint.ProviderSpecific) - for k, v := range ep.ProviderSpecific { - newEp.ProviderSpecific.Set(k, v) - } + maps.Copy(newEp.ProviderSpecific, ep.ProviderSpecific) } records = append(records, newEp) } diff --git a/source/wrappers/nat64source.go b/source/wrappers/nat64source.go index f36eabb43d..1d8b0ddd68 100644 --- a/source/wrappers/nat64source.go +++ b/source/wrappers/nat64source.go @@ -19,6 +19,7 @@ package wrappers import ( "context" "fmt" + "maps" "net/netip" log "github.com/sirupsen/logrus" @@ -112,16 +113,12 @@ func (s *nat64Source) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, erro if ep.Labels != nil { v4EP.Labels = make(endpoint.Labels, len(ep.Labels)) - for k, v := range ep.Labels { - v4EP.Labels[k] = v - } + maps.Copy(v4EP.Labels, ep.Labels) } if ep.ProviderSpecific != nil { v4EP.ProviderSpecific = make(endpoint.ProviderSpecific, len(ep.ProviderSpecific)) - for k, v := range ep.ProviderSpecific { - v4EP.ProviderSpecific.Set(k, v) - } + maps.Copy(v4EP.ProviderSpecific, ep.ProviderSpecific) } additionalEndpoints = append(additionalEndpoints, v4EP) From a8f89fd77562a0efc418c7c75a630ae7ecf3cdd6 Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Fri, 12 Sep 2025 18:16:13 +0900 Subject: [PATCH 04/15] add omitempty to DNSEndpointSpec.Endpoints --- apis/v1alpha1/dnsendpoint.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apis/v1alpha1/dnsendpoint.go b/apis/v1alpha1/dnsendpoint.go index c580ba5475..eaf6ea5c41 100644 --- a/apis/v1alpha1/dnsendpoint.go +++ b/apis/v1alpha1/dnsendpoint.go @@ -49,7 +49,7 @@ type DNSEndpointList struct { // DNSEndpointSpec defines the desired state of DNSEndpoint type DNSEndpointSpec struct { - Endpoints []*Endpoint `json:"endpoints"` + Endpoints []*Endpoint `json:"endpoints,omitempty"` } // ProviderSpecificProperty holds the name and value of a configuration which is specific to individual DNS providers From bbe0387225f439056935b113d8974d941063b78f Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Fri, 19 Sep 2025 20:52:33 +0900 Subject: [PATCH 05/15] refactor(endpoint): remove ProviderSpecific String() methad and fix tests Signed-off-by: u-kai <76635578+u-kai@users.noreply.github.com> --- endpoint/endpoint.go | 33 ----------------- endpoint/endpoint_test.go | 61 -------------------------------- source/gateway_httproute_test.go | 4 +-- 3 files changed, 2 insertions(+), 96 deletions(-) diff --git a/endpoint/endpoint.go b/endpoint/endpoint.go index 60b7ac0bc2..aaafab62e5 100644 --- a/endpoint/endpoint.go +++ b/endpoint/endpoint.go @@ -222,39 +222,6 @@ func (ps ProviderSpecific) Delete(key string) { delete(ps, key) } -// String returns a stable, backwards-compatible string form. -// For compatibility with older branches where ProviderSpecific was -// []ProviderSpecificProperty, we render: -// - empty or nil as "[]" -// - non-empty as slice-like entries sorted by key, e.g. "[{k1 v1} {k2 v2}]" -// This preserves previous log expectations and keeps output deterministic. -func (ps ProviderSpecific) String() string { - if len(ps) == 0 { - return "[]" - } - // Collect and sort keys for stable output. - keys := make([]string, 0, len(ps)) - for k := range ps { - keys = append(keys, k) - } - sort.Strings(keys) - // Build entries like "{key value}" preserving stable order. - b := strings.Builder{} - b.WriteByte('[') - for i, k := range keys { - if i > 0 { - b.WriteByte(' ') - } - b.WriteByte('{') - b.WriteString(k) - b.WriteByte(' ') - b.WriteString(ps[k]) - b.WriteByte('}') - } - b.WriteByte(']') - return b.String() -} - // EndpointKey is the type of a map key for separating endpoints or targets. type EndpointKey struct { DNSName string diff --git a/endpoint/endpoint_test.go b/endpoint/endpoint_test.go index ebe41d55ae..3b0a38c4f2 100644 --- a/endpoint/endpoint_test.go +++ b/endpoint/endpoint_test.go @@ -973,64 +973,3 @@ func TestEndpoint_WithMinTTL(t *testing.T) { }) } } - -func TestProviderSpecificString(t *testing.T) { - tests := []struct { - name string - ps ProviderSpecific - want string - }{ - {name: "nil", ps: nil, want: "[]"}, - {name: "empty", ps: ProviderSpecific{}, want: "[]"}, - {name: "single", ps: ProviderSpecific{"k": "v"}, want: "[{k v}]"}, - {name: "multi_sorted", ps: ProviderSpecific{"b": "2", "a": "1"}, want: "[{a 1} {b 2}]"}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := tt.ps.String(); got != tt.want { - t.Fatalf("ProviderSpecific.String() = %q, want %q", got, tt.want) - } - }) - } -} - -func TestEndpointString(t *testing.T) { - tests := []struct { - name string - ep *Endpoint - want string - }{ - { - name: "no set id, nil ps", - ep: &Endpoint{DNSName: "x.example.com", RecordType: RecordTypeA, Targets: Targets{"1.2.3.4"}}, - want: "x.example.com 0 IN A 1.2.3.4 []", - }, - { - name: "no set id, empty ps", - ep: &Endpoint{DNSName: "x.example.com", RecordType: RecordTypeA, Targets: Targets{"1.2.3.4"}, ProviderSpecific: ProviderSpecific{}}, - want: "x.example.com 0 IN A 1.2.3.4 []", - }, - { - name: "with set id and ps", - ep: &Endpoint{DNSName: "x.example.com", RecordType: RecordTypeA, SetIdentifier: "id", Targets: Targets{"1.2.3.4"}, ProviderSpecific: ProviderSpecific{"k": "v"}}, - want: "x.example.com 0 IN A id 1.2.3.4 [{k v}]", - }, - { - name: "multiple targets, nil ps", - ep: &Endpoint{DNSName: "x.example.com", RecordType: RecordTypeA, Targets: Targets{"1.2.3.4", "2.3.4.5"}}, - want: "x.example.com 0 IN A 1.2.3.4;2.3.4.5 []", - }, - { - name: "multiple targets, with set id and ps", - ep: &Endpoint{DNSName: "x.example.com", RecordType: RecordTypeA, SetIdentifier: "id", Targets: Targets{"1.2.3.4", "2.3.4.5"}, ProviderSpecific: ProviderSpecific{"a": "1", "b": "2"}}, - want: "x.example.com 0 IN A id 1.2.3.4;2.3.4.5 [{a 1} {b 2}]", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := tt.ep.String(); got != tt.want { - t.Fatalf("Endpoint.String() = %q, want %q", got, tt.want) - } - }) - } -} diff --git a/source/gateway_httproute_test.go b/source/gateway_httproute_test.go index 2edbf89654..10957ce568 100644 --- a/source/gateway_httproute_test.go +++ b/source/gateway_httproute_test.go @@ -1471,8 +1471,8 @@ func TestGatewayHTTPRouteSourceEndpoints(t *testing.T) { newTestEndpoint("test.two.internal", "A", "2.3.4.5"), }, logExpectations: []string{ - "Endpoints generated from HTTPRoute default/one: [test.one.internal 0 IN A 1.2.3.4 []]", - "Endpoints generated from HTTPRoute default/two: [test.two.internal 0 IN A 2.3.4.5 []]", + "Endpoints generated from HTTPRoute default/one: [test.one.internal 0 IN A 1.2.3.4 map[]]", + "Endpoints generated from HTTPRoute default/two: [test.two.internal 0 IN A 2.3.4.5 map[]]", }, }, { From 1301503f85f37cd9fefe49e94221940e75a4b2f1 Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Mon, 22 Sep 2025 20:54:09 +0900 Subject: [PATCH 06/15] refactor(crd): move endpoint conversion logic to ToInternal method Signed-off-by: u-kai <76635578+u-kai@users.noreply.github.com> --- apis/v1alpha1/conversion.go | 44 +++++++++++++++++++++++++++++++++++++ source/crd.go | 19 +--------------- source/crd_test.go | 2 +- 3 files changed, 46 insertions(+), 19 deletions(-) create mode 100644 apis/v1alpha1/conversion.go diff --git a/apis/v1alpha1/conversion.go b/apis/v1alpha1/conversion.go new file mode 100644 index 0000000000..53fc7154a7 --- /dev/null +++ b/apis/v1alpha1/conversion.go @@ -0,0 +1,44 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha1 + +import "sigs.k8s.io/external-dns/endpoint" + +// ToInternal converts the CRD Endpoint to the internal endpoint representation. +// This conversion is necessary to maintain backward compatibility with external APIs +// while allowing internal performance optimizations. The internal endpoint package +// uses a map-based ProviderSpecific structure for O(1) property access, whereas +// the CRD maintains the original slice-based structure to preserve API stability. +func (e *Endpoint) ToInternal() *endpoint.Endpoint { + if e == nil { + return nil + } + p := make(endpoint.ProviderSpecific) + for _, ps := range e.ProviderSpecific { + p.Set(ps.Name, ps.Value) + } + ep := &endpoint.Endpoint{ + DNSName: e.DNSName, + Targets: e.Targets, + RecordType: e.RecordType, + SetIdentifier: e.SetIdentifier, + RecordTTL: endpoint.TTL(e.RecordTTL), + Labels: e.Labels, + ProviderSpecific: p, + } + return ep +} diff --git a/source/crd.go b/source/crd.go index ac47064426..30dde6d45a 100644 --- a/source/crd.go +++ b/source/crd.go @@ -160,23 +160,6 @@ func (cs *crdSource) AddEventHandler(_ context.Context, handler func()) { } } -func fromCRDEndpoint(crdEp *apiv1alpha1.Endpoint) *endpoint.Endpoint { - p := make(endpoint.ProviderSpecific) - for _, ps := range crdEp.ProviderSpecific { - p.Set(ps.Name, ps.Value) - } - ep := &endpoint.Endpoint{ - DNSName: crdEp.DNSName, - Targets: crdEp.Targets, - RecordType: crdEp.RecordType, - SetIdentifier: crdEp.SetIdentifier, - RecordTTL: endpoint.TTL(crdEp.RecordTTL), - Labels: crdEp.Labels, - ProviderSpecific: p, - } - return ep -} - // Endpoints returns endpoint objects. func (cs *crdSource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, error) { endpoints := []*endpoint.Endpoint{} @@ -199,7 +182,7 @@ func (cs *crdSource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, error for _, dnsEndpoint := range result.Items { var crdEndpoints []*endpoint.Endpoint for _, crdEp := range dnsEndpoint.Spec.Endpoints { - ep := fromCRDEndpoint(crdEp) + ep := crdEp.ToInternal() if (ep.RecordType == endpoint.RecordTypeCNAME || ep.RecordType == endpoint.RecordTypeA || ep.RecordType == endpoint.RecordTypeAAAA) && len(ep.Targets) < 1 { log.Debugf("Endpoint %s with DNSName %s has an empty list of targets, allowing it to pass through for default-targets processing", dnsEndpoint.Name, ep.DNSName) } diff --git a/source/crd_test.go b/source/crd_test.go index b189c66577..014d4a468c 100644 --- a/source/crd_test.go +++ b/source/crd_test.go @@ -515,7 +515,7 @@ func testCRDSourceEndpoints(t *testing.T) { // Validate received endpoints against expected endpoints. endpoints := make([]*endpoint.Endpoint, 0, len(ti.endpoints)) for _, ep := range ti.endpoints { - endpoints = append(endpoints, fromCRDEndpoint(ep)) + endpoints = append(endpoints, ep.ToInternal()) } validateEndpoints(t, receivedEndpoints, endpoints) From aeeff6f5caebef5b7bdf3d6f12671433658c850a Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Mon, 22 Sep 2025 20:57:27 +0900 Subject: [PATCH 07/15] fix(webhook): maintain backward compatibility for records conversion Signed-off-by: u-kai <76635578+u-kai@users.noreply.github.com> --- apis/v1alpha1/conversion.go | 44 -------------------- pkg/adapter/endpoint.go | 71 ++++++++++++++++++++++++++++++++ provider/webhook/api/httpapi.go | 3 ++ provider/webhook/webhook.go | 10 +++-- provider/webhook/webhook_test.go | 21 +++++++++- source/crd.go | 3 +- source/crd_test.go | 6 +-- 7 files changed, 104 insertions(+), 54 deletions(-) delete mode 100644 apis/v1alpha1/conversion.go create mode 100644 pkg/adapter/endpoint.go diff --git a/apis/v1alpha1/conversion.go b/apis/v1alpha1/conversion.go deleted file mode 100644 index 53fc7154a7..0000000000 --- a/apis/v1alpha1/conversion.go +++ /dev/null @@ -1,44 +0,0 @@ -/* -Copyright 2017 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package v1alpha1 - -import "sigs.k8s.io/external-dns/endpoint" - -// ToInternal converts the CRD Endpoint to the internal endpoint representation. -// This conversion is necessary to maintain backward compatibility with external APIs -// while allowing internal performance optimizations. The internal endpoint package -// uses a map-based ProviderSpecific structure for O(1) property access, whereas -// the CRD maintains the original slice-based structure to preserve API stability. -func (e *Endpoint) ToInternal() *endpoint.Endpoint { - if e == nil { - return nil - } - p := make(endpoint.ProviderSpecific) - for _, ps := range e.ProviderSpecific { - p.Set(ps.Name, ps.Value) - } - ep := &endpoint.Endpoint{ - DNSName: e.DNSName, - Targets: e.Targets, - RecordType: e.RecordType, - SetIdentifier: e.SetIdentifier, - RecordTTL: endpoint.TTL(e.RecordTTL), - Labels: e.Labels, - ProviderSpecific: p, - } - return ep -} diff --git a/pkg/adapter/endpoint.go b/pkg/adapter/endpoint.go new file mode 100644 index 0000000000..1dc2ba9b7e --- /dev/null +++ b/pkg/adapter/endpoint.go @@ -0,0 +1,71 @@ +package adapter + +import ( + apiv1alpha1 "sigs.k8s.io/external-dns/apis/v1alpha1" + "sigs.k8s.io/external-dns/endpoint" +) + +func ToInternalEndpoint(crdEp *apiv1alpha1.Endpoint) *endpoint.Endpoint { + if crdEp == nil { + return nil + } + p := make(endpoint.ProviderSpecific) + for _, ps := range crdEp.ProviderSpecific { + p.Set(ps.Name, ps.Value) + } + ep := &endpoint.Endpoint{ + DNSName: crdEp.DNSName, + Targets: crdEp.Targets, + RecordType: crdEp.RecordType, + SetIdentifier: crdEp.SetIdentifier, + RecordTTL: endpoint.TTL(crdEp.RecordTTL), + Labels: crdEp.Labels, + ProviderSpecific: p, + } + return ep +} + +func ToInternalEndpoints(crdEps []*apiv1alpha1.Endpoint) []*endpoint.Endpoint { + if len(crdEps) == 0 { + return nil + } + eps := make([]*endpoint.Endpoint, 0, len(crdEps)) + for _, crdEp := range crdEps { + eps = append(eps, ToInternalEndpoint(crdEp)) + } + return eps +} + +func ToAPIEndpoint(ep *endpoint.Endpoint) *apiv1alpha1.Endpoint { + if ep == nil { + return nil + } + ps := make([]apiv1alpha1.ProviderSpecificProperty, 0, len(ep.ProviderSpecific)) + for k, v := range ep.ProviderSpecific { + ps = append(ps, apiv1alpha1.ProviderSpecificProperty{ + Name: k, + Value: v, + }) + } + crdEp := &apiv1alpha1.Endpoint{ + DNSName: ep.DNSName, + Targets: ep.Targets, + RecordType: ep.RecordType, + SetIdentifier: ep.SetIdentifier, + RecordTTL: int64(ep.RecordTTL), + Labels: ep.Labels, + ProviderSpecific: ps, + } + return crdEp +} + +func ToAPIEndpoints(eps []*endpoint.Endpoint) []*apiv1alpha1.Endpoint { + if len(eps) == 0 { + return nil + } + crdEps := make([]*apiv1alpha1.Endpoint, 0, len(eps)) + for _, ep := range eps { + crdEps = append(crdEps, ToAPIEndpoint(ep)) + } + return crdEps +} diff --git a/provider/webhook/api/httpapi.go b/provider/webhook/api/httpapi.go index fde7d3ab87..f55f8da5e8 100644 --- a/provider/webhook/api/httpapi.go +++ b/provider/webhook/api/httpapi.go @@ -53,11 +53,13 @@ func (p *WebhookServer) RecordsHandler(w http.ResponseWriter, req *http.Request) } w.Header().Set(ContentTypeHeader, MediaTypeFormatAndVersion) w.WriteHeader(http.StatusOK) + // TODO: convert if err := json.NewEncoder(w).Encode(records); err != nil { log.Errorf("Failed to encode records: %v", err) } return case http.MethodPost: + // TODO: use another type var changes plan.Changes if err := json.NewDecoder(req.Body).Decode(&changes); err != nil { log.Errorf("Failed to decode changes: %v", err) @@ -85,6 +87,7 @@ func (p *WebhookServer) AdjustEndpointsHandler(w http.ResponseWriter, req *http. return } + // TODO: use another type var pve []*endpoint.Endpoint if err := json.NewDecoder(req.Body).Decode(&pve); err != nil { log.Errorf("Failed to decode in adjustEndpointsHandler: %v", err) diff --git a/provider/webhook/webhook.go b/provider/webhook/webhook.go index 787ace17f6..424c978e14 100644 --- a/provider/webhook/webhook.go +++ b/provider/webhook/webhook.go @@ -24,7 +24,9 @@ import ( "net/http" "net/url" + apiv1alpha1 "sigs.k8s.io/external-dns/apis/v1alpha1" "sigs.k8s.io/external-dns/endpoint" + "sigs.k8s.io/external-dns/pkg/adapter" "sigs.k8s.io/external-dns/pkg/metrics" "sigs.k8s.io/external-dns/plan" "sigs.k8s.io/external-dns/provider" @@ -184,13 +186,14 @@ func (p WebhookProvider) Records(ctx context.Context) ([]*endpoint.Endpoint, err return nil, err } - var endpoints []*endpoint.Endpoint - if err := json.NewDecoder(resp.Body).Decode(&endpoints); err != nil { + var apiEndpoints []*apiv1alpha1.Endpoint + if err := json.NewDecoder(resp.Body).Decode(&apiEndpoints); err != nil { recordsErrorsGauge.Gauge.Inc() log.Debugf("Failed to decode response body: %s", err.Error()) return nil, err } - return endpoints, nil + + return adapter.ToInternalEndpoints(apiEndpoints), nil } // ApplyChanges will make a POST to remoteServerURL/records with the changes @@ -199,6 +202,7 @@ func (p WebhookProvider) ApplyChanges(_ context.Context, changes *plan.Changes) u := p.remoteServerURL.JoinPath(webhookapi.UrlRecords).String() b := new(bytes.Buffer) + // TODO: convert changes if err := json.NewEncoder(b).Encode(changes); err != nil { applyChangesErrorsGauge.Gauge.Inc() log.Debugf("Failed to encode changes: %s", err.Error()) diff --git a/provider/webhook/webhook_test.go b/provider/webhook/webhook_test.go index f9697c6109..8c4af99e09 100644 --- a/provider/webhook/webhook_test.go +++ b/provider/webhook/webhook_test.go @@ -127,7 +127,12 @@ func TestRecords(t *testing.T) { } assert.Equal(t, "/records", r.URL.Path) w.Write([]byte(`[{ - "dnsName" : "test.example.com" + "dnsName" : "test.example.com", + "recordType" : "A", + "targets" : ["1.1.1.1"], + "recordTTL" : 300, + "labels" : {"owner": "external-dns"}, + "providerSpecific" : [{"name": "prop1", "value": "value1"}] }]`)) })) defer svr.Close() @@ -139,6 +144,17 @@ func TestRecords(t *testing.T) { require.NotNil(t, endpoints) require.Equal(t, []*endpoint.Endpoint{{ DNSName: "test.example.com", + Targets: endpoint.Targets{ + "1.1.1.1", + }, + RecordType: "A", + RecordTTL: 300, + Labels: map[string]string{ + "owner": "external-dns", + }, + ProviderSpecific: endpoint.ProviderSpecific{ + "prop1": "value1", + }, }}, endpoints) } @@ -410,10 +426,11 @@ func TestApplyChangesWithProviderSpecificProperty(t *testing.T) { DNSName: "test.example.com", RecordTTL: 10, RecordType: "A", - Targets: endpoint.Targets{ + Targets: []string{ "", }, ProviderSpecific: endpoint.ProviderSpecific{ + //{Name: "prop1", Value: "value1"}, "prop1": "value1", }, } diff --git a/source/crd.go b/source/crd.go index 30dde6d45a..a8b0318780 100644 --- a/source/crd.go +++ b/source/crd.go @@ -26,6 +26,7 @@ import ( "k8s.io/apimachinery/pkg/watch" "k8s.io/client-go/tools/cache" + "sigs.k8s.io/external-dns/pkg/adapter" "sigs.k8s.io/external-dns/source/annotations" log "github.com/sirupsen/logrus" @@ -182,7 +183,7 @@ func (cs *crdSource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, error for _, dnsEndpoint := range result.Items { var crdEndpoints []*endpoint.Endpoint for _, crdEp := range dnsEndpoint.Spec.Endpoints { - ep := crdEp.ToInternal() + ep := adapter.ToInternalEndpoint(crdEp) if (ep.RecordType == endpoint.RecordTypeCNAME || ep.RecordType == endpoint.RecordTypeA || ep.RecordType == endpoint.RecordTypeAAAA) && len(ep.Targets) < 1 { log.Debugf("Endpoint %s with DNSName %s has an empty list of targets, allowing it to pass through for default-targets processing", dnsEndpoint.Name, ep.DNSName) } diff --git a/source/crd_test.go b/source/crd_test.go index 014d4a468c..ec84bed812 100644 --- a/source/crd_test.go +++ b/source/crd_test.go @@ -43,6 +43,7 @@ import ( cachetesting "k8s.io/client-go/tools/cache/testing" apiv1alpha1 "sigs.k8s.io/external-dns/apis/v1alpha1" "sigs.k8s.io/external-dns/endpoint" + "sigs.k8s.io/external-dns/pkg/adapter" ) type CRDSuite struct { @@ -513,10 +514,7 @@ func testCRDSourceEndpoints(t *testing.T) { } // Validate received endpoints against expected endpoints. - endpoints := make([]*endpoint.Endpoint, 0, len(ti.endpoints)) - for _, ep := range ti.endpoints { - endpoints = append(endpoints, ep.ToInternal()) - } + endpoints := adapter.ToInternalEndpoints(ti.endpoints) validateEndpoints(t, receivedEndpoints, endpoints) for _, e := range receivedEndpoints { From 46bc513aca5839093bfacb17c53aaf39e3dc517d Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Mon, 22 Sep 2025 21:31:19 +0900 Subject: [PATCH 08/15] fix(webhook): maintain backward compatibility for adjustEndpoints conversion Signed-off-by: u-kai <76635578+u-kai@users.noreply.github.com> --- provider/webhook/api/httpapi.go | 10 ++++++---- provider/webhook/webhook.go | 10 +++++----- provider/webhook/webhook_test.go | 17 ++++++++++++++--- 3 files changed, 25 insertions(+), 12 deletions(-) diff --git a/provider/webhook/api/httpapi.go b/provider/webhook/api/httpapi.go index f55f8da5e8..fec1dffbf8 100644 --- a/provider/webhook/api/httpapi.go +++ b/provider/webhook/api/httpapi.go @@ -23,7 +23,8 @@ import ( "net/http" "time" - "sigs.k8s.io/external-dns/endpoint" + apiv1alpha1 "sigs.k8s.io/external-dns/apis/v1alpha1" + "sigs.k8s.io/external-dns/pkg/adapter" "sigs.k8s.io/external-dns/plan" "sigs.k8s.io/external-dns/provider" @@ -87,19 +88,20 @@ func (p *WebhookServer) AdjustEndpointsHandler(w http.ResponseWriter, req *http. return } - // TODO: use another type - var pve []*endpoint.Endpoint + var pve []*apiv1alpha1.Endpoint if err := json.NewDecoder(req.Body).Decode(&pve); err != nil { log.Errorf("Failed to decode in adjustEndpointsHandler: %v", err) w.WriteHeader(http.StatusBadRequest) return } w.Header().Set(ContentTypeHeader, MediaTypeFormatAndVersion) - pve, err := p.Provider.AdjustEndpoints(pve) + endpoints := adapter.ToInternalEndpoints(pve) + endpoints, err := p.Provider.AdjustEndpoints(endpoints) if err != nil { log.Errorf("Failed to call adjust endpoints: %v", err) w.WriteHeader(http.StatusInternalServerError) } + pve = adapter.ToAPIEndpoints(endpoints) if err := json.NewEncoder(w).Encode(&pve); err != nil { log.Errorf("Failed to encode in adjustEndpointsHandler: %v", err) w.WriteHeader(http.StatusInternalServerError) diff --git a/provider/webhook/webhook.go b/provider/webhook/webhook.go index 424c978e14..c2c0efdd48 100644 --- a/provider/webhook/webhook.go +++ b/provider/webhook/webhook.go @@ -244,7 +244,6 @@ func (p WebhookProvider) ApplyChanges(_ context.Context, changes *plan.Changes) // This method returns an empty slice in case there is a technical error on the provider's side so that no endpoints will be considered. func (p WebhookProvider) AdjustEndpoints(e []*endpoint.Endpoint) ([]*endpoint.Endpoint, error) { adjustEndpointsRequestsGauge.Gauge.Inc() - var endpoints []*endpoint.Endpoint u, err := url.JoinPath(p.remoteServerURL.String(), webhookapi.UrlAdjustEndpoints) if err != nil { adjustEndpointsErrorsGauge.Gauge.Inc() @@ -253,7 +252,8 @@ func (p WebhookProvider) AdjustEndpoints(e []*endpoint.Endpoint) ([]*endpoint.En } b := new(bytes.Buffer) - if err := json.NewEncoder(b).Encode(e); err != nil { + apiEps := adapter.ToAPIEndpoints(e) + if err := json.NewEncoder(b).Encode(apiEps); err != nil { adjustEndpointsErrorsGauge.Gauge.Inc() log.Debugf("Failed to encode endpoints, %s", err) return nil, err @@ -286,14 +286,14 @@ func (p WebhookProvider) AdjustEndpoints(e []*endpoint.Endpoint) ([]*endpoint.En } return nil, err } - - if err := json.NewDecoder(resp.Body).Decode(&endpoints); err != nil { + apiEps = []*apiv1alpha1.Endpoint{} + if err := json.NewDecoder(resp.Body).Decode(&apiEps); err != nil { adjustEndpointsErrorsGauge.Gauge.Inc() log.Debugf("Failed to decode response body: %s", err.Error()) return nil, err } - return endpoints, nil + return adapter.ToInternalEndpoints(apiEps), nil } // GetDomainFilter make calls to get the serialized version of the domain filter diff --git a/provider/webhook/webhook_test.go b/provider/webhook/webhook_test.go index 8c4af99e09..dd96db4e7a 100644 --- a/provider/webhook/webhook_test.go +++ b/provider/webhook/webhook_test.go @@ -28,7 +28,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - + apiv1alpha1 "sigs.k8s.io/external-dns/apis/v1alpha1" "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/plan" "sigs.k8s.io/external-dns/provider" @@ -320,7 +320,7 @@ func TestAdjustEndpoints(t *testing.T) { } assert.Equal(t, webhookapi.UrlAdjustEndpoints, r.URL.Path) - var endpoints []*endpoint.Endpoint + var endpoints []*apiv1alpha1.Endpoint defer r.Body.Close() b, err := io.ReadAll(r.Body) if err != nil { @@ -349,6 +349,12 @@ func TestAdjustEndpoints(t *testing.T) { Targets: endpoint.Targets{ "", }, + Labels: map[string]string{ + "owner": "external-dns", + }, + ProviderSpecific: endpoint.ProviderSpecific{ + "prop1": "value1", + }, }, } adjustedEndpoints, err := provider.AdjustEndpoints(endpoints) @@ -360,6 +366,12 @@ func TestAdjustEndpoints(t *testing.T) { Targets: endpoint.Targets{ "", }, + Labels: map[string]string{ + "owner": "external-dns", + }, + ProviderSpecific: endpoint.ProviderSpecific{ + "prop1": "value1", + }, }}, adjustedEndpoints) } @@ -430,7 +442,6 @@ func TestApplyChangesWithProviderSpecificProperty(t *testing.T) { "", }, ProviderSpecific: endpoint.ProviderSpecific{ - //{Name: "prop1", Value: "value1"}, "prop1": "value1", }, } From 3cbefbfd9354b61d8dbab057085e19d8f6579b4b Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Mon, 22 Sep 2025 22:16:05 +0900 Subject: [PATCH 09/15] fix(webhook): maintain backward compatibility for ApplyChanges conversion Signed-off-by: u-kai <76635578+u-kai@users.noreply.github.com> --- provider/webhook/webhook.go | 19 +++++++- provider/webhook/webhook_test.go | 76 +++++++++++++++++++++++++++----- 2 files changed, 82 insertions(+), 13 deletions(-) diff --git a/provider/webhook/webhook.go b/provider/webhook/webhook.go index c2c0efdd48..f6c16b4569 100644 --- a/provider/webhook/webhook.go +++ b/provider/webhook/webhook.go @@ -196,14 +196,29 @@ func (p WebhookProvider) Records(ctx context.Context) ([]*endpoint.Endpoint, err return adapter.ToInternalEndpoints(apiEndpoints), nil } +type webhookPlanChanges struct { + Create []*apiv1alpha1.Endpoint `json:"create,omitempty"` + Delete []*apiv1alpha1.Endpoint `json:"delete,omitempty"` + UpdateOld []*apiv1alpha1.Endpoint `json:"updateOld,omitempty"` + UpdateNew []*apiv1alpha1.Endpoint `json:"updateNew,omitempty"` +} + // ApplyChanges will make a POST to remoteServerURL/records with the changes func (p WebhookProvider) ApplyChanges(_ context.Context, changes *plan.Changes) error { applyChangesRequestsGauge.Gauge.Inc() u := p.remoteServerURL.JoinPath(webhookapi.UrlRecords).String() b := new(bytes.Buffer) - // TODO: convert changes - if err := json.NewEncoder(b).Encode(changes); err != nil { + var webhookChanges *webhookPlanChanges + if changes != nil { + webhookChanges = &webhookPlanChanges{ + Create: adapter.ToAPIEndpoints(changes.Create), + Delete: adapter.ToAPIEndpoints(changes.Delete), + UpdateOld: adapter.ToAPIEndpoints(changes.UpdateOld), + UpdateNew: adapter.ToAPIEndpoints(changes.UpdateNew), + } + } + if err := json.NewEncoder(b).Encode(webhookChanges); err != nil { applyChangesErrorsGauge.Gauge.Inc() log.Debugf("Failed to encode changes: %s", err.Error()) return err diff --git a/provider/webhook/webhook_test.go b/provider/webhook/webhook_test.go index dd96db4e7a..2b23867092 100644 --- a/provider/webhook/webhook_test.go +++ b/provider/webhook/webhook_test.go @@ -240,29 +240,84 @@ func TestRecords_NonOKStatusCode(t *testing.T) { } func TestApplyChanges(t *testing.T) { - successfulApplyChanges := true svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path == "/" { w.Header().Set(webhookapi.ContentTypeHeader, webhookapi.MediaTypeFormatAndVersion) w.Write([]byte(`{}`)) return } + assert.Equal(t, "/records", r.URL.Path) - if successfulApplyChanges { - w.WriteHeader(http.StatusNoContent) - } else { - w.WriteHeader(http.StatusInternalServerError) - } + defer r.Body.Close() + body, err := io.ReadAll(r.Body) + require.NoError(t, err) + require.JSONEq(t, `{ + "create": [ + { + "dnsName": "foo", + "providerSpecific": [{ "name": "prop1", "value": "value1" }], + "labels": { "owner": "external-dns" }, + "recordTTL": 10 + } + ], + "updateOld": [{ "dnsName": "bar" }], + "updateNew": [{ "dnsName": "baz" }], + "delete": [{ "dnsName": "qux" }] + }`, string(body), + ) + w.WriteHeader(http.StatusNoContent) })) defer svr.Close() p, err := NewWebhookProvider(svr.URL) require.NoError(t, err) - err = p.ApplyChanges(context.TODO(), nil) + err = p.ApplyChanges(context.TODO(), &plan.Changes{ + Create: []*endpoint.Endpoint{ + { + DNSName: "foo", + RecordTTL: 10, + Labels: map[string]string{ + "owner": "external-dns", + }, + ProviderSpecific: endpoint.ProviderSpecific{ + "prop1": "value1", + }, + }, + }, + UpdateOld: []*endpoint.Endpoint{ + { + DNSName: "bar", + }, + }, + UpdateNew: []*endpoint.Endpoint{ + { + DNSName: "baz", + }, + }, + Delete: []*endpoint.Endpoint{ + { + DNSName: "qux", + }, + }, + }) require.NoError(t, err) +} - successfulApplyChanges = false +func TestApplyChanges_InternalServerError(t *testing.T) { + svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/" { + w.Header().Set(webhookapi.ContentTypeHeader, webhookapi.MediaTypeFormatAndVersion) + w.Write([]byte(`{}`)) + return + } + assert.Equal(t, "/records", r.URL.Path) + w.WriteHeader(http.StatusInternalServerError) + })) + defer svr.Close() + + p, err := NewWebhookProvider(svr.URL) + require.NoError(t, err) err = p.ApplyChanges(context.TODO(), nil) require.Error(t, err) require.ErrorIs(t, err, provider.SoftError) @@ -415,7 +470,7 @@ func TestApplyChangesWithProviderSpecificProperty(t *testing.T) { if r.URL.Path == "/records" { w.Header().Set(webhookapi.ContentTypeHeader, webhookapi.MediaTypeFormatAndVersion) // assert that the request contains the provider-specific property - var changes plan.Changes + var changes webhookPlanChanges defer r.Body.Close() b, err := io.ReadAll(r.Body) assert.NoError(t, err) @@ -423,8 +478,7 @@ func TestApplyChangesWithProviderSpecificProperty(t *testing.T) { assert.NoError(t, err) assert.Len(t, changes.Create, 1) assert.Len(t, changes.Create[0].ProviderSpecific, 1) - v, ok := changes.Create[0].ProviderSpecific.Get("prop1") - assert.True(t, ok) + v := changes.Create[0].ProviderSpecific[0].Value assert.Equal(t, "value1", v) w.WriteHeader(http.StatusNoContent) return From 63dd99db786bc808a9be7b7da68bcec52d2f39fd Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Tue, 23 Sep 2025 18:05:18 +0900 Subject: [PATCH 10/15] fix(webhook): improve type conversion and maintain backward compatibility Signed-off-by: u-kai <76635578+u-kai@users.noreply.github.com> --- provider/webhook/api/httpapi.go | 22 +++++++--- provider/webhook/api/httpapi_test.go | 65 +++++++++++++++++++++++----- provider/webhook/webhook.go | 11 +---- provider/webhook/webhook_test.go | 2 +- 4 files changed, 75 insertions(+), 25 deletions(-) diff --git a/provider/webhook/api/httpapi.go b/provider/webhook/api/httpapi.go index fec1dffbf8..8f12ae686f 100644 --- a/provider/webhook/api/httpapi.go +++ b/provider/webhook/api/httpapi.go @@ -39,6 +39,13 @@ const ( UrlRecords = "/records" ) +type Changes struct { + Create []*apiv1alpha1.Endpoint `json:"create,omitempty"` + Delete []*apiv1alpha1.Endpoint `json:"delete,omitempty"` + UpdateOld []*apiv1alpha1.Endpoint `json:"updateOld,omitempty"` + UpdateNew []*apiv1alpha1.Endpoint `json:"updateNew,omitempty"` +} + type WebhookServer struct { Provider provider.Provider } @@ -54,20 +61,25 @@ func (p *WebhookServer) RecordsHandler(w http.ResponseWriter, req *http.Request) } w.Header().Set(ContentTypeHeader, MediaTypeFormatAndVersion) w.WriteHeader(http.StatusOK) - // TODO: convert - if err := json.NewEncoder(w).Encode(records); err != nil { + apiRecords := adapter.ToAPIEndpoints(records) + if err := json.NewEncoder(w).Encode(apiRecords); err != nil { log.Errorf("Failed to encode records: %v", err) } return case http.MethodPost: - // TODO: use another type - var changes plan.Changes + var changes Changes if err := json.NewDecoder(req.Body).Decode(&changes); err != nil { log.Errorf("Failed to decode changes: %v", err) w.WriteHeader(http.StatusBadRequest) return } - err := p.Provider.ApplyChanges(context.Background(), &changes) + internalChanges := &plan.Changes{ + Create: adapter.ToInternalEndpoints(changes.Create), + Delete: adapter.ToInternalEndpoints(changes.Delete), + UpdateOld: adapter.ToInternalEndpoints(changes.UpdateOld), + UpdateNew: adapter.ToInternalEndpoints(changes.UpdateNew), + } + err := p.Provider.ApplyChanges(context.Background(), internalChanges) if err != nil { log.Errorf("Failed to apply changes: %v", err) w.WriteHeader(http.StatusInternalServerError) diff --git a/provider/webhook/api/httpapi_test.go b/provider/webhook/api/httpapi_test.go index 795e59b524..f0c073491b 100644 --- a/provider/webhook/api/httpapi_test.go +++ b/provider/webhook/api/httpapi_test.go @@ -31,7 +31,9 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + apiv1alpha1 "sigs.k8s.io/external-dns/apis/v1alpha1" "sigs.k8s.io/external-dns/endpoint" + "sigs.k8s.io/external-dns/pkg/adapter" "sigs.k8s.io/external-dns/plan" ) @@ -78,6 +80,13 @@ func TestMain(m *testing.M) { { DNSName: "foo.bar.com", RecordType: "A", + ProviderSpecific: endpoint.ProviderSpecific{ + "prop1": "value", + }, + Labels: map[string]string{ + "label1": "value1", + }, + RecordTTL: 10, }, } m.Run() @@ -98,11 +107,11 @@ func TestRecordsHandlerRecords(t *testing.T) { // require that the res has the same endpoints as the records slice defer res.Body.Close() require.NotNil(t, res.Body) - var endpoints []*endpoint.Endpoint + var endpoints []*apiv1alpha1.Endpoint if err := json.NewDecoder(res.Body).Decode(&endpoints); err != nil { t.Errorf("Failed to decode response body: %s", err.Error()) } - require.Equal(t, records, endpoints) + require.Equal(t, adapter.ToAPIEndpoints(records), endpoints) } func TestRecordsHandlerRecordsWithErrors(t *testing.T) { @@ -132,12 +141,22 @@ func TestRecordsHandlerApplyChangesWithBadRequest(t *testing.T) { } func TestRecordsHandlerApplyChangesWithValidRequest(t *testing.T) { - changes := &plan.Changes{ - Create: []*endpoint.Endpoint{ + changes := &Changes{ + Create: []*apiv1alpha1.Endpoint{ { DNSName: "foo.bar.com", RecordType: "A", Targets: endpoint.Targets{}, + RecordTTL: 10, + ProviderSpecific: []apiv1alpha1.ProviderSpecificProperty{ + { + Name: "prop1", + Value: "value", + }, + }, + Labels: map[string]string{ + "label1": "value1", + }, }, }, } @@ -150,7 +169,25 @@ func TestRecordsHandlerApplyChangesWithValidRequest(t *testing.T) { w := httptest.NewRecorder() providerAPIServer := &WebhookServer{ - Provider: &FakeWebhookProvider{}, + Provider: &FakeWebhookProvider{ + assertChanges: func(changes *plan.Changes) { + t.Helper() + require.Equal(t, []*endpoint.Endpoint{ + { + DNSName: "foo.bar.com", + RecordType: "A", + Targets: nil, + RecordTTL: 10, + ProviderSpecific: endpoint.ProviderSpecific{ + "prop1": "value", + }, + Labels: map[string]string{ + "label1": "value1", + }, + }, + }, changes.Create) + }, + }, } providerAPIServer.RecordsHandler(w, req) res := w.Result() @@ -158,8 +195,8 @@ func TestRecordsHandlerApplyChangesWithValidRequest(t *testing.T) { } func TestRecordsHandlerApplyChangesWithErrors(t *testing.T) { - changes := &plan.Changes{ - Create: []*endpoint.Endpoint{ + changes := &Changes{ + Create: []*apiv1alpha1.Endpoint{ { DNSName: "foo.bar.com", RecordType: "A", @@ -198,7 +235,7 @@ func TestRecordsHandlerWithWrongHTTPMethod(t *testing.T) { } func TestRecordsHandlerWithMixedCase(t *testing.T) { - input := `{"Create":[{"dnsName":"foo"}],"updateOld":[{"dnsName":"bar"}],"updateNew":[{"dnsName":"baz"}],"Delete":[{"dnsName":"qux"}]}` + input := `{"Create":[{"dnsName":"foo","providerSpecific":[{"name":"prop1","value":"value"}]}],"updateOld":[{"dnsName":"bar"}],"updateNew":[{"dnsName":"baz"}],"Delete":[{"dnsName":"qux","labels":{"label1":"value1"}}]}` req := httptest.NewRequest(http.MethodPost, UrlRecords, strings.NewReader(input)) w := httptest.NewRecorder() @@ -211,16 +248,24 @@ func TestRecordsHandlerWithMixedCase(t *testing.T) { require.Equal(t, []*endpoint.Endpoint{ { DNSName: "foo", + ProviderSpecific: endpoint.ProviderSpecific{ + "prop1": "value", + }, }, }, changes.Create) require.Equal(t, []*endpoint.Endpoint{ { - DNSName: "bar", + DNSName: "bar", + ProviderSpecific: endpoint.ProviderSpecific{}, }, }, changes.UpdateOld) require.Equal(t, []*endpoint.Endpoint{ { - DNSName: "qux", + DNSName: "qux", + ProviderSpecific: endpoint.ProviderSpecific{}, + Labels: map[string]string{ + "label1": "value1", + }, }, }, changes.Delete) }, diff --git a/provider/webhook/webhook.go b/provider/webhook/webhook.go index f6c16b4569..d32228c9bb 100644 --- a/provider/webhook/webhook.go +++ b/provider/webhook/webhook.go @@ -196,22 +196,15 @@ func (p WebhookProvider) Records(ctx context.Context) ([]*endpoint.Endpoint, err return adapter.ToInternalEndpoints(apiEndpoints), nil } -type webhookPlanChanges struct { - Create []*apiv1alpha1.Endpoint `json:"create,omitempty"` - Delete []*apiv1alpha1.Endpoint `json:"delete,omitempty"` - UpdateOld []*apiv1alpha1.Endpoint `json:"updateOld,omitempty"` - UpdateNew []*apiv1alpha1.Endpoint `json:"updateNew,omitempty"` -} - // ApplyChanges will make a POST to remoteServerURL/records with the changes func (p WebhookProvider) ApplyChanges(_ context.Context, changes *plan.Changes) error { applyChangesRequestsGauge.Gauge.Inc() u := p.remoteServerURL.JoinPath(webhookapi.UrlRecords).String() b := new(bytes.Buffer) - var webhookChanges *webhookPlanChanges + var webhookChanges *webhookapi.Changes if changes != nil { - webhookChanges = &webhookPlanChanges{ + webhookChanges = &webhookapi.Changes{ Create: adapter.ToAPIEndpoints(changes.Create), Delete: adapter.ToAPIEndpoints(changes.Delete), UpdateOld: adapter.ToAPIEndpoints(changes.UpdateOld), diff --git a/provider/webhook/webhook_test.go b/provider/webhook/webhook_test.go index 2b23867092..aafab1c4b1 100644 --- a/provider/webhook/webhook_test.go +++ b/provider/webhook/webhook_test.go @@ -470,7 +470,7 @@ func TestApplyChangesWithProviderSpecificProperty(t *testing.T) { if r.URL.Path == "/records" { w.Header().Set(webhookapi.ContentTypeHeader, webhookapi.MediaTypeFormatAndVersion) // assert that the request contains the provider-specific property - var changes webhookPlanChanges + var changes webhookapi.Changes defer r.Body.Close() b, err := io.ReadAll(r.Body) assert.NoError(t, err) From 8d70d35194d9053f95c42f742ab2e44aca0ff6cb Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Tue, 23 Sep 2025 18:07:00 +0900 Subject: [PATCH 11/15] fix lint Signed-off-by: u-kai <76635578+u-kai@users.noreply.github.com> --- pkg/adapter/endpoint.go | 15 +++++++++++++++ provider/webhook/webhook_test.go | 4 ++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/pkg/adapter/endpoint.go b/pkg/adapter/endpoint.go index 1dc2ba9b7e..98b3121bb1 100644 --- a/pkg/adapter/endpoint.go +++ b/pkg/adapter/endpoint.go @@ -1,3 +1,18 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ package adapter import ( diff --git a/provider/webhook/webhook_test.go b/provider/webhook/webhook_test.go index aafab1c4b1..ff8494f887 100644 --- a/provider/webhook/webhook_test.go +++ b/provider/webhook/webhook_test.go @@ -250,8 +250,8 @@ func TestApplyChanges(t *testing.T) { assert.Equal(t, "/records", r.URL.Path) defer r.Body.Close() body, err := io.ReadAll(r.Body) - require.NoError(t, err) - require.JSONEq(t, `{ + assert.NoError(t, err) + assert.JSONEq(t, `{ "create": [ { "dnsName": "foo", From 061b7ca17faa4a97ddbdbe245513e30564f46987 Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Tue, 23 Sep 2025 18:23:51 +0900 Subject: [PATCH 12/15] add endpoint adapter tests Signed-off-by: u-kai <76635578+u-kai@users.noreply.github.com> --- pkg/adapter/endpoint_test.go | 519 +++++++++++++++++++++++++++++++++++ 1 file changed, 519 insertions(+) create mode 100644 pkg/adapter/endpoint_test.go diff --git a/pkg/adapter/endpoint_test.go b/pkg/adapter/endpoint_test.go new file mode 100644 index 0000000000..a983a23946 --- /dev/null +++ b/pkg/adapter/endpoint_test.go @@ -0,0 +1,519 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package adapter + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + apiv1alpha1 "sigs.k8s.io/external-dns/apis/v1alpha1" + "sigs.k8s.io/external-dns/endpoint" + "sigs.k8s.io/external-dns/internal/testutils" +) + +func TestToInternalEndpoint(t *testing.T) { + tests := []struct { + name string + input *apiv1alpha1.Endpoint + expected *endpoint.Endpoint + }{ + { + name: "nil input", + input: nil, + expected: nil, + }, + { + name: "basic endpoint conversion", + input: &apiv1alpha1.Endpoint{ + DNSName: "test.example.com", + Targets: []string{"1.2.3.4"}, + RecordType: "A", + RecordTTL: 300, + }, + expected: &endpoint.Endpoint{ + DNSName: "test.example.com", + Targets: []string{"1.2.3.4"}, + RecordType: "A", + RecordTTL: 300, + ProviderSpecific: endpoint.ProviderSpecific{}, + }, + }, + { + name: "endpoint with provider specific properties", + input: &apiv1alpha1.Endpoint{ + DNSName: "test.example.com", + Targets: []string{"1.2.3.4"}, + RecordType: "A", + RecordTTL: 300, + ProviderSpecific: []apiv1alpha1.ProviderSpecificProperty{ + {Name: "aws-alias", Value: "true"}, + {Name: "aws-zone-id", Value: "Z12345"}, + }, + }, + expected: &endpoint.Endpoint{ + DNSName: "test.example.com", + Targets: []string{"1.2.3.4"}, + RecordType: "A", + RecordTTL: 300, + ProviderSpecific: endpoint.ProviderSpecific{ + "aws-alias": "true", + "aws-zone-id": "Z12345", + }, + }, + }, + { + name: "endpoint with labels and set identifier", + input: &apiv1alpha1.Endpoint{ + DNSName: "test.example.com", + Targets: []string{"1.2.3.4"}, + RecordType: "A", + RecordTTL: 300, + SetIdentifier: "us-east-1", + Labels: map[string]string{ + "owner": "external-dns", + "resource": "ingress/default/test", + "namespace": "default", + }, + }, + expected: &endpoint.Endpoint{ + DNSName: "test.example.com", + Targets: []string{"1.2.3.4"}, + RecordType: "A", + RecordTTL: 300, + SetIdentifier: "us-east-1", + Labels: map[string]string{ + "owner": "external-dns", + "resource": "ingress/default/test", + "namespace": "default", + }, + ProviderSpecific: endpoint.ProviderSpecific{}, + }, + }, + { + name: "complete endpoint with all fields", + input: &apiv1alpha1.Endpoint{ + DNSName: "api.example.com", + Targets: []string{"10.0.0.1", "10.0.0.2"}, + RecordType: "A", + RecordTTL: 600, + SetIdentifier: "primary", + Labels: map[string]string{ + "owner": "external-dns", + "type": "public", + }, + ProviderSpecific: []apiv1alpha1.ProviderSpecificProperty{ + {Name: "weight", Value: "100"}, + {Name: "policy", Value: "weighted"}, + }, + }, + expected: &endpoint.Endpoint{ + DNSName: "api.example.com", + Targets: []string{"10.0.0.1", "10.0.0.2"}, + RecordType: "A", + RecordTTL: 600, + SetIdentifier: "primary", + Labels: map[string]string{ + "owner": "external-dns", + "type": "public", + }, + ProviderSpecific: endpoint.ProviderSpecific{ + "weight": "100", + "policy": "weighted", + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := ToInternalEndpoint(tt.input) + if tt.expected == nil { + assert.Nil(t, result) + } else { + assert.True(t, testutils.SameEndpoint(tt.expected, result)) + } + }) + } +} + +func TestToAPIEndpoint(t *testing.T) { + tests := []struct { + name string + input *endpoint.Endpoint + expected *apiv1alpha1.Endpoint + }{ + { + name: "nil input", + input: nil, + expected: nil, + }, + { + name: "basic endpoint conversion", + input: &endpoint.Endpoint{ + DNSName: "test.example.com", + Targets: []string{"1.2.3.4"}, + RecordType: "A", + RecordTTL: 300, + ProviderSpecific: endpoint.ProviderSpecific{}, + }, + expected: &apiv1alpha1.Endpoint{ + DNSName: "test.example.com", + Targets: []string{"1.2.3.4"}, + RecordType: "A", + RecordTTL: 300, + ProviderSpecific: []apiv1alpha1.ProviderSpecificProperty{}, + }, + }, + { + name: "endpoint with provider specific properties", + input: &endpoint.Endpoint{ + DNSName: "test.example.com", + Targets: []string{"1.2.3.4"}, + RecordType: "A", + RecordTTL: 300, + ProviderSpecific: endpoint.ProviderSpecific{ + "aws-alias": "true", + "aws-zone-id": "Z12345", + }, + }, + expected: &apiv1alpha1.Endpoint{ + DNSName: "test.example.com", + Targets: []string{"1.2.3.4"}, + RecordType: "A", + RecordTTL: 300, + ProviderSpecific: []apiv1alpha1.ProviderSpecificProperty{ + {Name: "aws-alias", Value: "true"}, + {Name: "aws-zone-id", Value: "Z12345"}, + }, + }, + }, + { + name: "endpoint with labels and set identifier", + input: &endpoint.Endpoint{ + DNSName: "test.example.com", + Targets: []string{"1.2.3.4"}, + RecordType: "A", + RecordTTL: 300, + SetIdentifier: "us-east-1", + Labels: map[string]string{ + "owner": "external-dns", + "resource": "ingress/default/test", + "namespace": "default", + }, + ProviderSpecific: endpoint.ProviderSpecific{}, + }, + expected: &apiv1alpha1.Endpoint{ + DNSName: "test.example.com", + Targets: []string{"1.2.3.4"}, + RecordType: "A", + RecordTTL: 300, + SetIdentifier: "us-east-1", + Labels: map[string]string{ + "owner": "external-dns", + "resource": "ingress/default/test", + "namespace": "default", + }, + ProviderSpecific: []apiv1alpha1.ProviderSpecificProperty{}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := ToAPIEndpoint(tt.input) + if tt.expected == nil { + assert.Nil(t, result) + } else { + assert.Equal(t, tt.expected.DNSName, result.DNSName) + assert.Equal(t, tt.expected.Targets, result.Targets) + assert.Equal(t, tt.expected.RecordType, result.RecordType) + assert.Equal(t, tt.expected.RecordTTL, result.RecordTTL) + assert.Equal(t, tt.expected.SetIdentifier, result.SetIdentifier) + assert.Equal(t, tt.expected.Labels, result.Labels) + + // Convert ProviderSpecific slice to map for comparison since order doesn't matter + expectedPS := make(map[string]string) + for _, ps := range tt.expected.ProviderSpecific { + expectedPS[ps.Name] = ps.Value + } + resultPS := make(map[string]string) + for _, ps := range result.ProviderSpecific { + resultPS[ps.Name] = ps.Value + } + assert.Equal(t, expectedPS, resultPS) + } + }) + } +} + +func TestToInternalEndpoints(t *testing.T) { + tests := []struct { + name string + input []*apiv1alpha1.Endpoint + expected []*endpoint.Endpoint + }{ + { + name: "empty slice", + input: []*apiv1alpha1.Endpoint{}, + expected: nil, + }, + { + name: "nil slice", + input: nil, + expected: nil, + }, + { + name: "single endpoint", + input: []*apiv1alpha1.Endpoint{ + { + DNSName: "test.example.com", + Targets: []string{"1.2.3.4"}, + RecordType: "A", + RecordTTL: 300, + }, + }, + expected: []*endpoint.Endpoint{ + { + DNSName: "test.example.com", + Targets: []string{"1.2.3.4"}, + RecordType: "A", + RecordTTL: 300, + ProviderSpecific: endpoint.ProviderSpecific{}, + }, + }, + }, + { + name: "multiple endpoints", + input: []*apiv1alpha1.Endpoint{ + { + DNSName: "api.example.com", + Targets: []string{"1.2.3.4"}, + RecordType: "A", + RecordTTL: 300, + ProviderSpecific: []apiv1alpha1.ProviderSpecificProperty{ + {Name: "type", Value: "api"}, + }, + }, + { + DNSName: "www.example.com", + Targets: []string{"example.com"}, + RecordType: "CNAME", + RecordTTL: 600, + Labels: map[string]string{ + "owner": "external-dns", + }, + }, + }, + expected: []*endpoint.Endpoint{ + { + DNSName: "api.example.com", + Targets: []string{"1.2.3.4"}, + RecordType: "A", + RecordTTL: 300, + ProviderSpecific: endpoint.ProviderSpecific{ + "type": "api", + }, + }, + { + DNSName: "www.example.com", + Targets: []string{"example.com"}, + RecordType: "CNAME", + RecordTTL: 600, + Labels: map[string]string{ + "owner": "external-dns", + }, + ProviderSpecific: endpoint.ProviderSpecific{}, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := ToInternalEndpoints(tt.input) + assert.True(t, testutils.SameEndpoints(tt.expected, result)) + }) + } +} + +func TestToAPIEndpoints(t *testing.T) { + tests := []struct { + name string + input []*endpoint.Endpoint + expected []*apiv1alpha1.Endpoint + }{ + { + name: "empty slice", + input: []*endpoint.Endpoint{}, + expected: nil, + }, + { + name: "nil slice", + input: nil, + expected: nil, + }, + { + name: "single endpoint", + input: []*endpoint.Endpoint{ + { + DNSName: "test.example.com", + Targets: []string{"1.2.3.4"}, + RecordType: "A", + RecordTTL: 300, + ProviderSpecific: endpoint.ProviderSpecific{}, + }, + }, + expected: []*apiv1alpha1.Endpoint{ + { + DNSName: "test.example.com", + Targets: []string{"1.2.3.4"}, + RecordType: "A", + RecordTTL: 300, + ProviderSpecific: []apiv1alpha1.ProviderSpecificProperty{}, + }, + }, + }, + { + name: "multiple endpoints with provider specific", + input: []*endpoint.Endpoint{ + { + DNSName: "api.example.com", + Targets: []string{"1.2.3.4"}, + RecordType: "A", + RecordTTL: 300, + ProviderSpecific: endpoint.ProviderSpecific{ + "weight": "100", + }, + }, + { + DNSName: "www.example.com", + Targets: []string{"example.com"}, + RecordType: "CNAME", + RecordTTL: 600, + Labels: map[string]string{ + "owner": "external-dns", + }, + ProviderSpecific: endpoint.ProviderSpecific{ + "alias": "true", + }, + }, + }, + expected: []*apiv1alpha1.Endpoint{ + { + DNSName: "api.example.com", + Targets: []string{"1.2.3.4"}, + RecordType: "A", + RecordTTL: 300, + ProviderSpecific: []apiv1alpha1.ProviderSpecificProperty{ + {Name: "weight", Value: "100"}, + }, + }, + { + DNSName: "www.example.com", + Targets: []string{"example.com"}, + RecordType: "CNAME", + RecordTTL: 600, + Labels: map[string]string{ + "owner": "external-dns", + }, + ProviderSpecific: []apiv1alpha1.ProviderSpecificProperty{ + {Name: "alias", Value: "true"}, + }, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := ToAPIEndpoints(tt.input) + assert.Len(t, result, len(tt.expected)) + + for i, expected := range tt.expected { + if expected == nil { + assert.Nil(t, result[i]) + continue + } + + assert.Equal(t, expected.DNSName, result[i].DNSName) + assert.Equal(t, expected.Targets, result[i].Targets) + assert.Equal(t, expected.RecordType, result[i].RecordType) + assert.Equal(t, expected.RecordTTL, result[i].RecordTTL) + assert.Equal(t, expected.SetIdentifier, result[i].SetIdentifier) + assert.Equal(t, expected.Labels, result[i].Labels) + + // Convert ProviderSpecific slice to map for comparison since order doesn't matter + expectedPS := make(map[string]string) + for _, ps := range expected.ProviderSpecific { + expectedPS[ps.Name] = ps.Value + } + resultPS := make(map[string]string) + for _, ps := range result[i].ProviderSpecific { + resultPS[ps.Name] = ps.Value + } + assert.Equal(t, expectedPS, resultPS) + } + }) + } +} + +func TestRoundTripConversion(t *testing.T) { + tests := []struct { + name string + original *endpoint.Endpoint + }{ + { + name: "basic endpoint round trip", + original: &endpoint.Endpoint{ + DNSName: "test.example.com", + Targets: []string{"1.2.3.4"}, + RecordType: "A", + RecordTTL: 300, + ProviderSpecific: endpoint.ProviderSpecific{}, + }, + }, + { + name: "complex endpoint round trip", + original: &endpoint.Endpoint{ + DNSName: "api.example.com", + Targets: []string{"10.0.0.1", "10.0.0.2"}, + RecordType: "A", + RecordTTL: 600, + SetIdentifier: "primary", + Labels: map[string]string{ + "owner": "external-dns", + "type": "public", + }, + ProviderSpecific: endpoint.ProviderSpecific{ + "weight": "100", + "policy": "weighted", + "health-check": "enabled", + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Convert internal -> API -> internal + apiEndpoint := ToAPIEndpoint(tt.original) + result := ToInternalEndpoint(apiEndpoint) + + assert.True(t, testutils.SameEndpoint(tt.original, result)) + }) + } +} From b2a6334cfc5cd6c3ae1c73c04ccbda569d169d67 Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Tue, 23 Sep 2025 18:36:48 +0900 Subject: [PATCH 13/15] fix: resolve merge error Signed-off-by: u-kai <76635578+u-kai@users.noreply.github.com> --- source/crd_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/source/crd_test.go b/source/crd_test.go index c87f6c1417..544f621264 100644 --- a/source/crd_test.go +++ b/source/crd_test.go @@ -468,7 +468,7 @@ func testCRDSourceEndpoints(t *testing.T) { endpoints: []*apiv1alpha1.Endpoint{ { DNSName: "example.org", - Targets: endpoint.Targets{`100 10 "S" "SIP+D2U" "!^.*$!sip:customer-service@example.org!" _sip._udp.example.org`, `102 10 "S" "SIP+D2T" "!^.*$!sip:customer-service@example.org!" _sip._tcp.example.org`}, + Targets: []string{`100 10 "S" "SIP+D2U" "!^.*$!sip:customer-service@example.org!" _sip._udp.example.org`, `102 10 "S" "SIP+D2T" "!^.*$!sip:customer-service@example.org!" _sip._tcp.example.org`}, RecordType: endpoint.RecordTypeNAPTR, RecordTTL: 180, }, @@ -486,10 +486,10 @@ func testCRDSourceEndpoints(t *testing.T) { registeredNamespace: "foo", labels: map[string]string{"test": "that"}, labelFilter: "test=that", - endpoints: []*endpoint.Endpoint{ + endpoints: []*apiv1alpha1.Endpoint{ { DNSName: "example.org", - Targets: endpoint.Targets{"foo.example.org."}, + Targets: []string{"foo.example.org."}, RecordType: endpoint.RecordTypeTXT, RecordTTL: 180, }, @@ -507,10 +507,10 @@ func testCRDSourceEndpoints(t *testing.T) { registeredNamespace: "foo", labels: map[string]string{"test": "that"}, labelFilter: "test=that", - endpoints: []*endpoint.Endpoint{ + endpoints: []*apiv1alpha1.Endpoint{ { DNSName: "example.org", - Targets: endpoint.Targets{"1.2.3.4."}, + Targets: []string{"1.2.3.4."}, RecordType: endpoint.RecordTypeA, RecordTTL: 180, }, From 92d91e064ba4550e2445f1eaa6bf79789a70540c Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Thu, 25 Sep 2025 20:30:15 +0900 Subject: [PATCH 14/15] add provider specific test coverage Signed-off-by: u-kai <76635578+u-kai@users.noreply.github.com> --- endpoint/endpoint_test.go | 89 ++++++++++++++++++++++++++++++++------- 1 file changed, 73 insertions(+), 16 deletions(-) diff --git a/endpoint/endpoint_test.go b/endpoint/endpoint_test.go index 3ee3d6f5c4..2c0ce88c59 100644 --- a/endpoint/endpoint_test.go +++ b/endpoint/endpoint_test.go @@ -191,27 +191,57 @@ func TestIsLess(t *testing.T) { } func TestGetProviderSpecificProperty(t *testing.T) { - e := &Endpoint{ - ProviderSpecific: ProviderSpecific{ - "name": "value", + tests := []struct { + name string + endpoint *Endpoint + key string + expectedValue string + expectedOk bool + }{ + { + name: "key exists", + endpoint: &Endpoint{ + ProviderSpecific: ProviderSpecific{ + "region": "us-west-1", + }, + }, + key: "region", + expectedValue: "us-west-1", + expectedOk: true, + }, + { + name: "key does not exist", + endpoint: &Endpoint{ + ProviderSpecific: ProviderSpecific{ + "region": "us-west-1", + }, + }, + key: "zone", + expectedValue: "", + expectedOk: false, + }, + { + name: "empty ProviderSpecific", + endpoint: &Endpoint{ + ProviderSpecific: nil, + }, + key: "region", + expectedValue: "", + expectedOk: false, }, } - t.Run("key is not present in provider specific", func(t *testing.T) { - val, ok := e.GetProviderSpecificProperty("hello") - assert.False(t, ok) - assert.Empty(t, val) - }) - - t.Run("key is present in provider specific", func(t *testing.T) { - val, ok := e.GetProviderSpecificProperty("name") - assert.True(t, ok) - assert.NotEmpty(t, val) - - }) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + value, ok := tt.endpoint.GetProviderSpecificProperty(tt.key) + if value != tt.expectedValue || ok != tt.expectedOk { + t.Errorf("GetProviderSpecificProperty(%q) = (%q, %v); want (%q, %v)", tt.key, value, ok, tt.expectedValue, tt.expectedOk) + } + }) + } } -func TestSetProviderSpecficProperty(t *testing.T) { +func TestSetProviderSpecificProperty(t *testing.T) { cases := []struct { name string endpoint Endpoint @@ -298,6 +328,25 @@ func TestSetProviderSpecficProperty(t *testing.T) { "name1": "value2", }, }, + { + name: "provider specific is nil", + endpoint: Endpoint{ + DNSName: "example.org", + RecordTTL: TTL(0), + RecordType: RecordTypeA, + SetIdentifier: "identifier", + Targets: Targets{ + "example.org", "example.com", "1.2.4.5", + }, + ProviderSpecific: nil, + }, + expectedIdentifier: "identifier", + key: "name1", + value: "value1", + expected: ProviderSpecific{ + "name1": "value1", + }, + }, } for _, c := range cases { @@ -358,6 +407,14 @@ func TestDeleteProviderSpecificProperty(t *testing.T) { key: "name1", expected: ProviderSpecific{}, }, + { + name: "provider specific is nil", + endpoint: Endpoint{ + ProviderSpecific: nil, + }, + key: "name1", + expected: nil, + }, } for _, c := range cases { From 17052dad16fa2d48337cf68e1b814aff1980c5fa Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Thu, 25 Sep 2025 20:51:05 +0900 Subject: [PATCH 15/15] fix and rename endpoint adapter Signed-off-by: u-kai <76635578+u-kai@users.noreply.github.com> --- pkg/adapter/endpoint.go | 72 +++++++++++++++++---------------- pkg/adapter/endpoint_test.go | 6 +-- provider/webhook/api/httpapi.go | 10 ++--- provider/webhook/webhook.go | 4 +- source/crd.go | 2 +- source/crd_test.go | 2 +- 6 files changed, 49 insertions(+), 47 deletions(-) diff --git a/pkg/adapter/endpoint.go b/pkg/adapter/endpoint.go index 98b3121bb1..ff217bb7fb 100644 --- a/pkg/adapter/endpoint.go +++ b/pkg/adapter/endpoint.go @@ -1,5 +1,5 @@ /* -Copyright 2017 The Kubernetes Authors. +Copyright 2025 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -20,67 +20,69 @@ import ( "sigs.k8s.io/external-dns/endpoint" ) -func ToInternalEndpoint(crdEp *apiv1alpha1.Endpoint) *endpoint.Endpoint { - if crdEp == nil { +// FromAPIEndpoint converts an API endpoint (v1alpha1.Endpoint) to an internal endpoint (endpoint.Endpoint). +func FromAPIEndpoint(apiEndpoint *apiv1alpha1.Endpoint) *endpoint.Endpoint { + if apiEndpoint == nil { return nil } p := make(endpoint.ProviderSpecific) - for _, ps := range crdEp.ProviderSpecific { + for _, ps := range apiEndpoint.ProviderSpecific { p.Set(ps.Name, ps.Value) } - ep := &endpoint.Endpoint{ - DNSName: crdEp.DNSName, - Targets: crdEp.Targets, - RecordType: crdEp.RecordType, - SetIdentifier: crdEp.SetIdentifier, - RecordTTL: endpoint.TTL(crdEp.RecordTTL), - Labels: crdEp.Labels, + return &endpoint.Endpoint{ + DNSName: apiEndpoint.DNSName, + Targets: apiEndpoint.Targets, + RecordType: apiEndpoint.RecordType, + SetIdentifier: apiEndpoint.SetIdentifier, + RecordTTL: endpoint.TTL(apiEndpoint.RecordTTL), + Labels: apiEndpoint.Labels, ProviderSpecific: p, } - return ep } -func ToInternalEndpoints(crdEps []*apiv1alpha1.Endpoint) []*endpoint.Endpoint { - if len(crdEps) == 0 { +// FromAPIEndpoints converts a slice of API endpoints to internal endpoints. +func FromAPIEndpoints(apiEndpoints []*apiv1alpha1.Endpoint) []*endpoint.Endpoint { + if len(apiEndpoints) == 0 { return nil } - eps := make([]*endpoint.Endpoint, 0, len(crdEps)) - for _, crdEp := range crdEps { - eps = append(eps, ToInternalEndpoint(crdEp)) + eps := make([]*endpoint.Endpoint, 0, len(apiEndpoints)) + for _, apiEndpoint := range apiEndpoints { + eps = append(eps, FromAPIEndpoint(apiEndpoint)) } return eps } -func ToAPIEndpoint(ep *endpoint.Endpoint) *apiv1alpha1.Endpoint { - if ep == nil { +// ToAPIEndpoint converts an internal endpoint (endpoint.Endpoint) to an API endpoint (v1alpha1.Endpoint). +func ToAPIEndpoint(internalEndpoint *endpoint.Endpoint) *apiv1alpha1.Endpoint { + if internalEndpoint == nil { return nil } - ps := make([]apiv1alpha1.ProviderSpecificProperty, 0, len(ep.ProviderSpecific)) - for k, v := range ep.ProviderSpecific { + ps := make([]apiv1alpha1.ProviderSpecificProperty, 0, len(internalEndpoint.ProviderSpecific)) + for k, v := range internalEndpoint.ProviderSpecific { ps = append(ps, apiv1alpha1.ProviderSpecificProperty{ Name: k, Value: v, }) } - crdEp := &apiv1alpha1.Endpoint{ - DNSName: ep.DNSName, - Targets: ep.Targets, - RecordType: ep.RecordType, - SetIdentifier: ep.SetIdentifier, - RecordTTL: int64(ep.RecordTTL), - Labels: ep.Labels, + return &apiv1alpha1.Endpoint{ + DNSName: internalEndpoint.DNSName, + Targets: internalEndpoint.Targets, + RecordType: internalEndpoint.RecordType, + SetIdentifier: internalEndpoint.SetIdentifier, + RecordTTL: int64(internalEndpoint.RecordTTL), + Labels: internalEndpoint.Labels, ProviderSpecific: ps, } - return crdEp } -func ToAPIEndpoints(eps []*endpoint.Endpoint) []*apiv1alpha1.Endpoint { - if len(eps) == 0 { +// ToAPIEndpoints converts a slice of internal endpoints to API endpoints. +func ToAPIEndpoints(internalEndpoints []*endpoint.Endpoint) []*apiv1alpha1.Endpoint { + if len(internalEndpoints) == 0 { return nil } - crdEps := make([]*apiv1alpha1.Endpoint, 0, len(eps)) - for _, ep := range eps { - crdEps = append(crdEps, ToAPIEndpoint(ep)) + apiEndpoints := make([]*apiv1alpha1.Endpoint, 0, len(internalEndpoints)) + for _, internalEndpoint := range internalEndpoints { + apiEndpoints = append(apiEndpoints, ToAPIEndpoint(internalEndpoint)) } - return crdEps + return apiEndpoints } diff --git a/pkg/adapter/endpoint_test.go b/pkg/adapter/endpoint_test.go index a983a23946..02c3562ef7 100644 --- a/pkg/adapter/endpoint_test.go +++ b/pkg/adapter/endpoint_test.go @@ -140,7 +140,7 @@ func TestToInternalEndpoint(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := ToInternalEndpoint(tt.input) + result := FromAPIEndpoint(tt.input) if tt.expected == nil { assert.Nil(t, result) } else { @@ -344,7 +344,7 @@ func TestToInternalEndpoints(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := ToInternalEndpoints(tt.input) + result := FromAPIEndpoints(tt.input) assert.True(t, testutils.SameEndpoints(tt.expected, result)) }) } @@ -511,7 +511,7 @@ func TestRoundTripConversion(t *testing.T) { t.Run(tt.name, func(t *testing.T) { // Convert internal -> API -> internal apiEndpoint := ToAPIEndpoint(tt.original) - result := ToInternalEndpoint(apiEndpoint) + result := FromAPIEndpoint(apiEndpoint) assert.True(t, testutils.SameEndpoint(tt.original, result)) }) diff --git a/provider/webhook/api/httpapi.go b/provider/webhook/api/httpapi.go index 8f12ae686f..6739bd733b 100644 --- a/provider/webhook/api/httpapi.go +++ b/provider/webhook/api/httpapi.go @@ -74,10 +74,10 @@ func (p *WebhookServer) RecordsHandler(w http.ResponseWriter, req *http.Request) return } internalChanges := &plan.Changes{ - Create: adapter.ToInternalEndpoints(changes.Create), - Delete: adapter.ToInternalEndpoints(changes.Delete), - UpdateOld: adapter.ToInternalEndpoints(changes.UpdateOld), - UpdateNew: adapter.ToInternalEndpoints(changes.UpdateNew), + Create: adapter.FromAPIEndpoints(changes.Create), + Delete: adapter.FromAPIEndpoints(changes.Delete), + UpdateOld: adapter.FromAPIEndpoints(changes.UpdateOld), + UpdateNew: adapter.FromAPIEndpoints(changes.UpdateNew), } err := p.Provider.ApplyChanges(context.Background(), internalChanges) if err != nil { @@ -107,7 +107,7 @@ func (p *WebhookServer) AdjustEndpointsHandler(w http.ResponseWriter, req *http. return } w.Header().Set(ContentTypeHeader, MediaTypeFormatAndVersion) - endpoints := adapter.ToInternalEndpoints(pve) + endpoints := adapter.FromAPIEndpoints(pve) endpoints, err := p.Provider.AdjustEndpoints(endpoints) if err != nil { log.Errorf("Failed to call adjust endpoints: %v", err) diff --git a/provider/webhook/webhook.go b/provider/webhook/webhook.go index d32228c9bb..1922c12f4a 100644 --- a/provider/webhook/webhook.go +++ b/provider/webhook/webhook.go @@ -193,7 +193,7 @@ func (p WebhookProvider) Records(ctx context.Context) ([]*endpoint.Endpoint, err return nil, err } - return adapter.ToInternalEndpoints(apiEndpoints), nil + return adapter.FromAPIEndpoints(apiEndpoints), nil } // ApplyChanges will make a POST to remoteServerURL/records with the changes @@ -301,7 +301,7 @@ func (p WebhookProvider) AdjustEndpoints(e []*endpoint.Endpoint) ([]*endpoint.En return nil, err } - return adapter.ToInternalEndpoints(apiEps), nil + return adapter.FromAPIEndpoints(apiEps), nil } // GetDomainFilter make calls to get the serialized version of the domain filter diff --git a/source/crd.go b/source/crd.go index f9d4b56d50..c423eb05b2 100644 --- a/source/crd.go +++ b/source/crd.go @@ -183,7 +183,7 @@ func (cs *crdSource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, error for _, dnsEndpoint := range result.Items { var crdEndpoints []*endpoint.Endpoint for _, crdEp := range dnsEndpoint.Spec.Endpoints { - ep := adapter.ToInternalEndpoint(crdEp) + ep := adapter.FromAPIEndpoint(crdEp) if (ep.RecordType == endpoint.RecordTypeCNAME || ep.RecordType == endpoint.RecordTypeA || ep.RecordType == endpoint.RecordTypeAAAA) && len(ep.Targets) < 1 { log.Debugf("Endpoint %s with DNSName %s has an empty list of targets, allowing it to pass through for default-targets processing", dnsEndpoint.Name, ep.DNSName) } diff --git a/source/crd_test.go b/source/crd_test.go index 544f621264..46afabc3cd 100644 --- a/source/crd_test.go +++ b/source/crd_test.go @@ -556,7 +556,7 @@ func testCRDSourceEndpoints(t *testing.T) { } // Validate received endpoints against expected endpoints. - endpoints := adapter.ToInternalEndpoints(ti.endpoints) + endpoints := adapter.FromAPIEndpoints(ti.endpoints) validateEndpoints(t, receivedEndpoints, endpoints) for _, e := range receivedEndpoints {