Skip to content

Commit d854cef

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 290f8c8 commit d854cef

File tree

2 files changed

+138
-10
lines changed

2 files changed

+138
-10
lines changed

‎provider/oci/oci.go

+62-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,19 @@ 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 p.domainFilter.Match(ep.DNSName) {
238+
for _, t := range ep.Targets {
239+
singleTargetEp := &endpoint.Endpoint{
240+
DNSName: ep.DNSName,
241+
Targets: []string{t},
242+
RecordType: ep.RecordType,
243+
RecordTTL: ep.RecordTTL,
244+
Labels: ep.Labels,
245+
ProviderSpecific: ep.ProviderSpecific,
246+
}
247+
ops = append(ops, newRecordOperation(singleTargetEp, opType))
248+
}
206249
}
207250
}
208251
return ops
@@ -248,6 +291,8 @@ func (p *OCIProvider) Records(ctx context.Context) ([]*endpoint.Endpoint, error)
248291
}
249292
}
250293

294+
endpoints = mergeEndpointsMultiTargets(endpoints)
295+
251296
return endpoints, nil
252297
}
253298

@@ -299,6 +344,20 @@ func (p *OCIProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) e
299344
return nil
300345
}
301346

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

‎provider/oci/oci_test.go

+76-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,12 @@ 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
582586
}
583587

584588
func (c *mutableMockOCIDNSClient) PatchZoneRecords(ctx context.Context, request dns.PatchZoneRecordsRequest) (response dns.PatchZoneRecordsResponse, err error) {
@@ -599,7 +603,7 @@ func (c *mutableMockOCIDNSClient) PatchZoneRecords(ctx context.Context, request
599603
})
600604

601605
for _, op := range request.Items {
602-
k := ociRecordKey(*op.Rtype, *op.Domain)
606+
k := ociRecordKey(*op.Rtype, *op.Domain, *op.Rdata)
603607
switch op.Operation {
604608
case dns.RecordOperationOperationAdd:
605609
records[k] = dns.Record{
@@ -702,6 +706,7 @@ func TestMutableMockOCIDNSClient(t *testing.T) {
702706
}
703707

704708
func TestOCIApplyChanges(t *testing.T) {
709+
705710
testCases := []struct {
706711
name string
707712
zones []dns.ZoneSummary
@@ -840,21 +845,26 @@ func TestOCIApplyChanges(t *testing.T) {
840845
Rtype: common.String(endpoint.RecordTypeA),
841846
Ttl: common.Int(ociRecordTTL),
842847
}, {
843-
Domain: common.String("bar.foo.com"),
848+
Domain: common.String("car.foo.com"),
844849
Rdata: common.String("bar.com."),
845850
Rtype: common.String(endpoint.RecordTypeCNAME),
846851
Ttl: common.Int(ociRecordTTL),
852+
}, {
853+
Domain: common.String("bar.foo.com"),
854+
Rdata: common.String("baz.com."),
855+
Rtype: common.String(endpoint.RecordTypeCNAME),
856+
Ttl: common.Int(ociRecordTTL),
847857
}},
848858
},
849859
changes: &plan.Changes{
850860
Delete: []*endpoint.Endpoint{endpoint.NewEndpointWithTTL(
851861
"foo.foo.com",
852862
endpoint.RecordTypeA,
853863
endpoint.TTL(ociRecordTTL),
854-
"baz.com.",
864+
"127.0.0.1",
855865
)},
856866
UpdateOld: []*endpoint.Endpoint{endpoint.NewEndpointWithTTL(
857-
"bar.foo.com",
867+
"car.foo.com",
858868
endpoint.RecordTypeCNAME,
859869
endpoint.TTL(ociRecordTTL),
860870
"baz.com.",
@@ -886,6 +896,65 @@ func TestOCIApplyChanges(t *testing.T) {
886896
"127.0.0.1"),
887897
},
888898
},
899+
{
900+
name: "combine_multi_target",
901+
zones: []dns.ZoneSummary{{
902+
Id: common.String("ocid1.dns-zone.oc1..e1e042ef0bfbb5c251b9713fd7bf8959"),
903+
Name: common.String("foo.com"),
904+
}},
905+
906+
changes: &plan.Changes{
907+
Create: []*endpoint.Endpoint{endpoint.NewEndpointWithTTL(
908+
"foo.foo.com",
909+
endpoint.RecordTypeA,
910+
endpoint.TTL(ociRecordTTL),
911+
"192.168.1.2",
912+
), endpoint.NewEndpointWithTTL(
913+
"foo.foo.com",
914+
endpoint.RecordTypeA,
915+
endpoint.TTL(ociRecordTTL),
916+
"192.168.2.5",
917+
)},
918+
},
919+
expectedEndpoints: []*endpoint.Endpoint{endpoint.NewEndpointWithTTL(
920+
"foo.foo.com",
921+
endpoint.RecordTypeA,
922+
endpoint.TTL(ociRecordTTL), "192.168.1.2", "192.168.2.5",
923+
)},
924+
},
925+
{
926+
name: "remove_from_multi_target",
927+
zones: []dns.ZoneSummary{{
928+
Id: common.String("ocid1.dns-zone.oc1..e1e042ef0bfbb5c251b9713fd7bf8959"),
929+
Name: common.String("foo.com"),
930+
}},
931+
records: map[string][]dns.Record{
932+
"ocid1.dns-zone.oc1..e1e042ef0bfbb5c251b9713fd7bf8959": {{
933+
Domain: common.String("foo.foo.com"),
934+
Rdata: common.String("192.168.1.2"),
935+
Rtype: common.String(endpoint.RecordTypeA),
936+
Ttl: common.Int(ociRecordTTL),
937+
}, {
938+
Domain: common.String("foo.foo.com"),
939+
Rdata: common.String("192.168.2.5"),
940+
Rtype: common.String(endpoint.RecordTypeA),
941+
Ttl: common.Int(ociRecordTTL),
942+
}},
943+
},
944+
changes: &plan.Changes{
945+
Delete: []*endpoint.Endpoint{endpoint.NewEndpointWithTTL(
946+
"foo.foo.com",
947+
endpoint.RecordTypeA,
948+
endpoint.TTL(ociRecordTTL),
949+
"192.168.1.2",
950+
)},
951+
},
952+
expectedEndpoints: []*endpoint.Endpoint{endpoint.NewEndpointWithTTL(
953+
"foo.foo.com",
954+
endpoint.RecordTypeA,
955+
endpoint.TTL(ociRecordTTL), "192.168.2.5",
956+
)},
957+
},
889958
}
890959

891960
for _, tc := range testCases {

0 commit comments

Comments
 (0)