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

change Remove by RemoveName to fix DeleteRecords not working #2

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

Conversation

mcarbonneaux
Copy link

after the issue: #1

i've tested replacing Remove by RemoveName and is working fine (i use bind as dns server).

this example work fine (we need to specify Type record with Name, without that we crash on RemoveName because rr is nil).

// delete records 
deletedRecs, err := provider.DeleteRecords(ctx, zone, []libdns.Record{
{
	Name: "sub",
	Type: "A",
},
})

@mholt
Copy link

mholt commented Dec 13, 2023

@mcarbonneaux Let me know if you don't hear from @KoHcoJlb soon -- then ping me, and I assume your change is a good fix and will merge it.

@conblem
Copy link

conblem commented Mar 19, 2024

In that case could this be merged now?

@mholt
Copy link

mholt commented Mar 20, 2024

I just noticed that the implementation by @emersion (which is nearly identical to this repo, oops -- we're working on addressing that soon) uses Remove():

https://github.com/libdns/dnsupdate/blob/2e79c50ea2ee12b484ae312b13e4ac81ffe78623/provider.go#L135

Are we sure that deleting all the RRSets is right?

@KoHcoJlb
Copy link
Collaborator

I've also checked this today.
RemoveName would remove all records with the same name, which I believe is non intended usage of the interface.
Separate method could be added for this, or just simple Get-Filter-Delete)

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.

4 participants