[AISOS-1942] [ORC] Add openstack designate recordset controller to openstack-resource-controller by forgeSmith-bot · Pull Request #3 · eshulman2/openstack-resource-controller

forgeSmith-bot · 2026-06-25T17:38:00Z

Summary

This PR implements the DNSRecordset custom resource controller within the OpenStack Resource Controller (ORC) framework. It enables users to declare and manage OpenStack Designate DNS recordsets as native Kubernetes resources, specifying properties like records, types, and parent zone references with full lifecycle reconciliation.

Changes

API & Code Generation

Created api/v1alpha1/dnsrecordset_types.go declaring the DNSRecordset CRD schema, featuring OpenAPI kubebuilder validations (e.g., name period suffixes, and CEL validation rules making type and dnsZoneRef fields immutable), with the optional TTL field intentionally omitted from the Spec.
Registered the new DNSRecordset resource in cmd/resource-generator/main.go and ran code generation to produce clients, informers, listers, CRD schema bases, and deepcopy implementations.

Low-Level OpenStack Client Integration

Created the low-level DNSRecordsetClient interface and Gophercloud v2 implementation in internal/osclients/dnsrecordset.go.
Exposed the client via the provider scope framework in internal/scope/scope.go and internal/scope/provider.go.
Added mock mappings in internal/scope/mock.go and regenerated test mock files under internal/osclients/mock/.

Controller Actuator & Status Reconciliation

Created the controller setup in internal/controllers/dnsrecordset/controller.go, registering watchers for credentials, a deletion guard on the parent DNSZone dependency, and a simple dependency resolver for DNSZoneImport (redefined to remove its deletion guard and external finalizer registration, avoiding deletion hanging on imported zones).
Implemented actuator lifecycle hooks (Create, Read, Update, Delete, Import, Adoption) in internal/controllers/dnsrecordset/actuator.go, including a parallel deletion deadlock check in newActuator and DeleteResource to return zoneGone: true when the parent DNSZone is deleted or deleting.
Implemented extensive local DNS formatting validations in internal/controllers/dnsrecordset/validation.go (e.g., strict subdomain validation (exact match or dot prefix) to prevent suffix collision vulnerabilities, TXT quote checks, and A/AAAA IP patterns).
Created internal/controllers/dnsrecordset/status.go to handle status mappings and map Designate states (ACTIVE, PENDING, ERROR) to Kubernetes condition states.

Documentation

Updated website/docs/user-guide/dnszone.md (renamed to cover DNS Zones & Recordsets) to comprehensively detail the usage, attributes, and lifecycle patterns of the DNSRecordset resource.
Updated navigation structure in website/mkdocs.yml.

Implementation Notes

Dependency Ordering: Established parent DNSZone and DNSZoneImport dependency checks in the reconciler. Creation will safely wait if the referenced zone is not yet active, and a deletion guard prevents the parent DNSZone from being deleted while active child recordsets exist. To resolve deletion hanging on the imported DNSZone, the dnsZoneImportDependency was redefined as a simple dependency rather than a deletion guard, removing external finalizers and deletion guards from the imported zone. To handle parallel deletion (such as namespace deletion) without deadlocks, a parallel deletion deadlock check in newActuator and DeleteResource identifies when the parent DNSZone dependency is deleted or in the process of deletion, allowing the deletion actuator to return zoneGone: true to skip unnecessary OpenStack calls and allow smooth garbage collection.
Pre-flight Validations & Normalization: Syntactic and formatting validation checks are executed locally inside the actuator before calling OpenStack APIs, returning clear terminal errors (ConditionReasonInvalidConfiguration) for invalid configurations to avoid API-level failures. For name validation, it strictly enforces that the recordset name is either an exact match for the zone or has a dot prefix indicating a valid subdomain to prevent suffix collision vulnerabilities. Normalization of TXT record double-quotes is handled safely without in-place slice mutation to prevent read-only shared informer cache corruption.
Race Condition Resilience & Import Restrictions: Handled 409 Conflict duplicate creation races by attempting to list and adopt the existing out-of-band recordset if all its properties match the specification; if there is a properties mismatch, a terminal UnrecoverableError is correctly propagated. Property comparison and adoption mismatches in ListOSResourcesForAdoption also produce terminal errors on property mismatches. Additionally, defined a custom DNSRecordsetImport structure with only Filter (avoiding exposing ID) because OpenStack Designate recordset operations are scoped under a zone and require a parent DNSZone reference.

Testing

