#4141 - Look up nameservers in the database to use for updating the registry - [aa] #4547

abe-alam-ecs · 2026-01-07T14:06:53Z

Ticket

Resolves #4141

Changes

Nameservers now read from the database
dns_setup no longer returns nameservers
added helper to fetch nameservers from DB
updated views to stop relying on return values
updated tests

Context for reviewers

Setup

To test, approve a new domain request or find a domain that needs DNS
Ensure that you are a manager of the domain and the waffle flag for dns_hosting is on.
Navigate to the domain > DNS > Records
Add a record
In the local database, you should see rainbow dns nameservers that match the mock.
You can also test successful dns_setup by doing:
docker compose exec app ./manage.py shell
from registrar.models import DnsZone
zone = DnsZone.objects.get(name="<domain_name>")
zone.nameservers

This will print out the nameservers (['rainbow.dns.gov', 'rainbow2.dns.gov'])

Code Review Verification Steps

As the original developer, I have

Satisfied acceptance criteria and met development standards

Met the acceptance criteria, or will meet them in a subsequent PR
Created/modified automated tests
Update documentation in READMEs and/or onboarding guide

Ensured code standards are met (Original Developer)

If any updated dependencies on Pipfile, also update dependencies in requirements.txt.
Interactions with external systems are wrapped in try/except
Error handling exists for unusual or missing values

Validated user-facing changes (if applicable)

Follow the process for requesting a design review. If code is not user-facing, delete design reviewer checklist
Verify new pages have been added to .pa11yci file so that they will be tested with our automated accessibility testing
Checked keyboard navigability
Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI)

As a code reviewer, I have

Reviewed, tested, and left feedback about the changes

Pulled this branch locally and tested it
Verified code meets all checks above. Address any checks that are not satisfied
Reviewed this code and left comments. Indicate if comments must be addressed before code is merged
Checked that all code is adequately covered by tests
Verify migrations are valid and do not conflict with existing migrations

Validated user-facing changes as a developer

Note: Multiple code reviewers can share the checklists above, a second reviewer should not make a duplicate checklist. All checks should be checked before approving, even those labeled N/A.

New pages have been added to .pa11yci file so that they will be tested with our automated accessibility testing
Checked keyboard navigability
Meets all designs and user flows provided by design/product
Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI)
(Rarely needed) Tested as both an analyst and applicant user

As a designer reviewer, I have

Verified that the changes match the design intention

Checked that the design translated visually
Checked behavior. Comment any found issues or broken flows.
Checked different states (empty, one, some, error)
Checked for landmarks, page heading structure, and links

Validated user-facing changes as a designer

Checked keyboard navigability
Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI)
Tested with multiple browsers (check off which ones were used)
- Chrome
- Microsoft Edge
- FireFox
- Safari
(Rarely needed) Tested as both an analyst and applicant user

References

Code review best practices

Screenshots

…ervers-in-db

github-actions · 2026-01-07T14:11:18Z

🥳 Successfully deployed to developer sandbox aa.

…ervers-in-db

github-actions · 2026-01-07T14:59:29Z

🥳 Successfully deployed to developer sandbox aa.

github-actions · 2026-01-07T15:15:54Z

🥳 Successfully deployed to developer sandbox aa.

kimallen

Looks good- thank you for the detailed instructions in the PR description (helpful after a couple weeks of vacation!)

There's a few minor nits, a question about a test, and a suggested refactor to simplify.

kimallen · 2026-01-07T23:42:41Z

src/registrar/services/dns_host_service.py

+            return
+
+        try:
+            nameservers, zone_data = self._find_existing_zone_in_cf(domain_name, x_account_id)


You can remove nameservers from here now that we are not returning it from the function

kimallen · 2026-01-07T23:43:39Z

src/registrar/services/dns_host_service.py

        # For now, we only expect one zone per account
        has_zone = DnsZone.objects.filter(name=domain_name).exists()
        # Remove after we are getting nameservers from db
        _, nameservers = self.get_x_zone_id_if_zone_exists(domain_name)


You can remove this line and the comment above

kimallen · 2026-01-08T00:15:11Z

src/registrar/views/domain.py

                    zone_name = domain_name
-                    # post nameservers to registry
+
+                    nameservers = self.dns_host_service._get_nameservers_from_db(domain_name)


SInce you are already doing a query on zones above, you could simplify this (and eliminate the addtional query in the helper function) by doing something like this:

zones = DnsZone.objects.filter(name=domain_name) if zones.exists(): zone = zones.first() nameservers = zone.nameservers if not nameservers: logger.error(f"No nameservers found in DB for domain {domain_name}") return JsonResponse( {"status": "error", "message": "DNS nameservers not available"}, status=400, ) try: self.dns_host_service.register_nameservers(zone.name, nameservers)

