Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix:If multiple targets are part of an OCI provider record operation, create a new record for each target. #4993

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix:If multiple targets are part of an OCI provider record operation, create a new record for each target. #4993

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 62 additions & 3 deletions provider/oci/oci.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,39 @@ func (p *OCIProvider) zones(ctx context.Context) (map[string]dns.ZoneSummary, er
return zones, nil
}

// Merge Endpoints with the same Name and Type into a single endpoint with multiple Targets.
func mergeEndpointsMultiTargets(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint {
endpointsByNameType := map[string][]*endpoint.Endpoint{}

for _, ep := range endpoints {
key := fmt.Sprintf("%s-%s", ep.DNSName, ep.RecordType)
endpointsByNameType[key] = append(endpointsByNameType[key], ep)
}

// If there were no merges, return endpoints.
if len(endpointsByNameType) == len(endpoints) {
return endpoints
}

// Otherwise, create a new list of endpoints with the consolidated targets.
var mergedEndpoints []*endpoint.Endpoint
for _, endpoints := range endpointsByNameType {
dnsName := endpoints[0].DNSName
recordType := endpoints[0].RecordType
recordTTL := endpoints[0].RecordTTL

targets := make([]string, len(endpoints))
for i, e := range endpoints {
targets[i] = e.Targets[0]
}

e := endpoint.NewEndpointWithTTL(dnsName, recordType, recordTTL, targets...)
mergedEndpoints = append(mergedEndpoints, e)
}

return mergedEndpoints
}

func (p *OCIProvider) addPaginatedZones(ctx context.Context, zones map[string]dns.ZoneSummary, scope dns.GetZoneScopeEnum) error {
var page *string
// Loop until we have listed all zones.
Expand Down Expand Up @@ -200,9 +233,19 @@ func (p *OCIProvider) addPaginatedZones(ctx context.Context, zones map[string]dn

func (p *OCIProvider) newFilteredRecordOperations(endpoints []*endpoint.Endpoint, opType dns.RecordOperationOperationEnum) []dns.RecordOperation {
ops := []dns.RecordOperation{}
for _, endpoint := range endpoints {
if p.domainFilter.Match(endpoint.DNSName) {
ops = append(ops, newRecordOperation(endpoint, opType))
for _, ep := range endpoints {
if p.domainFilter.Match(ep.DNSName) {
for _, t := range ep.Targets {
singleTargetEp := &endpoint.Endpoint{
DNSName: ep.DNSName,
Targets: []string{t},
RecordType: ep.RecordType,
RecordTTL: ep.RecordTTL,
Labels: ep.Labels,
ProviderSpecific: ep.ProviderSpecific,
}
ops = append(ops, newRecordOperation(singleTargetEp, opType))
}
}
}
return ops
Expand Down Expand Up @@ -248,6 +291,8 @@ func (p *OCIProvider) Records(ctx context.Context) ([]*endpoint.Endpoint, error)
}
}

endpoints = mergeEndpointsMultiTargets(endpoints)

return endpoints, nil
}

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

// AdjustEndpoints modifies the endpoints as needed by the specific provider
func (p *OCIProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) ([]*endpoint.Endpoint, error) {
adjustedEndpoints := []*endpoint.Endpoint{}
for _, e := range endpoints {
// OCI DNS does not support the set-identifier attribute, so we remove it to avoid plan failure
if e.SetIdentifier != "" {
log.Warnf("Adjusting endpont: %v. Ignoring unsupported annotation 'set-identifier': %s", *e, e.SetIdentifier)
e.SetIdentifier = ""
}
adjustedEndpoints = append(adjustedEndpoints, e)
}
return adjustedEndpoints, nil
}

// newRecordOperation returns a RecordOperation based on a given endpoint.
func newRecordOperation(ep *endpoint.Endpoint, opType dns.RecordOperationOperationEnum) dns.RecordOperation {
targets := make([]string, len(ep.Targets))
Expand Down
82 changes: 75 additions & 7 deletions provider/oci/oci_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,7 @@ func newMutableMockOCIDNSClient(zones []dns.ZoneSummary, recordsByZone map[strin

for zoneID, records := range recordsByZone {
for _, record := range records {
c.records[zoneID][ociRecordKey(*record.Rtype, *record.Domain)] = record
c.records[zoneID][ociRecordKey(*record.Rtype, *record.Domain, *record.Rdata)] = record
}
}

Expand Down Expand Up @@ -587,8 +587,12 @@ func (c *mutableMockOCIDNSClient) GetZoneRecords(ctx context.Context, request dn
return
}

func ociRecordKey(rType, domain string) string {
return rType + "/" + domain
func ociRecordKey(rType, domain string, ip string) string {
rdata := ""
if rType == "A" { // adds support for multi-targets with same rtype and domain
rdata = "_" + ip
}
return rType + "_" + domain + rdata
}

func (c *mutableMockOCIDNSClient) PatchZoneRecords(ctx context.Context, request dns.PatchZoneRecordsRequest) (response dns.PatchZoneRecordsResponse, err error) {
Expand All @@ -609,7 +613,7 @@ func (c *mutableMockOCIDNSClient) PatchZoneRecords(ctx context.Context, request
})

for _, op := range request.Items {
k := ociRecordKey(*op.Rtype, *op.Domain)
k := ociRecordKey(*op.Rtype, *op.Domain, *op.Rdata)
switch op.Operation {
case dns.RecordOperationOperationAdd:
records[k] = dns.Record{
Expand Down Expand Up @@ -850,21 +854,26 @@ func TestOCIApplyChanges(t *testing.T) {
Rtype: common.String(endpoint.RecordTypeA),
Ttl: common.Int(ociRecordTTL),
}, {
Domain: common.String("bar.foo.com"),
Domain: common.String("car.foo.com"),
Rdata: common.String("bar.com."),
Rtype: common.String(endpoint.RecordTypeCNAME),
Ttl: common.Int(ociRecordTTL),
}, {
Domain: common.String("bar.foo.com"),
Rdata: common.String("baz.com."),
Rtype: common.String(endpoint.RecordTypeCNAME),
Ttl: common.Int(ociRecordTTL),
}},
},
changes: &plan.Changes{
Delete: []*endpoint.Endpoint{endpoint.NewEndpointWithTTL(
"foo.foo.com",
endpoint.RecordTypeA,
endpoint.TTL(ociRecordTTL),
"baz.com.",
"127.0.0.1",
)},
UpdateOld: []*endpoint.Endpoint{endpoint.NewEndpointWithTTL(
"bar.foo.com",
"car.foo.com",
endpoint.RecordTypeCNAME,
endpoint.TTL(ociRecordTTL),
"baz.com.",
Expand Down Expand Up @@ -896,6 +905,65 @@ func TestOCIApplyChanges(t *testing.T) {
"127.0.0.1"),
},
},
{
name: "combine_multi_target",
zones: []dns.ZoneSummary{{
Id: common.String("ocid1.dns-zone.oc1..e1e042ef0bfbb5c251b9713fd7bf8959"),
Name: common.String("foo.com"),
}},

changes: &plan.Changes{
Create: []*endpoint.Endpoint{endpoint.NewEndpointWithTTL(
"foo.foo.com",
endpoint.RecordTypeA,
endpoint.TTL(ociRecordTTL),
"192.168.1.2",
), endpoint.NewEndpointWithTTL(
"foo.foo.com",
endpoint.RecordTypeA,
endpoint.TTL(ociRecordTTL),
"192.168.2.5",
)},
},
expectedEndpoints: []*endpoint.Endpoint{endpoint.NewEndpointWithTTL(
"foo.foo.com",
endpoint.RecordTypeA,
endpoint.TTL(ociRecordTTL), "192.168.1.2", "192.168.2.5",
)},
},
{
name: "remove_from_multi_target",
zones: []dns.ZoneSummary{{
Id: common.String("ocid1.dns-zone.oc1..e1e042ef0bfbb5c251b9713fd7bf8959"),
Name: common.String("foo.com"),
}},
records: map[string][]dns.Record{
"ocid1.dns-zone.oc1..e1e042ef0bfbb5c251b9713fd7bf8959": {{
Domain: common.String("foo.foo.com"),
Rdata: common.String("192.168.1.2"),
Rtype: common.String(endpoint.RecordTypeA),
Ttl: common.Int(ociRecordTTL),
}, {
Domain: common.String("foo.foo.com"),
Rdata: common.String("192.168.2.5"),
Rtype: common.String(endpoint.RecordTypeA),
Ttl: common.Int(ociRecordTTL),
}},
},
changes: &plan.Changes{
Delete: []*endpoint.Endpoint{endpoint.NewEndpointWithTTL(
"foo.foo.com",
endpoint.RecordTypeA,
endpoint.TTL(ociRecordTTL),
"192.168.1.2",
)},
},
expectedEndpoints: []*endpoint.Endpoint{endpoint.NewEndpointWithTTL(
"foo.foo.com",
endpoint.RecordTypeA,
endpoint.TTL(ociRecordTTL), "192.168.2.5",
)},
},
}

for _, tc := range testCases {
Expand Down