Low-level Client Tests: Verified low-level client errors and interface implementations via unit tests in dnsrecordset_test.go.
Controller Unit Tests: Created comprehensive mock-based unit tests in actuator_test.go, status_test.go, and validation_test.go verifying creation (SC-001), parent zone waiting (SC-002), parallel deletion zoneGone behavior, local validation formatting, and duplicate creation 409 Conflict properties match/mismatch handling (adoption on properties match, terminal failure mapping on mismatch).
API Validation Tests: Wrote API schema and management policy Ginkgo validation tests under test/apivalidations/dnsrecordset_test.go.
E2E Integration Testing: Generated and customized Kuttl E2E tests under internal/controllers/dnsrecordset/tests/ to verify recordset provisioning, using dnsZoneRef parent references, valid record types/names, and removing invalid dNSZoneID status assertions. Also resolved a DNS zone suffix mismatch in the dnsrecordset-dependency test specification, updated expected assertion messages in the dnsrecordset-import test, and updated the dnsrecordset-create-minimal test to assert the absence of the TTL field in the status block when omitted from the spec.
System Verification: Ran make build, make test, and make lint to confirm zero compilation, testing, or linting issues across the updated codebase.

Related Tickets

Generated by Forge SDLC Orchestrator

…Markers Detailed description: - Created and defined the hand-written DNSRecordset custom resource definitions in 'api/v1alpha1/dnsrecordset_types.go'. - Configured 'DNSRecordsetResourceSpec', 'DNSRecordsetFilter', and 'DNSRecordsetResourceStatus' with robust kubebuilder annotations (XValidation, MinLength, MaxLength, MinItems, MaxItems, and listType). - Updated 'cmd/resource-generator/main.go' to include DNSRecordset in the templates list and generated deepcopy, applyconfigurations, client interfaces, informers, listers, openapi docs, CRD reference docs, and controller files. - Added mock-capable openstack client interfaces for recordsets in 'internal/osclients/dnsrecordset.go' and declared a controller name placeholder in 'internal/controllers/dnsrecordset/controller.go' to ensure build compatibility. - Implemented and executed API validation test suites in 'test/apivalidations/dnsrecordset_test.go', which successfully verify required fields, name formatting (period suffix), and TTL limits. Closes: AISOS-1946

Detailed description: - Updated parameter names of DNSRecordsetClient interface and implementation in dnsrecordset.go to match the design. - Created dnsrecordset_test.go unit tests to verify the DNSRecordsetErrorClient returns configured errors. - Regenerated mock files using 'make generate-go'. Closes: AISOS-1948

…generate Mocks Detailed description: - Added NewDNSRecordsetClient() to the Scope interface in internal/scope/scope.go. - Implemented NewDNSRecordsetClient() in internal/scope/provider.go returning the dns recordset client wrapper. - Added MockDNSRecordsetClient mapping in MockScopeFactory within internal/scope/mock.go so tests can mock it. - Executed mock and code generation. Closes: AISOS-1949

Detailed description: - Created the entrypoint, reconciler setup, and controller registration for the DNSRecordset controller in internal/controllers/dnsrecordset/controller.go. - Implemented SetupWithManager to watch DNSRecordset events and set up the reconciler with the scope factory, dnsRecordsetHelperFactory, and dnsRecordsetStatusWriter. - Configured deletion guard dependency dnsZoneDependency on spec.resource.dnsZoneRef to block provisioning if parent zone is not ready, and prevent parent zone deletion if recordsets are active. - Registered the credentials watch and the parent DNSZone watch correctly in SetupWithManager. - Registered the new dnsrecordset controller in cmd/manager/main.go. Closes: AISOS-1950

Detailed description: - Created internal/controllers/dnsrecordset/actuator.go and moved/implemented all the generic actuator methods (GetResourceID, GetOSResourceByID, ListOSResourcesForImport, CreateResource, DeleteResource, and ListOSResourcesForAdoption) as well as the dnsRecordsetHelperFactory. - Created internal/controllers/dnsrecordset/status.go and implemented the dnsRecordsetStatusWriter to correctly map OpenStack Designate recordset status and update conditions. - Resolved and checked parent DNSZone dependencies in CreateResource (returning WaitingOnObject when empty/not present). - Handled property comparison and adoption mismatch in ListOSResourcesForAdoption, producing a terminal UnrecoverableError on mismatch. - Added comprehensive unit tests in actuator_test.go covering all cases. Closes: AISOS-1951

