Skip to content

Commit b9d033b

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #4993 from jrosinsk/oci-provider-handle-multiple-ip-4940
fix(oci): records with multiple IP addresses
2 parents 814d4ff + 97a9054 commit b9d033b

File tree

2 files changed

+213
-10
lines changed

2 files changed

+213
-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

+148-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,129 @@ 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+
},
964+
{
965+
name: "update_multi_target",
966+
zones: []dns.ZoneSummary{{
967+
Id: common.String("ocid1.dns-zone.oc1..e1e042ef0bfbb5c251b9713fd7bf8959"),
968+
Name: common.String("foo.com"),
969+
}},
970+
records: map[string][]dns.Record{
971+
"ocid1.dns-zone.oc1..e1e042ef0bfbb5c251b9713fd7bf8959": {{
972+
Domain: common.String("first.foo.com"),
973+
Rdata: common.String("10.77.4.5"),
974+
Rtype: common.String(endpoint.RecordTypeA),
975+
Ttl: common.Int(ociRecordTTL),
976+
}},
977+
},
978+
changes: &plan.Changes{
979+
UpdateOld: []*endpoint.Endpoint{endpoint.NewEndpointWithTTL(
980+
"first.foo.com",
981+
endpoint.RecordTypeA,
982+
endpoint.TTL(ociRecordTTL),
983+
"10.77.4.5",
984+
)},
985+
UpdateNew: []*endpoint.Endpoint{endpoint.NewEndpointWithTTL(
986+
"first.foo.com",
987+
endpoint.RecordTypeA,
988+
endpoint.TTL(ociRecordTTL),
989+
"10.77.6.10",
990+
)},
991+
},
992+
expectedEndpoints: []*endpoint.Endpoint{endpoint.NewEndpointWithTTL(
993+
"first.foo.com",
994+
endpoint.RecordTypeA,
995+
endpoint.TTL(ociRecordTTL),
996+
"10.77.6.10",
997+
)},
998+
},
999+
{
1000+
name: "increase_multi_target",
1001+
zones: []dns.ZoneSummary{{
1002+
Id: common.String("ocid1.dns-zone.oc1..e1e042ef0bfbb5c251b9713fd7bf8959"),
1003+
Name: common.String("foo.com"),
1004+
}},
1005+
records: map[string][]dns.Record{
1006+
"ocid1.dns-zone.oc1..e1e042ef0bfbb5c251b9713fd7bf8959": {{
1007+
Domain: common.String("first.foo.com"),
1008+
Rdata: common.String("10.77.4.5"),
1009+
Rtype: common.String(endpoint.RecordTypeA),
1010+
Ttl: common.Int(ociRecordTTL),
1011+
}},
1012+
},
1013+
changes: &plan.Changes{
1014+
Create: []*endpoint.Endpoint{endpoint.NewEndpointWithTTL(
1015+
"first.foo.com",
1016+
endpoint.RecordTypeA,
1017+
endpoint.TTL(ociRecordTTL),
1018+
"10.77.6.10",
1019+
)},
1020+
},
1021+
expectedEndpoints: []*endpoint.Endpoint{endpoint.NewEndpointWithTTL(
1022+
"first.foo.com",
1023+
endpoint.RecordTypeA,
1024+
endpoint.TTL(ociRecordTTL),
1025+
"10.77.4.5", "10.77.6.10",
1026+
)},
1027+
},
8891028
}
8901029

8911030
for _, tc := range testCases {
@@ -904,6 +1043,8 @@ func TestOCIApplyChanges(t *testing.T) {
9041043
require.Equal(t, tc.err, err)
9051044
endpoints, err := provider.Records(ctx)
9061045
require.NoError(t, err)
1046+
sortEndpointTargets(endpoints)
1047+
sortEndpointTargets(tc.expectedEndpoints)
9071048
require.ElementsMatch(t, tc.expectedEndpoints, endpoints)
9081049
})
9091050
}

0 commit comments

Comments
 (0)