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

Jump to bottom

OCPBUGS-32776: Fix IBM Public Cloud DNS Provider Update Logic #1133

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

OCPBUGS-32776: Fix IBM Public Cloud DNS Provider Update Logic #1133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0b9e89b
Clear IBM DNS Provider Unit Tests CallHistory
gcs278 Aug 22, 2024
f988aad
Add Missing Test Cases in IBM Cloud Unit Tests
gcs278 Aug 22, 2024
2223383
OCPBUGS-32776: Fix IBM Public Cloud DNS Provider Update Logic
gcs278 Aug 22, 2024
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
74 changes: 30 additions & 44 deletions pkg/dns/ibm/private/client/fake_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,33 +5,37 @@ import (

"github.com/IBM/go-sdk-core/v5/core"
"github.com/IBM/networking-go-sdk/dnssvcsv1"
iov1 "github.com/openshift/api/operatoringress/v1"
)

type FakeDnsClient struct {
CallHistory map[string]string
DeleteDnsRecordInputOutput DeleteDnsRecordInputOutput
ListAllDnsRecordsInputOutput ListAllDnsRecordsInputOutput
UpdateDnsRecordInputOutput UpdateDnsRecordInputOutput
CreateDnsRecordInputOutput CreateDnsRecordInputOutput
}

type DeleteDnsRecordInputOutput struct {
InputId string
OutputError error
OutputStatusCode int
InputId string
OutputError error
OutputResponse *core.DetailedResponse
}

type UpdateDnsRecordInputOutput struct {
InputId string
OutputError error
OutputStatusCode int
InputId string
OutputError error
OutputResponse *core.DetailedResponse
}
type CreateDnsRecordInputOutput struct {
InputId string
OutputError error
OutputResponse *core.DetailedResponse
}

type ListAllDnsRecordsInputOutput struct {
RecordName string
RecordTarget string
OutputError error
OutputStatusCode int
OutputResult *dnssvcsv1.ListResourceRecords
OutputError error
OutputResponse *core.DetailedResponse
}