Detailed description: - Implemented ValidateDNSRecordset helper functions under internal/controllers/dnsrecordset/validation.go. - Implemented name suffix validation, ensuring the recordset name ends with the parent DNSZone's domain suffix. - Implemented type-specific format validations for A (IPv4), AAAA (IPv6), CNAME (trailing dot ending), and TXT records (mismatched quotes syntax check, and automatic double-quote wrapping if missing). - Integrated validation checks in newActuator, CreateResource, and updateResource. - Implemented updateResource in dnsRecordsetActuator to handle mutable configuration updates for recordsets (Description, TTL, Records) using gophercloud UpdateOpts. - Added comprehensive validation and actuator tests to verify all formatting rules and update lifecycles. Closes: AISOS-1952

…ates Detailed description: - Refactored internal/controllers/dnsrecordset/status.go to implement property mapping from recordsets.RecordSet to DNSRecordsetResourceStatusApplyConfiguration with appropriate checks for optional/zero fields. - Implemented ResourceAvailableStatus to correctly map ACTIVE, PENDING, ERROR, and unknown states to their respective Kubernetes Available and Progressing conditions, and handle retry intervals. - Created internal/controllers/dnsrecordset/status_test.go with comprehensive unit tests for ResourceAvailableStatus and ApplyResourceStatus. Closes: AISOS-1953

…ordset Controller Detailed description: - Added comprehensive unit tests in actuator_test.go covering successful creation/reconciliation (SC-001), parent zone dependency wait (SC-002), and duplicate creation 409 Conflict terminal failure/unrecoverable error mapping. - Verified that API schema validations, management policy choices, and invalid/empty fields are properly tested and pass successfully against the API server. Closes: AISOS-1954

Detailed description: - Added omitempty JSON tag to DNSZoneRef field in DNSRecordsetFilter struct to resolve kubeapilinter issues. - Ran make generate to regenerate client configs, schema validations, CRD manifests, and CRD reference documentation. - Formatted changed files with make fmt and verified build, compile, and linter runs successfully with zero issues. - Ran targeted unit tests for dnsrecordset controller verifying that all tests pass. Closes: AISOS-1942-review

Detailed description: - Added the required 'dnsZoneRef' property to 'applyValidFilter' in dnsrecordset_test.go - Fixed a breaking test failure ('should permit valid import filter' in common_test.go) Closes: AISOS-1942-review

…SRecordset resource Detailed description: - Updated the existing DNS zone documentation (website/docs/user-guide/dnszone.md) to comprehensively cover the new DNSRecordset Custom Resource, detailing its core concepts, parent zone references, validation rules, management policies, status conditions, and troubleshooting workflows. - Renamed the navigation item in website/mkdocs.yml and the heading of website/docs/user-guide/dnszone.md to 'DNS (Zones & Recordsets)'. - Verified that all changes compile and pass make lint perfectly. Closes: AISOS-1942-docs

Detailed description: - Created missing sample file config/samples/openstack_v1alpha1_dnsrecordset.yaml with valid fields. - Ran controller scaffolding tool for DNSRecordset and generated kuttl e2e tests in internal/controllers/dnsrecordset/tests/. - Restored original files modified by scaffolding, leaving only the newly created test directory, the sample file, and the updated bundle manifests. - Customized generated kuttl YAML test files to use dnsZoneRef instead of dNSZoneRef, set required type and records fields, valid end-with-period names, and removed invalid dNSZoneID status assertions. Closes: AISOS-1942-ci-fix

…former cache mutation Detailed description: - Removed in-place slice mutation of TXT record quotes inside ValidateDNSRecordset to prevent corruption of the read-only shared informer cache. - Defined getNormalizedRecords helper function in internal/controllers/dnsrecordset/actuator.go and used it across ListOSResourcesForAdoption, CreateResource, and updateResource. - Modified validation_test.go to verify correctness on the normalized records rather than testing for in-place mutation. - Added a Terminal error check in newActuator to handle Import.ID gracefully, as OpenStack Designate recordset operations are scoped under a zone and require a parent DNSZone reference. Closes: AISOS-1942-review-ci-fix-1

…ncy to fix controller manager startup Detailed description: - Added `dependency.OverrideDependencyName("dnszoneimport")` as an option to `dnsZoneImportDependency` in the dnsrecordset controller. - This gives the dependency a unique deletion guard controller name (`"dnszoneimport_deletion_guard_for_dnsrecordset"`), resolving the registration collision with `dnsZoneDependency`. Closes: AISOS-1942-ci-fix

