Sitelet https://github.com/k-orc/openstack-resource-controller/pull/956
Skip to content

Add DNSZone controller for Designate DNS zone support - #956

Open
zunken1337 wants to merge 3 commits into
k-orc:mainfrom
zunken1337:new-controller-dnszone
Open

zunken1337 wants to merge 3 commits into
k-orc:mainfrom
zunken1337:new-controller-dnszone

Conversation

@zunken1337

@zunken1337 zunken1337 commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

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 they
extend the same shared DNSClient this 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 projectID status field missing entirely, and masters
typed as an unvalidated []string rather 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

  • Introduces osclients.DNSClient, a single shared client for all Designate resources (the "one
    service, one client" convention NetworkClient already established for Neutron), rather than
    a dedicated client per kind. Zone-only for now; RecordSet and DNSZoneShare each extend this
    same interface with their own methods in their own PRs.
  • DNSZone.spec.resource.name must be explicit and end with a period - Designate rejects
    anything 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) and Masters/TransferredAt model Designate's own DNS-protocol
    zone transfer (AXFR master/slave replication) - a different thing from Designate's
    project-ownership zone transfer (transfer_requests/transfer_accepts), which this PR does
    not 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 alternative
    we chose to support first.

Testing

  • Unit test for the one mutable-field update path (description/ttl/masters, masters compared as
    a set, not positionally, since Designate doesn't guarantee return order).
  • Full test/apivalidations suite extended with immutability, required-field, and PRIMARY/
    SECONDARY cross-field CEL validation tests.
  • All 5 required KUTTL e2e scenarios.
  • Caught and fixed a real bug via the full go test ./... run (not just the package under active
    work): 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-gen doesn't validate CEL
    syntax, 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 an
    empty string literal, which also matches the convention every other CEL rule in this codebase
    already uses.
  • This is the first K-ORC controller to need Designate, so the e2e workflow's devstack deployment
    never had it enabled - added enable_plugin designate ... + DESIGNATE_BACKEND_DRIVER=bind9
    to .github/workflows/e2e.yaml, same pattern already used for neutron/manila.

Test plan

  • go build ./..., go vet ./..., gofmt -l . clean
  • go test ./... passes (only unrelated test/e2e, which needs a live kind cluster, fails in
    this environment)
  • make generate output committed (CRDs, deepcopy, adapters/controllers,
    applyconfiguration/clientset/lister/informer, mocks, API reference docs)
  • make lint clean (0 issues)
  • KUTTL e2e suite run against a real OpenStack cloud in CI

🤖 Generated with Claude Code

@mandre

mandre commented Oct 5, 2026

Copy link
Copy Markdown
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
zunken1337 force-pushed the new-controller-dnszone branch from f8fe340 to 5e9bd71 Compare October 6, 2026 05:48
@zunken1337

Copy link
Copy Markdown
Contributor Author

Restructured per #959: three commits, scaffolding (raw output, with the command in the message), generated code, and then the implementation. Commits are authored by my own account. Stacked #957 and #958 will follow the same structure.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver:major Breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Designate DNS zone + record support (DNSZone, DNSZoneShare, RecordSet)

2 participants