kimallen · 2026-01-08T00:31:01Z

src/registrar/services/dns_host_service.py

-                    raise
-
-        return nameservers
+        logger.info(f"DNS setup completed successfully for domain {domain_name}")


I appreciate this additional log! It helped me when I was testing to ensure it worked.

kimallen · 2026-01-08T00:32:03Z

src/registrar/services/dns_host_service.py

        """Find an item by name in a list of dictionaries."""
        return next((item.get("name_servers") for item in items if item.get("id") == x_zone_id), None)

+    def _get_nameservers_from_db(self, domain_name) -> list[str]:


I like the thinking around creating a helper function here. I made a comment about simplifying our queries so that this actually wouldn't be necessary.

kimallen · 2026-01-08T00:45:22Z

src/registrar/tests/services/test_dns_host_service.py

@@ -65,7 +63,7 @@
                "cf_zone": None,


Noting that we don't actually use either of these values in the test (cf_account and cf_zone) in from any of the test cases. Would you mind removing these from the test cases?

kimallen · 2026-01-08T00:48:00Z

src/registrar/tests/services/test_dns_host_service.py


        for case in test_cases:
            with self.subTest(msg=case["test_name"], **case):
+                if case["test_name"] != "has db account, has db zone":


I'm not sure I understand why this was added- why do we delete the zone here if the test case is that it has a zone?

Earlier in the test, we setup
DnsZone.objects.create( domain=domain, dns_account=dns_acc, name=domain_name, nameservers=["ex1.dns.gov", "ex2.dns.gov"] )

I remove that Zone so that it can't be pulled from the table in the cases where there is no db zone.

I guess the alternative would be to do something like...
if case["test_name"] == "has db account, has db zone":
and then create the zone here.

I think I like that second option better, it makes more sense and is easier to read. Should I add a comment too, to make that clear?

…ervers-in-db

…ccount" logic

github-actions · 2026-01-09T18:59:46Z

🥳 Successfully deployed to developer sandbox aa.

abe-alam-ecs added 3 commits December 31, 2025 11:41

updates to remove nameservers

79bdf4f

Merge remote-tracking branch 'origin/main' into aa/4141-look-up-names…

21069d8

…ervers-in-db

additional nameserver update (missed previously)

3a8fd5c

github-project-automation bot added this to .gov Product Board Jan 7, 2026

abe-alam-ecs added 2 commits January 7, 2026 09:54

linter fixes, logging addition

312d9cf

Merge remote-tracking branch 'origin/main' into aa/4141-look-up-names…

e981c73

…ervers-in-db

additional linter fix

41a3a43

abe-alam-ecs changed the title ~~[DRAFT] - #4141 - Look up nameservers in the database to use for updating the registry [aa]~~ #4141 - Look up nameservers in the database to use for updating the registry - [aa] Jan 7, 2026

kimallen requested changes Jan 8, 2026

View reviewed changes

github-project-automation bot moved this to 👀 In review in .gov Product Board Jan 8, 2026

kimallen self-assigned this Jan 8, 2026

abe-alam-ecs added 2 commits January 9, 2026 10:08

Merge remote-tracking branch 'origin/main' into aa/4141-look-up-names…

be992a2

…ervers-in-db

Updates to logic. Need to update has "no db account or zone, has cf a…

39cd085

…ccount" logic

#4141 - Look up nameservers in the database to use for updating the registry - [aa] #4547

Are you sure you want to change the base?

#4141 - Look up nameservers in the database to use for updating the registry - [aa] #4547

Conversation

abe-alam-ecs commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Ticket

Changes

Context for reviewers

Setup

Code Review Verification Steps

As the original developer, I have

Satisfied acceptance criteria and met development standards

Ensured code standards are met (Original Developer)

Validated user-facing changes (if applicable)

As a code reviewer, I have

Reviewed, tested, and left feedback about the changes

Validated user-facing changes as a developer

As a designer reviewer, I have

Verified that the changes match the design intention

Validated user-facing changes as a designer

References

Screenshots

Uh oh!

github-actions bot commented Jan 7, 2026

Uh oh!

github-actions bot commented Jan 7, 2026

Uh oh!

github-actions bot commented Jan 7, 2026

Uh oh!

kimallen left a comment

Choose a reason for hiding this comment

Uh oh!

kimallen Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

kimallen Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

kimallen Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

kimallen Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

kimallen Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

kimallen Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

kimallen Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

abe-alam-ecs Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jan 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

abe-alam-ecs commented Jan 7, 2026 •

edited

Loading

abe-alam-ecs Jan 9, 2026 •

edited

Loading