…pendency utility Detailed description: - Added a check for '!obj.GetDeletionTimestamp().IsZero()' in EnsureFinalizer. - Key files modified: internal/util/dependency/dependency.go - This check ensures that when a dependency is undergoing deletion, the reconciler does not attempt to add a finalizer to it, which would otherwise result in a Forbidden invalid object error from the Kubernetes API server and block the reconciliation. Closes: AISOS-1942-ci-fix

…n DNSRecordset validation Detailed description: - Modified ValidateDNSRecordset in validation.go to strictly enforce that the recordset name is either an exact match for the zone (apex record) or has a dot prefix indicating a valid subdomain (e.g., '.example.com.'). - This prevents suffix collision vulnerabilities where a domain name ending with the zone name but not being a subdomain of it (e.g. 'notexample.com.' with zone 'example.com.') could be erroneously validated. - Added comprehensive unit test cases in validation_test.go to verify this behavior. Closes: AISOS-1942-review-ci-fix-3

…resolve E2E test specification mismatches Detailed description: - Implemented a parallel deletion deadlock check in dnsrecordset controller's 'newActuator' and 'DeleteResource' methods to identify when the parent 'DNSZone' dependency is deleted or in the process of deletion. - Returns 'zoneGone: true' from the deletion actuator to skip unnecessary OpenStack calls and allow smooth garbage collection of recordsets during namespace deletion. - Added comprehensive unit tests covering the 'zoneGone' behavior in 'actuator_test.go'. - Fixed a DNS zone suffix mismatch in the 'dnsrecordset-dependency' E2E test specification. - Updated expected test assertion messages under the 'dnsrecordset-import' E2E test. Closes: AISOS-1942-ci-fix

…pendencies Detailed description: - Appended suffix to finalizers in multiple deletion guard dependencies targeting the same resource type in the same controller. - Modified 'internal/controllers/dnsrecordset/controller.go' to use 'finalizer+"-import"' for 'dnsZoneImportDependency'. - Modified 'internal/controllers/server/controller.go' to use 'finalizer+"-bootvolume"' for 'bootVolumeDependency'. Closes: AISOS-1942-ci-analyze

… to resolve DNSZone deletion hanging Detailed description: - Redefined 'dnsZoneImportDependency' in internal/controllers/dnsrecordset/controller.go as a dependency.NewDependency rather than dependency.NewDeletionGuardDependency. This removes the deletion guard and external finalizer registration. - Updated internal/controllers/dnsrecordset/actuator.go to resolve this unmanaged/imported DNSZone dependency via dependency.FetchDependency. - This prevents adding external finalizers and deletion guards to the imported DNSZone, allowing the DNSZone to be deleted without hanging and resolving the dnsrecordset-import-dependency E2E test timeout failure. Closes: AISOS-1942-ci-fix

eshulman2

Controller/API review using the ORC controller review checklist. I focused on API contract, mutable fields, CRUD reconciliation, and KUTTL coverage.

forgeSmith-bot · 2026-06-29T08:31:56Z

Forge is addressing PR review feedback now. This status update is informational.

Detailed description: - Defined custom DNSRecordsetImport with only Filter and updated generator templates and main.go to avoid exposing ID - Added CEL validation rules to make type and dnsZoneRef fields immutable - Documented and tested intentional omission/removal of optional TTL from Spec - Reverted unrelated changes in Server controller and shared dependency utility - Removed unnecessary Client Wrapper test for DNSRecordset - Regenerated derived files and verified with make generate, make build, and make test Closes: AISOS-1942-review-fix

…dation fixes Detailed description: - Fixed kubeapilinter issue by adding a //nolint:kubeapilinter bypass directive to DNSRecordsetImport.Filter. - Added a nil safety guard in the generated resource template getResourceName helper in order to prevent nil-pointer dereferences across all controllers during import flow. - Regenerated all derived controller adapters, client configurations, and schemas using 'make generate'. - Verified that all unit tests, API validation tests, formatting, and linting pass successfully. Closes: AISOS-1942-review-review-impl

…TTL CEL expression Detailed description: - Modified 'internal/controllers/dnsrecordset/tests/dnsrecordset-create-minimal/00-assert.yaml' to change the CEL assertion from checking that 'ttl' exists to checking that it is absent ('!has(dnsrecordset.status.resource.ttl)'). - This matches the behavior of minimal DNSRecordset creation, where the TTL is omitted from the resource spec, and therefore omitted from status block. Closes: AISOS-1942-ci-analyze

forgeSmith-bot · 2026-06-29T12:13:53Z

Forge is addressing PR review feedback now. This status update is informational.

eshulman2 · 2026-06-30T12:19:29Z

+				} else if resourceSpec.TTL != nil && f.TTL != int(*resourceSpec.TTL) {
+					matches = false
+					mismatchMsg = fmt.Sprintf("TTL mismatch: OpenStack has %d, spec has %d", f.TTL, *resourceSpec.TTL)
+				} else if resourceSpec.Description != nil && f.Description != *resourceSpec.Description {


Adoption currently ignores description when spec.resource.description is omitted, but updateResource treats an omitted description as desired empty string and will clear a non-empty OpenStack description. That means a managed DNSRecordset with no status ID can adopt an existing same-name/type/records recordset and then mutate it. Please either require the OpenStack description to be empty for adoption when the spec omits it, matching the DNSZone pattern, or make omitted description explicitly unmanaged in update.

forgeSmith-bot · 2026-06-30T12:19:35Z

Forge is addressing PR review feedback now. This status update is informational.

eshulman2 · 2026-06-30T12:22:04Z

+
+In the `unmanaged` policy, ORC imports an existing DNS recordset from OpenStack Designate. ORC will **never** modify or delete the OpenStack resource; it is imported for read-only status propagation.
+
+You can import an existing recordset using either its **ID (UUID)** or by matching its **properties (filters)**.


This DNSRecordset import-by-ID documentation no longer matches the API: DNSRecordsetImport only exposes filter, so spec.import.id will be rejected by the CRD. Please remove the import-by-ID section for DNSRecordset and document that recordset import is filter-only with dnsZoneRef.

…flict resolution for DNSRecordset Detailed description: - Modified the DNSRecordset actuator to handle 409 Conflict when creating a recordset by attempting to list and adopt an existing out-of-band recordset if all its properties match the resource specification. - If there is a properties mismatch on conflict, a Terminal error with UnrecoverableError reason is correctly propagated, satisfying the duplicate creation race edge case behavior in the specification. - Added comprehensive unit tests to cover properties mismatch and properties match conflict scenarios. Closes: AISOS-1942-review-review-impl

Forge added 11 commits June 25, 2026 15:28

github-actions Bot added the semver:major Breaking change label Jun 25, 2026

Forge added 8 commits June 25, 2026 18:30

eshulman2 reviewed Jun 29, 2026

View reviewed changes

Forge added 3 commits June 29, 2026 08:49

eshulman2 reviewed Jun 29, 2026

View reviewed changes

Comment thread api/v1alpha1/dnsrecordset_types.go

eshulman2 reviewed Jun 29, 2026

View reviewed changes

Comment thread api/v1alpha1/dnsrecordset_types.go

Forge and others added 2 commits June 29, 2026 12:43

[AISOS-1942] review: address PR feedback

bbdc188

Increase Designate zone quota in e2e jobs

d69b45a

eshulman2 reviewed Jun 30, 2026

View reviewed changes

Forge added 2 commits June 30, 2026 12:39

[AISOS-1942] review: address PR feedback

f66265a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[AISOS-1942] [ORC] Add openstack designate recordset controller to openstack-resource-controller#3

[AISOS-1942] [ORC] Add openstack designate recordset controller to openstack-resource-controller#3
forgeSmith-bot wants to merge 26 commits into
eshulman2:mainfrom
forgeSmith-bot:forge/aisos-1942

forgeSmith-bot commented Jun 25, 2026 •

edited

Loading

Uh oh!

eshulman2 left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

forgeSmith-bot commented Jun 29, 2026

Uh oh!

Uh oh!

Uh oh!

forgeSmith-bot commented Jun 29, 2026

Uh oh!

eshulman2 Jun 30, 2026

Uh oh!

forgeSmith-bot commented Jun 30, 2026

Uh oh!

eshulman2 Jun 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		In the `unmanaged` policy, ORC imports an existing DNS recordset from OpenStack Designate. ORC will never modify or delete the OpenStack resource; it is imported for read-only status propagation.

		You can import an existing recordset using either its ID (UUID) or by matching its properties (filters).

Conversation

forgeSmith-bot commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

API & Code Generation

Low-Level OpenStack Client Integration

Controller Actuator & Status Reconciliation

Documentation

Implementation Notes

Testing

Related Tickets

Uh oh!

eshulman2 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

forgeSmith-bot commented Jun 29, 2026

Uh oh!

Uh oh!

Uh oh!

forgeSmith-bot commented Jun 29, 2026

Uh oh!

eshulman2 Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

forgeSmith-bot commented Jun 30, 2026

Uh oh!

eshulman2 Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

forgeSmith-bot commented Jun 25, 2026 •

edited

Loading