New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API: Allow disabling DNSSEC #5936

Merged
merged 1 commit into from Nov 16, 2017

Conversation

Projects
None yet
3 participants
@pieterlexis
Member

pieterlexis commented Nov 9, 2017

Short description

This PR makes sure we can disable DNSSEC via the API, this is equivalent to doing pdnsutil disable-dnssec.

Closes #5909
Closes #5910

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled and tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)

@pieterlexis pieterlexis added the auth label Nov 9, 2017

@pieterlexis pieterlexis added this to the auth-4.1.x milestone Nov 9, 2017

@rgacogne

A couple nits, nothing blocking this PR from being merged.

bool DNSSECKeeper::unSecureZone(const DNSName& zone, string& error, string& info) {
// Not calling isSecuredZone(), as it will return false for zones with zero
// active keys.
DNSSECKeeper::keyset_t keyset=getKeys(zone);

This comment has been minimized.

@rgacogne

rgacogne Nov 9, 2017

Member

Just a nit, but perhaps isSecureZone() could get an optional boolean to return true as long as there is one key, be it inactive?

@rgacogne

rgacogne Nov 9, 2017

Member

Just a nit, but perhaps isSecureZone() could get an optional boolean to return true as long as there is one key, be it inactive?

This comment has been minimized.

@pieterlexis

pieterlexis Nov 10, 2017

Member

That might make sense, perhaps turn that into a ticket?

@pieterlexis

pieterlexis Nov 10, 2017

Member

That might make sense, perhaps turn that into a ticket?

@@ -589,6 +589,26 @@ bool DNSSECKeeper::getTSIGForAccess(const DNSName& zone, const string& master, D
return false;
}
bool DNSSECKeeper::unSecureZone(const DNSName& zone, string& error, string& info) {

This comment has been minimized.

@rgacogne

rgacogne Nov 9, 2017

Member

info doesn't seem to be used?

@rgacogne

rgacogne Nov 9, 2017

Member

info doesn't seem to be used?

This comment has been minimized.

@pieterlexis

pieterlexis Nov 10, 2017

Member

Correct, should we ever add more functionality to this function, then it might be. I kept is similar to rectifyZone().

@pieterlexis

pieterlexis Nov 10, 2017

Member

Correct, should we ever add more functionality to this function, then it might be. I kept is similar to rectifyZone().

@aerique aerique modified the milestones: auth-4.1.x, auth-4.1.0 Nov 16, 2017

@aerique aerique merged commit 28be3c9 into PowerDNS:master Nov 16, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@pieterlexis pieterlexis deleted the pieterlexis:api-allow-deactivate-dnssec branch Dec 5, 2017

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