puppetlabs / puppetlabs-stdlib

Puppet Labs Standard Library module
http://forge.puppetlabs.com/puppetlabs/stdlib
Apache License 2.0
348 stars 580 forks source link

types - dns: add a new dns type with more specific fqdn definitions #1347

Open b4ldr opened 1 year ago

b4ldr commented 1 year ago

This PR introduces a new set of types for validating FQDN's. it creates a type for the more loose rfc definitions of domain names and a stricter and likely more useful iana type.

This allows us to create a new type that more does to what most users expect i.e. that a dns name is one that works on the internt, without breaking current uses for users that may be using the Stdlib::Fqdn to validate validate DNS names that don't work with the IANA roots.

The intention of this patch would be to deprecate the currnet Stdlib::Fqdn type and encourage users to move to the appropriate Stdlib::DNS::* type which for most users will likely be the stricter Stdlib::DNS::Fqdn type

Note: this PR is intentionally a bit rough to first garner thoughts as to if this is the correct direction

Fixes #1282 (not sure it fixes but want it tagged)

b4ldr commented 1 year ago

This look very complex, and makes me think that choosing the correct type will be tricky.

The intention is that Stdlib::DNS::Fqdn will point to the "correct" or at least most useful type and picky users can pick the more specific type if needed, this is sort of similar to the Stdlib::IP namespace

They would need an extended documentation to help choose the right one.

If people think this is the right direction happy to clean up the pr and add some better docs

I am not sure that such a level of detail is important at the stdlib level, but understand it is required in some advanced cases.

My intention here is to provide a migration path away from Stdlib::Fqdn which i suspect is not what most people want with something more strict which likely matches what they do want, whilst at the same time provide types for the edge case that may break if we make Stdlib::Fqdn stricter.

smortex commented 1 year ago

Fine! This looks reasonable to me. Let's collect more people feedback!

JonasVerhofste commented 1 year ago

I just took 15 min trying to figure out why my logic was taking a certain codepath, until I figured out that $foo =~ Stdlib::Fqdn also matches on IPv4 addresses. So +1 from me for making it a bit more specific so it doesn't, or at least provide an alternative that doesn't.