Skip to content

OVH DNS Provider (#143)#175

Merged
tlimoncelli merged 3 commits intoStackExchange:masterfrom
masterzen:feature/ovh-provider
Nov 10, 2017
Merged

OVH DNS Provider (#143)#175
tlimoncelli merged 3 commits intoStackExchange:masterfrom
masterzen:feature/ovh-provider

Conversation

@masterzen
Copy link
Copy Markdown
Collaborator

@masterzen masterzen commented Aug 7, 2017

This adds the OVH provider along with its documentation:

  • DNS Provider
  • Registrar Provider
  • documentation

A few comments:

  • the provider unfortunately can't use CanUsePTR because OVH supports only setting PTR with a target in the arpa zone. This makes the integration tests 31 and 32 fail horribly (can we make those use the arpa zone?)
  • OVH now supports setting apex NS records, so dual providers scenario can be accomplished
  • OVH doesn't support CAA (I'm going to ask them when they plan to support those).

@masterzen masterzen force-pushed the feature/ovh-provider branch from 337c2ef to 301e0b1 Compare August 9, 2017 14:40
@masterzen masterzen mentioned this pull request Aug 9, 2017
@captncraig captncraig self-assigned this Aug 9, 2017
@masterzen masterzen force-pushed the feature/ovh-provider branch from 5b17774 to 891455e Compare September 4, 2017 13:33
@masterzen
Copy link
Copy Markdown
Collaborator Author

I've rebased to the latest master.

@tlimoncelli
Copy link
Copy Markdown
Contributor

Sorry for the delay.

Could you rebase and re-run the integration tests? I'd be glad to merge this.

@masterzen
Copy link
Copy Markdown
Collaborator Author

@tlimoncelli I'll do that this week-end. Thanks!

@masterzen masterzen force-pushed the feature/ovh-provider branch 2 times, most recently from d99752c to 4d26587 Compare November 10, 2017 09:00
@masterzen
Copy link
Copy Markdown
Collaborator Author

@tlimoncelli,

I've rebased and made sure documentation is generated (I also implemented the docNotes).

I've also enabled support for TLSA, but that required a modification of the integration test because OVH doesn't allow TLSA records for a sha256 matching type with a checksum that doesn't have exactly 64 characters (this makes sense). I couldn't test the other providers with this integration test change, so I hope I haven't introduced any test regressions.
If that proves that it is an issue, let me know and I'll revert that part of the change and exclude those tests for the OVH provider.

Thanks!

}
actual = append(actual, rec)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Insert this (all providers now must do this; though if you were passing the integration tests it might be a no-op):

  // Normalize
  models.Downcase(actual)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I missed this change, I'll do that.

"domain": "$VULTR_DOMAIN"
},
"OVH": {
"COMMENT": "47-48 OVH doesn't support TLSA records with matching type 0 (full certificate)",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a matchingtype other than 0 that the tests could use so that the tests can run? (I don't know much about TLSA)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, matching type 2 is for sha512, but since they check the length of the checksum we'd have to have a string of 128 hex characters.
Do you want me to modify the integration test to check changing matching type to 2 instead of 0 ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! The extra 128 bytes won't fill my hard disk :-) . (but seriously... I'd rather have working tests than not)

Brice Figureau added 3 commits November 10, 2017 19:36
This adds the OVH Provider along with its documentation.

Unfortunately we can't set this DNS provider to support `CanUsePTR`,
because OVH only supports setting PTR target on the Arpa zone.
This implements OVH as a registrar provider.
Note that NS modifications are done in a "best effort" mode, as the
provider doesn't wait for the modifications to be fully applied
(the operation that can take a long time).
Since OVH released their APIv6, it is now possible to update
zone apex NS records, opening the door to complete dual providers
scenarii.

This change implements apex NS management in an OVH zone.
@masterzen masterzen force-pushed the feature/ovh-provider branch from 4d26587 to 9addcb0 Compare November 10, 2017 18:42
@masterzen
Copy link
Copy Markdown
Collaborator Author

@tlimoncelli I've pushed the fixes you asked for. Let me know if there's anything I should do.

@tlimoncelli tlimoncelli merged commit e44dde5 into StackExchange:master Nov 10, 2017
@tlimoncelli
Copy link
Copy Markdown
Contributor

Thank you for your contribution!

koesie10 pushed a commit to koesie10/dnscontrol that referenced this pull request Nov 14, 2017
* OVH DNS Provider (StackExchange#143)

This adds the OVH Provider along with its documentation.

Unfortunately we can't set this DNS provider to support `CanUsePTR`,
because OVH only supports setting PTR target on the Arpa zone.

* OVH Registrar provider (StackExchange#143)

This implements OVH as a registrar provider.
Note that NS modifications are done in a "best effort" mode, as the
provider doesn't wait for the modifications to be fully applied
(the operation that can take a long time).

* Allow support for dual providers scenarios

Since OVH released their APIv6, it is now possible to update
zone apex NS records, opening the door to complete dual providers
scenarii.

This change implements apex NS management in an OVH zone.
pmoroney pushed a commit to pmoroney/dnscontrol that referenced this pull request Jan 11, 2018
* OVH DNS Provider (StackExchange#143)

This adds the OVH Provider along with its documentation.

Unfortunately we can't set this DNS provider to support `CanUsePTR`,
because OVH only supports setting PTR target on the Arpa zone.

* OVH Registrar provider (StackExchange#143)

This implements OVH as a registrar provider.
Note that NS modifications are done in a "best effort" mode, as the
provider doesn't wait for the modifications to be fully applied
(the operation that can take a long time).

* Allow support for dual providers scenarios

Since OVH released their APIv6, it is now possible to update
zone apex NS records, opening the door to complete dual providers
scenarii.

This change implements apex NS management in an OVH zone.
rblenkinsopp pushed a commit to rblenkinsopp/dnscontrol that referenced this pull request Aug 21, 2020
* OVH DNS Provider (StackExchange#143)

This adds the OVH Provider along with its documentation.

Unfortunately we can't set this DNS provider to support `CanUsePTR`,
because OVH only supports setting PTR target on the Arpa zone.

* OVH Registrar provider (StackExchange#143)

This implements OVH as a registrar provider.
Note that NS modifications are done in a "best effort" mode, as the
provider doesn't wait for the modifications to be fully applied
(the operation that can take a long time).

* Allow support for dual providers scenarios

Since OVH released their APIv6, it is now possible to update
zone apex NS records, opening the door to complete dual providers
scenarii.

This change implements apex NS management in an OVH zone.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants