Upgrade to libdns 1.1.1, rely on external-dns libs, remove weight & prio labels, use stdlib slog, build using go build, revert to golangci-lint standard config#2
Open
andsens wants to merge 21 commits intoproject0:mainfrom
Conversation
Remove management of weights and priorities, I *think* that is just part of the target, so no special handling is needed
Author
Scratch that. Vendors like INWX rather infuriatingly separate out the preference/priority field in their API. However, there is no need to special case things: That is the job of the libdns provider, and it already does that: https://github.com/libdns/inwx/blob/ab63b13631a13a592aafaf3e04b327b38c7ac0bc/provider.go#L242-L249 So the only thing we need to do is call I also noticed that I only propagated the first target in the list, that is fixed now as well. Please let me know if maintaining this with all my changes is too much. That'd be perfectly understandable. |
6bde5e4 to
0897adb
Compare
…l changed records does not match
The parsed type does not contain the Type field, which is rather important for looking up records e.g. https://github.com/libdns/inwx/blob/ab63b13631a13a592aafaf3e04b327b38c7ac0bc/client.go#L149
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
OK, soooo... I've made a "few" changes
First off: It works! :-)
This is quite a rewrite mainly motivated by relying on the external-dns webhook library instead of the code that was in
internal/externaldns. That alone removes a whole bunch of code (and likely brings with it quite a few bugfixes just by running external-dns v0.19.0).I have updated libdns from
v0.2.3tov1.1.1. That bump introduced a major API change, which not all providers support yet, so I've had to remove those (I also had to add a small go mod replacement for the INWX provider to fix an API inconsistencymerged to upstream in ab63b136).In total 18 out of the 42 providers are now commented out in
go.mod.I was able to reproduce the issue you mentioned regarding labels. As far as I understand, the weight and priority of an e.g. SRV record is part of the
targetfield. So I don't really see the point of managing those separately. Additionally, the ID field was not used for anything, so I remove that as well. This all means no more provider specific labels.In conclusion the whole issue is gone because I simply nuked the code from orbit :-)
The linting rules were really hard to manage after I migrated to golangci-lint v2, so I just reverted it to the default config (I think that's a better approach anyways, standard setups are always easier to contribute to).
I replaced
zerologwithslog. You had a commit yourself where you reduced external dependencies. Seeing howslogcan do the same kind of structure logging it seemed like a fair replacement to reduce deps.gorelease works really well if you use it fully. The half-way approach I introduced in #1 where Github actions invokes gorelease, it outputs the binaries and
docker/build-push-actionbuilds the images introduces more workaround code than it saves. So I reverted to standardgo buildand usedebug.ReadBuildInfo()to display the version (go buildautomatically embeds VCS info into the binary when available). This has the added benefit of being standard tooling that everybody knows.All in all around 320 lines of non-generated code have been obsoleted.
I'm running this in my cluster right now, and it looks to be working well! I'll report back once I have loaded it with a bit more data.
p.s.: Have you considered asking the external-dns team to take over this project so it has some more visibility? They have stopped accepting PRs for new providers and recommend using webhooks. If they embraced your approach people could maybe focus on implementing things on libdns, which would benefit way more projects than just external-dns.