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.

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

Add RecordNS support #170

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

Add RecordNS support #170

wants to merge 1 commit into from

Conversation

k0da
Copy link

@k0da k0da commented Apr 8, 2022

This adds AddRecordNS and DeleteRecordNS operations, this is a
requirement to extend external-dns with NS record support.

Signed-off-by: Dinar Valeev dinar.valeev@absa.africa

This adds AddRecordNS and DeleteRecordNS operations, this is a
requirement to extend external-dns with NS record support.

Signed-off-by: Dinar Valeev <dinar.valeev@absa.africa>
Copy link
Collaborator

@hemanthKa677 hemanthKa677 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @k0da , Can you please provide us a working example of arguments passed to the CreateNSRecord() function?

var addresses []ZoneNameServer
for _, addr := range addr {
addresses = append(addresses, ZoneNameServer{Address: addr})
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @k0da , Can you please add some validations on errors that might rise?
Ex: if recordname == "" {
return nil, fmt.Errorf("recordname must not be empty")
}

var actualRef string
var err error
It("should pass expected NS record Ref to DeleteObject", func() {
actualRef, err = objMgr.DeleteTXTRecord(deleteRef)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @k0da , Please change this DeleteTXTRecord to DeleteNSRecord.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants