Page MenuHomekolab.org

Basic PowerDNS functionality
ClosedPublic

Authored by vanmeeuwen on Aug 16 2021, 3:00 PM.

Details

Reviewers
machniak
Group Reviewers
Restricted Project
Commits
rK6cf47624770c: Basic PowerDNS functionality
Summary

This change adds basic powerdns functionality (for, among others, woat)

Test Plan

./phpunit

Diff Detail

Repository
rK kolab
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

vanmeeuwen requested review of this revision.Aug 16 2021, 3:00 PM
vanmeeuwen created this revision.
vanmeeuwen updated this revision to Diff 7750.Aug 16 2021, 3:04 PM
  • Remove useless change
vanmeeuwen updated this revision to Diff 7753.Aug 16 2021, 3:05 PM
  • Remove another irrelevant change
machniak requested changes to this revision.Aug 16 2021, 4:08 PM
machniak added a subscriber: machniak.

Minor comments just after the code review, no testing and closer look on it yet.

src/app/Console/Commands/PowerDNS/Domain/CreateCommand.php
41

Theinput should be validated.

src/app/Observers/PowerDNS/DomainObserver.php
15

Just $domain->name everywhere.

src/app/PowerDNS/Domain.php
18

You could use $this->records()->where('type', 'SOA')->first() here.

src/tests/Feature/PowerDNS/DomainTest.php
60

It should have a test for the domain observer - domain creation.

This revision now requires changes to proceed.Aug 16 2021, 4:08 PM
machniak added inline comments.Aug 16 2021, 4:31 PM
src/app/Observers/PowerDNS/DomainObserver.php
26

Four of these Record::create() calls will invoke bumpSerial() on the domain. We should disable the Record 'created' event handler temporarily. Look for withoutEvents on https://stackoverflow.com/questions/29407818/is-it-possible-to-temporarily-disable-event-in-laravel/51301753 or do manual inserts.

vanmeeuwen planned changes to this revision.Aug 17 2021, 10:16 AM
vanmeeuwen marked 5 inline comments as done.

Incoming! ;-)

vanmeeuwen updated this revision to Diff 7777.Aug 17 2021, 10:16 AM
  • Minimal validation.
  • Just use $domain->name everywhere
  • Disable domain observer events in turn bumping the serial
  • Make use of the records relation
  • Fix typo, test domain creation/observer

One last issue. Other than that it looks good. I didn't test the docker part, I guess we'll need some intergration tests later.

src/tests/Feature/PowerDNS/DomainTest.php
19

This 'domain' property needs to be declared, otherwise there's a phpstan error.

vanmeeuwen updated this revision to Diff 7819.Aug 17 2021, 11:58 AM
  • Describe $domain
  • Rebase on master
vanmeeuwen marked an inline comment as done.Aug 17 2021, 11:59 AM
machniak accepted this revision.Aug 17 2021, 11:59 AM
This revision is now accepted and ready to land.Aug 17 2021, 11:59 AM
This revision was automatically updated to reflect the committed changes.