Add DNSZone controller for Designate DNS zone support - #956
Open
zunken1337 wants to merge 3 commits into
Open
zunken1337 wants to merge 3 commits into
zunken1337 wants to merge 3 commits into
Conversation
This was referenced Oct 2, 2026
This was referenced Oct 2, 2026
Collaborator
|
Thanks for splitting out the changes into multiple PRs, that's makes them more digestible, but we can still make them easier to review. Please have a look at #959 where I'm clarifying some of the implicit guidelines we had for contributing to the project. See in particular the commit structure, detailing how the new controllers PRs should be at least 3 commits (scaffolding, generated code, and finally implementation), and also the AI-assisted contributions. |
$ go run ./cmd/scaffold-controller -interactive=false \
-kind=DNSZone \
-gophercloud-client=NewDNSV2 \
-gophercloud-module=github.com/gophercloud/gophercloud/v2/openstack/dns/v2/zones \
-gophercloud-type=Zone
Registration in cmd/resource-generator and cmd/manager, scope wiring in internal/scope, and make generate and make generate-bundle output. No hand-written logic. The scaffolded stubs do not compile against this gophercloud version yet; the implementation commit replaces them.
Implements the scaffolded DNSZone controller on the Designate v2 zones API: PRIMARY and SECONDARY zones, adoption and import by name, and update of description, TTL and masters. Adds a DNSZone client to osclients, mock regeneration, apivalidation tests, and kuttl create, import and update tests. Credit: the original zone-transfer draft is k-orc#825 by @eshulman2, superseded here.
zunken1337
force-pushed
the
new-controller-dnszone
branch
from
October 6, 2026 05:48
f8fe340 to
5e9bd71
Compare
Contributor
Author
This branch has not been deployed
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.
Summary
Closes #954. First of three Designate controllers split out of #955 at @mandre's request
(#955 (comment)) -
+16k LOC in one PR was too large to review. This one adds
DNSZone(zone CRUD).RecordSet(#957) and
DNSZoneShare(#958) follow as separate PRs, both stacked on this one since theyextend the same shared
DNSClientthis PR introduces.Credit
This supersedes @eshulman2's stalled #825, which has been without activity since late June and
has accumulated merge conflicts against the current generic-controller framework. Per @mandre's
own suggestion on #825, this is a fresh implementation built from the current scaffolding rather
than a rebase of #825 - but #825's work is gratefully acknowledged: reviewing it first is what
surfaced two real gaps fixed here (a
projectIDstatus field missing entirely, andmasterstyped as an unvalidated
[]stringrather than a real IP type), and its own review thread(#825 (comment)) has the
full writeup. @eshulman2, if you'd rather finish #825 yourself instead, just say so - happy to
close this in favor of that.
Architecture notes
osclients.DNSClient, a single shared client for all Designate resources (the "oneservice, one client" convention
NetworkClientalready established for Neutron), rather thana dedicated client per kind. Zone-only for now;
RecordSetandDNSZoneShareeach extend thissame interface with their own methods in their own PRs.
DNSZone.spec.resource.namemust be explicit and end with a period - Designate rejectsanything else, and unlike most other resources there's no sensible object-name fallback (the
ORC object's own Kubernetes name won't have a trailing period).
Type(PRIMARY/SECONDARY) andMasters/TransferredAtmodel Designate's own DNS-protocolzone transfer (AXFR master/slave replication) - a different thing from Designate's
project-ownership zone transfer (
transfer_requests/transfer_accepts), which this PR doesnot implement. That's intentional scope control, not an oversight: project-ownership transfer
needs two different projects' credentials in one flow (see Enhancement: Glance image member sharing (grant + accept) #948's Image sharing for the same
shape), while
DNSZoneShare(next PR in this series) covers the single-credential alternativewe chose to support first.
Testing
a set, not positionally, since Designate doesn't guarantee return order).
test/apivalidationssuite extended with immutability, required-field, and PRIMARY/SECONDARY cross-field CEL validation tests.
go test ./...run (not just the package under activework): a curly/smart quote had snuck into one CEL rule's string literal, which would have made
the CRD uninstallable on any real Kubernetes API server (
controller-gendoesn't validate CELsyntax, it just transcribes the rule string - the breakage only surfaced when envtest tried to
actually install the CRD). Fixed by using
has(self.email)instead of comparing against anempty string literal, which also matches the convention every other CEL rule in this codebase
already uses.
never had it enabled - added
enable_plugin designate ...+DESIGNATE_BACKEND_DRIVER=bind9to
.github/workflows/e2e.yaml, same pattern already used for neutron/manila.Test plan
go build ./...,go vet ./...,gofmt -l .cleango test ./...passes (only unrelatedtest/e2e, which needs a live kind cluster, fails inthis environment)
make generateoutput committed (CRDs, deepcopy, adapters/controllers,applyconfiguration/clientset/lister/informer, mocks, API reference docs)
make lintclean (0 issues)🤖 Generated with Claude Code