Skip to content

Commit 6797848

Browse files
jrosinskjrosinsk
committed
Fix:If multiple targets are part of an OCI provider record operation, create a new record for each target.
1 parent 9619e6b commit 6797848

File tree

2 files changed

+149
-10
lines changed

2 files changed

+149
-10
lines changed

‎provider/oci/oci.go

+65-3
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,39 @@ func (p *OCIProvider) zones(ctx context.Context) (map[string]dns.ZoneSummary, er
170170
return zones, nil
171171
}
172172

173+
// Merge Endpoints with the same Name and Type into a single endpoint with multiple Targets.
174+
func mergeEndpointsMultiTargets(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint {
175+
endpointsByNameType := map[string][]*endpoint.Endpoint{}
176+
177+
for _, ep := range endpoints {
178+
key := fmt.Sprintf("%s-%s", ep.DNSName, ep.RecordType)
179+
endpointsByNameType[key] = append(endpointsByNameType[key], ep)
180+
}
181+
182+
// If there were no merges, return endpoints.
183+
if len(endpointsByNameType) == len(endpoints) {
184+
return endpoints
185+
}
186+
187+
// Otherwise, create a new list of endpoints with the consolidated targets.
188+
var mergedEndpoints []*endpoint.Endpoint
189+
for _, endpoints := range endpointsByNameType {
190+
dnsName := endpoints[0].DNSName
191+
recordType := endpoints[0].RecordType
192+
recordTTL := endpoints[0].RecordTTL
193+
194+
targets := make([]string, len(endpoints))
195+
for i, e := range endpoints {
196+
targets[i] = e.Targets[0]
197+
}
198+
199+
e := endpoint.NewEndpointWithTTL(dnsName, recordType, recordTTL, targets...)
200+
mergedEndpoints = append(mergedEndpoints, e)
201+
}
202+
203+
return mergedEndpoints
204+
}
205+
173206
func (p *OCIProvider) addPaginatedZones(ctx context.Context, zones map[string]dns.ZoneSummary, scope dns.GetZoneScopeEnum) error {
174207
var page *string
175208
// Loop until we have listed all zones.
@@ -200,9 +233,22 @@ func (p *OCIProvider) addPaginatedZones(ctx context.Context, zones map[string]dn
200233

201234
func (p *OCIProvider) newFilteredRecordOperations(endpoints []*endpoint.Endpoint, opType dns.RecordOperationOperationEnum) []dns.RecordOperation {
202235
ops := []dns.RecordOperation{}
203-
for _, endpoint := range endpoints {
204-
if p.domainFilter.Match(endpoint.DNSName) {
205-
ops = append(ops, newRecordOperation(endpoint, opType))
236+
for _, ep := range endpoints {
237+
if ep == nil {
238+
continue
239+
}
240+
if p.domainFilter.Match(ep.DNSName) {
241+
for _, t := range ep.Targets {
242+
singleTargetEp := &endpoint.Endpoint{
243+
DNSName: ep.DNSName,
244+
Targets: []string{t},
245+
RecordType: ep.RecordType,
246+
RecordTTL: ep.RecordTTL,
247+
Labels: ep.Labels,
248+
ProviderSpecific: ep.ProviderSpecific,
249+
}
250+
ops = append(ops, newRecordOperation(singleTargetEp, opType))
251+
}
206252
}
207253
}
208254
return ops
@@ -248,6 +294,8 @@ func (p *OCIProvider) Records(ctx context.Context) ([]*endpoint.Endpoint, error)
248294
}
249295
}
250296

297+
endpoints = mergeEndpointsMultiTargets(endpoints)
298+
251299
return endpoints, nil
252300
}
253301

@@ -299,6 +347,20 @@ func (p *OCIProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) e
299347
return nil
300348
}
301349

350+
// AdjustEndpoints modifies the endpoints as needed by the specific provider
351+
func (p *OCIProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) ([]*endpoint.Endpoint, error) {
352+
adjustedEndpoints := []*endpoint.Endpoint{}
353+
for _, e := range endpoints {
354+
// OCI DNS does not support the set-identifier attribute, so we remove it to avoid plan failure
355+
if e.SetIdentifier != "" {
356+
log.Warnf("Adjusting endpont: %v. Ignoring unsupported annotation 'set-identifier': %s", *e, e.SetIdentifier)
357+
e.SetIdentifier = ""
358+
}
359+
adjustedEndpoints = append(adjustedEndpoints, e)
360+
}
361+
return adjustedEndpoints, nil
362+
}
363+
302364
// newRecordOperation returns a RecordOperation based on a given endpoint.
303365
func newRecordOperation(ep *endpoint.Endpoint, opType dns.RecordOperationOperationEnum) dns.RecordOperation {
304366
targets := make([]string, len(ep.Targets))

‎provider/oci/oci_test.go

+84-7
Original file line numberDiff line numberDiff line change
@@ -541,7 +541,7 @@ func newMutableMockOCIDNSClient(zones []dns.ZoneSummary, recordsByZone map[strin
541541

542542
for zoneID, records := range recordsByZone {
543543
for _, record := range records {
544-
c.records[zoneID][ociRecordKey(*record.Rtype, *record.Domain)] = record
544+
c.records[zoneID][ociRecordKey(*record.Rtype, *record.Domain, *record.Rdata)] = record
545545
}
546546
}
547547

@@ -577,8 +577,18 @@ func (c *mutableMockOCIDNSClient) GetZoneRecords(ctx context.Context, request dn
577577
return
578578
}
579579

580-
func ociRecordKey(rType, domain string) string {
581-
return rType + "/" + domain
580+
func ociRecordKey(rType, domain string, ip string) string {
581+
rdata := ""
582+
if rType == "A" { // adds support for multi-targets with same rtype and domain
583+
rdata = "_" + ip
584+
}
585+
return rType + "_" + domain + rdata
586+
}
587+
588+
func sortEndpointTargets(endpoints []*endpoint.Endpoint) {
589+
for _, ep := range endpoints {
590+
sort.Strings([]string(ep.Targets))
591+
}
582592
}
583593

584594
func (c *mutableMockOCIDNSClient) PatchZoneRecords(ctx context.Context, request dns.PatchZoneRecordsRequest) (response dns.PatchZoneRecordsResponse, err error) {
@@ -599,7 +609,7 @@ func (c *mutableMockOCIDNSClient) PatchZoneRecords(ctx context.Context, request
599609
})
600610

601611
for _, op := range request.Items {
602-
k := ociRecordKey(*op.Rtype, *op.Domain)
612+
k := ociRecordKey(*op.Rtype, *op.Domain, *op.Rdata)
603613
switch op.Operation {
604614
case dns.RecordOperationOperationAdd:
605615
records[k] = dns.Record{
@@ -702,6 +712,7 @@ func TestMutableMockOCIDNSClient(t *testing.T) {
702712
}
703713

704714
func TestOCIApplyChanges(t *testing.T) {
715+
705716
testCases := []struct {
706717
name string
707718
zones []dns.ZoneSummary
@@ -840,21 +851,26 @@ func TestOCIApplyChanges(t *testing.T) {
840851
Rtype: common.String(endpoint.RecordTypeA),
841852
Ttl: common.Int(ociRecordTTL),
842853
}, {
843-
Domain: common.String("bar.foo.com"),
854+
Domain: common.String("car.foo.com"),
844855
Rdata: common.String("bar.com."),
845856
Rtype: common.String(endpoint.RecordTypeCNAME),
846857
Ttl: common.Int(ociRecordTTL),
858+
}, {
859+
Domain: common.String("bar.foo.com"),
860+
Rdata: common.String("baz.com."),
861+
Rtype: common.String(endpoint.RecordTypeCNAME),
862+
Ttl: common.Int(ociRecordTTL),
847863
}},
848864
},
849865
changes: &plan.Changes{
850866
Delete: []*endpoint.Endpoint{endpoint.NewEndpointWithTTL(
851867
"foo.foo.com",
852868
endpoint.RecordTypeA,
853869
endpoint.TTL(ociRecordTTL),
854-
"baz.com.",
870+
"127.0.0.1",
855871
)},
856872
UpdateOld: []*endpoint.Endpoint{endpoint.NewEndpointWithTTL(
857-
"bar.foo.com",
873+
"car.foo.com",
858874
endpoint.RecordTypeCNAME,
859875
endpoint.TTL(ociRecordTTL),
860876
"baz.com.",
@@ -886,6 +902,65 @@ func TestOCIApplyChanges(t *testing.T) {
886902
"127.0.0.1"),
887903
},
888904
},
905+
{
906+
name: "combine_multi_target",
907+
zones: []dns.ZoneSummary{{
908+
Id: common.String("ocid1.dns-zone.oc1..e1e042ef0bfbb5c251b9713fd7bf8959"),
909+
Name: common.String("foo.com"),
910+
}},
911+
912+
changes: &plan.Changes{
913+
Create: []*endpoint.Endpoint{endpoint.NewEndpointWithTTL(
914+
"foo.foo.com",
915+
endpoint.RecordTypeA,
916+
endpoint.TTL(ociRecordTTL),
917+
"192.168.1.2",
918+
), endpoint.NewEndpointWithTTL(
919+
"foo.foo.com",
920+
endpoint.RecordTypeA,
921+
endpoint.TTL(ociRecordTTL),
922+
"192.168.2.5",
923+
)},
924+
},
925+
expectedEndpoints: []*endpoint.Endpoint{endpoint.NewEndpointWithTTL(
926+
"foo.foo.com",
927+
endpoint.RecordTypeA,
928+
endpoint.TTL(ociRecordTTL), "192.168.1.2", "192.168.2.5",
929+
)},
930+
},
931+
{
932+
name: "remove_from_multi_target",
933+
zones: []dns.ZoneSummary{{
934+
Id: common.String("ocid1.dns-zone.oc1..e1e042ef0bfbb5c251b9713fd7bf8959"),
935+
Name: common.String("foo.com"),
936+
}},
937+
records: map[string][]dns.Record{
938+
"ocid1.dns-zone.oc1..e1e042ef0bfbb5c251b9713fd7bf8959": {{
939+
Domain: common.String("foo.foo.com"),
940+
Rdata: common.String("192.168.1.2"),
941+
Rtype: common.String(endpoint.RecordTypeA),
942+
Ttl: common.Int(ociRecordTTL),
943+
}, {
944+
Domain: common.String("foo.foo.com"),
945+
Rdata: common.String("192.168.2.5"),
946+
Rtype: common.String(endpoint.RecordTypeA),
947+
Ttl: common.Int(ociRecordTTL),
948+
}},
949+
},
950+
changes: &plan.Changes{
951+
Delete: []*endpoint.Endpoint{endpoint.NewEndpointWithTTL(
952+
"foo.foo.com",
953+
endpoint.RecordTypeA,
954+
endpoint.TTL(ociRecordTTL),
955+
"192.168.1.2",
956+
)},
957+
},
958+
expectedEndpoints: []*endpoint.Endpoint{endpoint.NewEndpointWithTTL(
959+
"foo.foo.com",
960+
endpoint.RecordTypeA,
961+
endpoint.TTL(ociRecordTTL), "192.168.2.5",
962+
)},
963+
},
889964
}
890965

891966
for _, tc := range testCases {
@@ -904,6 +979,8 @@ func TestOCIApplyChanges(t *testing.T) {
904979
require.Equal(t, tc.err, err)
905980
endpoints, err := provider.Records(ctx)
906981
require.NoError(t, err)
982+
sortEndpointTargets(endpoints)
983+
sortEndpointTargets(tc.expectedEndpoints)
907984
require.ElementsMatch(t, tc.expectedEndpoints, endpoints)
908985
})
909986
}

0 commit comments

Comments
 (0)