func NewFake() (*FakeDnsClient, error) {
Expand All @@ -43,24 +47,15 @@ func (c *FakeDnsClient) RecordedCall(zoneID string) (string, bool) {
return call, ok
}

func (c *FakeDnsClient) ClearCallHistory() {
c.CallHistory = map[string]string{}
}

func (FakeDnsClient) NewListResourceRecordsOptions(instanceID string, dnszoneID string) *dnssvcsv1.ListResourceRecordsOptions {
return &dnssvcsv1.ListResourceRecordsOptions{}
}
func (fdc FakeDnsClient) ListResourceRecords(listResourceRecordsOptions *dnssvcsv1.ListResourceRecordsOptions) (result *dnssvcsv1.ListResourceRecords, response *core.DetailedResponse, err error) {
fakeListDnsrecordsResp := &dnssvcsv1.ListResourceRecords{}
recordType := string(iov1.ARecordType)
rData := map[string]interface{}{"ip": fdc.ListAllDnsRecordsInputOutput.RecordTarget}

fakeListDnsrecordsResp.ResourceRecords = append(fakeListDnsrecordsResp.ResourceRecords, dnssvcsv1.ResourceRecord{ID: &fdc.ListAllDnsRecordsInputOutput.RecordName, Name: &fdc.ListAllDnsRecordsInputOutput.RecordName, Type: &recordType, Rdata: rData})

resp := &core.DetailedResponse{
StatusCode: fdc.ListAllDnsRecordsInputOutput.OutputStatusCode,
Headers: map[string][]string{},
Result: result,
RawResult: []byte{},
}

return fakeListDnsrecordsResp, resp, fdc.ListAllDnsRecordsInputOutput.OutputError
return fdc.ListAllDnsRecordsInputOutput.OutputResult, fdc.ListAllDnsRecordsInputOutput.OutputResponse, fdc.ListAllDnsRecordsInputOutput.OutputError
}
func (FakeDnsClient) NewDeleteResourceRecordOptions(instanceID string, dnszoneID string, recordID string) *dnssvcsv1.DeleteResourceRecordOptions {
return &dnssvcsv1.DeleteResourceRecordOptions{InstanceID: &instanceID, DnszoneID: &dnszoneID, RecordID: &recordID}
Expand All @@ -70,15 +65,8 @@ func (fdc FakeDnsClient) DeleteResourceRecord(deleteResourceRecordOptions *dnssv
return nil, errors.New("deleteDnsRecord: inputs don't match")
}

resp := &core.DetailedResponse{
StatusCode: fdc.DeleteDnsRecordInputOutput.OutputStatusCode,
Headers: map[string][]string{},
Result: response,
RawResult: []byte{},
}

fdc.CallHistory[*deleteResourceRecordOptions.RecordID] = "DELETE"
return resp, fdc.DeleteDnsRecordInputOutput.OutputError
return fdc.DeleteDnsRecordInputOutput.OutputResponse, fdc.DeleteDnsRecordInputOutput.OutputError
}
func (FakeDnsClient) NewUpdateResourceRecordOptions(instanceID string, dnszoneID string, recordID string) *dnssvcsv1.UpdateResourceRecordOptions {
return &dnssvcsv1.UpdateResourceRecordOptions{InstanceID: &instanceID, DnszoneID: &dnszoneID, RecordID: &recordID}
Expand All @@ -94,27 +82,25 @@ func (fdc FakeDnsClient) UpdateResourceRecord(updateResourceRecordOptions *dnssv
return nil, nil, errors.New("updateDnsRecord: inputs don't match")
}

resp := &core.DetailedResponse{
StatusCode: fdc.UpdateDnsRecordInputOutput.OutputStatusCode,
Headers: map[string][]string{},
Result: response,
RawResult: []byte{},
}

fdc.CallHistory[*updateResourceRecordOptions.RecordID] = "PUT"
return nil, resp, fdc.UpdateDnsRecordInputOutput.OutputError
return nil, fdc.UpdateDnsRecordInputOutput.OutputResponse, fdc.UpdateDnsRecordInputOutput.OutputError
}
func (FakeDnsClient) NewCreateResourceRecordOptions(instanceID string, dnszoneID string) *dnssvcsv1.CreateResourceRecordOptions {
return nil
return &dnssvcsv1.CreateResourceRecordOptions{}
}
func (FakeDnsClient) NewResourceRecordInputRdataRdataCnameRecord(cname string) (_model *dnssvcsv1.ResourceRecordInputRdataRdataCnameRecord, err error) {
return nil, nil
}
func (FakeDnsClient) NewResourceRecordInputRdataRdataARecord(ip string) (_model *dnssvcsv1.ResourceRecordInputRdataRdataARecord, err error) {
return nil, nil
}
func (FakeDnsClient) CreateResourceRecord(createResourceRecordOptions *dnssvcsv1.CreateResourceRecordOptions) (result *dnssvcsv1.ResourceRecord, response *core.DetailedResponse, err error) {
return nil, nil, nil
func (fdc FakeDnsClient) CreateResourceRecord(createResourceRecordOptions *dnssvcsv1.CreateResourceRecordOptions) (result *dnssvcsv1.ResourceRecord, response *core.DetailedResponse, err error) {
if fdc.CreateDnsRecordInputOutput.InputId != *createResourceRecordOptions.Name {
return nil, nil, errors.New("createResourceRecord: inputs don't match")
}

fdc.CallHistory[*createResourceRecordOptions.Name] = "CREATE"
return nil, fdc.CreateDnsRecordInputOutput.OutputResponse, fdc.CreateDnsRecordInputOutput.OutputError
}
func (FakeDnsClient) NewGetDnszoneOptions(instanceID string, dnszoneID string) *dnssvcsv1.GetDnszoneOptions {
return nil
Expand Down
118 changes: 64 additions & 54 deletions pkg/dns/ibm/private/dnssvcs_provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,13 +126,17 @@ func (p *Provider) Delete(record *iov1.DNSRecord, zone configv1.DNSZone) error {
default:
return fmt.Errorf("delete: resource data has record with unknown type: %v", *resourceRecord.Type)
}

// While creating DNS records with multiple targets is unsupported, we still
// iterate through all targets during deletion to be extra cautious.
for _, target := range record.Spec.Targets {
if *resourceRecord.Name == dnsName {
if resourceRecordTarget != target {
log.Info("delete: ignoring record with matching name but unexpected target", "record", record, "target", resourceRecordTarget)
continue
}
if resourceRecord.ID == nil {
return fmt.Errorf("delete: record id is nil")
}
delOpt := p.dnsService.NewDeleteResourceRecordOptions(p.config.InstanceID, zone.ID, *resourceRecord.ID)
delResponse, err := p.dnsService.DeleteResourceRecord(delOpt)
if err != nil {
Expand Down Expand Up @@ -190,6 +194,10 @@ func (p *Provider) createOrUpdateDNSRecord(record *iov1.DNSRecord, zone configv1
log.Info("Warning: TTL must be one of [1 60 120 300 600 900 1800 3600 7200 18000 43200]. RecordTTL set to default", "default DSNSVCS record TTL", defaultDNSSVCSRecordTTL)
record.Spec.RecordTTL = defaultDNSSVCSRecordTTL
}
// We only support one target, warn the user.
if len(record.Spec.Targets) > 1 {
log.Info("Warning: Only one DNSRecord target is supported. Additional targets will be ignored.", "targets", record.Spec.Targets)
}

listResult, response, err := p.dnsService.ListResourceRecords(listOpt)
if err != nil {
Expand All @@ -201,72 +209,74 @@ func (p *Provider) createOrUpdateDNSRecord(record *iov1.DNSRecord, zone configv1
return fmt.Errorf("createOrUpdateDNSRecord: ListResourceRecords returned nil as result")
}

for _, target := range record.Spec.Targets {
updated := false
for _, resourceRecord := range listResult.ResourceRecords {
if *resourceRecord.Name == dnsName {
updateOpt := p.dnsService.NewUpdateResourceRecordOptions(p.config.InstanceID, zone.ID, *resourceRecord.ID)
updateOpt.SetName(dnsName)

if resourceRecord.Type == nil {
return fmt.Errorf("createOrUpdateDNSRecord: failed to get resource type, resourceRecord.Type is nil")
}
target := record.Spec.Targets[0]
updated := false
for _, resourceRecord := range listResult.ResourceRecords {
if *resourceRecord.Name == dnsName {
if resourceRecord.ID == nil {
return fmt.Errorf("createOrUpdateDNSRecord: record id is nil")
}
updateOpt := p.dnsService.NewUpdateResourceRecordOptions(p.config.InstanceID, zone.ID, *resourceRecord.ID)
updateOpt.SetName(dnsName)

// TODO DNS record update should handle the case where we have an A record and want a CNAME record or vice versa
switch *resourceRecord.Type {
case string(iov1.CNAMERecordType):
inputRData, err := p.dnsService.NewResourceRecordUpdateInputRdataRdataCnameRecord(target)
if err != nil {
return fmt.Errorf("createOrUpdateDNSRecord: failed to create CNAME inputRData for the dns record: %w", err)
}
updateOpt.SetRdata(inputRData)
case string(iov1.ARecordType):
inputRData, err := p.dnsService.NewResourceRecordUpdateInputRdataRdataARecord(target)
if err != nil {
return fmt.Errorf("createOrUpdateDNSRecord: failed to create A inputRData for the dns record: %w", err)
}
updateOpt.SetRdata(inputRData)
default:
return fmt.Errorf("createOrUpdateDNSRecord: resource data has record with unknown type: %v", *resourceRecord.Type)
}
updateOpt.SetTTL(record.Spec.RecordTTL)
_, _, err := p.dnsService.UpdateResourceRecord(updateOpt)
if err != nil {
return fmt.Errorf("createOrUpdateDNSRecord: failed to update the dns record: %w", err)
}
updated = true
log.Info("updated DNS record", "record", record.Spec, "zone", zone, "target", target)
if resourceRecord.Type == nil {
return fmt.Errorf("createOrUpdateDNSRecord: failed to get resource type, resourceRecord.Type is nil")
}
}
if !updated {
createOpt := p.dnsService.NewCreateResourceRecordOptions(p.config.InstanceID, zone.ID)
createOpt.SetName(dnsName)
createOpt.SetType(string(record.Spec.RecordType))

switch record.Spec.RecordType {
case iov1.CNAMERecordType:
inputRData, err := p.dnsService.NewResourceRecordInputRdataRdataCnameRecord(target)

// TODO DNS record update should handle the case where we have an A record and want a CNAME record or vice versa
switch *resourceRecord.Type {
case string(iov1.CNAMERecordType):
inputRData, err := p.dnsService.NewResourceRecordUpdateInputRdataRdataCnameRecord(target)
if err != nil {
return fmt.Errorf("createOrUpdateDNSRecord: failed to create CNAME inputRData for the dns record: %w", err)
}
createOpt.SetRdata(inputRData)
case iov1.ARecordType:
inputRData, err := p.dnsService.NewResourceRecordInputRdataRdataARecord(target)
updateOpt.SetRdata(inputRData)
case string(iov1.ARecordType):
inputRData, err := p.dnsService.NewResourceRecordUpdateInputRdataRdataARecord(target)
if err != nil {
return fmt.Errorf("createOrUpdateDNSRecord: failed to create A inputRData for the dns record: %w", err)
}
createOpt.SetRdata(inputRData)
updateOpt.SetRdata(inputRData)
default:
return fmt.Errorf("createOrUpdateDNSRecord: resource data has record with unknown type: %v", record.Spec.RecordType)

return fmt.Errorf("createOrUpdateDNSRecord: resource data has record with unknown type: %v", *resourceRecord.Type)
}
updateOpt.SetTTL(record.Spec.RecordTTL)
_, _, err := p.dnsService.UpdateResourceRecord(updateOpt)
if err != nil {
return fmt.Errorf("createOrUpdateDNSRecord: failed to update the dns record: %w", err)
}
updated = true
log.Info("updated DNS record", "record", record.Spec, "zone", zone, "target", target)
}
}
if !updated {
createOpt := p.dnsService.NewCreateResourceRecordOptions(p.config.InstanceID, zone.ID)
createOpt.SetName(dnsName)
createOpt.SetType(string(record.Spec.RecordType))

switch record.Spec.RecordType {
case iov1.CNAMERecordType:
inputRData, err := p.dnsService.NewResourceRecordInputRdataRdataCnameRecord(target)
if err != nil {
return fmt.Errorf("createOrUpdateDNSRecord: failed to create CNAME inputRData for the dns record: %w", err)
}
createOpt.SetTTL(record.Spec.RecordTTL)
_, _, err := p.dnsService.CreateResourceRecord(createOpt)
createOpt.SetRdata(inputRData)
case iov1.ARecordType:
inputRData, err := p.dnsService.NewResourceRecordInputRdataRdataARecord(target)
if err != nil {
return fmt.Errorf("createOrUpdateDNSRecord: failed to create the dns record: %w", err)
return fmt.Errorf("createOrUpdateDNSRecord: failed to create A inputRData for the dns record: %w", err)
}
log.Info("created DNS record", "record", record.Spec, "zone", zone, "target", target)
createOpt.SetRdata(inputRData)
default:
return fmt.Errorf("createOrUpdateDNSRecord: resource data has record with unknown type: %v", record.Spec.RecordType)

}
createOpt.SetTTL(record.Spec.RecordTTL)
_, _, err := p.dnsService.CreateResourceRecord(createOpt)
if err != nil {
return fmt.Errorf("createOrUpdateDNSRecord: failed to create the dns record: %w", err)
}
log.Info("created DNS record", "record", record.Spec, "zone", zone, "target", target)
}
return nil
}
Loading